| Message ID | 1592828876-7724-5-git-send-email-sagar.kadam@sifive.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | add DM based reset driver for SiFive SoC's | expand |
On 6/24/20 1:01 AM, Bin Meng wrote: > Hi Sean, > > On Wed, Jun 24, 2020 at 12:17 PM Sean Anderson <seanga2 at gmail.com> wrote: >> >> On 6/22/20 8:27 AM, Sagar Shrikant Kadam wrote: >>> The resets to DDR and ethernet sub-system are connected to >>> PRCI device reset control register, these reset signals >>> are active low and are held low at power-up. Add these reset >>> producer and consumer details needed by the reset driver. >>> >>> Signed-off-by: Sagar Shrikant Kadam <sagar.kadam at sifive.com> >>> Reviewed-by: Pragnesh Patel <Pragnesh.patel at sifive.com> >>> --- >>> arch/riscv/dts/fu540-c000-u-boot.dtsi | 10 ++++++++++ >>> 1 file changed, 10 insertions(+) >>> >>> diff --git a/arch/riscv/dts/fu540-c000-u-boot.dtsi b/arch/riscv/dts/fu540-c000-u-boot.dtsi >>> index 9bba554..b37241e 100644 >>> --- a/arch/riscv/dts/fu540-c000-u-boot.dtsi >>> +++ b/arch/riscv/dts/fu540-c000-u-boot.dtsi >>> @@ -59,6 +59,16 @@ >>> reg = <0x0 0x2000000 0x0 0xc0000>; >>> u-boot,dm-spl; >>> }; >>> + prci: clock-controller at 10000000 { >> >> Shouldn't this have a compatible property? > > This is the U-Boot specific dts fragment. See fu540-c000.dtsi I ask because this node sits in /soc, and all the other nodes in /soc have compatible strings. Since this device is bound by the clock driver, perhaps it should be located under /soc/prci instead. --Sean > >> >>> + #reset-cells = <1>; >>> + resets = <&prci PRCI_RST_DDR_CTRL_N>, >>> + <&prci PRCI_RST_DDR_AXI_N>, >>> + <&prci PRCI_RST_DDR_AHB_N>, >>> + <&prci PRCI_RST_DDR_PHY_N>, >>> + <&prci PRCI_RST_GEMGXL_N>; >>> + reset-names = "ddr_ctrl", "ddr_axi", "ddr_ahb", >>> + "ddr_phy", "gemgxl_reset"; >>> + }; >>> dmc: dmc at 100b0000 { >>> compatible = "sifive,fu540-c000-ddr"; >>> reg = <0x0 0x100b0000 0x0 0x0800 > > Regards, > Bin >
Hi Sean, On Wed, Jun 24, 2020 at 1:04 PM Sean Anderson <seanga2 at gmail.com> wrote: > > On 6/24/20 1:01 AM, Bin Meng wrote: > > Hi Sean, > > > > On Wed, Jun 24, 2020 at 12:17 PM Sean Anderson <seanga2 at gmail.com> wrote: > >> > >> On 6/22/20 8:27 AM, Sagar Shrikant Kadam wrote: > >>> The resets to DDR and ethernet sub-system are connected to > >>> PRCI device reset control register, these reset signals > >>> are active low and are held low at power-up. Add these reset > >>> producer and consumer details needed by the reset driver. > >>> > >>> Signed-off-by: Sagar Shrikant Kadam <sagar.kadam at sifive.com> > >>> Reviewed-by: Pragnesh Patel <Pragnesh.patel at sifive.com> > >>> --- > >>> arch/riscv/dts/fu540-c000-u-boot.dtsi | 10 ++++++++++ > >>> 1 file changed, 10 insertions(+) > >>> > >>> diff --git a/arch/riscv/dts/fu540-c000-u-boot.dtsi b/arch/riscv/dts/fu540-c000-u-boot.dtsi > >>> index 9bba554..b37241e 100644 > >>> --- a/arch/riscv/dts/fu540-c000-u-boot.dtsi > >>> +++ b/arch/riscv/dts/fu540-c000-u-boot.dtsi > >>> @@ -59,6 +59,16 @@ > >>> reg = <0x0 0x2000000 0x0 0xc0000>; > >>> u-boot,dm-spl; > >>> }; > >>> + prci: clock-controller at 10000000 { > >> > >> Shouldn't this have a compatible property? > > > > This is the U-Boot specific dts fragment. See fu540-c000.dtsi > > I ask because this node sits in /soc, and all the other nodes in /soc > have compatible strings. Since this device is bound by the clock driver, > perhaps it should be located under /soc/prci instead. fu540-c000.dtsi has everything you asked. Regards, Bin
On 6/24/20 1:15 AM, Bin Meng wrote: > Hi Sean, > > On Wed, Jun 24, 2020 at 1:04 PM Sean Anderson <seanga2 at gmail.com> wrote: >> >> On 6/24/20 1:01 AM, Bin Meng wrote: >>> Hi Sean, >>> >>> On Wed, Jun 24, 2020 at 12:17 PM Sean Anderson <seanga2 at gmail.com> wrote: >>>> >>>> On 6/22/20 8:27 AM, Sagar Shrikant Kadam wrote: >>>>> The resets to DDR and ethernet sub-system are connected to >>>>> PRCI device reset control register, these reset signals >>>>> are active low and are held low at power-up. Add these reset >>>>> producer and consumer details needed by the reset driver. >>>>> >>>>> Signed-off-by: Sagar Shrikant Kadam <sagar.kadam at sifive.com> >>>>> Reviewed-by: Pragnesh Patel <Pragnesh.patel at sifive.com> >>>>> --- >>>>> arch/riscv/dts/fu540-c000-u-boot.dtsi | 10 ++++++++++ >>>>> 1 file changed, 10 insertions(+) >>>>> >>>>> diff --git a/arch/riscv/dts/fu540-c000-u-boot.dtsi b/arch/riscv/dts/fu540-c000-u-boot.dtsi >>>>> index 9bba554..b37241e 100644 >>>>> --- a/arch/riscv/dts/fu540-c000-u-boot.dtsi >>>>> +++ b/arch/riscv/dts/fu540-c000-u-boot.dtsi >>>>> @@ -59,6 +59,16 @@ >>>>> reg = <0x0 0x2000000 0x0 0xc0000>; >>>>> u-boot,dm-spl; >>>>> }; >>>>> + prci: clock-controller at 10000000 { >>>> >>>> Shouldn't this have a compatible property? >>> >>> This is the U-Boot specific dts fragment. See fu540-c000.dtsi >> >> I ask because this node sits in /soc, and all the other nodes in /soc >> have compatible strings. Since this device is bound by the clock driver, >> perhaps it should be located under /soc/prci instead. > > fu540-c000.dtsi has everything you asked. > Ah, I didn't see that this was an extension. Looks good to me. --Sean
On Mon, Jun 22, 2020 at 8:28 PM Sagar Shrikant Kadam <sagar.kadam at sifive.com> wrote: > > PRCI module within SiFive SoC's has register with which we can > reset the sub-systems within the SoC. The resets to DDR and ethernet > sub systems within FU540-C000 SoC are active low, and are hold low > by default on power-up. Currently these are directly asserted within > prci driver via register read/write. > With the DM based reset driver support here, we bind the reset > driver with clock (prci) driver and assert the reset signals of > both sub-system's appropriately. > > Signed-off-by: Sagar Shrikant Kadam <sagar.kadam at sifive.com> > Reviewed-by: Pragnesh Patel <Pragnesh.patel at sifive.com> > --- > arch/riscv/include/asm/arch-fu540/reset.h | 13 ++++ > drivers/clk/sifive/fu540-prci.c | 73 ++++++++++++++---- > drivers/reset/reset-sifive.c | 118 ++++++++++++++++++++++++++++++ > 3 files changed, 189 insertions(+), 15 deletions(-) > create mode 100644 arch/riscv/include/asm/arch-fu540/reset.h > create mode 100644 drivers/reset/reset-sifive.c > Reviewed-by: Bin Meng <bin.meng at windriver.com> Tested-by: Bin Meng <bin.meng at windriver.com>
Hi, > -----Original Message----- > From: Sean Anderson <seanga2 at gmail.com> > Sent: Wednesday, June 24, 2020 10:49 AM > To: Bin Meng <bmeng.cn at gmail.com> > Cc: Sagar Kadam <sagar.kadam at sifive.com>; U-Boot Mailing List <u- > boot at lists.denx.de>; Rick Chen <rick at andestech.com>; Paul Walmsley ( > Sifive) <paul.walmsley at sifive.com>; Palmer Dabbelt > <palmer at dabbelt.com>; Anup Patel <anup.patel at wdc.com>; Atish Patra > <atish.patra at wdc.com>; Lukasz Majewski <lukma at denx.de>; Pragnesh > Patel <pragnesh.patel at sifive.com>; Jagan Teki > <jagan at amarulasolutions.com>; Simon Glass <sjg at chromium.org>; > twoerner at gmail.com; Patrick Wildt <patrick at blueri.se>; Fabio Estevam > <festevam at gmail.com>; Weijie Gao <weijie.gao at mediatek.com>; Eugeniy > Paltsev <Eugeniy.Paltsev at synopsys.com> > Subject: Re: [PATCH 4/5] sifive: reset: add DM based reset driver for SiFive > SoC's > > [External Email] Do not click links or attachments unless you recognize the > sender and know the content is safe > > On 6/24/20 1:15 AM, Bin Meng wrote: > > Hi Sean, > > > > On Wed, Jun 24, 2020 at 1:04 PM Sean Anderson <seanga2 at gmail.com> > wrote: > >> > >> On 6/24/20 1:01 AM, Bin Meng wrote: > >>> Hi Sean, > >>> > >>> On Wed, Jun 24, 2020 at 12:17 PM Sean Anderson > <seanga2 at gmail.com> wrote: > >>>> > >>>> On 6/22/20 8:27 AM, Sagar Shrikant Kadam wrote: > >>>>> The resets to DDR and ethernet sub-system are connected to PRCI > >>>>> device reset control register, these reset signals are active low > >>>>> and are held low at power-up. Add these reset producer and > >>>>> consumer details needed by the reset driver. > >>>>> > >>>>> Signed-off-by: Sagar Shrikant Kadam <sagar.kadam at sifive.com> > >>>>> Reviewed-by: Pragnesh Patel <Pragnesh.patel at sifive.com> > >>>>> --- > >>>>> arch/riscv/dts/fu540-c000-u-boot.dtsi | 10 ++++++++++ > >>>>> 1 file changed, 10 insertions(+) > >>>>> > >>>>> diff --git a/arch/riscv/dts/fu540-c000-u-boot.dtsi > >>>>> b/arch/riscv/dts/fu540-c000-u-boot.dtsi > >>>>> index 9bba554..b37241e 100644 > >>>>> --- a/arch/riscv/dts/fu540-c000-u-boot.dtsi > >>>>> +++ b/arch/riscv/dts/fu540-c000-u-boot.dtsi > >>>>> @@ -59,6 +59,16 @@ > >>>>> reg = <0x0 0x2000000 0x0 0xc0000>; > >>>>> u-boot,dm-spl; > >>>>> }; > >>>>> + prci: clock-controller at 10000000 { > >>>> > >>>> Shouldn't this have a compatible property? > >>> > >>> This is the U-Boot specific dts fragment. See fu540-c000.dtsi > >> > >> I ask because this node sits in /soc, and all the other nodes in /soc > >> have compatible strings. Since this device is bound by the clock > >> driver, perhaps it should be located under /soc/prci instead. > > > > fu540-c000.dtsi has everything you asked. > > > > Ah, I didn't see that this was an extension. Looks good to me. > > --Sean Thanks Bin and Sean for looking at it. BR, Sagar Kadam
diff --git a/arch/riscv/include/asm/arch-fu540/reset.h b/arch/riscv/include/asm/arch-fu540/reset.h new file mode 100644 index 0000000..e42797a --- /dev/null +++ b/arch/riscv/include/asm/arch-fu540/reset.h @@ -0,0 +1,13 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * Copyright (c) 2020 SiFive, Inc. + * + * Author: Sagar Kadam <sagar.kadam at sifive.com> + */ + +#ifndef __RESET_SIFIVE_H +#define __RESET_SIFIVE_H + +int sifive_reset_bind(struct udevice *dev, ulong count); + +#endif diff --git a/drivers/clk/sifive/fu540-prci.c b/drivers/clk/sifive/fu540-prci.c index 57d811e..675b508 100644 --- a/drivers/clk/sifive/fu540-prci.c +++ b/drivers/clk/sifive/fu540-prci.c @@ -30,11 +30,15 @@ #include <common.h> #include <asm/io.h> +#include <asm/arch/reset.h> #include <clk-uclass.h> #include <clk.h> #include <div64.h> #include <dm.h> #include <errno.h> +#include <reset-uclass.h> +#include <dm/device.h> +#include <dm/uclass.h> #include <linux/delay.h> #include <linux/err.h> @@ -131,6 +135,7 @@ /* DEVICESRESETREG */ #define PRCI_DEVICESRESETREG_OFFSET 0x28 +#define PRCI_DEVICERESETCNT 5 #define PRCI_DEVICESRESETREG_DDR_CTRL_RST_N_MASK \ (0x1 << PRCI_RST_DDR_CTRL_N) @@ -524,6 +529,41 @@ static const struct __prci_clock_ops sifive_fu540_prci_tlclksel_clk_ops = { .recalc_rate = sifive_fu540_prci_tlclksel_recalc_rate, }; +static int __prci_consumer_reset(const char *rst_name, bool trigger) +{ + struct udevice *dev; + struct reset_ctl rst_sig; + int ret; + + ret = uclass_get_device_by_driver(UCLASS_RESET, + DM_GET_DRIVER(sifive_reset), + &dev); + if (ret) { + dev_err(dev, "Reset driver not found: %d\n", ret); + return ret; + } + + ret = reset_get_by_name(dev, rst_name, &rst_sig); + if (ret) { + dev_err(dev, "failed to get %s reset\n", rst_name); + return ret; + } + + if (reset_valid(&rst_sig)) { + if (trigger) + ret = reset_deassert(&rst_sig); + else + ret = reset_assert(&rst_sig); + if (ret) { + dev_err(dev, "failed to trigger reset id = %ld\n", + rst_sig.id); + return ret; + } + } + + return ret; +} + /** * __prci_ddr_release_reset() - Release DDR reset * @pd: struct __prci_data * for the PRCI containing the DDRCLK mux reg @@ -531,19 +571,20 @@ static const struct __prci_clock_ops sifive_fu540_prci_tlclksel_clk_ops = { */ static void __prci_ddr_release_reset(struct __prci_data *pd) { - u32 v; - - v = __prci_readl(pd, PRCI_DEVICESRESETREG_OFFSET); - v |= PRCI_DEVICESRESETREG_DDR_CTRL_RST_N_MASK; - __prci_writel(v, PRCI_DEVICESRESETREG_OFFSET, pd); + /* Release DDR ctrl reset */ + __prci_consumer_reset("ddr_ctrl", true); /* HACK to get the '1 full controller clock cycle'. */ asm volatile ("fence"); - v = __prci_readl(pd, PRCI_DEVICESRESETREG_OFFSET); - v |= (PRCI_DEVICESRESETREG_DDR_AXI_RST_N_MASK | - PRCI_DEVICESRESETREG_DDR_AHB_RST_N_MASK | - PRCI_DEVICESRESETREG_DDR_PHY_RST_N_MASK); - __prci_writel(v, PRCI_DEVICESRESETREG_OFFSET, pd); + + /* Release DDR AXI reset */ + __prci_consumer_reset("ddr_axi", true); + + /* Release DDR AHB reset */ + __prci_consumer_reset("ddr_ahb", true); + + /* Release DDR PHY reset */ + __prci_consumer_reset("ddr_phy", true); /* HACK to get the '1 full controller clock cycle'. */ asm volatile ("fence"); @@ -563,12 +604,8 @@ static void __prci_ddr_release_reset(struct __prci_data *pd) */ static void __prci_ethernet_release_reset(struct __prci_data *pd) { - u32 v; - /* Release GEMGXL reset */ - v = __prci_readl(pd, PRCI_DEVICESRESETREG_OFFSET); - v |= PRCI_DEVICESRESETREG_GEMGXL_RST_N_MASK; - __prci_writel(v, PRCI_DEVICESRESETREG_OFFSET, pd); + __prci_consumer_reset("gemgxl_reset", true); /* Procmon => core clock */ __prci_writel(PRCI_PROCMONCFG_CORE_CLOCK_MASK, PRCI_PROCMONCFG_OFFSET, @@ -753,6 +790,11 @@ static struct clk_ops sifive_fu540_prci_ops = { .disable = sifive_fu540_prci_disable, }; +static int sifive_fu540_clk_bind(struct udevice *dev) +{ + return sifive_reset_bind(dev, PRCI_DEVICERESETCNT); +} + static const struct udevice_id sifive_fu540_prci_ids[] = { { .compatible = "sifive,fu540-c000-prci" }, { } @@ -765,4 +807,5 @@ U_BOOT_DRIVER(sifive_fu540_prci) = { .probe = sifive_fu540_prci_probe, .ops = &sifive_fu540_prci_ops, .priv_auto_alloc_size = sizeof(struct __prci_data), + .bind = sifive_fu540_clk_bind, }; diff --git a/drivers/reset/reset-sifive.c b/drivers/reset/reset-sifive.c new file mode 100644 index 0000000..527757f --- /dev/null +++ b/drivers/reset/reset-sifive.c @@ -0,0 +1,118 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (C) 2020 Sifive, Inc. + * Author: Sagar Kadam <sagar.kadam at sifive.com> + */ + +#include <common.h> +#include <dm.h> +#include <reset-uclass.h> +#include <asm/io.h> +#include <dm/device_compat.h> +#include <dm/lists.h> +#include <linux/bitops.h> + +#define PRCI_RESETREG_OFFSET 0x28 + +struct sifive_reset_priv { + void *base; + /* number of reset signals */ + int nr_reset; +}; + +static int sifive_rst_trigger(struct reset_ctl *rst, bool level) +{ + struct sifive_reset_priv *priv = dev_get_priv(rst->dev); + int id = rst->id; + int regval = readl(priv->base + PRCI_RESETREG_OFFSET); + + /* Derive bitposition from rst id */ + if (level) + /* Reset deassert */ + regval |= BIT(id); + else + /* Reset assert */ + regval &= ~BIT(id); + + writel(regval, priv->base + PRCI_RESETREG_OFFSET); + + return 0; +} + +static int sifive_reset_assert(struct reset_ctl *rst) +{ + return sifive_rst_trigger(rst, false); +} + +static int sifive_reset_deassert(struct reset_ctl *rst) +{ + return sifive_rst_trigger(rst, true); +} + +static int sifive_reset_request(struct reset_ctl *rst) +{ + struct sifive_reset_priv *priv = dev_get_priv(rst->dev); + + debug("%s(rst=%p) (dev=%p, id=%lu) (nr_reset=%d)\n", __func__, + rst, rst->dev, rst->id, priv->nr_reset); + + if (rst->id > priv->nr_reset) + return -EINVAL; + + return 0; +} + +static int sifive_reset_free(struct reset_ctl *rst) +{ + struct sifive_reset_priv *priv = dev_get_priv(rst->dev); + + debug("%s(rst=%p) (dev=%p, id=%lu) (nr_reset=%d)\n", __func__, + rst, rst->dev, rst->id, priv->nr_reset); + + return 0; +} + +static int sifive_reset_probe(struct udevice *dev) +{ + struct sifive_reset_priv *priv = dev_get_priv(dev); + + priv->base = dev_remap_addr(dev); + if (!priv->base) + return -ENOMEM; + + return 0; +} + +int sifive_reset_bind(struct udevice *dev, ulong count) +{ + struct udevice *rst_dev; + struct sifive_reset_priv *priv; + int ret; + + ret = device_bind_driver_to_node(dev, "sifive-reset", "reset", + dev_ofnode(dev), &rst_dev); + if (ret) { + dev_err(dev, "failed to bind sifive_reset driver (ret=%d)\n", ret); + return ret; + } + priv = malloc(sizeof(struct sifive_reset_priv)); + priv->nr_reset = count; + rst_dev->priv = priv; + + return 0; +} + +const struct reset_ops sifive_reset_ops = { + .request = sifive_reset_request, + .rfree = sifive_reset_free, + .rst_assert = sifive_reset_assert, + .rst_deassert = sifive_reset_deassert, +}; + +U_BOOT_DRIVER(sifive_reset) = { + .name = "sifive-reset", + .id = UCLASS_RESET, + .ops = &sifive_reset_ops, + .probe = sifive_reset_probe, + .priv_auto_alloc_size = sizeof(struct sifive_reset_priv), +};