linux-sh.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/3] SuperH DMAC fixes
@ 2023-05-27 16:44 Artur Rojek
  2023-05-27 16:44 ` [PATCH v2 1/3] sh: dma: Fix dma channel offset calculation Artur Rojek
                   ` (3 more replies)
  0 siblings, 4 replies; 20+ messages in thread
From: Artur Rojek @ 2023-05-27 16:44 UTC (permalink / raw)
  To: Yoshinori Sato, Rich Felker, John Paul Adrian Glaubitz,
	Geert Uytterhoeven
  Cc: Rafael Ignacio Zurita, linux-sh, linux-kernel, Artur Rojek

Hi all,

This is v2 of the DMAC fixes.

Patch [1/3] now also addresses varying numbers of DMAC modules and
channels.

Patch [2/3] removes SH_DMAC_BASE1 for SH4 family. To my knowledge, none
of these SoCs feature two DMAC modules.

Patch [3/3] now also sorts all the targets and the description stays
within 80 characters per line.

Tested on Jornada 680 (SH7709 compatible).

I went ahead and verified the above changes against datasheets of all
the SoCs that are currently supported. Only SoCs found in defconfigs
which enable CONFIG_SH_DMA/CONFIG_SH_DMA_API have been surveyed.

---------+--------+--------+--------------+------------------+----------
 SoC     | Family | Refs.  | DMAC modules | Chans per module | Notes  
---------+--------+--------+--------------+------------------+----------
 SH7724  | SH4A   | [1]    | 2            | 6                |
 SH7780  | SH4A   | [2]    | 2            | 6                |
 SH7786  | SH4A   | [3]    | 1 (+ 1)      | 6 (+ 4)          | #1
 SH7091  | SH4    | [4][5] | 1            | 4                | #2
 SH7751R | SH4    | [6]    | 1            | 8                |
 SH7760  | SH4    | [7]    | 1            | 8                |
 SH4-202 | SH4    | n/a    | ?            | ?                | #3
 SH7709  | SH3    | [8]    | 1            | 4                |
 SH7720  | SH3    | [9]    | 1            | 6                |
---------+--------+--------+--------------+------------------+----------

Note #1:
Technically, SH7786 features 2 DMAC modules, for a total of 10 channels.
However, only DMAC0 (6 channels) is hw register compatible with the
existing dma-sh driver.

Note #2:
This SoC, used in SEGA Dreamcast, has no publicly available datasheet.
Apparently it's an SH7750 [5] derivative. Number of modules/channels
has been cross-referenced with the KallistiOS project's source code [4].

Note #3:
No publicly available datasheet for this SoC. Apparently this CPU is
used in an FPGA-based board [10], so perhaps the DMAC properties are
synthesized in FPGA bitstream? As this is SH4, it could potentially
impact patch [2/3].

[1] https://www.renesas.com/us/en/document/mat/sh7724-users-manual-hardware p. 537
[2] https://www.renesas.com/us/en/document/mah/sh7780-hardware-manual p. 609
[3] https://www.renesas.com/us/en/document/mah/sh7786-group-users-manual-hardware p. 1081
[4] https://github.com/KallistiOS/KallistiOS/blob/ebf8d528cd8d1909150f60bef98e1a68318cbb95/kernel/arch/dreamcast/include/dc/asic.h#L91-L94
[5] https://www.renesas.com/us/en/document/mah/sh7750-sh7750s-sh7750r-group-users-manual-hardware p. 597
[6] https://www.renesas.com/us/en/document/mah/sh7751-group-sh7751r-group-users-manual-hardware p. 551
[7] https://www.renesas.com/us/en/document/mah/sh7760-group-hardware-manual p. 463
[8] https://www.renesas.com/us/en/document/mah/sh7709s-group-hardware-manual p. 373
[9] https://www.renesas.com/us/en/document/mah/sh7720-group-sh7721-group-users-manual-hardware p. 467
[10] https://web.archive.org/web/20050405021907/http://www.superh.com/products/microdev.htm

Cheers,
Artur

Artur Rojek (3):
  sh: dma: Fix dma channel offset calculation
  sh: dma: Drop incorrect SH_DMAC_BASE1 for SH4
  sh: dma: Correct the number of DMA channels in SH7709

 arch/sh/drivers/dma/Kconfig       | 14 +++++++-----
 arch/sh/drivers/dma/dma-sh.c      | 37 ++++++++++++++++++++-----------
 arch/sh/include/cpu-sh4/cpu/dma.h |  1 -
 3 files changed, 32 insertions(+), 20 deletions(-)

-- 
2.40.1


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v2 1/3] sh: dma: Fix dma channel offset calculation
  2023-05-27 16:44 [PATCH v2 0/3] SuperH DMAC fixes Artur Rojek
@ 2023-05-27 16:44 ` Artur Rojek
  2023-06-07  9:04   ` Geert Uytterhoeven
  2023-07-04 20:30   ` John Paul Adrian Glaubitz
  2023-05-27 16:44 ` [PATCH v2 2/3] sh: dma: Drop incorrect SH_DMAC_BASE1 for SH4 Artur Rojek
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 20+ messages in thread
From: Artur Rojek @ 2023-05-27 16:44 UTC (permalink / raw)
  To: Yoshinori Sato, Rich Felker, John Paul Adrian Glaubitz,
	Geert Uytterhoeven
  Cc: Rafael Ignacio Zurita, linux-sh, linux-kernel, Artur Rojek

Various SoCs of the SH3, SH4 and SH4A family, which use this driver,
feature a differing number of DMA channels, which can be distributed
between up to two DMAC modules. Existing implementation fails to
correctly accommodate for all those variations, resulting in wrong
channel offset calculations and leading to kernel panics.

Rewrite dma_base_addr() in order to properly calculate channel offsets
in a DMAC module. Fix dmaor_read_reg() and dmaor_write_reg(), so that
the correct DMAC module base is selected for the DMAOR register.

Fixes: 7f47c7189b3e8f19 ("sh: dma: More legacy cpu dma chainsawing.")
Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
---

v2: also handle differing numbers of DMAC modules and channels

 arch/sh/drivers/dma/dma-sh.c | 37 +++++++++++++++++++++++-------------
 1 file changed, 24 insertions(+), 13 deletions(-)

diff --git a/arch/sh/drivers/dma/dma-sh.c b/arch/sh/drivers/dma/dma-sh.c
index 96c626c2cd0a..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,12 +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))
-#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) * \
+						    SH_DMAC_NR_MD_CH) + DMAOR)
+#define dmaor_write_reg(n, data)	__raw_writew(data, \
+						     dma_find_base((n) * \
+						     SH_DMAC_NR_MD_CH) + DMAOR)
 
 static inline int dmaor_reset(int no)
 {
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 2/3] sh: dma: Drop incorrect SH_DMAC_BASE1 for SH4
  2023-05-27 16:44 [PATCH v2 0/3] SuperH DMAC fixes Artur Rojek
  2023-05-27 16:44 ` [PATCH v2 1/3] sh: dma: Fix dma channel offset calculation Artur Rojek
@ 2023-05-27 16:44 ` Artur Rojek
  2023-06-07  9:05   ` Geert Uytterhoeven
  2023-07-04 20:32   ` John Paul Adrian Glaubitz
  2023-05-27 16:44 ` [PATCH v2 3/3] sh: dma: Correct the number of DMA channels in SH7709 Artur Rojek
  2023-07-05 17:08 ` [PATCH v2 0/3] SuperH DMAC fixes John Paul Adrian Glaubitz
  3 siblings, 2 replies; 20+ messages in thread
From: Artur Rojek @ 2023-05-27 16:44 UTC (permalink / raw)
  To: Yoshinori Sato, Rich Felker, John Paul Adrian Glaubitz,
	Geert Uytterhoeven
  Cc: Rafael Ignacio Zurita, linux-sh, linux-kernel, Artur Rojek

None of the supported SH4 family SoCs features a second DMAC module. As
this define negatively impacts DMA channel calculation for the above
targets, remove it from the code.

Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
---

v2: new patch

 arch/sh/include/cpu-sh4/cpu/dma.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/sh/include/cpu-sh4/cpu/dma.h b/arch/sh/include/cpu-sh4/cpu/dma.h
index 38187d06b234..e97fb2c79177 100644
--- a/arch/sh/include/cpu-sh4/cpu/dma.h
+++ b/arch/sh/include/cpu-sh4/cpu/dma.h
@@ -13,6 +13,5 @@
 #define DMAE0_IRQ	evt2irq(0x6c0)
 
 #define SH_DMAC_BASE0	0xffa00000
-#define SH_DMAC_BASE1	0xffa00070
 
 #endif /* __ASM_CPU_SH4_DMA_H */
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 3/3] sh: dma: Correct the number of DMA channels in SH7709
  2023-05-27 16:44 [PATCH v2 0/3] SuperH DMAC fixes Artur Rojek
  2023-05-27 16:44 ` [PATCH v2 1/3] sh: dma: Fix dma channel offset calculation Artur Rojek
  2023-05-27 16:44 ` [PATCH v2 2/3] sh: dma: Drop incorrect SH_DMAC_BASE1 for SH4 Artur Rojek
@ 2023-05-27 16:44 ` Artur Rojek
  2023-06-07  9:16   ` Geert Uytterhoeven
  2023-07-04 20:35   ` John Paul Adrian Glaubitz
  2023-07-05 17:08 ` [PATCH v2 0/3] SuperH DMAC fixes John Paul Adrian Glaubitz
  3 siblings, 2 replies; 20+ messages in thread
From: Artur Rojek @ 2023-05-27 16:44 UTC (permalink / raw)
  To: Yoshinori Sato, Rich Felker, John Paul Adrian Glaubitz,
	Geert Uytterhoeven
  Cc: Rafael Ignacio Zurita, linux-sh, linux-kernel, Artur Rojek

According to the hardware manual [1], the DMAC found in SH7709 features
only 4 channels.

While at it, also sort the existing targets and clarify that
NR_ONCHIP_DMA_CHANNELS must be a multiply of two.

[1] https://www.renesas.com/us/en/document/mah/sh7709s-group-hardware-manual (p. 373)

Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
---

v2: - sort existing targets
    - clarify that the value must be a multiply of two

 arch/sh/drivers/dma/Kconfig | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/arch/sh/drivers/dma/Kconfig b/arch/sh/drivers/dma/Kconfig
index 7d54f284ce10..382fbb189fcf 100644
--- a/arch/sh/drivers/dma/Kconfig
+++ b/arch/sh/drivers/dma/Kconfig
@@ -28,17 +28,19 @@ 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_SH7750S || CPU_SUBTYPE_SH7751 || \
+		       CPU_SUBTYPE_SH7091
 	default "8" if CPU_SUBTYPE_SH7750R || CPU_SUBTYPE_SH7751R || \
 		       CPU_SUBTYPE_SH7760
-	default "12" if CPU_SUBTYPE_SH7723 || CPU_SUBTYPE_SH7780  || \
-			CPU_SUBTYPE_SH7785 || CPU_SUBTYPE_SH7724
+	default "12" if CPU_SUBTYPE_SH7723 || CPU_SUBTYPE_SH7724  || \
+			CPU_SUBTYPE_SH7780 || CPU_SUBTYPE_SH7785
 	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/SH7750S/SH7751/SH7091,
+	  8 for SH7750R/SH7751R/SH7760, and 12 for SH7723/SH7724/SH7780/SH7785.
+	  Default is 6. Must be an even number.
 
 config SH_DMABRG
 	bool "SH7760 DMABRG support"
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 1/3] sh: dma: Fix dma channel offset calculation
  2023-05-27 16:44 ` [PATCH v2 1/3] sh: dma: Fix dma channel offset calculation Artur Rojek
@ 2023-06-07  9:04   ` Geert Uytterhoeven
  2023-07-04 20:30   ` John Paul Adrian Glaubitz
  1 sibling, 0 replies; 20+ messages in thread
From: Geert Uytterhoeven @ 2023-06-07  9:04 UTC (permalink / raw)
  To: Artur Rojek
  Cc: Yoshinori Sato, Rich Felker, John Paul Adrian Glaubitz,
	Rafael Ignacio Zurita, linux-sh, linux-kernel

On Sat, May 27, 2023 at 6:45 PM Artur Rojek <contact@artur-rojek.eu> wrote:
> Various SoCs of the SH3, SH4 and SH4A family, which use this driver,
> feature a differing number of DMA channels, which can be distributed
> between up to two DMAC modules. Existing implementation fails to
> correctly accommodate for all those variations, resulting in wrong
> channel offset calculations and leading to kernel panics.
>
> Rewrite dma_base_addr() in order to properly calculate channel offsets
> in a DMAC module. Fix dmaor_read_reg() and dmaor_write_reg(), so that
> the correct DMAC module base is selected for the DMAOR register.
>
> Fixes: 7f47c7189b3e8f19 ("sh: dma: More legacy cpu dma chainsawing.")
> Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
> ---
>
> v2: also handle differing numbers of DMAC modules and channels

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

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 2/3] sh: dma: Drop incorrect SH_DMAC_BASE1 for SH4
  2023-05-27 16:44 ` [PATCH v2 2/3] sh: dma: Drop incorrect SH_DMAC_BASE1 for SH4 Artur Rojek
@ 2023-06-07  9:05   ` Geert Uytterhoeven
  2023-07-04 20:32   ` John Paul Adrian Glaubitz
  1 sibling, 0 replies; 20+ messages in thread
From: Geert Uytterhoeven @ 2023-06-07  9:05 UTC (permalink / raw)
  To: Artur Rojek
  Cc: Yoshinori Sato, Rich Felker, John Paul Adrian Glaubitz,
	Rafael Ignacio Zurita, linux-sh, linux-kernel

On Sat, May 27, 2023 at 6:45 PM Artur Rojek <contact@artur-rojek.eu> wrote:
> None of the supported SH4 family SoCs features a second DMAC module. As
> this define negatively impacts DMA channel calculation for the above
> targets, remove it from the code.
>
> Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
> ---
>
> v2: new patch

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

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 3/3] sh: dma: Correct the number of DMA channels in SH7709
  2023-05-27 16:44 ` [PATCH v2 3/3] sh: dma: Correct the number of DMA channels in SH7709 Artur Rojek
@ 2023-06-07  9:16   ` Geert Uytterhoeven
  2023-06-08  9:54     ` John Paul Adrian Glaubitz
  2023-07-04 20:35   ` John Paul Adrian Glaubitz
  1 sibling, 1 reply; 20+ messages in thread
From: Geert Uytterhoeven @ 2023-06-07  9:16 UTC (permalink / raw)
  To: Artur Rojek
  Cc: Yoshinori Sato, Rich Felker, John Paul Adrian Glaubitz,
	Rafael Ignacio Zurita, linux-sh, linux-kernel

Hi Artur,

On Sat, May 27, 2023 at 6:45 PM Artur Rojek <contact@artur-rojek.eu> wrote:
> According to the hardware manual [1], the DMAC found in SH7709 features
> only 4 channels.
>
> While at it, also sort the existing targets and clarify that
> NR_ONCHIP_DMA_CHANNELS must be a multiply of two.
>
> [1] https://www.renesas.com/us/en/document/mah/sh7709s-group-hardware-manual (p. 373)
>
> Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
> ---
>
> v2: - sort existing targets

Thanks for the update!

>     - clarify that the value must be a multiply of two

That's only true when there are two DMACs, right?

Even in that case, you could mitigate that by avoiding the division by

    #ifdef SH_DMAC_BASE1
   -#define        SH_DMAC_NR_MD_CH        (CONFIG_NR_ONCHIP_DMA_CHANNELS / 2)
   +#define        SH_DMAC_NR_MD_CH        6
    #else
    #define        SH_DMAC_NR_MD_CH        CONFIG_NR_ONCHIP_DMA_CHANNELS
    #endif

That is actually safer, as the user can override NR_ONCHIP_DMA_CHANNELS
when configuring his kernel, thus breaking DMA  due to an incorrect
value of SH_DMAC_NR_MD_CH.

Unfortunately we cannot protect against that when using a single DMAC,
as SH_DMAC_NR_MD_CH can be either 4, 6, or 8.

Perhaps this configuration should be moved from Kconfig to <cpu/dma.h>,
to protect against a user overriding this value?

> --- a/arch/sh/drivers/dma/Kconfig
> +++ b/arch/sh/drivers/dma/Kconfig
> @@ -28,17 +28,19 @@ 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_SH7750S || CPU_SUBTYPE_SH7751 || \
> +                      CPU_SUBTYPE_SH7091
>         default "8" if CPU_SUBTYPE_SH7750R || CPU_SUBTYPE_SH7751R || \
>                        CPU_SUBTYPE_SH7760
> -       default "12" if CPU_SUBTYPE_SH7723 || CPU_SUBTYPE_SH7780  || \
> -                       CPU_SUBTYPE_SH7785 || CPU_SUBTYPE_SH7724
> +       default "12" if CPU_SUBTYPE_SH7723 || CPU_SUBTYPE_SH7724  || \
> +                       CPU_SUBTYPE_SH7780 || CPU_SUBTYPE_SH7785
>         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/SH7750S/SH7751/SH7091,
> +         8 for SH7750R/SH7751R/SH7760, and 12 for SH7723/SH7724/SH7780/SH7785.
> +         Default is 6. Must be an even number.

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 3/3] sh: dma: Correct the number of DMA channels in SH7709
  2023-06-07  9:16   ` Geert Uytterhoeven
@ 2023-06-08  9:54     ` John Paul Adrian Glaubitz
  2023-06-08  9:58       ` Geert Uytterhoeven
  0 siblings, 1 reply; 20+ messages in thread
From: John Paul Adrian Glaubitz @ 2023-06-08  9:54 UTC (permalink / raw)
  To: Geert Uytterhoeven, Artur Rojek
  Cc: Yoshinori Sato, Rich Felker, Rafael Ignacio Zurita, linux-sh,
	linux-kernel

Hi!

Sorry for being so late to the party.

On Wed, 2023-06-07 at 11:16 +0200, Geert Uytterhoeven wrote:
> Hi Artur,
> 
> On Sat, May 27, 2023 at 6:45 PM Artur Rojek <contact@artur-rojek.eu> wrote:
> > According to the hardware manual [1], the DMAC found in SH7709 features
> > only 4 channels.
> > 
> > While at it, also sort the existing targets and clarify that
> > NR_ONCHIP_DMA_CHANNELS must be a multiply of two.
> > 
> > [1] https://www.renesas.com/us/en/document/mah/sh7709s-group-hardware-manual (p. 373)
> > 
> > Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
> > ---
> > 
> > v2: - sort existing targets
> 
> Thanks for the update!
> 
> >     - clarify that the value must be a multiply of two
> 
> That's only true when there are two DMACs, right?
> 
> Even in that case, you could mitigate that by avoiding the division by
> 
>     #ifdef SH_DMAC_BASE1
>    -#define        SH_DMAC_NR_MD_CH        (CONFIG_NR_ONCHIP_DMA_CHANNELS / 2)
>    +#define        SH_DMAC_NR_MD_CH        6
>     #else
>     #define        SH_DMAC_NR_MD_CH        CONFIG_NR_ONCHIP_DMA_CHANNELS
>     #endif

Aren't we dropping SH_DMAC_BASE1 in the other patch anyway?

> That is actually safer, as the user can override NR_ONCHIP_DMA_CHANNELS
> when configuring his kernel, thus breaking DMA  due to an incorrect
> value of SH_DMAC_NR_MD_CH.
> 
> Unfortunately we cannot protect against that when using a single DMAC,
> as SH_DMAC_NR_MD_CH can be either 4, 6, or 8.
> 
> Perhaps this configuration should be moved from Kconfig to <cpu/dma.h>,
> to protect against a user overriding this value?

Isn't SH_DMAC_NR_MD_CH already hardwired to the SoC being used?

Adrian

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 3/3] sh: dma: Correct the number of DMA channels in SH7709
  2023-06-08  9:54     ` John Paul Adrian Glaubitz
@ 2023-06-08  9:58       ` Geert Uytterhoeven
  2023-06-08 10:03         ` John Paul Adrian Glaubitz
  0 siblings, 1 reply; 20+ messages in thread
From: Geert Uytterhoeven @ 2023-06-08  9:58 UTC (permalink / raw)
  To: John Paul Adrian Glaubitz
  Cc: Artur Rojek, Yoshinori Sato, Rich Felker, Rafael Ignacio Zurita,
	linux-sh, linux-kernel

Hi Adrian,

On Thu, Jun 8, 2023 at 11:54 AM John Paul Adrian Glaubitz
<glaubitz@physik.fu-berlin.de> wrote:
> On Wed, 2023-06-07 at 11:16 +0200, Geert Uytterhoeven wrote:
> > On Sat, May 27, 2023 at 6:45 PM Artur Rojek <contact@artur-rojek.eu> wrote:
> > > According to the hardware manual [1], the DMAC found in SH7709 features
> > > only 4 channels.
> > >
> > > While at it, also sort the existing targets and clarify that
> > > NR_ONCHIP_DMA_CHANNELS must be a multiply of two.
> > >
> > > [1] https://www.renesas.com/us/en/document/mah/sh7709s-group-hardware-manual (p. 373)
> > >
> > > Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
> > > ---
> > >
> > > v2: - sort existing targets
> >
> > Thanks for the update!
> >
> > >     - clarify that the value must be a multiply of two
> >
> > That's only true when there are two DMACs, right?
> >
> > Even in that case, you could mitigate that by avoiding the division by
> >
> >     #ifdef SH_DMAC_BASE1
> >    -#define        SH_DMAC_NR_MD_CH        (CONFIG_NR_ONCHIP_DMA_CHANNELS / 2)
> >    +#define        SH_DMAC_NR_MD_CH        6
> >     #else
> >     #define        SH_DMAC_NR_MD_CH        CONFIG_NR_ONCHIP_DMA_CHANNELS
> >     #endif
>
> Aren't we dropping SH_DMAC_BASE1 in the other patch anyway?

Only for the SH4 parts that do not have it.
It is still set in arch/sh/include/cpu-sh4a/cpu/dma.h for the SH4a parts with
12 channels and 2 DMACs.

> > That is actually safer, as the user can override NR_ONCHIP_DMA_CHANNELS
> > when configuring his kernel, thus breaking DMA  due to an incorrect
> > value of SH_DMAC_NR_MD_CH.
> >
> > Unfortunately we cannot protect against that when using a single DMAC,
> > as SH_DMAC_NR_MD_CH can be either 4, 6, or 8.
> >
> > Perhaps this configuration should be moved from Kconfig to <cpu/dma.h>,
> > to protect against a user overriding this value?
>
> Isn't SH_DMAC_NR_MD_CH already hardwired to the SoC being used?

It depends on CONFIG_NR_ONCHIP_DMA_CHANNELS, while it
should be fixed based on the SoC.

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 3/3] sh: dma: Correct the number of DMA channels in SH7709
  2023-06-08  9:58       ` Geert Uytterhoeven
@ 2023-06-08 10:03         ` John Paul Adrian Glaubitz
  2023-06-17  7:31           ` John Paul Adrian Glaubitz
  0 siblings, 1 reply; 20+ messages in thread
From: John Paul Adrian Glaubitz @ 2023-06-08 10:03 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Artur Rojek, Yoshinori Sato, Rich Felker, Rafael Ignacio Zurita,
	linux-sh, linux-kernel

Hi Geert!

On Thu, 2023-06-08 at 11:58 +0200, Geert Uytterhoeven wrote:
> > 
> > Aren't we dropping SH_DMAC_BASE1 in the other patch anyway?
> 
> Only for the SH4 parts that do not have it.
> It is still set in arch/sh/include/cpu-sh4a/cpu/dma.h for the SH4a parts with
> 12 channels and 2 DMACs.

OK, thanks for the clarification. I will review the other patches tonight.

> > > That is actually safer, as the user can override NR_ONCHIP_DMA_CHANNELS
> > > when configuring his kernel, thus breaking DMA  due to an incorrect
> > > value of SH_DMAC_NR_MD_CH.
> > > 
> > > Unfortunately we cannot protect against that when using a single DMAC,
> > > as SH_DMAC_NR_MD_CH can be either 4, 6, or 8.
> > > 
> > > Perhaps this configuration should be moved from Kconfig to <cpu/dma.h>,
> > > to protect against a user overriding this value?
> > 
> > Isn't SH_DMAC_NR_MD_CH already hardwired to the SoC being used?
> 
> It depends on CONFIG_NR_ONCHIP_DMA_CHANNELS, while it
> should be fixed based on the SoC.

I agree. However, I would be fine with merging this patch set first and fixing
this particular issue in a follow-up series.

Adrian

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 3/3] sh: dma: Correct the number of DMA channels in SH7709
  2023-06-08 10:03         ` John Paul Adrian Glaubitz
@ 2023-06-17  7:31           ` John Paul Adrian Glaubitz
  2023-06-17 11:09             ` Geert Uytterhoeven
  0 siblings, 1 reply; 20+ messages in thread
From: John Paul Adrian Glaubitz @ 2023-06-17  7:31 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Artur Rojek, Yoshinori Sato, Rich Felker, Rafael Ignacio Zurita,
	linux-sh, linux-kernel

Hi Geert!

On Thu, 2023-06-08 at 12:03 +0200, John Paul Adrian Glaubitz wrote:
> > > > That is actually safer, as the user can override NR_ONCHIP_DMA_CHANNELS
> > > > when configuring his kernel, thus breaking DMA  due to an incorrect
> > > > value of SH_DMAC_NR_MD_CH.
> > > > 
> > > > Unfortunately we cannot protect against that when using a single DMAC,
> > > > as SH_DMAC_NR_MD_CH can be either 4, 6, or 8.
> > > > 
> > > > Perhaps this configuration should be moved from Kconfig to <cpu/dma.h>,
> > > > to protect against a user overriding this value?
> > > 
> > > Isn't SH_DMAC_NR_MD_CH already hardwired to the SoC being used?
> > 
> > It depends on CONFIG_NR_ONCHIP_DMA_CHANNELS, while it
> > should be fixed based on the SoC.
> 
> I agree. However, I would be fine with merging this patch set first and fixing
> this particular issue in a follow-up series.

So, my suggestion is to take this series as-is for 6.5, then get the other issues
you mentioned fixed for 6.6. I think it's already a gain when these issues are
fixed and the kernel boots on the HP Journada 680 again.

Adrian

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 3/3] sh: dma: Correct the number of DMA channels in SH7709
  2023-06-17  7:31           ` John Paul Adrian Glaubitz
@ 2023-06-17 11:09             ` Geert Uytterhoeven
  2023-07-04  5:45               ` John Paul Adrian Glaubitz
  0 siblings, 1 reply; 20+ messages in thread
From: Geert Uytterhoeven @ 2023-06-17 11:09 UTC (permalink / raw)
  To: John Paul Adrian Glaubitz
  Cc: Artur Rojek, Yoshinori Sato, Rich Felker, Rafael Ignacio Zurita,
	linux-sh, linux-kernel

Hi Adrian,

On Sat, Jun 17, 2023 at 9:32 AM John Paul Adrian Glaubitz
<glaubitz@physik.fu-berlin.de> wrote:
> On Thu, 2023-06-08 at 12:03 +0200, John Paul Adrian Glaubitz wrote:
> > > > > That is actually safer, as the user can override NR_ONCHIP_DMA_CHANNELS
> > > > > when configuring his kernel, thus breaking DMA  due to an incorrect
> > > > > value of SH_DMAC_NR_MD_CH.
> > > > >
> > > > > Unfortunately we cannot protect against that when using a single DMAC,
> > > > > as SH_DMAC_NR_MD_CH can be either 4, 6, or 8.
> > > > >
> > > > > Perhaps this configuration should be moved from Kconfig to <cpu/dma.h>,
> > > > > to protect against a user overriding this value?
> > > >
> > > > Isn't SH_DMAC_NR_MD_CH already hardwired to the SoC being used?
> > >
> > > It depends on CONFIG_NR_ONCHIP_DMA_CHANNELS, while it
> > > should be fixed based on the SoC.
> >
> > I agree. However, I would be fine with merging this patch set first and fixing
> > this particular issue in a follow-up series.
>
> So, my suggestion is to take this series as-is for 6.5, then get the other issues
> you mentioned fixed for 6.6. I think it's already a gain when these issues are
> fixed and the kernel boots on the HP Journada 680 again.

Sure, I don't want to block the acceptance of this series at all.
Thanks!

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 3/3] sh: dma: Correct the number of DMA channels in SH7709
  2023-06-17 11:09             ` Geert Uytterhoeven
@ 2023-07-04  5:45               ` John Paul Adrian Glaubitz
  2023-07-04  7:32                 ` Geert Uytterhoeven
  0 siblings, 1 reply; 20+ messages in thread
From: John Paul Adrian Glaubitz @ 2023-07-04  5:45 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Artur Rojek, Yoshinori Sato, Rich Felker, Rafael Ignacio Zurita,
	linux-sh, linux-kernel

Hi Geert!

On Sat, 2023-06-17 at 13:09 +0200, Geert Uytterhoeven wrote:
> Hi Adrian,
> 
> On Sat, Jun 17, 2023 at 9:32 AM John Paul Adrian Glaubitz
> <glaubitz@physik.fu-berlin.de> wrote:
> > On Thu, 2023-06-08 at 12:03 +0200, John Paul Adrian Glaubitz wrote:
> > > > > > That is actually safer, as the user can override NR_ONCHIP_DMA_CHANNELS
> > > > > > when configuring his kernel, thus breaking DMA  due to an incorrect
> > > > > > value of SH_DMAC_NR_MD_CH.
> > > > > > 
> > > > > > Unfortunately we cannot protect against that when using a single DMAC,
> > > > > > as SH_DMAC_NR_MD_CH can be either 4, 6, or 8.
> > > > > > 
> > > > > > Perhaps this configuration should be moved from Kconfig to <cpu/dma.h>,
> > > > > > to protect against a user overriding this value?
> > > > > 
> > > > > Isn't SH_DMAC_NR_MD_CH already hardwired to the SoC being used?
> > > > 
> > > > It depends on CONFIG_NR_ONCHIP_DMA_CHANNELS, while it
> > > > should be fixed based on the SoC.
> > > 
> > > I agree. However, I would be fine with merging this patch set first and fixing
> > > this particular issue in a follow-up series.
> > 
> > So, my suggestion is to take this series as-is for 6.5, then get the other issues
> > you mentioned fixed for 6.6. I think it's already a gain when these issues are
> > fixed and the kernel boots on the HP Journada 680 again.
> 
> Sure, I don't want to block the acceptance of this series at all.
> Thanks!

Apologies for the late reply. Would you mind adding your Reviewed-by to this patch
before I review and apply the series?

Adrian

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 3/3] sh: dma: Correct the number of DMA channels in SH7709
  2023-07-04  5:45               ` John Paul Adrian Glaubitz
@ 2023-07-04  7:32                 ` Geert Uytterhoeven
  2023-07-04  7:43                   ` John Paul Adrian Glaubitz
  0 siblings, 1 reply; 20+ messages in thread
From: Geert Uytterhoeven @ 2023-07-04  7:32 UTC (permalink / raw)
  To: John Paul Adrian Glaubitz
  Cc: Artur Rojek, Yoshinori Sato, Rich Felker, Rafael Ignacio Zurita,
	linux-sh, linux-kernel

Hi Adrian,

On Tue, Jul 4, 2023 at 7:45 AM John Paul Adrian Glaubitz
<glaubitz@physik.fu-berlin.de> wrote:
> On Sat, 2023-06-17 at 13:09 +0200, Geert Uytterhoeven wrote:
> > On Sat, Jun 17, 2023 at 9:32 AM John Paul Adrian Glaubitz
> > <glaubitz@physik.fu-berlin.de> wrote:
> > > On Thu, 2023-06-08 at 12:03 +0200, John Paul Adrian Glaubitz wrote:
> > > > > > > That is actually safer, as the user can override NR_ONCHIP_DMA_CHANNELS
> > > > > > > when configuring his kernel, thus breaking DMA  due to an incorrect
> > > > > > > value of SH_DMAC_NR_MD_CH.
> > > > > > >
> > > > > > > Unfortunately we cannot protect against that when using a single DMAC,
> > > > > > > as SH_DMAC_NR_MD_CH can be either 4, 6, or 8.
> > > > > > >
> > > > > > > Perhaps this configuration should be moved from Kconfig to <cpu/dma.h>,
> > > > > > > to protect against a user overriding this value?
> > > > > >
> > > > > > Isn't SH_DMAC_NR_MD_CH already hardwired to the SoC being used?
> > > > >
> > > > > It depends on CONFIG_NR_ONCHIP_DMA_CHANNELS, while it
> > > > > should be fixed based on the SoC.
> > > >
> > > > I agree. However, I would be fine with merging this patch set first and fixing
> > > > this particular issue in a follow-up series.
> > >
> > > So, my suggestion is to take this series as-is for 6.5, then get the other issues
> > > you mentioned fixed for 6.6. I think it's already a gain when these issues are
> > > fixed and the kernel boots on the HP Journada 680 again.
> >
> > Sure, I don't want to block the acceptance of this series at all.
> > Thanks!
>
> Apologies for the late reply. Would you mind adding your Reviewed-by to this patch
> before I review and apply the series?

With "must be a multiply of two" and "Must be an even number" removed.
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 3/3] sh: dma: Correct the number of DMA channels in SH7709
  2023-07-04  7:32                 ` Geert Uytterhoeven
@ 2023-07-04  7:43                   ` John Paul Adrian Glaubitz
  2023-07-04  7:54                     ` Geert Uytterhoeven
  0 siblings, 1 reply; 20+ messages in thread
From: John Paul Adrian Glaubitz @ 2023-07-04  7:43 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Artur Rojek, Yoshinori Sato, Rich Felker, Rafael Ignacio Zurita,
	linux-sh, linux-kernel

Hi Geert!

On Tue, 2023-07-04 at 09:32 +0200, Geert Uytterhoeven wrote:
> Hi Adrian,
> 
> On Tue, Jul 4, 2023 at 7:45 AM John Paul Adrian Glaubitz
> <glaubitz@physik.fu-berlin.de> wrote:
> > On Sat, 2023-06-17 at 13:09 +0200, Geert Uytterhoeven wrote:
> > > On Sat, Jun 17, 2023 at 9:32 AM John Paul Adrian Glaubitz
> > > <glaubitz@physik.fu-berlin.de> wrote:
> > > > On Thu, 2023-06-08 at 12:03 +0200, John Paul Adrian Glaubitz wrote:
> > > > > > > > That is actually safer, as the user can override NR_ONCHIP_DMA_CHANNELS
> > > > > > > > when configuring his kernel, thus breaking DMA  due to an incorrect
> > > > > > > > value of SH_DMAC_NR_MD_CH.
> > > > > > > > 
> > > > > > > > Unfortunately we cannot protect against that when using a single DMAC,
> > > > > > > > as SH_DMAC_NR_MD_CH can be either 4, 6, or 8.
> > > > > > > > 
> > > > > > > > Perhaps this configuration should be moved from Kconfig to <cpu/dma.h>,
> > > > > > > > to protect against a user overriding this value?
> > > > > > > 
> > > > > > > Isn't SH_DMAC_NR_MD_CH already hardwired to the SoC being used?
> > > > > > 
> > > > > > It depends on CONFIG_NR_ONCHIP_DMA_CHANNELS, while it
> > > > > > should be fixed based on the SoC.
> > > > > 
> > > > > I agree. However, I would be fine with merging this patch set first and fixing
> > > > > this particular issue in a follow-up series.
> > > > 
> > > > So, my suggestion is to take this series as-is for 6.5, then get the other issues
> > > > you mentioned fixed for 6.6. I think it's already a gain when these issues are
> > > > fixed and the kernel boots on the HP Journada 680 again.
> > > 
> > > Sure, I don't want to block the acceptance of this series at all.
> > > Thanks!
> > 
> > Apologies for the late reply. Would you mind adding your Reviewed-by to this patch
> > before I review and apply the series?
> 
> With "must be a multiply of two" and "Must be an even number" removed.
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Thanks. I guess, I will drop the whole

	"and clarify that NR_ONCHIP_DMA_CHANNELS must be a multiply of two"

then. Correct?

Adrian

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 3/3] sh: dma: Correct the number of DMA channels in SH7709
  2023-07-04  7:43                   ` John Paul Adrian Glaubitz
@ 2023-07-04  7:54                     ` Geert Uytterhoeven
  0 siblings, 0 replies; 20+ messages in thread
From: Geert Uytterhoeven @ 2023-07-04  7:54 UTC (permalink / raw)
  To: John Paul Adrian Glaubitz
  Cc: Artur Rojek, Yoshinori Sato, Rich Felker, Rafael Ignacio Zurita,
	linux-sh, linux-kernel

Hi Adrian.

On Tue, Jul 4, 2023 at 9:43 AM John Paul Adrian Glaubitz
<glaubitz@physik.fu-berlin.de> wrote:
> On Tue, 2023-07-04 at 09:32 +0200, Geert Uytterhoeven wrote:
> > On Tue, Jul 4, 2023 at 7:45 AM John Paul Adrian Glaubitz
> > <glaubitz@physik.fu-berlin.de> wrote:
> > > On Sat, 2023-06-17 at 13:09 +0200, Geert Uytterhoeven wrote:
> > > > On Sat, Jun 17, 2023 at 9:32 AM John Paul Adrian Glaubitz
> > > > <glaubitz@physik.fu-berlin.de> wrote:
> > > > > On Thu, 2023-06-08 at 12:03 +0200, John Paul Adrian Glaubitz wrote:
> > > > > > > > > That is actually safer, as the user can override NR_ONCHIP_DMA_CHANNELS
> > > > > > > > > when configuring his kernel, thus breaking DMA  due to an incorrect
> > > > > > > > > value of SH_DMAC_NR_MD_CH.
> > > > > > > > >
> > > > > > > > > Unfortunately we cannot protect against that when using a single DMAC,
> > > > > > > > > as SH_DMAC_NR_MD_CH can be either 4, 6, or 8.
> > > > > > > > >
> > > > > > > > > Perhaps this configuration should be moved from Kconfig to <cpu/dma.h>,
> > > > > > > > > to protect against a user overriding this value?
> > > > > > > >
> > > > > > > > Isn't SH_DMAC_NR_MD_CH already hardwired to the SoC being used?
> > > > > > >
> > > > > > > It depends on CONFIG_NR_ONCHIP_DMA_CHANNELS, while it
> > > > > > > should be fixed based on the SoC.
> > > > > >
> > > > > > I agree. However, I would be fine with merging this patch set first and fixing
> > > > > > this particular issue in a follow-up series.
> > > > >
> > > > > So, my suggestion is to take this series as-is for 6.5, then get the other issues
> > > > > you mentioned fixed for 6.6. I think it's already a gain when these issues are
> > > > > fixed and the kernel boots on the HP Journada 680 again.
> > > >
> > > > Sure, I don't want to block the acceptance of this series at all.
> > > > Thanks!
> > >
> > > Apologies for the late reply. Would you mind adding your Reviewed-by to this patch
> > > before I review and apply the series?
> >
> > With "must be a multiply of two" and "Must be an even number" removed.
> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> Thanks. I guess, I will drop the whole
>
>         "and clarify that NR_ONCHIP_DMA_CHANNELS must be a multiply of two"
>
> then. Correct?

Correct. Also in the help text.
Thanks!

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 1/3] sh: dma: Fix dma channel offset calculation
  2023-05-27 16:44 ` [PATCH v2 1/3] sh: dma: Fix dma channel offset calculation Artur Rojek
  2023-06-07  9:04   ` Geert Uytterhoeven
@ 2023-07-04 20:30   ` John Paul Adrian Glaubitz
  1 sibling, 0 replies; 20+ messages in thread
From: John Paul Adrian Glaubitz @ 2023-07-04 20:30 UTC (permalink / raw)
  To: Artur Rojek, Yoshinori Sato, Rich Felker, Geert Uytterhoeven
  Cc: Rafael Ignacio Zurita, linux-sh, linux-kernel

On Sat, 2023-05-27 at 18:44 +0200, Artur Rojek wrote:
> Various SoCs of the SH3, SH4 and SH4A family, which use this driver,
> feature a differing number of DMA channels, which can be distributed
> between up to two DMAC modules. Existing implementation fails to
> correctly accommodate for all those variations, resulting in wrong
> channel offset calculations and leading to kernel panics.
> 
> Rewrite dma_base_addr() in order to properly calculate channel offsets
> in a DMAC module. Fix dmaor_read_reg() and dmaor_write_reg(), so that
> the correct DMAC module base is selected for the DMAOR register.
> 
> Fixes: 7f47c7189b3e8f19 ("sh: dma: More legacy cpu dma chainsawing.")
> Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
> ---
> 
> v2: also handle differing numbers of DMAC modules and channels
> 
>  arch/sh/drivers/dma/dma-sh.c | 37 +++++++++++++++++++++++-------------
>  1 file changed, 24 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/sh/drivers/dma/dma-sh.c b/arch/sh/drivers/dma/dma-sh.c
> index 96c626c2cd0a..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,12 +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))
> -#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) * \
> +						    SH_DMAC_NR_MD_CH) + DMAOR)
> +#define dmaor_write_reg(n, data)	__raw_writew(data, \
> +						     dma_find_base((n) * \
> +						     SH_DMAC_NR_MD_CH) + 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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 2/3] sh: dma: Drop incorrect SH_DMAC_BASE1 for SH4
  2023-05-27 16:44 ` [PATCH v2 2/3] sh: dma: Drop incorrect SH_DMAC_BASE1 for SH4 Artur Rojek
  2023-06-07  9:05   ` Geert Uytterhoeven
@ 2023-07-04 20:32   ` John Paul Adrian Glaubitz
  1 sibling, 0 replies; 20+ messages in thread
From: John Paul Adrian Glaubitz @ 2023-07-04 20:32 UTC (permalink / raw)
  To: Artur Rojek, Yoshinori Sato, Rich Felker, Geert Uytterhoeven
  Cc: Rafael Ignacio Zurita, linux-sh, linux-kernel

On Sat, 2023-05-27 at 18:44 +0200, Artur Rojek wrote:
> None of the supported SH4 family SoCs features a second DMAC module. As
> this define negatively impacts DMA channel calculation for the above
> targets, remove it from the code.
> 
> Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
> ---
> 
> v2: new patch
> 
>  arch/sh/include/cpu-sh4/cpu/dma.h | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/arch/sh/include/cpu-sh4/cpu/dma.h b/arch/sh/include/cpu-sh4/cpu/dma.h
> index 38187d06b234..e97fb2c79177 100644
> --- a/arch/sh/include/cpu-sh4/cpu/dma.h
> +++ b/arch/sh/include/cpu-sh4/cpu/dma.h
> @@ -13,6 +13,5 @@
>  #define DMAE0_IRQ	evt2irq(0x6c0)
>  
>  #define SH_DMAC_BASE0	0xffa00000
> -#define SH_DMAC_BASE1	0xffa00070
>  
>  #endif /* __ASM_CPU_SH4_DMA_H */

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 3/3] sh: dma: Correct the number of DMA channels in SH7709
  2023-05-27 16:44 ` [PATCH v2 3/3] sh: dma: Correct the number of DMA channels in SH7709 Artur Rojek
  2023-06-07  9:16   ` Geert Uytterhoeven
@ 2023-07-04 20:35   ` John Paul Adrian Glaubitz
  1 sibling, 0 replies; 20+ messages in thread
From: John Paul Adrian Glaubitz @ 2023-07-04 20:35 UTC (permalink / raw)
  To: Artur Rojek, Yoshinori Sato, Rich Felker, Geert Uytterhoeven
  Cc: Rafael Ignacio Zurita, linux-sh, linux-kernel

On Sat, 2023-05-27 at 18:44 +0200, Artur Rojek wrote:
> According to the hardware manual [1], the DMAC found in SH7709 features
> only 4 channels.
> 
> While at it, also sort the existing targets and clarify that
> NR_ONCHIP_DMA_CHANNELS must be a multiply of two.
> 
> [1] https://www.renesas.com/us/en/document/mah/sh7709s-group-hardware-manual (p. 373)
> 
> Signed-off-by: Artur Rojek <contact@artur-rojek.eu>
> ---
> 
> v2: - sort existing targets
>     - clarify that the value must be a multiply of two
> 
>  arch/sh/drivers/dma/Kconfig | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/sh/drivers/dma/Kconfig b/arch/sh/drivers/dma/Kconfig
> index 7d54f284ce10..382fbb189fcf 100644
> --- a/arch/sh/drivers/dma/Kconfig
> +++ b/arch/sh/drivers/dma/Kconfig
> @@ -28,17 +28,19 @@ 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_SH7750S || CPU_SUBTYPE_SH7751 || \
> +		       CPU_SUBTYPE_SH7091
>  	default "8" if CPU_SUBTYPE_SH7750R || CPU_SUBTYPE_SH7751R || \
>  		       CPU_SUBTYPE_SH7760
> -	default "12" if CPU_SUBTYPE_SH7723 || CPU_SUBTYPE_SH7780  || \
> -			CPU_SUBTYPE_SH7785 || CPU_SUBTYPE_SH7724
> +	default "12" if CPU_SUBTYPE_SH7723 || CPU_SUBTYPE_SH7724  || \
> +			CPU_SUBTYPE_SH7780 || CPU_SUBTYPE_SH7785
>  	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/SH7750S/SH7751/SH7091,
> +	  8 for SH7750R/SH7751R/SH7760, and 12 for SH7723/SH7724/SH7780/SH7785.
> +	  Default is 6. Must be an even number.
>  
>  config SH_DMABRG
>  	bool "SH7760 DMABRG support"

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 0/3] SuperH DMAC fixes
  2023-05-27 16:44 [PATCH v2 0/3] SuperH DMAC fixes Artur Rojek
                   ` (2 preceding siblings ...)
  2023-05-27 16:44 ` [PATCH v2 3/3] sh: dma: Correct the number of DMA channels in SH7709 Artur Rojek
@ 2023-07-05 17:08 ` John Paul Adrian Glaubitz
  3 siblings, 0 replies; 20+ messages in thread
From: John Paul Adrian Glaubitz @ 2023-07-05 17:08 UTC (permalink / raw)
  To: Artur Rojek, Yoshinori Sato, Rich Felker, Geert Uytterhoeven
  Cc: Rafael Ignacio Zurita, linux-sh, linux-kernel

On Sat, 2023-05-27 at 18:44 +0200, Artur Rojek wrote:
> Hi all,
> 
> This is v2 of the DMAC fixes.
> 
> Patch [1/3] now also addresses varying numbers of DMAC modules and
> channels.
> 
> Patch [2/3] removes SH_DMAC_BASE1 for SH4 family. To my knowledge, none
> of these SoCs feature two DMAC modules.
> 
> Patch [3/3] now also sorts all the targets and the description stays
> within 80 characters per line.
> 
> Tested on Jornada 680 (SH7709 compatible).
> 
> I went ahead and verified the above changes against datasheets of all
> the SoCs that are currently supported. Only SoCs found in defconfigs
> which enable CONFIG_SH_DMA/CONFIG_SH_DMA_API have been surveyed.
> 
> ---------+--------+--------+--------------+------------------+----------
>  SoC     | Family | Refs.  | DMAC modules | Chans per module | Notes  
> ---------+--------+--------+--------------+------------------+----------
>  SH7724  | SH4A   | [1]    | 2            | 6                |
>  SH7780  | SH4A   | [2]    | 2            | 6                |
>  SH7786  | SH4A   | [3]    | 1 (+ 1)      | 6 (+ 4)          | #1
>  SH7091  | SH4    | [4][5] | 1            | 4                | #2
>  SH7751R | SH4    | [6]    | 1            | 8                |
>  SH7760  | SH4    | [7]    | 1            | 8                |
>  SH4-202 | SH4    | n/a    | ?            | ?                | #3
>  SH7709  | SH3    | [8]    | 1            | 4                |
>  SH7720  | SH3    | [9]    | 1            | 6                |
> ---------+--------+--------+--------------+------------------+----------
> 
> Note #1:
> Technically, SH7786 features 2 DMAC modules, for a total of 10 channels.
> However, only DMAC0 (6 channels) is hw register compatible with the
> existing dma-sh driver.
> 
> Note #2:
> This SoC, used in SEGA Dreamcast, has no publicly available datasheet.
> Apparently it's an SH7750 [5] derivative. Number of modules/channels
> has been cross-referenced with the KallistiOS project's source code [4].
> 
> Note #3:
> No publicly available datasheet for this SoC. Apparently this CPU is
> used in an FPGA-based board [10], so perhaps the DMAC properties are
> synthesized in FPGA bitstream? As this is SH4, it could potentially
> impact patch [2/3].
> 
> [1] https://www.renesas.com/us/en/document/mat/sh7724-users-manual-hardware p. 537
> [2] https://www.renesas.com/us/en/document/mah/sh7780-hardware-manual p. 609
> [3] https://www.renesas.com/us/en/document/mah/sh7786-group-users-manual-hardware p. 1081
> [4] https://github.com/KallistiOS/KallistiOS/blob/ebf8d528cd8d1909150f60bef98e1a68318cbb95/kernel/arch/dreamcast/include/dc/asic.h#L91-L94
> [5] https://www.renesas.com/us/en/document/mah/sh7750-sh7750s-sh7750r-group-users-manual-hardware p. 597
> [6] https://www.renesas.com/us/en/document/mah/sh7751-group-sh7751r-group-users-manual-hardware p. 551
> [7] https://www.renesas.com/us/en/document/mah/sh7760-group-hardware-manual p. 463
> [8] https://www.renesas.com/us/en/document/mah/sh7709s-group-hardware-manual p. 373
> [9] https://www.renesas.com/us/en/document/mah/sh7720-group-sh7721-group-users-manual-hardware p. 467
> [10] https://web.archive.org/web/20050405021907/http://www.superh.com/products/microdev.htm
> 
> Cheers,
> Artur
> 
> Artur Rojek (3):
>   sh: dma: Fix dma channel offset calculation
>   sh: dma: Drop incorrect SH_DMAC_BASE1 for SH4
>   sh: dma: Correct the number of DMA channels in SH7709
> 
>  arch/sh/drivers/dma/Kconfig       | 14 +++++++-----
>  arch/sh/drivers/dma/dma-sh.c      | 37 ++++++++++++++++++++-----------
>  arch/sh/include/cpu-sh4/cpu/dma.h |  1 -
>  3 files changed, 32 insertions(+), 20 deletions(-)
> 

Applied to my for-next tree for 6.5. PR will be sent tomorrow.

Thanks,
Adrian

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2023-07-05 17:09 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-27 16:44 [PATCH v2 0/3] SuperH DMAC fixes Artur Rojek
2023-05-27 16:44 ` [PATCH v2 1/3] sh: dma: Fix dma channel offset calculation Artur Rojek
2023-06-07  9:04   ` Geert Uytterhoeven
2023-07-04 20:30   ` John Paul Adrian Glaubitz
2023-05-27 16:44 ` [PATCH v2 2/3] sh: dma: Drop incorrect SH_DMAC_BASE1 for SH4 Artur Rojek
2023-06-07  9:05   ` Geert Uytterhoeven
2023-07-04 20:32   ` John Paul Adrian Glaubitz
2023-05-27 16:44 ` [PATCH v2 3/3] sh: dma: Correct the number of DMA channels in SH7709 Artur Rojek
2023-06-07  9:16   ` Geert Uytterhoeven
2023-06-08  9:54     ` John Paul Adrian Glaubitz
2023-06-08  9:58       ` Geert Uytterhoeven
2023-06-08 10:03         ` John Paul Adrian Glaubitz
2023-06-17  7:31           ` John Paul Adrian Glaubitz
2023-06-17 11:09             ` Geert Uytterhoeven
2023-07-04  5:45               ` John Paul Adrian Glaubitz
2023-07-04  7:32                 ` Geert Uytterhoeven
2023-07-04  7:43                   ` John Paul Adrian Glaubitz
2023-07-04  7:54                     ` Geert Uytterhoeven
2023-07-04 20:35   ` John Paul Adrian Glaubitz
2023-07-05 17:08 ` [PATCH v2 0/3] SuperH DMAC fixes John Paul Adrian Glaubitz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).