Re: [mtd:nand/next 11/31] drivers/mtd/nand/raw/cadence-nand-controller.c:1893:4: error: implicit declaration of function 'ioread64_rep' is invalid in C99

From: Miquel Raynal
Date: Wed Sep 21 2022 - 10:47:51 EST


Hi Valentin,

+ Arnd

The (erased) context of this thread:
https://lore.kernel.org/llvm/202209210641.MziHAbW7-lkp@xxxxxxxxx/

vkorenblit@xxxxxxxxxxx wrote on Wed, 21 Sep 2022 14:27:47 +0200:

> Hi Miquel,
>
> I see that x86_64 doesn't support readsq/writesq because of
> CONFIG_GENERIC_IOMAP (I was building for arm64). However,
> __raw_readq/writeq are available and there are a few drivers
> using them.

I would suggest to rather try using [read|write]sq() to get rid
of the CONFIG_GENERIC_IOMAP dependency. But it looks like those
functions are not defined on 32-bit architectures anyway. So if we want
the driver to compile on both arm and aarch64, it will not be enough.

In practice, I guess we should never have the 64-bit accessor executed
when running on a 32-bit platform thanks to the following conditions:

1885 u8 data_dma_width = cdns_ctrl->caps2.data_dma_width;
1886
1887 int len_in_words = (data_dma_width == 4) ? len >> 2 : len >> 3;
1888
1889 /* read alingment data */
1890 if (data_dma_width == 4)
1891 ioread32_rep(cdns_ctrl->io.virt, buf, len_in_words);
1892 else
> 1893 ioread64_rep(cdns_ctrl->io.virt, buf, len_in_words);

So maybe we could have something awful yet simple, like the following
within the Cadence driver:

#if !CONFIG_64BIT
readsq(...) { BUG()? }
#endif

Arnd, I've seen a couple of similar issues on the mailing lists in the
past 5 years but I could not find any working solution, I don't know
how all these threads have settled in the end. I thought maybe you
would have a better idea than the above hack :)

> Do you want me to re-implement readsq and writesq in Cadence
> driver privately or do you suggest a different (cleaner) approach?

I would rather prefer to avoid this solution, as anyway I believe there
is no "generic" implementation working in all cases, there were a
couple of attempts IIRC to bring generic helpers like the above for all
architectures, but none of them landed in Linus' tree, probably because
it just cannot be made...

Thanks,
Miquèl