[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZldFpSQPAmAhbW2I@smile.fi.intel.com>
Date: Wed, 29 May 2024 18:11:33 +0300
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: "Nechita, Ramona" <Ramona.Nechita@...log.com>
Cc: "linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
Jonathan Cameron <jic23@...nel.org>,
Lars-Peter Clausen <lars@...afoo.de>,
"Hennerich, Michael" <Michael.Hennerich@...log.com>,
Liam Girdwood <lgirdwood@...il.com>,
Mark Brown <broonie@...nel.org>, "Sa, Nuno" <Nuno.Sa@...log.com>,
Marius Cristea <marius.cristea@...rochip.com>,
"Schmitt, Marcelo" <Marcelo.Schmitt@...log.com>,
Maksim Kiselev <bigunclemax@...il.com>,
Ivan Mikhaylov <fr0st61te@...il.com>,
Marcus Folkesson <marcus.folkesson@...il.com>,
Liam Beguin <liambeguin@...il.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] drivers: iio: adc: add support for ad777x family
On Wed, May 29, 2024 at 03:01:06PM +0000, Nechita, Ramona wrote:
..
> >> + /*
> >> + * DMA (thus cache coherency maintenance) requires the
> >> + * transfer buffers to live in their own cache lines.
> >> + */
> >> + u8 reg_rx_buf[3] ____cacheline_aligned;
> >> + u8 reg_tx_buf[3];
> >
> >> + u8 spidata_rx[32];
> >> + u8 spidata_tx[32];
> >
> >These will not be cache aligned. Is it a problem?
>
> No, it should be fine without the alignment.
I.o.w. it means that only reg_*x_buf are supposed to be in the different cache lines, correct?
..
> >Btw, can't you use regmap for IO?
>
> Unfortunately, I don't think regmap could be used, because of the crc and the
> fact that data is shifted out on the SPI SDO line in the interrupt. I
> consider perhaps adding regmap to the mix might complicate things a bit.
Can you add this into the comment area of the patch?
..
> >> + ret = ad777x_spi_write(st, AD777X_REG_SRC_N_LSB, lsb);
> >> + if (ret)
> >> + return ret;
> >> + ret = ad777x_spi_write(st, AD777X_REG_SRC_N_MSB, msb);
> >> + if (ret)
> >> + return ret;
> >
> >Can you use 16-bit writes?
> >Same Q to all similar LSB/MSB write groups.
>
> I cannot do 16-bit writes due to how the spi functions on the chip and
> because the registers for MSB/LSB are at different addresses.
They are supposed to be on the different addresses.
You mean the distance between them > than stride?
..
> >> + ret = devm_add_action_or_reset(&spi->dev,
> >> + ad777x_clk_disable,
> >> + st->mclk);
> >> + if (ret)
> >> + return ret;
> >
> >So, what's wrong with the _enabled() API?
>
> Sorry, I am not sure what you mean here by _enabled() API, is there a
> different mechanism that can be used for this type of operations?
devm_clk_get_enabled()
--
With Best Regards,
Andy Shevchenko
Powered by blists - more mailing lists