* [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
* 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
* 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 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 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 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: [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 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: [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 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
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