[PATCH 0/2] SH7709 DMA fixes

[PATCH 0/2] SH7709 DMA fixes

[Artur Rojek]

Hi, 

this series provides fixes to the SH7709 DMA controller, allowing the HP
Jornada 680 palmtop to boot Linux again. To my knowledge, this is the
first time in ~14 years someone tests upstream Linux on this device. And
with the included patches applied, I'm happy to announce that it still
works like a charm.

PS. What might be of interest to the sh-linux community, there's further
work towards this platform in my pipeline. E.g. I've added support for
the HD6446x PCMCIA controller, allowing Jornada 680 to use Ethernet and
Wi-Fi cards. However, what prevents proper upstream of such a driver is
the crude and antiquated code of the underlying platform base. For
example, the HD6446x bridge (`cchips/hd6446x/hd64461.c`) lacks the
concept of clocks, preventing the PCMCIA driver from hardware agnostic
clock gating. So what I'm looking into next is the cleanup of existing
platform code base, bringing it up-to-date with modern kernel APIs.
Hopefully it's not trying to bite more than I can chew, but eventually
ending up with Device Tree support would be really nice :)

Artur Rojek (2):
  sh: dma: fix `dmaor_read_reg`/`dmaor_write_reg` macros
  sh: dma: Correct the number of DMA channels in SH7709

 arch/sh/drivers/dma/Kconfig  | 10 ++++++----
 arch/sh/drivers/dma/dma-sh.c |  7 +++++--
 2 files changed, 11 insertions(+), 6 deletions(-)

-- 
2.40.1

[PATCH 1/2] sh: dma: fix `dmaor_read_reg`/`dmaor_write_reg` macros

[Artur Rojek]

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)
 {
-- 
2.40.1

Re: [PATCH 1/2] sh: dma: fix `dmaor_read_reg`/`dmaor_write_reg` macros

[John Paul Adrian Glaubitz]

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.

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?

Adrian

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

Re: [PATCH 1/2] sh: dma: fix `dmaor_read_reg`/`dmaor_write_reg` macros

[Artur Rojek]

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.

> 
> 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.

Cheers,
Artur

> 
> Adrian

Re: [PATCH 1/2] sh: dma: fix `dmaor_read_reg`/`dmaor_write_reg` macros

[John Paul Adrian Glaubitz]

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

Re: [PATCH 1/2] sh: dma: fix `dmaor_read_reg`/`dmaor_write_reg` macros

[John Paul Adrian Glaubitz]

On Sun, 2023-05-07 at 12:32 +0200, John Paul Adrian Glaubitz wrote:
> > 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.

I have successfully booted a current kernel with both patches applied on my
SH7785LCR board and will let it run for a few days to make sure it runs stable
and then apply the two patches to my sh-linux tree.

Thanks,
Adrian

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

Re: [PATCH 1/2] sh: dma: fix `dmaor_read_reg`/`dmaor_write_reg` macros

[Geert Uytterhoeven]

Hi Artur,

On Sun, May 7, 2023 at 11:43 AM Artur Rojek <contact@artur-rojek.eu> wrote:
> 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>\

Thanks for your patch!

> >> --- 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)

Fixes: 7f47c7189b3e8f19 ("sh: dma: More legacy cpu dma chainsawing.")
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

> >>  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.

Landisk does not seem to use DMA.
I did have CONFIG_SH_DMA=n, but enabling it does not make any difference.

> > 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.

Looks like this is still broken on e.g. SH7751R, which has 8 channels,
both handled by a single DMAOR register at offset 0x40...

While e.g. dma_base_addr() seems to have some provision for this
(cfr. the "chan >= 9" (not "8") check), dma_find_base() will fail, as
arch/sh/include/cpu-sh4/cpu/dma.h defines SH_DMAC_BASE1.
Anyway, that's not new, so I have no objection to your patch.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/2] sh: dma: fix `dmaor_read_reg`/`dmaor_write_reg` macros

[John Paul Adrian Glaubitz]

Hi Geert!

On Mon, 2023-05-08 at 13:20 +0200, Geert Uytterhoeven wrote:
> Looks like this is still broken on e.g. SH7751R, which has 8 channels,
> both handled by a single DMAOR register at offset 0x40...
> 
> While e.g. dma_base_addr() seems to have some provision for this
> (cfr. the "chan >= 9" (not "8") check), dma_find_base() will fail, as
> arch/sh/include/cpu-sh4/cpu/dma.h defines SH_DMAC_BASE1.
> Anyway, that's not new, so I have no objection to your patch.

Was SH7751R broken by 7f47c7189b3e8f19 as well?

Adrian

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

Re: [PATCH 1/2] sh: dma: fix `dmaor_read_reg`/`dmaor_write_reg` macros

[Geert Uytterhoeven]

Hi Adrian,

On Mon, May 8, 2023 at 1:28 PM John Paul Adrian Glaubitz
<glaubitz@physik.fu-berlin.de> wrote:
> On Mon, 2023-05-08 at 13:20 +0200, Geert Uytterhoeven wrote:
> > Looks like this is still broken on e.g. SH7751R, which has 8 channels,
> > both handled by a single DMAOR register at offset 0x40...
> >
> > While e.g. dma_base_addr() seems to have some provision for this
> > (cfr. the "chan >= 9" (not "8") check), dma_find_base() will fail, as
> > arch/sh/include/cpu-sh4/cpu/dma.h defines SH_DMAC_BASE1.
> > Anyway, that's not new, so I have no objection to your patch.
>
> Was SH7751R broken by 7f47c7189b3e8f19 as well?

I think so.
Before, the code to use 1 or 2 DMA engine relied on the presence of
DMAE1_IRQ, which is/was defined in arch/sh/include/cpu-sh4a/cpu/dma.h,
but not in arch/sh/include/cpu-sh4/cpu/dma.h.

It might be sufficient to fix this by just dropping the SH_DMAC_BASE1
definition from arch/sh/include/cpu-sh4/cpu/dma.h.  I'm actually
wondering why it was added (in commit 71b973a42c545682 ("sh: dma-sh
updates for multi IRQ and new SH-4A CPUs.")), because it looks like
none of the SH4-based (not SH4A!) SoCs have a second base...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/2] sh: dma: fix `dmaor_read_reg`/`dmaor_write_reg` macros

[Artur Rojek]

On 2023-05-08 13:20, Geert Uytterhoeven wrote:
> Hi Artur,
> 
> On Sun, May 7, 2023 at 11:43 AM Artur Rojek <contact@artur-rojek.eu> 
> wrote:
>> 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>\
> 
> Thanks for your patch!
> 
>> >> --- 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)
> 
> Fixes: 7f47c7189b3e8f19 ("sh: dma: More legacy cpu dma chainsawing.")
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
>> >>  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.
> 
> Landisk does not seem to use DMA.
> I did have CONFIG_SH_DMA=n, but enabling it does not make any 
> difference.
> 
>> > 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.
> 
> Looks like this is still broken on e.g. SH7751R, which has 8 channels,
> both handled by a single DMAOR register at offset 0x40...
> 
> While e.g. dma_base_addr() seems to have some provision for this
> (cfr. the "chan >= 9" (not "8") check), dma_find_base() will fail, as
> arch/sh/include/cpu-sh4/cpu/dma.h defines SH_DMAC_BASE1.

Yikes!
If this series hasn't been merged yet, perhaps we could fix this issue
in v2. I have something like this in mind (untested):
```
diff --git a/arch/sh/drivers/dma/dma-sh.c b/arch/sh/drivers/dma/dma-sh.c
index 14c18ebda400..306fba1564e5 100644
--- a/arch/sh/drivers/dma/dma-sh.c
+++ b/arch/sh/drivers/dma/dma-sh.c
@@ -18,6 +18,18 @@
  #include <cpu/dma-register.h>
  #include <cpu/dma.h>

+/*
+ * Some of the SoCs feature two DMAC modules. In such a case, the 
channels are
+ * distributed equally among them.
+ */
+#ifdef SH_DMAC_BASE1
+#define        SH_DMAC_NR_MD_CH        (CONFIG_NR_ONCHIP_DMA_CHANNELS / 
2)
+#else
+#define        SH_DMAC_NR_MD_CH        CONFIG_NR_ONCHIP_DMA_CHANNELS
+#endif
+
+#define        SH_DMAC_CH_SZ           0x10
+
  /*
   * Define the default configuration for dual address memory-memory 
transfer.
   * The 0x400 value represents auto-request, external->external.
@@ -29,7 +41,7 @@ static unsigned long dma_find_base(unsigned int chan)
         unsigned long base = SH_DMAC_BASE0;

  #ifdef SH_DMAC_BASE1
-       if (chan >= 6)
+       if (chan >= SH_DMAC_NR_MD_CH)
                 base = SH_DMAC_BASE1;
  #endif

@@ -40,13 +52,13 @@ static unsigned long dma_base_addr(unsigned int 
chan)
  {
         unsigned long base = dma_find_base(chan);

-       /* Normalize offset calculation */
-       if (chan >= 9)
-               chan -= 6;
-       if (chan >= 4)
-               base += 0x10;
+       chan = (chan % SH_DMAC_NR_MD_CH) * SH_DMAC_CH_SZ;
+
+       /* DMAOR is placed inside the channel register space. Step over 
it. */
+       if (chan >= DMAOR)
+               base += SH_DMAC_CH_SZ;

-       return base + (chan * 0x10);
+       return base + chan;
  }

  #ifdef CONFIG_SH_DMA_IRQ_MULTI
@@ -250,15 +262,11 @@ static int sh_dmac_get_dma_residue(struct 
dma_channel *chan)
  #define NR_DMAOR       1
  #endif

-/*
- * 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) + \
-                                                   DMAOR)
+#define dmaor_read_reg(n)              __raw_readw(dma_find_base((n) * 
\
+                                                   SH_DMAC_NR_MD_CH) + 
DMAOR)
  #define dmaor_write_reg(n, data)       __raw_writew(data, \
-                                                    dma_find_base((n) * 
6) + \
-                                                    DMAOR)
+                                                    dma_find_base((n) * 
\
+                                                    SH_DMAC_NR_MD_CH) + 
DMAOR)

  static inline int dmaor_reset(int no)
  {

```
Otherwise, I'll send it in separately. Of course we'll also need to fix
`SH_DMAC_BASE1` so that it's set only for SoCs that feature two DMAC
modules...

Cheers,
Artur

> Anyway, that's not new, so I have no objection to your patch.
> 
> Gr{oetje,eeting}s,
> 
>                         Geert

Re: [PATCH 1/2] sh: dma: fix `dmaor_read_reg`/`dmaor_write_reg` macros

[John Paul Adrian Glaubitz]

Hi Artur!

On Sat, 2023-05-13 at 13:41 +0200, Artur Rojek wrote:
> Yikes!
> If this series hasn't been merged yet, perhaps we could fix this issue
> in v2. I have something like this in mind (untested):
> (...)
> Otherwise, I'll send it in separately. Of course we'll also need to fix
> `SH_DMAC_BASE1` so that it's set only for SoCs that feature two DMAC
> modules...

No worries, nothing has been merged yet. For one, the merge windows for 6.4
has been closed and I also haven't merged your patches into my tree yet. Please
take your time to spin up a v2 of your patch set and test them properly.

Maybe you're also interested in the clean-up that Geert suggested in this
thread (ordering of the CPU subtypes and capitalization issues)?

Also, can you write "processor manual" instead of "PM" in the other patch
as well as don't use backticks for the macro names? In fact, I would suggest
retitling the subject to:

	sh: dma: Fix dmaor_read_reg() and dmaor_write_reg() macros

Oh, and I will retest your v2 patches before merging them, of course ;-).

Thanks,
Adrian

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

Re: [PATCH 1/2] sh: dma: fix `dmaor_read_reg`/`dmaor_write_reg` macros

[Artur Rojek]

On 2023-05-13 16:45, John Paul Adrian Glaubitz wrote:
> Hi Artur!
> 
> On Sat, 2023-05-13 at 13:41 +0200, Artur Rojek wrote:
>> Yikes!
>> If this series hasn't been merged yet, perhaps we could fix this issue
>> in v2. I have something like this in mind (untested):
>> (...)
>> Otherwise, I'll send it in separately. Of course we'll also need to 
>> fix
>> `SH_DMAC_BASE1` so that it's set only for SoCs that feature two DMAC
>> modules...
> 
> No worries, nothing has been merged yet. For one, the merge windows for 
> 6.4
> has been closed and I also haven't merged your patches into my tree 
> yet. Please
> take your time to spin up a v2 of your patch set and test them 
> properly.

Great!

> 
> Maybe you're also interested in the clean-up that Geert suggested in 
> this
> thread (ordering of the CPU subtypes and capitalization issues)?

Sure, why not - the more clean-up we do, the better :)

> 
> Also, can you write "processor manual" instead of "PM" in the other 
> patch
> as well as don't use backticks for the macro names? In fact, I would 
> suggest
> retitling the subject to:
> 
> 	sh: dma: Fix dmaor_read_reg() and dmaor_write_reg() macros
> 

Of course.
On a side note, it was supposed to be "programming manual", however I
now see that Renesas named that document as "hardware manual", so that's
what I'll put into the commit description, if you don't mind.

cheers,
Artur

> Oh, and I will retest your v2 patches before merging them, of course 
> ;-).
> 
> Thanks,
> Adrian

Re: [PATCH 1/2] sh: dma: fix `dmaor_read_reg`/`dmaor_write_reg` macros

[John Paul Adrian Glaubitz]

Hi Artur!

On Sat, 2023-05-13 at 16:57 +0200, Artur Rojek wrote:
> > Maybe you're also interested in the clean-up that Geert suggested in 
> > this
> > thread (ordering of the CPU subtypes and capitalization issues)?
> 
> Sure, why not - the more clean-up we do, the better :)

Great, thanks a lot!

> > Also, can you write "processor manual" instead of "PM" in the other 
> > patch
> > as well as don't use backticks for the macro names? In fact, I would 
> > suggest
> > retitling the subject to:
> > 
> > 	sh: dma: Fix dmaor_read_reg() and dmaor_write_reg() macros
> > 
> 
> Of course.
> On a side note, it was supposed to be "programming manual", however I
> now see that Renesas named that document as "hardware manual", so that's
> what I'll put into the commit description, if you don't mind.

Absolutely not! Looking forward to your v2 series and please take your time!

Adrian

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

Re: [PATCH 1/2] sh: dma: fix `dmaor_read_reg`/`dmaor_write_reg` macros

[John Paul Adrian Glaubitz]

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)
>  {

Reviewed-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>

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

[PATCH 2/2] sh: dma: Correct the number of DMA channels in SH7709

[Artur Rojek]

According to the PM, the DMAC found in SH7709 features only 4 channels.

Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
---
 arch/sh/drivers/dma/Kconfig | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/sh/drivers/dma/Kconfig b/arch/sh/drivers/dma/Kconfig
index 7d54f284ce10..4494d09597e9 100644
--- a/arch/sh/drivers/dma/Kconfig
+++ b/arch/sh/drivers/dma/Kconfig
@@ -28,8 +28,9 @@ config SH_DMA_API
 config NR_ONCHIP_DMA_CHANNELS
 	int
 	depends on SH_DMA
-	default "4" if CPU_SUBTYPE_SH7750  || CPU_SUBTYPE_SH7751  || \
-		       CPU_SUBTYPE_SH7750S || CPU_SUBTYPE_SH7091
+	default "4" if CPU_SUBTYPE_SH7709 || CPU_SUBTYPE_SH7750  || \
+		       CPU_SUBTYPE_SH7751 || CPU_SUBTYPE_SH7750S || \
+		       CPU_SUBTYPE_SH7091
 	default "8" if CPU_SUBTYPE_SH7750R || CPU_SUBTYPE_SH7751R || \
 		       CPU_SUBTYPE_SH7760
 	default "12" if CPU_SUBTYPE_SH7723 || CPU_SUBTYPE_SH7780  || \
@@ -37,8 +38,9 @@ config NR_ONCHIP_DMA_CHANNELS
 	default "6"
 	help
 	  This allows you to specify the number of channels that the on-chip
-	  DMAC supports. This will be 4 for SH7750/SH7751/Sh7750S/SH7091 and 8 for the
-	  SH7750R/SH7751R/SH7760, 12 for the SH7723/SH7780/SH7785/SH7724, default is 6.
+	  DMAC supports. This will be 4 for SH7709/SH7750/SH7751/Sh7750S/SH7091
+	  and 8 for the SH7750R/SH7751R/SH7760, 12 for the SH7723/SH7780/SH7785/SH7724,
+	  default is 6.
 
 config SH_DMABRG
 	bool "SH7760 DMABRG support"
-- 
2.40.1

Re: [PATCH 2/2] sh: dma: Correct the number of DMA channels in SH7709

[John Paul Adrian Glaubitz]

On Sat, 2023-05-06 at 16:17 +0200, Artur Rojek wrote:
> According to the PM, the DMAC found in SH7709 features only 4 channels.
> 
> Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
> ---
>  arch/sh/drivers/dma/Kconfig | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/sh/drivers/dma/Kconfig b/arch/sh/drivers/dma/Kconfig
> index 7d54f284ce10..4494d09597e9 100644
> --- a/arch/sh/drivers/dma/Kconfig
> +++ b/arch/sh/drivers/dma/Kconfig
> @@ -28,8 +28,9 @@ config SH_DMA_API
>  config NR_ONCHIP_DMA_CHANNELS
>  	int
>  	depends on SH_DMA
> -	default "4" if CPU_SUBTYPE_SH7750  || CPU_SUBTYPE_SH7751  || \
> -		       CPU_SUBTYPE_SH7750S || CPU_SUBTYPE_SH7091
> +	default "4" if CPU_SUBTYPE_SH7709 || CPU_SUBTYPE_SH7750  || \
> +		       CPU_SUBTYPE_SH7751 || CPU_SUBTYPE_SH7750S || \
> +		       CPU_SUBTYPE_SH7091
>  	default "8" if CPU_SUBTYPE_SH7750R || CPU_SUBTYPE_SH7751R || \
>  		       CPU_SUBTYPE_SH7760
>  	default "12" if CPU_SUBTYPE_SH7723 || CPU_SUBTYPE_SH7780  || \
> @@ -37,8 +38,9 @@ config NR_ONCHIP_DMA_CHANNELS
>  	default "6"
>  	help
>  	  This allows you to specify the number of channels that the on-chip
> -	  DMAC supports. This will be 4 for SH7750/SH7751/Sh7750S/SH7091 and 8 for the
> -	  SH7750R/SH7751R/SH7760, 12 for the SH7723/SH7780/SH7785/SH7724, default is 6.
> +	  DMAC supports. This will be 4 for SH7709/SH7750/SH7751/Sh7750S/SH7091
> +	  and 8 for the SH7750R/SH7751R/SH7760, 12 for the SH7723/SH7780/SH7785/SH7724,
> +	  default is 6.
>  
>  config SH_DMABRG
>  	bool "SH7760 DMABRG support"

I will replace "PM" with "processor manual" since the acronym is not necessarily
unambiguous, at least I didn't know at first what you were referring to. I checked
the manual [1] myself and four DMA channels is correct, thus:

Reviewed-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>

Adrian

> [1] https://www.renesas.com/us/en/document/mah/sh7709s-group-hardware-manual?r=1055106 (S. 373)

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

Re: [PATCH 2/2] sh: dma: Correct the number of DMA channels in SH7709

[Geert Uytterhoeven]

On Sat, May 6, 2023 at 4:22 PM Artur Rojek <contact@artur-rojek.eu> wrote:
> According to the PM, the DMAC found in SH7709 features only 4 channels.
>
> Signed-off-by: Artur Rojek <contact@artur-rojek.eu>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

> --- a/arch/sh/drivers/dma/Kconfig
> +++ b/arch/sh/drivers/dma/Kconfig
> @@ -28,8 +28,9 @@ config SH_DMA_API
>  config NR_ONCHIP_DMA_CHANNELS
>         int
>         depends on SH_DMA
> -       default "4" if CPU_SUBTYPE_SH7750  || CPU_SUBTYPE_SH7751  || \
> -                      CPU_SUBTYPE_SH7750S || CPU_SUBTYPE_SH7091
> +       default "4" if CPU_SUBTYPE_SH7709 || CPU_SUBTYPE_SH7750  || \
> +                      CPU_SUBTYPE_SH7751 || CPU_SUBTYPE_SH7750S || \
> +                      CPU_SUBTYPE_SH7091
>         default "8" if CPU_SUBTYPE_SH7750R || CPU_SUBTYPE_SH7751R || \
>                        CPU_SUBTYPE_SH7760
>         default "12" if CPU_SUBTYPE_SH7723 || CPU_SUBTYPE_SH7780  || \
> @@ -37,8 +38,9 @@ config NR_ONCHIP_DMA_CHANNELS
>         default "6"
>         help
>           This allows you to specify the number of channels that the on-chip
> -         DMAC supports. This will be 4 for SH7750/SH7751/Sh7750S/SH7091 and 8 for the

Might be a good opportunity to s/Sh7750S/SH7750S/

> -         SH7750R/SH7751R/SH7760, 12 for the SH7723/SH7780/SH7785/SH7724, default is 6.
> +         DMAC supports. This will be 4 for SH7709/SH7750/SH7751/Sh7750S/SH7091
> +         and 8 for the SH7750R/SH7751R/SH7760, 12 for the SH7723/SH7780/SH7785/SH7724,

... and sort the list for SoCs with 12 channels.

> +         default is 6.
>
>  config SH_DMABRG
>         bool "SH7760 DMABRG support"

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/2] sh: dma: Correct the number of DMA channels in SH7709

[John Paul Adrian Glaubitz]

Hi Geert!

On Mon, 2023-05-08 at 12:55 +0200, Geert Uytterhoeven wrote:
> On Sat, May 6, 2023 at 4:22 PM Artur Rojek <contact@artur-rojek.eu> wrote:
> > According to the PM, the DMAC found in SH7709 features only 4 channels.
> > 
> > Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
> 
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

I assume we can't find a commit for the Fixes tag? Looking at the "git blame"
for the Kconfig file, it seems the corresponding lines were changed before
the source tree was imported into git in 1da177e4c3f4.

> > --- a/arch/sh/drivers/dma/Kconfig
> > +++ b/arch/sh/drivers/dma/Kconfig
> > @@ -28,8 +28,9 @@ config SH_DMA_API
> >  config NR_ONCHIP_DMA_CHANNELS
> >         int
> >         depends on SH_DMA
> > -       default "4" if CPU_SUBTYPE_SH7750  || CPU_SUBTYPE_SH7751  || \
> > -                      CPU_SUBTYPE_SH7750S || CPU_SUBTYPE_SH7091
> > +       default "4" if CPU_SUBTYPE_SH7709 || CPU_SUBTYPE_SH7750  || \
> > +                      CPU_SUBTYPE_SH7751 || CPU_SUBTYPE_SH7750S || \
> > +                      CPU_SUBTYPE_SH7091
> >         default "8" if CPU_SUBTYPE_SH7750R || CPU_SUBTYPE_SH7751R || \
> >                        CPU_SUBTYPE_SH7760
> >         default "12" if CPU_SUBTYPE_SH7723 || CPU_SUBTYPE_SH7780  || \
> > @@ -37,8 +38,9 @@ config NR_ONCHIP_DMA_CHANNELS
> >         default "6"
> >         help
> >           This allows you to specify the number of channels that the on-chip
> > -         DMAC supports. This will be 4 for SH7750/SH7751/Sh7750S/SH7091 and 8 for the
> 
> Might be a good opportunity to s/Sh7750S/SH7750S/
> 
> > -         SH7750R/SH7751R/SH7760, 12 for the SH7723/SH7780/SH7785/SH7724, default is 6.
> > +         DMAC supports. This will be 4 for SH7709/SH7750/SH7751/Sh7750S/SH7091
> > +         and 8 for the SH7750R/SH7751R/SH7760, 12 for the SH7723/SH7780/SH7785/SH7724,
> 
> ... and sort the list for SoCs with 12 channels.
> 
> > +         default is 6.
> > 
> >  config SH_DMABRG
> >         bool "SH7760 DMABRG support"

Good point. I will send a follow-up patch to clean that up.

Adrian

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

Re: [PATCH 2/2] sh: dma: Correct the number of DMA channels in SH7709

[Geert Uytterhoeven]

Hi Adrian,

On Thu, May 11, 2023 at 10:20 PM John Paul Adrian Glaubitz
<glaubitz@physik.fu-berlin.de> wrote:
> On Mon, 2023-05-08 at 12:55 +0200, Geert Uytterhoeven wrote:
> > On Sat, May 6, 2023 at 4:22 PM Artur Rojek <contact@artur-rojek.eu> wrote:
> > > According to the PM, the DMAC found in SH7709 features only 4 channels.
> > >
> > > Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
> >
> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> I assume we can't find a commit for the Fixes tag? Looking at the "git blame"
> for the Kconfig file, it seems the corresponding lines were changed before
> the source tree was imported into git in 1da177e4c3f4.

I don't think Fixes has much relevance, as the issue is present in all
LTS kernel versions that are still maintained.
The stable machinery AI will just pick it up, based on the word "correct" in
the description.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/2] sh: dma: Correct the number of DMA channels in SH7709

[John Paul Adrian Glaubitz]

On Fri, 2023-05-12 at 09:34 +0200, Geert Uytterhoeven wrote:
> Hi Adrian,
> 
> On Thu, May 11, 2023 at 10:20 PM John Paul Adrian Glaubitz
> <glaubitz@physik.fu-berlin.de> wrote:
> > On Mon, 2023-05-08 at 12:55 +0200, Geert Uytterhoeven wrote:
> > > On Sat, May 6, 2023 at 4:22 PM Artur Rojek <contact@artur-rojek.eu> wrote:
> > > > According to the PM, the DMAC found in SH7709 features only 4 channels.
> > > > 
> > > > Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
> > > 
> > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > 
> > I assume we can't find a commit for the Fixes tag? Looking at the "git blame"
> > for the Kconfig file, it seems the corresponding lines were changed before
> > the source tree was imported into git in 1da177e4c3f4.
> 
> I don't think Fixes has much relevance, as the issue is present in all
> LTS kernel versions that are still maintained.
> The stable machinery AI will just pick it up, based on the word "correct" in
> the description.

Ah, I didn't know about that. Thanks for the clarification.

Adrian

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

Re: [PATCH 0/2] SH7709 DMA fixes

[John Paul Adrian Glaubitz]

Hi Artur!

Thanks a lot for your patches!

On Sat, 2023-05-06 at 16:17 +0200, Artur Rojek wrote:
> this series provides fixes to the SH7709 DMA controller, allowing the HP
> Jornada 680 palmtop to boot Linux again. To my knowledge, this is the
> first time in ~14 years someone tests upstream Linux on this device. And
> with the included patches applied, I'm happy to announce that it still
> works like a charm.

Wow, that sounds great! I will be happy to review your patches.

> PS. What might be of interest to the sh-linux community, there's further
> work towards this platform in my pipeline. E.g. I've added support for
> the HD6446x PCMCIA controller, allowing Jornada 680 to use Ethernet and
> Wi-Fi cards. However, what prevents proper upstream of such a driver is
> the crude and antiquated code of the underlying platform base. For
> example, the HD6446x bridge (`cchips/hd6446x/hd64461.c`) lacks the
> concept of clocks, preventing the PCMCIA driver from hardware agnostic
> clock gating. So what I'm looking into next is the cleanup of existing
> platform code base, bringing it up-to-date with modern kernel APIs.
> Hopefully it's not trying to bite more than I can chew, but eventually
> ending up with Device Tree support would be really nice :)

Yes, there is still a lot of modernization work to be done for SuperH!

Device tree has actually been worked on in the past and there is a patch
set by Yoshinori Sato to add support for device tree that got never merged,
see [1].

Geert and I are planning to have another look at these patches and see whether
we can get them into a shape where they can be merged. Maybe you can help us
with that effort.

I might not have the time for your patches today, but certainly next week
starting Monday. Very much looking forward to reviewing and merging them
into my SuperH tree [2].

Adrian

> [1] https://lore.kernel.org/all/1467564402-2649-1-git-send-email-ysato@users.sourceforge.jp/
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/glaubitz/sh-linux.git/

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

Re: [PATCH 0/2] SH7709 DMA fixes

[Rich Felker]

On Sat, May 06, 2023 at 05:25:01PM +0200, John Paul Adrian Glaubitz wrote:
> Hi Artur!
> 
> Thanks a lot for your patches!
> 
> On Sat, 2023-05-06 at 16:17 +0200, Artur Rojek wrote:
> > this series provides fixes to the SH7709 DMA controller, allowing the HP
> > Jornada 680 palmtop to boot Linux again. To my knowledge, this is the
> > first time in ~14 years someone tests upstream Linux on this device. And
> > with the included patches applied, I'm happy to announce that it still
> > works like a charm.
> 
> Wow, that sounds great! I will be happy to review your patches.
> 
> > PS. What might be of interest to the sh-linux community, there's further
> > work towards this platform in my pipeline. E.g. I've added support for
> > the HD6446x PCMCIA controller, allowing Jornada 680 to use Ethernet and
> > Wi-Fi cards. However, what prevents proper upstream of such a driver is
> > the crude and antiquated code of the underlying platform base. For
> > example, the HD6446x bridge (`cchips/hd6446x/hd64461.c`) lacks the
> > concept of clocks, preventing the PCMCIA driver from hardware agnostic
> > clock gating. So what I'm looking into next is the cleanup of existing
> > platform code base, bringing it up-to-date with modern kernel APIs.
> > Hopefully it's not trying to bite more than I can chew, but eventually
> > ending up with Device Tree support would be really nice :)
> 
> Yes, there is still a lot of modernization work to be done for SuperH!
> 
> Device tree has actually been worked on in the past and there is a patch
> set by Yoshinori Sato to add support for device tree that got never merged,
> see [1].
> 
> Geert and I are planning to have another look at these patches and see whether
> we can get them into a shape where they can be merged. Maybe you can help us
> with that effort.

At one point I tried to rebase these to run on what was (at the time)
current, and had partial success -- I got it to start booting with DT
under qemu, but my work rebasing the PCI stuff had problems and IIRC
prevented getting virtio working -- it was crashing at that stage. If
there's interest I can see if I can dig up that rebased branch in case
it would be useful to look at. It probably has mistakes but might be a
start for looking at what changed out from under the patches that
needs to change.

Rich

Re: [PATCH 0/2] SH7709 DMA fixes

[John Paul Adrian Glaubitz]

Hi Rich!

On Sat, 2023-05-06 at 12:56 -0400, Rich Felker wrote:
> At one point I tried to rebase these to run on what was (at the time)
> current, and had partial success -- I got it to start booting with DT
> under qemu, but my work rebasing the PCI stuff had problems and IIRC
> prevented getting virtio working -- it was crashing at that stage. If
> there's interest I can see if I can dig up that rebased branch in case
> it would be useful to look at. It probably has mistakes but might be a
> start for looking at what changed out from under the patches that
> needs to change.

Yes, there is definitely interest. Apologies for the late reply, I wanted
to send an answer earlier but it unfortunately went off my radar.

Adrian

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

Re: [PATCH 0/2] SH7709 DMA fixes

[John Paul Adrian Glaubitz]

Hi Artur!

On Sat, 2023-05-06 at 16:17 +0200, Artur Rojek wrote:
> this series provides fixes to the SH7709 DMA controller, allowing the HP
> Jornada 680 palmtop to boot Linux again. To my knowledge, this is the
> first time in ~14 years someone tests upstream Linux on this device. And
> with the included patches applied, I'm happy to announce that it still
> works like a charm.

Your first batch of patches has been merged into Linus' tree now:

> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c17414a273b81fe4e34e11d69fc30cc8b1431614

> PS. What might be of interest to the sh-linux community, there's further
> work towards this platform in my pipeline. E.g. I've added support for
> the HD6446x PCMCIA controller, allowing Jornada 680 to use Ethernet and
> Wi-Fi cards. However, what prevents proper upstream of such a driver is
> the crude and antiquated code of the underlying platform base. For
> example, the HD6446x bridge (`cchips/hd6446x/hd64461.c`) lacks the
> concept of clocks, preventing the PCMCIA driver from hardware agnostic
> clock gating. So what I'm looking into next is the cleanup of existing
> platform code base, bringing it up-to-date with modern kernel APIs.
> Hopefully it's not trying to bite more than I can chew, but eventually
> ending up with Device Tree support would be really nice :)

Feel free to start sending in more patches addressing the issues mentioned above.

Adrian

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