Messages in this thread | | | Subject | Re: [PATCH 1/2] sh: dma: fix `dmaor_read_reg`/`dmaor_write_reg` macros | From | John Paul Adrian Glaubitz <> | Date | Sun, 07 May 2023 12:32:45 +0200 |
| |
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
| |