Messages in this thread | | | Subject | Re: [PATCH] net: ethernet: fs-enet: remove casting dma_alloc_coherent | From | Christophe Leroy <> | Date | Tue, 16 Mar 2021 19:10:19 +0100 |
| |
Le 11/12/2020 à 17:55, David Laight a écrit : > From: Christophe Leroy >> Sent: 11 December 2020 16:43 >> >> Le 11/12/2020 à 17:07, David Laight a écrit : >>> From: Christophe Leroy >>>> Sent: 11 December 2020 15:22 >>>> >>>> Le 11/12/2020 à 09:52, Xu Wang a écrit : >>>>> Remove casting the values returned by dma_alloc_coherent. >>>> >>>> Can you explain more in the commit log ? >>>> >>>> As far as I can see, dma_alloc_coherent() doesn't return __iomem, and ring_base member is __iomem >>> >>> Which is probably wrong - that is the kernel address of kernel memory. >>> So it shouldn't have the __iomem marker. >> >> That's where the buffer descriptors are, the driver accesses to the content of the buffer >> descriptors using the IO accessors in_be16()/out_be16(). Is it not correct ? > > I've just been looking at the crap in there. > My understanding is that IO accessors are for IO devices (eg addresses > from io_remap() etc). > > Buffers allocated by dma_alloc_coherent() are normal kernel memory > and don't need any accessors. > Now you might need some barriers - mostly because an ethernet chip > can typically read a ring entry without being prodded. > IIRC there is a barrier in writel() to ensure the dma master will > 'see' all memory writes done before the IO write that kicks it into > doing some processing.
As far as I can see, writel() is using __iomem memory, see https://elixir.bootlin.com/linux/v5.12-rc2/source/include/asm-generic/io.h#L221
> > The fact that the driver contains so many __iomem casts (eg in > tx_restart) is an indication that something is badly awry. > __iomem exists to check you are using the correct type of pointer. > Any __iomem casts are dubious.
I agree, but what else can we do to guarantee proper access to that memory ?
Christophe
| |