Re: [PATCH V7 1/2] csky/dma: bugfix dma_sync_for_cpu/device

From: Arnd Bergmann
Date: Tue Sep 25 2018 - 06:00:08 EST


On Tue, Sep 25, 2018 at 11:46 AM Guo Ren <ren_guo@xxxxxxxxx> wrote:
>
> On Mon, Sep 24, 2018 at 10:38:04PM +0200, Arnd Bergmann wrote:
> > On Tue, Sep 18, 2018 at 3:48 PM Guo Ren <ren_guo@xxxxxxxxx> wrote:
> > >
> > > ref: https://lkml.org/lkml/2018/5/18/1068
> > >
> > > map for_cpu for_device unmap
> > > TO_DEV writeback none writeback none
> > > TO_CPU invalidate invalidate* invalidate invalidate*
> > > BIDIR writeback invalidate writeback invalidate
> > >
> > > Signed-off-by: Guo Ren <ren_guo@xxxxxxxxx>
> >
> > Same comment as for the other patch: Explain why the original
> > version is wrong first. When giving a reference to some other
> > discussion, use the "Link" tag above your Signed-off-by line.
> > To point to a discussion on lkml, lore.kernel.org is the
> > recommended archive, so it would become
> >
> > Link: https://lore.kernel.org/lkml/20180518215548.GH17671@xxxxxxxxxxxxxxxxxxxxx/
> Ok, I'll improve the comment:
>
> Fixup dma_mapping error in linux-4.19-rc3, and we must implement all
> DMA_TO_DEVICE/FROM_DEVICE/BIDIRECTIONAL for both sync_dma_for_device/cpu.
> The implementation of arch should follow the following rules:
>
> ...

That seems ok, but it's better to start with a description of the problem
rather than the 'Fixup dma_mapping error in linux-4.19-rc3' part.
I would write this like:

| The arch_sync_dma_for_cpu()/arch_sync_dma_for_device()
| implementation is broken for some combinations that end up
| in a BUG() instead of performing the necessary flushes.
|
| The implementation of arch should follow the following rules:
| ...

The imperative 'Fix up dma_mapping error ...' is what belongs
into the subject line, and is ideally revisited at the end of the
changelog comment if you want to add more detail about what
you do.

Arnd