[PATCH v1 0/6] clk: microchip: mpfs: incremental fixes

[PATCH v1 0/6] clk: microchip: mpfs: incremental fixes

[Conor Dooley]

Hey All,
When trying to sync the Linux device tree with U-Boot, the clock driver's
expectation that the input clock it gets is the mss pll presented itself
as a problem. The mss pll is not a fixed frequency clock and the Linux
devicetree has the actual off-chip oscillator & the clock driver there
uses that. This had gone un-noticed in the original dt upstreaming to
Linux and I noticed it while upstreaming the clock driver there.

I order to switch the clock driver over to using the real reference, we
first need to switch over from reading a property of what we assume to be
a fixed-frequency clock in the device tree to using clk_get_rate() and
fix the parentage of the "periph" clocks to represent their actual
relationships to their parents rather than using this fixed-frequency
clock in the devicetree to determine their rates.

With that done, we can then introduce a small driver for the msspll
itself, which we will use if the corrected devicetree node for the clock
controller is present.

There are some more fixes required, specifically handling of the mtimer
clock and of the refclk for the rtc which I have not addressed here, but
will deal with in a follow-up series. I've omitted these fixes here to
do just what is needed to unblock the dt sync.

Thanks,
Conor.

Conor Dooley (6):
  dt-bindings: clk: add missing clk ids for microchip mpfs
  clk: microchip: mpfs: convert parent rate acquistion to get_get_rate()
  clk: microchip: mpfs: fix reference clock handling
  clk: microchip: mpfs: fix periph clk parentage
  clk: microchip: mpfs: fix criticality of peripheral clocks
  riscv: dts: fix the mpfs's reference clock frequency

 arch/riscv/dts/microchip-mpfs-icicle-kit.dts  |   4 +
 arch/riscv/dts/microchip-mpfs.dtsi            |  14 +--
 drivers/clk/microchip/Makefile                |   2 +-
 drivers/clk/microchip/mpfs_clk.c              |  37 ++++--
 drivers/clk/microchip/mpfs_clk.h              |  20 +--
 drivers/clk/microchip/mpfs_clk_cfg.c          |   7 +-
 drivers/clk/microchip/mpfs_clk_msspll.c       | 119 ++++++++++++++++++
 drivers/clk/microchip/mpfs_clk_periph.c       |  96 +++++++-------
 .../dt-bindings/clock/microchip-mpfs-clock.h  |   3 +
 9 files changed, 229 insertions(+), 73 deletions(-)
 create mode 100644 drivers/clk/microchip/mpfs_clk_msspll.c

-- 
2.38.0

[PATCH v1 1/6] dt-bindings: clk: add missing clk ids for microchip mpfs

[Conor Dooley]

When this binding header was initally upstreamed, the PLL clocking the
microprocessor subsystem (MSS) and the RTC reference clocks were
omitted. Add them now, matching the IDs used in Linux.

Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 include/dt-bindings/clock/microchip-mpfs-clock.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/dt-bindings/clock/microchip-mpfs-clock.h b/include/dt-bindings/clock/microchip-mpfs-clock.h
index 55fe64693f..c7ed0a8db7 100644
--- a/include/dt-bindings/clock/microchip-mpfs-clock.h
+++ b/include/dt-bindings/clock/microchip-mpfs-clock.h
@@ -42,4 +42,7 @@
 #define CLK_ATHENA	31
 #define CLK_CFM		32
 
+#define CLK_RTCREF	33
+#define CLK_MSSPLL	34
+
 #endif	/* _DT_BINDINGS_CLK_MICROCHIP_MPFS_H_ */
-- 
2.38.0

Re: [PATCH v1 1/6] dt-bindings: clk: add missing clk ids for microchip mpfs

[Leo Liang]

On Tue, Oct 25, 2022 at 08:58:44AM +0100, Conor Dooley wrote:
> When this binding header was initally upstreamed, the PLL clocking the
> microprocessor subsystem (MSS) and the RTC reference clocks were
> omitted. Add them now, matching the IDs used in Linux.
> 
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>  include/dt-bindings/clock/microchip-mpfs-clock.h | 3 +++
>  1 file changed, 3 insertions(+)

Reviewed-by: Leo Yu-Chi Liang <ycliang@andestech.com>

Re: [PATCH v1 1/6] dt-bindings: clk: add missing clk ids for microchip mpfs

[Padmarao.Begari]

> On Tue, 2022-10-25 at 08:58 +0100, Conor Dooley wrote:
> When this binding header was initally upstreamed, the PLL clocking
> the
> microprocessor subsystem (MSS) and the RTC reference clocks were
> omitted. Add them now, matching the IDs used in Linux.
> 
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>  include/dt-bindings/clock/microchip-mpfs-clock.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/include/dt-bindings/clock/microchip-mpfs-clock.h
> b/include/dt-bindings/clock/microchip-mpfs-clock.h
> index 55fe64693f..c7ed0a8db7 100644
> --- a/include/dt-bindings/clock/microchip-mpfs-clock.h
> +++ b/include/dt-bindings/clock/microchip-mpfs-clock.h
> @@ -42,4 +42,7 @@
>  #define CLK_ATHENA	31
>  #define CLK_CFM		32
>  
> +#define CLK_RTCREF	33
> +#define CLK_MSSPLL	34
> +
>  #endif	/* _DT_BINDINGS_CLK_MICROCHIP_MPFS_H_ */

Reviewed-by: Padmarao Begari <padmarao.begari@microchip.com>

[PATCH v1 2/6] clk: microchip: mpfs: convert parent rate acquistion to get_get_rate()

[Conor Dooley]

Currently the clock driver for PolarFire SoC takes a very naive approach
to the relationship between clocks. It reads the dt to get an input
clock, assumes that that is fixed frequency, reads the "clock-frequency"
property & uses that to set up both the "cfg" and "periph" clocks.

Simplifying for the sake of incremental fixes, the "correct" parentage for
the clocks currently supported in U-Boot is that the "cfg" clocks should
be children of the fixed frequency clock in the dt. The AHB clock is one
of these "cfg" clocks and is the parent of the "periph" clocks.

Instead of passing the clock rate of the fixed-frequency clock to the
"cfg" and "periph" registration functions and the name of the parents,
pass their actual parents & use clk_get_rate() to determine their parents
rates.

The "periph" clocks are purely gate clocks and should not be reading the
AHB clocks registers to determine their rates, as they can simply report
the output of clk_get_rate() on their parent.

Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 drivers/clk/microchip/Makefile          |  2 +-
 drivers/clk/microchip/mpfs_clk.c        | 18 ++++++++----------
 drivers/clk/microchip/mpfs_clk.h        | 12 ++++--------
 drivers/clk/microchip/mpfs_clk_cfg.c    |  7 +++----
 drivers/clk/microchip/mpfs_clk_periph.c | 16 ++++------------
 5 files changed, 20 insertions(+), 35 deletions(-)

diff --git a/drivers/clk/microchip/Makefile b/drivers/clk/microchip/Makefile
index 904b345d75..329b2c0c93 100644
--- a/drivers/clk/microchip/Makefile
+++ b/drivers/clk/microchip/Makefile
@@ -1 +1 @@
-obj-y += mpfs_clk.o mpfs_clk_cfg.o mpfs_clk_periph.o
+obj-y += mpfs_clk.o mpfs_clk_cfg.o mpfs_clk_periph.o mpfs_clk_msspll.o
diff --git a/drivers/clk/microchip/mpfs_clk.c b/drivers/clk/microchip/mpfs_clk.c
index 67828c9bf4..7ba1218b56 100644
--- a/drivers/clk/microchip/mpfs_clk.c
+++ b/drivers/clk/microchip/mpfs_clk.c
@@ -11,34 +11,32 @@
 #include <dm/device.h>
 #include <dm/devres.h>
 #include <dm/uclass.h>
+#include <dt-bindings/clock/microchip-mpfs-clock.h>
 #include <linux/err.h>
 
 #include "mpfs_clk.h"
 
 static int mpfs_clk_probe(struct udevice *dev)
 {
-	int ret;
+	struct clk *parent_clk = dev_get_priv(dev);
+	struct clk clk_ahb = { .id = CLK_AHB };
 	void __iomem *base;
-	u32 clk_rate;
-	const char *parent_clk_name;
-	struct clk *clk = dev_get_priv(dev);
+	int ret;
 
 	base = dev_read_addr_ptr(dev);
 	if (!base)
 		return -EINVAL;
 
-	ret = clk_get_by_index(dev, 0, clk);
+	ret = clk_get_by_index(dev, 0, parent_clk);
 	if (ret)
 		return ret;
 
-	dev_read_u32(clk->dev, "clock-frequency", &clk_rate);
-	parent_clk_name = clk->dev->name;
-
-	ret = mpfs_clk_register_cfgs(base, clk_rate, parent_clk_name);
+	ret = mpfs_clk_register_cfgs(base, parent_clk);
 	if (ret)
 		return ret;
 
-	ret = mpfs_clk_register_periphs(base, clk_rate, "clk_ahb");
+	clk_request(dev, &clk_ahb);
+	ret = mpfs_clk_register_periphs(base, &clk_ahb);
 
 	return ret;
 }
diff --git a/drivers/clk/microchip/mpfs_clk.h b/drivers/clk/microchip/mpfs_clk.h
index 442562a5e7..35cfeac92e 100644
--- a/drivers/clk/microchip/mpfs_clk.h
+++ b/drivers/clk/microchip/mpfs_clk.h
@@ -11,22 +11,18 @@
  * mpfs_clk_register_cfgs() - register configuration clocks
  *
  * @base: base address of the mpfs system register.
- * @clk_rate: the mpfs pll clock rate.
- * @parent_name: a pointer to parent clock name.
+ * @parent: a pointer to parent clock.
  * Return: zero on success, or a negative error code.
  */
-int mpfs_clk_register_cfgs(void __iomem *base, u32 clk_rate,
-			   const char *parent_name);
+int mpfs_clk_register_cfgs(void __iomem *base, struct clk *parent);
 /**
  * mpfs_clk_register_periphs() - register peripheral clocks
  *
  * @base: base address of the mpfs system register.
- * @clk_rate: the mpfs pll clock rate.
- * @parent_name: a pointer to parent clock name.
+ * @parent: a pointer to parent clock.
  * Return: zero on success, or a negative error code.
  */
-int mpfs_clk_register_periphs(void __iomem *base, u32 clk_rate,
-			      const char *parent_name);
+int mpfs_clk_register_periphs(void __iomem *base, struct clk *parent);
 /**
  * divider_get_val() - get the clock divider value
  *
diff --git a/drivers/clk/microchip/mpfs_clk_cfg.c b/drivers/clk/microchip/mpfs_clk_cfg.c
index fefddd1413..5739fd66e8 100644
--- a/drivers/clk/microchip/mpfs_clk_cfg.c
+++ b/drivers/clk/microchip/mpfs_clk_cfg.c
@@ -117,8 +117,7 @@ static struct mpfs_cfg_hw_clock mpfs_cfg_clks[] = {
 	CLK_CFG(CLK_AHB, "clk_ahb", 4, 2, mpfs_div_ahb_table, 0),
 };
 
-int mpfs_clk_register_cfgs(void __iomem *base, u32 clk_rate,
-			   const char *parent_name)
+int mpfs_clk_register_cfgs(void __iomem *base, struct clk *parent)
 {
 	int ret;
 	int i, id, num_clks;
@@ -129,9 +128,9 @@ int mpfs_clk_register_cfgs(void __iomem *base, u32 clk_rate,
 	for (i = 0; i < num_clks; i++) {
 		hw = &mpfs_cfg_clks[i].hw;
 		mpfs_cfg_clks[i].sys_base = base;
-		mpfs_cfg_clks[i].prate = clk_rate;
+		mpfs_cfg_clks[i].prate = clk_get_rate(parent);
 		name = mpfs_cfg_clks[i].cfg.name;
-		ret = clk_register(hw, MPFS_CFG_CLOCK, name, parent_name);
+		ret = clk_register(hw, MPFS_CFG_CLOCK, name, parent->dev->name);
 		if (ret)
 			ERR_PTR(ret);
 		id = mpfs_cfg_clks[i].cfg.id;
diff --git a/drivers/clk/microchip/mpfs_clk_periph.c b/drivers/clk/microchip/mpfs_clk_periph.c
index 61d90eb4a8..1488ef503e 100644
--- a/drivers/clk/microchip/mpfs_clk_periph.c
+++ b/drivers/clk/microchip/mpfs_clk_periph.c
@@ -99,16 +99,9 @@ static int mpfs_periph_clk_disable(struct clk *hw)
 static ulong mpfs_periph_clk_recalc_rate(struct clk *hw)
 {
 	struct mpfs_periph_hw_clock *periph_hw = to_mpfs_periph_clk(hw);
-	void __iomem *base_addr = periph_hw->sys_base;
-	unsigned long rate;
-	u32 val;
 
-	val = readl(base_addr + REG_CLOCK_CONFIG_CR) >> CFG_AHB_SHIFT;
-	val &= clk_div_mask(CFG_WIDTH);
-	rate = periph_hw->prate / (1u << val);
-	hw->rate = rate;
+	return periph_hw->prate;
 
-	return rate;
 }
 
 #define CLK_PERIPH(_id, _name, _shift, _flags) {	\
@@ -150,8 +143,7 @@ static struct mpfs_periph_hw_clock mpfs_periph_clks[] = {
 	CLK_PERIPH(CLK_CFM, "clk_periph_cfm", 29, 0),
 };
 
-int mpfs_clk_register_periphs(void __iomem *base, u32 clk_rate,
-			      const char *parent_name)
+int mpfs_clk_register_periphs(void __iomem *base, struct clk *parent)
 {
 	int ret;
 	int i, id, num_clks;
@@ -162,9 +154,9 @@ int mpfs_clk_register_periphs(void __iomem *base, u32 clk_rate,
 	for (i = 0; i < num_clks; i++)  {
 		hw = &mpfs_periph_clks[i].hw;
 		mpfs_periph_clks[i].sys_base = base;
-		mpfs_periph_clks[i].prate = clk_rate;
+		mpfs_periph_clks[i].prate = clk_get_rate(parent);
 		name = mpfs_periph_clks[i].periph.name;
-		ret = clk_register(hw, MPFS_PERIPH_CLOCK, name, parent_name);
+		ret = clk_register(hw, MPFS_PERIPH_CLOCK, name, parent->dev->name);
 		if (ret)
 			ERR_PTR(ret);
 		id = mpfs_periph_clks[i].periph.id;
-- 
2.38.0

Re: [PATCH v1 2/6] clk: microchip: mpfs: convert parent rate acquistion to get_get_rate()

[Leo Liang]

On Tue, Oct 25, 2022 at 08:58:45AM +0100, Conor Dooley wrote:
> Currently the clock driver for PolarFire SoC takes a very naive approach
> to the relationship between clocks. It reads the dt to get an input
> clock, assumes that that is fixed frequency, reads the "clock-frequency"
> property & uses that to set up both the "cfg" and "periph" clocks.
> 
> Simplifying for the sake of incremental fixes, the "correct" parentage for
> the clocks currently supported in U-Boot is that the "cfg" clocks should
> be children of the fixed frequency clock in the dt. The AHB clock is one
> of these "cfg" clocks and is the parent of the "periph" clocks.
> 
> Instead of passing the clock rate of the fixed-frequency clock to the
> "cfg" and "periph" registration functions and the name of the parents,
> pass their actual parents & use clk_get_rate() to determine their parents
> rates.
> 
> The "periph" clocks are purely gate clocks and should not be reading the
> AHB clocks registers to determine their rates, as they can simply report
> the output of clk_get_rate() on their parent.
> 
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>  drivers/clk/microchip/Makefile          |  2 +-
>  drivers/clk/microchip/mpfs_clk.c        | 18 ++++++++----------
>  drivers/clk/microchip/mpfs_clk.h        | 12 ++++--------
>  drivers/clk/microchip/mpfs_clk_cfg.c    |  7 +++----
>  drivers/clk/microchip/mpfs_clk_periph.c | 16 ++++------------
>  5 files changed, 20 insertions(+), 35 deletions(-)

Reviewed-by: Leo Yu-Chi Liang <ycliang@andestech.com>

Re: [PATCH v1 2/6] clk: microchip: mpfs: convert parent rate acquistion to get_get_rate()

[Padmarao.Begari]

Hi Conor,

> On Tue, 2022-10-25 at 08:58 +0100, Conor Dooley wrote:
> Currently the clock driver for PolarFire SoC takes a very naive
> approach
> to the relationship between clocks. It reads the dt to get an input
> clock, assumes that that is fixed frequency, reads the "clock-
s/that that/that

> frequency"
> property & uses that to set up both the "cfg" and "periph" clocks.
> 
> Simplifying for the sake of incremental fixes, the "correct"
> parentage for
> the clocks currently supported in U-Boot is that the "cfg" clocks
> should
> be children of the fixed frequency clock in the dt. The AHB clock is
> one
> of these "cfg" clocks and is the parent of the "periph" clocks.
> 
> Instead of passing the clock rate of the fixed-frequency clock to the
> "cfg" and "periph" registration functions and the name of the
> parents,
> pass their actual parents & use clk_get_rate() to determine their
> parents
> rates.
> 
> The "periph" clocks are purely gate clocks and should not be reading
> the
> AHB clocks registers to determine their rates, as they can simply
> report
> the output of clk_get_rate() on their parent.
> 
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>  drivers/clk/microchip/Makefile          |  2 +-
>  drivers/clk/microchip/mpfs_clk.c        | 18 ++++++++----------
>  drivers/clk/microchip/mpfs_clk.h        | 12 ++++--------
>  drivers/clk/microchip/mpfs_clk_cfg.c    |  7 +++----
>  drivers/clk/microchip/mpfs_clk_periph.c | 16 ++++------------
>  5 files changed, 20 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/clk/microchip/Makefile
> b/drivers/clk/microchip/Makefile
> index 904b345d75..329b2c0c93 100644
> --- a/drivers/clk/microchip/Makefile
> +++ b/drivers/clk/microchip/Makefile
> @@ -1 +1 @@
> -obj-y += mpfs_clk.o mpfs_clk_cfg.o mpfs_clk_periph.o
> +obj-y += mpfs_clk.o mpfs_clk_cfg.o mpfs_clk_periph.o
> mpfs_clk_msspll.o
> diff --git a/drivers/clk/microchip/mpfs_clk.c
> b/drivers/clk/microchip/mpfs_clk.c
> index 67828c9bf4..7ba1218b56 100644
> --- a/drivers/clk/microchip/mpfs_clk.c
> +++ b/drivers/clk/microchip/mpfs_clk.c
> @@ -11,34 +11,32 @@
>  #include <dm/device.h>
>  #include <dm/devres.h>
>  #include <dm/uclass.h>
> +#include <dt-bindings/clock/microchip-mpfs-clock.h>
>  #include <linux/err.h>
>  
>  #include "mpfs_clk.h"
>  
>  static int mpfs_clk_probe(struct udevice *dev)
>  {
> -	int ret;
> +	struct clk *parent_clk = dev_get_priv(dev);
> +	struct clk clk_ahb = { .id = CLK_AHB };
The peripheral clock updated code added in this patch but removed it in
the patch 4, you can update only related code in this patch instead of
removing it later.

Other than that:
Reviewed-by: Padmarao Begari <padmarao.begari@microchip.com>

>  	void __iomem *base;
> -	u32 clk_rate;
> -	const char *parent_clk_name;
> -	struct clk *clk = dev_get_priv(dev);
> +	int ret;
>  
>  	base = dev_read_addr_ptr(dev);
>  	if (!base)
>  		return -EINVAL;
>  
> -	ret = clk_get_by_index(dev, 0, clk);
> +	ret = clk_get_by_index(dev, 0, parent_clk);
>  	if (ret)
>  		return ret;
>  
> -	dev_read_u32(clk->dev, "clock-frequency", &clk_rate);
> -	parent_clk_name = clk->dev->name;
> -
> -	ret = mpfs_clk_register_cfgs(base, clk_rate, parent_clk_name);
> +	ret = mpfs_clk_register_cfgs(base, parent_clk);
>  	if (ret)
>  		return ret;
>  
> -	ret = mpfs_clk_register_periphs(base, clk_rate, "clk_ahb");
> +	clk_request(dev, &clk_ahb);
> +	ret = mpfs_clk_register_periphs(base, &clk_ahb);
>  
>  	return ret;
>  }
> diff --git a/drivers/clk/microchip/mpfs_clk.h
> b/drivers/clk/microchip/mpfs_clk.h
> index 442562a5e7..35cfeac92e 100644
> --- a/drivers/clk/microchip/mpfs_clk.h
> +++ b/drivers/clk/microchip/mpfs_clk.h
> @@ -11,22 +11,18 @@
>   * mpfs_clk_register_cfgs() - register configuration clocks
>   *
>   * @base: base address of the mpfs system register.
> - * @clk_rate: the mpfs pll clock rate.
> - * @parent_name: a pointer to parent clock name.
> + * @parent: a pointer to parent clock.
>   * Return: zero on success, or a negative error code.
>   */
> -int mpfs_clk_register_cfgs(void __iomem *base, u32 clk_rate,
> -			   const char *parent_name);
> +int mpfs_clk_register_cfgs(void __iomem *base, struct clk *parent);
>  /**
>   * mpfs_clk_register_periphs() - register peripheral clocks
>   *
>   * @base: base address of the mpfs system register.
> - * @clk_rate: the mpfs pll clock rate.
> - * @parent_name: a pointer to parent clock name.
> + * @parent: a pointer to parent clock.
>   * Return: zero on success, or a negative error code.
>   */
> -int mpfs_clk_register_periphs(void __iomem *base, u32 clk_rate,
> -			      const char *parent_name);
> +int mpfs_clk_register_periphs(void __iomem *base, struct clk
> *parent);
>  /**
>   * divider_get_val() - get the clock divider value
>   *
> diff --git a/drivers/clk/microchip/mpfs_clk_cfg.c
> b/drivers/clk/microchip/mpfs_clk_cfg.c
> index fefddd1413..5739fd66e8 100644
> --- a/drivers/clk/microchip/mpfs_clk_cfg.c
> +++ b/drivers/clk/microchip/mpfs_clk_cfg.c
> @@ -117,8 +117,7 @@ static struct mpfs_cfg_hw_clock mpfs_cfg_clks[] =
> {
>  	CLK_CFG(CLK_AHB, "clk_ahb", 4, 2, mpfs_div_ahb_table, 0),
>  };
>  
> -int mpfs_clk_register_cfgs(void __iomem *base, u32 clk_rate,
> -			   const char *parent_name)
> +int mpfs_clk_register_cfgs(void __iomem *base, struct clk *parent)
>  {
>  	int ret;
>  	int i, id, num_clks;
> @@ -129,9 +128,9 @@ int mpfs_clk_register_cfgs(void __iomem *base,
> u32 clk_rate,
>  	for (i = 0; i < num_clks; i++) {
>  		hw = &mpfs_cfg_clks[i].hw;
>  		mpfs_cfg_clks[i].sys_base = base;
> -		mpfs_cfg_clks[i].prate = clk_rate;
> +		mpfs_cfg_clks[i].prate = clk_get_rate(parent);
>  		name = mpfs_cfg_clks[i].cfg.name;
> -		ret = clk_register(hw, MPFS_CFG_CLOCK, name,
> parent_name);
> +		ret = clk_register(hw, MPFS_CFG_CLOCK, name, parent-
> >dev->name);
>  		if (ret)
>  			ERR_PTR(ret);
>  		id = mpfs_cfg_clks[i].cfg.id;
> diff --git a/drivers/clk/microchip/mpfs_clk_periph.c
> b/drivers/clk/microchip/mpfs_clk_periph.c
> index 61d90eb4a8..1488ef503e 100644
> --- a/drivers/clk/microchip/mpfs_clk_periph.c
> +++ b/drivers/clk/microchip/mpfs_clk_periph.c
> @@ -99,16 +99,9 @@ static int mpfs_periph_clk_disable(struct clk *hw)
>  static ulong mpfs_periph_clk_recalc_rate(struct clk *hw)
>  {
>  	struct mpfs_periph_hw_clock *periph_hw =
> to_mpfs_periph_clk(hw);
> -	void __iomem *base_addr = periph_hw->sys_base;
> -	unsigned long rate;
> -	u32 val;
>  
> -	val = readl(base_addr + REG_CLOCK_CONFIG_CR) >> CFG_AHB_SHIFT;
> -	val &= clk_div_mask(CFG_WIDTH);
> -	rate = periph_hw->prate / (1u << val);
> -	hw->rate = rate;
> +	return periph_hw->prate;
>  
> -	return rate;
>  }
>  
>  #define CLK_PERIPH(_id, _name, _shift, _flags) {	\
> @@ -150,8 +143,7 @@ static struct mpfs_periph_hw_clock
> mpfs_periph_clks[] = {
>  	CLK_PERIPH(CLK_CFM, "clk_periph_cfm", 29, 0),
>  };
>  
> -int mpfs_clk_register_periphs(void __iomem *base, u32 clk_rate,
> -			      const char *parent_name)
> +int mpfs_clk_register_periphs(void __iomem *base, struct clk
> *parent)
>  {
>  	int ret;
>  	int i, id, num_clks;
> @@ -162,9 +154,9 @@ int mpfs_clk_register_periphs(void __iomem *base,
> u32 clk_rate,
>  	for (i = 0; i < num_clks; i++)  {
>  		hw = &mpfs_periph_clks[i].hw;
>  		mpfs_periph_clks[i].sys_base = base;
> -		mpfs_periph_clks[i].prate = clk_rate;
> +		mpfs_periph_clks[i].prate = clk_get_rate(parent);
>  		name = mpfs_periph_clks[i].periph.name;
> -		ret = clk_register(hw, MPFS_PERIPH_CLOCK, name,
> parent_name);
> +		ret = clk_register(hw, MPFS_PERIPH_CLOCK, name, parent-
> >dev->name);
>  		if (ret)
>  			ERR_PTR(ret);
>  		id = mpfs_periph_clks[i].periph.id;

Re: [PATCH v1 2/6] clk: microchip: mpfs: convert parent rate acquistion to get_get_rate()

[Conor Dooley]

On Wed, Nov 02, 2022 at 01:20:33PM +0000, Padmarao.Begari@microchip.com wrote:
> Hi Conor,
> 
> > On Tue, 2022-10-25 at 08:58 +0100, Conor Dooley wrote:
> > Currently the clock driver for PolarFire SoC takes a very naive
> > approach
> > to the relationship between clocks. It reads the dt to get an input
> > clock, assumes that that is fixed frequency, reads the "clock-
> s/that that/that

Nope, not a typo ;)

> 
> > frequency"
> > property & uses that to set up both the "cfg" and "periph" clocks.
> > 
> > Simplifying for the sake of incremental fixes, the "correct"
> > parentage for
> > the clocks currently supported in U-Boot is that the "cfg" clocks
> > should
> > be children of the fixed frequency clock in the dt. The AHB clock is
> > one
> > of these "cfg" clocks and is the parent of the "periph" clocks.
> > 
> > Instead of passing the clock rate of the fixed-frequency clock to the
> > "cfg" and "periph" registration functions and the name of the
> > parents,
> > pass their actual parents & use clk_get_rate() to determine their
> > parents
> > rates.
> > 
> > The "periph" clocks are purely gate clocks and should not be reading
> > the
> > AHB clocks registers to determine their rates, as they can simply
> > report
> > the output of clk_get_rate() on their parent.
> > 
> > Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> > ---
> >  drivers/clk/microchip/Makefile          |  2 +-
> >  drivers/clk/microchip/mpfs_clk.c        | 18 ++++++++----------
> >  drivers/clk/microchip/mpfs_clk.h        | 12 ++++--------
> >  drivers/clk/microchip/mpfs_clk_cfg.c    |  7 +++----
> >  drivers/clk/microchip/mpfs_clk_periph.c | 16 ++++------------
> >  5 files changed, 20 insertions(+), 35 deletions(-)
> > 
> > diff --git a/drivers/clk/microchip/Makefile
> > b/drivers/clk/microchip/Makefile
> > index 904b345d75..329b2c0c93 100644
> > --- a/drivers/clk/microchip/Makefile
> > +++ b/drivers/clk/microchip/Makefile
> > @@ -1 +1 @@
> > -obj-y += mpfs_clk.o mpfs_clk_cfg.o mpfs_clk_periph.o
> > +obj-y += mpfs_clk.o mpfs_clk_cfg.o mpfs_clk_periph.o
> > mpfs_clk_msspll.o
> > diff --git a/drivers/clk/microchip/mpfs_clk.c
> > b/drivers/clk/microchip/mpfs_clk.c
> > index 67828c9bf4..7ba1218b56 100644
> > --- a/drivers/clk/microchip/mpfs_clk.c
> > +++ b/drivers/clk/microchip/mpfs_clk.c
> > @@ -11,34 +11,32 @@
> >  #include <dm/device.h>
> >  #include <dm/devres.h>
> >  #include <dm/uclass.h>
> > +#include <dt-bindings/clock/microchip-mpfs-clock.h>
> >  #include <linux/err.h>
> >  
> >  #include "mpfs_clk.h"
> >  
> >  static int mpfs_clk_probe(struct udevice *dev)
> >  {
> > -	int ret;
> > +	struct clk *parent_clk = dev_get_priv(dev);
> > +	struct clk clk_ahb = { .id = CLK_AHB };
> The peripheral clock updated code added in this patch but removed it in
> the patch 4, you can update only related code in this patch instead of
> removing it later.

Right. I did that intentionally so that each patch did the minimum
required to fix individual issues. This patch does some reorganisation
so that the follow up patches were only as minimal fixes as possible.
I'll reorganise things if you think that's the better way to go about it
though.

> 
> Other than that:
> Reviewed-by: Padmarao Begari <padmarao.begari@microchip.com>
> 
> >  	void __iomem *base;
> > -	u32 clk_rate;
> > -	const char *parent_clk_name;
> > -	struct clk *clk = dev_get_priv(dev);
> > +	int ret;
> >  
> >  	base = dev_read_addr_ptr(dev);
> >  	if (!base)
> >  		return -EINVAL;
> >  
> > -	ret = clk_get_by_index(dev, 0, clk);
> > +	ret = clk_get_by_index(dev, 0, parent_clk);
> >  	if (ret)
> >  		return ret;
> >  
> > -	dev_read_u32(clk->dev, "clock-frequency", &clk_rate);
> > -	parent_clk_name = clk->dev->name;
> > -
> > -	ret = mpfs_clk_register_cfgs(base, clk_rate, parent_clk_name);
> > +	ret = mpfs_clk_register_cfgs(base, parent_clk);
> >  	if (ret)
> >  		return ret;
> >  
> > -	ret = mpfs_clk_register_periphs(base, clk_rate, "clk_ahb");
> > +	clk_request(dev, &clk_ahb);
> > +	ret = mpfs_clk_register_periphs(base, &clk_ahb);
> >  
> >  	return ret;
> >  }
> > diff --git a/drivers/clk/microchip/mpfs_clk.h
> > b/drivers/clk/microchip/mpfs_clk.h
> > index 442562a5e7..35cfeac92e 100644
> > --- a/drivers/clk/microchip/mpfs_clk.h
> > +++ b/drivers/clk/microchip/mpfs_clk.h
> > @@ -11,22 +11,18 @@
> >   * mpfs_clk_register_cfgs() - register configuration clocks
> >   *
> >   * @base: base address of the mpfs system register.
> > - * @clk_rate: the mpfs pll clock rate.
> > - * @parent_name: a pointer to parent clock name.
> > + * @parent: a pointer to parent clock.
> >   * Return: zero on success, or a negative error code.
> >   */
> > -int mpfs_clk_register_cfgs(void __iomem *base, u32 clk_rate,
> > -			   const char *parent_name);
> > +int mpfs_clk_register_cfgs(void __iomem *base, struct clk *parent);
> >  /**
> >   * mpfs_clk_register_periphs() - register peripheral clocks
> >   *
> >   * @base: base address of the mpfs system register.
> > - * @clk_rate: the mpfs pll clock rate.
> > - * @parent_name: a pointer to parent clock name.
> > + * @parent: a pointer to parent clock.
> >   * Return: zero on success, or a negative error code.
> >   */
> > -int mpfs_clk_register_periphs(void __iomem *base, u32 clk_rate,
> > -			      const char *parent_name);
> > +int mpfs_clk_register_periphs(void __iomem *base, struct clk
> > *parent);
> >  /**
> >   * divider_get_val() - get the clock divider value
> >   *
> > diff --git a/drivers/clk/microchip/mpfs_clk_cfg.c
> > b/drivers/clk/microchip/mpfs_clk_cfg.c
> > index fefddd1413..5739fd66e8 100644
> > --- a/drivers/clk/microchip/mpfs_clk_cfg.c
> > +++ b/drivers/clk/microchip/mpfs_clk_cfg.c
> > @@ -117,8 +117,7 @@ static struct mpfs_cfg_hw_clock mpfs_cfg_clks[] =
> > {
> >  	CLK_CFG(CLK_AHB, "clk_ahb", 4, 2, mpfs_div_ahb_table, 0),
> >  };
> >  
> > -int mpfs_clk_register_cfgs(void __iomem *base, u32 clk_rate,
> > -			   const char *parent_name)
> > +int mpfs_clk_register_cfgs(void __iomem *base, struct clk *parent)
> >  {
> >  	int ret;
> >  	int i, id, num_clks;
> > @@ -129,9 +128,9 @@ int mpfs_clk_register_cfgs(void __iomem *base,
> > u32 clk_rate,
> >  	for (i = 0; i < num_clks; i++) {
> >  		hw = &mpfs_cfg_clks[i].hw;
> >  		mpfs_cfg_clks[i].sys_base = base;
> > -		mpfs_cfg_clks[i].prate = clk_rate;
> > +		mpfs_cfg_clks[i].prate = clk_get_rate(parent);
> >  		name = mpfs_cfg_clks[i].cfg.name;
> > -		ret = clk_register(hw, MPFS_CFG_CLOCK, name,
> > parent_name);
> > +		ret = clk_register(hw, MPFS_CFG_CLOCK, name, parent-
> > >dev->name);
> >  		if (ret)
> >  			ERR_PTR(ret);
> >  		id = mpfs_cfg_clks[i].cfg.id;
> > diff --git a/drivers/clk/microchip/mpfs_clk_periph.c
> > b/drivers/clk/microchip/mpfs_clk_periph.c
> > index 61d90eb4a8..1488ef503e 100644
> > --- a/drivers/clk/microchip/mpfs_clk_periph.c
> > +++ b/drivers/clk/microchip/mpfs_clk_periph.c
> > @@ -99,16 +99,9 @@ static int mpfs_periph_clk_disable(struct clk *hw)
> >  static ulong mpfs_periph_clk_recalc_rate(struct clk *hw)
> >  {
> >  	struct mpfs_periph_hw_clock *periph_hw =
> > to_mpfs_periph_clk(hw);
> > -	void __iomem *base_addr = periph_hw->sys_base;
> > -	unsigned long rate;
> > -	u32 val;
> >  
> > -	val = readl(base_addr + REG_CLOCK_CONFIG_CR) >> CFG_AHB_SHIFT;
> > -	val &= clk_div_mask(CFG_WIDTH);
> > -	rate = periph_hw->prate / (1u << val);
> > -	hw->rate = rate;
> > +	return periph_hw->prate;
> >  
> > -	return rate;
> >  }
> >  
> >  #define CLK_PERIPH(_id, _name, _shift, _flags) {	\
> > @@ -150,8 +143,7 @@ static struct mpfs_periph_hw_clock
> > mpfs_periph_clks[] = {
> >  	CLK_PERIPH(CLK_CFM, "clk_periph_cfm", 29, 0),
> >  };
> >  
> > -int mpfs_clk_register_periphs(void __iomem *base, u32 clk_rate,
> > -			      const char *parent_name)
> > +int mpfs_clk_register_periphs(void __iomem *base, struct clk
> > *parent)
> >  {
> >  	int ret;
> >  	int i, id, num_clks;
> > @@ -162,9 +154,9 @@ int mpfs_clk_register_periphs(void __iomem *base,
> > u32 clk_rate,
> >  	for (i = 0; i < num_clks; i++)  {
> >  		hw = &mpfs_periph_clks[i].hw;
> >  		mpfs_periph_clks[i].sys_base = base;
> > -		mpfs_periph_clks[i].prate = clk_rate;
> > +		mpfs_periph_clks[i].prate = clk_get_rate(parent);
> >  		name = mpfs_periph_clks[i].periph.name;
> > -		ret = clk_register(hw, MPFS_PERIPH_CLOCK, name,
> > parent_name);
> > +		ret = clk_register(hw, MPFS_PERIPH_CLOCK, name, parent-
> > >dev->name);
> >  		if (ret)
> >  			ERR_PTR(ret);
> >  		id = mpfs_periph_clks[i].periph.id;

[PATCH v1 3/6] clk: microchip: mpfs: fix reference clock handling

[Conor Dooley]

The original devicetrees for PolarFire SoC messed up & defined the
msspll's output as a fixed-frequency, 600 MHz clock & used that as the
input for the clock controller node. The msspll is not a fixed
frequency clock and later devicetrees handled this properly. Check the
devicetree & if it is one of the fixed ones, register the msspll.
Otherwise, skip registering it & pass the reference clock directly to
the cfg clock registration function so that existing devicetrees are
not broken by this change.

As the MSS PLL is not a "cfg" or a "periph" clock, add a new driver for
it, based on the one in Linux.

Fixes: 2f27c9219e ("clk: Add Microchip PolarFire SoC clock driver")
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 drivers/clk/microchip/mpfs_clk.c        |  23 ++++-
 drivers/clk/microchip/mpfs_clk.h        |   8 ++
 drivers/clk/microchip/mpfs_clk_msspll.c | 119 ++++++++++++++++++++++++
 3 files changed, 149 insertions(+), 1 deletion(-)
 create mode 100644 drivers/clk/microchip/mpfs_clk_msspll.c

diff --git a/drivers/clk/microchip/mpfs_clk.c b/drivers/clk/microchip/mpfs_clk.c
index 7ba1218b56..f16f716f00 100644
--- a/drivers/clk/microchip/mpfs_clk.c
+++ b/drivers/clk/microchip/mpfs_clk.c
@@ -20,10 +20,12 @@ static int mpfs_clk_probe(struct udevice *dev)
 {
 	struct clk *parent_clk = dev_get_priv(dev);
 	struct clk clk_ahb = { .id = CLK_AHB };
+	struct clk clk_msspll = { .id = CLK_MSSPLL };
 	void __iomem *base;
+	void __iomem *msspll_base;
 	int ret;
 
-	base = dev_read_addr_ptr(dev);
+	base = dev_read_addr_index_ptr(dev, 0);
 	if (!base)
 		return -EINVAL;
 
@@ -31,6 +33,25 @@ static int mpfs_clk_probe(struct udevice *dev)
 	if (ret)
 		return ret;
 
+	/*
+	 * The original devicetrees for mpfs messed up & defined the msspll's
+	 * output as a fixed-frequency, 600 MHz clock & used that as the input
+	 * for the clock controller node. The msspll is however not a fixed
+	 * frequency clock and later devicetrees handled this properly. Check
+	 * the devicetree & if it is one of the fixed ones, register the msspll.
+	 * Otherwise, skip registering it & pass the reference clock directly
+	 * to the cfg clock registration function.
+	 */
+	msspll_base = dev_read_addr_index_ptr(dev, 1);
+	if (msspll_base) {
+		ret = mpfs_clk_register_msspll(msspll_base, parent_clk);
+		if (ret)
+			return ret;
+
+		clk_request(dev, &clk_msspll);
+		parent_clk = &clk_msspll;
+	}
+
 	ret = mpfs_clk_register_cfgs(base, parent_clk);
 	if (ret)
 		return ret;
diff --git a/drivers/clk/microchip/mpfs_clk.h b/drivers/clk/microchip/mpfs_clk.h
index 35cfeac92e..cb7d303e67 100644
--- a/drivers/clk/microchip/mpfs_clk.h
+++ b/drivers/clk/microchip/mpfs_clk.h
@@ -15,6 +15,14 @@
  * Return: zero on success, or a negative error code.
  */
 int mpfs_clk_register_cfgs(void __iomem *base, struct clk *parent);
+/**
+ * mpfs_clk_register_msspll() - register the mss pll
+ *
+ * @base: base address of the mpfs system register.
+ * @parent: a pointer to parent clock.
+ * Return: zero on success, or a negative error code.
+ */
+int mpfs_clk_register_msspll(void __iomem *base, struct clk *parent);
 /**
  * mpfs_clk_register_periphs() - register peripheral clocks
  *
diff --git a/drivers/clk/microchip/mpfs_clk_msspll.c b/drivers/clk/microchip/mpfs_clk_msspll.c
new file mode 100644
index 0000000000..f37c0d8604
--- /dev/null
+++ b/drivers/clk/microchip/mpfs_clk_msspll.c
@@ -0,0 +1,119 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2022 Microchip Technology Inc.
+ */
+#include <common.h>
+#include <clk.h>
+#include <clk-uclass.h>
+#include <asm/io.h>
+#include <dm/device.h>
+#include <dm/devres.h>
+#include <dm/uclass.h>
+#include <dt-bindings/clock/microchip-mpfs-clock.h>
+#include <linux/err.h>
+
+#include "mpfs_clk.h"
+
+#define MPFS_MSSPLL_CLOCK "mpfs_msspll_clock"
+
+/* address offset of control registers */
+#define REG_MSSPLL_REF_CR	0x08u
+#define REG_MSSPLL_POSTDIV_CR	0x10u
+#define REG_MSSPLL_SSCG_2_CR	0x2Cu
+
+#define MSSPLL_FBDIV_SHIFT	0x00u
+#define MSSPLL_FBDIV_WIDTH	0x0Cu
+#define MSSPLL_REFDIV_SHIFT	0x08u
+#define MSSPLL_REFDIV_WIDTH	0x06u
+#define MSSPLL_POSTDIV_SHIFT	0x08u
+#define MSSPLL_POSTDIV_WIDTH	0x07u
+#define MSSPLL_FIXED_DIV	4u
+
+/**
+ * struct mpfs_msspll_hw_clock
+ * @id: index of the msspll clock
+ * @name: the msspll clocks name
+ * @reg_offset: offset to the core complex's output of the msspll
+ * @shift: shift to the divider bit field of a msspll clock output
+ * @width: width of the divider bit field of the msspll clock output
+ * @flags: common clock framework flags
+ * @prate: the reference clock rate
+ * @hw: clock instance
+ */
+struct mpfs_msspll_hw_clock {
+	void __iomem *base;
+	unsigned int id;
+	const char *name;
+	u32 reg_offset;
+	u32 shift;
+	u32 width;
+	u32 flags;
+	u32 prate;
+	struct clk hw;
+};
+
+#define to_mpfs_msspll_clk(_hw) container_of(_hw, struct mpfs_msspll_hw_clock, hw)
+
+static unsigned long mpfs_clk_msspll_recalc_rate(struct clk *hw)
+{
+	struct mpfs_msspll_hw_clock *msspll_hw = to_mpfs_msspll_clk(hw);
+	void __iomem *mult_addr = msspll_hw->base + msspll_hw->reg_offset;
+	void __iomem *ref_div_addr = msspll_hw->base + REG_MSSPLL_REF_CR;
+	void __iomem *postdiv_addr = msspll_hw->base + REG_MSSPLL_POSTDIV_CR;
+	u32 mult, ref_div, postdiv;
+	unsigned long temp;
+
+	mult = readl(mult_addr) >> MSSPLL_FBDIV_SHIFT;
+	mult &= clk_div_mask(MSSPLL_FBDIV_WIDTH);
+	ref_div = readl(ref_div_addr) >> MSSPLL_REFDIV_SHIFT;
+	ref_div &= clk_div_mask(MSSPLL_REFDIV_WIDTH);
+	postdiv = readl(postdiv_addr) >> MSSPLL_POSTDIV_SHIFT;
+	postdiv &= clk_div_mask(MSSPLL_POSTDIV_WIDTH);
+
+	temp = msspll_hw->prate / (ref_div * MSSPLL_FIXED_DIV * postdiv);
+	return temp * mult;
+}
+
+#define CLK_PLL(_id, _name, _shift, _width, _reg_offset, _flags) {	\
+	.id = _id,							\
+	.name = _name,							\
+	.shift = _shift,						\
+	.width = _width,						\
+	.reg_offset = _reg_offset,					\
+	.flags = _flags,						\
+}
+
+static struct mpfs_msspll_hw_clock mpfs_msspll_clks[] = {
+	CLK_PLL(CLK_MSSPLL, "clk_msspll", MSSPLL_FBDIV_SHIFT,
+		MSSPLL_FBDIV_WIDTH, REG_MSSPLL_SSCG_2_CR, 0),
+};
+
+int mpfs_clk_register_msspll(void __iomem *base, struct clk *parent)
+{
+	int id, ret;
+	const char *name;
+	struct clk *hw;
+
+	hw = &mpfs_msspll_clks[0].hw;
+	mpfs_msspll_clks[0].base = base;
+	mpfs_msspll_clks[0].prate = clk_get_rate(parent);
+	name = mpfs_msspll_clks[0].name;
+	ret = clk_register(hw, MPFS_MSSPLL_CLOCK, name, parent->dev->name);
+	if (ret)
+		ERR_PTR(ret);
+	id = mpfs_msspll_clks[0].id;
+	clk_dm(id, hw);
+
+	return 0;
+}
+
+const struct clk_ops mpfs_msspll_clk_ops = {
+	.get_rate = mpfs_clk_msspll_recalc_rate,
+};
+
+U_BOOT_DRIVER(mpfs_msspll_clock) = {
+	.name	= MPFS_MSSPLL_CLOCK,
+	.id	= UCLASS_CLK,
+	.ops	= &mpfs_msspll_clk_ops,
+};
+
-- 
2.38.0

Re: [PATCH v1 3/6] clk: microchip: mpfs: fix reference clock handling

[Leo Liang]

On Tue, Oct 25, 2022 at 08:58:46AM +0100, Conor Dooley wrote:
> The original devicetrees for PolarFire SoC messed up & defined the
> msspll's output as a fixed-frequency, 600 MHz clock & used that as the
> input for the clock controller node. The msspll is not a fixed
> frequency clock and later devicetrees handled this properly. Check the
> devicetree & if it is one of the fixed ones, register the msspll.
> Otherwise, skip registering it & pass the reference clock directly to
> the cfg clock registration function so that existing devicetrees are
> not broken by this change.
> 
> As the MSS PLL is not a "cfg" or a "periph" clock, add a new driver for
> it, based on the one in Linux.
> 
> Fixes: 2f27c9219e ("clk: Add Microchip PolarFire SoC clock driver")
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>  drivers/clk/microchip/mpfs_clk.c        |  23 ++++-
>  drivers/clk/microchip/mpfs_clk.h        |   8 ++
>  drivers/clk/microchip/mpfs_clk_msspll.c | 119 ++++++++++++++++++++++++
>  3 files changed, 149 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/clk/microchip/mpfs_clk_msspll.c

Reviewed-by: Leo Yu-Chi Liang <ycliang@andestech.com>

Re: [PATCH v1 3/6] clk: microchip: mpfs: fix reference clock handling

[Padmarao.Begari]

> On Tue, 2022-10-25 at 08:58 +0100, Conor Dooley wrote:
> The original devicetrees for PolarFire SoC messed up & defined the
> msspll's output as a fixed-frequency, 600 MHz clock & used that as
> the
> input for the clock controller node. The msspll is not a fixed
> frequency clock and later devicetrees handled this properly. Check
> the
> devicetree & if it is one of the fixed ones, register the msspll.
> Otherwise, skip registering it & pass the reference clock directly to
> the cfg clock registration function so that existing devicetrees are
> not broken by this change.
> 
> As the MSS PLL is not a "cfg" or a "periph" clock, add a new driver
> for
> it, based on the one in Linux.
> 
> Fixes: 2f27c9219e ("clk: Add Microchip PolarFire SoC clock driver")
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>  drivers/clk/microchip/mpfs_clk.c        |  23 ++++-
>  drivers/clk/microchip/mpfs_clk.h        |   8 ++
>  drivers/clk/microchip/mpfs_clk_msspll.c | 119
> ++++++++++++++++++++++++
>  3 files changed, 149 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/clk/microchip/mpfs_clk_msspll.c
> 
> diff --git a/drivers/clk/microchip/mpfs_clk.c
> b/drivers/clk/microchip/mpfs_clk.c
> index 7ba1218b56..f16f716f00 100644
> --- a/drivers/clk/microchip/mpfs_clk.c
> +++ b/drivers/clk/microchip/mpfs_clk.c
> @@ -20,10 +20,12 @@ static int mpfs_clk_probe(struct udevice *dev)
>  {
>  	struct clk *parent_clk = dev_get_priv(dev);
>  	struct clk clk_ahb = { .id = CLK_AHB };
> +	struct clk clk_msspll = { .id = CLK_MSSPLL };
>  	void __iomem *base;
> +	void __iomem *msspll_base;
>  	int ret;
>  
> -	base = dev_read_addr_ptr(dev);
> +	base = dev_read_addr_index_ptr(dev, 0);
>  	if (!base)
>  		return -EINVAL;
>  
> @@ -31,6 +33,25 @@ static int mpfs_clk_probe(struct udevice *dev)
>  	if (ret)
>  		return ret;
>  
> +	/*
> +	 * The original devicetrees for mpfs messed up & defined the
> msspll's
> +	 * output as a fixed-frequency, 600 MHz clock & used that as
> the input
> +	 * for the clock controller node. The msspll is however not a
> fixed
> +	 * frequency clock and later devicetrees handled this properly.
> Check
> +	 * the devicetree & if it is one of the fixed ones, register
> the msspll.
> +	 * Otherwise, skip registering it & pass the reference clock
> directly
> +	 * to the cfg clock registration function.
> +	 */
> +	msspll_base = dev_read_addr_index_ptr(dev, 1);
> +	if (msspll_base) {
> +		ret = mpfs_clk_register_msspll(msspll_base,
> parent_clk);
> +		if (ret)
> +			return ret;
> +
> +		clk_request(dev, &clk_msspll);
> +		parent_clk = &clk_msspll;
> +	}
> +
>  	ret = mpfs_clk_register_cfgs(base, parent_clk);
>  	if (ret)
>  		return ret;
> diff --git a/drivers/clk/microchip/mpfs_clk.h
> b/drivers/clk/microchip/mpfs_clk.h
> index 35cfeac92e..cb7d303e67 100644
> --- a/drivers/clk/microchip/mpfs_clk.h
> +++ b/drivers/clk/microchip/mpfs_clk.h
> @@ -15,6 +15,14 @@
>   * Return: zero on success, or a negative error code.
>   */
>  int mpfs_clk_register_cfgs(void __iomem *base, struct clk *parent);
> +/**
> + * mpfs_clk_register_msspll() - register the mss pll
> + *
> + * @base: base address of the mpfs system register.
> + * @parent: a pointer to parent clock.
> + * Return: zero on success, or a negative error code.
> + */
> +int mpfs_clk_register_msspll(void __iomem *base, struct clk
> *parent);
>  /**
>   * mpfs_clk_register_periphs() - register peripheral clocks
>   *
> diff --git a/drivers/clk/microchip/mpfs_clk_msspll.c
> b/drivers/clk/microchip/mpfs_clk_msspll.c
> new file mode 100644
> index 0000000000..f37c0d8604
> --- /dev/null
> +++ b/drivers/clk/microchip/mpfs_clk_msspll.c
> @@ -0,0 +1,119 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2022 Microchip Technology Inc.
> + */
> +#include <common.h>
> +#include <clk.h>
> +#include <clk-uclass.h>
> +#include <asm/io.h>
> +#include <dm/device.h>
> +#include <dm/devres.h>
> +#include <dm/uclass.h>
> +#include <dt-bindings/clock/microchip-mpfs-clock.h>
> +#include <linux/err.h>
> +
> +#include "mpfs_clk.h"
> +
> +#define MPFS_MSSPLL_CLOCK "mpfs_msspll_clock"
> +
> +/* address offset of control registers */
> +#define REG_MSSPLL_REF_CR	0x08u
> +#define REG_MSSPLL_POSTDIV_CR	0x10u
> +#define REG_MSSPLL_SSCG_2_CR	0x2Cu
> +
> +#define MSSPLL_FBDIV_SHIFT	0x00u
> +#define MSSPLL_FBDIV_WIDTH	0x0Cu
> +#define MSSPLL_REFDIV_SHIFT	0x08u
> +#define MSSPLL_REFDIV_WIDTH	0x06u
> +#define MSSPLL_POSTDIV_SHIFT	0x08u
> +#define MSSPLL_POSTDIV_WIDTH	0x07u
> +#define MSSPLL_FIXED_DIV	4u
> +
> +/**
> + * struct mpfs_msspll_hw_clock
> + * @id: index of the msspll clock
> + * @name: the msspll clocks name
> + * @reg_offset: offset to the core complex's output of the msspll
> + * @shift: shift to the divider bit field of a msspll clock output
> + * @width: width of the divider bit field of the msspll clock output
> + * @flags: common clock framework flags
> + * @prate: the reference clock rate
> + * @hw: clock instance
> + */
> +struct mpfs_msspll_hw_clock {
> +	void __iomem *base;
> +	unsigned int id;
> +	const char *name;
> +	u32 reg_offset;
> +	u32 shift;
> +	u32 width;
> +	u32 flags;
> +	u32 prate;
> +	struct clk hw;
> +};
> +
> +#define to_mpfs_msspll_clk(_hw) container_of(_hw, struct
> mpfs_msspll_hw_clock, hw)
> +
> +static unsigned long mpfs_clk_msspll_recalc_rate(struct clk *hw)
> +{
> +	struct mpfs_msspll_hw_clock *msspll_hw =
> to_mpfs_msspll_clk(hw);
> +	void __iomem *mult_addr = msspll_hw->base + msspll_hw-
> >reg_offset;
> +	void __iomem *ref_div_addr = msspll_hw->base +
> REG_MSSPLL_REF_CR;
> +	void __iomem *postdiv_addr = msspll_hw->base +
> REG_MSSPLL_POSTDIV_CR;
> +	u32 mult, ref_div, postdiv;
> +	unsigned long temp;
> +
> +	mult = readl(mult_addr) >> MSSPLL_FBDIV_SHIFT;
> +	mult &= clk_div_mask(MSSPLL_FBDIV_WIDTH);
> +	ref_div = readl(ref_div_addr) >> MSSPLL_REFDIV_SHIFT;
> +	ref_div &= clk_div_mask(MSSPLL_REFDIV_WIDTH);
> +	postdiv = readl(postdiv_addr) >> MSSPLL_POSTDIV_SHIFT;
> +	postdiv &= clk_div_mask(MSSPLL_POSTDIV_WIDTH);
> +
> +	temp = msspll_hw->prate / (ref_div * MSSPLL_FIXED_DIV *
> postdiv);
> +	return temp * mult;
> +}
> +
> +#define CLK_PLL(_id, _name, _shift, _width, _reg_offset, _flags) {	
> \
> +	.id = _id,							
> \
> +	.name = _name,							
> \
> +	.shift = _shift,						\
> +	.width = _width,						\
> +	.reg_offset = _reg_offset,					
> \
> +	.flags = _flags,						\
> +}
> +
> +static struct mpfs_msspll_hw_clock mpfs_msspll_clks[] = {
> +	CLK_PLL(CLK_MSSPLL, "clk_msspll", MSSPLL_FBDIV_SHIFT,
> +		MSSPLL_FBDIV_WIDTH, REG_MSSPLL_SSCG_2_CR, 0),
> +};
> +
> +int mpfs_clk_register_msspll(void __iomem *base, struct clk *parent)
> +{
> +	int id, ret;
> +	const char *name;
> +	struct clk *hw;
> +
> +	hw = &mpfs_msspll_clks[0].hw;
> +	mpfs_msspll_clks[0].base = base;
> +	mpfs_msspll_clks[0].prate = clk_get_rate(parent);
> +	name = mpfs_msspll_clks[0].name;
> +	ret = clk_register(hw, MPFS_MSSPLL_CLOCK, name, parent->dev-
> >name);
> +	if (ret)
> +		ERR_PTR(ret);
> +	id = mpfs_msspll_clks[0].id;
> +	clk_dm(id, hw);
> +
> +	return 0;
> +}
> +
> +const struct clk_ops mpfs_msspll_clk_ops = {
> +	.get_rate = mpfs_clk_msspll_recalc_rate,
> +};
> +
> +U_BOOT_DRIVER(mpfs_msspll_clock) = {
> +	.name	= MPFS_MSSPLL_CLOCK,
> +	.id	= UCLASS_CLK,
> +	.ops	= &mpfs_msspll_clk_ops,
> +};
> +
Remove new blank line at EOF

Other than that:
Reviewed-by: Padmarao Begari <padmarao.begari@microchip.com>
Tested-by: Padmarao Begari <padmarao.begari@microchip.com>

[PATCH v1 4/6] clk: microchip: mpfs: fix periph clk parentage

[Conor Dooley]

Not all "periph" clocks are children of the AHB clock, some have the AXI
clock as their parent & the mtimer clock is derived from the external
reference clock directly. Stop assuming the AHB clock to be the parent
of all "periph" clocks and define their correct parents instead.

Fixes: 2f27c9219e ("clk: Add Microchip PolarFire SoC clock driver")
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 drivers/clk/microchip/mpfs_clk.c        |  4 +-
 drivers/clk/microchip/mpfs_clk.h        |  4 +-
 drivers/clk/microchip/mpfs_clk_periph.c | 72 +++++++++++++------------
 3 files changed, 42 insertions(+), 38 deletions(-)

diff --git a/drivers/clk/microchip/mpfs_clk.c b/drivers/clk/microchip/mpfs_clk.c
index f16f716f00..08f8bfcecb 100644
--- a/drivers/clk/microchip/mpfs_clk.c
+++ b/drivers/clk/microchip/mpfs_clk.c
@@ -19,7 +19,6 @@
 static int mpfs_clk_probe(struct udevice *dev)
 {
 	struct clk *parent_clk = dev_get_priv(dev);
-	struct clk clk_ahb = { .id = CLK_AHB };
 	struct clk clk_msspll = { .id = CLK_MSSPLL };
 	void __iomem *base;
 	void __iomem *msspll_base;
@@ -56,8 +55,7 @@ static int mpfs_clk_probe(struct udevice *dev)
 	if (ret)
 		return ret;
 
-	clk_request(dev, &clk_ahb);
-	ret = mpfs_clk_register_periphs(base, &clk_ahb);
+	ret = mpfs_clk_register_periphs(base, dev);
 
 	return ret;
 }
diff --git a/drivers/clk/microchip/mpfs_clk.h b/drivers/clk/microchip/mpfs_clk.h
index cb7d303e67..72288cc971 100644
--- a/drivers/clk/microchip/mpfs_clk.h
+++ b/drivers/clk/microchip/mpfs_clk.h
@@ -27,10 +27,10 @@ int mpfs_clk_register_msspll(void __iomem *base, struct clk *parent);
  * mpfs_clk_register_periphs() - register peripheral clocks
  *
  * @base: base address of the mpfs system register.
- * @parent: a pointer to parent clock.
+ * @dev: udevice representing the clock controller.
  * Return: zero on success, or a negative error code.
  */
-int mpfs_clk_register_periphs(void __iomem *base, struct clk *parent);
+int mpfs_clk_register_periphs(void __iomem *base, struct udevice *dev);
 /**
  * divider_get_val() - get the clock divider value
  *
diff --git a/drivers/clk/microchip/mpfs_clk_periph.c b/drivers/clk/microchip/mpfs_clk_periph.c
index 1488ef503e..e23eb552c3 100644
--- a/drivers/clk/microchip/mpfs_clk_periph.c
+++ b/drivers/clk/microchip/mpfs_clk_periph.c
@@ -29,12 +29,14 @@
 /**
  * struct mpfs_periph_clock - per instance of peripheral clock
  * @id: index of a peripheral clock
+ * @parent_id: index of the parent clock
  * @name: name of a peripheral clock
  * @shift: shift to a peripheral clock bit field
  * @flags: common clock framework flags
  */
 struct mpfs_periph_clock {
 	unsigned int id;
+	unsigned int parent_id;
 	const char *name;
 	u8 shift;
 	unsigned long flags;
@@ -104,46 +106,47 @@ static ulong mpfs_periph_clk_recalc_rate(struct clk *hw)
 
 }
 
-#define CLK_PERIPH(_id, _name, _shift, _flags) {	\
+#define CLK_PERIPH(_id, _name, _parent_id, _shift, _flags) {	\
 		.periph.id = _id,			\
+		.periph.parent_id = _parent_id,		\
 		.periph.name = _name,			\
 		.periph.shift = _shift,			\
 		.periph.flags = _flags,			\
 	}
 
 static struct mpfs_periph_hw_clock mpfs_periph_clks[] = {
-	CLK_PERIPH(CLK_ENVM, "clk_periph_envm", 0, CLK_IS_CRITICAL),
-	CLK_PERIPH(CLK_MAC0, "clk_periph_mac0", 1, 0),
-	CLK_PERIPH(CLK_MAC1, "clk_periph_mac1", 2, 0),
-	CLK_PERIPH(CLK_MMC, "clk_periph_mmc", 3, 0),
-	CLK_PERIPH(CLK_TIMER, "clk_periph_timer", 4, 0),
-	CLK_PERIPH(CLK_MMUART0, "clk_periph_mmuart0", 5, 0),
-	CLK_PERIPH(CLK_MMUART1, "clk_periph_mmuart1", 6, 0),
-	CLK_PERIPH(CLK_MMUART2, "clk_periph_mmuart2", 7, 0),
-	CLK_PERIPH(CLK_MMUART3, "clk_periph_mmuart3", 8, 0),
-	CLK_PERIPH(CLK_MMUART4, "clk_periph_mmuart4", 9, 0),
-	CLK_PERIPH(CLK_SPI0, "clk_periph_spi0", 10, 0),
-	CLK_PERIPH(CLK_SPI1, "clk_periph_spi1", 11, 0),
-	CLK_PERIPH(CLK_I2C0, "clk_periph_i2c0", 12, 0),
-	CLK_PERIPH(CLK_I2C1, "clk_periph_i2c1", 13, 0),
-	CLK_PERIPH(CLK_CAN0, "clk_periph_can0", 14, 0),
-	CLK_PERIPH(CLK_CAN1, "clk_periph_can1", 15, 0),
-	CLK_PERIPH(CLK_USB, "clk_periph_usb", 16, 0),
-	CLK_PERIPH(CLK_RTC, "clk_periph_rtc", 18, 0),
-	CLK_PERIPH(CLK_QSPI, "clk_periph_qspi", 19, 0),
-	CLK_PERIPH(CLK_GPIO0, "clk_periph_gpio0", 20, 0),
-	CLK_PERIPH(CLK_GPIO1, "clk_periph_gpio1", 21, 0),
-	CLK_PERIPH(CLK_GPIO2, "clk_periph_gpio2", 22, 0),
-	CLK_PERIPH(CLK_DDRC, "clk_periph_ddrc", 23, CLK_IS_CRITICAL),
-	CLK_PERIPH(CLK_FIC0, "clk_periph_fic0", 24, 0),
-	CLK_PERIPH(CLK_FIC1, "clk_periph_fic1", 25, 0),
-	CLK_PERIPH(CLK_FIC2, "clk_periph_fic2", 26, 0),
-	CLK_PERIPH(CLK_FIC3, "clk_periph_fic3", 27, 0),
-	CLK_PERIPH(CLK_ATHENA, "clk_periph_athena", 28, 0),
-	CLK_PERIPH(CLK_CFM, "clk_periph_cfm", 29, 0),
+	CLK_PERIPH(CLK_ENVM, "clk_periph_envm", CLK_AHB, 0, CLK_IS_CRITICAL),
+	CLK_PERIPH(CLK_MAC0, "clk_periph_mac0", CLK_AHB, 1, 0),
+	CLK_PERIPH(CLK_MAC1, "clk_periph_mac1", CLK_AHB, 2, 0),
+	CLK_PERIPH(CLK_MMC, "clk_periph_mmc", CLK_AHB, 3, 0),
+	CLK_PERIPH(CLK_TIMER, "clk_periph_timer", CLK_RTCREF, 4, 0),
+	CLK_PERIPH(CLK_MMUART0, "clk_periph_mmuart0", CLK_AHB, 5, 0),
+	CLK_PERIPH(CLK_MMUART1, "clk_periph_mmuart1", CLK_AHB, 6, 0),
+	CLK_PERIPH(CLK_MMUART2, "clk_periph_mmuart2", CLK_AHB, 7, 0),
+	CLK_PERIPH(CLK_MMUART3, "clk_periph_mmuart3", CLK_AHB, 8, 0),
+	CLK_PERIPH(CLK_MMUART4, "clk_periph_mmuart4", CLK_AHB, 9, 0),
+	CLK_PERIPH(CLK_SPI0, "clk_periph_spi0", CLK_AHB, 10, 0),
+	CLK_PERIPH(CLK_SPI1, "clk_periph_spi1", CLK_AHB, 11, 0),
+	CLK_PERIPH(CLK_I2C0, "clk_periph_i2c0", CLK_AHB, 12, 0),
+	CLK_PERIPH(CLK_I2C1, "clk_periph_i2c1", CLK_AHB, 13, 0),
+	CLK_PERIPH(CLK_CAN0, "clk_periph_can0", CLK_AHB, 14, 0),
+	CLK_PERIPH(CLK_CAN1, "clk_periph_can1", CLK_AHB, 15, 0),
+	CLK_PERIPH(CLK_USB, "clk_periph_usb", CLK_AHB, 16, 0),
+	CLK_PERIPH(CLK_RTC, "clk_periph_rtc", CLK_AHB, 18, 0),
+	CLK_PERIPH(CLK_QSPI, "clk_periph_qspi", CLK_AHB, 19, 0),
+	CLK_PERIPH(CLK_GPIO0, "clk_periph_gpio0", CLK_AHB, 20, 0),
+	CLK_PERIPH(CLK_GPIO1, "clk_periph_gpio1", CLK_AHB, 21, 0),
+	CLK_PERIPH(CLK_GPIO2, "clk_periph_gpio2", CLK_AHB, 22, 0),
+	CLK_PERIPH(CLK_DDRC, "clk_periph_ddrc", CLK_AHB, 23, CLK_IS_CRITICAL),
+	CLK_PERIPH(CLK_FIC0, "clk_periph_fic0", CLK_AXI, 24, 0),
+	CLK_PERIPH(CLK_FIC1, "clk_periph_fic1", CLK_AXI, 25, 0),
+	CLK_PERIPH(CLK_FIC2, "clk_periph_fic2", CLK_AXI, 26, 0),
+	CLK_PERIPH(CLK_FIC3, "clk_periph_fic3", CLK_AXI, 27, 0),
+	CLK_PERIPH(CLK_ATHENA, "clk_periph_athena", CLK_AXI, 28, 0),
+	CLK_PERIPH(CLK_CFM, "clk_periph_cfm", CLK_AHB, 29, 0),
 };
 
-int mpfs_clk_register_periphs(void __iomem *base, struct clk *parent)
+int mpfs_clk_register_periphs(void __iomem *base, struct udevice *dev)
 {
 	int ret;
 	int i, id, num_clks;
@@ -152,11 +155,14 @@ int mpfs_clk_register_periphs(void __iomem *base, struct clk *parent)
 
 	num_clks = ARRAY_SIZE(mpfs_periph_clks);
 	for (i = 0; i < num_clks; i++)  {
+		struct clk parent = { .id = mpfs_periph_clks[i].periph.parent_id };
+
+		clk_request(dev, &parent);
 		hw = &mpfs_periph_clks[i].hw;
 		mpfs_periph_clks[i].sys_base = base;
-		mpfs_periph_clks[i].prate = clk_get_rate(parent);
+		mpfs_periph_clks[i].prate = clk_get_rate(&parent);
 		name = mpfs_periph_clks[i].periph.name;
-		ret = clk_register(hw, MPFS_PERIPH_CLOCK, name, parent->dev->name);
+		ret = clk_register(hw, MPFS_PERIPH_CLOCK, name, parent.dev->name);
 		if (ret)
 			ERR_PTR(ret);
 		id = mpfs_periph_clks[i].periph.id;
-- 
2.38.0

Re: [PATCH v1 4/6] clk: microchip: mpfs: fix periph clk parentage

[Leo Liang]

On Tue, Oct 25, 2022 at 08:58:47AM +0100, Conor Dooley wrote:
> Not all "periph" clocks are children of the AHB clock, some have the AXI
> clock as their parent & the mtimer clock is derived from the external
> reference clock directly. Stop assuming the AHB clock to be the parent
> of all "periph" clocks and define their correct parents instead.
> 
> Fixes: 2f27c9219e ("clk: Add Microchip PolarFire SoC clock driver")
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>  drivers/clk/microchip/mpfs_clk.c        |  4 +-
>  drivers/clk/microchip/mpfs_clk.h        |  4 +-
>  drivers/clk/microchip/mpfs_clk_periph.c | 72 +++++++++++++------------
>  3 files changed, 42 insertions(+), 38 deletions(-)

Reviewed-by: Leo Yu-Chi Liang <ycliang@andestech.com>

Re: [PATCH v1 4/6] clk: microchip: mpfs: fix periph clk parentage

[Padmarao.Begari]

> On Tue, 2022-10-25 at 08:58 +0100, Conor Dooley wrote:
> Not all "periph" clocks are children of the AHB clock, some have the
> AXI
> clock as their parent & the mtimer clock is derived from the external
> reference clock directly. Stop assuming the AHB clock to be the
> parent
> of all "periph" clocks and define their correct parents instead.
> 
> Fixes: 2f27c9219e ("clk: Add Microchip PolarFire SoC clock driver")
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>  drivers/clk/microchip/mpfs_clk.c        |  4 +-
>  drivers/clk/microchip/mpfs_clk.h        |  4 +-
>  drivers/clk/microchip/mpfs_clk_periph.c | 72 +++++++++++++--------
> ----
>  3 files changed, 42 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/clk/microchip/mpfs_clk.c
> b/drivers/clk/microchip/mpfs_clk.c
> index f16f716f00..08f8bfcecb 100644
> --- a/drivers/clk/microchip/mpfs_clk.c
> +++ b/drivers/clk/microchip/mpfs_clk.c
> @@ -19,7 +19,6 @@
>  static int mpfs_clk_probe(struct udevice *dev)
>  {
>  	struct clk *parent_clk = dev_get_priv(dev);
> -	struct clk clk_ahb = { .id = CLK_AHB };
>  	struct clk clk_msspll = { .id = CLK_MSSPLL };
>  	void __iomem *base;
>  	void __iomem *msspll_base;
> @@ -56,8 +55,7 @@ static int mpfs_clk_probe(struct udevice *dev)
>  	if (ret)
>  		return ret;
>  
> -	clk_request(dev, &clk_ahb);
> -	ret = mpfs_clk_register_periphs(base, &clk_ahb);
> +	ret = mpfs_clk_register_periphs(base, dev);
>  
>  	return ret;
>  }
> diff --git a/drivers/clk/microchip/mpfs_clk.h
> b/drivers/clk/microchip/mpfs_clk.h
> index cb7d303e67..72288cc971 100644
> --- a/drivers/clk/microchip/mpfs_clk.h
> +++ b/drivers/clk/microchip/mpfs_clk.h
> @@ -27,10 +27,10 @@ int mpfs_clk_register_msspll(void __iomem *base,
> struct clk *parent);
>   * mpfs_clk_register_periphs() - register peripheral clocks
>   *
>   * @base: base address of the mpfs system register.
> - * @parent: a pointer to parent clock.
> + * @dev: udevice representing the clock controller.
>   * Return: zero on success, or a negative error code.
>   */
> -int mpfs_clk_register_periphs(void __iomem *base, struct clk
> *parent);
> +int mpfs_clk_register_periphs(void __iomem *base, struct udevice
> *dev);
>  /**
>   * divider_get_val() - get the clock divider value
>   *
> diff --git a/drivers/clk/microchip/mpfs_clk_periph.c
> b/drivers/clk/microchip/mpfs_clk_periph.c
> index 1488ef503e..e23eb552c3 100644
> --- a/drivers/clk/microchip/mpfs_clk_periph.c
> +++ b/drivers/clk/microchip/mpfs_clk_periph.c
> @@ -29,12 +29,14 @@
>  /**
>   * struct mpfs_periph_clock - per instance of peripheral clock
>   * @id: index of a peripheral clock
> + * @parent_id: index of the parent clock
>   * @name: name of a peripheral clock
>   * @shift: shift to a peripheral clock bit field
>   * @flags: common clock framework flags
>   */
>  struct mpfs_periph_clock {
>  	unsigned int id;
> +	unsigned int parent_id;
>  	const char *name;
>  	u8 shift;
>  	unsigned long flags;
> @@ -104,46 +106,47 @@ static ulong mpfs_periph_clk_recalc_rate(struct
> clk *hw)
>  
>  }
>  
> -#define CLK_PERIPH(_id, _name, _shift, _flags) {	\
> +#define CLK_PERIPH(_id, _name, _parent_id, _shift, _flags) {	\
>  		.periph.id = _id,			\
> +		.periph.parent_id = _parent_id,		\
>  		.periph.name = _name,			\
>  		.periph.shift = _shift,			\
>  		.periph.flags = _flags,			\
>  	}
>  
>  static struct mpfs_periph_hw_clock mpfs_periph_clks[] = {
> -	CLK_PERIPH(CLK_ENVM, "clk_periph_envm", 0, CLK_IS_CRITICAL),
> -	CLK_PERIPH(CLK_MAC0, "clk_periph_mac0", 1, 0),
> -	CLK_PERIPH(CLK_MAC1, "clk_periph_mac1", 2, 0),
> -	CLK_PERIPH(CLK_MMC, "clk_periph_mmc", 3, 0),
> -	CLK_PERIPH(CLK_TIMER, "clk_periph_timer", 4, 0),
> -	CLK_PERIPH(CLK_MMUART0, "clk_periph_mmuart0", 5, 0),
> -	CLK_PERIPH(CLK_MMUART1, "clk_periph_mmuart1", 6, 0),
> -	CLK_PERIPH(CLK_MMUART2, "clk_periph_mmuart2", 7, 0),
> -	CLK_PERIPH(CLK_MMUART3, "clk_periph_mmuart3", 8, 0),
> -	CLK_PERIPH(CLK_MMUART4, "clk_periph_mmuart4", 9, 0),
> -	CLK_PERIPH(CLK_SPI0, "clk_periph_spi0", 10, 0),
> -	CLK_PERIPH(CLK_SPI1, "clk_periph_spi1", 11, 0),
> -	CLK_PERIPH(CLK_I2C0, "clk_periph_i2c0", 12, 0),
> -	CLK_PERIPH(CLK_I2C1, "clk_periph_i2c1", 13, 0),
> -	CLK_PERIPH(CLK_CAN0, "clk_periph_can0", 14, 0),
> -	CLK_PERIPH(CLK_CAN1, "clk_periph_can1", 15, 0),
> -	CLK_PERIPH(CLK_USB, "clk_periph_usb", 16, 0),
> -	CLK_PERIPH(CLK_RTC, "clk_periph_rtc", 18, 0),
> -	CLK_PERIPH(CLK_QSPI, "clk_periph_qspi", 19, 0),
> -	CLK_PERIPH(CLK_GPIO0, "clk_periph_gpio0", 20, 0),
> -	CLK_PERIPH(CLK_GPIO1, "clk_periph_gpio1", 21, 0),
> -	CLK_PERIPH(CLK_GPIO2, "clk_periph_gpio2", 22, 0),
> -	CLK_PERIPH(CLK_DDRC, "clk_periph_ddrc", 23, CLK_IS_CRITICAL),
> -	CLK_PERIPH(CLK_FIC0, "clk_periph_fic0", 24, 0),
> -	CLK_PERIPH(CLK_FIC1, "clk_periph_fic1", 25, 0),
> -	CLK_PERIPH(CLK_FIC2, "clk_periph_fic2", 26, 0),
> -	CLK_PERIPH(CLK_FIC3, "clk_periph_fic3", 27, 0),
> -	CLK_PERIPH(CLK_ATHENA, "clk_periph_athena", 28, 0),
> -	CLK_PERIPH(CLK_CFM, "clk_periph_cfm", 29, 0),
> +	CLK_PERIPH(CLK_ENVM, "clk_periph_envm", CLK_AHB, 0,
> CLK_IS_CRITICAL),
> +	CLK_PERIPH(CLK_MAC0, "clk_periph_mac0", CLK_AHB, 1, 0),
> +	CLK_PERIPH(CLK_MAC1, "clk_periph_mac1", CLK_AHB, 2, 0),
> +	CLK_PERIPH(CLK_MMC, "clk_periph_mmc", CLK_AHB, 3, 0),
> +	CLK_PERIPH(CLK_TIMER, "clk_periph_timer", CLK_RTCREF, 4, 0),
> +	CLK_PERIPH(CLK_MMUART0, "clk_periph_mmuart0", CLK_AHB, 5, 0),
> +	CLK_PERIPH(CLK_MMUART1, "clk_periph_mmuart1", CLK_AHB, 6, 0),
> +	CLK_PERIPH(CLK_MMUART2, "clk_periph_mmuart2", CLK_AHB, 7, 0),
> +	CLK_PERIPH(CLK_MMUART3, "clk_periph_mmuart3", CLK_AHB, 8, 0),
> +	CLK_PERIPH(CLK_MMUART4, "clk_periph_mmuart4", CLK_AHB, 9, 0),
> +	CLK_PERIPH(CLK_SPI0, "clk_periph_spi0", CLK_AHB, 10, 0),
> +	CLK_PERIPH(CLK_SPI1, "clk_periph_spi1", CLK_AHB, 11, 0),
> +	CLK_PERIPH(CLK_I2C0, "clk_periph_i2c0", CLK_AHB, 12, 0),
> +	CLK_PERIPH(CLK_I2C1, "clk_periph_i2c1", CLK_AHB, 13, 0),
> +	CLK_PERIPH(CLK_CAN0, "clk_periph_can0", CLK_AHB, 14, 0),
> +	CLK_PERIPH(CLK_CAN1, "clk_periph_can1", CLK_AHB, 15, 0),
> +	CLK_PERIPH(CLK_USB, "clk_periph_usb", CLK_AHB, 16, 0),
> +	CLK_PERIPH(CLK_RTC, "clk_periph_rtc", CLK_AHB, 18, 0),
> +	CLK_PERIPH(CLK_QSPI, "clk_periph_qspi", CLK_AHB, 19, 0),
> +	CLK_PERIPH(CLK_GPIO0, "clk_periph_gpio0", CLK_AHB, 20, 0),
> +	CLK_PERIPH(CLK_GPIO1, "clk_periph_gpio1", CLK_AHB, 21, 0),
> +	CLK_PERIPH(CLK_GPIO2, "clk_periph_gpio2", CLK_AHB, 22, 0),
> +	CLK_PERIPH(CLK_DDRC, "clk_periph_ddrc", CLK_AHB, 23,
> CLK_IS_CRITICAL),
> +	CLK_PERIPH(CLK_FIC0, "clk_periph_fic0", CLK_AXI, 24, 0),
> +	CLK_PERIPH(CLK_FIC1, "clk_periph_fic1", CLK_AXI, 25, 0),
> +	CLK_PERIPH(CLK_FIC2, "clk_periph_fic2", CLK_AXI, 26, 0),
> +	CLK_PERIPH(CLK_FIC3, "clk_periph_fic3", CLK_AXI, 27, 0),
> +	CLK_PERIPH(CLK_ATHENA, "clk_periph_athena", CLK_AXI, 28, 0),
> +	CLK_PERIPH(CLK_CFM, "clk_periph_cfm", CLK_AHB, 29, 0),
>  };
>  
> -int mpfs_clk_register_periphs(void __iomem *base, struct clk
> *parent)
> +int mpfs_clk_register_periphs(void __iomem *base, struct udevice
> *dev)
>  {
>  	int ret;
>  	int i, id, num_clks;
> @@ -152,11 +155,14 @@ int mpfs_clk_register_periphs(void __iomem
> *base, struct clk *parent)
>  
>  	num_clks = ARRAY_SIZE(mpfs_periph_clks);
>  	for (i = 0; i < num_clks; i++)  {
> +		struct clk parent = { .id =
> mpfs_periph_clks[i].periph.parent_id };
> +
> +		clk_request(dev, &parent);
>  		hw = &mpfs_periph_clks[i].hw;
>  		mpfs_periph_clks[i].sys_base = base;
> -		mpfs_periph_clks[i].prate = clk_get_rate(parent);
> +		mpfs_periph_clks[i].prate = clk_get_rate(&parent);
>  		name = mpfs_periph_clks[i].periph.name;
> -		ret = clk_register(hw, MPFS_PERIPH_CLOCK, name, parent-
> >dev->name);
> +		ret = clk_register(hw, MPFS_PERIPH_CLOCK, name,
> parent.dev->name);
>  		if (ret)
>  			ERR_PTR(ret);
>  		id = mpfs_periph_clks[i].periph.id;

Reviewed-by: Padmarao Begari <padmarao.begari@microchip.com>
Tested-by: Padmarao Begari <padmarao.begari@microchip.com>

[PATCH v1 5/6] clk: microchip: mpfs: fix criticality of peripheral clocks

[Conor Dooley]

Sync the critical clocks in the U-Boot driver with those marked as
critical in Linux. The Linux driver has an explanation of why each clock
is considered to be critical, so import that too.

Fixes: 2f27c9219e ("clk: Add Microchip PolarFire SoC clock driver")
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 drivers/clk/microchip/mpfs_clk_periph.c | 28 ++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/microchip/mpfs_clk_periph.c b/drivers/clk/microchip/mpfs_clk_periph.c
index e23eb552c3..ddeccb9145 100644
--- a/drivers/clk/microchip/mpfs_clk_periph.c
+++ b/drivers/clk/microchip/mpfs_clk_periph.c
@@ -114,13 +114,27 @@ static ulong mpfs_periph_clk_recalc_rate(struct clk *hw)
 		.periph.flags = _flags,			\
 	}
 
+/*
+ * Critical clocks:
+ * - CLK_ENVM: reserved by hart software services (hss) superloop monitor/m mode interrupt
+ *   trap handler
+ * - CLK_MMUART0: reserved by the hss
+ * - CLK_DDRC: provides clock to the ddr subsystem
+ * - CLK_RTC: the onboard RTC's AHB bus clock must be kept running as the rtc will stop
+ *   if the AHB interface clock is disabled
+ * - CLK_FICx: these provide the processor side clocks to the "FIC" (Fabric InterConnect)
+ *   clock domain crossers which provide the interface to the FPGA fabric. Disabling them
+ *   causes the FPGA fabric to go into reset.
+ * - CLK_ATHENA: The athena clock is FIC4, which is reserved for the Athena TeraFire.
+ */
+
 static struct mpfs_periph_hw_clock mpfs_periph_clks[] = {
 	CLK_PERIPH(CLK_ENVM, "clk_periph_envm", CLK_AHB, 0, CLK_IS_CRITICAL),
 	CLK_PERIPH(CLK_MAC0, "clk_periph_mac0", CLK_AHB, 1, 0),
 	CLK_PERIPH(CLK_MAC1, "clk_periph_mac1", CLK_AHB, 2, 0),
 	CLK_PERIPH(CLK_MMC, "clk_periph_mmc", CLK_AHB, 3, 0),
 	CLK_PERIPH(CLK_TIMER, "clk_periph_timer", CLK_RTCREF, 4, 0),
-	CLK_PERIPH(CLK_MMUART0, "clk_periph_mmuart0", CLK_AHB, 5, 0),
+	CLK_PERIPH(CLK_MMUART0, "clk_periph_mmuart0", CLK_AHB, 5, CLK_IS_CRITICAL),
 	CLK_PERIPH(CLK_MMUART1, "clk_periph_mmuart1", CLK_AHB, 6, 0),
 	CLK_PERIPH(CLK_MMUART2, "clk_periph_mmuart2", CLK_AHB, 7, 0),
 	CLK_PERIPH(CLK_MMUART3, "clk_periph_mmuart3", CLK_AHB, 8, 0),
@@ -132,17 +146,17 @@ static struct mpfs_periph_hw_clock mpfs_periph_clks[] = {
 	CLK_PERIPH(CLK_CAN0, "clk_periph_can0", CLK_AHB, 14, 0),
 	CLK_PERIPH(CLK_CAN1, "clk_periph_can1", CLK_AHB, 15, 0),
 	CLK_PERIPH(CLK_USB, "clk_periph_usb", CLK_AHB, 16, 0),
-	CLK_PERIPH(CLK_RTC, "clk_periph_rtc", CLK_AHB, 18, 0),
+	CLK_PERIPH(CLK_RTC, "clk_periph_rtc", CLK_AHB, 18, CLK_IS_CRITICAL),
 	CLK_PERIPH(CLK_QSPI, "clk_periph_qspi", CLK_AHB, 19, 0),
 	CLK_PERIPH(CLK_GPIO0, "clk_periph_gpio0", CLK_AHB, 20, 0),
 	CLK_PERIPH(CLK_GPIO1, "clk_periph_gpio1", CLK_AHB, 21, 0),
 	CLK_PERIPH(CLK_GPIO2, "clk_periph_gpio2", CLK_AHB, 22, 0),
 	CLK_PERIPH(CLK_DDRC, "clk_periph_ddrc", CLK_AHB, 23, CLK_IS_CRITICAL),
-	CLK_PERIPH(CLK_FIC0, "clk_periph_fic0", CLK_AXI, 24, 0),
-	CLK_PERIPH(CLK_FIC1, "clk_periph_fic1", CLK_AXI, 25, 0),
-	CLK_PERIPH(CLK_FIC2, "clk_periph_fic2", CLK_AXI, 26, 0),
-	CLK_PERIPH(CLK_FIC3, "clk_periph_fic3", CLK_AXI, 27, 0),
-	CLK_PERIPH(CLK_ATHENA, "clk_periph_athena", CLK_AXI, 28, 0),
+	CLK_PERIPH(CLK_FIC0, "clk_periph_fic0", CLK_AXI, 24, CLK_IS_CRITICAL),
+	CLK_PERIPH(CLK_FIC1, "clk_periph_fic1", CLK_AXI, 25, CLK_IS_CRITICAL),
+	CLK_PERIPH(CLK_FIC2, "clk_periph_fic2", CLK_AXI, 26, CLK_IS_CRITICAL),
+	CLK_PERIPH(CLK_FIC3, "clk_periph_fic3", CLK_AXI, 27, CLK_IS_CRITICAL),
+	CLK_PERIPH(CLK_ATHENA, "clk_periph_athena", CLK_AXI, 28, CLK_IS_CRITICAL),
 	CLK_PERIPH(CLK_CFM, "clk_periph_cfm", CLK_AHB, 29, 0),
 };
 
-- 
2.38.0

Re: [PATCH v1 5/6] clk: microchip: mpfs: fix criticality of peripheral clocks

[Leo Liang]

On Tue, Oct 25, 2022 at 08:58:48AM +0100, Conor Dooley wrote:
> Sync the critical clocks in the U-Boot driver with those marked as
> critical in Linux. The Linux driver has an explanation of why each clock
> is considered to be critical, so import that too.
> 
> Fixes: 2f27c9219e ("clk: Add Microchip PolarFire SoC clock driver")
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>  drivers/clk/microchip/mpfs_clk_periph.c | 28 ++++++++++++++++++-------
>  1 file changed, 21 insertions(+), 7 deletions(-)

Reviewed-by: Leo Yu-Chi Liang <ycliang@andestech.com>

Re: [PATCH v1 5/6] clk: microchip: mpfs: fix criticality of peripheral clocks

[Padmarao.Begari]

> On Tue, 2022-10-25 at 08:58 +0100, Conor Dooley wrote:
> Sync the critical clocks in the U-Boot driver with those marked as
> critical in Linux. The Linux driver has an explanation of why each
> clock
> is considered to be critical, so import that too.
> 
> Fixes: 2f27c9219e ("clk: Add Microchip PolarFire SoC clock driver")
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>  drivers/clk/microchip/mpfs_clk_periph.c | 28 ++++++++++++++++++-----
> --
>  1 file changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/clk/microchip/mpfs_clk_periph.c
> b/drivers/clk/microchip/mpfs_clk_periph.c
> index e23eb552c3..ddeccb9145 100644
> --- a/drivers/clk/microchip/mpfs_clk_periph.c
> +++ b/drivers/clk/microchip/mpfs_clk_periph.c
> @@ -114,13 +114,27 @@ static ulong mpfs_periph_clk_recalc_rate(struct
> clk *hw)
>  		.periph.flags = _flags,			\
>  	}
>  
> +/*
> + * Critical clocks:
> + * - CLK_ENVM: reserved by hart software services (hss) superloop
> monitor/m mode interrupt
> + *   trap handler
> + * - CLK_MMUART0: reserved by the hss
> + * - CLK_DDRC: provides clock to the ddr subsystem
> + * - CLK_RTC: the onboard RTC's AHB bus clock must be kept running
> as the rtc will stop
> + *   if the AHB interface clock is disabled
> + * - CLK_FICx: these provide the processor side clocks to the "FIC"
> (Fabric InterConnect)
> + *   clock domain crossers which provide the interface to the FPGA
> fabric. Disabling them
> + *   causes the FPGA fabric to go into reset.
> + * - CLK_ATHENA: The athena clock is FIC4, which is reserved for the
> Athena TeraFire.
> + */
> +
>  static struct mpfs_periph_hw_clock mpfs_periph_clks[] = {
>  	CLK_PERIPH(CLK_ENVM, "clk_periph_envm", CLK_AHB, 0,
> CLK_IS_CRITICAL),
>  	CLK_PERIPH(CLK_MAC0, "clk_periph_mac0", CLK_AHB, 1, 0),
>  	CLK_PERIPH(CLK_MAC1, "clk_periph_mac1", CLK_AHB, 2, 0),
>  	CLK_PERIPH(CLK_MMC, "clk_periph_mmc", CLK_AHB, 3, 0),
>  	CLK_PERIPH(CLK_TIMER, "clk_periph_timer", CLK_RTCREF, 4, 0),
> -	CLK_PERIPH(CLK_MMUART0, "clk_periph_mmuart0", CLK_AHB, 5, 0),
> +	CLK_PERIPH(CLK_MMUART0, "clk_periph_mmuart0", CLK_AHB, 5,
> CLK_IS_CRITICAL),
>  	CLK_PERIPH(CLK_MMUART1, "clk_periph_mmuart1", CLK_AHB, 6, 0),
>  	CLK_PERIPH(CLK_MMUART2, "clk_periph_mmuart2", CLK_AHB, 7, 0),
>  	CLK_PERIPH(CLK_MMUART3, "clk_periph_mmuart3", CLK_AHB, 8, 0),
> @@ -132,17 +146,17 @@ static struct mpfs_periph_hw_clock
> mpfs_periph_clks[] = {
>  	CLK_PERIPH(CLK_CAN0, "clk_periph_can0", CLK_AHB, 14, 0),
>  	CLK_PERIPH(CLK_CAN1, "clk_periph_can1", CLK_AHB, 15, 0),
>  	CLK_PERIPH(CLK_USB, "clk_periph_usb", CLK_AHB, 16, 0),
> -	CLK_PERIPH(CLK_RTC, "clk_periph_rtc", CLK_AHB, 18, 0),
> +	CLK_PERIPH(CLK_RTC, "clk_periph_rtc", CLK_AHB, 18,
> CLK_IS_CRITICAL),
>  	CLK_PERIPH(CLK_QSPI, "clk_periph_qspi", CLK_AHB, 19, 0),
>  	CLK_PERIPH(CLK_GPIO0, "clk_periph_gpio0", CLK_AHB, 20, 0),
>  	CLK_PERIPH(CLK_GPIO1, "clk_periph_gpio1", CLK_AHB, 21, 0),
>  	CLK_PERIPH(CLK_GPIO2, "clk_periph_gpio2", CLK_AHB, 22, 0),
>  	CLK_PERIPH(CLK_DDRC, "clk_periph_ddrc", CLK_AHB, 23,
> CLK_IS_CRITICAL),
> -	CLK_PERIPH(CLK_FIC0, "clk_periph_fic0", CLK_AXI, 24, 0),
> -	CLK_PERIPH(CLK_FIC1, "clk_periph_fic1", CLK_AXI, 25, 0),
> -	CLK_PERIPH(CLK_FIC2, "clk_periph_fic2", CLK_AXI, 26, 0),
> -	CLK_PERIPH(CLK_FIC3, "clk_periph_fic3", CLK_AXI, 27, 0),
> -	CLK_PERIPH(CLK_ATHENA, "clk_periph_athena", CLK_AXI, 28, 0),
> +	CLK_PERIPH(CLK_FIC0, "clk_periph_fic0", CLK_AXI, 24,
> CLK_IS_CRITICAL),
> +	CLK_PERIPH(CLK_FIC1, "clk_periph_fic1", CLK_AXI, 25,
> CLK_IS_CRITICAL),
> +	CLK_PERIPH(CLK_FIC2, "clk_periph_fic2", CLK_AXI, 26,
> CLK_IS_CRITICAL),
> +	CLK_PERIPH(CLK_FIC3, "clk_periph_fic3", CLK_AXI, 27,
> CLK_IS_CRITICAL),
> +	CLK_PERIPH(CLK_ATHENA, "clk_periph_athena", CLK_AXI, 28,
> CLK_IS_CRITICAL),
>  	CLK_PERIPH(CLK_CFM, "clk_periph_cfm", CLK_AHB, 29, 0),
>  };
>  

Reviewed-by: Padmarao Begari <padmarao.begari@microchip.com>

[PATCH v1 6/6] riscv: dts: fix the mpfs's reference clock frequency

[Conor Dooley]

The initial devicetree for PolarFire SoC incorrectly created a fixed
frequency clock in the devicetree to represent the msspll, but the
msspll is not a fixed frequency clock. The actual reference clock on a
board is either 125 or 100 MHz, 125 MHz in the case of the icicle kit.
Swap the incorrect representation of the msspll out for the actual
reference clock.

Fixes: dd4ee416a6 ("riscv: dts: Add device tree for Microchip Icicle Kit")
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 arch/riscv/dts/microchip-mpfs-icicle-kit.dts |  4 ++++
 arch/riscv/dts/microchip-mpfs.dtsi           | 14 ++++++--------
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/arch/riscv/dts/microchip-mpfs-icicle-kit.dts b/arch/riscv/dts/microchip-mpfs-icicle-kit.dts
index e1fbedc507..7d87b181db 100644
--- a/arch/riscv/dts/microchip-mpfs-icicle-kit.dts
+++ b/arch/riscv/dts/microchip-mpfs-icicle-kit.dts
@@ -53,6 +53,10 @@
 	};
 };
 
+&refclk {
+	clock-frequency = <125000000>;
+};
+
 &uart1 {
 	status = "okay";
 };
diff --git a/arch/riscv/dts/microchip-mpfs.dtsi b/arch/riscv/dts/microchip-mpfs.dtsi
index 4f449a3a93..891dd0918b 100644
--- a/arch/riscv/dts/microchip-mpfs.dtsi
+++ b/arch/riscv/dts/microchip-mpfs.dtsi
@@ -170,6 +170,11 @@
 		};
 	};
 
+	refclk: refclk {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
 	soc {
 		#address-cells = <2>;
 		#size-cells = <2>;
@@ -225,16 +230,9 @@
 					&cpu4_intc HART_INT_M_EXT &cpu4_intc HART_INT_S_EXT>;
 		};
 
-		refclk: refclk {
-			compatible = "fixed-clock";
-			#clock-cells = <0>;
-			clock-frequency = <600000000>;
-			clock-output-names = "msspllclk";
-		};
-
 		clkcfg: clkcfg@20002000 {
 			compatible = "microchip,mpfs-clkcfg";
-			reg = <0x0 0x20002000 0x0 0x1000>;
+			reg = <0x0 0x20002000 0x0 0x1000>, <0x0 0x3E001000 0x0 0x1000>;
 			reg-names = "mss_sysreg";
 			clocks = <&refclk>;
 			#clock-cells = <1>;
-- 
2.38.0

Re: [PATCH v1 6/6] riscv: dts: fix the mpfs's reference clock frequency

[Leo Liang]

On Tue, Oct 25, 2022 at 08:58:49AM +0100, Conor Dooley wrote:
> The initial devicetree for PolarFire SoC incorrectly created a fixed
> frequency clock in the devicetree to represent the msspll, but the
> msspll is not a fixed frequency clock. The actual reference clock on a
> board is either 125 or 100 MHz, 125 MHz in the case of the icicle kit.
> Swap the incorrect representation of the msspll out for the actual
> reference clock.
> 
> Fixes: dd4ee416a6 ("riscv: dts: Add device tree for Microchip Icicle Kit")
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>  arch/riscv/dts/microchip-mpfs-icicle-kit.dts |  4 ++++
>  arch/riscv/dts/microchip-mpfs.dtsi           | 14 ++++++--------
>  2 files changed, 10 insertions(+), 8 deletions(-)

Reviewed-by: Leo Yu-Chi Liang <ycliang@andestech.com>

Re: [PATCH v1 6/6] riscv: dts: fix the mpfs's reference clock frequency

[Padmarao.Begari]

> On Tue, 2022-10-25 at 08:58 +0100, Conor Dooley wrote:
> The initial devicetree for PolarFire SoC incorrectly created a fixed
> frequency clock in the devicetree to represent the msspll, but the
> msspll is not a fixed frequency clock. The actual reference clock on
> a
> board is either 125 or 100 MHz, 125 MHz in the case of the icicle
> kit.
> Swap the incorrect representation of the msspll out for the actual
> reference clock.
> 
> Fixes: dd4ee416a6 ("riscv: dts: Add device tree for Microchip Icicle
> Kit")
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>  arch/riscv/dts/microchip-mpfs-icicle-kit.dts |  4 ++++
>  arch/riscv/dts/microchip-mpfs.dtsi           | 14 ++++++--------
>  2 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/riscv/dts/microchip-mpfs-icicle-kit.dts
> b/arch/riscv/dts/microchip-mpfs-icicle-kit.dts
> index e1fbedc507..7d87b181db 100644
> --- a/arch/riscv/dts/microchip-mpfs-icicle-kit.dts
> +++ b/arch/riscv/dts/microchip-mpfs-icicle-kit.dts
> @@ -53,6 +53,10 @@
>  	};
>  };
>  
> +&refclk {
> +	clock-frequency = <125000000>;
> +};
> +
>  &uart1 {
>  	status = "okay";
>  };
> diff --git a/arch/riscv/dts/microchip-mpfs.dtsi
> b/arch/riscv/dts/microchip-mpfs.dtsi
> index 4f449a3a93..891dd0918b 100644
> --- a/arch/riscv/dts/microchip-mpfs.dtsi
> +++ b/arch/riscv/dts/microchip-mpfs.dtsi
> @@ -170,6 +170,11 @@
>  		};
>  	};
>  
> +	refclk: refclk {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +	};
> +
>  	soc {
>  		#address-cells = <2>;
>  		#size-cells = <2>;
> @@ -225,16 +230,9 @@
>  					&cpu4_intc HART_INT_M_EXT
> &cpu4_intc HART_INT_S_EXT>;
>  		};
>  
> -		refclk: refclk {
> -			compatible = "fixed-clock";
> -			#clock-cells = <0>;
> -			clock-frequency = <600000000>;
> -			clock-output-names = "msspllclk";
> -		};
> -
>  		clkcfg: clkcfg@20002000 {
>  			compatible = "microchip,mpfs-clkcfg";
> -			reg = <0x0 0x20002000 0x0 0x1000>;
> +			reg = <0x0 0x20002000 0x0 0x1000>, <0x0
> 0x3E001000 0x0 0x1000>;
>  			reg-names = "mss_sysreg";
>  			clocks = <&refclk>;
>  			#clock-cells = <1>;

Reviewed-by: Padmarao Begari <padmarao.begari@microchip.com>