[PATCH 0/2] regulator: get voltage & duty table from dts for st-pwm

[PATCH 0/2] regulator: get voltage & duty table from dts for st-pwm

[Chris Zhong]

get voltage & duty table from device tree might be better, other platforms can also use this
driver without any modify.
Tested on a rk3288 sdk board as logic voltage regulator.


Chris Zhong (2):
  regulator: st-pwm: get voltage and duty table from dts
  dt-bindings: add devicetree bindings for st-pwm regulator

 .../devicetree/bindings/regulator/st-pwm.txt       |   35 +++++++++
 drivers/regulator/Kconfig                          |    1 -
 drivers/regulator/st-pwm.c                         |   80 ++++++++++----------
 3 files changed, 77 insertions(+), 39 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/regulator/st-pwm.txt

-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] dt-bindings: add devicetree bindings for st-pwm regulator

[Chris Zhong]

Document the st-pwm regulator

Signed-off-by: Chris Zhong <zyw@rock-chips.com>

---

 .../devicetree/bindings/regulator/st-pwm.txt       |   35 ++++++++++++++++++++
 1 file changed, 35 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/st-pwm.txt

diff --git a/Documentation/devicetree/bindings/regulator/st-pwm.txt b/Documentation/devicetree/bindings/regulator/st-pwm.txt
new file mode 100644
index 0000000..38fec1d
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/st-pwm.txt
@@ -0,0 +1,35 @@
+st pwm regulator bindings
+
+Required properties:
+  - compatible: "pwm-regulator"
+  - pwms: OF device-tree PWM specification (see PWM binding pwm.txt)
+  - voltage-table: voltage and duty table, include 2 merbers in each set of
+    brackets, first one is voltage(unit: uv), the next is duty(unit: percent)
+  - pwm-reg-period: duration (in nanoseconds) of one cycle
+
+Any property defined as part of the core regulator binding defined in
+regulator.txt can also be used.
+
+Example:
+	pwm_regulator {
+                compatible = "st,b2105-pwm-regulator;
+                pwms = <&pwm1 0 1000000 0>;
+
+		voltage-table = <1114000 0>,
+				<1095000 10>,
+				<1076000 20>,
+				<1056000 30>,
+				<1036000 40>,
+				<1016000 50>;
+
+		pwm-reg-period = <8448>;
+		regulators {
+			vdd_logic: pwm-regulator {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1016000>;
+				regulator-max-microvolt = <1114000>;
+				regulator-name = "vdd_logic";
+			};
+		};
+	};
-- 
1.7.9.5

Re: [PATCH 2/2] dt-bindings: add devicetree bindings for st-pwm regulator

[Doug Anderson]

Chris,

On Wed, Sep 17, 2014 at 6:08 AM, Chris Zhong <zyw-TNX95d0MmH7DzftRWevZcw@public.gmane.org> wrote:
> Document the st-pwm regulator
>
> Signed-off-by: Chris Zhong <zyw-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
>
> ---
>
>  .../devicetree/bindings/regulator/st-pwm.txt       |   35 ++++++++++++++++++++
>  1 file changed, 35 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/regulator/st-pwm.txt
>
> diff --git a/Documentation/devicetree/bindings/regulator/st-pwm.txt b/Documentation/devicetree/bindings/regulator/st-pwm.txt
> new file mode 100644
> index 0000000..38fec1d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/st-pwm.txt
> @@ -0,0 +1,35 @@
> +st pwm regulator bindings
> +
> +Required properties:
> +  - compatible: "pwm-regulator"

This compatible string doesn't include "st,b2105-pwm-regulator".

Should be something like:

  - compatible: Should be "pwm-regulator" to get voltage table / regulator
    period from the device tree.  Deprecated: if "st,b2105-pwm-regulator" then
    voltage table and regulator will be handled by the driver.

Assuming that everyone is OK calling "st,b2105-pwm-regulator" the
deprecated way of doing things.


> +  - pwms: OF device-tree PWM specification (see PWM binding pwm.txt)
> +  - voltage-table: voltage and duty table, include 2 merbers in each set of
> +    brackets, first one is voltage(unit: uv), the next is duty(unit: percent)
> +  - pwm-reg-period: duration (in nanoseconds) of one cycle

The voltage-table and pwm-reg-period should not be required if we're
using "st,b2105-pwm-regulator".  If someone lists both
"st,b2105-pwm-regulator" and "pwm-regulator" then I'd assume that
you'd allow them to override via the device tree but fallback to the
old hardcoded values.


> +
> +Any property defined as part of the core regulator binding defined in
> +regulator.txt can also be used.
> +
> +Example:
> +       pwm_regulator {
> +                compatible = "st,b2105-pwm-regulator;
> +                pwms = <&pwm1 0 1000000 0>;
> +
> +               voltage-table = <1114000 0>,
> +                               <1095000 10>,
> +                               <1076000 20>,
> +                               <1056000 30>,
> +                               <1036000 40>,
> +                               <1016000 50>;
> +
> +               pwm-reg-period = <8448>;
> +               regulators {
> +                       vdd_logic: pwm-regulator {
> +                               regulator-always-on;
> +                               regulator-boot-on;
> +                               regulator-min-microvolt = <1016000>;
> +                               regulator-max-microvolt = <1114000>;
> +                               regulator-name = "vdd_logic";
> +                       };
> +               };

I _think_ that the "regulators" subnode and the "pwm-regulator"
subnode are not needed at all and should be removed.  Other instances
of devices that are "just" regulators don't have it (like
fixed-regulator, gpio-regulator, etc).

I think your final example should be:

+       vdd_logic: pwm-regulator {
+                compatible = "pwm-regulator;
+                pwms = <&pwm1 0 1000000 0>;
+
+               voltage-table = <1114000 0>,
+                               <1095000 10>,
+                               <1076000 20>,
+                               <1056000 30>,
+                               <1036000 40>,
+                               <1016000 50>;
+
+               pwm-reg-period = <8448>;
+
+               regulator-always-on;
+               regulator-boot-on;
+               regulator-min-microvolt = <1016000>;
+               regulator-max-microvolt = <1114000>;
+               regulator-name = "vdd_logic";
+       };

-Doug
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] dt-bindings: add devicetree bindings for st-pwm regulator

[Mark Brown]

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

On Wed, Sep 17, 2014 at 11:16:23AM -0700, Doug Anderson wrote:

> I _think_ that the "regulators" subnode and the "pwm-regulator"
> subnode are not needed at all and should be removed.  Other instances
> of devices that are "just" regulators don't have it (like
> fixed-regulator, gpio-regulator, etc).
> 
> I think your final example should be:

Yes, please.

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

Re: [PATCH 2/2] dt-bindings: add devicetree bindings for st-pwm regulator

[Mark Rutland]

On Wed, Sep 17, 2014 at 07:16:23PM +0100, Doug Anderson wrote:
> Chris,
> 
> On Wed, Sep 17, 2014 at 6:08 AM, Chris Zhong <zyw-TNX95d0MmH7DzftRWevZcw@public.gmane.org> wrote:
> > Document the st-pwm regulator
> >
> > Signed-off-by: Chris Zhong <zyw-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> >
> > ---
> >
> >  .../devicetree/bindings/regulator/st-pwm.txt       |   35 ++++++++++++++++++++
> >  1 file changed, 35 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/regulator/st-pwm.txt
> >
> > diff --git a/Documentation/devicetree/bindings/regulator/st-pwm.txt b/Documentation/devicetree/bindings/regulator/st-pwm.txt
> > new file mode 100644
> > index 0000000..38fec1d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/regulator/st-pwm.txt
> > @@ -0,0 +1,35 @@
> > +st pwm regulator bindings
> > +
> > +Required properties:
> > +  - compatible: "pwm-regulator"
> 
> This compatible string doesn't include "st,b2105-pwm-regulator".
> 
> Should be something like:
> 
>   - compatible: Should be "pwm-regulator" to get voltage table / regulator
>     period from the device tree.  Deprecated: if "st,b2105-pwm-regulator" then
>     voltage table and regulator will be handled by the driver.

Do a list, with a bullet point for each string, like:

- compatible: should contain:
  * "pwm-regulator" when using voltage-table
  * "st,b2105-pwm-regulator" for .... (DEPRECATED)

That's easier to extend and it's clearer as to which strings the
comments above apply to.

> 
> Assuming that everyone is OK calling "st,b2105-pwm-regulator" the
> deprecated way of doing things.
> 
> 
> > +  - pwms: OF device-tree PWM specification (see PWM binding pwm.txt)
> > +  - voltage-table: voltage and duty table, include 2 merbers in each set of
> > +    brackets, first one is voltage(unit: uv), the next is duty(unit: percent)
> > +  - pwm-reg-period: duration (in nanoseconds) of one cycle

Perhaps just 'period-ns'? We know it applies to the PWM we're using as a
regulator.

That said, is this even needed? The pwm bindings describe that the
period would typically be described in the pwm-specifier, so duplicating
that feels wrong.

> 
> The voltage-table and pwm-reg-period should not be required if we're
> using "st,b2105-pwm-regulator".  If someone lists both
> "st,b2105-pwm-regulator" and "pwm-regulator" then I'd assume that
> you'd allow them to override via the device tree but fallback to the
> old hardcoded values.
> 
> 
> > +
> > +Any property defined as part of the core regulator binding defined in
> > +regulator.txt can also be used.
> > +
> > +Example:
> > +       pwm_regulator {
> > +                compatible = "st,b2105-pwm-regulator;
> > +                pwms = <&pwm1 0 1000000 0>;
> > +
> > +               voltage-table = <1114000 0>,
> > +                               <1095000 10>,
> > +                               <1076000 20>,
> > +                               <1056000 30>,
> > +                               <1036000 40>,
> > +                               <1016000 50>;
> > +
> > +               pwm-reg-period = <8448>;
> > +               regulators {
> > +                       vdd_logic: pwm-regulator {
> > +                               regulator-always-on;
> > +                               regulator-boot-on;
> > +                               regulator-min-microvolt = <1016000>;
> > +                               regulator-max-microvolt = <1114000>;
> > +                               regulator-name = "vdd_logic";
> > +                       };
> > +               };
> 
> I _think_ that the "regulators" subnode and the "pwm-regulator"
> subnode are not needed at all and should be removed.  Other instances
> of devices that are "just" regulators don't have it (like
> fixed-regulator, gpio-regulator, etc).

Yes. The pwm_regulator node _is_ the regulator, so the subnode is
bizarre.

Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] dt-bindings: add devicetree bindings for st-pwm regulator

[Lee Jones]

On Wed, 17 Sep 2014, Doug Anderson wrote:
> On Wed, Sep 17, 2014 at 6:08 AM, Chris Zhong <zyw@rock-chips.com> wrote:
> > Document the st-pwm regulator
> >
> > Signed-off-by: Chris Zhong <zyw@rock-chips.com>
> >
> > ---
> >
> >  .../devicetree/bindings/regulator/st-pwm.txt       |   35 ++++++++++++++++++++
> >  1 file changed, 35 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/regulator/st-pwm.txt
> >
> > diff --git a/Documentation/devicetree/bindings/regulator/st-pwm.txt b/Documentation/devicetree/bindings/regulator/st-pwm.txt
> > new file mode 100644
> > index 0000000..38fec1d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/regulator/st-pwm.txt
> > @@ -0,0 +1,35 @@
> > +st pwm regulator bindings
> > +
> > +Required properties:
> > +  - compatible: "pwm-regulator"
> 
> This compatible string doesn't include "st,b2105-pwm-regulator".
> 
> Should be something like:
> 
>   - compatible: Should be "pwm-regulator" to get voltage table / regulator
>     period from the device tree.  Deprecated: if "st,b2105-pwm-regulator" then
>     voltage table and regulator will be handled by the driver.
> 
> Assuming that everyone is OK calling "st,b2105-pwm-regulator" the
> deprecated way of doing things.

It's okay not to deprecate "st,b2105-pwm-regulator" as there have
never been any users.  The only DTS file with a reference to it is in
my local repo.  Just rename it.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog