Jonathan Cameron wrote on 2011-06-30:
> On 06/30/11 15:04, Hennerich, Michael wrote:
>> Hi Jonathan,
>>
>> static int ad7280_channel_init(struct ad7280_state *st) {
>> int dev, ch, cnt;
>>
>> st->channels = kzalloc(sizeof(*st->channels) *
>> ((st->slave_num + 1) * 12 + 1), GFP_KERNEL);
>> if (st->channels == NULL)
>> return -ENOMEM;
>> for (dev = 0, cnt = 0; dev <= st->slave_num; dev++)
>> for (ch = AD7280A_CELL_VOLTAGE_1; ch <=
> AD7280A_AUX_ADC_6; ch++, cnt++) {
>> st->channels[cnt].type = IIO_IN;
>> st->channels[cnt].indexed = 1;
>> st->channels[cnt].extend_name = NULL;
>> st->channels[cnt].channel = cnt;
>> st->channels[cnt].info_mask = (1 <<
>> IIO_CHAN_INFO_SCALE_SHARED);
>> st->channels[cnt].address = dev << 8 | ch;
>> st->channels[cnt].scan_index = cnt;
>> st->channels[cnt].scan_type.sign = 'u';
>> st->channels[cnt].scan_type.realbits = 12;
>> st->channels[cnt].scan_type.storagebits = 32;
>> st->channels[cnt].scan_type.shift = 0;
>> }
>> return cnt;
>> }
>>
>> Trying to dynamically allocate and populate a channel spec.
>> However some members are declared const. Do they really need to be
> const?
> hmm.. technically no, although a large point in the purpose of moving
> to chan spec based registration was to allow all this stuff to be
> constant static arrays. I guess there is no particular harm in taking
> them away from const though. The functions should all assume they are
> constant, but they don't really need to be before any are called.
>
> As to whether it is the right thing to do here, I guess it depends on
> how large slave num can be. Google tells me the answer is really quite
> large (50)! Hence I'm convinced you have a very good reason.
Well that's true for the AD7280. It supports up to 50 chained devices each handling
6 cells. However this drivers targets the AD7280A which only supports 8 chain devices.
Still a good reason?
> I'll push back on any drivers doing dynamic just to handle small
> numbers of channels with variants, but here I'm convinced you have a
> very good reason.
>
> Please send the patch to make these elements non constant with your
> driver (as that will clearly provide justification)
>
> I'll admit I never conceived of anyone having the ability to chain
> this many sensors.
> For smaller chained devices I'd just advise doing them as a big static
> array then setting num_channels appropriately. (glad you brought this
> up as it just made me realise my bug in the previous set I sent out!)
>
> Jonathan
>
>>
>> drivers/staging/iio/adc/ad7280a_core.c: In function
>> 'ad7280_channel_init': drivers/staging/iio/adc/ad7280a_core.c:231:
>> error: assignment of read-only member 'info_mask'
>>
>> Regards,
>> Michael
>>
>> ------------------------------------------------------------------
>> ********* Analog Devices GmbH
>> ** *****
>> ** ** Wilhelm-Wagenfeld-Strasse 6
>> ** ***** D-80807 Munich
>> ********* Germany
>> Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB
>> 40368;
>> Geschaeftsfuehrer: Dr.Carsten Suckrow, Thomas Wessel, William A.
>> Martin, Margaret Seif
>>
>>
>>
>
Greetings,
Michael
--
Analog Devices GmbH Wilhelm-Wagenfeld-Str. 6 80807 Muenchen
Sitz der Gesellschaft: Muenchen; Registergericht: Muenchen HRB 40368; Geschaeftsfuehrer:Dr.Carsten Suckrow, Thomas Wessel, William A. Martin, Margaret Seif
--
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
