All of lore.kernel.org
 help / color / mirror / Atom feed
From: heiko@sntech.de (Heiko Stübner)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 0/2] IO voltage domain support for rk3188 and rk3288
Date: Sat, 30 Aug 2014 13:27:43 +0200	[thread overview]
Message-ID: <1450179.xJuMFg9Vvn@diego> (raw)
In-Reply-To: <CAD=FV=Vcj+tAno92c38Prviyi-v2CmuGSzak1ppETL033KAF4w@mail.gmail.com>

Am Freitag, 29. August 2014, 21:51:46 schrieb Doug Anderson:
> Santosh,
> 
> On Thu, Aug 28, 2014 at 2:26 PM, Santosh Shilimkar
> 
> <santosh.shilimkar@ti.com> wrote:
> > On Thursday 28 August 2014 03:36 PM, Doug Anderson wrote:
> >> These two patches add support for automatically configuring the IO
> >> voltage domains on rk3188 and rk3288 SoCs.  The first patch adds some
> >> new notification types to the regulator code.  It's used by the second
> >> patch which actually implements the IO voltage domain driver.
> >> 
> >> These two patches were co-developed by Heiko St?bner and Doug Anderson
> >> (proof of concept patches were written by Heiko).  They were tested in
> >> a private branch on an rk3288 board using rk808 instead of mainline
> >> since rk808 support isn't finalized in mainline yet.
> >> 
> >> (sorry if you got this series twice; my mailer seems unhappy with me)
> >> 
> >> Heiko St?bner (2):
> >>   regulator: core: Add REGULATOR_EVENT_PRE_VOLTAGE_CHANGE (and ABORT)
> >>   soc/rockchip: io-domain: add driver handling io domains
> > 
> > Sorry to shot down but your IO domains are nothing but voltage domains
> > and you should really build something in the drivers/power/*
> 
> If everyone agrees that this belongs in drivers/power that's totally
> OK.  Neither Heiko nor I was confident that it should be in
> drivers/soc.  I had even though that the code wouldn't be totally out
> of place in the Rockchip pinctrl driver (adding Linus W since I think
> some SoCs did add code to handle 3.3V vs. 1.8V in pinctrl).

a bit of context for Linus ...

This is essentially the continuation of  the thread "io-domain voltages as 
regulators?" from the beginning of august. After more discussions we found out 
that the io voltage selection I asked about is not an independent supply, but 
instead has to reflect the voltage of the real supplying regulator.

And setting the io-voltage setting to 1.8V while the regulator is supplying 
3.3V for example may actually damage the chip.


So in our current approach here, we added a driver that tracks voltage changes  
of the supplying regulator via a notifier and sets the register bits 
accordingly.


> 
> > Please have a look at the RFC [1]. You should really go on those
> > lines and collaborate to make a generic voltage domain layer instead of
> > throwing the driver under drivers/soc.
> 
> Trying to base things on a 7-month old RFC that hasn't been touched is
> not something I'm going to do.  Maybe that makes me a bad person...
> 
> I would also say that I'm not convinced that we really need a
> complicated framework here.  Maybe when we're talking about things
> like ABB and DevFreq and the like then having a nice framework is a
> good idea.  Really here we're just setting properties associated with
> IO pins.  There's no decisions about latency, power tradeoffs, etc.
> If the pin is connected to 1.8V we need to set the 1.8V bit.  If it's
> connected to 3.3V we need to set the 3.3V bit.  The end.
> 
> The only remotely complicated thing (and why this isn't just a pinctrl
> property) is what happens with dynamic voltages.  SD Card IO lines can
> change voltage depending on UHS negotiation.  In that case the SD Card
> Driver will request that its regulator change from 3.3V to 1.8V.  The
> bit in the IO domain register needs to update in tandem.
> 
> 
> The driver is really quite tiny (333 lines).  If we find that lots of
> people copy it and they have code that's nearly the same then we
> should try to abstract things out then.
> 
> 
> I'd be interested in hearing other opinions, though.
> 
> -Doug

WARNING: multiple messages have this Message-ID (diff)
From: "Heiko Stübner" <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
To: Doug Anderson <dianders-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Cc: Santosh Shilimkar
	<santosh.shilimkar-l0cyMroinI0@public.gmane.org>,
	Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	Olof Johansson <olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org>,
	Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>,
	Addy Ke <addy.ke-TNX95d0MmH7DzftRWevZcw@public.gmane.org>,
	Sonny Rao <sonnyrao-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>,
	"linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org"
	<linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>,
	Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>,
	"devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>,
	Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>,
	Ian Campbell
	<ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>,
	Peter De Schrijver
	<pdeschrijver-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>,
	agross-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org,
	Liam Girdwood <lgirdwood-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	"linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	sandeep_n-l0cyMroinI0@public.gmane.org,
	Kumar Gala <galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>,
	Grant Likely
	<grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
	Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>,
	Nishanth Menon <nm-l0cyMroinI0@public.gmane.org>,
	Linus
Subject: Re: [PATCH 0/2] IO voltage domain support for rk3188 and rk3288
Date: Sat, 30 Aug 2014 13:27:43 +0200	[thread overview]
Message-ID: <1450179.xJuMFg9Vvn@diego> (raw)
In-Reply-To: <CAD=FV=Vcj+tAno92c38Prviyi-v2CmuGSzak1ppETL033KAF4w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Am Freitag, 29. August 2014, 21:51:46 schrieb Doug Anderson:
> Santosh,
> 
> On Thu, Aug 28, 2014 at 2:26 PM, Santosh Shilimkar
> 
> <santosh.shilimkar-l0cyMroinI0@public.gmane.org> wrote:
> > On Thursday 28 August 2014 03:36 PM, Doug Anderson wrote:
> >> These two patches add support for automatically configuring the IO
> >> voltage domains on rk3188 and rk3288 SoCs.  The first patch adds some
> >> new notification types to the regulator code.  It's used by the second
> >> patch which actually implements the IO voltage domain driver.
> >> 
> >> These two patches were co-developed by Heiko Stübner and Doug Anderson
> >> (proof of concept patches were written by Heiko).  They were tested in
> >> a private branch on an rk3288 board using rk808 instead of mainline
> >> since rk808 support isn't finalized in mainline yet.
> >> 
> >> (sorry if you got this series twice; my mailer seems unhappy with me)
> >> 
> >> Heiko Stübner (2):
> >>   regulator: core: Add REGULATOR_EVENT_PRE_VOLTAGE_CHANGE (and ABORT)
> >>   soc/rockchip: io-domain: add driver handling io domains
> > 
> > Sorry to shot down but your IO domains are nothing but voltage domains
> > and you should really build something in the drivers/power/*
> 
> If everyone agrees that this belongs in drivers/power that's totally
> OK.  Neither Heiko nor I was confident that it should be in
> drivers/soc.  I had even though that the code wouldn't be totally out
> of place in the Rockchip pinctrl driver (adding Linus W since I think
> some SoCs did add code to handle 3.3V vs. 1.8V in pinctrl).

a bit of context for Linus ...

This is essentially the continuation of  the thread "io-domain voltages as 
regulators?" from the beginning of august. After more discussions we found out 
that the io voltage selection I asked about is not an independent supply, but 
instead has to reflect the voltage of the real supplying regulator.

And setting the io-voltage setting to 1.8V while the regulator is supplying 
3.3V for example may actually damage the chip.


So in our current approach here, we added a driver that tracks voltage changes  
of the supplying regulator via a notifier and sets the register bits 
accordingly.


> 
> > Please have a look at the RFC [1]. You should really go on those
> > lines and collaborate to make a generic voltage domain layer instead of
> > throwing the driver under drivers/soc.
> 
> Trying to base things on a 7-month old RFC that hasn't been touched is
> not something I'm going to do.  Maybe that makes me a bad person...
> 
> I would also say that I'm not convinced that we really need a
> complicated framework here.  Maybe when we're talking about things
> like ABB and DevFreq and the like then having a nice framework is a
> good idea.  Really here we're just setting properties associated with
> IO pins.  There's no decisions about latency, power tradeoffs, etc.
> If the pin is connected to 1.8V we need to set the 1.8V bit.  If it's
> connected to 3.3V we need to set the 3.3V bit.  The end.
> 
> The only remotely complicated thing (and why this isn't just a pinctrl
> property) is what happens with dynamic voltages.  SD Card IO lines can
> change voltage depending on UHS negotiation.  In that case the SD Card
> Driver will request that its regulator change from 3.3V to 1.8V.  The
> bit in the IO domain register needs to update in tandem.
> 
> 
> The driver is really quite tiny (333 lines).  If we find that lots of
> people copy it and they have code that's nearly the same then we
> should try to abstract things out then.
> 
> 
> I'd be interested in hearing other opinions, though.
> 
> -Doug

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: "Heiko Stübner" <heiko@sntech.de>
To: Doug Anderson <dianders@chromium.org>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>,
	Mark Brown <broonie@kernel.org>, Olof Johansson <olof@lixom.net>,
	Arnd Bergmann <arnd@arndb.de>, Addy Ke <addy.ke@rock-chips.com>,
	Sonny Rao <sonnyrao@chromium.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	Mark Rutland <mark.rutland@arm.com>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	Stephen Warren <swarren@nvidia.com>,
	Pawel Moll <pawel.moll@arm.com>,
	Ian Campbell <ijc+devicetree@hellion.org.uk>,
	Peter De Schrijver <pdeschrijver@nvidia.com>,
	agross@codeaurora.org, Liam Girdwood <lgirdwood@gmail.com>,
	Rob Herring <robh+dt@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	sandeep_n@ti.com, Kumar Gala <galak@codeaurora.org>,
	Grant Likely <grant.likely@linaro.org>,
	Thierry Reding <treding@nvidia.com>, Nishanth Menon <nm@ti.com>,
	Linus Walleij <linus.walleij@linaro.org>
Subject: Re: [PATCH 0/2] IO voltage domain support for rk3188 and rk3288
Date: Sat, 30 Aug 2014 13:27:43 +0200	[thread overview]
Message-ID: <1450179.xJuMFg9Vvn@diego> (raw)
In-Reply-To: <CAD=FV=Vcj+tAno92c38Prviyi-v2CmuGSzak1ppETL033KAF4w@mail.gmail.com>

Am Freitag, 29. August 2014, 21:51:46 schrieb Doug Anderson:
> Santosh,
> 
> On Thu, Aug 28, 2014 at 2:26 PM, Santosh Shilimkar
> 
> <santosh.shilimkar@ti.com> wrote:
> > On Thursday 28 August 2014 03:36 PM, Doug Anderson wrote:
> >> These two patches add support for automatically configuring the IO
> >> voltage domains on rk3188 and rk3288 SoCs.  The first patch adds some
> >> new notification types to the regulator code.  It's used by the second
> >> patch which actually implements the IO voltage domain driver.
> >> 
> >> These two patches were co-developed by Heiko Stübner and Doug Anderson
> >> (proof of concept patches were written by Heiko).  They were tested in
> >> a private branch on an rk3288 board using rk808 instead of mainline
> >> since rk808 support isn't finalized in mainline yet.
> >> 
> >> (sorry if you got this series twice; my mailer seems unhappy with me)
> >> 
> >> Heiko Stübner (2):
> >>   regulator: core: Add REGULATOR_EVENT_PRE_VOLTAGE_CHANGE (and ABORT)
> >>   soc/rockchip: io-domain: add driver handling io domains
> > 
> > Sorry to shot down but your IO domains are nothing but voltage domains
> > and you should really build something in the drivers/power/*
> 
> If everyone agrees that this belongs in drivers/power that's totally
> OK.  Neither Heiko nor I was confident that it should be in
> drivers/soc.  I had even though that the code wouldn't be totally out
> of place in the Rockchip pinctrl driver (adding Linus W since I think
> some SoCs did add code to handle 3.3V vs. 1.8V in pinctrl).

a bit of context for Linus ...

This is essentially the continuation of  the thread "io-domain voltages as 
regulators?" from the beginning of august. After more discussions we found out 
that the io voltage selection I asked about is not an independent supply, but 
instead has to reflect the voltage of the real supplying regulator.

And setting the io-voltage setting to 1.8V while the regulator is supplying 
3.3V for example may actually damage the chip.


So in our current approach here, we added a driver that tracks voltage changes  
of the supplying regulator via a notifier and sets the register bits 
accordingly.


> 
> > Please have a look at the RFC [1]. You should really go on those
> > lines and collaborate to make a generic voltage domain layer instead of
> > throwing the driver under drivers/soc.
> 
> Trying to base things on a 7-month old RFC that hasn't been touched is
> not something I'm going to do.  Maybe that makes me a bad person...
> 
> I would also say that I'm not convinced that we really need a
> complicated framework here.  Maybe when we're talking about things
> like ABB and DevFreq and the like then having a nice framework is a
> good idea.  Really here we're just setting properties associated with
> IO pins.  There's no decisions about latency, power tradeoffs, etc.
> If the pin is connected to 1.8V we need to set the 1.8V bit.  If it's
> connected to 3.3V we need to set the 3.3V bit.  The end.
> 
> The only remotely complicated thing (and why this isn't just a pinctrl
> property) is what happens with dynamic voltages.  SD Card IO lines can
> change voltage depending on UHS negotiation.  In that case the SD Card
> Driver will request that its regulator change from 3.3V to 1.8V.  The
> bit in the IO domain register needs to update in tandem.
> 
> 
> The driver is really quite tiny (333 lines).  If we find that lots of
> people copy it and they have code that's nearly the same then we
> should try to abstract things out then.
> 
> 
> I'd be interested in hearing other opinions, though.
> 
> -Doug


  reply	other threads:[~2014-08-30 11:27 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-28 19:36 [PATCH 0/2] IO voltage domain support for rk3188 and rk3288 Doug Anderson
2014-08-28 19:36 ` Doug Anderson
2014-08-28 19:36 ` Doug Anderson
2014-08-28 19:36 ` [PATCH 1/2] regulator: core: Add REGULATOR_EVENT_PRE_VOLTAGE_CHANGE (and ABORT) Doug Anderson
2014-08-28 19:36   ` Doug Anderson
2014-08-29 11:05   ` Mark Brown
2014-08-29 11:05     ` Mark Brown
2014-08-28 19:36 ` [PATCH 2/2] soc/rockchip: io-domain: add driver handling io domains Doug Anderson
2014-08-28 19:36   ` Doug Anderson
2014-08-28 21:26 ` [PATCH 0/2] IO voltage domain support for rk3188 and rk3288 Santosh Shilimkar
2014-08-28 21:26   ` Santosh Shilimkar
2014-08-28 21:26   ` Santosh Shilimkar
2014-08-30  4:51   ` Doug Anderson
2014-08-30  4:51     ` Doug Anderson
2014-08-30  4:51     ` Doug Anderson
2014-08-30 11:27     ` Heiko Stübner [this message]
2014-08-30 11:27       ` Heiko Stübner
2014-08-30 11:27       ` Heiko Stübner
2014-09-04 16:51       ` Linus Walleij
2014-09-04 16:51         ` Linus Walleij
2014-09-04 16:51         ` Linus Walleij
2014-09-10 23:12         ` Kevin Hilman
2014-09-10 23:12           ` Kevin Hilman
2014-09-10 23:12           ` Kevin Hilman
  -- strict thread matches above, loose matches on Subject: below --
2014-08-28 19:19 Doug Anderson
2014-08-28 19:19 ` Doug Anderson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1450179.xJuMFg9Vvn@diego \
    --to=heiko@sntech.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.