* Re: Device tree binding for DVFS table
From: Peter De Schrijver @ 2012-07-13 10:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20120712171016.GL2772@gmail.com>
Hi Mike,
>
> Peter,
>
> I agree with your observations in general, but I think some specificity
> is needed:
>
> > + frequency/voltage relationships
>
> We should be clear that the voltage does NOT belong to the clock, but to
> the device/module/IP block that consumes that clock. This is an
> important detail since it means that a clock does not have a
> corresponding table of voltages (e.g. one table per clock), but instead
> a device has a table of voltages corresponding to each clock.
>
Or the other way around, a table of clock frequencies, 1 for each voltage.
> This is very necessary when a single clock drives multiple devices which
> are driven by separate voltage rails.
>
Ah ok. How does this work in practice? A device can only run at a given clock
rate if all the rails are at a certain voltage?
> > + power rail constraints (eg voltage difference limit between 2 rails)
>
> This should come from regulator DT data and not anything DVFS-specific,
> correct?
>
That's true. I think it can even be open-coded as this is a SoC internal
thing. All boards using this SoC will have the same limits, so I see little
reason to move this info to DT.
> > + clock constraints (eg. clock x frequency must be a fixed ratio of clock y
> > frequency)
>
> Yeah, after sending my email above yesterday I instantly regretted it.
> It is true that *functional* clock dependencies are really the purview
> of the device driver. E.g. for Device X to operate at FAST_SPEED, scale
> functional_clk up to 200MHz and l3_ddr_clk up to 100MHz. On OMAP our
> display subsystem block also has clock ratio rules that must be honored,
> but it just open-coded.
>
> It is possible to model those in DT if we really want, but shouldn't be
> a priority for these dvfs-specific bindings.
>
Agreed.
Cheers,
Peter.
^ permalink raw reply
* Re: [RFC][PATCH V2 3/3] tegra: add pwm backlight device tree nodes
From: Alex Courbot @ 2012-07-13 5:32 UTC (permalink / raw)
To: Simon Glass
Cc: Thierry Reding, linux-tegra@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-fbdev@vger.kernel.org,
devicetree-discuss@lists.ozlabs.org
In-Reply-To: <CAPnjgZ2=Ga2P-40trA_=UABm=i7Uy4NTAM=ehea11mE22VfDew@mail.gmail.com>
On 07/12/2012 11:27 PM, Simon Glass wrote
>> I agree the type strings are a problem in the current form - if we could get
>> constants in the device tree, that would be much better. Your way of
>> representing the sequences is interesting though, if we can solve the type
>> issue (and also evaluate its cost in terms of memory footprint), it would be
>> interesting to consider it as well.
>
> At a guess:
>
>>>> + power-on-sequence = "REGULATOR", "power", <1>,
>>>> + "DELAY", <10>,
>>>> + "PWM", "backlight", <1>,
>>>> + "GPIO", "enable", <1>;
>
> About 106 bytes I think
>
>>> step@0 { 16
> type = "regulator"; 24
>>> phandle = <&backlight_reg>; 16
>>> enable = <1>; 16
>>> post-delay = <10>; 16
>>> }
>>> step@1 { 16
> type = "pwm"; 16
>>> phandle = <&pwm 2 5000000>; 24
>>> }
>>> step@2 { 16
> type = "gpio"; 20
>>> phandle = <&gpio 28 0>; 24
>>> enable = <1>; 16
>>> }
>
> 220?
I compiled both versions to try it out. Your version was just 50 bytes
larger than mine (I assumed that with yours, we would be able to remove
the top-level pwm/regulator/gpio definitions that are referred by the
sequence). The question here is do we want to have something more
DT-ish, or are we trying to save every possible byte in the DT structure?
As Thierry also mentionned, we are trying to provide the same feature
using the platform interface. I am not sure how we can elegantly support
both ways through this.
> From my understanding mixing strings and numbers in a property is
> frowned on though.
But doesn't it make sense in the current case? The power sequence is
basically a program that is run by an interpreter. From this
perspective, it makes more sense to me to have it as a binary field
rather than a hierarchy of nodes and properties that will be harder to
parse and will make error detection more complicated. I don't really see
any practical benefit from turning the steps into sub-nodes, but then
again I am not so familiar with the DT.
Alex.
^ permalink raw reply
* Re: Device tree binding for DVFS table
From: Mark Brown @ 2012-07-12 17:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20120712171016.GL2772@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1266 bytes --]
On Thu, Jul 12, 2012 at 10:10:16AM -0700, Mike Turquette wrote:
> On 20120712-17:10, Peter De Schrijver wrote:
> > + power rail constraints (eg voltage difference limit between 2 rails)
> This should come from regulator DT data and not anything DVFS-specific,
> correct?
Currently the regulator API will only constrain individual rails (and
shortly likely parent/child relationships). It will not do anything to
constrain between different rails, this has been left to the consumers.
For this stuff it may still make sense to do that if we've got a set of
operating points defined which apply over multiple inputs (like clock
and frequency) - we'll naturally impose these constraints by virtue of
selecting the operating point.
> > + clock constraints (eg. clock x frequency must be a fixed ratio of clock y
> > frequency)
> Yeah, after sending my email above yesterday I instantly regretted it.
> It is true that *functional* clock dependencies are really the purview
> of the device driver. E.g. for Device X to operate at FAST_SPEED, scale
> functional_clk up to 200MHz and l3_ddr_clk up to 100MHz. On OMAP our
> display subsystem block also has clock ratio rules that must be honored,
> but it just open-coded.
This is similar to the regulator thing.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: Device tree binding for DVFS table
From: Mike Turquette @ 2012-07-12 17:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20120712141001.GE9437@tbergstrom-lnx.Nvidia.com>
On 20120712-17:10, Peter De Schrijver wrote:
> On Wed, Jul 11, 2012 at 10:04:02PM +0200, Mike Turquette wrote:
> > On 20120711-15:44, Mark Brown wrote:
> > > On Wed, Jul 11, 2012 at 09:03:41AM -0500, Rob Herring wrote:
> > >
> > > > I'd expect a single property with freq/volt pairs or 2 properties for
> > > > freq and voltage where there is a 1:1 relationship (freq N uses voltage N).
> > >
> > > I strongly agree - the current proposal is very hard to read due to the
> > > separation between the voltage and frequency values. Some devices do
> > > also need to scale multiple rails together, especially when this gets
> > > used for I/O devices.
> > >
> > > I'd also expect to see a range of voltages for each frequency rather
> > > than a specific voltage; usually things are at least characterised with
> > > a specified tolerance.
> >
> > Not only should we support multiple voltage rails but also multiple
> > clocks. For some devices a DVFS transition is composed of scaling
> > multiple clock rates together. So some sort of clock identifier
> > (phandle?) is needed as well. (forgive my ignorance on the phandle
> > part, as I am a DT noob)
>
> I would say this constraint should be expressed in a seperate DT node. In
> short I think we have 3 things to model:
>
Peter,
I agree with your observations in general, but I think some specificity
is needed:
> + frequency/voltage relationships
We should be clear that the voltage does NOT belong to the clock, but to
the device/module/IP block that consumes that clock. This is an
important detail since it means that a clock does not have a
corresponding table of voltages (e.g. one table per clock), but instead
a device has a table of voltages corresponding to each clock.
This is very necessary when a single clock drives multiple devices which
are driven by separate voltage rails.
> + power rail constraints (eg voltage difference limit between 2 rails)
This should come from regulator DT data and not anything DVFS-specific,
correct?
> + clock constraints (eg. clock x frequency must be a fixed ratio of clock y
> frequency)
Yeah, after sending my email above yesterday I instantly regretted it.
It is true that *functional* clock dependencies are really the purview
of the device driver. E.g. for Device X to operate at FAST_SPEED, scale
functional_clk up to 200MHz and l3_ddr_clk up to 100MHz. On OMAP our
display subsystem block also has clock ratio rules that must be honored,
but it just open-coded.
It is possible to model those in DT if we really want, but shouldn't be
a priority for these dvfs-specific bindings.
Regards,
Mike
>
> Cheers,
>
> Peter.
^ permalink raw reply
* Re: Device tree binding for DVFS table
From: Mike Turquette @ 2012-07-12 17:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4FFE4FE6.20109@nvidia.com>
On 20120712-09:47, Prashant Gaikwad wrote:
> On Wednesday 11 July 2012 08:14 PM, Mark Brown wrote:
> >On Wed, Jul 11, 2012 at 09:03:41AM -0500, Rob Herring wrote:
> >
> >>I'd expect a single property with freq/volt pairs or 2 properties for
> >>freq and voltage where there is a 1:1 relationship (freq N uses voltage N).
> >I strongly agree - the current proposal is very hard to read due to the
> >separation between the voltage and frequency values. Some devices do
> >also need to scale multiple rails together, especially when this gets
> >used for I/O devices.
>
> Multiple rails for single clock or would it be multiple clocks?
>
Voltage rails do not correspond to clocks. They correspond to the IP
block in question (e.g. the "module" or "device" that consumes clocks
and voltage).
Regards,
Mike
^ permalink raw reply
* Re: Device tree binding for DVFS table
From: Mark Brown @ 2012-07-12 15:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4FFE4FE6.20109@nvidia.com>
[-- Attachment #1: Type: text/plain, Size: 472 bytes --]
On Thu, Jul 12, 2012 at 09:47:42AM +0530, Prashant Gaikwad wrote:
> On Wednesday 11 July 2012 08:14 PM, Mark Brown wrote:
> >I'd also expect to see a range of voltages for each frequency rather
> >than a specific voltage; usually things are at least characterised with
> >a specified tolerance.
> But then how will you determine the operating voltage for a frequency?
By passing the range into the regulator API - the regulator API always
takes a voltage range anyway.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [RFC][PATCH V2 3/3] tegra: add pwm backlight device tree nodes
From: Simon Glass @ 2012-07-12 14:27 UTC (permalink / raw)
To: Alex Courbot
Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
In-Reply-To: <4FFEA2D4.9050308-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Hi Alex,
On Thu, Jul 12, 2012 at 12:11 PM, Alex Courbot <acourbot@nvidia.com> wrote:
> Hi Simon,
>
>
> On Thu 12 Jul 2012 06:37:33 PM JST, Simon Glass wrote:
>>
>> I would like to do something similar in U-Boot for Tegra, although
>> perhaps not right away. For now I will go with something considerably
>> simpler! But if this is merged into the kernel we will move to it in
>> U-Boot.
>
>
> Cool, I'd love to see that used in U-Boot as well.
>
>
>>> + default-brightness-level = <12>;
>>> +
>>> + pwms = <&pwm 2 5000000>;
>>> + pwm-names = "backlight";
>>> + power-supply = <&backlight_reg>;
>>> + enable-gpios = <&gpio 28 0>;
>>> +
>>> + power-on-sequence = "REGULATOR", "power", <1>,
>>> + "DELAY", <10>,
>>> + "PWM", "backlight", <1>,
>>> + "GPIO", "enable", <1>;
>>
>>
>> So the names REGULATOR, DELAY, etc. here are looked up through some
>> existing mechanism? In general I am not a big fan of mixing strings
>> and numbers in a property.
>
>
> Yes, these are strings we are looking up to know the type of the next
> element to power on/off in the sequence. I don't like these, honestly, and
> would rather have them replaced by constants - however there is no way to
> define constants in the DT AFAIK (but I have heard some other persons are
> interested in having them too), and this is the only way I have found to
> keep the sequence readable.
That might be the 100th time that I have heard this. I have brought a
patch from Stephen Warren into our tree to solve this locally, but it
is not merged upstream.
>
>
>> Maybe something like:
>>
>> power_on_sequence {
>> step@0 {
>> phandle = <&backlight_reg>;
>> enable = <1>;
>> post-delay = <10>;
>> }
>> step@1 {
>> phandle = <&pwm 2 5000000>;
>> }
>> step@2 {
>> phandle = <&gpio 28 0>;
>> enable = <1>;
>> }
>
>
> I see a few problems with this: first, how do you know the types of your
> phandles? At step 0, we should get a regulator, step 1 is a PWM and step 2
> is a GPIO. This is why I have used strings to identify the type of the
> phandle and the number (and type) of additional arguments to parse for a
> step.
Well it's similar to giving them names - you would need to look up the
phandle and see its compatible string or which driver type owns it.
Maybe too complicated, and no such infrastructure exists, so:
>> power_on_sequence {
>> step@0 {
type = "regulator";
>> phandle = <&backlight_reg>;
>> enable = <1>;
>> post-delay = <10>;
>> }
>> step@1 {
type = "pwm";
>> phandle = <&pwm 2 5000000>;
>> }
>> step@2 {
type = "gpio";
>> phandle = <&gpio 28 0>;
>> enable = <1>;
>> }
>
> Second, I am afraid the representation in memory would be significantly
> bigger since the properties names would have to be stored as well (I am no
> DT expert, please correct me if I am wrong). Lastly, the additional freedom
> of having extra properties might make the parser more complicated.
The property names are only stored one, in the string table. I am not
sure about parsing complexity, but it might actually be easier.
>
> I agree the type strings are a problem in the current form - if we could get
> constants in the device tree, that would be much better. Your way of
> representing the sequences is interesting though, if we can solve the type
> issue (and also evaluate its cost in terms of memory footprint), it would be
> interesting to consider it as well.
At a guess:
>>> + power-on-sequence = "REGULATOR", "power", <1>,
>>> + "DELAY", <10>,
>>> + "PWM", "backlight", <1>,
>>> + "GPIO", "enable", <1>;
About 106 bytes I think
>> step@0 { 16
type = "regulator"; 24
>> phandle = <&backlight_reg>; 16
>> enable = <1>; 16
>> post-delay = <10>; 16
>> }
>> step@1 { 16
type = "pwm"; 16
>> phandle = <&pwm 2 5000000>; 24
>> }
>> step@2 { 16
type = "gpio"; 20
>> phandle = <&gpio 28 0>; 24
>> enable = <1>; 16
>> }
220?
From my understanding mixing strings and numbers in a property is
frowned on though.
>
> Alex.
Regards,
Simon
^ permalink raw reply
* RE: [PATCH 1/2] video: da8xx-fb: configure FIFO threshold to reduce underflow errors
From: Manjunathappa, Prakash @ 2012-07-12 14:13 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <4FFECFCE.2000308@mvista.com>
Hi Sergei,
On Thu, Jul 12, 2012 at 18:53:26, Sergei Shtylyov wrote:
> Hello.
>
> On 11-07-2012 19:14, Manjunathappa, Prakash wrote:
>
> > Patch works around the below silicon errata:
> > During LCDC initialization, there is the potential for a FIFO
> > underflow condition to occur. A FIFO underflow condition
> > occurs when the input FIFO is completely empty and the LCDC
> > raster controller logic that drives data to the output pins
> > attempts to fetch data from the FIFO. When a FIFO underflow
> > condition occurs, incorrect data will be driven out on the
> > LCDC data pins.
>
> > Software should poll the FUF bit field in the LCD_STAT register
> > to check if an error condition has occurred or service the
> > interrupt if FUF_EN is enabled when FUF occurs. If the FUF bit
> > field has been set to 1, this will indicate an underflow
> > condition has occurred and then the software should execute a
> > reset of the LCDC via the LPSC.
>
> > This problem may occur if the LCDC FIFO threshold size
> > (LCDDMA_CTRL[TH_FIFO_READY]) is left at its default value after
> > reset. Increasing the FIFO threshold size will reduce or
> > eliminate underflows. Setting the threshold size to 256 double
> > words or larger is recommended.
>
> > Above issue is described in section 2.1.3 of silicon errata
> > http://www.ti.com/lit/er/sprz313e/sprz313e.pdf
>
> > Signed-off-by: Rajashekhara, Sudhakar <sudhakar.raj@ti.com>
> > Signed-off-by: Manjunathappa, Prakash <prakash.pm@ti.com>
> > ---
> > drivers/video/da8xx-fb.c | 15 +++++++++++----
> > include/video/da8xx-fb.h | 3 +++
> > 2 files changed, 14 insertions(+), 4 deletions(-)
>
> > diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
> > index 47118c7..2010dd7 100644
> > --- a/drivers/video/da8xx-fb.c
> > +++ b/drivers/video/da8xx-fb.c
> > @@ -344,8 +344,8 @@ static void lcd_blit(int load_mode, struct da8xx_fb_par *par)
> > lcd_enable_raster();
> > }
> >
> > -/* Configure the Burst Size of DMA */
> > -static int lcd_cfg_dma(int burst_size)
> > +/* Configure the Burst Size and fifo threhold of DMA */
> > +static int lcd_cfg_dma(int burst_size, int fifo_th)
>
> One space too much.
>
Yes, I will correct this. Don't know how checkpatch missing it.
> > {
> > u32 reg;
> >
> > @@ -719,8 +722,10 @@ static irqreturn_t lcdc_irq_handler_rev02(int irq, void *arg)
> >
> > if ((stat & LCD_SYNC_LOST) && (stat & LCD_FIFO_UNDERFLOW)) {
> > lcd_disable_raster();
> > + clk_disable(par->lcdc_clk);
> > lcdc_write(stat, LCD_MASKED_STAT_REG);
>
> Will LCDC register wrtite work without LCDC clock?
>
Yes it cannot happen. I will remove it.
Thanks,
Prakash
^ permalink raw reply
* Re: Device tree binding for DVFS table
From: Peter De Schrijver @ 2012-07-12 14:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20120711200402.GC2772@gmail.com>
On Wed, Jul 11, 2012 at 10:04:02PM +0200, Mike Turquette wrote:
> On 20120711-15:44, Mark Brown wrote:
> > On Wed, Jul 11, 2012 at 09:03:41AM -0500, Rob Herring wrote:
> >
> > > I'd expect a single property with freq/volt pairs or 2 properties for
> > > freq and voltage where there is a 1:1 relationship (freq N uses voltage N).
> >
> > I strongly agree - the current proposal is very hard to read due to the
> > separation between the voltage and frequency values. Some devices do
> > also need to scale multiple rails together, especially when this gets
> > used for I/O devices.
> >
> > I'd also expect to see a range of voltages for each frequency rather
> > than a specific voltage; usually things are at least characterised with
> > a specified tolerance.
>
> Not only should we support multiple voltage rails but also multiple
> clocks. For some devices a DVFS transition is composed of scaling
> multiple clock rates together. So some sort of clock identifier
> (phandle?) is needed as well. (forgive my ignorance on the phandle
> part, as I am a DT noob)
I would say this constraint should be expressed in a seperate DT node. In
short I think we have 3 things to model:
+ frequency/voltage relationships
+ power rail constraints (eg voltage difference limit between 2 rails)
+ clock constraints (eg. clock x frequency must be a fixed ratio of clock y
frequency)
Cheers,
Peter.
^ permalink raw reply
* Re: [PATCH 1/2] video: da8xx-fb: configure FIFO threshold to reduce underflow errors
From: Sergei Shtylyov @ 2012-07-12 13:23 UTC (permalink / raw)
To: linux-fbdev
Hello.
On 11-07-2012 19:14, Manjunathappa, Prakash wrote:
> Patch works around the below silicon errata:
> During LCDC initialization, there is the potential for a FIFO
> underflow condition to occur. A FIFO underflow condition
> occurs when the input FIFO is completely empty and the LCDC
> raster controller logic that drives data to the output pins
> attempts to fetch data from the FIFO. When a FIFO underflow
> condition occurs, incorrect data will be driven out on the
> LCDC data pins.
> Software should poll the FUF bit field in the LCD_STAT register
> to check if an error condition has occurred or service the
> interrupt if FUF_EN is enabled when FUF occurs. If the FUF bit
> field has been set to 1, this will indicate an underflow
> condition has occurred and then the software should execute a
> reset of the LCDC via the LPSC.
> This problem may occur if the LCDC FIFO threshold size
> (LCDDMA_CTRL[TH_FIFO_READY]) is left at its default value after
> reset. Increasing the FIFO threshold size will reduce or
> eliminate underflows. Setting the threshold size to 256 double
> words or larger is recommended.
> Above issue is described in section 2.1.3 of silicon errata
> http://www.ti.com/lit/er/sprz313e/sprz313e.pdf
> Signed-off-by: Rajashekhara, Sudhakar <sudhakar.raj@ti.com>
> Signed-off-by: Manjunathappa, Prakash <prakash.pm@ti.com>
> ---
> drivers/video/da8xx-fb.c | 15 +++++++++++----
> include/video/da8xx-fb.h | 3 +++
> 2 files changed, 14 insertions(+), 4 deletions(-)
> diff --git a/drivers/video/da8xx-fb.c b/drivers/video/da8xx-fb.c
> index 47118c7..2010dd7 100644
> --- a/drivers/video/da8xx-fb.c
> +++ b/drivers/video/da8xx-fb.c
> @@ -344,8 +344,8 @@ static void lcd_blit(int load_mode, struct da8xx_fb_par *par)
> lcd_enable_raster();
> }
>
> -/* Configure the Burst Size of DMA */
> -static int lcd_cfg_dma(int burst_size)
> +/* Configure the Burst Size and fifo threhold of DMA */
> +static int lcd_cfg_dma(int burst_size, int fifo_th)
One space too much.
> {
> u32 reg;
>
> @@ -719,8 +722,10 @@ static irqreturn_t lcdc_irq_handler_rev02(int irq, void *arg)
>
> if ((stat & LCD_SYNC_LOST) && (stat & LCD_FIFO_UNDERFLOW)) {
> lcd_disable_raster();
> + clk_disable(par->lcdc_clk);
> lcdc_write(stat, LCD_MASKED_STAT_REG);
Will LCDC register wrtite work without LCDC clock?
> lcd_enable_raster();
> + clk_enable(par->lcdc_clk);
> } else if (stat & LCD_PL_LOAD_DONE) {
> /*
> * Must disable raster before changing state of any control bit.
> @@ -774,8 +779,10 @@ static irqreturn_t lcdc_irq_handler_rev01(int irq, void *arg)
>
> if ((stat & LCD_SYNC_LOST) && (stat & LCD_FIFO_UNDERFLOW)) {
> lcd_disable_raster();
> + clk_disable(par->lcdc_clk);
> lcdc_write(stat, LCD_STAT_REG);
Same question.
WBR, Sergei
^ permalink raw reply
* Re: [RFC][PATCH V2 3/3] tegra: add pwm backlight device tree nodes
From: Alex Courbot @ 2012-07-12 10:11 UTC (permalink / raw)
To: Simon Glass
Cc: Thierry Reding, linux-tegra@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-fbdev@vger.kernel.org,
devicetree-discuss@lists.ozlabs.org
In-Reply-To: <CAPnjgZ1QbjE+-tr0c01K2feUdEE2wMBfR=bKpTxnyDOJbY8+1Q@mail.gmail.com>
Hi Simon,
On Thu 12 Jul 2012 06:37:33 PM JST, Simon Glass wrote:
> I would like to do something similar in U-Boot for Tegra, although
> perhaps not right away. For now I will go with something considerably
> simpler! But if this is merged into the kernel we will move to it in
> U-Boot.
Cool, I'd love to see that used in U-Boot as well.
>> + default-brightness-level = <12>;
>> +
>> + pwms = <&pwm 2 5000000>;
>> + pwm-names = "backlight";
>> + power-supply = <&backlight_reg>;
>> + enable-gpios = <&gpio 28 0>;
>> +
>> + power-on-sequence = "REGULATOR", "power", <1>,
>> + "DELAY", <10>,
>> + "PWM", "backlight", <1>,
>> + "GPIO", "enable", <1>;
>
> So the names REGULATOR, DELAY, etc. here are looked up through some
> existing mechanism? In general I am not a big fan of mixing strings
> and numbers in a property.
Yes, these are strings we are looking up to know the type of the next
element to power on/off in the sequence. I don't like these, honestly,
and would rather have them replaced by constants - however there is no
way to define constants in the DT AFAIK (but I have heard some other
persons are interested in having them too), and this is the only way I
have found to keep the sequence readable.
> Maybe something like:
>
> power_on_sequence {
> step@0 {
> phandle = <&backlight_reg>;
> enable = <1>;
> post-delay = <10>;
> }
> step@1 {
> phandle = <&pwm 2 5000000>;
> }
> step@2 {
> phandle = <&gpio 28 0>;
> enable = <1>;
> }
I see a few problems with this: first, how do you know the types of your
phandles? At step 0, we should get a regulator, step 1 is a PWM and step
2 is a GPIO. This is why I have used strings to identify the type of the
phandle and the number (and type) of additional arguments to parse for a
step.
Second, I am afraid the representation in memory would be significantly
bigger since the properties names would have to be stored as well (I am
no DT expert, please correct me if I am wrong). Lastly, the additional
freedom of having extra properties might make the parser more complicated.
I agree the type strings are a problem in the current form - if we could
get constants in the device tree, that would be much better. Your way of
representing the sequences is interesting though, if we can solve the
type issue (and also evaluate its cost in terms of memory footprint), it
would be interesting to consider it as well.
Alex.
^ permalink raw reply
* Re: [RFC][PATCH V2 3/3] tegra: add pwm backlight device tree nodes
From: Thierry Reding @ 2012-07-12 10:04 UTC (permalink / raw)
To: Simon Glass
Cc: Alexandre Courbot, linux-tegra-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ
In-Reply-To: <CAPnjgZ1QbjE+-tr0c01K2feUdEE2wMBfR=bKpTxnyDOJbY8+1Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 3691 bytes --]
On Thu, Jul 12, 2012 at 11:37:33AM +0200, Simon Glass wrote:
> Hi Alex,
>
> On Mon, Jul 9, 2012 at 8:08 AM, Alexandre Courbot <acourbot@nvidia.com> wrote:
> > Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> > ---
> > arch/arm/boot/dts/tegra20-ventana.dts | 31 +++++++++++++++++++++++++++++++
> > arch/arm/boot/dts/tegra20.dtsi | 2 +-
> > 2 files changed, 32 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/boot/dts/tegra20-ventana.dts b/arch/arm/boot/dts/tegra20-ventana.dts
> > index be90544..c67d9e1 100644
> > --- a/arch/arm/boot/dts/tegra20-ventana.dts
> > +++ b/arch/arm/boot/dts/tegra20-ventana.dts
> > @@ -317,6 +317,37 @@
> > bus-width = <8>;
> > };
> >
>
> I would like to do something similar in U-Boot for Tegra, although
> perhaps not right away. For now I will go with something considerably
> simpler! But if this is merged into the kernel we will move to it in
> U-Boot. Anyway here are my comments:
>
> > + backlight {
> > + compatible = "pwm-backlight";
> > + brightness-levels = <0 16 32 48 64 80 96 112 128 144 160 176 192 208 224 240 255>;
>
> We seem to have a lot of these - should we move to a range and step size?
These actually seem to be a little bogus. The reason for introducing the
levels was to allow calibration of these values because they in fact
usually do not scale linearly. Instead, a linear brightness increase
usually maps to something like a logarithmic (at best) increase of the
duty cycle.
> > + default-brightness-level = <12>;
> > +
> > + pwms = <&pwm 2 5000000>;
> > + pwm-names = "backlight";
> > + power-supply = <&backlight_reg>;
> > + enable-gpios = <&gpio 28 0>;
> > +
> > + power-on-sequence = "REGULATOR", "power", <1>,
> > + "DELAY", <10>,
> > + "PWM", "backlight", <1>,
> > + "GPIO", "enable", <1>;
>
> So the names REGULATOR, DELAY, etc. here are looked up through some
> existing mechanism? In general I am not a big fan of mixing strings
> and numbers in a property. Maybe something like:
>
> power_on_sequence {
> step@0 {
> phandle = <&backlight_reg>;
> enable = <1>;
> post-delay = <10>;
> }
> step@1 {
> phandle = <&pwm 2 5000000>;
> }
> step@2 {
> phandle = <&gpio 28 0>;
> enable = <1>;
> }
I actually like that a lot. What's missing from your example is a way to
specify the type of the phandle because it cannot be safely inferred.
Perhaps this could be done by just the property name:
power_on_sequence {
step@0 {
power-supply = <&backlight_reg>;
enable = <1>;
post-delay = <10>;
};
step@1 {
pwm = <&pwm 2 5000000>;
};
step@2 {
enable-gpios = <&gpio 28 0>;
enable = <1>;
};
};
However I think one of the reasons for Alex to choose his particular
representation is that people want this to be representable in the
platform data as well, which unfortunately can't deal very well with
this kind of situation.
Usually in the kernel you have one API to obtain a resource (pwm_get(),
regulator_get(), ...) which takes as an argument a device handle and a
name. For device tree, that API will lookup the resource by phandle,
while the non-DT code path typically uses a static lookup table.
It's a shame that we have to keep this kind of backwards compatibility
because it prevents these things from being represented in a logic way
in the DT sense.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [RFC][PATCH V2 3/3] tegra: add pwm backlight device tree nodes
From: Simon Glass @ 2012-07-12 9:37 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Thierry Reding, linux-tegra-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ
In-Reply-To: <1341814105-20690-4-git-send-email-acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Hi Alex,
On Mon, Jul 9, 2012 at 8:08 AM, Alexandre Courbot <acourbot@nvidia.com> wrote:
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---
> arch/arm/boot/dts/tegra20-ventana.dts | 31 +++++++++++++++++++++++++++++++
> arch/arm/boot/dts/tegra20.dtsi | 2 +-
> 2 files changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/tegra20-ventana.dts b/arch/arm/boot/dts/tegra20-ventana.dts
> index be90544..c67d9e1 100644
> --- a/arch/arm/boot/dts/tegra20-ventana.dts
> +++ b/arch/arm/boot/dts/tegra20-ventana.dts
> @@ -317,6 +317,37 @@
> bus-width = <8>;
> };
>
I would like to do something similar in U-Boot for Tegra, although
perhaps not right away. For now I will go with something considerably
simpler! But if this is merged into the kernel we will move to it in
U-Boot. Anyway here are my comments:
> + backlight {
> + compatible = "pwm-backlight";
> + brightness-levels = <0 16 32 48 64 80 96 112 128 144 160 176 192 208 224 240 255>;
We seem to have a lot of these - should we move to a range and step size?
> + default-brightness-level = <12>;
> +
> + pwms = <&pwm 2 5000000>;
> + pwm-names = "backlight";
> + power-supply = <&backlight_reg>;
> + enable-gpios = <&gpio 28 0>;
> +
> + power-on-sequence = "REGULATOR", "power", <1>,
> + "DELAY", <10>,
> + "PWM", "backlight", <1>,
> + "GPIO", "enable", <1>;
So the names REGULATOR, DELAY, etc. here are looked up through some
existing mechanism? In general I am not a big fan of mixing strings
and numbers in a property. Maybe something like:
power_on_sequence {
step@0 {
phandle = <&backlight_reg>;
enable = <1>;
post-delay = <10>;
}
step@1 {
phandle = <&pwm 2 5000000>;
}
step@2 {
phandle = <&gpio 28 0>;
enable = <1>;
}
> + power-off-sequence = "GPIO", "enable", <0>,
> + "PWM", "backlight", <0>,
> + "DELAY", <10>,
> + "REGULATOR", "power", <0>;
> + };
> +
> + backlight_reg: fixedregulator@176 {
> + compatible = "regulator-fixed";
> + regulator-name = "backlight_regulator";
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + gpio = <&gpio 176 0>;
> + startup-delay-us = <0>;
> + enable-active-high;
> + regulator-boot-off;
> + };
> +
> sound {
> compatible = "nvidia,tegra-audio-wm8903-ventana",
> "nvidia,tegra-audio-wm8903";
> diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
> index 405d167..67a6cd9 100644
> --- a/arch/arm/boot/dts/tegra20.dtsi
> +++ b/arch/arm/boot/dts/tegra20.dtsi
> @@ -123,7 +123,7 @@
> status = "disabled";
> };
>
> - pwm {
> + pwm: pwm {
> compatible = "nvidia,tegra20-pwm";
> reg = <0x7000a000 0x100>;
> #pwm-cells = <2>;
> --
> 1.7.11.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Regards,
Simon
^ permalink raw reply
* Re: Device tree binding for DVFS table
From: Peter De Schrijver @ 2012-07-12 8:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20120711144449.GA23654@sirena.org.uk>
On Wed, Jul 11, 2012 at 04:44:50PM +0200, Mark Brown wrote:
> On Wed, Jul 11, 2012 at 09:03:41AM -0500, Rob Herring wrote:
>
> > I'd expect a single property with freq/volt pairs or 2 properties for
> > freq and voltage where there is a 1:1 relationship (freq N uses voltage N).
>
> I strongly agree - the current proposal is very hard to read due to the
> separation between the voltage and frequency values. Some devices do
> also need to scale multiple rails together, especially when this gets
> used for I/O devices.
>
I don't think dependencies between rails should be modelled in the same DT
nodes. We have those too in Tegra30, but as this is SoC internal, I am not
sure if it needs to be modelled in DT at all, or if we can leave it in SoC
specific code. While our current dependency is of the form 'voltage difference
between rail x and rail y must be smaller then z mV', I have been told these
dependencies might become more complicated in the future. Hence they might
need to be expressed in code anyway.
Anyway, I think voltage dependencies between rails is a related but seperate
topic from DVFS.
Cheers,
Peter.
^ permalink raw reply
* [PATCH] video: exynos_dp: fix wrong DPCD address during Link Training
From: Jingoo Han @ 2012-07-12 6:10 UTC (permalink / raw)
To: linux-fbdev
Wrong DPCD addresses were used for clock recovery during Link Training.
The training pattern should be set by TRAINING_PATTERN_SET (0x102), while
voltage swing and pre-emphasis should be set by TRAINING_LANE0_SET (0x103).
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/video/exynos/exynos_dp_core.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index 9db7b9f..bf55e51 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -304,7 +304,7 @@ static void exynos_dp_link_start(struct exynos_dp_device *dp)
buf[lane] = DPCD_PRE_EMPHASIS_PATTERN2_LEVEL0 |
DPCD_VOLTAGE_SWING_PATTERN1_LEVEL0;
exynos_dp_write_bytes_to_dpcd(dp,
- DPCD_ADDR_TRAINING_PATTERN_SET,
+ DPCD_ADDR_TRAINING_LANE0_SET,
lane_count, buf);
}
@@ -504,7 +504,7 @@ static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp)
buf[0] = DPCD_SCRAMBLING_DISABLED |
DPCD_TRAINING_PATTERN_2;
exynos_dp_write_byte_to_dpcd(dp,
- DPCD_ADDR_TRAINING_LANE0_SET,
+ DPCD_ADDR_TRAINING_PATTERN_SET,
buf[0]);
for (lane = 0; lane < lane_count; lane++) {
--
1.7.1
^ permalink raw reply related
* Re: Device tree binding for DVFS table
From: Prashant Gaikwad @ 2012-07-12 4:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20120711134956.GB9437@tbergstrom-lnx.Nvidia.com>
On Wednesday 11 July 2012 07:19 PM, Peter De Schrijver wrote:
> On Wed, Jul 11, 2012 at 03:08:14PM +0200, 함명주 wrote:
>>> Hi,
>>>
>>> I am working on DT binding for Tegra DVFS.
>>>
>>> For Tegra, DVFS node mainly consists of frequency and voltage pairs.
>>> Frequency in the pair may change for different process. E.g. for process
>>> 1 CPU clock frequency could be 900MHz at 1V while for process 2 it could
>>> be 1GHz at 1V.
>>> Tegra uses vendor specific ids to identify the correct frequency table.
>> Hello,
>>
>> It seems that in the example, the values in "voltage-array" and
>> "frequencies" are switched.
>>
>> Anyway, what about SoCs that reads information from IEM (or any other module)
>> to measure gate delays or some other value to set the appriorate voltage values
>> for every possible frequency? I remember some of Exynos SoCs have been doing
>> it; dynamically measure the characteristics at boot-up time and apply voltages
>> accordingly; they couldn't identify it based on the chip-id or simply by reading
>> a single register.
>>
>
> But in that case you would have a nominal voltage for each OPP which gets
> adjusted at boottime or runtime depending on the exact silicon characteristics?
> I would say the DT binding should then specify 1 table with the nominal values
> and leave the dynamics to the driver.
It makes sense or is it different than this?
> Cheers,
>
> Peter.
>
^ permalink raw reply
* Re: Device tree binding for DVFS table
From: Prashant Gaikwad @ 2012-07-12 4:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20120711144449.GA23654@sirena.org.uk>
On Wednesday 11 July 2012 08:14 PM, Mark Brown wrote:
> On Wed, Jul 11, 2012 at 09:03:41AM -0500, Rob Herring wrote:
>
>> I'd expect a single property with freq/volt pairs or 2 properties for
>> freq and voltage where there is a 1:1 relationship (freq N uses voltage N).
> I strongly agree - the current proposal is very hard to read due to the
> separation between the voltage and frequency values. Some devices do
> also need to scale multiple rails together, especially when this gets
> used for I/O devices.
Multiple rails for single clock or would it be multiple clocks?
> I'd also expect to see a range of voltages for each frequency rather
> than a specific voltage; usually things are at least characterised with
> a specified tolerance.
But then how will you determine the operating voltage for a frequency?
^ permalink raw reply
* Re: Device tree binding for DVFS table
From: Prashant Gaikwad @ 2012-07-12 4:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20120711200402.GC2772@gmail.com>
On Thursday 12 July 2012 01:34 AM, Mike Turquette wrote:
> On 20120711-15:44, Mark Brown wrote:
>> On Wed, Jul 11, 2012 at 09:03:41AM -0500, Rob Herring wrote:
>>
>>> I'd expect a single property with freq/volt pairs or 2 properties for
>>> freq and voltage where there is a 1:1 relationship (freq N uses voltage N).
>> I strongly agree - the current proposal is very hard to read due to the
>> separation between the voltage and frequency values. Some devices do
>> also need to scale multiple rails together, especially when this gets
>> used for I/O devices.
>>
>> I'd also expect to see a range of voltages for each frequency rather
>> than a specific voltage; usually things are at least characterised with
>> a specified tolerance.
> Not only should we support multiple voltage rails but also multiple
> clocks. For some devices a DVFS transition is composed of scaling
> multiple clock rates together. So some sort of clock identifier
> (phandle?) is needed as well. (forgive my ignorance on the phandle
> part, as I am a DT noob)
How about adding phandle of clock consumer node to frequency table?
> Regards,
> Mike
^ permalink raw reply
* Re: Device tree binding for DVFS table
From: Prashant Gaikwad @ 2012-07-12 4:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4FFD87BD.2030206@gmail.com>
On Wednesday 11 July 2012 07:33 PM, Rob Herring wrote:
> On 07/11/2012 07:56 AM, Prashant Gaikwad wrote:
>> Hi,
>>
>> I am working on DT binding for Tegra DVFS.
>>
>> For Tegra, DVFS node mainly consists of frequency and voltage pairs.
>> Frequency in the pair may change for different process. E.g. for process
>> 1 CPU clock frequency could be 900MHz at 1V while for process 2 it could
>> be 1GHz at 1V.
>> Tegra uses vendor specific ids to identify the correct frequency table.
>>
>> Following is the proposed binding for voltage and frequency tables used
>> in DVFS. Looking for comments/suggestions to make it generic.
>>
>> ====DVFS table node=========>> This node defines the voltage configuration for the DVFS which includes
>> the regulator and voltage array.
>>
>> Required properties:
>>
>> reg_id :<regulator phandle>;
>> voltage-table :<voltage array, values in mV>;
>>
>> #address-cells and #size-cells : To identify correct frequency table
>> using process id (or some other vendor specific way).
>>
>> ====Frequency table node=======>>
>> This node defines frequency configuration for the device DVFS.
>>
>> Required properties:
>>
>> In device node need a reference to the DVFS table node.
>>
>> dvfs : the phandle for the DVFS table node
>>
>> frequency-table@n
>> reg =<n>; dependent on address-cells and size-cells in DVFS
>> table node.
>> frequencies =<frequency array, values in MHz>;
> You are storing u32 values, why limit the resolution to MHz.
>
> Put the unit in the property names.
>
Unit can be changed, will do it.
>> }
>>
>> Example:
>> -------------------------------------------------------------------------
>>
>> cpu-dvfs-table : dvfs-table {
> This should be located with the node that the frequencies correspond to.
>
With CAR node?
>> compatible = "nvidia,tegra30-dvfs-table";
>> reg_id =<&sm0>;
>> #address-cells =<1>;
>> #size-cells =<0>;
>> voltage-array =<750 775 800 825 850 875 900 925 950 975
>> 1000 1025 1050 1100 1125>;
> The SOC is really characterized at all these voltages?
Not really, but different processes of single SoC are characterized for
different voltages and this array covers all those voltages.
>> };
>>
>> device {
>> dvfs =<&cpu-dvfs-table>;
>> frequency-table@102 {
>> reg =<0x102>;
>> frequencies =<314 314 314 456 456 456 608 608 608
>> 760 817 817 912 1000>;
> I don't see the point of repeating frequencies.
>> };
>> frequency-table@002 {
>> reg =<0x002>;
>> frequencies =<598 598 750 750 893 893 1000>;
>> };
> How do you determine the voltage for a frequency on table 2?
>
> I'd expect a single property with freq/volt pairs or 2 properties for
> freq and voltage where there is a 1:1 relationship (freq N uses voltage N).
How this will work:
voltage-array =<750 775 800 825 850 875 900 925 950 975 1000 1025 1050 1100 1125>
frequencies-1 =<314 314 314 456 456 456 608 608 608 760 817 817 912 1000>;
frequencies-2 =<598 598 750 750 893 893 1000>;
Freq and voltage has 1:1 relationship but as single voltage table is
used for different processes we have more entries in voltage table than
freq table.
Frequency table 1 is mapped till 1100mV while frequency table 2 is
mapped till 900mV only, it maintains 1:1 relationship.
About repeating frequencies, operating voltage for a frequency would be
the highest one mapped in the table.
For example, in frequency table 2 operating voltage for 750MHz would be
825mV while for 893MHz it would be 875mV. Unmapped entries could be
replaced with 0 to make reading better.
Advantage it provides is single voltage table used for multiple
frequency tables, as can be observed from above tables, operating
voltage for 314MHz in freq table 1 is 800mV while there is no frequency
in table 2 at that voltage.
I know this makes reading difficult but it provides flexibility,
I hope it explains the implementation.
> Rob
>
>> };
>>
>> Thanks& Regards,
>> Prashant G
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply
* [GIT PULL] fbdev fixes for 3.5#2
From: Florian Tobias Schandinat @ 2012-07-11 22:41 UTC (permalink / raw)
To: Linus Torvalds; +Cc: LKML, linux-fbdev@vger.kernel.org
Hi Linus,
please pull the fixes below.
Thanks,
Florian Tobias Schandinat
The following changes since commit b67989515defba7412acff01162e5bb1f0f5923a:
video: s3c-fb: fix possible division by zero in s3c_fb_calc_pixclk (2012-06-13 17:34:16 +0000)
are available in the git repository at:
git://github.com/schandinat/linux-2.6.git fbdev-fixes-for-3.5-2
for you to fetch changes up to 373b43652150c9342168c846a1efbd81438ea241:
OMAPDSS: fix warnings if CONFIG_PM_RUNTIME=n (2012-07-08 14:00:27 +0000)
----------------------------------------------------------------
fbdev fixes for 3.5
- two fixes for OMAPDSS by Tomi Valkeinen
one to avoid warnings when runtime PM is not enabled
one workaround to dependancy issues during suspend/resume
----------------------------------------------------------------
Tomi Valkeinen (2):
OMAPDSS: Use PM notifiers for system suspend
OMAPDSS: fix warnings if CONFIG_PM_RUNTIME=n
drivers/video/omap2/dss/core.c | 43 ++++++++++++++++++++++++--------------
drivers/video/omap2/dss/dispc.c | 2 +-
drivers/video/omap2/dss/dsi.c | 2 +-
drivers/video/omap2/dss/dss.c | 2 +-
drivers/video/omap2/dss/hdmi.c | 2 +-
drivers/video/omap2/dss/rfbi.c | 2 +-
drivers/video/omap2/dss/venc.c | 2 +-
7 files changed, 33 insertions(+), 22 deletions(-)
^ permalink raw reply
* Re: [PATCH v2] of: Add videomode helper
From: Guennadi Liakhovetski @ 2012-07-11 20:40 UTC (permalink / raw)
To: Sascha Hauer
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Laurent Pinchart,
kernel-bIcnvbaLZ9MEGnE8C9+IrQ
In-Reply-To: <20120711190414.GQ30009-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
On Wed, 11 Jul 2012, Sascha Hauer wrote:
> Hi Guennadi,
>
> On Wed, Jul 11, 2012 at 10:34:54AM +0200, Guennadi Liakhovetski wrote:
> > Hi Sascha
> >
> > > +
> > > +Optional properties:
> > > + - width-mm, height-mm: Display dimensions in mm
> > > + - hsync-active-high (bool): Hsync pulse is active high
> > > + - vsync-active-high (bool): Vsync pulse is active high
> >
> > How about
> >
> > + - hsync-active: Hsync pulse polarity: 1 for high, 0 for low
>
> I am unsure if it's good to mix this with the other bool flags like:
>
> >
> > > + - interlaced (bool): This is an interlaced mode
> > > + - doublescan (bool): This is a doublescan mode
>
> Which behave differently. 'interlaced' will be evaluated as true if
> the property is present, no matter which value it has. This might
> lead to confusion.
I don't feel strongly either way either. I don't think it'd be confusing -
you have integer properties there too, and the logic in this case is not
"active high - yes or now," but rather "active level - logical 1 (high) or
0 (low)." But as I said - that was just an idea, unless someone has strong
arguments either way - you're the original author, it's your call:-)
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply
* Re: Device tree binding for DVFS table
From: Mike Turquette @ 2012-07-11 20:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20120711144449.GA23654@sirena.org.uk>
On 20120711-15:44, Mark Brown wrote:
> On Wed, Jul 11, 2012 at 09:03:41AM -0500, Rob Herring wrote:
>
> > I'd expect a single property with freq/volt pairs or 2 properties for
> > freq and voltage where there is a 1:1 relationship (freq N uses voltage N).
>
> I strongly agree - the current proposal is very hard to read due to the
> separation between the voltage and frequency values. Some devices do
> also need to scale multiple rails together, especially when this gets
> used for I/O devices.
>
> I'd also expect to see a range of voltages for each frequency rather
> than a specific voltage; usually things are at least characterised with
> a specified tolerance.
Not only should we support multiple voltage rails but also multiple
clocks. For some devices a DVFS transition is composed of scaling
multiple clock rates together. So some sort of clock identifier
(phandle?) is needed as well. (forgive my ignorance on the phandle
part, as I am a DT noob)
Regards,
Mike
^ permalink raw reply
* Re: [PATCH v2] of: Add videomode helper
From: Sascha Hauer @ 2012-07-11 19:04 UTC (permalink / raw)
To: Guennadi Liakhovetski
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Laurent Pinchart,
kernel-bIcnvbaLZ9MEGnE8C9+IrQ
In-Reply-To: <Pine.LNX.4.64.1207111031200.18999-0199iw4Nj15frtckUFj5Ag@public.gmane.org>
Hi Guennadi,
On Wed, Jul 11, 2012 at 10:34:54AM +0200, Guennadi Liakhovetski wrote:
> Hi Sascha
>
> > +
> > +Optional properties:
> > + - width-mm, height-mm: Display dimensions in mm
> > + - hsync-active-high (bool): Hsync pulse is active high
> > + - vsync-active-high (bool): Vsync pulse is active high
>
> How about
>
> + - hsync-active: Hsync pulse polarity: 1 for high, 0 for low
I am unsure if it's good to mix this with the other bool flags like:
>
> > + - interlaced (bool): This is an interlaced mode
> > + - doublescan (bool): This is a doublescan mode
Which behave differently. 'interlaced' will be evaluated as true if
the property is present, no matter which value it has. This might
lead to confusion.
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: Device tree binding for DVFS table
From: Mark Brown @ 2012-07-11 14:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4FFD87BD.2030206@gmail.com>
On Wed, Jul 11, 2012 at 09:03:41AM -0500, Rob Herring wrote:
> I'd expect a single property with freq/volt pairs or 2 properties for
> freq and voltage where there is a 1:1 relationship (freq N uses voltage N).
I strongly agree - the current proposal is very hard to read due to the
separation between the voltage and frequency values. Some devices do
also need to scale multiple rails together, especially when this gets
used for I/O devices.
I'd also expect to see a range of voltages for each frequency rather
than a specific voltage; usually things are at least characterised with
a specified tolerance.
^ permalink raw reply
* Re: Device tree binding for DVFS table
From: Rob Herring @ 2012-07-11 14:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4FFD77FE.8050206@nvidia.com>
On 07/11/2012 07:56 AM, Prashant Gaikwad wrote:
> Hi,
>
> I am working on DT binding for Tegra DVFS.
>
> For Tegra, DVFS node mainly consists of frequency and voltage pairs.
> Frequency in the pair may change for different process. E.g. for process
> 1 CPU clock frequency could be 900MHz at 1V while for process 2 it could
> be 1GHz at 1V.
> Tegra uses vendor specific ids to identify the correct frequency table.
>
> Following is the proposed binding for voltage and frequency tables used
> in DVFS. Looking for comments/suggestions to make it generic.
>
> ====DVFS table node=========> This node defines the voltage configuration for the DVFS which includes
> the regulator and voltage array.
>
> Required properties:
>
> reg_id : <regulator phandle>;
> voltage-table : <voltage array, values in mV>;
>
> #address-cells and #size-cells : To identify correct frequency table
> using process id (or some other vendor specific way).
>
> ====Frequency table node=======>
> This node defines frequency configuration for the device DVFS.
>
> Required properties:
>
> In device node need a reference to the DVFS table node.
>
> dvfs : the phandle for the DVFS table node
>
> frequency-table@n
> reg = <n>; dependent on address-cells and size-cells in DVFS
> table node.
> frequencies = <frequency array, values in MHz>;
You are storing u32 values, why limit the resolution to MHz.
Put the unit in the property names.
> }
>
> Example:
> -------------------------------------------------------------------------
>
> cpu-dvfs-table : dvfs-table {
This should be located with the node that the frequencies correspond to.
> compatible = "nvidia,tegra30-dvfs-table";
> reg_id = <&sm0>;
> #address-cells = <1>;
> #size-cells = <0>;
> voltage-array = <750 775 800 825 850 875 900 925 950 975
> 1000 1025 1050 1100 1125>;
The SOC is really characterized at all these voltages?
> };
>
> device {
> dvfs = <&cpu-dvfs-table>;
> frequency-table@102 {
> reg = <0x102>;
> frequencies = <314 314 314 456 456 456 608 608 608
> 760 817 817 912 1000>;
I don't see the point of repeating frequencies.
> };
> frequency-table@002 {
> reg = <0x002>;
> frequencies = <598 598 750 750 893 893 1000>;
> };
How do you determine the voltage for a frequency on table 2?
I'd expect a single property with freq/volt pairs or 2 properties for
freq and voltage where there is a 1:1 relationship (freq N uses voltage N).
Rob
> };
>
> Thanks & Regards,
> Prashant G
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ 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