On 09/10/2012 05:40 PM, anish kumar wrote: > From: anish kumar <anish198519851985@xxxxxxxxx> > > This is the cleaned up code after the valuable inputs from > the Jonathan, Lars and Anton. > > I have tried to accomodate all the concerns however please > let me know incase something is missed out. > > Signed-off-by: anish kumar <anish198519851985@xxxxxxxxx> Hi, > --- > drivers/power/generic-adc-battery.c | 431 +++++++++++++++++++++++++++++ > include/linux/power/generic-adc-battery.h | 33 +++ > 2 files changed, 464 insertions(+), 0 deletions(-) > create mode 100644 drivers/power/generic-adc-battery.c > create mode 100644 include/linux/power/generic-adc-battery.h > > diff --git a/drivers/power/generic-adc-battery.c b/drivers/power/generic-adc-battery.c > new file mode 100644 > index 0000000..3459740 > --- /dev/null > +++ b/drivers/power/generic-adc-battery.c > @@ -0,0 +1,431 @@ [...] > + > +static int generic_adc_bat_get_property(struct power_supply *psy, > + enum power_supply_property psp, > + union power_supply_propval *val) > +{ > + struct generic_adc_bat *adc_bat; > + int scaleint, scalepart, iio_val, ret = 0; > + long result = 0; > + > + adc_bat = to_generic_bat(psy); > + if (!adc_bat) { > + dev_err(psy->dev, "no battery infos ?!\n"); > + return -EINVAL; > + } > + > + switch (psp) { > + case POWER_SUPPLY_PROP_POWER_NOW: > + ret = iio_read_channel_raw(adc_bat->channel[POWER], > + &iio_val); > + ret = iio_read_channel_scale(adc_bat->channel[POWER], > + &scaleint, &scalepart); > + if (ret < 0) > + return ret; > + break; > + case POWER_SUPPLY_PROP_VOLTAGE_NOW: > + ret = iio_read_channel_raw(adc_bat->channel[VOLTAGE], > + &iio_val); > + ret = iio_read_channel_scale(adc_bat->channel[VOLTAGE], > + &scaleint, &scalepart); > + if (ret < 0) > + return ret; > + break; > + case POWER_SUPPLY_PROP_CURRENT_NOW: > + ret = iio_read_channel_raw(adc_bat->channel[CURRENT], > + &iio_val); > + ret = iio_read_channel_scale(adc_bat->channel[CURRENT], > + &scaleint, &scalepart); > + if (ret < 0) > + return ret; > + break; > + default: > + break; > + } > + > + switch (ret) { > + case IIO_VAL_INT: > + result = iio_val * scaleint; > + break; > + case IIO_VAL_INT_PLUS_MICRO: > + result = (s64)iio_val * (s64)scaleint + > + div_s64((s64)iio_val * (s64)scalepart, 1000000LL); > + break; > + case IIO_VAL_INT_PLUS_NANO: > + result = (s64)iio_val * (s64)scaleint + > + div_s64((s64)iio_val * (s64)scalepart, 1000000000LL); > + break; > + } I think it's a good idea to factor the channel reading and scale conversion out into a helper function. > + > + switch (psp) { > + case POWER_SUPPLY_PROP_STATUS: > + if (adc_bat->pdata.gpio_charge_finished < 0) gpio_is_valid > + val->intval = adc_bat->level == 100000 ? > + POWER_SUPPLY_STATUS_FULL : adc_bat->status; > + else > + val->intval = adc_bat->status; > + break; [...] > + return ret; > +} > + [...] > +static void generic_adc_bat_work(struct work_struct *work) > +{ > + struct generic_adc_bat *adc_bat; > + struct delayed_work *delayed_work; > + int is_charged; > + int is_plugged; > + > + delayed_work = container_of(work, > + struct delayed_work, work); > + adc_bat = container_of(delayed_work, > + struct generic_adc_bat, bat_work); > + > + /* backup battery doesn't have current monitoring capability anyway */ > + is_plugged = power_supply_am_i_supplied(&adc_bat->psy); > + adc_bat->cable_plugged = is_plugged; > + if (is_plugged != adc_bat->was_plugged) { > + adc_bat->was_plugged = is_plugged; > + if (is_plugged) > + adc_bat->status = POWER_SUPPLY_STATUS_CHARGING; > + else > + adc_bat->status = POWER_SUPPLY_STATUS_DISCHARGING; > + } else { > + if ((adc_bat->pdata.gpio_charge_finished >= 0) && is_plugged) { gpio_is_valid > + is_charged = charge_finished(adc_bat); > + if (is_charged) > + adc_bat->status = POWER_SUPPLY_STATUS_FULL; > + else > + adc_bat->status = POWER_SUPPLY_STATUS_CHARGING; > + } > + } > + > + power_supply_changed(&adc_bat->psy); > +} [...] > + > +/* > + * compare the pdata->channel names with the predefined channels and > + * returns the index of the channel in channel_name array or -1 in the > + * case of not-found. > + */ > +int channel_cmp(char *name) > +{ > + if (!strcmp(name, channel_name[VOLTAGE])) > + return VOLTAGE; > + else if (!strcmp(name, channel_name[CURRENT])) > + return CURRENT; > + else if (!strcmp(name, channel_name[POWER])) > + return POWER; > + else > + return -1; > +} > + > +static int __devinit generic_adc_bat_probe(struct platform_device *pdev) > +{ > + struct iio_battery_platform_data *pdata = pdev->dev.platform_data; > + int ret, chan_index; > + > + /* copying the battery name from platform data */ > + adc_bat.psy.name = pdata->battery_name; You should make a per device copy of adc_bat, otherwise there can only be one device at a time. > + > + /* bootup default values for the battery */ > + adc_bat.volt_value = -1; > + adc_bat.cur_value = -1; > + adc_bat.cable_plugged = 0; > + adc_bat.status = POWER_SUPPLY_STATUS_DISCHARGING; > + > + /* calculate the total number of channels */ > + for (chan_index = 0; pdata->channel_name[chan_index]; chan_index++) > + ; > + > + if (!chan_index) { > + pr_err("atleast provide one channel\n"); > + return -EINVAL; > + } > + > + /* copying the static properties and allocating extra memory for holding > + * the extra configurable properties received from platform data. > + */ > + adc_bat.psy.properties = kzalloc(sizeof(bat_props) > + + sizeof(int)*chan_index, GFP_KERNEL); > + if (!adc_bat.psy.properties) { > + ret = -ENOMEM; > + goto first_mem_fail; > + } > + memcpy(adc_bat.psy.properties, bat_props, sizeof(bat_props)); > + > + /* allocating memory for iio channels */ > + adc_bat.channel = kzalloc(chan_index * sizeof(struct iio_channel), > + GFP_KERNEL); > + if (!adc_bat.channel) { > + ret = -ENOMEM; > + goto second_mem_fail; > + } > + > + /* > + * getting channel from iio and copying the battery properties > + * based on the channel set in the platform data. > + */ > + for (chan_index = 0; pdata->channel_name[chan_index]; chan_index++) { > + int channel = channel_cmp(pdata->channel_name[chan_index]); > + if (channel < 0) { > + ret = -EINVAL; > + goto second_mem_fail; > + } > + > + adc_bat.channel[chan_index] = > + iio_channel_get(dev_name(&pdev->dev), > + pdata->channel_name[chan_index]); Just request the channel with the hardcoded channel names. There is no need to pass these in via platform data. If a channel is not found just skip it and continue with the next one. Only if 0 channels were found return an error. > + if (IS_ERR(adc_bat.channel[chan_index])) { > + ret = PTR_ERR(adc_bat.channel[chan_index]); > + goto second_mem_fail; > + } > + > + memcpy(adc_bat.psy.properties+sizeof(bat_props), > + &dyn_props[channel], sizeof(dyn_props[channel])); You need to increase the offset for each new property. > + } > + > + ret = power_supply_register(&pdev->dev, &adc_bat.psy); > + if (ret) > + goto err_reg_fail; > + > + INIT_DELAYED_WORK(&adc_bat.bat_work, generic_adc_bat_work); > + > + if (gpio_is_valid(pdata->gpio_charge_finished)) { > + ret = gpio_request(pdata->gpio_charge_finished, "charged"); > + if (ret) > + goto err_gpio; > + > + ret = request_irq(gpio_to_irq(pdata->gpio_charge_finished), > + generic_adc_bat_charged, > + IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING, > + "battery charged", &adc_bat); I'd make this request_any_context_irq, some IRQ expander use nested IRQs. > + if (ret) > + goto err_gpio; > + } else > + goto err_gpio; /* let's bail out */ > + > + platform_set_drvdata(pdev, &adc_bat); > + /* Schedule timer to check current status */ > + schedule_delayed_work(&adc_bat.bat_work, > + msecs_to_jiffies(0)); > + return 0; > + > +err_gpio: > + power_supply_unregister(&adc_bat.psy); > +err_reg_fail: > + for (chan_index = 0; pdata->channel_name[chan_index]; chan_index++) > + iio_channel_release(adc_bat.channel[chan_index]); > + kfree(adc_bat.channel); > +second_mem_fail: > + kfree(adc_bat.psy.properties); > +first_mem_fail: > + return ret; > +} > + > +static int generic_adc_bat_remove(struct platform_device *pdev) > +{ > + int chan_index; > + struct iio_battery_platform_data *pdata = pdev->dev.platform_data; > + struct generic_adc_bat *adc_bat = to_generic_bat(pdata); > + > + power_supply_unregister(&adc_bat->psy); > + > + if (pdata->gpio_charge_finished >= 0) { gpio_is_valid > + free_irq(gpio_to_irq(pdata->gpio_charge_finished), NULL); > + gpio_free(pdata->gpio_charge_finished); > + } > + > + for (chan_index = 0; pdata->channel_name[chan_index]; chan_index++) > + iio_channel_release(adc_bat->channel[chan_index]); > + cancel_delayed_work(&adc_bat->bat_work); > + return 0; > +} -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html