Hello Linus, > On Wed, Nov 22, 2023 at 8:47 AM Gregory CLEMENT > <gregory.clement@xxxxxxxxxxx> wrote: > >> From: Arnd Bergmann <arnd@xxxxxxxx> >> >> Since there is no guarantee that the memory returned by >> dma_alloc_coherent() is associated with a 'struct page', using the >> architecture specific phys_to_page() is wrong, but using >> virt_to_page() would be as well. >> >> Stop using sg lists altogether and just use the *_single() functions >> instead. This also simplifies the code a bit since the scatterlists in >> this driver always have only one entry anyway. >> >> gcl: Add a commit log from the initial thread: > > First I thought this was Grant C. Likely, but now I realized it is > Gregory :) :) Next time I will only use gc then. > >> https://lore.kernel.org/lkml/86db0fe5-930d-4cbb-bd7d-03367da38951@xxxxxxxxxxxxxxxx/ >> >> Fixes: cb06ff102e2d7 ("ARM: PL011: Add support for Rx DMA buffer polling.") >> Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx> >> Tested-by: Gregory CLEMENT <gregory.clement@xxxxxxxxxxx> >> Signed-off-by: Gregory CLEMENT <gregory.clement@xxxxxxxxxxx> > > Fair enough. > >> struct pl011_sgbuf { >> - struct scatterlist sg; >> - char *buf; >> + dma_addr_t dma; >> + size_t len; >> + char *buf; >> }; > > Should this struct really be named "pl011_sgbuf" after this? > > It breaks Rusty Russell's API naming rules. > > What about renaming it pl011_dmabuf? OK > >> @@ -369,18 +371,11 @@ static int pl011_fifo_to_tty(struct uart_amba_port *uap) >> static int pl011_sgbuf_init(struct dma_chan *chan, struct pl011_sgbuf *sg, > > And then parameters and variables named "sg" don't make much sense > either, so just db or so for "dma buffer" if you wanna keep it short. I am preparing a v2. Thanks, Gregory > > Yours, > Linus Walleij -- Gregory Clement, Bootlin Embedded Linux and Kernel engineering http://bootlin.com
