* Re: [PATCH 1/2] regulator: st-pwm: get voltage and duty table from dts
2014-09-17 13:07 ` [PATCH 1/2] regulator: st-pwm: get voltage and duty table from dts Chris Zhong
@ 2014-09-17 15:15 ` Heiko Stübner
2014-09-17 16:51 ` Mark Brown
2014-09-17 18:28 ` Doug Anderson
2 siblings, 0 replies; 14+ messages in thread
From: Heiko Stübner @ 2014-09-17 15:15 UTC (permalink / raw)
To: Chris Zhong
Cc: dianders, huangtao, cf, zhangqing, Liam Girdwood, Mark Brown,
linux-kernel
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);
> }
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 1/2] regulator: st-pwm: get voltage and duty table from dts
2014-09-17 13:07 ` [PATCH 1/2] regulator: st-pwm: get voltage and duty table from dts Chris Zhong
2014-09-17 15:15 ` Heiko Stübner
@ 2014-09-17 16:51 ` Mark Brown
2014-09-17 16:54 ` Doug Anderson
2014-09-17 18:28 ` Doug Anderson
2 siblings, 1 reply; 14+ messages in thread
From: Mark Brown @ 2014-09-17 16:51 UTC (permalink / raw)
To: Chris Zhong
Cc: dianders, heiko, huangtao, cf, zhangqing, Liam Girdwood,
linux-kernel
[-- 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 --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] regulator: st-pwm: get voltage and duty table from dts
2014-09-17 16:51 ` Mark Brown
@ 2014-09-17 16:54 ` Doug Anderson
2014-09-18 1:37 ` Chris Zhong
0 siblings, 1 reply; 14+ messages in thread
From: Doug Anderson @ 2014-09-17 16:54 UTC (permalink / raw)
To: Mark Brown
Cc: Chris Zhong, Heiko Stübner, Tao Huang, Eddie Cai, zhangqing,
Liam Girdwood, linux-kernel@vger.kernel.org
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] regulator: st-pwm: get voltage and duty table from dts
2014-09-17 16:54 ` Doug Anderson
@ 2014-09-18 1:37 ` Chris Zhong
2014-09-18 3:36 ` Doug Anderson
0 siblings, 1 reply; 14+ messages in thread
From: Chris Zhong @ 2014-09-18 1:37 UTC (permalink / raw)
To: Doug Anderson, Mark Brown
Cc: Heiko Stübner, Tao Huang, Eddie Cai, zhangqing,
Liam Girdwood, linux-kernel@vger.kernel.org
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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] regulator: st-pwm: get voltage and duty table from dts
2014-09-18 1:37 ` Chris Zhong
@ 2014-09-18 3:36 ` Doug Anderson
0 siblings, 0 replies; 14+ messages in thread
From: Doug Anderson @ 2014-09-18 3:36 UTC (permalink / raw)
To: Chris Zhong
Cc: Mark Brown, Heiko Stübner, Tao Huang, Eddie Cai, zhangqing,
Liam Girdwood, linux-kernel@vger.kernel.org
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] regulator: st-pwm: get voltage and duty table from dts
2014-09-17 13:07 ` [PATCH 1/2] regulator: st-pwm: get voltage and duty table from dts Chris Zhong
2014-09-17 15:15 ` Heiko Stübner
2014-09-17 16:51 ` Mark Brown
@ 2014-09-17 18:28 ` Doug Anderson
2014-09-17 21:26 ` Lee Jones
2 siblings, 1 reply; 14+ messages in thread
From: Doug Anderson @ 2014-09-17 18:28 UTC (permalink / raw)
To: Chris Zhong
Cc: Heiko Stübner, Tao Huang, Eddie Cai, zhangqing,
Liam Girdwood, Mark Brown, linux-kernel@vger.kernel.org,
Lee Jones
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
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 1/2] regulator: st-pwm: get voltage and duty table from dts
2014-09-17 18:28 ` Doug Anderson
@ 2014-09-17 21:26 ` Lee Jones
0 siblings, 0 replies; 14+ messages in thread
From: Lee Jones @ 2014-09-17 21:26 UTC (permalink / raw)
To: Doug Anderson
Cc: Chris Zhong, Heiko Stübner, Tao Huang, Eddie Cai, zhangqing,
Liam Girdwood, Mark Brown, linux-kernel@vger.kernel.org
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
^ permalink raw reply [flat|nested] 14+ messages in thread