On Thu, Sep 19, 2013 at 06:41:22AM +0100, Jonathan Cameron wrote:
> 
> 
> >...
> >> >
> >> >  struct tiadc_device {
> >> >          struct ti_tscadc_dev *mfd_tscadc;
> >> >          int channels;
> >> >          u8 channel_line[8];
> >> >          u8 channel_step[8];
> >> >+ int buffer_en_ch_steps;
> >> >+ u32 *data;
> >> u16 *data;
> >> 
> >> Also it might actually save memory to simply have a fixed size array
> >> of the maximum size used here and avoid the extra allocations for
> >> the cost
> >> of 16 bytes in here.
> >> 
> >> Hence,
> >> 
> >> u16 data[8];
> >> >  };
> >
> >Why data[8]? The length is dynamic. 
> >This would be valid for the usual one sample per trigger case.
> >But here its continuous sampling and the hardware pushes samples
> >*quickly*
> >Dynamic allocation is needed.
> 
> As far as I can see you pull one set of channels into data[] then push that 
> out to the kfifo. 
> 
> That is repeated lots of times but only up to 8 entries are ever used in this 
> array. If not what is the maximum scanbytes can be?
> 

You have a good eye :).
I understand and will update.

Thanks
Zubair

> >
> >
> >...
> >
> >> >+static irqreturn_t tiadc_worker_h(int irq, void *private)
> >> >+{
> >> >+ struct iio_dev *indio_dev = private;
> >> >+ struct tiadc_device *adc_dev = iio_priv(indio_dev);
> >> >+ int i, k, fifo1count, read;
> >> >+ u32 *data = adc_dev->data;
> >>    u16* data = adc_dev->data;
> >> >+
> >> >+ fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
> >> >+ for (k = 0; k < fifo1count; k = k + i) {
> >> >+         for (i = 0; i < (indio_dev->scan_bytes)/4; i++) {
> >> >+                 read = tiadc_readl(adc_dev, REG_FIFO1);
> >> >+                 data[i] = read & FIFOREAD_DATA_MASK;
> 
> i is only ever up to scanbytes / 4.
> Hence max of 8 I think?
> 
> Now there is a good argument for adding some bulk filling code for the 
> buffers but that is not happening here.
> >> //This is a 12 bit number after the mask so will fit just fine into
> >16 bits.
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to