Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* 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 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 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: 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 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 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 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: 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 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: 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 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 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  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: Alex Courbot @ 2012-09-13  7: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-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 Thursday 13 September 2012 14:54:09 Tomi Valkeinen wrote:
> * PGP Signed by an unknown key
> 
> 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.

I took care of that when naming the feature - it is not a "sequence" anymore 
if you have conditionals. :P

> 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 shall be satisfied the day the kernel is released as one big DT node along 
with the 5KB interpreter that runs it.

Alex.


^ permalink raw reply

* Re: [PATCH v6 1/4] Runtime Interpreted Power Sequences
From: Sascha Hauer @ 2012-09-13  7:18 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: <1347519807.7471.45.camel@lappyti>

On Thu, Sep 13, 2012 at 10:03:27AM +0300, Tomi Valkeinen wrote:
> 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.

These memory writes can be avoided when these registers are abstracted
as a regular gpio/regulator/pwm driver.

> > > 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?

Sure, tons of. One board needs a gpio to be set high to enable backlight,
the next one to low, a regulator has to be enabled, and to avoid
flickering a certain timing has to be ensured. This is all highly board
specific.

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 0/4] Runtime Interpreted Power Sequences
From: Mark Brown @ 2012-09-13  7:19 UTC (permalink / raw)
  To: Alex Courbot
  Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	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, Tomi Valkeinen,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	David Woodhouse,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
In-Reply-To: <4473898.CeAQBgUhKL@percival>

On Thu, Sep 13, 2012 at 03:42:11PM +0900, Alex Courbot wrote:
> On Thursday 13 September 2012 14:25:53 Mark Brown wrote:

> > 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.

Right, but the sequencing for enabling them is currently open coded in
each driver.

^ permalink raw reply

* Re: [PATCH v6 1/4] Runtime Interpreted Power Sequences
From: Alex Courbot @ 2012-09-13  7:21 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Sascha Hauer, 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: <1347519807.7471.45.camel@lappyti>

On Thursday 13 September 2012 15:03:27 Tomi Valkeinen wrote:
> * PGP Signed by an unknown key
> 
> 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.

I think the confusion comes from the fact that in practice, people just wrote 
their hooks into the board files instead of writing more "specialized" drivers 
(which I agree would have been the correct way of doing). That is why hooks 
like those of the pwm_backlight driver were "board specific code" to me too.

Alex.


^ permalink raw reply

* Re: [PATCH v6 0/4] Runtime Interpreted Power Sequences
From: Alex Courbot @ 2012-09-13  7:26 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: <20120913071928.GA20959@opensource.wolfsonmicro.com>

On Thursday 13 September 2012 15:19:30 Mark Brown wrote:
> On Thu, Sep 13, 2012 at 03:42:11PM +0900, Alex Courbot wrote:
> > On Thursday 13 September 2012 14:25:53 Mark Brown wrote:
> > > 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.
> 
> Right, but the sequencing for enabling them is currently open coded in
> each driver.

Mmm then I'm afraid I don't see what you wanted to say initially - could you 
elaborate?

Alex.


^ permalink raw reply

* Re: [PATCH v6 1/4] Runtime Interpreted Power Sequences
From: Tomi Valkeinen @ 2012-09-13  7:27 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: <20120913071829.GE6180-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 2351 bytes --]

On Thu, 2012-09-13 at 09:18 +0200, Sascha Hauer wrote:
> On Thu, Sep 13, 2012 at 10:03:27AM +0300, Tomi Valkeinen wrote:
> > 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.
> 
> These memory writes can be avoided when these registers are abstracted
> as a regular gpio/regulator/pwm driver.

Only if they are gpios/regulators/pwms. Yes, I agree most of the
possible things to configure would be among those (or perhaps
pinmuxing). But there's always the odd one that's not one of those.

> > Do you have examples of board specific power sequences or such?
> 
> Sure, tons of. One board needs a gpio to be set high to enable backlight,
> the next one to low, a regulator has to be enabled, and to avoid
> flickering a certain timing has to be ensured. This is all highly board
> specific.

Okay. In my experience these have always been device specific. In the
case of backlight, the backlight device requires one gpio to be set
high, other one low, etc.

Can you share a bit more what kind of HW configuration you have that
requires this? The backlight is not a single piece of HW added to the
board (or embedded into a panel module), but consists of multiple HW
blocks integrated in a custom way to the board?

 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: Thierry Reding @ 2012-09-13  7:29 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Stephen Warren, linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Leela Krishna Amudala, Sascha Hauer, Mark Brown,
	linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring,
	Anton Vorontsov,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	David Woodhouse
In-Reply-To: <1347519807.7471.45.camel@lappyti>

[-- Attachment #1: Type: text/plain, Size: 3225 bytes --]

On Thu, Sep 13, 2012 at 10:03:27AM +0300, Tomi Valkeinen wrote:
> 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?

It is true that most (perhaps all) power sequences can be associated
with a specific device, but if we go and implement drivers for these
kinds of devices we will probably end up with loads of variations of
the same scheme.

Lets take display panels as an example. One of the devices that we build
has gone through two generations so far and both are slightly different
in how they control the panel backlight: one has an external backlight
controller, the other has the display controller built into the panel.
However, from the board's perspective the control of the backlight
doesn't change, because both devices get the same inputs (an enable pin
and a PWM) that map to the same pins on the SoC.

This may not be a very good example because the timing isn't relevant,
but the basic point is still valid: if we provide a driver for both
panel devices, the code will be exactly the same. So we end up having to
refactor to avoid code duplication and use the same driver for a number
of backlight/panel combinations. Which in itself isn't very bad, but it
also means that we'll probably get to see a large number of "generic"
drivers which aren't very generic after all.

Another problem, which also applies to the case of power-sequences, is
that often the panel and backlight are not the same device. So you could
have the same panel with any number of different backlight controllers
or vice-versa any number of different panels with the same backlight
controller.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH v6 0/4] Runtime Interpreted Power Sequences
From: Mark Brown @ 2012-09-13  7:29 UTC (permalink / raw)
  To: Alex Courbot
  Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	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, Tomi Valkeinen,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	David Woodhouse,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
In-Reply-To: <1378218.yLeQheNFT0@percival>

On Thu, Sep 13, 2012 at 04:26:34PM +0900, Alex Courbot wrote:
> On Thursday 13 September 2012 15:19:30 Mark Brown wrote:
> > > On Thursday 13 September 2012 14:25:53 Mark Brown wrote:
> > > > 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.

> > Right, but the sequencing for enabling them is currently open coded in
> > each driver.

> Mmm then I'm afraid I don't see what you wanted to say initially - could you 
> elaborate?

The driver knows the power sequence.  Having to type the same sequence
into the DT or platform data for each board using the device wouuld be
retarded so we need the drivers to be able to give the sequence to the
library if they're going to be able to reuse it (which is a lot of what
Tomi is talking about).

^ permalink raw reply

* Re: [PATCH v6 1/4] Runtime Interpreted Power Sequences
From: Sascha Hauer @ 2012-09-13  7:50 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Tomi Valkeinen, Alex Courbot, Stephen Warren, 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: <20120913072920.GA11459@avionic-0098.mockup.avionic-design.de>

On Thu, Sep 13, 2012 at 09:29:20AM +0200, Thierry Reding wrote:
> On Thu, Sep 13, 2012 at 10:03:27AM +0300, Tomi Valkeinen wrote:
> > 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?
> 
> It is true that most (perhaps all) power sequences can be associated
> with a specific device, but if we go and implement drivers for these
> kinds of devices we will probably end up with loads of variations of
> the same scheme.
> 
> Lets take display panels as an example. One of the devices that we build
> has gone through two generations so far and both are slightly different
> in how they control the panel backlight: one has an external backlight
> controller, the other has the display controller built into the panel.
> However, from the board's perspective the control of the backlight
> doesn't change, because both devices get the same inputs (an enable pin
> and a PWM) that map to the same pins on the SoC.
> 
> This may not be a very good example because the timing isn't relevant,
> but the basic point is still valid: if we provide a driver for both
> panel devices, the code will be exactly the same. So we end up having to
> refactor to avoid code duplication and use the same driver for a number
> of backlight/panel combinations. Which in itself isn't very bad, but it
> also means that we'll probably get to see a large number of "generic"
> drivers which aren't very generic after all.
> 
> Another problem, which also applies to the case of power-sequences, is
> that often the panel and backlight are not the same device.

Maybe that is the problem that needs to be addressed? They *are* not the
same device, still they are handled in a single platform callback (or
now power sequence). Maybe the amount of combinations dastrically go
down if we really make them two devices.

Most of our panels have:

- A regulator (or gpio) for turning them on

And the backlights have:

- A regulator (or gpio) for turning them on
- A PWM for controlling brightness.

The power sequence for the above is clear: Turn on the panel the panel,
wait until it stabilized and afterwards turn on the backlight.

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  8:00 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Sascha Hauer, Alex Courbot, Stephen Warren, 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: <20120913072920.GA11459-RM9K5IK7kjIQXX3q8xo1gnVAuStQJXxyR5q1nwbD4aMs9pC9oP6+/A@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 3955 bytes --]

On Thu, 2012-09-13 at 09:29 +0200, Thierry Reding wrote:
> On Thu, Sep 13, 2012 at 10:03:27AM +0300, Tomi Valkeinen wrote:
> > 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?
> 
> It is true that most (perhaps all) power sequences can be associated
> with a specific device, but if we go and implement drivers for these
> kinds of devices we will probably end up with loads of variations of
> the same scheme.
> 
> Lets take display panels as an example. One of the devices that we build
> has gone through two generations so far and both are slightly different
> in how they control the panel backlight: one has an external backlight
> controller, the other has the display controller built into the panel.
> However, from the board's perspective the control of the backlight
> doesn't change, because both devices get the same inputs (an enable pin
> and a PWM) that map to the same pins on the SoC.

We had something a bit similar in Nokia. First versions had an
"independent" backlight controlled via pwm. Later versions had a
backlight that is controlled by the panel IP, so it was changed by
sending DSI commands to the panel.

> This may not be a very good example because the timing isn't relevant,
> but the basic point is still valid: if we provide a driver for both
> panel devices, the code will be exactly the same. So we end up having to
> refactor to avoid code duplication and use the same driver for a number
> of backlight/panel combinations. Which in itself isn't very bad, but it
> also means that we'll probably get to see a large number of "generic"
> drivers which aren't very generic after all.
> 
> Another problem, which also applies to the case of power-sequences, is
> that often the panel and backlight are not the same device. So you could
> have the same panel with any number of different backlight controllers
> or vice-versa any number of different panels with the same backlight
> controller.

Yes, I think the backlight and the panel should be considered separate
devices. Just like, say, a touch screen and a panel may happen to be in
the same display module, a backlight and a panel can be in the same
display module. They are still separate, independent things, although
they are, of course, used together.

 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: Mark Brown @ 2012-09-13  8:09 UTC (permalink / raw)
  To: Alexandre Courbot
  Cc: Stephen Warren, Thierry Reding, Simon Glass, Grant Likely,
	Rob Herring, 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 Wed, Sep 12, 2012 at 06:57:44PM +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.

It does make me a little sad that the DT bindings need to specify the
number of steps but otherwise this looks good (modulo the minor comments
Stephen had as well):

Reviewed-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

I think regardless of the current discussion about some of the
applications (like pwm-backlight) there are going to be cases where this
is useful even if it ends up being more as library code for drivers than 
as something that users work with directly.

^ permalink raw reply

* Re: [PATCH v6 1/4] Runtime Interpreted Power Sequences
From: Alex Courbot @ 2012-09-13  8:21 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Thierry Reding, Tomi Valkeinen, Stephen Warren, 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: <20120913075037.GG6180@pengutronix.de>

On Thursday 13 September 2012 15:50:37 Sascha Hauer wrote:
> On Thu, Sep 13, 2012 at 09:29:20AM +0200, Thierry Reding wrote:
> > On Thu, Sep 13, 2012 at 10:03:27AM +0300, Tomi Valkeinen wrote:
> > > 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?
> > 
> > It is true that most (perhaps all) power sequences can be associated
> > with a specific device, but if we go and implement drivers for these
> > kinds of devices we will probably end up with loads of variations of
> > the same scheme.
> > 
> > Lets take display panels as an example. One of the devices that we build
> > has gone through two generations so far and both are slightly different
> > in how they control the panel backlight: one has an external backlight
> > controller, the other has the display controller built into the panel.
> > However, from the board's perspective the control of the backlight
> > doesn't change, because both devices get the same inputs (an enable pin
> > and a PWM) that map to the same pins on the SoC.
> > 
> > This may not be a very good example because the timing isn't relevant,
> > but the basic point is still valid: if we provide a driver for both
> > panel devices, the code will be exactly the same. So we end up having to
> > refactor to avoid code duplication and use the same driver for a number
> > of backlight/panel combinations. Which in itself isn't very bad, but it
> > also means that we'll probably get to see a large number of "generic"
> > drivers which aren't very generic after all.
> > 
> > Another problem, which also applies to the case of power-sequences, is
> > that often the panel and backlight are not the same device.
> 
> Maybe that is the problem that needs to be addressed? They *are* not the
> same device, still they are handled in a single platform callback (or
> now power sequence). Maybe the amount of combinations dastrically go
> down if we really make them two devices.
> 
> Most of our panels have:
> 
> - A regulator (or gpio) for turning them on
> 
> And the backlights have:
> 
> - A regulator (or gpio) for turning them on
> - A PWM for controlling brightness.
> 
> The power sequence for the above is clear: Turn on the panel the panel,
> wait until it stabilized and afterwards turn on the backlight.

Actually the sequence I submitted in this patchset only takes care of the 
backlight device (the panel - or LCD - should have its own). The regulator 
controls the power supply, the PWM the intensity, and on top of that it also 
has an enable GPIO. These 3 resources are exclusively for the LED - the LCD 
uses other ones. So as of now it seems that the LCD/backlight separation is 
effective and the resources needed are not so uniform across backlights (not 
even mentioning the delays).

The LCD's power sequence is even weirder - VDD must take at least 0.5ms for 
going from 10% to 90% of its power, you must wait 400ms after switching it off 
before switching it on again, and you should also transmit data for 200ms 
before switching the backlight's LED on (using its own sequence). That last 
point is interesting since it somehow makes the LCD and LED dependent on each 
other - on an unrelated note, this might be something to consider in Laurent's 
proposal for a panel framework.

Alex.


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox