lkml.org 
[lkml]   [2012]   [Nov]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 4/7] mfd: max8925: support dt for power supply
    On 11/19/2012 08:24 AM, Anton Vorontsov wrote:
    > On Tue, Nov 06, 2012 at 03:42:44PM +0800, Qing Xu wrote:
    >> From: Qing Xu <qingx@marvell.com>
    >>
    >> Signed-off-by: Qing Xu <qingx@marvell.com>
    >> ---
    > W/ this patch I'm getting this warning:
    >
    > CC drivers/power/max8925_power.o
    > drivers/power/max8925_power.c: In function ‘max8925_power_probe’:
    > drivers/power/max8925_power.c:479:3: warning: statement with no effect
    > [-Wunused-value]
    >
    >> drivers/power/max8925_power.c | 57 ++++++++++++++++++++++++++++++++++++----
    >> 1 files changed, 51 insertions(+), 6 deletions(-)
    >>
    >> diff --git a/drivers/power/max8925_power.c b/drivers/power/max8925_power.c
    >> index daa333b..dd2ac2d 100644
    >> --- a/drivers/power/max8925_power.c
    >> +++ b/drivers/power/max8925_power.c
    >> @@ -426,14 +426,58 @@ static __devexit int max8925_deinit_charger(struct max8925_power_info *info)
    >> return 0;
    >> }
    >>
    >> +#ifdef CONFIG_OF
    >> +static int max8925_power_dt_init(struct platform_device *pdev,
    >> + struct max8925_power_pdata *pdata)
    >> +{
    >> + struct device_node *nproot = pdev->dev.parent->of_node, *np;
    >> + int batt_detect, topoff_threshold, fast_charge,
    >> + no_temp_support, no_insert_detect;
    > One variable declaration per line please.
    >
    >> +
    >> + if (!nproot)
    >> + return -ENODEV;
    > Please add an empty line here.
    >
    >> + np = of_find_node_by_name(nproot, "charger");
    >> + if (!np) {
    >> + dev_err(&pdev->dev, "failed to find charger node\n");
    >> + return -ENODEV;
    >> + }
    > ditto.
    >
    >> + 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;
    > ditto
    >
    >> + pr_err("batt_detect%d, topoff_threshold%d, fast_charge%d,no_temp_support%d, no_insert_detect%d\n",
    >> + batt_detect, topoff_threshold, fast_charge,
    >> + no_temp_support, no_insert_detect);
    > pr_err, are you sure? :)
    >
    >> + return 0;
    >> +}
    >> +#else
    >> +#define max8925_power_dt_init(x, y) (-1)
    > You should make it static inline function to get rid of the warning above.
    >
    >> +#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);
    >> - struct max8925_power_pdata *pdata = NULL;
    >> + struct max8925_power_pdata *pdata = pdev->dev.platform_data;
    >> struct max8925_power_info *info;
    >> int ret;
    >>
    >> - pdata = pdev->dev.platform_data;
    >> + if (pdev->dev.parent->of_node && !pdata) {
    >> + pdata = devm_kzalloc(&pdev->dev,
    >> + sizeof(struct max8925_power_pdata),
    >> + GFP_KERNEL);
    >>
    > Please move this logic into max8925_power_dt_init().
    > So it will look like
    >
    > if (!pdata)
    > pdata = max8925_power_dt_init(pdev);
    > if (!pdata) {
    > dev_err(...);
    > return ...;
    > }
    >
    >> + max8925_power_dt_init(pdev, pdata);
    >> + }
    >> if (!pdata) {
    >> dev_err(&pdev->dev, "platform data isn't assigned to "
    >> "power supply\n");
    >> @@ -453,8 +497,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 +509,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)
    >> @@ -491,6 +535,7 @@ static __devinit int max8925_power_probe(struct platform_device *pdev)
    >> info->no_insert_detect = pdata->no_insert_detect;
    >>
    >> max8925_init_charger(chip, info);
    >> +
    > This change is unrelated.
    >
    >> return 0;
    >> out_battery:
    >> power_supply_unregister(&info->battery);
    >> --
    >> 1.7.0.4
    > Thanks,
    > Anton.
    Anton, thanks for your very careful review!
    I updated in patch v2, please help to review it again.


    Thanks,
    Qing

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2012-11-19 09:02    [W:3.830 / U:0.020 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site