2022-07-04 12:27:49

by Conor Dooley

[permalink] [raw]
Subject: [PATCH v2 00/12] PolarFire SoC reset controller & clock cleanups

Hey all,
I know I have not sat on the RFC I sent about the aux. bus parts
for too long, but figured I'd just send the whole thing anyway to all
lists etc.

Kinda two things happening in this series, but I sent it together to
ensure the second part would apply correctly.

The first is the reset controller that I promised after discovering the
issue triggered by CONFIG_PM & the phy not coming up correctly. I have
now removed all the messing with resets from clock enable/disable
functions & now use the aux bus to set up a reset controller driver.
Since I needed something to test it, I hooked up the reset for the
Cadence MACB on PolarFire SoC. This has been split into a second series
for v2:
https://lore.kernel.org/all/[email protected]/

The second part adds rate control for the MSS PLL clock, followed by
some simplifications to the driver & conversions of some custom structs
to the corresponding structs in the framework.

Thanks,
Conor.

Changes since v1:
- split off the net patches
- clk: actually pass the spinlock to the converted dividers & gates
- reset: added a spinlock around RMW access to registers
- reset: switched to BIT(i) macros
- reset: used local copies of some variables as pointed out by Philipp
- reset: dropped the success printout

Conor Dooley (12):
dt-bindings: clk: microchip: mpfs: add reset controller support
clk: microchip: mpfs: add reset controller
reset: add polarfire soc reset support
MAINTAINERS: add polarfire soc reset controller
riscv: dts: microchip: add mpfs specific macb reset support
clk: microchip: mpfs: add module_authors entries
clk: microchip: mpfs: add MSS pll's set & round rate
clk: microchip: mpfs: move id & offset out of clock structs
clk: microchip: mpfs: simplify control reg access
clk: microchip: mpfs: delete 2 line mpfs_clk_register_foo()
clk: microchip: mpfs: convert cfg_clk to clk_divider
clk: microchip: mpfs: convert periph_clk to clk_gate

.../bindings/clock/microchip,mpfs.yaml | 17 +-
MAINTAINERS | 1 +
arch/riscv/boot/dts/microchip/mpfs.dtsi | 7 +-
drivers/clk/microchip/Kconfig | 1 +
drivers/clk/microchip/clk-mpfs.c | 379 +++++++++---------
drivers/reset/Kconfig | 7 +
drivers/reset/Makefile | 2 +-
drivers/reset/reset-mpfs.c | 157 ++++++++
include/soc/microchip/mpfs.h | 8 +
9 files changed, 386 insertions(+), 193 deletions(-)
create mode 100644 drivers/reset/reset-mpfs.c


base-commit: b13baccc3850ca8b8cccbf8ed9912dbaa0fdf7f3
--
2.36.1


2022-07-04 12:28:06

by Conor Dooley

[permalink] [raw]
Subject: [PATCH v2 07/12] clk: microchip: mpfs: add MSS pll's set & round rate

The MSS pll is not a fixed frequency clock, so add set() & round_rate()
support.
Control is limited to a 7 bit output divider as other devices on the
FPGA occupy the other three outputs of the PLL & prevent changing
the multiplier.

Signed-off-by: Conor Dooley <[email protected]>
---
drivers/clk/microchip/clk-mpfs.c | 54 ++++++++++++++++++++++++++++++++
1 file changed, 54 insertions(+)

diff --git a/drivers/clk/microchip/clk-mpfs.c b/drivers/clk/microchip/clk-mpfs.c
index c7cafd61b7f7..a23f63bcd074 100644
--- a/drivers/clk/microchip/clk-mpfs.c
+++ b/drivers/clk/microchip/clk-mpfs.c
@@ -131,8 +131,62 @@ static unsigned long mpfs_clk_msspll_recalc_rate(struct clk_hw *hw, unsigned lon
return prate * mult / (ref_div * MSSPLL_FIXED_DIV * postdiv);
}

+static long mpfs_clk_msspll_round_rate(struct clk_hw *hw, unsigned long rate, 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;
+ u32 mult, ref_div;
+ unsigned long rate_before_ctrl;
+
+ 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);
+
+ rate_before_ctrl = rate * (ref_div * MSSPLL_FIXED_DIV) / mult;
+
+ return divider_round_rate(hw, rate_before_ctrl, prate, NULL, MSSPLL_POSTDIV_WIDTH,
+ msspll_hw->flags);
+}
+
+static int mpfs_clk_msspll_set_rate(struct clk_hw *hw, unsigned long rate, 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;
+ int divider_setting;
+ unsigned long rate_before_ctrl, flags;
+
+ 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);
+
+ rate_before_ctrl = rate * (ref_div * MSSPLL_FIXED_DIV) / mult;
+ divider_setting = divider_get_val(rate_before_ctrl, prate, NULL, MSSPLL_POSTDIV_WIDTH,
+ msspll_hw->flags);
+
+ if (divider_setting < 0)
+ return divider_setting;
+
+ spin_lock_irqsave(&mpfs_clk_lock, flags);
+
+ postdiv = readl_relaxed(postdiv_addr);
+ postdiv &= ~(clk_div_mask(MSSPLL_POSTDIV_WIDTH) << MSSPLL_POSTDIV_SHIFT);
+ writel_relaxed(postdiv, postdiv_addr);
+
+ spin_unlock_irqrestore(&mpfs_clk_lock, flags);
+
+ return 0;
+}
+
static const struct clk_ops mpfs_clk_msspll_ops = {
.recalc_rate = mpfs_clk_msspll_recalc_rate,
+ .round_rate = mpfs_clk_msspll_round_rate,
+ .set_rate = mpfs_clk_msspll_set_rate,
};

#define CLK_PLL(_id, _name, _parent, _shift, _width, _flags, _offset) { \
--
2.36.1

2022-07-04 12:28:07

by Conor Dooley

[permalink] [raw]
Subject: [PATCH v2 02/12] clk: microchip: mpfs: add reset controller

Add a reset controller to PolarFire SoC's clock driver. This reset
controller is registered as an aux device and read/write functions
exported to the drivers namespace so that the reset controller can
access the peripheral device reset register.

Signed-off-by: Conor Dooley <[email protected]>
---
drivers/clk/microchip/Kconfig | 1 +
drivers/clk/microchip/clk-mpfs.c | 116 ++++++++++++++++++++++++++++---
include/soc/microchip/mpfs.h | 8 +++
3 files changed, 114 insertions(+), 11 deletions(-)

diff --git a/drivers/clk/microchip/Kconfig b/drivers/clk/microchip/Kconfig
index a5a99873c4f5..b46e864b3bd8 100644
--- a/drivers/clk/microchip/Kconfig
+++ b/drivers/clk/microchip/Kconfig
@@ -6,5 +6,6 @@ config COMMON_CLK_PIC32
config MCHP_CLK_MPFS
bool "Clk driver for PolarFire SoC"
depends on (RISCV && SOC_MICROCHIP_POLARFIRE) || COMPILE_TEST
+ select AUXILIARY_BUS
help
Supports Clock Configuration for PolarFire SoC
diff --git a/drivers/clk/microchip/clk-mpfs.c b/drivers/clk/microchip/clk-mpfs.c
index 070c3b896559..a93f78619dc3 100644
--- a/drivers/clk/microchip/clk-mpfs.c
+++ b/drivers/clk/microchip/clk-mpfs.c
@@ -3,12 +3,14 @@
* Daire McNamara,<[email protected]>
* Copyright (C) 2020 Microchip Technology Inc. All rights reserved.
*/
+#include <linux/auxiliary_bus.h>
#include <linux/clk-provider.h>
#include <linux/io.h>
#include <linux/module.h>
#include <linux/platform_device.h>
#include <linux/slab.h>
#include <dt-bindings/clock/microchip,mpfs-clock.h>
+#include <soc/microchip/mpfs.h>

/* address offset of control registers */
#define REG_MSSPLL_REF_CR 0x08u
@@ -28,6 +30,7 @@
#define MSSPLL_FIXED_DIV 4u

struct mpfs_clock_data {
+ struct device *dev;
void __iomem *base;
void __iomem *msspll_base;
struct clk_hw_onecell_data hw_data;
@@ -302,10 +305,6 @@ static int mpfs_periph_clk_enable(struct clk_hw *hw)

spin_lock_irqsave(&mpfs_clk_lock, flags);

- reg = readl_relaxed(base_addr + REG_SUBBLK_RESET_CR);
- val = reg & ~(1u << periph->shift);
- writel_relaxed(val, base_addr + REG_SUBBLK_RESET_CR);
-
reg = readl_relaxed(base_addr + REG_SUBBLK_CLOCK_CR);
val = reg | (1u << periph->shift);
writel_relaxed(val, base_addr + REG_SUBBLK_CLOCK_CR);
@@ -339,12 +338,9 @@ static int mpfs_periph_clk_is_enabled(struct clk_hw *hw)
void __iomem *base_addr = periph_hw->sys_base;
u32 reg;

- reg = readl_relaxed(base_addr + REG_SUBBLK_RESET_CR);
- if ((reg & (1u << periph->shift)) == 0u) {
- reg = readl_relaxed(base_addr + REG_SUBBLK_CLOCK_CR);
- if (reg & (1u << periph->shift))
- return 1;
- }
+ reg = readl_relaxed(base_addr + REG_SUBBLK_CLOCK_CR);
+ if (reg & (1u << periph->shift))
+ return 1;

return 0;
}
@@ -438,6 +434,98 @@ static int mpfs_clk_register_periphs(struct device *dev, struct mpfs_periph_hw_c
return 0;
}

+/*
+ * Peripheral clock resets
+ */
+
+#if IS_ENABLED(CONFIG_RESET_CONTROLLER)
+
+u32 mpfs_reset_read(struct device *dev)
+{
+ struct mpfs_clock_data *clock_data = dev_get_drvdata(dev->parent);
+
+ return readl_relaxed(clock_data->base + REG_SUBBLK_RESET_CR);
+}
+EXPORT_SYMBOL_NS_GPL(mpfs_reset_read, MCHP_CLK_MPFS);
+
+void mpfs_reset_write(struct device *dev, u32 val)
+{
+ struct mpfs_clock_data *clock_data = dev_get_drvdata(dev->parent);
+
+ writel_relaxed(val, clock_data->base + REG_SUBBLK_RESET_CR);
+}
+EXPORT_SYMBOL_NS_GPL(mpfs_reset_write, MCHP_CLK_MPFS);
+
+static void mpfs_reset_unregister_adev(void *_adev)
+{
+ struct auxiliary_device *adev = _adev;
+
+ auxiliary_device_delete(adev);
+}
+
+static void mpfs_reset_adev_release(struct device *dev)
+{
+ struct auxiliary_device *adev = to_auxiliary_dev(dev);
+
+ auxiliary_device_uninit(adev);
+
+ kfree(adev);
+}
+
+static struct auxiliary_device *mpfs_reset_adev_alloc(struct mpfs_clock_data *clk_data)
+{
+ struct auxiliary_device *adev;
+ int ret;
+
+ adev = kzalloc(sizeof(*adev), GFP_KERNEL);
+ if (!adev)
+ return ERR_PTR(-ENOMEM);
+
+ adev->name = "reset-mpfs";
+ adev->dev.parent = clk_data->dev;
+ adev->dev.release = mpfs_reset_adev_release;
+ adev->id = 666u;
+
+ ret = auxiliary_device_init(adev);
+ if (ret) {
+ kfree(adev);
+ return ERR_PTR(ret);
+ }
+
+ return adev;
+}
+
+static int mpfs_reset_controller_register(struct mpfs_clock_data *clk_data)
+{
+ struct auxiliary_device *adev;
+ int ret;
+
+ adev = mpfs_reset_adev_alloc(clk_data);
+ if (IS_ERR(adev))
+ return PTR_ERR(adev);
+
+ ret = auxiliary_device_add(adev);
+ if (ret) {
+ auxiliary_device_uninit(adev);
+ return ret;
+ }
+
+ ret = devm_add_action_or_reset(clk_data->dev, mpfs_reset_unregister_adev, adev);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+#else /* !CONFIG_RESET_CONTROLLER */
+
+static int mpfs_reset_controller_register(struct mpfs_clock_data *clk_data)
+{
+ return 0;
+}
+
+#endif /* !CONFIG_RESET_CONTROLLER */
+
static int mpfs_clk_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -462,6 +550,8 @@ static int mpfs_clk_probe(struct platform_device *pdev)
return PTR_ERR(clk_data->msspll_base);

clk_data->hw_data.num = num_clks;
+ clk_data->dev = dev;
+ dev_set_drvdata(dev, clk_data);

ret = mpfs_clk_register_mssplls(dev, mpfs_msspll_clks, ARRAY_SIZE(mpfs_msspll_clks),
clk_data);
@@ -481,6 +571,10 @@ static int mpfs_clk_probe(struct platform_device *pdev)
if (ret)
return ret;

+ ret = mpfs_reset_controller_register(clk_data);
+ if (ret)
+ return ret;
+
return ret;
}

@@ -488,7 +582,7 @@ static const struct of_device_id mpfs_clk_of_match_table[] = {
{ .compatible = "microchip,mpfs-clkcfg", },
{}
};
-MODULE_DEVICE_TABLE(of, mpfs_clk_match_table);
+MODULE_DEVICE_TABLE(of, mpfs_clk_of_match_table);

static struct platform_driver mpfs_clk_driver = {
.probe = mpfs_clk_probe,
diff --git a/include/soc/microchip/mpfs.h b/include/soc/microchip/mpfs.h
index 6466515262bd..f916dcde457f 100644
--- a/include/soc/microchip/mpfs.h
+++ b/include/soc/microchip/mpfs.h
@@ -40,4 +40,12 @@ struct mpfs_sys_controller *mpfs_sys_controller_get(struct device *dev);

#endif /* if IS_ENABLED(CONFIG_POLARFIRE_SOC_SYS_CTRL) */

+#if IS_ENABLED(CONFIG_MCHP_CLK_MPFS)
+
+u32 mpfs_reset_read(struct device *dev);
+
+void mpfs_reset_write(struct device *dev, u32 val);
+
+#endif /* if IS_ENABLED(CONFIG_MCHP_CLK_MPFS) */
+
#endif /* __SOC_MPFS_H__ */
--
2.36.1

2022-07-04 12:28:11

by Conor Dooley

[permalink] [raw]
Subject: [PATCH v2 11/12] clk: microchip: mpfs: convert cfg_clk to clk_divider

The cfg_clk struct is now just a redefinition of the clk_divider struct
with custom implentations of the ops, that implement an extra level of
redirection. Remove the custom struct and replace it with clk_divider.

Signed-off-by: Conor Dooley <[email protected]>
---
drivers/clk/microchip/clk-mpfs.c | 76 ++++----------------------------
1 file changed, 8 insertions(+), 68 deletions(-)

diff --git a/drivers/clk/microchip/clk-mpfs.c b/drivers/clk/microchip/clk-mpfs.c
index e58d0bc4669a..1d9e8c1e56b6 100644
--- a/drivers/clk/microchip/clk-mpfs.c
+++ b/drivers/clk/microchip/clk-mpfs.c
@@ -51,24 +51,13 @@ struct mpfs_msspll_hw_clock {

#define to_mpfs_msspll_clk(_hw) container_of(_hw, struct mpfs_msspll_hw_clock, hw)

-struct mpfs_cfg_clock {
- void __iomem *reg;
- const struct clk_div_table *table;
- u8 shift;
- u8 width;
- u8 flags;
-};
-
struct mpfs_cfg_hw_clock {
- struct mpfs_cfg_clock cfg;
- struct clk_hw hw;
+ struct clk_divider cfg;
struct clk_init_data init;
unsigned int id;
u32 reg_offset;
};

-#define to_mpfs_cfg_clk(_hw) container_of(_hw, struct mpfs_cfg_hw_clock, hw)
-
struct mpfs_periph_clock {
void __iomem *reg;
u8 shift;
@@ -228,56 +217,6 @@ static int mpfs_clk_register_mssplls(struct device *dev, struct mpfs_msspll_hw_c
* "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);
- struct mpfs_cfg_clock *cfg = &cfg_hw->cfg;
- u32 val;
-
- val = readl_relaxed(cfg->reg) >> cfg->shift;
- val &= clk_div_mask(cfg->width);
-
- 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)
-{
- struct mpfs_cfg_hw_clock *cfg_hw = to_mpfs_cfg_clk(hw);
- struct mpfs_cfg_clock *cfg = &cfg_hw->cfg;
-
- return divider_round_rate(hw, rate, prate, cfg->table, cfg->width, 0);
-}
-
-static int mpfs_cfg_clk_set_rate(struct clk_hw *hw, unsigned long rate, unsigned long prate)
-{
- struct mpfs_cfg_hw_clock *cfg_hw = to_mpfs_cfg_clk(hw);
- struct mpfs_cfg_clock *cfg = &cfg_hw->cfg;
- unsigned long flags;
- u32 val;
- int divider_setting;
-
- divider_setting = divider_get_val(rate, prate, cfg->table, cfg->width, 0);
-
- if (divider_setting < 0)
- return divider_setting;
-
- spin_lock_irqsave(&mpfs_clk_lock, flags);
- val = readl_relaxed(cfg->reg);
- val &= ~(clk_div_mask(cfg->width) << cfg_hw->cfg.shift);
- val |= divider_setting << cfg->shift;
- writel_relaxed(val, cfg->reg);
-
- spin_unlock_irqrestore(&mpfs_clk_lock, flags);
-
- return 0;
-}
-
-static const struct clk_ops mpfs_clk_cfg_ops = {
- .recalc_rate = mpfs_cfg_clk_recalc_rate,
- .round_rate = mpfs_cfg_clk_round_rate,
- .set_rate = mpfs_cfg_clk_set_rate,
-};
-
#define CLK_CFG(_id, _name, _parent, _shift, _width, _table, _flags, _offset) { \
.id = _id, \
.cfg.shift = _shift, \
@@ -285,7 +224,8 @@ static const struct clk_ops mpfs_clk_cfg_ops = {
.cfg.table = _table, \
.reg_offset = _offset, \
.cfg.flags = _flags, \
- .hw.init = CLK_HW_INIT(_name, _parent, &mpfs_clk_cfg_ops, 0), \
+ .cfg.hw.init = CLK_HW_INIT(_name, _parent, &clk_divider_ops, 0), \
+ .cfg.lock = &mpfs_clk_lock, \
}

static struct mpfs_cfg_hw_clock mpfs_cfg_clks[] = {
@@ -302,8 +242,8 @@ static struct mpfs_cfg_hw_clock mpfs_cfg_clks[] = {
.cfg.table = mpfs_div_rtcref_table,
.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),
+ .cfg.hw.init =
+ CLK_HW_INIT_PARENTS_DATA("clk_rtcref", mpfs_ext_ref, &clk_divider_ops, 0),
}
};

@@ -317,13 +257,13 @@ static int mpfs_clk_register_cfgs(struct device *dev, struct mpfs_cfg_hw_clock *
struct mpfs_cfg_hw_clock *cfg_hw = &cfg_hws[i];

cfg_hw->cfg.reg = data->base + cfg_hw->reg_offset;
- ret = devm_clk_hw_register(dev, &cfg_hw->hw);
+ ret = devm_clk_hw_register(dev, &cfg_hw->cfg.hw);
if (ret)
return dev_err_probe(dev, ret, "failed to register clock id: %d\n",
cfg_hw->id);

id = cfg_hw->id;
- data->hw_data.hws[id] = &cfg_hw->hw;
+ data->hw_data.hws[id] = &cfg_hw->cfg.hw;
}

return 0;
@@ -393,7 +333,7 @@ static const struct clk_ops mpfs_periph_clk_ops = {
_flags), \
}

-#define PARENT_CLK(PARENT) (&mpfs_cfg_clks[CLK_##PARENT].hw)
+#define PARENT_CLK(PARENT) (&mpfs_cfg_clks[CLK_##PARENT].cfg.hw)

/*
* Critical clocks:
--
2.36.1

2022-07-04 12:28:12

by Conor Dooley

[permalink] [raw]
Subject: [PATCH v2 09/12] clk: microchip: mpfs: simplify control reg access

The control reg addresses are known when the clocks are registered, so
we can, instead of assigning a base pointer to the structs, assign the
control reg addresses directly. Accordingly, remove the interim
variables used during reads/writes to those registers.

Signed-off-by: Conor Dooley <[email protected]>
---
drivers/clk/microchip/clk-mpfs.c | 42 +++++++++++++-------------------
1 file changed, 17 insertions(+), 25 deletions(-)

diff --git a/drivers/clk/microchip/clk-mpfs.c b/drivers/clk/microchip/clk-mpfs.c
index 750f28481498..0330c2839a24 100644
--- a/drivers/clk/microchip/clk-mpfs.c
+++ b/drivers/clk/microchip/clk-mpfs.c
@@ -52,6 +52,7 @@ struct mpfs_msspll_hw_clock {
#define to_mpfs_msspll_clk(_hw) container_of(_hw, struct mpfs_msspll_hw_clock, hw)

struct mpfs_cfg_clock {
+ void __iomem *reg;
const struct clk_div_table *table;
u8 shift;
u8 width;
@@ -60,7 +61,6 @@ struct mpfs_cfg_clock {

struct mpfs_cfg_hw_clock {
struct mpfs_cfg_clock cfg;
- void __iomem *sys_base;
struct clk_hw hw;
struct clk_init_data init;
unsigned int id;
@@ -70,12 +70,12 @@ struct mpfs_cfg_hw_clock {
#define to_mpfs_cfg_clk(_hw) container_of(_hw, struct mpfs_cfg_hw_clock, hw)

struct mpfs_periph_clock {
+ void __iomem *reg;
u8 shift;
};

struct mpfs_periph_hw_clock {
struct mpfs_periph_clock periph;
- void __iomem *sys_base;
struct clk_hw hw;
unsigned int id;
};
@@ -214,14 +214,13 @@ static int mpfs_clk_register_msspll(struct device *dev, struct mpfs_msspll_hw_cl
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);
+ ret = mpfs_clk_register_msspll(dev, msspll_hw, data->msspll_base);
if (ret)
return dev_err_probe(dev, ret, "failed to register msspll id: %d\n",
CLK_MSSPLL);
@@ -240,10 +239,9 @@ 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;
u32 val;

- val = readl_relaxed(base_addr + cfg_hw->reg_offset) >> cfg->shift;
+ val = readl_relaxed(cfg->reg) >> cfg->shift;
val &= clk_div_mask(cfg->width);

return divider_recalc_rate(hw, prate, val, cfg->table, cfg->flags, cfg->width);
@@ -261,7 +259,6 @@ 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;
unsigned long flags;
u32 val;
int divider_setting;
@@ -272,10 +269,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 + cfg_hw->reg_offset);
+ val = readl_relaxed(cfg->reg);
val &= ~(clk_div_mask(cfg->width) << cfg_hw->cfg.shift);
val |= divider_setting << cfg->shift;
- writel_relaxed(val, base_addr + cfg_hw->reg_offset);
+ writel_relaxed(val, cfg->reg);

spin_unlock_irqrestore(&mpfs_clk_lock, flags);

@@ -318,9 +315,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->cfg.reg = base + cfg_hw->reg_offset;

return devm_clk_hw_register(dev, &cfg_hw->hw);
}
@@ -328,14 +325,13 @@ 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;
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, data->base);
if (ret)
return dev_err_probe(dev, ret, "failed to register clock id: %d\n",
cfg_hw->id);
@@ -355,15 +351,14 @@ 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;
u32 reg, val;
unsigned long flags;

spin_lock_irqsave(&mpfs_clk_lock, flags);

- reg = readl_relaxed(base_addr + REG_SUBBLK_CLOCK_CR);
+ reg = readl_relaxed(periph->reg);
val = reg | (1u << periph->shift);
- writel_relaxed(val, base_addr + REG_SUBBLK_CLOCK_CR);
+ writel_relaxed(val, periph->reg);

spin_unlock_irqrestore(&mpfs_clk_lock, flags);

@@ -374,15 +369,14 @@ 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;
u32 reg, val;
unsigned long flags;

spin_lock_irqsave(&mpfs_clk_lock, flags);

- reg = readl_relaxed(base_addr + REG_SUBBLK_CLOCK_CR);
+ reg = readl_relaxed(periph->reg);
val = reg & ~(1u << periph->shift);
- writel_relaxed(val, base_addr + REG_SUBBLK_CLOCK_CR);
+ writel_relaxed(val, periph->reg);

spin_unlock_irqrestore(&mpfs_clk_lock, flags);
}
@@ -391,10 +385,9 @@ 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;
u32 reg;

- reg = readl_relaxed(base_addr + REG_SUBBLK_CLOCK_CR);
+ reg = readl_relaxed(periph->reg);
if (reg & (1u << periph->shift))
return 1;

@@ -461,9 +454,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->periph.reg = base + REG_SUBBLK_CLOCK_CR;

return devm_clk_hw_register(dev, &periph_hw->hw);
}
@@ -471,14 +464,13 @@ 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;
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, data->base);
if (ret)
return dev_err_probe(dev, ret, "failed to register clock id: %d\n",
periph_hw->id);
--
2.36.1

2022-07-04 12:28:40

by Conor Dooley

[permalink] [raw]
Subject: [PATCH v2 05/12] riscv: dts: microchip: add mpfs specific macb reset support

The macb on PolarFire SoC has reset support which the generic compatible
does not use. Add the newly introduced MPFS specific compatible as the
primary compatible to avail of this support & wire up the reset to the
clock controllers devicetree entry.

Signed-off-by: Conor Dooley <[email protected]>
---
arch/riscv/boot/dts/microchip/mpfs.dtsi | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/boot/dts/microchip/mpfs.dtsi b/arch/riscv/boot/dts/microchip/mpfs.dtsi
index 8c3259134194..5a33cbf9467a 100644
--- a/arch/riscv/boot/dts/microchip/mpfs.dtsi
+++ b/arch/riscv/boot/dts/microchip/mpfs.dtsi
@@ -197,6 +197,7 @@ clkcfg: clkcfg@20002000 {
reg = <0x0 0x20002000 0x0 0x1000>, <0x0 0x3E001000 0x0 0x1000>;
clocks = <&refclk>;
#clock-cells = <1>;
+ #reset-cells = <1>;
};

mmuart0: serial@20000000 {
@@ -331,7 +332,7 @@ i2c1: i2c@2010b000 {
};

mac0: ethernet@20110000 {
- compatible = "cdns,macb";
+ compatible = "microchip,mpfs-macb", "cdns,macb";
reg = <0x0 0x20110000 0x0 0x2000>;
#address-cells = <1>;
#size-cells = <0>;
@@ -340,11 +341,12 @@ mac0: ethernet@20110000 {
local-mac-address = [00 00 00 00 00 00];
clocks = <&clkcfg CLK_MAC0>, <&clkcfg CLK_AHB>;
clock-names = "pclk", "hclk";
+ resets = <&clkcfg CLK_MAC0>;
status = "disabled";
};

mac1: ethernet@20112000 {
- compatible = "cdns,macb";
+ compatible = "microchip,mpfs-macb", "cdns,macb";
reg = <0x0 0x20112000 0x0 0x2000>;
#address-cells = <1>;
#size-cells = <0>;
@@ -353,6 +355,7 @@ mac1: ethernet@20112000 {
local-mac-address = [00 00 00 00 00 00];
clocks = <&clkcfg CLK_MAC1>, <&clkcfg CLK_AHB>;
clock-names = "pclk", "hclk";
+ resets = <&clkcfg CLK_MAC1>;
status = "disabled";
};

--
2.36.1

2022-07-04 12:46:33

by Conor Dooley

[permalink] [raw]
Subject: [PATCH v2 01/12] dt-bindings: clk: microchip: mpfs: add reset controller support

The "peripheral" devices on PolarFire SoC can be put into reset, so
update the device tree binding to reflect the presence of a reset
controller.

Reviewed-by: Rob Herring <[email protected]>
Signed-off-by: Conor Dooley <[email protected]>
---
.../bindings/clock/microchip,mpfs.yaml | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/microchip,mpfs.yaml b/Documentation/devicetree/bindings/clock/microchip,mpfs.yaml
index 016a4f378b9b..1d0b6a4fda42 100644
--- a/Documentation/devicetree/bindings/clock/microchip,mpfs.yaml
+++ b/Documentation/devicetree/bindings/clock/microchip,mpfs.yaml
@@ -40,8 +40,21 @@ properties:
const: 1
description: |
The clock consumer should specify the desired clock by having the clock
- ID in its "clocks" phandle cell. See include/dt-bindings/clock/microchip,mpfs-clock.h
- for the full list of PolarFire clock IDs.
+ ID in its "clocks" phandle cell.
+ See include/dt-bindings/clock/microchip,mpfs-clock.h for the full list of
+ PolarFire clock IDs.
+
+ resets:
+ maxItems: 1
+
+ '#reset-cells':
+ description:
+ The AHB/AXI peripherals on the PolarFire SoC have reset support, so from
+ CLK_ENVM to CLK_CFM. The reset consumer should specify the desired
+ peripheral via the clock ID in its "resets" phandle cell.
+ See include/dt-bindings/clock/microchip,mpfs-clock.h for the full list of
+ PolarFire clock IDs.
+ const: 1

required:
- compatible
--
2.36.1

2022-07-04 12:48:27

by Conor Dooley

[permalink] [raw]
Subject: [PATCH v2 12/12] clk: microchip: mpfs: convert periph_clk to clk_gate

With the reset code moved to the recently added reset controller, there
is no need for custom ops any longer. Remove the custom ops and the
custom struct by converting to a clk_gate.

Signed-off-by: Conor Dooley <[email protected]>
---
drivers/clk/microchip/clk-mpfs.c | 74 +++-----------------------------
1 file changed, 7 insertions(+), 67 deletions(-)

diff --git a/drivers/clk/microchip/clk-mpfs.c b/drivers/clk/microchip/clk-mpfs.c
index 1d9e8c1e56b6..2d5585c6ef21 100644
--- a/drivers/clk/microchip/clk-mpfs.c
+++ b/drivers/clk/microchip/clk-mpfs.c
@@ -58,19 +58,11 @@ struct mpfs_cfg_hw_clock {
u32 reg_offset;
};

-struct mpfs_periph_clock {
- void __iomem *reg;
- u8 shift;
-};
-
struct mpfs_periph_hw_clock {
- struct mpfs_periph_clock periph;
- struct clk_hw hw;
+ struct clk_gate periph;
unsigned int id;
};

-#define to_mpfs_periph_clk(_hw) container_of(_hw, struct mpfs_periph_hw_clock, hw)
-
/*
* mpfs_clk_lock prevents anything else from writing to the
* mpfs clk block while a software locked register is being written.
@@ -273,64 +265,12 @@ static int mpfs_clk_register_cfgs(struct device *dev, struct mpfs_cfg_hw_clock *
* 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);
- struct mpfs_periph_clock *periph = &periph_hw->periph;
- u32 reg, val;
- unsigned long flags;
-
- spin_lock_irqsave(&mpfs_clk_lock, flags);
-
- reg = readl_relaxed(periph->reg);
- val = reg | (1u << periph->shift);
- writel_relaxed(val, periph->reg);
-
- spin_unlock_irqrestore(&mpfs_clk_lock, flags);
-
- return 0;
-}
-
-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;
- u32 reg, val;
- unsigned long flags;
-
- spin_lock_irqsave(&mpfs_clk_lock, flags);
-
- reg = readl_relaxed(periph->reg);
- val = reg & ~(1u << periph->shift);
- writel_relaxed(val, periph->reg);
-
- spin_unlock_irqrestore(&mpfs_clk_lock, flags);
-}
-
-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;
- u32 reg;
-
- reg = readl_relaxed(periph->reg);
- if (reg & (1u << periph->shift))
- return 1;
-
- return 0;
-}
-
-static const struct clk_ops mpfs_periph_clk_ops = {
- .enable = mpfs_periph_clk_enable,
- .disable = mpfs_periph_clk_disable,
- .is_enabled = mpfs_periph_clk_is_enabled,
-};
-
#define CLK_PERIPH(_id, _name, _parent, _shift, _flags) { \
- .id = _id, \
- .periph.shift = _shift, \
- .hw.init = CLK_HW_INIT_HW(_name, _parent, &mpfs_periph_clk_ops, \
+ .id = _id, \
+ .periph.bit_idx = _shift, \
+ .periph.hw.init = CLK_HW_INIT_HW(_name, _parent, &clk_gate_ops, \
_flags), \
+ .periph.lock = &mpfs_clk_lock, \
}

#define PARENT_CLK(PARENT) (&mpfs_cfg_clks[CLK_##PARENT].cfg.hw)
@@ -389,13 +329,13 @@ static int mpfs_clk_register_periphs(struct device *dev, struct mpfs_periph_hw_c
struct mpfs_periph_hw_clock *periph_hw = &periph_hws[i];

periph_hw->periph.reg = data->base + REG_SUBBLK_CLOCK_CR;
- ret = devm_clk_hw_register(dev, &periph_hw->hw);
+ ret = devm_clk_hw_register(dev, &periph_hw->periph.hw);
if (ret)
return dev_err_probe(dev, ret, "failed to register clock id: %d\n",
periph_hw->id);

id = periph_hws[i].id;
- data->hw_data.hws[id] = &periph_hw->hw;
+ data->hw_data.hws[id] = &periph_hw->periph.hw;
}

return 0;
--
2.36.1

2022-07-20 13:50:11

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2 00/12] PolarFire SoC reset controller & clock cleanups

On 04/07/2022 13:15, Conor Dooley wrote:
> Hey all,
> I know I have not sat on the RFC I sent about the aux. bus parts
> for too long, but figured I'd just send the whole thing anyway to all
> lists etc.

Hey Stephen,

Any comments on the clk/auxdev side of this series?

Thanks,
Conor.

>
> Kinda two things happening in this series, but I sent it together to
> ensure the second part would apply correctly.
>
> The first is the reset controller that I promised after discovering the
> issue triggered by CONFIG_PM & the phy not coming up correctly. I have
> now removed all the messing with resets from clock enable/disable
> functions & now use the aux bus to set up a reset controller driver.
> Since I needed something to test it, I hooked up the reset for the
> Cadence MACB on PolarFire SoC. This has been split into a second series
> for v2:
> https://lore.kernel.org/all/[email protected]/
>
> The second part adds rate control for the MSS PLL clock, followed by
> some simplifications to the driver & conversions of some custom structs
> to the corresponding structs in the framework.
>
> Thanks,
> Conor.
>
> Changes since v1:
> - split off the net patches
> - clk: actually pass the spinlock to the converted dividers & gates
> - reset: added a spinlock around RMW access to registers
> - reset: switched to BIT(i) macros
> - reset: used local copies of some variables as pointed out by Philipp
> - reset: dropped the success printout
>
> Conor Dooley (12):
> dt-bindings: clk: microchip: mpfs: add reset controller support
> clk: microchip: mpfs: add reset controller
> reset: add polarfire soc reset support
> MAINTAINERS: add polarfire soc reset controller
> riscv: dts: microchip: add mpfs specific macb reset support
> clk: microchip: mpfs: add module_authors entries
> clk: microchip: mpfs: add MSS pll's set & round rate
> clk: microchip: mpfs: move id & offset out of clock structs
> clk: microchip: mpfs: simplify control reg access
> clk: microchip: mpfs: delete 2 line mpfs_clk_register_foo()
> clk: microchip: mpfs: convert cfg_clk to clk_divider
> clk: microchip: mpfs: convert periph_clk to clk_gate
>
> .../bindings/clock/microchip,mpfs.yaml | 17 +-
> MAINTAINERS | 1 +
> arch/riscv/boot/dts/microchip/mpfs.dtsi | 7 +-
> drivers/clk/microchip/Kconfig | 1 +
> drivers/clk/microchip/clk-mpfs.c | 379 +++++++++---------
> drivers/reset/Kconfig | 7 +
> drivers/reset/Makefile | 2 +-
> drivers/reset/reset-mpfs.c | 157 ++++++++
> include/soc/microchip/mpfs.h | 8 +
> 9 files changed, 386 insertions(+), 193 deletions(-)
> create mode 100644 drivers/reset/reset-mpfs.c
>
>
> base-commit: b13baccc3850ca8b8cccbf8ed9912dbaa0fdf7f3

2022-08-09 23:10:48

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2 00/12] PolarFire SoC reset controller & clock cleanups

+CC clang people :)

Got an odd one here and would appreciate some pointers for where to
look. This code when built with gcc boots fine, for example with:
riscv64-unknown-linux-gnu-gcc (g5964b5cd727) 11.1.0
The same code but build with clang build it fails to boot but prior to
that applying this patchset it boots fine. Specifically it is the patch
"clk: microchip: mpfs: move id & offset out of clock structs"

I applied this patchset on top of tonight's master (15205c2829ca) but
I've been seeing the same problem for a few weeks on -next too. I tried
the following 2 versions of clang/llvm:
ClangBuiltLinux clang version 15.0.0 (5b0788fef86ed7008a11f6ee19b9d86d42b6fcfa), LLD 15.0.0
ClangBuiltLinux clang version 15.0.0 (bab8af8ea062f6332b5c5d13ae688bb8900f244a), LLD 15.0.0

It's probably something silly that I've overlooked but I am not au
fait with these sort of things unfortunately, but hey - at least I'll
learn something then.

Thanks,
Conor.

The boot log is fairly short so here ya go:

[ 0.000000] Linux version 5.19.0-13253-g374b508ee318 (conor@spud) (ClangBuiltLinux clang version 15.0.0 (git@g
ithub.com:llvm/llvm-project.git 5b0788fef86ed7008a11f6ee19b9d86d42b6fcfa), LLD 15.0.0) #1 SMP Tue Aug 9 22:42:10 IST
2022
[ 0.000000] OF: fdt: Ignoring memory range 0x80000000 - 0x80200000
[ 0.000000] Machine model: Microchip PolarFire-SoC Icicle Kit
[ 0.000000] earlycon: ns16550a0 at MMIO32 0x0000000020100000 (options '115200n8')
[ 0.000000] printk: bootconsole [ns16550a0] enabled
[ 0.000000] efi: UEFI not found.
[ 0.000000] Zone ranges:
[ 0.000000] DMA32 [mem 0x0000000080200000-0x00000000ffffffff]
[ 0.000000] Normal [mem 0x0000000100000000-0x000000103fffffff]
[ 0.000000] Movable zone start for each node
[ 0.000000] Early memory node ranges
[ 0.000000] node 0: [mem 0x0000000080200000-0x00000000adffffff]
[ 0.000000] node 0: [mem 0x0000001000000000-0x000000103fffffff]
[ 0.000000] Initmem setup node 0 [mem 0x0000000080200000-0x000000103fffffff]
[ 0.000000] On node 0, zone Normal: 16064512 pages in unavailable ranges
[ 0.000000] SBI specification v0.3 detected
[ 0.000000] SBI implementation ID=0x1 Version=0x9
[ 0.000000] SBI TIME extension detected
[ 0.000000] SBI IPI extension detected
[ 0.000000] SBI RFENCE extension detected
[ 0.000000] SBI HSM extension detected
[ 0.000000] CPU with hartid=0 is not available
[ 0.000000] CPU with hartid=0 is not available
[ 0.000000] riscv: base ISA extensions acdfim
[ 0.000000] riscv: ELF capabilities acdfim
[ 0.000000] percpu: Embedded 18 pages/cpu s34168 r8192 d31368 u73728
[ 0.000000] pcpu-alloc: s34168 r8192 d31368 u73728 alloc=18*4096
[ 0.000000] pcpu-alloc: [0] 0 [0] 1 [0] 2 [0] 3
[ 0.000000] CPU node for /cpus/cpu@0 exist but the possible cpu range is :0-3
[ 0.000000] Built 1 zonelists, mobility grouping on. Total pages: 224263
[ 0.000000] Kernel command line: root=/dev/nfs ip=dhcp debug nfsroot=192.168.2.5:/stuff/nfs_share earlycon
[ 0.000000] Dentry cache hash table entries: 131072 (order: 8, 1048576 bytes, linear)
[ 0.000000] Inode-cache hash table entries: 65536 (order: 7, 524288 bytes, linear)
[ 0.000000] mem auto-init: stack:all(zero), heap alloc:off, heap free:off
[ 0.000000] software IO TLB: area num 4.
[ 0.000000] software IO TLB: mapped [mem 0x00000000aa000000-0x00000000ae000000] (64MB)
[ 0.000000] Virtual kernel memory layout:
[ 0.000000] fixmap : 0xffffffc6fee00000 - 0xffffffc6ff000000 (2048 kB)
[ 0.000000] pci io : 0xffffffc6ff000000 - 0xffffffc700000000 ( 16 MB)
[ 0.000000] vmemmap : 0xffffffc700000000 - 0xffffffc800000000 (4096 MB)
[ 0.000000] vmalloc : 0xffffffc800000000 - 0xffffffd800000000 ( 64 GB)
[ 0.000000] lowmem : 0xffffffd800000000 - 0xffffffe7bfe00000 ( 62 GB)
[ 0.000000] kernel : 0xffffffff80000000 - 0xffffffffffffffff (2047 MB)
[ 0.000000] Memory: 803032K/1800192K available (7485K kernel code, 2858K rwdata, 4096K rodata, 2181K init, 394K bss, 997160K reserved, 0K cma-reserved)
[ 0.000000] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=4, Nodes=1
[ 0.000000] rcu: Hierarchical RCU implementation.
[ 0.000000] rcu: RCU restricting CPUs from NR_CPUS=8 to nr_cpu_ids=4.
[ 0.000000] rcu: RCU debug extended QS entry/exit.
[ 0.000000] Tracing variant of Tasks RCU enabled.
[ 0.000000] rcu: RCU calculated value of scheduler-enlistment delay is 25 jiffies.
[ 0.000000] rcu: Adjusting geometry for rcu_fanout_leaf=16, nr_cpu_ids=4
[ 0.000000] NR_IRQS: 64, nr_irqs: 64, preallocated irqs: 0
[ 0.000000] CPU with hartid=0 is not available
[ 0.000000] riscv-intc: unable to find hart id for /cpus/cpu@0/interrupt-controller
[ 0.000000] riscv-intc: 64 local interrupts mapped
[ 0.000000] plic: interrupt-controller@c000000: mapped 186 interrupts with 4 handlers for 9 contexts.
[ 0.000000] rcu: srcu_init: Setting srcu_struct sizes based on contention.
[ 0.000000] riscv_timer_init_dt: Registering clocksource cpuid [0] hartid [4]
[ 0.000000] clocksource: riscv_clocksource: mask: 0xffffffffffffffff max_cycles: 0x1d854df40, max_idle_ns: 3526361616960 ns
[ 0.000003] sched_clock: 64 bits at 1000kHz, resolution 1000ns, wraps every 2199023255500ns
[ 0.009713] Console: colour dummy device 80x25
[ 0.014676] printk: console [tty0] enabled
[ 0.019217] printk: bootconsole [ns16550a0] disabled

FWIW this is right about when the clock driver gets loaded
as you might imagine.

My config is here, but it's been reproduced on a few different
defconfigs:
https://raw.githubusercontent.com/ConchuOD/polarfire-soc-buildroot-sdk/dev/conf/lowmem/defconfig

On 04/07/2022 13:15, Conor Dooley wrote:
> Hey all,
> I know I have not sat on the RFC I sent about the aux. bus parts
> for too long, but figured I'd just send the whole thing anyway to all
> lists etc.
>
> Kinda two things happening in this series, but I sent it together to
> ensure the second part would apply correctly.
>
> The first is the reset controller that I promised after discovering the
> issue triggered by CONFIG_PM & the phy not coming up correctly. I have
> now removed all the messing with resets from clock enable/disable
> functions & now use the aux bus to set up a reset controller driver.
> Since I needed something to test it, I hooked up the reset for the
> Cadence MACB on PolarFire SoC. This has been split into a second series
> for v2:
> https://lore.kernel.org/all/[email protected]/
>
> The second part adds rate control for the MSS PLL clock, followed by
> some simplifications to the driver & conversions of some custom structs
> to the corresponding structs in the framework.
>
> Thanks,
> Conor.
>
> Changes since v1:
> - split off the net patches
> - clk: actually pass the spinlock to the converted dividers & gates
> - reset: added a spinlock around RMW access to registers
> - reset: switched to BIT(i) macros
> - reset: used local copies of some variables as pointed out by Philipp
> - reset: dropped the success printout
>
> Conor Dooley (12):
> dt-bindings: clk: microchip: mpfs: add reset controller support
> clk: microchip: mpfs: add reset controller
> reset: add polarfire soc reset support
> MAINTAINERS: add polarfire soc reset controller
> riscv: dts: microchip: add mpfs specific macb reset support
> clk: microchip: mpfs: add module_authors entries
> clk: microchip: mpfs: add MSS pll's set & round rate
> clk: microchip: mpfs: move id & offset out of clock structs
> clk: microchip: mpfs: simplify control reg access
> clk: microchip: mpfs: delete 2 line mpfs_clk_register_foo()
> clk: microchip: mpfs: convert cfg_clk to clk_divider
> clk: microchip: mpfs: convert periph_clk to clk_gate
>
> .../bindings/clock/microchip,mpfs.yaml | 17 +-
> MAINTAINERS | 1 +
> arch/riscv/boot/dts/microchip/mpfs.dtsi | 7 +-
> drivers/clk/microchip/Kconfig | 1 +
> drivers/clk/microchip/clk-mpfs.c | 379 +++++++++---------
> drivers/reset/Kconfig | 7 +
> drivers/reset/Makefile | 2 +-
> drivers/reset/reset-mpfs.c | 157 ++++++++
> include/soc/microchip/mpfs.h | 8 +
> 9 files changed, 386 insertions(+), 193 deletions(-)
> create mode 100644 drivers/reset/reset-mpfs.c
>
>
> base-commit: b13baccc3850ca8b8cccbf8ed9912dbaa0fdf7f3

2022-08-10 12:09:49

by Daire McNamara

[permalink] [raw]
Subject: Re: [PATCH v2 00/12] PolarFire SoC reset controller & clock cleanups

Reviewed-by: Daire McNamara <[email protected]>

On Mon, 2022-07-04 at 13:15 +0100, Conor Dooley wrote:
> Hey all,
> I know I have not sat on the RFC I sent about the aux. bus parts
> for too long, but figured I'd just send the whole thing anyway to all
> lists etc.
>
> Kinda two things happening in this series, but I sent it together to
> ensure the second part would apply correctly.
>
> The first is the reset controller that I promised after discovering
> the
> issue triggered by CONFIG_PM & the phy not coming up correctly. I
> have
> now removed all the messing with resets from clock enable/disable
> functions & now use the aux bus to set up a reset controller driver.
> Since I needed something to test it, I hooked up the reset for the
> Cadence MACB on PolarFire SoC. This has been split into a second
> series
> for v2:
> https://lore.kernel.org/all/[email protected]/
>
> The second part adds rate control for the MSS PLL clock, followed by
> some simplifications to the driver & conversions of some custom
> structs
> to the corresponding structs in the framework.
>
> Thanks,
> Conor.
>
> Changes since v1:
> - split off the net patches
> - clk: actually pass the spinlock to the converted dividers & gates
> - reset: added a spinlock around RMW access to registers
> - reset: switched to BIT(i) macros
> - reset: used local copies of some variables as pointed out by
> Philipp
> - reset: dropped the success printout
>
> Conor Dooley (12):
> dt-bindings: clk: microchip: mpfs: add reset controller support
> clk: microchip: mpfs: add reset controller
> reset: add polarfire soc reset support
> MAINTAINERS: add polarfire soc reset controller
> riscv: dts: microchip: add mpfs specific macb reset support
> clk: microchip: mpfs: add module_authors entries
> clk: microchip: mpfs: add MSS pll's set & round rate
> clk: microchip: mpfs: move id & offset out of clock structs
> clk: microchip: mpfs: simplify control reg access
> clk: microchip: mpfs: delete 2 line mpfs_clk_register_foo()
> clk: microchip: mpfs: convert cfg_clk to clk_divider
> clk: microchip: mpfs: convert periph_clk to clk_gate
>
> .../bindings/clock/microchip,mpfs.yaml | 17 +-
> MAINTAINERS | 1 +
> arch/riscv/boot/dts/microchip/mpfs.dtsi | 7 +-
> drivers/clk/microchip/Kconfig | 1 +
> drivers/clk/microchip/clk-mpfs.c | 379 +++++++++-------
> --
> drivers/reset/Kconfig | 7 +
> drivers/reset/Makefile | 2 +-
> drivers/reset/reset-mpfs.c | 157 ++++++++
> include/soc/microchip/mpfs.h | 8 +
> 9 files changed, 386 insertions(+), 193 deletions(-)
> create mode 100644 drivers/reset/reset-mpfs.c
>
>
> base-commit: b13baccc3850ca8b8cccbf8ed9912dbaa0fdf7f3

2022-08-10 19:31:41

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2 00/12] PolarFire SoC reset controller & clock cleanups

On 10/08/2022 19:56, Nathan Chancellor wrote:
> Hi Conor,
>
> On Tue, Aug 09, 2022 at 11:05:32PM +0000, [email protected] wrote:
>> +CC clang people :)
>>
>> Got an odd one here and would appreciate some pointers for where to
>> look. This code when built with gcc boots fine, for example with:
>> riscv64-unknown-linux-gnu-gcc (g5964b5cd727) 11.1.0
>> The same code but build with clang build it fails to boot but prior to
>> that applying this patchset it boots fine. Specifically it is the patch
>> "clk: microchip: mpfs: move id & offset out of clock structs"
>>
>> I applied this patchset on top of tonight's master (15205c2829ca) but
>> I've been seeing the same problem for a few weeks on -next too. I tried
>> the following 2 versions of clang/llvm:
>> ClangBuiltLinux clang version 15.0.0 (5b0788fef86ed7008a11f6ee19b9d86d42b6fcfa), LLD 15.0.0
>> ClangBuiltLinux clang version 15.0.0 (bab8af8ea062f6332b5c5d13ae688bb8900f244a), LLD 15.0.0
>
> Good to know that it reproduces with fairly recent versions of LLVM :)
>
>> It's probably something silly that I've overlooked but I am not au
>> fait with these sort of things unfortunately, but hey - at least I'll
>> learn something then.
>
> I took a quick glance at the patch you mentioned above and I don't
> immediately see anything as problematic...

Yeah, I couldn't see any low hanging fruit either.

> I was going to see if I could
> reproduce this locally in QEMU since I do see there is a machine
> 'microchip-icicle-kit' but I am not having much success getting the
> machine past SBI. Does this reproduce in QEMU or are you working with
> the real hardware? If QEMU, do you happen to have a working invocation
> handy?

Yeah... So there was a QEMU incantation that worked at some point in
the past (ie when someone wrote the QEMU port) but most peripherals
are not implemented and current versions of our openSBI implementation
requires more than one of the unimplemented peripherals. I was trying to
get it working lately in the evenings based on some patches that were a
year old but no joy :/

I'm running on the real hardware, I'll give the older combo of qemu
"bios" etc a go again over the weekend & try to get it working. In the
meantime, any suggestions?
Thanks Nathan,
Conor.

>
>> The boot log is fairly short so here ya go:
>>
>> [ 0.000000] Linux version 5.19.0-13253-g374b508ee318 (conor@spud) (ClangBuiltLinux clang version 15.0.0 (git@g
>> ithub.com:llvm/llvm-project.git 5b0788fef86ed7008a11f6ee19b9d86d42b6fcfa), LLD 15.0.0) #1 SMP Tue Aug 9 22:42:10 IST
>> 2022
>> [ 0.000000] OF: fdt: Ignoring memory range 0x80000000 - 0x80200000
>> [ 0.000000] Machine model: Microchip PolarFire-SoC Icicle Kit
>> [ 0.000000] earlycon: ns16550a0 at MMIO32 0x0000000020100000 (options '115200n8')
>> [ 0.000000] printk: bootconsole [ns16550a0] enabled
>> [ 0.000000] efi: UEFI not found.
>> [ 0.000000] Zone ranges:
>> [ 0.000000] DMA32 [mem 0x0000000080200000-0x00000000ffffffff]
>> [ 0.000000] Normal [mem 0x0000000100000000-0x000000103fffffff]
>> [ 0.000000] Movable zone start for each node
>> [ 0.000000] Early memory node ranges
>> [ 0.000000] node 0: [mem 0x0000000080200000-0x00000000adffffff]
>> [ 0.000000] node 0: [mem 0x0000001000000000-0x000000103fffffff]
>> [ 0.000000] Initmem setup node 0 [mem 0x0000000080200000-0x000000103fffffff]
>> [ 0.000000] On node 0, zone Normal: 16064512 pages in unavailable ranges
>> [ 0.000000] SBI specification v0.3 detected
>> [ 0.000000] SBI implementation ID=0x1 Version=0x9
>> [ 0.000000] SBI TIME extension detected
>> [ 0.000000] SBI IPI extension detected
>> [ 0.000000] SBI RFENCE extension detected
>> [ 0.000000] SBI HSM extension detected
>> [ 0.000000] CPU with hartid=0 is not available
>> [ 0.000000] CPU with hartid=0 is not available
>> [ 0.000000] riscv: base ISA extensions acdfim
>> [ 0.000000] riscv: ELF capabilities acdfim
>> [ 0.000000] percpu: Embedded 18 pages/cpu s34168 r8192 d31368 u73728
>> [ 0.000000] pcpu-alloc: s34168 r8192 d31368 u73728 alloc=18*4096
>> [ 0.000000] pcpu-alloc: [0] 0 [0] 1 [0] 2 [0] 3
>> [ 0.000000] CPU node for /cpus/cpu@0 exist but the possible cpu range is :0-3
>> [ 0.000000] Built 1 zonelists, mobility grouping on. Total pages: 224263
>> [ 0.000000] Kernel command line: root=/dev/nfs ip=dhcp debug nfsroot=192.168.2.5:/stuff/nfs_share earlycon
>> [ 0.000000] Dentry cache hash table entries: 131072 (order: 8, 1048576 bytes, linear)
>> [ 0.000000] Inode-cache hash table entries: 65536 (order: 7, 524288 bytes, linear)
>> [ 0.000000] mem auto-init: stack:all(zero), heap alloc:off, heap free:off
>> [ 0.000000] software IO TLB: area num 4.
>> [ 0.000000] software IO TLB: mapped [mem 0x00000000aa000000-0x00000000ae000000] (64MB)
>> [ 0.000000] Virtual kernel memory layout:
>> [ 0.000000] fixmap : 0xffffffc6fee00000 - 0xffffffc6ff000000 (2048 kB)
>> [ 0.000000] pci io : 0xffffffc6ff000000 - 0xffffffc700000000 ( 16 MB)
>> [ 0.000000] vmemmap : 0xffffffc700000000 - 0xffffffc800000000 (4096 MB)
>> [ 0.000000] vmalloc : 0xffffffc800000000 - 0xffffffd800000000 ( 64 GB)
>> [ 0.000000] lowmem : 0xffffffd800000000 - 0xffffffe7bfe00000 ( 62 GB)
>> [ 0.000000] kernel : 0xffffffff80000000 - 0xffffffffffffffff (2047 MB)
>> [ 0.000000] Memory: 803032K/1800192K available (7485K kernel code, 2858K rwdata, 4096K rodata, 2181K init, 394K bss, 997160K reserved, 0K cma-reserved)
>> [ 0.000000] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=4, Nodes=1
>> [ 0.000000] rcu: Hierarchical RCU implementation.
>> [ 0.000000] rcu: RCU restricting CPUs from NR_CPUS=8 to nr_cpu_ids=4.
>> [ 0.000000] rcu: RCU debug extended QS entry/exit.
>> [ 0.000000] Tracing variant of Tasks RCU enabled.
>> [ 0.000000] rcu: RCU calculated value of scheduler-enlistment delay is 25 jiffies.
>> [ 0.000000] rcu: Adjusting geometry for rcu_fanout_leaf=16, nr_cpu_ids=4
>> [ 0.000000] NR_IRQS: 64, nr_irqs: 64, preallocated irqs: 0
>> [ 0.000000] CPU with hartid=0 is not available
>> [ 0.000000] riscv-intc: unable to find hart id for /cpus/cpu@0/interrupt-controller
>> [ 0.000000] riscv-intc: 64 local interrupts mapped
>> [ 0.000000] plic: interrupt-controller@c000000: mapped 186 interrupts with 4 handlers for 9 contexts.
>> [ 0.000000] rcu: srcu_init: Setting srcu_struct sizes based on contention.
>> [ 0.000000] riscv_timer_init_dt: Registering clocksource cpuid [0] hartid [4]
>> [ 0.000000] clocksource: riscv_clocksource: mask: 0xffffffffffffffff max_cycles: 0x1d854df40, max_idle_ns: 3526361616960 ns
>> [ 0.000003] sched_clock: 64 bits at 1000kHz, resolution 1000ns, wraps every 2199023255500ns
>> [ 0.009713] Console: colour dummy device 80x25
>> [ 0.014676] printk: console [tty0] enabled
>> [ 0.019217] printk: bootconsole [ns16550a0] disabled
>>
>> FWIW this is right about when the clock driver gets loaded
>> as you might imagine.
>>
>> My config is here, but it's been reproduced on a few different
>> defconfigs:
>> https://raw.githubusercontent.com/ConchuOD/polarfire-soc-buildroot-sdk/dev/conf/lowmem/defconfig
>>
>> On 04/07/2022 13:15, Conor Dooley wrote:
>>> Hey all,
>>> I know I have not sat on the RFC I sent about the aux. bus parts
>>> for too long, but figured I'd just send the whole thing anyway to all
>>> lists etc.
>>>
>>> Kinda two things happening in this series, but I sent it together to
>>> ensure the second part would apply correctly.
>>>
>>> The first is the reset controller that I promised after discovering the
>>> issue triggered by CONFIG_PM & the phy not coming up correctly. I have
>>> now removed all the messing with resets from clock enable/disable
>>> functions & now use the aux bus to set up a reset controller driver.
>>> Since I needed something to test it, I hooked up the reset for the
>>> Cadence MACB on PolarFire SoC. This has been split into a second series
>>> for v2:
>>> https://lore.kernel.org/all/[email protected]/
>>>
>>> The second part adds rate control for the MSS PLL clock, followed by
>>> some simplifications to the driver & conversions of some custom structs
>>> to the corresponding structs in the framework.
>>>
>>> Thanks,
>>> Conor.
>>>
>>> Changes since v1:
>>> - split off the net patches
>>> - clk: actually pass the spinlock to the converted dividers & gates
>>> - reset: added a spinlock around RMW access to registers
>>> - reset: switched to BIT(i) macros
>>> - reset: used local copies of some variables as pointed out by Philipp
>>> - reset: dropped the success printout
>>>
>>> Conor Dooley (12):
>>> dt-bindings: clk: microchip: mpfs: add reset controller support
>>> clk: microchip: mpfs: add reset controller
>>> reset: add polarfire soc reset support
>>> MAINTAINERS: add polarfire soc reset controller
>>> riscv: dts: microchip: add mpfs specific macb reset support
>>> clk: microchip: mpfs: add module_authors entries
>>> clk: microchip: mpfs: add MSS pll's set & round rate
>>> clk: microchip: mpfs: move id & offset out of clock structs
>>> clk: microchip: mpfs: simplify control reg access
>>> clk: microchip: mpfs: delete 2 line mpfs_clk_register_foo()
>>> clk: microchip: mpfs: convert cfg_clk to clk_divider
>>> clk: microchip: mpfs: convert periph_clk to clk_gate
>>>
>>> .../bindings/clock/microchip,mpfs.yaml | 17 +-
>>> MAINTAINERS | 1 +
>>> arch/riscv/boot/dts/microchip/mpfs.dtsi | 7 +-
>>> drivers/clk/microchip/Kconfig | 1 +
>>> drivers/clk/microchip/clk-mpfs.c | 379 +++++++++---------
>>> drivers/reset/Kconfig | 7 +
>>> drivers/reset/Makefile | 2 +-
>>> drivers/reset/reset-mpfs.c | 157 ++++++++
>>> include/soc/microchip/mpfs.h | 8 +
>>> 9 files changed, 386 insertions(+), 193 deletions(-)
>>> create mode 100644 drivers/reset/reset-mpfs.c
>>>
>>>
>>> base-commit: b13baccc3850ca8b8cccbf8ed9912dbaa0fdf7f3

2022-08-10 20:06:12

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH v2 00/12] PolarFire SoC reset controller & clock cleanups

Hi Conor,

On Tue, Aug 09, 2022 at 11:05:32PM +0000, [email protected] wrote:
> +CC clang people :)
>
> Got an odd one here and would appreciate some pointers for where to
> look. This code when built with gcc boots fine, for example with:
> riscv64-unknown-linux-gnu-gcc (g5964b5cd727) 11.1.0
> The same code but build with clang build it fails to boot but prior to
> that applying this patchset it boots fine. Specifically it is the patch
> "clk: microchip: mpfs: move id & offset out of clock structs"
>
> I applied this patchset on top of tonight's master (15205c2829ca) but
> I've been seeing the same problem for a few weeks on -next too. I tried
> the following 2 versions of clang/llvm:
> ClangBuiltLinux clang version 15.0.0 (5b0788fef86ed7008a11f6ee19b9d86d42b6fcfa), LLD 15.0.0
> ClangBuiltLinux clang version 15.0.0 (bab8af8ea062f6332b5c5d13ae688bb8900f244a), LLD 15.0.0

Good to know that it reproduces with fairly recent versions of LLVM :)

> It's probably something silly that I've overlooked but I am not au
> fait with these sort of things unfortunately, but hey - at least I'll
> learn something then.

I took a quick glance at the patch you mentioned above and I don't
immediately see anything as problematic... I was going to see if I could
reproduce this locally in QEMU since I do see there is a machine
'microchip-icicle-kit' but I am not having much success getting the
machine past SBI. Does this reproduce in QEMU or are you working with
the real hardware? If QEMU, do you happen to have a working invocation
handy?

Cheers,
Nathan

> The boot log is fairly short so here ya go:
>
> [ 0.000000] Linux version 5.19.0-13253-g374b508ee318 (conor@spud) (ClangBuiltLinux clang version 15.0.0 (git@g
> ithub.com:llvm/llvm-project.git 5b0788fef86ed7008a11f6ee19b9d86d42b6fcfa), LLD 15.0.0) #1 SMP Tue Aug 9 22:42:10 IST
> 2022
> [ 0.000000] OF: fdt: Ignoring memory range 0x80000000 - 0x80200000
> [ 0.000000] Machine model: Microchip PolarFire-SoC Icicle Kit
> [ 0.000000] earlycon: ns16550a0 at MMIO32 0x0000000020100000 (options '115200n8')
> [ 0.000000] printk: bootconsole [ns16550a0] enabled
> [ 0.000000] efi: UEFI not found.
> [ 0.000000] Zone ranges:
> [ 0.000000] DMA32 [mem 0x0000000080200000-0x00000000ffffffff]
> [ 0.000000] Normal [mem 0x0000000100000000-0x000000103fffffff]
> [ 0.000000] Movable zone start for each node
> [ 0.000000] Early memory node ranges
> [ 0.000000] node 0: [mem 0x0000000080200000-0x00000000adffffff]
> [ 0.000000] node 0: [mem 0x0000001000000000-0x000000103fffffff]
> [ 0.000000] Initmem setup node 0 [mem 0x0000000080200000-0x000000103fffffff]
> [ 0.000000] On node 0, zone Normal: 16064512 pages in unavailable ranges
> [ 0.000000] SBI specification v0.3 detected
> [ 0.000000] SBI implementation ID=0x1 Version=0x9
> [ 0.000000] SBI TIME extension detected
> [ 0.000000] SBI IPI extension detected
> [ 0.000000] SBI RFENCE extension detected
> [ 0.000000] SBI HSM extension detected
> [ 0.000000] CPU with hartid=0 is not available
> [ 0.000000] CPU with hartid=0 is not available
> [ 0.000000] riscv: base ISA extensions acdfim
> [ 0.000000] riscv: ELF capabilities acdfim
> [ 0.000000] percpu: Embedded 18 pages/cpu s34168 r8192 d31368 u73728
> [ 0.000000] pcpu-alloc: s34168 r8192 d31368 u73728 alloc=18*4096
> [ 0.000000] pcpu-alloc: [0] 0 [0] 1 [0] 2 [0] 3
> [ 0.000000] CPU node for /cpus/cpu@0 exist but the possible cpu range is :0-3
> [ 0.000000] Built 1 zonelists, mobility grouping on. Total pages: 224263
> [ 0.000000] Kernel command line: root=/dev/nfs ip=dhcp debug nfsroot=192.168.2.5:/stuff/nfs_share earlycon
> [ 0.000000] Dentry cache hash table entries: 131072 (order: 8, 1048576 bytes, linear)
> [ 0.000000] Inode-cache hash table entries: 65536 (order: 7, 524288 bytes, linear)
> [ 0.000000] mem auto-init: stack:all(zero), heap alloc:off, heap free:off
> [ 0.000000] software IO TLB: area num 4.
> [ 0.000000] software IO TLB: mapped [mem 0x00000000aa000000-0x00000000ae000000] (64MB)
> [ 0.000000] Virtual kernel memory layout:
> [ 0.000000] fixmap : 0xffffffc6fee00000 - 0xffffffc6ff000000 (2048 kB)
> [ 0.000000] pci io : 0xffffffc6ff000000 - 0xffffffc700000000 ( 16 MB)
> [ 0.000000] vmemmap : 0xffffffc700000000 - 0xffffffc800000000 (4096 MB)
> [ 0.000000] vmalloc : 0xffffffc800000000 - 0xffffffd800000000 ( 64 GB)
> [ 0.000000] lowmem : 0xffffffd800000000 - 0xffffffe7bfe00000 ( 62 GB)
> [ 0.000000] kernel : 0xffffffff80000000 - 0xffffffffffffffff (2047 MB)
> [ 0.000000] Memory: 803032K/1800192K available (7485K kernel code, 2858K rwdata, 4096K rodata, 2181K init, 394K bss, 997160K reserved, 0K cma-reserved)
> [ 0.000000] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=4, Nodes=1
> [ 0.000000] rcu: Hierarchical RCU implementation.
> [ 0.000000] rcu: RCU restricting CPUs from NR_CPUS=8 to nr_cpu_ids=4.
> [ 0.000000] rcu: RCU debug extended QS entry/exit.
> [ 0.000000] Tracing variant of Tasks RCU enabled.
> [ 0.000000] rcu: RCU calculated value of scheduler-enlistment delay is 25 jiffies.
> [ 0.000000] rcu: Adjusting geometry for rcu_fanout_leaf=16, nr_cpu_ids=4
> [ 0.000000] NR_IRQS: 64, nr_irqs: 64, preallocated irqs: 0
> [ 0.000000] CPU with hartid=0 is not available
> [ 0.000000] riscv-intc: unable to find hart id for /cpus/cpu@0/interrupt-controller
> [ 0.000000] riscv-intc: 64 local interrupts mapped
> [ 0.000000] plic: interrupt-controller@c000000: mapped 186 interrupts with 4 handlers for 9 contexts.
> [ 0.000000] rcu: srcu_init: Setting srcu_struct sizes based on contention.
> [ 0.000000] riscv_timer_init_dt: Registering clocksource cpuid [0] hartid [4]
> [ 0.000000] clocksource: riscv_clocksource: mask: 0xffffffffffffffff max_cycles: 0x1d854df40, max_idle_ns: 3526361616960 ns
> [ 0.000003] sched_clock: 64 bits at 1000kHz, resolution 1000ns, wraps every 2199023255500ns
> [ 0.009713] Console: colour dummy device 80x25
> [ 0.014676] printk: console [tty0] enabled
> [ 0.019217] printk: bootconsole [ns16550a0] disabled
>
> FWIW this is right about when the clock driver gets loaded
> as you might imagine.
>
> My config is here, but it's been reproduced on a few different
> defconfigs:
> https://raw.githubusercontent.com/ConchuOD/polarfire-soc-buildroot-sdk/dev/conf/lowmem/defconfig
>
> On 04/07/2022 13:15, Conor Dooley wrote:
> > Hey all,
> > I know I have not sat on the RFC I sent about the aux. bus parts
> > for too long, but figured I'd just send the whole thing anyway to all
> > lists etc.
> >
> > Kinda two things happening in this series, but I sent it together to
> > ensure the second part would apply correctly.
> >
> > The first is the reset controller that I promised after discovering the
> > issue triggered by CONFIG_PM & the phy not coming up correctly. I have
> > now removed all the messing with resets from clock enable/disable
> > functions & now use the aux bus to set up a reset controller driver.
> > Since I needed something to test it, I hooked up the reset for the
> > Cadence MACB on PolarFire SoC. This has been split into a second series
> > for v2:
> > https://lore.kernel.org/all/[email protected]/
> >
> > The second part adds rate control for the MSS PLL clock, followed by
> > some simplifications to the driver & conversions of some custom structs
> > to the corresponding structs in the framework.
> >
> > Thanks,
> > Conor.
> >
> > Changes since v1:
> > - split off the net patches
> > - clk: actually pass the spinlock to the converted dividers & gates
> > - reset: added a spinlock around RMW access to registers
> > - reset: switched to BIT(i) macros
> > - reset: used local copies of some variables as pointed out by Philipp
> > - reset: dropped the success printout
> >
> > Conor Dooley (12):
> > dt-bindings: clk: microchip: mpfs: add reset controller support
> > clk: microchip: mpfs: add reset controller
> > reset: add polarfire soc reset support
> > MAINTAINERS: add polarfire soc reset controller
> > riscv: dts: microchip: add mpfs specific macb reset support
> > clk: microchip: mpfs: add module_authors entries
> > clk: microchip: mpfs: add MSS pll's set & round rate
> > clk: microchip: mpfs: move id & offset out of clock structs
> > clk: microchip: mpfs: simplify control reg access
> > clk: microchip: mpfs: delete 2 line mpfs_clk_register_foo()
> > clk: microchip: mpfs: convert cfg_clk to clk_divider
> > clk: microchip: mpfs: convert periph_clk to clk_gate
> >
> > .../bindings/clock/microchip,mpfs.yaml | 17 +-
> > MAINTAINERS | 1 +
> > arch/riscv/boot/dts/microchip/mpfs.dtsi | 7 +-
> > drivers/clk/microchip/Kconfig | 1 +
> > drivers/clk/microchip/clk-mpfs.c | 379 +++++++++---------
> > drivers/reset/Kconfig | 7 +
> > drivers/reset/Makefile | 2 +-
> > drivers/reset/reset-mpfs.c | 157 ++++++++
> > include/soc/microchip/mpfs.h | 8 +
> > 9 files changed, 386 insertions(+), 193 deletions(-)
> > create mode 100644 drivers/reset/reset-mpfs.c
> >
> >
> > base-commit: b13baccc3850ca8b8cccbf8ed9912dbaa0fdf7f3

2022-08-10 20:47:35

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH v2 00/12] PolarFire SoC reset controller & clock cleanups

On Wed, Aug 10, 2022 at 07:20:24PM +0000, [email protected] wrote:
> On 10/08/2022 19:56, Nathan Chancellor wrote:
> > Hi Conor,
> >
> > On Tue, Aug 09, 2022 at 11:05:32PM +0000, [email protected] wrote:
> >> +CC clang people :)
> >>
> >> Got an odd one here and would appreciate some pointers for where to
> >> look. This code when built with gcc boots fine, for example with:
> >> riscv64-unknown-linux-gnu-gcc (g5964b5cd727) 11.1.0
> >> The same code but build with clang build it fails to boot but prior to
> >> that applying this patchset it boots fine. Specifically it is the patch
> >> "clk: microchip: mpfs: move id & offset out of clock structs"
> >>
> >> I applied this patchset on top of tonight's master (15205c2829ca) but
> >> I've been seeing the same problem for a few weeks on -next too. I tried
> >> the following 2 versions of clang/llvm:
> >> ClangBuiltLinux clang version 15.0.0 (5b0788fef86ed7008a11f6ee19b9d86d42b6fcfa), LLD 15.0.0
> >> ClangBuiltLinux clang version 15.0.0 (bab8af8ea062f6332b5c5d13ae688bb8900f244a), LLD 15.0.0
> >
> > Good to know that it reproduces with fairly recent versions of LLVM :)
> >
> >> It's probably something silly that I've overlooked but I am not au
> >> fait with these sort of things unfortunately, but hey - at least I'll
> >> learn something then.
> >
> > I took a quick glance at the patch you mentioned above and I don't
> > immediately see anything as problematic...
>
> Yeah, I couldn't see any low hanging fruit either.
>
> > I was going to see if I could
> > reproduce this locally in QEMU since I do see there is a machine
> > 'microchip-icicle-kit' but I am not having much success getting the
> > machine past SBI. Does this reproduce in QEMU or are you working with
> > the real hardware? If QEMU, do you happen to have a working invocation
> > handy?
>
> Yeah... So there was a QEMU incantation that worked at some point in
> the past (ie when someone wrote the QEMU port) but most peripherals
> are not implemented and current versions of our openSBI implementation
> requires more than one of the unimplemented peripherals. I was trying to
> get it working lately in the evenings based on some patches that were a
> year old but no joy :/

Heh, I guess that would explain why it wasn't working for me :)

> I'm running on the real hardware, I'll give the older combo of qemu
> "bios" etc a go again over the weekend & try to get it working. In the
> meantime, any suggestions?

Are you building with 'LLVM=1' or just 'CC=clang'? If 'LLVM=1', I would
try breaking it apart into the individual options (LD=ld.lld,
OBJCOPY=llvm-objcopy) and see if dropping one of those makes a
difference. We have had subtle differences between the GNU and LLVM
tools before and it is much easier to look into that difference if we
know it happens in only one tool.

Otherwise, I am not sure I have any immediate ideas other than looking
at the disassembly and trying to see if something is going wrong. Is
the object file being modified in any other way (I don't think there is
something like objtool for RISC-V but I could be wrong)?

Cheers,
Nathan

> >> The boot log is fairly short so here ya go:
> >>
> >> [ 0.000000] Linux version 5.19.0-13253-g374b508ee318 (conor@spud) (ClangBuiltLinux clang version 15.0.0 (git@g
> >> ithub.com:llvm/llvm-project.git 5b0788fef86ed7008a11f6ee19b9d86d42b6fcfa), LLD 15.0.0) #1 SMP Tue Aug 9 22:42:10 IST
> >> 2022
> >> [ 0.000000] OF: fdt: Ignoring memory range 0x80000000 - 0x80200000
> >> [ 0.000000] Machine model: Microchip PolarFire-SoC Icicle Kit
> >> [ 0.000000] earlycon: ns16550a0 at MMIO32 0x0000000020100000 (options '115200n8')
> >> [ 0.000000] printk: bootconsole [ns16550a0] enabled
> >> [ 0.000000] efi: UEFI not found.
> >> [ 0.000000] Zone ranges:
> >> [ 0.000000] DMA32 [mem 0x0000000080200000-0x00000000ffffffff]
> >> [ 0.000000] Normal [mem 0x0000000100000000-0x000000103fffffff]
> >> [ 0.000000] Movable zone start for each node
> >> [ 0.000000] Early memory node ranges
> >> [ 0.000000] node 0: [mem 0x0000000080200000-0x00000000adffffff]
> >> [ 0.000000] node 0: [mem 0x0000001000000000-0x000000103fffffff]
> >> [ 0.000000] Initmem setup node 0 [mem 0x0000000080200000-0x000000103fffffff]
> >> [ 0.000000] On node 0, zone Normal: 16064512 pages in unavailable ranges
> >> [ 0.000000] SBI specification v0.3 detected
> >> [ 0.000000] SBI implementation ID=0x1 Version=0x9
> >> [ 0.000000] SBI TIME extension detected
> >> [ 0.000000] SBI IPI extension detected
> >> [ 0.000000] SBI RFENCE extension detected
> >> [ 0.000000] SBI HSM extension detected
> >> [ 0.000000] CPU with hartid=0 is not available
> >> [ 0.000000] CPU with hartid=0 is not available
> >> [ 0.000000] riscv: base ISA extensions acdfim
> >> [ 0.000000] riscv: ELF capabilities acdfim
> >> [ 0.000000] percpu: Embedded 18 pages/cpu s34168 r8192 d31368 u73728
> >> [ 0.000000] pcpu-alloc: s34168 r8192 d31368 u73728 alloc=18*4096
> >> [ 0.000000] pcpu-alloc: [0] 0 [0] 1 [0] 2 [0] 3
> >> [ 0.000000] CPU node for /cpus/cpu@0 exist but the possible cpu range is :0-3
> >> [ 0.000000] Built 1 zonelists, mobility grouping on. Total pages: 224263
> >> [ 0.000000] Kernel command line: root=/dev/nfs ip=dhcp debug nfsroot=192.168.2.5:/stuff/nfs_share earlycon
> >> [ 0.000000] Dentry cache hash table entries: 131072 (order: 8, 1048576 bytes, linear)
> >> [ 0.000000] Inode-cache hash table entries: 65536 (order: 7, 524288 bytes, linear)
> >> [ 0.000000] mem auto-init: stack:all(zero), heap alloc:off, heap free:off
> >> [ 0.000000] software IO TLB: area num 4.
> >> [ 0.000000] software IO TLB: mapped [mem 0x00000000aa000000-0x00000000ae000000] (64MB)
> >> [ 0.000000] Virtual kernel memory layout:
> >> [ 0.000000] fixmap : 0xffffffc6fee00000 - 0xffffffc6ff000000 (2048 kB)
> >> [ 0.000000] pci io : 0xffffffc6ff000000 - 0xffffffc700000000 ( 16 MB)
> >> [ 0.000000] vmemmap : 0xffffffc700000000 - 0xffffffc800000000 (4096 MB)
> >> [ 0.000000] vmalloc : 0xffffffc800000000 - 0xffffffd800000000 ( 64 GB)
> >> [ 0.000000] lowmem : 0xffffffd800000000 - 0xffffffe7bfe00000 ( 62 GB)
> >> [ 0.000000] kernel : 0xffffffff80000000 - 0xffffffffffffffff (2047 MB)
> >> [ 0.000000] Memory: 803032K/1800192K available (7485K kernel code, 2858K rwdata, 4096K rodata, 2181K init, 394K bss, 997160K reserved, 0K cma-reserved)
> >> [ 0.000000] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=4, Nodes=1
> >> [ 0.000000] rcu: Hierarchical RCU implementation.
> >> [ 0.000000] rcu: RCU restricting CPUs from NR_CPUS=8 to nr_cpu_ids=4.
> >> [ 0.000000] rcu: RCU debug extended QS entry/exit.
> >> [ 0.000000] Tracing variant of Tasks RCU enabled.
> >> [ 0.000000] rcu: RCU calculated value of scheduler-enlistment delay is 25 jiffies.
> >> [ 0.000000] rcu: Adjusting geometry for rcu_fanout_leaf=16, nr_cpu_ids=4
> >> [ 0.000000] NR_IRQS: 64, nr_irqs: 64, preallocated irqs: 0
> >> [ 0.000000] CPU with hartid=0 is not available
> >> [ 0.000000] riscv-intc: unable to find hart id for /cpus/cpu@0/interrupt-controller
> >> [ 0.000000] riscv-intc: 64 local interrupts mapped
> >> [ 0.000000] plic: interrupt-controller@c000000: mapped 186 interrupts with 4 handlers for 9 contexts.
> >> [ 0.000000] rcu: srcu_init: Setting srcu_struct sizes based on contention.
> >> [ 0.000000] riscv_timer_init_dt: Registering clocksource cpuid [0] hartid [4]
> >> [ 0.000000] clocksource: riscv_clocksource: mask: 0xffffffffffffffff max_cycles: 0x1d854df40, max_idle_ns: 3526361616960 ns
> >> [ 0.000003] sched_clock: 64 bits at 1000kHz, resolution 1000ns, wraps every 2199023255500ns
> >> [ 0.009713] Console: colour dummy device 80x25
> >> [ 0.014676] printk: console [tty0] enabled
> >> [ 0.019217] printk: bootconsole [ns16550a0] disabled
> >>
> >> FWIW this is right about when the clock driver gets loaded
> >> as you might imagine.
> >>
> >> My config is here, but it's been reproduced on a few different
> >> defconfigs:
> >> https://raw.githubusercontent.com/ConchuOD/polarfire-soc-buildroot-sdk/dev/conf/lowmem/defconfig
> >>
> >> On 04/07/2022 13:15, Conor Dooley wrote:
> >>> Hey all,
> >>> I know I have not sat on the RFC I sent about the aux. bus parts
> >>> for too long, but figured I'd just send the whole thing anyway to all
> >>> lists etc.
> >>>
> >>> Kinda two things happening in this series, but I sent it together to
> >>> ensure the second part would apply correctly.
> >>>
> >>> The first is the reset controller that I promised after discovering the
> >>> issue triggered by CONFIG_PM & the phy not coming up correctly. I have
> >>> now removed all the messing with resets from clock enable/disable
> >>> functions & now use the aux bus to set up a reset controller driver.
> >>> Since I needed something to test it, I hooked up the reset for the
> >>> Cadence MACB on PolarFire SoC. This has been split into a second series
> >>> for v2:
> >>> https://lore.kernel.org/all/[email protected]/
> >>>
> >>> The second part adds rate control for the MSS PLL clock, followed by
> >>> some simplifications to the driver & conversions of some custom structs
> >>> to the corresponding structs in the framework.
> >>>
> >>> Thanks,
> >>> Conor.
> >>>
> >>> Changes since v1:
> >>> - split off the net patches
> >>> - clk: actually pass the spinlock to the converted dividers & gates
> >>> - reset: added a spinlock around RMW access to registers
> >>> - reset: switched to BIT(i) macros
> >>> - reset: used local copies of some variables as pointed out by Philipp
> >>> - reset: dropped the success printout
> >>>
> >>> Conor Dooley (12):
> >>> dt-bindings: clk: microchip: mpfs: add reset controller support
> >>> clk: microchip: mpfs: add reset controller
> >>> reset: add polarfire soc reset support
> >>> MAINTAINERS: add polarfire soc reset controller
> >>> riscv: dts: microchip: add mpfs specific macb reset support
> >>> clk: microchip: mpfs: add module_authors entries
> >>> clk: microchip: mpfs: add MSS pll's set & round rate
> >>> clk: microchip: mpfs: move id & offset out of clock structs
> >>> clk: microchip: mpfs: simplify control reg access
> >>> clk: microchip: mpfs: delete 2 line mpfs_clk_register_foo()
> >>> clk: microchip: mpfs: convert cfg_clk to clk_divider
> >>> clk: microchip: mpfs: convert periph_clk to clk_gate
> >>>
> >>> .../bindings/clock/microchip,mpfs.yaml | 17 +-
> >>> MAINTAINERS | 1 +
> >>> arch/riscv/boot/dts/microchip/mpfs.dtsi | 7 +-
> >>> drivers/clk/microchip/Kconfig | 1 +
> >>> drivers/clk/microchip/clk-mpfs.c | 379 +++++++++---------
> >>> drivers/reset/Kconfig | 7 +
> >>> drivers/reset/Makefile | 2 +-
> >>> drivers/reset/reset-mpfs.c | 157 ++++++++
> >>> include/soc/microchip/mpfs.h | 8 +
> >>> 9 files changed, 386 insertions(+), 193 deletions(-)
> >>> create mode 100644 drivers/reset/reset-mpfs.c
> >>>
> >>>
> >>> base-commit: b13baccc3850ca8b8cccbf8ed9912dbaa0fdf7f3

2022-08-10 21:04:35

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2 00/12] PolarFire SoC reset controller & clock cleanups

On 10/08/2022 20:32, Nathan Chancellor wrote:
> On Wed, Aug 10, 2022 at 07:20:24PM +0000, [email protected] wrote:
>> On 10/08/2022 19:56, Nathan Chancellor wrote:
>>> Hi Conor,
>>>
>>> On Tue, Aug 09, 2022 at 11:05:32PM +0000, [email protected] wrote:
>>>> +CC clang people :)
>>>>
>>>> Got an odd one here and would appreciate some pointers for where to
>>>> look. This code when built with gcc boots fine, for example with:
>>>> riscv64-unknown-linux-gnu-gcc (g5964b5cd727) 11.1.0
>>>> The same code but build with clang build it fails to boot but prior to
>>>> that applying this patchset it boots fine. Specifically it is the patch
>>>> "clk: microchip: mpfs: move id & offset out of clock structs"
>>>>
>>>> I applied this patchset on top of tonight's master (15205c2829ca) but
>>>> I've been seeing the same problem for a few weeks on -next too. I tried
>>>> the following 2 versions of clang/llvm:
>>>> ClangBuiltLinux clang version 15.0.0 (5b0788fef86ed7008a11f6ee19b9d86d42b6fcfa), LLD 15.0.0
>>>> ClangBuiltLinux clang version 15.0.0 (bab8af8ea062f6332b5c5d13ae688bb8900f244a), LLD 15.0.0
>>>
>>> Good to know that it reproduces with fairly recent versions of LLVM :)
>>>
>>>> It's probably something silly that I've overlooked but I am not au
>>>> fait with these sort of things unfortunately, but hey - at least I'll
>>>> learn something then.
>>>
>>> I took a quick glance at the patch you mentioned above and I don't
>>> immediately see anything as problematic...
>>
>> Yeah, I couldn't see any low hanging fruit either.
>>
>>> I was going to see if I could
>>> reproduce this locally in QEMU since I do see there is a machine
>>> 'microchip-icicle-kit' but I am not having much success getting the
>>> machine past SBI. Does this reproduce in QEMU or are you working with
>>> the real hardware? If QEMU, do you happen to have a working invocation
>>> handy?
>>
>> Yeah... So there was a QEMU incantation that worked at some point in
>> the past (ie when someone wrote the QEMU port) but most peripherals
>> are not implemented and current versions of our openSBI implementation
>> requires more than one of the unimplemented peripherals. I was trying to
>> get it working lately in the evenings based on some patches that were a
>> year old but no joy :/
>
> Heh, I guess that would explain why it wasn't working for me :)
>
>> I'm running on the real hardware, I'll give the older combo of qemu
>> "bios" etc a go again over the weekend & try to get it working. In the
>> meantime, any suggestions?
>
> Are you building with 'LLVM=1' or just 'CC=clang'? If 'LLVM=1', I would
> try breaking it apart into the individual options (LD=ld.lld,
> OBJCOPY=llvm-objcopy) and see if dropping one of those makes a
> difference. We have had subtle differences between the GNU and LLVM
> tools before and it is much easier to look into that difference if we
> know it happens in only one tool.

LLVM=1.

>
> Otherwise, I am not sure I have any immediate ideas other than looking
> at the disassembly and trying to see if something is going wrong. Is
> the object file being modified in any other way (I don't think there is
> something like objtool for RISC-V but I could be wrong)?

I'll give the options a go so, I'll LYK how I get on.
Thanks,
Conor.

>
> Cheers,
> Nathan
>
>>>> The boot log is fairly short so here ya go:
>>>>
>>>> [ 0.000000] Linux version 5.19.0-13253-g374b508ee318 (conor@spud) (ClangBuiltLinux clang version 15.0.0 (git@g
>>>> ithub.com:llvm/llvm-project.git 5b0788fef86ed7008a11f6ee19b9d86d42b6fcfa), LLD 15.0.0) #1 SMP Tue Aug 9 22:42:10 IST
>>>> 2022
>>>> [ 0.000000] OF: fdt: Ignoring memory range 0x80000000 - 0x80200000
>>>> [ 0.000000] Machine model: Microchip PolarFire-SoC Icicle Kit
>>>> [ 0.000000] earlycon: ns16550a0 at MMIO32 0x0000000020100000 (options '115200n8')
>>>> [ 0.000000] printk: bootconsole [ns16550a0] enabled
>>>> [ 0.000000] efi: UEFI not found.
>>>> [ 0.000000] Zone ranges:
>>>> [ 0.000000] DMA32 [mem 0x0000000080200000-0x00000000ffffffff]
>>>> [ 0.000000] Normal [mem 0x0000000100000000-0x000000103fffffff]
>>>> [ 0.000000] Movable zone start for each node
>>>> [ 0.000000] Early memory node ranges
>>>> [ 0.000000] node 0: [mem 0x0000000080200000-0x00000000adffffff]
>>>> [ 0.000000] node 0: [mem 0x0000001000000000-0x000000103fffffff]
>>>> [ 0.000000] Initmem setup node 0 [mem 0x0000000080200000-0x000000103fffffff]
>>>> [ 0.000000] On node 0, zone Normal: 16064512 pages in unavailable ranges
>>>> [ 0.000000] SBI specification v0.3 detected
>>>> [ 0.000000] SBI implementation ID=0x1 Version=0x9
>>>> [ 0.000000] SBI TIME extension detected
>>>> [ 0.000000] SBI IPI extension detected
>>>> [ 0.000000] SBI RFENCE extension detected
>>>> [ 0.000000] SBI HSM extension detected
>>>> [ 0.000000] CPU with hartid=0 is not available
>>>> [ 0.000000] CPU with hartid=0 is not available
>>>> [ 0.000000] riscv: base ISA extensions acdfim
>>>> [ 0.000000] riscv: ELF capabilities acdfim
>>>> [ 0.000000] percpu: Embedded 18 pages/cpu s34168 r8192 d31368 u73728
>>>> [ 0.000000] pcpu-alloc: s34168 r8192 d31368 u73728 alloc=18*4096
>>>> [ 0.000000] pcpu-alloc: [0] 0 [0] 1 [0] 2 [0] 3
>>>> [ 0.000000] CPU node for /cpus/cpu@0 exist but the possible cpu range is :0-3
>>>> [ 0.000000] Built 1 zonelists, mobility grouping on. Total pages: 224263
>>>> [ 0.000000] Kernel command line: root=/dev/nfs ip=dhcp debug nfsroot=192.168.2.5:/stuff/nfs_share earlycon
>>>> [ 0.000000] Dentry cache hash table entries: 131072 (order: 8, 1048576 bytes, linear)
>>>> [ 0.000000] Inode-cache hash table entries: 65536 (order: 7, 524288 bytes, linear)
>>>> [ 0.000000] mem auto-init: stack:all(zero), heap alloc:off, heap free:off
>>>> [ 0.000000] software IO TLB: area num 4.
>>>> [ 0.000000] software IO TLB: mapped [mem 0x00000000aa000000-0x00000000ae000000] (64MB)
>>>> [ 0.000000] Virtual kernel memory layout:
>>>> [ 0.000000] fixmap : 0xffffffc6fee00000 - 0xffffffc6ff000000 (2048 kB)
>>>> [ 0.000000] pci io : 0xffffffc6ff000000 - 0xffffffc700000000 ( 16 MB)
>>>> [ 0.000000] vmemmap : 0xffffffc700000000 - 0xffffffc800000000 (4096 MB)
>>>> [ 0.000000] vmalloc : 0xffffffc800000000 - 0xffffffd800000000 ( 64 GB)
>>>> [ 0.000000] lowmem : 0xffffffd800000000 - 0xffffffe7bfe00000 ( 62 GB)
>>>> [ 0.000000] kernel : 0xffffffff80000000 - 0xffffffffffffffff (2047 MB)
>>>> [ 0.000000] Memory: 803032K/1800192K available (7485K kernel code, 2858K rwdata, 4096K rodata, 2181K init, 394K bss, 997160K reserved, 0K cma-reserved)
>>>> [ 0.000000] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=4, Nodes=1
>>>> [ 0.000000] rcu: Hierarchical RCU implementation.
>>>> [ 0.000000] rcu: RCU restricting CPUs from NR_CPUS=8 to nr_cpu_ids=4.
>>>> [ 0.000000] rcu: RCU debug extended QS entry/exit.
>>>> [ 0.000000] Tracing variant of Tasks RCU enabled.
>>>> [ 0.000000] rcu: RCU calculated value of scheduler-enlistment delay is 25 jiffies.
>>>> [ 0.000000] rcu: Adjusting geometry for rcu_fanout_leaf=16, nr_cpu_ids=4
>>>> [ 0.000000] NR_IRQS: 64, nr_irqs: 64, preallocated irqs: 0
>>>> [ 0.000000] CPU with hartid=0 is not available
>>>> [ 0.000000] riscv-intc: unable to find hart id for /cpus/cpu@0/interrupt-controller
>>>> [ 0.000000] riscv-intc: 64 local interrupts mapped
>>>> [ 0.000000] plic: interrupt-controller@c000000: mapped 186 interrupts with 4 handlers for 9 contexts.
>>>> [ 0.000000] rcu: srcu_init: Setting srcu_struct sizes based on contention.
>>>> [ 0.000000] riscv_timer_init_dt: Registering clocksource cpuid [0] hartid [4]
>>>> [ 0.000000] clocksource: riscv_clocksource: mask: 0xffffffffffffffff max_cycles: 0x1d854df40, max_idle_ns: 3526361616960 ns
>>>> [ 0.000003] sched_clock: 64 bits at 1000kHz, resolution 1000ns, wraps every 2199023255500ns
>>>> [ 0.009713] Console: colour dummy device 80x25
>>>> [ 0.014676] printk: console [tty0] enabled
>>>> [ 0.019217] printk: bootconsole [ns16550a0] disabled
>>>>
>>>> FWIW this is right about when the clock driver gets loaded
>>>> as you might imagine.
>>>>
>>>> My config is here, but it's been reproduced on a few different
>>>> defconfigs:
>>>> https://raw.githubusercontent.com/ConchuOD/polarfire-soc-buildroot-sdk/dev/conf/lowmem/defconfig
>>>>
>>>> On 04/07/2022 13:15, Conor Dooley wrote:
>>>>> Hey all,
>>>>> I know I have not sat on the RFC I sent about the aux. bus parts
>>>>> for too long, but figured I'd just send the whole thing anyway to all
>>>>> lists etc.
>>>>>
>>>>> Kinda two things happening in this series, but I sent it together to
>>>>> ensure the second part would apply correctly.
>>>>>
>>>>> The first is the reset controller that I promised after discovering the
>>>>> issue triggered by CONFIG_PM & the phy not coming up correctly. I have
>>>>> now removed all the messing with resets from clock enable/disable
>>>>> functions & now use the aux bus to set up a reset controller driver.
>>>>> Since I needed something to test it, I hooked up the reset for the
>>>>> Cadence MACB on PolarFire SoC. This has been split into a second series
>>>>> for v2:
>>>>> https://lore.kernel.org/all/[email protected]/
>>>>>
>>>>> The second part adds rate control for the MSS PLL clock, followed by
>>>>> some simplifications to the driver & conversions of some custom structs
>>>>> to the corresponding structs in the framework.
>>>>>
>>>>> Thanks,
>>>>> Conor.
>>>>>
>>>>> Changes since v1:
>>>>> - split off the net patches
>>>>> - clk: actually pass the spinlock to the converted dividers & gates
>>>>> - reset: added a spinlock around RMW access to registers
>>>>> - reset: switched to BIT(i) macros
>>>>> - reset: used local copies of some variables as pointed out by Philipp
>>>>> - reset: dropped the success printout
>>>>>
>>>>> Conor Dooley (12):
>>>>> dt-bindings: clk: microchip: mpfs: add reset controller support
>>>>> clk: microchip: mpfs: add reset controller
>>>>> reset: add polarfire soc reset support
>>>>> MAINTAINERS: add polarfire soc reset controller
>>>>> riscv: dts: microchip: add mpfs specific macb reset support
>>>>> clk: microchip: mpfs: add module_authors entries
>>>>> clk: microchip: mpfs: add MSS pll's set & round rate
>>>>> clk: microchip: mpfs: move id & offset out of clock structs
>>>>> clk: microchip: mpfs: simplify control reg access
>>>>> clk: microchip: mpfs: delete 2 line mpfs_clk_register_foo()
>>>>> clk: microchip: mpfs: convert cfg_clk to clk_divider
>>>>> clk: microchip: mpfs: convert periph_clk to clk_gate
>>>>>
>>>>> .../bindings/clock/microchip,mpfs.yaml | 17 +-
>>>>> MAINTAINERS | 1 +
>>>>> arch/riscv/boot/dts/microchip/mpfs.dtsi | 7 +-
>>>>> drivers/clk/microchip/Kconfig | 1 +
>>>>> drivers/clk/microchip/clk-mpfs.c | 379 +++++++++---------
>>>>> drivers/reset/Kconfig | 7 +
>>>>> drivers/reset/Makefile | 2 +-
>>>>> drivers/reset/reset-mpfs.c | 157 ++++++++
>>>>> include/soc/microchip/mpfs.h | 8 +
>>>>> 9 files changed, 386 insertions(+), 193 deletions(-)
>>>>> create mode 100644 drivers/reset/reset-mpfs.c
>>>>>
>>>>>
>>>>> base-commit: b13baccc3850ca8b8cccbf8ed9912dbaa0fdf7f3

2022-08-11 13:24:45

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2 00/12] PolarFire SoC reset controller & clock cleanups

Hey Nathan,

On 10/08/2022 20:43, Conor Dooley - M52691 wrote:
> On 10/08/2022 20:32, Nathan Chancellor wrote:
>> On Wed, Aug 10, 2022 at 07:20:24PM +0000, [email protected] wrote:
>>> On 10/08/2022 19:56, Nathan Chancellor wrote:
>>>> Hi Conor,
>>>>
>>>> On Tue, Aug 09, 2022 at 11:05:32PM +0000, [email protected] wrote:
>>>>> +CC clang people :)
>>>>>
>>>>> Got an odd one here and would appreciate some pointers for where to
>>>>> look. This code when built with gcc boots fine, for example with:
>>>>> riscv64-unknown-linux-gnu-gcc (g5964b5cd727) 11.1.0
>>>>> The same code but build with clang build it fails to boot but prior to
>>>>> that applying this patchset it boots fine. Specifically it is the patch
>>>>> "clk: microchip: mpfs: move id & offset out of clock structs"
>>>>>
>>>>> I applied this patchset on top of tonight's master (15205c2829ca) but
>>>>> I've been seeing the same problem for a few weeks on -next too. I tried
>>>>> the following 2 versions of clang/llvm:
>>>>> ClangBuiltLinux clang version 15.0.0 (5b0788fef86ed7008a11f6ee19b9d86d42b6fcfa), LLD 15.0.0
>>>>> ClangBuiltLinux clang version 15.0.0 (bab8af8ea062f6332b5c5d13ae688bb8900f244a), LLD 15.0.0
>>>>
>>>> Good to know that it reproduces with fairly recent versions of LLVM :)
>>>>
>>>>> It's probably something silly that I've overlooked but I am not au
>>>>> fait with these sort of things unfortunately, but hey - at least I'll
>>>>> learn something then.
>>>>
>>>> I took a quick glance at the patch you mentioned above and I don't
>>>> immediately see anything as problematic...
>>>
>>> Yeah, I couldn't see any low hanging fruit either.
>>>
>>>> I was going to see if I could
>>>> reproduce this locally in QEMU since I do see there is a machine
>>>> 'microchip-icicle-kit' but I am not having much success getting the
>>>> machine past SBI. Does this reproduce in QEMU or are you working with
>>>> the real hardware? If QEMU, do you happen to have a working invocation
>>>> handy?
>>>
>>> Yeah... So there was a QEMU incantation that worked at some point in
>>> the past (ie when someone wrote the QEMU port) but most peripherals
>>> are not implemented and current versions of our openSBI implementation
>>> requires more than one of the unimplemented peripherals. I was trying to
>>> get it working lately in the evenings based on some patches that were a
>>> year old but no joy :/
>>
>> Heh, I guess that would explain why it wasn't working for me :)
>>
>>> I'm running on the real hardware, I'll give the older combo of qemu
>>> "bios" etc a go again over the weekend & try to get it working. In the
>>> meantime, any suggestions?
>>
>> Are you building with 'LLVM=1' or just 'CC=clang'? If 'LLVM=1', I would
>> try breaking it apart into the individual options (LD=ld.lld,
>> OBJCOPY=llvm-objcopy) and see if dropping one of those makes a
>> difference. We have had subtle differences between the GNU and LLVM
>> tools before and it is much easier to look into that difference if we
>> know it happens in only one tool.
>
> LLVM=1.
>
>>
>> Otherwise, I am not sure I have any immediate ideas other than looking
>> at the disassembly and trying to see if something is going wrong. Is
>> the object file being modified in any other way (I don't think there is
>> something like objtool for RISC-V but I could be wrong)?
>
> I'll give the options a go so, I'll LYK how I get on.

So I managed to wrangle QEMU into repro-ing. booting with bootloaders
etc isn't going to work (nor will the config with gcc actually boot
properly) but it gets far enough to reproduce the problem.
You've got to jump right to the kernel for which the magic incantation
is:

$(QEMU)/qemu-system-riscv64 -M microchip-icicle-kit \
-m 2G -smp 5 \
-kernel $(wrkdir)/vmlinux.bin \
-dtb $(wrkdir)/riscvpc.dtb \
-display none -serial null \
-serial stdio

(serial0 is disabled in the dt)

With gcc there'll be a bunch of warnings like:
clk_ahb: Zero divisor and CLK_DIVIDER_ALLOW_ZERO not set
That's "fine", not sure if it's the lack of bootloaders or the
emulation but 0 isn't a value the hardware will see. With the defconfig
I provided it'll fail to boot fairly late on because of missing musb
emulation.

Doesn't really matter since thats long enough to get past the switch
out of earlycon which is where the clang built kernel dies.

Didn't get a chance to look at disassembly etc today, but as I said
last night it reproduces with GNU binutils.

Thanks,
Conor.

On another note, brought up our QEMU port's state today so fixing
it is now on the good ole, ever expanding todo list :)

2022-08-14 11:59:28

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2 00/12] PolarFire SoC reset controller & clock cleanups

On 11/08/2022 14:13, Conor Dooley wrote:
> Hey Nathan,
>
> On 10/08/2022 20:43, Conor Dooley - M52691 wrote:
>> On 10/08/2022 20:32, Nathan Chancellor wrote:
>>> On Wed, Aug 10, 2022 at 07:20:24PM +0000, [email protected] wrote:
>>>> On 10/08/2022 19:56, Nathan Chancellor wrote:
>>>>> Hi Conor,
>>>>>
>>>>> On Tue, Aug 09, 2022 at 11:05:32PM +0000, [email protected] wrote:
>>>>>> +CC clang people :)
>>>>>>
>>>>>> Got an odd one here and would appreciate some pointers for where to
>>>>>> look. This code when built with gcc boots fine, for example with:
>>>>>> riscv64-unknown-linux-gnu-gcc (g5964b5cd727) 11.1.0
>>>>>> The same code but build with clang build it fails to boot but prior to
>>>>>> that applying this patchset it boots fine. Specifically it is the patch
>>>>>> "clk: microchip: mpfs: move id & offset out of clock structs"
>>>>>>
>>>>>> I applied this patchset on top of tonight's master (15205c2829ca) but
>>>>>> I've been seeing the same problem for a few weeks on -next too. I tried
>>>>>> the following 2 versions of clang/llvm:
>>>>>> ClangBuiltLinux clang version 15.0.0 (5b0788fef86ed7008a11f6ee19b9d86d42b6fcfa), LLD 15.0.0
>>>>>> ClangBuiltLinux clang version 15.0.0 (bab8af8ea062f6332b5c5d13ae688bb8900f244a), LLD 15.0.0
>>>>>
>>>>> Good to know that it reproduces with fairly recent versions of LLVM :)
>>>>>
>>>>>> It's probably something silly that I've overlooked but I am not au
>>>>>> fait with these sort of things unfortunately, but hey - at least I'll
>>>>>> learn something then.
>>>>>
>>>>> I took a quick glance at the patch you mentioned above and I don't
>>>>> immediately see anything as problematic...
>>>>
>>>> Yeah, I couldn't see any low hanging fruit either.
>>>>
>>>>> I was going to see if I could
>>>>> reproduce this locally in QEMU since I do see there is a machine
>>>>> 'microchip-icicle-kit' but I am not having much success getting the
>>>>> machine past SBI. Does this reproduce in QEMU or are you working with
>>>>> the real hardware? If QEMU, do you happen to have a working invocation
>>>>> handy?
>>>>
>>>> Yeah... So there was a QEMU incantation that worked at some point in
>>>> the past (ie when someone wrote the QEMU port) but most peripherals
>>>> are not implemented and current versions of our openSBI implementation
>>>> requires more than one of the unimplemented peripherals. I was trying to
>>>> get it working lately in the evenings based on some patches that were a
>>>> year old but no joy :/
>>>
>>> Heh, I guess that would explain why it wasn't working for me :)
>>>
>>>> I'm running on the real hardware, I'll give the older combo of qemu
>>>> "bios" etc a go again over the weekend & try to get it working. In the
>>>> meantime, any suggestions?
>>>
>>> Are you building with 'LLVM=1' or just 'CC=clang'? If 'LLVM=1', I would
>>> try breaking it apart into the individual options (LD=ld.lld,
>>> OBJCOPY=llvm-objcopy) and see if dropping one of those makes a
>>> difference. We have had subtle differences between the GNU and LLVM
>>> tools before and it is much easier to look into that difference if we
>>> know it happens in only one tool.
>>
>> LLVM=1.
>>
>>>
>>> Otherwise, I am not sure I have any immediate ideas other than looking
>>> at the disassembly and trying to see if something is going wrong. Is
>>> the object file being modified in any other way (I don't think there is
>>> something like objtool for RISC-V but I could be wrong)?
>>
>> I'll give the options a go so, I'll LYK how I get on.
>
> So I managed to wrangle QEMU into repro-ing. booting with bootloaders
> etc isn't going to work (nor will the config with gcc actually boot
> properly) but it gets far enough to reproduce the problem.
> You've got to jump right to the kernel for which the magic incantation
> is:
>
> $(QEMU)/qemu-system-riscv64 -M microchip-icicle-kit \
>     -m 2G -smp 5 \
>     -kernel $(wrkdir)/vmlinux.bin \
>     -dtb $(wrkdir)/riscvpc.dtb \
>     -display none -serial null \
>     -serial stdio
>
> (serial0 is disabled in the dt)
>
> With gcc there'll be a bunch of warnings like:
> clk_ahb: Zero divisor and CLK_DIVIDER_ALLOW_ZERO not set
> That's "fine", not sure if it's the lack of bootloaders or the
> emulation but 0 isn't a value the hardware will see. With the defconfig
> I provided it'll fail to boot fairly late on because of missing musb
> emulation.

FWIW, I posted a QEMU patch to fix the missing peripherals, so a direct
kernel boot works now for GCC:
https://lore.kernel.org/qemu-devel/[email protected]

(btw, I am on libera as conchuod in #riscv if you ever wanna ping me about
something, usually still about for "sane" NA working hours too)

>
> Doesn't really matter since thats long enough to get past the switch
> out of earlycon which is where the clang built kernel dies.
>
> Didn't get a chance to look at disassembly etc today, but as I said
> last night it reproduces with GNU binutils.
>
> Thanks,
> Conor.
>
> On another note, brought up our QEMU port's state today so fixing
> it is now on the good ole, ever expanding todo list :)

2022-08-17 12:30:43

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v2 00/12] PolarFire SoC reset controller & clock cleanups

On 14/08/2022 12:41, [email protected] wrote:
>>
>> Doesn't really matter since thats long enough to get past the switch
>> out of earlycon which is where the clang built kernel dies.

I had forgotten something obvious: keep_bootcon. With that, on the sbi
console I get a nice:

[ 0.581754] Unable to handle kernel NULL pointer dereference at virtual address 00000000000000b1
[ 0.591520] Oops [#1]
[ 0.594045] Modules linked in:
[ 0.597435] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.0.0-rc1-00011-g8e1459cf4eca #1
[ 0.606188] Hardware name: Microchip PolarFire-SoC Icicle Kit (DT)
[ 0.613012] epc : __clk_register+0x4a6/0x85c
[ 0.617759] ra : __clk_register+0x49e/0x85c
[ 0.622489] epc : ffffffff803faf7c ra : ffffffff803faf74 sp : ffffffc80400b720
[ 0.630466] gp : ffffffff810e93f8 tp : ffffffe77fe60000 t0 : ffffffe77ffb3800
[ 0.638443] t1 : 000000000000000a t2 : ffffffffffffffff s0 : ffffffc80400b7c0
[ 0.646420] s1 : 0000000000000001 a0 : 0000000000000001 a1 : 0000000000000000
[ 0.654396] a2 : 0000000000000001 a3 : 0000000000000000 a4 : 0000000000000000
[ 0.662373] a5 : ffffffff803a5810 a6 : 0000000200000022 a7 : 0000000000000006
[ 0.670350] s2 : ffffffff81099d48 s3 : ffffffff80d6e28e s4 : 0000000000000028
[ 0.678327] s5 : ffffffff810ed3c8 s6 : ffffffff810ed3d0 s7 : ffffffe77ffbc100
[ 0.686304] s8 : ffffffe77ffb1540 s9 : ffffffe77ffb1540 s10: 0000000000000008
[ 0.694281] s11: 0000000000000000 t3 : 00000000000000c6 t4 : 0000000000000007
[ 0.702258] t5 : ffffffff810c78c0 t6 : ffffffe77ff88cd0
[ 0.708125] status: 0000000200000120 badaddr: 00000000000000b1 cause: 000000000000000d
[ 0.716869] [<ffffffff803fb892>] devm_clk_hw_register+0x62/0xaa
[ 0.723420] [<ffffffff80403412>] mpfs_clk_probe+0x1e0/0x244
[ 0.729592] [<ffffffff80457dea>] platform_probe+0x82/0xa6
[ 0.735581] [<ffffffff8045593c>] call_driver_probe+0x22/0xa4
[ 0.741848] [<ffffffff804557da>] really_probe+0x13a/0x27a
[ 0.747819] [<ffffffff804549f8>] __driver_probe_device+0xc4/0xee
[ 0.754460] [<ffffffff804554d0>] driver_probe_device+0x3c/0x196
[ 0.761013] [<ffffffff804552cc>] __device_attach_driver+0xa2/0x18c
[ 0.767853] [<ffffffff8045284a>] bus_for_each_drv+0x76/0xba
[ 0.774016] [<ffffffff804547ca>] __device_attach+0xa0/0x15a
[ 0.780179] [<ffffffff80454896>] device_initial_probe+0x12/0x1a
[ 0.786732] [<ffffffff804529c6>] bus_probe_device+0x2e/0x7c
[ 0.792895] [<ffffffff8044f290>] device_add+0x21a/0x3aa
[ 0.798693] [<ffffffff805c323a>] of_device_add+0x28/0x32
[ 0.804578] [<ffffffff805c3c54>] of_platform_device_create_pdata+0x88/0xb2
[ 0.812181] [<ffffffff805c3e70>] of_platform_bus_create+0x14a/0x1a8
[ 0.819117] [<ffffffff805c3ea6>] of_platform_bus_create+0x180/0x1a8
[ 0.826053] [<ffffffff805c3f2a>] of_platform_populate+0x5c/0x96
[ 0.832606] [<ffffffff8082bbaa>] of_platform_default_populate_init+0xc2/0xd4
[ 0.840400] [<ffffffff800020e8>] do_one_initcall+0xbc/0x216
[ 0.846563] [<ffffffff8080126c>] do_initcall_level+0x82/0x94
[ 0.852830] [<ffffffff808011ae>] do_initcalls+0x5c/0x98
[ 0.858611] [<ffffffff8080114a>] do_basic_setup+0x20/0x28
[ 0.864583] [<ffffffff808010e0>] kernel_init_freeable+0xba/0x104
[ 0.871223] [<ffffffff80746856>] kernel_init+0x22/0x1ba
[ 0.877013] [<ffffffff80003700>] ret_from_exception+0x0/0xc
[ 0.883218] ---[ end trace 0000000000000000 ]---

At least I had somewhere to start. Since QEMU repros, I continued there with:
$(QEMU)/qemu-system-riscv64 -s -S -M microchip-icicle-kit \
-m 2G -smp 5 \
-kernel $(vmlinux_bin) \
-dtb $(dtb) \
-initrd $(initramfs) \
-display none -serial null \
-serial stdio

Dumping the backtrace at the devm_clk_hw_register() callsites shows that
it passes for:
- the main pll in mpfs_clk_register_mssplls()
- the "cfg" clocks in mpfs_clk_register_cfgs()
- the first 4 periph clocks...

It fails on "clk_periph_timer" - which uses a different parent, that it
tries to find using the macro:
#define PARENT_CLK(PARENT) (&mpfs_cfg_clks[CLK_##PARENT].cfg.hw)

If parent is RTCREF, that becomes: &mpfs_cfg_clks[33].cfg.hw

And yeah, thats well beyond the end of the array!

Booting with gcc, and looking at the clock summary:
mssrefclk 1 1 0 125000000 0 0 50000 Y
clk_rtcref 0 0 0 1000000 0 0 50000 Y
clk_periph_timer 0 0 0 1000000 0 0 50000 N

This is reported correctly, so how this works for either gcc or clang now, or
gcc with the patches applied, I am not really too sure..

On v6.0-rc1, gcc and clang show:
mssrefclk 1 1 0 125000000 0 0 50000 Y
clk_rtcref 0 0 0 1000000 0 0 50000 Y

I guess this may fail gracefully as the pointer is actually null, but
the extra level of indirection added by my patch started tripping up
the error checking. I must just not have noticed because I added RTCREF
in a bit of a hurry trying to fix a bug in time for 5.18:
https://lore.kernel.org/linux-clk/[email protected]/#t

Obviously I've got to fix the bug itself now, but now I am left wondering
about GCC's behaviour rather than clang/llvm's!

Thanks Nathan - if even just for being a rubber duck :)

Conor.

>>
>> Didn't get a chance to look at disassembly etc today, but as I said
>> last night it reproduces with GNU binutils.
>>
>> Thanks,
>> Conor.
>>
>> On another note, brought up our QEMU port's state today so fixing
>> it is now on the good ole, ever expanding todo list :)
>

2022-08-17 16:34:02

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH v2 00/12] PolarFire SoC reset controller & clock cleanups

On Wed, Aug 17, 2022 at 12:17:11PM +0000, [email protected] wrote:
> On 14/08/2022 12:41, [email protected] wrote:
> >>
> >> Doesn't really matter since thats long enough to get past the switch
> >> out of earlycon which is where the clang built kernel dies.
>
> I had forgotten something obvious: keep_bootcon. With that, on the sbi
> console I get a nice:
>
> [ 0.581754] Unable to handle kernel NULL pointer dereference at virtual address 00000000000000b1
> [ 0.591520] Oops [#1]
> [ 0.594045] Modules linked in:
> [ 0.597435] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.0.0-rc1-00011-g8e1459cf4eca #1
> [ 0.606188] Hardware name: Microchip PolarFire-SoC Icicle Kit (DT)
> [ 0.613012] epc : __clk_register+0x4a6/0x85c
> [ 0.617759] ra : __clk_register+0x49e/0x85c
> [ 0.622489] epc : ffffffff803faf7c ra : ffffffff803faf74 sp : ffffffc80400b720
> [ 0.630466] gp : ffffffff810e93f8 tp : ffffffe77fe60000 t0 : ffffffe77ffb3800
> [ 0.638443] t1 : 000000000000000a t2 : ffffffffffffffff s0 : ffffffc80400b7c0
> [ 0.646420] s1 : 0000000000000001 a0 : 0000000000000001 a1 : 0000000000000000
> [ 0.654396] a2 : 0000000000000001 a3 : 0000000000000000 a4 : 0000000000000000
> [ 0.662373] a5 : ffffffff803a5810 a6 : 0000000200000022 a7 : 0000000000000006
> [ 0.670350] s2 : ffffffff81099d48 s3 : ffffffff80d6e28e s4 : 0000000000000028
> [ 0.678327] s5 : ffffffff810ed3c8 s6 : ffffffff810ed3d0 s7 : ffffffe77ffbc100
> [ 0.686304] s8 : ffffffe77ffb1540 s9 : ffffffe77ffb1540 s10: 0000000000000008
> [ 0.694281] s11: 0000000000000000 t3 : 00000000000000c6 t4 : 0000000000000007
> [ 0.702258] t5 : ffffffff810c78c0 t6 : ffffffe77ff88cd0
> [ 0.708125] status: 0000000200000120 badaddr: 00000000000000b1 cause: 000000000000000d
> [ 0.716869] [<ffffffff803fb892>] devm_clk_hw_register+0x62/0xaa
> [ 0.723420] [<ffffffff80403412>] mpfs_clk_probe+0x1e0/0x244
> [ 0.729592] [<ffffffff80457dea>] platform_probe+0x82/0xa6
> [ 0.735581] [<ffffffff8045593c>] call_driver_probe+0x22/0xa4
> [ 0.741848] [<ffffffff804557da>] really_probe+0x13a/0x27a
> [ 0.747819] [<ffffffff804549f8>] __driver_probe_device+0xc4/0xee
> [ 0.754460] [<ffffffff804554d0>] driver_probe_device+0x3c/0x196
> [ 0.761013] [<ffffffff804552cc>] __device_attach_driver+0xa2/0x18c
> [ 0.767853] [<ffffffff8045284a>] bus_for_each_drv+0x76/0xba
> [ 0.774016] [<ffffffff804547ca>] __device_attach+0xa0/0x15a
> [ 0.780179] [<ffffffff80454896>] device_initial_probe+0x12/0x1a
> [ 0.786732] [<ffffffff804529c6>] bus_probe_device+0x2e/0x7c
> [ 0.792895] [<ffffffff8044f290>] device_add+0x21a/0x3aa
> [ 0.798693] [<ffffffff805c323a>] of_device_add+0x28/0x32
> [ 0.804578] [<ffffffff805c3c54>] of_platform_device_create_pdata+0x88/0xb2
> [ 0.812181] [<ffffffff805c3e70>] of_platform_bus_create+0x14a/0x1a8
> [ 0.819117] [<ffffffff805c3ea6>] of_platform_bus_create+0x180/0x1a8
> [ 0.826053] [<ffffffff805c3f2a>] of_platform_populate+0x5c/0x96
> [ 0.832606] [<ffffffff8082bbaa>] of_platform_default_populate_init+0xc2/0xd4
> [ 0.840400] [<ffffffff800020e8>] do_one_initcall+0xbc/0x216
> [ 0.846563] [<ffffffff8080126c>] do_initcall_level+0x82/0x94
> [ 0.852830] [<ffffffff808011ae>] do_initcalls+0x5c/0x98
> [ 0.858611] [<ffffffff8080114a>] do_basic_setup+0x20/0x28
> [ 0.864583] [<ffffffff808010e0>] kernel_init_freeable+0xba/0x104
> [ 0.871223] [<ffffffff80746856>] kernel_init+0x22/0x1ba
> [ 0.877013] [<ffffffff80003700>] ret_from_exception+0x0/0xc
> [ 0.883218] ---[ end trace 0000000000000000 ]---
>
> At least I had somewhere to start. Since QEMU repros, I continued there with:
> $(QEMU)/qemu-system-riscv64 -s -S -M microchip-icicle-kit \
> -m 2G -smp 5 \
> -kernel $(vmlinux_bin) \
> -dtb $(dtb) \
> -initrd $(initramfs) \
> -display none -serial null \
> -serial stdio
>
> Dumping the backtrace at the devm_clk_hw_register() callsites shows that
> it passes for:
> - the main pll in mpfs_clk_register_mssplls()
> - the "cfg" clocks in mpfs_clk_register_cfgs()
> - the first 4 periph clocks...
>
> It fails on "clk_periph_timer" - which uses a different parent, that it
> tries to find using the macro:
> #define PARENT_CLK(PARENT) (&mpfs_cfg_clks[CLK_##PARENT].cfg.hw)
>
> If parent is RTCREF, that becomes: &mpfs_cfg_clks[33].cfg.hw
>
> And yeah, thats well beyond the end of the array!
>
> Booting with gcc, and looking at the clock summary:
> mssrefclk 1 1 0 125000000 0 0 50000 Y
> clk_rtcref 0 0 0 1000000 0 0 50000 Y
> clk_periph_timer 0 0 0 1000000 0 0 50000 N
>
> This is reported correctly, so how this works for either gcc or clang now, or
> gcc with the patches applied, I am not really too sure..
>
> On v6.0-rc1, gcc and clang show:
> mssrefclk 1 1 0 125000000 0 0 50000 Y
> clk_rtcref 0 0 0 1000000 0 0 50000 Y
>
> I guess this may fail gracefully as the pointer is actually null, but
> the extra level of indirection added by my patch started tripping up
> the error checking. I must just not have noticed because I added RTCREF
> in a bit of a hurry trying to fix a bug in time for 5.18:
> https://lore.kernel.org/linux-clk/[email protected]/#t
>
> Obviously I've got to fix the bug itself now, but now I am left wondering
> about GCC's behaviour rather than clang/llvm's!
>
> Thanks Nathan - if even just for being a rubber duck :)

Hey, at least it turned out to be something other than a compiler bug,
those are not very fun :) I am definitely surprised that GCC has no
issues, I wonder if KASAN would have exposed this problem sooner?

Cheers,
Nathan