Re: [PATCH 1/2] dmaengine: sh: Do not enable SH_DMAE_BASE by default during compile testing

Re: [PATCH 1/2] dmaengine: sh: Do not enable SH_DMAE_BASE by default during compile testing

[Geert Uytterhoeven]

CC linux-sh

Hi Krzysztof,

On Fri, 4 Apr 2025 at 14:22, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
> Enabling the compile test should not cause automatic enabling of all
> drivers.
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Thanks for your patch, which is now commit 587dd30449fb1012
("dmaengine: sh: Do not enable SH_DMAE_BASE by default during
compile testing") in dmaengine/next.

> --- a/drivers/dma/sh/Kconfig
> +++ b/drivers/dma/sh/Kconfig
> @@ -16,7 +16,7 @@ config SH_DMAE_BASE
>         depends on SUPERH || COMPILE_TEST
>         depends on !SUPERH || SH_DMA
>         depends on !SH_DMA_API
> -       default y
> +       default SUPERH || SH_DMA

I think the check for SUPERH is superfluous, due to the dependency on
"!SUPERH || SH_DMA" above.

>         select RENESAS_DMA
>         help
>           Enable support for the Renesas SuperH DMA controllers.

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] dmaengine: sh: Do not enable SH_DMAE_BASE by default during compile testing

[Krzysztof Kozlowski]

On 08/07/2025 15:07, Geert Uytterhoeven wrote:
> CC linux-sh
> 
> Hi Krzysztof,
> 
> On Fri, 4 Apr 2025 at 14:22, Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>> Enabling the compile test should not cause automatic enabling of all
>> drivers.
>>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> Thanks for your patch, which is now commit 587dd30449fb1012
> ("dmaengine: sh: Do not enable SH_DMAE_BASE by default during
> compile testing") in dmaengine/next.
> 
>> --- a/drivers/dma/sh/Kconfig
>> +++ b/drivers/dma/sh/Kconfig
>> @@ -16,7 +16,7 @@ config SH_DMAE_BASE
>>         depends on SUPERH || COMPILE_TEST
>>         depends on !SUPERH || SH_DMA
>>         depends on !SH_DMA_API
>> -       default y
>> +       default SUPERH || SH_DMA
> 
> I think the check for SUPERH is superfluous, due to the dependency on
> "!SUPERH || SH_DMA" above.
> 

Indeed it might be, but I must admit I don't understand the dependencies
here at all. I think commit 9f2c2bb31258 ("dmaengine: sh: Rework Kconfig
and Makefile") from Laurent made it confusing and this later just grew
to even more confusing.

What is the intention for "depends on"? This should be enabled when
SUPERH AND SH_DMA are enabled?

SH_DMA cannot be enabled without SUPERH (no compile test), right? But
this "depends on !SUPERH || SH_DMA" suggests it could be. This should be
read for humans as "if not SUPERH, then require at least SH_DMA".
Otherwise what is the meaning for humans? This driver will work fine
without SUERPH?

My change for default could be rewritten but I don't understand the goal
behind current depends, so not sure how should I rewrite it.

Best regards,
Krzysztof

Re: [PATCH 1/2] dmaengine: sh: Do not enable SH_DMAE_BASE by default during compile testing

[Geert Uytterhoeven]

Hi Krzysztof,

On Tue, 8 Jul 2025 at 15:21, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
> On 08/07/2025 15:07, Geert Uytterhoeven wrote:
> > On Fri, 4 Apr 2025 at 14:22, Krzysztof Kozlowski
> > <krzysztof.kozlowski@linaro.org> wrote:
> >> Enabling the compile test should not cause automatic enabling of all
> >> drivers.
> >>
> >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >
> > Thanks for your patch, which is now commit 587dd30449fb1012
> > ("dmaengine: sh: Do not enable SH_DMAE_BASE by default during
> > compile testing") in dmaengine/next.
> >
> >> --- a/drivers/dma/sh/Kconfig
> >> +++ b/drivers/dma/sh/Kconfig
> >> @@ -16,7 +16,7 @@ config SH_DMAE_BASE
> >>         depends on SUPERH || COMPILE_TEST
> >>         depends on !SUPERH || SH_DMA
> >>         depends on !SH_DMA_API
> >> -       default y
> >> +       default SUPERH || SH_DMA
> >
> > I think the check for SUPERH is superfluous, due to the dependency on
> > "!SUPERH || SH_DMA" above.
>
> Indeed it might be, but I must admit I don't understand the dependencies
> here at all. I think commit 9f2c2bb31258 ("dmaengine: sh: Rework Kconfig
> and Makefile") from Laurent made it confusing and this later just grew
> to even more confusing.
>
> What is the intention for "depends on"? This should be enabled when
> SUPERH AND SH_DMA are enabled?
>
> SH_DMA cannot be enabled without SUPERH (no compile test), right? But
> this "depends on !SUPERH || SH_DMA" suggests it could be. This should be
> read for humans as "if not SUPERH, then require at least SH_DMA".
> Otherwise what is the meaning for humans? This driver will work fine
> without SUERPH?
>
> My change for default could be rewritten but I don't understand the goal
> behind current depends, so not sure how should I rewrite it.

I think the original plan was to use the SH DMA drivers on ARM SH-Mobile
SoCs, too.  But enabling SH_DMA on ARM SH-Mobile was never integrated
upstream, and the focus shifted to ARM R-Car SoCs, for which the shiny new
R-Car DMA driver was written...

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