| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAD=FV=XpwK+HzybKMck8KJ6VDYt3h5LHXtkU=RCa4DUX0t-MGg@mail.gmail.com>
Date: Wed, 17 Sep 2014 11:28:22 -0700
From: Doug Anderson <dianders@...omium.org>
To: Chris Zhong <zyw@...k-chips.com>
Cc: Heiko Stübner <heiko@...ech.de>,
Tao Huang <huangtao@...k-chips.com>,
Eddie Cai <cf@...k-chips.com>,
zhangqing <zhangqing@...k-chips.com>,
Liam Girdwood <lgirdwood@...il.com>,
Mark Brown <broonie@...nel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Lee Jones <lee.jones@...aro.org>
Subject: Re: [PATCH 1/2] regulator: st-pwm: get voltage and duty table from dts
Chris,
On Wed, Sep 17, 2014 at 6:07 AM, Chris Zhong <zyw@...k-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@...k-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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists