[PATCH v3 0/9] Use correct LDO5 control registers for PCA9450

[Frieder Schrempf]

Hi,

On 18.12.24 4:27 PM, Frieder Schrempf wrote:
> From: Frieder Schrempf <frieder.schrempf at kontron.de>
> 
> This is a follow-up of [1].
> 
> The main objective of this is to fix the PCA9450 driver to
> use the correct control register for the LDO5 regulator.
> 
> Currently the control register to use for LDO5 is hardcoded to
> LDO5CTRL_H. This is wrong for two reasons:
> 
> 1. LDO5CTRL_H doesn't contain the bits for enabling/disabling
>    the regulator. Only LDO5CTRL_L does.
> 
> 2. The actual output voltage of the regulator is determined by
>    the LDO5CTRL_H only if the SD_VSEL input is HIGH. If it is
>    low, then LDO5CTRL_L is used. The driver does not take this
>    into account.
> 
> This can cause several problems:
> 
> 1. LDO5 can not be turned on/off and we rely on the bootloader
>    to leave it turned on. On the other hand we can't save
>    power if LDO5 is unused.
> 
> 2. There is a potential for corner-cases where switching
>    SD_VSEL via USDHC_VSELECT and writing to the (wrong)
>    control register can cause wrong output voltage and therfore
>    SD card failures (not observed yet).
> 
> 3. Reading the current voltage of the LDO5 regulator (e. g. via
>    sysfs can yield the wrong value as the voltage is read from
>    the wrong control register.
> 
> At the same time there is now hardware that hardwires SD_VSEL
> to a fixed LOW level and therefore relies on switching the
> voltage only via a single control register. We add support for
> this through an additional property "nxp,sd-vsel-fixed-low" in
> the LDO5 node.
> 
> Summary of binding changes (patch 1):
> 
> 1. Adjust the bindings to remove the old and abandoned use of
>    sd-vsel-gpios property.
> 
> 2. Adjust the bindings to use sd-vsel-gpios in the LDO5 node to
>    retrieve an input that can be used to sample the SD_VSEL
>    status.
> 
> 3. Adjust bindings to allow "nxp,sd-vsel-fixed-low" to be used
>    for boards that have SD_VSEL hardwired to low level.
> 
> Summary of driver changes (patch 2-6):
> 
> 1. Remove the old sd-vsel-gpios handling.
> 
> 2. Use the new sd-vsel-gpios property to determine the correct
>    control register for LDO5.
> 
> 3. Fix the enable register for LDO5.
> 
> 4. Support hardware with fixed low level of SD_VSEL.
> 
> Summary of devicetree changes (patch 7-9):
> 
> Implement the changes in the devicetrees for Kontron hardware
> (i.MX8MM, i.MX8MP and i.MX93).
> 
> Changelog:
> 
> v2 -> v3:
> 
> * Merge binding patches into one and extend commit message
> * Rebase to next-20241218
> 
> v1 -> v2:
> 
> * Split binding patch
> * Add solution for hardwired SD_VSEL
> * Leave regulator core untouched as requested by Mark
> * Add devicetree changes for i.MX8MP and i.MX93
> 
> [1] https://lore.kernel.org/lkml/20230213155833.1644366-1-frieder@fris.de/
> 
> Frieder Schrempf (9):
>   dt-bindings: regulator: pca9450: Add properties for handling LDO5
>   arm64: dts: imx8mp-skov-reva: Use hardware signal for SD card VSELECT
>   Revert "regulator: pca9450: Add SD_VSEL GPIO for LDO5"
>   regulator: pca9450: Fix control register for LDO5
>   regulator: pca9450: Fix enable register for LDO5
>   regulator: pca9450: Handle hardware with fixed SD_VSEL for LDO5
>   arm64: dts: imx8mm-kontron: Add support for reading SD_VSEL signal
>   arm64: dts: imx93-kontron: Fix SD card IO voltage control
>   arm64: dts: imx8mp-kontron: Add support for reading SD_VSEL signal
> 
>  .../regulator/nxp,pca9450-regulator.yaml      |  29 ++++-
>  .../boot/dts/freescale/imx8mm-kontron-bl.dts  |  10 +-
>  .../dts/freescale/imx8mm-kontron-osm-s.dtsi   |   7 +-
>  .../dts/freescale/imx8mp-kontron-osm-s.dtsi   |   7 +-
>  .../boot/dts/freescale/imx8mp-skov-reva.dtsi  |   5 +-
>  .../dts/freescale/imx93-kontron-osm-s.dtsi    |   5 +-
>  drivers/regulator/pca9450-regulator.c         | 103 +++++++++++++++---
>  7 files changed, 133 insertions(+), 33 deletions(-)
> 

I just want to ask kindly if there is any feedback for the driver and
devicetree changes in this set?

Thanks
Frieder