[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

[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 1/2] regulator: st-pwm: get voltage and duty table from dts

[Chris Zhong]

Get voltage & duty table from device tree might be better, other platforms can also use this
driver without any modify.

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

 drivers/regulator/Kconfig  |    1 -
 drivers/regulator/st-pwm.c |   80 +++++++++++++++++++++++---------------------
 2 files changed, 42 insertions(+), 39 deletions(-)

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index fb32bab..06a9632 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -495,7 +495,6 @@ config REGULATOR_S5M8767
 
 config REGULATOR_ST_PWM
 	tristate "STMicroelectronics PWM voltage regulator"
-	depends on ARCH_STI
 	help
 	 This driver supports ST's PWM controlled voltage regulators.
 
diff --git a/drivers/regulator/st-pwm.c b/drivers/regulator/st-pwm.c
index 5ea78df..877381b 100644
--- a/drivers/regulator/st-pwm.c
+++ b/drivers/regulator/st-pwm.c
@@ -20,16 +20,11 @@
 #include <linux/of_device.h>
 #include <linux/pwm.h>
 
-#define ST_PWM_REG_PERIOD 8448
-
-struct st_pwm_regulator_pdata {
-	const struct regulator_desc *desc;
-	struct st_pwm_voltages *duty_cycle_table;
-};
-
 struct st_pwm_regulator_data {
-	const struct st_pwm_regulator_pdata *pdata;
+	struct regulator_desc *desc;
+	struct st_pwm_voltages *duty_cycle_table;
 	struct pwm_device *pwm;
+	int pwm_reg_period;
 	bool enabled;
 	int state;
 };
@@ -53,10 +48,10 @@ static int st_pwm_regulator_set_voltage_sel(struct regulator_dev *dev,
 	int dutycycle;
 	int ret;
 
-	dutycycle = (ST_PWM_REG_PERIOD / 100) *
-		drvdata->pdata->duty_cycle_table[selector].dutycycle;
+	dutycycle = (drvdata->pwm_reg_period / 100) *
+		    drvdata->duty_cycle_table[selector].dutycycle;
 
-	ret = pwm_config(drvdata->pwm, dutycycle, ST_PWM_REG_PERIOD);
+	ret = pwm_config(drvdata->pwm, dutycycle, drvdata->pwm_reg_period);
 	if (ret) {
 		dev_err(&dev->dev, "Failed to configure PWM\n");
 		return ret;
@@ -84,7 +79,7 @@ static int st_pwm_regulator_list_voltage(struct regulator_dev *dev,
 	if (selector >= dev->desc->n_voltages)
 		return -EINVAL;
 
-	return drvdata->pdata->duty_cycle_table[selector].uV;
+	return drvdata->duty_cycle_table[selector].uV;
 }
 
 static struct regulator_ops st_pwm_regulator_voltage_ops = {
@@ -94,32 +89,16 @@ static struct regulator_ops st_pwm_regulator_voltage_ops = {
 	.map_voltage     = regulator_map_voltage_iterate,
 };
 
-static struct st_pwm_voltages b2105_duty_cycle_table[] = {
-	{ .uV = 1114000, .dutycycle = 0,  },
-	{ .uV = 1095000, .dutycycle = 10, },
-	{ .uV = 1076000, .dutycycle = 20, },
-	{ .uV = 1056000, .dutycycle = 30, },
-	{ .uV = 1036000, .dutycycle = 40, },
-	{ .uV = 1016000, .dutycycle = 50, },
-	/* WARNING: Values above 50% duty-cycle cause boot failures. */
-};
-
-static const struct regulator_desc b2105_desc = {
+static struct regulator_desc b2105_desc = {
 	.name		= "b2105-pwm-regulator",
 	.ops		= &st_pwm_regulator_voltage_ops,
 	.type		= REGULATOR_VOLTAGE,
 	.owner		= THIS_MODULE,
-	.n_voltages	= ARRAY_SIZE(b2105_duty_cycle_table),
 	.supply_name    = "pwm",
 };
 
-static const struct st_pwm_regulator_pdata b2105_info = {
-	.desc		  = &b2105_desc,
-	.duty_cycle_table = b2105_duty_cycle_table,
-};
-
 static const struct of_device_id st_pwm_of_match[] = {
-	{ .compatible = "st,b2105-pwm-regulator", .data = &b2105_info, },
+	{ .compatible = "st,b2105-pwm-regulator" },
 	{ },
 };
 MODULE_DEVICE_TABLE(of, st_pwm_of_match);
@@ -127,10 +106,11 @@ MODULE_DEVICE_TABLE(of, st_pwm_of_match);
 static int st_pwm_regulator_probe(struct platform_device *pdev)
 {
 	struct st_pwm_regulator_data *drvdata;
+	struct property *prop;
 	struct regulator_dev *regulator;
 	struct regulator_config config = { };
 	struct device_node *np = pdev->dev.of_node;
-	const struct of_device_id *of_match;
+	int length, ret;
 
 	if (!np) {
 		dev_err(&pdev->dev, "Device Tree node missing\n");
@@ -141,12 +121,36 @@ static int st_pwm_regulator_probe(struct platform_device *pdev)
 	if (!drvdata)
 		return -ENOMEM;
 
-	of_match = of_match_device(st_pwm_of_match, &pdev->dev);
-	if (!of_match) {
-		dev_err(&pdev->dev, "failed to match of device\n");
-		return -ENODEV;
+	drvdata->desc = &b2105_desc;
+
+	/* determine the number of voltage-table */
+	prop = of_find_property(np, "voltage-table", &length);
+	if (!prop)
+		return -EINVAL;
+
+	drvdata->desc->n_voltages = length / sizeof(*drvdata->duty_cycle_table);
+
+	/* read voltage table from DT property */
+	if (length > 0) {
+		size_t size = sizeof(*drvdata->duty_cycle_table) *
+			      drvdata->desc->n_voltages;
+
+		drvdata->duty_cycle_table = devm_kzalloc(&pdev->dev,
+							 size, GFP_KERNEL);
+		if (!drvdata->duty_cycle_table)
+			return -ENOMEM;
+
+		ret = of_property_read_u32_array(np, "voltage-table",
+				(u32 *)drvdata->duty_cycle_table,
+				length / sizeof(u32));
+		if (ret < 0)
+			return ret;
 	}
-	drvdata->pdata = of_match->data;
+
+	ret = of_property_read_u32(np, "pwm-reg-period",
+				   &drvdata->pwm_reg_period);
+	if (ret)
+		return -EINVAL;
 
 	config.init_data = of_get_regulator_init_data(&pdev->dev, np);
 	if (!config.init_data)
@@ -163,10 +167,10 @@ static int st_pwm_regulator_probe(struct platform_device *pdev)
 	}
 
 	regulator = devm_regulator_register(&pdev->dev,
-					    drvdata->pdata->desc, &config);
+					    drvdata->desc, &config);
 	if (IS_ERR(regulator)) {
 		dev_err(&pdev->dev, "Failed to register regulator %s\n",
-			drvdata->pdata->desc->name);
+			drvdata->desc->name);
 		return PTR_ERR(regulator);
 	}
 
-- 
1.7.9.5

[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 1/2] regulator: st-pwm: get voltage and duty table from dts

[Heiko Stübner]

Hi Chris,

Am Mittwoch, 17. September 2014, 21:07:59 schrieb Chris Zhong:
> Get voltage & duty table from device tree might be better, other platforms
> can also use this driver without any modify.
> 
> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
> ---
> 
>  drivers/regulator/Kconfig  |    1 -
>  drivers/regulator/st-pwm.c |   80
> +++++++++++++++++++++++--------------------- 2 files changed, 42
> insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
> index fb32bab..06a9632 100644
> --- a/drivers/regulator/Kconfig
> +++ b/drivers/regulator/Kconfig
> @@ -495,7 +495,6 @@ config REGULATOR_S5M8767
> 
>  config REGULATOR_ST_PWM
>  	tristate "STMicroelectronics PWM voltage regulator"
> -	depends on ARCH_STI
>  	help
>  	 This driver supports ST's PWM controlled voltage regulators.
> 
> diff --git a/drivers/regulator/st-pwm.c b/drivers/regulator/st-pwm.c
> index 5ea78df..877381b 100644
> --- a/drivers/regulator/st-pwm.c
> +++ b/drivers/regulator/st-pwm.c
> @@ -20,16 +20,11 @@
>  #include <linux/of_device.h>
>  #include <linux/pwm.h>
> 
> -#define ST_PWM_REG_PERIOD 8448
> -
> -struct st_pwm_regulator_pdata {
> -	const struct regulator_desc *desc;
> -	struct st_pwm_voltages *duty_cycle_table;
> -};
> -
>  struct st_pwm_regulator_data {
> -	const struct st_pwm_regulator_pdata *pdata;
> +	struct regulator_desc *desc;
> +	struct st_pwm_voltages *duty_cycle_table;
>  	struct pwm_device *pwm;
> +	int pwm_reg_period;
>  	bool enabled;
>  	int state;
>  };
> @@ -53,10 +48,10 @@ static int st_pwm_regulator_set_voltage_sel(struct
> regulator_dev *dev, int dutycycle;
>  	int ret;
> 
> -	dutycycle = (ST_PWM_REG_PERIOD / 100) *
> -		drvdata->pdata->duty_cycle_table[selector].dutycycle;
> +	dutycycle = (drvdata->pwm_reg_period / 100) *
> +		    drvdata->duty_cycle_table[selector].dutycycle;
> 
> -	ret = pwm_config(drvdata->pwm, dutycycle, ST_PWM_REG_PERIOD);
> +	ret = pwm_config(drvdata->pwm, dutycycle, drvdata->pwm_reg_period);
>  	if (ret) {
>  		dev_err(&dev->dev, "Failed to configure PWM\n");
>  		return ret;
> @@ -84,7 +79,7 @@ static int st_pwm_regulator_list_voltage(struct
> regulator_dev *dev, if (selector >= dev->desc->n_voltages)
>  		return -EINVAL;
> 
> -	return drvdata->pdata->duty_cycle_table[selector].uV;
> +	return drvdata->duty_cycle_table[selector].uV;
>  }
> 
>  static struct regulator_ops st_pwm_regulator_voltage_ops = {
> @@ -94,32 +89,16 @@ static struct regulator_ops st_pwm_regulator_voltage_ops
> = { .map_voltage     = regulator_map_voltage_iterate,
>  };
> 
> -static struct st_pwm_voltages b2105_duty_cycle_table[] = {
> -	{ .uV = 1114000, .dutycycle = 0,  },
> -	{ .uV = 1095000, .dutycycle = 10, },
> -	{ .uV = 1076000, .dutycycle = 20, },
> -	{ .uV = 1056000, .dutycycle = 30, },
> -	{ .uV = 1036000, .dutycycle = 40, },
> -	{ .uV = 1016000, .dutycycle = 50, },
> -	/* WARNING: Values above 50% duty-cycle cause boot failures. */
> -};
> -
> -static const struct regulator_desc b2105_desc = {
> +static struct regulator_desc b2105_desc = {
>  	.name		= "b2105-pwm-regulator",
>  	.ops		= &st_pwm_regulator_voltage_ops,
>  	.type		= REGULATOR_VOLTAGE,
>  	.owner		= THIS_MODULE,
> -	.n_voltages	= ARRAY_SIZE(b2105_duty_cycle_table),
>  	.supply_name    = "pwm",
>  };
> 
> -static const struct st_pwm_regulator_pdata b2105_info = {
> -	.desc		  = &b2105_desc,
> -	.duty_cycle_table = b2105_duty_cycle_table,
> -};
> -
>  static const struct of_device_id st_pwm_of_match[] = {
> -	{ .compatible = "st,b2105-pwm-regulator", .data = &b2105_info, },
> +	{ .compatible = "st,b2105-pwm-regulator" },

This is breaking compatibility with the existing binding for the b2105-pwm-
regulator - which has to stay intact. Please add a separate compatiblity like 
"pwm-regulator" that reads the table from the dt and let the b2105 keep it's 
voltage<->duty_cycle table in the driver as it is now.


Heiko


>  	{ },
>  };
>  MODULE_DEVICE_TABLE(of, st_pwm_of_match);
> @@ -127,10 +106,11 @@ MODULE_DEVICE_TABLE(of, st_pwm_of_match);
>  static int st_pwm_regulator_probe(struct platform_device *pdev)
>  {
>  	struct st_pwm_regulator_data *drvdata;
> +	struct property *prop;
>  	struct regulator_dev *regulator;
>  	struct regulator_config config = { };
>  	struct device_node *np = pdev->dev.of_node;
> -	const struct of_device_id *of_match;
> +	int length, ret;
> 
>  	if (!np) {
>  		dev_err(&pdev->dev, "Device Tree node missing\n");
> @@ -141,12 +121,36 @@ static int st_pwm_regulator_probe(struct
> platform_device *pdev) if (!drvdata)
>  		return -ENOMEM;
> 
> -	of_match = of_match_device(st_pwm_of_match, &pdev->dev);
> -	if (!of_match) {
> -		dev_err(&pdev->dev, "failed to match of device\n");
> -		return -ENODEV;
> +	drvdata->desc = &b2105_desc;
> +
> +	/* determine the number of voltage-table */
> +	prop = of_find_property(np, "voltage-table", &length);
> +	if (!prop)
> +		return -EINVAL;
> +
> +	drvdata->desc->n_voltages = length / sizeof(*drvdata->duty_cycle_table);
> +
> +	/* read voltage table from DT property */
> +	if (length > 0) {
> +		size_t size = sizeof(*drvdata->duty_cycle_table) *
> +			      drvdata->desc->n_voltages;
> +
> +		drvdata->duty_cycle_table = devm_kzalloc(&pdev->dev,
> +							 size, GFP_KERNEL);
> +		if (!drvdata->duty_cycle_table)
> +			return -ENOMEM;
> +
> +		ret = of_property_read_u32_array(np, "voltage-table",
> +				(u32 *)drvdata->duty_cycle_table,
> +				length / sizeof(u32));
> +		if (ret < 0)
> +			return ret;
>  	}
> -	drvdata->pdata = of_match->data;
> +
> +	ret = of_property_read_u32(np, "pwm-reg-period",
> +				   &drvdata->pwm_reg_period);
> +	if (ret)
> +		return -EINVAL;
> 
>  	config.init_data = of_get_regulator_init_data(&pdev->dev, np);
>  	if (!config.init_data)
> @@ -163,10 +167,10 @@ static int st_pwm_regulator_probe(struct
> platform_device *pdev) }
> 
>  	regulator = devm_regulator_register(&pdev->dev,
> -					    drvdata->pdata->desc, &config);
> +					    drvdata->desc, &config);
>  	if (IS_ERR(regulator)) {
>  		dev_err(&pdev->dev, "Failed to register regulator %s\n",
> -			drvdata->pdata->desc->name);
> +			drvdata->desc->name);
>  		return PTR_ERR(regulator);
>  	}

Re: [PATCH 1/2] regulator: st-pwm: get voltage and duty table from dts

[Mark Brown]

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

On Wed, Sep 17, 2014 at 09:07:59PM +0800, Chris Zhong wrote:
> Get voltage & duty table from device tree might be better, other platforms can also use this
> driver without any modify.
> 
> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
> ---
> 
>  drivers/regulator/Kconfig  |    1 -
>  drivers/regulator/st-pwm.c |   80 +++++++++++++++++++++++---------------------

You need to update the binding document if you're updating the DT
bindings.

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

Re: [PATCH 1/2] regulator: st-pwm: get voltage and duty table from dts

[Doug Anderson]

Hi,

On Wed, Sep 17, 2014 at 9:51 AM, Mark Brown <broonie@kernel.org> wrote:
> On Wed, Sep 17, 2014 at 09:07:59PM +0800, Chris Zhong wrote:
>> Get voltage & duty table from device tree might be better, other platforms can also use this
>> driver without any modify.
>>
>> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
>> ---
>>
>>  drivers/regulator/Kconfig  |    1 -
>>  drivers/regulator/st-pwm.c |   80 +++++++++++++++++++++++---------------------
>
> You need to update the binding document if you're updating the DT
> bindings.

Mark: See part 2 <https://patchwork.kernel.org/patch/4924381/>.
Unfortunately you weren't CCed on that.  It also should have been
patch #1, not patch #2.

Chris: please change the order and make sure Mark is CCed on the
bindings.  Probably anyone CCed on the code change should be on the
bindings change (and probably vice versa, but maybe not as critical).

-Doug

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


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

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 1/2] regulator: st-pwm: get voltage and duty table from dts

[Doug Anderson]

Chris,

On Wed, Sep 17, 2014 at 6:07 AM, Chris Zhong <zyw@rock-chips.com> wrote:
> Get voltage & duty table from device tree might be better, other platforms can also use this
> driver without any modify.
>
> Signed-off-by: Chris Zhong <zyw@rock-chips.com>

Why didn't you CC Lee Jones on your patches?  He's the author of the
driver and should certainly be involved in reviewing your patches.

CCed him now.


>  drivers/regulator/Kconfig  |    1 -
>  drivers/regulator/st-pwm.c |   80 +++++++++++++++++++++++---------------------
>  2 files changed, 42 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
> index fb32bab..06a9632 100644
> --- a/drivers/regulator/Kconfig
> +++ b/drivers/regulator/Kconfig
> @@ -495,7 +495,6 @@ config REGULATOR_S5M8767
>
>  config REGULATOR_ST_PWM
>         tristate "STMicroelectronics PWM voltage regulator"
> -       depends on ARCH_STI

Seems like you should add some wording saying that this driver is also
useful for other PWM-based voltage regulators.


>         help
>          This driver supports ST's PWM controlled voltage regulators.
>
> diff --git a/drivers/regulator/st-pwm.c b/drivers/regulator/st-pwm.c
> index 5ea78df..877381b 100644
> --- a/drivers/regulator/st-pwm.c
> +++ b/drivers/regulator/st-pwm.c
> @@ -20,16 +20,11 @@
>  #include <linux/of_device.h>
>  #include <linux/pwm.h>
>
> -#define ST_PWM_REG_PERIOD 8448
> -
> -struct st_pwm_regulator_pdata {
> -       const struct regulator_desc *desc;
> -       struct st_pwm_voltages *duty_cycle_table;
> -};
> -
>  struct st_pwm_regulator_data {
> -       const struct st_pwm_regulator_pdata *pdata;
> +       struct regulator_desc *desc;
> +       struct st_pwm_voltages *duty_cycle_table;
>         struct pwm_device *pwm;
> +       int pwm_reg_period;
>         bool enabled;
>         int state;
>  };
> @@ -53,10 +48,10 @@ static int st_pwm_regulator_set_voltage_sel(struct regulator_dev *dev,
>         int dutycycle;
>         int ret;
>
> -       dutycycle = (ST_PWM_REG_PERIOD / 100) *
> -               drvdata->pdata->duty_cycle_table[selector].dutycycle;
> +       dutycycle = (drvdata->pwm_reg_period / 100) *
> +                   drvdata->duty_cycle_table[selector].dutycycle;
>
> -       ret = pwm_config(drvdata->pwm, dutycycle, ST_PWM_REG_PERIOD);
> +       ret = pwm_config(drvdata->pwm, dutycycle, drvdata->pwm_reg_period);
>         if (ret) {
>                 dev_err(&dev->dev, "Failed to configure PWM\n");
>                 return ret;
> @@ -84,7 +79,7 @@ static int st_pwm_regulator_list_voltage(struct regulator_dev *dev,
>         if (selector >= dev->desc->n_voltages)
>                 return -EINVAL;
>
> -       return drvdata->pdata->duty_cycle_table[selector].uV;
> +       return drvdata->duty_cycle_table[selector].uV;
>  }
>
>  static struct regulator_ops st_pwm_regulator_voltage_ops = {
> @@ -94,32 +89,16 @@ static struct regulator_ops st_pwm_regulator_voltage_ops = {
>         .map_voltage     = regulator_map_voltage_iterate,
>  };
>
> -static struct st_pwm_voltages b2105_duty_cycle_table[] = {
> -       { .uV = 1114000, .dutycycle = 0,  },
> -       { .uV = 1095000, .dutycycle = 10, },
> -       { .uV = 1076000, .dutycycle = 20, },
> -       { .uV = 1056000, .dutycycle = 30, },
> -       { .uV = 1036000, .dutycycle = 40, },
> -       { .uV = 1016000, .dutycycle = 50, },
> -       /* WARNING: Values above 50% duty-cycle cause boot failures. */

I wonder if this warning should be transferred to the device tree (if
it's generally a problem for anyone using the b2105)?


> -};
> -
> -static const struct regulator_desc b2105_desc = {
> +static struct regulator_desc b2105_desc = {
>         .name           = "b2105-pwm-regulator",
>         .ops            = &st_pwm_regulator_voltage_ops,
>         .type           = REGULATOR_VOLTAGE,
>         .owner          = THIS_MODULE,
> -       .n_voltages     = ARRAY_SIZE(b2105_duty_cycle_table),
>         .supply_name    = "pwm",
>  };
>
> -static const struct st_pwm_regulator_pdata b2105_info = {
> -       .desc             = &b2105_desc,
> -       .duty_cycle_table = b2105_duty_cycle_table,
> -};
> -
>  static const struct of_device_id st_pwm_of_match[] = {
> -       { .compatible = "st,b2105-pwm-regulator", .data = &b2105_info, },
> +       { .compatible = "st,b2105-pwm-regulator" },
>         { },
>  };
>  MODULE_DEVICE_TABLE(of, st_pwm_of_match);
> @@ -127,10 +106,11 @@ MODULE_DEVICE_TABLE(of, st_pwm_of_match);
>  static int st_pwm_regulator_probe(struct platform_device *pdev)
>  {
>         struct st_pwm_regulator_data *drvdata;
> +       struct property *prop;
>         struct regulator_dev *regulator;
>         struct regulator_config config = { };
>         struct device_node *np = pdev->dev.of_node;
> -       const struct of_device_id *of_match;
> +       int length, ret;
>
>         if (!np) {
>                 dev_err(&pdev->dev, "Device Tree node missing\n");
> @@ -141,12 +121,36 @@ static int st_pwm_regulator_probe(struct platform_device *pdev)
>         if (!drvdata)
>                 return -ENOMEM;
>
> -       of_match = of_match_device(st_pwm_of_match, &pdev->dev);
> -       if (!of_match) {
> -               dev_err(&pdev->dev, "failed to match of device\n");
> -               return -ENODEV;
> +       drvdata->desc = &b2105_desc;
> +
> +       /* determine the number of voltage-table */
> +       prop = of_find_property(np, "voltage-table", &length);
> +       if (!prop)
> +               return -EINVAL;
> +
> +       drvdata->desc->n_voltages = length / sizeof(*drvdata->duty_cycle_table);
> +
> +       /* read voltage table from DT property */
> +       if (length > 0) {
> +               size_t size = sizeof(*drvdata->duty_cycle_table) *
> +                             drvdata->desc->n_voltages;
> +
> +               drvdata->duty_cycle_table = devm_kzalloc(&pdev->dev,
> +                                                        size, GFP_KERNEL);
> +               if (!drvdata->duty_cycle_table)
> +                       return -ENOMEM;
> +
> +               ret = of_property_read_u32_array(np, "voltage-table",
> +                               (u32 *)drvdata->duty_cycle_table,
> +                               length / sizeof(u32));
> +               if (ret < 0)
> +                       return ret;
>         }
> -       drvdata->pdata = of_match->data;
> +
> +       ret = of_property_read_u32(np, "pwm-reg-period",
> +                                  &drvdata->pwm_reg_period);
> +       if (ret)
> +               return -EINVAL;
>
>         config.init_data = of_get_regulator_init_data(&pdev->dev, np);
>         if (!config.init_data)
> @@ -163,10 +167,10 @@ static int st_pwm_regulator_probe(struct platform_device *pdev)
>         }
>
>         regulator = devm_regulator_register(&pdev->dev,
> -                                           drvdata->pdata->desc, &config);
> +                                           drvdata->desc, &config);
>         if (IS_ERR(regulator)) {
>                 dev_err(&pdev->dev, "Failed to register regulator %s\n",
> -                       drvdata->pdata->desc->name);
> +                       drvdata->desc->name);
>                 return PTR_ERR(regulator);
>         }

-Doug

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

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.

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

Re: [PATCH 1/2] regulator: st-pwm: get voltage and duty table from dts

[Lee Jones]

On Wed, 17 Sep 2014, Doug Anderson wrote:
> On Wed, Sep 17, 2014 at 6:07 AM, Chris Zhong <zyw@rock-chips.com> wrote:
> > Get voltage & duty table from device tree might be better, other platforms can also use this
> > driver without any modify.
> >
> > Signed-off-by: Chris Zhong <zyw@rock-chips.com>
> 
> Why didn't you CC Lee Jones on your patches?  He's the author of the
> driver and should certainly be involved in reviewing your patches.
> 
> CCed him now.

Thanks Doug.

> >  drivers/regulator/Kconfig  |    1 -
> >  drivers/regulator/st-pwm.c |   80 +++++++++++++++++++++++---------------------
> >  2 files changed, 42 insertions(+), 39 deletions(-)
> >
> > diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
> > index fb32bab..06a9632 100644
> > --- a/drivers/regulator/Kconfig
> > +++ b/drivers/regulator/Kconfig
> > @@ -495,7 +495,6 @@ config REGULATOR_S5M8767
> >
> >  config REGULATOR_ST_PWM
> >         tristate "STMicroelectronics PWM voltage regulator"

It's okay to generify the driver and remove references to ST.

> > -       depends on ARCH_STI
> 
> Seems like you should add some wording saying that this driver is also
> useful for other PWM-based voltage regulators.

In the commit message yes, I wouldn't worry about doing that here.

> >         help
> >          This driver supports ST's PWM controlled voltage regulators.
> >
> > diff --git a/drivers/regulator/st-pwm.c b/drivers/regulator/st-pwm.c
> > index 5ea78df..877381b 100644
> > --- a/drivers/regulator/st-pwm.c
> > +++ b/drivers/regulator/st-pwm.c

In order to avoid confusion to future users, rename the file too.

[...]

> > -static const struct regulator_desc b2105_desc = {
> > +static struct regulator_desc b2105_desc = {
> >         .name           = "b2105-pwm-regulator",

Generify.

> >         .ops            = &st_pwm_regulator_voltage_ops,
> >         .type           = REGULATOR_VOLTAGE,
> >         .owner          = THIS_MODULE,
> > -       .n_voltages     = ARRAY_SIZE(b2105_duty_cycle_table),
> >         .supply_name    = "pwm",
> >  };
> >
> > -static const struct st_pwm_regulator_pdata b2105_info = {
> > -       .desc             = &b2105_desc,
> > -       .duty_cycle_table = b2105_duty_cycle_table,
> > -};
> > -
> >  static const struct of_device_id st_pwm_of_match[] = {
> > -       { .compatible = "st,b2105-pwm-regulator", .data = &b2105_info, },
> > +       { .compatible = "st,b2105-pwm-regulator" },

Remove and replace with "pwm-regulator".

As we're no longer matching, you can move this down to the bottom.

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

Re: [PATCH 1/2] regulator: st-pwm: get voltage and duty table from dts

[Chris Zhong]

On 09/18/2014 12:54 AM, Doug Anderson wrote:
> Hi,
>
> On Wed, Sep 17, 2014 at 9:51 AM, Mark Brown <broonie@kernel.org> wrote:
>> On Wed, Sep 17, 2014 at 09:07:59PM +0800, Chris Zhong wrote:
>>> Get voltage & duty table from device tree might be better, other platforms can also use this
>>> driver without any modify.
>>>
>>> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
>>> ---
>>>
>>>   drivers/regulator/Kconfig  |    1 -
>>>   drivers/regulator/st-pwm.c |   80 +++++++++++++++++++++++---------------------
>> You need to update the binding document if you're updating the DT
>> bindings.
> Mark: See part 2 <https://patchwork.kernel.org/patch/4924381/>.
> Unfortunately you weren't CCed on that.  It also should have been
> patch #1, not patch #2.
>
> Chris: please change the order and make sure Mark is CCed on the
> bindings.  Probably anyone CCed on the code change should be on the
> bindings change (and probably vice versa, but maybe not as critical).
>
> -Doug
>
>
Thanks.
I should not only use patman's script to created cc list.

Re: [PATCH 1/2] regulator: st-pwm: get voltage and duty table from dts

[Doug Anderson]

Chris,

On Wed, Sep 17, 2014 at 6:37 PM, Chris Zhong <zyw@rock-chips.com> wrote:
>> Chris: please change the order and make sure Mark is CCed on the
>> bindings.  Probably anyone CCed on the code change should be on the
>> bindings change (and probably vice versa, but maybe not as critical).
>>
>> -Doug
>>
>>
> Thanks.
> I should not only use patman's script to created cc list.

Yup, patman relies on get_maintainer and that's not perfect.  I tend
to rely heavily on it but it's good to sanity check...


Speaking of which: Lee, do you think it would be a good idea to add
yourself as a MAINTAINER for st-pwm?

  (next-20140917))$ ./scripts/get_maintainer.pl -f drivers/regulator/st-pwm.c
  Liam Girdwood <lgirdwood@gmail.com> (supporter:VOLTAGE AND CURRE...)
  Mark Brown <broonie@kernel.org> (supporter:VOLTAGE AND CURRE...)
  Grant Likely <grant.likely@linaro.org> (maintainer:OPEN FIRMWARE AND...)
  Rob Herring <robh+dt@kernel.org> (maintainer:OPEN FIRMWARE AND...)
  linux-kernel@vger.kernel.org (open list:VOLTAGE AND CURRE...)
  devicetree@vger.kernel.org (open list:OPEN FIRMWARE AND...)


...ideally whatever rule added would also catch the bindings file when
Chris adds it.


-Doug