Re: [PATCH v2 5/6] riscv: lib: move platform-related libraries to sperate folder

[Sean Anderson <seanga2@gmail.com>]

On 8/10/21 3:04 AM, Zong Li wrote:
> On Tue, Aug 10, 2021 at 12:55 PM Sean Anderson <seanga2@gmail.com> wrote:
>>
>>> Re: [PATCH v2 5/6] riscv: lib: move platform-related libraries to sperate folder
>>
>> nit: separate
>>
> 
> Thanks for catching it. Fix it in the next version.
> 
>> On 8/3/21 12:44 AM, Zong Li wrote:
>>> Put the platform-related implementation into their own folder
>>> respectively. Just leave the common library in the top of lib
>>> folder.
>>>
>>> Signed-off-by: Zong Li <zong.li@sifive.com>
>>> ---
>>>    arch/riscv/Kconfig                          | 7 +++++++
>>>    arch/riscv/lib/Makefile                     | 9 ++++-----
>>>    arch/riscv/lib/andestech/Kconfig            | 8 ++++++++
>>>    arch/riscv/lib/andestech/Makefile           | 7 +++++++
>>>    arch/riscv/lib/{ => andestech}/andes_plic.c | 0
>>>    arch/riscv/lib/sifive/Kconfig               | 8 ++++++++
>>>    arch/riscv/lib/sifive/Makefile              | 9 +++++++++
>>>    arch/riscv/lib/{ => sifive}/sifive_cache.c  | 0
>>>    arch/riscv/lib/{ => sifive}/sifive_clint.c  | 0
>>>    9 files changed, 43 insertions(+), 5 deletions(-)
>>>    create mode 100644 arch/riscv/lib/andestech/Kconfig
>>>    create mode 100644 arch/riscv/lib/andestech/Makefile
>>>    rename arch/riscv/lib/{ => andestech}/andes_plic.c (100%)
>>>    create mode 100644 arch/riscv/lib/sifive/Kconfig
>>>    create mode 100644 arch/riscv/lib/sifive/Makefile
>>>    rename arch/riscv/lib/{ => sifive}/sifive_cache.c (100%)
>>>    rename arch/riscv/lib/{ => sifive}/sifive_clint.c (100%)
>>
>> NAK from me. I'd much rather see organization by function (e.g.
>> clint/sbi/plic together) than by vendor. Plus, the clint/plic are not
>> really specific to one vendor like ccache.
>>
> 
> Yes, it makes more sense to me. In this case, there are three
> functionalities, so I'd like to separate clint, plic and cache at this
> time, does it make sense to you?

No, it does not. clint and plic should be grouped with SBI because in
U-Boot they are all used for IPIs. But frankly, I don't really see the
need to place them in subdirectories yet...

--Sean

> 
>> --Sean
>>
>>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>>> index ec651fe0a4..ed1bf2f6c8 100644
>>> --- a/arch/riscv/Kconfig
>>> +++ b/arch/riscv/Kconfig
>>> @@ -72,6 +72,10 @@ source "arch/riscv/cpu/fu540/Kconfig"
>>>    source "arch/riscv/cpu/fu740/Kconfig"
>>>    source "arch/riscv/cpu/generic/Kconfig"
>>>
>>> +# library-specific options below
>>> +source "arch/riscv/lib/sifive/Kconfig"
>>> +source "arch/riscv/lib/andestech/Kconfig"
>>> +
>>>    # architecture-specific options below
>>>
>>>    choice
>>> @@ -175,18 +179,21 @@ config SIFIVE_CLINT
>>>    config SPL_SIFIVE_CLINT
>>>        bool
>>>        depends on SPL_RISCV_MMODE
>>> +     select SIFIVE_LIB
>>>        help
>>>          The SiFive CLINT block holds memory-mapped control and status registers
>>>          associated with software and timer interrupts.
>>>
>>>    config SIFIVE_CACHE
>>>        bool
>>> +     select SIFIVE_LIB
>>>        help
>>>          This enables the operations to configure SiFive cache
>>>
>>>    config ANDES_PLIC
>>>        bool
>>>        depends on RISCV_MMODE || SPL_RISCV_MMODE
>>> +     select ANDESTECH_LIB
>>>        select REGMAP
>>>        select SYSCON
>>>        select SPL_REGMAP if SPL
>>> diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile
>>> index 06020fcc2a..f58d1f9819 100644
>>> --- a/arch/riscv/lib/Makefile
>>> +++ b/arch/riscv/lib/Makefile
>>> @@ -10,11 +10,7 @@ obj-$(CONFIG_CMD_BOOTM) += bootm.o
>>>    obj-$(CONFIG_CMD_BOOTI) += bootm.o image.o
>>>    obj-$(CONFIG_CMD_GO) += boot.o
>>>    obj-y       += cache.o
>>> -obj-$(CONFIG_SIFIVE_CACHE) += sifive_cache.o
>>> -ifeq ($(CONFIG_$(SPL_)RISCV_MMODE),y)
>>> -obj-$(CONFIG_$(SPL_)SIFIVE_CLINT) += sifive_clint.o
>>> -obj-$(CONFIG_ANDES_PLIC) += andes_plic.o
>>> -else
>>> +ifeq ($(CONFIG_$(SPL_)RISCV_MMODE),)
>>>    obj-$(CONFIG_SBI) += sbi.o
>>>    obj-$(CONFIG_SBI_IPI) += sbi_ipi.o
>>>    endif
>>> @@ -42,3 +38,6 @@ extra-$(CONFIG_EFI) += $(EFI_CRT0) $(EFI_RELOC)
>>>    obj-$(CONFIG_$(SPL_TPL_)USE_ARCH_MEMSET) += memset.o
>>>    obj-$(CONFIG_$(SPL_TPL_)USE_ARCH_MEMMOVE) += memmove.o
>>>    obj-$(CONFIG_$(SPL_TPL_)USE_ARCH_MEMCPY) += memcpy.o
>>> +
>>> +obj-$(CONFIG_SIFIVE_LIB) += sifive/
>>> +obj-$(CONFIG_ANDESTECH_LIB) += andestech/
>>> diff --git a/arch/riscv/lib/andestech/Kconfig b/arch/riscv/lib/andestech/Kconfig
>>> new file mode 100644
>>> index 0000000000..75f83a8123
>>> --- /dev/null
>>> +++ b/arch/riscv/lib/andestech/Kconfig
>>> @@ -0,0 +1,8 @@
>>> +# SPDX-License-Identifier: GPL-2.0+
>>> +#
>>> +# Copyright (C) 2021 SiFive, Inc
>>> +
>>> +config ANDESTECH_LIB
>>> +     bool
>>> +     help
>>> +       This supports the specific libraries for AndesTech platforms
>>> diff --git a/arch/riscv/lib/andestech/Makefile b/arch/riscv/lib/andestech/Makefile
>>> new file mode 100644
>>> index 0000000000..49f45d0a29
>>> --- /dev/null
>>> +++ b/arch/riscv/lib/andestech/Makefile
>>> @@ -0,0 +1,7 @@
>>> +# SPDX-License-Identifier: GPL-2.0+
>>> +#
>>> +# Copyright (C) 2021 SiFive, Inc
>>> +
>>> +ifeq ($(CONFIG_$(SPL_)RISCV_MMODE),y)
>>> +obj-$(CONFIG_ANDES_PLIC) += andes_plic.o
>>> +endif
>>> diff --git a/arch/riscv/lib/andes_plic.c b/arch/riscv/lib/andestech/andes_plic.c
>>> similarity index 100%
>>> rename from arch/riscv/lib/andes_plic.c
>>> rename to arch/riscv/lib/andestech/andes_plic.c
>>> diff --git a/arch/riscv/lib/sifive/Kconfig b/arch/riscv/lib/sifive/Kconfig
>>> new file mode 100644
>>> index 0000000000..20574079e9
>>> --- /dev/null
>>> +++ b/arch/riscv/lib/sifive/Kconfig
>>> @@ -0,0 +1,8 @@
>>> +# SPDX-License-Identifier: GPL-2.0+
>>> +#
>>> +# Copyright (C) 2021 SiFive, Inc
>>> +
>>> +config SIFIVE_LIB
>>> +     bool
>>> +     help
>>> +       This supports the specific libraries for SiFive platforms
>>> diff --git a/arch/riscv/lib/sifive/Makefile b/arch/riscv/lib/sifive/Makefile
>>> new file mode 100644
>>> index 0000000000..ba120db26a
>>> --- /dev/null
>>> +++ b/arch/riscv/lib/sifive/Makefile
>>> @@ -0,0 +1,9 @@
>>> +# SPDX-License-Identifier: GPL-2.0+
>>> +#
>>> +# Copyright (C) 2021 SiFive, Inc
>>> +
>>> +obj-$(CONFIG_SIFIVE_CACHE) += sifive_cache.o
>>> +
>>> +ifeq ($(CONFIG_$(SPL_)RISCV_MMODE),y)
>>> +obj-$(CONFIG_$(SPL_)SIFIVE_CLINT) += sifive_clint.o
>>> +endif
>>> diff --git a/arch/riscv/lib/sifive_cache.c b/arch/riscv/lib/sifive/sifive_cache.c
>>> similarity index 100%
>>> rename from arch/riscv/lib/sifive_cache.c
>>> rename to arch/riscv/lib/sifive/sifive_cache.c
>>> diff --git a/arch/riscv/lib/sifive_clint.c b/arch/riscv/lib/sifive/sifive_clint.c
>>> similarity index 100%
>>> rename from arch/riscv/lib/sifive_clint.c
>>> rename to arch/riscv/lib/sifive/sifive_clint.c
>>>
>>