linux-kernel - RE: [PATCH v6 3/3] drivers: iio: adc: add support for ad777x family

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]

Message-ID:
 <DM6PR03MB4315785FB25B980264748BCBF3782@DM6PR03MB4315.namprd03.prod.outlook.com>
Date: Thu, 10 Oct 2024 14:32:49 +0000
From: "Nechita, Ramona" <Ramona.Nechita@...log.com>
To: Andy Shevchenko <andy@...nel.org>
CC: Jonathan Cameron <jic23@...nel.org>, Lars-Peter Clausen <lars@...afoo.de>,
        "Hennerich, Michael" <Michael.Hennerich@...log.com>,
        Rob Herring
	<robh@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley
	<conor+dt@...nel.org>, "Sa, Nuno" <Nuno.Sa@...log.com>,
        David Lechner
	<dlechner@...libre.com>,
        "Schmitt, Marcelo" <Marcelo.Schmitt@...log.com>,
        Olivier Moysan <olivier.moysan@...s.st.com>,
        Dumitru Ceclan
	<mitrutzceclan@...il.com>,
        Matteo Martelli <matteomartelli3@...il.com>,
        João Paulo Gonçalves <joao.goncalves@...adex.com>,
        Alisa-Dariana Roman <alisadariana@...il.com>,
        "linux-iio@...r.kernel.org"
	<linux-iio@...r.kernel.org>,
        "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>,
        "devicetree@...r.kernel.org"
	<devicetree@...r.kernel.org>
Subject: RE: [PATCH v6 3/3] drivers: iio: adc: add support for ad777x family

Hello,

I have some questions inline before sending out a new patch.


....
>> +struct ad7779_state {
>> +	struct spi_device *spi;
>> +	const struct ad7779_chip_info *chip_info;
>> +	struct clk *mclk;
>> +	struct iio_trigger *trig;
>> +	struct completion completion;
>> +	unsigned int sampling_freq;
>> +	enum ad7779_filter filter_enabled;
>> +	/*
>> +	 * DMA (thus cache coherency maintenance) requires the
>> +	 * transfer buffers to live in their own cache lines.
>> +	 */
>> +	struct {
>> +		u32 chans[8];
>> +		s64 timestamp;
>
>	aligned_s64 timestamp;
>
>while it makes no difference in this case, this makes code aligned inside the IIO subsystem.

I might be missing something but I can't find the aligned_s64 data type, should I define it myself
in the driver?

>
>> +	} data __aligned(IIO_DMA_MINALIGN);
>
>Note, this is different alignment to the above. And isn't the buffer below should have it instead?
>
>> +	u32			spidata_tx[8];
>> +	u8			reg_rx_buf[3];
>> +	u8			reg_tx_buf[3];
>> +	u8			reset_buf[8];
>> +};
>
>....
>
>> +static int ad7779_write_raw(struct iio_dev *indio_dev,
>> +			    struct iio_chan_spec const *chan, int val, int val2,
>> +			    long mask)
>
>long? Not unsigned long?

I copied the function header directly from iio.h, shouldn't it be left as such?

>
>> +{
>> +	struct ad7779_state *st = iio_priv(indio_dev);
>> +
>> +	iio_device_claim_direct_scoped(return -EBUSY, indio_dev) {
>> +		switch (mask) {
>> +		case IIO_CHAN_INFO_CALIBSCALE:
>> +			return ad7779_set_calibscale(st, chan->channel, val2);
>> +		case IIO_CHAN_INFO_CALIBBIAS:
>> +			return ad7779_set_calibbias(st, chan->channel, val);
>> +		case IIO_CHAN_INFO_SAMP_FREQ:
>> +			return ad7779_set_sampling_frequency(st, val);
>> +		default:
>> +			return -EINVAL;
>> +		}
>> +	}
>> +	unreachable();
>> +}
>
>...
>
>> +static irqreturn_t ad7779_trigger_handler(int irq, void *p) {
>> +	struct iio_poll_func *pf = p;
>> +	struct iio_dev *indio_dev = pf->indio_dev;
>> +	struct ad7779_state *st = iio_priv(indio_dev);
>> +	int ret;
>
>...
>

Thank you!

Best Regards,
Ramona