| Message ID | 20220929143225.17907-1-hal.feng@linux.starfivetech.com |
|---|---|
| Headers | show |
| Series | Basic StarFive JH7110 RISC-V SoC support | expand |
On 29/09/2022 16:31, Hal Feng wrote: > From: Emil Renner Berthing <kernel@esmil.dk> > > This cache controller is also used on the StarFive JH7100 and JH7110 > SoCs. Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
On 29/09/2022 16:31, Hal Feng wrote: > This series is also available at > https://github.com/hal-feng/linux/commits/visionfive2-minimal > > [1] https://www.cnx-software.com/2022/08/23/starfive-visionfive-2-quad-core-risc-v-sbc-linux/ > [2] https://wiki.rvspace.org/ > > Emil Renner Berthing (17): > dt-bindings: riscv: Add StarFive JH7110 bindings > dt-bindings: timer: Add StarFive JH7110 clint > dt-bindings: interrupt-controller: Add StarFive JH7110 plic > dt-bindings: sifive-l2-cache: Support StarFive JH71x0 SoCs > soc: sifive: l2 cache: Convert to platform driver > soc: sifive: l2 cache: Add StarFive JH71x0 support > reset: starfive: jh7100: Use 32bit I/O on 32bit registers > dt-bindings: reset: Add StarFive JH7110 reset definitions > clk: starfive: Factor out common clock driver code > dt-bindings: clock: Add StarFive JH7110 system clock definitions > dt-bindings: clock: Add starfive,jh7110-clkgen-sys bindings > clk: starfive: Add StarFive JH7110 system clock driver > dt-bindings: clock: Add StarFive JH7110 always-on definitions > dt-bindings: clock: Add starfive,jh7110-clkgen-aon bindings > clk: starfive: Add StarFive JH7110 always-on clock driver > RISC-V: Add initial StarFive JH7110 device tree > RISC-V: Add StarFive JH7110 VisionFive2 board device tree Where is the rest of patches? Lists got only 5 of them. Anyway this is a bit too big patchset. Split per subsystem. Best regards, Krzysztof
On Thu, Sep 29, 2022 at 10:32:00PM +0800, Hal Feng wrote: > From: Emil Renner Berthing <kernel@esmil.dk> > > This converts the driver to use the builtin_platform_driver_probe macro > to initialize the driver. This macro ends up calling device_initcall as > was used previously, but also allocates a platform device which gives us > access to much nicer APIs such as platform_ioremap_resource, > platform_get_irq and dev_err_probe. You should resend the series ignoring this comment, but for v2, I think you should pay attention to following patchset: https://lore.kernel.org/linux-riscv/20220913061817.22564-1-zong.li@sifive.com/ Hopefully by the time you get to a v2, that patchset will have been applied as 6.1 material.. Thanks, Conor. > > Signed-off-by: Emil Renner Berthing <kernel@esmil.dk> > Signed-off-by: Hal Feng <hal.feng@linux.starfivetech.com> > --- > drivers/soc/sifive/sifive_l2_cache.c | 79 ++++++++++++++-------------- > 1 file changed, 40 insertions(+), 39 deletions(-) > > diff --git a/drivers/soc/sifive/sifive_l2_cache.c b/drivers/soc/sifive/sifive_l2_cache.c > index 59640a1d0b28..010d612f7420 100644 > --- a/drivers/soc/sifive/sifive_l2_cache.c > +++ b/drivers/soc/sifive/sifive_l2_cache.c > @@ -7,9 +7,9 @@ > */ > #include <linux/debugfs.h> > #include <linux/interrupt.h> > -#include <linux/of_irq.h> > -#include <linux/of_address.h> > -#include <linux/device.h> > +#include <linux/io.h> > +#include <linux/mod_devicetable.h> > +#include <linux/platform_device.h> > #include <asm/cacheinfo.h> > #include <soc/sifive/sifive_l2_cache.h> > > @@ -96,12 +96,6 @@ static void l2_config_read(void) > pr_info("L2CACHE: Index of the largest way enabled: %d\n", regval); > } > > -static const struct of_device_id sifive_l2_ids[] = { > - { .compatible = "sifive,fu540-c000-ccache" }, > - { .compatible = "sifive,fu740-c000-ccache" }, > - { /* end of table */ }, > -}; > - > static ATOMIC_NOTIFIER_HEAD(l2_err_chain); > > int register_sifive_l2_error_notifier(struct notifier_block *nb) > @@ -192,36 +186,29 @@ static irqreturn_t l2_int_handler(int irq, void *device) > return IRQ_HANDLED; > } > > -static int __init sifive_l2_init(void) > +static int __init sifive_l2_probe(struct platform_device *pdev) > { > - struct device_node *np; > - struct resource res; > - int i, rc, intr_num; > - > - np = of_find_matching_node(NULL, sifive_l2_ids); > - if (!np) > - return -ENODEV; > - > - if (of_address_to_resource(np, 0, &res)) > - return -ENODEV; > - > - l2_base = ioremap(res.start, resource_size(&res)); > - if (!l2_base) > - return -ENOMEM; > - > - intr_num = of_property_count_u32_elems(np, "interrupts"); > - if (!intr_num) { > - pr_err("L2CACHE: no interrupts property\n"); > - return -ENODEV; > - } > - > - for (i = 0; i < intr_num; i++) { > - g_irq[i] = irq_of_parse_and_map(np, i); > - rc = request_irq(g_irq[i], l2_int_handler, 0, "l2_ecc", NULL); > - if (rc) { > - pr_err("L2CACHE: Could not request IRQ %d\n", g_irq[i]); > - return rc; > - } > + struct device *dev = &pdev->dev; > + int nirqs; > + int ret; > + int i; > + > + l2_base = devm_platform_ioremap_resource(pdev, 0); > + if (IS_ERR(l2_base)) > + return PTR_ERR(l2_base); > + > + nirqs = platform_irq_count(pdev); > + if (nirqs <= 0) > + return dev_err_probe(dev, -ENODEV, "no interrupts\n"); > + > + for (i = 0; i < nirqs; i++) { > + g_irq[i] = platform_get_irq(pdev, i); > + if (g_irq[i] < 0) > + return g_irq[i]; > + > + ret = devm_request_irq(dev, g_irq[i], l2_int_handler, 0, pdev->name, NULL); > + if (ret) > + return dev_err_probe(dev, ret, "Could not request IRQ %d\n", g_irq[i]); > } > > l2_config_read(); > @@ -234,4 +221,18 @@ static int __init sifive_l2_init(void) > #endif > return 0; > } > -device_initcall(sifive_l2_init); > + > +static const struct of_device_id sifive_l2_match[] = { > + { .compatible = "sifive,fu540-c000-ccache" }, > + { .compatible = "sifive,fu740-c000-ccache" }, > + { /* sentinel */ } > +}; > + > +static struct platform_driver sifive_l2_driver = { > + .driver = { > + .name = "sifive_l2_cache", > + .of_match_table = sifive_l2_match, > + .suppress_bind_attrs = true, > + }, > +}; > +builtin_platform_driver_probe(sifive_l2_driver, sifive_l2_probe); > -- > 2.17.1 > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
On 29/09/2022 15:32, Hal Feng wrote: > From: Emil Renner Berthing <kernel@esmil.dk> > > This converts the driver to use the builtin_platform_driver_probe macro > to initialize the driver. This macro ends up calling device_initcall as > was used previously, but also allocates a platform device which gives us > access to much nicer APIs such as platform_ioremap_resource, > platform_get_irq and dev_err_probe. This is useful, but also there are other changes currently being sorted out by Zong Li (cc'd into this message) which have already been reviewed and are hopefully queued for the next kernel release. > Signed-off-by: Emil Renner Berthing <kernel@esmil.dk> > Signed-off-by: Hal Feng <hal.feng@linux.starfivetech.com> > --- > drivers/soc/sifive/sifive_l2_cache.c | 79 ++++++++++++++-------------- > 1 file changed, 40 insertions(+), 39 deletions(-) > > diff --git a/drivers/soc/sifive/sifive_l2_cache.c b/drivers/soc/sifive/sifive_l2_cache.c > index 59640a1d0b28..010d612f7420 100644 > --- a/drivers/soc/sifive/sifive_l2_cache.c > +++ b/drivers/soc/sifive/sifive_l2_cache.c > @@ -7,9 +7,9 @@ > */ > #include <linux/debugfs.h> > #include <linux/interrupt.h> > -#include <linux/of_irq.h> > -#include <linux/of_address.h> > -#include <linux/device.h> > +#include <linux/io.h> > +#include <linux/mod_devicetable.h> > +#include <linux/platform_device.h> > #include <asm/cacheinfo.h> > #include <soc/sifive/sifive_l2_cache.h> > > @@ -96,12 +96,6 @@ static void l2_config_read(void) > pr_info("L2CACHE: Index of the largest way enabled: %d\n", regval); > } > > -static const struct of_device_id sifive_l2_ids[] = { > - { .compatible = "sifive,fu540-c000-ccache" }, > - { .compatible = "sifive,fu740-c000-ccache" }, > - { /* end of table */ }, > -}; > - > static ATOMIC_NOTIFIER_HEAD(l2_err_chain); > > int register_sifive_l2_error_notifier(struct notifier_block *nb) > @@ -192,36 +186,29 @@ static irqreturn_t l2_int_handler(int irq, void *device) > return IRQ_HANDLED; > } > > -static int __init sifive_l2_init(void) > +static int __init sifive_l2_probe(struct platform_device *pdev) > { > - struct device_node *np; > - struct resource res; > - int i, rc, intr_num; > - > - np = of_find_matching_node(NULL, sifive_l2_ids); > - if (!np) > - return -ENODEV; > - > - if (of_address_to_resource(np, 0, &res)) > - return -ENODEV; > - > - l2_base = ioremap(res.start, resource_size(&res)); > - if (!l2_base) > - return -ENOMEM; > - > - intr_num = of_property_count_u32_elems(np, "interrupts"); > - if (!intr_num) { > - pr_err("L2CACHE: no interrupts property\n"); > - return -ENODEV; > - } > - > - for (i = 0; i < intr_num; i++) { > - g_irq[i] = irq_of_parse_and_map(np, i); > - rc = request_irq(g_irq[i], l2_int_handler, 0, "l2_ecc", NULL); > - if (rc) { > - pr_err("L2CACHE: Could not request IRQ %d\n", g_irq[i]); > - return rc; > - } > + struct device *dev = &pdev->dev; > + int nirqs; > + int ret; > + int i; > + > + l2_base = devm_platform_ioremap_resource(pdev, 0); > + if (IS_ERR(l2_base)) > + return PTR_ERR(l2_base); > + > + nirqs = platform_irq_count(pdev); > + if (nirqs <= 0) > + return dev_err_probe(dev, -ENODEV, "no interrupts\n"); I wonder if zero irqs is an actual issue here? > + for (i = 0; i < nirqs; i++) { > + g_irq[i] = platform_get_irq(pdev, i); I wonder if we need to keep g_irq[] around now? Is it going to be useful in the future? > + if (g_irq[i] < 0) > + return g_irq[i]; > + > + ret = devm_request_irq(dev, g_irq[i], l2_int_handler, 0, pdev->name, NULL); > + if (ret) > + return dev_err_probe(dev, ret, "Could not request IRQ %d\n", g_irq[i]); > } > > l2_config_read(); > @@ -234,4 +221,18 @@ static int __init sifive_l2_init(void) > #endif > return 0; > } > -device_initcall(sifive_l2_init); > + > +static const struct of_device_id sifive_l2_match[] = { > + { .compatible = "sifive,fu540-c000-ccache" }, > + { .compatible = "sifive,fu740-c000-ccache" }, > + { /* sentinel */ } > +}; > + > +static struct platform_driver sifive_l2_driver = { > + .driver = { > + .name = "sifive_l2_cache", > + .of_match_table = sifive_l2_match, > + .suppress_bind_attrs = true, > + }, > +}; > +builtin_platform_driver_probe(sifive_l2_driver, sifive_l2_probe);
On Thu, Sep 29, 2022 at 04:45:26PM +0200, Krzysztof Kozlowski wrote: > On 29/09/2022 16:31, Hal Feng wrote: > > > This series is also available at > > https://github.com/hal-feng/linux/commits/visionfive2-minimal > > > > [1] https://www.cnx-software.com/2022/08/23/starfive-visionfive-2-quad-core-risc-v-sbc-linux/ > > [2] https://wiki.rvspace.org/ > > > > Emil Renner Berthing (17): > > dt-bindings: riscv: Add StarFive JH7110 bindings > > dt-bindings: timer: Add StarFive JH7110 clint > > dt-bindings: interrupt-controller: Add StarFive JH7110 plic > > dt-bindings: sifive-l2-cache: Support StarFive JH71x0 SoCs > > soc: sifive: l2 cache: Convert to platform driver > > soc: sifive: l2 cache: Add StarFive JH71x0 support > > reset: starfive: jh7100: Use 32bit I/O on 32bit registers > > dt-bindings: reset: Add StarFive JH7110 reset definitions > > clk: starfive: Factor out common clock driver code > > dt-bindings: clock: Add StarFive JH7110 system clock definitions > > dt-bindings: clock: Add starfive,jh7110-clkgen-sys bindings > > clk: starfive: Add StarFive JH7110 system clock driver > > dt-bindings: clock: Add StarFive JH7110 always-on definitions > > dt-bindings: clock: Add starfive,jh7110-clkgen-aon bindings > > clk: starfive: Add StarFive JH7110 always-on clock driver > > RISC-V: Add initial StarFive JH7110 device tree > > RISC-V: Add StarFive JH7110 VisionFive2 board device tree > > Where is the rest of patches? Lists got only 5 of them. Anyway this is a > bit too big patchset. Split per subsystem. They seem to be coming in over time in dribs and drabs. I assume it is not a mailing list problem given how many lists are CCed on the mail and the fact that they have different providers. For v2 (or multiple v2s) please fix up your process so that this gets sent normally and not a couple of patches every hour. Thanks, Conor.
On Fri, Sep 30, 2022 at 08:23:18PM +0800, Hal Feng wrote: > Add Kconfig options to select the specified StarFive SoC. Select > necessary Kconfig options required by the specified SoC for booting. > > Signed-off-by: Hal Feng <hal.feng@linux.starfivetech.com> > --- > arch/riscv/Kconfig.socs | 27 ++++++++++++++++++++++++++- > arch/riscv/boot/dts/starfive/Makefile | 4 ++-- > drivers/clk/starfive/Kconfig | 14 ++++++-------- > drivers/pinctrl/starfive/Kconfig | 6 ++---- > drivers/reset/Kconfig | 1 - Firstly, you cannot change all of these files in one commit, sorry. > 5 files changed, 36 insertions(+), 16 deletions(-) > > diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs > index 10f68a4359f9..321c448e7b6f 100644 > --- a/arch/riscv/Kconfig.socs > +++ b/arch/riscv/Kconfig.socs > @@ -22,10 +22,35 @@ config SOC_STARFIVE > bool "StarFive SoCs" > select PINCTRL > select RESET_CONTROLLER > + select RESET_STARFIVE Secondly, we are trying to get rid of selects in arch/riscv at the moment, not add them. use "default SOC_STARFIVE" in drivers/reset/kconfig instead please. > + help > + This enables support for StarFive SoC platform hardware. > + > +if SOC_STARFIVE I don't think we want to have per soc selection menus in arch code, I think this should move to drivers/soc (a la Renesas) if you want to have a per soc selection menu or else just do "default SOC_STARFIVE" for both clock and pinctrl drivers in the clk and pinctrl Kconfig entries. Thanks, Conor. > + > +config SOC_JH7100 > + bool "StarFive JH7100 SoC support" > + depends on SOC_STARFIVE > select SIFIVE_L2 > select SIFIVE_PLIC > + select CLK_STARFIVE_JH7100 > + select PINCTRL_STARFIVE_JH7100 > + default SOC_STARFIVE > help > - This enables support for StarFive SoC platform hardware. > + This enables support for StarFive JH7100 SoC. > + > +config SOC_JH7110 > + bool "StarFive JH7110 SoC support" > + depends on SOC_STARFIVE > + select SIFIVE_L2 > + select SIFIVE_PLIC > + select CLK_STARFIVE_JH7110_SYS > + select PINCTRL_STARFIVE_JH7110 > + default SOC_STARFIVE > + help > + This enables support for StarFive JH7110 SoC. > + > +endif > > config SOC_VIRT > bool "QEMU Virt Machine" > diff --git a/arch/riscv/boot/dts/starfive/Makefile b/arch/riscv/boot/dts/starfive/Makefile > index e1237dbc6aac..a6ecd3c2ec7d 100644 > --- a/arch/riscv/boot/dts/starfive/Makefile > +++ b/arch/riscv/boot/dts/starfive/Makefile > @@ -1,3 +1,3 @@ > # SPDX-License-Identifier: GPL-2.0 > -dtb-$(CONFIG_SOC_STARFIVE) += jh7100-beaglev-starlight.dtb > -dtb-$(CONFIG_SOC_STARFIVE) += jh7110-starfive-visionfive-v2.dtb > +dtb-$(CONFIG_SOC_JH7100) += jh7100-beaglev-starlight.dtb > +dtb-$(CONFIG_SOC_JH7110) += jh7110-starfive-visionfive-v2.dtb > diff --git a/drivers/clk/starfive/Kconfig b/drivers/clk/starfive/Kconfig > index 42aad3b553cb..d0490e9f42db 100644 > --- a/drivers/clk/starfive/Kconfig > +++ b/drivers/clk/starfive/Kconfig > @@ -5,36 +5,34 @@ config CLK_STARFIVE > > config CLK_STARFIVE_JH7100 > bool "StarFive JH7100 clock support" > - depends on SOC_STARFIVE || COMPILE_TEST > + depends on SOC_JH7100 || COMPILE_TEST > select CLK_STARFIVE > - default SOC_STARFIVE > help > Say yes here to support the clock controller on the StarFive JH7100 > SoC. > > config CLK_STARFIVE_JH7100_AUDIO > tristate "StarFive JH7100 audio clock support" > - depends on SOC_STARFIVE || COMPILE_TEST > + depends on SOC_JH7100 || COMPILE_TEST > select CLK_STARFIVE > - default m if SOC_STARFIVE > + default m if SOC_JH7100 > help > Say Y or M here to support the audio clocks on the StarFive JH7100 > SoC. > > config CLK_STARFIVE_JH7110_SYS > bool "StarFive JH7110 system clock support" > - depends on SOC_STARFIVE || COMPILE_TEST > + depends on SOC_JH7110 || COMPILE_TEST > select CLK_STARFIVE > - default SOC_STARFIVE > help > Say yes here to support the system clock controller on the > StarFive JH7110 SoC. > > config CLK_STARFIVE_JH7110_AON > tristate "StarFive JH7110 always-on clock support" > - depends on SOC_STARFIVE || COMPILE_TEST > + depends on SOC_JH7110 || COMPILE_TEST > select CLK_STARFIVE > - default m if SOC_STARFIVE > + default m if SOC_JH7110 > help > Say yes here to support the always-on clock controller on the > StarFive JH7110 SoC. > diff --git a/drivers/pinctrl/starfive/Kconfig b/drivers/pinctrl/starfive/Kconfig > index fde39f4a7922..d09bdf6d3029 100644 > --- a/drivers/pinctrl/starfive/Kconfig > +++ b/drivers/pinctrl/starfive/Kconfig > @@ -2,7 +2,7 @@ > > config PINCTRL_STARFIVE_JH7100 > tristate "Pinctrl and GPIO driver for the StarFive JH7100 SoC" > - depends on SOC_STARFIVE || COMPILE_TEST > + depends on SOC_JH7100 || COMPILE_TEST > depends on OF > select GENERIC_PINCTRL_GROUPS > select GENERIC_PINMUX_FUNCTIONS > @@ -10,7 +10,6 @@ config PINCTRL_STARFIVE_JH7100 > select GPIOLIB > select GPIOLIB_IRQCHIP > select OF_GPIO > - default SOC_STARFIVE > help > Say yes here to support pin control on the StarFive JH7100 SoC. > This also provides an interface to the GPIO pins not used by other > @@ -28,10 +27,9 @@ config PINCTRL_STARFIVE > > config PINCTRL_STARFIVE_JH7110 > bool "Pinctrl and GPIO driver for the StarFive JH7110 SoC" > - depends on SOC_STARFIVE || COMPILE_TEST > + depends on SOC_JH7110 || COMPILE_TEST > depends on OF > select PINCTRL_STARFIVE > - default SOC_STARFIVE > help > Say yes here to support pin control on the StarFive JH7110 SoC. > This also provides an interface to the GPIO pins not used by other > diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig > index 8121de5ecc3c..c001879bd890 100644 > --- a/drivers/reset/Kconfig > +++ b/drivers/reset/Kconfig > @@ -228,7 +228,6 @@ config RESET_SOCFPGA > config RESET_STARFIVE > bool "StarFive SoC Reset Driver" > depends on SOC_STARFIVE || COMPILE_TEST > - default SOC_STARFIVE > help > This enables the reset controller driver for the StarFive SoCs. > > -- > 2.17.1 >
On 30/09/2022 10:06, Hal Feng wrote: > Add CONFIG_SERIAL_8250_DW=y, which is a necessary option for > StarFive JH7110 and JH7100 SoCs to boot with serial ports. > > Signed-off-by: Hal Feng <hal.feng@linux.starfivetech.com> That might be useful for other users at some point an I don't think it adds much code. > --- > arch/riscv/configs/defconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/riscv/configs/defconfig b/arch/riscv/configs/defconfig > index aed332a9d4ea..0c44484cd3a4 100644 > --- a/arch/riscv/configs/defconfig > +++ b/arch/riscv/configs/defconfig > @@ -122,6 +122,7 @@ CONFIG_MICROSEMI_PHY=y > CONFIG_INPUT_MOUSEDEV=y > CONFIG_SERIAL_8250=y > CONFIG_SERIAL_8250_CONSOLE=y > +CONFIG_SERIAL_8250_DW=y > CONFIG_SERIAL_OF_PLATFORM=y > CONFIG_VIRTIO_CONSOLE=y > CONFIG_HW_RANDOM=y
On Fri, Sep 30, 2022 at 09:54:14PM +0100, Ben Dooks wrote: > On 30/09/2022 10:06, Hal Feng wrote: > > Add CONFIG_SERIAL_8250_DW=y, which is a necessary option for > > StarFive JH7110 and JH7100 SoCs to boot with serial ports. > > > > Signed-off-by: Hal Feng <hal.feng@linux.starfivetech.com> > > That might be useful for other users at some point an I don't > think it adds much code. Honestly I think this should be applied for 6.1, for parity with the other SoCs that have their serial console enabled by default. Reviewed-by: Conor Dooley <conor.dooley@microchip.com> Thanks, Conor. > > > --- > > arch/riscv/configs/defconfig | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/arch/riscv/configs/defconfig b/arch/riscv/configs/defconfig > > index aed332a9d4ea..0c44484cd3a4 100644 > > --- a/arch/riscv/configs/defconfig > > +++ b/arch/riscv/configs/defconfig > > @@ -122,6 +122,7 @@ CONFIG_MICROSEMI_PHY=y > > CONFIG_INPUT_MOUSEDEV=y > > CONFIG_SERIAL_8250=y > > CONFIG_SERIAL_8250_CONSOLE=y > > +CONFIG_SERIAL_8250_DW=y > > CONFIG_SERIAL_OF_PLATFORM=y > > CONFIG_VIRTIO_CONSOLE=y > > CONFIG_HW_RANDOM=y > > -- > Ben Dooks http://www.codethink.co.uk/ > Senior Engineer Codethink - Providing Genius > > https://www.codethink.co.uk/privacy.html > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
Quoting Hal Feng (2022-09-29 10:54:59) > From: Emil Renner Berthing <kernel@esmil.dk> > > The clock control registers on the StarFive SoCs work identically, > so factor out the code then drivers for different SoCs can share > it without depending on each other. No functional change. Sounds great! No functional change! But to verify that it is pretty hard. Can you generate the patch with `git format-patch -M -C` and not rename anything initially? I hope that will allow us to see that really nothing has changed except code is moved from one file to another. Then the next patch can be the sed command to rename to starfive. As the patch is right now, I'm not particularly interested in going through 700 lines to make sure nothing really changed.
Quoting Hal Feng (2022-09-29 10:56:02) > Clock registers address region is shared with reset controller > on the new StarFive JH7110 SoC. Change to use regmap framework > to allow base address sharing and preparation for JH7110 clock > support. Do the reset and clk parts share actual registers, where we would need to lock between rmw? Or is regmap just nice to have because it wraps up the register APIs with some extra features? > > Signed-off-by: Hal Feng <hal.feng@linux.starfivetech.com> > --- [...] > diff --git a/drivers/clk/starfive/clk-starfive-jh7100.c b/drivers/clk/starfive/clk-starfive-jh7100.c > index 014e36f17595..410aa6e06842 100644 > --- a/drivers/clk/starfive/clk-starfive-jh7100.c > +++ b/drivers/clk/starfive/clk-starfive-jh7100.c > @@ -10,6 +10,7 @@ > #include <linux/clk-provider.h> > #include <linux/device.h> > #include <linux/init.h> > +#include <linux/mfd/syscon.h> > #include <linux/mod_devicetable.h> > #include <linux/platform_device.h> > > @@ -295,11 +296,13 @@ static int __init clk_starfive_jh7100_probe(struct platform_device *pdev) > if (!priv) > return -ENOMEM; > > - spin_lock_init(&priv->rmw_lock); > priv->dev = &pdev->dev; > - priv->base = devm_platform_ioremap_resource(pdev, 0); > - if (IS_ERR(priv->base)) > - return PTR_ERR(priv->base); > + priv->regmap = device_node_to_regmap(priv->dev->of_node); This is sad. Why do we need to make a syscon? Can we instead use the auxiliary bus to make a reset device that either gets a regmap made here in this driver or uses a void __iomem * mapped with ioremap (priv->base)? > + if (IS_ERR(priv->regmap)) { > + dev_err(priv->dev, "failed to get regmap (error %ld)\n", > + PTR_ERR(priv->regmap)); > + return PTR_ERR(priv->regmap);
On Thu, Sep 29, 2022 at 18:59:24 +0100, Conor Dooley wrote: > On Thu, Sep 29, 2022 at 04:45:26PM +0200, Krzysztof Kozlowski wrote: > > On 29/09/2022 16:31, Hal Feng wrote: > > > > > This series is also available at > > > https://github.com/hal-feng/linux/commits/visionfive2-minimal > > > > > > [1] > > > https://www.cnx-software.com/2022/08/23/starfive-visionfive-2-quad-c > > > ore-risc-v-sbc-linux/ > > > [2] https://wiki.rvspace.org/ > > > > > > Emil Renner Berthing (17): > > > dt-bindings: riscv: Add StarFive JH7110 bindings > > > dt-bindings: timer: Add StarFive JH7110 clint > > > dt-bindings: interrupt-controller: Add StarFive JH7110 plic > > > dt-bindings: sifive-l2-cache: Support StarFive JH71x0 SoCs > > > soc: sifive: l2 cache: Convert to platform driver > > > soc: sifive: l2 cache: Add StarFive JH71x0 support > > > reset: starfive: jh7100: Use 32bit I/O on 32bit registers > > > dt-bindings: reset: Add StarFive JH7110 reset definitions > > > clk: starfive: Factor out common clock driver code > > > dt-bindings: clock: Add StarFive JH7110 system clock definitions > > > dt-bindings: clock: Add starfive,jh7110-clkgen-sys bindings > > > clk: starfive: Add StarFive JH7110 system clock driver > > > dt-bindings: clock: Add StarFive JH7110 always-on definitions > > > dt-bindings: clock: Add starfive,jh7110-clkgen-aon bindings > > > clk: starfive: Add StarFive JH7110 always-on clock driver > > > RISC-V: Add initial StarFive JH7110 device tree > > > RISC-V: Add StarFive JH7110 VisionFive2 board device tree > > > > Where is the rest of patches? Lists got only 5 of them. Anyway this is > > a bit too big patchset. Split per subsystem. > > They seem to be coming in over time in dribs and drabs. I assume it is not a > mailing list problem given how many lists are CCed on the mail and the fact > that they have different providers. > > For v2 (or multiple v2s) please fix up your process so that this gets sent > normally and not a couple of patches every hour. Our email server has technical issue and we are aware of this. Will fix in next revision. Sorry for the inconvenience caused. Best Regards, Hal
On Fri, Sep 30, 2022 at 03:49:14PM +0800, Hal Feng wrote: > From: Emil Renner Berthing <kernel@esmil.dk> > > Add initial device tree for the JH7110 RISC-V SoC by > StarFive Technology Ltd. > > Signed-off-by: Emil Renner Berthing <kernel@esmil.dk> > Signed-off-by: Jianlong Huang <jianlong.huang@starfivetech.com> > Signed-off-by: Hal Feng <hal.feng@linux.starfivetech.com> There's little point reviewing this dt since there's a load of issues that you can trivially find by running dtbs_check/dt_binding_check, but this SoB change is wrong - if Emil wrote the patch, then Jianlong's SoB is either redundant or should be accompanied by a Co-developed-by tag. Ditto for patch 28/30 "RISC-V: Add StarFive JH7110 VisionFive2 board device tree". > --- > arch/riscv/boot/dts/starfive/jh7110.dtsi | 449 +++++++++++++++++++++++ > 1 file changed, 449 insertions(+) > create mode 100644 arch/riscv/boot/dts/starfive/jh7110.dtsi > > diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi > new file mode 100644 > index 000000000000..46f418d4198a > --- /dev/null > +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi > + > + osc: osc { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + }; > + > + clk_rtc: clk_rtc { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + }; > + > + gmac0_rmii_refin: gmac0_rmii_refin { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <50000000>; I assume, given osc has it's frequency set in the board dts, that these are all oscillators on the SoC? > + }; > + > + gmac0_rgmii_rxin: gmac0_rgmii_rxin { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <125000000>; > + }; > + > + gmac1_rmii_refin: gmac1_rmii_refin { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <50000000>; > + }; > + > + gmac1_rgmii_rxin: gmac1_rgmii_rxin { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <125000000>; > + }; > + > + i2stx_bclk_ext: i2stx_bclk_ext { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <12288000>; > + }; > + > + i2stx_lrck_ext: i2stx_lrck_ext { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <192000>; > + }; > + > + i2srx_bclk_ext: i2srx_bclk_ext { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <12288000>; > + }; > + > + i2srx_lrck_ext: i2srx_lrck_ext { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <192000>; > + }; > + > + tdm_ext: tdm_ext { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <49152000>; > + }; > + > + mclk_ext: mclk_ext { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <49152000>; > + }; > + syscrg: syscrg@13020000 { The generic node name for syscons is just "syscon" afaik. > + compatible = "syscon", "simple-mfd"; > + reg = <0x0 0x13020000 0x0 0x10000>; > + > + aoncrg: aoncrg@17000000 { Again, syscon as the node name? > + compatible = "syscon", "simple-mfd"; > + reg = <0x0 0x17000000 0x0 0x10000>; > + > + gpio: gpio@13040000 { Someone else (Krzysztof maybe?) should comment, but is "pinctrl" not the genric node name for pinctrl nodes? Thanks, Conor. > + compatible = "starfive,jh7110-sys-pinctrl"; > + reg = <0x0 0x13040000 0x0 0x10000>; > + reg-names = "control"; > + clocks = <&syscrg_clk JH7110_SYSCLK_IOMUX>; > + resets = <&syscrg_rst JH7110_SYSRST_IOMUX>; > + interrupts = <86>; > + interrupt-controller; > + #gpio-cells = <2>; > + ngpios = <64>; > + }; > + > + gpioa: gpio@17020000 { > + compatible = "starfive,jh7110-aon-pinctrl"; > + reg = <0x0 0x17020000 0x0 0x10000>; > + reg-names = "control"; > + resets = <&aoncrg_rst JH7110_AONRST_AON_IOMUX>; > + interrupts = <85>; > + interrupt-controller; > + #gpio-cells = <2>; > + ngpios = <4>; > + }; > + > + uart0: serial@10000000 { > + compatible = "snps,dw-apb-uart"; > + reg = <0x0 0x10000000 0x0 0x10000>; > + clocks = <&syscrg_clk JH7110_SYSCLK_UART0_CORE>, > + <&syscrg_clk JH7110_SYSCLK_UART0_APB>; > + clock-names = "baudclk", "apb_pclk"; > + resets = <&syscrg_rst JH7110_SYSRST_UART0_APB>, > + <&syscrg_rst JH7110_SYSRST_UART0_CORE>; > + interrupts = <32>; > + reg-io-width = <4>; > + reg-shift = <2>; > + status = "disabled"; > + }; > + > + uart1: serial@10010000 { > + compatible = "snps,dw-apb-uart"; > + reg = <0x0 0x10010000 0x0 0x10000>; > + clocks = <&syscrg_clk JH7110_SYSCLK_UART1_CORE>, > + <&syscrg_clk JH7110_SYSCLK_UART1_APB>; > + clock-names = "baudclk", "apb_pclk"; > + resets = <&syscrg_rst JH7110_SYSRST_UART1_APB>, > + <&syscrg_rst JH7110_SYSRST_UART1_CORE>; > + interrupts = <33>; > + reg-io-width = <4>; > + reg-shift = <2>; > + status = "disabled"; > + }; > + > + uart2: serial@10020000 { > + compatible = "snps,dw-apb-uart"; > + reg = <0x0 0x10020000 0x0 0x10000>; > + clocks = <&syscrg_clk JH7110_SYSCLK_UART2_CORE>, > + <&syscrg_clk JH7110_SYSCLK_UART2_APB>; > + clock-names = "baudclk", "apb_pclk"; > + resets = <&syscrg_rst JH7110_SYSRST_UART2_APB>, > + <&syscrg_rst JH7110_SYSRST_UART2_CORE>; > + interrupts = <34>; > + reg-io-width = <4>; > + reg-shift = <2>; > + status = "disabled"; > + }; > + > + uart3: serial@12000000 { > + compatible = "snps,dw-apb-uart"; > + reg = <0x0 0x12000000 0x0 0x10000>; > + clocks = <&syscrg_clk JH7110_SYSCLK_UART3_CORE>, > + <&syscrg_clk JH7110_SYSCLK_UART3_APB>; > + clock-names = "baudclk", "apb_pclk"; > + resets = <&syscrg_rst JH7110_SYSRST_UART3_APB>, > + <&syscrg_rst JH7110_SYSRST_UART3_CORE>; > + interrupts = <45>; > + reg-io-width = <4>; > + reg-shift = <2>; > + status = "disabled"; > + }; > + > + uart4: serial@12010000 { > + compatible = "snps,dw-apb-uart"; > + reg = <0x0 0x12010000 0x0 0x10000>; > + clocks = <&syscrg_clk JH7110_SYSCLK_UART4_CORE>, > + <&syscrg_clk JH7110_SYSCLK_UART4_APB>; > + clock-names = "baudclk", "apb_pclk"; > + resets = <&syscrg_rst JH7110_SYSRST_UART4_APB>, > + <&syscrg_rst JH7110_SYSRST_UART4_CORE>; > + interrupts = <46>; > + reg-io-width = <4>; > + reg-shift = <2>; > + status = "disabled"; > + }; > + > + uart5: serial@12020000 { > + compatible = "snps,dw-apb-uart"; > + reg = <0x0 0x12020000 0x0 0x10000>; > + clocks = <&syscrg_clk JH7110_SYSCLK_UART5_CORE>, > + <&syscrg_clk JH7110_SYSCLK_UART5_APB>; > + clock-names = "baudclk", "apb_pclk"; > + resets = <&syscrg_rst JH7110_SYSRST_UART5_APB>, > + <&syscrg_rst JH7110_SYSRST_UART5_CORE>; > + interrupts = <47>; > + reg-io-width = <4>; > + reg-shift = <2>; > + status = "disabled"; > + }; > + }; > +}; > -- > 2.17.1 >
On Fri, Sep 30, 2022 at 03:53:53PM +0800, Hal Feng wrote: > From: Emil Renner Berthing <kernel@esmil.dk> > > Add a minimal device tree for StarFive JH7110 VisionFive2 board. > Support booting and basic clock/reset/pinctrl/uart drivers. > I would like to see a link to the publicly available datasheet or documentation for the board (and for the SoC in patch 28) please. > Signed-off-by: Emil Renner Berthing <kernel@esmil.dk> > Signed-off-by: Jianlong Huang <jianlong.huang@starfivetech.com> Ditto from patch 28 re: the SoB chain. > Signed-off-by: Hal Feng <hal.feng@linux.starfivetech.com> > --- > diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts > new file mode 100644 > index 000000000000..6b9fe32c7eac > --- /dev/null > +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts > @@ -0,0 +1,91 @@ > +// SPDX-License-Identifier: GPL-2.0 OR MIT > +/* > + * Copyright (C) 2022 StarFive Technology Co., Ltd. > + * Copyright (C) 2022 Emil Renner Berthing <kernel@esmil.dk> > + */ > + > +/dts-v1/; > +#include "jh7110.dtsi" > +#include <dt-bindings/gpio/gpio.h> > +#include <dt-bindings/pinctrl/pinctrl-starfive-jh7110.h> > + > +/ { > + model = "StarFive VisionFive V2"; > + compatible = "starfive,visionfive-v2", "starfive,jh7110"; > + > + aliases { > + serial0 = &uart0; > + }; Should we also have a chosen node here? > + > + cpus { > + timebase-frequency = <4000000>; > + }; > + > + memory@40000000 { > + device_type = "memory"; > + reg = <0x0 0x40000000 0x1 0x0>; What is going to happen to the 2 GB variant if they attempt to use this devicetree? > + }; > + > + reserved-memory { > + #address-cells = <2>; > + #size-cells = <2>; > + ranges; > + > + linux,cma { > + compatible = "shared-dma-pool"; > + reusable; > + size = <0x0 0x20000000>; > + alignment = <0x0 0x1000>; > + alloc-ranges = <0x0 0xa0000000 0x0 0x20000000>; > + linux,cma-default; > + }; > + > + e24_mem: e24@c0000000 { I had a conversation previously with Icenowy [0] about the e24 on the jh7100 that didn't really come to a conclusion about how to represent it there - but looks like you've decided that it should be a remoteproc for the jh7100? Is this another situation where peripherals appear at different addresses for the e24 compared to the u74s? Or has that changed for the jh7100, and really the e24 should be described in the CPUs node? If it is the latter, you can pick the first patch from [0] into your series. 0 - https://lore.kernel.org/linux-riscv/e8543838cd221ab6699da16c985eed7514daa786.camel@icenowy.me/ > + reg = <0x0 0xc0110000 0x0 0xf0000>; > + no-map; > + }; > + > + xrp_reserved: xrpbuffer@f0000000 { "Following the generic-names recommended practice, node names should reflect the purpose of the node (ie. “framebuffer” or “dma-pool”)." I tried googling around for an explanation for what the xrp was, and all I could find was this out-of-tree text binding: https://github.com/foss-xtensa/xrp/blob/master/xrp-kernel/cdns%2Cxrp-hw-simple%2Cv1.txt Thanks, Conor. > + reg = <0x0 0xf0000000 0x0 0x01ffffff>, > + <0x0 0xf2000000 0x0 0x00001000>, > + <0x0 0xf2001000 0x0 0x00fff000>, > + <0x0 0xf3000000 0x0 0x00001000>; > + }; > + > + }; > + > + gpio-restart { > + compatible = "gpio-restart"; > + gpios = <&gpio 35 GPIO_ACTIVE_HIGH>; > + priority = <224>; > + }; > +}; > + > +&gpio { > + uart0_pins: uart0-pins { > + uart0-pins-tx { > + starfive,pins = <PAD_GPIO5>; > + starfive,pin-ioconfig = <IO(GPIO_IE(1) | GPIO_DS(3))>; > + starfive,pin-gpio-dout = <GPO_UART0_SOUT>; > + starfive,pin-gpio-doen = <OEN_LOW>; > + }; > + > + uart0-pins-rx { > + starfive,pins = <PAD_GPIO6>; > + starfive,pinmux = <PAD_GPIO6_FUNC_SEL 0>; > + starfive,pin-ioconfig = <IO(GPIO_IE(1) | GPIO_PU(1))>; > + starfive,pin-gpio-doen = <OEN_HIGH>; > + starfive,pin-gpio-din = <GPI_UART0_SIN>; > + }; > + }; > +}; > + > +&osc { > + clock-frequency = <24000000>; > +}; > + > +&uart0 { > + pinctrl-names = "default"; > + pinctrl-0 = <&uart0_pins>; > + status = "okay"; > +}; > -- > 2.17.1 >
On 01/10/2022 12:52, Conor Dooley wrote: > On Fri, Sep 30, 2022 at 03:49:14PM +0800, Hal Feng wrote: >> From: Emil Renner Berthing <kernel@esmil.dk> >> >> Add initial device tree for the JH7110 RISC-V SoC by >> StarFive Technology Ltd. >> >> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk> >> Signed-off-by: Jianlong Huang <jianlong.huang@starfivetech.com> >> Signed-off-by: Hal Feng <hal.feng@linux.starfivetech.com> > > There's little point reviewing this dt since there's a load of issues > that you can trivially find by running dtbs_check/dt_binding_check, but Yep... > this SoB change is wrong - if Emil wrote the patch, then Jianlong's SoB > is either redundant or should be accompanied by a Co-developed-by tag. Depends. Jianlong might have just rebased the patch. > > Ditto for patch 28/30 "RISC-V: Add StarFive JH7110 VisionFive2 board > device tree". > >> --- >> arch/riscv/boot/dts/starfive/jh7110.dtsi | 449 +++++++++++++++++++++++ >> 1 file changed, 449 insertions(+) >> create mode 100644 arch/riscv/boot/dts/starfive/jh7110.dtsi >> >> diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi >> new file mode 100644 >> index 000000000000..46f418d4198a >> --- /dev/null >> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi > >> + >> + osc: osc { >> + compatible = "fixed-clock"; >> + #clock-cells = <0>; >> + }; >> + >> + clk_rtc: clk_rtc { >> + compatible = "fixed-clock"; >> + #clock-cells = <0>; >> + }; >> + >> + gmac0_rmii_refin: gmac0_rmii_refin { >> + compatible = "fixed-clock"; >> + #clock-cells = <0>; >> + clock-frequency = <50000000>; > > I assume, given osc has it's frequency set in the board dts, that these > are all oscillators on the SoC? > >> + }; >> + >> + gmac0_rgmii_rxin: gmac0_rgmii_rxin { >> + compatible = "fixed-clock"; >> + #clock-cells = <0>; >> + clock-frequency = <125000000>; >> + }; >> + >> + gmac1_rmii_refin: gmac1_rmii_refin { >> + compatible = "fixed-clock"; >> + #clock-cells = <0>; >> + clock-frequency = <50000000>; >> + }; >> + >> + gmac1_rgmii_rxin: gmac1_rgmii_rxin { >> + compatible = "fixed-clock"; >> + #clock-cells = <0>; >> + clock-frequency = <125000000>; >> + }; >> + >> + i2stx_bclk_ext: i2stx_bclk_ext { >> + compatible = "fixed-clock"; >> + #clock-cells = <0>; >> + clock-frequency = <12288000>; >> + }; >> + >> + i2stx_lrck_ext: i2stx_lrck_ext { >> + compatible = "fixed-clock"; >> + #clock-cells = <0>; >> + clock-frequency = <192000>; >> + }; >> + >> + i2srx_bclk_ext: i2srx_bclk_ext { >> + compatible = "fixed-clock"; >> + #clock-cells = <0>; >> + clock-frequency = <12288000>; >> + }; >> + >> + i2srx_lrck_ext: i2srx_lrck_ext { >> + compatible = "fixed-clock"; >> + #clock-cells = <0>; >> + clock-frequency = <192000>; >> + }; >> + >> + tdm_ext: tdm_ext { >> + compatible = "fixed-clock"; >> + #clock-cells = <0>; >> + clock-frequency = <49152000>; >> + }; >> + >> + mclk_ext: mclk_ext { >> + compatible = "fixed-clock"; >> + #clock-cells = <0>; >> + clock-frequency = <49152000>; >> + }; > >> + syscrg: syscrg@13020000 { > > The generic node name for syscons is just "syscon" afaik. Yes. > >> + compatible = "syscon", "simple-mfd"; And this is not allowed. Needs specific compatible. >> + reg = <0x0 0x13020000 0x0 0x10000>; >> + > >> + aoncrg: aoncrg@17000000 { > > Again, syscon as the node name? Yes. > >> + compatible = "syscon", "simple-mfd"; And this is a NAK. >> + reg = <0x0 0x17000000 0x0 0x10000>; >> + >> + gpio: gpio@13040000 { > > Someone else (Krzysztof maybe?) should comment, but is "pinctrl" not the > genric node name for pinctrl nodes? Yes, for pin controller nodes, this should be "pinctrl" and schema requires it. The problem was that his driver did not use generic pinctrl bindings, which is no-go on its own. This could be a gpio controller (so "gpio" would be fine), although compatible suggests otherwise. Best regards, Krzysztof
On Fri, Sep 30, 2022 at 8:08 AM Hal Feng <hal.feng@linux.starfivetech.com> wrote: > From: Jianlong Huang <jianlong.huang@starfivetech.com> > > Move the StarFive JH7100 pinctrl driver to a new subdirectory > in preparation for adding more StarFive pinctrl drivers. No > functional change. > > Signed-off-by: Jianlong Huang <jianlong.huang@starfivetech.com> > Signed-off-by: Hal Feng <hal.feng@linux.starfivetech.com> I applied this patch for v6.1 so you don't have to reiterate it, it's clearly just infrastructure that you'll need going forward. Yours, Linus Walleij
On Fri, Sep 30, 2022 at 9:45 AM Hal Feng <hal.feng@linux.starfivetech.com> wrote: > From: Jianlong Huang <jianlong.huang@starfivetech.com> > > Add pinctrl driver for StarFive JH7110 SoC. > > Signed-off-by: Jianlong Huang <jianlong.huang@starfivetech.com> > Signed-off-by: Hal Feng <hal.feng@linux.starfivetech.com> Since Emil submitted the first driver I would really appreciate his review on this version. Yours, Linus Walleij
On Thu, 29 Sept 2022 at 16:34, Hal Feng <hal.feng@linux.starfivetech.com> wrote: > This series adds basic support for the StarFive JH7110 RISC-V SoC to > boot up and get a serial console. This series includes basic clock, > reset, pinctrl and uart drivers, which are necessary for booting. > It's should be noted that the reset and clock driver codes of JH7110 > are partly common with those of JH7100, so the common codes are > factored out and can be reused by drivers of JH7110 and other more > SoCs from StarFive. > > The JH7110 is the upgraded version of JH7100 and also the first official > released version of JH71XX series SoCs from StarFive Technology Ltd. > The VisionFive 2 boards equipped with JH7110 SoCs are launched > recently [1]. More information and support can visit RVspace wiki [2]. > > This series is also available at > https://github.com/hal-feng/linux/commits/visionfive2-minimal > > [1] https://www.cnx-software.com/2022/08/23/starfive-visionfive-2-quad-core-risc-v-sbc-linux/ > [2] https://wiki.rvspace.org/ Hi Hal, Firstly thanks for working on this! And sorry about the late reply. On the next version could you please cc emil.renner.berthing@canonical.com since it seems to handle the mailing list a bit better. I see you've changed the clock/reset and pinctrl quite a bit, so I'll comment on that separately. /Emil > Emil Renner Berthing (17): > dt-bindings: riscv: Add StarFive JH7110 bindings > dt-bindings: timer: Add StarFive JH7110 clint > dt-bindings: interrupt-controller: Add StarFive JH7110 plic > dt-bindings: sifive-l2-cache: Support StarFive JH71x0 SoCs > soc: sifive: l2 cache: Convert to platform driver > soc: sifive: l2 cache: Add StarFive JH71x0 support > reset: starfive: jh7100: Use 32bit I/O on 32bit registers > dt-bindings: reset: Add StarFive JH7110 reset definitions > clk: starfive: Factor out common clock driver code > dt-bindings: clock: Add StarFive JH7110 system clock definitions > dt-bindings: clock: Add starfive,jh7110-clkgen-sys bindings > clk: starfive: Add StarFive JH7110 system clock driver > dt-bindings: clock: Add StarFive JH7110 always-on definitions > dt-bindings: clock: Add starfive,jh7110-clkgen-aon bindings > clk: starfive: Add StarFive JH7110 always-on clock driver > RISC-V: Add initial StarFive JH7110 device tree > RISC-V: Add StarFive JH7110 VisionFive2 board device tree > > Hal Feng (8): > reset: starfive: jh7100: Use regmap APIs to operate registers > reset: starfive: jh7100: Move necessary properties to device tree > reset: starfive: Rename 'reset-starfive-jh7100.c' to > 'reset-starfive.c' > dt-bindings: reset: Add starfive,jh7110-reset bindings > reset: starfive: Add StarFive JH7110 SoC support > clk: starfive: Use regmap APIs to operate registers > RISC-V: defconfig: Enable CONFIG_SERIAL_8250_DW > RISC-V: Add StarFive JH7100 and JH7110 SoC Kconfig options > > Jianlong Huang (5): > pinctrl: Create subdirectory for StarFive drivers > pinctrl: starfive: Rename "pinctrl-starfive" to > "pinctrl-starfive-jh7100" > dt-bindings: pinctrl: Add StarFive JH7110 pinctrl definitions > dt-bindings: pinctrl: Add StarFive JH7110 pinctrl bindings > pinctrl: starfive: Add StarFive JH7110 driver > > .../clock/starfive,jh7110-clkgen-aon.yaml | 62 ++ > .../clock/starfive,jh7110-clkgen-sys.yaml | 69 ++ > .../sifive,plic-1.0.0.yaml | 1 + > .../pinctrl/starfive,jh7100-pinctrl.yaml | 2 +- > .../pinctrl/starfive,jh7110-pinctrl.yaml | 202 ++++ > .../bindings/reset/starfive,jh7100-reset.yaml | 20 + > .../bindings/reset/starfive,jh7110-reset.yaml | 54 + > .../bindings/riscv/sifive-l2-cache.yaml | 4 + > .../devicetree/bindings/riscv/starfive.yaml | 3 + > .../bindings/timer/sifive,clint.yaml | 1 + > MAINTAINERS | 27 +- > arch/riscv/Kconfig.socs | 28 +- > arch/riscv/boot/dts/starfive/Makefile | 3 +- > .../dts/starfive/jh7100-beaglev-starlight.dts | 2 +- > arch/riscv/boot/dts/starfive/jh7100.dtsi | 3 + > .../jh7110-starfive-visionfive-v2.dts | 91 ++ > arch/riscv/boot/dts/starfive/jh7110.dtsi | 449 +++++++++ > arch/riscv/configs/defconfig | 1 + > drivers/clk/starfive/Kconfig | 29 +- > drivers/clk/starfive/Makefile | 6 +- > .../clk/starfive/clk-starfive-jh7100-audio.c | 138 +-- > drivers/clk/starfive/clk-starfive-jh7100.c | 836 +++++----------- > drivers/clk/starfive/clk-starfive-jh7100.h | 112 --- > .../clk/starfive/clk-starfive-jh7110-aon.c | 161 +++ > .../clk/starfive/clk-starfive-jh7110-sys.c | 648 ++++++++++++ > drivers/clk/starfive/clk-starfive.c | 349 +++++++ > drivers/clk/starfive/clk-starfive.h | 112 +++ > drivers/pinctrl/Kconfig | 18 +- > drivers/pinctrl/Makefile | 2 +- > drivers/pinctrl/starfive/Kconfig | 37 + > drivers/pinctrl/starfive/Makefile | 8 + > drivers/pinctrl/starfive/pinctrl-jh7110-aon.c | 718 ++++++++++++++ > drivers/pinctrl/starfive/pinctrl-jh7110-sys.c | 925 +++++++++++++++++ > .../pinctrl-starfive-jh7100.c} | 10 +- > drivers/pinctrl/starfive/pinctrl-starfive.c | 539 ++++++++++ > drivers/pinctrl/starfive/pinctrl-starfive.h | 131 +++ > drivers/reset/Kconfig | 7 +- > drivers/reset/Makefile | 2 +- > drivers/reset/reset-starfive-jh7100.c | 173 ---- > drivers/reset/reset-starfive.c | 218 ++++ > drivers/soc/Makefile | 2 +- > drivers/soc/sifive/Kconfig | 2 +- > drivers/soc/sifive/sifive_l2_cache.c | 86 +- > .../dt-bindings/clock/starfive-jh7110-aon.h | 26 + > .../dt-bindings/clock/starfive-jh7110-sys.h | 215 ++++ > ...l-starfive.h => pinctrl-starfive-jh7100.h} | 6 +- > .../pinctrl/pinctrl-starfive-jh7110.h | 931 ++++++++++++++++++ > include/dt-bindings/reset/starfive-jh7110.h | 154 +++ > 48 files changed, 6604 insertions(+), 1019 deletions(-) > create mode 100644 Documentation/devicetree/bindings/clock/starfive,jh7110-clkgen-aon.yaml > create mode 100644 Documentation/devicetree/bindings/clock/starfive,jh7110-clkgen-sys.yaml > create mode 100644 Documentation/devicetree/bindings/pinctrl/starfive,jh7110-pinctrl.yaml > create mode 100644 Documentation/devicetree/bindings/reset/starfive,jh7110-reset.yaml > create mode 100644 arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts > create mode 100644 arch/riscv/boot/dts/starfive/jh7110.dtsi > delete mode 100644 drivers/clk/starfive/clk-starfive-jh7100.h > create mode 100644 drivers/clk/starfive/clk-starfive-jh7110-aon.c > create mode 100644 drivers/clk/starfive/clk-starfive-jh7110-sys.c > create mode 100644 drivers/clk/starfive/clk-starfive.c > create mode 100644 drivers/clk/starfive/clk-starfive.h > create mode 100644 drivers/pinctrl/starfive/Kconfig > create mode 100644 drivers/pinctrl/starfive/Makefile > create mode 100644 drivers/pinctrl/starfive/pinctrl-jh7110-aon.c > create mode 100644 drivers/pinctrl/starfive/pinctrl-jh7110-sys.c > rename drivers/pinctrl/{pinctrl-starfive.c => starfive/pinctrl-starfive-jh7100.c} (99%) > create mode 100644 drivers/pinctrl/starfive/pinctrl-starfive.c > create mode 100644 drivers/pinctrl/starfive/pinctrl-starfive.h > delete mode 100644 drivers/reset/reset-starfive-jh7100.c > create mode 100644 drivers/reset/reset-starfive.c > create mode 100644 include/dt-bindings/clock/starfive-jh7110-aon.h > create mode 100644 include/dt-bindings/clock/starfive-jh7110-sys.h > rename include/dt-bindings/pinctrl/{pinctrl-starfive.h => pinctrl-starfive-jh7100.h} (98%) > create mode 100644 include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h > create mode 100644 include/dt-bindings/reset/starfive-jh7110.h > > -- > 2.17.1 > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
On Fri, 30 Sept 2022 at 23:50, Stephen Boyd <sboyd@kernel.org> wrote: > > Quoting Hal Feng (2022-09-29 10:56:02) > > Clock registers address region is shared with reset controller > > on the new StarFive JH7110 SoC. Change to use regmap framework > > to allow base address sharing and preparation for JH7110 clock > > support. > > Do the reset and clk parts share actual registers, where we would need > to lock between rmw? Or is regmap just nice to have because it wraps up > the register APIs with some extra features? No, the registers aren't shared, but on the JH7100 clock and reset had separate ranges, but on the JH7110 there is just one memory range for each "CRG", clock and reset generator I presume, and the reset registers are placed after the clock registers in the same range. > > > > Signed-off-by: Hal Feng <hal.feng@linux.starfivetech.com> > > --- > [...] > > diff --git a/drivers/clk/starfive/clk-starfive-jh7100.c b/drivers/clk/starfive/clk-starfive-jh7100.c > > index 014e36f17595..410aa6e06842 100644 > > --- a/drivers/clk/starfive/clk-starfive-jh7100.c > > +++ b/drivers/clk/starfive/clk-starfive-jh7100.c > > @@ -10,6 +10,7 @@ > > #include <linux/clk-provider.h> > > #include <linux/device.h> > > #include <linux/init.h> > > +#include <linux/mfd/syscon.h> > > #include <linux/mod_devicetable.h> > > #include <linux/platform_device.h> > > > > @@ -295,11 +296,13 @@ static int __init clk_starfive_jh7100_probe(struct platform_device *pdev) > > if (!priv) > > return -ENOMEM; > > > > - spin_lock_init(&priv->rmw_lock); > > priv->dev = &pdev->dev; > > - priv->base = devm_platform_ioremap_resource(pdev, 0); > > - if (IS_ERR(priv->base)) > > - return PTR_ERR(priv->base); > > + priv->regmap = device_node_to_regmap(priv->dev->of_node); > > This is sad. Why do we need to make a syscon? Can we instead use the > auxiliary bus to make a reset device that either gets a regmap made here > in this driver or uses a void __iomem * mapped with ioremap > (priv->base)? In my original code the clock driver just registers the resets too similar to other combined clock and reset drivers. I wonder what you think about that approach: https://github.com/esmil/linux/commit/36f15e1b827b02d7f493dc5fce31060b21976e68 and https://github.com/esmil/linux/commit/4ccafadb72968480aa3dd28c227fcccae411c13b#diff-ffec81f902f810cb210012c25e8d88217ea5b4021419a4206d1fd4dd19edfce8R471 > > + if (IS_ERR(priv->regmap)) { > > + dev_err(priv->dev, "failed to get regmap (error %ld)\n", > > + PTR_ERR(priv->regmap)); > > + return PTR_ERR(priv->regmap); > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
On Tue, 4 Oct 2022 at 10:57, Linus Walleij <linus.walleij@linaro.org> wrote: > > On Fri, Sep 30, 2022 at 9:45 AM Hal Feng > <hal.feng@linux.starfivetech.com> wrote: > > > From: Jianlong Huang <jianlong.huang@starfivetech.com> > > > > Add pinctrl driver for StarFive JH7110 SoC. > > > > Signed-off-by: Jianlong Huang <jianlong.huang@starfivetech.com> > > Signed-off-by: Hal Feng <hal.feng@linux.starfivetech.com> > > Since Emil submitted the first driver I would really appreciate his review > on this version. I tried really hard to come up with a good way to share code between the JH7100 and JH7110 drivers, but so many details different on the JH7110 that it's probably best to just have a separate driver, so that part is fine. As mentioned elsewhere this driver certainly shouldn't be accepted without following the generic pinctrl and pinmux bindings. You can see the driver I wrote here: https://github.com/esmil/linux/commit/c2633315385fef1a25aa3711facef07d915820e1 It is certainly not perfect and far from complete, but at least it does follow the generic bindings. Feel free to copy all or parts of that. /Emil > > Yours, > Linus Walleij > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
On Thu, 29 Sept 2022 at 19:59, Ben Dooks <ben.dooks@sifive.com> wrote: > > On 29/09/2022 15:32, Hal Feng wrote: > > From: Emil Renner Berthing <kernel@esmil.dk> > > > > This converts the driver to use the builtin_platform_driver_probe macro > > to initialize the driver. This macro ends up calling device_initcall as > > was used previously, but also allocates a platform device which gives us > > access to much nicer APIs such as platform_ioremap_resource, > > platform_get_irq and dev_err_probe. > > This is useful, but also there are other changes currently being sorted > out by Zong Li (cc'd into this message) which have already been reviewed > and are hopefully queued for the next kernel release. > > > Signed-off-by: Emil Renner Berthing <kernel@esmil.dk> > > Signed-off-by: Hal Feng <hal.feng@linux.starfivetech.com> I'm ok with something like this being merged, but please note that if we ever want to support the JH7100 which uses registers in this peripheral to flush the cache for its non-coherent DMAs then this driver needs to be loaded before other peripherals or we will trigger the 2nd warning in arch/riscv/mm/dma-noncoherent.c. I'm not sure we can do that when it's a platform driver. See this patch for an alternative to support the JH71x0s: https://github.com/esmil/linux/commit/9c5b29da56ae29159c9572c5bb195fe3a1b535c5 /Emil > > drivers/soc/sifive/sifive_l2_cache.c | 79 ++++++++++++++-------------- > > 1 file changed, 40 insertions(+), 39 deletions(-) > > > > diff --git a/drivers/soc/sifive/sifive_l2_cache.c b/drivers/soc/sifive/sifive_l2_cache.c > > index 59640a1d0b28..010d612f7420 100644 > > --- a/drivers/soc/sifive/sifive_l2_cache.c > > +++ b/drivers/soc/sifive/sifive_l2_cache.c > > @@ -7,9 +7,9 @@ > > */ > > #include <linux/debugfs.h> > > #include <linux/interrupt.h> > > -#include <linux/of_irq.h> > > -#include <linux/of_address.h> > > -#include <linux/device.h> > > +#include <linux/io.h> > > +#include <linux/mod_devicetable.h> > > +#include <linux/platform_device.h> > > #include <asm/cacheinfo.h> > > #include <soc/sifive/sifive_l2_cache.h> > > > > @@ -96,12 +96,6 @@ static void l2_config_read(void) > > pr_info("L2CACHE: Index of the largest way enabled: %d\n", regval); > > } > > > > -static const struct of_device_id sifive_l2_ids[] = { > > - { .compatible = "sifive,fu540-c000-ccache" }, > > - { .compatible = "sifive,fu740-c000-ccache" }, > > - { /* end of table */ }, > > -}; > > - > > static ATOMIC_NOTIFIER_HEAD(l2_err_chain); > > > > int register_sifive_l2_error_notifier(struct notifier_block *nb) > > @@ -192,36 +186,29 @@ static irqreturn_t l2_int_handler(int irq, void *device) > > return IRQ_HANDLED; > > } > > > > -static int __init sifive_l2_init(void) > > +static int __init sifive_l2_probe(struct platform_device *pdev) > > { > > - struct device_node *np; > > - struct resource res; > > - int i, rc, intr_num; > > - > > - np = of_find_matching_node(NULL, sifive_l2_ids); > > - if (!np) > > - return -ENODEV; > > - > > - if (of_address_to_resource(np, 0, &res)) > > - return -ENODEV; > > - > > - l2_base = ioremap(res.start, resource_size(&res)); > > - if (!l2_base) > > - return -ENOMEM; > > - > > - intr_num = of_property_count_u32_elems(np, "interrupts"); > > - if (!intr_num) { > > - pr_err("L2CACHE: no interrupts property\n"); > > - return -ENODEV; > > - } > > - > > - for (i = 0; i < intr_num; i++) { > > - g_irq[i] = irq_of_parse_and_map(np, i); > > - rc = request_irq(g_irq[i], l2_int_handler, 0, "l2_ecc", NULL); > > - if (rc) { > > - pr_err("L2CACHE: Could not request IRQ %d\n", g_irq[i]); > > - return rc; > > - } > > + struct device *dev = &pdev->dev; > > + int nirqs; > > + int ret; > > + int i; > > + > > + l2_base = devm_platform_ioremap_resource(pdev, 0); > > + if (IS_ERR(l2_base)) > > + return PTR_ERR(l2_base); > > + > > + nirqs = platform_irq_count(pdev); > > + if (nirqs <= 0) > > + return dev_err_probe(dev, -ENODEV, "no interrupts\n"); > > I wonder if zero irqs is an actual issue here? > > > + for (i = 0; i < nirqs; i++) { > > + g_irq[i] = platform_get_irq(pdev, i); > > I wonder if we need to keep g_irq[] around now? Is it going to be useful > in the future? > > > + if (g_irq[i] < 0) > > + return g_irq[i]; > > + > > + ret = devm_request_irq(dev, g_irq[i], l2_int_handler, 0, pdev->name, NULL); > > + if (ret) > > + return dev_err_probe(dev, ret, "Could not request IRQ %d\n", g_irq[i]); > > } > > > > l2_config_read(); > > @@ -234,4 +221,18 @@ static int __init sifive_l2_init(void) > > #endif > > return 0; > > } > > -device_initcall(sifive_l2_init); > > + > > +static const struct of_device_id sifive_l2_match[] = { > > + { .compatible = "sifive,fu540-c000-ccache" }, > > + { .compatible = "sifive,fu740-c000-ccache" }, > > + { /* sentinel */ } > > +}; > > + > > +static struct platform_driver sifive_l2_driver = { > > + .driver = { > > + .name = "sifive_l2_cache", > > + .of_match_table = sifive_l2_match, > > + .suppress_bind_attrs = true, > > + }, > > +}; > > +builtin_platform_driver_probe(sifive_l2_driver, sifive_l2_probe); > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
On 05/10/2022 14:44, Emil Renner Berthing wrote: > On Thu, 29 Sept 2022 at 19:59, Ben Dooks <ben.dooks@sifive.com> wrote: >> >> On 29/09/2022 15:32, Hal Feng wrote: >>> From: Emil Renner Berthing <kernel@esmil.dk> >>> >>> This converts the driver to use the builtin_platform_driver_probe macro >>> to initialize the driver. This macro ends up calling device_initcall as >>> was used previously, but also allocates a platform device which gives us >>> access to much nicer APIs such as platform_ioremap_resource, >>> platform_get_irq and dev_err_probe. >> >> This is useful, but also there are other changes currently being sorted >> out by Zong Li (cc'd into this message) which have already been reviewed >> and are hopefully queued for the next kernel release. >> >>> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk> >>> Signed-off-by: Hal Feng <hal.feng@linux.starfivetech.com> > > I'm ok with something like this being merged, but please note that if > we ever want to support the JH7100 which uses registers in this > peripheral to flush the cache for its non-coherent DMAs then this > driver needs to be loaded before other peripherals or we will trigger > the 2nd warning in arch/riscv/mm/dma-noncoherent.c. I'm not sure we > can do that when it's a platform driver. See this patch for an > alternative to support the JH71x0s: > https://github.com/esmil/linux/commit/9c5b29da56ae29159c9572c5bb195fe3a1b535c5 > > /Emil Are you replying to your own patch that does the conversion to platform driver and then saying that it could actually cause issues? I'm all for dropping this for the moment and keeping the old early init for the ccache. >>> drivers/soc/sifive/sifive_l2_cache.c | 79 ++++++++++++++-------------- >>> 1 file changed, 40 insertions(+), 39 deletions(-) >>> >>> diff --git a/drivers/soc/sifive/sifive_l2_cache.c b/drivers/soc/sifive/sifive_l2_cache.c >>> index 59640a1d0b28..010d612f7420 100644 >>> --- a/drivers/soc/sifive/sifive_l2_cache.c >>> +++ b/drivers/soc/sifive/sifive_l2_cache.c >>> @@ -7,9 +7,9 @@ >>> */ >>> #include <linux/debugfs.h> >>> #include <linux/interrupt.h> >>> -#include <linux/of_irq.h> >>> -#include <linux/of_address.h> >>> -#include <linux/device.h> >>> +#include <linux/io.h> >>> +#include <linux/mod_devicetable.h> >>> +#include <linux/platform_device.h> >>> #include <asm/cacheinfo.h> >>> #include <soc/sifive/sifive_l2_cache.h> >>> >>> @@ -96,12 +96,6 @@ static void l2_config_read(void) >>> pr_info("L2CACHE: Index of the largest way enabled: %d\n", regval); >>> } >>> >>> -static const struct of_device_id sifive_l2_ids[] = { >>> - { .compatible = "sifive,fu540-c000-ccache" }, >>> - { .compatible = "sifive,fu740-c000-ccache" }, >>> - { /* end of table */ }, >>> -}; >>> - >>> static ATOMIC_NOTIFIER_HEAD(l2_err_chain); >>> >>> int register_sifive_l2_error_notifier(struct notifier_block *nb) >>> @@ -192,36 +186,29 @@ static irqreturn_t l2_int_handler(int irq, void *device) >>> return IRQ_HANDLED; >>> } >>> >>> -static int __init sifive_l2_init(void) >>> +static int __init sifive_l2_probe(struct platform_device *pdev) >>> { >>> - struct device_node *np; >>> - struct resource res; >>> - int i, rc, intr_num; >>> - >>> - np = of_find_matching_node(NULL, sifive_l2_ids); >>> - if (!np) >>> - return -ENODEV; >>> - >>> - if (of_address_to_resource(np, 0, &res)) >>> - return -ENODEV; >>> - >>> - l2_base = ioremap(res.start, resource_size(&res)); >>> - if (!l2_base) >>> - return -ENOMEM; >>> - >>> - intr_num = of_property_count_u32_elems(np, "interrupts"); >>> - if (!intr_num) { >>> - pr_err("L2CACHE: no interrupts property\n"); >>> - return -ENODEV; >>> - } >>> - >>> - for (i = 0; i < intr_num; i++) { >>> - g_irq[i] = irq_of_parse_and_map(np, i); >>> - rc = request_irq(g_irq[i], l2_int_handler, 0, "l2_ecc", NULL); >>> - if (rc) { >>> - pr_err("L2CACHE: Could not request IRQ %d\n", g_irq[i]); >>> - return rc; >>> - } >>> + struct device *dev = &pdev->dev; >>> + int nirqs; >>> + int ret; >>> + int i; >>> + >>> + l2_base = devm_platform_ioremap_resource(pdev, 0); >>> + if (IS_ERR(l2_base)) >>> + return PTR_ERR(l2_base); >>> + >>> + nirqs = platform_irq_count(pdev); >>> + if (nirqs <= 0) >>> + return dev_err_probe(dev, -ENODEV, "no interrupts\n"); >> >> I wonder if zero irqs is an actual issue here? >> >>> + for (i = 0; i < nirqs; i++) { >>> + g_irq[i] = platform_get_irq(pdev, i); >> >> I wonder if we need to keep g_irq[] around now? Is it going to be useful >> in the future? >> >>> + if (g_irq[i] < 0) >>> + return g_irq[i]; >>> + >>> + ret = devm_request_irq(dev, g_irq[i], l2_int_handler, 0, pdev->name, NULL); >>> + if (ret) >>> + return dev_err_probe(dev, ret, "Could not request IRQ %d\n", g_irq[i]); >>> } >>> >>> l2_config_read(); >>> @@ -234,4 +221,18 @@ static int __init sifive_l2_init(void) >>> #endif >>> return 0; >>> } >>> -device_initcall(sifive_l2_init); >>> + >>> +static const struct of_device_id sifive_l2_match[] = { >>> + { .compatible = "sifive,fu540-c000-ccache" }, >>> + { .compatible = "sifive,fu740-c000-ccache" }, >>> + { /* sentinel */ } >>> +}; >>> + >>> +static struct platform_driver sifive_l2_driver = { >>> + .driver = { >>> + .name = "sifive_l2_cache", >>> + .of_match_table = sifive_l2_match, >>> + .suppress_bind_attrs = true, >>> + }, >>> +}; >>> +builtin_platform_driver_probe(sifive_l2_driver, sifive_l2_probe); >> >> >> _______________________________________________ >> linux-riscv mailing list >> linux-riscv@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-riscv > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv >
On Wed, 5 Oct 2022 at 15:48, Ben Dooks <ben.dooks@codethink.co.uk> wrote: > > On 05/10/2022 14:44, Emil Renner Berthing wrote: > > On Thu, 29 Sept 2022 at 19:59, Ben Dooks <ben.dooks@sifive.com> wrote: > >> > >> On 29/09/2022 15:32, Hal Feng wrote: > >>> From: Emil Renner Berthing <kernel@esmil.dk> > >>> > >>> This converts the driver to use the builtin_platform_driver_probe macro > >>> to initialize the driver. This macro ends up calling device_initcall as > >>> was used previously, but also allocates a platform device which gives us > >>> access to much nicer APIs such as platform_ioremap_resource, > >>> platform_get_irq and dev_err_probe. > >> > >> This is useful, but also there are other changes currently being sorted > >> out by Zong Li (cc'd into this message) which have already been reviewed > >> and are hopefully queued for the next kernel release. > >> > >>> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk> > >>> Signed-off-by: Hal Feng <hal.feng@linux.starfivetech.com> > > > > I'm ok with something like this being merged, but please note that if > > we ever want to support the JH7100 which uses registers in this > > peripheral to flush the cache for its non-coherent DMAs then this > > driver needs to be loaded before other peripherals or we will trigger > > the 2nd warning in arch/riscv/mm/dma-noncoherent.c. I'm not sure we > > can do that when it's a platform driver. See this patch for an > > alternative to support the JH71x0s: > > https://github.com/esmil/linux/commit/9c5b29da56ae29159c9572c5bb195fe3a1b535c5 > > > > /Emil > > Are you replying to your own patch that does the conversion to > platform driver and then saying that it could actually cause > issues? Yes, I can see it seems odd, but this patch lived for a while in the kernel repo for the JH7100 until I rebased on 6.0-rc1 and realized the above. Hal Feng must have based his patches on a version of the code before that when preparing this series. > I'm all for dropping this for the moment and keeping the old > early init for the ccache. Cool. /Emil > >>> drivers/soc/sifive/sifive_l2_cache.c | 79 ++++++++++++++-------------- > >>> 1 file changed, 40 insertions(+), 39 deletions(-) > >>> > >>> diff --git a/drivers/soc/sifive/sifive_l2_cache.c b/drivers/soc/sifive/sifive_l2_cache.c > >>> index 59640a1d0b28..010d612f7420 100644 > >>> --- a/drivers/soc/sifive/sifive_l2_cache.c > >>> +++ b/drivers/soc/sifive/sifive_l2_cache.c > >>> @@ -7,9 +7,9 @@ > >>> */ > >>> #include <linux/debugfs.h> > >>> #include <linux/interrupt.h> > >>> -#include <linux/of_irq.h> > >>> -#include <linux/of_address.h> > >>> -#include <linux/device.h> > >>> +#include <linux/io.h> > >>> +#include <linux/mod_devicetable.h> > >>> +#include <linux/platform_device.h> > >>> #include <asm/cacheinfo.h> > >>> #include <soc/sifive/sifive_l2_cache.h> > >>> > >>> @@ -96,12 +96,6 @@ static void l2_config_read(void) > >>> pr_info("L2CACHE: Index of the largest way enabled: %d\n", regval); > >>> } > >>> > >>> -static const struct of_device_id sifive_l2_ids[] = { > >>> - { .compatible = "sifive,fu540-c000-ccache" }, > >>> - { .compatible = "sifive,fu740-c000-ccache" }, > >>> - { /* end of table */ }, > >>> -}; > >>> - > >>> static ATOMIC_NOTIFIER_HEAD(l2_err_chain); > >>> > >>> int register_sifive_l2_error_notifier(struct notifier_block *nb) > >>> @@ -192,36 +186,29 @@ static irqreturn_t l2_int_handler(int irq, void *device) > >>> return IRQ_HANDLED; > >>> } > >>> > >>> -static int __init sifive_l2_init(void) > >>> +static int __init sifive_l2_probe(struct platform_device *pdev) > >>> { > >>> - struct device_node *np; > >>> - struct resource res; > >>> - int i, rc, intr_num; > >>> - > >>> - np = of_find_matching_node(NULL, sifive_l2_ids); > >>> - if (!np) > >>> - return -ENODEV; > >>> - > >>> - if (of_address_to_resource(np, 0, &res)) > >>> - return -ENODEV; > >>> - > >>> - l2_base = ioremap(res.start, resource_size(&res)); > >>> - if (!l2_base) > >>> - return -ENOMEM; > >>> - > >>> - intr_num = of_property_count_u32_elems(np, "interrupts"); > >>> - if (!intr_num) { > >>> - pr_err("L2CACHE: no interrupts property\n"); > >>> - return -ENODEV; > >>> - } > >>> - > >>> - for (i = 0; i < intr_num; i++) { > >>> - g_irq[i] = irq_of_parse_and_map(np, i); > >>> - rc = request_irq(g_irq[i], l2_int_handler, 0, "l2_ecc", NULL); > >>> - if (rc) { > >>> - pr_err("L2CACHE: Could not request IRQ %d\n", g_irq[i]); > >>> - return rc; > >>> - } > >>> + struct device *dev = &pdev->dev; > >>> + int nirqs; > >>> + int ret; > >>> + int i; > >>> + > >>> + l2_base = devm_platform_ioremap_resource(pdev, 0); > >>> + if (IS_ERR(l2_base)) > >>> + return PTR_ERR(l2_base); > >>> + > >>> + nirqs = platform_irq_count(pdev); > >>> + if (nirqs <= 0) > >>> + return dev_err_probe(dev, -ENODEV, "no interrupts\n"); > >> > >> I wonder if zero irqs is an actual issue here? > >> > >>> + for (i = 0; i < nirqs; i++) { > >>> + g_irq[i] = platform_get_irq(pdev, i); > >> > >> I wonder if we need to keep g_irq[] around now? Is it going to be useful > >> in the future? > >> > >>> + if (g_irq[i] < 0) > >>> + return g_irq[i]; > >>> + > >>> + ret = devm_request_irq(dev, g_irq[i], l2_int_handler, 0, pdev->name, NULL); > >>> + if (ret) > >>> + return dev_err_probe(dev, ret, "Could not request IRQ %d\n", g_irq[i]); > >>> } > >>> > >>> l2_config_read(); > >>> @@ -234,4 +221,18 @@ static int __init sifive_l2_init(void) > >>> #endif > >>> return 0; > >>> } > >>> -device_initcall(sifive_l2_init); > >>> + > >>> +static const struct of_device_id sifive_l2_match[] = { > >>> + { .compatible = "sifive,fu540-c000-ccache" }, > >>> + { .compatible = "sifive,fu740-c000-ccache" }, > >>> + { /* sentinel */ } > >>> +}; > >>> + > >>> +static struct platform_driver sifive_l2_driver = { > >>> + .driver = { > >>> + .name = "sifive_l2_cache", > >>> + .of_match_table = sifive_l2_match, > >>> + .suppress_bind_attrs = true, > >>> + }, > >>> +}; > >>> +builtin_platform_driver_probe(sifive_l2_driver, sifive_l2_probe); > >> > >> > >> _______________________________________________ > >> linux-riscv mailing list > >> linux-riscv@lists.infradead.org > >> http://lists.infradead.org/mailman/listinfo/linux-riscv > > > > _______________________________________________ > > linux-riscv mailing list > > linux-riscv@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-riscv > > > > -- > Ben Dooks http://www.codethink.co.uk/ > Senior Engineer Codethink - Providing Genius > > https://www.codethink.co.uk/privacy.html >
On Wed, Oct 05, 2022 at 03:55:17PM +0200, Emil Renner Berthing wrote: > On Wed, 5 Oct 2022 at 15:48, Ben Dooks <ben.dooks@codethink.co.uk> wrote: > > > > On 05/10/2022 14:44, Emil Renner Berthing wrote: > > > On Thu, 29 Sept 2022 at 19:59, Ben Dooks <ben.dooks@sifive.com> wrote: > > >> > > >> On 29/09/2022 15:32, Hal Feng wrote: > > >>> From: Emil Renner Berthing <kernel@esmil.dk> > > >>> > > >>> This converts the driver to use the builtin_platform_driver_probe macro > > >>> to initialize the driver. This macro ends up calling device_initcall as > > >>> was used previously, but also allocates a platform device which gives us > > >>> access to much nicer APIs such as platform_ioremap_resource, > > >>> platform_get_irq and dev_err_probe. > > >> > > >> This is useful, but also there are other changes currently being sorted > > >> out by Zong Li (cc'd into this message) which have already been reviewed > > >> and are hopefully queued for the next kernel release. > > >> > > >>> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk> > > >>> Signed-off-by: Hal Feng <hal.feng@linux.starfivetech.com> > > > > > > I'm ok with something like this being merged, but please note that if > > > we ever want to support the JH7100 which uses registers in this > > > peripheral to flush the cache for its non-coherent DMAs then this > > > driver needs to be loaded before other peripherals or we will trigger > > > the 2nd warning in arch/riscv/mm/dma-noncoherent.c. I'm not sure we > > > can do that when it's a platform driver. See this patch for an > > > alternative to support the JH71x0s: > > > https://github.com/esmil/linux/commit/9c5b29da56ae29159c9572c5bb195fe3a1b535c5 > > > > > > /Emil > > > > Are you replying to your own patch that does the conversion to > > platform driver and then saying that it could actually cause > > issues? > > Yes, I can see it seems odd, but this patch lived for a while in the > kernel repo for the JH7100 until I rebased on 6.0-rc1 and realized the > above. > Hal Feng must have based his patches on a version of the code before > that when preparing this series. > > > I'm all for dropping this for the moment and keeping the old > > early init for the ccache. > > Cool. FWIW, if converting to a platform driver will inhibit using the driver for doing non-coherent stuff I would like to NAK the patch :) > > /Emil > > > >>> drivers/soc/sifive/sifive_l2_cache.c | 79 ++++++++++++++-------------- > > >>> 1 file changed, 40 insertions(+), 39 deletions(-) > > >>> > > >>> diff --git a/drivers/soc/sifive/sifive_l2_cache.c b/drivers/soc/sifive/sifive_l2_cache.c > > >>> index 59640a1d0b28..010d612f7420 100644 > > >>> --- a/drivers/soc/sifive/sifive_l2_cache.c > > >>> +++ b/drivers/soc/sifive/sifive_l2_cache.c > > >>> @@ -7,9 +7,9 @@ > > >>> */ > > >>> #include <linux/debugfs.h> > > >>> #include <linux/interrupt.h> > > >>> -#include <linux/of_irq.h> > > >>> -#include <linux/of_address.h> > > >>> -#include <linux/device.h> > > >>> +#include <linux/io.h> > > >>> +#include <linux/mod_devicetable.h> > > >>> +#include <linux/platform_device.h> > > >>> #include <asm/cacheinfo.h> > > >>> #include <soc/sifive/sifive_l2_cache.h> > > >>> > > >>> @@ -96,12 +96,6 @@ static void l2_config_read(void) > > >>> pr_info("L2CACHE: Index of the largest way enabled: %d\n", regval); > > >>> } > > >>> > > >>> -static const struct of_device_id sifive_l2_ids[] = { > > >>> - { .compatible = "sifive,fu540-c000-ccache" }, > > >>> - { .compatible = "sifive,fu740-c000-ccache" }, > > >>> - { /* end of table */ }, > > >>> -}; > > >>> - > > >>> static ATOMIC_NOTIFIER_HEAD(l2_err_chain); > > >>> > > >>> int register_sifive_l2_error_notifier(struct notifier_block *nb) > > >>> @@ -192,36 +186,29 @@ static irqreturn_t l2_int_handler(int irq, void *device) > > >>> return IRQ_HANDLED; > > >>> } > > >>> > > >>> -static int __init sifive_l2_init(void) > > >>> +static int __init sifive_l2_probe(struct platform_device *pdev) > > >>> { > > >>> - struct device_node *np; > > >>> - struct resource res; > > >>> - int i, rc, intr_num; > > >>> - > > >>> - np = of_find_matching_node(NULL, sifive_l2_ids); > > >>> - if (!np) > > >>> - return -ENODEV; > > >>> - > > >>> - if (of_address_to_resource(np, 0, &res)) > > >>> - return -ENODEV; > > >>> - > > >>> - l2_base = ioremap(res.start, resource_size(&res)); > > >>> - if (!l2_base) > > >>> - return -ENOMEM; > > >>> - > > >>> - intr_num = of_property_count_u32_elems(np, "interrupts"); > > >>> - if (!intr_num) { > > >>> - pr_err("L2CACHE: no interrupts property\n"); > > >>> - return -ENODEV; > > >>> - } > > >>> - > > >>> - for (i = 0; i < intr_num; i++) { > > >>> - g_irq[i] = irq_of_parse_and_map(np, i); > > >>> - rc = request_irq(g_irq[i], l2_int_handler, 0, "l2_ecc", NULL); > > >>> - if (rc) { > > >>> - pr_err("L2CACHE: Could not request IRQ %d\n", g_irq[i]); > > >>> - return rc; > > >>> - } > > >>> + struct device *dev = &pdev->dev; > > >>> + int nirqs; > > >>> + int ret; > > >>> + int i; > > >>> + > > >>> + l2_base = devm_platform_ioremap_resource(pdev, 0); > > >>> + if (IS_ERR(l2_base)) > > >>> + return PTR_ERR(l2_base); > > >>> + > > >>> + nirqs = platform_irq_count(pdev); > > >>> + if (nirqs <= 0) > > >>> + return dev_err_probe(dev, -ENODEV, "no interrupts\n"); > > >> > > >> I wonder if zero irqs is an actual issue here? > > >> > > >>> + for (i = 0; i < nirqs; i++) { > > >>> + g_irq[i] = platform_get_irq(pdev, i); > > >> > > >> I wonder if we need to keep g_irq[] around now? Is it going to be useful > > >> in the future? > > >> > > >>> + if (g_irq[i] < 0) > > >>> + return g_irq[i]; > > >>> + > > >>> + ret = devm_request_irq(dev, g_irq[i], l2_int_handler, 0, pdev->name, NULL); > > >>> + if (ret) > > >>> + return dev_err_probe(dev, ret, "Could not request IRQ %d\n", g_irq[i]); > > >>> } > > >>> > > >>> l2_config_read(); > > >>> @@ -234,4 +221,18 @@ static int __init sifive_l2_init(void) > > >>> #endif > > >>> return 0; > > >>> } > > >>> -device_initcall(sifive_l2_init); > > >>> + > > >>> +static const struct of_device_id sifive_l2_match[] = { > > >>> + { .compatible = "sifive,fu540-c000-ccache" }, > > >>> + { .compatible = "sifive,fu740-c000-ccache" }, > > >>> + { /* sentinel */ } > > >>> +}; > > >>> + > > >>> +static struct platform_driver sifive_l2_driver = { > > >>> + .driver = { > > >>> + .name = "sifive_l2_cache", > > >>> + .of_match_table = sifive_l2_match, > > >>> + .suppress_bind_attrs = true, > > >>> + }, > > >>> +}; > > >>> +builtin_platform_driver_probe(sifive_l2_driver, sifive_l2_probe); > > >> > > >> > > >> _______________________________________________ > > >> linux-riscv mailing list > > >> linux-riscv@lists.infradead.org > > >> http://lists.infradead.org/mailman/listinfo/linux-riscv > > > > > > _______________________________________________ > > > linux-riscv mailing list > > > linux-riscv@lists.infradead.org > > > http://lists.infradead.org/mailman/listinfo/linux-riscv > > > > > > > -- > > Ben Dooks http://www.codethink.co.uk/ > > Senior Engineer Codethink - Providing Genius > > > > https://www.codethink.co.uk/privacy.html > >
On Wed, 5 Oct 2022 15:05:45 +0200, Emil Renner Berthing wrote: > On Thu, 29 Sept 2022 at 16:34, Hal Feng <hal.feng@linux.starfivetech.com> wrote: > > This series adds basic support for the StarFive JH7110 RISC-V SoC to > > boot up and get a serial console. This series includes basic clock, > > reset, pinctrl and uart drivers, which are necessary for booting. > > It's should be noted that the reset and clock driver codes of JH7110 > > are partly common with those of JH7100, so the common codes are > > factored out and can be reused by drivers of JH7110 and other more > > SoCs from StarFive. > > > > The JH7110 is the upgraded version of JH7100 and also the first official > > released version of JH71XX series SoCs from StarFive Technology Ltd. > > The VisionFive 2 boards equipped with JH7110 SoCs are launched > > recently [1]. More information and support can visit RVspace wiki [2]. > > > > This series is also available at > > https://github.com/hal-feng/linux/commits/visionfive2-minimal > > > > [1] https://www.cnx-software.com/2022/08/23/starfive-visionfive-2-quad-core-risc-v-sbc-linux/ > > [2] https://wiki.rvspace.org/ > > Hi Hal, > > Firstly thanks for working on this! And sorry about the late reply. On > the next version could you please cc > emil.renner.berthing@canonical.com since it seems to handle the > mailing list a bit better. OK, I will cc to your new email instead on v2. Best Regards, Hal > I see you've changed the clock/reset and pinctrl quite a bit, so I'll > comment on that separatel
On Wed, 5 Oct 2022 15:05:03 +0100, Conor Dooley wrote: > On Wed, Oct 05, 2022 at 03:55:17PM +0200, Emil Renner Berthing wrote: > > On Wed, 5 Oct 2022 at 15:48, Ben Dooks <ben.dooks@codethink.co.uk> wrote: > > > > > > On 05/10/2022 14:44, Emil Renner Berthing wrote: > > > > On Thu, 29 Sept 2022 at 19:59, Ben Dooks <ben.dooks@sifive.com> wrote: > > > >> > > > >> On 29/09/2022 15:32, Hal Feng wrote: > > > >>> From: Emil Renner Berthing <kernel@esmil.dk> > > > >>> > > > >>> This converts the driver to use the builtin_platform_driver_probe macro > > > >>> to initialize the driver. This macro ends up calling device_initcall as > > > >>> was used previously, but also allocates a platform device which gives us > > > >>> access to much nicer APIs such as platform_ioremap_resource, > > > >>> platform_get_irq and dev_err_probe. > > > >> > > > >> This is useful, but also there are other changes currently being sorted > > > >> out by Zong Li (cc'd into this message) which have already been reviewed > > > >> and are hopefully queued for the next kernel release. > > > >> > > > >>> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk> > > > >>> Signed-off-by: Hal Feng <hal.feng@linux.starfivetech.com> > > > > > > > > I'm ok with something like this being merged, but please note that if > > > > we ever want to support the JH7100 which uses registers in this > > > > peripheral to flush the cache for its non-coherent DMAs then this > > > > driver needs to be loaded before other peripherals or we will trigger > > > > the 2nd warning in arch/riscv/mm/dma-noncoherent.c. I'm not sure we > > > > can do that when it's a platform driver. See this patch for an > > > > alternative to support the JH71x0s: > > > > https://github.com/esmil/linux/commit/9c5b29da56ae29159c9572c5bb195fe3a1b535c5 > > > > > > > > /Emil > > > > > > Are you replying to your own patch that does the conversion to > > > platform driver and then saying that it could actually cause > > > issues? > > > > Yes, I can see it seems odd, but this patch lived for a while in the > > kernel repo for the JH7100 until I rebased on 6.0-rc1 and realized the > > above. > > Hal Feng must have based his patches on a version of the code before > > that when preparing this series. > > > > > I'm all for dropping this for the moment and keeping the old > > > early init for the ccache. > > > > Cool. > > FWIW, if converting to a platform driver will inhibit using the driver > for doing non-coherent stuff I would like to NAK the patch :) > Yeah, I agree, and this patch will be dropped on the next version.
On Fri, 30 Sep 2022 13:37:28 +0100, Conor Dooley wrote: > On Fri, Sep 30, 2022 at 08:23:18PM +0800, Hal Feng wrote: > > Add Kconfig options to select the specified StarFive SoC. Select > > necessary Kconfig options required by the specified SoC for booting. > > > > Signed-off-by: Hal Feng <hal.feng@linux.starfivetech.com> > > --- > > arch/riscv/Kconfig.socs | 27 ++++++++++++++++++++++++++- > > arch/riscv/boot/dts/starfive/Makefile | 4 ++-- > > drivers/clk/starfive/Kconfig | 14 ++++++-------- > > drivers/pinctrl/starfive/Kconfig | 6 ++---- > > drivers/reset/Kconfig | 1 - > > Firstly, you cannot change all of these files in one commit, sorry. > > > 5 files changed, 36 insertions(+), 16 deletions(-) > > > > diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs > > index 10f68a4359f9..321c448e7b6f 100644 > > --- a/arch/riscv/Kconfig.socs > > +++ b/arch/riscv/Kconfig.socs > > @@ -22,10 +22,35 @@ config SOC_STARFIVE > > bool "StarFive SoCs" > > select PINCTRL > > select RESET_CONTROLLER > > + select RESET_STARFIVE > > Secondly, we are trying to get rid of selects in arch/riscv at the > moment, not add them. use "default SOC_STARFIVE" in > drivers/reset/kconfig instead please. > > > + help > > + This enables support for StarFive SoC platform hardware. > > + > > +if SOC_STARFIVE > > I don't think we want to have per soc selection menus in arch code, > I think this should move to drivers/soc (a la Renesas) if you want to > have a per soc selection menu or else just do "default SOC_STARFIVE" > for both clock and pinctrl drivers in the clk and pinctrl Kconfig > entries. Thanks for your helpful comments. I will drop this patch. Best regards, Hal
Quoting Emil Renner Berthing (2022-10-05 06:14:44) > > > @@ -295,11 +296,13 @@ static int __init clk_starfive_jh7100_probe(struct platform_device *pdev) > > > if (!priv) > > > return -ENOMEM; > > > > > > - spin_lock_init(&priv->rmw_lock); > > > priv->dev = &pdev->dev; > > > - priv->base = devm_platform_ioremap_resource(pdev, 0); > > > - if (IS_ERR(priv->base)) > > > - return PTR_ERR(priv->base); > > > + priv->regmap = device_node_to_regmap(priv->dev->of_node); > > > > This is sad. Why do we need to make a syscon? Can we instead use the > > auxiliary bus to make a reset device that either gets a regmap made here > > in this driver or uses a void __iomem * mapped with ioremap > > (priv->base)? > > In my original code the clock driver just registers the resets too > similar to other combined clock and reset drivers. I wonder what you > think about that approach: > https://github.com/esmil/linux/commit/36f15e1b827b02d7f493dc5fce31060b21976e68 > and > https://github.com/esmil/linux/commit/4ccafadb72968480aa3dd28c227fcccae411c13b#diff-ffec81f902f810cb210012c25e8d88217ea5b4021419a4206d1fd4dd19edfce8R471 I think we should use auxiliary bus and split the driver logically into a reset driver in drivers/reset and a clk driver in drivers/clk. That way the appropriate maintainers can review the code. There is only one platform device with a single reg property and node in DT, but there are two drivers.
On Wed, 5 Oct 2022 15:31:59 +0200, Emil Renner Berthing wrote: > On Tue, 4 Oct 2022 at 10:57, Linus Walleij <linus.walleij@linaro.org> wrote: > > > > On Fri, Sep 30, 2022 at 9:45 AM Hal Feng > > <hal.feng@linux.starfivetech.com> wrote: > > > > > From: Jianlong Huang <jianlong.huang@starfivetech.com> > > > > > > Add pinctrl driver for StarFive JH7110 SoC. > > > > > > Signed-off-by: Jianlong Huang <jianlong.huang@starfivetech.com> > > > Signed-off-by: Hal Feng <hal.feng@linux.starfivetech.com> > > > > Since Emil submitted the first driver I would really appreciate his review > > on this version. > > I tried really hard to come up with a good way to share code between > the JH7100 and JH7110 drivers, but so many details different on the > JH7110 that it's probably best to just have a separate driver, so that > part is fine. > > As mentioned elsewhere this driver certainly shouldn't be accepted > without following the generic pinctrl and pinmux bindings. You can see > the driver I wrote here: > https://github.com/esmil/linux/commit/c2633315385fef1a25aa3711facef07d915820e1 > > It is certainly not perfect and far from complete, but at least it > does follow the generic bindings. Feel free to copy all or parts of > that. Thanks for your great contribution! We will rewrite the pinctrl driver and bindings so make them follow the generic pinctrl framework. Best regards, Hal
On Fri, 30 Sep 2022 22:41:23 +0100, Conor Dooley wrote: > On Fri, Sep 30, 2022 at 09:54:14PM +0100, Ben Dooks wrote: > > On 30/09/2022 10:06, Hal Feng wrote: > > > Add CONFIG_SERIAL_8250_DW=y, which is a necessary option for > > > StarFive JH7110 and JH7100 SoCs to boot with serial ports. > > > > > > Signed-off-by: Hal Feng <hal.feng@linux.starfivetech.com> > > > > That might be useful for other users at some point an I don't > > think it adds much code. > > Honestly I think this should be applied for 6.1, for parity with the > other SoCs that have their serial console enabled by default. > > Reviewed-by: Conor Dooley <conor.dooley@microchip.com> Could this patch be pulled out and applied for v6.1? So the JH7100 and the coming JH7110 can enable serial console by default when booting. Thanks. Best regards, Hal
On Sat, 1 Oct 2022 11:52:00 +0100, Conor Dooley wrote: > On Fri, Sep 30, 2022 at 03:49:14PM +0800, Hal Feng wrote: > > From: Emil Renner Berthing <kernel@esmil.dk> > > > > Add initial device tree for the JH7110 RISC-V SoC by > > StarFive Technology Ltd. > > > > Signed-off-by: Emil Renner Berthing <kernel@esmil.dk> > > Signed-off-by: Jianlong Huang <jianlong.huang@starfivetech.com> > > Signed-off-by: Hal Feng <hal.feng@linux.starfivetech.com> > > There's little point reviewing this dt since there's a load of issues > that you can trivially find by running dtbs_check/dt_binding_check, but > this SoB change is wrong - if Emil wrote the patch, then Jianlong's SoB > is either redundant or should be accompanied by a Co-developed-by tag. > > Ditto for patch 28/30 "RISC-V: Add StarFive JH7110 VisionFive2 board > device tree". Will add Co-developed-by tag for Jianlong. Thanks. > > > --- > > arch/riscv/boot/dts/starfive/jh7110.dtsi | 449 +++++++++++++++++++++++ > > 1 file changed, 449 insertions(+) > > create mode 100644 arch/riscv/boot/dts/starfive/jh7110.dtsi > > > > diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi > > new file mode 100644 > > index 000000000000..46f418d4198a > > --- /dev/null > > +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi > > > + > > + osc: osc { > > + compatible = "fixed-clock"; > > + #clock-cells = <0>; > > + }; > > + > > + clk_rtc: clk_rtc { > > + compatible = "fixed-clock"; > > + #clock-cells = <0>; > > + }; > > + > > + gmac0_rmii_refin: gmac0_rmii_refin { > > + compatible = "fixed-clock"; > > + #clock-cells = <0>; > > + clock-frequency = <50000000>; > > I assume, given osc has it's frequency set in the board dts, that these > are all oscillators on the SoC? These are all on the board. Should move all "clock-frequency" to the board dts. I will recheck and modify this patch. Best regards, Hal
On Wed, 12 Oct 2022 16:05:23 -0700, Stephen Boyd wrote: > Quoting Emil Renner Berthing (2022-10-05 06:14:44) > > > > @@ -295,11 +296,13 @@ static int __init clk_starfive_jh7100_probe(struct platform_device *pdev) > > > > if (!priv) > > > > return -ENOMEM; > > > > > > > > - spin_lock_init(&priv->rmw_lock); > > > > priv->dev = &pdev->dev; > > > > - priv->base = devm_platform_ioremap_resource(pdev, 0); > > > > - if (IS_ERR(priv->base)) > > > > - return PTR_ERR(priv->base); > > > > + priv->regmap = device_node_to_regmap(priv->dev->of_node); > > > > > > This is sad. Why do we need to make a syscon? Can we instead use the > > > auxiliary bus to make a reset device that either gets a regmap made here > > > in this driver or uses a void __iomem * mapped with ioremap > > > (priv->base)? > > > > In my original code the clock driver just registers the resets too > > similar to other combined clock and reset drivers. I wonder what you > > think about that approach: > > https://github.com/esmil/linux/commit/36f15e1b827b02d7f493dc5fce31060b21976e68 > > and > > https://github.com/esmil/linux/commit/4ccafadb72968480aa3dd28c227fcccae411c13b#diff-ffec81f902f810cb210012c25e8d88217ea5b4021419a4206d1fd4dd19edfce8R471 > > I think we should use auxiliary bus and split the driver logically into > a reset driver in drivers/reset and a clk driver in drivers/clk. That > way the appropriate maintainers can review the code. There is only one > platform device with a single reg property and node in DT, but there are > two drivers. Yes, I agree that the reset driver and the clock driver should be split. However, I think using auxiliary bus is a little bit complicated in this case, because the reset is not a part of functionality of the clock in JH7110. They just share a common register base address. I think it is better to use ioremap for the same address, and the dt will be like syscrg_clk: clock-controller@13020000 { compatible = "starfive,jh7110-clkgen-sys"; reg = <0x0 0x13020000 0x0 0x10000>; ... }; syscrg_rst: reset-controller@13020000 { compatible = "starfive,jh7110-reset-sys"; reg = <0x0 0x13020000 0x0 0x10000>; ... }; What do you think of this approach? I would appreciate your suggestions. Best regards, Hal
On 23 October 2022 05:11:41 IST, Hal Feng <hal.feng@linux.starfivetech.com> wrote: >On Wed, 12 Oct 2022 16:05:23 -0700, Stephen Boyd wrote: >> Quoting Emil Renner Berthing (2022-10-05 06:14:44) >> > > > @@ -295,11 +296,13 @@ static int __init clk_starfive_jh7100_probe(struct platform_device *pdev) >> > > > if (!priv) >> > > > return -ENOMEM; >> > > > >> > > > - spin_lock_init(&priv->rmw_lock); >> > > > priv->dev = &pdev->dev; >> > > > - priv->base = devm_platform_ioremap_resource(pdev, 0); >> > > > - if (IS_ERR(priv->base)) >> > > > - return PTR_ERR(priv->base); >> > > > + priv->regmap = device_node_to_regmap(priv->dev->of_node); >> > > >> > > This is sad. Why do we need to make a syscon? Can we instead use the >> > > auxiliary bus to make a reset device that either gets a regmap made here >> > > in this driver or uses a void __iomem * mapped with ioremap >> > > (priv->base)? >> > >> > In my original code the clock driver just registers the resets too >> > similar to other combined clock and reset drivers. I wonder what you >> > think about that approach: >> > https://github.com/esmil/linux/commit/36f15e1b827b02d7f493dc5fce31060b21976e68 >> > and >> > https://github.com/esmil/linux/commit/4ccafadb72968480aa3dd28c227fcccae411c13b#diff-ffec81f902f810cb210012c25e8d88217ea5b4021419a4206d1fd4dd19edfce8R471 >> >> I think we should use auxiliary bus and split the driver logically into >> a reset driver in drivers/reset and a clk driver in drivers/clk. That >> way the appropriate maintainers can review the code. There is only one >> platform device with a single reg property and node in DT, but there are >> two drivers. > >Yes, I agree that the reset driver and the clock driver should be split. >However, I think using auxiliary bus is a little bit complicated in this >case, because the reset is not a part of functionality of the clock in >JH7110. They just share a common register base address. I think it is >better to use ioremap for the same address, and the dt will be like > >syscrg_clk: clock-controller@13020000 { > compatible = "starfive,jh7110-clkgen-sys"; > reg = <0x0 0x13020000 0x0 0x10000>; > ... >}; >syscrg_rst: reset-controller@13020000 { > compatible = "starfive,jh7110-reset-sys"; > reg = <0x0 0x13020000 0x0 0x10000>; > ... >}; > >What do you think of this approach? I would appreciate your suggestions. No, the dtb checks will all start warning for this. Aux bus is not that difficult, you can likely copy much of what I did recently in clk-mpfs.c > >Best regards, >Hal
Quoting Hal Feng (2022-10-22 21:11:41) > On Wed, 12 Oct 2022 16:05:23 -0700, Stephen Boyd wrote: > > I think we should use auxiliary bus and split the driver logically into > > a reset driver in drivers/reset and a clk driver in drivers/clk. That > > way the appropriate maintainers can review the code. There is only one > > platform device with a single reg property and node in DT, but there are > > two drivers. > > Yes, I agree that the reset driver and the clock driver should be split. > However, I think using auxiliary bus is a little bit complicated in this > case, because the reset is not a part of functionality of the clock in > JH7110. They just share a common register base address. That is why auxiliary bus exists. > I think it is > better to use ioremap for the same address, and the dt will be like > > syscrg_clk: clock-controller@13020000 { > compatible = "starfive,jh7110-clkgen-sys"; > reg = <0x0 0x13020000 0x0 0x10000>; > ... > }; > syscrg_rst: reset-controller@13020000 { > compatible = "starfive,jh7110-reset-sys"; > reg = <0x0 0x13020000 0x0 0x10000>; > ... > }; > > What do you think of this approach? I would appreciate your suggestions. > We shouldn't have two different nodes with the same reg property. Please ioremap in whatever driver probes and creates the auxiliary device(s) and then pass the void __iomem * to it.
On Wed, 26 Oct 2022 18:26:03 -0700, Stephen Boyd wrote: > Quoting Hal Feng (2022-10-22 21:11:41) > > On Wed, 12 Oct 2022 16:05:23 -0700, Stephen Boyd wrote: > > > I think we should use auxiliary bus and split the driver logically into > > > a reset driver in drivers/reset and a clk driver in drivers/clk. That > > > way the appropriate maintainers can review the code. There is only one > > > platform device with a single reg property and node in DT, but there are > > > two drivers. > > > > Yes, I agree that the reset driver and the clock driver should be split. > > However, I think using auxiliary bus is a little bit complicated in this > > case, because the reset is not a part of functionality of the clock in > > JH7110. They just share a common register base address. > > That is why auxiliary bus exists. > > > I think it is > > better to use ioremap for the same address, and the dt will be like > > > > syscrg_clk: clock-controller@13020000 { > > compatible = "starfive,jh7110-clkgen-sys"; > > reg = <0x0 0x13020000 0x0 0x10000>; > > ... > > }; > > syscrg_rst: reset-controller@13020000 { > > compatible = "starfive,jh7110-reset-sys"; > > reg = <0x0 0x13020000 0x0 0x10000>; > > ... > > }; > > > > What do you think of this approach? I would appreciate your suggestions. > > > > We shouldn't have two different nodes with the same reg property. Please > ioremap in whatever driver probes and creates the auxiliary device(s) > and then pass the void __iomem * to it. Okay, I will use auxiliary bus for clock and reset driver on the next version. Best regards, Hal
On Sun, 23 Oct 2022 11:25:26 +0100, Conor Dooley wrote: > On 23 October 2022 05:11:41 IST, Hal Feng <hal.feng@linux.starfivetech.com> wrote: > >On Wed, 12 Oct 2022 16:05:23 -0700, Stephen Boyd wrote: > >> Quoting Emil Renner Berthing (2022-10-05 06:14:44) > >> > > > @@ -295,11 +296,13 @@ static int __init clk_starfive_jh7100_probe(struct platform_device *pdev) > >> > > > if (!priv) > >> > > > return -ENOMEM; > >> > > > > >> > > > - spin_lock_init(&priv->rmw_lock); > >> > > > priv->dev = &pdev->dev; > >> > > > - priv->base = devm_platform_ioremap_resource(pdev, 0); > >> > > > - if (IS_ERR(priv->base)) > >> > > > - return PTR_ERR(priv->base); > >> > > > + priv->regmap = device_node_to_regmap(priv->dev->of_node); > >> > > > >> > > This is sad. Why do we need to make a syscon? Can we instead use the > >> > > auxiliary bus to make a reset device that either gets a regmap made here > >> > > in this driver or uses a void __iomem * mapped with ioremap > >> > > (priv->base)? > >> > > >> > In my original code the clock driver just registers the resets too > >> > similar to other combined clock and reset drivers. I wonder what you > >> > think about that approach: > >> > https://github.com/esmil/linux/commit/36f15e1b827b02d7f493dc5fce31060b21976e68 > >> > and > >> > https://github.com/esmil/linux/commit/4ccafadb72968480aa3dd28c227fcccae411c13b#diff-ffec81f902f810cb210012c25e8d88217ea5b4021419a4206d1fd4dd19edfce8R471 > >> > >> I think we should use auxiliary bus and split the driver logically into > >> a reset driver in drivers/reset and a clk driver in drivers/clk. That > >> way the appropriate maintainers can review the code. There is only one > >> platform device with a single reg property and node in DT, but there are > >> two drivers. > > > >Yes, I agree that the reset driver and the clock driver should be split. > >However, I think using auxiliary bus is a little bit complicated in this > >case, because the reset is not a part of functionality of the clock in > >JH7110. They just share a common register base address. I think it is > >better to use ioremap for the same address, and the dt will be like > > > >syscrg_clk: clock-controller@13020000 { > > compatible = "starfive,jh7110-clkgen-sys"; > > reg = <0x0 0x13020000 0x0 0x10000>; > > ... > >}; > >syscrg_rst: reset-controller@13020000 { > > compatible = "starfive,jh7110-reset-sys"; > > reg = <0x0 0x13020000 0x0 0x10000>; > > ... > >}; > > > >What do you think of this approach? I would appreciate your suggestions. > > No, the dtb checks will all start warning for this. > Aux bus is not that difficult, you can likely copy much of what I did recently in clk-mpfs.c Thanks for reminding and your helpful example. Best regards, Hal
On Sat, 1 Oct 2022 12:14:49 +0100, Conor Dooley wrote: > On Fri, Sep 30, 2022 at 03:53:53PM +0800, Hal Feng wrote: > > From: Emil Renner Berthing <kernel@esmil.dk> > > > > Add a minimal device tree for StarFive JH7110 VisionFive2 board. > > Support booting and basic clock/reset/pinctrl/uart drivers. > > > > I would like to see a link to the publicly available datasheet or > documentation for the board (and for the SoC in patch 28) please. All documents can be found at RVspace Documentation Center maintained by StarFive. The related documents of JH7110 SoC and VisionFive2 board are as follows. StarFive JH7110 SoC: https://doc-en.rvspace.org/Doc_Center/jh7110.html StarFive VisionFive2 board: https://doc-en.rvspace.org/Doc_Center/visionfive_2.html > > > Signed-off-by: Emil Renner Berthing <kernel@esmil.dk> > > Signed-off-by: Jianlong Huang <jianlong.huang@starfivetech.com> > > Ditto from patch 28 re: the SoB chain. > > > Signed-off-by: Hal Feng <hal.feng@linux.starfivetech.com> > > --- > > > diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts > > new file mode 100644 > > index 000000000000..6b9fe32c7eac > > --- /dev/null > > +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts > > @@ -0,0 +1,91 @@ > > +// SPDX-License-Identifier: GPL-2.0 OR MIT > > +/* > > + * Copyright (C) 2022 StarFive Technology Co., Ltd. > > + * Copyright (C) 2022 Emil Renner Berthing <kernel@esmil.dk> > > + */ > > + > > +/dts-v1/; > > +#include "jh7110.dtsi" > > +#include <dt-bindings/gpio/gpio.h> > > +#include <dt-bindings/pinctrl/pinctrl-starfive-jh7110.h> > > + > > +/ { > > + model = "StarFive VisionFive V2"; > > + compatible = "starfive,visionfive-v2", "starfive,jh7110"; > > + > > + aliases { > > + serial0 = &uart0; > > + }; > > Should we also have a chosen node here? Will add it. Thanks. > > > + > > + cpus { > > + timebase-frequency = <4000000>; > > + }; > > + > > + memory@40000000 { > > + device_type = "memory"; > > + reg = <0x0 0x40000000 0x1 0x0>; > > What is going to happen to the 2 GB variant if they attempt to use this > devicetree? The VisionFive2 board now has 4GB version and 8GB version only. Before linux startup, we will change this property in dtb through u-boot to make sure the board can boot up with the correct memory size. > > > + }; > > + > > + reserved-memory { > > + #address-cells = <2>; > > + #size-cells = <2>; > > + ranges; > > + > > + linux,cma { > > + compatible = "shared-dma-pool"; > > + reusable; > > + size = <0x0 0x20000000>; > > + alignment = <0x0 0x1000>; > > + alloc-ranges = <0x0 0xa0000000 0x0 0x20000000>; > > + linux,cma-default; > > + }; > > + > > + e24_mem: e24@c0000000 { > > I had a conversation previously with Icenowy [0] about the e24 on the > jh7100 that didn't really come to a conclusion about how to represent > it there - but looks like you've decided that it should be a remoteproc > for the jh7100? Yes, we treat it as a remoteproc outside the cpus node. But after communication with my colleagues, I found that all nodes in "reserved-memory" are not used in the minimal support for VisionFive2 board. So for this series, I would like to remove "reserved-memory" in v2. > > Is this another situation where peripherals appear at different > addresses for the e24 compared to the u74s? Or has that changed for the> jh7100, and really the e24 should be described in the CPUs node? If it > is the latter, you can pick the first patch from [0] into your series. > > 0 - https://lore.kernel.org/linux-riscv/e8543838cd221ab6699da16c985eed7514daa786.camel@icenowy.me> > + reg = <0x0 0xc0110000 0x0 0xf0000>; > > + no-map; > > + }; > > + > > + xrp_reserved: xrpbuffer@f0000000 { > > "Following the generic-names recommended practice, node names should > reflect the purpose of the node (ie. “framebuffer” or “dma-pool”)." > > I tried googling around for an explanation for what the xrp was, and all > I could find was this out-of-tree text binding: > https://github.com/foss-xtensa/xrp/blob/master/xrp-kernel/cdns%2Cxrp-hw-simple%2Cv1.txt The name is from the device driver of HiFi4 DSP provided by Cadence, which is not in the mainline. "xrp" is a short name of "Xtensa Remote Processing". Best regards, Hal
This series adds basic support for the StarFive JH7110 RISC-V SoC to boot up and get a serial console. This series includes basic clock, reset, pinctrl and uart drivers, which are necessary for booting. It's should be noted that the reset and clock driver codes of JH7110 are partly common with those of JH7100, so the common codes are factored out and can be reused by drivers of JH7110 and other more SoCs from StarFive. The JH7110 is the upgraded version of JH7100 and also the first official released version of JH71XX series SoCs from StarFive Technology Ltd. The VisionFive 2 boards equipped with JH7110 SoCs are launched recently [1]. More information and support can visit RVspace wiki [2]. This series is also available at https://github.com/hal-feng/linux/commits/visionfive2-minimal [1] https://www.cnx-software.com/2022/08/23/starfive-visionfive-2-quad-core-risc-v-sbc-linux/ [2] https://wiki.rvspace.org/ Emil Renner Berthing (17): dt-bindings: riscv: Add StarFive JH7110 bindings dt-bindings: timer: Add StarFive JH7110 clint dt-bindings: interrupt-controller: Add StarFive JH7110 plic dt-bindings: sifive-l2-cache: Support StarFive JH71x0 SoCs soc: sifive: l2 cache: Convert to platform driver soc: sifive: l2 cache: Add StarFive JH71x0 support reset: starfive: jh7100: Use 32bit I/O on 32bit registers dt-bindings: reset: Add StarFive JH7110 reset definitions clk: starfive: Factor out common clock driver code dt-bindings: clock: Add StarFive JH7110 system clock definitions dt-bindings: clock: Add starfive,jh7110-clkgen-sys bindings clk: starfive: Add StarFive JH7110 system clock driver dt-bindings: clock: Add StarFive JH7110 always-on definitions dt-bindings: clock: Add starfive,jh7110-clkgen-aon bindings clk: starfive: Add StarFive JH7110 always-on clock driver RISC-V: Add initial StarFive JH7110 device tree RISC-V: Add StarFive JH7110 VisionFive2 board device tree Hal Feng (8): reset: starfive: jh7100: Use regmap APIs to operate registers reset: starfive: jh7100: Move necessary properties to device tree reset: starfive: Rename 'reset-starfive-jh7100.c' to 'reset-starfive.c' dt-bindings: reset: Add starfive,jh7110-reset bindings reset: starfive: Add StarFive JH7110 SoC support clk: starfive: Use regmap APIs to operate registers RISC-V: defconfig: Enable CONFIG_SERIAL_8250_DW RISC-V: Add StarFive JH7100 and JH7110 SoC Kconfig options Jianlong Huang (5): pinctrl: Create subdirectory for StarFive drivers pinctrl: starfive: Rename "pinctrl-starfive" to "pinctrl-starfive-jh7100" dt-bindings: pinctrl: Add StarFive JH7110 pinctrl definitions dt-bindings: pinctrl: Add StarFive JH7110 pinctrl bindings pinctrl: starfive: Add StarFive JH7110 driver .../clock/starfive,jh7110-clkgen-aon.yaml | 62 ++ .../clock/starfive,jh7110-clkgen-sys.yaml | 69 ++ .../sifive,plic-1.0.0.yaml | 1 + .../pinctrl/starfive,jh7100-pinctrl.yaml | 2 +- .../pinctrl/starfive,jh7110-pinctrl.yaml | 202 ++++ .../bindings/reset/starfive,jh7100-reset.yaml | 20 + .../bindings/reset/starfive,jh7110-reset.yaml | 54 + .../bindings/riscv/sifive-l2-cache.yaml | 4 + .../devicetree/bindings/riscv/starfive.yaml | 3 + .../bindings/timer/sifive,clint.yaml | 1 + MAINTAINERS | 27 +- arch/riscv/Kconfig.socs | 28 +- arch/riscv/boot/dts/starfive/Makefile | 3 +- .../dts/starfive/jh7100-beaglev-starlight.dts | 2 +- arch/riscv/boot/dts/starfive/jh7100.dtsi | 3 + .../jh7110-starfive-visionfive-v2.dts | 91 ++ arch/riscv/boot/dts/starfive/jh7110.dtsi | 449 +++++++++ arch/riscv/configs/defconfig | 1 + drivers/clk/starfive/Kconfig | 29 +- drivers/clk/starfive/Makefile | 6 +- .../clk/starfive/clk-starfive-jh7100-audio.c | 138 +-- drivers/clk/starfive/clk-starfive-jh7100.c | 836 +++++----------- drivers/clk/starfive/clk-starfive-jh7100.h | 112 --- .../clk/starfive/clk-starfive-jh7110-aon.c | 161 +++ .../clk/starfive/clk-starfive-jh7110-sys.c | 648 ++++++++++++ drivers/clk/starfive/clk-starfive.c | 349 +++++++ drivers/clk/starfive/clk-starfive.h | 112 +++ drivers/pinctrl/Kconfig | 18 +- drivers/pinctrl/Makefile | 2 +- drivers/pinctrl/starfive/Kconfig | 37 + drivers/pinctrl/starfive/Makefile | 8 + drivers/pinctrl/starfive/pinctrl-jh7110-aon.c | 718 ++++++++++++++ drivers/pinctrl/starfive/pinctrl-jh7110-sys.c | 925 +++++++++++++++++ .../pinctrl-starfive-jh7100.c} | 10 +- drivers/pinctrl/starfive/pinctrl-starfive.c | 539 ++++++++++ drivers/pinctrl/starfive/pinctrl-starfive.h | 131 +++ drivers/reset/Kconfig | 7 +- drivers/reset/Makefile | 2 +- drivers/reset/reset-starfive-jh7100.c | 173 ---- drivers/reset/reset-starfive.c | 218 ++++ drivers/soc/Makefile | 2 +- drivers/soc/sifive/Kconfig | 2 +- drivers/soc/sifive/sifive_l2_cache.c | 86 +- .../dt-bindings/clock/starfive-jh7110-aon.h | 26 + .../dt-bindings/clock/starfive-jh7110-sys.h | 215 ++++ ...l-starfive.h => pinctrl-starfive-jh7100.h} | 6 +- .../pinctrl/pinctrl-starfive-jh7110.h | 931 ++++++++++++++++++ include/dt-bindings/reset/starfive-jh7110.h | 154 +++ 48 files changed, 6604 insertions(+), 1019 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/starfive,jh7110-clkgen-aon.yaml create mode 100644 Documentation/devicetree/bindings/clock/starfive,jh7110-clkgen-sys.yaml create mode 100644 Documentation/devicetree/bindings/pinctrl/starfive,jh7110-pinctrl.yaml create mode 100644 Documentation/devicetree/bindings/reset/starfive,jh7110-reset.yaml create mode 100644 arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts create mode 100644 arch/riscv/boot/dts/starfive/jh7110.dtsi delete mode 100644 drivers/clk/starfive/clk-starfive-jh7100.h create mode 100644 drivers/clk/starfive/clk-starfive-jh7110-aon.c create mode 100644 drivers/clk/starfive/clk-starfive-jh7110-sys.c create mode 100644 drivers/clk/starfive/clk-starfive.c create mode 100644 drivers/clk/starfive/clk-starfive.h create mode 100644 drivers/pinctrl/starfive/Kconfig create mode 100644 drivers/pinctrl/starfive/Makefile create mode 100644 drivers/pinctrl/starfive/pinctrl-jh7110-aon.c create mode 100644 drivers/pinctrl/starfive/pinctrl-jh7110-sys.c rename drivers/pinctrl/{pinctrl-starfive.c => starfive/pinctrl-starfive-jh7100.c} (99%) create mode 100644 drivers/pinctrl/starfive/pinctrl-starfive.c create mode 100644 drivers/pinctrl/starfive/pinctrl-starfive.h delete mode 100644 drivers/reset/reset-starfive-jh7100.c create mode 100644 drivers/reset/reset-starfive.c create mode 100644 include/dt-bindings/clock/starfive-jh7110-aon.h create mode 100644 include/dt-bindings/clock/starfive-jh7110-sys.h rename include/dt-bindings/pinctrl/{pinctrl-starfive.h => pinctrl-starfive-jh7100.h} (98%) create mode 100644 include/dt-bindings/pinctrl/pinctrl-starfive-jh7110.h create mode 100644 include/dt-bindings/reset/starfive-jh7110.h