Continuous mode uses the PWM regulator's maximum and minimum supplied
voltages specified in the regulator-{min,max}-microvolt properties to
calculate appropriate duty-cycle values. This allows for a much more
fine grained solution when compared with voltage-table mode, which
this driver already supports. This solution does make an assumption
that a %50 duty-cycle value will cause the regulator voltage to run
at half way between the supplied max_uV and min_uV values.
Lee Jones (5):
ARM: STi: STiH407: Add PWM Regulator node
regulator: pwm-regulator: Separate voltage-table initialisation
regulator: pwm-regulator: Add support for continuous-voltage
dt: regulator: pwm-regulator: Re-write bindings
regulator: pwm-regulator: Don't assign structure attributes right away
.../bindings/regulator/pwm-regulator.txt | 66 ++++++--
arch/arm/boot/dts/stih407-family.dtsi | 11 ++
drivers/mfd/mfd-child.c | 47 ++++++
drivers/regulator/pwm-regulator.c | 185 +++++++++++++++++----
4 files changed, 261 insertions(+), 48 deletions(-)
create mode 100644 drivers/mfd/mfd-child.c
--
1.9.1
Signed-off-by: Lee Jones <[email protected]>
---
arch/arm/boot/dts/stih407-family.dtsi | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/arch/arm/boot/dts/stih407-family.dtsi b/arch/arm/boot/dts/stih407-family.dtsi
index e096105..be201aa 100644
--- a/arch/arm/boot/dts/stih407-family.dtsi
+++ b/arch/arm/boot/dts/stih407-family.dtsi
@@ -65,6 +65,17 @@
interrupts = <GIC_PPI 15 IRQ_TYPE_LEVEL_HIGH>;
};
+ pwm_regulator: pwm-regulator {
+ compatible = "pwm-regulator";
+ pwms = <&pwm1 3 8448>;
+ regulator-name = "CPU_1V0_AVS";
+ regulator-min-microvolt = <784000>;
+ regulator-max-microvolt = <1299000>;
+ regulator-always-on;
+ max-duty-cycle = <255>;
+ status = "okay";
+ };
+
soc {
#address-cells = <1>;
#size-cells = <1>;
--
1.9.1
Take this out of the main .probe() routine in order to facilitate the
introduction of different ways to obtain 'duty cycle' information.
Signed-off-by: Lee Jones <[email protected]>
---
drivers/mfd/mfd-child.c | 47 ++++++++++++++++++++++++
drivers/regulator/pwm-regulator.c | 77 +++++++++++++++++++++++----------------
2 files changed, 92 insertions(+), 32 deletions(-)
create mode 100644 drivers/mfd/mfd-child.c
diff --git a/drivers/mfd/mfd-child.c b/drivers/mfd/mfd-child.c
new file mode 100644
index 0000000..f233add
--- /dev/null
+++ b/drivers/mfd/mfd-child.c
@@ -0,0 +1,47 @@
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/of_irq.h>
+
+static int simple_mfd_child_probe(struct platform_device *pdev)
+{
+ struct resource *res;
+ void __iomem *base;
+ int irq;
+
+ printk("LEE: %s %s()[%d]: Enter\n", __FILE__, __func__, __LINE__);
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ dev_err(&pdev->dev, "Phys: %x: %x\n", res->start, resource_size(res));
+
+ base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(base))
+ return PTR_ERR(base);
+
+ dev_err(&pdev->dev, "Virt: %p\n", base);
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0) {
+ dev_err(&pdev->dev, "failed to get IRQ: %d\n", irq);
+ return irq;
+ }
+
+ dev_err(&pdev->dev, "IRQ: %d\n", irq);
+
+ return 0;
+}
+
+static const struct of_device_id simple_mfd_child_of_match[] = {
+ { .compatible = "simple-mfd-child" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, simple_mfd_child_of_match);
+
+static struct platform_driver simple_mfd_child_driver = {
+ .probe = simple_mfd_child_probe,
+ .driver = {
+ .name = "simple-mfd-child",
+ .of_match_table = of_match_ptr(simple_mfd_child_of_match),
+ },
+};
+module_platform_driver(simple_mfd_child_driver);
diff --git a/drivers/regulator/pwm-regulator.c b/drivers/regulator/pwm-regulator.c
index ffa9612..25560fc 100644
--- a/drivers/regulator/pwm-regulator.c
+++ b/drivers/regulator/pwm-regulator.c
@@ -78,8 +78,7 @@ static int pwm_regulator_list_voltage(struct regulator_dev *rdev,
return drvdata->duty_cycle_table[selector].uV;
}
-
-static struct regulator_ops pwm_regulator_voltage_ops = {
+static struct regulator_ops pwm_regulator_voltage_table_ops = {
.set_voltage_sel = pwm_regulator_set_voltage_sel,
.get_voltage_sel = pwm_regulator_get_voltage_sel,
.list_voltage = pwm_regulator_list_voltage,
@@ -88,20 +87,55 @@ static struct regulator_ops pwm_regulator_voltage_ops = {
static struct regulator_desc pwm_regulator_desc = {
.name = "pwm-regulator",
- .ops = &pwm_regulator_voltage_ops,
.type = REGULATOR_VOLTAGE,
.owner = THIS_MODULE,
.supply_name = "pwm",
};
+static int pwm_regulator_init_table(struct platform_device *pdev,
+ struct pwm_regulator_data *drvdata)
+{
+ struct device_node *np = pdev->dev.of_node;
+ struct pwm_voltages *duty_cycle_table;
+ int length;
+ int ret;
+
+ of_find_property(np, "voltage-table", &length);
+
+ if ((length < sizeof(*duty_cycle_table)) ||
+ (length % sizeof(*duty_cycle_table))) {
+ dev_err(&pdev->dev,
+ "voltage-table length(%d) is invalid\n",
+ length);
+ return -EINVAL;
+ }
+
+ duty_cycle_table = devm_kzalloc(&pdev->dev, length, GFP_KERNEL);
+ if (!duty_cycle_table)
+ return -ENOMEM;
+
+ ret = of_property_read_u32_array(np, "voltage-table",
+ (u32 *)duty_cycle_table,
+ length / sizeof(u32));
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to read voltage-table\n");
+ return ret;
+ }
+
+ drvdata->duty_cycle_table = duty_cycle_table;
+ pwm_regulator_desc.ops = &pwm_regulator_voltage_table_ops;
+ pwm_regulator_desc.n_voltages = length / sizeof(*duty_cycle_table);
+
+ return 0;
+}
+
static int pwm_regulator_probe(struct platform_device *pdev)
{
struct pwm_regulator_data *drvdata;
- struct property *prop;
struct regulator_dev *regulator;
struct regulator_config config = { };
struct device_node *np = pdev->dev.of_node;
- int length, ret;
+ int ret;
if (!np) {
dev_err(&pdev->dev, "Device Tree node missing\n");
@@ -112,36 +146,15 @@ static int pwm_regulator_probe(struct platform_device *pdev)
if (!drvdata)
return -ENOMEM;
- /* determine the number of voltage-table */
- prop = of_find_property(np, "voltage-table", &length);
- if (!prop) {
- dev_err(&pdev->dev, "No voltage-table\n");
- return -EINVAL;
- }
-
- if ((length < sizeof(*drvdata->duty_cycle_table)) ||
- (length % sizeof(*drvdata->duty_cycle_table))) {
- dev_err(&pdev->dev, "voltage-table length(%d) is invalid\n",
- length);
+ if (of_find_property(np, "voltage-table", NULL)) {
+ ret = pwm_regulator_init_table(pdev, drvdata);
+ if (ret)
+ return ret;
+ } else {
+ dev_err(&pdev->dev, "No \"voltage-table\" supplied\n");
return -EINVAL;
}
- pwm_regulator_desc.n_voltages = length / sizeof(*drvdata->duty_cycle_table);
-
- drvdata->duty_cycle_table = devm_kzalloc(&pdev->dev,
- length, GFP_KERNEL);
- if (!drvdata->duty_cycle_table)
- return -ENOMEM;
-
- /* read voltage table from DT property */
- ret = of_property_read_u32_array(np, "voltage-table",
- (u32 *)drvdata->duty_cycle_table,
- length / sizeof(u32));
- if (ret < 0) {
- dev_err(&pdev->dev, "read voltage-table failed\n");
- return ret;
- }
-
config.init_data = of_get_regulator_init_data(&pdev->dev, np,
&pwm_regulator_desc);
if (!config.init_data)
--
1.9.1
The current version of PWM regulator only supports a static table
approach, where pre-calculated values are supplied by the vendor and
obtained via DT. The continuous-voltage method takes min_uV and
max_uV, and divides the difference between them up into a number of
slices. The number of slices depend on how large the duty cycle
register is. This information is provided by a DT property.
As the name alludes, this provides values for a continuous voltage
range between min_uV and max_uV, which has obvious benefits over
either limited voltage possibilities, or the requirement to provide
a large voltage-table.
Signed-off-by: Lee Jones <[email protected]>
---
drivers/regulator/pwm-regulator.c | 114 +++++++++++++++++++++++++++++++++++---
1 file changed, 106 insertions(+), 8 deletions(-)
diff --git a/drivers/regulator/pwm-regulator.c b/drivers/regulator/pwm-regulator.c
index 25560fc..b37b616 100644
--- a/drivers/regulator/pwm-regulator.c
+++ b/drivers/regulator/pwm-regulator.c
@@ -10,6 +10,7 @@
* published by the Free Software Foundation.
*/
+#include <linux/delay.h>
#include <linux/module.h>
#include <linux/init.h>
#include <linux/err.h>
@@ -21,9 +22,16 @@
#include <linux/pwm.h>
struct pwm_regulator_data {
- struct pwm_voltages *duty_cycle_table;
+ /* Shared */
struct pwm_device *pwm;
+
+ /* Voltage table */
+ struct pwm_voltages *duty_cycle_table;
int state;
+
+ /* Continuous voltage */
+ u32 max_duty_cycle;
+ int volt_uV;
};
struct pwm_voltages {
@@ -31,6 +39,9 @@ struct pwm_voltages {
unsigned int dutycycle;
};
+/**
+ * Voltage table call-backs
+ */
static int pwm_regulator_get_voltage_sel(struct regulator_dev *rdev)
{
struct pwm_regulator_data *drvdata = rdev_get_drvdata(rdev);
@@ -78,6 +89,71 @@ static int pwm_regulator_list_voltage(struct regulator_dev *rdev,
return drvdata->duty_cycle_table[selector].uV;
}
+
+/**
+ * Continuous voltage call-backs
+ */
+static int pwm_voltage_to_duty_cycle(struct regulator_dev *rdev,
+ int volt_mV)
+{
+ struct pwm_regulator_data *drvdata = rdev_get_drvdata(rdev);
+ int min_mV = rdev->constraints->min_uV / 1000;
+ int max_mV = rdev->constraints->max_uV / 1000;
+ int max_duty_cycle = drvdata->max_duty_cycle;
+ int vdiff = min_mV - max_mV;
+ int pwm_code;
+ int tmp;
+
+ tmp = max_duty_cycle - min_mV * max_duty_cycle / vdiff;
+ pwm_code = tmp + max_duty_cycle * volt_mV / vdiff;
+
+ if (pwm_code < 0)
+ pwm_code = 0;
+ if (pwm_code > max_duty_cycle)
+ pwm_code = max_duty_cycle;
+
+ return pwm_code * 100 / max_duty_cycle;
+}
+
+static int pwm_regulator_get_voltage(struct regulator_dev *rdev)
+{
+ struct pwm_regulator_data *drvdata = rdev_get_drvdata(rdev);
+
+ return drvdata->volt_uV;
+}
+
+static int pwm_regulator_set_voltage(struct regulator_dev *rdev,
+ int min_uV, int max_uV,
+ unsigned *selector)
+{
+ struct pwm_regulator_data *drvdata = rdev_get_drvdata(rdev);
+ unsigned int ramp_delay = rdev->constraints->ramp_delay;
+ int duty_cycle;
+ int ret;
+
+ duty_cycle = pwm_voltage_to_duty_cycle(rdev, min_uV / 1000);
+
+ ret = pwm_config(drvdata->pwm,
+ (drvdata->pwm->period / 100) * duty_cycle,
+ drvdata->pwm->period);
+ if (ret) {
+ dev_err(&rdev->dev, "Failed to configure PWM\n");
+ return ret;
+ }
+
+ ret = pwm_enable(drvdata->pwm);
+ if (ret) {
+ dev_err(&rdev->dev, "Failed to enable PWM\n");
+ return ret;
+ }
+ drvdata->volt_uV = min_uV;
+
+ /* Delay required by PWM regulator to settle to the new voltage */
+ usleep_range(ramp_delay, ramp_delay + 1000);
+
+ return 0;
+}
+
static struct regulator_ops pwm_regulator_voltage_table_ops = {
.set_voltage_sel = pwm_regulator_set_voltage_sel,
.get_voltage_sel = pwm_regulator_get_voltage_sel,
@@ -85,6 +161,11 @@ static struct regulator_ops pwm_regulator_voltage_table_ops = {
.map_voltage = regulator_map_voltage_iterate,
};
+static struct regulator_ops pwm_regulator_voltage_continuous_ops = {
+ .get_voltage = pwm_regulator_get_voltage,
+ .set_voltage = pwm_regulator_set_voltage,
+};
+
static struct regulator_desc pwm_regulator_desc = {
.name = "pwm-regulator",
.type = REGULATOR_VOLTAGE,
@@ -129,6 +210,25 @@ static int pwm_regulator_init_table(struct platform_device *pdev,
return 0;
}
+static int pwm_regulator_init_continuous(struct platform_device *pdev,
+ struct pwm_regulator_data *drvdata)
+{
+ struct device_node *np = pdev->dev.of_node;
+ int ret;
+
+ ret = of_property_read_u32(np, "max-duty-cycle",
+ &drvdata->max_duty_cycle);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to read \"pwm-max-value\"\n");
+ return ret;
+ }
+
+ pwm_regulator_desc.ops = &pwm_regulator_voltage_continuous_ops;
+ pwm_regulator_desc.continuous_voltage_range = true;
+
+ return 0;
+}
+
static int pwm_regulator_probe(struct platform_device *pdev)
{
struct pwm_regulator_data *drvdata;
@@ -146,14 +246,12 @@ static int pwm_regulator_probe(struct platform_device *pdev)
if (!drvdata)
return -ENOMEM;
- if (of_find_property(np, "voltage-table", NULL)) {
+ if (of_find_property(np, "voltage-table", NULL))
ret = pwm_regulator_init_table(pdev, drvdata);
- if (ret)
- return ret;
- } else {
- dev_err(&pdev->dev, "No \"voltage-table\" supplied\n");
- return -EINVAL;
- }
+ else
+ ret = pwm_regulator_init_continuous(pdev, drvdata);
+ if (ret)
+ return ret;
config.init_data = of_get_regulator_init_data(&pdev->dev, np,
&pwm_regulator_desc);
--
1.9.1
* Add support for continuous-voltage mode
* Put more meat on the bones with regards to voltage-table mode
* Sort out formatting for ease of consumption
Cc: [email protected]
Signed-off-by: Lee Jones <[email protected]>
---
.../bindings/regulator/pwm-regulator.txt | 66 ++++++++++++++++++----
1 file changed, 54 insertions(+), 12 deletions(-)
diff --git a/Documentation/devicetree/bindings/regulator/pwm-regulator.txt b/Documentation/devicetree/bindings/regulator/pwm-regulator.txt
index ce91f61..0ae7895 100644
--- a/Documentation/devicetree/bindings/regulator/pwm-regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/pwm-regulator.txt
@@ -1,27 +1,69 @@
-pwm regulator bindings
+Bindings for the Generic PWM Regulator
+======================================
+
+Currently supports 2 modes of operation:
+
+voltage-table: When in this mode, a voltage table (See below) of predefined
+ voltage <=> duty-cycle values must be provided via DT.
+ Limitations are that the regulator can only operate at the
+ voltages supplied in the table. Intermediary duty-cycle
+ values which would normally allow finer grained voltage
+ selection are ignored and rendered useless. Although more
+ control is given to the user if the assumptions made in
+ continuous-voltage mode do not reign true.
+
+continuous-voltage: This mode uses the regulator's maximum and minimum supplied
+ voltages specified in the regulator-{min,max}-microvolt
+ properties to calculate appropriate duty-cycle values. This
+ allows for a much more fine grained solution when compared
+ with voltage-table mode above. This solution does make an
+ assumption that a %50 duty-cycle value will cause the
+ regulator voltage to run at half way between the supplied
+ max_uV and min_uV values.
Required properties:
-- compatible: Should be "pwm-regulator"
-- pwms: OF device-tree PWM specification (see PWM binding pwm.txt)
-- voltage-table: voltage and duty table, include 2 members in each set of
- brackets, first one is voltage(unit: uv), the next is duty(unit: percent)
+--------------------
+- compatible: Should be "pwm-regulator"
+
+- pwms: PWM specification (See: ../pwm/pwm.txt)
+
+One of these must be provided:
+- voltage-table: Voltage and Duty-Cycle table consisting of 2 cells
+ First cell is voltage in microvolts (uV)
+ Second cell is duty-cycle in percent (%)
+
+- max-duty-cycle: Maximum Duty-Cycle value -- this will normally be 255 (0xff)
+ for an 8 bit PWM device
-Any property defined as part of the core regulator binding defined in
-regulator.txt can also be used.
+If both are provided, the current default is voltage-table mode.
-Example:
+Any property defined as part of the core regulator binding can also be used.
+(See: ../regulator/regulator.txt)
+
+Continuous Voltage Example:
pwm_regulator {
compatible = "pwm-regulator;
pwms = <&pwm1 0 8448 0>;
+ regulator-min-microvolt = <1016000>;
+ regulator-max-microvolt = <1114000>;
+ regulator-name = "vdd_logic";
+
+ max-duty-cycle = <255>; /* 8bit PWM */
+ };
+Voltage Table Example:
+ pwm_regulator {
+ compatible = "pwm-regulator;
+ pwms = <&pwm1 0 8448 0>;
+ regulator-min-microvolt = <1016000>;
+ regulator-max-microvolt = <1114000>;
+ regulator-name = "vdd_logic";
+
+ /* Voltage Duty-Cycle */
voltage-table = <1114000 0>,
<1095000 10>,
<1076000 20>,
<1056000 30>,
<1036000 40>,
<1016000 50>;
-
- regulator-min-microvolt = <1016000>;
- regulator-max-microvolt = <1114000>;
- regulator-name = "vdd_logic";
};
--
1.9.1
Perhaps this is just personal preference, but ...
This patch introduces a new local variable to receive and test regulator
initialisation data. It simplifies and cleans up the code making it
that little bit easier to read and maintain. The local value is assigned
to the structure attribute when all the others are. This is the way we
usually do things.
Prevents this kind of nonsense:
this->is->just.silly = fetch_silly_value(&pointer);
if (!this->is->just.silly) {
printk("Silly value failed: %d\n", this->is->just.silly);
return this->is->just.silly;
}
Signed-off-by: Lee Jones <[email protected]>
---
drivers/regulator/pwm-regulator.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/regulator/pwm-regulator.c b/drivers/regulator/pwm-regulator.c
index b37b616..2bfcb52 100644
--- a/drivers/regulator/pwm-regulator.c
+++ b/drivers/regulator/pwm-regulator.c
@@ -231,6 +231,7 @@ static int pwm_regulator_init_continuous(struct platform_device *pdev,
static int pwm_regulator_probe(struct platform_device *pdev)
{
+ const struct regulator_init_data *init_data;
struct pwm_regulator_data *drvdata;
struct regulator_dev *regulator;
struct regulator_config config = { };
@@ -253,14 +254,15 @@ static int pwm_regulator_probe(struct platform_device *pdev)
if (ret)
return ret;
- config.init_data = of_get_regulator_init_data(&pdev->dev, np,
- &pwm_regulator_desc);
- if (!config.init_data)
+ init_data = of_get_regulator_init_data(&pdev->dev, np,
+ &pwm_regulator_desc);
+ if (!init_data)
return -ENOMEM;
config.of_node = np;
config.dev = &pdev->dev;
config.driver_data = drvdata;
+ config.init_data = init_data;
drvdata->pwm = devm_pwm_get(&pdev->dev, NULL);
if (IS_ERR(drvdata->pwm)) {
--
1.9.1
Hi,
> Take this out of the main .probe() routine in order to facilitate the
> introduction of different ways to obtain 'duty cycle' information.
>
> Signed-off-by: Lee Jones <[email protected]>
> ---
> drivers/mfd/mfd-child.c | 47 ++++++++++++++++++++++++
> drivers/regulator/pwm-regulator.c | 77 +++++++++++++++++++++++----------------
> 2 files changed, 92 insertions(+), 32 deletions(-)
> create mode 100644 drivers/mfd/mfd-child.c
>
> diff --git a/drivers/mfd/mfd-child.c b/drivers/mfd/mfd-child.c
> new file mode 100644
> index 0000000..f233add
> --- /dev/null
> +++ b/drivers/mfd/mfd-child.c
> @@ -0,0 +1,47 @@
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/of_irq.h>
> +
> +static int simple_mfd_child_probe(struct platform_device *pdev)
> +{
> + struct resource *res;
> + void __iomem *base;
> + int irq;
> +
> + printk("LEE: %s %s()[%d]: Enter\n", __FILE__, __func__, __LINE__);
> +
Debugging remnant?
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + dev_err(&pdev->dev, "Phys: %x: %x\n", res->start, resource_size(res));
> +
That's not an error message and thus shouldn't be printed with
dev_err(). dev_dbg() would be more appropriate here.
> + base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> + dev_err(&pdev->dev, "Virt: %p\n", base);
dto.
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0) {
> + dev_err(&pdev->dev, "failed to get IRQ: %d\n", irq);
> + return irq;
> + }
> +
> + dev_err(&pdev->dev, "IRQ: %d\n", irq);
> +
dto.
> + return 0;
> +}
> +
> +static const struct of_device_id simple_mfd_child_of_match[] = {
> + { .compatible = "simple-mfd-child" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, simple_mfd_child_of_match);
> +
> +static struct platform_driver simple_mfd_child_driver = {
> + .probe = simple_mfd_child_probe,
> + .driver = {
> + .name = "simple-mfd-child",
> + .of_match_table = of_match_ptr(simple_mfd_child_of_match),
> + },
> +};
> +module_platform_driver(simple_mfd_child_driver);
> diff --git a/drivers/regulator/pwm-regulator.c b/drivers/regulator/pwm-regulator.c
> index ffa9612..25560fc 100644
> --- a/drivers/regulator/pwm-regulator.c
> +++ b/drivers/regulator/pwm-regulator.c
> @@ -78,8 +78,7 @@ static int pwm_regulator_list_voltage(struct regulator_dev *rdev,
>
> return drvdata->duty_cycle_table[selector].uV;
> }
> -
> -static struct regulator_ops pwm_regulator_voltage_ops = {
> +static struct regulator_ops pwm_regulator_voltage_table_ops = {
> .set_voltage_sel = pwm_regulator_set_voltage_sel,
> .get_voltage_sel = pwm_regulator_get_voltage_sel,
> .list_voltage = pwm_regulator_list_voltage,
> @@ -88,20 +87,55 @@ static struct regulator_ops pwm_regulator_voltage_ops = {
>
> static struct regulator_desc pwm_regulator_desc = {
> .name = "pwm-regulator",
> - .ops = &pwm_regulator_voltage_ops,
> .type = REGULATOR_VOLTAGE,
> .owner = THIS_MODULE,
> .supply_name = "pwm",
> };
>
> +static int pwm_regulator_init_table(struct platform_device *pdev,
> + struct pwm_regulator_data *drvdata)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + struct pwm_voltages *duty_cycle_table;
> + int length;
> + int ret;
> +
> + of_find_property(np, "voltage-table", &length);
> +
> + if ((length < sizeof(*duty_cycle_table)) ||
> + (length % sizeof(*duty_cycle_table))) {
> + dev_err(&pdev->dev,
> + "voltage-table length(%d) is invalid\n",
> + length);
> + return -EINVAL;
> + }
> +
> + duty_cycle_table = devm_kzalloc(&pdev->dev, length, GFP_KERNEL);
> + if (!duty_cycle_table)
> + return -ENOMEM;
> +
> + ret = of_property_read_u32_array(np, "voltage-table",
> + (u32 *)duty_cycle_table,
> + length / sizeof(u32));
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to read voltage-table\n");
> + return ret;
> + }
> +
> + drvdata->duty_cycle_table = duty_cycle_table;
> + pwm_regulator_desc.ops = &pwm_regulator_voltage_table_ops;
> + pwm_regulator_desc.n_voltages = length / sizeof(*duty_cycle_table);
> +
> + return 0;
> +}
> +
> static int pwm_regulator_probe(struct platform_device *pdev)
> {
> struct pwm_regulator_data *drvdata;
> - struct property *prop;
> struct regulator_dev *regulator;
> struct regulator_config config = { };
> struct device_node *np = pdev->dev.of_node;
> - int length, ret;
> + int ret;
>
> if (!np) {
> dev_err(&pdev->dev, "Device Tree node missing\n");
> @@ -112,36 +146,15 @@ static int pwm_regulator_probe(struct platform_device *pdev)
> if (!drvdata)
> return -ENOMEM;
>
> - /* determine the number of voltage-table */
> - prop = of_find_property(np, "voltage-table", &length);
> - if (!prop) {
> - dev_err(&pdev->dev, "No voltage-table\n");
> - return -EINVAL;
> - }
> -
> - if ((length < sizeof(*drvdata->duty_cycle_table)) ||
> - (length % sizeof(*drvdata->duty_cycle_table))) {
> - dev_err(&pdev->dev, "voltage-table length(%d) is invalid\n",
> - length);
> + if (of_find_property(np, "voltage-table", NULL)) {
> + ret = pwm_regulator_init_table(pdev, drvdata);
> + if (ret)
> + return ret;
> + } else {
> + dev_err(&pdev->dev, "No \"voltage-table\" supplied\n");
> return -EINVAL;
> }
>
> - pwm_regulator_desc.n_voltages = length / sizeof(*drvdata->duty_cycle_table);
> -
> - drvdata->duty_cycle_table = devm_kzalloc(&pdev->dev,
> - length, GFP_KERNEL);
> - if (!drvdata->duty_cycle_table)
> - return -ENOMEM;
> -
> - /* read voltage table from DT property */
> - ret = of_property_read_u32_array(np, "voltage-table",
> - (u32 *)drvdata->duty_cycle_table,
> - length / sizeof(u32));
> - if (ret < 0) {
> - dev_err(&pdev->dev, "read voltage-table failed\n");
> - return ret;
> - }
> -
> config.init_data = of_get_regulator_init_data(&pdev->dev, np,
> &pwm_regulator_desc);
> if (!config.init_data)
> --
> 1.9.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
http://www.karo-electronics.de | [email protected]
___________________________________________________________
Oh my ...
On Wed, 10 Jun 2015, Lothar Waßmann wrote:
> > Take this out of the main .probe() routine in order to facilitate the
> > introduction of different ways to obtain 'duty cycle' information.
> >
> > Signed-off-by: Lee Jones <[email protected]>
> > ---
> > drivers/mfd/mfd-child.c | 47 ++++++++++++++++++++++++
> > drivers/regulator/pwm-regulator.c | 77 +++++++++++++++++++++++----------------
> > 2 files changed, 92 insertions(+), 32 deletions(-)
> > create mode 100644 drivers/mfd/mfd-child.c
> >
> > diff --git a/drivers/mfd/mfd-child.c b/drivers/mfd/mfd-child.c
> > new file mode 100644
> > index 0000000..f233add
> > --- /dev/null
> > +++ b/drivers/mfd/mfd-child.c
> > @@ -0,0 +1,47 @@
> > +#include <linux/interrupt.h>
> > +#include <linux/module.h>
> > +#include <linux/of_device.h>
> > +#include <linux/of_irq.h>
> > +
> > +static int simple_mfd_child_probe(struct platform_device *pdev)
> > +{
> > + struct resource *res;
> > + void __iomem *base;
> > + int irq;
> > +
> > + printk("LEE: %s %s()[%d]: Enter\n", __FILE__, __func__, __LINE__);
> > +
> Debugging remnant?
>
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + dev_err(&pdev->dev, "Phys: %x: %x\n", res->start, resource_size(res));
> > +
> That's not an error message and thus shouldn't be printed with
> dev_err(). dev_dbg() would be more appropriate here.
This entire file is cruft and has nothing to do with this set.
It was knocked it up quickly to test something unrelated.
I will resubmit this patch with it removed.
Sorry for the noise.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
Take this out of the main .probe() routine in order to facilitate the
introduction of different ways to obtain 'duty cycle' information.
Signed-off-by: Lee Jones <[email protected]>
---
drivers/regulator/pwm-regulator.c | 77 +++++++++++++++++++++++----------------
1 file changed, 45 insertions(+), 32 deletions(-)
diff --git a/drivers/regulator/pwm-regulator.c b/drivers/regulator/pwm-regulator.c
index ffa9612..25560fc 100644
--- a/drivers/regulator/pwm-regulator.c
+++ b/drivers/regulator/pwm-regulator.c
@@ -78,8 +78,7 @@ static int pwm_regulator_list_voltage(struct regulator_dev *rdev,
return drvdata->duty_cycle_table[selector].uV;
}
-
-static struct regulator_ops pwm_regulator_voltage_ops = {
+static struct regulator_ops pwm_regulator_voltage_table_ops = {
.set_voltage_sel = pwm_regulator_set_voltage_sel,
.get_voltage_sel = pwm_regulator_get_voltage_sel,
.list_voltage = pwm_regulator_list_voltage,
@@ -88,20 +87,55 @@ static struct regulator_ops pwm_regulator_voltage_ops = {
static struct regulator_desc pwm_regulator_desc = {
.name = "pwm-regulator",
- .ops = &pwm_regulator_voltage_ops,
.type = REGULATOR_VOLTAGE,
.owner = THIS_MODULE,
.supply_name = "pwm",
};
+static int pwm_regulator_init_table(struct platform_device *pdev,
+ struct pwm_regulator_data *drvdata)
+{
+ struct device_node *np = pdev->dev.of_node;
+ struct pwm_voltages *duty_cycle_table;
+ int length;
+ int ret;
+
+ of_find_property(np, "voltage-table", &length);
+
+ if ((length < sizeof(*duty_cycle_table)) ||
+ (length % sizeof(*duty_cycle_table))) {
+ dev_err(&pdev->dev,
+ "voltage-table length(%d) is invalid\n",
+ length);
+ return -EINVAL;
+ }
+
+ duty_cycle_table = devm_kzalloc(&pdev->dev, length, GFP_KERNEL);
+ if (!duty_cycle_table)
+ return -ENOMEM;
+
+ ret = of_property_read_u32_array(np, "voltage-table",
+ (u32 *)duty_cycle_table,
+ length / sizeof(u32));
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to read voltage-table\n");
+ return ret;
+ }
+
+ drvdata->duty_cycle_table = duty_cycle_table;
+ pwm_regulator_desc.ops = &pwm_regulator_voltage_table_ops;
+ pwm_regulator_desc.n_voltages = length / sizeof(*duty_cycle_table);
+
+ return 0;
+}
+
static int pwm_regulator_probe(struct platform_device *pdev)
{
struct pwm_regulator_data *drvdata;
- struct property *prop;
struct regulator_dev *regulator;
struct regulator_config config = { };
struct device_node *np = pdev->dev.of_node;
- int length, ret;
+ int ret;
if (!np) {
dev_err(&pdev->dev, "Device Tree node missing\n");
@@ -112,36 +146,15 @@ static int pwm_regulator_probe(struct platform_device *pdev)
if (!drvdata)
return -ENOMEM;
- /* determine the number of voltage-table */
- prop = of_find_property(np, "voltage-table", &length);
- if (!prop) {
- dev_err(&pdev->dev, "No voltage-table\n");
- return -EINVAL;
- }
-
- if ((length < sizeof(*drvdata->duty_cycle_table)) ||
- (length % sizeof(*drvdata->duty_cycle_table))) {
- dev_err(&pdev->dev, "voltage-table length(%d) is invalid\n",
- length);
+ if (of_find_property(np, "voltage-table", NULL)) {
+ ret = pwm_regulator_init_table(pdev, drvdata);
+ if (ret)
+ return ret;
+ } else {
+ dev_err(&pdev->dev, "No \"voltage-table\" supplied\n");
return -EINVAL;
}
- pwm_regulator_desc.n_voltages = length / sizeof(*drvdata->duty_cycle_table);
-
- drvdata->duty_cycle_table = devm_kzalloc(&pdev->dev,
- length, GFP_KERNEL);
- if (!drvdata->duty_cycle_table)
- return -ENOMEM;
-
- /* read voltage table from DT property */
- ret = of_property_read_u32_array(np, "voltage-table",
- (u32 *)drvdata->duty_cycle_table,
- length / sizeof(u32));
- if (ret < 0) {
- dev_err(&pdev->dev, "read voltage-table failed\n");
- return ret;
- }
-
config.init_data = of_get_regulator_init_data(&pdev->dev, np,
&pwm_regulator_desc);
if (!config.init_data)
The patch
regulator: pwm-regulator: Don't assign structure attributes right away
has been applied to the regulator tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
>From 5ad2cb14f5b8f3cb3d8688115037751b1ff45455 Mon Sep 17 00:00:00 2001
From: Lee Jones <[email protected]>
Date: Tue, 7 Jul 2015 16:06:53 +0100
Subject: [PATCH] regulator: pwm-regulator: Don't assign structure attributes
right away
Perhaps this is just personal preference, but ...
This patch introduces a new local variable to receive and test regulator
initialisation data. It simplifies and cleans up the code making it
that little bit easier to read and maintain. The local value is assigned
to the structure attribute when all the others are. This is the way we
usually do things.
Prevents this kind of nonsense:
this->is->just.silly = fetch_silly_value(&pointer);
if (!this->is->just.silly) {
printk("Silly value failed: %d\n", this->is->just.silly);
return this->is->just.silly;
}
Signed-off-by: Lee Jones <[email protected]>
Signed-off-by: Mark Brown <[email protected]>
---
drivers/regulator/pwm-regulator.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/regulator/pwm-regulator.c b/drivers/regulator/pwm-regulator.c
index d5cb267fa192..cb482089050b 100644
--- a/drivers/regulator/pwm-regulator.c
+++ b/drivers/regulator/pwm-regulator.c
@@ -218,6 +218,7 @@ static int pwm_regulator_init_continuous(struct platform_device *pdev,
static int pwm_regulator_probe(struct platform_device *pdev)
{
+ const struct regulator_init_data *init_data;
struct pwm_regulator_data *drvdata;
struct regulator_dev *regulator;
struct regulator_config config = { };
@@ -240,14 +241,15 @@ static int pwm_regulator_probe(struct platform_device *pdev)
if (ret)
return ret;
- config.init_data = of_get_regulator_init_data(&pdev->dev, np,
- &pwm_regulator_desc);
- if (!config.init_data)
+ init_data = of_get_regulator_init_data(&pdev->dev, np,
+ &pwm_regulator_desc);
+ if (!init_data)
return -ENOMEM;
config.of_node = np;
config.dev = &pdev->dev;
config.driver_data = drvdata;
+ config.init_data = init_data;
drvdata->pwm = devm_pwm_get(&pdev->dev, NULL);
if (IS_ERR(drvdata->pwm)) {
--
2.1.4