[RFC,V4,6/6] riscv: soc: Add Allwinner SoC kconfig option

Commit Message

Guo Ren Sept. 11, 2021, 9:21 a.m. UTC

From: Liu Shaohua <[e-mail]>

Add Allwinner kconfig option which selects SoC specific and common
drivers that is required for this SoC.

Allwinner D1 uses custom PTE attributes to solve non-coherency SOC
interconnect issues for dma synchronization, so we set the default
value when SOC_SUNXI selected.

Signed-off-by: Liu Shaohua <[e-mail]>
Signed-off-by: Guo Ren <[e-mail]>
Signed-off-by: Wei Fu <[e-mail]>
Cc: Anup Patel <[e-mail]>
Cc: Atish Patra <[e-mail]>
Cc: Christoph Hellwig <[e-mail]>
Cc: Chen-Yu Tsai <[e-mail]>
Cc: Drew Fustini <[e-mail]>
Cc: Maxime Ripard <[e-mail]>
Cc: Palmer Dabbelt <[e-mail]>
Cc: Wei Wu <[e-mail]>
---
 arch/riscv/Kconfig.socs      | 15 +++++++++++++++
 arch/riscv/configs/defconfig |  1 +
 2 files changed, 16 insertions(+)

Comments

Maxime Ripard Sept. 13, 2021, 8:45 a.m. UTC | #1

Hi,

On Sat, Sep 11, 2021 at 05:21:39PM +0800, [e-mail] wrote:
> From: Liu Shaohua <[e-mail]>
> 
> Add Allwinner kconfig option which selects SoC specific and common
> drivers that is required for this SoC.
> 
> Allwinner D1 uses custom PTE attributes to solve non-coherency SOC
> interconnect issues for dma synchronization, so we set the default
> value when SOC_SUNXI selected.
> 
> Signed-off-by: Liu Shaohua <[e-mail]>
> Signed-off-by: Guo Ren <[e-mail]>
> Signed-off-by: Wei Fu <[e-mail]>
> Cc: Anup Patel <[e-mail]>
> Cc: Atish Patra <[e-mail]>
> Cc: Christoph Hellwig <[e-mail]>
> Cc: Chen-Yu Tsai <[e-mail]>
> Cc: Drew Fustini <[e-mail]>
> Cc: Maxime Ripard <[e-mail]>
> Cc: Palmer Dabbelt <[e-mail]>
> Cc: Wei Wu <[e-mail]>
> ---
>  arch/riscv/Kconfig.socs      | 15 +++++++++++++++
>  arch/riscv/configs/defconfig |  1 +
>  2 files changed, 16 insertions(+)
> 
> diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
> index 30676ebb16eb..8721c000ef23 100644
> --- a/arch/riscv/Kconfig.socs
> +++ b/arch/riscv/Kconfig.socs
> @@ -70,4 +70,19 @@ config SOC_CANAAN_K210_DTB_SOURCE
>  
>  endif
>  
> +config SOC_SUNXI
> +	bool "Allwinner SoCs"
> +	depends on MMU
> +	select DWMAC_GENERIC
> +	select ERRATA_THEAD
> +	select RISCV_DMA_NONCOHERENT
> +	select RISCV_ERRATA_ALTERNATIVE
> +	select SERIAL_8250
> +	select SERIAL_8250_CONSOLE
> +	select SERIAL_8250_DW
> +	select SIFIVE_PLIC
> +	select STMMAC_ETH
> +	help
> +	  This enables support for Allwinner SoC platforms like the D1.
> +

I'm not sure we should select the drivers there. We could very well
imagine a board without UART, or even more so without ethernet.

These options should be in the defconfig.

Maxime

Guo Ren Sept. 13, 2021, 9:20 a.m. UTC | #2

On Mon, Sep 13, 2021 at 4:45 PM Maxime Ripard <[e-mail]> wrote:
>
> Hi,
>
> On Sat, Sep 11, 2021 at 05:21:39PM +0800, [e-mail] wrote:
> > From: Liu Shaohua <[e-mail]>
> >
> > Add Allwinner kconfig option which selects SoC specific and common
> > drivers that is required for this SoC.
> >
> > Allwinner D1 uses custom PTE attributes to solve non-coherency SOC
> > interconnect issues for dma synchronization, so we set the default
> > value when SOC_SUNXI selected.
> >
> > Signed-off-by: Liu Shaohua <[e-mail]>
> > Signed-off-by: Guo Ren <[e-mail]>
> > Signed-off-by: Wei Fu <[e-mail]>
> > Cc: Anup Patel <[e-mail]>
> > Cc: Atish Patra <[e-mail]>
> > Cc: Christoph Hellwig <[e-mail]>
> > Cc: Chen-Yu Tsai <[e-mail]>
> > Cc: Drew Fustini <[e-mail]>
> > Cc: Maxime Ripard <[e-mail]>
> > Cc: Palmer Dabbelt <[e-mail]>
> > Cc: Wei Wu <[e-mail]>
> > ---
> >  arch/riscv/Kconfig.socs      | 15 +++++++++++++++
> >  arch/riscv/configs/defconfig |  1 +
> >  2 files changed, 16 insertions(+)
> >
> > diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
> > index 30676ebb16eb..8721c000ef23 100644
> > --- a/arch/riscv/Kconfig.socs
> > +++ b/arch/riscv/Kconfig.socs
> > @@ -70,4 +70,19 @@ config SOC_CANAAN_K210_DTB_SOURCE
> >
> >  endif
> >
> > +config SOC_SUNXI
> > +     bool "Allwinner SoCs"
> > +     depends on MMU
> > +     select DWMAC_GENERIC
> > +     select ERRATA_THEAD
> > +     select RISCV_DMA_NONCOHERENT
> > +     select RISCV_ERRATA_ALTERNATIVE
> > +     select SERIAL_8250
> > +     select SERIAL_8250_CONSOLE
> > +     select SERIAL_8250_DW
> > +     select SIFIVE_PLIC
> > +     select STMMAC_ETH
> > +     help
> > +       This enables support for Allwinner SoC platforms like the D1.
> > +
>
> I'm not sure we should select the drivers there. We could very well
> imagine a board without UART, or even more so without ethernet.
We just want people could bring D1 up easier, 8250 is the basic component.


>
> These options should be in the defconfig.
>
> Maxime



--
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

Randy Dunlap Sept. 13, 2021, 6:48 p.m. UTC | #3

On 9/13/21 2:20 AM, Guo Ren wrote:
> On Mon, Sep 13, 2021 at 4:45 PM Maxime Ripard <[e-mail]> wrote:
>>
>> Hi,
>>
>> On Sat, Sep 11, 2021 at 05:21:39PM +0800, [e-mail] wrote:
>>> From: Liu Shaohua <[e-mail]>
>>>
>>> Add Allwinner kconfig option which selects SoC specific and common
>>> drivers that is required for this SoC.
>>>
>>> Allwinner D1 uses custom PTE attributes to solve non-coherency SOC
>>> interconnect issues for dma synchronization, so we set the default
>>> value when SOC_SUNXI selected.
>>>
>>> Signed-off-by: Liu Shaohua <[e-mail]>
>>> Signed-off-by: Guo Ren <[e-mail]>
>>> Signed-off-by: Wei Fu <[e-mail]>
>>> Cc: Anup Patel <[e-mail]>
>>> Cc: Atish Patra <[e-mail]>
>>> Cc: Christoph Hellwig <[e-mail]>
>>> Cc: Chen-Yu Tsai <[e-mail]>
>>> Cc: Drew Fustini <[e-mail]>
>>> Cc: Maxime Ripard <[e-mail]>
>>> Cc: Palmer Dabbelt <[e-mail]>
>>> Cc: Wei Wu <[e-mail]>
>>> ---
>>>   arch/riscv/Kconfig.socs      | 15 +++++++++++++++
>>>   arch/riscv/configs/defconfig |  1 +
>>>   2 files changed, 16 insertions(+)
>>>
>>> diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
>>> index 30676ebb16eb..8721c000ef23 100644
>>> --- a/arch/riscv/Kconfig.socs
>>> +++ b/arch/riscv/Kconfig.socs
>>> @@ -70,4 +70,19 @@ config SOC_CANAAN_K210_DTB_SOURCE
>>>
>>>   endif
>>>
>>> +config SOC_SUNXI
>>> +     bool "Allwinner SoCs"
>>> +     depends on MMU
>>> +     select DWMAC_GENERIC
>>> +     select ERRATA_THEAD
>>> +     select RISCV_DMA_NONCOHERENT
>>> +     select RISCV_ERRATA_ALTERNATIVE
>>> +     select SERIAL_8250
>>> +     select SERIAL_8250_CONSOLE
>>> +     select SERIAL_8250_DW
>>> +     select SIFIVE_PLIC
>>> +     select STMMAC_ETH
>>> +     help
>>> +       This enables support for Allwinner SoC platforms like the D1.
>>> +
>>
>> I'm not sure we should select the drivers there. We could very well
>> imagine a board without UART, or even more so without ethernet.
> We just want people could bring D1 up easier, 8250 is the basic component.
> 
> 
>>
>> These options should be in the defconfig.

Agreed, using a defconfig is the right way to do this.

Guo Ren Sept. 14, 2021, 2:34 a.m. UTC | #4

On Tue, Sep 14, 2021 at 2:49 AM Randy Dunlap <[e-mail]> wrote:
>
> On 9/13/21 2:20 AM, Guo Ren wrote:
> > On Mon, Sep 13, 2021 at 4:45 PM Maxime Ripard <[e-mail]> wrote:
> >>
> >> Hi,
> >>
> >> On Sat, Sep 11, 2021 at 05:21:39PM +0800, [e-mail] wrote:
> >>> From: Liu Shaohua <[e-mail]>
> >>>
> >>> Add Allwinner kconfig option which selects SoC specific and common
> >>> drivers that is required for this SoC.
> >>>
> >>> Allwinner D1 uses custom PTE attributes to solve non-coherency SOC
> >>> interconnect issues for dma synchronization, so we set the default
> >>> value when SOC_SUNXI selected.
> >>>
> >>> Signed-off-by: Liu Shaohua <[e-mail]>
> >>> Signed-off-by: Guo Ren <[e-mail]>
> >>> Signed-off-by: Wei Fu <[e-mail]>
> >>> Cc: Anup Patel <[e-mail]>
> >>> Cc: Atish Patra <[e-mail]>
> >>> Cc: Christoph Hellwig <[e-mail]>
> >>> Cc: Chen-Yu Tsai <[e-mail]>
> >>> Cc: Drew Fustini <[e-mail]>
> >>> Cc: Maxime Ripard <[e-mail]>
> >>> Cc: Palmer Dabbelt <[e-mail]>
> >>> Cc: Wei Wu <[e-mail]>
> >>> ---
> >>>   arch/riscv/Kconfig.socs      | 15 +++++++++++++++
> >>>   arch/riscv/configs/defconfig |  1 +
> >>>   2 files changed, 16 insertions(+)
> >>>
> >>> diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
> >>> index 30676ebb16eb..8721c000ef23 100644
> >>> --- a/arch/riscv/Kconfig.socs
> >>> +++ b/arch/riscv/Kconfig.socs
> >>> @@ -70,4 +70,19 @@ config SOC_CANAAN_K210_DTB_SOURCE
> >>>
> >>>   endif
> >>>
> >>> +config SOC_SUNXI
> >>> +     bool "Allwinner SoCs"
> >>> +     depends on MMU
> >>> +     select DWMAC_GENERIC
> >>> +     select ERRATA_THEAD
> >>> +     select RISCV_DMA_NONCOHERENT
> >>> +     select RISCV_ERRATA_ALTERNATIVE
> >>> +     select SERIAL_8250
> >>> +     select SERIAL_8250_CONSOLE
> >>> +     select SERIAL_8250_DW
> >>> +     select SIFIVE_PLIC
> >>> +     select STMMAC_ETH
> >>> +     help
> >>> +       This enables support for Allwinner SoC platforms like the D1.
> >>> +
> >>
> >> I'm not sure we should select the drivers there. We could very well
> >> imagine a board without UART, or even more so without ethernet.
> > We just want people could bring D1 up easier, 8250 is the basic component.
> >
> >
> >>
> >> These options should be in the defconfig.
>
> Agreed, using a defconfig is the right way to do this.
Put 8250 related configs into arch/riscv/configs/defconfig?

 @Palmer Dabbelt @Arnd Bergmann, How do you think about that?
(defconfig or Kconfig.soc)
My purpose is when people make the Image from riscv/defconfig, then
the Image could run on all platforms include D1.

>
> --
> ~Randy
>

Randy Dunlap Sept. 14, 2021, 3:06 a.m. UTC | #5

On 9/13/21 7:34 PM, Guo Ren wrote:
> On Tue, Sep 14, 2021 at 2:49 AM Randy Dunlap <[e-mail]> wrote:
>>
>> On 9/13/21 2:20 AM, Guo Ren wrote:
>>> On Mon, Sep 13, 2021 at 4:45 PM Maxime Ripard <[e-mail]> wrote:
>>>>
>>>> Hi,
>>>>
>>>> On Sat, Sep 11, 2021 at 05:21:39PM +0800, [e-mail] wrote:
>>>>> From: Liu Shaohua <[e-mail]>
>>>>>
>>>>> Add Allwinner kconfig option which selects SoC specific and common
>>>>> drivers that is required for this SoC.
>>>>>
>>>>> Allwinner D1 uses custom PTE attributes to solve non-coherency SOC
>>>>> interconnect issues for dma synchronization, so we set the default
>>>>> value when SOC_SUNXI selected.
>>>>>
>>>>> Signed-off-by: Liu Shaohua <[e-mail]>
>>>>> Signed-off-by: Guo Ren <[e-mail]>
>>>>> Signed-off-by: Wei Fu <[e-mail]>
>>>>> Cc: Anup Patel <[e-mail]>
>>>>> Cc: Atish Patra <[e-mail]>
>>>>> Cc: Christoph Hellwig <[e-mail]>
>>>>> Cc: Chen-Yu Tsai <[e-mail]>
>>>>> Cc: Drew Fustini <[e-mail]>
>>>>> Cc: Maxime Ripard <[e-mail]>
>>>>> Cc: Palmer Dabbelt <[e-mail]>
>>>>> Cc: Wei Wu <[e-mail]>
>>>>> ---
>>>>>    arch/riscv/Kconfig.socs      | 15 +++++++++++++++
>>>>>    arch/riscv/configs/defconfig |  1 +
>>>>>    2 files changed, 16 insertions(+)
>>>>>
>>>>> diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
>>>>> index 30676ebb16eb..8721c000ef23 100644
>>>>> --- a/arch/riscv/Kconfig.socs
>>>>> +++ b/arch/riscv/Kconfig.socs
>>>>> @@ -70,4 +70,19 @@ config SOC_CANAAN_K210_DTB_SOURCE
>>>>>
>>>>>    endif
>>>>>
>>>>> +config SOC_SUNXI
>>>>> +     bool "Allwinner SoCs"
>>>>> +     depends on MMU
>>>>> +     select DWMAC_GENERIC
>>>>> +     select ERRATA_THEAD
>>>>> +     select RISCV_DMA_NONCOHERENT
>>>>> +     select RISCV_ERRATA_ALTERNATIVE
>>>>> +     select SERIAL_8250
>>>>> +     select SERIAL_8250_CONSOLE
>>>>> +     select SERIAL_8250_DW
>>>>> +     select SIFIVE_PLIC
>>>>> +     select STMMAC_ETH
>>>>> +     help
>>>>> +       This enables support for Allwinner SoC platforms like the D1.
>>>>> +
>>>>
>>>> I'm not sure we should select the drivers there. We could very well
>>>> imagine a board without UART, or even more so without ethernet.
>>> We just want people could bring D1 up easier, 8250 is the basic component.
>>>
>>>
>>>>
>>>> These options should be in the defconfig.
>>
>> Agreed, using a defconfig is the right way to do this.
> Put 8250 related configs into arch/riscv/configs/defconfig?
> 
>   @Palmer Dabbelt @Arnd Bergmann, How do you think about that?
> (defconfig or Kconfig.soc)
> My purpose is when people make the Image from riscv/defconfig, then
> the Image could run on all platforms include D1.

Hi,

I certainly did not understand your purpose with the patch being
able to build a kernel that would run on multiple platforms.
Still, I would not expect to see one platform cause unnecessary
drivers to be built for platforms that don't need them.

Kconfig.socs in arch/riscv/ is a bit of an unusual Kconfig file
IMO -- I suppose what you want to do fits into its style.

AFAIK the suggestion to use a defconfig (at least my suggestion)
was expecting to have a defconfig for each platform, but that
would not give you a boot image that could run on all platforms.

Anyway, it's Palmer's choice.

thanks.

Heinrich Schuchardt Sept. 14, 2021, 3:49 a.m. UTC | #6

On 9/13/21 10:45 AM, Maxime Ripard wrote:
> Hi,
> 
> On Sat, Sep 11, 2021 at 05:21:39PM +0800, [e-mail] wrote:
>> From: Liu Shaohua <[e-mail]>
>>
>> Add Allwinner kconfig option which selects SoC specific and common
>> drivers that is required for this SoC.
>>
>> Allwinner D1 uses custom PTE attributes to solve non-coherency SOC
>> interconnect issues for dma synchronization, so we set the default
>> value when SOC_SUNXI selected.
>>
>> Signed-off-by: Liu Shaohua <[e-mail]>
>> Signed-off-by: Guo Ren <[e-mail]>
>> Signed-off-by: Wei Fu <[e-mail]>
>> Cc: Anup Patel <[e-mail]>
>> Cc: Atish Patra <[e-mail]>
>> Cc: Christoph Hellwig <[e-mail]>
>> Cc: Chen-Yu Tsai <[e-mail]>
>> Cc: Drew Fustini <[e-mail]>
>> Cc: Maxime Ripard <[e-mail]>
>> Cc: Palmer Dabbelt <[e-mail]>
>> Cc: Wei Wu <[e-mail]>
>> ---
>>   arch/riscv/Kconfig.socs      | 15 +++++++++++++++
>>   arch/riscv/configs/defconfig |  1 +
>>   2 files changed, 16 insertions(+)
>>
>> diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
>> index 30676ebb16eb..8721c000ef23 100644
>> --- a/arch/riscv/Kconfig.socs
>> +++ b/arch/riscv/Kconfig.socs
>> @@ -70,4 +70,19 @@ config SOC_CANAAN_K210_DTB_SOURCE
>>   
>>   endif
>>   
>> +config SOC_SUNXI
>> +	bool "Allwinner SoCs"
>> +	depends on MMU
>> +	select DWMAC_GENERIC
>> +	select ERRATA_THEAD
>> +	select RISCV_DMA_NONCOHERENT
>> +	select RISCV_ERRATA_ALTERNATIVE
>> +	select SERIAL_8250
>> +	select SERIAL_8250_CONSOLE
>> +	select SERIAL_8250_DW
>> +	select SIFIVE_PLIC
>> +	select STMMAC_ETH
>> +	help
>> +	  This enables support for Allwinner SoC platforms like the D1.
>> +
> 
> I'm not sure we should select the drivers there. We could very well
> imagine a board without UART, or even more so without ethernet.

The draft of the RISC-V platform specification is available here:
https://github.com/riscv/riscv-platform-specs/blob/main/riscv-platform-spec.adoc#uartserial-console

The specification requires in section "2.1.5.1. UART/Serial Console" 
that on platforms with a rich operating system (e.g. Linux) you have a 
serial console. Hence requiring 8250 support for the D1 CPU is justified.

In the riscv defconfig as of v5.14 we have:

CONFIG_SERIAL_8250=y
CONFIG_SERIAL_8250_CONSOLE=y
# CONFIG_SERIAL_8250_DW is not set
(Support for Synopsys DesignWare 8250 quirks)

CONFIG_SERIAL_8250_DW should be enabled (=y) in the defconfig.

As the specification requires a 16550 UART and marks 8250 as deprecated 
I expect that future Allwinner SoCs will move to 16550. Calling a 
Kconfig menu item "Allwinner SoCs" which includes all future Allwinner 
SoCs irritates me. How about CONFIG_SOC_SUNXI_D1 instead?

Why does the patch use 'depends on MMU' and does not 'select MMU'?

Best regards

Heinrich

> 
> These options should be in the defconfig.
> 
> Maxime
>

Samuel Holland Sept. 14, 2021, 5:16 a.m. UTC | #7

On 9/13/21 10:49 PM, Heinrich Schuchardt wrote:
> Calling a Kconfig menu item "Allwinner SoCs" which includes all
> future Allwinner SoCs irritates me. How about CONFIG_SOC_SUNXI_D1
> instead?

Would you want to have a separate option for each new SoC? That seems
like the only way to split things up, if you want to be more specific
than than "sunxi" (or equivalently "sun20i", which is the codename for
the RISC-V series).

Except at the very beginning (sun4i-sun7i), there have not been clear
generational boundaries between the various sunxi SoCs, so most of the
32-bit ones already get lumped into a single symbol (MACH_SUN8I). And
there is a single Kconfig symbol, ARCH_SUNXI, for all 64-bit Allwinner SoCs.

There is enough overlap in peripherals that you need a common symbol for
the peripheral drivers anyway. How about... ARCH_SUNXI? There are 90+
uses of this symbol throughout drivers/ and sound/, and I have found
that more than half of them apply to the D1 (see e.g. this commit[1] and
some of its ancestors).

RISC-V has so far adopted a CONFIG_SOC_xxx naming scheme, which is
different from arm/arm64's CONFIG_ARCH_xxx pattern. But now we have a
case where a SoC family is split between the two architectures. I'm all
for consistency with the names of other RISC-V platform symbols, but it
seems that reusing the existing ARCH_SUNXI symbol would be better than
cluttering up the driver Kconfig files with a duplicate.

Regards,
Samuel

[1]: https://github.com/smaeul/linux/commit/7841e5c32366

Anup Patel Sept. 14, 2021, 5:16 a.m. UTC | #8

On Tue, Sep 14, 2021 at 8:36 AM Randy Dunlap <[e-mail]> wrote:
>
> On 9/13/21 7:34 PM, Guo Ren wrote:
> > On Tue, Sep 14, 2021 at 2:49 AM Randy Dunlap <[e-mail]> wrote:
> >>
> >> On 9/13/21 2:20 AM, Guo Ren wrote:
> >>> On Mon, Sep 13, 2021 at 4:45 PM Maxime Ripard <[e-mail]> wrote:
> >>>>
> >>>> Hi,
> >>>>
> >>>> On Sat, Sep 11, 2021 at 05:21:39PM +0800, [e-mail] wrote:
> >>>>> From: Liu Shaohua <[e-mail]>
> >>>>>
> >>>>> Add Allwinner kconfig option which selects SoC specific and common
> >>>>> drivers that is required for this SoC.
> >>>>>
> >>>>> Allwinner D1 uses custom PTE attributes to solve non-coherency SOC
> >>>>> interconnect issues for dma synchronization, so we set the default
> >>>>> value when SOC_SUNXI selected.
> >>>>>
> >>>>> Signed-off-by: Liu Shaohua <[e-mail]>
> >>>>> Signed-off-by: Guo Ren <[e-mail]>
> >>>>> Signed-off-by: Wei Fu <[e-mail]>
> >>>>> Cc: Anup Patel <[e-mail]>
> >>>>> Cc: Atish Patra <[e-mail]>
> >>>>> Cc: Christoph Hellwig <[e-mail]>
> >>>>> Cc: Chen-Yu Tsai <[e-mail]>
> >>>>> Cc: Drew Fustini <[e-mail]>
> >>>>> Cc: Maxime Ripard <[e-mail]>
> >>>>> Cc: Palmer Dabbelt <[e-mail]>
> >>>>> Cc: Wei Wu <[e-mail]>
> >>>>> ---
> >>>>>    arch/riscv/Kconfig.socs      | 15 +++++++++++++++
> >>>>>    arch/riscv/configs/defconfig |  1 +
> >>>>>    2 files changed, 16 insertions(+)
> >>>>>
> >>>>> diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
> >>>>> index 30676ebb16eb..8721c000ef23 100644
> >>>>> --- a/arch/riscv/Kconfig.socs
> >>>>> +++ b/arch/riscv/Kconfig.socs
> >>>>> @@ -70,4 +70,19 @@ config SOC_CANAAN_K210_DTB_SOURCE
> >>>>>
> >>>>>    endif
> >>>>>
> >>>>> +config SOC_SUNXI
> >>>>> +     bool "Allwinner SoCs"
> >>>>> +     depends on MMU
> >>>>> +     select DWMAC_GENERIC
> >>>>> +     select ERRATA_THEAD
> >>>>> +     select RISCV_DMA_NONCOHERENT
> >>>>> +     select RISCV_ERRATA_ALTERNATIVE
> >>>>> +     select SERIAL_8250
> >>>>> +     select SERIAL_8250_CONSOLE
> >>>>> +     select SERIAL_8250_DW
> >>>>> +     select SIFIVE_PLIC
> >>>>> +     select STMMAC_ETH
> >>>>> +     help
> >>>>> +       This enables support for Allwinner SoC platforms like the D1.
> >>>>> +
> >>>>
> >>>> I'm not sure we should select the drivers there. We could very well
> >>>> imagine a board without UART, or even more so without ethernet.
> >>> We just want people could bring D1 up easier, 8250 is the basic component.
> >>>
> >>>
> >>>>
> >>>> These options should be in the defconfig.
> >>
> >> Agreed, using a defconfig is the right way to do this.
> > Put 8250 related configs into arch/riscv/configs/defconfig?
> >
> >   @Palmer Dabbelt @Arnd Bergmann, How do you think about that?
> > (defconfig or Kconfig.soc)
> > My purpose is when people make the Image from riscv/defconfig, then
> > the Image could run on all platforms include D1.
>
> Hi,
>
> I certainly did not understand your purpose with the patch being
> able to build a kernel that would run on multiple platforms.
> Still, I would not expect to see one platform cause unnecessary
> drivers to be built for platforms that don't need them.
>
> Kconfig.socs in arch/riscv/ is a bit of an unusual Kconfig file
> IMO -- I suppose what you want to do fits into its style.
>
> AFAIK the suggestion to use a defconfig (at least my suggestion)
> was expecting to have a defconfig for each platform, but that
> would not give you a boot image that could run on all platforms.

AFAIK, having a separate defconfig for each platform is not going
to fly with distros (AFAIK). We can't expect dirstros to release
separate RISC-V kernel image for each platform. In fact, ARM64
kernel has just one defconfig whereas ARM32 kernel has
consolidated and minimized number of defconfigs.

The long term goal for Linux RISC-V is to support single kernel
image booting on multiple-platforms. Of course, users can always
strip down the kernel using their custom defconfigs.

Regards,
Anup

>
> Anyway, it's Palmer's choice.
>
> thanks.
> --
> ~Randy
>

Randy Dunlap Sept. 14, 2021, 5:20 a.m. UTC | #9

On 9/13/21 10:16 PM, Anup Patel wrote:
> On Tue, Sep 14, 2021 at 8:36 AM Randy Dunlap <[e-mail]> wrote:
>>
>> On 9/13/21 7:34 PM, Guo Ren wrote:
>>> On Tue, Sep 14, 2021 at 2:49 AM Randy Dunlap <[e-mail]> wrote:
>>>>
>>>> On 9/13/21 2:20 AM, Guo Ren wrote:
>>>>> On Mon, Sep 13, 2021 at 4:45 PM Maxime Ripard <[e-mail]> wrote:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> On Sat, Sep 11, 2021 at 05:21:39PM +0800, [e-mail] wrote:
>>>>>>> From: Liu Shaohua <[e-mail]>
>>>>>>>
>>>>>>> Add Allwinner kconfig option which selects SoC specific and common
>>>>>>> drivers that is required for this SoC.
>>>>>>>
>>>>>>> Allwinner D1 uses custom PTE attributes to solve non-coherency SOC
>>>>>>> interconnect issues for dma synchronization, so we set the default
>>>>>>> value when SOC_SUNXI selected.
>>>>>>>
>>>>>>> Signed-off-by: Liu Shaohua <[e-mail]>
>>>>>>> Signed-off-by: Guo Ren <[e-mail]>
>>>>>>> Signed-off-by: Wei Fu <[e-mail]>
>>>>>>> Cc: Anup Patel <[e-mail]>
>>>>>>> Cc: Atish Patra <[e-mail]>
>>>>>>> Cc: Christoph Hellwig <[e-mail]>
>>>>>>> Cc: Chen-Yu Tsai <[e-mail]>
>>>>>>> Cc: Drew Fustini <[e-mail]>
>>>>>>> Cc: Maxime Ripard <[e-mail]>
>>>>>>> Cc: Palmer Dabbelt <[e-mail]>
>>>>>>> Cc: Wei Wu <[e-mail]>
>>>>>>> ---
>>>>>>>     arch/riscv/Kconfig.socs      | 15 +++++++++++++++
>>>>>>>     arch/riscv/configs/defconfig |  1 +
>>>>>>>     2 files changed, 16 insertions(+)
>>>>>>>
>>>>>>> diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
>>>>>>> index 30676ebb16eb..8721c000ef23 100644
>>>>>>> --- a/arch/riscv/Kconfig.socs
>>>>>>> +++ b/arch/riscv/Kconfig.socs
>>>>>>> @@ -70,4 +70,19 @@ config SOC_CANAAN_K210_DTB_SOURCE
>>>>>>>
>>>>>>>     endif
>>>>>>>
>>>>>>> +config SOC_SUNXI
>>>>>>> +     bool "Allwinner SoCs"
>>>>>>> +     depends on MMU
>>>>>>> +     select DWMAC_GENERIC
>>>>>>> +     select ERRATA_THEAD
>>>>>>> +     select RISCV_DMA_NONCOHERENT
>>>>>>> +     select RISCV_ERRATA_ALTERNATIVE
>>>>>>> +     select SERIAL_8250
>>>>>>> +     select SERIAL_8250_CONSOLE
>>>>>>> +     select SERIAL_8250_DW
>>>>>>> +     select SIFIVE_PLIC
>>>>>>> +     select STMMAC_ETH
>>>>>>> +     help
>>>>>>> +       This enables support for Allwinner SoC platforms like the D1.
>>>>>>> +
>>>>>>
>>>>>> I'm not sure we should select the drivers there. We could very well
>>>>>> imagine a board without UART, or even more so without ethernet.
>>>>> We just want people could bring D1 up easier, 8250 is the basic component.
>>>>>
>>>>>
>>>>>>
>>>>>> These options should be in the defconfig.
>>>>
>>>> Agreed, using a defconfig is the right way to do this.
>>> Put 8250 related configs into arch/riscv/configs/defconfig?
>>>
>>>    @Palmer Dabbelt @Arnd Bergmann, How do you think about that?
>>> (defconfig or Kconfig.soc)
>>> My purpose is when people make the Image from riscv/defconfig, then
>>> the Image could run on all platforms include D1.
>>
>> Hi,
>>
>> I certainly did not understand your purpose with the patch being
>> able to build a kernel that would run on multiple platforms.
>> Still, I would not expect to see one platform cause unnecessary
>> drivers to be built for platforms that don't need them.
>>
>> Kconfig.socs in arch/riscv/ is a bit of an unusual Kconfig file
>> IMO -- I suppose what you want to do fits into its style.
>>
>> AFAIK the suggestion to use a defconfig (at least my suggestion)
>> was expecting to have a defconfig for each platform, but that
>> would not give you a boot image that could run on all platforms.
> 
> AFAIK, having a separate defconfig for each platform is not going
> to fly with distros (AFAIK). We can't expect dirstros to release
> separate RISC-V kernel image for each platform. In fact, ARM64
> kernel has just one defconfig whereas ARM32 kernel has
> consolidated and minimized number of defconfigs.
> 
> The long term goal for Linux RISC-V is to support single kernel
> image booting on multiple-platforms. Of course, users can always
> strip down the kernel using their custom defconfigs.

OK, thanks for the info.

Heinrich Schuchardt Sept. 14, 2021, 6:30 a.m. UTC | #10

On 9/14/21 7:16 AM, Samuel Holland wrote:
> On 9/13/21 10:49 PM, Heinrich Schuchardt wrote:
>> Calling a Kconfig menu item "Allwinner SoCs" which includes all
>> future Allwinner SoCs irritates me. How about CONFIG_SOC_SUNXI_D1
>> instead?
> 
> Would you want to have a separate option for each new SoC? That seems
> like the only way to split things up, if you want to be more specific
> than than "sunxi" (or equivalently "sun20i", which is the codename for
> the RISC-V series).
> 
> Except at the very beginning (sun4i-sun7i), there have not been clear
> generational boundaries between the various sunxi SoCs, so most of the
> 32-bit ones already get lumped into a single symbol (MACH_SUN8I). And
> there is a single Kconfig symbol, ARCH_SUNXI, for all 64-bit Allwinner SoCs.

On arm64 we have avoided SoC specific Kconfig symbols and left it to the 
defconfig to select all SoC specific drivers. Generally the tendency of 
the defconfigs is to provide a multiarch kernel. So this should be ok on 
RISC-V too.

I was just concerned about the 8250 driver assigned to something called 
"Allwinner SoCs" which didn't seem future proof.

> 
> There is enough overlap in peripherals that you need a common symbol for
> the peripheral drivers anyway. How about... ARCH_SUNXI? There are 90+
> uses of this symbol throughout drivers/ and sound/, and I have found
> that more than half of them apply to the D1 (see e.g. this commit[1] and
> some of its ancestors).

Looking at the current use of ARCH_SUNXI in drivers reusing the same 
name on RISC-V makes sense.

Best regards

Heinrich

> 
> RISC-V has so far adopted a CONFIG_SOC_xxx naming scheme, which is
> different from arm/arm64's CONFIG_ARCH_xxx pattern. But now we have a
> case where a SoC family is split between the two architectures. I'm all
> for consistency with the names of other RISC-V platform symbols, but it
> seems that reusing the existing ARCH_SUNXI symbol would be better than
> cluttering up the driver Kconfig files with a duplicate.
> 
> Regards,
> Samuel
> 
> [1]: https://github.com/smaeul/linux/commit/7841e5c32366
>

Maxime Ripard Sept. 14, 2021, 7:20 a.m. UTC | #11

Hi,

On Tue, Sep 14, 2021 at 05:49:52AM +0200, Heinrich Schuchardt wrote:
> > Hi,
> > 
> > On Sat, Sep 11, 2021 at 05:21:39PM +0800, [e-mail] wrote:
> > > From: Liu Shaohua <[e-mail]>
> > > 
> > > Add Allwinner kconfig option which selects SoC specific and common
> > > drivers that is required for this SoC.
> > > 
> > > Allwinner D1 uses custom PTE attributes to solve non-coherency SOC
> > > interconnect issues for dma synchronization, so we set the default
> > > value when SOC_SUNXI selected.
> > > 
> > > Signed-off-by: Liu Shaohua <[e-mail]>
> > > Signed-off-by: Guo Ren <[e-mail]>
> > > Signed-off-by: Wei Fu <[e-mail]>
> > > Cc: Anup Patel <[e-mail]>
> > > Cc: Atish Patra <[e-mail]>
> > > Cc: Christoph Hellwig <[e-mail]>
> > > Cc: Chen-Yu Tsai <[e-mail]>
> > > Cc: Drew Fustini <[e-mail]>
> > > Cc: Maxime Ripard <[e-mail]>
> > > Cc: Palmer Dabbelt <[e-mail]>
> > > Cc: Wei Wu <[e-mail]>
> > > ---
> > >   arch/riscv/Kconfig.socs      | 15 +++++++++++++++
> > >   arch/riscv/configs/defconfig |  1 +
> > >   2 files changed, 16 insertions(+)
> > > 
> > > diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
> > > index 30676ebb16eb..8721c000ef23 100644
> > > --- a/arch/riscv/Kconfig.socs
> > > +++ b/arch/riscv/Kconfig.socs
> > > @@ -70,4 +70,19 @@ config SOC_CANAAN_K210_DTB_SOURCE
> > >   endif
> > > +config SOC_SUNXI
> > > +	bool "Allwinner SoCs"
> > > +	depends on MMU
> > > +	select DWMAC_GENERIC
> > > +	select ERRATA_THEAD
> > > +	select RISCV_DMA_NONCOHERENT
> > > +	select RISCV_ERRATA_ALTERNATIVE
> > > +	select SERIAL_8250
> > > +	select SERIAL_8250_CONSOLE
> > > +	select SERIAL_8250_DW
> > > +	select SIFIVE_PLIC
> > > +	select STMMAC_ETH
> > > +	help
> > > +	  This enables support for Allwinner SoC platforms like the D1.
> > > +
> > 
> > I'm not sure we should select the drivers there. We could very well
> > imagine a board without UART, or even more so without ethernet.
> 
> The draft of the RISC-V platform specification is available here:
> https://github.com/riscv/riscv-platform-specs/blob/main/riscv-platform-spec.adoc#uartserial-console
> 
> The specification requires in section "2.1.5.1. UART/Serial Console" that on
> platforms with a rich operating system (e.g. Linux) you have a serial
> console. Hence requiring 8250 support for the D1 CPU is justified.

I mean, not really? The platform is required to have a UART, but nothing
requires the kernel to have a driver for it as far as I'm aware.

Maxime

Ben Dooks Sept. 14, 2021, 9:26 a.m. UTC | #12

On 13/09/2021 09:45, Maxime Ripard wrote:
> Hi,
> 
> On Sat, Sep 11, 2021 at 05:21:39PM +0800, [e-mail] wrote:
>> From: Liu Shaohua <[e-mail]>
>>
>> Add Allwinner kconfig option which selects SoC specific and common
>> drivers that is required for this SoC.
>>
>> Allwinner D1 uses custom PTE attributes to solve non-coherency SOC
>> interconnect issues for dma synchronization, so we set the default
>> value when SOC_SUNXI selected.
>>
>> Signed-off-by: Liu Shaohua <[e-mail]>
>> Signed-off-by: Guo Ren <[e-mail]>
>> Signed-off-by: Wei Fu <[e-mail]>
>> Cc: Anup Patel <[e-mail]>
>> Cc: Atish Patra <[e-mail]>
>> Cc: Christoph Hellwig <[e-mail]>
>> Cc: Chen-Yu Tsai <[e-mail]>
>> Cc: Drew Fustini <[e-mail]>
>> Cc: Maxime Ripard <[e-mail]>
>> Cc: Palmer Dabbelt <[e-mail]>
>> Cc: Wei Wu <[e-mail]>
>> ---
>>   arch/riscv/Kconfig.socs      | 15 +++++++++++++++
>>   arch/riscv/configs/defconfig |  1 +
>>   2 files changed, 16 insertions(+)
>>
>> diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
>> index 30676ebb16eb..8721c000ef23 100644
>> --- a/arch/riscv/Kconfig.socs
>> +++ b/arch/riscv/Kconfig.socs
>> @@ -70,4 +70,19 @@ config SOC_CANAAN_K210_DTB_SOURCE
>>   
>>   endif
>>   
>> +config SOC_SUNXI
>> +	bool "Allwinner SoCs"
>> +	depends on MMU
>> +	select DWMAC_GENERIC
>> +	select ERRATA_THEAD
>> +	select RISCV_DMA_NONCOHERENT
>> +	select RISCV_ERRATA_ALTERNATIVE
>> +	select SERIAL_8250
>> +	select SERIAL_8250_CONSOLE
>> +	select SERIAL_8250_DW
>> +	select SIFIVE_PLIC
>> +	select STMMAC_ETH
>> +	help
>> +	  This enables support for Allwinner SoC platforms like the D1.
>> +
> 
> I'm not sure we should select the drivers there. We could very well
> imagine a board without UART, or even more so without ethernet.

I could make a case for selecting the serial as it is probably the
console, however the ethernet is not necessary for operation and I
would prefer to see this removed.

I wonder if we should have a new Kconfig keyword of suggest which
marks drivers as suggested for a given SoC/board/platform.

Arnd Bergmann Sept. 14, 2021, 9:29 a.m. UTC | #13

On Tue, Sep 14, 2021 at 4:36 AM Guo Ren <[e-mail]> wrote:
> On Tue, Sep 14, 2021 at 2:49 AM Randy Dunlap <[e-mail]> wrote:
> > On 9/13/21 2:20 AM, Guo Ren wrote:
> > > On Mon, Sep 13, 2021 at 4:45 PM Maxime Ripard <[e-mail]> wrote:
> > >> On Sat, Sep 11, 2021 at 05:21:39PM +0800, [e-mail] wrote:
> > >>> From: Liu Shaohua <[e-mail]>
> > >>>
> > >>> Add Allwinner kconfig option which selects SoC specific and common
> > >>> drivers that is required for this SoC.
> > >>>
> > >>> Allwinner D1 uses custom PTE attributes to solve non-coherency SOC
> > >>> interconnect issues for dma synchronization, so we set the default
> > >>> value when SOC_SUNXI selected.
> > >>>
> > >>>
> > >>> +config SOC_SUNXI
> > >>> +     bool "Allwinner SoCs"
> > >>> +     depends on MMU
> > >>> +     select DWMAC_GENERIC
> > >>> +     select ERRATA_THEAD
> > >>> +     select RISCV_DMA_NONCOHERENT
> > >>> +     select RISCV_ERRATA_ALTERNATIVE
> > >>> +     select SERIAL_8250
> > >>> +     select SERIAL_8250_CONSOLE
> > >>> +     select SERIAL_8250_DW
> > >>> +     select SIFIVE_PLIC
> > >>> +     select STMMAC_ETH
> > >>> +     help
> > >>> +       This enables support for Allwinner SoC platforms like the D1.
> > >>> +
> > >>
> > >> I'm not sure we should select the drivers there. We could very well
> > >> imagine a board without UART, or even more so without ethernet.
> > > We just want people could bring D1 up easier, 8250 is the basic component.
> > >
> > >
> > >>
> > >> These options should be in the defconfig.
> >
> > Agreed, using a defconfig is the right way to do this.
> Put 8250 related configs into arch/riscv/configs/defconfig?

I think that would be best, as well as the STMMAC_ETH and
DWMAC_GENERIC options.

If all RISC-V chips are required to have a 8250 compatible uart,
selecting it from CONFIG_RISCV would work as well, but for
consistency I'd give users the option to leave it out, just like
any other driver that is not required to have a useful system.

>  @Palmer Dabbelt @Arnd Bergmann, How do you think about that?
> (defconfig or Kconfig.soc)
> My purpose is when people make the Image from riscv/defconfig, then
> the Image could run on all platforms include D1.

I would try to keep the Kconfig.soc as short as possible. As a general
rule, only use 'select' to enable symbols that are otherwise not user
visible, such as the specific errata if you want to hide them. For individual
SoCs, I prefer not having separate Kconfig options, but instead have
those per driver. We have some SoC families that have part specific
options elsewhere, e.g. drivers/soc/renesas/Kconfig, but I'd only add
those if you can't avoid it. Having it in drivers/soc/ may be better for
sunxi than spreading them over arch/{arm,arm64,riscv}.

Some subsystem maintainers want drivers to be selected by the SoC
option, this is why you need the 'select SIFIVE_PLIC', but usually
the drivers are selectable with a 'depends on ARCH_SUNXI ||
COMPILE_TEST' and enabled in the defconfig.

If you want to get fancy, you can use something like:

config RESET_SUNXI
        bool "Allwinner SoCs Reset Driver" if COMPILE_TEST && !ARCH_SUNXI
        default ARCH_SUNXI

This will make an option that
 - always enabled when the platform is built-in
 - user selectable when compile-testing for any other platform
 - always disabled otherwise

       Arnd

Krzysztof Kozlowski Sept. 14, 2021, 10:07 a.m. UTC | #14

On Tue, 14 Sept 2021 at 11:31, Arnd Bergmann <[e-mail]> wrote:
> Some subsystem maintainers want drivers to be selected by the SoC
> option, this is why you need the 'select SIFIVE_PLIC', but usually
> the drivers are selectable with a 'depends on ARCH_SUNXI ||
> COMPILE_TEST' and enabled in the defconfig.

I would say selecting drivers is even more useful for distros and
other downstream users. Especially in the ARM world where we have so
many different SoCs - how could a distro person know which driver is
necessary, important or useful? Having all main SoC drivers enabled
when ARCH_SUNXI=y, helps distro a lot.

> If you want to get fancy, you can use something like:
>
> config RESET_SUNXI
>         bool "Allwinner SoCs Reset Driver" if COMPILE_TEST && !ARCH_SUNXI
>         default ARCH_SUNXI
>
> This will make an option that
>  - always enabled when the platform is built-in
>  - user selectable when compile-testing for any other platform
>  - always disabled otherwise

+1 for this pattern.

Best regards,
Krzysztof

Maxime Ripard Sept. 14, 2021, 10:13 a.m. UTC | #15

On Tue, Sep 14, 2021 at 12:07:08PM +0200, Krzysztof Kozlowski wrote:
> On Tue, 14 Sept 2021 at 11:31, Arnd Bergmann <[e-mail]> wrote:
> > Some subsystem maintainers want drivers to be selected by the SoC
> > option, this is why you need the 'select SIFIVE_PLIC', but usually
> > the drivers are selectable with a 'depends on ARCH_SUNXI ||
> > COMPILE_TEST' and enabled in the defconfig.
> 
> I would say selecting drivers is even more useful for distros and
> other downstream users. Especially in the ARM world where we have so
> many different SoCs - how could a distro person know which driver is
> necessary, important or useful? Having all main SoC drivers enabled
> when ARCH_SUNXI=y, helps distro a lot.

Imply, maybe, but select is far too painful for everyone else.

> > If you want to get fancy, you can use something like:
> >
> > config RESET_SUNXI
> >         bool "Allwinner SoCs Reset Driver" if COMPILE_TEST && !ARCH_SUNXI
> >         default ARCH_SUNXI
> >
> > This will make an option that
> >  - always enabled when the platform is built-in
> >  - user selectable when compile-testing for any other platform
> >  - always disabled otherwise
> 
> +1 for this pattern.

Yeah, or a default

Maxime

Krzysztof Kozlowski Sept. 14, 2021, 12:09 p.m. UTC | #16

On 14/09/2021 12:13, Maxime Ripard wrote:
> On Tue, Sep 14, 2021 at 12:07:08PM +0200, Krzysztof Kozlowski wrote:
>> On Tue, 14 Sept 2021 at 11:31, Arnd Bergmann <[e-mail]> wrote:
>>> Some subsystem maintainers want drivers to be selected by the SoC
>>> option, this is why you need the 'select SIFIVE_PLIC', but usually
>>> the drivers are selectable with a 'depends on ARCH_SUNXI ||
>>> COMPILE_TEST' and enabled in the defconfig.
>>
>> I would say selecting drivers is even more useful for distros and
>> other downstream users. Especially in the ARM world where we have so
>> many different SoCs - how could a distro person know which driver is
>> necessary, important or useful? Having all main SoC drivers enabled
>> when ARCH_SUNXI=y, helps distro a lot.
> 
> Imply, maybe, but select is far too painful for everyone else.

If we talk about UART driver, then sure - imply makes sense. But if we
talk about core SoC components necessary for boot (e.g. timers, clocks,
pinctrl), then select is appropriate. There is no point to enable
ARCH_XXX without these core components.


Best regards,
Krzysztof

Arnd Bergmann Sept. 14, 2021, 1:02 p.m. UTC | #17

On Tue, Sep 14, 2021 at 2:11 PM Krzysztof Kozlowski <[e-mail]> wrote:
>
> On 14/09/2021 12:13, Maxime Ripard wrote:
> > On Tue, Sep 14, 2021 at 12:07:08PM +0200, Krzysztof Kozlowski wrote:
> >> On Tue, 14 Sept 2021 at 11:31, Arnd Bergmann <[e-mail]> wrote:
> >>> Some subsystem maintainers want drivers to be selected by the SoC
> >>> option, this is why you need the 'select SIFIVE_PLIC', but usually
> >>> the drivers are selectable with a 'depends on ARCH_SUNXI ||
> >>> COMPILE_TEST' and enabled in the defconfig.
> >>
> >> I would say selecting drivers is even more useful for distros and
> >> other downstream users. Especially in the ARM world where we have so
> >> many different SoCs - how could a distro person know which driver is
> >> necessary, important or useful? Having all main SoC drivers enabled
> >> when ARCH_SUNXI=y, helps distro a lot.
> >
> > Imply, maybe, but select is far too painful for everyone else.
>
> If we talk about UART driver, then sure - imply makes sense. But if we
> talk about core SoC components necessary for boot (e.g. timers, clocks,
> pinctrl), then select is appropriate. There is no point to enable
> ARCH_XXX without these core components.

Please never use 'imply', this is functionally the same as 'default', just
the wrong way around, like the infamous 'comefrom' instruction
in programming languages ;-)

I still prefer using defaults and defconfig files over 'select', but I can
see the use of select in some cases, as long as the symbol you
are selecting is not already user visible.

        Arnd

Guo Ren Sept. 16, 2021, 6:37 a.m. UTC | #18

On Tue, Sep 14, 2021 at 5:30 PM Arnd Bergmann <[e-mail]> wrote:
>
> On Tue, Sep 14, 2021 at 4:36 AM Guo Ren <[e-mail]> wrote:
> > On Tue, Sep 14, 2021 at 2:49 AM Randy Dunlap <[e-mail]> wrote:
> > > On 9/13/21 2:20 AM, Guo Ren wrote:
> > > > On Mon, Sep 13, 2021 at 4:45 PM Maxime Ripard <[e-mail]> wrote:
> > > >> On Sat, Sep 11, 2021 at 05:21:39PM +0800, [e-mail] wrote:
> > > >>> From: Liu Shaohua <[e-mail]>
> > > >>>
> > > >>> Add Allwinner kconfig option which selects SoC specific and common
> > > >>> drivers that is required for this SoC.
> > > >>>
> > > >>> Allwinner D1 uses custom PTE attributes to solve non-coherency SOC
> > > >>> interconnect issues for dma synchronization, so we set the default
> > > >>> value when SOC_SUNXI selected.
> > > >>>
> > > >>>
> > > >>> +config SOC_SUNXI
> > > >>> +     bool "Allwinner SoCs"
> > > >>> +     depends on MMU
> > > >>> +     select DWMAC_GENERIC
> > > >>> +     select ERRATA_THEAD
> > > >>> +     select RISCV_DMA_NONCOHERENT
> > > >>> +     select RISCV_ERRATA_ALTERNATIVE
> > > >>> +     select SERIAL_8250
> > > >>> +     select SERIAL_8250_CONSOLE
> > > >>> +     select SERIAL_8250_DW
> > > >>> +     select SIFIVE_PLIC
> > > >>> +     select STMMAC_ETH
> > > >>> +     help
> > > >>> +       This enables support for Allwinner SoC platforms like the D1.
> > > >>> +
> > > >>
> > > >> I'm not sure we should select the drivers there. We could very well
> > > >> imagine a board without UART, or even more so without ethernet.
> > > > We just want people could bring D1 up easier, 8250 is the basic component.
> > > >
> > > >
> > > >>
> > > >> These options should be in the defconfig.
> > >
> > > Agreed, using a defconfig is the right way to do this.
> > Put 8250 related configs into arch/riscv/configs/defconfig?
>
> I think that would be best, as well as the STMMAC_ETH and
> DWMAC_GENERIC options.
Okay, I would move STMMAC_ETH SERIAL_8250_DW SERIAL_8250_CONSOLE
SERIAL_8250 & SIFIVE_PLIC into defconfig in the next version of the
patch.

>
> If all RISC-V chips are required to have a 8250 compatible uart,
> selecting it from CONFIG_RISCV would work as well, but for
> consistency I'd give users the option to leave it out, just like
> any other driver that is not required to have a useful system.
>
> >  @Palmer Dabbelt @Arnd Bergmann, How do you think about that?
> > (defconfig or Kconfig.soc)
> > My purpose is when people make the Image from riscv/defconfig, then
> > the Image could run on all platforms include D1.
>
> I would try to keep the Kconfig.soc as short as possible. As a general
> rule, only use 'select' to enable symbols that are otherwise not user
> visible, such as the specific errata if you want to hide them. For individual
> SoCs, I prefer not having separate Kconfig options, but instead have
> those per driver. We have some SoC families that have part specific
> options elsewhere, e.g. drivers/soc/renesas/Kconfig, but I'd only add
> those if you can't avoid it. Having it in drivers/soc/ may be better for
> sunxi than spreading them over arch/{arm,arm64,riscv}.
>
> Some subsystem maintainers want drivers to be selected by the SoC
> option, this is why you need the 'select SIFIVE_PLIC', but usually
> the drivers are selectable with a 'depends on ARCH_SUNXI ||
> COMPILE_TEST' and enabled in the defconfig.
>
> If you want to get fancy, you can use something like:
>
> config RESET_SUNXI
>         bool "Allwinner SoCs Reset Driver" if COMPILE_TEST && !ARCH_SUNXI
>         default ARCH_SUNXI
>
> This will make an option that
>  - always enabled when the platform is built-in
>  - user selectable when compile-testing for any other platform
>  - always disabled otherwise
>
>        Arnd

Patch

diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
index 30676ebb16eb..8721c000ef23 100644
--- a/arch/riscv/Kconfig.socs
+++ b/arch/riscv/Kconfig.socs
@@ -70,4 +70,19 @@  config SOC_CANAAN_K210_DTB_SOURCE
 
 endif
 
+config SOC_SUNXI
+	bool "Allwinner SoCs"
+	depends on MMU
+	select DWMAC_GENERIC
+	select ERRATA_THEAD
+	select RISCV_DMA_NONCOHERENT
+	select RISCV_ERRATA_ALTERNATIVE
+	select SERIAL_8250
+	select SERIAL_8250_CONSOLE
+	select SERIAL_8250_DW
+	select SIFIVE_PLIC
+	select STMMAC_ETH
+	help
+	  This enables support for Allwinner SoC platforms like the D1.
+
 endmenu
diff --git a/arch/riscv/configs/defconfig b/arch/riscv/configs/defconfig
index bc68231a8fb7..a50f250fbdd8 100644
--- a/arch/riscv/configs/defconfig
+++ b/arch/riscv/configs/defconfig
@@ -15,6 +15,7 @@  CONFIG_BLK_DEV_INITRD=y
 CONFIG_EXPERT=y
 CONFIG_BPF_SYSCALL=y
 CONFIG_SOC_SIFIVE=y
+CONFIG_SOC_SUNXI=y
 CONFIG_SOC_VIRT=y
 CONFIG_SOC_MICROCHIP_POLARFIRE=y
 CONFIG_SMP=y