Re: [PATCH v2] dt-binding: eeprom: at24: add supply properties

From: Tomasz Figa <tfiga@chromium.org>
To: Bartosz Golaszewski <bgolaszewski@baylibre.com>,
	Wolfram Sang <wsa@the-dreams.de>
Cc: Bibby Hsieh <bibby.hsieh@mediatek.com>,
	linux-i2c <linux-i2c@vger.kernel.org>,
	Nicolas Boichat <drinkcat@chromium.org>,
	srv_heupstream <srv_heupstream@mediatek.com>,
	Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	linux-devicetree <devicetree@vger.kernel.org>
Subject: Re: [PATCH v2] dt-binding: eeprom: at24: add supply properties
Date: Thu, 24 Oct 2019 18:32:38 +0900	[thread overview]
Message-ID: <CAAFQd5DNdmm4sn1JNPhnuMor50ZP4EJmymtS4hB4WkNHmKOs6w@mail.gmail.com> (raw)
In-Reply-To: <CAMpxmJWzEER4iBo9-WhmumuH1nmWYvy=xud+=7wzp3op8-P7uw@mail.gmail.com>

On Thu, Oct 24, 2019 at 5:40 PM Bartosz Golaszewski
<bgolaszewski@baylibre.com> wrote:
>
> czw., 24 paź 2019 o 09:02 Tomasz Figa <tfiga@chromium.org> napisał(a):
> >
> > On Thu, Oct 24, 2019 at 3:22 PM Bartosz Golaszewski
> > <bgolaszewski@baylibre.com> wrote:
> > >
> > > pt., 18 paź 2019 o 10:26 Bibby Hsieh <bibby.hsieh@mediatek.com> napisał(a):
> > > >
> > > > In some platforms, they disable the power-supply of eeprom and i2c due
> > > > to power consumption reduction.
> > > >
> > > > This patch add two supply properties: vcc-supply, i2c-supply.
> > > >
> > > > Changes since v1:
> > > >  - change supply name
> > > >  - rebase to next
> > > >
> > > > Signed-off-by: Bibby Hsieh <bibby.hsieh@mediatek.com>
> > > > ---
> > > >  Documentation/devicetree/bindings/eeprom/at24.yaml | 8 ++++++++
> > > >  1 file changed, 8 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/eeprom/at24.yaml b/Documentation/devicetree/bindings/eeprom/at24.yaml
> > > > index e8778560d966..578487a5d9b7 100644
> > > > --- a/Documentation/devicetree/bindings/eeprom/at24.yaml
> > > > +++ b/Documentation/devicetree/bindings/eeprom/at24.yaml
> > > > @@ -167,6 +167,14 @@ properties:
> > > >      minimum: 1
> > > >      maximum: 8
> > > >
> > > > +  vcc-supply:
> > > > +    description:
> > > > +      phandle of the regulator that provides the supply voltage.
> > > > +
> > > > +  i2c-sypply:
> > > > +    description:
> > > > +      phandle to the regulator that provides power to i2c.
> > > > +
> > >
> > > Something was bothering me about this patch so I came back to take a
> > > look. Can you explain what i2c actually stands for in this doc? I hope
> > > I'm misinterpreting something and it isn't that the driver disables
> > > the regulator powering the i2c bus controller?
> >
> > In our case it's the regulator that the I2C bus is pulled up to.
> >
>
> Then it has nothing to do with a generic EEPROM driver IMO. I think
> you need to add the control for this regulator to your i2c controller
> driver and create a power domain where the EEPROM would be lower in
> hierarchy.

While I agree that the generic EEPROM driver may not be the best place
to do it, neither is a driver for a specific SoC i2c controller. The
hardware design is not specific to any particular i2c controller.

Perhaps we need the generic i2c core to take into account an
i2c-supply? Wolfram, any thoughts on this?

Best regards,
Tomasz