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

John Paul Adrian Glaubitz <[email protected]>
Newsgroups gmane.linux.ports.sh.devel,gmane.linux.kernel
Message-ID <f7b9ceb9739f8ae5cbee4f6073ce3af3921a2540.camel@physik.fu-berlin.de>
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 <[email protected]> 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 <[email protected]>
> > ---
> > 
> > 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.