From: Qing Xu <[email protected]>
Signed-off-by: Qing Xu <[email protected]>
---
drivers/power/max8925_power.c | 61 +++++++++++++++++++++++++++++++++++++---
1 files changed, 56 insertions(+), 5 deletions(-)
diff --git a/drivers/power/max8925_power.c b/drivers/power/max8925_power.c
index daa333b..aca3f68 100644
--- a/drivers/power/max8925_power.c
+++ b/drivers/power/max8925_power.c
@@ -426,6 +426,57 @@ static __devexit int max8925_deinit_charger(struct max8925_power_info *info)
return 0;
}
+#ifdef CONFIG_OF
+static struct max8925_power_pdata*
+max8925_power_dt_init( struct platform_device *pdev)
+{
+ struct device_node *nproot = pdev->dev.parent->of_node, *np;
+ int batt_detect;
+ int topoff_threshold;
+ int fast_charge;
+ int no_temp_support;
+ int no_insert_detect;
+ struct max8925_power_pdata *pdata;
+
+ if (!nproot)
+ return pdev->dev.platform_data;
+
+ np = of_find_node_by_name(nproot, "charger");
+ if (!np) {
+ dev_err(&pdev->dev, "failed to find charger node\n");
+ return NULL;
+ }
+
+ pdata = devm_kzalloc(&pdev->dev,
+ sizeof(struct max8925_power_pdata),
+ GFP_KERNEL);
+
+ of_property_read_u32(np, "topoff-threshold", &topoff_threshold);
+ of_property_read_u32(np, "batt-detect", &batt_detect);
+ of_property_read_u32(np, "fast-charge", &fast_charge);
+ of_property_read_u32(np, "no-insert-detect", &no_insert_detect);
+ of_property_read_u32(np, "no-temp-support", &no_temp_support);
+
+ pdata->batt_detect = batt_detect;
+ pdata->fast_charge = fast_charge;
+ pdata->topoff_threshold = topoff_threshold;
+ pdata->no_insert_detect = no_insert_detect;
+ pdata->no_temp_support = no_temp_support;
+
+ return pdata;
+}
+#else
+static struct max8925_power_pdata*
+max8925_power_dt_init( struct platform_device *pdev)
+{
+ return pdev->dev.platform_data;
+}
+#endif
+
+static char *power_supplicants[] = {
+ "max8925-battery",
+};
+
static __devinit int max8925_power_probe(struct platform_device *pdev)
{
struct max8925_chip *chip = dev_get_drvdata(pdev->dev.parent);
@@ -433,7 +484,7 @@ static __devinit int max8925_power_probe(struct platform_device *pdev)
struct max8925_power_info *info;
int ret;
- pdata = pdev->dev.platform_data;
+ pdata = max8925_power_dt_init(pdev);
if (!pdata) {
dev_err(&pdev->dev, "platform data isn't assigned to "
"power supply\n");
@@ -453,8 +504,8 @@ static __devinit int max8925_power_probe(struct platform_device *pdev)
info->ac.properties = max8925_ac_props;
info->ac.num_properties = ARRAY_SIZE(max8925_ac_props);
info->ac.get_property = max8925_ac_get_prop;
- info->ac.supplied_to = pdata->supplied_to;
- info->ac.num_supplicants = pdata->num_supplicants;
+ info->ac.supplied_to = power_supplicants;
+ info->ac.num_supplicants = ARRAY_SIZE(power_supplicants);
ret = power_supply_register(&pdev->dev, &info->ac);
if (ret)
goto out;
@@ -465,8 +516,8 @@ static __devinit int max8925_power_probe(struct platform_device *pdev)
info->usb.properties = max8925_usb_props;
info->usb.num_properties = ARRAY_SIZE(max8925_usb_props);
info->usb.get_property = max8925_usb_get_prop;
- info->usb.supplied_to = pdata->supplied_to;
- info->usb.num_supplicants = pdata->num_supplicants;
+ info->usb.supplied_to = power_supplicants;
+ info->usb.num_supplicants = ARRAY_SIZE(power_supplicants);
ret = power_supply_register(&pdev->dev, &info->usb);
if (ret)
--
1.7.0.4
On Mon, Nov 19, 2012 at 02:52:24PM +0800, Qing Xu wrote:
> From: Qing Xu <[email protected]>
>
> Signed-off-by: Qing Xu <[email protected]>
> ---
> drivers/power/max8925_power.c | 61 +++++++++++++++++++++++++++++++++++++---
> 1 files changed, 56 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/power/max8925_power.c b/drivers/power/max8925_power.c
> index daa333b..aca3f68 100644
> --- a/drivers/power/max8925_power.c
> +++ b/drivers/power/max8925_power.c
> @@ -426,6 +426,57 @@ static __devexit int max8925_deinit_charger(struct max8925_power_info *info)
> return 0;
> }
>
> +#ifdef CONFIG_OF
> +static struct max8925_power_pdata*
Need a whitespace before *.
> +max8925_power_dt_init( struct platform_device *pdev)
Unneeded tab.
> +{
> + struct device_node *nproot = pdev->dev.parent->of_node, *np;
Hmmm. No, this is cryptic. One variable declaration per line please.
> + int batt_detect;
> + int topoff_threshold;
> + int fast_charge;
> + int no_temp_support;
> + int no_insert_detect;
> + struct max8925_power_pdata *pdata;
> +
> + if (!nproot)
> + return pdev->dev.platform_data;
> +
> + np = of_find_node_by_name(nproot, "charger");
> + if (!np) {
> + dev_err(&pdev->dev, "failed to find charger node\n");
> + return NULL;
> + }
> +
> + pdata = devm_kzalloc(&pdev->dev,
> + sizeof(struct max8925_power_pdata),
> + GFP_KERNEL);
> +
> + of_property_read_u32(np, "topoff-threshold", &topoff_threshold);
> + of_property_read_u32(np, "batt-detect", &batt_detect);
> + of_property_read_u32(np, "fast-charge", &fast_charge);
> + of_property_read_u32(np, "no-insert-detect", &no_insert_detect);
> + of_property_read_u32(np, "no-temp-support", &no_temp_support);
> +
> + pdata->batt_detect = batt_detect;
> + pdata->fast_charge = fast_charge;
> + pdata->topoff_threshold = topoff_threshold;
> + pdata->no_insert_detect = no_insert_detect;
> + pdata->no_temp_support = no_temp_support;
> +
> + return pdata;
> +}
> +#else
> +static struct max8925_power_pdata*
> +max8925_power_dt_init( struct platform_device *pdev)
Unneeded tab.
> +{
> + return pdev->dev.platform_data;
> +}
> +#endif
> +
> +static char *power_supplicants[] = {
The rest of the driver uses max8925_ prefix. Also, should it be const char
*?
> + "max8925-battery",
> +};
> +
> static __devinit int max8925_power_probe(struct platform_device *pdev)
> {
> struct max8925_chip *chip = dev_get_drvdata(pdev->dev.parent);
> @@ -433,7 +484,7 @@ static __devinit int max8925_power_probe(struct platform_device *pdev)
> struct max8925_power_info *info;
> int ret;
>
> - pdata = pdev->dev.platform_data;
> + pdata = max8925_power_dt_init(pdev);
> if (!pdata) {
> dev_err(&pdev->dev, "platform data isn't assigned to "
> "power supply\n");
> @@ -453,8 +504,8 @@ static __devinit int max8925_power_probe(struct platform_device *pdev)
> info->ac.properties = max8925_ac_props;
> info->ac.num_properties = ARRAY_SIZE(max8925_ac_props);
> info->ac.get_property = max8925_ac_get_prop;
> - info->ac.supplied_to = pdata->supplied_to;
> - info->ac.num_supplicants = pdata->num_supplicants;
> + info->ac.supplied_to = power_supplicants;
So you no longer able to change supplied_to via platform data. This is a
backwards-incompatible change.
Commit message should at least explain why this is safe to do (no users in
the kernel?).
> + info->ac.num_supplicants = ARRAY_SIZE(power_supplicants);
> ret = power_supply_register(&pdev->dev, &info->ac);
> if (ret)
> goto out;
> @@ -465,8 +516,8 @@ static __devinit int max8925_power_probe(struct platform_device *pdev)
> info->usb.properties = max8925_usb_props;
> info->usb.num_properties = ARRAY_SIZE(max8925_usb_props);
> info->usb.get_property = max8925_usb_get_prop;
> - info->usb.supplied_to = pdata->supplied_to;
> - info->usb.num_supplicants = pdata->num_supplicants;
> + info->usb.supplied_to = power_supplicants;
Ditto.
> + info->usb.num_supplicants = ARRAY_SIZE(power_supplicants);
>
> ret = power_supply_register(&pdev->dev, &info->usb);
> if (ret)
> --
> 1.7.0.4
Thanks,
Anton.
On 11/19/2012 03:06 PM, Anton Vorontsov wrote:
> On Mon, Nov 19, 2012 at 02:52:24PM +0800, Qing Xu wrote:
>> From: Qing Xu <[email protected]>
>>
>> Signed-off-by: Qing Xu <[email protected]>
>> ---
>> drivers/power/max8925_power.c | 61 +++++++++++++++++++++++++++++++++++++---
>> 1 files changed, 56 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/power/max8925_power.c b/drivers/power/max8925_power.c
>> index daa333b..aca3f68 100644
>> --- a/drivers/power/max8925_power.c
>> +++ b/drivers/power/max8925_power.c
>> @@ -426,6 +426,57 @@ static __devexit int max8925_deinit_charger(struct max8925_power_info *info)
>> return 0;
>> }
>>
>> +#ifdef CONFIG_OF
>> +static struct max8925_power_pdata*
> Need a whitespace before *.
>
>> +max8925_power_dt_init( struct platform_device *pdev)
> Unneeded tab.
>
>> +{
>> + struct device_node *nproot = pdev->dev.parent->of_node, *np;
> Hmmm. No, this is cryptic. One variable declaration per line please.
>
>> + int batt_detect;
>> + int topoff_threshold;
>> + int fast_charge;
>> + int no_temp_support;
>> + int no_insert_detect;
>> + struct max8925_power_pdata *pdata;
>> +
>> + if (!nproot)
>> + return pdev->dev.platform_data;
>> +
>> + np = of_find_node_by_name(nproot, "charger");
>> + if (!np) {
>> + dev_err(&pdev->dev, "failed to find charger node\n");
>> + return NULL;
>> + }
>> +
>> + pdata = devm_kzalloc(&pdev->dev,
>> + sizeof(struct max8925_power_pdata),
>> + GFP_KERNEL);
>> +
>> + of_property_read_u32(np, "topoff-threshold", &topoff_threshold);
>> + of_property_read_u32(np, "batt-detect", &batt_detect);
>> + of_property_read_u32(np, "fast-charge", &fast_charge);
>> + of_property_read_u32(np, "no-insert-detect", &no_insert_detect);
>> + of_property_read_u32(np, "no-temp-support", &no_temp_support);
>> +
>> + pdata->batt_detect = batt_detect;
>> + pdata->fast_charge = fast_charge;
>> + pdata->topoff_threshold = topoff_threshold;
>> + pdata->no_insert_detect = no_insert_detect;
>> + pdata->no_temp_support = no_temp_support;
>> +
>> + return pdata;
>> +}
>> +#else
>> +static struct max8925_power_pdata*
>> +max8925_power_dt_init( struct platform_device *pdev)
> Unneeded tab.
>
>> +{
>> + return pdev->dev.platform_data;
>> +}
>> +#endif
>> +
>> +static char *power_supplicants[] = {
> The rest of the driver uses max8925_ prefix. Also, should it be const char
> *?
>
>> + "max8925-battery",
>> +};
>> +
>> static __devinit int max8925_power_probe(struct platform_device *pdev)
>> {
>> struct max8925_chip *chip = dev_get_drvdata(pdev->dev.parent);
>> @@ -433,7 +484,7 @@ static __devinit int max8925_power_probe(struct platform_device *pdev)
>> struct max8925_power_info *info;
>> int ret;
>>
>> - pdata = pdev->dev.platform_data;
>> + pdata = max8925_power_dt_init(pdev);
>> if (!pdata) {
>> dev_err(&pdev->dev, "platform data isn't assigned to "
>> "power supply\n");
>> @@ -453,8 +504,8 @@ static __devinit int max8925_power_probe(struct platform_device *pdev)
>> info->ac.properties = max8925_ac_props;
>> info->ac.num_properties = ARRAY_SIZE(max8925_ac_props);
>> info->ac.get_property = max8925_ac_get_prop;
>> - info->ac.supplied_to = pdata->supplied_to;
>> - info->ac.num_supplicants = pdata->num_supplicants;
>> + info->ac.supplied_to = power_supplicants;
> So you no longer able to change supplied_to via platform data. This is a
> backwards-incompatible change.
>
> Commit message should at least explain why this is safe to do (no users in
> the kernel?).
oh, I take test_power.c as example. I will remove this modification.
>> + info->ac.num_supplicants = ARRAY_SIZE(power_supplicants);
>> ret = power_supply_register(&pdev->dev, &info->ac);
>> if (ret)
>> goto out;
>> @@ -465,8 +516,8 @@ static __devinit int max8925_power_probe(struct platform_device *pdev)
>> info->usb.properties = max8925_usb_props;
>> info->usb.num_properties = ARRAY_SIZE(max8925_usb_props);
>> info->usb.get_property = max8925_usb_get_prop;
>> - info->usb.supplied_to = pdata->supplied_to;
>> - info->usb.num_supplicants = pdata->num_supplicants;
>> + info->usb.supplied_to = power_supplicants;
> Ditto.
>
>> + info->usb.num_supplicants = ARRAY_SIZE(power_supplicants);
>>
>> ret = power_supply_register(&pdev->dev, &info->usb);
>> if (ret)
>> --
>> 1.7.0.4
> Thanks,
> Anton.