| Message ID | 1392384259-14311-1-git-send-email-marek@goldelico.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Fri, Feb 14, 2014 at 01:24:19PM +0000, Marek Belisko wrote: > Signed-off-by: Marek Belisko <marek@goldelico.com> > --- > .../bindings/power_supply/twl4030_madc_battery.txt | 15 +++ > drivers/power/twl4030_madc_battery.c | 109 +++++++++++++++++++++ > 2 files changed, 124 insertions(+) > create mode 100644 Documentation/devicetree/bindings/power_supply/twl4030_madc_battery.txt > > diff --git a/Documentation/devicetree/bindings/power_supply/twl4030_madc_battery.txt b/Documentation/devicetree/bindings/power_supply/twl4030_madc_battery.txt > new file mode 100644 > index 0000000..bebc876 > --- /dev/null > +++ b/Documentation/devicetree/bindings/power_supply/twl4030_madc_battery.txt > @@ -0,0 +1,15 @@ > +twl4030_madc_battery > + > +Required properties: > + - compatible : "ti,twl4030-madc-battery" > + - capacity : battery capacity in uAh > + - charging-calibration-data : list of voltage(mV):level(%) values for charging calibration (see example) > + - discharging-calibration-data : list of voltage(mV):level(%) values for discharging calibration (see example) > + > +Example: > + madc-battery { > + compatible = "ti,twl4030-madc-battery"; > + capacity = <1200000>; > + charging-calibration-data = <4200 100 4100 75 4000 55 3900 25 3800 5 3700 2 3600 1 3300 0>; > + discharging-calibration-data = <4200 100 4100 95 4000 70 3800 50 3700 10 3600 5 3300 0>; Please bracket list entries individually. > + }; > diff --git a/drivers/power/twl4030_madc_battery.c b/drivers/power/twl4030_madc_battery.c > index 7ef445a..2843382 100644 > --- a/drivers/power/twl4030_madc_battery.c > +++ b/drivers/power/twl4030_madc_battery.c > @@ -19,6 +19,7 @@ > #include <linux/sort.h> > #include <linux/i2c/twl4030-madc.h> > #include <linux/power/twl4030_madc_battery.h> > +#include <linux/of.h> > > struct twl4030_madc_battery { > struct power_supply psy; > @@ -188,6 +189,110 @@ static int twl4030_cmp(const void *a, const void *b) > ((struct twl4030_madc_bat_calibration *)a)->voltage; > } > > +#ifdef CONFIG_OF > +static struct twl4030_madc_bat_platform_data * > + twl4030_madc_dt_probe(struct platform_device *pdev) > +{ > + struct twl4030_madc_bat_platform_data *pdata; > + struct device_node *np = pdev->dev.of_node; > + struct property *prop; > + int ret; > + int sz, i, j = 0; > + > + pdata = devm_kzalloc(&pdev->dev, > + sizeof(struct twl4030_madc_bat_platform_data), > + GFP_KERNEL); > + if (!pdata) > + return ERR_PTR(-ENOMEM); > + > + ret = of_property_read_u32(np, "capacity", &pdata->capacity); > + if (ret != 0) > + return ERR_PTR(-EINVAL); > + > + /* parse and prepare charging data */ > + prop = of_find_property(np, "charging-calibration-data", &sz); > + if (!prop) > + return ERR_PTR(-EINVAL); > + > + if (sz % 2) { > + dev_warn(&pdev->dev, "Count of charging-calibration-data must be even!\n"); > + return ERR_PTR(-EINVAL); > + } As sz is in bytes this checks that the property is a multiple of 2 bytes, not that it has an even number of u32 elements. Heiko Stübner recently added of_property_count_u32_elems [1,2]. Use that instead. > + > + sz /= sizeof(u32); > + > + { > + u32 data[sz]; > + > + ret = of_property_read_u32_array(np, > + "charging-calibration-data", &data[0], sz); > + if (ret) > + return ERR_PTR(ret); Why not just allocate then try to read, possibly having to free if the read fails? Otherwise we're trying to put an arbitrarily large value onto the stack for no good reason. > + > + pdata->charging = devm_kzalloc(&pdev->dev, > + sizeof(struct twl4030_madc_bat_calibration) * (sz / 2), > + GFP_KERNEL); > + > + for (i = 0; i < sz; i += 2) { > + pdata->charging[j].voltage = data[i]; > + pdata->charging[j].level = data[i+1]; > + j++; Why not have (i = 0; i < sz/2; i++), and get rid of j? > + } > + > + pdata->charging_size = sz / 2; > + } > + > + /* parse and prepare discharging data */ > + prop = of_find_property(np, "discharging-calibration-data", &sz); > + if (!prop) > + return ERR_PTR(-EINVAL); > + > + if (sz % 2) { > + dev_warn(&pdev->dev, "Count of discharging-calibration-data must be even!\n"); > + return ERR_PTR(-EINVAL); > + } This has the same issues as with charging-calibration-data. Thanks, Mark. [1] http://www.spinics.net/lists/devicetree/msg21358.html [2] http://www.spinics.net/lists/devicetree/msg21502.html -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Feb 14, 2014 at 2:43 PM, Mark Rutland <mark.rutland@arm.com> wrote: > On Fri, Feb 14, 2014 at 01:24:19PM +0000, Marek Belisko wrote: >> Signed-off-by: Marek Belisko <marek@goldelico.com> >> --- >> .../bindings/power_supply/twl4030_madc_battery.txt | 15 +++ >> drivers/power/twl4030_madc_battery.c | 109 +++++++++++++++++++++ >> 2 files changed, 124 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/power_supply/twl4030_madc_battery.txt >> >> diff --git a/Documentation/devicetree/bindings/power_supply/twl4030_madc_battery.txt b/Documentation/devicetree/bindings/power_supply/twl4030_madc_battery.txt >> new file mode 100644 >> index 0000000..bebc876 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/power_supply/twl4030_madc_battery.txt >> @@ -0,0 +1,15 @@ >> +twl4030_madc_battery >> + >> +Required properties: >> + - compatible : "ti,twl4030-madc-battery" >> + - capacity : battery capacity in uAh >> + - charging-calibration-data : list of voltage(mV):level(%) values for charging calibration (see example) >> + - discharging-calibration-data : list of voltage(mV):level(%) values for discharging calibration (see example) >> + >> +Example: >> + madc-battery { >> + compatible = "ti,twl4030-madc-battery"; >> + capacity = <1200000>; >> + charging-calibration-data = <4200 100 4100 75 4000 55 3900 25 3800 5 3700 2 3600 1 3300 0>; >> + discharging-calibration-data = <4200 100 4100 95 4000 70 3800 50 3700 10 3600 5 3300 0>; > > Please bracket list entries individually. OK. > >> + }; >> diff --git a/drivers/power/twl4030_madc_battery.c b/drivers/power/twl4030_madc_battery.c >> index 7ef445a..2843382 100644 >> --- a/drivers/power/twl4030_madc_battery.c >> +++ b/drivers/power/twl4030_madc_battery.c >> @@ -19,6 +19,7 @@ >> #include <linux/sort.h> >> #include <linux/i2c/twl4030-madc.h> >> #include <linux/power/twl4030_madc_battery.h> >> +#include <linux/of.h> >> >> struct twl4030_madc_battery { >> struct power_supply psy; >> @@ -188,6 +189,110 @@ static int twl4030_cmp(const void *a, const void *b) >> ((struct twl4030_madc_bat_calibration *)a)->voltage; >> } >> >> +#ifdef CONFIG_OF >> +static struct twl4030_madc_bat_platform_data * >> + twl4030_madc_dt_probe(struct platform_device *pdev) >> +{ >> + struct twl4030_madc_bat_platform_data *pdata; >> + struct device_node *np = pdev->dev.of_node; >> + struct property *prop; >> + int ret; >> + int sz, i, j = 0; >> + >> + pdata = devm_kzalloc(&pdev->dev, >> + sizeof(struct twl4030_madc_bat_platform_data), >> + GFP_KERNEL); >> + if (!pdata) >> + return ERR_PTR(-ENOMEM); >> + >> + ret = of_property_read_u32(np, "capacity", &pdata->capacity); >> + if (ret != 0) >> + return ERR_PTR(-EINVAL); >> + >> + /* parse and prepare charging data */ >> + prop = of_find_property(np, "charging-calibration-data", &sz); >> + if (!prop) >> + return ERR_PTR(-EINVAL); >> + >> + if (sz % 2) { >> + dev_warn(&pdev->dev, "Count of charging-calibration-data must be even!\n"); >> + return ERR_PTR(-EINVAL); >> + } > > As sz is in bytes this checks that the property is a multiple of 2 > bytes, not that it has an even number of u32 elements. > > Heiko Stübner recently added of_property_count_u32_elems [1,2]. Use that > instead. OK I'll convert it to that approach. Then seems code will be much easier. Thanks for pointing to it. > >> + >> + sz /= sizeof(u32); >> + >> + { >> + u32 data[sz]; >> + >> + ret = of_property_read_u32_array(np, >> + "charging-calibration-data", &data[0], sz); >> + if (ret) >> + return ERR_PTR(ret); > > Why not just allocate then try to read, possibly having to free if the > read fails? > > Otherwise we're trying to put an arbitrarily large value onto the stack > for no good reason. > >> + >> + pdata->charging = devm_kzalloc(&pdev->dev, >> + sizeof(struct twl4030_madc_bat_calibration) * (sz / 2), >> + GFP_KERNEL); >> + >> + for (i = 0; i < sz; i += 2) { >> + pdata->charging[j].voltage = data[i]; >> + pdata->charging[j].level = data[i+1]; >> + j++; > > Why not have (i = 0; i < sz/2; i++), and get rid of j? > >> + } >> + >> + pdata->charging_size = sz / 2; >> + } >> + >> + /* parse and prepare discharging data */ >> + prop = of_find_property(np, "discharging-calibration-data", &sz); >> + if (!prop) >> + return ERR_PTR(-EINVAL); >> + >> + if (sz % 2) { >> + dev_warn(&pdev->dev, "Count of discharging-calibration-data must be even!\n"); >> + return ERR_PTR(-EINVAL); >> + } > > This has the same issues as with charging-calibration-data. > > Thanks, > Mark. > > [1] http://www.spinics.net/lists/devicetree/msg21358.html > [2] http://www.spinics.net/lists/devicetree/msg21502.html BR, marek
diff --git a/Documentation/devicetree/bindings/power_supply/twl4030_madc_battery.txt b/Documentation/devicetree/bindings/power_supply/twl4030_madc_battery.txt new file mode 100644 index 0000000..bebc876 --- /dev/null +++ b/Documentation/devicetree/bindings/power_supply/twl4030_madc_battery.txt @@ -0,0 +1,15 @@ +twl4030_madc_battery + +Required properties: + - compatible : "ti,twl4030-madc-battery" + - capacity : battery capacity in uAh + - charging-calibration-data : list of voltage(mV):level(%) values for charging calibration (see example) + - discharging-calibration-data : list of voltage(mV):level(%) values for discharging calibration (see example) + +Example: + madc-battery { + compatible = "ti,twl4030-madc-battery"; + capacity = <1200000>; + charging-calibration-data = <4200 100 4100 75 4000 55 3900 25 3800 5 3700 2 3600 1 3300 0>; + discharging-calibration-data = <4200 100 4100 95 4000 70 3800 50 3700 10 3600 5 3300 0>; + }; diff --git a/drivers/power/twl4030_madc_battery.c b/drivers/power/twl4030_madc_battery.c index 7ef445a..2843382 100644 --- a/drivers/power/twl4030_madc_battery.c +++ b/drivers/power/twl4030_madc_battery.c @@ -19,6 +19,7 @@ #include <linux/sort.h> #include <linux/i2c/twl4030-madc.h> #include <linux/power/twl4030_madc_battery.h> +#include <linux/of.h> struct twl4030_madc_battery { struct power_supply psy; @@ -188,6 +189,110 @@ static int twl4030_cmp(const void *a, const void *b) ((struct twl4030_madc_bat_calibration *)a)->voltage; } +#ifdef CONFIG_OF +static struct twl4030_madc_bat_platform_data * + twl4030_madc_dt_probe(struct platform_device *pdev) +{ + struct twl4030_madc_bat_platform_data *pdata; + struct device_node *np = pdev->dev.of_node; + struct property *prop; + int ret; + int sz, i, j = 0; + + pdata = devm_kzalloc(&pdev->dev, + sizeof(struct twl4030_madc_bat_platform_data), + GFP_KERNEL); + if (!pdata) + return ERR_PTR(-ENOMEM); + + ret = of_property_read_u32(np, "capacity", &pdata->capacity); + if (ret != 0) + return ERR_PTR(-EINVAL); + + /* parse and prepare charging data */ + prop = of_find_property(np, "charging-calibration-data", &sz); + if (!prop) + return ERR_PTR(-EINVAL); + + if (sz % 2) { + dev_warn(&pdev->dev, "Count of charging-calibration-data must be even!\n"); + return ERR_PTR(-EINVAL); + } + + sz /= sizeof(u32); + + { + u32 data[sz]; + + ret = of_property_read_u32_array(np, + "charging-calibration-data", &data[0], sz); + if (ret) + return ERR_PTR(ret); + + pdata->charging = devm_kzalloc(&pdev->dev, + sizeof(struct twl4030_madc_bat_calibration) * (sz / 2), + GFP_KERNEL); + + for (i = 0; i < sz; i += 2) { + pdata->charging[j].voltage = data[i]; + pdata->charging[j].level = data[i+1]; + j++; + } + + pdata->charging_size = sz / 2; + } + + /* parse and prepare discharging data */ + prop = of_find_property(np, "discharging-calibration-data", &sz); + if (!prop) + return ERR_PTR(-EINVAL); + + if (sz % 2) { + dev_warn(&pdev->dev, "Count of discharging-calibration-data must be even!\n"); + return ERR_PTR(-EINVAL); + } + + sz /= sizeof(u32); + + { + u32 data[sz]; + + ret = of_property_read_u32_array(np, + "discharging-calibration-data", &data[0], sz); + if (ret) + return ERR_PTR(ret); + + j = 0; + + pdata->discharging = devm_kzalloc(&pdev->dev, + sizeof(struct twl4030_madc_bat_calibration) * (sz / 2), + GFP_KERNEL); + + for (i = 0; i < sz; i += 2) { + pdata->discharging[j].voltage = data[i]; + pdata->discharging[j].level = data[i+1]; + j++; + } + + pdata->discharging_size = sz / 2; + } + + return pdata; +} + +static const struct of_device_id of_twl4030_madc_match[] = { + { .compatible = "ti,twl4030-madc-battery", }, + {}, +}; + +#else +static struct twl4030_madc_bat_platform_data * + twl4030_madc_dt_probe(struct platform_device *pdev) +{ + return ERR_PTR(-ENODEV); +} +#endif + static int twl4030_madc_battery_probe(struct platform_device *pdev) { struct twl4030_madc_battery *twl4030_madc_bat; @@ -197,6 +302,9 @@ static int twl4030_madc_battery_probe(struct platform_device *pdev) if (!twl4030_madc_bat) return -ENOMEM; + if (!pdata) + pdata = twl4030_madc_dt_probe(pdev); + twl4030_madc_bat->psy.name = "twl4030_battery"; twl4030_madc_bat->psy.type = POWER_SUPPLY_TYPE_BATTERY; twl4030_madc_bat->psy.properties = twl4030_madc_bat_props; @@ -234,6 +342,7 @@ static int twl4030_madc_battery_remove(struct platform_device *pdev) static struct platform_driver twl4030_madc_battery_driver = { .driver = { .name = "twl4030_madc_battery", + .of_match_table = of_match_ptr(of_twl4030_madc_match), }, .probe = twl4030_madc_battery_probe, .remove = twl4030_madc_battery_remove,
Signed-off-by: Marek Belisko <marek@goldelico.com> --- .../bindings/power_supply/twl4030_madc_battery.txt | 15 +++ drivers/power/twl4030_madc_battery.c | 109 +++++++++++++++++++++ 2 files changed, 124 insertions(+) create mode 100644 Documentation/devicetree/bindings/power_supply/twl4030_madc_battery.txt