* Re: [PATCH v6 1/4] Runtime Interpreted Power Sequences
From: Tomi Valkeinen @ 2012-09-13 7:03 UTC (permalink / raw)
To: Sascha Hauer
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Mark Brown,
Stephen Warren, linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Leela Krishna Amudala,
linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring,
Anton Vorontsov,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
David Woodhouse,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
In-Reply-To: <20120913070012.GC6180-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1605 bytes --]
On Thu, 2012-09-13 at 09:00 +0200, Sascha Hauer wrote:
> On Thu, Sep 13, 2012 at 09:54:09AM +0300, Tomi Valkeinen wrote:
> > On Thu, 2012-09-13 at 15:36 +0900, Alex Courbot wrote:
> > > On Thursday 13 September 2012 14:22:57 Tomi Valkeinen wrote:
> > >
> > > > However, I fear these board specific things may be quite a bit anything,
> > > > so it may well be pwm, gpios and regulators are not enough for them. For
> > > > example, there could be an FPGA on the board which requires some
> > > > configuration to accomplish the task at hand. It could be rather
> > > > difficult to handle it with a generic power sequence.
> > >
> > > Right. Note that this framework is supposed to be extended - I would like to
> > > at least add regulator voltage setting, and maybe even support for clocks and
> > > pinmux (but that might be out of place).
> >
> > Yes, that's one concern of mine... I already can imagine someone
> > suggesting adding conditionals to the power sequence data. Perhaps also
> > direct memory read/writes so you can twiddle registers directly. And so
> > on. Where's the limit what it should contain? Can we soon write full
> > drivers with the DT data? =)
>
> I have this concern aswell, that's why I'm sceptical about this patch
> set. But what are the alternatives? Adding power code to the drivers and
> thus adding board specific code to them is backwards.
As was pointed out in earlier posts in this thread, these are almost
always device specific, not board specific.
Do you have examples of board specific power sequences or such?
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v6 1/4] Runtime Interpreted Power Sequences
From: Sascha Hauer @ 2012-09-13 7:00 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Alex Courbot, Stephen Warren, Thierry Reding, Simon Glass,
Grant Likely, Rob Herring, Mark Brown, Anton Vorontsov,
David Woodhouse, Arnd Bergmann, Leela Krishna Amudala,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1347519249.7471.42.camel@lappyti>
On Thu, Sep 13, 2012 at 09:54:09AM +0300, Tomi Valkeinen wrote:
> On Thu, 2012-09-13 at 15:36 +0900, Alex Courbot wrote:
> > On Thursday 13 September 2012 14:22:57 Tomi Valkeinen wrote:
> >
> > > However, I fear these board specific things may be quite a bit anything,
> > > so it may well be pwm, gpios and regulators are not enough for them. For
> > > example, there could be an FPGA on the board which requires some
> > > configuration to accomplish the task at hand. It could be rather
> > > difficult to handle it with a generic power sequence.
> >
> > Right. Note that this framework is supposed to be extended - I would like to
> > at least add regulator voltage setting, and maybe even support for clocks and
> > pinmux (but that might be out of place).
>
> Yes, that's one concern of mine... I already can imagine someone
> suggesting adding conditionals to the power sequence data. Perhaps also
> direct memory read/writes so you can twiddle registers directly. And so
> on. Where's the limit what it should contain? Can we soon write full
> drivers with the DT data? =)
I have this concern aswell, that's why I'm sceptical about this patch
set. But what are the alternatives? Adding power code to the drivers and
thus adding board specific code to them is backwards.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [PATCH v6 1/4] Runtime Interpreted Power Sequences
From: Tomi Valkeinen @ 2012-09-13 6:54 UTC (permalink / raw)
To: Alex Courbot
Cc: Stephen Warren, Thierry Reding, Simon Glass, Grant Likely,
Rob Herring, Mark Brown, Anton Vorontsov, David Woodhouse,
Arnd Bergmann, Leela Krishna Amudala,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <2689722.93BQTh4lSC@percival>
[-- Attachment #1: Type: text/plain, Size: 1016 bytes --]
On Thu, 2012-09-13 at 15:36 +0900, Alex Courbot wrote:
> On Thursday 13 September 2012 14:22:57 Tomi Valkeinen wrote:
>
> > However, I fear these board specific things may be quite a bit anything,
> > so it may well be pwm, gpios and regulators are not enough for them. For
> > example, there could be an FPGA on the board which requires some
> > configuration to accomplish the task at hand. It could be rather
> > difficult to handle it with a generic power sequence.
>
> Right. Note that this framework is supposed to be extended - I would like to
> at least add regulator voltage setting, and maybe even support for clocks and
> pinmux (but that might be out of place).
Yes, that's one concern of mine... I already can imagine someone
suggesting adding conditionals to the power sequence data. Perhaps also
direct memory read/writes so you can twiddle registers directly. And so
on. Where's the limit what it should contain? Can we soon write full
drivers with the DT data? =)
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v6 0/4] Runtime Interpreted Power Sequences
From: Tomi Valkeinen @ 2012-09-13 6:48 UTC (permalink / raw)
To: Alex Courbot
Cc: Stephen Warren, Thierry Reding, Simon Glass, Grant Likely,
Rob Herring, Mark Brown, Anton Vorontsov, David Woodhouse,
Arnd Bergmann, Leela Krishna Amudala, linux-tegra@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-fbdev@vger.kernel.org,
devicetree-discuss@lists.ozlabs.org, linux-pm@vger.kernel.org,
linux-doc@vger.kernel.org
In-Reply-To: <1749811.4qrG1GZfBf@percival>
[-- Attachment #1: Type: text/plain, Size: 581 bytes --]
On Thu, 2012-09-13 at 15:23 +0900, Alex Courbot wrote:
> DT support is actually the main point of power sequences, as outside of the DT
> we can always work the old way and use callbacks. If we were to remove DT
> support, I am not sure this work would still be worth being merged.
Ah, I guess you meant hooks in the driver, not hooks to board files?
Yes, that would work, but if all the hooks do essentially the same
things with just minor modifications like sleep-time, it makes more
sense to have just one piece of code which gets the sequence as data.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v6 0/4] Runtime Interpreted Power Sequences
From: Tomi Valkeinen @ 2012-09-13 6:42 UTC (permalink / raw)
To: Alex Courbot
Cc: Stephen Warren, Thierry Reding, Simon Glass, Grant Likely,
Rob Herring, Mark Brown, Anton Vorontsov, David Woodhouse,
Arnd Bergmann, Leela Krishna Amudala, linux-tegra@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-fbdev@vger.kernel.org,
devicetree-discuss@lists.ozlabs.org, linux-pm@vger.kernel.org,
linux-doc@vger.kernel.org
In-Reply-To: <1749811.4qrG1GZfBf@percival>
[-- Attachment #1: Type: text/plain, Size: 2952 bytes --]
On Thu, 2012-09-13 at 15:23 +0900, Alex Courbot wrote:
> On Thursday 13 September 2012 13:50:47 Tomi Valkeinen wrote:
> > I want to reiterate my opinion that I think power sequences in DT data
> > is the wrong way to go. Powering sequences are device specific issues
> > and should be handled in the device driver. But I also think that power
> > sequences inside the drivers would probably be useful.
>
> I understand the logic behind handling powering sequences in the device
> driver, but as we discussed for some classes of devices this might just not
> scale. I don't know how many different panels (each with different powering
> sequences) are relying on pwm_backlight, but the alternative of embedding
> support for all of them into the kernel (and bloating the kernel image) or
> having a 3 kilometers list in the kernel configuration to individually chose
> which panel to support (which would be cumbersome and make the kernel less
> portable across boards) does not look much appealing to me. With power
> sequences encoded in the DT, we could have one .dtsi file per panel that would
> be included from the board's .dts file - no bloat, no drivers explosion,
> portability preserved.
Yes, I see that side of the argument also. And to be honest, I don't
know what kind of data is DT supposed to contain (or if there even is a
strict definition for that).
I have my opinion because I think that's how things should be: DT tells
us what devices there are and how they connect, and the driver handles
the rest. I may be a perfectionist, though, which is not good =).
As for the kernel bloat, it's a valid issue, but I wonder if it would be
an issue in practice. I don't know how many different supported devices
we'd have, and how many bytes the data for each device would consume.
I'm not even sure what amount of bytes would be acceptable.
But I'm guessing that we wouldn't have very many devices, and if the per
device data is made compact there wouldn't be that many bytes per
device. And with non-hotpluggable platform devices the unused device
data could be discarded after init.
Anyway, having the power sequences doesn't affect me if I don't use
them, so I have nothing against them =).
> DT support is actually the main point of power sequences, as outside of the DT
> we can always work the old way and use callbacks. If we were to remove DT
> support, I am not sure this work would still be worth being merged.
We can't use board callbacks when running with a DT enabled kernel. What
I meant is that the driver could contain a power sequence for the device
(or multiple supported devices). So it'd essentially be the same as
getting the power sequence from the DT data.
But I haven't looked at the power sequence data structures, so I'm not
sure if they are geared for DT use. If so, they would probably need
tuning to be good for in-kernel use.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v6 0/4] Runtime Interpreted Power Sequences
From: Alex Courbot @ 2012-09-13 6:42 UTC (permalink / raw)
To: Mark Brown
Cc: Tomi Valkeinen, Stephen Warren, Thierry Reding, Simon Glass,
Grant Likely, Rob Herring, Anton Vorontsov, David Woodhouse,
Arnd Bergmann, Leela Krishna Amudala, linux-tegra@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-fbdev@vger.kernel.org,
devicetree-discuss@lists.ozlabs.org, linux-pm@vger.kernel.org,
linux-doc@vger.kernel.org
In-Reply-To: <20120913062552.GB17869@opensource.wolfsonmicro.com>
On Thursday 13 September 2012 14:25:53 Mark Brown wrote:
> On Thu, Sep 13, 2012 at 03:23:06PM +0900, Alex Courbot wrote:
> > I understand the logic behind handling powering sequences in the device
> > driver, but as we discussed for some classes of devices this might just
> > not
> > scale. I don't know how many different panels (each with different
> > powering
>
> It would be sensible to make sure that the framework is done in such a
> way that drivers can use it - there will be drivers (perhaps not display
> ones) that have a known power sequence and which could benefit from the
> ability to use library code to implement it based on the user simply
> supplying named resources.
Not sure I understand what you mean, but things should be working this way
already - regulators and PWMs are acquired by name using the standard
regulator_get() and pwm_get() functions. GPIOs do not, AFAIK, have a way to be
referenced by name so their number is used instead.
Alex.
^ permalink raw reply
* Re: [PATCH v6 1/4] Runtime Interpreted Power Sequences
From: Alex Courbot @ 2012-09-13 6:36 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Stephen Warren, Thierry Reding, Simon Glass, Grant Likely,
Rob Herring, Mark Brown, Anton Vorontsov, David Woodhouse,
Arnd Bergmann, Leela Krishna Amudala, linux-tegra@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-fbdev@vger.kernel.org,
devicetree-discuss@lists.ozlabs.org, linux-pm@vger.kernel.org,
linux-doc@vger.kernel.org
In-Reply-To: <1347517377.7471.23.camel@lappyti>
On Thursday 13 September 2012 14:22:57 Tomi Valkeinen wrote:
> * PGP Signed by an unknown key
>
> On Thu, 2012-09-13 at 15:08 +0900, Alex Courbot wrote:
>
> > On Thursday 13 September 2012 13:45:39 Tomi Valkeinen wrote:
> >
> > > > Old Signed by an unknown key
> > >
> > >
> > > On Wed, 2012-09-12 at 18:57 +0900, Alexandre Courbot wrote:
> > >
> > >
> > > > Some device drivers (panel backlights especially) need to follow
> > > > precise
> > > > sequences for powering on and off, involving gpios, regulators, PWMs
> > > > with a precise powering order and delays to respect between each
> > > > steps.
> > > > These sequences are board-specific, and do not belong to a particular
> > > > driver - therefore they have been performed by board-specific hook
> > > > functions to far.
> > >
> > >
> > >
> > > The sequences are not board-specific, they are device (backlight, etc.)
> > > specific. The sequences have been handled in board-specific hook
> > > functions so far because there hasn't been proper drivers for the
> > > devices.
> > >
> > > If I were to take the same panel (and backlight) you have and install
> > > it
> > > on my board, I would need the same power sequence.
> >
> >
> > You could also have power sequences that control a set of GPIOs for an
> > external interface (and would then be more board-specific), but you are
> > right
>
> What do you mean with "external interface"?
Any crazy circuit design that would make the regular power sequence not usable
on a specific board. Sorry, I don't have any concrete example in mind, the
above is just speculation.
> But it's true that there can always be interesting board specific
> hardware designs, and they truly are board specific. In my experience
> these are quite rare, though, but perhaps not so rare that we wouldn't
> need to care about them.
>
> However, I fear these board specific things may be quite a bit anything,
> so it may well be pwm, gpios and regulators are not enough for them. For
> example, there could be an FPGA on the board which requires some
> configuration to accomplish the task at hand. It could be rather
> difficult to handle it with a generic power sequence.
Right. Note that this framework is supposed to be extended - I would like to
at least add regulator voltage setting, and maybe even support for clocks and
pinmux (but that might be out of place).
> So I guess what I'm saying is that mostly these issues are device
> specific, and when they are not, they may be rather complex/strange and
> require c code.
You're definitely right about the powering issue being a device issue 99% of
the time. For the rest I do not have enough insight to emit an opinion.
Alex.
^ permalink raw reply
* Re: [PATCH v6 0/4] Runtime Interpreted Power Sequences
From: Mark Brown @ 2012-09-13 6:25 UTC (permalink / raw)
To: Alex Courbot
Cc: Tomi Valkeinen, Stephen Warren, Thierry Reding, Simon Glass,
Grant Likely, Rob Herring, Anton Vorontsov, David Woodhouse,
Arnd Bergmann, Leela Krishna Amudala, linux-tegra@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-fbdev@vger.kernel.org,
devicetree-discuss@lists.ozlabs.org, linux-pm@vger.kernel.org,
linux-doc@vger.kernel.org
In-Reply-To: <1749811.4qrG1GZfBf@percival>
On Thu, Sep 13, 2012 at 03:23:06PM +0900, Alex Courbot wrote:
> I understand the logic behind handling powering sequences in the device
> driver, but as we discussed for some classes of devices this might just not
> scale. I don't know how many different panels (each with different powering
It would be sensible to make sure that the framework is done in such a
way that drivers can use it - there will be drivers (perhaps not display
ones) that have a known power sequence and which could benefit from the
ability to use library code to implement it based on the user simply
supplying named resources.
^ permalink raw reply
* Re: [PATCH v6 0/4] Runtime Interpreted Power Sequences
From: Alex Courbot @ 2012-09-13 6:23 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Stephen Warren, Thierry Reding, Simon Glass, Grant Likely,
Rob Herring, Mark Brown, Anton Vorontsov, David Woodhouse,
Arnd Bergmann, Leela Krishna Amudala, linux-tegra@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-fbdev@vger.kernel.org,
devicetree-discuss@lists.ozlabs.org, linux-pm@vger.kernel.org,
linux-doc@vger.kernel.org
In-Reply-To: <1347515447.7471.12.camel@lappyti>
On Thursday 13 September 2012 13:50:47 Tomi Valkeinen wrote:
> * PGP Signed by an unknown key
>
> On Wed, 2012-09-12 at 18:57 +0900, Alexandre Courbot wrote:
>
> > New revision of the power sequences, taking as usual the feedback that
> > was
> > kindly provided about the last version.
> >
> > I think now is a good time to discuss integrating this and to start
> > looking for a maintainer who would be willing to merge this into his/her
> > tree (I am especially thinking about the power framework maintainers,
> > since this is where the code is right now. The second patch in this
> > series enables the pwm_backlight driver to be used with the device tree,
> > without relying on board-dependent callbacks to support complex power
> > sequences. We also plan to use power sequences in other Tegra drivers,
> > and other people have expressed interest in this work during earlier
> > reviews. See for instance
> >
> > https://lists.ozlabs.org/pipermail/devicetree-discuss/2012-
August/018532.html
> > and
> >
> > https://lkml.org/lkml/2012/9/6/270
> >
> > There is probably some more details to fix and improve, but the current
> > shape should be enough to know if we want this and where - therefore any
> > sign from a maintainer would be greatly appreciated!
>
>
> I want to reiterate my opinion that I think power sequences in DT data
> is the wrong way to go. Powering sequences are device specific issues
> and should be handled in the device driver. But I also think that power
> sequences inside the drivers would probably be useful.
I understand the logic behind handling powering sequences in the device
driver, but as we discussed for some classes of devices this might just not
scale. I don't know how many different panels (each with different powering
sequences) are relying on pwm_backlight, but the alternative of embedding
support for all of them into the kernel (and bloating the kernel image) or
having a 3 kilometers list in the kernel configuration to individually chose
which panel to support (which would be cumbersome and make the kernel less
portable across boards) does not look much appealing to me. With power
sequences encoded in the DT, we could have one .dtsi file per panel that would
be included from the board's .dts file - no bloat, no drivers explosion,
portability preserved.
DT support is actually the main point of power sequences, as outside of the DT
we can always work the old way and use callbacks. If we were to remove DT
support, I am not sure this work would still be worth being merged.
Alex.
^ permalink raw reply
* Re: [PATCH v6 1/4] Runtime Interpreted Power Sequences
From: Tomi Valkeinen @ 2012-09-13 6:22 UTC (permalink / raw)
To: Alex Courbot
Cc: Stephen Warren, Thierry Reding, Simon Glass, Grant Likely,
Rob Herring, Mark Brown, Anton Vorontsov, David Woodhouse,
Arnd Bergmann, Leela Krishna Amudala, linux-tegra@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-fbdev@vger.kernel.org,
devicetree-discuss@lists.ozlabs.org, linux-pm@vger.kernel.org,
linux-doc@vger.kernel.org
In-Reply-To: <1464760.6eqxJ2IzZ2@percival>
[-- Attachment #1: Type: text/plain, Size: 1981 bytes --]
On Thu, 2012-09-13 at 15:08 +0900, Alex Courbot wrote:
> On Thursday 13 September 2012 13:45:39 Tomi Valkeinen wrote:
> > * PGP Signed by an unknown key
> >
> > On Wed, 2012-09-12 at 18:57 +0900, Alexandre Courbot wrote:
> >
> > > Some device drivers (panel backlights especially) need to follow precise
> > > sequences for powering on and off, involving gpios, regulators, PWMs
> > > with a precise powering order and delays to respect between each steps.
> > > These sequences are board-specific, and do not belong to a particular
> > > driver - therefore they have been performed by board-specific hook
> > > functions to far.
> >
> >
> > The sequences are not board-specific, they are device (backlight, etc.)
> > specific. The sequences have been handled in board-specific hook
> > functions so far because there hasn't been proper drivers for the
> > devices.
> >
> > If I were to take the same panel (and backlight) you have and install it
> > on my board, I would need the same power sequence.
>
> You could also have power sequences that control a set of GPIOs for an
> external interface (and would then be more board-specific), but you are right
What do you mean with "external interface"?
But it's true that there can always be interesting board specific
hardware designs, and they truly are board specific. In my experience
these are quite rare, though, but perhaps not so rare that we wouldn't
need to care about them.
However, I fear these board specific things may be quite a bit anything,
so it may well be pwm, gpios and regulators are not enough for them. For
example, there could be an FPGA on the board which requires some
configuration to accomplish the task at hand. It could be rather
difficult to handle it with a generic power sequence.
So I guess what I'm saying is that mostly these issues are device
specific, and when they are not, they may be rather complex/strange and
require c code.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v6 1/4] Runtime Interpreted Power Sequences
From: Alex Courbot @ 2012-09-13 6:08 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Stephen Warren, Thierry Reding, Simon Glass, Grant Likely,
Rob Herring, Mark Brown, Anton Vorontsov, David Woodhouse,
Arnd Bergmann, Leela Krishna Amudala, linux-tegra@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-fbdev@vger.kernel.org,
devicetree-discuss@lists.ozlabs.org, linux-pm@vger.kernel.org,
linux-doc@vger.kernel.org
In-Reply-To: <1347515139.7471.7.camel@lappyti>
On Thursday 13 September 2012 13:45:39 Tomi Valkeinen wrote:
> * PGP Signed by an unknown key
>
> On Wed, 2012-09-12 at 18:57 +0900, Alexandre Courbot wrote:
>
> > Some device drivers (panel backlights especially) need to follow precise
> > sequences for powering on and off, involving gpios, regulators, PWMs
> > with a precise powering order and delays to respect between each steps.
> > These sequences are board-specific, and do not belong to a particular
> > driver - therefore they have been performed by board-specific hook
> > functions to far.
>
>
> The sequences are not board-specific, they are device (backlight, etc.)
> specific. The sequences have been handled in board-specific hook
> functions so far because there hasn't been proper drivers for the
> devices.
>
> If I were to take the same panel (and backlight) you have and install it
> on my board, I would need the same power sequence.
You could also have power sequences that control a set of GPIOs for an
external interface (and would then be more board-specific), but you are right
that for most of the case power seqs apply to devices and this statement is
misleading. I will fix that in the commit message and wherever this might
appear.
Alex.
^ permalink raw reply
* Re: [PATCH v6 1/4] Runtime Interpreted Power Sequences
From: Alex Courbot @ 2012-09-13 6:02 UTC (permalink / raw)
To: Stephen Warren
Cc: Stephen Warren, Thierry Reding, Simon Glass, Grant Likely,
Rob Herring, Mark Brown, Anton Vorontsov, David Woodhouse,
Arnd Bergmann, Leela Krishna Amudala, linux-tegra@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-fbdev@vger.kernel.org,
devicetree-discuss@lists.ozlabs.org, linux-pm@vger.kernel.org,
linux-doc@vger.kernel.org
In-Reply-To: <50510791.8080302@wwwdotorg.org>
On Thursday 13 September 2012 06:07:13 Stephen Warren wrote:
> On 09/12/2012 03:57 AM, Alexandre Courbot wrote:
> > Some device drivers (panel backlights especially) need to follow precise
> > sequences for powering on and off, involving gpios, regulators, PWMs
> > with a precise powering order and delays to respect between each steps.
> > These sequences are board-specific, and do not belong to a particular
> > driver - therefore they have been performed by board-specific hook
> > functions to far.
> >
> > With the advent of the device tree and of ARM kernels that are not
> > board-tied, we cannot rely on these board-specific hooks anymore but
> > need a way to implement these sequences in a portable manner. This patch
> > introduces a simple interpreter that can execute such power sequences
> > encoded either as platform data or within the device tree.
> >
> > Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> >
> > diff --git a/Documentation/power/power_seq.txt
> > b/Documentation/power/power_seq.txt
> >
> > +Sometimes, you may want to browse the list of resources allocated by a
> > sequence, +for instance to ensure that a resource of a given type is
> > present. The +power_seq_set_resources() function returns a list head that
> > can be used with +the power_seq_for_each_resource() macro to browse all
> > the resources of a set: +
> > + struct list_head *power_seq_set_resources(struct power_seq_set *seqs);
>
> I don't think you need to include that prototype here?
Why not? I thought it was customary to include the prototypes in the
documentation, and this seems to be the right place for this function.
> > + power_seq_for_each_resource(pos, seqs)
> > +
> > +Here "pos" will be a pointer to a struct power_seq_resource. This
> > structure +contains the type of the resource, the information used for
> > identifying it, and +the resolved resource itself.
> >
> > diff --git a/drivers/power/power_seq/Makefile
> > b/drivers/power/power_seq/Makefile new file mode 100644
> > index 0000000..f77a359
> > --- /dev/null
> > +++ b/drivers/power/power_seq/Makefile
> > @@ -0,0 +1 @@
> > +obj-$(CONFIG_POWER_SEQ) += power_seq.o
>
> Don't you need to compile all the power_seq_*.c too?
>
> Oh, I see the following in power_seq.c:
> > +#include "power_seq_delay.c"
> > +#include "power_seq_regulator.c"
> > +#include "power_seq_pwm.c"
> > +#include "power_seq_gpio.c"
>
> It's probably better just to compile them separately and link them.
>
> > diff --git a/drivers/power/power_seq/power_seq.c
> > b/drivers/power/power_seq/power_seq.c
> >
> > +struct power_seq_step {
> > + /* Copy of the platform data */
> > + struct platform_power_seq_step pdata;
>
> I'd reword the comment to "Copy of the step", and name the field "step".
That would make a step within a step - doesn't pdata make it more explicit
what this member is for (containing the platform data for this step)?
> > +static const struct power_seq_res_ops
> > power_seq_types[POWER_SEQ_NUM_TYPES] = { + [POWER_SEQ_DELAY] > > POWER_SEQ_DELAY_TYPE,
> > + [POWER_SEQ_REGULATOR] = POWER_SEQ_REGULATOR_TYPE,
> > + [POWER_SEQ_PWM] = POWER_SEQ_PWM_TYPE,
> > + [POWER_SEQ_GPIO] = POWER_SEQ_GPIO_TYPE,
> > +};
>
> Ah, I see why you're using #include now.
We could also go with something more dynamic and compile these files
separately, but that would require some registration mechanism which I don't
think is needed for such a simple feature.
>
> > +MODULE_LICENSE("GPL");
>
> s/GPL/GPL v2/ given the license header.
>
> > diff --git a/drivers/power/power_seq/power_seq_gpio.c
> > b/drivers/power/power_seq/power_seq_gpio.c
> >
> > +static int power_seq_res_alloc_gpio(struct device *dev,
> > + struct platform_power_seq_step *pstep,
> > + struct power_seq_resource *res)
> > +{
> > + int err;
> > +
> > + err = devm_gpio_request_one(dev, pstep->gpio.gpio,
> > + GPIOF_OUT_INIT_LOW, dev_name(dev));
>
> Hmm. The INIT_LOW part of that might be somewhat presumptive. I would
> suggest simply requesting the GPIO here, and using
> gpio_direction_output() in power_seq_step_run_gpio(), thus deferring the
> decision of what value to set the GPIO to until a real sequence is
> actually run.
>
> > diff --git a/drivers/power/power_seq/power_seq_pwm.c
> > b/drivers/power/power_seq/power_seq_pwm.c
> >
> > diff --git a/drivers/power/power_seq/power_seq_regulator.c
> > b/drivers/power/power_seq/power_seq_regulator.c
> >
> > diff --git a/include/linux/power_seq.h b/include/linux/power_seq.h
> >
> > +#include <net/irda/parameters.h>
>
> That looks out of place.
Totally, thanks. I don't even understand how it landed there in the first
place.
>
> > +/**
> > + * struct power_seq_resource - resource used by a power sequence set
> > + * @pdata: Pointer to the platform data used to resolve this resource
> > + * @regulator: Resolved regulator if of type POWER_SEQ_REGULATOR
> > + * @pwm: Resolved PWM if of type POWER_SEQ_PWM
> > + * @list: Used to link resources together
> > + */
>
> I think that kerneldoc is stale.
>
> > +struct power_seq_resource {
> > + enum power_seq_res_type type;
> > + /* resolved resource and identifier */
> > + union {
> > + struct {
> > + struct regulator *regulator;
> > + const char *id;
> > + } regulator;
> > + struct {
> > + struct pwm_device *pwm;
> > + const char *id;
> > + } pwm;
> > + struct {
> > + int gpio;
> > + } gpio;
> > + };
> > + struct list_head list;
> > +};
>
> Aside from those minor issues, this all looks reasonable to me, so,
> Reviewed-by: Stephen Warren <swarren@wwwdotorg.org>
Thanks!
Alex.
^ permalink raw reply
* Re: [PATCH v6 0/4] Runtime Interpreted Power Sequences
From: Alex Courbot @ 2012-09-13 5:53 UTC (permalink / raw)
To: Anton Vorontsov
Cc: Stephen Warren, Stephen Warren, Thierry Reding, Simon Glass,
Grant Likely, Rob Herring, Mark Brown, David Woodhouse,
Arnd Bergmann, linux-fbdev@vger.kernel.org,
linux-pm@vger.kernel.org, Leela Krishna Amudala,
devicetree-discuss@lists.ozlabs.org, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-tegra@vger.kernel.org
In-Reply-To: <20120912213355.GA3342@lizard>
On Thursday 13 September 2012 05:33:56 Anton Vorontsov wrote:
> On Wed, Sep 12, 2012 at 03:27:04PM -0600, Stephen Warren wrote:
>
> > On 09/12/2012 03:57 AM, Alexandre Courbot wrote:
> >
> > > New revision of the power sequences, taking as usual the feedback that
> > > was
> > > kindly provided about the last version.
> > >
> > > I think now is a good time to discuss integrating this and to start
> > > looking for a maintainer who would be willing to merge this into
> > > his/her tree (I am especially thinking about the power framework
> > > maintainers, since this is where the code is right now.
> >
> >
> > The other alternative is for you to maintain this going forward; I
> > believe that would be as simple as:
> >
> > * Create a patch to add yourself to MAINTAINERS for the
> > drivers/power/power_seq/ directory.
> >
> > * Get a kernel.org account, push this patch to a branch there, and add
> > the branch into linux-next.
> >
> > * Send a pull request to Linus at the appropriate time.
> >
> > * Ongoing: Accept any patches, perform any maintenance required, etc.
> >
> > Does anyone see any issue with Alexandre doing this? Nobody else has
> > volunteered yet:-)
>
>
> Yup, looks like the best way.
I am fine this way too - it will just take some time for me to get ready as I
will need to get my GPG key signed by some kernel developers first (that's what
I forgot to do during the last LinuxCon!). I know a few here so it should not
be too hard to get them drunk and sign my key, but I don't expect to be ready
for the 3.7 merge window. Would anybody be concerned by this delay? By the
meantime I can also make a branch available somewhere.
Alex.
^ permalink raw reply
* Re: [PATCH v6 0/4] Runtime Interpreted Power Sequences
From: Tomi Valkeinen @ 2012-09-13 5:50 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Stephen Warren, Thierry Reding, Simon Glass, Grant Likely,
Rob Herring, Mark Brown, Anton Vorontsov, David Woodhouse,
Arnd Bergmann, Leela Krishna Amudala, linux-tegra, linux-kernel,
linux-fbdev, devicetree-discuss, linux-pm, linux-doc
In-Reply-To: <1347443867-18868-1-git-send-email-acourbot@nvidia.com>
[-- Attachment #1: Type: text/plain, Size: 1432 bytes --]
On Wed, 2012-09-12 at 18:57 +0900, Alexandre Courbot wrote:
> New revision of the power sequences, taking as usual the feedback that was
> kindly provided about the last version.
>
> I think now is a good time to discuss integrating this and to start looking for
> a maintainer who would be willing to merge this into his/her tree (I am
> especially thinking about the power framework maintainers, since this is where
> the code is right now. The second patch in this series enables the pwm_backlight
> driver to be used with the device tree, without relying on board-dependent
> callbacks to support complex power sequences. We also plan to use power
> sequences in other Tegra drivers, and other people have expressed interest in
> this work during earlier reviews. See for instance
>
> https://lists.ozlabs.org/pipermail/devicetree-discuss/2012-August/018532.html
>
> and
>
> https://lkml.org/lkml/2012/9/6/270
>
> There is probably some more details to fix and improve, but the current shape
> should be enough to know if we want this and where - therefore any sign from
> a maintainer would be greatly appreciated!
I want to reiterate my opinion that I think power sequences in DT data
is the wrong way to go. Powering sequences are device specific issues
and should be handled in the device driver. But I also think that power
sequences inside the drivers would probably be useful.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v6 1/4] Runtime Interpreted Power Sequences
From: Tomi Valkeinen @ 2012-09-13 5:45 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Stephen Warren, Thierry Reding, Simon Glass, Grant Likely,
Rob Herring, Mark Brown, Anton Vorontsov, David Woodhouse,
Arnd Bergmann, Leela Krishna Amudala,
linux-tegra-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-pm-u79uwXL29TY76Z2rM5mHXA, linux-doc-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1347443867-18868-2-git-send-email-acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 805 bytes --]
On Wed, 2012-09-12 at 18:57 +0900, Alexandre Courbot wrote:
> Some device drivers (panel backlights especially) need to follow precise
> sequences for powering on and off, involving gpios, regulators, PWMs
> with a precise powering order and delays to respect between each steps.
> These sequences are board-specific, and do not belong to a particular
> driver - therefore they have been performed by board-specific hook
> functions to far.
The sequences are not board-specific, they are device (backlight, etc.)
specific. The sequences have been handled in board-specific hook
functions so far because there hasn't been proper drivers for the
devices.
If I were to take the same panel (and backlight) you have and install it
on my board, I would need the same power sequence.
Tomi
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [RFC 1/5] video: Add generic display panel core
From: Laurent Pinchart @ 2012-09-13 1:40 UTC (permalink / raw)
To: Sascha Hauer
Cc: linux-fbdev, dri-devel, linux-leds, linux-media, Bryan Wu,
Richard Purdie, Tomi Valkeinen, Marcus Lorentzon, Sumit Semwal,
Archit Taneja, Sebastien Guiriec, Inki Dae, Kyungmin Park
In-Reply-To: <20120904092446.GN24458@pengutronix.de>
Hi Sascha,
On Tuesday 04 September 2012 11:24:46 Sascha Hauer wrote:
> Hi Laurent,
>
> On Fri, Aug 17, 2012 at 02:49:39AM +0200, Laurent Pinchart wrote:
> > +/**
> > + * panel_get_modes - Get video modes supported by the panel
> > + * @panel: The panel
> > + * @modes: Pointer to an array of modes
> > + *
> > + * Fill the modes argument with a pointer to an array of video modes. The
> > array
> > + * is owned by the panel.
> > + *
> > + * Return the number of supported modes on success (including 0 if no
> > mode is
> > + * supported) or a negative error code otherwise.
> > + */
> > +int panel_get_modes(struct panel *panel, const struct fb_videomode
> > **modes)
> > +{
> > + if (!panel->ops || !panel->ops->get_modes)
> > + return 0;
> > +
> > + return panel->ops->get_modes(panel, modes);
> > +}
> > +EXPORT_SYMBOL_GPL(panel_get_modes);
>
> You have seen my of videomode helper proposal. One result there was that
> we want to have ranges for the margin/synclen fields. Does it make sense
> to base this new panel framework on a more sophisticated internal
> reprentation of the panel parameters?
I think it does, yes. We need a common video mode structure, and the panel
framework should use it. I'll try to rebase my patches on top of your
proposal. Have you posted the latest version ?
> This could then be converted to struct fb_videomode / struct
> drm_display_mode as needed. This would also make it more suitable for drm
> drivers which are not interested in struct fb_videomode.
>
> Related to this I suggest to change the API to be able to iterate over
> the different modes, like:
>
> struct videomode *panel_get_mode(struct panel *panel, int num);
>
> This was we do not have to have an array of modes in memory, which may
> be a burden for some panel drivers.
I currently have mixed feelings about this. Both approaches have pros and
cons. Iterating over the modes would be more complex for drivers that use
panels, and would be race-prone if the modes can change at runtime (OK, this
isn't supported by the current panel API proposal).
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH v6 2/4] pwm_backlight: use power sequences
From: Stephen Warren @ 2012-09-12 22:15 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Stephen Warren, Thierry Reding, Simon Glass, Grant Likely,
Rob Herring, Mark Brown, Anton Vorontsov, David Woodhouse,
Arnd Bergmann, Leela Krishna Amudala, linux-tegra, linux-kernel,
linux-fbdev, devicetree-discuss, linux-pm, linux-doc
In-Reply-To: <1347443867-18868-3-git-send-email-acourbot@nvidia.com>
On 09/12/2012 03:57 AM, Alexandre Courbot wrote:
> Make use of the power sequences specified in the device tree or platform
> data to control how the backlight is powered on and off.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Stephen Warren <swarren@wwwdotorg.org>
^ permalink raw reply
* Re: [PATCH v6 1/4] Runtime Interpreted Power Sequences
From: Stephen Warren @ 2012-09-12 22:07 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Stephen Warren, Thierry Reding, Simon Glass, Grant Likely,
Rob Herring, Mark Brown, Anton Vorontsov, David Woodhouse,
Arnd Bergmann, Leela Krishna Amudala, linux-tegra, linux-kernel,
linux-fbdev, devicetree-discuss, linux-pm, linux-doc
In-Reply-To: <1347443867-18868-2-git-send-email-acourbot@nvidia.com>
On 09/12/2012 03:57 AM, Alexandre Courbot wrote:
> Some device drivers (panel backlights especially) need to follow precise
> sequences for powering on and off, involving gpios, regulators, PWMs
> with a precise powering order and delays to respect between each steps.
> These sequences are board-specific, and do not belong to a particular
> driver - therefore they have been performed by board-specific hook
> functions to far.
>
> With the advent of the device tree and of ARM kernels that are not
> board-tied, we cannot rely on these board-specific hooks anymore but
> need a way to implement these sequences in a portable manner. This patch
> introduces a simple interpreter that can execute such power sequences
> encoded either as platform data or within the device tree.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> diff --git a/Documentation/power/power_seq.txt b/Documentation/power/power_seq.txt
> +Sometimes, you may want to browse the list of resources allocated by a sequence,
> +for instance to ensure that a resource of a given type is present. The
> +power_seq_set_resources() function returns a list head that can be used with
> +the power_seq_for_each_resource() macro to browse all the resources of a set:
> +
> + struct list_head *power_seq_set_resources(struct power_seq_set *seqs);
I don't think you need to include that prototype here?
> + power_seq_for_each_resource(pos, seqs)
> +
> +Here "pos" will be a pointer to a struct power_seq_resource. This structure
> +contains the type of the resource, the information used for identifying it, and
> +the resolved resource itself.
> diff --git a/drivers/power/power_seq/Makefile b/drivers/power/power_seq/Makefile
> new file mode 100644
> index 0000000..f77a359
> --- /dev/null
> +++ b/drivers/power/power_seq/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_POWER_SEQ) += power_seq.o
Don't you need to compile all the power_seq_*.c too?
Oh, I see the following in power_seq.c:
> +#include "power_seq_delay.c"
> +#include "power_seq_regulator.c"
> +#include "power_seq_pwm.c"
> +#include "power_seq_gpio.c"
It's probably better just to compile them separately and link them.
> diff --git a/drivers/power/power_seq/power_seq.c b/drivers/power/power_seq/power_seq.c
> +struct power_seq_step {
> + /* Copy of the platform data */
> + struct platform_power_seq_step pdata;
I'd reword the comment to "Copy of the step", and name the field "step".
> +static const struct power_seq_res_ops power_seq_types[POWER_SEQ_NUM_TYPES] = {
> + [POWER_SEQ_DELAY] = POWER_SEQ_DELAY_TYPE,
> + [POWER_SEQ_REGULATOR] = POWER_SEQ_REGULATOR_TYPE,
> + [POWER_SEQ_PWM] = POWER_SEQ_PWM_TYPE,
> + [POWER_SEQ_GPIO] = POWER_SEQ_GPIO_TYPE,
> +};
Ah, I see why you're using #include now.
> +MODULE_LICENSE("GPL");
s/GPL/GPL v2/ given the license header.
> diff --git a/drivers/power/power_seq/power_seq_gpio.c b/drivers/power/power_seq/power_seq_gpio.c
> +static int power_seq_res_alloc_gpio(struct device *dev,
> + struct platform_power_seq_step *pstep,
> + struct power_seq_resource *res)
> +{
> + int err;
> +
> + err = devm_gpio_request_one(dev, pstep->gpio.gpio,
> + GPIOF_OUT_INIT_LOW, dev_name(dev));
Hmm. The INIT_LOW part of that might be somewhat presumptive. I would
suggest simply requesting the GPIO here, and using
gpio_direction_output() in power_seq_step_run_gpio(), thus deferring the
decision of what value to set the GPIO to until a real sequence is
actually run.
> diff --git a/drivers/power/power_seq/power_seq_pwm.c b/drivers/power/power_seq/power_seq_pwm.c
> diff --git a/drivers/power/power_seq/power_seq_regulator.c b/drivers/power/power_seq/power_seq_regulator.c
> diff --git a/include/linux/power_seq.h b/include/linux/power_seq.h
> +#include <net/irda/parameters.h>
That looks out of place.
> +/**
> + * struct power_seq_resource - resource used by a power sequence set
> + * @pdata: Pointer to the platform data used to resolve this resource
> + * @regulator: Resolved regulator if of type POWER_SEQ_REGULATOR
> + * @pwm: Resolved PWM if of type POWER_SEQ_PWM
> + * @list: Used to link resources together
> + */
I think that kerneldoc is stale.
> +struct power_seq_resource {
> + enum power_seq_res_type type;
> + /* resolved resource and identifier */
> + union {
> + struct {
> + struct regulator *regulator;
> + const char *id;
> + } regulator;
> + struct {
> + struct pwm_device *pwm;
> + const char *id;
> + } pwm;
> + struct {
> + int gpio;
> + } gpio;
> + };
> + struct list_head list;
> +};
Aside from those minor issues, this all looks reasonable to me, so,
Reviewed-by: Stephen Warren <swarren@wwwdotorg.org>
^ permalink raw reply
* Re: [PATCH v6 0/4] Runtime Interpreted Power Sequences
From: Anton Vorontsov @ 2012-09-12 21:33 UTC (permalink / raw)
To: Stephen Warren
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA, Mark Brown, Stephen Warren,
linux-pm-u79uwXL29TY76Z2rM5mHXA, Leela Krishna Amudala,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
linux-tegra-u79uwXL29TY76Z2rM5mHXA, David Woodhouse,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ
In-Reply-To: <5050FE28.2080502-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
On Wed, Sep 12, 2012 at 03:27:04PM -0600, Stephen Warren wrote:
> On 09/12/2012 03:57 AM, Alexandre Courbot wrote:
> > New revision of the power sequences, taking as usual the feedback that was
> > kindly provided about the last version.
> >
> > I think now is a good time to discuss integrating this and to start looking for
> > a maintainer who would be willing to merge this into his/her tree (I am
> > especially thinking about the power framework maintainers, since this is where
> > the code is right now.
>
> The other alternative is for you to maintain this going forward; I
> believe that would be as simple as:
>
> * Create a patch to add yourself to MAINTAINERS for the
> drivers/power/power_seq/ directory.
>
> * Get a kernel.org account, push this patch to a branch there, and add
> the branch into linux-next.
>
> * Send a pull request to Linus at the appropriate time.
>
> * Ongoing: Accept any patches, perform any maintenance required, etc.
>
> Does anyone see any issue with Alexandre doing this? Nobody else has
> volunteered yet:-)
Yup, looks like the best way.
Anton.
^ permalink raw reply
* Re: [PATCH v6 0/4] Runtime Interpreted Power Sequences
From: Stephen Warren @ 2012-09-12 21:27 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Stephen Warren, Thierry Reding, Simon Glass, Grant Likely,
Rob Herring, Mark Brown, Anton Vorontsov, David Woodhouse,
Arnd Bergmann, linux-fbdev, linux-pm, Leela Krishna Amudala,
devicetree-discuss, linux-doc, linux-kernel, linux-tegra
In-Reply-To: <1347443867-18868-1-git-send-email-acourbot@nvidia.com>
On 09/12/2012 03:57 AM, Alexandre Courbot wrote:
> New revision of the power sequences, taking as usual the feedback that was
> kindly provided about the last version.
>
> I think now is a good time to discuss integrating this and to start looking for
> a maintainer who would be willing to merge this into his/her tree (I am
> especially thinking about the power framework maintainers, since this is where
> the code is right now.
The other alternative is for you to maintain this going forward; I
believe that would be as simple as:
* Create a patch to add yourself to MAINTAINERS for the
drivers/power/power_seq/ directory.
* Get a kernel.org account, push this patch to a branch there, and add
the branch into linux-next.
* Send a pull request to Linus at the appropriate time.
* Ongoing: Accept any patches, perform any maintenance required, etc.
Does anyone see any issue with Alexandre doing this? Nobody else has
volunteered yet:-)
^ permalink raw reply
* Re: [PATCH v6 4/4] tegra: ventana: add pwm backlight DT nodes
From: Stephen Warren @ 2012-09-12 21:23 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Stephen Warren, Thierry Reding, Simon Glass, Grant Likely,
Rob Herring, Mark Brown, Anton Vorontsov, David Woodhouse,
Arnd Bergmann, linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
linux-pm-u79uwXL29TY76Z2rM5mHXA, Leela Krishna Amudala,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1347443867-18868-5-git-send-email-acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
On 09/12/2012 03:57 AM, Alexandre Courbot wrote:
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Both patches 3 and 4 in the series,
Acked-by: Stephen Warren <swarren@wwwdotorg.org>
I will take those patches into the Tegra tree when (hopefully rather
than if) patches 1 and 2 are accepted.
^ permalink raw reply
* [PATCH v6 4/4] tegra: ventana: add pwm backlight DT nodes
From: Alexandre Courbot @ 2012-09-12 9:57 UTC (permalink / raw)
To: Stephen Warren, Thierry Reding, Simon Glass, Grant Likely,
Rob Herring, Mark Brown, Anton Vorontsov, David Woodhouse,
Arnd Bergmann
Cc: Leela Krishna Amudala, linux-tegra, linux-kernel, linux-fbdev,
devicetree-discuss, linux-pm, linux-doc, Alexandre Courbot
In-Reply-To: <1347443867-18868-1-git-send-email-acourbot@nvidia.com>
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
arch/arm/boot/dts/tegra20-ventana.dts | 59 ++++++++++++++++++++++++++++++++++-
1 file changed, 58 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/tegra20-ventana.dts b/arch/arm/boot/dts/tegra20-ventana.dts
index 3e5952f..4ca3ceb 100644
--- a/arch/arm/boot/dts/tegra20-ventana.dts
+++ b/arch/arm/boot/dts/tegra20-ventana.dts
@@ -469,6 +469,63 @@
bus-width = <8>;
};
+ backlight {
+ compatible = "pwm-backlight";
+ brightness-levels = <0 16 32 48 64 80 96 112 128 144 160 176 192 208 224 240 255>;
+ default-brightness-level = <12>;
+
+ /* resources used by the power sequences */
+ pwms = <&pwm 2 5000000>;
+ pwm-names = "backlight";
+ power-supply = <&vdd_bl_reg>;
+
+ power-sequences {
+ power-on {
+ step0 {
+ type = "regulator";
+ id = "power";
+ enable;
+ };
+ step1 {
+ type = "delay";
+ delay = <10000>;
+ };
+ step2 {
+ type = "pwm";
+ id = "backlight";
+ enable;
+ };
+ step3 {
+ type = "gpio";
+ gpio = <&gpio 28 0>;
+ value = <1>;
+ };
+ };
+
+ power-off {
+ step0 {
+ type = "gpio";
+ gpio = <&gpio 28 0>;
+ value = <0>;
+ };
+ step1 {
+ type = "pwm";
+ id = "backlight";
+ disable;
+ };
+ step2 {
+ type = "delay";
+ delay = <10000>;
+ };
+ step3 {
+ type = "regulator";
+ id = "power";
+ disable;
+ };
+ };
+ };
+ };
+
regulators {
compatible = "simple-bus";
#address-cells = <1>;
@@ -512,7 +569,7 @@
enable-active-high;
};
- regulator@4 {
+ vdd_bl_reg: regulator@4 {
compatible = "regulator-fixed";
reg = <4>;
regulator-name = "vdd_bl";
--
1.7.12
^ permalink raw reply related
* [PATCH v6 3/4] tegra: dt: add label to tegra20's PWM
From: Alexandre Courbot @ 2012-09-12 9:57 UTC (permalink / raw)
To: Stephen Warren, Thierry Reding, Simon Glass, Grant Likely,
Rob Herring, Mark Brown, Anton Vorontsov, David Woodhouse,
Arnd Bergmann
Cc: Leela Krishna Amudala, linux-tegra-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-pm-u79uwXL29TY76Z2rM5mHXA, linux-doc-u79uwXL29TY76Z2rM5mHXA,
Alexandre Courbot
In-Reply-To: <1347443867-18868-1-git-send-email-acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
This is necessary to reference the PWM in board device trees.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
arch/arm/boot/dts/tegra20.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
index 405d167..67a6cd9 100644
--- a/arch/arm/boot/dts/tegra20.dtsi
+++ b/arch/arm/boot/dts/tegra20.dtsi
@@ -123,7 +123,7 @@
status = "disabled";
};
- pwm {
+ pwm: pwm {
compatible = "nvidia,tegra20-pwm";
reg = <0x7000a000 0x100>;
#pwm-cells = <2>;
--
1.7.12
^ permalink raw reply related
* [PATCH v6 2/4] pwm_backlight: use power sequences
From: Alexandre Courbot @ 2012-09-12 9:57 UTC (permalink / raw)
To: Stephen Warren, Thierry Reding, Simon Glass, Grant Likely,
Rob Herring, Mark Brown, Anton Vorontsov, David Woodhouse,
Arnd Bergmann
Cc: Leela Krishna Amudala, linux-tegra, linux-kernel, linux-fbdev,
devicetree-discuss, linux-pm, linux-doc, Alexandre Courbot
In-Reply-To: <1347443867-18868-1-git-send-email-acourbot@nvidia.com>
Make use of the power sequences specified in the device tree or platform
data to control how the backlight is powered on and off.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
.../bindings/video/backlight/pwm-backlight.txt | 65 +++++++-
drivers/video/backlight/Kconfig | 1 +
drivers/video/backlight/pwm_bl.c | 180 +++++++++++++++------
include/linux/pwm_backlight.h | 15 +-
4 files changed, 208 insertions(+), 53 deletions(-)
diff --git a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
index 689c7d2..3b7e99b 100644
--- a/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
+++ b/Documentation/devicetree/bindings/video/backlight/pwm-backlight.txt
@@ -2,7 +2,8 @@ pwm-backlight bindings
Required properties:
- compatible: "pwm-backlight"
- - pwms: OF device-tree PWM specification (see PWM binding[0])
+ - pwms: OF device-tree PWM specification (see PWM binding[0]). Exactly one PWM
+ must be specified
- brightness-levels: Array of distinct brightness levels. Typically these
are in the range from 0 to 255, but any range starting at 0 will do.
The actual brightness level (PWM duty cycle) will be interpolated
@@ -16,17 +17,75 @@ Optional properties:
"pwms" property (see PWM binding[0])
- low_threshold_brightness: brightness threshold low level. (get linear
scales in brightness in low end of brightness levels)
+ - pwm-names: name for the PWM device specified in the "pwms" property (see PWM
+ binding[0]). Necessary if power sequences are used
+ - power-sequences: Power sequences (see Power sequences[1]) used to bring the
+ backlight on and off. If this property is present, then two power
+ sequences named "power-on" and "power-off" must be defined to control how
+ the backlight is to be powered on and off. These sequences must reference
+ the PWM specified in the pwms property by its name, and can also reference
+ other resources supported by the power sequences mechanism
[0]: Documentation/devicetree/bindings/pwm/pwm.txt
+[1]: Documentation/devicetree/bindings/power_seq/power_seq.txt
Example:
backlight {
compatible = "pwm-backlight";
- pwms = <&pwm 0 5000000>;
-
brightness-levels = <0 4 8 16 32 64 128 255>;
default-brightness-level = <6>;
+
+ /* resources used by the sequences */
+ pwms = <&pwm 2 5000000>;
+ pwm-names = "backlight";
+ power-supply = <&backlight_reg>;
+
+ power-sequences {
+ power-on {
+ step0 {
+ type = "regulator";
+ id = "power";
+ enable;
+ };
+ step1 {
+ type = "delay";
+ delay = <10000>;
+ };
+ step2 {
+ type = "pwm";
+ id = "backlight";
+ enable;
+ };
+ step3 {
+ type = "gpio";
+ gpio = <&gpio 28 0>;
+ value = <1>;
+ };
+ };
+
+ power-off {
+ step0 {
+ type = "gpio";
+ gpio = <&gpio 28 0>;
+ value = <0>;
+ };
+ step1 {
+ type = "pwm";
+ id = "backlight";
+ disable;
+ };
+ step2 {
+ type = "delay";
+ delay = <10000>;
+ };
+ step3 {
+ type = "regulator";
+ id = "power";
+ disable;
+ };
+ };
+ };
};
Example for brightness_threshold_level:
diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index 950a131..ca86125 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -239,6 +239,7 @@ config BACKLIGHT_CARILLO_RANCH
config BACKLIGHT_PWM
tristate "Generic PWM based Backlight Driver"
depends on PWM
+ select POWER_SEQ
help
If you have a LCD backlight adjustable by PWM, say Y to enable
this driver.
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
index 4965408..37a23c5 100644
--- a/drivers/video/backlight/pwm_bl.c
+++ b/drivers/video/backlight/pwm_bl.c
@@ -27,6 +27,12 @@ struct pwm_bl_data {
unsigned int period;
unsigned int lth_brightness;
unsigned int *levels;
+ bool enabled;
+ struct power_seq_set *power_seqs;
+ struct power_seq *power_on_seq;
+ struct power_seq *power_off_seq;
+
+ /* Legacy callbacks */
int (*notify)(struct device *,
int brightness);
void (*notify_after)(struct device *,
@@ -35,6 +41,52 @@ struct pwm_bl_data {
void (*exit)(struct device *);
};
+static void pwm_backlight_on(struct backlight_device *bl)
+{
+ struct pwm_bl_data *pb = dev_get_drvdata(&bl->dev);
+ int ret;
+
+ if (pb->enabled)
+ return;
+
+ if (pb->power_seqs) {
+ ret = power_seq_run(pb->power_on_seq);
+ if (ret < 0) {
+ dev_err(&bl->dev, "cannot run power on sequence\n");
+ return;
+ }
+ } else {
+ /* legacy framework */
+ pwm_config(pb->pwm, 0, pb->period);
+ pwm_disable(pb->pwm);
+ }
+
+ pb->enabled = true;
+}
+
+static void pwm_backlight_off(struct backlight_device *bl)
+{
+ struct pwm_bl_data *pb = dev_get_drvdata(&bl->dev);
+ int ret;
+
+ if (!pb->enabled)
+ return;
+
+ if (pb->power_seqs) {
+ ret = power_seq_run(pb->power_off_seq);
+ if (ret < 0) {
+ dev_err(&bl->dev, "cannot run power off sequence\n");
+ return;
+ }
+ } else {
+ /* legacy framework */
+ pwm_enable(pb->pwm);
+ return;
+ }
+
+ pb->enabled = false;
+}
+
static int pwm_backlight_update_status(struct backlight_device *bl)
{
struct pwm_bl_data *pb = dev_get_drvdata(&bl->dev);
@@ -51,8 +103,7 @@ static int pwm_backlight_update_status(struct backlight_device *bl)
brightness = pb->notify(pb->dev, brightness);
if (brightness = 0) {
- pwm_config(pb->pwm, 0, pb->period);
- pwm_disable(pb->pwm);
+ pwm_backlight_off(bl);
} else {
int duty_cycle;
@@ -66,7 +117,7 @@ static int pwm_backlight_update_status(struct backlight_device *bl)
duty_cycle = pb->lth_brightness +
(duty_cycle * (pb->period - pb->lth_brightness) / max);
pwm_config(pb->pwm, duty_cycle, pb->period);
- pwm_enable(pb->pwm);
+ pwm_backlight_on(bl);
}
if (pb->notify_after)
@@ -150,12 +201,6 @@ static int pwm_backlight_parse_dt(struct device *dev,
data->lth_brightness = value;
}
- /*
- * TODO: Most users of this driver use a number of GPIOs to control
- * backlight power. Support for specifying these needs to be
- * added.
- */
-
return 0;
}
@@ -177,33 +222,97 @@ static int pwm_backlight_probe(struct platform_device *pdev)
{
struct platform_pwm_backlight_data *data = pdev->dev.platform_data;
struct platform_pwm_backlight_data defdata;
+ struct power_seq_resource *res;
struct backlight_properties props;
struct backlight_device *bl;
struct pwm_bl_data *pb;
unsigned int max;
int ret;
+ pb = devm_kzalloc(&pdev->dev, sizeof(*pb), GFP_KERNEL);
+ if (!pb) {
+ dev_err(&pdev->dev, "no memory for state\n");
+ return -ENOMEM;
+ }
+
+ /* using device tree? */
if (!data) {
+ /* build platform data from device tree */
ret = pwm_backlight_parse_dt(&pdev->dev, &defdata);
- if (ret < 0) {
+ if (ret = -EPROBE_DEFER) {
+ return ret;
+ } else if (ret < 0) {
dev_err(&pdev->dev, "failed to find platform data\n");
return ret;
}
-
data = &defdata;
}
+ /* using power sequences? */
+ pb->power_seqs = devm_power_seq_set_build(&pdev->dev, data->power_seqs);
+ if (pb->power_seqs) {
+ struct power_seq_set *seqs = pb->power_seqs;
+
+ if (IS_ERR(seqs))
+ return PTR_ERR(seqs);
+
+ pb->power_on_seq = power_seq_lookup(seqs, "power-on");
+ if (!pb->power_on_seq) {
+ dev_err(&pdev->dev, "missing power-on sequence\n");
+ return -EINVAL;
+ }
+ pb->power_off_seq = power_seq_lookup(seqs, "power-off");
+ if (!pb->power_off_seq) {
+ dev_err(&pdev->dev, "missing power-off sequence\n");
+ return -EINVAL;
+ }
+
+ /* we must have exactly one PWM for this driver */
+ power_seq_for_each_resource(res, seqs) {
+ if (res->type != POWER_SEQ_PWM)
+ continue;
+ if (pb->pwm) {
+ dev_err(&pdev->dev, "more than one PWM used\n");
+ return -EINVAL;
+ }
+ /* keep the pwm at hand */
+ pb->pwm = res->pwm.pwm;
+ }
+ }
+ /* using legacy interface? */
+ else {
+ pb->pwm = devm_pwm_get(&pdev->dev, NULL);
+ if (IS_ERR(pb->pwm)) {
+ dev_err(&pdev->dev,
+ "unable to request PWM, trying legacy API\n");
+
+ pb->pwm = pwm_request(data->pwm_id, "pwm-backlight");
+ if (IS_ERR(pb->pwm)) {
+ dev_err(&pdev->dev,
+ "unable to request legacy PWM\n");
+ return PTR_ERR(pb->pwm);
+ }
+ }
+
+ /*
+ * The DT case will set the pwm_period_ns field to 0 and store
+ * the period, parsed from the DT, in the PWM device. For the
+ * non-DT case, set the period from platform data.
+ */
+ if (data->pwm_period_ns > 0)
+ pwm_set_period(pb->pwm, data->pwm_period_ns);
+ }
+
if (data->init) {
ret = data->init(&pdev->dev);
if (ret < 0)
- return ret;
+ goto err;
}
- pb = devm_kzalloc(&pdev->dev, sizeof(*pb), GFP_KERNEL);
- if (!pb) {
- dev_err(&pdev->dev, "no memory for state\n");
- ret = -ENOMEM;
- goto err_alloc;
+ /* from here we should have a PWM */
+ if (!pb->pwm) {
+ dev_err(&pdev->dev, "no PWM defined!\n");
+ return -EINVAL;
}
if (data->levels) {
@@ -218,28 +327,6 @@ static int pwm_backlight_probe(struct platform_device *pdev)
pb->exit = data->exit;
pb->dev = &pdev->dev;
- pb->pwm = pwm_get(&pdev->dev, NULL);
- if (IS_ERR(pb->pwm)) {
- dev_err(&pdev->dev, "unable to request PWM, trying legacy API\n");
-
- pb->pwm = pwm_request(data->pwm_id, "pwm-backlight");
- if (IS_ERR(pb->pwm)) {
- dev_err(&pdev->dev, "unable to request legacy PWM\n");
- ret = PTR_ERR(pb->pwm);
- goto err_alloc;
- }
- }
-
- dev_dbg(&pdev->dev, "got pwm for backlight\n");
-
- /*
- * The DT case will set the pwm_period_ns field to 0 and store the
- * period, parsed from the DT, in the PWM device. For the non-DT case,
- * set the period from platform data.
- */
- if (data->pwm_period_ns > 0)
- pwm_set_period(pb->pwm, data->pwm_period_ns);
-
pb->period = pwm_get_period(pb->pwm);
pb->lth_brightness = data->lth_brightness * (pb->period / max);
@@ -251,18 +338,17 @@ static int pwm_backlight_probe(struct platform_device *pdev)
if (IS_ERR(bl)) {
dev_err(&pdev->dev, "failed to register backlight\n");
ret = PTR_ERR(bl);
- goto err_bl;
+ goto err;
}
bl->props.brightness = data->dft_brightness;
backlight_update_status(bl);
platform_set_drvdata(pdev, bl);
+
return 0;
-err_bl:
- pwm_put(pb->pwm);
-err_alloc:
+err:
if (data->exit)
data->exit(&pdev->dev);
return ret;
@@ -274,9 +360,8 @@ static int pwm_backlight_remove(struct platform_device *pdev)
struct pwm_bl_data *pb = dev_get_drvdata(&bl->dev);
backlight_device_unregister(bl);
- pwm_config(pb->pwm, 0, pb->period);
- pwm_disable(pb->pwm);
- pwm_put(pb->pwm);
+ pwm_backlight_off(bl);
+
if (pb->exit)
pb->exit(&pdev->dev);
return 0;
@@ -290,8 +375,7 @@ static int pwm_backlight_suspend(struct device *dev)
if (pb->notify)
pb->notify(pb->dev, 0);
- pwm_config(pb->pwm, 0, pb->period);
- pwm_disable(pb->pwm);
+ pwm_backlight_off(bl);
if (pb->notify_after)
pb->notify_after(pb->dev, 0);
return 0;
diff --git a/include/linux/pwm_backlight.h b/include/linux/pwm_backlight.h
index 56f4a86..70d22f2 100644
--- a/include/linux/pwm_backlight.h
+++ b/include/linux/pwm_backlight.h
@@ -5,14 +5,25 @@
#define __LINUX_PWM_BACKLIGHT_H
#include <linux/backlight.h>
+#include <linux/power_seq.h>
struct platform_pwm_backlight_data {
- int pwm_id;
unsigned int max_brightness;
unsigned int dft_brightness;
unsigned int lth_brightness;
- unsigned int pwm_period_ns;
unsigned int *levels;
+ /*
+ * New interface using power sequences
+ */
+ struct platform_power_seq_set *power_seqs;
+ /*
+ * Legacy interface - use power sequences instead!
+ *
+ * pwm_id and pwm_period_ns need only be specified
+ * if get_pwm(dev, NULL) would return NULL.
+ */
+ int pwm_id;
+ unsigned int pwm_period_ns;
int (*init)(struct device *dev);
int (*notify)(struct device *dev, int brightness);
void (*notify_after)(struct device *dev, int brightness);
--
1.7.12
^ permalink raw reply related
* [PATCH v6 1/4] Runtime Interpreted Power Sequences
From: Alexandre Courbot @ 2012-09-12 9:57 UTC (permalink / raw)
To: Stephen Warren, Thierry Reding, Simon Glass, Grant Likely,
Rob Herring, Mark Brown, Anton Vorontsov, David Woodhouse,
Arnd Bergmann
Cc: Leela Krishna Amudala, linux-tegra-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
linux-pm-u79uwXL29TY76Z2rM5mHXA, linux-doc-u79uwXL29TY76Z2rM5mHXA,
Alexandre Courbot
In-Reply-To: <1347443867-18868-1-git-send-email-acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Some device drivers (panel backlights especially) need to follow precise
sequences for powering on and off, involving gpios, regulators, PWMs
with a precise powering order and delays to respect between each steps.
These sequences are board-specific, and do not belong to a particular
driver - therefore they have been performed by board-specific hook
functions to far.
With the advent of the device tree and of ARM kernels that are not
board-tied, we cannot rely on these board-specific hooks anymore but
need a way to implement these sequences in a portable manner. This patch
introduces a simple interpreter that can execute such power sequences
encoded either as platform data or within the device tree.
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
.../devicetree/bindings/power_seq/power_seq.txt | 122 ++++++
Documentation/power/power_seq.txt | 215 ++++++++++
drivers/power/Kconfig | 1 +
drivers/power/Makefile | 1 +
drivers/power/power_seq/Kconfig | 2 +
drivers/power/power_seq/Makefile | 1 +
drivers/power/power_seq/power_seq.c | 446 +++++++++++++++++++++
drivers/power/power_seq/power_seq_delay.c | 51 +++
drivers/power/power_seq/power_seq_gpio.c | 91 +++++
drivers/power/power_seq/power_seq_pwm.c | 87 ++++
drivers/power/power_seq/power_seq_regulator.c | 87 ++++
include/linux/power_seq.h | 172 ++++++++
12 files changed, 1276 insertions(+)
create mode 100644 Documentation/devicetree/bindings/power_seq/power_seq.txt
create mode 100644 Documentation/power/power_seq.txt
create mode 100644 drivers/power/power_seq/Kconfig
create mode 100644 drivers/power/power_seq/Makefile
create mode 100644 drivers/power/power_seq/power_seq.c
create mode 100644 drivers/power/power_seq/power_seq_delay.c
create mode 100644 drivers/power/power_seq/power_seq_gpio.c
create mode 100644 drivers/power/power_seq/power_seq_pwm.c
create mode 100644 drivers/power/power_seq/power_seq_regulator.c
create mode 100644 include/linux/power_seq.h
diff --git a/Documentation/devicetree/bindings/power_seq/power_seq.txt b/Documentation/devicetree/bindings/power_seq/power_seq.txt
new file mode 100644
index 0000000..2b394eb
--- /dev/null
+++ b/Documentation/devicetree/bindings/power_seq/power_seq.txt
@@ -0,0 +1,122 @@
+Runtime Interpreted Power Sequences
+=================+
+Power sequences are sequential descriptions of actions to be performed on
+power-related resources. Having these descriptions in a well-defined data format
+allows us to take much of the board-specific power control code out of the
+kernel and place it into the device tree instead, making kernels less
+board-dependant.
+
+A device typically makes use of multiple power sequences, for different purposes
+such as powering on and off. All the power sequences of a given device are
+grouped into a set. In the device tree, this set is a sub-node of the device
+node named "power-sequences".
+
+Power Sequences Structure
+-------------------------
+Every device that makes use of power sequences must have a "power-sequences"
+node into which individual power sequences are declared as sub-nodes. The name
+of the node becomes the name of the sequence within the power sequences
+framework.
+
+Similarly, each power sequence declares its steps as sub-nodes of itself. Steps
+must be named sequentially, with the first step named step0, the second step1,
+etc. Failure to follow this rule will result in a parsing error.
+
+Power Sequences Steps
+---------------------
+Steps of a sequence describe an action to be performed on a resource. They
+always include a "type" property which indicates what kind of resource this
+step works on. Depending on the resource type, additional properties are defined
+to control the action to be performed.
+
+"delay" type required properties:
+ - delay: delay to wait (in microseconds)
+
+"regulator" type required properties:
+ - id: name of the regulator to use. Regulator is obtained by
+ regulator_get(dev, id)
+ - enable / disable: one of these two empty properties must be present to
+ enable or disable the resource
+
+"pwm" type required properties:
+ - id: name of the PWM to use. PWM is obtained by pwm_get(dev, id)
+ - enable / disable: one of these two empty properties must be present to
+ enable or disable the resource
+
+"gpio" type required properties:
+ - gpio: phandle of the GPIO to use.
+ - value: value this GPIO should take. Must be 0 or 1.
+
+Example
+-------
+Here are example sequences declared within a backlight device that use all the
+supported resources types:
+
+ backlight {
+ compatible = "pwm-backlight";
+ ...
+
+ /* resources used by the power sequences */
+ pwms = <&pwm 2 5000000>;
+ pwm-names = "backlight";
+ power-supply = <&backlight_reg>;
+
+ power-sequences {
+ power-on {
+ step0 {
+ type = "regulator";
+ id = "power";
+ enable;
+ };
+ step1 {
+ type = "delay";
+ delay = <10000>;
+ };
+ step2 {
+ type = "pwm";
+ id = "backlight";
+ enable;
+ };
+ step3 {
+ type = "gpio";
+ gpio = <&gpio 28 0>;
+ value = <1>;
+ };
+ };
+
+ power-off {
+ step0 {
+ type = "gpio";
+ gpio = <&gpio 28 0>;
+ value = <0>;
+ };
+ step1 {
+ type = "pwm";
+ id = "backlight";
+ disable;
+ };
+ step2 {
+ type = "delay";
+ delay = <10000>;
+ };
+ step3 {
+ type = "regulator";
+ id = "power";
+ disable;
+ };
+ };
+ };
+ };
+
+The first part lists the PWM and regulator resources used by the sequences.
+These resources will be requested on behalf of the backlight device when the
+sequences are built and are declared according to their own framework (for
+instance, regulators and pwms are resolved by name using regulator_get() or
+pwm_get()).
+
+After the resources declaration, two sequences follow for powering the backlight
+on and off. Their names are specified by the pwm-backlight device bindings. Once
+the sequences are built by calling devm_power_seq_set_build() with a NULL pseq
+argument, their names can be passed to power_seq_lookup() to retrieve an actual
+sequence.
diff --git a/Documentation/power/power_seq.txt b/Documentation/power/power_seq.txt
new file mode 100644
index 0000000..7f3f2d2
--- /dev/null
+++ b/Documentation/power/power_seq.txt
@@ -0,0 +1,215 @@
+Runtime Interpreted Power Sequences
+=================+
+Problem
+-------
+Very commonly, boards need the help of out-of-driver code to turn some of their
+devices on and off. For instance, SoC boards very commonly use a GPIO
+(abstracted to a regulator or not) to control the power supply of a backlight,
+disabling it when the backlight is not used in order to save power. The GPIO
+that should be used, however, as well as the exact power sequence that may
+also involve other resources, is board-dependent and thus unknown to the driver.
+
+This was previously addressed by having hooks in the device's platform data that
+are called whenever the state of the device might reflect a power change. This
+approach, however, introduces board-dependant code into the kernel and is not
+compatible with the device tree.
+
+The Runtime Interpreted Power Sequences (or power sequences for short) aim at
+turning this code into platform data or device tree nodes. Power sequences are
+described using a simple format and run by a lightweight interpreter whenever
+needed. This allows device drivers to work without power callbacks and makes the
+kernel less board-dependant.
+
+What are Power Sequences?
+-------------------------
+A power sequence is a suite of sequential steps each describing an action to be
+performed on a resource. The supported resources and actions operations are:
+- delay (just wait for a given number of microseconds)
+- GPIO (set to 0 or 1)
+- regulator (enable or disable)
+- PWM (enable or disable)
+
+When a power sequence is run, each of its steps is executed one after the other
+until one step fails or the end of the sequence is reached.
+
+Power sequences are named, and grouped into "sets" which correspond to all the
+sequences of a device as well as the resources they use.
+
+Power sequences can be declared as platform data or in the device tree.
+
+Platform Data Format
+--------------------
+All relevant data structures for declaring power sequences are located in
+include/linux/power_seq.h.
+
+The platform data for a device may include an instance of platform_power_seq_set
+which references instances of platform_power_seq. Each platform_power_seq is a
+single power sequence, and is itself made of a variable length array of steps.
+
+A step is a union of all the platform step structures, which are defined and
+documented in include/linux/power_seq.h. Which one is to be used depends on the
+type of the step. This might sound complicated, but the following example should
+make it clear how the platform data for power sequences is defined. It declares
+two power sequences named "power-on" and "power-off". The
+"power-on" sequence enables a regulator named "power" (retrieved using
+regulator_get()), waits for 10ms, and then set GPIO 110 to 1. "power-off" does
+the opposite.
+
+static struct platform_power_seq power_on_seq = {
+ .id = "power-on",
+ .num_steps = 3,
+ .steps = {
+ {
+ .type = POWER_SEQ_REGULATOR,
+ .regulator = {
+ .id = "power",
+ .enable = true,
+ },
+ },
+ {
+ .type = POWER_SEQ_DELAY,
+ .delay = {
+ .delay = 10000,
+ },
+ },
+ {
+ .type = POWER_SEQ_GPIO,
+ .gpio = {
+ .gpio = 110,
+ .value = 1,
+ },
+ },
+ },
+};
+
+static struct platform_power_seq power_off_seq = {
+ .id = "power-off",
+ .num_steps = 3,
+ .steps = {
+ {
+ .type = POWER_SEQ_GPIO,
+ .gpio = {
+ .gpio = 110,
+ .value = 0,
+ },
+ },
+ {
+ .type = POWER_SEQ_DELAY,
+ .delay = {
+ .delay = 10000,
+ },
+ },
+ {
+ .type = POWER_SEQ_REGULATOR,
+ .regulator = {
+ .id = "power",
+ .enable = false,
+ },
+ },
+ },
+};
+
+static struct platform_power_seq_set platform_power_sequences = {
+ .num_seqs = 2,
+ .seqs = {
+ &power_on_seq,
+ &power_off_seq,
+ },
+};
+
+"platform_power_sequences" can then be passed to devm_power_seq_set_build() in
+order to build the corresponding sequences set and allocate all the necessary
+resources. More on this later in this document.
+
+Device Tree
+-----------
+Power sequences can also be encoded as device tree nodes. The following
+properties and nodes are equivalent to the platform data defined previously:
+
+power-supply = <&power_reg>;
+
+power-sequences {
+ power-on {
+ step0 {
+ type = "regulator";
+ id = "power";
+ enable;
+ };
+ step1 {
+ type = "delay";
+ delay = <10000>;
+ };
+ step2 {
+ type = "gpio";
+ gpio = <&gpio 110 0>;
+ value = <1>;
+ };
+ }
+ power-off {
+ step0 {
+ type = "gpio";
+ gpio = <&gpio 110 0>;
+ value = <0>;
+ };
+ step1 {
+ type = "delay";
+ delay = <10000>;
+ };
+ step2 {
+ type = "regulator";
+ id = "power";
+ disable;
+ };
+ }
+};
+
+See Documentation/devicetree/bindings/power_seq/power_seq.txt for the complete
+syntax of the DT bindings.
+
+Usage by Drivers and Resources Management
+-----------------------------------------
+Power sequences make use of resources that must be properly allocated and
+managed. The devm_power_seq_set_build() function builds a power sequence set
+from platform data. It also takes care of resolving and allocating the resources
+referenced by the sequence:
+
+ struct power_seq_set *devm_power_seq_set_build(struct device *dev,
+ struct platform_power_seq_set *pseq);
+
+As its name states, all memory and resources are devm-allocated. The 'dev'
+argument is the device in the name of which the resources are to be allocated.
+
+If the pseq argument is NULL, then the power sequences are built from the device
+tree data, if any.
+
+On success, the function returns a devm allocated resolved sequences set for
+which all the resources are allocated. In case of failure, an error code is
+returned. If no set can be built because no platform data is passed and the
+device tree node for the device contains no power sequence, the function returns
+NULL.
+
+Once the set is built, power sequences can be retrieved by their name using
+power_seq_lookup:
+
+ struct power_seq *power_seq_lookup(struct power_seq_set *seqs,
+ const char *id);
+
+A retrieved power sequence can then be executed by power_seq_run:
+
+ int power_seq_run(struct power_seq *seq);
+
+It returns 0 if the sequence has successfully been run, or an error code if a
+problem occurred.
+
+Sometimes, you may want to browse the list of resources allocated by a sequence,
+for instance to ensure that a resource of a given type is present. The
+power_seq_set_resources() function returns a list head that can be used with
+the power_seq_for_each_resource() macro to browse all the resources of a set:
+
+ struct list_head *power_seq_set_resources(struct power_seq_set *seqs);
+ power_seq_for_each_resource(pos, seqs)
+
+Here "pos" will be a pointer to a struct power_seq_resource. This structure
+contains the type of the resource, the information used for identifying it, and
+the resolved resource itself.
diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index fcc1bb0..5fdfd84 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -312,3 +312,4 @@ config AB8500_BATTERY_THERM_ON_BATCTRL
endif # POWER_SUPPLY
source "drivers/power/avs/Kconfig"
+source "drivers/power/power_seq/Kconfig"
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index ee58afb..d3c893b 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -45,3 +45,4 @@ obj-$(CONFIG_CHARGER_MAX8997) += max8997_charger.o
obj-$(CONFIG_CHARGER_MAX8998) += max8998_charger.o
obj-$(CONFIG_POWER_AVS) += avs/
obj-$(CONFIG_CHARGER_SMB347) += smb347-charger.o
+obj-$(CONFIG_POWER_SEQ) += power_seq/
diff --git a/drivers/power/power_seq/Kconfig b/drivers/power/power_seq/Kconfig
new file mode 100644
index 0000000..3bff26e
--- /dev/null
+++ b/drivers/power/power_seq/Kconfig
@@ -0,0 +1,2 @@
+config POWER_SEQ
+ bool
diff --git a/drivers/power/power_seq/Makefile b/drivers/power/power_seq/Makefile
new file mode 100644
index 0000000..f77a359
--- /dev/null
+++ b/drivers/power/power_seq/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_POWER_SEQ) += power_seq.o
diff --git a/drivers/power/power_seq/power_seq.c b/drivers/power/power_seq/power_seq.c
new file mode 100644
index 0000000..0924a03
--- /dev/null
+++ b/drivers/power/power_seq/power_seq.c
@@ -0,0 +1,446 @@
+/*
+ * power_seq.c - A simple power sequence interpreter for platform devices
+ * and device tree.
+ *
+ * Author: Alexandre Courbot <acourbot@nvidia.com>
+ *
+ * Copyright (c) 2012 NVIDIA Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ */
+
+#include <linux/power_seq.h>
+#include <linux/module.h>
+#include <linux/err.h>
+#include <linux/device.h>
+
+#include <linux/of.h>
+
+struct power_seq_set {
+ struct device *dev;
+ struct list_head resources;
+ struct list_head sequences;
+};
+
+struct power_seq_step {
+ /* Copy of the platform data */
+ struct platform_power_seq_step pdata;
+ /* Resolved resource */
+ struct power_seq_resource *resource;
+};
+
+struct power_seq {
+ /* Set this sequence belongs to */
+ struct power_seq_set *parent_set;
+ const char *id;
+ /* To thread into power_seqs structure */
+ struct list_head list;
+ unsigned int num_steps;
+ struct power_seq_step steps[];
+};
+
+#define power_seq_err(dev, seq, step_nbr, format, ...) \
+ dev_err(dev, "%s[%d]: " format, seq->id, step_nbr, ##__VA_ARGS__);
+
+/**
+ * struct power_seq_res_type - operators for power sequences resources
+ * @name: Name of the resource type. Set to null when a resource
+ * type support is not compiled in
+ * @need_resource: Whether a resource needs to be allocated when steps of
+ * this kind are met. If set to false, res_compare and
+ * res_alloc need not be set
+ * @of_parse: Parse a step for this kind of resource from a device
+ * tree node. The result of parsing must be written into
+ * step step_nbr of seq
+ * @step_run: Run a step for this kind of resource
+ * @res_compare: Return true if the resource used by the resource is the
+ * same as the one referenced by the step, false otherwise.
+ * @res_alloc: Resolve and allocate the resource used by pstep into
+ * seq. The memory for seq is already allocated and its
+ * type member is already set when this function is called.
+ * Return error code if the resource cannot be allocated, 0
+ * if allocation went successfully.
+ */
+struct power_seq_res_ops {
+ const char *name;
+ bool need_resource;
+ int (*of_parse)(struct device *dev, struct device_node *node,
+ struct platform_power_seq *seq, unsigned int step_nbr);
+ int (*step_run)(struct power_seq_step *step);
+ bool (*res_compare)(struct power_seq_resource *res,
+ struct platform_power_seq_step *step);
+ int (*res_alloc)(struct device *dev,
+ struct platform_power_seq_step *pstep,
+ struct power_seq_resource *seq);
+};
+
+static const struct power_seq_res_ops power_seq_types[POWER_SEQ_NUM_TYPES];
+
+#ifdef CONFIG_OF
+static int of_power_seq_parse_enable_properties(struct device *dev,
+ struct device_node *node,
+ struct platform_power_seq *seq,
+ unsigned int step_nbr,
+ bool *enable)
+{
+ if (of_find_property(node, "enable", NULL)) {
+ *enable = true;
+ } else if (of_find_property(node, "disable", NULL)) {
+ *enable = false;
+ } else {
+ power_seq_err(dev, seq, step_nbr,
+ "missing enable or disable property\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int of_power_seq_parse_step(struct device *dev, struct device_node *node,
+ struct platform_power_seq *seq,
+ unsigned int step_nbr)
+{
+ struct platform_power_seq_step *step = &seq->steps[step_nbr];
+ const char *type;
+ int i, err;
+
+ err = of_property_read_string(node, "type", &type);
+ if (err < 0) {
+ power_seq_err(dev, seq, step_nbr,
+ "cannot read type property\n");
+ return err;
+ }
+ for (i = 0; i < POWER_SEQ_NUM_TYPES; i++) {
+ if (power_seq_types[i].name = NULL)
+ continue;
+ if (!strcmp(type, power_seq_types[i].name))
+ break;
+ }
+ if (i >= POWER_SEQ_NUM_TYPES) {
+ power_seq_err(dev, seq, step_nbr, "unknown type %s\n", type);
+ return -EINVAL;
+ }
+ step->type = i;
+ err = power_seq_types[step->type].of_parse(dev, node, seq, step_nbr);
+
+ return err;
+}
+
+static struct platform_power_seq *of_parse_power_seq(struct device *dev,
+ struct device_node *node)
+{
+ struct device_node *child = NULL;
+ struct platform_power_seq *pseq;
+ int num_steps, sz;
+ int err;
+
+ if (!node)
+ return ERR_PTR(-EINVAL);
+
+ num_steps = of_get_child_count(node);
+ sz = sizeof(*pseq) + sizeof(pseq->steps[0]) * num_steps;
+ pseq = devm_kzalloc(dev, sz, GFP_KERNEL);
+ if (!pseq)
+ return ERR_PTR(-ENOMEM);
+ pseq->num_steps = num_steps;
+ pseq->id = node->name;
+
+ for_each_child_of_node(node, child) {
+ unsigned int pos;
+
+ /* Check that the name's format is correct and within bounds */
+ if (strncmp("step", child->name, 4)) {
+ err = -EINVAL;
+ goto parse_error;
+ }
+
+ err = kstrtouint(child->name + 4, 10, &pos);
+ if (err < 0)
+ goto parse_error;
+
+ if (pos >= num_steps || pseq->steps[pos].type != 0) {
+ err = -EINVAL;
+ goto parse_error;
+ }
+
+ err = of_power_seq_parse_step(dev, child, pseq, pos);
+ if (err)
+ return ERR_PTR(err);
+ }
+
+ return pseq;
+
+parse_error:
+ dev_err(dev, "%s: invalid power step name %s!\n", pseq->id,
+ child->name);
+ return ERR_PTR(err);
+}
+
+/*
+ * build power sequences platform data from a device tree node.
+ *
+ * Sequences must be contained into a subnode named "power-sequences" of the
+ * device root node.
+ *
+ * Memory for the platform sequence is allocated using devm_kzalloc on dev and
+ * can be freed by devm_kfree after power_seq_set_build returned. Beware that on
+ * top of the set itself, platform data for individual sequences should also be
+ * freed.
+ *
+ * Returns the built power sequence set on success, or an error code in case of
+ * failure.
+ */
+static
+struct platform_power_seq_set *devm_of_parse_power_seq_set(struct device *dev)
+{
+ struct platform_power_seq_set *seqs;
+ struct device_node *root = dev->of_node;
+ struct device_node *seq;
+ int num_seqs, sz, i = 0;
+
+ if (!root)
+ return NULL;
+
+ root = of_find_node_by_name(root, "power-sequences");
+ if (!root)
+ return NULL;
+
+ num_seqs = of_get_child_count(root);
+ sz = sizeof(*seqs) + sizeof(seqs->seqs[0]) * num_seqs;
+ seqs = devm_kzalloc(dev, sz, GFP_KERNEL);
+ if (!seqs)
+ return ERR_PTR(-ENOMEM);
+ seqs->num_seqs = num_seqs;
+
+ for_each_child_of_node(root, seq) {
+ struct platform_power_seq *pseq;
+
+ pseq = of_parse_power_seq(dev, seq);
+ if (IS_ERR(pseq))
+ return (void *)pseq;
+
+ seqs->seqs[i++] = pseq;
+ }
+
+ return seqs;
+}
+#endif /* CONFIG_OF */
+
+static struct power_seq_resource *
+power_seq_find_resource(struct list_head *ress,
+ struct platform_power_seq_step *step)
+{
+ struct power_seq_resource *res;
+
+ list_for_each_entry(res, ress, list) {
+ if (res->type != step->type)
+ continue;
+
+ if (power_seq_types[res->type].res_compare(res, step))
+ return res;
+ }
+
+ return NULL;
+}
+
+static struct power_seq *power_seq_build_one(struct device *dev,
+ struct power_seq_set *seqs,
+ struct platform_power_seq *pseq)
+{
+ struct power_seq *seq;
+ struct power_seq_resource *res;
+ int i, err;
+
+ seq = devm_kzalloc(dev, sizeof(*seq) + sizeof(seq->steps[0]) *
+ pseq->num_steps, GFP_KERNEL);
+ if (!seq)
+ return ERR_PTR(-ENOMEM);
+
+ INIT_LIST_HEAD(&seq->list);
+ seq->parent_set = seqs;
+ seq->num_steps = pseq->num_steps;
+ seq->id = pseq->id;
+
+ for (i = 0; i < seq->num_steps; i++) {
+ struct platform_power_seq_step *pstep = &pseq->steps[i];
+ struct power_seq_step *step = &seq->steps[i];
+
+ if (pstep->type >= POWER_SEQ_NUM_TYPES ||
+ power_seq_types[pstep->type].name = NULL) {
+ power_seq_err(dev, seq, i,
+ "invalid power sequence type %d!",
+ pstep->type);
+ return ERR_PTR(-EINVAL);
+ }
+
+ memcpy(&step->pdata, pstep, sizeof(step->pdata));
+
+ /* Steps without resource need not to continue */
+ if (!power_seq_types[pstep->type].need_resource)
+ continue;
+
+ /* create resource node if not referenced already */
+ res = power_seq_find_resource(&seqs->resources, pstep);
+ if (!res) {
+ res = devm_kzalloc(dev, sizeof(*res), GFP_KERNEL);
+ if (!res)
+ return ERR_PTR(-ENOMEM);
+
+ res->type = step->pdata.type;
+
+ err = power_seq_types[res->type].res_alloc(dev,
+ &step->pdata, res);
+ if (err < 0)
+ return ERR_PTR(err);
+
+ list_add_tail(&res->list, &seqs->resources);
+ }
+ step->resource = res;
+ }
+
+ return seq;
+}
+
+/**
+ * power_seq_set_build() - build a set of runnable sequences from platform data or device tree
+ * @dev: Device that will use the power sequences. All resources will be
+ * devm-allocated against it
+ * @pseq: Platform data for the power sequences. It can be freed after
+ * this function returns. If this parameter is null, power
+ * sequences are looked up in the device tree.
+ *
+ * All memory and resources (regulators, GPIOs, etc.) are allocated using devm
+ * functions.
+ *
+ * Returns the built sequence on success, an error code in case or failure,
+ * and NULL if neither platform data or device tree nodes are defined.
+ */
+struct power_seq_set *devm_power_seq_set_build(struct device *dev,
+ struct platform_power_seq_set *pseq)
+{
+ struct power_seq_set *seqs;
+ int i;
+ bool use_dt = false;
+
+ if (!pseq) {
+ use_dt = true;
+ pseq = devm_of_parse_power_seq_set(dev);
+ }
+
+ if (!pseq)
+ return NULL;
+
+ seqs = devm_kzalloc(dev, sizeof(*seqs), GFP_KERNEL);
+
+ if (!seqs)
+ return ERR_PTR(-ENOMEM);
+
+ INIT_LIST_HEAD(&seqs->resources);
+ INIT_LIST_HEAD(&seqs->sequences);
+ for (i = 0; i < pseq->num_seqs; i++) {
+ struct power_seq *seq;
+
+ seq = power_seq_build_one(dev, seqs, pseq->seqs[i]);
+ if (IS_ERR(seq))
+ return (void *)seq;
+
+ list_add_tail(&seq->list, &seqs->sequences);
+ }
+
+ /* if we used the DT, free the temporarily built platform data */
+ if (use_dt) {
+ for (i = 0; i < pseq->num_seqs; i++)
+ devm_kfree(dev, pseq->seqs[i]);
+
+ devm_kfree(dev, pseq);
+ }
+
+ return seqs;
+}
+EXPORT_SYMBOL_GPL(devm_power_seq_set_build);
+
+/**
+ * power_seq_lookup - Lookup a power sequence by name from a set
+ * @seqs: The set to look in
+ * @id: Name to look after
+ *
+ * Returns a matching power sequence if it exists, NULL if it does not.
+ */
+struct power_seq *power_seq_lookup(struct power_seq_set *seqs, const char *id)
+{
+ struct power_seq *seq;
+
+ list_for_each_entry(seq, &seqs->sequences, list) {
+ if (!strcmp(seq->id, id))
+ return seq;
+ }
+
+ return NULL;
+}
+EXPORT_SYMBOL_GPL(power_seq_lookup);
+
+/**
+ * power_seq_set_resources - return a list of all the resources used by a set
+ * @seqs: Power sequences set we are interested in getting the resources
+ *
+ * The returned list can be parsed using the power_seq_for_each_resource macro.
+ */
+struct list_head *power_seq_set_resources(struct power_seq_set *seqs)
+{
+ return &seqs->resources;
+}
+EXPORT_SYMBOL_GPL(power_seq_set_resources);
+
+/**
+ * power_seq_run() - run a power sequence
+ * @seq: The power sequence to run
+ *
+ * Returns 0 on success, error code in case of failure.
+ */
+int power_seq_run(struct power_seq *seq)
+{
+ unsigned int i;
+ int err;
+
+ if (!seq)
+ return 0;
+
+ for (i = 0; i < seq->num_steps; i++) {
+ unsigned int type = seq->steps[i].pdata.type;
+
+ err = power_seq_types[type].step_run(&seq->steps[i]);
+ if (err) {
+ power_seq_err(seq->parent_set->dev, seq, i,
+ "error %d while running power sequence step\n",
+ err);
+ return err;
+ }
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(power_seq_run);
+
+#include "power_seq_delay.c"
+#include "power_seq_regulator.c"
+#include "power_seq_pwm.c"
+#include "power_seq_gpio.c"
+
+static const struct power_seq_res_ops power_seq_types[POWER_SEQ_NUM_TYPES] = {
+ [POWER_SEQ_DELAY] = POWER_SEQ_DELAY_TYPE,
+ [POWER_SEQ_REGULATOR] = POWER_SEQ_REGULATOR_TYPE,
+ [POWER_SEQ_PWM] = POWER_SEQ_PWM_TYPE,
+ [POWER_SEQ_GPIO] = POWER_SEQ_GPIO_TYPE,
+};
+
+MODULE_AUTHOR("Alexandre Courbot <acourbot@nvidia.com>");
+MODULE_DESCRIPTION("Runtime Interpreted Power Sequences");
+MODULE_LICENSE("GPL");
diff --git a/drivers/power/power_seq/power_seq_delay.c b/drivers/power/power_seq/power_seq_delay.c
new file mode 100644
index 0000000..ae94131
--- /dev/null
+++ b/drivers/power/power_seq/power_seq_delay.c
@@ -0,0 +1,51 @@
+/*
+ * Copyright (c) 2012 NVIDIA Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ */
+
+#include <linux/delay.h>
+
+#ifdef CONFIG_OF
+static int of_power_seq_parse_delay(struct device *dev,
+ struct device_node *node,
+ struct platform_power_seq *seq,
+ unsigned int step_nbr)
+{
+ struct platform_power_seq_step *step = &seq->steps[step_nbr];
+ int err;
+
+ err = of_property_read_u32(node, "delay",
+ &step->delay.delay);
+ if (err < 0)
+ power_seq_err(dev, seq, step_nbr,
+ "error reading delay property\n");
+
+ return err;
+}
+#else
+#define of_power_seq_parse_delay NULL
+#endif
+
+static int power_seq_step_run_delay(struct power_seq_step *step)
+{
+ usleep_range(step->pdata.delay.delay,
+ step->pdata.delay.delay + 1000);
+
+ return 0;
+}
+
+#define POWER_SEQ_DELAY_TYPE { \
+ .name = "delay", \
+ .need_resource = false, \
+ .of_parse = of_power_seq_parse_delay, \
+ .step_run = power_seq_step_run_delay, \
+}
diff --git a/drivers/power/power_seq/power_seq_gpio.c b/drivers/power/power_seq/power_seq_gpio.c
new file mode 100644
index 0000000..4f83d4c
--- /dev/null
+++ b/drivers/power/power_seq/power_seq_gpio.c
@@ -0,0 +1,91 @@
+/*
+ * Copyright (c) 2012 NVIDIA Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ */
+
+#include <linux/gpio.h>
+#include <linux/of_gpio.h>
+
+#ifdef CONFIG_OF
+static int power_seq_of_parse_gpio(struct device *dev,
+ struct device_node *node,
+ struct platform_power_seq *seq,
+ unsigned int step_nbr)
+{
+ struct platform_power_seq_step *step = &seq->steps[step_nbr];
+ int gpio;
+ int err;
+
+ gpio = of_get_named_gpio(node, "gpio", 0);
+ if (gpio < 0) {
+ power_seq_err(dev, seq, step_nbr,
+ "error reading gpio property\n");
+ return gpio;
+ }
+ step->gpio.gpio = gpio;
+
+ err = of_property_read_u32(node, "value", &step->gpio.value);
+ if (err < 0) {
+ power_seq_err(dev, seq, step_nbr,
+ "error reading value property\n");
+ } else if (step->gpio.value < 0 || step->gpio.value > 1) {
+ power_seq_err(dev, seq, step_nbr,
+ "value out of range (must be 0 or 1)\n");
+ err = -EINVAL;
+ }
+
+ return err;
+}
+#else
+#define of_power_seq_parse_gpio NULL
+#endif
+
+static bool power_seq_res_compare_gpio(struct power_seq_resource *res,
+ struct platform_power_seq_step *step)
+{
+ return res->gpio.gpio = step->gpio.gpio;
+}
+
+static int power_seq_res_alloc_gpio(struct device *dev,
+ struct platform_power_seq_step *pstep,
+ struct power_seq_resource *res)
+{
+ int err;
+
+ err = devm_gpio_request_one(dev, pstep->gpio.gpio,
+ GPIOF_OUT_INIT_LOW, dev_name(dev));
+ if (err) {
+ dev_err(dev, "cannot get gpio %d\n", pstep->gpio.gpio);
+ return err;
+ }
+
+ res->gpio.gpio = pstep->gpio.gpio;
+
+ return 0;
+}
+
+static int power_seq_step_run_gpio(struct power_seq_step *step)
+{
+ gpio_set_value_cansleep(step->resource->gpio.gpio,
+ step->pdata.gpio.value);
+
+ return 0;
+}
+
+#define POWER_SEQ_GPIO_TYPE { \
+ .name = "gpio", \
+ .need_resource = true, \
+ .of_parse = power_seq_of_parse_gpio, \
+ .step_run = power_seq_step_run_gpio, \
+ .res_compare = power_seq_res_compare_gpio, \
+ .res_alloc = power_seq_res_alloc_gpio, \
+}
diff --git a/drivers/power/power_seq/power_seq_pwm.c b/drivers/power/power_seq/power_seq_pwm.c
new file mode 100644
index 0000000..14f21e1
--- /dev/null
+++ b/drivers/power/power_seq/power_seq_pwm.c
@@ -0,0 +1,87 @@
+/*
+ * Copyright (c) 2012 NVIDIA Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ */
+
+#ifdef CONFIG_PWM
+
+#include <linux/pwm.h>
+
+#ifdef CONFIG_OF
+static int power_seq_of_parse_pwm(struct device *dev,
+ struct device_node *node,
+ struct platform_power_seq *seq,
+ unsigned int step_nbr)
+{
+ struct platform_power_seq_step *step = &seq->steps[step_nbr];
+ int err;
+
+ err = of_property_read_string(node, "id",
+ &step->pwm.id);
+ if (err) {
+ power_seq_err(dev, seq, step_nbr,
+ "error reading id property\n");
+ return err;
+ }
+
+ err = of_power_seq_parse_enable_properties(dev, node, seq, step_nbr,
+ &step->pwm.enable);
+ return err;
+}
+#else
+#define of_power_seq_parse_pwm NULL
+#endif
+
+static bool power_seq_res_compare_pwm(struct power_seq_resource *res,
+ struct platform_power_seq_step *step)
+{
+ return !strcmp(res->pwm.id, step->pwm.id);
+}
+
+static int power_seq_res_alloc_pwm(struct device *dev,
+ struct platform_power_seq_step *pstep,
+ struct power_seq_resource *res)
+{
+ res->pwm.pwm = devm_pwm_get(dev, pstep->pwm.id);
+ if (IS_ERR(res->pwm.pwm)) {
+ dev_err(dev, "cannot get pwm \"%s\"\n", pstep->pwm.id);
+ return PTR_ERR(res->pwm.pwm);
+ }
+ res->pwm.id = pstep->pwm.id;
+
+ return 0;
+}
+
+static int power_seq_step_run_pwm(struct power_seq_step *step)
+{
+ if (step->pdata.pwm.enable) {
+ return pwm_enable(step->resource->pwm.pwm);
+ } else {
+ pwm_disable(step->resource->pwm.pwm);
+ return 0;
+ }
+}
+
+#define POWER_SEQ_PWM_TYPE { \
+ .name = "pwm", \
+ .need_resource = true, \
+ .of_parse = power_seq_of_parse_pwm, \
+ .step_run = power_seq_step_run_pwm, \
+ .res_compare = power_seq_res_compare_pwm, \
+ .res_alloc = power_seq_res_alloc_pwm, \
+}
+
+#else
+
+#define POWER_SEQ_PWM_TYPE {}
+
+#endif
diff --git a/drivers/power/power_seq/power_seq_regulator.c b/drivers/power/power_seq/power_seq_regulator.c
new file mode 100644
index 0000000..2356578
--- /dev/null
+++ b/drivers/power/power_seq/power_seq_regulator.c
@@ -0,0 +1,87 @@
+/*
+ * Copyright (c) 2012 NVIDIA Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ */
+
+#ifdef CONFIG_REGULATOR
+
+#include <linux/regulator/consumer.h>
+
+#ifdef CONFIG_OF
+static int power_seq_of_parse_regulator(struct device *dev,
+ struct device_node *node,
+ struct platform_power_seq *seq,
+ unsigned int step_nbr)
+{
+ struct platform_power_seq_step *step = &seq->steps[step_nbr];
+ int err;
+
+ err = of_property_read_string(node, "id",
+ &step->regulator.id);
+ if (err) {
+ power_seq_err(dev, seq, step_nbr,
+ "error reading id property\n");
+ return err;
+ }
+
+ err = of_power_seq_parse_enable_properties(dev, node, seq, step_nbr,
+ &step->regulator.enable);
+ return err;
+}
+#else
+#define of_power_seq_parse_regulator NULL
+#endif
+
+static bool
+power_seq_res_compare_regulator(struct power_seq_resource *res,
+ struct platform_power_seq_step *step)
+{
+ return !strcmp(res->regulator.id, step->regulator.id);
+}
+
+static int power_seq_res_alloc_regulator(struct device *dev,
+ struct platform_power_seq_step *pstep,
+ struct power_seq_resource *res)
+{
+ res->regulator.regulator = devm_regulator_get(dev, pstep->regulator.id);
+ if (IS_ERR(res->regulator.regulator)) {
+ dev_err(dev, "cannot get regulator \"%s\"\n",
+ pstep->regulator.id);
+ return PTR_ERR(res->regulator.regulator);
+ }
+ res->regulator.id = pstep->regulator.id;
+
+ return 0;
+}
+
+static int power_seq_step_run_regulator(struct power_seq_step *step)
+{
+ if (step->pdata.regulator.enable)
+ return regulator_enable(step->resource->regulator.regulator);
+ else
+ return regulator_disable(step->resource->regulator.regulator);
+}
+
+#define POWER_SEQ_REGULATOR_TYPE { \
+ .name = "regulator", \
+ .need_resource = true, \
+ .of_parse = power_seq_of_parse_regulator, \
+ .step_run = power_seq_step_run_regulator, \
+ .res_compare = power_seq_res_compare_regulator, \
+ .res_alloc = power_seq_res_alloc_regulator, \
+}
+
+#else
+
+#define POWER_SEQ_REGULATOR_TYPE {}
+
+#endif
diff --git a/include/linux/power_seq.h b/include/linux/power_seq.h
new file mode 100644
index 0000000..437a61a
--- /dev/null
+++ b/include/linux/power_seq.h
@@ -0,0 +1,172 @@
+/*
+ * power_seq.h
+ *
+ * Simple interpreter for defining power sequences as platform data or device
+ * tree properties.
+ *
+ * Power sequences are designed to replace the callbacks typically used in
+ * board-specific files that implement board-specific power sequences of devices
+ * such as backlights. A power sequence is an array of resources (which can a
+ * regulator, a GPIO, a PWM, ...) with an action to perform on it (enable or
+ * disable) and optional pre and post step delays. By having them interpreted
+ * instead of arbitrarily executed, it is possible to describe these in the
+ * device tree and thus remove board-specific code from the kernel.
+ *
+ * Author: Alexandre Courbot <acourbot@nvidia.com>
+ *
+ * Copyright (c) 2012 NVIDIA Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ */
+
+#ifndef __LINUX_POWER_SEQ_H
+#define __LINUX_POWER_SEQ_H
+
+#include <linux/types.h>
+#include <net/irda/parameters.h>
+
+struct device;
+struct regulator;
+struct pwm_device;
+struct device_node;
+
+/**
+ * The different kinds of resources that can be controlled during the sequences.
+ */
+enum power_seq_res_type {
+ POWER_SEQ_DELAY,
+ POWER_SEQ_REGULATOR,
+ POWER_SEQ_PWM,
+ POWER_SEQ_GPIO,
+ POWER_SEQ_NUM_TYPES,
+};
+
+/**
+ * struct platform_power_seq_delay_step - platform data for delay steps
+ * @delay: Amount of time to wait, in microseconds.
+ */
+struct platform_power_seq_delay_step {
+ unsigned int delay;
+};
+
+/**
+ * struct platform_power_seq_regulator_step - platform data for regulator steps
+ * @id: Name of the regulator to use. The regulator will be obtained
+ * using devm_regulator_get(dev, name)
+ * @enable: Whether to enable or disable the regulator during this step
+ */
+struct platform_power_seq_regulator_step {
+ const char *id;
+ bool enable;
+};
+
+/**
+ * struct platform_power_seq_pwm_step - platform data for PWM steps
+ * @id: Name of the pwm to use. The PWM will be obtained using
+ * devm_pwm_get(dev, name)
+ * @enable: Whether to enable or disable the PWM during this step
+ */
+struct platform_power_seq_pwm_step {
+ const char *id;
+ bool enable;
+};
+
+/**
+ * struct platform_power_seq_gpio_step - platform data for GPIO steps
+ * @gpio: Number of the GPIO to use. The GPIO will be obtained using
+ * devm_gpio_request_one(dev, number)
+ * @enable: Whether to enable or disable the GPIO during this step
+ */
+struct platform_power_seq_gpio_step {
+ int gpio;
+ int value;
+};
+
+/**
+ * struct platform_power_seq_step - platform data for power sequences steps
+ * @type: The type of this step. This decides which member of the union is
+ * valid for this step.
+ * @delay: Used if @type = POWER_SEQ_DELAY
+ * @regulator: Used if @type = POWER_SEQ_REGULATOR
+ * @pwm: Used if @type = POWER_SEQ_PWN
+ * @gpio: Used if @type = POWER_SEQ_GPIO
+ */
+struct platform_power_seq_step {
+ enum power_seq_res_type type;
+ union {
+ struct platform_power_seq_delay_step delay;
+ struct platform_power_seq_regulator_step regulator;
+ struct platform_power_seq_pwm_step pwm;
+ struct platform_power_seq_gpio_step gpio;
+ };
+};
+
+/**
+ * struct platform_power_seq - platform data for power sequences
+ * @id: Name through which this sequence is refered
+ * @num_steps: Number of steps in that sequence
+ * @steps: Array of num_steps steps describing the sequence
+ */
+struct platform_power_seq {
+ const char *id;
+ unsigned int num_steps;
+ struct platform_power_seq_step steps[];
+};
+
+/**
+ * struct platform_power_seq_set - platform data for sets of sequences
+ * @num_seqs: Number of sequences in this set
+ * @seqs: Array of pointers to individual sequences
+ */
+struct platform_power_seq_set {
+ unsigned int num_seqs;
+ struct platform_power_seq *seqs[];
+};
+
+/**
+ * struct power_seq_resource - resource used by a power sequence set
+ * @pdata: Pointer to the platform data used to resolve this resource
+ * @regulator: Resolved regulator if of type POWER_SEQ_REGULATOR
+ * @pwm: Resolved PWM if of type POWER_SEQ_PWM
+ * @list: Used to link resources together
+ */
+struct power_seq_resource {
+ enum power_seq_res_type type;
+ /* resolved resource and identifier */
+ union {
+ struct {
+ struct regulator *regulator;
+ const char *id;
+ } regulator;
+ struct {
+ struct pwm_device *pwm;
+ const char *id;
+ } pwm;
+ struct {
+ int gpio;
+ } gpio;
+ };
+ struct list_head list;
+};
+#define power_seq_for_each_resource(pos, seqs) \
+ list_for_each_entry(pos, power_seq_set_resources(seqs), list)
+
+struct power_seq_resource;
+struct power_seq;
+struct power_seq_set;
+
+struct power_seq_set *devm_power_seq_set_build(struct device *dev,
+ struct platform_power_seq_set *pseq);
+struct list_head *power_seq_set_resources(struct power_seq_set *seqs);
+struct power_seq *power_seq_lookup(struct power_seq_set *seqs, const char *id);
+int power_seq_run(struct power_seq *seq);
+
+#endif
--
1.7.12
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox