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

Add support for LT8722 #2637

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add support for LT8722 #2637

wants to merge 3 commits into from

Conversation

rccalam
Copy link

@rccalam rccalam commented Oct 29, 2024

PR Description

  • This adds driver support and device tree bindings for LT8722 full bridge DC/DC converter.
  • Datasheet: LT8722
  • Tested on RPI4 using the following demo board: DC3145A

PR Type

  • Bug fix (a change that fixes an issue)
  • New feature (a change that adds new functionality)
  • Breaking change (a change that affects other repos or cause CIs to fail)

PR Checklist

  • I have conducted a self-review of my own code changes
  • I have tested the changes on the relevant hardware
  • I have updated the documentation outside this repo accordingly (if there is the case)

@rccalam rccalam changed the title Dev/lt8722 regulator Add support for LT8722 Oct 29, 2024
@rccalam rccalam force-pushed the dev/lt8722-regulator branch 3 times, most recently from 792b158 to 7be3866 Compare October 29, 2024 04:49
Copy link
Collaborator

@nunojsa nunojsa left a comment

Choose a reason for hiding this comment

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

here it goes my first round

return ret;

reg_val &= ~mask;
reg_val |= (val << __ffs(mask)) & mask;
Copy link
Collaborator

Choose a reason for hiding this comment

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

instead of open coding FIELD_PREP(). Can we make sure val is already properly set (in the same way we would pass it to regmap_update_bits()) and then have a one liner:

reg_val = (reg_val & ~mask) | (reg_val & mask);

drivers/regulator/lt8722-regulator.c Show resolved Hide resolved
drivers/regulator/lt8722-regulator.c Show resolved Hide resolved
drivers/regulator/lt8722-regulator.c Outdated Show resolved Hide resolved
drivers/regulator/lt8722-regulator.c Outdated Show resolved Hide resolved
@rccalam rccalam force-pushed the dev/lt8722-regulator branch 14 times, most recently from c5bb8db to bd3502a Compare October 31, 2024 05:55
@rccalam
Copy link
Author

rccalam commented Oct 31, 2024

V2:

DT Bindings

  • Renamed en-gpios to enable-gpios and swen-gpios switch-enable-gpios for clarity
  • Improved descriptions for enable-gpios and switch-enable-gpios to reflect their specific roles
  • Ensured description is the first attribute in property definitions
  • Refactored description remove default value mentions, replacing them with actual default definitions in the schema
  • Updated adi,switch-frequency and adi,duty-cycle-range to use descriptive string values for clarity

Kconfig

  • Updated Kconfig to reflect that LT8722 depends on both SPI and OF

Driver

  • Sorted header files alphabetically
  • Adopted correct kernel style single line comment
  • Removed extra lines
  • Removed redundant GPIO direction setting
  • Replaced ret == 0 with !ret
  • Moved device tree properties retrieval into a separate helper function.
  • Used fsleep()
  • Used gpiod_set_value_cansleep()
  • Introduced LT8722_RAMP_STEP constant to replace hardcoded value
  • Removed unnecessary NULL check for SPI device
  • Ensured all declarations are positioned before any code
  • Deleted _lt8722_crc helper function
  • Prepared val before passing value to lt8722_reg_write_mask
  • Removed use of clamp()
  • Removed SPI setup

Copy link
Collaborator

@nunojsa nunojsa left a comment

Choose a reason for hiding this comment

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

Looks better. take my inputs and send it upstream... I think it's already good enough to be sent. Of course you'll get more feedback from the maintainers.

drivers/regulator/lt8722-regulator.c Outdated Show resolved Hide resolved
drivers/regulator/lt8722-regulator.c Outdated Show resolved Hide resolved
drivers/regulator/lt8722-regulator.c Outdated Show resolved Hide resolved
drivers/regulator/lt8722-regulator.c Outdated Show resolved Hide resolved
drivers/regulator/lt8722-regulator.c Outdated Show resolved Hide resolved
drivers/regulator/lt8722-regulator.c Outdated Show resolved Hide resolved
drivers/regulator/lt8722-regulator.c Outdated Show resolved Hide resolved
drivers/regulator/lt8722-regulator.c Outdated Show resolved Hide resolved
drivers/regulator/lt8722-regulator.c Show resolved Hide resolved
@rccalam rccalam force-pushed the dev/lt8722-regulator branch from bd3502a to ce61751 Compare November 5, 2024 05:22
imply REGULATOR_LT8722.

Signed-off-by: Ramon Cristopher M. Calam <[email protected]>
Add ADI LT8722 full bridge DC/DC converter driver support.

Signed-off-by: Ramon Cristopher M. Calam <[email protected]>
@rccalam rccalam force-pushed the dev/lt8722-regulator branch 2 times, most recently from c591733 to e09f688 Compare November 8, 2024 11:01
Add documentation for devicetree bindings for LT8722.

Signed-off-by: Ramon Cristopher M. Calam <[email protected]>
@rccalam rccalam force-pushed the dev/lt8722-regulator branch from e09f688 to e7e58cf Compare November 8, 2024 11:07
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