Hi!

> The global variables for private data are not too nice. I'd like some
> more, and that would clutter the global name space even further.
> 
> Signed-off-by: Lubomir Rintel <lkund...@v3.sk>
> Reviewed-by: Andy Shevchenko <andy.shevche...@gmail.com>

Ok...

> -     olpc_bat = power_supply_register(&pdev->dev, &olpc_bat_desc, NULL);
> -     if (IS_ERR(olpc_bat)) {
> -             ret = PTR_ERR(olpc_bat);
> -             goto battery_failed;
> -     }
> +     data->olpc_bat = devm_power_supply_register(&pdev->dev, &olpc_bat_desc, 
> &psy_cfg);
> +     if (IS_ERR(data->olpc_bat))
> +             return PTR_ERR(data->olpc_bat);
>  
> -     ret = device_create_bin_file(&olpc_bat->dev, &olpc_bat_eeprom);
> +     ret = device_create_bin_file(&data->olpc_bat->dev, &olpc_bat_eeprom);
>       if (ret)
> -             goto eeprom_failed;
> +             return ret;
>  
> -     ret = device_create_file(&olpc_bat->dev, &olpc_bat_error);
> +     ret = device_create_file(&data->olpc_bat->dev, &olpc_bat_error);
>       if (ret)
>               goto error_failed;
>  
>       if (olpc_ec_wakeup_available()) {
> -             device_set_wakeup_capable(&olpc_ac->dev, true);
> -             device_set_wakeup_capable(&olpc_bat->dev, true);
> +             device_set_wakeup_capable(&data->olpc_ac->dev, true);
> +             device_set_wakeup_capable(&data->olpc_bat->dev, true);
>       }
>  
>       return 0;
>  
>  error_failed:
> -     device_remove_bin_file(&olpc_bat->dev, &olpc_bat_eeprom);
> -eeprom_failed:
> -     power_supply_unregister(olpc_bat);
> -battery_failed:
> -     power_supply_unregister(olpc_ac);
> +     device_remove_bin_file(&data->olpc_bat->dev, &olpc_bat_eeprom);
>       return ret;
>  }

...but you are changing error handling here, which is not mentioned in
the changelog, and I'm nut sure you got it right.

Are you sure?

>  static int olpc_battery_remove(struct platform_device *pdev)
>  {
> -     device_remove_file(&olpc_bat->dev, &olpc_bat_error);
> -     device_remove_bin_file(&olpc_bat->dev, &olpc_bat_eeprom);
> -     power_supply_unregister(olpc_bat);
> -     power_supply_unregister(olpc_ac);
> +     struct olpc_battery_data *data = platform_get_drvdata(pdev);
> +
> +     device_remove_file(&data->olpc_bat->dev, &olpc_bat_error);
> +     device_remove_bin_file(&data->olpc_bat->dev, &olpc_bat_eeprom);
>       return 0;
>  }

Here too.
                                                                        Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Attachment: signature.asc
Description: Digital signature

_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to