Re: [PATCH v2] mtd: rawnand: cadence: support 64-bit slave dma interface

["Arnd Bergmann" <arnd@arndb.de>]

On Mon, Oct 17, 2022, at 5:26 PM, Valentin Korenblit wrote:
> 32-bit accesses on 64-bit sdma trigger sdma_err in intr_status register.
>
> Check dma capabilities before reading/writing from/to sdma interface.
>
> Link to discussion:
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org/thread/3NMACGIM5NDUBPXRT5RTBZON6LQE5A3B/
>
> Signed-off-by: Valentin Korenblit <vkorenblit@sequans.com>

Seems reasonable overall based on the discussion so far.

Minor comments:

> Changes v1 -> v2:
> 	- Replaced ioread64_rep by cadence_nand_readsq (suggested by Arnd)
> 	- Replaced iowrite64_rep by cadence_nand_writesq (suggested by Arnd)
> 	- Do not try to access 64-bit sdma if __raw_readq/__raw_writeq are not defined


> +static inline void cadence_nand_readsq(struct cdns_nand_ctrl 
> *cdns_ctrl,
> +				       void *buffer, unsigned int count)
> +{
> +#ifdef __raw_readq
> +	const volatile void __iomem *addr = cdns_ctrl->io.virt;
> +
> +        if (count) {
> +                u64 *buf = buffer;
> +
> +                do {
> +                        u64 x = __raw_readq(addr);
> +                        *buf++ = x;
> +                } while (--count);
> +        }

While this is correct, I suppose you don't actually need that
loop any more, since architectures that have __raw_readq()
should also also have ioread64_rep() or readsq(). We actually
just had a few issues with that in another context, but I think
starting with 6.1-rc1 you should be able to rely on that ;-)

> +#else
> +	dev_err(cdns_ctrl->dev,
> +		"cannot read 64-bit sdma on !64-bit architectures");
> +#endif

It would be nice to print an error at probe time and fail attaching
the device rather than waiting for the first transfer to continue
with incorrect data.

      Arnd

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/