| Message ID | 20220929175602.19946-1-hal.feng@linux.starfivetech.com |
|---|---|
| State | New |
| Headers | show |
| Series | Basic StarFive JH7110 RISC-V SoC support | expand |
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 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
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, 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
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
diff --git a/drivers/clk/starfive/clk-starfive-jh7100-audio.c b/drivers/clk/starfive/clk-starfive-jh7100-audio.c index 41389cacfe03..4168209d6600 100644 --- a/drivers/clk/starfive/clk-starfive-jh7100-audio.c +++ b/drivers/clk/starfive/clk-starfive-jh7100-audio.c @@ -9,6 +9,7 @@ #include <linux/clk-provider.h> #include <linux/device.h> #include <linux/kernel.h> +#include <linux/mfd/syscon.h> #include <linux/mod_devicetable.h> #include <linux/module.h> #include <linux/of_device.h> @@ -108,11 +109,13 @@ static int jh7100_audclk_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); + 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); + } for (idx = 0; idx < JH7100_AUDCLK_END; idx++) { u32 max = jh7100_audclk_data[idx].max; 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); + 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); + } priv->pll[0] = devm_clk_hw_register_fixed_factor(priv->dev, "pll0_out", "osc_sys", 0, 40, 1); diff --git a/drivers/clk/starfive/clk-starfive.c b/drivers/clk/starfive/clk-starfive.c index 76e3d45b5d86..e428476417c5 100644 --- a/drivers/clk/starfive/clk-starfive.c +++ b/drivers/clk/starfive/clk-starfive.c @@ -9,6 +9,9 @@ #include <linux/debugfs.h> #include <linux/device.h> #include <linux/io.h> +#include <linux/of.h> +#include <linux/of_address.h> +#include <linux/regmap.h> #include "clk-starfive.h" @@ -25,36 +28,36 @@ static struct starfive_clk_priv *starfive_priv_from(struct starfive_clk *clk) static u32 starfive_clk_reg_get(struct starfive_clk *clk) { struct starfive_clk_priv *priv = starfive_priv_from(clk); - void __iomem *reg = priv->base + 4 * clk->idx; + unsigned int reg = sizeof(u32) * clk->idx; + unsigned int value; + int ret; - return readl_relaxed(reg); -} - -static void starfive_clk_reg_rmw(struct starfive_clk *clk, u32 mask, u32 value) -{ - struct starfive_clk_priv *priv = starfive_priv_from(clk); - void __iomem *reg = priv->base + 4 * clk->idx; - unsigned long flags; + ret = regmap_read(priv->regmap, reg, &value); + if (ret) { + dev_warn(priv->dev, "Failed to read clock register: %d\n", ret); + value = 0; + } - spin_lock_irqsave(&priv->rmw_lock, flags); - value |= readl_relaxed(reg) & ~mask; - writel_relaxed(value, reg); - spin_unlock_irqrestore(&priv->rmw_lock, flags); + return value; } static int starfive_clk_enable(struct clk_hw *hw) { struct starfive_clk *clk = starfive_clk_from(hw); + struct starfive_clk_priv *priv = starfive_priv_from(clk); + unsigned int reg = sizeof(u32) * clk->idx; - starfive_clk_reg_rmw(clk, STARFIVE_CLK_ENABLE, STARFIVE_CLK_ENABLE); - return 0; + return regmap_update_bits(priv->regmap, reg, + STARFIVE_CLK_ENABLE, STARFIVE_CLK_ENABLE); } static void starfive_clk_disable(struct clk_hw *hw) { struct starfive_clk *clk = starfive_clk_from(hw); + struct starfive_clk_priv *priv = starfive_priv_from(clk); + unsigned int reg = sizeof(u32) * clk->idx; - starfive_clk_reg_rmw(clk, STARFIVE_CLK_ENABLE, 0); + regmap_update_bits(priv->regmap, reg, STARFIVE_CLK_ENABLE, 0); } static int starfive_clk_is_enabled(struct clk_hw *hw) @@ -107,11 +110,12 @@ static int starfive_clk_set_rate(struct clk_hw *hw, unsigned long parent_rate) { struct starfive_clk *clk = starfive_clk_from(hw); + struct starfive_clk_priv *priv = starfive_priv_from(clk); + unsigned int reg = sizeof(u32) * clk->idx; unsigned long div = clamp(DIV_ROUND_CLOSEST(parent_rate, rate), 1UL, (unsigned long)clk->max_div); - starfive_clk_reg_rmw(clk, STARFIVE_CLK_DIV_MASK, div); - return 0; + return regmap_update_bits(priv->regmap, reg, STARFIVE_CLK_DIV_MASK, div); } static unsigned long starfive_clk_frac_recalc_rate(struct clk_hw *hw, @@ -149,12 +153,13 @@ static int starfive_clk_frac_set_rate(struct clk_hw *hw, unsigned long parent_rate) { struct starfive_clk *clk = starfive_clk_from(hw); + struct starfive_clk_priv *priv = starfive_priv_from(clk); + unsigned int reg = sizeof(u32) * clk->idx; unsigned long div100 = clamp(DIV_ROUND_CLOSEST(100 * parent_rate, rate), STARFIVE_CLK_FRAC_MIN, STARFIVE_CLK_FRAC_MAX); u32 value = ((div100 % 100) << STARFIVE_CLK_FRAC_SHIFT) | (div100 / 100); - starfive_clk_reg_rmw(clk, STARFIVE_CLK_DIV_MASK, value); - return 0; + return regmap_update_bits(priv->regmap, reg, STARFIVE_CLK_DIV_MASK, value); } static u8 starfive_clk_get_parent(struct clk_hw *hw) @@ -168,10 +173,11 @@ static u8 starfive_clk_get_parent(struct clk_hw *hw) static int starfive_clk_set_parent(struct clk_hw *hw, u8 index) { struct starfive_clk *clk = starfive_clk_from(hw); + struct starfive_clk_priv *priv = starfive_priv_from(clk); + unsigned int reg = sizeof(u32) * clk->idx; u32 value = (u32)index << STARFIVE_CLK_MUX_SHIFT; - starfive_clk_reg_rmw(clk, STARFIVE_CLK_MUX_MASK, value); - return 0; + return regmap_update_bits(priv->regmap, reg, STARFIVE_CLK_MUX_MASK, value); } static int starfive_clk_mux_determine_rate(struct clk_hw *hw, @@ -191,6 +197,8 @@ static int starfive_clk_get_phase(struct clk_hw *hw) static int starfive_clk_set_phase(struct clk_hw *hw, int degrees) { struct starfive_clk *clk = starfive_clk_from(hw); + struct starfive_clk_priv *priv = starfive_priv_from(clk); + unsigned int reg = sizeof(u32) * clk->idx; u32 value; if (degrees == 0) @@ -200,8 +208,7 @@ static int starfive_clk_set_phase(struct clk_hw *hw, int degrees) else return -EINVAL; - starfive_clk_reg_rmw(clk, STARFIVE_CLK_INVERT, value); - return 0; + return regmap_update_bits(priv->regmap, reg, STARFIVE_CLK_INVERT, value); } #ifdef CONFIG_DEBUG_FS @@ -214,6 +221,7 @@ static void starfive_clk_debug_init(struct clk_hw *hw, struct dentry *dentry) struct starfive_clk *clk = starfive_clk_from(hw); struct starfive_clk_priv *priv = starfive_priv_from(clk); struct debugfs_regset32 *regset; + void __iomem *base; regset = devm_kzalloc(priv->dev, sizeof(*regset), GFP_KERNEL); if (!regset) @@ -221,7 +229,15 @@ static void starfive_clk_debug_init(struct clk_hw *hw, struct dentry *dentry) regset->regs = &starfive_clk_reg; regset->nregs = 1; - regset->base = priv->base + 4 * clk->idx; + + base = of_iomap(priv->dev->of_node, 0); + if (!base) { + base = of_iomap(priv->dev->of_node->parent, 0); + if (!base) + return; + } + + regset->base = base + sizeof(u32) * clk->idx; debugfs_create_regset32("registers", 0400, dentry, regset); } diff --git a/drivers/clk/starfive/clk-starfive.h b/drivers/clk/starfive/clk-starfive.h index 6b05cf1bfbb6..99cf74e8cbde 100644 --- a/drivers/clk/starfive/clk-starfive.h +++ b/drivers/clk/starfive/clk-starfive.h @@ -101,10 +101,8 @@ struct starfive_clk { }; struct starfive_clk_priv { - /* protect clk enable and set rate/parent from happening at the same time */ - spinlock_t rmw_lock; struct device *dev; - void __iomem *base; + struct regmap *regmap; struct clk_hw *pll[3]; struct starfive_clk reg[]; };
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. Signed-off-by: Hal Feng <hal.feng@linux.starfivetech.com> --- .../clk/starfive/clk-starfive-jh7100-audio.c | 11 ++-- drivers/clk/starfive/clk-starfive-jh7100.c | 11 ++-- drivers/clk/starfive/clk-starfive.c | 66 ++++++++++++------- drivers/clk/starfive/clk-starfive.h | 4 +- 4 files changed, 56 insertions(+), 36 deletions(-)