Re: [PATCH v2 1/2] dt-bindings: leds: aw2013: Document vddio-supply

[Stephan Gerhold <stephan@gerhold.net>]

On Mon, Mar 20, 2023 at 07:04:22PM +0100, Krzysztof Kozlowski wrote:
> On 20/03/2023 18:55, Lin, Meng-Bo wrote:
> > Some LEDs controllers are used with external pull-up for the interrupt
> > line and the I2C lines, so we might need to enable a regulator to bring
> > the lines into usable state.
> 
> Not a property of this device.
> 
> > Otherwise, this might cause spurious
> > interrupts and reading from I2C will fail.
> > 
> > Document support for "vddio-supply" that is enabled by the aw2013 driver
> > so that the regulator gets enabled when needed.
> > 
> > Signed-off-by: Lin, Meng-Bo <linmengbo0689@protonmail.com>
> > ---
> >  Documentation/devicetree/bindings/leds/leds-aw2013.yaml | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/leds/leds-aw2013.yaml b/Documentation/devicetree/bindings/leds/leds-aw2013.yaml
> > index 08f3e1cfc1b1..79b69cf1d1fe 100644
> > --- a/Documentation/devicetree/bindings/leds/leds-aw2013.yaml
> > +++ b/Documentation/devicetree/bindings/leds/leds-aw2013.yaml
> > @@ -23,6 +23,11 @@ properties:
> >    vcc-supply:
> >      description: Regulator providing power to the "VCC" pin.
> >  
> > +  vddio-supply:
> > +    description: |
> > +      Optional regulator that provides digital I/O voltage,
> 
> NAK. I responded to your patch and you just send a v2 without explanation.
> 
> The device does not have VDDIO pin, either.
> 

The power supply Lin is trying to add here is basically the "VIO1"
example in "Figure 1 AW2013 Typical Application Circuit" on page 1 of
the AW2013 datasheet [1]. The I2C pins and the interrupt output are both
open-drain and therefore require external pull-up resistors, connected
to a power supply that might not be always on.

Because of the open-drain pins AW2013 does indeed not have a dedicated
input pin for the I/O supply voltage. However, it is still necessary to
describe the power supply _somewhere_, to ensure that it is enabled when
needed.

It is hard to model this properly but it's generally easiest to handle
this inside the peripheral driver since it knows exactly when I2C and/or
interrupt lines are currently needed or not. This situation is fairly
common for I2C devices so there are several precedents, e.g.:

  1. cypress,tm2-touchkey.yaml: "vddio-supply"
     https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3e730ec11d51283ad62a98436967c01b718132ab
  2. goodix,gt7375p.yaml: "mainboard-vddio-supply"
     https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=1d18c1f3b7d938bdefc44289d137b4e6c7a3d502

Would "mainboard-vddio-supply" from the second example be more clear
than the simple "vddio-supply"? Naming things properly while staying
concise is hard. :)

And if you have any suggestions how to describe this in a better way
please let us know!

Thanks,
Stephan

[1]: https://datasheet.lcsc.com/lcsc/1808311530_AWINIC-Shanghai-Awinic-Tech-AW2013DNR_C252440.pdf