* [PATCH v1 0/7] Add rtc refclk support for PolarFire SoC
@ 2022-04-08 14:36 Conor Dooley
2022-04-08 14:36 ` [PATCH v1 1/7] dt-bindings: clk: mpfs document msspll dri registers Conor Dooley
` (7 more replies)
0 siblings, 8 replies; 17+ messages in thread
From: Conor Dooley @ 2022-04-08 14:36 UTC (permalink / raw)
To: mturquette, sboyd, aou, paul.walmsley, palmer, a.zummo,
alexandre.belloni, robh+dt, krzk+dt
Cc: daire.mcnamara, linux-rtc, devicetree, linux-clk, linux-riscv,
Conor Dooley
Hey,
As I mentioned in my fixes for 5.18 [0], found out that the reference
clock for the rtc is actually missing from the clock driver (and the
dt binding).
Currently the mpfs clock driver uses a reference clock called the
"msspll", set in the device tree, as the parent for the cpu/axi/ahb
(config) clocks. The frequency of the msspll is determined by the FPGA
bitstream & the bootloader configures the clock to match the bitstream.
The real reference is provided by a 100 or 125 MHz off chip oscillator.
However, the msspll clock is not actually the parent of all clocks on
the system - the reference clock for the rtc/mtimer actually has the
off chip oscillator as its parent.
This series enables reading the rate of the msspll clock, converts
the refclock in the device tree to the external reference & adds
the missing rtc reference clock.
I assume it is okay not to add fixes tags for the rtc dt binding?
Since the clock was previously missing, the binding is wrong, but
idk if that qualifies as a fix?
Clock driver changes depend on the fixes I sent in [0].
Please lmk if you want me to respin into a single series w/ the fixes.
Thanks,
Conor.
[0]: https://lore.kernel.org/linux-riscv/20220408133543.3537118-1-conor.dooley@microchip.com/
Conor Dooley (7):
dt-bindings: clk: mpfs document msspll dri registers
dt-bindings: clk: mpfs: add defines for two new clocks
dt-bindings: rtc: add refclk to mpfs-rtc
clk: microchip: mpfs: re-parent the configurable clocks
clk: microchip: mpfs: rename sys_base to base
clk: microchip: mpfs: add RTCREF clock control
riscv: dts: microchip: reparent mpfs clocks
.../bindings/clock/microchip,mpfs.yaml | 11 +-
.../bindings/rtc/microchip,mfps-rtc.yaml | 14 +-
.../microchip/microchip-mpfs-icicle-kit.dts | 2 +-
.../boot/dts/microchip/microchip-mpfs.dtsi | 8 +-
drivers/clk/microchip/clk-mpfs.c | 205 +++++++++++++++---
.../dt-bindings/clock/microchip,mpfs-clock.h | 5 +-
6 files changed, 199 insertions(+), 46 deletions(-)
--
2.35.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v1 1/7] dt-bindings: clk: mpfs document msspll dri registers
2022-04-08 14:36 [PATCH v1 0/7] Add rtc refclk support for PolarFire SoC Conor Dooley
@ 2022-04-08 14:36 ` Conor Dooley
2022-04-08 14:54 ` Krzysztof Kozlowski
2022-04-08 14:36 ` [PATCH v1 2/7] dt-bindings: clk: mpfs: add defines for two new clocks Conor Dooley
` (6 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Conor Dooley @ 2022-04-08 14:36 UTC (permalink / raw)
To: mturquette, sboyd, aou, paul.walmsley, palmer, a.zummo,
alexandre.belloni, robh+dt, krzk+dt
Cc: daire.mcnamara, linux-rtc, devicetree, linux-clk, linux-riscv,
Conor Dooley
As there are two sections of registers that are responsible for clock
configuration on the PolarFire SoC: add the dynamic reconfiguration
interface section to the binding & describe what each of the sections
are used for.
Reviewed-by: Daire McNamara <daire.mcnamara@microchip.com>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
.../devicetree/bindings/clock/microchip,mpfs.yaml | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/clock/microchip,mpfs.yaml b/Documentation/devicetree/bindings/clock/microchip,mpfs.yaml
index 0c15afa2214c..42919df322ab 100644
--- a/Documentation/devicetree/bindings/clock/microchip,mpfs.yaml
+++ b/Documentation/devicetree/bindings/clock/microchip,mpfs.yaml
@@ -22,7 +22,14 @@ properties:
const: microchip,mpfs-clkcfg
reg:
- maxItems: 1
+ items:
+ - description: |
+ clock config registers:
+ These registers contain enable, reset & divider tables for the, cpu, axi, ahb and
+ rtc/mtimer reference clocks as well as enable and reset for the peripheral clocks.
+ - description: |
+ mss pll dri registers:
+ Block of registers responsible for dynamic reconfiguration of the mss pll
clocks:
maxItems: 1
@@ -51,7 +58,7 @@ examples:
#size-cells = <2>;
clkcfg: clock-controller@20002000 {
compatible = "microchip,mpfs-clkcfg";
- reg = <0x0 0x20002000 0x0 0x1000>;
+ reg = <0x0 0x20002000 0x0 0x1000>, <0x0 0x3E001000 0x0 0x1000>;
clocks = <&ref>;
#clock-cells = <1>;
};
--
2.35.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v1 2/7] dt-bindings: clk: mpfs: add defines for two new clocks
2022-04-08 14:36 [PATCH v1 0/7] Add rtc refclk support for PolarFire SoC Conor Dooley
2022-04-08 14:36 ` [PATCH v1 1/7] dt-bindings: clk: mpfs document msspll dri registers Conor Dooley
@ 2022-04-08 14:36 ` Conor Dooley
2022-04-08 14:36 ` [PATCH v1 3/7] dt-bindings: rtc: add refclk to mpfs-rtc Conor Dooley
` (5 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Conor Dooley @ 2022-04-08 14:36 UTC (permalink / raw)
To: mturquette, sboyd, aou, paul.walmsley, palmer, a.zummo,
alexandre.belloni, robh+dt, krzk+dt
Cc: daire.mcnamara, linux-rtc, devicetree, linux-clk, linux-riscv,
Conor Dooley
The RTC reference and MSSPLL were previously not documented or defined,
as they were unused. Add their defines to the PolarFire SoC header.
Reviewed-by: Daire McNamara <daire.mcnamara@microchip.com>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
include/dt-bindings/clock/microchip,mpfs-clock.h | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/include/dt-bindings/clock/microchip,mpfs-clock.h b/include/dt-bindings/clock/microchip,mpfs-clock.h
index 73f2a9324857..3cba46b9191f 100644
--- a/include/dt-bindings/clock/microchip,mpfs-clock.h
+++ b/include/dt-bindings/clock/microchip,mpfs-clock.h
@@ -1,15 +1,18 @@
/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
/*
* Daire McNamara,<daire.mcnamara@microchip.com>
- * Copyright (C) 2020 Microchip Technology Inc. All rights reserved.
+ * Copyright (C) 2020-2022 Microchip Technology Inc. All rights reserved.
*/
#ifndef _DT_BINDINGS_CLK_MICROCHIP_MPFS_H_
#define _DT_BINDINGS_CLK_MICROCHIP_MPFS_H_
+#define CLK_MSSPLL 34
+
#define CLK_CPU 0
#define CLK_AXI 1
#define CLK_AHB 2
+#define CLK_RTCREF 33
#define CLK_ENVM 3
#define CLK_MAC0 4
--
2.35.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v1 3/7] dt-bindings: rtc: add refclk to mpfs-rtc
2022-04-08 14:36 [PATCH v1 0/7] Add rtc refclk support for PolarFire SoC Conor Dooley
2022-04-08 14:36 ` [PATCH v1 1/7] dt-bindings: clk: mpfs document msspll dri registers Conor Dooley
2022-04-08 14:36 ` [PATCH v1 2/7] dt-bindings: clk: mpfs: add defines for two new clocks Conor Dooley
@ 2022-04-08 14:36 ` Conor Dooley
2022-04-08 14:56 ` Krzysztof Kozlowski
2022-04-08 14:36 ` [PATCH v1 4/7] clk: microchip: mpfs: re-parent the configurable clocks Conor Dooley
` (4 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Conor Dooley @ 2022-04-08 14:36 UTC (permalink / raw)
To: mturquette, sboyd, aou, paul.walmsley, palmer, a.zummo,
alexandre.belloni, robh+dt, krzk+dt
Cc: daire.mcnamara, linux-rtc, devicetree, linux-clk, linux-riscv,
Conor Dooley
The rtc on PolarFire SoC does not use the AHB clock as its reference
frequency, but rather a 1 MHz refclk that it shares with MTIMER. Add
this second clock to the binding as a required property.
Reviewed-by: Daire McNamara <daire.mcnamara@microchip.com>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
.../bindings/rtc/microchip,mfps-rtc.yaml | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/rtc/microchip,mfps-rtc.yaml b/Documentation/devicetree/bindings/rtc/microchip,mfps-rtc.yaml
index a2e984ea3553..1ffd97dbe6b9 100644
--- a/Documentation/devicetree/bindings/rtc/microchip,mfps-rtc.yaml
+++ b/Documentation/devicetree/bindings/rtc/microchip,mfps-rtc.yaml
@@ -31,11 +31,18 @@ properties:
to that of the RTC's count register.
clocks:
- maxItems: 1
+ items:
+ - description: |
+ AHB clock
+ - description: |
+ Reference clock: divided by the prescaler to create a time-based strobe (typically 1 Hz)
+ for the calendar counter. By default, the rtc on the PolarFire SoC shares it's reference
+ with MTIMER so this will be a 1 MHz clock.
clock-names:
items:
- const: rtc
+ - const: rtcref
required:
- compatible
@@ -48,11 +55,12 @@ additionalProperties: false
examples:
- |
+ #include "dt-bindings/clock/microchip,mpfs-clock.h"
rtc@20124000 {
compatible = "microchip,mpfs-rtc";
reg = <0x20124000 0x1000>;
- clocks = <&clkcfg 21>;
- clock-names = "rtc";
+ clocks = <&clkcfg CLK_RTC>, <&clkcfg CLK_RTCREF>;
+ clock-names = "rtc", "rtcref";
interrupts = <80>, <81>;
};
...
--
2.35.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v1 4/7] clk: microchip: mpfs: re-parent the configurable clocks
2022-04-08 14:36 [PATCH v1 0/7] Add rtc refclk support for PolarFire SoC Conor Dooley
` (2 preceding siblings ...)
2022-04-08 14:36 ` [PATCH v1 3/7] dt-bindings: rtc: add refclk to mpfs-rtc Conor Dooley
@ 2022-04-08 14:36 ` Conor Dooley
2022-04-08 14:36 ` [PATCH v1 5/7] clk: microchip: mpfs: rename sys_base to base Conor Dooley
` (3 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Conor Dooley @ 2022-04-08 14:36 UTC (permalink / raw)
To: mturquette, sboyd, aou, paul.walmsley, palmer, a.zummo,
alexandre.belloni, robh+dt, krzk+dt
Cc: daire.mcnamara, linux-rtc, devicetree, linux-clk, linux-riscv,
Conor Dooley
Currently the mpfs clock driver uses a reference clock called the
"msspll", set in the device tree, as the parent for the cpu/axi/ahb
(config) clocks. The frequency of the msspll is determined by the FPGA
bitstream & the bootloader configures the clock to match the bitstream.
The real reference is provided by a 100 or 125 MHz off chip oscillator.
However, the msspll clock is not actually the parent of all clocks on
the system - the reference clock for the rtc/mtimer actually has the
off chip oscillator as its parent.
In order to fix this, add support for reading the configuration of the
msspll & reparent the "config" clocks so that they are derived from
this clock rather than the reference in the device tree.
Reviewed-by: Daire McNamara <daire.mcnamara@microchip.com>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
@Stephen/Mike: Is it acceptable to add the recalc rate without a set
rate? If not lmk and I will add one.
drivers/clk/microchip/clk-mpfs.c | 151 +++++++++++++++++++++++++++----
1 file changed, 132 insertions(+), 19 deletions(-)
diff --git a/drivers/clk/microchip/clk-mpfs.c b/drivers/clk/microchip/clk-mpfs.c
index c82a79a5979f..66251a5f4a03 100644
--- a/drivers/clk/microchip/clk-mpfs.c
+++ b/drivers/clk/microchip/clk-mpfs.c
@@ -11,19 +11,46 @@
#include <dt-bindings/clock/microchip,mpfs-clock.h>
/* 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 REG_CLOCK_CONFIG_CR 0x08u
#define REG_SUBBLK_CLOCK_CR 0x84u
+#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_clock_data {
void __iomem *base;
+ void __iomem *msspll_base;
struct clk_hw_onecell_data hw_data;
};
+struct mpfs_msspll_hw_clock {
+ void __iomem *base;
+ unsigned int id;
+ u32 reg_offset;
+ u32 shift;
+ u32 width;
+ u32 flags;
+ struct clk_hw hw;
+ struct clk_init_data init;
+};
+
+#define to_mpfs_msspll_clk(_hw) container_of(_hw, struct mpfs_msspll_hw_clock, hw)
+
struct mpfs_cfg_clock {
const struct clk_div_table *table;
unsigned int id;
+ u32 reg_offset;
u8 shift;
u8 width;
+ u8 flags;
};
struct mpfs_cfg_hw_clock {
@@ -54,7 +81,7 @@ struct mpfs_periph_hw_clock {
*/
static DEFINE_SPINLOCK(mpfs_clk_lock);
-static const struct clk_parent_data mpfs_cfg_parent[] = {
+static const struct clk_parent_data mpfs_ext_ref[] = {
{ .index = 0 },
};
@@ -68,6 +95,75 @@ static const struct clk_div_table mpfs_div_ahb_table[] = {
{ 0, 0 }
};
+static unsigned long mpfs_clk_msspll_recalc_rate(struct clk_hw *hw, unsigned long prate)
+{
+ 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;
+
+ mult = readl_relaxed(mult_addr) >> MSSPLL_FBDIV_SHIFT;
+ mult &= clk_div_mask(MSSPLL_FBDIV_WIDTH);
+ ref_div = readl_relaxed(ref_div_addr) >> MSSPLL_REFDIV_SHIFT;
+ ref_div &= clk_div_mask(MSSPLL_REFDIV_WIDTH);
+ postdiv = readl_relaxed(postdiv_addr) >> MSSPLL_POSTDIV_SHIFT;
+ postdiv &= clk_div_mask(MSSPLL_POSTDIV_WIDTH);
+
+ return prate * mult / (ref_div * MSSPLL_FIXED_DIV * postdiv);
+}
+
+static const struct clk_ops mpfs_clk_msspll_ops = {
+ .recalc_rate = mpfs_clk_msspll_recalc_rate,
+};
+
+#define CLK_PLL(_id, _name, _parent, _shift, _width, _flags, _offset) { \
+ .id = _id, \
+ .shift = _shift, \
+ .width = _width, \
+ .reg_offset = _offset, \
+ .flags = _flags, \
+ .hw.init = CLK_HW_INIT_PARENTS_DATA(_name, _parent, &mpfs_clk_msspll_ops, 0), \
+}
+
+static struct mpfs_msspll_hw_clock mpfs_msspll_clks[] = {
+ CLK_PLL(CLK_MSSPLL, "clk_msspll", mpfs_ext_ref, MSSPLL_FBDIV_SHIFT,
+ MSSPLL_FBDIV_WIDTH, 0, REG_MSSPLL_SSCG_2_CR),
+};
+
+static int mpfs_clk_register_msspll(struct device *dev, struct mpfs_msspll_hw_clock *msspll_hw,
+ void __iomem *base)
+{
+ msspll_hw->base = base;
+
+ return devm_clk_hw_register(dev, &msspll_hw->hw);
+}
+
+static int mpfs_clk_register_mssplls(struct device *dev, struct mpfs_msspll_hw_clock *msspll_hws,
+ unsigned int num_clks, struct mpfs_clock_data *data)
+{
+ void __iomem *base = data->msspll_base;
+ unsigned int i;
+ int ret;
+
+ for (i = 0; i < num_clks; i++) {
+ struct mpfs_msspll_hw_clock *msspll_hw = &msspll_hws[i];
+
+ ret = mpfs_clk_register_msspll(dev, msspll_hw, base);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to register msspll id: %d\n",
+ CLK_MSSPLL);
+
+ data->hw_data.hws[msspll_hw->id] = &msspll_hw->hw;
+ }
+
+ return 0;
+}
+
+/*
+ * "CFG" clocks
+ */
+
static unsigned long mpfs_cfg_clk_recalc_rate(struct clk_hw *hw, unsigned long prate)
{
struct mpfs_cfg_hw_clock *cfg_hw = to_mpfs_cfg_clk(hw);
@@ -75,10 +171,10 @@ static unsigned long mpfs_cfg_clk_recalc_rate(struct clk_hw *hw, unsigned long p
void __iomem *base_addr = cfg_hw->sys_base;
u32 val;
- val = readl_relaxed(base_addr + REG_CLOCK_CONFIG_CR) >> cfg->shift;
+ val = readl_relaxed(base_addr + cfg->reg_offset) >> cfg->shift;
val &= clk_div_mask(cfg->width);
- return prate / (1u << val);
+ return divider_recalc_rate(hw, prate, val, cfg->table, cfg->flags, cfg->width);
}
static long mpfs_cfg_clk_round_rate(struct clk_hw *hw, unsigned long rate, unsigned long *prate)
@@ -104,11 +200,10 @@ static int mpfs_cfg_clk_set_rate(struct clk_hw *hw, unsigned long rate, unsigned
return divider_setting;
spin_lock_irqsave(&mpfs_clk_lock, flags);
-
- val = readl_relaxed(base_addr + REG_CLOCK_CONFIG_CR);
+ val = readl_relaxed(base_addr + cfg->reg_offset);
val &= ~(clk_div_mask(cfg->width) << cfg_hw->cfg.shift);
val |= divider_setting << cfg->shift;
- writel_relaxed(val, base_addr + REG_CLOCK_CONFIG_CR);
+ writel_relaxed(val, base_addr + cfg->reg_offset);
spin_unlock_irqrestore(&mpfs_clk_lock, flags);
@@ -121,19 +216,23 @@ static const struct clk_ops mpfs_clk_cfg_ops = {
.set_rate = mpfs_cfg_clk_set_rate,
};
-#define CLK_CFG(_id, _name, _parent, _shift, _width, _table, _flags) { \
- .cfg.id = _id, \
- .cfg.shift = _shift, \
- .cfg.width = _width, \
- .cfg.table = _table, \
- .hw.init = CLK_HW_INIT_PARENTS_DATA(_name, _parent, &mpfs_clk_cfg_ops, \
- _flags), \
+#define CLK_CFG(_id, _name, _parent, _shift, _width, _table, _flags, _offset) { \
+ .cfg.id = _id, \
+ .cfg.shift = _shift, \
+ .cfg.width = _width, \
+ .cfg.table = _table, \
+ .cfg.reg_offset = _offset, \
+ .cfg.flags = _flags, \
+ .hw.init = CLK_HW_INIT(_name, _parent, &mpfs_clk_cfg_ops, 0), \
}
static struct mpfs_cfg_hw_clock mpfs_cfg_clks[] = {
- CLK_CFG(CLK_CPU, "clk_cpu", mpfs_cfg_parent, 0, 2, mpfs_div_cpu_axi_table, 0),
- CLK_CFG(CLK_AXI, "clk_axi", mpfs_cfg_parent, 2, 2, mpfs_div_cpu_axi_table, 0),
- CLK_CFG(CLK_AHB, "clk_ahb", mpfs_cfg_parent, 4, 2, mpfs_div_ahb_table, 0),
+ CLK_CFG(CLK_CPU, "clk_cpu", "clk_msspll", 0, 2, mpfs_div_cpu_axi_table, 0,
+ REG_CLOCK_CONFIG_CR),
+ CLK_CFG(CLK_AXI, "clk_axi", "clk_msspll", 2, 2, mpfs_div_cpu_axi_table, 0,
+ REG_CLOCK_CONFIG_CR),
+ CLK_CFG(CLK_AHB, "clk_ahb", "clk_msspll", 4, 2, mpfs_div_ahb_table, 0,
+ REG_CLOCK_CONFIG_CR),
};
static int mpfs_clk_register_cfg(struct device *dev, struct mpfs_cfg_hw_clock *cfg_hw,
@@ -159,13 +258,17 @@ static int mpfs_clk_register_cfgs(struct device *dev, struct mpfs_cfg_hw_clock *
return dev_err_probe(dev, ret, "failed to register clock id: %d\n",
cfg_hw->cfg.id);
- id = cfg_hws[i].cfg.id;
+ id = cfg_hw->cfg.id;
data->hw_data.hws[id] = &cfg_hw->hw;
}
return 0;
}
+/*
+ * peripheral clocks - devices connected to axi or ahb buses.
+ */
+
static int mpfs_periph_clk_enable(struct clk_hw *hw)
{
struct mpfs_periph_hw_clock *periph_hw = to_mpfs_periph_clk(hw);
@@ -312,8 +415,9 @@ static int mpfs_clk_probe(struct platform_device *pdev)
unsigned int num_clks;
int ret;
- /* CLK_RESERVED is not part of cfg_clks nor periph_clks, so add 1 */
- num_clks = ARRAY_SIZE(mpfs_cfg_clks) + ARRAY_SIZE(mpfs_periph_clks) + 1;
+ /* CLK_RESERVED is not part of clock arrays, so add 1 */
+ num_clks = ARRAY_SIZE(mpfs_msspll_clks) + ARRAY_SIZE(mpfs_cfg_clks)
+ + ARRAY_SIZE(mpfs_periph_clks) + 1;
clk_data = devm_kzalloc(dev, struct_size(clk_data, hw_data.hws, num_clks), GFP_KERNEL);
if (!clk_data)
@@ -323,8 +427,17 @@ static int mpfs_clk_probe(struct platform_device *pdev)
if (IS_ERR(clk_data->base))
return PTR_ERR(clk_data->base);
+ clk_data->msspll_base = devm_platform_ioremap_resource(pdev, 1);
+ if (IS_ERR(clk_data->msspll_base))
+ return PTR_ERR(clk_data->msspll_base);
+
clk_data->hw_data.num = num_clks;
+ ret = mpfs_clk_register_mssplls(dev, mpfs_msspll_clks, ARRAY_SIZE(mpfs_msspll_clks),
+ clk_data);
+ if (ret)
+ return ret;
+
ret = mpfs_clk_register_cfgs(dev, mpfs_cfg_clks, ARRAY_SIZE(mpfs_cfg_clks), clk_data);
if (ret)
return ret;
--
2.35.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v1 5/7] clk: microchip: mpfs: rename sys_base to base
2022-04-08 14:36 [PATCH v1 0/7] Add rtc refclk support for PolarFire SoC Conor Dooley
` (3 preceding siblings ...)
2022-04-08 14:36 ` [PATCH v1 4/7] clk: microchip: mpfs: re-parent the configurable clocks Conor Dooley
@ 2022-04-08 14:36 ` Conor Dooley
2022-04-08 14:36 ` [PATCH v1 6/7] clk: microchip: mpfs: add RTCREF clock control Conor Dooley
` (2 subsequent siblings)
7 siblings, 0 replies; 17+ messages in thread
From: Conor Dooley @ 2022-04-08 14:36 UTC (permalink / raw)
To: mturquette, sboyd, aou, paul.walmsley, palmer, a.zummo,
alexandre.belloni, robh+dt, krzk+dt
Cc: daire.mcnamara, linux-rtc, devicetree, linux-clk, linux-riscv,
Conor Dooley
Having added a second set of registers for the msspll, sys_base no longer
really makes sense as a variable name. Renaming it to base will make it
consistent with mpfs_clock_data & several function arguments.
Reviewed-by: Daire McNamara <daire.mcnamara@microchip.com>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
drivers/clk/microchip/clk-mpfs.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/clk/microchip/clk-mpfs.c b/drivers/clk/microchip/clk-mpfs.c
index 66251a5f4a03..f22d4b40ef28 100644
--- a/drivers/clk/microchip/clk-mpfs.c
+++ b/drivers/clk/microchip/clk-mpfs.c
@@ -55,7 +55,7 @@ struct mpfs_cfg_clock {
struct mpfs_cfg_hw_clock {
struct mpfs_cfg_clock cfg;
- void __iomem *sys_base;
+ void __iomem *base;
struct clk_hw hw;
struct clk_init_data init;
};
@@ -69,7 +69,7 @@ struct mpfs_periph_clock {
struct mpfs_periph_hw_clock {
struct mpfs_periph_clock periph;
- void __iomem *sys_base;
+ void __iomem *base;
struct clk_hw hw;
};
@@ -168,7 +168,7 @@ static unsigned long mpfs_cfg_clk_recalc_rate(struct clk_hw *hw, unsigned long p
{
struct mpfs_cfg_hw_clock *cfg_hw = to_mpfs_cfg_clk(hw);
struct mpfs_cfg_clock *cfg = &cfg_hw->cfg;
- void __iomem *base_addr = cfg_hw->sys_base;
+ void __iomem *base_addr = cfg_hw->base;
u32 val;
val = readl_relaxed(base_addr + cfg->reg_offset) >> cfg->shift;
@@ -189,7 +189,7 @@ static int mpfs_cfg_clk_set_rate(struct clk_hw *hw, unsigned long rate, unsigned
{
struct mpfs_cfg_hw_clock *cfg_hw = to_mpfs_cfg_clk(hw);
struct mpfs_cfg_clock *cfg = &cfg_hw->cfg;
- void __iomem *base_addr = cfg_hw->sys_base;
+ void __iomem *base_addr = cfg_hw->base;
unsigned long flags;
u32 val;
int divider_setting;
@@ -236,9 +236,9 @@ static struct mpfs_cfg_hw_clock mpfs_cfg_clks[] = {
};
static int mpfs_clk_register_cfg(struct device *dev, struct mpfs_cfg_hw_clock *cfg_hw,
- void __iomem *sys_base)
+ void __iomem *base)
{
- cfg_hw->sys_base = sys_base;
+ cfg_hw->base = base;
return devm_clk_hw_register(dev, &cfg_hw->hw);
}
@@ -246,14 +246,14 @@ static int mpfs_clk_register_cfg(struct device *dev, struct mpfs_cfg_hw_clock *c
static int mpfs_clk_register_cfgs(struct device *dev, struct mpfs_cfg_hw_clock *cfg_hws,
unsigned int num_clks, struct mpfs_clock_data *data)
{
- void __iomem *sys_base = data->base;
+ void __iomem *base = data->base;
unsigned int i, id;
int ret;
for (i = 0; i < num_clks; i++) {
struct mpfs_cfg_hw_clock *cfg_hw = &cfg_hws[i];
- ret = mpfs_clk_register_cfg(dev, cfg_hw, sys_base);
+ ret = mpfs_clk_register_cfg(dev, cfg_hw, base);
if (ret)
return dev_err_probe(dev, ret, "failed to register clock id: %d\n",
cfg_hw->cfg.id);
@@ -273,7 +273,7 @@ static int mpfs_periph_clk_enable(struct clk_hw *hw)
{
struct mpfs_periph_hw_clock *periph_hw = to_mpfs_periph_clk(hw);
struct mpfs_periph_clock *periph = &periph_hw->periph;
- void __iomem *base_addr = periph_hw->sys_base;
+ void __iomem *base_addr = periph_hw->base;
u32 reg, val;
unsigned long flags;
@@ -292,7 +292,7 @@ static void mpfs_periph_clk_disable(struct clk_hw *hw)
{
struct mpfs_periph_hw_clock *periph_hw = to_mpfs_periph_clk(hw);
struct mpfs_periph_clock *periph = &periph_hw->periph;
- void __iomem *base_addr = periph_hw->sys_base;
+ void __iomem *base_addr = periph_hw->base;
u32 reg, val;
unsigned long flags;
@@ -309,7 +309,7 @@ static int mpfs_periph_clk_is_enabled(struct clk_hw *hw)
{
struct mpfs_periph_hw_clock *periph_hw = to_mpfs_periph_clk(hw);
struct mpfs_periph_clock *periph = &periph_hw->periph;
- void __iomem *base_addr = periph_hw->sys_base;
+ void __iomem *base_addr = periph_hw->base;
u32 reg;
reg = readl_relaxed(base_addr + REG_SUBBLK_CLOCK_CR);
@@ -379,9 +379,9 @@ static struct mpfs_periph_hw_clock mpfs_periph_clks[] = {
};
static int mpfs_clk_register_periph(struct device *dev, struct mpfs_periph_hw_clock *periph_hw,
- void __iomem *sys_base)
+ void __iomem *base)
{
- periph_hw->sys_base = sys_base;
+ periph_hw->base = base;
return devm_clk_hw_register(dev, &periph_hw->hw);
}
@@ -389,14 +389,14 @@ static int mpfs_clk_register_periph(struct device *dev, struct mpfs_periph_hw_cl
static int mpfs_clk_register_periphs(struct device *dev, struct mpfs_periph_hw_clock *periph_hws,
int num_clks, struct mpfs_clock_data *data)
{
- void __iomem *sys_base = data->base;
+ void __iomem *base = data->base;
unsigned int i, id;
int ret;
for (i = 0; i < num_clks; i++) {
struct mpfs_periph_hw_clock *periph_hw = &periph_hws[i];
- ret = mpfs_clk_register_periph(dev, periph_hw, sys_base);
+ ret = mpfs_clk_register_periph(dev, periph_hw, base);
if (ret)
return dev_err_probe(dev, ret, "failed to register clock id: %d\n",
periph_hw->periph.id);
--
2.35.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v1 6/7] clk: microchip: mpfs: add RTCREF clock control
2022-04-08 14:36 [PATCH v1 0/7] Add rtc refclk support for PolarFire SoC Conor Dooley
` (4 preceding siblings ...)
2022-04-08 14:36 ` [PATCH v1 5/7] clk: microchip: mpfs: rename sys_base to base Conor Dooley
@ 2022-04-08 14:36 ` Conor Dooley
2022-04-08 14:36 ` [PATCH v1 7/7] riscv: dts: microchip: reparent mpfs clocks Conor Dooley
2022-04-08 14:57 ` [PATCH v1 0/7] Add rtc refclk support for PolarFire SoC Krzysztof Kozlowski
7 siblings, 0 replies; 17+ messages in thread
From: Conor Dooley @ 2022-04-08 14:36 UTC (permalink / raw)
To: mturquette, sboyd, aou, paul.walmsley, palmer, a.zummo,
alexandre.belloni, robh+dt, krzk+dt
Cc: daire.mcnamara, linux-rtc, devicetree, linux-clk, linux-riscv,
Conor Dooley
The reference clock used by the PolarFire SoC's onboard rtc was missing
from the clock driver. Add this clock at the "config" clock level, with
the external reference clock as its parent.
Reviewed-by: Daire McNamara <daire.mcnamara@microchip.com>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
drivers/clk/microchip/clk-mpfs.c | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/microchip/clk-mpfs.c b/drivers/clk/microchip/clk-mpfs.c
index f22d4b40ef28..4a506d0140d4 100644
--- a/drivers/clk/microchip/clk-mpfs.c
+++ b/drivers/clk/microchip/clk-mpfs.c
@@ -15,6 +15,7 @@
#define REG_MSSPLL_POSTDIV_CR 0x10u
#define REG_MSSPLL_SSCG_2_CR 0x2Cu
#define REG_CLOCK_CONFIG_CR 0x08u
+#define REG_RTC_CLOCK_CR 0x0Cu
#define REG_SUBBLK_CLOCK_CR 0x84u
#define MSSPLL_FBDIV_SHIFT 0x00u
@@ -95,6 +96,17 @@ static const struct clk_div_table mpfs_div_ahb_table[] = {
{ 0, 0 }
};
+/*
+ * The only two supported reference clock frequencies for the PolarFire SoC are
+ * 100 and 125 MHz, as the rtc reference is required to be 1 MHz.
+ * It therefore only needs to have divider table entries corresponding to
+ * divide by 100 and 125.
+ */
+static const struct clk_div_table mpfs_div_rtcref_table[] = {
+ { 100, 100 }, { 125, 125 },
+ { 0, 0 }
+};
+
static unsigned long mpfs_clk_msspll_recalc_rate(struct clk_hw *hw, unsigned long prate)
{
struct mpfs_msspll_hw_clock *msspll_hw = to_mpfs_msspll_clk(hw);
@@ -233,6 +245,16 @@ static struct mpfs_cfg_hw_clock mpfs_cfg_clks[] = {
REG_CLOCK_CONFIG_CR),
CLK_CFG(CLK_AHB, "clk_ahb", "clk_msspll", 4, 2, mpfs_div_ahb_table, 0,
REG_CLOCK_CONFIG_CR),
+ {
+ .cfg.id = CLK_RTCREF,
+ .cfg.shift = 0,
+ .cfg.width = 12,
+ .cfg.table = mpfs_div_rtcref_table,
+ .cfg.reg_offset = REG_RTC_CLOCK_CR,
+ .cfg.flags = CLK_DIVIDER_ONE_BASED,
+ .hw.init =
+ CLK_HW_INIT_PARENTS_DATA("clk_rtcref", mpfs_ext_ref, &mpfs_clk_cfg_ops, 0),
+ }
};
static int mpfs_clk_register_cfg(struct device *dev, struct mpfs_cfg_hw_clock *cfg_hw,
@@ -351,7 +373,7 @@ static struct mpfs_periph_hw_clock mpfs_periph_clks[] = {
CLK_PERIPH(CLK_MAC0, "clk_periph_mac0", PARENT_CLK(AHB), 1, 0),
CLK_PERIPH(CLK_MAC1, "clk_periph_mac1", PARENT_CLK(AHB), 2, 0),
CLK_PERIPH(CLK_MMC, "clk_periph_mmc", PARENT_CLK(AHB), 3, 0),
- CLK_PERIPH(CLK_TIMER, "clk_periph_timer", PARENT_CLK(AHB), 4, 0),
+ CLK_PERIPH(CLK_TIMER, "clk_periph_timer", PARENT_CLK(RTCREF), 4, 0),
CLK_PERIPH(CLK_MMUART0, "clk_periph_mmuart0", PARENT_CLK(AHB), 5, CLK_IS_CRITICAL),
CLK_PERIPH(CLK_MMUART1, "clk_periph_mmuart1", PARENT_CLK(AHB), 6, 0),
CLK_PERIPH(CLK_MMUART2, "clk_periph_mmuart2", PARENT_CLK(AHB), 7, 0),
--
2.35.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v1 7/7] riscv: dts: microchip: reparent mpfs clocks
2022-04-08 14:36 [PATCH v1 0/7] Add rtc refclk support for PolarFire SoC Conor Dooley
` (5 preceding siblings ...)
2022-04-08 14:36 ` [PATCH v1 6/7] clk: microchip: mpfs: add RTCREF clock control Conor Dooley
@ 2022-04-08 14:36 ` Conor Dooley
2022-04-08 14:57 ` [PATCH v1 0/7] Add rtc refclk support for PolarFire SoC Krzysztof Kozlowski
7 siblings, 0 replies; 17+ messages in thread
From: Conor Dooley @ 2022-04-08 14:36 UTC (permalink / raw)
To: mturquette, sboyd, aou, paul.walmsley, palmer, a.zummo,
alexandre.belloni, robh+dt, krzk+dt
Cc: daire.mcnamara, linux-rtc, devicetree, linux-clk, linux-riscv,
Conor Dooley
The 600M clock in the fabric is not the real reference, replace it with
a 125M clock which is the correct value for the icicle kit. Rename the
msspllclk node to mssrefclk since this is now the input to, not the
output of, the msspll clock. Control of the msspll clock has been moved
into the clock configurator, so add the register range for it to the clk
configurator. Finally, add a new output of the clock config block which
will provide the 1M reference clock for the MTIMER and the rtc.
Reviewed-by: Daire McNamara <daire.mcnamara@microchip.com>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
.../boot/dts/microchip/microchip-mpfs-icicle-kit.dts | 2 +-
arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi | 8 ++++----
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts b/arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts
index cd2fe80fa81a..3392153dd0f1 100644
--- a/arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts
+++ b/arch/riscv/boot/dts/microchip/microchip-mpfs-icicle-kit.dts
@@ -45,7 +45,7 @@ ddrc_cache_hi: memory@1000000000 {
};
&refclk {
- clock-frequency = <600000000>;
+ clock-frequency = <125000000>;
};
&mmuart1 {
diff --git a/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi b/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi
index 3b48b7f35410..746c4d4e7686 100644
--- a/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi
+++ b/arch/riscv/boot/dts/microchip/microchip-mpfs.dtsi
@@ -141,7 +141,7 @@ cpu4_intc: interrupt-controller {
};
};
- refclk: msspllclk {
+ refclk: mssrefclk {
compatible = "fixed-clock";
#clock-cells = <0>;
};
@@ -190,7 +190,7 @@ plic: interrupt-controller@c000000 {
clkcfg: clkcfg@20002000 {
compatible = "microchip,mpfs-clkcfg";
- reg = <0x0 0x20002000 0x0 0x1000>;
+ reg = <0x0 0x20002000 0x0 0x1000>, <0x0 0x3E001000 0x0 0x1000>;
clocks = <&refclk>;
#clock-cells = <1>;
};
@@ -393,8 +393,8 @@ rtc: rtc@20124000 {
reg = <0x0 0x20124000 0x0 0x1000>;
interrupt-parent = <&plic>;
interrupts = <80>, <81>;
- clocks = <&clkcfg CLK_RTC>;
- clock-names = "rtc";
+ clocks = <&clkcfg CLK_RTC>, <&clkcfg CLK_RTCREF>;
+ clock-names = "rtc", "rtcref";
status = "disabled";
};
--
2.35.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v1 1/7] dt-bindings: clk: mpfs document msspll dri registers
2022-04-08 14:36 ` [PATCH v1 1/7] dt-bindings: clk: mpfs document msspll dri registers Conor Dooley
@ 2022-04-08 14:54 ` Krzysztof Kozlowski
0 siblings, 0 replies; 17+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-08 14:54 UTC (permalink / raw)
To: Conor Dooley, mturquette, sboyd, aou, paul.walmsley, palmer,
a.zummo, alexandre.belloni, robh+dt, krzk+dt
Cc: daire.mcnamara, linux-rtc, devicetree, linux-clk, linux-riscv
On 08/04/2022 16:36, Conor Dooley wrote:
> As there are two sections of registers that are responsible for clock
> configuration on the PolarFire SoC: add the dynamic reconfiguration
> interface section to the binding & describe what each of the sections
> are used for.
(...)
>
> reg:
> - maxItems: 1
> + items:
> + - description: |
> + clock config registers:
> + These registers contain enable, reset & divider tables for the, cpu, axi, ahb and
> + rtc/mtimer reference clocks as well as enable and reset for the peripheral clocks.
> + - description: |
> + mss pll dri registers:
> + Block of registers responsible for dynamic reconfiguration of the mss pll
>
This breaks all of DTS - in and out of tree.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1 3/7] dt-bindings: rtc: add refclk to mpfs-rtc
2022-04-08 14:36 ` [PATCH v1 3/7] dt-bindings: rtc: add refclk to mpfs-rtc Conor Dooley
@ 2022-04-08 14:56 ` Krzysztof Kozlowski
0 siblings, 0 replies; 17+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-08 14:56 UTC (permalink / raw)
To: Conor Dooley, mturquette, sboyd, aou, paul.walmsley, palmer,
a.zummo, alexandre.belloni, robh+dt, krzk+dt
Cc: daire.mcnamara, linux-rtc, devicetree, linux-clk, linux-riscv
On 08/04/2022 16:36, Conor Dooley wrote:
> The rtc on PolarFire SoC does not use the AHB clock as its reference
> frequency, but rather a 1 MHz refclk that it shares with MTIMER. Add
> this second clock to the binding as a required property.
>
> Reviewed-by: Daire McNamara <daire.mcnamara@microchip.com>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> .../bindings/rtc/microchip,mfps-rtc.yaml | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/rtc/microchip,mfps-rtc.yaml b/Documentation/devicetree/bindings/rtc/microchip,mfps-rtc.yaml
> index a2e984ea3553..1ffd97dbe6b9 100644
> --- a/Documentation/devicetree/bindings/rtc/microchip,mfps-rtc.yaml
> +++ b/Documentation/devicetree/bindings/rtc/microchip,mfps-rtc.yaml
> @@ -31,11 +31,18 @@ properties:
> to that of the RTC's count register.
>
> clocks:
> - maxItems: 1
> + items:
> + - description: |
> + AHB clock
> + - description: |
Same as your patch #1 - this breaks the ABI.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1 0/7] Add rtc refclk support for PolarFire SoC
2022-04-08 14:36 [PATCH v1 0/7] Add rtc refclk support for PolarFire SoC Conor Dooley
` (6 preceding siblings ...)
2022-04-08 14:36 ` [PATCH v1 7/7] riscv: dts: microchip: reparent mpfs clocks Conor Dooley
@ 2022-04-08 14:57 ` Krzysztof Kozlowski
2022-04-08 15:29 ` Conor Dooley
7 siblings, 1 reply; 17+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-08 14:57 UTC (permalink / raw)
To: Conor Dooley, mturquette, sboyd, aou, paul.walmsley, palmer,
a.zummo, alexandre.belloni, robh+dt, krzk+dt
Cc: daire.mcnamara, linux-rtc, devicetree, linux-clk, linux-riscv
On 08/04/2022 16:36, Conor Dooley wrote:
> Hey,
> As I mentioned in my fixes for 5.18 [0], found out that the reference
> clock for the rtc is actually missing from the clock driver (and the
> dt binding).
>
> Currently the mpfs clock driver uses a reference clock called the
> "msspll", set in the device tree, as the parent for the cpu/axi/ahb
> (config) clocks. The frequency of the msspll is determined by the FPGA
> bitstream & the bootloader configures the clock to match the bitstream.
> The real reference is provided by a 100 or 125 MHz off chip oscillator.
>
> However, the msspll clock is not actually the parent of all clocks on
> the system - the reference clock for the rtc/mtimer actually has the
> off chip oscillator as its parent.
>
> This series enables reading the rate of the msspll clock, converts
> the refclock in the device tree to the external reference & adds
> the missing rtc reference clock.
>
> I assume it is okay not to add fixes tags for the rtc dt binding?
> Since the clock was previously missing, the binding is wrong, but
> idk if that qualifies as a fix?
Usually ABI breakage, even if accepted, should be be tagged as fix
because it is clearly then a break of other peoples' trees...
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1 0/7] Add rtc refclk support for PolarFire SoC
2022-04-08 14:57 ` [PATCH v1 0/7] Add rtc refclk support for PolarFire SoC Krzysztof Kozlowski
@ 2022-04-08 15:29 ` Conor Dooley
2022-04-09 7:14 ` Conor Dooley
2022-04-09 10:45 ` Krzysztof Kozlowski
0 siblings, 2 replies; 17+ messages in thread
From: Conor Dooley @ 2022-04-08 15:29 UTC (permalink / raw)
To: Krzysztof Kozlowski, Conor Dooley, mturquette, sboyd, aou,
paul.walmsley, palmer, a.zummo, alexandre.belloni, robh+dt,
krzk+dt
Cc: daire.mcnamara, linux-rtc, devicetree, linux-clk, linux-riscv
On 08/04/2022 15:57, Krzysztof Kozlowski wrote:
> On 08/04/2022 16:36, Conor Dooley wrote:
>> Hey,
>> As I mentioned in my fixes for 5.18 [0], found out that the reference
>> clock for the rtc is actually missing from the clock driver (and the
>> dt binding).
>>
>> Currently the mpfs clock driver uses a reference clock called the
>> "msspll", set in the device tree, as the parent for the cpu/axi/ahb
>> (config) clocks. The frequency of the msspll is determined by the FPGA
>> bitstream & the bootloader configures the clock to match the bitstream.
>> The real reference is provided by a 100 or 125 MHz off chip oscillator.
>>
>> However, the msspll clock is not actually the parent of all clocks on
>> the system - the reference clock for the rtc/mtimer actually has the
>> off chip oscillator as its parent.
>>
>> This series enables reading the rate of the msspll clock, converts
>> the refclock in the device tree to the external reference & adds
>> the missing rtc reference clock.
>>
>> I assume it is okay not to add fixes tags for the rtc dt binding?
>> Since the clock was previously missing, the binding is wrong, but
>> idk if that qualifies as a fix?
>
> Usually ABI breakage, even if accepted, should be be tagged as fix
> because it is clearly then a break of other peoples' trees...
>
That means either a) do something messy in the clock driver or b) mark
the whole series as fixes (and roll it into [0]).
The second option seems far more sensible to me, do you agree?
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1 0/7] Add rtc refclk support for PolarFire SoC
2022-04-08 15:29 ` Conor Dooley
@ 2022-04-09 7:14 ` Conor Dooley
2022-04-09 10:48 ` Krzysztof Kozlowski
2022-04-09 10:45 ` Krzysztof Kozlowski
1 sibling, 1 reply; 17+ messages in thread
From: Conor Dooley @ 2022-04-09 7:14 UTC (permalink / raw)
To: Krzysztof Kozlowski, Conor Dooley, sboyd, aou, paul.walmsley,
palmer, a.zummo, alexandre.belloni, robh+dt, krzk+dt, mturquette
Cc: daire.mcnamara, linux-rtc, devicetree, linux-clk, linux-riscv
On 08/04/2022 16:29, Conor Dooley wrote:
>
>
> On 08/04/2022 15:57, Krzysztof Kozlowski wrote:
>> On 08/04/2022 16:36, Conor Dooley wrote:
>>> Hey,
>>> As I mentioned in my fixes for 5.18 [0], found out that the reference
>>> clock for the rtc is actually missing from the clock driver (and the
>>> dt binding).
>>>
>>> Currently the mpfs clock driver uses a reference clock called the
>>> "msspll", set in the device tree, as the parent for the cpu/axi/ahb
>>> (config) clocks. The frequency of the msspll is determined by the FPGA
>>> bitstream & the bootloader configures the clock to match the bitstream.
>>> The real reference is provided by a 100 or 125 MHz off chip oscillator.
>>>
>>> However, the msspll clock is not actually the parent of all clocks on
>>> the system - the reference clock for the rtc/mtimer actually has the
>>> off chip oscillator as its parent.
>>>
>>> This series enables reading the rate of the msspll clock, converts
>>> the refclock in the device tree to the external reference & adds
>>> the missing rtc reference clock.
>>>
>>> I assume it is okay not to add fixes tags for the rtc dt binding?
>>> Since the clock was previously missing, the binding is wrong, but
>>> idk if that qualifies as a fix?
>>
>> Usually ABI breakage, even if accepted, should be be tagged as fix
>> because it is clearly then a break of other peoples' trees...
>>
>
> That means either a) do something messy in the clock driver or b) mark
> the whole series as fixes (and roll it into [0]).
>
> The second option seems far more sensible to me, do you agree?
Having thought some more about it, patches 2, 3 and the rtc part of 7
should be moved into [0] since they're fixing a binding that only
arrived in 5.18-rc1.
For the rest, make the second part of the reg optional and if it doesnt
exist just return prate for the msspll clock?
Thanks,
Conor.
[0]
https://lore.kernel.org/linux-riscv/20220408133543.3537118-1-conor.dooley@microchip.com/
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1 0/7] Add rtc refclk support for PolarFire SoC
2022-04-08 15:29 ` Conor Dooley
2022-04-09 7:14 ` Conor Dooley
@ 2022-04-09 10:45 ` Krzysztof Kozlowski
1 sibling, 0 replies; 17+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-09 10:45 UTC (permalink / raw)
To: Conor Dooley, Conor Dooley, mturquette, sboyd, aou, paul.walmsley,
palmer, a.zummo, alexandre.belloni, robh+dt, krzk+dt
Cc: daire.mcnamara, linux-rtc, devicetree, linux-clk, linux-riscv
On 08/04/2022 17:29, Conor Dooley wrote:
>
>
> On 08/04/2022 15:57, Krzysztof Kozlowski wrote:
>> On 08/04/2022 16:36, Conor Dooley wrote:
>>> Hey,
>>> As I mentioned in my fixes for 5.18 [0], found out that the reference
>>> clock for the rtc is actually missing from the clock driver (and the
>>> dt binding).
>>>
>>> Currently the mpfs clock driver uses a reference clock called the
>>> "msspll", set in the device tree, as the parent for the cpu/axi/ahb
>>> (config) clocks. The frequency of the msspll is determined by the FPGA
>>> bitstream & the bootloader configures the clock to match the bitstream.
>>> The real reference is provided by a 100 or 125 MHz off chip oscillator.
>>>
>>> However, the msspll clock is not actually the parent of all clocks on
>>> the system - the reference clock for the rtc/mtimer actually has the
>>> off chip oscillator as its parent.
>>>
>>> This series enables reading the rate of the msspll clock, converts
>>> the refclock in the device tree to the external reference & adds
>>> the missing rtc reference clock.
>>>
>>> I assume it is okay not to add fixes tags for the rtc dt binding?
>>> Since the clock was previously missing, the binding is wrong, but
>>> idk if that qualifies as a fix?
>>
>> Usually ABI breakage, even if accepted, should be be tagged as fix
>> because it is clearly then a break of other peoples' trees...
>>
>
> That means either a) do something messy in the clock driver or b) mark
> the whole series as fixes (and roll it into [0]).
>
> The second option seems far more sensible to me, do you agree?
I think ate part of my sentence... it should be:
"Usually ABI breakage, even if accepted, should NOT be tagged as fix..."
So usually it should not be a fix.
The binding actually could be backported, because the driver changes
bring the real ABI breakage.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1 0/7] Add rtc refclk support for PolarFire SoC
2022-04-09 7:14 ` Conor Dooley
@ 2022-04-09 10:48 ` Krzysztof Kozlowski
2022-04-09 20:17 ` Conor Dooley
0 siblings, 1 reply; 17+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-09 10:48 UTC (permalink / raw)
To: Conor Dooley, Conor Dooley, sboyd, aou, paul.walmsley, palmer,
a.zummo, alexandre.belloni, robh+dt, krzk+dt, mturquette
Cc: daire.mcnamara, linux-rtc, devicetree, linux-clk, linux-riscv
On 09/04/2022 09:14, Conor Dooley wrote:
>
>
> On 08/04/2022 16:29, Conor Dooley wrote:
>>
>>
>> On 08/04/2022 15:57, Krzysztof Kozlowski wrote:
>>> On 08/04/2022 16:36, Conor Dooley wrote:
>>>> Hey,
>>>> As I mentioned in my fixes for 5.18 [0], found out that the reference
>>>> clock for the rtc is actually missing from the clock driver (and the
>>>> dt binding).
>>>>
>>>> Currently the mpfs clock driver uses a reference clock called the
>>>> "msspll", set in the device tree, as the parent for the cpu/axi/ahb
>>>> (config) clocks. The frequency of the msspll is determined by the FPGA
>>>> bitstream & the bootloader configures the clock to match the bitstream.
>>>> The real reference is provided by a 100 or 125 MHz off chip oscillator.
>>>>
>>>> However, the msspll clock is not actually the parent of all clocks on
>>>> the system - the reference clock for the rtc/mtimer actually has the
>>>> off chip oscillator as its parent.
>>>>
>>>> This series enables reading the rate of the msspll clock, converts
>>>> the refclock in the device tree to the external reference & adds
>>>> the missing rtc reference clock.
>>>>
>>>> I assume it is okay not to add fixes tags for the rtc dt binding?
>>>> Since the clock was previously missing, the binding is wrong, but
>>>> idk if that qualifies as a fix?
>>>
>>> Usually ABI breakage, even if accepted, should be be tagged as fix
>>> because it is clearly then a break of other peoples' trees...
>>>
>>
>> That means either a) do something messy in the clock driver or b) mark
>> the whole series as fixes (and roll it into [0]).
>>
>> The second option seems far more sensible to me, do you agree?
>
> Having thought some more about it, patches 2, 3 and the rtc part of 7
> should be moved into [0] since they're fixing a binding that only
> arrived in 5.18-rc1.
> For the rest, make the second part of the reg optional and if it doesnt
> exist just return prate for the msspll clock?
Ah, so this got into v5.18-rc1? I think I missed that information from
the patches description and focused on backporting to stables. Then
indeed you could combine all fixes together, mark them with Fixes.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1 0/7] Add rtc refclk support for PolarFire SoC
2022-04-09 10:48 ` Krzysztof Kozlowski
@ 2022-04-09 20:17 ` Conor Dooley
2022-04-10 8:12 ` Krzysztof Kozlowski
0 siblings, 1 reply; 17+ messages in thread
From: Conor Dooley @ 2022-04-09 20:17 UTC (permalink / raw)
To: Krzysztof Kozlowski, Conor Dooley, sboyd, aou, paul.walmsley,
palmer, a.zummo, alexandre.belloni, robh+dt, krzk+dt, mturquette
Cc: daire.mcnamara, linux-rtc, devicetree, linux-clk, linux-riscv
On 09/04/2022 11:48, Krzysztof Kozlowski wrote:
> On 09/04/2022 09:14, Conor Dooley wrote:
>> On 08/04/2022 16:29, Conor Dooley wrote:
>>> On 08/04/2022 15:57, Krzysztof Kozlowski wrote:
>>>> On 08/04/2022 16:36, Conor Dooley wrote:
>>>>> Hey,
>>>>> As I mentioned in my fixes for 5.18 [0], found out that the reference
>>>>> clock for the rtc is actually missing from the clock driver (and the
>>>>> dt binding).
>>>>>
>>>>> Currently the mpfs clock driver uses a reference clock called the
>>>>> "msspll", set in the device tree, as the parent for the cpu/axi/ahb
>>>>> (config) clocks. The frequency of the msspll is determined by the FPGA
>>>>> bitstream & the bootloader configures the clock to match the bitstream.
>>>>> The real reference is provided by a 100 or 125 MHz off chip oscillator.
>>>>>
>>>>> However, the msspll clock is not actually the parent of all clocks on
>>>>> the system - the reference clock for the rtc/mtimer actually has the
>>>>> off chip oscillator as its parent.
>>>>>
>>>>> This series enables reading the rate of the msspll clock, converts
>>>>> the refclock in the device tree to the external reference & adds
>>>>> the missing rtc reference clock.
>>>>>
>>>>> I assume it is okay not to add fixes tags for the rtc dt binding?
>>>>> Since the clock was previously missing, the binding is wrong, but
>>>>> idk if that qualifies as a fix?
>>>>
>>>> Usually ABI breakage, even if accepted, should be be tagged as fix
>>>> because it is clearly then a break of other peoples' trees...
>>>>
>>>
>>> That means either a) do something messy in the clock driver or b) mark
>>> the whole series as fixes (and roll it into [0]).
>>>
>>> The second option seems far more sensible to me, do you agree?
>>
>> Having thought some more about it, patches 2, 3 and the rtc part of 7
>> should be moved into [0] since they're fixing a binding that only
>> arrived in 5.18-rc1.
>> For the rest, make the second part of the reg optional and if it doesnt
>> exist just return prate for the msspll clock?
>
> Ah, so this got into v5.18-rc1?
Yeah, so for context he clock driver & relevant the dt-bindings only
arrived in 5.18-rc1. The device tree itself has been around (I think)
5.12 but it wasn't bootable until now. The rtc stanza is also new.
The clock stanza & "wrong" refclk existed since the device tree was
first added.
> I think I missed that information from
> the patches description and focused on backporting to stables.
Yeah, zero intention of backporting any of this. Pretty pointless since
the board hasn't booted until now anyway.
> Then indeed you could combine all fixes together, mark them with Fixes.
I had split the series (plural) since the clock driver change is fairly
big, adding a new "layer" of clocks & like 200 lines to a 400 line
driver. I will respin then, with the dt binding patches marked as fixes
and combined with the other series.
Do I have to maintain backwards compatibility with the device tree
from before the board actually booted mainline? If not, I'll merge
it all into one series, marked as fixes. Otherwise I'll keep the clock
changes in this series out of the fixes, mark the second reg entry in
the clock binding as optional & handle the old, "naive" dt stanza in the
driver.
Thanks!
Conor.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v1 0/7] Add rtc refclk support for PolarFire SoC
2022-04-09 20:17 ` Conor Dooley
@ 2022-04-10 8:12 ` Krzysztof Kozlowski
0 siblings, 0 replies; 17+ messages in thread
From: Krzysztof Kozlowski @ 2022-04-10 8:12 UTC (permalink / raw)
To: Conor Dooley, Conor Dooley, sboyd, aou, paul.walmsley, palmer,
a.zummo, alexandre.belloni, robh+dt, krzk+dt, mturquette
Cc: daire.mcnamara, linux-rtc, devicetree, linux-clk, linux-riscv
On 09/04/2022 22:17, Conor Dooley wrote:
>
> Do I have to maintain backwards compatibility with the device tree
> from before the board actually booted mainline? If not, I'll merge
> it all into one series, marked as fixes.
Usually not, although it is not actually my call. :)
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2022-04-10 8:12 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-04-08 14:36 [PATCH v1 0/7] Add rtc refclk support for PolarFire SoC Conor Dooley
2022-04-08 14:36 ` [PATCH v1 1/7] dt-bindings: clk: mpfs document msspll dri registers Conor Dooley
2022-04-08 14:54 ` Krzysztof Kozlowski
2022-04-08 14:36 ` [PATCH v1 2/7] dt-bindings: clk: mpfs: add defines for two new clocks Conor Dooley
2022-04-08 14:36 ` [PATCH v1 3/7] dt-bindings: rtc: add refclk to mpfs-rtc Conor Dooley
2022-04-08 14:56 ` Krzysztof Kozlowski
2022-04-08 14:36 ` [PATCH v1 4/7] clk: microchip: mpfs: re-parent the configurable clocks Conor Dooley
2022-04-08 14:36 ` [PATCH v1 5/7] clk: microchip: mpfs: rename sys_base to base Conor Dooley
2022-04-08 14:36 ` [PATCH v1 6/7] clk: microchip: mpfs: add RTCREF clock control Conor Dooley
2022-04-08 14:36 ` [PATCH v1 7/7] riscv: dts: microchip: reparent mpfs clocks Conor Dooley
2022-04-08 14:57 ` [PATCH v1 0/7] Add rtc refclk support for PolarFire SoC Krzysztof Kozlowski
2022-04-08 15:29 ` Conor Dooley
2022-04-09 7:14 ` Conor Dooley
2022-04-09 10:48 ` Krzysztof Kozlowski
2022-04-09 20:17 ` Conor Dooley
2022-04-10 8:12 ` Krzysztof Kozlowski
2022-04-09 10:45 ` Krzysztof Kozlowski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).