linux-kernel - Re: [PATCH 2/2] iio: ti_am335x_adc: Add continuous sampling support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 19 Sep 2013 10:55:44 +0500
From:	"Zubair Lutfullah :" <zubair.lutfullah@...il.com>
To:	Jonathan Cameron <jic23@...23.retrosnub.co.uk>
Cc:	"Zubair Lutfullah :" <zubair.lutfullah@...il.com>, jic23@....ac.uk,
	dmitry.torokhov@...il.com, linux-iio@...r.kernel.org,
	linux-input@...r.kernel.org, linux-kernel@...r.kernel.org,
	bigeasy@...utronix.de, gregkh@...uxfoundation.org
Subject: Re: [PATCH 2/2] iio: ti_am335x_adc: Add continuous sampling support

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 majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/