[PATCH v1 00/11] Redo PolarFire SoC's mailbox/clock devicestrees and related code

[PATCH v1 00/11] Redo PolarFire SoC's mailbox/clock devicestrees and related code

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

Yo,

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, the Meson clk-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've got all the Meson clock folks on CC, given I am moving their
clk-regmap implementation to common code. There were one or two things I
didn't really like about the implementation, but it is better than the
Qualcomm one IMO and creating a third copy of clk-regmap is certainly
not an improvement on having two copies!

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 - I've got some
WIP work that I need to rebase and send for it before the dts patches
here will be applied:
https://github.com/ConchuOD/u-boot/commits/syscon-clocks/

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.

Cheers,
Conor.

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: Jassi Brar <jassisinghbrar@gmail.com>
CC: Lee Jones <lee@kernel.org>
CC: Paul Walmsley <paul.walmsley@sifive.com>
CC: Palmer Dabbelt <palmer@dabbelt.com>
CC: Albert Ou <aou@eecs.berkeley.edu>
CC: Neil Armstrong <neil.armstrong@linaro.org>
CC: Jerome Brunet <jbrunet@baylibre.com>
CC: Kevin Hilman <khilman@baylibre.com>
CC: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
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
CC: linux-amlogic@lists.infradead.org
CC: linux-arm-kernel@lists.infradead.org

Conor Dooley (11):
  dt-bindings: mailbox: mpfs: fix reg properties
  mailbox: mpfs: support new, syscon based, devicetree configuration
  dt-bindings: mfd: syscon document the non simple-mfd syscon on
    PolarFire SoC
  dt-bindings: soc: microchip: document the two simple-mfd syscons 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: move meson clk-regmap implementation to common code
  clk: microchip: mpfs: use regmap clock types
  riscv: dts: microchip: fix mailbox description
  riscv: dts: microchip: convert clock and reset to use syscon

 .../bindings/clock/microchip,mpfs-clkcfg.yaml |  36 +++---
 .../mailbox/microchip,mpfs-mailbox.yaml       |  13 +-
 .../devicetree/bindings/mfd/syscon.yaml       |   2 +
 .../microchip/microchip,mpfs-control-scb.yaml |  44 +++++++
 .../microchip,mpfs-mss-top-sysreg.yaml        |  54 +++++++++
 arch/riscv/boot/dts/microchip/mpfs.dtsi       |  34 ++++--
 drivers/clk/Kconfig                           |   4 +
 drivers/clk/Makefile                          |   1 +
 drivers/clk/{meson => }/clk-regmap.c          |   2 +-
 drivers/clk/meson/Kconfig                     |  46 ++++---
 drivers/clk/meson/Makefile                    |   1 -
 drivers/clk/meson/a1-peripherals.c            |   2 +-
 drivers/clk/meson/a1-pll.c                    |   2 +-
 drivers/clk/meson/axg-aoclk.c                 |   2 +-
 drivers/clk/meson/axg-audio.c                 |   2 +-
 drivers/clk/meson/axg.c                       |   2 +-
 drivers/clk/meson/c3-peripherals.c            |   2 +-
 drivers/clk/meson/c3-pll.c                    |   2 +-
 drivers/clk/meson/clk-cpu-dyndiv.c            |   2 +-
 drivers/clk/meson/clk-dualdiv.c               |   2 +-
 drivers/clk/meson/clk-mpll.c                  |   2 +-
 drivers/clk/meson/clk-phase.c                 |   2 +-
 drivers/clk/meson/clk-pll.c                   |   2 +-
 drivers/clk/meson/g12a-aoclk.c                |   2 +-
 drivers/clk/meson/g12a.c                      |   2 +-
 drivers/clk/meson/gxbb-aoclk.c                |   2 +-
 drivers/clk/meson/gxbb.c                      |   2 +-
 drivers/clk/meson/meson-aoclk.h               |   2 +-
 drivers/clk/meson/meson-eeclk.c               |   2 +-
 drivers/clk/meson/meson-eeclk.h               |   2 +-
 drivers/clk/meson/meson8-ddr.c                |   2 +-
 drivers/clk/meson/meson8b.c                   |   2 +-
 drivers/clk/meson/s4-peripherals.c            |   2 +-
 drivers/clk/meson/s4-pll.c                    |   2 +-
 drivers/clk/meson/sclk-div.c                  |   2 +-
 drivers/clk/meson/vclk.h                      |   2 +-
 drivers/clk/meson/vid-pll-div.c               |   2 +-
 drivers/clk/microchip/Kconfig                 |   3 +
 drivers/clk/microchip/clk-mpfs.c              | 114 +++++++++++++-----
 drivers/mailbox/Kconfig                       |   1 +
 drivers/mailbox/mailbox-mpfs.c                |  87 ++++++++++---
 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 ++++++++
 .../meson => include/linux/clk}/clk-regmap.h  |   0
 47 files changed, 541 insertions(+), 143 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/soc/microchip/microchip,mpfs-control-scb.yaml
 create mode 100644 Documentation/devicetree/bindings/soc/microchip/microchip,mpfs-mss-top-sysreg.yaml
 rename drivers/clk/{meson => }/clk-regmap.c (99%)
 create mode 100644 drivers/soc/microchip/mpfs-control-scb.c
 create mode 100644 drivers/soc/microchip/mpfs-mss-top-sysreg.c
 rename {drivers/clk/meson => include/linux/clk}/clk-regmap.h (100%)

-- 
2.45.2

[PATCH v1 01/11] dt-bindings: mailbox: mpfs: fix reg properties

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

When the binding for this 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.

This is now coming to a head, because the control/status registers share
a register region with the "tvs" (temperature & voltage sensors)
registers and, as it turns out, people do want to monitor temperatures
and voltages...

Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 .../bindings/mailbox/microchip,mpfs-mailbox.yaml    | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/mailbox/microchip,mpfs-mailbox.yaml b/Documentation/devicetree/bindings/mailbox/microchip,mpfs-mailbox.yaml
index 404477910f029..1332aab9a888f 100644
--- a/Documentation/devicetree/bindings/mailbox/microchip,mpfs-mailbox.yaml
+++ b/Documentation/devicetree/bindings/mailbox/microchip,mpfs-mailbox.yaml
@@ -15,6 +15,8 @@ properties:
 
   reg:
     oneOf:
+      - items:
+          - description: mailbox data registers
       - items:
           - description: mailbox control & data registers
           - description: mailbox interrupt registers
@@ -23,6 +25,7 @@ properties:
           - description: mailbox control registers
           - description: mailbox interrupt registers
           - description: mailbox data registers
+        deprecated: true
 
   interrupts:
     maxItems: 1
@@ -41,12 +44,12 @@ additionalProperties: false
 examples:
   - |
     soc {
-      #address-cells = <2>;
-      #size-cells = <2>;
-      mbox: mailbox@37020000 {
+      #address-cells = <1>;
+      #size-cells = <1>;
+
+      mailbox@37020800 {
         compatible = "microchip,mpfs-mailbox";
-        reg = <0x0 0x37020000 0x0 0x58>, <0x0 0x2000318C 0x0 0x40>,
-              <0x0 0x37020800 0x0 0x100>;
+        reg = <0x37020800 0x100>;
         interrupt-parent = <&L1>;
         interrupts = <96>;
         #mbox-cells = <1>;
-- 
2.45.2

[PATCH v1 02/11] mailbox: mpfs: support new, syscon based, devicetree configuration

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

The two previous bindings for this hardware were incorrect, as the
control/status and interrupt register regions should have been described
as syscons and dealt with via regmap in the driver. Add support for
accessing these registers using that method now, so that the hwmon
driver can be supported without using auxdev or hacks with io_remap().

Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 drivers/mailbox/Kconfig        |  1 +
 drivers/mailbox/mailbox-mpfs.c | 87 +++++++++++++++++++++++++++-------
 2 files changed, 71 insertions(+), 17 deletions(-)

diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index 6fb995778636a..f856e01429aae 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -168,6 +168,7 @@ config MAILBOX_TEST
 config POLARFIRE_SOC_MAILBOX
 	tristate "PolarFire SoC (MPFS) Mailbox"
 	depends on HAS_IOMEM
+	depends on MFD_SYSCON
 	depends on ARCH_MICROCHIP_POLARFIRE || COMPILE_TEST
 	help
 	  This driver adds support for the PolarFire SoC (MPFS) mailbox controller.
diff --git a/drivers/mailbox/mailbox-mpfs.c b/drivers/mailbox/mailbox-mpfs.c
index 20ee283a04cc6..4df546e3b7eae 100644
--- a/drivers/mailbox/mailbox-mpfs.c
+++ b/drivers/mailbox/mailbox-mpfs.c
@@ -13,12 +13,15 @@
 #include <linux/init.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
+#include <linux/regmap.h>
 #include <linux/interrupt.h>
+#include <linux/mfd/syscon.h>
 #include <linux/mod_devicetable.h>
 #include <linux/platform_device.h>
 #include <linux/mailbox_controller.h>
 #include <soc/microchip/mpfs.h>
 
+#define MESSAGE_INT_OFFSET		0x18cu
 #define SERVICES_CR_OFFSET		0x50u
 #define SERVICES_SR_OFFSET		0x54u
 #define MAILBOX_REG_OFFSET		0x800u
@@ -68,6 +71,7 @@ struct mpfs_mbox {
 	void __iomem *int_reg;
 	struct mbox_chan chans[1];
 	struct mpfs_mss_response *response;
+	struct regmap *sysreg_scb, *control_scb;
 	u16 resp_offset;
 };
 
@@ -75,7 +79,10 @@ static bool mpfs_mbox_busy(struct mpfs_mbox *mbox)
 {
 	u32 status;
 
-	status = readl_relaxed(mbox->ctrl_base + SERVICES_SR_OFFSET);
+	if (mbox->control_scb)
+		regmap_read(mbox->control_scb, SERVICES_SR_OFFSET, &status);
+	else
+		status = readl_relaxed(mbox->ctrl_base + SERVICES_SR_OFFSET);
 
 	return status & SCB_STATUS_BUSY_MASK;
 }
@@ -95,7 +102,11 @@ static bool mpfs_mbox_last_tx_done(struct mbox_chan *chan)
 	 * Failed services are intended to generated interrupts, but in reality
 	 * this does not happen, so the status must be checked here.
 	 */
-	val = readl_relaxed(mbox->ctrl_base + SERVICES_SR_OFFSET);
+	if (mbox->control_scb)
+		regmap_read(mbox->control_scb, SERVICES_SR_OFFSET, &val);
+	else
+		val = readl_relaxed(mbox->ctrl_base + SERVICES_SR_OFFSET);
+
 	response->resp_status = (val & SCB_STATUS_MASK) >> SCB_STATUS_POS;
 
 	return true;
@@ -143,7 +154,12 @@ static int mpfs_mbox_send_data(struct mbox_chan *chan, void *data)
 
 	tx_trigger = (opt_sel << SCB_CTRL_POS) & SCB_CTRL_MASK;
 	tx_trigger |= SCB_CTRL_REQ_MASK | SCB_STATUS_NOTIFY_MASK;
-	writel_relaxed(tx_trigger, mbox->ctrl_base + SERVICES_CR_OFFSET);
+
+	if (mbox->control_scb)
+		regmap_write(mbox->control_scb, SERVICES_CR_OFFSET, tx_trigger);
+	else
+		writel_relaxed(tx_trigger, mbox->ctrl_base + SERVICES_CR_OFFSET);
+
 
 	return 0;
 }
@@ -185,7 +201,10 @@ static irqreturn_t mpfs_mbox_inbox_isr(int irq, void *data)
 	struct mbox_chan *chan = data;
 	struct mpfs_mbox *mbox = (struct mpfs_mbox *)chan->con_priv;
 
-	writel_relaxed(0, mbox->int_reg);
+	if (mbox->control_scb)
+		regmap_write(mbox->sysreg_scb, MESSAGE_INT_OFFSET, 0);
+	else
+		writel_relaxed(0, mbox->int_reg);
 
 	mpfs_mbox_rx_data(chan);
 
@@ -221,28 +240,62 @@ static const struct mbox_chan_ops mpfs_mbox_ops = {
 	.last_tx_done = mpfs_mbox_last_tx_done,
 };
 
+static inline int mpfs_mbox_syscon_probe(struct mpfs_mbox *mbox, struct platform_device *pdev)
+{
+	mbox->control_scb = syscon_regmap_lookup_by_compatible("microchip,mpfs-control-scb");
+	if (IS_ERR(mbox->control_scb))
+		return PTR_ERR(mbox->control_scb);
+
+	mbox->sysreg_scb = syscon_regmap_lookup_by_compatible("microchip,mpfs-sysreg-scb");
+	if (IS_ERR(mbox->sysreg_scb))
+		return PTR_ERR(mbox->sysreg_scb);
+
+	mbox->mbox_base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(mbox->ctrl_base))
+		return PTR_ERR(mbox->mbox_base);
+
+	return 0;
+}
+
+static inline int mpfs_mbox_old_format_probe(struct mpfs_mbox *mbox, struct platform_device *pdev)
+{
+	dev_warn(&pdev->dev, "falling back to old devicetree format");
+
+	mbox->ctrl_base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(mbox->ctrl_base))
+		return PTR_ERR(mbox->ctrl_base);
+
+	mbox->int_reg = devm_platform_ioremap_resource(pdev, 1);
+	if (IS_ERR(mbox->int_reg))
+		return PTR_ERR(mbox->int_reg);
+
+	mbox->mbox_base = devm_platform_ioremap_resource(pdev, 2);
+	if (IS_ERR(mbox->mbox_base)) // account for the old dt-binding w/ 2 regs
+		mbox->mbox_base = mbox->ctrl_base + MAILBOX_REG_OFFSET;
+
+	return 0;
+}
+
 static int mpfs_mbox_probe(struct platform_device *pdev)
 {
 	struct mpfs_mbox *mbox;
-	struct resource *regs;
 	int ret;
 
 	mbox = devm_kzalloc(&pdev->dev, sizeof(*mbox), GFP_KERNEL);
 	if (!mbox)
 		return -ENOMEM;
 
-	mbox->ctrl_base = devm_platform_get_and_ioremap_resource(pdev, 0, &regs);
-	if (IS_ERR(mbox->ctrl_base))
-		return PTR_ERR(mbox->ctrl_base);
-
-	mbox->int_reg = devm_platform_get_and_ioremap_resource(pdev, 1, &regs);
-	if (IS_ERR(mbox->int_reg))
-		return PTR_ERR(mbox->int_reg);
-
-	mbox->mbox_base = devm_platform_get_and_ioremap_resource(pdev, 2, &regs);
-	if (IS_ERR(mbox->mbox_base)) // account for the old dt-binding w/ 2 regs
-		mbox->mbox_base = mbox->ctrl_base + MAILBOX_REG_OFFSET;
-
+	ret = mpfs_mbox_syscon_probe(mbox, pdev);
+	if (ret) {
+		/*
+		 * set this to null, so it can be used as the decision for to
+		 * regmap or not to regmap
+		 */
+		mbox->control_scb = NULL;
+		ret = mpfs_mbox_old_format_probe(mbox, pdev);
+		if (ret)
+			return ret;
+	}
 	mbox->irq = platform_get_irq(pdev, 0);
 	if (mbox->irq < 0)
 		return mbox->irq;
-- 
2.45.2

[PATCH v1 03/11] dt-bindings: mfd: syscon document the non simple-mfd syscon on PolarFire SoC

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

The "mss_top_scb" register region on PolarFire SoC contains many
different functions, including controls for the AXI bus and other things
mainly of interest to the bootloader. The interrupt register for the
system controller's mailbox is also in here, which is needed by the
operating system.

Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 Documentation/devicetree/bindings/mfd/syscon.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/syscon.yaml b/Documentation/devicetree/bindings/mfd/syscon.yaml
index cc9b17ad69f23..b414de4fa779b 100644
--- a/Documentation/devicetree/bindings/mfd/syscon.yaml
+++ b/Documentation/devicetree/bindings/mfd/syscon.yaml
@@ -88,6 +88,7 @@ select:
           - mediatek,mt8173-pctl-a-syscfg
           - mediatek,mt8365-syscfg
           - microchip,lan966x-cpu-syscon
+          - microchip,mpfs-sysreg-scb
           - microchip,sam9x60-sfr
           - microchip,sama7g5-ddr3phy
           - mscc,ocelot-cpu-syscon
@@ -185,6 +186,7 @@ properties:
           - mediatek,mt8173-pctl-a-syscfg
           - mediatek,mt8365-syscfg
           - microchip,lan966x-cpu-syscon
+          - microchip,mpfs-sysreg-scb
           - microchip,sam9x60-sfr
           - microchip,sama7g5-ddr3phy
           - mscc,ocelot-cpu-syscon
-- 
2.45.2

[PATCH v1 04/11] dt-bindings: soc: microchip: document the two simple-mfd syscons on PolarFire SoC

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

There are two syscons on PolarFire SoC that provide various functionality of
use to the OS.

The first of these is the "control-scb" region, that contains the "tvs"
temperature and voltage sensors and the control/status registers for the
system controller's mailbox. The mailbox has a dedicated node, so
there's no need for a child node describing it, looking the syscon up by
compatible is sufficient.

The second, "mss-top-sysreg", contains clocks, pinctrl, resets, and
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]
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 .../microchip/microchip,mpfs-control-scb.yaml | 44 +++++++++++++++
 .../microchip,mpfs-mss-top-sysreg.yaml        | 54 +++++++++++++++++++
 2 files changed, 98 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/microchip/microchip,mpfs-control-scb.yaml
 create mode 100644 Documentation/devicetree/bindings/soc/microchip/microchip,mpfs-mss-top-sysreg.yaml

diff --git a/Documentation/devicetree/bindings/soc/microchip/microchip,mpfs-control-scb.yaml b/Documentation/devicetree/bindings/soc/microchip/microchip,mpfs-control-scb.yaml
new file mode 100644
index 0000000000000..4f9168320243c
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/microchip/microchip,mpfs-control-scb.yaml
@@ -0,0 +1,44 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/soc/microchip/microchip,mpfs-control-scb.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip PolarFire SoC System Controller Bus (SCB) Control Register region
+
+maintainers:
+  - Conor Dooley <conor.dooley@microchip.com>
+
+description:
+  An assortment of system controller related registers, including voltage and
+  temperature sensors and the status/control registers for the system
+  controller's mailbox.
+
+properties:
+  compatible:
+    items:
+      - const: microchip,mpfs-control-scb
+      - const: syscon
+      - const: simple-mfd
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    soc {
+      #address-cells = <1>;
+      #size-cells = <1>;
+
+      syscon@37020000 {
+        compatible = "microchip,mpfs-control-scb", "syscon", "simple-mfd";
+        reg = <0x37020000 0x100>;
+      };
+    };
+
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 0000000000000..98ccec3caad51
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/microchip/microchip,mpfs-mss-top-sysreg.yaml
@@ -0,0 +1,54 @@
+# 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
+      - const: simple-mfd
+
+  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
+  - '#reset-cells'
+
+additionalProperties: false
+
+examples:
+  - |
+    soc {
+      #address-cells = <1>;
+      #size-cells = <1>;
+
+      syscon@20002000 {
+        compatible = "microchip,mpfs-mss-top-sysreg", "syscon", "simple-mfd";
+        reg = <0x20002000 0x1000>;
+        #reset-cells = <1>;
+      };
+    };
+
-- 
2.45.2

[PATCH v1 05/11] soc: microchip: add mfd drivers for two syscon regions on PolarFire SoC

[Conor Dooley]

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 19f4b576f822b..31d188311e05f 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 14489919fe4b3..1a3a1594b089b 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 0000000000000..d1a8e79c232e3
--- /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 0000000000000..9b2e7b84cdba2
--- /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.45.2

[PATCH v1 06/11] reset: mpfs: add non-auxiliary bus probing

[Conor Dooley]

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>
---
 drivers/reset/reset-mpfs.c | 83 ++++++++++++++++++++++++++++++++------
 1 file changed, 71 insertions(+), 12 deletions(-)

diff --git a/drivers/reset/reset-mpfs.c b/drivers/reset/reset-mpfs.c
index 710f9c1676f93..ac72e0fc405ed 100644
--- a/drivers/reset/reset-mpfs.c
+++ b/drivers/reset/reset-mpfs.c
@@ -9,10 +9,12 @@
 #include <linux/auxiliary_bus.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 <dt-bindings/clock/microchip,mpfs-clock.h>
 #include <soc/microchip/mpfs.h>
@@ -27,14 +29,37 @@
 #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;
 };
 
+static inline u32 mpfs_reset_read(struct mpfs_reset *rst)
+{
+	u32 ret;
+
+	if (rst->regmap)
+		regmap_read(rst->regmap, REG_SUBBLK_RESET_CR, &ret);
+	else
+		ret = readl(rst->base);
+
+	return ret;
+}
+
+static inline void mpfs_reset_write(struct mpfs_reset *rst, u32 val)
+{
+	if (rst->regmap)
+		regmap_write(rst->regmap, REG_SUBBLK_RESET_CR, val);
+	else
+		writel(val, rst->base);
+}
+
 static inline struct mpfs_reset *to_mpfs_reset(struct reset_controller_dev *rcdev)
 {
 	return container_of(rcdev, struct mpfs_reset, rcdev);
@@ -51,9 +76,9 @@ static int mpfs_assert(struct reset_controller_dev *rcdev, unsigned long id)
 
 	spin_lock_irqsave(&mpfs_reset_lock, flags);
 
-	reg = readl(rst->base);
+	reg = mpfs_reset_read(rst);
 	reg |= BIT(id);
-	writel(reg, rst->base);
+	mpfs_reset_write(rst, reg);
 
 	spin_unlock_irqrestore(&mpfs_reset_lock, flags);
 
@@ -68,9 +93,9 @@ static int mpfs_deassert(struct reset_controller_dev *rcdev, unsigned long id)
 
 	spin_lock_irqsave(&mpfs_reset_lock, flags);
 
-	reg = readl(rst->base);
+	reg = mpfs_reset_read(rst);
 	reg &= ~BIT(id);
-	writel(reg, rst->base);
+	mpfs_reset_write(rst, reg);
 
 	spin_unlock_irqrestore(&mpfs_reset_lock, flags);
 
@@ -80,7 +105,7 @@ static int mpfs_deassert(struct reset_controller_dev *rcdev, unsigned long id)
 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 = mpfs_reset_read(rst);
 
 	/*
 	 * It is safe to return here as MPFS_NUM_RESETS makes sure the sign bit
@@ -130,11 +155,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 +204,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;
@@ -222,12 +281,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.45.2

[PATCH v1 07/11] dt-bindings: clk: microchip: mpfs: remove first reg region

[Conor Dooley]

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.

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 e4e1c31267d2a..ee4f31596d978 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.45.2

[PATCH v1 08/11] clk: move meson clk-regmap implementation to common code

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

I like this one better than qualcomms and wish to use it for the
PolarFire SoC clock drivers.

Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 drivers/clk/Kconfig                           |  4 ++
 drivers/clk/Makefile                          |  1 +
 drivers/clk/{meson => }/clk-regmap.c          |  2 +-
 drivers/clk/meson/Kconfig                     | 46 +++++++++----------
 drivers/clk/meson/Makefile                    |  1 -
 drivers/clk/meson/a1-peripherals.c            |  2 +-
 drivers/clk/meson/a1-pll.c                    |  2 +-
 drivers/clk/meson/axg-aoclk.c                 |  2 +-
 drivers/clk/meson/axg-audio.c                 |  2 +-
 drivers/clk/meson/axg.c                       |  2 +-
 drivers/clk/meson/c3-peripherals.c            |  2 +-
 drivers/clk/meson/c3-pll.c                    |  2 +-
 drivers/clk/meson/clk-cpu-dyndiv.c            |  2 +-
 drivers/clk/meson/clk-dualdiv.c               |  2 +-
 drivers/clk/meson/clk-mpll.c                  |  2 +-
 drivers/clk/meson/clk-phase.c                 |  2 +-
 drivers/clk/meson/clk-pll.c                   |  2 +-
 drivers/clk/meson/g12a-aoclk.c                |  2 +-
 drivers/clk/meson/g12a.c                      |  2 +-
 drivers/clk/meson/gxbb-aoclk.c                |  2 +-
 drivers/clk/meson/gxbb.c                      |  2 +-
 drivers/clk/meson/meson-aoclk.h               |  2 +-
 drivers/clk/meson/meson-eeclk.c               |  2 +-
 drivers/clk/meson/meson-eeclk.h               |  2 +-
 drivers/clk/meson/meson8-ddr.c                |  2 +-
 drivers/clk/meson/meson8b.c                   |  2 +-
 drivers/clk/meson/s4-peripherals.c            |  2 +-
 drivers/clk/meson/s4-pll.c                    |  2 +-
 drivers/clk/meson/sclk-div.c                  |  2 +-
 drivers/clk/meson/vclk.h                      |  2 +-
 drivers/clk/meson/vid-pll-div.c               |  2 +-
 .../meson => include/linux/clk}/clk-regmap.h  |  0
 32 files changed, 53 insertions(+), 53 deletions(-)
 rename drivers/clk/{meson => }/clk-regmap.c (99%)
 rename {drivers/clk/meson => include/linux/clk}/clk-regmap.h (100%)

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 299bc678ed1b9..85397308a74f4 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -33,6 +33,10 @@ menuconfig COMMON_CLK
 
 if COMMON_CLK
 
+config COMMON_CLK_REGMAP
+	bool
+	select REGMAP
+
 config COMMON_CLK_WM831X
 	tristate "Clock driver for WM831x/2x PMICs"
 	depends on MFD_WM831X
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index fb8878a5d7d93..bffdbfb932beb 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_COMMON_CLK)	+= clk-gate.o
 obj-$(CONFIG_CLK_GATE_KUNIT_TEST) += clk-gate_test.o
 obj-$(CONFIG_COMMON_CLK)	+= clk-multiplier.o
 obj-$(CONFIG_COMMON_CLK)	+= clk-mux.o
+obj-$(CONFIG_COMMON_CLK_REGMAP)	+= clk-regmap.o
 obj-$(CONFIG_COMMON_CLK)	+= clk-composite.o
 obj-$(CONFIG_COMMON_CLK)	+= clk-fractional-divider.o
 obj-$(CONFIG_CLK_FD_KUNIT_TEST) += clk-fractional-divider_test.o
diff --git a/drivers/clk/meson/clk-regmap.c b/drivers/clk/clk-regmap.c
similarity index 99%
rename from drivers/clk/meson/clk-regmap.c
rename to drivers/clk/clk-regmap.c
index 07f7e441b9161..4ec0ed8f72011 100644
--- a/drivers/clk/meson/clk-regmap.c
+++ b/drivers/clk/clk-regmap.c
@@ -5,7 +5,7 @@
  */
 
 #include <linux/module.h>
-#include "clk-regmap.h"
+#include <linux/clk/clk-regmap.h>
 
 static int clk_regmap_gate_endisable(struct clk_hw *hw, int enable)
 {
diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
index 78f648c9c97dc..ee4599dab7ff7 100644
--- a/drivers/clk/meson/Kconfig
+++ b/drivers/clk/meson/Kconfig
@@ -2,61 +2,57 @@
 menu "Clock support for Amlogic platforms"
 	depends on ARCH_MESON || COMPILE_TEST
 
-config COMMON_CLK_MESON_REGMAP
-	tristate
-	select REGMAP
-
 config COMMON_CLK_MESON_DUALDIV
 	tristate
-	select COMMON_CLK_MESON_REGMAP
+	select COMMON_CLK_REGMAP
 
 config COMMON_CLK_MESON_MPLL
 	tristate
-	select COMMON_CLK_MESON_REGMAP
+	select COMMON_CLK_REGMAP
 
 config COMMON_CLK_MESON_PHASE
 	tristate
-	select COMMON_CLK_MESON_REGMAP
+	select COMMON_CLK_REGMAP
 
 config COMMON_CLK_MESON_PLL
 	tristate
-	select COMMON_CLK_MESON_REGMAP
+	select COMMON_CLK_REGMAP
 
 config COMMON_CLK_MESON_SCLK_DIV
 	tristate
-	select COMMON_CLK_MESON_REGMAP
+	select COMMON_CLK_REGMAP
 
 config COMMON_CLK_MESON_VID_PLL_DIV
 	tristate
-	select COMMON_CLK_MESON_REGMAP
+	select COMMON_CLK_REGMAP
 
 config COMMON_CLK_MESON_VCLK
 	tristate
-	select COMMON_CLK_MESON_REGMAP
+	select COMMON_CLK_REGMAP
 
 config COMMON_CLK_MESON_CLKC_UTILS
 	tristate
 
 config COMMON_CLK_MESON_AO_CLKC
 	tristate
-	select COMMON_CLK_MESON_REGMAP
+	select COMMON_CLK_REGMAP
 	select COMMON_CLK_MESON_CLKC_UTILS
 	select RESET_CONTROLLER
 
 config COMMON_CLK_MESON_EE_CLKC
 	tristate
-	select COMMON_CLK_MESON_REGMAP
+	select COMMON_CLK_REGMAP
 	select COMMON_CLK_MESON_CLKC_UTILS
 
 config COMMON_CLK_MESON_CPU_DYNDIV
 	tristate
-	select COMMON_CLK_MESON_REGMAP
+	select COMMON_CLK_REGMAP
 
 config COMMON_CLK_MESON8B
 	bool "Meson8 SoC Clock controller support"
 	depends on ARM
 	default y
-	select COMMON_CLK_MESON_REGMAP
+	select COMMON_CLK_REGMAP
 	select COMMON_CLK_MESON_CLKC_UTILS
 	select COMMON_CLK_MESON_MPLL
 	select COMMON_CLK_MESON_PLL
@@ -71,7 +67,7 @@ config COMMON_CLK_GXBB
 	tristate "GXBB and GXL SoC clock controllers support"
 	depends on ARM64
 	default y
-	select COMMON_CLK_MESON_REGMAP
+	select COMMON_CLK_REGMAP
 	select COMMON_CLK_MESON_DUALDIV
 	select COMMON_CLK_MESON_VID_PLL_DIV
 	select COMMON_CLK_MESON_MPLL
@@ -87,7 +83,7 @@ config COMMON_CLK_AXG
 	tristate "AXG SoC clock controllers support"
 	depends on ARM64
 	default y
-	select COMMON_CLK_MESON_REGMAP
+	select COMMON_CLK_REGMAP
 	select COMMON_CLK_MESON_DUALDIV
 	select COMMON_CLK_MESON_MPLL
 	select COMMON_CLK_MESON_PLL
@@ -101,7 +97,7 @@ config COMMON_CLK_AXG
 config COMMON_CLK_AXG_AUDIO
 	tristate "Meson AXG Audio Clock Controller Driver"
 	depends on ARM64
-	select COMMON_CLK_MESON_REGMAP
+	select COMMON_CLK_REGMAP
 	select COMMON_CLK_MESON_PHASE
 	select COMMON_CLK_MESON_SCLK_DIV
 	select COMMON_CLK_MESON_CLKC_UTILS
@@ -113,7 +109,7 @@ config COMMON_CLK_AXG_AUDIO
 config COMMON_CLK_A1_PLL
 	tristate "Amlogic A1 SoC PLL controller support"
 	depends on ARM64
-	select COMMON_CLK_MESON_REGMAP
+	select COMMON_CLK_REGMAP
 	select COMMON_CLK_MESON_CLKC_UTILS
 	select COMMON_CLK_MESON_PLL
 	help
@@ -125,7 +121,7 @@ config COMMON_CLK_A1_PERIPHERALS
 	tristate "Amlogic A1 SoC Peripherals clock controller support"
 	depends on ARM64
 	select COMMON_CLK_MESON_DUALDIV
-	select COMMON_CLK_MESON_REGMAP
+	select COMMON_CLK_REGMAP
 	select COMMON_CLK_MESON_CLKC_UTILS
 	help
 	  Support for the Peripherals clock controller on Amlogic A113L based
@@ -136,7 +132,7 @@ config COMMON_CLK_C3_PLL
 	tristate "Amlogic C3 PLL clock controller"
 	depends on ARM64
 	default y
-	select COMMON_CLK_MESON_REGMAP
+	select COMMON_CLK_REGMAP
 	select COMMON_CLK_MESON_PLL
 	select COMMON_CLK_MESON_CLKC_UTILS
 	imply COMMON_CLK_SCMI
@@ -149,7 +145,7 @@ config COMMON_CLK_C3_PERIPHERALS
 	tristate "Amlogic C3 peripherals clock controller"
 	depends on ARM64
 	default y
-	select COMMON_CLK_MESON_REGMAP
+	select COMMON_CLK_REGMAP
 	select COMMON_CLK_MESON_DUALDIV
 	select COMMON_CLK_MESON_CLKC_UTILS
 	imply COMMON_CLK_SCMI
@@ -163,7 +159,7 @@ config COMMON_CLK_G12A
 	tristate "G12 and SM1 SoC clock controllers support"
 	depends on ARM64
 	default y
-	select COMMON_CLK_MESON_REGMAP
+	select COMMON_CLK_REGMAP
 	select COMMON_CLK_MESON_DUALDIV
 	select COMMON_CLK_MESON_MPLL
 	select COMMON_CLK_MESON_PLL
@@ -184,7 +180,7 @@ config COMMON_CLK_S4_PLL
 	select COMMON_CLK_MESON_CLKC_UTILS
 	select COMMON_CLK_MESON_MPLL
 	select COMMON_CLK_MESON_PLL
-	select COMMON_CLK_MESON_REGMAP
+	select COMMON_CLK_REGMAP
 	help
 	  Support for the PLL clock controller on Amlogic S805X2 and S905Y4 devices,
 	  AKA S4. Say Y if you want the board to work, because PLLs are the parent of
@@ -195,7 +191,7 @@ config COMMON_CLK_S4_PERIPHERALS
 	depends on ARM64
 	default y
 	select COMMON_CLK_MESON_CLKC_UTILS
-	select COMMON_CLK_MESON_REGMAP
+	select COMMON_CLK_REGMAP
 	select COMMON_CLK_MESON_DUALDIV
 	select COMMON_CLK_MESON_VID_PLL_DIV
 	help
diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
index bc56a47931c1d..cd870281d82c7 100644
--- a/drivers/clk/meson/Makefile
+++ b/drivers/clk/meson/Makefile
@@ -9,7 +9,6 @@ obj-$(CONFIG_COMMON_CLK_MESON_EE_CLKC) += meson-eeclk.o
 obj-$(CONFIG_COMMON_CLK_MESON_MPLL) += clk-mpll.o
 obj-$(CONFIG_COMMON_CLK_MESON_PHASE) += clk-phase.o
 obj-$(CONFIG_COMMON_CLK_MESON_PLL) += clk-pll.o
-obj-$(CONFIG_COMMON_CLK_MESON_REGMAP) += clk-regmap.o
 obj-$(CONFIG_COMMON_CLK_MESON_SCLK_DIV) += sclk-div.o
 obj-$(CONFIG_COMMON_CLK_MESON_VID_PLL_DIV) += vid-pll-div.o
 obj-$(CONFIG_COMMON_CLK_MESON_VCLK) += vclk.o
diff --git a/drivers/clk/meson/a1-peripherals.c b/drivers/clk/meson/a1-peripherals.c
index 7aa6abb2eb1f2..6178e6a153394 100644
--- a/drivers/clk/meson/a1-peripherals.c
+++ b/drivers/clk/meson/a1-peripherals.c
@@ -12,7 +12,7 @@
 #include <linux/platform_device.h>
 #include "a1-peripherals.h"
 #include "clk-dualdiv.h"
-#include "clk-regmap.h"
+#include <linux/clk/clk-regmap.h>
 #include "meson-clkc-utils.h"
 
 #include <dt-bindings/clock/amlogic,a1-peripherals-clkc.h>
diff --git a/drivers/clk/meson/a1-pll.c b/drivers/clk/meson/a1-pll.c
index 8e5a42d1afbbc..48ba3981b22df 100644
--- a/drivers/clk/meson/a1-pll.c
+++ b/drivers/clk/meson/a1-pll.c
@@ -11,7 +11,7 @@
 #include <linux/mod_devicetable.h>
 #include <linux/platform_device.h>
 #include "a1-pll.h"
-#include "clk-regmap.h"
+#include <linux/clk/clk-regmap.h>
 #include "meson-clkc-utils.h"
 
 #include <dt-bindings/clock/amlogic,a1-pll-clkc.h>
diff --git a/drivers/clk/meson/axg-aoclk.c b/drivers/clk/meson/axg-aoclk.c
index 1dabc81535a6f..ee89edf05a443 100644
--- a/drivers/clk/meson/axg-aoclk.c
+++ b/drivers/clk/meson/axg-aoclk.c
@@ -15,7 +15,7 @@
 #include <linux/module.h>
 #include "meson-aoclk.h"
 
-#include "clk-regmap.h"
+#include <linux/clk/clk-regmap.h>
 #include "clk-dualdiv.h"
 
 #include <dt-bindings/clock/axg-aoclkc.h>
diff --git a/drivers/clk/meson/axg-audio.c b/drivers/clk/meson/axg-audio.c
index beda863493899..06ccf1db63f58 100644
--- a/drivers/clk/meson/axg-audio.c
+++ b/drivers/clk/meson/axg-audio.c
@@ -17,7 +17,7 @@
 
 #include "meson-clkc-utils.h"
 #include "axg-audio.h"
-#include "clk-regmap.h"
+#include <linux/clk/clk-regmap.h>
 #include "clk-phase.h"
 #include "sclk-div.h"
 
diff --git a/drivers/clk/meson/axg.c b/drivers/clk/meson/axg.c
index 757c7a28c53de..73a0cad223c9c 100644
--- a/drivers/clk/meson/axg.c
+++ b/drivers/clk/meson/axg.c
@@ -15,7 +15,7 @@
 #include <linux/platform_device.h>
 #include <linux/module.h>
 
-#include "clk-regmap.h"
+#include <linux/clk/clk-regmap.h>
 #include "clk-pll.h"
 #include "clk-mpll.h"
 #include "axg.h"
diff --git a/drivers/clk/meson/c3-peripherals.c b/drivers/clk/meson/c3-peripherals.c
index 7dcbf4ebee078..13c13ead6bc66 100644
--- a/drivers/clk/meson/c3-peripherals.c
+++ b/drivers/clk/meson/c3-peripherals.c
@@ -8,7 +8,7 @@
 
 #include <linux/clk-provider.h>
 #include <linux/platform_device.h>
-#include "clk-regmap.h"
+#include <linux/clk/clk-regmap.h>
 #include "clk-dualdiv.h"
 #include "meson-clkc-utils.h"
 #include <dt-bindings/clock/amlogic,c3-peripherals-clkc.h>
diff --git a/drivers/clk/meson/c3-pll.c b/drivers/clk/meson/c3-pll.c
index 32bd2ed9d3044..06a7322403b53 100644
--- a/drivers/clk/meson/c3-pll.c
+++ b/drivers/clk/meson/c3-pll.c
@@ -8,7 +8,7 @@
 
 #include <linux/clk-provider.h>
 #include <linux/platform_device.h>
-#include "clk-regmap.h"
+#include <linux/clk/clk-regmap.h>
 #include "clk-pll.h"
 #include "meson-clkc-utils.h"
 #include <dt-bindings/clock/amlogic,c3-pll-clkc.h>
diff --git a/drivers/clk/meson/clk-cpu-dyndiv.c b/drivers/clk/meson/clk-cpu-dyndiv.c
index 6c1f58826e24a..d14bb1b5e4337 100644
--- a/drivers/clk/meson/clk-cpu-dyndiv.c
+++ b/drivers/clk/meson/clk-cpu-dyndiv.c
@@ -7,7 +7,7 @@
 #include <linux/clk-provider.h>
 #include <linux/module.h>
 
-#include "clk-regmap.h"
+#include <linux/clk/clk-regmap.h>
 #include "clk-cpu-dyndiv.h"
 
 static inline struct meson_clk_cpu_dyndiv_data *
diff --git a/drivers/clk/meson/clk-dualdiv.c b/drivers/clk/meson/clk-dualdiv.c
index 913bf25d3771b..8926f6fc94edf 100644
--- a/drivers/clk/meson/clk-dualdiv.c
+++ b/drivers/clk/meson/clk-dualdiv.c
@@ -24,7 +24,7 @@
 #include <linux/clk-provider.h>
 #include <linux/module.h>
 
-#include "clk-regmap.h"
+#include <linux/clk/clk-regmap.h>
 #include "clk-dualdiv.h"
 
 static inline struct meson_clk_dualdiv_data *
diff --git a/drivers/clk/meson/clk-mpll.c b/drivers/clk/meson/clk-mpll.c
index f639d56f0fd3f..cdf5c3cbeda12 100644
--- a/drivers/clk/meson/clk-mpll.c
+++ b/drivers/clk/meson/clk-mpll.c
@@ -15,7 +15,7 @@
 #include <linux/module.h>
 #include <linux/spinlock.h>
 
-#include "clk-regmap.h"
+#include <linux/clk/clk-regmap.h>
 #include "clk-mpll.h"
 
 #define SDM_DEN 16384
diff --git a/drivers/clk/meson/clk-phase.c b/drivers/clk/meson/clk-phase.c
index c1526fbfb6c4c..f48384c190e2f 100644
--- a/drivers/clk/meson/clk-phase.c
+++ b/drivers/clk/meson/clk-phase.c
@@ -7,7 +7,7 @@
 #include <linux/clk-provider.h>
 #include <linux/module.h>
 
-#include "clk-regmap.h"
+#include <linux/clk/clk-regmap.h>
 #include "clk-phase.h"
 
 #define phase_step(_width) (360 / (1 << (_width)))
diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
index bc570a2ff3a3f..44d87c6c3dcaa 100644
--- a/drivers/clk/meson/clk-pll.c
+++ b/drivers/clk/meson/clk-pll.c
@@ -33,7 +33,7 @@
 #include <linux/math64.h>
 #include <linux/module.h>
 
-#include "clk-regmap.h"
+#include <linux/clk/clk-regmap.h>
 #include "clk-pll.h"
 
 static inline struct meson_clk_pll_data *
diff --git a/drivers/clk/meson/g12a-aoclk.c b/drivers/clk/meson/g12a-aoclk.c
index f0a18d8c9fc23..25e6f2597407e 100644
--- a/drivers/clk/meson/g12a-aoclk.c
+++ b/drivers/clk/meson/g12a-aoclk.c
@@ -15,7 +15,7 @@
 #include <linux/module.h>
 #include "meson-aoclk.h"
 
-#include "clk-regmap.h"
+#include <linux/clk/clk-regmap.h>
 #include "clk-dualdiv.h"
 
 #include <dt-bindings/clock/g12a-aoclkc.h>
diff --git a/drivers/clk/meson/g12a.c b/drivers/clk/meson/g12a.c
index 02dda57105b10..b88b2519c9150 100644
--- a/drivers/clk/meson/g12a.c
+++ b/drivers/clk/meson/g12a.c
@@ -19,7 +19,7 @@
 
 #include "clk-mpll.h"
 #include "clk-pll.h"
-#include "clk-regmap.h"
+#include <linux/clk/clk-regmap.h>
 #include "clk-cpu-dyndiv.h"
 #include "vid-pll-div.h"
 #include "vclk.h"
diff --git a/drivers/clk/meson/gxbb-aoclk.c b/drivers/clk/meson/gxbb-aoclk.c
index 83b034157b353..f6facefc79041 100644
--- a/drivers/clk/meson/gxbb-aoclk.c
+++ b/drivers/clk/meson/gxbb-aoclk.c
@@ -8,7 +8,7 @@
 #include <linux/module.h>
 #include "meson-aoclk.h"
 
-#include "clk-regmap.h"
+#include <linux/clk/clk-regmap.h>
 #include "clk-dualdiv.h"
 
 #include <dt-bindings/clock/gxbb-aoclkc.h>
diff --git a/drivers/clk/meson/gxbb.c b/drivers/clk/meson/gxbb.c
index f071faad1ebb7..a9c5d73ee4bfb 100644
--- a/drivers/clk/meson/gxbb.c
+++ b/drivers/clk/meson/gxbb.c
@@ -11,7 +11,7 @@
 #include <linux/module.h>
 
 #include "gxbb.h"
-#include "clk-regmap.h"
+#include <linux/clk/clk-regmap.h>
 #include "clk-pll.h"
 #include "clk-mpll.h"
 #include "meson-eeclk.h"
diff --git a/drivers/clk/meson/meson-aoclk.h b/drivers/clk/meson/meson-aoclk.h
index 308be3e4814a9..099f4d5b55b10 100644
--- a/drivers/clk/meson/meson-aoclk.h
+++ b/drivers/clk/meson/meson-aoclk.h
@@ -16,7 +16,7 @@
 #include <linux/regmap.h>
 #include <linux/reset-controller.h>
 
-#include "clk-regmap.h"
+#include <linux/clk/clk-regmap.h>
 #include "meson-clkc-utils.h"
 
 struct meson_aoclk_data {
diff --git a/drivers/clk/meson/meson-eeclk.c b/drivers/clk/meson/meson-eeclk.c
index 66f79e384fe51..bbbaf9743abd5 100644
--- a/drivers/clk/meson/meson-eeclk.c
+++ b/drivers/clk/meson/meson-eeclk.c
@@ -11,7 +11,7 @@
 #include <linux/regmap.h>
 #include <linux/module.h>
 
-#include "clk-regmap.h"
+#include <linux/clk/clk-regmap.h>
 #include "meson-eeclk.h"
 
 int meson_eeclkc_probe(struct platform_device *pdev)
diff --git a/drivers/clk/meson/meson-eeclk.h b/drivers/clk/meson/meson-eeclk.h
index 37a48b75c6605..2def0370200a4 100644
--- a/drivers/clk/meson/meson-eeclk.h
+++ b/drivers/clk/meson/meson-eeclk.h
@@ -8,7 +8,7 @@
 #define __MESON_CLKC_H
 
 #include <linux/clk-provider.h>
-#include "clk-regmap.h"
+#include <linux/clk/clk-regmap.h>
 #include "meson-clkc-utils.h"
 
 struct platform_device;
diff --git a/drivers/clk/meson/meson8-ddr.c b/drivers/clk/meson/meson8-ddr.c
index 4b73ea244b630..22b1404ed3e1c 100644
--- a/drivers/clk/meson/meson8-ddr.c
+++ b/drivers/clk/meson/meson8-ddr.c
@@ -10,7 +10,7 @@
 #include <linux/clk-provider.h>
 #include <linux/platform_device.h>
 
-#include "clk-regmap.h"
+#include <linux/clk/clk-regmap.h>
 #include "clk-pll.h"
 
 #define AM_DDR_PLL_CNTL			0x00
diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c
index b7417ac262d33..8711c57e84b5c 100644
--- a/drivers/clk/meson/meson8b.c
+++ b/drivers/clk/meson/meson8b.c
@@ -17,7 +17,7 @@
 #include <linux/regmap.h>
 
 #include "meson8b.h"
-#include "clk-regmap.h"
+#include <linux/clk/clk-regmap.h>
 #include "meson-clkc-utils.h"
 #include "clk-pll.h"
 #include "clk-mpll.h"
diff --git a/drivers/clk/meson/s4-peripherals.c b/drivers/clk/meson/s4-peripherals.c
index c930cf0614a0f..e780bb0a07895 100644
--- a/drivers/clk/meson/s4-peripherals.c
+++ b/drivers/clk/meson/s4-peripherals.c
@@ -10,7 +10,7 @@
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
 
-#include "clk-regmap.h"
+#include <linux/clk/clk-regmap.h>
 #include "vid-pll-div.h"
 #include "clk-dualdiv.h"
 #include "s4-peripherals.h"
diff --git a/drivers/clk/meson/s4-pll.c b/drivers/clk/meson/s4-pll.c
index b0258933fb9d2..e5e71f0a23ebd 100644
--- a/drivers/clk/meson/s4-pll.c
+++ b/drivers/clk/meson/s4-pll.c
@@ -12,7 +12,7 @@
 
 #include "clk-mpll.h"
 #include "clk-pll.h"
-#include "clk-regmap.h"
+#include <linux/clk/clk-regmap.h>
 #include "s4-pll.h"
 #include "meson-clkc-utils.h"
 #include <dt-bindings/clock/amlogic,s4-pll-clkc.h>
diff --git a/drivers/clk/meson/sclk-div.c b/drivers/clk/meson/sclk-div.c
index ae03b048182f3..912b5c9b4c296 100644
--- a/drivers/clk/meson/sclk-div.c
+++ b/drivers/clk/meson/sclk-div.c
@@ -19,7 +19,7 @@
 #include <linux/clk-provider.h>
 #include <linux/module.h>
 
-#include "clk-regmap.h"
+#include <linux/clk/clk-regmap.h>
 #include "sclk-div.h"
 
 static inline struct meson_sclk_div_data *
diff --git a/drivers/clk/meson/vclk.h b/drivers/clk/meson/vclk.h
index 20b0b181db09d..6f0370b0d3a69 100644
--- a/drivers/clk/meson/vclk.h
+++ b/drivers/clk/meson/vclk.h
@@ -6,7 +6,7 @@
 #ifndef __VCLK_H
 #define __VCLK_H
 
-#include "clk-regmap.h"
+#include <linux/clk/clk-regmap.h>
 #include "parm.h"
 
 /**
diff --git a/drivers/clk/meson/vid-pll-div.c b/drivers/clk/meson/vid-pll-div.c
index 486cf68fc97a0..e3558b1a0744c 100644
--- a/drivers/clk/meson/vid-pll-div.c
+++ b/drivers/clk/meson/vid-pll-div.c
@@ -7,7 +7,7 @@
 #include <linux/clk-provider.h>
 #include <linux/module.h>
 
-#include "clk-regmap.h"
+#include <linux/clk/clk-regmap.h>
 #include "vid-pll-div.h"
 
 static inline struct meson_vid_pll_div_data *
diff --git a/drivers/clk/meson/clk-regmap.h b/include/linux/clk/clk-regmap.h
similarity index 100%
rename from drivers/clk/meson/clk-regmap.h
rename to include/linux/clk/clk-regmap.h
-- 
2.45.2

[PATCH v1 09/11] clk: microchip: mpfs: use regmap clock types

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

Convert the PolarFire SoC clock driver to use regmap clock types 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.

Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 drivers/clk/microchip/Kconfig    |   3 +
 drivers/clk/microchip/clk-mpfs.c | 114 ++++++++++++++++++++++---------
 2 files changed, 85 insertions(+), 32 deletions(-)

diff --git a/drivers/clk/microchip/Kconfig b/drivers/clk/microchip/Kconfig
index 0724ce65898f3..72da1e0f437d9 100644
--- a/drivers/clk/microchip/Kconfig
+++ b/drivers/clk/microchip/Kconfig
@@ -7,6 +7,9 @@ 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 COMMON_CLK_REGMAP
+	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 28ec0da88cb38..8cf963291317c 100644
--- a/drivers/clk/microchip/clk-mpfs.c
+++ b/drivers/clk/microchip/clk-mpfs.c
@@ -6,10 +6,13 @@
  */
 #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>
+#include <linux/clk/clk-regmap.h>
 
 /* address offset of control registers */
 #define REG_MSSPLL_REF_CR	0x08u
@@ -30,6 +33,14 @@
 #define MSSPLL_POSTDIV_WIDTH	0x07u
 #define MSSPLL_FIXED_DIV	4u
 
+static const struct regmap_config clk_mpfs_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 +50,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;
@@ -68,14 +80,14 @@ 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_hw_clock {
-	struct clk_divider cfg;
-	struct clk_init_data init;
+	struct clk_regmap sigh;
+	struct clk_regmap_div_data cfg;
 	unsigned int id;
-	u32 reg_offset;
 };
 
 struct mpfs_periph_hw_clock {
-	struct clk_gate periph;
+	struct clk_regmap sigh;
+	struct clk_regmap_gate_data periph;
 	unsigned int id;
 };
 
@@ -225,10 +237,9 @@ static int mpfs_clk_register_msspll_outs(struct device *dev,
 	.cfg.shift = _shift,								\
 	.cfg.width = _width,								\
 	.cfg.table = _table,								\
-	.reg_offset = _offset,								\
+	.cfg.offset = _offset,								\
 	.cfg.flags = _flags,								\
-	.cfg.hw.init = CLK_HW_INIT(_name, _parent, &clk_divider_ops, 0),		\
-	.cfg.lock = &mpfs_clk_lock,							\
+	.sigh.hw.init = CLK_HW_INIT(_name, _parent, &clk_regmap_divider_ops, 0),	\
 }
 
 #define CLK_CPU_OFFSET		0u
@@ -248,10 +259,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.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),
+		.sigh.hw.init =
+			CLK_HW_INIT_PARENTS_DATA("clk_rtcref", mpfs_ext_ref, &clk_regmap_divider_ops, 0),
 	}
 };
 
@@ -264,14 +275,16 @@ 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->sigh.map = data->regmap;
+		cfg_hw->sigh.data = &cfg_hw->cfg;
+
+		ret = devm_clk_hw_register(dev, &cfg_hw->sigh.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->sigh.hw;
 	}
 
 	return 0;
@@ -283,13 +296,13 @@ static int mpfs_clk_register_cfgs(struct device *dev, struct mpfs_cfg_hw_clock *
 
 #define CLK_PERIPH(_id, _name, _parent, _shift, _flags) {			\
 	.id = _id,								\
+	.periph.offset = REG_SUBBLK_CLOCK_CR,					\
 	.periph.bit_idx = _shift,						\
-	.periph.hw.init = CLK_HW_INIT_HW(_name, _parent, &clk_gate_ops,		\
-				  _flags),					\
-	.periph.lock = &mpfs_clk_lock,						\
+	.sigh.hw.init = CLK_HW_INIT_HW(_name, _parent, &clk_regmap_gate_ops,	\
+					 _flags),				\
 }
 
-#define PARENT_CLK(PARENT) (&mpfs_cfg_clks[CLK_##PARENT##_OFFSET].cfg.hw)
+#define PARENT_CLK(PARENT) (&mpfs_cfg_clks[CLK_##PARENT##_OFFSET].sigh.hw)
 
 /*
  * Critical clocks:
@@ -346,19 +359,61 @@ 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->sigh.map = data->regmap;
+		periph_hw->sigh.data = &periph_hw->periph;
+		ret = devm_clk_hw_register(dev, &periph_hw->sigh.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->sigh.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, &clk_mpfs_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 +429,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 +460,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.45.2

[PATCH v1 10/11] riscv: dts: microchip: fix mailbox description

[Conor Dooley]

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 9883ca3554c50..f8a45e4f00a0d 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", "simple-mfd";
+			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 0x100>;
 			interrupt-parent = <&plic>;
 			interrupts = <96>;
 			#mbox-cells = <1>;
-- 
2.45.2

[PATCH v1 11/11] riscv: dts: microchip: convert clock and reset to use syscon

[Conor Dooley]

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 f8a45e4f00a0d..08aa4fe03fd30 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.45.2

Re: [PATCH v1 08/11] clk: move meson clk-regmap implementation to common code

[Neil Armstrong]

On 02/10/2024 12:48, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> I like this one better than qualcomms and wish to use it for the
> PolarFire SoC clock drivers.
> 
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>   drivers/clk/Kconfig                           |  4 ++
>   drivers/clk/Makefile                          |  1 +
>   drivers/clk/{meson => }/clk-regmap.c          |  2 +-
>   drivers/clk/meson/Kconfig                     | 46 +++++++++----------
>   drivers/clk/meson/Makefile                    |  1 -
>   drivers/clk/meson/a1-peripherals.c            |  2 +-
>   drivers/clk/meson/a1-pll.c                    |  2 +-
>   drivers/clk/meson/axg-aoclk.c                 |  2 +-
>   drivers/clk/meson/axg-audio.c                 |  2 +-
>   drivers/clk/meson/axg.c                       |  2 +-
>   drivers/clk/meson/c3-peripherals.c            |  2 +-
>   drivers/clk/meson/c3-pll.c                    |  2 +-
>   drivers/clk/meson/clk-cpu-dyndiv.c            |  2 +-
>   drivers/clk/meson/clk-dualdiv.c               |  2 +-
>   drivers/clk/meson/clk-mpll.c                  |  2 +-
>   drivers/clk/meson/clk-phase.c                 |  2 +-
>   drivers/clk/meson/clk-pll.c                   |  2 +-
>   drivers/clk/meson/g12a-aoclk.c                |  2 +-
>   drivers/clk/meson/g12a.c                      |  2 +-
>   drivers/clk/meson/gxbb-aoclk.c                |  2 +-
>   drivers/clk/meson/gxbb.c                      |  2 +-
>   drivers/clk/meson/meson-aoclk.h               |  2 +-
>   drivers/clk/meson/meson-eeclk.c               |  2 +-
>   drivers/clk/meson/meson-eeclk.h               |  2 +-
>   drivers/clk/meson/meson8-ddr.c                |  2 +-
>   drivers/clk/meson/meson8b.c                   |  2 +-
>   drivers/clk/meson/s4-peripherals.c            |  2 +-
>   drivers/clk/meson/s4-pll.c                    |  2 +-
>   drivers/clk/meson/sclk-div.c                  |  2 +-
>   drivers/clk/meson/vclk.h                      |  2 +-
>   drivers/clk/meson/vid-pll-div.c               |  2 +-
>   .../meson => include/linux/clk}/clk-regmap.h  |  0
>   32 files changed, 53 insertions(+), 53 deletions(-)
>   rename drivers/clk/{meson => }/clk-regmap.c (99%)
>   rename {drivers/clk/meson => include/linux/clk}/clk-regmap.h (100%)
> 
<snip>

I don't have objections, but I think Stephen didn't like the idea
a few years ago, but perhaps it has changed...

Anyway, take my:
Acked-by: Neil Armstrong <neil.armstrong@linaro.org>

Thanks,
Neil

Re: [PATCH v1 06/11] reset: mpfs: add non-auxiliary bus probing

[Philipp Zabel]

On Mi, 2024-10-02 at 11:48 +0100, 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>
> ---
>  drivers/reset/reset-mpfs.c | 83 ++++++++++++++++++++++++++++++++------
>  1 file changed, 71 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/reset/reset-mpfs.c b/drivers/reset/reset-mpfs.c
> index 710f9c1676f93..ac72e0fc405ed 100644
> --- a/drivers/reset/reset-mpfs.c
> +++ b/drivers/reset/reset-mpfs.c
> @@ -9,10 +9,12 @@
>  #include <linux/auxiliary_bus.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 <dt-bindings/clock/microchip,mpfs-clock.h>
>  #include <soc/microchip/mpfs.h>
> @@ -27,14 +29,37 @@
>  #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;
>  };
>  
> +static inline u32 mpfs_reset_read(struct mpfs_reset *rst)
> +{
> +	u32 ret;
> +
> +	if (rst->regmap)
> +		regmap_read(rst->regmap, REG_SUBBLK_RESET_CR, &ret);
> +	else
> +		ret = readl(rst->base);
> +
> +	return ret;
> +}
> +
> +static inline void mpfs_reset_write(struct mpfs_reset *rst, u32 val)
> +{
> +	if (rst->regmap)
> +		regmap_write(rst->regmap, REG_SUBBLK_RESET_CR, val);
> +	else
> +		writel(val, rst->base);
> +}
> +
>  static inline struct mpfs_reset *to_mpfs_reset(struct reset_controller_dev *rcdev)
>  {
>  	return container_of(rcdev, struct mpfs_reset, rcdev);
> @@ -51,9 +76,9 @@ static int mpfs_assert(struct reset_controller_dev *rcdev, unsigned long id)
>  
>  	spin_lock_irqsave(&mpfs_reset_lock, flags);
>  
> -	reg = readl(rst->base);
> +	reg = mpfs_reset_read(rst);
>  	reg |= BIT(id);
> -	writel(reg, rst->base);
> +	mpfs_reset_write(rst, reg);

This should use regmap_update_bits() in the regmap case, same in
mpfs_deassert().

regards
Philipp

Re: [PATCH v1 08/11] clk: move meson clk-regmap implementation to common code

[Jerome Brunet]

On Wed 02 Oct 2024 at 13:20, Neil Armstrong <neil.armstrong@linaro.org> wrote:

> On 02/10/2024 12:48, Conor Dooley wrote:
>> From: Conor Dooley <conor.dooley@microchip.com>
>> I like this one better than qualcomms and wish to use it for the
>> PolarFire SoC clock drivers.
>> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
>> ---
>>   drivers/clk/Kconfig                           |  4 ++
>>   drivers/clk/Makefile                          |  1 +
>>   drivers/clk/{meson => }/clk-regmap.c          |  2 +-
>>   drivers/clk/meson/Kconfig                     | 46 +++++++++----------
>>   drivers/clk/meson/Makefile                    |  1 -
>>   drivers/clk/meson/a1-peripherals.c            |  2 +-
>>   drivers/clk/meson/a1-pll.c                    |  2 +-
>>   drivers/clk/meson/axg-aoclk.c                 |  2 +-
>>   drivers/clk/meson/axg-audio.c                 |  2 +-
>>   drivers/clk/meson/axg.c                       |  2 +-
>>   drivers/clk/meson/c3-peripherals.c            |  2 +-
>>   drivers/clk/meson/c3-pll.c                    |  2 +-
>>   drivers/clk/meson/clk-cpu-dyndiv.c            |  2 +-
>>   drivers/clk/meson/clk-dualdiv.c               |  2 +-
>>   drivers/clk/meson/clk-mpll.c                  |  2 +-
>>   drivers/clk/meson/clk-phase.c                 |  2 +-
>>   drivers/clk/meson/clk-pll.c                   |  2 +-
>>   drivers/clk/meson/g12a-aoclk.c                |  2 +-
>>   drivers/clk/meson/g12a.c                      |  2 +-
>>   drivers/clk/meson/gxbb-aoclk.c                |  2 +-
>>   drivers/clk/meson/gxbb.c                      |  2 +-
>>   drivers/clk/meson/meson-aoclk.h               |  2 +-
>>   drivers/clk/meson/meson-eeclk.c               |  2 +-
>>   drivers/clk/meson/meson-eeclk.h               |  2 +-
>>   drivers/clk/meson/meson8-ddr.c                |  2 +-
>>   drivers/clk/meson/meson8b.c                   |  2 +-
>>   drivers/clk/meson/s4-peripherals.c            |  2 +-
>>   drivers/clk/meson/s4-pll.c                    |  2 +-
>>   drivers/clk/meson/sclk-div.c                  |  2 +-
>>   drivers/clk/meson/vclk.h                      |  2 +-
>>   drivers/clk/meson/vid-pll-div.c               |  2 +-
>>   .../meson => include/linux/clk}/clk-regmap.h  |  0
>>   32 files changed, 53 insertions(+), 53 deletions(-)
>>   rename drivers/clk/{meson => }/clk-regmap.c (99%)
>>   rename {drivers/clk/meson => include/linux/clk}/clk-regmap.h (100%)
>> 
> <snip>
>
> I don't have objections, but I think Stephen didn't like the idea
> a few years ago, but perhaps it has changed...
>
> Anyway, take my:
> Acked-by: Neil Armstrong <neil.armstrong@linaro.org>

We had a similar discussion 3y ago indeed:
https://lore.kernel.org/linux-clk/162734682512.2368309.12015873010777083014@swboyd.mtv.corp.google.com/

There are needs for a common regmap backed clocks indeed, but allowing
meson flavored regmap clocks to spread in the kernel was not really the
prefered way to do it. 

IIRC, Stephen's idea was more the bring regmap support in clk-gate.c,
clk-mux, etc ... I'm not quite sure how make iomem and regmap co-exist
in a manageable/maintainable way within those drivers (without adding yet
another level of abstraction I mean) ? Silently creating a regmap maybe
? but that's probably a bit heavy. I did not really had time to dig more
on this, I guess no one did.

I don't really have a preference one way or the other but if it is going
to be exposed in 'include/linux', we need to be sure that's how we want
to do it. With clocks poping in many driver subsystems, it will
difficult to change afterward. 

>
> Thanks,
> Neil

-- 
Jerome

Re: [PATCH v1 01/11] dt-bindings: mailbox: mpfs: fix reg properties

[Rob Herring (Arm)]

On Wed, 02 Oct 2024 11:47:59 +0100, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> When the binding for this 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.
> 
> This is now coming to a head, because the control/status registers share
> a register region with the "tvs" (temperature & voltage sensors)
> registers and, as it turns out, people do want to monitor temperatures
> and voltages...
> 
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>  .../bindings/mailbox/microchip,mpfs-mailbox.yaml    | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 

Acked-by: Rob Herring (Arm) <robh@kernel.org>

Re: [PATCH v1 04/11] dt-bindings: soc: microchip: document the two simple-mfd syscons on PolarFire SoC

[Rob Herring]

On Wed, Oct 02, 2024 at 11:48:02AM +0100, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> There are two syscons on PolarFire SoC that provide various functionality of
> use to the OS.
> 
> The first of these is the "control-scb" region, that contains the "tvs"
> temperature and voltage sensors and the control/status registers for the
> system controller's mailbox. The mailbox has a dedicated node, so
> there's no need for a child node describing it, looking the syscon up by
> compatible is sufficient.
> 
> The second, "mss-top-sysreg", contains clocks, pinctrl, resets, and
> 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]
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>  .../microchip/microchip,mpfs-control-scb.yaml | 44 +++++++++++++++
>  .../microchip,mpfs-mss-top-sysreg.yaml        | 54 +++++++++++++++++++
>  2 files changed, 98 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/soc/microchip/microchip,mpfs-control-scb.yaml
>  create mode 100644 Documentation/devicetree/bindings/soc/microchip/microchip,mpfs-mss-top-sysreg.yaml
> 
> diff --git a/Documentation/devicetree/bindings/soc/microchip/microchip,mpfs-control-scb.yaml b/Documentation/devicetree/bindings/soc/microchip/microchip,mpfs-control-scb.yaml
> new file mode 100644
> index 0000000000000..4f9168320243c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/microchip/microchip,mpfs-control-scb.yaml
> @@ -0,0 +1,44 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/soc/microchip/microchip,mpfs-control-scb.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Microchip PolarFire SoC System Controller Bus (SCB) Control Register region
> +
> +maintainers:
> +  - Conor Dooley <conor.dooley@microchip.com>
> +
> +description:
> +  An assortment of system controller related registers, including voltage and
> +  temperature sensors and the status/control registers for the system
> +  controller's mailbox.
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: microchip,mpfs-control-scb
> +      - const: syscon
> +      - const: simple-mfd

Where's the child nodes?

> +
> +  reg:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    soc {
> +      #address-cells = <1>;
> +      #size-cells = <1>;

Don't need the soc node.

> +
> +      syscon@37020000 {
> +        compatible = "microchip,mpfs-control-scb", "syscon", "simple-mfd";
> +        reg = <0x37020000 0x100>;
> +      };
> +    };
> +
> 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 0000000000000..98ccec3caad51
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/microchip/microchip,mpfs-mss-top-sysreg.yaml
> @@ -0,0 +1,54 @@
> +# 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
> +      - const: simple-mfd
> +
> +  reg:
> +    maxItems: 1
> +
> +  '#reset-cells':
> +    description: |

Don't need '|'.

> +      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
> +  - '#reset-cells'
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    soc {
> +      #address-cells = <1>;
> +      #size-cells = <1>;
> +
> +      syscon@20002000 {
> +        compatible = "microchip,mpfs-mss-top-sysreg", "syscon", "simple-mfd";
> +        reg = <0x20002000 0x1000>;
> +        #reset-cells = <1>;
> +      };
> +    };
> +
> -- 
> 2.45.2
> 

Re: [PATCH v1 03/11] dt-bindings: mfd: syscon document the non simple-mfd syscon on PolarFire SoC

[Rob Herring (Arm)]

On Wed, 02 Oct 2024 11:48:01 +0100, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> The "mss_top_scb" register region on PolarFire SoC contains many
> different functions, including controls for the AXI bus and other things
> mainly of interest to the bootloader. The interrupt register for the
> system controller's mailbox is also in here, which is needed by the
> operating system.
> 
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>  Documentation/devicetree/bindings/mfd/syscon.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 

Acked-by: Rob Herring (Arm) <robh@kernel.org>

Re: [PATCH v1 07/11] dt-bindings: clk: microchip: mpfs: remove first reg region

[Rob Herring (Arm)]

On Wed, 02 Oct 2024 11:48:05 +0100, Conor Dooley wrote:
> 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.
> 
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>  .../bindings/clock/microchip,mpfs-clkcfg.yaml | 36 +++++++++++--------
>  1 file changed, 22 insertions(+), 14 deletions(-)
> 

Acked-by: Rob Herring (Arm) <robh@kernel.org>

Re: [PATCH v1 08/11] clk: move meson clk-regmap implementation to common code

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 4664 bytes --]

On Wed, Oct 02, 2024 at 03:21:16PM +0200, Jerome Brunet wrote:
> On Wed 02 Oct 2024 at 13:20, Neil Armstrong <neil.armstrong@linaro.org> wrote:
> 
> > On 02/10/2024 12:48, Conor Dooley wrote:
> >> From: Conor Dooley <conor.dooley@microchip.com>
> >> I like this one better than qualcomms and wish to use it for the
> >> PolarFire SoC clock drivers.
> >> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> >> ---
> >>   drivers/clk/Kconfig                           |  4 ++
> >>   drivers/clk/Makefile                          |  1 +
> >>   drivers/clk/{meson => }/clk-regmap.c          |  2 +-
> >>   drivers/clk/meson/Kconfig                     | 46 +++++++++----------
> >>   drivers/clk/meson/Makefile                    |  1 -
> >>   drivers/clk/meson/a1-peripherals.c            |  2 +-
> >>   drivers/clk/meson/a1-pll.c                    |  2 +-
> >>   drivers/clk/meson/axg-aoclk.c                 |  2 +-
> >>   drivers/clk/meson/axg-audio.c                 |  2 +-
> >>   drivers/clk/meson/axg.c                       |  2 +-
> >>   drivers/clk/meson/c3-peripherals.c            |  2 +-
> >>   drivers/clk/meson/c3-pll.c                    |  2 +-
> >>   drivers/clk/meson/clk-cpu-dyndiv.c            |  2 +-
> >>   drivers/clk/meson/clk-dualdiv.c               |  2 +-
> >>   drivers/clk/meson/clk-mpll.c                  |  2 +-
> >>   drivers/clk/meson/clk-phase.c                 |  2 +-
> >>   drivers/clk/meson/clk-pll.c                   |  2 +-
> >>   drivers/clk/meson/g12a-aoclk.c                |  2 +-
> >>   drivers/clk/meson/g12a.c                      |  2 +-
> >>   drivers/clk/meson/gxbb-aoclk.c                |  2 +-
> >>   drivers/clk/meson/gxbb.c                      |  2 +-
> >>   drivers/clk/meson/meson-aoclk.h               |  2 +-
> >>   drivers/clk/meson/meson-eeclk.c               |  2 +-
> >>   drivers/clk/meson/meson-eeclk.h               |  2 +-
> >>   drivers/clk/meson/meson8-ddr.c                |  2 +-
> >>   drivers/clk/meson/meson8b.c                   |  2 +-
> >>   drivers/clk/meson/s4-peripherals.c            |  2 +-
> >>   drivers/clk/meson/s4-pll.c                    |  2 +-
> >>   drivers/clk/meson/sclk-div.c                  |  2 +-
> >>   drivers/clk/meson/vclk.h                      |  2 +-
> >>   drivers/clk/meson/vid-pll-div.c               |  2 +-
> >>   .../meson => include/linux/clk}/clk-regmap.h  |  0
> >>   32 files changed, 53 insertions(+), 53 deletions(-)
> >>   rename drivers/clk/{meson => }/clk-regmap.c (99%)
> >>   rename {drivers/clk/meson => include/linux/clk}/clk-regmap.h (100%)
> >> 
> > <snip>
> >
> > I don't have objections, but I think Stephen didn't like the idea
> > a few years ago, but perhaps it has changed...
> >
> > Anyway, take my:
> > Acked-by: Neil Armstrong <neil.armstrong@linaro.org>
> 
> We had a similar discussion 3y ago indeed:
> https://lore.kernel.org/linux-clk/162734682512.2368309.12015873010777083014@swboyd.mtv.corp.google.com/
> 
> There are needs for a common regmap backed clocks indeed, but allowing
> meson flavored regmap clocks to spread in the kernel was not really the
> prefered way to do it. 

Cool, thanks for that link.

> IIRC, Stephen's idea was more the bring regmap support in clk-gate.c,
> clk-mux, etc ... I'm not quite sure how make iomem and regmap co-exist
> in a manageable/maintainable way within those drivers (without adding yet
> another level of abstraction I mean) ? Silently creating a regmap maybe
> ? but that's probably a bit heavy. I did not really had time to dig more
> on this, I guess no one did.

I guess I have some motivation to looking into it at the moment. I had
my reservations about the Meson approach too, liking it more than
Qualcomm's didn't mean I completely liked it.
It was already my intention to implement point b of your mail, had the
general idea here been acceptable, cos that's a divergence from how the
generic clock types (that the driver in question currently uses) work.
And on that note, I just noticed I left the mild-annoyance variable name
"sigh" in the submitted driver changes, which I had used for the
clk_regmap struct that your point b in the link relates to.

> I don't really have a preference one way or the other but if it is going
> to be exposed in 'include/linux', we need to be sure that's how we want
> to do it. With clocks poping in many driver subsystems, it will
> difficult to change afterward. 

Yeah, I agree. I didn't expect this to go in right away, and I also
didn't want to surge ahead on some rework of the clock types, were
people to hate even the reuse.



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: (subset) [PATCH v1 03/11] dt-bindings: mfd: syscon document the non simple-mfd syscon on PolarFire SoC

[Lee Jones]

On Wed, 02 Oct 2024 11:48:01 +0100, Conor Dooley wrote:
> The "mss_top_scb" register region on PolarFire SoC contains many
> different functions, including controls for the AXI bus and other things
> mainly of interest to the bootloader. The interrupt register for the
> system controller's mailbox is also in here, which is needed by the
> operating system.
> 
> 
> [...]

Applied, thanks!

[03/11] dt-bindings: mfd: syscon document the non simple-mfd syscon on PolarFire SoC
        commit: 8ae16d3b2ff6650a90be78881cb88dcdf1bd1370

--
Lee Jones [李琼斯]

Re: [PATCH v1 10/11] riscv: dts: microchip: fix mailbox description

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2406 bytes --]

On Wed, Oct 02, 2024 at 11:48:08AM +0100, Conor Dooley wrote:
> 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 9883ca3554c50..f8a45e4f00a0d 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", "simple-mfd";
> +			reg = <0x0 0x37020000 0x0 0x100>;

It came up today that this 0x100 isn't correct - the actual size here is
4 KiB, so there's a zero missing.

> +		};
> +
> +		mbox: mailbox@37020800 {
>  			compatible = "microchip,mpfs-mailbox";
> -			reg = <0x0 0x37020000 0x0 0x58>, <0x0 0x2000318C 0x0 0x40>,
> -			      <0x0 0x37020800 0x0 0x100>;
> +			reg = <0x0 0x37020800 0x0 0x100>;
>  			interrupt-parent = <&plic>;
>  			interrupts = <96>;
>  			#mbox-cells = <1>;
> -- 
> 2.45.2
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v1 08/11] clk: move meson clk-regmap implementation to common code

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 6279 bytes --]

Stephen,

On Thu, Oct 03, 2024 at 12:33:40PM +0100, Conor Dooley wrote:
> On Wed, Oct 02, 2024 at 03:21:16PM +0200, Jerome Brunet wrote:
> > On Wed 02 Oct 2024 at 13:20, Neil Armstrong <neil.armstrong@linaro.org> wrote:
> > 
> > > On 02/10/2024 12:48, Conor Dooley wrote:
> > >> From: Conor Dooley <conor.dooley@microchip.com>
> > >> I like this one better than qualcomms and wish to use it for the
> > >> PolarFire SoC clock drivers.
> > >> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> > >> ---
> > >>   drivers/clk/Kconfig                           |  4 ++
> > >>   drivers/clk/Makefile                          |  1 +
> > >>   drivers/clk/{meson => }/clk-regmap.c          |  2 +-
> > >>   drivers/clk/meson/Kconfig                     | 46 +++++++++----------
> > >>   drivers/clk/meson/Makefile                    |  1 -
> > >>   drivers/clk/meson/a1-peripherals.c            |  2 +-
> > >>   drivers/clk/meson/a1-pll.c                    |  2 +-
> > >>   drivers/clk/meson/axg-aoclk.c                 |  2 +-
> > >>   drivers/clk/meson/axg-audio.c                 |  2 +-
> > >>   drivers/clk/meson/axg.c                       |  2 +-
> > >>   drivers/clk/meson/c3-peripherals.c            |  2 +-
> > >>   drivers/clk/meson/c3-pll.c                    |  2 +-
> > >>   drivers/clk/meson/clk-cpu-dyndiv.c            |  2 +-
> > >>   drivers/clk/meson/clk-dualdiv.c               |  2 +-
> > >>   drivers/clk/meson/clk-mpll.c                  |  2 +-
> > >>   drivers/clk/meson/clk-phase.c                 |  2 +-
> > >>   drivers/clk/meson/clk-pll.c                   |  2 +-
> > >>   drivers/clk/meson/g12a-aoclk.c                |  2 +-
> > >>   drivers/clk/meson/g12a.c                      |  2 +-
> > >>   drivers/clk/meson/gxbb-aoclk.c                |  2 +-
> > >>   drivers/clk/meson/gxbb.c                      |  2 +-
> > >>   drivers/clk/meson/meson-aoclk.h               |  2 +-
> > >>   drivers/clk/meson/meson-eeclk.c               |  2 +-
> > >>   drivers/clk/meson/meson-eeclk.h               |  2 +-
> > >>   drivers/clk/meson/meson8-ddr.c                |  2 +-
> > >>   drivers/clk/meson/meson8b.c                   |  2 +-
> > >>   drivers/clk/meson/s4-peripherals.c            |  2 +-
> > >>   drivers/clk/meson/s4-pll.c                    |  2 +-
> > >>   drivers/clk/meson/sclk-div.c                  |  2 +-
> > >>   drivers/clk/meson/vclk.h                      |  2 +-
> > >>   drivers/clk/meson/vid-pll-div.c               |  2 +-
> > >>   .../meson => include/linux/clk}/clk-regmap.h  |  0
> > >>   32 files changed, 53 insertions(+), 53 deletions(-)
> > >>   rename drivers/clk/{meson => }/clk-regmap.c (99%)
> > >>   rename {drivers/clk/meson => include/linux/clk}/clk-regmap.h (100%)
> > >> 
> > > <snip>
> > >
> > > I don't have objections, but I think Stephen didn't like the idea
> > > a few years ago, but perhaps it has changed...
> > >
> > > Anyway, take my:
> > > Acked-by: Neil Armstrong <neil.armstrong@linaro.org>
> > 
> > We had a similar discussion 3y ago indeed:
> > https://lore.kernel.org/linux-clk/162734682512.2368309.12015873010777083014@swboyd.mtv.corp.google.com/
> > 
> > There are needs for a common regmap backed clocks indeed, but allowing
> > meson flavored regmap clocks to spread in the kernel was not really the
> > prefered way to do it. 
> 
> Cool, thanks for that link.
> 
> > IIRC, Stephen's idea was more the bring regmap support in clk-gate.c,
> > clk-mux, etc ... I'm not quite sure how make iomem and regmap co-exist
> > in a manageable/maintainable way within those drivers (without adding yet
> > another level of abstraction I mean) ? Silently creating a regmap maybe
> > ? but that's probably a bit heavy. I did not really had time to dig more
> > on this, I guess no one did.
> 
> I guess I have some motivation to looking into it at the moment. I had
> my reservations about the Meson approach too, liking it more than
> Qualcomm's didn't mean I completely liked it.
> It was already my intention to implement point b of your mail, had the
> general idea here been acceptable, cos that's a divergence from how the
> generic clock types (that the driver in question currently uses) work.
> And on that note, I just noticed I left the mild-annoyance variable name
> "sigh" in the submitted driver changes, which I had used for the
> clk_regmap struct that your point b in the link relates to.
> 
> > I don't really have a preference one way or the other but if it is going
> > to be exposed in 'include/linux', we need to be sure that's how we want
> > to do it. With clocks poping in many driver subsystems, it will
> > difficult to change afterward. 
> 
> Yeah, I agree. I didn't expect this to go in right away, and I also
> didn't want to surge ahead on some rework of the clock types, were
> people to hate even the reuse.

Hmm, so how (in-)complete of a regmap implementation can I get away
with? I only need clk-gate and clk-divider for this patchset...

Shoving the regmap into the clk structs makes things pretty trivial as I
don't need to do anything special in any function other than
clk_*_readl()/clk_*_writel() and the registration code. A flag isn't
even needed to determine if a clock is a regmap one I don't think, since
you can just check if the regmap pointer is non-NULL. My use case doesn't
actually need the registration code changes either as, currently, only reg
gets set at runtime, but leaving that out is a level of incomplete I'd not
let myself away with.
Obviously shoving the extra members into the clk structs has the downside
of taking up a pointer and a offset worth of memory for each clock of
that type registered, but it is substantially easier to support devices
with multiple regmaps that way. Probably moot though since the approach you
suggested in the thread linked above that implements a clk_hw_get_regmap()
has to store a pointer to the regmap's identifier which would take up an
identical amount of memory.

I don't really care which way you want it done, both are pretty easy to
implement if I can get away with just doing so for the two standard
clock types that I am using - is it okay to just do those two?

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v1 08/11] clk: move meson clk-regmap implementation to common code

[Stephen Boyd]

Quoting Conor Dooley (2024-11-06 04:56:25)
> On Thu, Oct 03, 2024 at 12:33:40PM +0100, Conor Dooley wrote:
> > > IIRC, Stephen's idea was more the bring regmap support in clk-gate.c,
> > > clk-mux, etc ... I'm not quite sure how make iomem and regmap co-exist
> > > in a manageable/maintainable way within those drivers (without adding yet
> > > another level of abstraction I mean) ? Silently creating a regmap maybe
> > > ? but that's probably a bit heavy. I did not really had time to dig more
> > > on this, I guess no one did.
> > 
> > I guess I have some motivation to looking into it at the moment. I had
> > my reservations about the Meson approach too, liking it more than
> > Qualcomm's didn't mean I completely liked it.
> > It was already my intention to implement point b of your mail, had the
> > general idea here been acceptable, cos that's a divergence from how the
> > generic clock types (that the driver in question currently uses) work.
> > And on that note, I just noticed I left the mild-annoyance variable name
> > "sigh" in the submitted driver changes, which I had used for the
> > clk_regmap struct that your point b in the link relates to.
> > 
> > > I don't really have a preference one way or the other but if it is going
> > > to be exposed in 'include/linux', we need to be sure that's how we want
> > > to do it. With clocks poping in many driver subsystems, it will
> > > difficult to change afterward. 
> > 
> > Yeah, I agree. I didn't expect this to go in right away, and I also
> > didn't want to surge ahead on some rework of the clock types, were
> > people to hate even the reuse.
> 
> Hmm, so how (in-)complete of a regmap implementation can I get away
> with? I only need clk-gate and clk-divider for this patchset...
> 
> Shoving the regmap into the clk structs makes things pretty trivial as I
> don't need to do anything special in any function other than
> clk_*_readl()/clk_*_writel() and the registration code. A flag isn't
> even needed to determine if a clock is a regmap one I don't think, since
> you can just check if the regmap pointer is non-NULL.

For the basic clk types I think it would be good to leave the old stuff
alone. We have already split the logic out into helpers, so I wonder if
we can do this better by making kernel modules for the different basic
regmap clk types and exposing registration APIs. If we force drivers
that use the basic regmap types to 'select' the module then we'll make
it so that we don't include code that isn't used anywhere. That's one of
the problems with the basic clk types, it's always built. It also lets
us avoid making regmap a dependency for the clk framework at large.

Doing that would also let us avoid the flag because it will be explicit
in any registration API, clk_register_divider() vs.
clk_register_regmap_divider(). Yes we duplicate some boiler plate logic
around read-only and registration paths, but this is alright as long as
we can share most of the code and gain the advantage of removing the
code entirely when it isn't used.

I wonder if we can even make a regmap on the fly for the iomem pointers
so that clk_divider_readl() can always use the regmap API to access the
hardware. Sounds wasteful but maybe it would work.

> My use case doesn't
> actually need the registration code changes either as, currently, only reg
> gets set at runtime, but leaving that out is a level of incomplete I'd not
> let myself away with.
> Obviously shoving the extra members into the clk structs has the downside
> of taking up a pointer and a offset worth of memory for each clock of
> that type registered, but it is substantially easier to support devices
> with multiple regmaps that way. Probably moot though since the approach you
> suggested in the thread linked above that implements a clk_hw_get_regmap()
> has to store a pointer to the regmap's identifier which would take up an
> identical amount of memory.

We don't need to store the regmap identifier in the struct clk. We can
store it in the 'struct clk_init_data' with some new field, and only do
that when/if we actually need to. We would need to pass the init data to
the clk_ops::init() callback though. We currently knock that out during
registration so that clk_hw->init is NULL. Probably we can just set that
to NULL after the init routine runs in __clk_core_init().

Long story short, don't add something to 'struct clk_core', 'struct
clk', or 'struct clk_hw' for these details. We can have a 'struct
clk_regmap_hw' that everyone else can build upon:

  struct clk_regmap_hw {
        struct regmap *regmap;
        struct clk_hw hw;
  };

and then set the regmap pointer during registration in
clk_hw_init_regmap().

int clk_hw_init_regmap(struct clk_hw *hw)
{
	struct device *dev;
	struct regmap *regmap;
	struct clk_regmap_hw *rhw;

	rhw = clk_hw_to_clk_regmap_hw(hw);

	dev = clk_hw_get_dev(hw);
	if (!dev)
		return -EINVAL;

	regmap = dev_get_regmap(dev, hw->init->regmap_name);
	if (!regmap)
		return -EINVAL; // Print helpful message
	rhw->regmap = regmap;

	return 0;
}

or we can even make it so that there's clk_hw_init_regmap() and
clk_hw_init_regmap_name() so that drivers can have multiple functions if
the clks need different regmaps.

int my_init_regmap(struct clk_hw *hw)
{
	int ret;

	ret = clk_hw_init_regmap_name(hw, "my_name");
	...
}

If you don't need the multiple regmap support then it's fine to punt
here until later.

> 
> I don't really care which way you want it done, both are pretty easy to
> implement if I can get away with just doing so for the two standard
> clock types that I am using - is it okay to just do those two?
> 

Of course, doing only what is minimally required is better than changing
everything if you're not sure the approach is going to land.

Re: [PATCH v1 08/11] clk: move meson clk-regmap implementation to common code

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2250 bytes --]

On Thu, Nov 14, 2024 at 05:29:54PM -0800, Stephen Boyd wrote:
> Quoting Conor Dooley (2024-11-06 04:56:25)
> > My use case doesn't
> > actually need the registration code changes either as, currently, only reg
> > gets set at runtime, but leaving that out is a level of incomplete I'd not
> > let myself away with.
> > Obviously shoving the extra members into the clk structs has the downside
> > of taking up a pointer and a offset worth of memory for each clock of
> > that type registered, but it is substantially easier to support devices
> > with multiple regmaps that way. Probably moot though since the approach you
> > suggested in the thread linked above that implements a clk_hw_get_regmap()
> > has to store a pointer to the regmap's identifier which would take up an
> > identical amount of memory.
> 
> We don't need to store the regmap identifier in the struct clk. We can
> store it in the 'struct clk_init_data' with some new field, and only do
> that when/if we actually need to. We would need to pass the init data to
> the clk_ops::init() callback though. We currently knock that out during
> registration so that clk_hw->init is NULL. Probably we can just set that
> to NULL after the init routine runs in __clk_core_init().
> 
> Long story short, don't add something to 'struct clk_core', 'struct
> clk', or 'struct clk_hw' for these details. We can have a 'struct
> clk_regmap_hw' that everyone else can build upon:
> 
>   struct clk_regmap_hw {
>         struct regmap *regmap;
>         struct clk_hw hw;
>   };

What's the point of this? I don't understand why you want to do this over
what clk_divider et al already do, where clk_hw and the iomem pointer
are in the struct itself.

> 
> and then set the regmap pointer during registration in
> clk_hw_init_regmap().
> 
> int clk_hw_init_regmap(struct clk_hw *hw)
> {
> 	struct device *dev;
> 	struct regmap *regmap;
> 	struct clk_regmap_hw *rhw;
> 
> 	rhw = clk_hw_to_clk_regmap_hw(hw);
> 
> 	dev = clk_hw_get_dev(hw);
> 	if (!dev)
> 		return -EINVAL;
> 
> 	regmap = dev_get_regmap(dev, hw->init->regmap_name);
> 	if (!regmap)
> 		return -EINVAL; // Print helpful message
> 	rhw->regmap = regmap;
> 
> 	return 0;
> }

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v1 08/11] clk: move meson clk-regmap implementation to common code

[Stephen Boyd]

Quoting Conor Dooley (2024-11-28 02:36:16)
> On Thu, Nov 14, 2024 at 05:29:54PM -0800, Stephen Boyd wrote:
> > Quoting Conor Dooley (2024-11-06 04:56:25)
> > > My use case doesn't
> > > actually need the registration code changes either as, currently, only reg
> > > gets set at runtime, but leaving that out is a level of incomplete I'd not
> > > let myself away with.
> > > Obviously shoving the extra members into the clk structs has the downside
> > > of taking up a pointer and a offset worth of memory for each clock of
> > > that type registered, but it is substantially easier to support devices
> > > with multiple regmaps that way. Probably moot though since the approach you
> > > suggested in the thread linked above that implements a clk_hw_get_regmap()
> > > has to store a pointer to the regmap's identifier which would take up an
> > > identical amount of memory.
> > 
> > We don't need to store the regmap identifier in the struct clk. We can
> > store it in the 'struct clk_init_data' with some new field, and only do
> > that when/if we actually need to. We would need to pass the init data to
> > the clk_ops::init() callback though. We currently knock that out during
> > registration so that clk_hw->init is NULL. Probably we can just set that
> > to NULL after the init routine runs in __clk_core_init().
> > 
> > Long story short, don't add something to 'struct clk_core', 'struct
> > clk', or 'struct clk_hw' for these details. We can have a 'struct
> > clk_regmap_hw' that everyone else can build upon:
> > 
> >   struct clk_regmap_hw {
> >         struct regmap *regmap;
> >         struct clk_hw hw;
> >   };
> 
> What's the point of this? I don't understand why you want to do this over
> what clk_divider et al already do, where clk_hw and the iomem pointer
> are in the struct itself.

Can you give an example? I don't understand what you're suggesting. I
prefer a struct clk_regmap_hw like above so that the existing struct
clk_hw in the kernel aren't increased by a pointer. SoC drivers can use
the same struct as a replacement for their struct clk_hw member today.

Re: [PATCH v1 08/11] clk: move meson clk-regmap implementation to common code

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3265 bytes --]

On Tue, Dec 03, 2024 at 02:50:31PM -0800, Stephen Boyd wrote:
> Quoting Conor Dooley (2024-11-28 02:36:16)
> > On Thu, Nov 14, 2024 at 05:29:54PM -0800, Stephen Boyd wrote:
> > > Quoting Conor Dooley (2024-11-06 04:56:25)
> > > > My use case doesn't
> > > > actually need the registration code changes either as, currently, only reg
> > > > gets set at runtime, but leaving that out is a level of incomplete I'd not
> > > > let myself away with.
> > > > Obviously shoving the extra members into the clk structs has the downside
> > > > of taking up a pointer and a offset worth of memory for each clock of
> > > > that type registered, but it is substantially easier to support devices
> > > > with multiple regmaps that way. Probably moot though since the approach you
> > > > suggested in the thread linked above that implements a clk_hw_get_regmap()
> > > > has to store a pointer to the regmap's identifier which would take up an
> > > > identical amount of memory.
> > > 
> > > We don't need to store the regmap identifier in the struct clk. We can
> > > store it in the 'struct clk_init_data' with some new field, and only do
> > > that when/if we actually need to. We would need to pass the init data to
> > > the clk_ops::init() callback though. We currently knock that out during
> > > registration so that clk_hw->init is NULL. Probably we can just set that
> > > to NULL after the init routine runs in __clk_core_init().
> > > 
> > > Long story short, don't add something to 'struct clk_core', 'struct
> > > clk', or 'struct clk_hw' for these details. We can have a 'struct
> > > clk_regmap_hw' that everyone else can build upon:
> > > 
> > >   struct clk_regmap_hw {
> > >         struct regmap *regmap;
> > >         struct clk_hw hw;
> > >   };
> > 
> > What's the point of this? I don't understand why you want to do this over
> > what clk_divider et al already do, where clk_hw and the iomem pointer
> > are in the struct itself.
> 
> Can you give an example? I don't understand what you're suggesting. I
> prefer a struct clk_regmap_hw like above so that the existing struct
> clk_hw in the kernel aren't increased by a pointer. SoC drivers can use
> the same struct as a replacement for their struct clk_hw member today.

Best example I guess is to link what I did? This one is the core
changes:
https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/commit/?h=syscon-rework-2&id=35904222355e971c24b3eb9b9fad3dd0c38d1393
clk-gate has my original hack that I did while trying to figure out
what you wanted, clk-divider-regmap is a 99% copy of clk-divider with
the types, function names and readl()/writel() implementations modified.
Before your last set of comments I was doing something identical to the
clk-gate change for clk-divider also.
Here's the changes required to my driver to make it work with the
updated:
https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/commit/?h=syscon-rework-2&id=ea40211fe20f8bc6ef0320b93e1baa5b3f244601
It's pretty much a drop in replacement, other than the additional
complexity in probe.

Hopefully that either gets my point across or lets you spot why I don't
understand the benefit of a wrapper around clk_hw.

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v1 08/11] clk: move meson clk-regmap implementation to common code

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3513 bytes --]

Hey Stephen,

Any thoughts on the example I gave below?

On Fri, Dec 06, 2024 at 01:56:08PM +0000, Conor Dooley wrote:
> On Tue, Dec 03, 2024 at 02:50:31PM -0800, Stephen Boyd wrote:
> > Quoting Conor Dooley (2024-11-28 02:36:16)
> > > On Thu, Nov 14, 2024 at 05:29:54PM -0800, Stephen Boyd wrote:
> > > > Quoting Conor Dooley (2024-11-06 04:56:25)
> > > > > My use case doesn't
> > > > > actually need the registration code changes either as, currently, only reg
> > > > > gets set at runtime, but leaving that out is a level of incomplete I'd not
> > > > > let myself away with.
> > > > > Obviously shoving the extra members into the clk structs has the downside
> > > > > of taking up a pointer and a offset worth of memory for each clock of
> > > > > that type registered, but it is substantially easier to support devices
> > > > > with multiple regmaps that way. Probably moot though since the approach you
> > > > > suggested in the thread linked above that implements a clk_hw_get_regmap()
> > > > > has to store a pointer to the regmap's identifier which would take up an
> > > > > identical amount of memory.
> > > > 
> > > > We don't need to store the regmap identifier in the struct clk. We can
> > > > store it in the 'struct clk_init_data' with some new field, and only do
> > > > that when/if we actually need to. We would need to pass the init data to
> > > > the clk_ops::init() callback though. We currently knock that out during
> > > > registration so that clk_hw->init is NULL. Probably we can just set that
> > > > to NULL after the init routine runs in __clk_core_init().
> > > > 
> > > > Long story short, don't add something to 'struct clk_core', 'struct
> > > > clk', or 'struct clk_hw' for these details. We can have a 'struct
> > > > clk_regmap_hw' that everyone else can build upon:
> > > > 
> > > >   struct clk_regmap_hw {
> > > >         struct regmap *regmap;
> > > >         struct clk_hw hw;
> > > >   };
> > > 
> > > What's the point of this? I don't understand why you want to do this over
> > > what clk_divider et al already do, where clk_hw and the iomem pointer
> > > are in the struct itself.
> > 
> > Can you give an example? I don't understand what you're suggesting. I
> > prefer a struct clk_regmap_hw like above so that the existing struct
> > clk_hw in the kernel aren't increased by a pointer. SoC drivers can use
> > the same struct as a replacement for their struct clk_hw member today.
> 
> Best example I guess is to link what I did? This one is the core
> changes:
> https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/commit/?h=syscon-rework-2&id=35904222355e971c24b3eb9b9fad3dd0c38d1393
> clk-gate has my original hack that I did while trying to figure out
> what you wanted, clk-divider-regmap is a 99% copy of clk-divider with
> the types, function names and readl()/writel() implementations modified.
> Before your last set of comments I was doing something identical to the
> clk-gate change for clk-divider also.
> Here's the changes required to my driver to make it work with the
> updated:
> https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/commit/?h=syscon-rework-2&id=ea40211fe20f8bc6ef0320b93e1baa5b3f244601
> It's pretty much a drop in replacement, other than the additional
> complexity in probe.
> 
> Hopefully that either gets my point across or lets you spot why I don't
> understand the benefit of a wrapper around clk_hw.
> 
> Cheers,
> Conor.



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v1 08/11] clk: move meson clk-regmap implementation to common code

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 4217 bytes --]

On Tue, Jan 21, 2025 at 05:38:53PM +0000, Conor Dooley wrote:
> Hey Stephen,
> 
> Any thoughts on the example I gave below?

I'll give you a few more days to comment, and then I'll just send a
fresh revision, implemented as below. The links seem to have expired in
a rebase along the way, so I've provided some fresh links.

Cheers,
Conor.

> 
> On Fri, Dec 06, 2024 at 01:56:08PM +0000, Conor Dooley wrote:
> > On Tue, Dec 03, 2024 at 02:50:31PM -0800, Stephen Boyd wrote:
> > > Quoting Conor Dooley (2024-11-28 02:36:16)
> > > > On Thu, Nov 14, 2024 at 05:29:54PM -0800, Stephen Boyd wrote:
> > > > > Quoting Conor Dooley (2024-11-06 04:56:25)
> > > > > > My use case doesn't
> > > > > > actually need the registration code changes either as, currently, only reg
> > > > > > gets set at runtime, but leaving that out is a level of incomplete I'd not
> > > > > > let myself away with.
> > > > > > Obviously shoving the extra members into the clk structs has the downside
> > > > > > of taking up a pointer and a offset worth of memory for each clock of
> > > > > > that type registered, but it is substantially easier to support devices
> > > > > > with multiple regmaps that way. Probably moot though since the approach you
> > > > > > suggested in the thread linked above that implements a clk_hw_get_regmap()
> > > > > > has to store a pointer to the regmap's identifier which would take up an
> > > > > > identical amount of memory.
> > > > > 
> > > > > We don't need to store the regmap identifier in the struct clk. We can
> > > > > store it in the 'struct clk_init_data' with some new field, and only do
> > > > > that when/if we actually need to. We would need to pass the init data to
> > > > > the clk_ops::init() callback though. We currently knock that out during
> > > > > registration so that clk_hw->init is NULL. Probably we can just set that
> > > > > to NULL after the init routine runs in __clk_core_init().
> > > > > 
> > > > > Long story short, don't add something to 'struct clk_core', 'struct
> > > > > clk', or 'struct clk_hw' for these details. We can have a 'struct
> > > > > clk_regmap_hw' that everyone else can build upon:
> > > > > 
> > > > >   struct clk_regmap_hw {
> > > > >         struct regmap *regmap;
> > > > >         struct clk_hw hw;
> > > > >   };
> > > > 
> > > > What's the point of this? I don't understand why you want to do this over
> > > > what clk_divider et al already do, where clk_hw and the iomem pointer
> > > > are in the struct itself.
> > > 
> > > Can you give an example? I don't understand what you're suggesting. I
> > > prefer a struct clk_regmap_hw like above so that the existing struct
> > > clk_hw in the kernel aren't increased by a pointer. SoC drivers can use
> > > the same struct as a replacement for their struct clk_hw member today.
> > 
> > Best example I guess is to link what I did? This one is the core
> > changes:
> > https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/commit/?h=syscon-rework-2&id=35904222355e971c24b3eb9b9fad3dd0c38d1393

https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/commit/?h=syscon-rework-2&id=435c8eb223ee804297a0491fae2b00d3d5a9c773

> > clk-gate has my original hack that I did while trying to figure out
> > what you wanted, clk-divider-regmap is a 99% copy of clk-divider with
> > the types, function names and readl()/writel() implementations modified.
> > Before your last set of comments I was doing something identical to the
> > clk-gate change for clk-divider also.
> > Here's the changes required to my driver to make it work with the
> > updated:
> > https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/commit/?h=syscon-rework-2&id=ea40211fe20f8bc6ef0320b93e1baa5b3f244601

https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/commit/?h=syscon-rework-2&id=f55e907e93c55c943725dd62c2fc7dc76cdbd8d5

> > It's pretty much a drop in replacement, other than the additional
> > complexity in probe.
> > 
> > Hopefully that either gets my point across or lets you spot why I don't
> > understand the benefit of a wrapper around clk_hw.
> > 
> > Cheers,
> > Conor.
> 
> 



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]