Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

drivers: regulator: max20335: add support for ship mode #67222

Conversation

bbilas
Copy link
Collaborator

@bbilas bbilas commented Jan 4, 2024

Allows the user to disable PMIC.

@bbilas bbilas force-pushed the drivers-max20335-add-enable-disable-functionallity branch 10 times, most recently from 5426b46 to 0a00b52 Compare January 9, 2024 06:55
@bbilas bbilas added the block: HW Test Testing on hardware required before merging label Jan 9, 2024
@bbilas bbilas marked this pull request as ready for review January 9, 2024 16:52
@bbilas bbilas requested a review from gmarull as a code owner January 9, 2024 16:52
@bbilas bbilas force-pushed the drivers-max20335-add-enable-disable-functionallity branch from 0a00b52 to cfd1ae2 Compare January 9, 2024 16:53
Comment on lines 54 to 63
struct i2c_dt_spec bus;
const struct regulator_max20335_desc *desc;
uint8_t source;
const struct device *parent;
Copy link
Member

Choose a reason for hiding this comment

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

note: not sure you save much by doing this (I removed it from some code a while ago), as it complicates things for just an address byte, depending on alignment you may actually save nothing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I followed the same pattern as in pca9420. Not sure what savings you have in mind?

Copy link
Contributor

Choose a reason for hiding this comment

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

Both the common and the individual regulator configs can have their own copy of struct i2c_dt_spec bus.
Then you can access ->bus directly instead of having to go via the parent.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As you can see, struct i2c_dt_spec has been removed from the individual regulator configs. Am I still wrong, am I still blind?

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe Gerard is suggesting that you keep struct i2c_dt_spec bus in both the common and individual configs.
Then you don't need to get the parent config pointer, you can keep the original code.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, I've fixed that.

Allows the user to disable PMIC.

Signed-off-by: Bartosz Bilas <b.bilas@grinn-global.com>
@bbilas bbilas force-pushed the drivers-max20335-add-enable-disable-functionallity branch from cfd1ae2 to 45b5199 Compare January 10, 2024 13:13
@bbilas bbilas requested a review from gmarull January 10, 2024 13:14
@bbilas bbilas removed the block: HW Test Testing on hardware required before merging label Jan 10, 2024
@nashif nashif merged commit c1a1e45 into zephyrproject-rtos:main Jan 10, 2024
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants