Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1685537 > unrolled thread

[RFC v2 5/6] drivers: boot_constraint: Add initial DT bindings

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [RFC v2 5/6] drivers: boot_constraint: Add initial DT bindings Viresh Kumar <viresh.kumar@linaro.org> - 2017-07-12 08:40 +0200
    Re: [RFC v2 5/6] drivers: boot_constraint: Add initial DT bindings Rob Herring <robh+dt@kernel.org> - 2017-07-12 23:30 +0200
      Re: [RFC v2 5/6] drivers: boot_constraint: Add initial DT bindings Chen-Yu Tsai <wens@csie.org> - 2017-07-13 05:00 +0200
        Re: [RFC v2 5/6] drivers: boot_constraint: Add initial DT bindings Viresh Kumar <viresh.kumar@linaro.org> - 2017-07-13 07:20 +0200
          Re: [RFC v2 5/6] drivers: boot_constraint: Add initial DT bindings Chen-Yu Tsai <wens@csie.org> - 2017-07-13 11:50 +0200
            Re: [RFC v2 5/6] drivers: boot_constraint: Add initial DT bindings Viresh Kumar <viresh.kumar@linaro.org> - 2017-07-13 12:00 +0200
      Re: [RFC v2 5/6] drivers: boot_constraint: Add initial DT bindings Viresh Kumar <viresh.kumar@linaro.org> - 2017-07-13 11:40 +0200
        Re: [RFC v2 5/6] drivers: boot_constraint: Add initial DT bindings Rob Herring <robh@kernel.org> - 2017-07-17 19:40 +0200
          Re: [RFC v2 5/6] drivers: boot_constraint: Add initial DT bindings Viresh Kumar <viresh.kumar@linaro.org> - 2017-07-18 08:00 +0200

#1685537 — [RFC v2 5/6] drivers: boot_constraint: Add initial DT bindings

This adds device tree bindings for boot constraints. Only power supply
constraint types are supported currently.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 .../devicetree/bindings/boot-constraints.txt       | 68 ++++++++++++++++++++++
 1 file changed, 68 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/boot-constraints.txt

diff --git a/Documentation/devicetree/bindings/boot-constraints.txt b/Documentation/devicetree/bindings/boot-constraints.txt
new file mode 100644
index 000000000000..9a01ea1e6e72
--- /dev/null
+++ b/Documentation/devicetree/bindings/boot-constraints.txt
@@ -0,0 +1,68 @@
+BOOT CONSTRAINTS
+================
+
+Some devices are powered ON by the bootloader before the bootloader handovers
+control to the Operating System (OS). It maybe important for those devices to
+keep working until the time the OS takes over and starts configuring the devices
+again.
+
+A typical example of that can be the LCD controller, which is used by the
+bootloaders to show image(s) while the platform is booting into the Operating
+System. The LCD controller can be using some resources, like clk, supplies, etc,
+that are shared between several devices. These shared resources should be
+configured to satisfy need of all the users. If another device's (X) driver gets
+probed before the LCD controller driver in this case, then it may end up
+reconfiguring these resources to ranges satisfying the current users (only
+device X) and that can make the LCD screen unstable.
+
+This document describes the binding used to specify such boot constraints to the
+OS.
+
+Power Supply Constraints:
+-------------------------
+
+This describes the binding of constraints for the power supply resources. These
+must be present directly in the consumer device's node.
+
+Required properties:
+- boot-constraint-supplies:
+
+  This contains an array of (one or more) strings, each of which must match with
+  the <name> of a corresponding <name>-supply property in the same device node.
+  This is required for the OS to know about the power supplies that are
+  configured (and enabled) by the bootloader for the consumer device.
+
+  It is assumed that the power supply is already enabled by the bootloader.
+
+- boot-constraint-uV:
+
+  This contains an array of {min max} microvolt tuples for the power supplies in
+  the same order in which they are present in "boot-constraint-supplies"
+  property. Here, min is the smallest and max is the largest voltage that the
+  consumer (corresponding to the device node where this property is present) may
+  set.
+
+Example of a consumer device node (mmc) referencing two regulators and setting
+their boot constraints (twl_reg1 and twl_reg2):
+
+	twl_reg1: regulator@0 {
+		...
+		...
+		...
+	};
+
+	twl_reg2: regulator@1 {
+		...
+		...
+		...
+	};
+
+	mmc: mmc@0x0 {
+		...
+		...
+		vmmc-supply = <&twl_reg1>;
+		vmmcaux-supply = <&twl_reg2>;
+		boot-constraint-supplies = "vmmc", "vmmcaux";
+		boot-constraint-uV = <1800000 2000000>, /* vmmc */
+				     <2000000 2000000>; /* vmmcaux */
+	};
-- 
2.13.0.71.gd7076ec9c9cb

[toc] | [next] | [standalone]

#1686047

On Wed, Jul 12, 2017 at 1:34 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> This adds device tree bindings for boot constraints. Only power supply
> constraint types are supported currently.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  .../devicetree/bindings/boot-constraints.txt       | 68 ++++++++++++++++++++++
>  1 file changed, 68 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/boot-constraints.txt
>
> diff --git a/Documentation/devicetree/bindings/boot-constraints.txt b/Documentation/devicetree/bindings/boot-constraints.txt
> new file mode 100644
> index 000000000000..9a01ea1e6e72
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/boot-constraints.txt
> @@ -0,0 +1,68 @@
> +BOOT CONSTRAINTS
> +================
> +
> +Some devices are powered ON by the bootloader before the bootloader handovers
> +control to the Operating System (OS). It maybe important for those devices to
> +keep working until the time the OS takes over and starts configuring the devices
> +again.
> +
> +A typical example of that can be the LCD controller, which is used by the
> +bootloaders to show image(s) while the platform is booting into the Operating
> +System. The LCD controller can be using some resources, like clk, supplies, etc,
> +that are shared between several devices. These shared resources should be
> +configured to satisfy need of all the users. If another device's (X) driver gets
> +probed before the LCD controller driver in this case, then it may end up
> +reconfiguring these resources to ranges satisfying the current users (only
> +device X) and that can make the LCD screen unstable.

Display is a pretty well known use case here. Do you have other
examples in mind? Other cases I've seen are automotive with keeping
the backup camera going and CAN bus handling. Though my new car has a
flicker shortly after coming on, so I guess the handoff doesn't have
to be completely seemless. :)

[...]

> +       mmc: mmc@0x0 {
> +               ...
> +               ...
> +               vmmc-supply = <&twl_reg1>;
> +               vmmcaux-supply = <&twl_reg2>;
> +               boot-constraint-supplies = "vmmc", "vmmcaux";
> +               boot-constraint-uV = <1800000 2000000>, /* vmmc */
> +                                    <2000000 2000000>; /* vmmcaux */

No. I don't like how this is going to extend to all the other bindings
people are going to want constraints for. We don't need a parallel set
of properties for each type of binding.

I'm not convinced that we need a general solution for what's probably
a handful of things that need a handoff versus just re-initialize.

Rob

[toc] | [prev] | [next] | [standalone]

#1686203

On Thu, Jul 13, 2017 at 5:28 AM, Rob Herring <robh+dt@kernel.org> wrote:
> On Wed, Jul 12, 2017 at 1:34 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>> This adds device tree bindings for boot constraints. Only power supply
>> constraint types are supported currently.
>>
>> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>> ---
>>  .../devicetree/bindings/boot-constraints.txt       | 68 ++++++++++++++++++++++
>>  1 file changed, 68 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/boot-constraints.txt
>>
>> diff --git a/Documentation/devicetree/bindings/boot-constraints.txt b/Documentation/devicetree/bindings/boot-constraints.txt
>> new file mode 100644
>> index 000000000000..9a01ea1e6e72
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/boot-constraints.txt
>> @@ -0,0 +1,68 @@
>> +BOOT CONSTRAINTS
>> +================
>> +
>> +Some devices are powered ON by the bootloader before the bootloader handovers
>> +control to the Operating System (OS). It maybe important for those devices to
>> +keep working until the time the OS takes over and starts configuring the devices
>> +again.
>> +
>> +A typical example of that can be the LCD controller, which is used by the
>> +bootloaders to show image(s) while the platform is booting into the Operating
>> +System. The LCD controller can be using some resources, like clk, supplies, etc,
>> +that are shared between several devices. These shared resources should be
>> +configured to satisfy need of all the users. If another device's (X) driver gets
>> +probed before the LCD controller driver in this case, then it may end up
>> +reconfiguring these resources to ranges satisfying the current users (only
>> +device X) and that can make the LCD screen unstable.
>
> Display is a pretty well known use case here. Do you have other
> examples in mind? Other cases I've seen are automotive with keeping
> the backup camera going and CAN bus handling. Though my new car has a
> flicker shortly after coming on, so I guess the handoff doesn't have
> to be completely seemless. :)
>
> [...]
>
>> +       mmc: mmc@0x0 {
>> +               ...
>> +               ...
>> +               vmmc-supply = <&twl_reg1>;
>> +               vmmcaux-supply = <&twl_reg2>;
>> +               boot-constraint-supplies = "vmmc", "vmmcaux";
>> +               boot-constraint-uV = <1800000 2000000>, /* vmmc */
>> +                                    <2000000 2000000>; /* vmmcaux */
>
> No. I don't like how this is going to extend to all the other bindings
> people are going to want constraints for. We don't need a parallel set
> of properties for each type of binding.
>
> I'm not convinced that we need a general solution for what's probably
> a handful of things that need a handoff versus just re-initialize.

I'm afraid the regulator case still doesn't make sense. The voltage
constraints should be set within each supplies device node. This was
explained in the discussion in v1 [1].

ChenYu

[1] https://www.spinics.net/lists/arm-kernel/msg591692.html

[toc] | [prev] | [next] | [standalone]

#1686248

On 13-07-17, 10:52, Chen-Yu Tsai wrote:
> I'm afraid the regulator case still doesn't make sense. The voltage
> constraints should be set within each supplies device node. This was
> explained in the discussion in v1 [1].

I thought we were discussing about something I mentioned in one of my example
but never to a point that the regulator problem doesn't exist at all. Perhaps I
misunderstood your concerns. Anyway, lemme try once more with a better example.

Regulator shared by: LCD and MMC (both can do DVFS) and the min/max constraint
that can be set by the consumers of the regulator (both LCD/MMC) are: 1.5 V to
3 V.

The bootloader has programmed the LCD to work at the highest pixel frequency,
which needs the voltage to be in range from 2.5 - 3 V.

Now MMC can get probed first and it can try to bring the voltages below 2.5 V.
Though, 1.5 - 2.5 is a valid range for the LCD, but not at the current pixel
frequency.

Does that sound like a valid problem?

-- 
viresh

[toc] | [prev] | [next] | [standalone]

#1686417

On Thu, Jul 13, 2017 at 1:09 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 13-07-17, 10:52, Chen-Yu Tsai wrote:
>> I'm afraid the regulator case still doesn't make sense. The voltage
>> constraints should be set within each supplies device node. This was
>> explained in the discussion in v1 [1].
>
> I thought we were discussing about something I mentioned in one of my example
> but never to a point that the regulator problem doesn't exist at all. Perhaps I
> misunderstood your concerns. Anyway, lemme try once more with a better example.
>
> Regulator shared by: LCD and MMC (both can do DVFS) and the min/max constraint
> that can be set by the consumers of the regulator (both LCD/MMC) are: 1.5 V to
> 3 V.
>
> The bootloader has programmed the LCD to work at the highest pixel frequency,
> which needs the voltage to be in range from 2.5 - 3 V.
>
> Now MMC can get probed first and it can try to bring the voltages below 2.5 V.
> Though, 1.5 - 2.5 is a valid range for the LCD, but not at the current pixel
> frequency.
>
> Does that sound like a valid problem?

This makes more sense. The LCD being able to do DVFS was missing from the last
discussion. I assume this is for power saving purposes? Otherwise one could just
say you should not use the lower part of the voltage range. And DVFS is for the
controller's core logic and not I/O?

ChenYu

[toc] | [prev] | [next] | [standalone]

#1686423

On 13-07-17, 17:46, Chen-Yu Tsai wrote:
> This makes more sense. The LCD being able to do DVFS was missing from the last
> discussion. I assume this is for power saving purposes?

Yeah.

> Otherwise one could just
> say you should not use the lower part of the voltage range. And DVFS is for the
> controller's core logic and not I/O?

Yeah.

But for many such cases in different platforms, the LCD controller may
not do DVFS. But we still need to make sure its clk, regulator and
power domain are enabled until the time the driver comes up. This
series would also help in doing proxy-voting for all such resources,
so that they don't get disabled while being used.

-- 
viresh

[toc] | [prev] | [next] | [standalone]

#1686410

On 12-07-17, 16:28, Rob Herring wrote:
> Display is a pretty well known use case here. Do you have other
> examples in mind?

No, I don't.

@Stephen: Do you have more cases like this for your Qcom products ?

> Other cases I've seen are automotive with keeping
> the backup camera going and CAN bus handling. Though my new car has a
> flicker shortly after coming on, so I guess the handoff doesn't have
> to be completely seemless. :)

:)

> [...]
> 
> > +       mmc: mmc@0x0 {
> > +               ...
> > +               ...
> > +               vmmc-supply = <&twl_reg1>;
> > +               vmmcaux-supply = <&twl_reg2>;
> > +               boot-constraint-supplies = "vmmc", "vmmcaux";
> > +               boot-constraint-uV = <1800000 2000000>, /* vmmc */
> > +                                    <2000000 2000000>; /* vmmcaux */
> 
> No. I don't like how this is going to extend to all the other bindings
> people are going to want constraints for. We don't need a parallel set
> of properties for each type of binding.

Fair enough.

> I'm not convinced that we need a general solution for what's probably
> a handful of things that need a handoff versus just re-initialize.

What about keeping the first four patches (mostly) as it is and adding
these constraints from a platform specific constraints driver ?

Will that be acceptable ?

-- 
viresh

[toc] | [prev] | [next] | [standalone]

#1689297

On Thu, Jul 13, 2017 at 03:06:08PM +0530, Viresh Kumar wrote:
> On 12-07-17, 16:28, Rob Herring wrote:
> > Display is a pretty well known use case here. Do you have other
> > examples in mind?
> 
> No, I don't.
> 
> @Stephen: Do you have more cases like this for your Qcom products ?
> 
> > Other cases I've seen are automotive with keeping
> > the backup camera going and CAN bus handling. Though my new car has a
> > flicker shortly after coming on, so I guess the handoff doesn't have
> > to be completely seemless. :)
> 
> :)
> 
> > [...]
> > 
> > > +       mmc: mmc@0x0 {
> > > +               ...
> > > +               ...
> > > +               vmmc-supply = <&twl_reg1>;
> > > +               vmmcaux-supply = <&twl_reg2>;
> > > +               boot-constraint-supplies = "vmmc", "vmmcaux";
> > > +               boot-constraint-uV = <1800000 2000000>, /* vmmc */
> > > +                                    <2000000 2000000>; /* vmmcaux */
> > 
> > No. I don't like how this is going to extend to all the other bindings
> > people are going to want constraints for. We don't need a parallel set
> > of properties for each type of binding.
> 
> Fair enough.
> 
> > I'm not convinced that we need a general solution for what's probably
> > a handful of things that need a handoff versus just re-initialize.
> 
> What about keeping the first four patches (mostly) as it is and adding
> these constraints from a platform specific constraints driver ?
> 
> Will that be acceptable ?

Meaning no DT binding? Then I don't care (from a DT perspective).

Rob

[toc] | [prev] | [next] | [standalone]

#1689740

On 17-07-17, 12:34, Rob Herring wrote:
> On Thu, Jul 13, 2017 at 03:06:08PM +0530, Viresh Kumar wrote:
> > On 12-07-17, 16:28, Rob Herring wrote:
> > > Display is a pretty well known use case here. Do you have other
> > > examples in mind?
> > 
> > No, I don't.
> > 
> > @Stephen: Do you have more cases like this for your Qcom products ?
> > 
> > > Other cases I've seen are automotive with keeping
> > > the backup camera going and CAN bus handling. Though my new car has a
> > > flicker shortly after coming on, so I guess the handoff doesn't have
> > > to be completely seemless. :)
> > 
> > :)
> > 
> > > [...]
> > > 
> > > > +       mmc: mmc@0x0 {
> > > > +               ...
> > > > +               ...
> > > > +               vmmc-supply = <&twl_reg1>;
> > > > +               vmmcaux-supply = <&twl_reg2>;
> > > > +               boot-constraint-supplies = "vmmc", "vmmcaux";
> > > > +               boot-constraint-uV = <1800000 2000000>, /* vmmc */
> > > > +                                    <2000000 2000000>; /* vmmcaux */
> > > 
> > > No. I don't like how this is going to extend to all the other bindings
> > > people are going to want constraints for. We don't need a parallel set
> > > of properties for each type of binding.
> > 
> > Fair enough.
> > 
> > > I'm not convinced that we need a general solution for what's probably
> > > a handful of things that need a handoff versus just re-initialize.
> > 
> > What about keeping the first four patches (mostly) as it is and adding
> > these constraints from a platform specific constraints driver ?
> > 
> > Will that be acceptable ?
> 
> Meaning no DT binding? Then I don't care (from a DT perspective).

Yeah, kind of the way we decided to do the first step in the
power domain performance state series [1].

And then later on we can see how to get such information from DT, as
the kernel needs this information irrespective of the way we solve
this problem in the kernel.

-- 
viresh

[1] https://marc.info/?l=linux-kernel&m=149802907711074&w=2

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web