lkml.org 
[lkml]   [2023]   [May]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH 1/2] sh: dma: fix `dmaor_read_reg`/`dmaor_write_reg` macros
    From
    Date
    On Sun, 2023-05-07 at 11:34 +0200, Artur Rojek wrote:
    > Hi Adrian,
    >
    > On 2023-05-07 10:39, John Paul Adrian Glaubitz wrote:
    > > On Sat, 2023-05-06 at 16:17 +0200, Artur Rojek wrote:
    > > > Squash two bugs introduced into said macros in 7f47c7189b3e,
    > > > preventing
    > > > them from proper operation:
    > > > 1) Add DMAOR register offset into the address of the hw reg access,
    > > > 2) Correct a nasty typo in the DMAOR base calculation for
    > > > `dmaor_write_reg`.
    > > >
    > > > Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
    > > > ---
    > > > arch/sh/drivers/dma/dma-sh.c | 7 +++++--
    > > > 1 file changed, 5 insertions(+), 2 deletions(-)
    > > >
    > > > diff --git a/arch/sh/drivers/dma/dma-sh.c
    > > > b/arch/sh/drivers/dma/dma-sh.c
    > > > index 96c626c2cd0a..14c18ebda400 100644
    > > > --- a/arch/sh/drivers/dma/dma-sh.c
    > > > +++ b/arch/sh/drivers/dma/dma-sh.c
    > > > @@ -254,8 +254,11 @@ static int sh_dmac_get_dma_residue(struct
    > > > dma_channel *chan)
    > > > * DMAOR bases are broken out amongst channel groups. DMAOR0 manages
    > > > * channels 0 - 5, DMAOR1 6 - 11 (optional).
    > > > */
    > > > -#define dmaor_read_reg(n) __raw_readw(dma_find_base((n)*6))
    > > > -#define dmaor_write_reg(n, data) __raw_writew(data,
    > > > dma_find_base(n)*6)
    > > > +#define dmaor_read_reg(n) __raw_readw(dma_find_base((n) * 6) + \
    > > > + DMAOR)
    > > > +#define dmaor_write_reg(n, data) __raw_writew(data, \
    > > > + dma_find_base((n) * 6) + \
    > > > + DMAOR)
    > > >
    > > > static inline int dmaor_reset(int no)
    > > > {
    > >
    > > I have looked through the changes and the code and I agree that there
    > > is a typo
    > > in dmaor_write_regn() that needs to be fixed and that the DMAOR offset
    > > is missing
    > > although I don't understand why that didn't break the kernel on other
    > > SuperH systems
    > > such as my SH-7785LCR evaluation board or the LANDISK board which Geert
    > > uses.
    >
    > I also wondered that. On SH7709 it's a hard panic, it should be the same
    > elsewhere.

    I will give the patch a spin on my SH-7785LCR and see if it breaks anything.

    Maybe Geert can test it on his LANDISK board as well as Rob on the J2 Turtleboard,
    just to be safe.

    > > What I also don't understand is the factor 6 the DMA channel number is
    > > multiplied
    > > with. When looking at the definition of dma_find_base(), it seems that
    > > every channel
    > > equal to 6 or higher will return SH_DMAC_BASE1 as DMA base address.
    > > But if we multiply
    > > the parameter with 6, this will apply to every n > 0. Is that correct?
    >
    > As confusing as they look, those macros take dmaor index (a number in
    > range 0 <= n < NR_DMAOR) as parameter, then multiply it by 6 to convert
    > it to a format compatible with `dma_find_base` (which expects a channel
    > index). In practice `n` will be either 0 or 1, so dma_find_base(0 * 6)
    > will return SH_DMAC_BASE0, while dma_find_base(1 * 6) SH_DMAC_BASE1.

    OK, thanks for the clarification. Let's wait what Geert has to say on this
    next week when he is back online.

    Adrian

    --
    .''`. John Paul Adrian Glaubitz
    : :' : Debian Developer
    `. `' Physicist
    `- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913

    \
     
     \ /
      Last update: 2023-05-07 12:33    [W:4.173 / U:0.224 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site