Re: [PATCH] power: add poodle battery driver

[Sebastian Reichel <sre@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2101 bytes --]

Hi,

On Tue, Apr 07, 2015 at 03:34:30PM +0300, Dmitry Eremin-Solenikov wrote:
> > On Mon, Mar 30, 2015 at 02:24:39PM +0300, Dmitry Eremin-Solenikov wrote:
> >> Add a driver supporting battery charging on Sharp SL-5600 (poodle).
> >> Voltage and temperature readings are provided through add7846 hwmon
> >> interface. Battery voltage is in1_input (mV) and temp in in0_input
> >> (values unknown, but should be less than 2441).
> >
> > You may want to convert add7846 to iio interface, which can be
> > exposed via hwmon and requested by other kernel drivers (e.g. this
> > one).
> 
> This needs a more careful thought. I have several devices in mind,
> which provide touchscreen + ADC interfaces, where ADC is used
> for battery voltage and temperature sensing. Sometimes additional
> GPIOs are in play. Thus I'd like for now to sustain from changing
> ads7846 driver. Or, at least to delay such changes, iff there will be
> no real design/ideas how to handle such devices in a generic enough
> way.

If you have more of these devices it's even more appealing to use
the IIO interface. You can basically keep the touchscreen part of
the ads7846 driver and replace hwmon with IIO. Then other kernel
drivers can request an IIO channel, but you are still able to use
hwmon interface from userspace via iio-hwmon.

> [...]

> >> +static int poodle_battery_probe(struct platform_device *dev)
> >> +{
> >> + int ret;
> >> +
> >> + ret = gpio_request_array(poodle_batt_gpios,
> >> + ARRAY_SIZE(poodle_batt_gpios));
> >> + if (ret)
> >> + return ret;
> >> +
> >> + mutex_init(&poodle_bat_main.work_lock);
> >> +
> >> + INIT_WORK(&bat_work, poodle_bat_work);
> >> +
> >> + ret = power_supply_register(&dev->dev, &poodle_bat_main.psy);
> >> + if (ret)
> >> + goto err_psy_reg_main;
> >
> > we have devm_power_supply_register() now.
> 
> Not so sure about it. I'd like to cancel work after unregistering power_supply.
> I think I'll even switch from devm_request_irq back to request_irq to be able
> to cancel IRQs before canceling work.

Why?

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]