Re: [PATCH] [RFC]power: battery: Generic battery driver using IIO

From: Jonathan Cameron
Date: Sun Sep 09 2012 - 05:59:38 EST


On 09/09/2012 10:46 AM, Lars-Peter Clausen wrote:
> On 09/09/2012 11:32 AM, anish kumar wrote:
>>>>>> + channels = iio_channel_get_all(dev_name(&pdev->dev));
>>>>> I would give these explicit names and get the two individual channels by
>>>>> name. I think it will give you cleaner code.
>>>> Yes, now it will be based on pdata->voltage_channel,
>>>> pdata->current_channel and so on.We will use this to get the channel.
>>> Maybe better to just do this via the iio_map structures in the platform data
>>> and standard naming for each channel.
>>>
>>> "voltage", "current", "power" would do nicely. Note we'll have to add the
>>> relevant naming for the other side to more IIO device drivers via the
>>> 'datasheet_name' element of iio_chan_spec. A quick grep shows this
>>> is only done in drivers/staging/iio/max1363_core.c so far (bet you can't
>>> guess what is soldered to my test board :)
>> Sorry I couldn't understand this but this is what I came up with.Hope it
>> doesn't disappoint.
>> It has the advantage of getting extended easily.
>>
>> enum chan_type{
>> VOLTAGE,
>> CURRENT,
>> POWER,
>> MAX_TYPE
>> };
>>
>> struct channel_map {
>> struct iio_map channel;
>> enum chan_type type;
>> };
>>
>> struct iio_battery_platform_data {
>> struct power_supply_info battery_info;
>> struct channel_map *map;
>> int num_map;
>> char battery_name[BAT_MAX_NAME];
>> int (*cal_charge)(long);
>> int gpio_charge_finished;
>> int gpio_inverted;
>> int bat_poll_interval;
>> int jitter_delay;
>> };
>>
>> here goes the assignment:
>> for(i = 0; i < pdata->num_map; i++) {
>> switch(pdata->map[i].type)
>> case VOLTAGE:
>> adc_bat->channel[VOLTAGE] =
>> iio_channel_get(
>> pdata->map[i].channel.consumer_dev_name,
>> pdata->map[i].channel.adc_channel_label);
>> adc_bat->chan_index = 1 << VOLTAGE;
>> break;
>> case CURRENT:
>> adc_bat->channel[CURRENT] =
>> iio_channel_get(
>> pdata->map->channel->consumer_dev_name,
>> pdata->map[i].channel.adc_channel_label);
>> adc_bat->chan_index = 1 << CURRENT;
>> break;
>> case POWER:
>> adc_bat->channel[POWER] =
>> iio_channel_get(
>> pdata->map[i].channel.consumer_dev_name,
>> pdata->map[i].channel.adc_channel_label);
>> adc_bat->chan_index = 1 << POWER;
>> break;
>> default:
>> pr_info("add any other channels here in the case\n");
>> }
>>
>> With the chan_index we can know which property is set or every time we
>> need to run through pdata->map[i].type to know the property set.
>
> Hi,
>
> I think what Jonathon meant is something like this.
>
> adc_bat->channel[VOLTAGE] = iio_channel_get(dev_name(&pdev->dev), "voltage");
> ...
> adc_bat->channel[CURRENT] = iio_channel_get(dev_name(&pdev->dev), "current");
> ...
> adc_bat->channel[POWER] = iio_channel_get(dev_name(&pdev->dev), "power");
>
> That's the beauty of the channel iio_map you can map an arbitrary channel of
> your ADC to a virtual identifier.
>
> E.g. an example map could look like.
>
> static struct iio_map map[] = {
> { "AIN1", "iio-battery.1", "voltage" },
> };
>
> With this when the battery driver calls iio_channel_get(... "voltage") it will
> get the "AIN1" channel of the device.
>
Exactly what I meant, thanks Lars.

I'm not sure any consumers are actually using that interface at the moment
as the main ones are hwmon and input (in my wip branch) and in both those
cases the association doesn't matter.

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