[RFC,0/4] regulator: lp87565: ignore ENx pins and add LP87524-Q1

Hi,

the first patch in this series is a small but significant variation in how
the lp87565 driver enables the output rails, allow the kernel to always
know when it is enabling an output. However it can change existing
behaviour (depending on the hardware setup) and thus it should be carefully
evaluated.

The following patches are a fairly straightforward addition of a new chip
variant along with some DT bindings cleanup.

Luca

Luca Ceresoli (4):
  regulator: lp87565: enable voltage regardless of ENx pin
  regulator: lp87565: dt: remove duplicated section
  regulator: lp87565: dt: add LP87524-Q1 variant
  regulator: lp87565: add LP87524-Q1 variant

 .../devicetree/bindings/mfd/lp87565.txt       | 66 ++++++++++++++-----
 drivers/mfd/lp87565.c                         |  4 ++
 drivers/regulator/lp87565-regulator.c         | 21 +++++-
 include/linux/mfd/lp87565.h                   |  1 +
 4 files changed, 73 insertions(+), 19 deletions(-)

On Wed, 03 Jun 2020, Luca Ceresoli wrote:

> Add support for the LP87524B/J/P-Q1 Four 4-MHz Buck Converter. This is a
> variant of the LP87565 having 4 single-phase outputs and up to 10 A of
> total output current.
> 
> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
> ---
>  drivers/mfd/lp87565.c       | 4 ++++
>  include/linux/mfd/lp87565.h | 1 +

Again, this is an MFD patch.  Please change the subject line.

>  2 files changed, 5 insertions(+)

Once changed, please re-submit with my:

For my own reference (apply this as-is to your sign-off block):

  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>

Hi Lee,

On 04/06/20 08:44, Lee Jones wrote:
> On Wed, 03 Jun 2020, Luca Ceresoli wrote:
> 
>> Add support for the LP87524B/J/P-Q1 Four 4-MHz Buck Converter. This is a
>> variant of the LP87565 having 4 single-phase outputs and up to 10 A of
>> total output current.
>>
>> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
>> ---
>>  drivers/mfd/lp87565.c       | 4 ++++
>>  include/linux/mfd/lp87565.h | 1 +
> 
> Again, this is an MFD patch.  Please change the subject line.

Right.

>>  2 files changed, 5 insertions(+)
> 
> Once changed, please re-submit with my:
> 
> For my own reference (apply this as-is to your sign-off block):
> 
>   Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>

Good, thank you.

On Wed, Jun 03, 2020 at 10:03:16PM +0200, Luca Ceresoli wrote:

> I suspect the only solution that allows to configure the EN_PIN_CTRLn bits
> correctly in all the possible hardware setups would be to tell in device
> tree / board info whether each enable pin is connected or not (which is a
> hardware _fact_) and which ENx pin should control which regulator output
> (which is a policy). But it would make this simple driver considerably more
> complex.

> Any suggestion about the correct way to handle this situation would be
> greatly appreciated.

We can tell if we've got a software controlled GPIO connected, if we
have then we should ensure that it continues to take effect.  That
should just be a single register write at startup from the sounds of it.
Otherwise yeah, just ignoring that there's a possibility of a GPIO we
don't know about seems sensible.

Hi Mark,

On 04/06/20 13:07, Mark Brown wrote:
> On Wed, Jun 03, 2020 at 10:03:16PM +0200, Luca Ceresoli wrote:
> 
>> I suspect the only solution that allows to configure the EN_PIN_CTRLn bits
>> correctly in all the possible hardware setups would be to tell in device
>> tree / board info whether each enable pin is connected or not (which is a
>> hardware _fact_) and which ENx pin should control which regulator output
>> (which is a policy). But it would make this simple driver considerably more
>> complex.
> 
>> Any suggestion about the correct way to handle this situation would be
>> greatly appreciated.
> 
> We can tell if we've got a software controlled GPIO connected, if we
> have then we should ensure that it continues to take effect.

Ideally yes, but it would be made more complex by the chip flexibility:
it's possible to choose which enable pin should drive each output.

For example this configuration is supported by the chip:

- BUCK0 is on if EN_BUCK0 high AND pin EN0 is active
- BUCK1 is on if EN_BUCK1 high AND pin EN0 is active
- BUCK2 is on if EN_BUCK2 high AND pin EN1 is active
- BUCK3 is on if EN_BUCK3 high (no pin used)
- pin EN2 is used as a GPIO (LP875xx acts as an I2C GPIO expander)

So it would be absolutely OK to describe in DT that EN0 and EN1 connect
the SoC to the LP875xx and that EN2 is connected to something else.

But describing in DT the association between enable pins and buck
outputs would be more a configuration than hardware description IMO.

And I'm not even considering the case where the enable pins are be
connected to something else, out of the SoC control, but still meant to
be used to control the buck output.

> That
> should just be a single register write at startup from the sounds of it.

Exactly, each buck output has a register containing the bits involved in
this discussion.

> Otherwise yeah, just ignoring that there's a possibility of a GPIO we
> don't know about seems sensible.

Thanks,