* Jonathan Cameron | 2013-06-02 18:40:27 [+0100]:
>On 05/27/2013 08:11 PM, Sebastian Andrzej Siewior wrote:
>> From: Pantelis Antoniou <panto@xxxxxxxxxxxxxxxxxxxxxxx>
>>
>> Add an IIO map interface that consumers can use.
>>
>> While we're here fix the mfd device in the case
>> where a subdevice might not be activated.
>Any time I read 'while we're here' it rings alarm bells.
>Two issues -> Two patches please.
>
>The level of reworking here is rather high. Probably better
>to do the structural changes in one patch then follow with the
>new functionality in another.
Okay, will try.
>Other comments inline.
>>
>> Signed-off-by: Pantelis Antoniou <panto@xxxxxxxxxxxxxxxxxxxxxxx>
>> Signed-off-by: Felipe Balbi <balbi@xxxxxx>
>> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
>> ---
>> drivers/iio/adc/ti_am335x_adc.c | 53 +++++++++++++++++++++++++++++-----
>> drivers/mfd/ti_am335x_tscadc.c | 29 ++++++++++++-------
>> include/linux/mfd/ti_am335x_tscadc.h | 8 ++---
>> 3 files changed, 67 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
>> index d821d88..cceff09 100644
>> --- a/drivers/iio/adc/ti_am335x_adc.c
>> +++ b/drivers/iio/adc/ti_am335x_adc.c
>> @@ -20,16 +20,19 @@
>> #include <linux/slab.h>
>> #include <linux/interrupt.h>
>> #include <linux/platform_device.h>
>> -#include <linux/io.h>
>why? Not immediately obvious this has anything to do with the rest of this patch.
Yeah, a patch later or two it is added again :P Will drop this nonsense.
>> #include <linux/iio/iio.h>
>> #include <linux/of.h>
>> #include <linux/of_device.h>
>> +#include <linux/iio/machine.h>
>> +#include <linux/iio/driver.h>
>>
>> #include <linux/mfd/ti_am335x_tscadc.h>
>>
>> struct tiadc_device {
>> struct ti_tscadc_dev *mfd_tscadc;
>> int channels;
>> + char *buf;
>> + struct iio_map *map;
>> };
>>
>> static unsigned int tiadc_readl(struct tiadc_device *adc, unsigned int reg)
>> @@ -76,25 +79,59 @@ static void tiadc_step_config(struct tiadc_device *adc_dev)
>> static int tiadc_channel_init(struct iio_dev *indio_dev, int channels)
>> {
>> struct iio_chan_spec *chan_array;
>
>> - int i;
>> -
>> - indio_dev->num_channels = channels;
>> - chan_array = kcalloc(indio_dev->num_channels,
>> - sizeof(struct iio_chan_spec), GFP_KERNEL);
>> + struct iio_chan_spec *chan;
>> + char *s;
>> + int i, len, size, ret;
>>
>
>This is less than beautiful... It'll cost you marginally more to
>allocate a separate array for the strings, but will be a lot nicer
>to read. If we are dealing with a relatively small maximum number
>then just do it as a static const char * array of all possible
>strings.
The EVM board takes 4 and the maximum is 8. This is 20 bytes on EVM and
40 for the max. This probably does not hurt in data segment, will
change.
>
>> + size = indio_dev->num_channels * (sizeof(struct iio_chan_spec) + 6);
>> + chan_array = kzalloc(size, GFP_KERNEL);
>> if (chan_array == NULL)
>> return -ENOMEM;
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
