* [PATCH v5 0/9] Redo PolarFire SoC's mailbox/clock devicestrees and related code
@ 2025-10-13 17:45 Conor Dooley
2025-10-13 17:45 ` [PATCH v5 1/9] dt-bindings: soc: microchip: document the simple-mfd syscon on PolarFire SoC Conor Dooley
` (9 more replies)
0 siblings, 10 replies; 20+ messages in thread
From: Conor Dooley @ 2025-10-13 17:45 UTC (permalink / raw)
To: claudiu.beznea
Cc: conor, Conor Dooley, Daire McNamara, pierre-henry.moussay,
valentina.fernandezalanis, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Philipp Zabel, linux-riscv,
linux-clk, devicetree, linux-kernel
From: Conor Dooley <conor.dooley@microchip.com>
In v5 the only real change is that I removed the attempt at a common
implementation of regmap-based divider/gate clocks. The series hasn't
managed to receive feedback on my approach in 2025, despite sending
several revisions and bumps, and it is blocking support for both new
drivers (gpio interrupt support, pinctrl and hwmon off the top of my
head) and a new platform so I have decided to strip out the attempt at
making something common in exchange for something that can be merged
through the clk-microchip tree without relying on feedback from the
clock maintainers.
Currently the driver uses the common gate and divider clocks, but the
driver used to use its own custom clock types. Reprising this version of
the code allows me to use regmap accessors in the driver without any
wider impact, or attempting to create something that works for any other
user. It has the advantage that it has already been tested in that prior
for, and all that is done to the clock implementations is replacing
readl()s and writel()s with their regmap equivalents.
Hopefully this change has made it possible to merge the series,
Conor.
v5:
- drop mfd patch applied by Lee
- remove attempt at common regmap divider/gate clocks, and replace it
with a return to how the code used to look, before it started using
the non-regmap versions of the common divider/gate, with the
readl()/writel()s replaced by their regmap equivalents.
v4:
- unify both regmap clk implementations under one option
- change map_offset to a u32, after Gabriel pointed out that u8 was
too restrictive.
- remove locking from regmap portion of reset driver, relying on
inherent regmap lock
v3 changes:
- drop simple-mfd (for now) from syscon node
v2 cover letter:
Here's something that I've been mulling over for a while, since I
started to understand how devicetree stuff was "meant" to be done.
There'd been little reason to actually press forward with it, because it
is fairly disruptive. I've finally opted to do it, because a user has
come along with a hwmon driver that needs to access the same register
region as the mailbox and the author is not keen on using the aux bus,
and because I do not want the new pic64gx SoC that's based on PolarFire
SoC to use bindings etc that I know to be incorrect.
Given backwards compatibility needs to be maintained, this patch series
isn't the prettiest thing I have ever written. The reset driver needs to
retain support for the auxiliary bus, which looks a bit mess, but not
much can be done there. The mailbox and clock drivers both have to have
an "old probe" function to handle the old layout. Thankfully in the
clock driver, regmap support can be used to identically
handle both old and new devicetree formats - but using a regmap in the
mailbox driver was only really possible for the new format, so the code
there is unfortunately a bit of an if/else mess that I'm both not proud
of, nor really sure is worth "improving".
The series should be pretty splitable per subsystem, only the dts change
has some sort of dependency, but I'll not be applying that till
everything else is in Linus' tree, so that's not a big deal.
I don't really want this stuff in stable, hence a lack of cc: stable
anywhere here, since what's currently in the tree works fine for the
currently supported hardware.
AFAIK, the only other project affected here is U-Boot, which I have
already modified to support the new format.
I previously submitted this as an RFC, only to Lee and the dt list, in
order to get some feedback on the syscon/mfd bindings:
https://lore.kernel.org/all/20240815-shindig-bunny-fd42792d638a@spud/
I'm not really going to bother with a proper changelog, since that was
submitted with lots of WIP code to get answers to some questions. The
main change was "removing" some of the child nodes of the syscons.
And as a "real" series where discussion lead to me dropping use of the
amlogic clk-regmap support:
https://lore.kernel.org/linux-clk/20241002-private-unequal-33cfa6101338@spud/
As a result of that, I've implemented what I think Stephen was asking
for - but I'm not at all sure that it is..
CC: Conor Dooley <conor.dooley@microchip.com>
CC: Daire McNamara <daire.mcnamara@microchip.com>
CC: pierre-henry.moussay@microchip.com
CC: valentina.fernandezalanis@microchip.com
CC: Michael Turquette <mturquette@baylibre.com>
CC: Stephen Boyd <sboyd@kernel.org>
CC: Rob Herring <robh@kernel.org>
CC: Krzysztof Kozlowski <krzk+dt@kernel.org>
CC: Philipp Zabel <p.zabel@pengutronix.de>
CC: linux-riscv@lists.infradead.org
CC: linux-clk@vger.kernel.org
CC: devicetree@vger.kernel.org
CC: linux-kernel@vger.kernel.org
Conor Dooley (9):
dt-bindings: soc: microchip: document the simple-mfd syscon on
PolarFire SoC
soc: microchip: add mfd drivers for two syscon regions on PolarFire
SoC
reset: mpfs: add non-auxiliary bus probing
dt-bindings: clk: microchip: mpfs: remove first reg region
clk: microchip: mpfs: use regmap for clocks
riscv: dts: microchip: fix mailbox description
riscv: dts: microchip: convert clock and reset to use syscon
MAINTAINERS: add new soc drivers to Microchip RISC-V entry
MAINTAINERS: rename Microchip RISC-V entry
.../bindings/clock/microchip,mpfs-clkcfg.yaml | 36 ++-
.../microchip,mpfs-mss-top-sysreg.yaml | 47 ++++
MAINTAINERS | 4 +-
arch/riscv/boot/dts/microchip/mpfs.dtsi | 34 ++-
drivers/clk/microchip/Kconfig | 2 +
drivers/clk/microchip/clk-mpfs.c | 250 +++++++++++++++---
drivers/reset/reset-mpfs.c | 83 ++++--
drivers/soc/microchip/Kconfig | 13 +
drivers/soc/microchip/Makefile | 1 +
drivers/soc/microchip/mpfs-control-scb.c | 45 ++++
drivers/soc/microchip/mpfs-mss-top-sysreg.c | 48 ++++
11 files changed, 480 insertions(+), 83 deletions(-)
create mode 100644 Documentation/devicetree/bindings/soc/microchip/microchip,mpfs-mss-top-sysreg.yaml
create mode 100644 drivers/soc/microchip/mpfs-control-scb.c
create mode 100644 drivers/soc/microchip/mpfs-mss-top-sysreg.c
--
2.51.0
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v5 1/9] dt-bindings: soc: microchip: document the simple-mfd syscon on PolarFire SoC
2025-10-13 17:45 [PATCH v5 0/9] Redo PolarFire SoC's mailbox/clock devicestrees and related code Conor Dooley
@ 2025-10-13 17:45 ` Conor Dooley
2025-10-13 17:45 ` [PATCH v5 2/9] soc: microchip: add mfd drivers for two syscon regions " Conor Dooley
` (8 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: Conor Dooley @ 2025-10-13 17:45 UTC (permalink / raw)
To: claudiu.beznea
Cc: conor, Conor Dooley, Daire McNamara, pierre-henry.moussay,
valentina.fernandezalanis, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Philipp Zabel, linux-riscv,
linux-clk, devicetree, linux-kernel, Krzysztof Kozlowski
From: Conor Dooley <conor.dooley@microchip.com>
"mss-top-sysreg" contains clocks, pinctrl, resets, an interrupt controller
and more. At this point, only the reset controller child is described as
that's all that is described by the existing bindings.
The clock controller already has a dedicated node, and will retain it as
there are other clock regions, so like the mailbox, a compatible-based
lookup of the syscon is sufficient to keep the clock driver working as
before, so no child is needed. There's also an interrupt multiplexing
service provided by this syscon, for which there is work in progress at
[1].
Link: https://lore.kernel.org/linux-gpio/20240723-uncouple-enforcer-7c48e4a4fefe@wendy/ [1]
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
v3:
- drop simple-mfd at Krzysztof's request since the child nodes do not
yet exist.
v2:
- clean up various minor comments from Rob on mpfs-mss-top-sysreg
- remove mpfs-control-scb from this patch
---
.../microchip,mpfs-mss-top-sysreg.yaml | 47 +++++++++++++++++++
1 file changed, 47 insertions(+)
create mode 100644 Documentation/devicetree/bindings/soc/microchip/microchip,mpfs-mss-top-sysreg.yaml
diff --git a/Documentation/devicetree/bindings/soc/microchip/microchip,mpfs-mss-top-sysreg.yaml b/Documentation/devicetree/bindings/soc/microchip/microchip,mpfs-mss-top-sysreg.yaml
new file mode 100644
index 000000000000..1ab691db8795
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/microchip/microchip,mpfs-mss-top-sysreg.yaml
@@ -0,0 +1,47 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/soc/microchip/microchip,mpfs-mss-top-sysreg.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip PolarFire SoC Microprocessor Subsystem (MSS) sysreg register region
+
+maintainers:
+ - Conor Dooley <conor.dooley@microchip.com>
+
+description:
+ An wide assortment of registers that control elements of the MSS on PolarFire
+ SoC, including pinmuxing, resets and clocks among others.
+
+properties:
+ compatible:
+ items:
+ - const: microchip,mpfs-mss-top-sysreg
+ - const: syscon
+
+ reg:
+ 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/reset IDs.
+ const: 1
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ syscon@20002000 {
+ compatible = "microchip,mpfs-mss-top-sysreg", "syscon";
+ reg = <0x20002000 0x1000>;
+ #reset-cells = <1>;
+ };
+
--
2.51.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v5 2/9] soc: microchip: add mfd drivers for two syscon regions on PolarFire SoC
2025-10-13 17:45 [PATCH v5 0/9] Redo PolarFire SoC's mailbox/clock devicestrees and related code Conor Dooley
2025-10-13 17:45 ` [PATCH v5 1/9] dt-bindings: soc: microchip: document the simple-mfd syscon on PolarFire SoC Conor Dooley
@ 2025-10-13 17:45 ` Conor Dooley
2025-10-23 4:04 ` Claudiu Beznea
2025-10-13 17:45 ` [PATCH v5 3/9] reset: mpfs: add non-auxiliary bus probing Conor Dooley
` (7 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: Conor Dooley @ 2025-10-13 17:45 UTC (permalink / raw)
To: claudiu.beznea
Cc: conor, Conor Dooley, Daire McNamara, pierre-henry.moussay,
valentina.fernandezalanis, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Philipp Zabel, linux-riscv,
linux-clk, devicetree, linux-kernel
From: Conor Dooley <conor.dooley@microchip.com>
The control-scb and mss-top-sysreg regions on PolarFire SoC both fulfill
multiple purposes. The former is used for mailbox functions in addition
to the temperature & voltage sensor while the latter is used for clocks,
resets, interrupt muxing and pinctrl.
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
drivers/soc/microchip/Kconfig | 13 ++++++
drivers/soc/microchip/Makefile | 1 +
drivers/soc/microchip/mpfs-control-scb.c | 45 +++++++++++++++++++
drivers/soc/microchip/mpfs-mss-top-sysreg.c | 48 +++++++++++++++++++++
4 files changed, 107 insertions(+)
create mode 100644 drivers/soc/microchip/mpfs-control-scb.c
create mode 100644 drivers/soc/microchip/mpfs-mss-top-sysreg.c
diff --git a/drivers/soc/microchip/Kconfig b/drivers/soc/microchip/Kconfig
index 19f4b576f822..31d188311e05 100644
--- a/drivers/soc/microchip/Kconfig
+++ b/drivers/soc/microchip/Kconfig
@@ -9,3 +9,16 @@ config POLARFIRE_SOC_SYS_CTRL
module will be called mpfs_system_controller.
If unsure, say N.
+
+config POLARFIRE_SOC_SYSCONS
+ bool "PolarFire SoC (MPFS) syscon drivers"
+ default y
+ depends on ARCH_MICROCHIP
+ select MFD_CORE
+ help
+ These drivers add support for the syscons on PolarFire SoC (MPFS).
+ Without these drivers core parts of the kernel such as clocks
+ and resets will not function correctly.
+
+ If unsure, and on a PolarFire SoC, say y.
+
diff --git a/drivers/soc/microchip/Makefile b/drivers/soc/microchip/Makefile
index 14489919fe4b..1a3a1594b089 100644
--- a/drivers/soc/microchip/Makefile
+++ b/drivers/soc/microchip/Makefile
@@ -1 +1,2 @@
obj-$(CONFIG_POLARFIRE_SOC_SYS_CTRL) += mpfs-sys-controller.o
+obj-$(CONFIG_POLARFIRE_SOC_SYSCONS) += mpfs-control-scb.o mpfs-mss-top-sysreg.o
diff --git a/drivers/soc/microchip/mpfs-control-scb.c b/drivers/soc/microchip/mpfs-control-scb.c
new file mode 100644
index 000000000000..d1a8e79c232e
--- /dev/null
+++ b/drivers/soc/microchip/mpfs-control-scb.c
@@ -0,0 +1,45 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/array_size.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+
+static const struct mfd_cell mpfs_control_scb_devs[] = {
+ { .name = "mpfs-tvs", },
+};
+
+static int mpfs_control_scb_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ int ret;
+
+ ret = mfd_add_devices(dev, PLATFORM_DEVID_NONE, mpfs_control_scb_devs,
+ 1, NULL, 0, NULL);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static const struct of_device_id mpfs_control_scb_of_match[] = {
+ {.compatible = "microchip,mpfs-control-scb", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, mpfs_control_scb_of_match);
+
+static struct platform_driver mpfs_control_scb_driver = {
+ .driver = {
+ .name = "mpfs-control-scb",
+ .of_match_table = mpfs_control_scb_of_match,
+ },
+ .probe = mpfs_control_scb_probe,
+};
+module_platform_driver(mpfs_control_scb_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Conor Dooley <conor.dooley@microchip.com>");
+MODULE_DESCRIPTION("PolarFire SoC control scb driver");
diff --git a/drivers/soc/microchip/mpfs-mss-top-sysreg.c b/drivers/soc/microchip/mpfs-mss-top-sysreg.c
new file mode 100644
index 000000000000..9b2e7b84cdba
--- /dev/null
+++ b/drivers/soc/microchip/mpfs-mss-top-sysreg.c
@@ -0,0 +1,48 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/array_size.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+
+static const struct mfd_cell mpfs_mss_top_sysreg_devs[] = {
+ { .name = "mpfs-reset", },
+};
+
+static int mpfs_mss_top_sysreg_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ int ret;
+
+ ret = mfd_add_devices(dev, PLATFORM_DEVID_NONE, mpfs_mss_top_sysreg_devs,
+ 1, NULL, 0, NULL);
+ if (ret)
+ return ret;
+
+ if (devm_of_platform_populate(dev))
+ dev_err(dev, "Error populating children\n");
+
+ return 0;
+}
+
+static const struct of_device_id mpfs_mss_top_sysreg_of_match[] = {
+ {.compatible = "microchip,mpfs-mss-top-sysreg", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, mpfs_mss_top_sysreg_of_match);
+
+static struct platform_driver mpfs_mss_top_sysreg_driver = {
+ .driver = {
+ .name = "mpfs-mss-top-sysreg",
+ .of_match_table = mpfs_mss_top_sysreg_of_match,
+ },
+ .probe = mpfs_mss_top_sysreg_probe,
+};
+module_platform_driver(mpfs_mss_top_sysreg_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Conor Dooley <conor.dooley@microchip.com>");
+MODULE_DESCRIPTION("PolarFire SoC mss top sysreg driver");
--
2.51.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v5 3/9] reset: mpfs: add non-auxiliary bus probing
2025-10-13 17:45 [PATCH v5 0/9] Redo PolarFire SoC's mailbox/clock devicestrees and related code Conor Dooley
2025-10-13 17:45 ` [PATCH v5 1/9] dt-bindings: soc: microchip: document the simple-mfd syscon on PolarFire SoC Conor Dooley
2025-10-13 17:45 ` [PATCH v5 2/9] soc: microchip: add mfd drivers for two syscon regions " Conor Dooley
@ 2025-10-13 17:45 ` Conor Dooley
2025-10-23 4:06 ` Claudiu Beznea
2025-10-13 17:45 ` [PATCH v5 4/9] dt-bindings: clk: microchip: mpfs: remove first reg region Conor Dooley
` (6 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: Conor Dooley @ 2025-10-13 17:45 UTC (permalink / raw)
To: claudiu.beznea
Cc: conor, Conor Dooley, Daire McNamara, pierre-henry.moussay,
valentina.fernandezalanis, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Philipp Zabel, linux-riscv,
linux-clk, devicetree, linux-kernel
From: Conor Dooley <conor.dooley@microchip.com>
While the auxiliary bus was a nice bandaid, and meant that re-writing
the representation of the clock regions in devicetree was not required,
it has run its course. The "mss_top_sysreg" region that contains the
clock and reset regions, also contains pinctrl and an interrupt
controller, so the time has come rewrite the devicetree and probe the
reset controller from an mfd devicetree node, rather than implement
those drivers using the auxiliary bus. Wanting to avoid propagating this
naive/incorrect description of the hardware to the new pic64gx SoC is a
major motivating factor here.
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
v4:
- Only use driver specific lock for non-regmap writes
v2:
- Implement the request to use regmap_update_bits(). I found that I then
hated the read/write helpers since they were just bloat, so I ripped
them out. I replaced the regular spin_lock_irqsave() stuff with a
guard(spinlock_irqsave), since that's a simpler way of handling the two
different paths through such a trivial pair of functions.
---
drivers/reset/reset-mpfs.c | 83 ++++++++++++++++++++++++++++++--------
1 file changed, 66 insertions(+), 17 deletions(-)
diff --git a/drivers/reset/reset-mpfs.c b/drivers/reset/reset-mpfs.c
index f6fa10e03ea8..8e5ed4deecf3 100644
--- a/drivers/reset/reset-mpfs.c
+++ b/drivers/reset/reset-mpfs.c
@@ -7,13 +7,16 @@
*
*/
#include <linux/auxiliary_bus.h>
+#include <linux/cleanup.h>
#include <linux/delay.h>
#include <linux/io.h>
+#include <linux/mfd/syscon.h>
#include <linux/module.h>
#include <linux/of.h>
#include <linux/platform_device.h>
-#include <linux/slab.h>
+#include <linux/regmap.h>
#include <linux/reset-controller.h>
+#include <linux/slab.h>
#include <dt-bindings/clock/microchip,mpfs-clock.h>
#include <soc/microchip/mpfs.h>
@@ -27,11 +30,14 @@
#define MPFS_SLEEP_MIN_US 100
#define MPFS_SLEEP_MAX_US 200
+#define REG_SUBBLK_RESET_CR 0x88u
+
/* block concurrent access to the soft reset register */
static DEFINE_SPINLOCK(mpfs_reset_lock);
struct mpfs_reset {
void __iomem *base;
+ struct regmap *regmap;
struct reset_controller_dev rcdev;
};
@@ -46,41 +52,50 @@ static inline struct mpfs_reset *to_mpfs_reset(struct reset_controller_dev *rcde
static int mpfs_assert(struct reset_controller_dev *rcdev, unsigned long id)
{
struct mpfs_reset *rst = to_mpfs_reset(rcdev);
- unsigned long flags;
u32 reg;
- spin_lock_irqsave(&mpfs_reset_lock, flags);
+ if (rst->regmap) {
+ regmap_update_bits(rst->regmap, REG_SUBBLK_RESET_CR, BIT(id), BIT(id));
+ return 0;
+ }
+
+ guard(spinlock_irqsave)(&mpfs_reset_lock);
reg = readl(rst->base);
reg |= BIT(id);
writel(reg, rst->base);
- spin_unlock_irqrestore(&mpfs_reset_lock, flags);
-
return 0;
}
static int mpfs_deassert(struct reset_controller_dev *rcdev, unsigned long id)
{
struct mpfs_reset *rst = to_mpfs_reset(rcdev);
- unsigned long flags;
u32 reg;
- spin_lock_irqsave(&mpfs_reset_lock, flags);
+ if (rst->regmap) {
+ regmap_update_bits(rst->regmap, REG_SUBBLK_RESET_CR, BIT(id), 0);
+ return 0;
+ }
+
+ guard(spinlock_irqsave)(&mpfs_reset_lock);
reg = readl(rst->base);
reg &= ~BIT(id);
writel(reg, rst->base);
- spin_unlock_irqrestore(&mpfs_reset_lock, flags);
-
return 0;
}
static int mpfs_status(struct reset_controller_dev *rcdev, unsigned long id)
{
struct mpfs_reset *rst = to_mpfs_reset(rcdev);
- u32 reg = readl(rst->base);
+ u32 reg;
+
+ if (rst->regmap)
+ regmap_read(rst->regmap, REG_SUBBLK_RESET_CR, ®);
+ else
+ reg = readl(rst->base);
/*
* It is safe to return here as MPFS_NUM_RESETS makes sure the sign bit
@@ -130,11 +145,45 @@ static int mpfs_reset_xlate(struct reset_controller_dev *rcdev,
return index - MPFS_PERIPH_OFFSET;
}
-static int mpfs_reset_probe(struct auxiliary_device *adev,
- const struct auxiliary_device_id *id)
+static int mpfs_reset_mfd_probe(struct platform_device *pdev)
{
- struct device *dev = &adev->dev;
struct reset_controller_dev *rcdev;
+ struct device *dev = &pdev->dev;
+ struct mpfs_reset *rst;
+
+ rst = devm_kzalloc(dev, sizeof(*rst), GFP_KERNEL);
+ if (!rst)
+ return -ENOMEM;
+
+ rcdev = &rst->rcdev;
+ rcdev->dev = dev;
+ rcdev->ops = &mpfs_reset_ops;
+
+ rcdev->of_node = pdev->dev.parent->of_node;
+ rcdev->of_reset_n_cells = 1;
+ rcdev->of_xlate = mpfs_reset_xlate;
+ rcdev->nr_resets = MPFS_NUM_RESETS;
+
+ rst->regmap = device_node_to_regmap(pdev->dev.parent->of_node);
+ if (IS_ERR(rst->regmap))
+ dev_err_probe(dev, PTR_ERR(rst->regmap), "Failed to find syscon regmap\n");
+
+ return devm_reset_controller_register(dev, rcdev);
+}
+
+static struct platform_driver mpfs_reset_mfd_driver = {
+ .probe = mpfs_reset_mfd_probe,
+ .driver = {
+ .name = "mpfs-reset",
+ },
+};
+module_platform_driver(mpfs_reset_mfd_driver);
+
+static int mpfs_reset_adev_probe(struct auxiliary_device *adev,
+ const struct auxiliary_device_id *id)
+{
+ struct reset_controller_dev *rcdev;
+ struct device *dev = &adev->dev;
struct mpfs_reset *rst;
rst = devm_kzalloc(dev, sizeof(*rst), GFP_KERNEL);
@@ -145,8 +194,8 @@ static int mpfs_reset_probe(struct auxiliary_device *adev,
rcdev = &rst->rcdev;
rcdev->dev = dev;
- rcdev->dev->parent = dev->parent;
rcdev->ops = &mpfs_reset_ops;
+
rcdev->of_node = dev->parent->of_node;
rcdev->of_reset_n_cells = 1;
rcdev->of_xlate = mpfs_reset_xlate;
@@ -176,12 +225,12 @@ static const struct auxiliary_device_id mpfs_reset_ids[] = {
};
MODULE_DEVICE_TABLE(auxiliary, mpfs_reset_ids);
-static struct auxiliary_driver mpfs_reset_driver = {
- .probe = mpfs_reset_probe,
+static struct auxiliary_driver mpfs_reset_aux_driver = {
+ .probe = mpfs_reset_adev_probe,
.id_table = mpfs_reset_ids,
};
-module_auxiliary_driver(mpfs_reset_driver);
+module_auxiliary_driver(mpfs_reset_aux_driver);
MODULE_DESCRIPTION("Microchip PolarFire SoC Reset Driver");
MODULE_AUTHOR("Conor Dooley <conor.dooley@microchip.com>");
--
2.51.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v5 4/9] dt-bindings: clk: microchip: mpfs: remove first reg region
2025-10-13 17:45 [PATCH v5 0/9] Redo PolarFire SoC's mailbox/clock devicestrees and related code Conor Dooley
` (2 preceding siblings ...)
2025-10-13 17:45 ` [PATCH v5 3/9] reset: mpfs: add non-auxiliary bus probing Conor Dooley
@ 2025-10-13 17:45 ` Conor Dooley
2025-10-13 17:45 ` [PATCH v5 5/9] clk: microchip: mpfs: use regmap for clocks Conor Dooley
` (5 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: Conor Dooley @ 2025-10-13 17:45 UTC (permalink / raw)
To: claudiu.beznea
Cc: conor, Conor Dooley, Daire McNamara, pierre-henry.moussay,
valentina.fernandezalanis, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Philipp Zabel, linux-riscv,
linux-clk, devicetree, linux-kernel
From: Conor Dooley <conor.dooley@microchip.com>
The first reg region in this binding is not exclusively for clocks, as
evidenced by the dual role of this device as a reset controller at
present. The first region is however better described by a simple-mfd
syscon, but this would have require a significant re-write of the
devicetree for the platform, so the easy way out was chosen when reset
support was first introduced. The region doesn't just contain clock and
reset registers, it also contains pinctrl and interrupt controller
functionality, so drop the region from the clock binding so that it can
be described instead by a simple-mfd syscon rather than propagate this
incorrect description of the hardware to the new pic64gx SoC.
Acked-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
.../bindings/clock/microchip,mpfs-clkcfg.yaml | 36 +++++++++++--------
1 file changed, 22 insertions(+), 14 deletions(-)
diff --git a/Documentation/devicetree/bindings/clock/microchip,mpfs-clkcfg.yaml b/Documentation/devicetree/bindings/clock/microchip,mpfs-clkcfg.yaml
index e4e1c31267d2..ee4f31596d97 100644
--- a/Documentation/devicetree/bindings/clock/microchip,mpfs-clkcfg.yaml
+++ b/Documentation/devicetree/bindings/clock/microchip,mpfs-clkcfg.yaml
@@ -22,16 +22,23 @@ properties:
const: microchip,mpfs-clkcfg
reg:
- items:
- - description: |
- clock config registers:
- These registers contain enable, reset & divider tables for the, cpu,
- axi, ahb and rtc/mtimer reference clocks as well as enable and reset
- for the peripheral clocks.
- - description: |
- mss pll dri registers:
- Block of registers responsible for dynamic reconfiguration of the mss
- pll
+ oneOf:
+ - items:
+ - description: |
+ clock config registers:
+ These registers contain enable, reset & divider tables for the, cpu,
+ axi, ahb and rtc/mtimer reference clocks as well as enable and reset
+ for the peripheral clocks.
+ - description: |
+ mss pll dri registers:
+ Block of registers responsible for dynamic reconfiguration of the mss
+ pll
+ deprecated: true
+ - items:
+ - description: |
+ mss pll dri registers:
+ Block of registers responsible for dynamic reconfiguration of the mss
+ pll
clocks:
maxItems: 1
@@ -69,11 +76,12 @@ examples:
- |
#include <dt-bindings/clock/microchip,mpfs-clock.h>
soc {
- #address-cells = <2>;
- #size-cells = <2>;
- clkcfg: clock-controller@20002000 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+
+ clkcfg: clock-controller@3E001000 {
compatible = "microchip,mpfs-clkcfg";
- reg = <0x0 0x20002000 0x0 0x1000>, <0x0 0x3E001000 0x0 0x1000>;
+ reg = <0x3E001000 0x1000>;
clocks = <&ref>;
#clock-cells = <1>;
};
--
2.51.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v5 5/9] clk: microchip: mpfs: use regmap for clocks
2025-10-13 17:45 [PATCH v5 0/9] Redo PolarFire SoC's mailbox/clock devicestrees and related code Conor Dooley
` (3 preceding siblings ...)
2025-10-13 17:45 ` [PATCH v5 4/9] dt-bindings: clk: microchip: mpfs: remove first reg region Conor Dooley
@ 2025-10-13 17:45 ` Conor Dooley
2025-10-23 4:06 ` Claudiu Beznea
2025-10-13 17:45 ` [PATCH v5 6/9] riscv: dts: microchip: fix mailbox description Conor Dooley
` (4 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: Conor Dooley @ 2025-10-13 17:45 UTC (permalink / raw)
To: claudiu.beznea
Cc: conor, Conor Dooley, Daire McNamara, pierre-henry.moussay,
valentina.fernandezalanis, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Philipp Zabel, linux-riscv,
linux-clk, devicetree, linux-kernel
From: Conor Dooley <conor.dooley@microchip.com>
Convert the PolarFire SoC clock driver to use regmaps instead of iomem
addresses as a preparatory work for supporting the new binding for this
device that will only provide the second of the two register regions, and
will require the use of syscon regmap to access the "cfg" and "periph"
clocks currently supported by the driver.
This is effectively a revert of commit 4da2404bb003 ("clk: microchip:
mpfs: convert cfg_clk to clk_divider") and commit d815569783e6 ("clk:
microchip: mpfs: convert periph_clk to clk_gate") as it resurrects the
ops structures removed in those commits, with the readl()s and
writel()s replaced by regmap_read()s and regmap_writes()s.
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
| 2 +
| 250 ++++++++++++++++++++++++++-----
2 files changed, 211 insertions(+), 41 deletions(-)
--git a/drivers/clk/microchip/Kconfig b/drivers/clk/microchip/Kconfig
index 0724ce65898f..1b9e43eb5497 100644
--- a/drivers/clk/microchip/Kconfig
+++ b/drivers/clk/microchip/Kconfig
@@ -7,6 +7,8 @@ config MCHP_CLK_MPFS
bool "Clk driver for PolarFire SoC"
depends on ARCH_MICROCHIP_POLARFIRE || COMPILE_TEST
default ARCH_MICROCHIP_POLARFIRE
+ depends on MFD_SYSCON
select AUXILIARY_BUS
+ select REGMAP_MMIO
help
Supports Clock Configuration for PolarFire SoC
--git a/drivers/clk/microchip/clk-mpfs.c b/drivers/clk/microchip/clk-mpfs.c
index c22632a7439c..e3362be9b266 100644
--- a/drivers/clk/microchip/clk-mpfs.c
+++ b/drivers/clk/microchip/clk-mpfs.c
@@ -6,8 +6,10 @@
*/
#include <linux/clk-provider.h>
#include <linux/io.h>
+#include <linux/mfd/syscon.h>
#include <linux/module.h>
#include <linux/platform_device.h>
+#include <linux/regmap.h>
#include <dt-bindings/clock/microchip,mpfs-clock.h>
#include <soc/microchip/mpfs.h>
@@ -30,6 +32,14 @@
#define MSSPLL_POSTDIV_WIDTH 0x07u
#define MSSPLL_FIXED_DIV 4u
+static const struct regmap_config mpfs_clk_regmap_config = {
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ .val_format_endian = REGMAP_ENDIAN_LITTLE,
+ .max_register = REG_SUBBLK_CLOCK_CR,
+};
+
/*
* This clock ID is defined here, rather than the binding headers, as it is an
* internal clock only, and therefore has no consumers in other peripheral
@@ -39,6 +49,7 @@
struct mpfs_clock_data {
struct device *dev;
+ struct regmap *regmap;
void __iomem *base;
void __iomem *msspll_base;
struct clk_hw_onecell_data hw_data;
@@ -67,18 +78,37 @@ struct mpfs_msspll_out_hw_clock {
#define to_mpfs_msspll_out_clk(_hw) container_of(_hw, struct mpfs_msspll_out_hw_clock, hw)
+struct mpfs_cfg_clock {
+ struct regmap *map;
+ const struct clk_div_table *table;
+ u8 map_offset;
+ u8 shift;
+ u8 width;
+ u8 flags;
+};
+
struct mpfs_cfg_hw_clock {
- struct clk_divider cfg;
- struct clk_init_data init;
+ struct mpfs_cfg_clock cfg;
+ struct clk_hw hw;
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 {
+ struct regmap *map;
+ u8 map_offset;
+ u8 shift;
};
struct mpfs_periph_hw_clock {
- struct clk_gate periph;
+ struct mpfs_periph_clock periph;
+ struct clk_hw hw;
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.
@@ -219,16 +249,66 @@ static int mpfs_clk_register_msspll_outs(struct device *dev,
/*
* "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;
-#define CLK_CFG(_id, _name, _parent, _shift, _width, _table, _flags, _offset) { \
- .id = _id, \
- .cfg.shift = _shift, \
- .cfg.width = _width, \
- .cfg.table = _table, \
- .reg_offset = _offset, \
- .cfg.flags = _flags, \
- .cfg.hw.init = CLK_HW_INIT(_name, _parent, &clk_divider_ops, 0), \
- .cfg.lock = &mpfs_clk_lock, \
+ regmap_read(cfg->map, cfg->map_offset, &val);
+ val >>= 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);
+
+ regmap_read(cfg->map, cfg->map_offset, &val);
+ val &= ~(clk_div_mask(cfg->width) << cfg_hw->cfg.shift);
+ val |= divider_setting << cfg->shift;
+ regmap_write(cfg->map, cfg->map_offset, val);
+
+ 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, \
+ .cfg.width = _width, \
+ .cfg.table = _table, \
+ .cfg.map_offset = _offset, \
+ .cfg.flags = _flags, \
+ .hw.init = CLK_HW_INIT(_name, _parent, &mpfs_clk_cfg_ops, 0), \
}
#define CLK_CPU_OFFSET 0u
@@ -248,10 +328,10 @@ static struct mpfs_cfg_hw_clock mpfs_cfg_clks[] = {
.cfg.shift = 0,
.cfg.width = 12,
.cfg.table = mpfs_div_rtcref_table,
- .reg_offset = REG_RTC_CLOCK_CR,
+ .cfg.map_offset = REG_RTC_CLOCK_CR,
.cfg.flags = CLK_DIVIDER_ONE_BASED,
- .cfg.hw.init =
- CLK_HW_INIT_PARENTS_DATA("clk_rtcref", mpfs_ext_ref, &clk_divider_ops, 0),
+ .hw.init =
+ CLK_HW_INIT_PARENTS_DATA("clk_rtcref", mpfs_ext_ref, &mpfs_clk_cfg_ops, 0),
}
};
@@ -264,14 +344,14 @@ static int mpfs_clk_register_cfgs(struct device *dev, struct mpfs_cfg_hw_clock *
for (i = 0; i < num_clks; i++) {
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->cfg.hw);
+ cfg_hw->cfg.map = data->regmap;
+ ret = devm_clk_hw_register(dev, &cfg_hw->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->cfg.hw;
+ data->hw_data.hws[id] = &cfg_hw->hw;
}
return 0;
@@ -281,15 +361,67 @@ static int mpfs_clk_register_cfgs(struct device *dev, struct mpfs_cfg_hw_clock *
* peripheral clocks - devices connected to axi or ahb buses.
*/
-#define CLK_PERIPH(_id, _name, _parent, _shift, _flags) { \
- .id = _id, \
- .periph.bit_idx = _shift, \
- .periph.hw.init = CLK_HW_INIT_HW(_name, _parent, &clk_gate_ops, \
- _flags), \
- .periph.lock = &mpfs_clk_lock, \
+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 val;
+ unsigned long flags;
+
+ spin_lock_irqsave(&mpfs_clk_lock, flags);
+
+ regmap_read(periph->map, periph->map_offset, &val);
+ val |= 1u << periph->shift;
+ regmap_write(periph->map, periph->map_offset, val);
+
+ spin_unlock_irqrestore(&mpfs_clk_lock, flags);
+
+ return 0;
}
-#define PARENT_CLK(PARENT) (&mpfs_cfg_clks[CLK_##PARENT##_OFFSET].cfg.hw)
+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 val;
+ unsigned long flags;
+
+ spin_lock_irqsave(&mpfs_clk_lock, flags);
+
+ regmap_read(periph->map, periph->map_offset, &val);
+ val &= ~(1u << periph->shift);
+ regmap_write(periph->map, periph->map_offset, val);
+
+ 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 val;
+
+ regmap_read(periph->map, periph->map_offset, &val);
+ if (val & (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.map_offset = REG_SUBBLK_CLOCK_CR, \
+ .periph.shift = _shift, \
+ .hw.init = CLK_HW_INIT_HW(_name, _parent, &mpfs_periph_clk_ops, _flags), \
+}
+
+#define PARENT_CLK(PARENT) (&mpfs_cfg_clks[CLK_##PARENT##_OFFSET].hw)
/*
* Critical clocks:
@@ -346,19 +478,60 @@ static int mpfs_clk_register_periphs(struct device *dev, struct mpfs_periph_hw_c
for (i = 0; i < num_clks; i++) {
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->periph.hw);
+ periph_hw->periph.map = data->regmap;
+ ret = devm_clk_hw_register(dev, &periph_hw->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->periph.hw;
+ data->hw_data.hws[id] = &periph_hw->hw;
}
return 0;
}
+static inline int mpfs_clk_syscon_probe(struct mpfs_clock_data *clk_data,
+ struct platform_device *pdev)
+{
+ clk_data->regmap = syscon_regmap_lookup_by_compatible("microchip,mpfs-mss-top-sysreg");
+ if (IS_ERR(clk_data->regmap))
+ return PTR_ERR(clk_data->regmap);
+
+ clk_data->msspll_base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(clk_data->msspll_base))
+ return PTR_ERR(clk_data->msspll_base);
+
+ return 0;
+}
+
+static inline int mpfs_clk_old_format_probe(struct mpfs_clock_data *clk_data,
+ struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ int ret;
+
+ dev_warn(&pdev->dev, "falling back to old devicetree format");
+
+ clk_data->base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(clk_data->base))
+ return PTR_ERR(clk_data->base);
+
+ clk_data->msspll_base = devm_platform_ioremap_resource(pdev, 1);
+ if (IS_ERR(clk_data->msspll_base))
+ return PTR_ERR(clk_data->msspll_base);
+
+ clk_data->regmap = devm_regmap_init_mmio(dev, clk_data->base, &mpfs_clk_regmap_config);
+ if (IS_ERR(clk_data->regmap))
+ return PTR_ERR(clk_data->regmap);
+
+ ret = mpfs_reset_controller_register(dev, clk_data->base + REG_SUBBLK_RESET_CR);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
static int mpfs_clk_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -374,13 +547,12 @@ static int mpfs_clk_probe(struct platform_device *pdev)
if (!clk_data)
return -ENOMEM;
- clk_data->base = devm_platform_ioremap_resource(pdev, 0);
- if (IS_ERR(clk_data->base))
- return PTR_ERR(clk_data->base);
-
- clk_data->msspll_base = devm_platform_ioremap_resource(pdev, 1);
- if (IS_ERR(clk_data->msspll_base))
- return PTR_ERR(clk_data->msspll_base);
+ ret = mpfs_clk_syscon_probe(clk_data, pdev);
+ if (ret) {
+ ret = mpfs_clk_old_format_probe(clk_data, pdev);
+ if (ret)
+ return ret;
+ }
clk_data->hw_data.num = num_clks;
clk_data->dev = dev;
@@ -406,11 +578,7 @@ static int mpfs_clk_probe(struct platform_device *pdev)
if (ret)
return ret;
- ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, &clk_data->hw_data);
- if (ret)
- return ret;
-
- return mpfs_reset_controller_register(dev, clk_data->base + REG_SUBBLK_RESET_CR);
+ return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, &clk_data->hw_data);
}
static const struct of_device_id mpfs_clk_of_match_table[] = {
--
2.51.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v5 6/9] riscv: dts: microchip: fix mailbox description
2025-10-13 17:45 [PATCH v5 0/9] Redo PolarFire SoC's mailbox/clock devicestrees and related code Conor Dooley
` (4 preceding siblings ...)
2025-10-13 17:45 ` [PATCH v5 5/9] clk: microchip: mpfs: use regmap for clocks Conor Dooley
@ 2025-10-13 17:45 ` Conor Dooley
2025-10-13 17:45 ` [PATCH v5 7/9] riscv: dts: microchip: convert clock and reset to use syscon Conor Dooley
` (3 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: Conor Dooley @ 2025-10-13 17:45 UTC (permalink / raw)
To: claudiu.beznea
Cc: conor, Conor Dooley, Daire McNamara, pierre-henry.moussay,
valentina.fernandezalanis, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Philipp Zabel, linux-riscv,
linux-clk, devicetree, linux-kernel
From: Conor Dooley <conor.dooley@microchip.com>
When the binding for the mailbox on PolarFire SoC was originally
written, and later modified, mistakes were made - and the precise
nature of the later modification should have been a giveaway, but alas
I was naive at the time.
A more correct modelling of the hardware is to use two syscons and have
a single reg entry for the mailbox, containing the mailbox region. The
two syscons contain the general control/status registers for the mailbox
and the interrupt related registers respectively. The reason for two
syscons is that the same mailbox is present on the non-SoC version of
the FPGA, which has no interrupt controller, and the shared part of the
rtl was unchanged between devices.
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
arch/riscv/boot/dts/microchip/mpfs.dtsi | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/arch/riscv/boot/dts/microchip/mpfs.dtsi b/arch/riscv/boot/dts/microchip/mpfs.dtsi
index 9883ca3554c5..f9d6bf08e717 100644
--- a/arch/riscv/boot/dts/microchip/mpfs.dtsi
+++ b/arch/riscv/boot/dts/microchip/mpfs.dtsi
@@ -259,6 +259,11 @@ clkcfg: clkcfg@20002000 {
#reset-cells = <1>;
};
+ sysreg_scb: syscon@20003000 {
+ compatible = "microchip,mpfs-sysreg-scb", "syscon";
+ reg = <0x0 0x20003000 0x0 0x1000>;
+ };
+
ccc_se: clock-controller@38010000 {
compatible = "microchip,mpfs-ccc";
reg = <0x0 0x38010000 0x0 0x1000>, <0x0 0x38020000 0x0 0x1000>,
@@ -521,10 +526,14 @@ usb: usb@20201000 {
status = "disabled";
};
- mbox: mailbox@37020000 {
+ control_scb: syscon@37020000 {
+ compatible = "microchip,mpfs-control-scb", "syscon";
+ reg = <0x0 0x37020000 0x0 0x100>;
+ };
+
+ mbox: mailbox@37020800 {
compatible = "microchip,mpfs-mailbox";
- reg = <0x0 0x37020000 0x0 0x58>, <0x0 0x2000318C 0x0 0x40>,
- <0x0 0x37020800 0x0 0x100>;
+ reg = <0x0 0x37020800 0x0 0x1000>;
interrupt-parent = <&plic>;
interrupts = <96>;
#mbox-cells = <1>;
--
2.51.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v5 7/9] riscv: dts: microchip: convert clock and reset to use syscon
2025-10-13 17:45 [PATCH v5 0/9] Redo PolarFire SoC's mailbox/clock devicestrees and related code Conor Dooley
` (5 preceding siblings ...)
2025-10-13 17:45 ` [PATCH v5 6/9] riscv: dts: microchip: fix mailbox description Conor Dooley
@ 2025-10-13 17:45 ` Conor Dooley
2025-10-13 17:45 ` [PATCH v5 8/9] MAINTAINERS: add new soc drivers to Microchip RISC-V entry Conor Dooley
` (2 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: Conor Dooley @ 2025-10-13 17:45 UTC (permalink / raw)
To: claudiu.beznea
Cc: conor, Conor Dooley, Daire McNamara, pierre-henry.moussay,
valentina.fernandezalanis, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Philipp Zabel, linux-riscv,
linux-clk, devicetree, linux-kernel
From: Conor Dooley <conor.dooley@microchip.com>
The "subblock" clocks and reset registers on PolarFire SoC are located
in the mss-top-sysreg region, alongside pinctrl and interrupt control
functionality. Re-write the devicetree to describe the sys explicitly,
as its own node, rather than as a region of the clock node.
Correspondingly, the phandles to the reset controller must be updated to
the new provider. The drivers will continue to support the old way of
doing things.
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
arch/riscv/boot/dts/microchip/mpfs.dtsi | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/arch/riscv/boot/dts/microchip/mpfs.dtsi b/arch/riscv/boot/dts/microchip/mpfs.dtsi
index f9d6bf08e717..5c2963e269b8 100644
--- a/arch/riscv/boot/dts/microchip/mpfs.dtsi
+++ b/arch/riscv/boot/dts/microchip/mpfs.dtsi
@@ -251,11 +251,9 @@ pdma: dma-controller@3000000 {
#dma-cells = <1>;
};
- clkcfg: clkcfg@20002000 {
- compatible = "microchip,mpfs-clkcfg";
- reg = <0x0 0x20002000 0x0 0x1000>, <0x0 0x3E001000 0x0 0x1000>;
- clocks = <&refclk>;
- #clock-cells = <1>;
+ mss_top_sysreg: syscon@20002000 {
+ compatible = "microchip,mpfs-mss-top-sysreg", "syscon", "simple-mfd";
+ reg = <0x0 0x20002000 0x0 0x1000>;
#reset-cells = <1>;
};
@@ -452,7 +450,7 @@ 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>;
+ resets = <&mss_top_sysreg CLK_MAC0>;
status = "disabled";
};
@@ -466,7 +464,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>;
+ resets = <&mss_top_sysreg CLK_MAC1>;
status = "disabled";
};
@@ -550,5 +548,12 @@ syscontroller_qspi: spi@37020100 {
clocks = <&scbclk>;
status = "disabled";
};
+
+ clkcfg: clkcfg@3e001000 {
+ compatible = "microchip,mpfs-clkcfg";
+ reg = <0x0 0x3e001000 0x0 0x1000>;
+ clocks = <&refclk>;
+ #clock-cells = <1>;
+ };
};
};
--
2.51.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v5 8/9] MAINTAINERS: add new soc drivers to Microchip RISC-V entry
2025-10-13 17:45 [PATCH v5 0/9] Redo PolarFire SoC's mailbox/clock devicestrees and related code Conor Dooley
` (6 preceding siblings ...)
2025-10-13 17:45 ` [PATCH v5 7/9] riscv: dts: microchip: convert clock and reset to use syscon Conor Dooley
@ 2025-10-13 17:45 ` Conor Dooley
2025-10-13 17:45 ` [PATCH v5 9/9] MAINTAINERS: rename " Conor Dooley
2025-10-21 13:31 ` (subset) [PATCH v5 0/9] Redo PolarFire SoC's mailbox/clock devicestrees and related code Conor Dooley
9 siblings, 0 replies; 20+ messages in thread
From: Conor Dooley @ 2025-10-13 17:45 UTC (permalink / raw)
To: claudiu.beznea
Cc: conor, Conor Dooley, Daire McNamara, pierre-henry.moussay,
valentina.fernandezalanis, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Philipp Zabel, linux-riscv,
linux-clk, devicetree, linux-kernel
From: Conor Dooley <conor.dooley@microchip.com>
Add the two new syscon drivers to the RISC-V entry for Microchip
platforms.
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
MAINTAINERS | 2 ++
1 file changed, 2 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 46126ce2f968..a28740a7d87a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -22105,6 +22105,8 @@ F: drivers/pci/controller/plda/pcie-microchip-host.c
F: drivers/pwm/pwm-microchip-core.c
F: drivers/reset/reset-mpfs.c
F: drivers/rtc/rtc-mpfs.c
+F: drivers/soc/microchip/mpfs-control-scb.c
+F: drivers/soc/microchip/mpfs-mss-top-sysreg.c
F: drivers/soc/microchip/mpfs-sys-controller.c
F: drivers/spi/spi-microchip-core-qspi.c
F: drivers/spi/spi-microchip-core.c
--
2.51.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v5 9/9] MAINTAINERS: rename Microchip RISC-V entry
2025-10-13 17:45 [PATCH v5 0/9] Redo PolarFire SoC's mailbox/clock devicestrees and related code Conor Dooley
` (7 preceding siblings ...)
2025-10-13 17:45 ` [PATCH v5 8/9] MAINTAINERS: add new soc drivers to Microchip RISC-V entry Conor Dooley
@ 2025-10-13 17:45 ` Conor Dooley
2025-10-21 13:31 ` (subset) [PATCH v5 0/9] Redo PolarFire SoC's mailbox/clock devicestrees and related code Conor Dooley
9 siblings, 0 replies; 20+ messages in thread
From: Conor Dooley @ 2025-10-13 17:45 UTC (permalink / raw)
To: claudiu.beznea
Cc: conor, Conor Dooley, Daire McNamara, pierre-henry.moussay,
valentina.fernandezalanis, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Philipp Zabel, linux-riscv,
linux-clk, devicetree, linux-kernel
From: Conor Dooley <conor.dooley@microchip.com>
There's now non-FPGA RISC-V SoCs from Microchip, so rename the entry
to reflect that.
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
MAINTAINERS | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index a28740a7d87a..24efae3df425 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -22079,7 +22079,7 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/iommu/linux.git
F: Documentation/devicetree/bindings/iommu/riscv,iommu.yaml
F: drivers/iommu/riscv/
-RISC-V MICROCHIP FPGA SUPPORT
+RISC-V MICROCHIP SUPPORT
M: Conor Dooley <conor.dooley@microchip.com>
M: Daire McNamara <daire.mcnamara@microchip.com>
L: linux-riscv@lists.infradead.org
--
2.51.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: (subset) [PATCH v5 0/9] Redo PolarFire SoC's mailbox/clock devicestrees and related code
2025-10-13 17:45 [PATCH v5 0/9] Redo PolarFire SoC's mailbox/clock devicestrees and related code Conor Dooley
` (8 preceding siblings ...)
2025-10-13 17:45 ` [PATCH v5 9/9] MAINTAINERS: rename " Conor Dooley
@ 2025-10-21 13:31 ` Conor Dooley
9 siblings, 0 replies; 20+ messages in thread
From: Conor Dooley @ 2025-10-21 13:31 UTC (permalink / raw)
To: claudiu.beznea, Conor Dooley
Cc: Conor Dooley, Daire McNamara, pierre-henry.moussay,
valentina.fernandezalanis, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Philipp Zabel, linux-riscv,
linux-clk, devicetree, linux-kernel
From: Conor Dooley <conor.dooley@microchip.com>
On Mon, 13 Oct 2025 18:45:32 +0100, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
>
> In v5 the only real change is that I removed the attempt at a common
> implementation of regmap-based divider/gate clocks. The series hasn't
> managed to receive feedback on my approach in 2025, despite sending
> several revisions and bumps, and it is blocking support for both new
> drivers (gpio interrupt support, pinctrl and hwmon off the top of my
> head) and a new platform so I have decided to strip out the attempt at
> making something common in exchange for something that can be merged
> through the clk-microchip tree without relying on feedback from the
> clock maintainers.
>
> [...]
Applied to riscv-soc-drivers-for-next, because the pinctrl series that
I am working on needs them.
[1/9] dt-bindings: soc: microchip: document the simple-mfd syscon on PolarFire SoC
https://git.kernel.org/conor/c/feaa716adc51
[2/9] soc: microchip: add mfd drivers for two syscon regions on PolarFire SoC
https://git.kernel.org/conor/c/5b59e62532fc
Thanks,
Conor.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 2/9] soc: microchip: add mfd drivers for two syscon regions on PolarFire SoC
2025-10-13 17:45 ` [PATCH v5 2/9] soc: microchip: add mfd drivers for two syscon regions " Conor Dooley
@ 2025-10-23 4:04 ` Claudiu Beznea
2025-10-23 9:12 ` Conor Dooley
2025-10-23 10:15 ` Conor Dooley
0 siblings, 2 replies; 20+ messages in thread
From: Claudiu Beznea @ 2025-10-23 4:04 UTC (permalink / raw)
To: Conor Dooley
Cc: Conor Dooley, Daire McNamara, pierre-henry.moussay,
valentina.fernandezalanis, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Philipp Zabel, linux-riscv,
linux-clk, devicetree, linux-kernel
Hi, Conor,
On 10/13/25 20:45, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
>
> The control-scb and mss-top-sysreg regions on PolarFire SoC both fulfill
> multiple purposes. The former is used for mailbox functions in addition
> to the temperature & voltage sensor while the latter is used for clocks,
> resets, interrupt muxing and pinctrl.
>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> drivers/soc/microchip/Kconfig | 13 ++++++
> drivers/soc/microchip/Makefile | 1 +
> drivers/soc/microchip/mpfs-control-scb.c | 45 +++++++++++++++++++
> drivers/soc/microchip/mpfs-mss-top-sysreg.c | 48 +++++++++++++++++++++
> 4 files changed, 107 insertions(+)
> create mode 100644 drivers/soc/microchip/mpfs-control-scb.c
> create mode 100644 drivers/soc/microchip/mpfs-mss-top-sysreg.c
>
> diff --git a/drivers/soc/microchip/Kconfig b/drivers/soc/microchip/Kconfig
> index 19f4b576f822..31d188311e05 100644
> --- a/drivers/soc/microchip/Kconfig
> +++ b/drivers/soc/microchip/Kconfig
> @@ -9,3 +9,16 @@ config POLARFIRE_SOC_SYS_CTRL
> module will be called mpfs_system_controller.
>
> If unsure, say N.
> +
> +config POLARFIRE_SOC_SYSCONS
> + bool "PolarFire SoC (MPFS) syscon drivers"
> + default y
> + depends on ARCH_MICROCHIP
> + select MFD_CORE
> + help
> + These drivers add support for the syscons on PolarFire SoC (MPFS).
> + Without these drivers core parts of the kernel such as clocks
> + and resets will not function correctly.
> +
> + If unsure, and on a PolarFire SoC, say y.
> +
This empty line could be dropped.
> diff --git a/drivers/soc/microchip/Makefile b/drivers/soc/microchip/Makefile
> index 14489919fe4b..1a3a1594b089 100644
> --- a/drivers/soc/microchip/Makefile
> +++ b/drivers/soc/microchip/Makefile
> @@ -1 +1,2 @@
> obj-$(CONFIG_POLARFIRE_SOC_SYS_CTRL) += mpfs-sys-controller.o
> +obj-$(CONFIG_POLARFIRE_SOC_SYSCONS) += mpfs-control-scb.o mpfs-mss-top-sysreg.o
> diff --git a/drivers/soc/microchip/mpfs-control-scb.c b/drivers/soc/microchip/mpfs-control-scb.c
> new file mode 100644
> index 000000000000..d1a8e79c232e
> --- /dev/null
> +++ b/drivers/soc/microchip/mpfs-control-scb.c
> @@ -0,0 +1,45 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/array_size.h>
Looks like this one can be dropped or maybe you want to use
ARRAY_SIZE(mpfs_control_scb_devs) as 4th argument of mfd_add_devices().
> +#include <linux/of.h>
> +#include <linux/of_address.h>
Looks like this one can be dropped.
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/of_platform.h>
Same with this one?
> +#include <linux/platform_device.h>
> +
> +static const struct mfd_cell mpfs_control_scb_devs[] = {
> + { .name = "mpfs-tvs", },
I think you can use:
MFD_CELL_NAME("mpfs-tvs")
> +};
> +
> +static int mpfs_control_scb_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + int ret;
> +
> + ret = mfd_add_devices(dev, PLATFORM_DEVID_NONE, mpfs_control_scb_devs,
> + 1, NULL, 0, NULL);
> + if (ret)
> + return ret;
> +
> + return 0;
You can use directly:
return mfd_add_device(...);
> +}
> +
> +static const struct of_device_id mpfs_control_scb_of_match[] = {
> + {.compatible = "microchip,mpfs-control-scb", },
This looks un-documented.
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, mpfs_control_scb_of_match);
> +
> +static struct platform_driver mpfs_control_scb_driver = {
> + .driver = {
> + .name = "mpfs-control-scb",
> + .of_match_table = mpfs_control_scb_of_match,
> + },
> + .probe = mpfs_control_scb_probe,
> +};
> +module_platform_driver(mpfs_control_scb_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Conor Dooley <conor.dooley@microchip.com>");
> +MODULE_DESCRIPTION("PolarFire SoC control scb driver");
> diff --git a/drivers/soc/microchip/mpfs-mss-top-sysreg.c b/drivers/soc/microchip/mpfs-mss-top-sysreg.c
> new file mode 100644
> index 000000000000..9b2e7b84cdba
> --- /dev/null
> +++ b/drivers/soc/microchip/mpfs-mss-top-sysreg.c
> @@ -0,0 +1,48 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/array_size.h>
Looks like this one can be dropped or maybe you want to use
ARRAY_SIZE(mpfs_mss_top_sysreg_devs) as 4th argument of mfd_add_devices()
> +#include <linux/of.h>
> +#include <linux/of_address.h>
Unused?
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/of_platform.h>
Unused?
> +#include <linux/platform_device.h>
> +
> +static const struct mfd_cell mpfs_mss_top_sysreg_devs[] = {
> + { .name = "mpfs-reset", },
MFD_CELL_NAME() ?
> +};
> +
> +static int mpfs_mss_top_sysreg_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + int ret;
> +
> + ret = mfd_add_devices(dev, PLATFORM_DEVID_NONE, mpfs_mss_top_sysreg_devs,
> + 1, NULL, 0, NULL);
> + if (ret)
> + return ret;
> +
> + if (devm_of_platform_populate(dev))
> + dev_err(dev, "Error populating children\n");
Is it OK return 0 above if there are failures here?
> +
> + return 0;
> +}
> +
> +static const struct of_device_id mpfs_mss_top_sysreg_of_match[] = {
> + {.compatible = "microchip,mpfs-mss-top-sysreg", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, mpfs_mss_top_sysreg_of_match);
> +
> +static struct platform_driver mpfs_mss_top_sysreg_driver = {
> + .driver = {
> + .name = "mpfs-mss-top-sysreg",
> + .of_match_table = mpfs_mss_top_sysreg_of_match,
> + },
> + .probe = mpfs_mss_top_sysreg_probe,
> +};
> +module_platform_driver(mpfs_mss_top_sysreg_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Conor Dooley <conor.dooley@microchip.com>");
> +MODULE_DESCRIPTION("PolarFire SoC mss top sysreg driver");
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 5/9] clk: microchip: mpfs: use regmap for clocks
2025-10-13 17:45 ` [PATCH v5 5/9] clk: microchip: mpfs: use regmap for clocks Conor Dooley
@ 2025-10-23 4:06 ` Claudiu Beznea
2025-10-23 14:42 ` Brian Masney
2025-10-24 10:20 ` Conor Dooley
0 siblings, 2 replies; 20+ messages in thread
From: Claudiu Beznea @ 2025-10-23 4:06 UTC (permalink / raw)
To: Conor Dooley
Cc: Conor Dooley, Daire McNamara, pierre-henry.moussay,
valentina.fernandezalanis, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Philipp Zabel, linux-riscv,
linux-clk, devicetree, linux-kernel, Brian Masney
Hi, Conor,
On 10/13/25 20:45, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
>
> Convert the PolarFire SoC clock driver to use regmaps instead of iomem
> addresses as a preparatory work for supporting the new binding for this
> device that will only provide the second of the two register regions, and
> will require the use of syscon regmap to access the "cfg" and "periph"
> clocks currently supported by the driver.
>
> This is effectively a revert of commit 4da2404bb003 ("clk: microchip:
> mpfs: convert cfg_clk to clk_divider") and commit d815569783e6 ("clk:
> microchip: mpfs: convert periph_clk to clk_gate") as it resurrects the
> ops structures removed in those commits, with the readl()s and
> writel()s replaced by regmap_read()s and regmap_writes()s.
>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> drivers/clk/microchip/Kconfig | 2 +
> drivers/clk/microchip/clk-mpfs.c | 250 ++++++++++++++++++++++++++-----
> 2 files changed, 211 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/clk/microchip/Kconfig b/drivers/clk/microchip/Kconfig
> index 0724ce65898f..1b9e43eb5497 100644
> --- a/drivers/clk/microchip/Kconfig
> +++ b/drivers/clk/microchip/Kconfig
> @@ -7,6 +7,8 @@ config MCHP_CLK_MPFS
> bool "Clk driver for PolarFire SoC"
> depends on ARCH_MICROCHIP_POLARFIRE || COMPILE_TEST
> default ARCH_MICROCHIP_POLARFIRE
> + depends on MFD_SYSCON
> select AUXILIARY_BUS
> + select REGMAP_MMIO
> help
> Supports Clock Configuration for PolarFire SoC
> diff --git a/drivers/clk/microchip/clk-mpfs.c b/drivers/clk/microchip/clk-mpfs.c
> index c22632a7439c..e3362be9b266 100644
> --- a/drivers/clk/microchip/clk-mpfs.c
> +++ b/drivers/clk/microchip/clk-mpfs.c
> @@ -6,8 +6,10 @@
> */
> #include <linux/clk-provider.h>
> #include <linux/io.h>
> +#include <linux/mfd/syscon.h>
> #include <linux/module.h>
> #include <linux/platform_device.h>
> +#include <linux/regmap.h>
> #include <dt-bindings/clock/microchip,mpfs-clock.h>
> #include <soc/microchip/mpfs.h>
>
> @@ -30,6 +32,14 @@
> #define MSSPLL_POSTDIV_WIDTH 0x07u
> #define MSSPLL_FIXED_DIV 4u
>
> +static const struct regmap_config mpfs_clk_regmap_config = {
> + .reg_bits = 32,
> + .reg_stride = 4,
> + .val_bits = 32,
> + .val_format_endian = REGMAP_ENDIAN_LITTLE,
> + .max_register = REG_SUBBLK_CLOCK_CR,
> +};
> +
> /*
> * This clock ID is defined here, rather than the binding headers, as it is an
> * internal clock only, and therefore has no consumers in other peripheral
> @@ -39,6 +49,7 @@
>
> struct mpfs_clock_data {
> struct device *dev;
> + struct regmap *regmap;
> void __iomem *base;
> void __iomem *msspll_base;
> struct clk_hw_onecell_data hw_data;
> @@ -67,18 +78,37 @@ struct mpfs_msspll_out_hw_clock {
>
> #define to_mpfs_msspll_out_clk(_hw) container_of(_hw, struct mpfs_msspll_out_hw_clock, hw)
>
> +struct mpfs_cfg_clock {
> + struct regmap *map;
> + const struct clk_div_table *table;
> + u8 map_offset;
> + u8 shift;
> + u8 width;
> + u8 flags;
> +};
> +
> struct mpfs_cfg_hw_clock {
> - struct clk_divider cfg;
> - struct clk_init_data init;
> + struct mpfs_cfg_clock cfg;
> + struct clk_hw hw;
This one could be moved first as its members are all pointers, to avoid
padding, if any.
> 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 {
> + struct regmap *map;
> + u8 map_offset;
> + u8 shift;
> };
>
> struct mpfs_periph_hw_clock {
> - struct clk_gate periph;
> + struct mpfs_periph_clock periph;
> + struct clk_hw hw;
> 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.
> @@ -219,16 +249,66 @@ static int mpfs_clk_register_msspll_outs(struct device *dev,
> /*
> * "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;
>
> -#define CLK_CFG(_id, _name, _parent, _shift, _width, _table, _flags, _offset) { \
> - .id = _id, \
> - .cfg.shift = _shift, \
> - .cfg.width = _width, \
> - .cfg.table = _table, \
> - .reg_offset = _offset, \
> - .cfg.flags = _flags, \
> - .cfg.hw.init = CLK_HW_INIT(_name, _parent, &clk_divider_ops, 0), \
> - .cfg.lock = &mpfs_clk_lock, \
> + regmap_read(cfg->map, cfg->map_offset, &val);
> + val >>= 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;
This could be moved near flags to keep the reverse christmas tree order as
in the rest of this patch.
> +
> + 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);
As spin locking is introduced in this file by this patch, you can go
directly w/ cleanup helpers for locking.
> +
> + regmap_read(cfg->map, cfg->map_offset, &val);
> + val &= ~(clk_div_mask(cfg->width) << cfg_hw->cfg.shift);
Why cfg_hw->cfg.shift here --------------------^ but cfg->shift on the next
line?
> + val |= divider_setting << cfg->shift;
> + regmap_write(cfg->map, cfg->map_offset, val);
Can't the regmap_read() + updated + regmap_write() be replaced by
regmap_update_bits() ?
> +
> + 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,
.round_rate is now considered deprecated. Brian (added to cc) tried to
remove all its users.
.determine_rate() should be used now.
> + .set_rate = mpfs_cfg_clk_set_rate,
> +};
> +
> +#define CLK_CFG(_id, _name, _parent, _shift, _width, _table, _flags, _offset) { \
> + .id = _id, \
> + .cfg.shift = _shift, \
> + .cfg.width = _width, \
> + .cfg.table = _table, \
> + .cfg.map_offset = _offset, \
> + .cfg.flags = _flags, \
> + .hw.init = CLK_HW_INIT(_name, _parent, &mpfs_clk_cfg_ops, 0), \
> }
>
> #define CLK_CPU_OFFSET 0u
> @@ -248,10 +328,10 @@ static struct mpfs_cfg_hw_clock mpfs_cfg_clks[] = {
> .cfg.shift = 0,
> .cfg.width = 12,
> .cfg.table = mpfs_div_rtcref_table,
> - .reg_offset = REG_RTC_CLOCK_CR,
> + .cfg.map_offset = REG_RTC_CLOCK_CR,
> .cfg.flags = CLK_DIVIDER_ONE_BASED,
> - .cfg.hw.init =
> - CLK_HW_INIT_PARENTS_DATA("clk_rtcref", mpfs_ext_ref, &clk_divider_ops, 0),
> + .hw.init =
> + CLK_HW_INIT_PARENTS_DATA("clk_rtcref", mpfs_ext_ref, &mpfs_clk_cfg_ops, 0),
> }
> };
>
> @@ -264,14 +344,14 @@ static int mpfs_clk_register_cfgs(struct device *dev, struct mpfs_cfg_hw_clock *
> for (i = 0; i < num_clks; i++) {
> 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->cfg.hw);
> + cfg_hw->cfg.map = data->regmap;
> + ret = devm_clk_hw_register(dev, &cfg_hw->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->cfg.hw;
> + data->hw_data.hws[id] = &cfg_hw->hw;
> }
>
> return 0;
> @@ -281,15 +361,67 @@ static int mpfs_clk_register_cfgs(struct device *dev, struct mpfs_cfg_hw_clock *
> * peripheral clocks - devices connected to axi or ahb buses.
> */
>
> -#define CLK_PERIPH(_id, _name, _parent, _shift, _flags) { \
> - .id = _id, \
> - .periph.bit_idx = _shift, \
> - .periph.hw.init = CLK_HW_INIT_HW(_name, _parent, &clk_gate_ops, \
> - _flags), \
> - .periph.lock = &mpfs_clk_lock, \
> +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 val;
> + unsigned long flags;
This could be moved above to have reverse chritmass tree order, or dropped
if using cleanup helpers for locking.
> +
> + spin_lock_irqsave(&mpfs_clk_lock, flags);
> +
> + regmap_read(periph->map, periph->map_offset, &val);
> + val |= 1u << periph->shift;
Maybe BIT(periph->shift) is simpler here?
> + regmap_write(periph->map, periph->map_offset, val);
Looks like this could be replaced by regmap_update_bits()
> +
> + spin_unlock_irqrestore(&mpfs_clk_lock, flags);
> +
> + return 0;
> }
>
> -#define PARENT_CLK(PARENT) (&mpfs_cfg_clks[CLK_##PARENT##_OFFSET].cfg.hw)
> +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 val;
> + unsigned long flags;
Same here.
> +
> + spin_lock_irqsave(&mpfs_clk_lock, flags);
> +
> + regmap_read(periph->map, periph->map_offset, &val);
> + val &= ~(1u << periph->shift);
Maybe BIT(periph->shift) ?
> + regmap_write(periph->map, periph->map_offset, val);
regmap_update_bits() ?
> +
> + 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 val;
> +
> + regmap_read(periph->map, periph->map_offset, &val);
> + if (val & (1u << periph->shift))
> + return 1;
> +
> + return 0;
Personal preference: this could be replaced by:
return !!(val & (1u << periph->shift));
or:
return !!(val & BIT(periph->shift));
> +}
> +
> +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.map_offset = REG_SUBBLK_CLOCK_CR, \
> + .periph.shift = _shift, \
> + .hw.init = CLK_HW_INIT_HW(_name, _parent, &mpfs_periph_clk_ops, _flags), \
> +}
> +
> +#define PARENT_CLK(PARENT) (&mpfs_cfg_clks[CLK_##PARENT##_OFFSET].hw)
>
> /*
> * Critical clocks:
> @@ -346,19 +478,60 @@ static int mpfs_clk_register_periphs(struct device *dev, struct mpfs_periph_hw_c
> for (i = 0; i < num_clks; i++) {
> 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->periph.hw);
> + periph_hw->periph.map = data->regmap;
> + ret = devm_clk_hw_register(dev, &periph_hw->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->periph.hw;
> + data->hw_data.hws[id] = &periph_hw->hw;
> }
>
> return 0;
> }
>
> +static inline int mpfs_clk_syscon_probe(struct mpfs_clock_data *clk_data,
> + struct platform_device *pdev)
> +{
> + clk_data->regmap = syscon_regmap_lookup_by_compatible("microchip,mpfs-mss-top-sysreg");
> + if (IS_ERR(clk_data->regmap))
> + return PTR_ERR(clk_data->regmap);
> +
> + clk_data->msspll_base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(clk_data->msspll_base))
> + return PTR_ERR(clk_data->msspll_base);
> +
> + return 0;
> +}
> +
> +static inline int mpfs_clk_old_format_probe(struct mpfs_clock_data *clk_data,
> + struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + int ret;
> +
> + dev_warn(&pdev->dev, "falling back to old devicetree format");
> +
> + clk_data->base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(clk_data->base))
> + return PTR_ERR(clk_data->base);
> +
> + clk_data->msspll_base = devm_platform_ioremap_resource(pdev, 1);
> + if (IS_ERR(clk_data->msspll_base))
> + return PTR_ERR(clk_data->msspll_base);
> +
> + clk_data->regmap = devm_regmap_init_mmio(dev, clk_data->base, &mpfs_clk_regmap_config);
> + if (IS_ERR(clk_data->regmap))
> + return PTR_ERR(clk_data->regmap);
> +
> + ret = mpfs_reset_controller_register(dev, clk_data->base + REG_SUBBLK_RESET_CR);
> + if (ret)
> + return ret;
> +
> + return 0;
The last lines here could be replaced by:
return mpfs_reset_controller_register();
> +}
> +
> static int mpfs_clk_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> @@ -374,13 +547,12 @@ static int mpfs_clk_probe(struct platform_device *pdev)
> if (!clk_data)
> return -ENOMEM;
>
> - clk_data->base = devm_platform_ioremap_resource(pdev, 0);
> - if (IS_ERR(clk_data->base))
> - return PTR_ERR(clk_data->base);
> -
> - clk_data->msspll_base = devm_platform_ioremap_resource(pdev, 1);
> - if (IS_ERR(clk_data->msspll_base))
> - return PTR_ERR(clk_data->msspll_base);
> + ret = mpfs_clk_syscon_probe(clk_data, pdev);
> + if (ret) {
> + ret = mpfs_clk_old_format_probe(clk_data, pdev);
> + if (ret)
> + return ret;
> + }
>
> clk_data->hw_data.num = num_clks;
> clk_data->dev = dev;
> @@ -406,11 +578,7 @@ static int mpfs_clk_probe(struct platform_device *pdev)
> if (ret)
> return ret;
>
> - ret = devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, &clk_data->hw_data);
> - if (ret)
> - return ret;
> -
> - return mpfs_reset_controller_register(dev, clk_data->base + REG_SUBBLK_RESET_CR);
> + return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, &clk_data->hw_data);
> }
>
> static const struct of_device_id mpfs_clk_of_match_table[] = {
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 3/9] reset: mpfs: add non-auxiliary bus probing
2025-10-13 17:45 ` [PATCH v5 3/9] reset: mpfs: add non-auxiliary bus probing Conor Dooley
@ 2025-10-23 4:06 ` Claudiu Beznea
2025-10-24 10:07 ` Conor Dooley
0 siblings, 1 reply; 20+ messages in thread
From: Claudiu Beznea @ 2025-10-23 4:06 UTC (permalink / raw)
To: Conor Dooley
Cc: Conor Dooley, Daire McNamara, pierre-henry.moussay,
valentina.fernandezalanis, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Philipp Zabel, linux-riscv,
linux-clk, devicetree, linux-kernel
Hi, Conor,
On 10/13/25 20:45, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
>
> While the auxiliary bus was a nice bandaid, and meant that re-writing
> the representation of the clock regions in devicetree was not required,
> it has run its course. The "mss_top_sysreg" region that contains the
> clock and reset regions, also contains pinctrl and an interrupt
> controller, so the time has come rewrite the devicetree and probe the
> reset controller from an mfd devicetree node, rather than implement
> those drivers using the auxiliary bus. Wanting to avoid propagating this
> naive/incorrect description of the hardware to the new pic64gx SoC is a
> major motivating factor here.
>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> v4:
> - Only use driver specific lock for non-regmap writes
>
> v2:
> - Implement the request to use regmap_update_bits(). I found that I then
> hated the read/write helpers since they were just bloat, so I ripped
> them out. I replaced the regular spin_lock_irqsave() stuff with a
> guard(spinlock_irqsave), since that's a simpler way of handling the two
> different paths through such a trivial pair of functions.
> ---
> drivers/reset/reset-mpfs.c | 83 ++++++++++++++++++++++++++++++--------
> 1 file changed, 66 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/reset/reset-mpfs.c b/drivers/reset/reset-mpfs.c
> index f6fa10e03ea8..8e5ed4deecf3 100644
> --- a/drivers/reset/reset-mpfs.c
> +++ b/drivers/reset/reset-mpfs.c
> @@ -7,13 +7,16 @@
> *
> */
> #include <linux/auxiliary_bus.h>
> +#include <linux/cleanup.h>
> #include <linux/delay.h>
> #include <linux/io.h>
> +#include <linux/mfd/syscon.h>
Should you add a depends on MFD_SYSCON ?
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/platform_device.h>
> -#include <linux/slab.h>
> +#include <linux/regmap.h>
> #include <linux/reset-controller.h>
> +#include <linux/slab.h>
> #include <dt-bindings/clock/microchip,mpfs-clock.h>
> #include <soc/microchip/mpfs.h>
>
> @@ -27,11 +30,14 @@
> #define MPFS_SLEEP_MIN_US 100
> #define MPFS_SLEEP_MAX_US 200
>
> +#define REG_SUBBLK_RESET_CR 0x88u
> +
> /* block concurrent access to the soft reset register */
> static DEFINE_SPINLOCK(mpfs_reset_lock);
>
> struct mpfs_reset {
> void __iomem *base;
> + struct regmap *regmap;
> struct reset_controller_dev rcdev;
> };
>
> @@ -46,41 +52,50 @@ static inline struct mpfs_reset *to_mpfs_reset(struct reset_controller_dev *rcde
> static int mpfs_assert(struct reset_controller_dev *rcdev, unsigned long id)
> {
> struct mpfs_reset *rst = to_mpfs_reset(rcdev);
> - unsigned long flags;
> u32 reg;
>
> - spin_lock_irqsave(&mpfs_reset_lock, flags);
> + if (rst->regmap) {
> + regmap_update_bits(rst->regmap, REG_SUBBLK_RESET_CR, BIT(id), BIT(id));
> + return 0;
You can:
return regmap_update_bits();
> + }
> +
> + guard(spinlock_irqsave)(&mpfs_reset_lock);
>
> reg = readl(rst->base);
> reg |= BIT(id);
> writel(reg, rst->base);
>
> - spin_unlock_irqrestore(&mpfs_reset_lock, flags);
> -
> return 0;
> }
>
> static int mpfs_deassert(struct reset_controller_dev *rcdev, unsigned long id)
> {
> struct mpfs_reset *rst = to_mpfs_reset(rcdev);
> - unsigned long flags;
> u32 reg;
>
> - spin_lock_irqsave(&mpfs_reset_lock, flags);
> + if (rst->regmap) {
> + regmap_update_bits(rst->regmap, REG_SUBBLK_RESET_CR, BIT(id), 0);
> + return 0;
> + }
> +
> + guard(spinlock_irqsave)(&mpfs_reset_lock);
>
> reg = readl(rst->base);
> reg &= ~BIT(id);
> writel(reg, rst->base);
>
> - spin_unlock_irqrestore(&mpfs_reset_lock, flags);
> -
> return 0;
> }
>
> static int mpfs_status(struct reset_controller_dev *rcdev, unsigned long id)
> {
> struct mpfs_reset *rst = to_mpfs_reset(rcdev);
> - u32 reg = readl(rst->base);
> + u32 reg;
> +
> + if (rst->regmap)
> + regmap_read(rst->regmap, REG_SUBBLK_RESET_CR, ®);
> + else
> + reg = readl(rst->base);
>
> /*
> * It is safe to return here as MPFS_NUM_RESETS makes sure the sign bit
> @@ -130,11 +145,45 @@ static int mpfs_reset_xlate(struct reset_controller_dev *rcdev,
> return index - MPFS_PERIPH_OFFSET;
> }
>
> -static int mpfs_reset_probe(struct auxiliary_device *adev,
> - const struct auxiliary_device_id *id)
> +static int mpfs_reset_mfd_probe(struct platform_device *pdev)
> {
> - struct device *dev = &adev->dev;
> struct reset_controller_dev *rcdev;
> + struct device *dev = &pdev->dev;
> + struct mpfs_reset *rst;
> +
> + rst = devm_kzalloc(dev, sizeof(*rst), GFP_KERNEL);
> + if (!rst)
> + return -ENOMEM;
> +
> + rcdev = &rst->rcdev;
> + rcdev->dev = dev;
> + rcdev->ops = &mpfs_reset_ops;
> +
> + rcdev->of_node = pdev->dev.parent->of_node;
> + rcdev->of_reset_n_cells = 1;
> + rcdev->of_xlate = mpfs_reset_xlate;
> + rcdev->nr_resets = MPFS_NUM_RESETS;
> +
> + rst->regmap = device_node_to_regmap(pdev->dev.parent->of_node);
> + if (IS_ERR(rst->regmap))
> + dev_err_probe(dev, PTR_ERR(rst->regmap), "Failed to find syscon regmap\n");
> +
> + return devm_reset_controller_register(dev, rcdev);
> +}
> +
> +static struct platform_driver mpfs_reset_mfd_driver = {
> + .probe = mpfs_reset_mfd_probe,
> + .driver = {
> + .name = "mpfs-reset",
> + },
> +};
> +module_platform_driver(mpfs_reset_mfd_driver);
> +
> +static int mpfs_reset_adev_probe(struct auxiliary_device *adev,
> + const struct auxiliary_device_id *id)
> +{
> + struct reset_controller_dev *rcdev;
> + struct device *dev = &adev->dev;
> struct mpfs_reset *rst;
>
> rst = devm_kzalloc(dev, sizeof(*rst), GFP_KERNEL);
> @@ -145,8 +194,8 @@ static int mpfs_reset_probe(struct auxiliary_device *adev,
>
> rcdev = &rst->rcdev;
> rcdev->dev = dev;
> - rcdev->dev->parent = dev->parent;
> rcdev->ops = &mpfs_reset_ops;
> +
> rcdev->of_node = dev->parent->of_node;
> rcdev->of_reset_n_cells = 1;
> rcdev->of_xlate = mpfs_reset_xlate;
> @@ -176,12 +225,12 @@ static const struct auxiliary_device_id mpfs_reset_ids[] = {
> };
> MODULE_DEVICE_TABLE(auxiliary, mpfs_reset_ids);
>
> -static struct auxiliary_driver mpfs_reset_driver = {
> - .probe = mpfs_reset_probe,
> +static struct auxiliary_driver mpfs_reset_aux_driver = {
> + .probe = mpfs_reset_adev_probe,
> .id_table = mpfs_reset_ids,
> };
>
> -module_auxiliary_driver(mpfs_reset_driver);
> +module_auxiliary_driver(mpfs_reset_aux_driver);
>
> MODULE_DESCRIPTION("Microchip PolarFire SoC Reset Driver");
> MODULE_AUTHOR("Conor Dooley <conor.dooley@microchip.com>");
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 2/9] soc: microchip: add mfd drivers for two syscon regions on PolarFire SoC
2025-10-23 4:04 ` Claudiu Beznea
@ 2025-10-23 9:12 ` Conor Dooley
2025-10-23 10:15 ` Conor Dooley
1 sibling, 0 replies; 20+ messages in thread
From: Conor Dooley @ 2025-10-23 9:12 UTC (permalink / raw)
To: Claudiu Beznea
Cc: Conor Dooley, Daire McNamara, pierre-henry.moussay,
valentina.fernandezalanis, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Philipp Zabel, linux-riscv,
linux-clk, devicetree, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 547 bytes --]
On Thu, Oct 23, 2025 at 07:04:33AM +0300, Claudiu Beznea wrote:
> On 10/13/25 20:45, Conor Dooley wrote:
> > +}
> > +
> > +static const struct of_device_id mpfs_control_scb_of_match[] = {
> > + {.compatible = "microchip,mpfs-control-scb", },
>
> This looks un-documented.
It is actually documented, this stuff all took so long to get done that
parts got applied piecemeal along the way, including any of the MFD
bits.
I'll fix these things up separately, cos I already applied the first two
patches here and just squash it in.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 2/9] soc: microchip: add mfd drivers for two syscon regions on PolarFire SoC
2025-10-23 4:04 ` Claudiu Beznea
2025-10-23 9:12 ` Conor Dooley
@ 2025-10-23 10:15 ` Conor Dooley
1 sibling, 0 replies; 20+ messages in thread
From: Conor Dooley @ 2025-10-23 10:15 UTC (permalink / raw)
To: Claudiu Beznea
Cc: Conor Dooley, Daire McNamara, pierre-henry.moussay,
valentina.fernandezalanis, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Philipp Zabel, linux-riscv,
linux-clk, devicetree, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1485 bytes --]
On Thu, Oct 23, 2025 at 07:04:33AM +0300, Claudiu Beznea wrote:
> On 10/13/25 20:45, Conor Dooley wrote:
> > +++ b/drivers/soc/microchip/mpfs-mss-top-sysreg.c
> > @@ -0,0 +1,48 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +#include <linux/array_size.h>
>
> Looks like this one can be dropped or maybe you want to use
> ARRAY_SIZE(mpfs_mss_top_sysreg_devs) as 4th argument of mfd_add_devices()
>
> > +#include <linux/of.h>
> > +#include <linux/of_address.h>
>
> Unused?
>
> > +#include <linux/mfd/core.h>
> > +#include <linux/mfd/syscon.h>
> > +#include <linux/of_platform.h>
>
> Unused?
This one is where devm_of_platform_populate comes from.
>
> > +#include <linux/platform_device.h>
> > +
> > +static const struct mfd_cell mpfs_mss_top_sysreg_devs[] = {
> > + { .name = "mpfs-reset", },
>
> MFD_CELL_NAME() ?
>
> > +};
> > +
> > +static int mpfs_mss_top_sysreg_probe(struct platform_device *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + int ret;
> > +
> > + ret = mfd_add_devices(dev, PLATFORM_DEVID_NONE, mpfs_mss_top_sysreg_devs,
> > + 1, NULL, 0, NULL);
> > + if (ret)
> > + return ret;
> > +
> > + if (devm_of_platform_populate(dev))
> > + dev_err(dev, "Error populating children\n");
>
> Is it OK return 0 above if there are failures here?
I think my rationale was that the mfd devices would be able to keep
working, but I think ultimately it doesn't matter much and I'll change
it.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 5/9] clk: microchip: mpfs: use regmap for clocks
2025-10-23 4:06 ` Claudiu Beznea
@ 2025-10-23 14:42 ` Brian Masney
2025-10-24 10:20 ` Conor Dooley
1 sibling, 0 replies; 20+ messages in thread
From: Brian Masney @ 2025-10-23 14:42 UTC (permalink / raw)
To: Claudiu Beznea
Cc: Conor Dooley, Conor Dooley, Daire McNamara, pierre-henry.moussay,
valentina.fernandezalanis, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Philipp Zabel, linux-riscv,
linux-clk, devicetree, linux-kernel
On Thu, Oct 23, 2025 at 07:06:01AM +0300, Claudiu Beznea wrote:
> On 10/13/25 20:45, Conor Dooley wrote:
> > +static const struct clk_ops mpfs_clk_cfg_ops = {
> > + .recalc_rate = mpfs_cfg_clk_recalc_rate,
> > + .round_rate = mpfs_cfg_clk_round_rate,
> .round_rate is now considered deprecated. Brian (added to cc) tried to
> remove all its users.
>
> .determine_rate() should be used now.
Thanks for referencing that. I'm still waiting for a few remaining
patches outside of drivers/clk to be merged. Hopefully I'm able to get
these merged for v6.19. Then I'll be able to post the patches to
actually remove round_rate from the clk core for v6.20.
Brian
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 3/9] reset: mpfs: add non-auxiliary bus probing
2025-10-23 4:06 ` Claudiu Beznea
@ 2025-10-24 10:07 ` Conor Dooley
0 siblings, 0 replies; 20+ messages in thread
From: Conor Dooley @ 2025-10-24 10:07 UTC (permalink / raw)
To: Claudiu Beznea
Cc: Conor Dooley, Daire McNamara, pierre-henry.moussay,
valentina.fernandezalanis, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Philipp Zabel, linux-riscv,
linux-clk, devicetree, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 2135 bytes --]
On Thu, Oct 23, 2025 at 07:06:48AM +0300, Claudiu Beznea wrote:
> Hi, Conor,
>
> On 10/13/25 20:45, Conor Dooley wrote:
> > From: Conor Dooley <conor.dooley@microchip.com>
> >
> > While the auxiliary bus was a nice bandaid, and meant that re-writing
> > the representation of the clock regions in devicetree was not required,
> > it has run its course. The "mss_top_sysreg" region that contains the
> > clock and reset regions, also contains pinctrl and an interrupt
> > controller, so the time has come rewrite the devicetree and probe the
> > reset controller from an mfd devicetree node, rather than implement
> > those drivers using the auxiliary bus. Wanting to avoid propagating this
> > naive/incorrect description of the hardware to the new pic64gx SoC is a
> > major motivating factor here.
> >
> > Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> > ---
> > v4:
> > - Only use driver specific lock for non-regmap writes
> >
> > v2:
> > - Implement the request to use regmap_update_bits(). I found that I then
> > hated the read/write helpers since they were just bloat, so I ripped
> > them out. I replaced the regular spin_lock_irqsave() stuff with a
> > guard(spinlock_irqsave), since that's a simpler way of handling the two
> > different paths through such a trivial pair of functions.
> > ---
> > drivers/reset/reset-mpfs.c | 83 ++++++++++++++++++++++++++++++--------
> > 1 file changed, 66 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/reset/reset-mpfs.c b/drivers/reset/reset-mpfs.c
> > index f6fa10e03ea8..8e5ed4deecf3 100644
> > --- a/drivers/reset/reset-mpfs.c
> > +++ b/drivers/reset/reset-mpfs.c
> > @@ -7,13 +7,16 @@
> > *
> > */
> > #include <linux/auxiliary_bus.h>
> > +#include <linux/cleanup.h>
> > #include <linux/delay.h>
> > #include <linux/io.h>
> > +#include <linux/mfd/syscon.h>
>
> Should you add a depends on MFD_SYSCON ?
Perhaps, but it's a NOP. This driver depends on the clock driver, which
does have a depends on MFD_SYSCON. I'll add it, but makes no difference
to either the build or functionality.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 5/9] clk: microchip: mpfs: use regmap for clocks
2025-10-23 4:06 ` Claudiu Beznea
2025-10-23 14:42 ` Brian Masney
@ 2025-10-24 10:20 ` Conor Dooley
2025-10-24 10:30 ` Conor Dooley
1 sibling, 1 reply; 20+ messages in thread
From: Conor Dooley @ 2025-10-24 10:20 UTC (permalink / raw)
To: Claudiu Beznea
Cc: Conor Dooley, Daire McNamara, pierre-henry.moussay,
valentina.fernandezalanis, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Philipp Zabel, linux-riscv,
linux-clk, devicetree, linux-kernel, Brian Masney
[-- Attachment #1: Type: text/plain, Size: 1836 bytes --]
On Thu, Oct 23, 2025 at 07:06:01AM +0300, Claudiu Beznea wrote:
> On 10/13/25 20:45, Conor Dooley wrote:
> > From: Conor Dooley <conor.dooley@microchip.com>
> > +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;
>
> This could be moved near flags to keep the reverse christmas tree order as
> in the rest of this patch.
The driver doesn't (intentionally) use reverse christmas tree. If it
does, that's just a byproduct of putting bigger types before smaller
ones.
> > + 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);
>
> As spin locking is introduced in this file by this patch, you can go
> directly w/ cleanup helpers for locking.
>
> > +
> > + regmap_read(cfg->map, cfg->map_offset, &val);
> > + val &= ~(clk_div_mask(cfg->width) << cfg_hw->cfg.shift);
>
> Why cfg_hw->cfg.shift here --------------------^ but cfg->shift on the next
> line?
>
> > + val |= divider_setting << cfg->shift;
> > + regmap_write(cfg->map, cfg->map_offset, val);
>
> Can't the regmap_read() + updated + regmap_write() be replaced by
> regmap_update_bits() ?
Yeah, I suppose it could. Ultimately what's here is a revert of with
readl()/writel() replaced by regmap operations directly, so the answer
to the above three items is that that's how they were done before the
patch I am reverting. That's probably the answer to 90% of the things
you've said here, this is how they were done prior to the commit I am
reverting.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v5 5/9] clk: microchip: mpfs: use regmap for clocks
2025-10-24 10:20 ` Conor Dooley
@ 2025-10-24 10:30 ` Conor Dooley
0 siblings, 0 replies; 20+ messages in thread
From: Conor Dooley @ 2025-10-24 10:30 UTC (permalink / raw)
To: Claudiu Beznea
Cc: Conor Dooley, Daire McNamara, pierre-henry.moussay,
valentina.fernandezalanis, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Philipp Zabel, linux-riscv,
linux-clk, devicetree, linux-kernel, Brian Masney
[-- Attachment #1: Type: text/plain, Size: 2149 bytes --]
On Fri, Oct 24, 2025 at 11:20:23AM +0100, Conor Dooley wrote:
> On Thu, Oct 23, 2025 at 07:06:01AM +0300, Claudiu Beznea wrote:
> > On 10/13/25 20:45, Conor Dooley wrote:
> > > From: Conor Dooley <conor.dooley@microchip.com>
> > > +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;
> >
> > This could be moved near flags to keep the reverse christmas tree order as
> > in the rest of this patch.
>
> The driver doesn't (intentionally) use reverse christmas tree. If it
> does, that's just a byproduct of putting bigger types before smaller
> ones.
>
> > > + 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);
> >
> > As spin locking is introduced in this file by this patch, you can go
> > directly w/ cleanup helpers for locking.
Actually, if using regmap_update_bits(), locking might be entirely
removable. Depends on if there's some rmw sequence that can't move to
that.
> >
> > > +
> > > + regmap_read(cfg->map, cfg->map_offset, &val);
> > > + val &= ~(clk_div_mask(cfg->width) << cfg_hw->cfg.shift);
> >
> > Why cfg_hw->cfg.shift here --------------------^ but cfg->shift on the next
> > line?
> >
> > > + val |= divider_setting << cfg->shift;
> > > + regmap_write(cfg->map, cfg->map_offset, val);
> >
> > Can't the regmap_read() + updated + regmap_write() be replaced by
> > regmap_update_bits() ?
>
> Yeah, I suppose it could. Ultimately what's here is a revert of with
> readl()/writel() replaced by regmap operations directly, so the answer
> to the above three items is that that's how they were done before the
> patch I am reverting. That's probably the answer to 90% of the things
> you've said here, this is how they were done prior to the commit I am
> reverting.
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2025-10-24 10:30 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-13 17:45 [PATCH v5 0/9] Redo PolarFire SoC's mailbox/clock devicestrees and related code Conor Dooley
2025-10-13 17:45 ` [PATCH v5 1/9] dt-bindings: soc: microchip: document the simple-mfd syscon on PolarFire SoC Conor Dooley
2025-10-13 17:45 ` [PATCH v5 2/9] soc: microchip: add mfd drivers for two syscon regions " Conor Dooley
2025-10-23 4:04 ` Claudiu Beznea
2025-10-23 9:12 ` Conor Dooley
2025-10-23 10:15 ` Conor Dooley
2025-10-13 17:45 ` [PATCH v5 3/9] reset: mpfs: add non-auxiliary bus probing Conor Dooley
2025-10-23 4:06 ` Claudiu Beznea
2025-10-24 10:07 ` Conor Dooley
2025-10-13 17:45 ` [PATCH v5 4/9] dt-bindings: clk: microchip: mpfs: remove first reg region Conor Dooley
2025-10-13 17:45 ` [PATCH v5 5/9] clk: microchip: mpfs: use regmap for clocks Conor Dooley
2025-10-23 4:06 ` Claudiu Beznea
2025-10-23 14:42 ` Brian Masney
2025-10-24 10:20 ` Conor Dooley
2025-10-24 10:30 ` Conor Dooley
2025-10-13 17:45 ` [PATCH v5 6/9] riscv: dts: microchip: fix mailbox description Conor Dooley
2025-10-13 17:45 ` [PATCH v5 7/9] riscv: dts: microchip: convert clock and reset to use syscon Conor Dooley
2025-10-13 17:45 ` [PATCH v5 8/9] MAINTAINERS: add new soc drivers to Microchip RISC-V entry Conor Dooley
2025-10-13 17:45 ` [PATCH v5 9/9] MAINTAINERS: rename " Conor Dooley
2025-10-21 13:31 ` (subset) [PATCH v5 0/9] Redo PolarFire SoC's mailbox/clock devicestrees and related code Conor Dooley
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).