Skip to content

Add TI DRV8846 support#51

Open
stsdc wants to merge 3 commits intosm7150-mainline:v7.0from
stsdc:stsdc/v7-add-drv8846-complimentary
Open

Add TI DRV8846 support#51
stsdc wants to merge 3 commits intosm7150-mainline:v7.0from
stsdc:stsdc/v7-add-drv8846-complimentary

Conversation

@stsdc
Copy link
Copy Markdown

@stsdc stsdc commented Mar 25, 2026

This PR introduces a DRV8846 motor driver (and DTS node) which is responsible for the pop-up camera mechanism.
It has been tested on Xiaomi 9T. Xiaomi 9T Pro (xiaomi-raphael) also uses this solution.
This will be useful for testing the selfie camera later.

Contains:

Can be tested with this GUI: https://gist.github.com/stsdc/20dcc6e83b5ae23f25523ac0c8567085
Proof of work: https://mas.to/@stsdc/116026121171513287

Copy link
Copy Markdown
Member

@Gelbpunkt Gelbpunkt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a very rough look at this. It's better than your last revision, but still smells like downstream. Please make sure to also run clang-format on the driver.

Comment thread arch/arm64/boot/dts/qcom/sm7150-xiaomi-davinci.dtsi Outdated
Comment thread drivers/misc/Kconfig Outdated
Comment on lines +650 to +652
Say 'y' here to include support for the H-Bridge Stepper
Motor Driver. The MISC peripheral holds the pwm and the
driver provides an API to drive stepper motor.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This description and the config option name are way too generic. Describe the actual hardware.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added motor controller manufacturer and model. And info about its purpose.

Comment thread drivers/misc/Makefile Outdated
Comment thread include/uapi/linux/drv8846.h Outdated
#define UP 1
#define DOWN 0

/* Move back add delta ms to guarantee go home success */
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What? This comment makes no sense

Copy link
Copy Markdown
Author

@stsdc stsdc Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't actually change this file. It is straight from downstream. I think what they meant is to add a few ms to the motor work time to make sure that camera hid completely.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment updated.

Comment thread drivers/misc/drv8846.c Outdated
pstate.period = pwm->period_ns;
pstate.duty_cycle = pwm->duty_ns;

pr_debug("enable %d, period %llu, duty %llu\n", pstate.enabled, pstate.period, pstate.duty_cycle);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would recommend using dev_dbg instead of pr_debug

Copy link
Copy Markdown
Author

@stsdc stsdc Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have wrapped dev_dbg in a macro to use function:line prefix formatting. Replaced all pr_debug with it.

Comment thread drivers/misc/drv8846.c Outdated
Comment thread drivers/misc/drv8846.c Outdated
Comment thread drivers/misc/drv8846.c Outdated
{
struct drv8846_soc_ctrl *mctrl = filp->private_data;

pr_debug("mctrl %p, application fasync!", mctrl);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These log messages do not add anything of value. Logs should be informative.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some debug messages have been removed and for some just added more context.

Stanislaw Dac added 3 commits April 5, 2026 19:59
other necessary gpio nodes

Signed-off-by: Stanislaw Dac <stadac@disroot.org>

arm64: dts: qcom: sm7150-xiaomi-davinci: fix dts code style

Signed-off-by: Stanislaw Dac <stadac@disroot.org>
…Motor Controller

Signed-off-by: Stanislaw Dac <stadac@disroot.org>

dt-bindings: pwm: ti,drv8846: remove the "ti" prefix from the properties

Signed-off-by: Stanislaw Dac <stadac@disroot.org>
Signed-off-by: Stanislaw Dac <stadac@disroot.org>

misc: drv8846: TI DRV8846 driver cleanup

Signed-off-by: Stanislaw Dac <stadac@disroot.org>
@stsdc stsdc force-pushed the stsdc/v7-add-drv8846-complimentary branch from 237f378 to a74334d Compare April 5, 2026 18:03
@stsdc stsdc requested a review from Gelbpunkt April 5, 2026 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants