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