2023-05-27 17:21:58

by Artur Rojek

[permalink] [raw]
Subject: [PATCH v2 0/3] SuperH DMAC fixes

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



2023-05-27 17:22:51

by Artur Rojek

[permalink] [raw]
Subject: [PATCH v2 1/3] sh: dma: Fix dma channel offset calculation

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 <[email protected]>
---

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


2023-06-07 09:21:40

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] sh: dma: Fix dma channel offset calculation

On Sat, May 27, 2023 at 6:45 PM Artur Rojek <[email protected]> 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 <[email protected]>
> ---
>
> v2: also handle differing numbers of DMAC modules and channels

Reviewed-by: Geert Uytterhoeven <[email protected]>

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

Subject: Re: [PATCH v2 1/3] sh: dma: Fix dma channel offset calculation

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 <[email protected]>
> ---
>
> 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 <[email protected]>

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

Subject: Re: [PATCH v2 0/3] SuperH DMAC fixes

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