[PATCH v3 0/4] Add reset support to EN7581 clk driver

[PATCH v3 0/4] Add reset support to EN7581 clk driver

[Lorenzo Bianconi]

Introduce reset-controller support to the Airoha EN7581 clock module.

Changes since v2:
- move reset io registers in a dedicated mapping since upcoming pinctrl driver
  will need to map some registers in the adjacent region
- drop patch 2/4
- remove pcie reset open drain configuration since it will be managed by
  upcoming pinctrl driver
Changes since v1:
- squash patch 1/5 and 2/5
- introduce reset line mapping in order to take into account possible holes in
  reset definitions
- fix error path in en7523_clk_probe()

Lorenzo Bianconi (4):
  dt-bindings: clock: airoha: Add reset support to EN7581 clock binding
  clk: en7523: Add reset-controller support for EN7581 SoC
  clk: en7523: Remove pcie prepare/unpreare callbacks for EN7581 SoC
  clk: en7523: remove pcie reset open drain configuration for EN7581

 .../bindings/clock/airoha,en7523-scu.yaml     |  25 +-
 drivers/clk/clk-en7523.c                      | 253 ++++++++++++++----
 .../dt-bindings/reset/airoha,en7581-reset.h   |  66 +++++
 3 files changed, 291 insertions(+), 53 deletions(-)
 create mode 100644 include/dt-bindings/reset/airoha,en7581-reset.h

-- 
2.45.1

[PATCH v3 1/4] dt-bindings: clock: airoha: Add reset support to EN7581 clock binding

[Lorenzo Bianconi]

Introduce reset capability to EN7581 device-tree clock binding
documentation.

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 .../bindings/clock/airoha,en7523-scu.yaml     | 25 ++++++-
 .../dt-bindings/reset/airoha,en7581-reset.h   | 66 +++++++++++++++++++
 2 files changed, 90 insertions(+), 1 deletion(-)
 create mode 100644 include/dt-bindings/reset/airoha,en7581-reset.h

diff --git a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
index 3f4266637733..84353fd09428 100644
--- a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
+++ b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
@@ -35,7 +35,7 @@ properties:
 
   reg:
     minItems: 2
-    maxItems: 3
+    maxItems: 4
 
   "#clock-cells":
     description:
@@ -43,6 +43,10 @@ properties:
       clocks.
     const: 1
 
+  '#reset-cells':
+    description: ID of the controller reset line
+    const: 1
+
 required:
   - compatible
   - reg
@@ -60,6 +64,8 @@ allOf:
             - description: scu base address
             - description: misc scu base address
 
+        '#reset-cells': false
+
   - if:
       properties:
         compatible:
@@ -70,6 +76,7 @@ allOf:
           items:
             - description: scu base address
             - description: misc scu base address
+            - description: reset base address
             - description: pb scu base address
 
 additionalProperties: false
@@ -83,3 +90,19 @@ examples:
             <0x1fb00000 0x1000>;
       #clock-cells = <1>;
     };
+
+  - |
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;
+
+      scuclk: clock-controller@1fa20000 {
+        compatible = "airoha,en7581-scu";
+        reg = <0x0 0x1fa20000 0x0 0x400>,
+              <0x0 0x1fb00000 0x0 0x90>,
+              <0x0 0x1fb00830 0x0 0x8>,
+              <0x0 0x1fbe3400 0x0 0xfc>;
+              #clock-cells = <1>;
+              #reset-cells = <1>;
+      };
+    };
diff --git a/include/dt-bindings/reset/airoha,en7581-reset.h b/include/dt-bindings/reset/airoha,en7581-reset.h
new file mode 100644
index 000000000000..6544a1790b83
--- /dev/null
+++ b/include/dt-bindings/reset/airoha,en7581-reset.h
@@ -0,0 +1,66 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2024 AIROHA Inc
+ * Author: Lorenzo Bianconi <lorenzo@kernel.org>
+ */
+
+#ifndef __DT_BINDINGS_RESET_CONTROLLER_AIROHA_EN7581_H_
+#define __DT_BINDINGS_RESET_CONTROLLER_AIROHA_EN7581_H_
+
+/* RST_CTRL2 */
+#define EN7581_XPON_PHY_RST		 0
+#define EN7581_CPU_TIMER2_RST		 1
+#define EN7581_HSUART_RST		 2
+#define EN7581_UART4_RST		 3
+#define EN7581_UART5_RST		 4
+#define EN7581_I2C2_RST			 5
+#define EN7581_XSI_MAC_RST		 6
+#define EN7581_XSI_PHY_RST		 7
+#define EN7581_NPU_RST			 8
+#define EN7581_I2S_RST			 9
+#define EN7581_TRNG_RST			10
+#define EN7581_TRNG_MSTART_RST		11
+#define EN7581_DUAL_HSI0_RST		12
+#define EN7581_DUAL_HSI1_RST		13
+#define EN7581_HSI_RST			14
+#define EN7581_DUAL_HSI0_MAC_RST	15
+#define EN7581_DUAL_HSI1_MAC_RST	16
+#define EN7581_HSI_MAC_RST		17
+#define EN7581_WDMA_RST			18
+#define EN7581_WOE0_RST			19
+#define EN7581_WOE1_RST			20
+#define EN7581_HSDMA_RST		21
+#define EN7581_TDMA_RST			22
+#define EN7581_EMMC_RST			23
+#define EN7581_SOE_RST			24
+#define EN7581_PCIE2_RST		25
+#define EN7581_XFP_MAC_RST		26
+#define EN7581_USB_HOST_P1_RST		27
+#define EN7581_USB_HOST_P1_U3_PHY_RST	28
+/* RST_CTRL1 */
+#define EN7581_PCM1_ZSI_ISI_RST		29
+#define EN7581_FE_PDMA_RST		30
+#define EN7581_FE_QDMA_RST		31
+#define EN7581_PCM_SPIWP_RST		32
+#define EN7581_CRYPTO_RST		33
+#define EN7581_TIMER_RST		34
+#define EN7581_PCM1_RST			35
+#define EN7581_UART_RST			36
+#define EN7581_GPIO_RST			37
+#define EN7581_GDMA_RST			38
+#define EN7581_I2C_MASTER_RST		39
+#define EN7581_PCM2_ZSI_ISI_RST		40
+#define EN7581_SFC_RST			41
+#define EN7581_UART2_RST		42
+#define EN7581_GDMP_RST			43
+#define EN7581_FE_RST			44
+#define EN7581_USB_HOST_P0_RST		45
+#define EN7581_GSW_RST			46
+#define EN7581_SFC2_PCM_RST		47
+#define EN7581_PCIE0_RST		48
+#define EN7581_PCIE1_RST		49
+#define EN7581_CPU_TIMER_RST		50
+#define EN7581_PCIE_HB_RST		51
+#define EN7581_XPON_MAC_RST		52
+
+#endif /* __DT_BINDINGS_RESET_CONTROLLER_AIROHA_EN7581_H_ */
-- 
2.45.1

[PATCH v3 2/4] clk: en7523: Add reset-controller support for EN7581 SoC

[Lorenzo Bianconi]

Introduce reset API support to EN7581 clock driver.

Tested-by: Zhengping Zhang <zhengping.zhang@airoha.com>
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/clk/clk-en7523.c | 200 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 197 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/clk-en7523.c b/drivers/clk/clk-en7523.c
index ccc394692671..00638714fe08 100644
--- a/drivers/clk/clk-en7523.c
+++ b/drivers/clk/clk-en7523.c
@@ -5,7 +5,11 @@
 #include <linux/io.h>
 #include <linux/platform_device.h>
 #include <linux/property.h>
+#include <linux/reset-controller.h>
 #include <dt-bindings/clock/en7523-clk.h>
+#include <dt-bindings/reset/airoha,en7581-reset.h>
+
+#define RST_NR_PER_BANK			32
 
 #define REG_PCI_CONTROL			0x88
 #define   REG_PCI_CONTROL_PERSTOUT	BIT(29)
@@ -40,6 +44,9 @@
 #define REG_PCIE_XSI0_SEL_MASK		GENMASK(14, 13)
 #define REG_PCIE_XSI1_SEL_MASK		GENMASK(12, 11)
 
+#define REG_RST_CTRL2			0x00
+#define REG_RST_CTRL1			0x04
+
 struct en_clk_desc {
 	int id;
 	const char *name;
@@ -64,8 +71,20 @@ struct en_clk_gate {
 	struct clk_hw hw;
 };
 
+struct en_rst_data {
+	const u16 *bank_ofs;
+	const u16 *idx_map;
+	void __iomem *base;
+	struct reset_controller_dev rcdev;
+};
+
 struct en_clk_soc_data {
 	const struct clk_ops pcie_ops;
+	struct {
+		const u16 *bank_ofs;
+		const u16 *idx_map;
+		u16 idx_map_nr;
+	} reset;
 	int (*hw_init)(struct platform_device *pdev, void __iomem *base,
 		       void __iomem *np_base);
 };
@@ -168,6 +187,69 @@ static const struct en_clk_desc en7523_base_clks[] = {
 	}
 };
 
+static const u16 en7581_rst_ofs[] = {
+	REG_RST_CTRL2,
+	REG_RST_CTRL1,
+};
+
+static const u16 en7581_rst_map[] = {
+	/* RST_CTRL2 */
+	[EN7581_XPON_PHY_RST]		= 0,
+	[EN7581_CPU_TIMER2_RST]		= 2,
+	[EN7581_HSUART_RST]		= 3,
+	[EN7581_UART4_RST]		= 4,
+	[EN7581_UART5_RST]		= 5,
+	[EN7581_I2C2_RST]		= 6,
+	[EN7581_XSI_MAC_RST]		= 7,
+	[EN7581_XSI_PHY_RST]		= 8,
+	[EN7581_NPU_RST]		= 9,
+	[EN7581_I2S_RST]		= 10,
+	[EN7581_TRNG_RST]		= 11,
+	[EN7581_TRNG_MSTART_RST]	= 12,
+	[EN7581_DUAL_HSI0_RST]		= 13,
+	[EN7581_DUAL_HSI1_RST]		= 14,
+	[EN7581_HSI_RST]		= 15,
+	[EN7581_DUAL_HSI0_MAC_RST]	= 16,
+	[EN7581_DUAL_HSI1_MAC_RST]	= 17,
+	[EN7581_HSI_MAC_RST]		= 18,
+	[EN7581_WDMA_RST]		= 19,
+	[EN7581_WOE0_RST]		= 20,
+	[EN7581_WOE1_RST]		= 21,
+	[EN7581_HSDMA_RST]		= 22,
+	[EN7581_TDMA_RST]		= 24,
+	[EN7581_EMMC_RST]		= 25,
+	[EN7581_SOE_RST]		= 26,
+	[EN7581_PCIE2_RST]		= 27,
+	[EN7581_XFP_MAC_RST]		= 28,
+	[EN7581_USB_HOST_P1_RST]	= 29,
+	[EN7581_USB_HOST_P1_U3_PHY_RST]	= 30,
+	/* RST_CTRL1 */
+	[EN7581_PCM1_ZSI_ISI_RST]	= RST_NR_PER_BANK + 0,
+	[EN7581_FE_PDMA_RST]		= RST_NR_PER_BANK + 1,
+	[EN7581_FE_QDMA_RST]		= RST_NR_PER_BANK + 2,
+	[EN7581_PCM_SPIWP_RST]		= RST_NR_PER_BANK + 4,
+	[EN7581_CRYPTO_RST]		= RST_NR_PER_BANK + 6,
+	[EN7581_TIMER_RST]		= RST_NR_PER_BANK + 8,
+	[EN7581_PCM1_RST]		= RST_NR_PER_BANK + 11,
+	[EN7581_UART_RST]		= RST_NR_PER_BANK + 12,
+	[EN7581_GPIO_RST]		= RST_NR_PER_BANK + 13,
+	[EN7581_GDMA_RST]		= RST_NR_PER_BANK + 14,
+	[EN7581_I2C_MASTER_RST]		= RST_NR_PER_BANK + 16,
+	[EN7581_PCM2_ZSI_ISI_RST]	= RST_NR_PER_BANK + 17,
+	[EN7581_SFC_RST]		= RST_NR_PER_BANK + 18,
+	[EN7581_UART2_RST]		= RST_NR_PER_BANK + 19,
+	[EN7581_GDMP_RST]		= RST_NR_PER_BANK + 20,
+	[EN7581_FE_RST]			= RST_NR_PER_BANK + 21,
+	[EN7581_USB_HOST_P0_RST]	= RST_NR_PER_BANK + 22,
+	[EN7581_GSW_RST]		= RST_NR_PER_BANK + 23,
+	[EN7581_SFC2_PCM_RST]		= RST_NR_PER_BANK + 25,
+	[EN7581_PCIE0_RST]		= RST_NR_PER_BANK + 26,
+	[EN7581_PCIE1_RST]		= RST_NR_PER_BANK + 27,
+	[EN7581_CPU_TIMER_RST]		= RST_NR_PER_BANK + 28,
+	[EN7581_PCIE_HB_RST]		= RST_NR_PER_BANK + 29,
+	[EN7581_XPON_MAC_RST]		= RST_NR_PER_BANK + 31,
+};
+
 static unsigned int en7523_get_base_rate(void __iomem *base, unsigned int i)
 {
 	const struct en_clk_desc *desc = &en7523_base_clks[i];
@@ -370,7 +452,7 @@ static int en7581_clk_hw_init(struct platform_device *pdev,
 	void __iomem *pb_base;
 	u32 val;
 
-	pb_base = devm_platform_ioremap_resource(pdev, 2);
+	pb_base = devm_platform_ioremap_resource(pdev, 3);
 	if (IS_ERR(pb_base))
 		return PTR_ERR(pb_base);
 
@@ -423,6 +505,102 @@ static void en7523_register_clocks(struct device *dev, struct clk_hw_onecell_dat
 	clk_data->num = EN7523_NUM_CLOCKS;
 }
 
+static int en7523_reset_update(struct reset_controller_dev *rcdev,
+			       unsigned long id, bool assert)
+{
+	struct en_rst_data *rst_data;
+	void __iomem *addr;
+	u32 val;
+
+	rst_data = container_of(rcdev, struct en_rst_data, rcdev);
+	addr = rst_data->base + rst_data->bank_ofs[id / RST_NR_PER_BANK];
+
+	val = readl(addr);
+	if (assert)
+		val |= BIT(id % RST_NR_PER_BANK);
+	else
+		val &= ~BIT(id % RST_NR_PER_BANK);
+	writel(val, addr);
+
+	return 0;
+}
+
+static int en7523_reset_assert(struct reset_controller_dev *rcdev,
+			       unsigned long id)
+{
+	return en7523_reset_update(rcdev, id, true);
+}
+
+static int en7523_reset_deassert(struct reset_controller_dev *rcdev,
+				 unsigned long id)
+{
+	return en7523_reset_update(rcdev, id, false);
+}
+
+static int en7523_reset_status(struct reset_controller_dev *rcdev,
+			       unsigned long id)
+{
+	struct en_rst_data *rst_data;
+	void __iomem *addr;
+
+	rst_data = container_of(rcdev, struct en_rst_data, rcdev);
+	addr = rst_data->base + rst_data->bank_ofs[id / RST_NR_PER_BANK];
+
+	return !!(readl(addr) & BIT(id % RST_NR_PER_BANK));
+}
+
+static int en7523_reset_xlate(struct reset_controller_dev *rcdev,
+			      const struct of_phandle_args *reset_spec)
+{
+	struct en_rst_data *rst_data;
+
+	rst_data = container_of(rcdev, struct en_rst_data, rcdev);
+	if (reset_spec->args[0] >= rcdev->nr_resets)
+		return -EINVAL;
+
+	return rst_data->idx_map[reset_spec->args[0]];
+}
+
+static const struct reset_control_ops en7523_reset_ops = {
+	.assert = en7523_reset_assert,
+	.deassert = en7523_reset_deassert,
+	.status = en7523_reset_status,
+};
+
+static int en7523_reset_register(struct platform_device *pdev,
+				 const struct en_clk_soc_data *soc_data)
+{
+	struct device *dev = &pdev->dev;
+	struct en_rst_data *rst_data;
+	void __iomem *base;
+
+	/* no reset lines available */
+	if (!soc_data->reset.idx_map_nr)
+		return 0;
+
+	base = devm_platform_ioremap_resource(pdev, 2);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	rst_data = devm_kzalloc(dev, sizeof(*rst_data), GFP_KERNEL);
+	if (!rst_data)
+		return -ENOMEM;
+
+	rst_data->bank_ofs = soc_data->reset.bank_ofs;
+	rst_data->idx_map = soc_data->reset.idx_map;
+	rst_data->base = base;
+
+	rst_data->rcdev.nr_resets = soc_data->reset.idx_map_nr;
+	rst_data->rcdev.of_xlate = en7523_reset_xlate;
+	rst_data->rcdev.ops = &en7523_reset_ops;
+	rst_data->rcdev.of_node = dev->of_node;
+	rst_data->rcdev.of_reset_n_cells = 1;
+	rst_data->rcdev.owner = THIS_MODULE;
+	rst_data->rcdev.dev = dev;
+
+	return devm_reset_controller_register(dev, &rst_data->rcdev);
+}
+
 static int en7523_clk_probe(struct platform_device *pdev)
 {
 	struct device_node *node = pdev->dev.of_node;
@@ -455,12 +633,23 @@ static int en7523_clk_probe(struct platform_device *pdev)
 	en7523_register_clocks(&pdev->dev, clk_data, base, np_base);
 
 	r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_data);
-	if (r)
+	if (r) {
 		dev_err(&pdev->dev,
 			"could not register clock provider: %s: %d\n",
 			pdev->name, r);
+		return r;
+	}
 
-	return r;
+	r = en7523_reset_register(pdev, soc_data);
+	if (r) {
+		dev_err(&pdev->dev,
+			"could not register reset controller: %s: %d\n",
+			pdev->name, r);
+		of_clk_del_provider(node);
+		return r;
+	}
+
+	return 0;
 }
 
 static const struct en_clk_soc_data en7523_data = {
@@ -479,6 +668,11 @@ static const struct en_clk_soc_data en7581_data = {
 		.unprepare = en7581_pci_unprepare,
 		.disable = en7581_pci_disable,
 	},
+	.reset = {
+		.bank_ofs = en7581_rst_ofs,
+		.idx_map = en7581_rst_map,
+		.idx_map_nr = ARRAY_SIZE(en7581_rst_map),
+	},
 	.hw_init = en7581_clk_hw_init,
 };
 
-- 
2.45.1

[PATCH v3 3/4] clk: en7523: Remove pcie prepare/unpreare callbacks for EN7581 SoC

[Lorenzo Bianconi]

Get rid of prepare and unpreare callbacks for PCIe clock since they can
be modeled as a reset line cosumed by the PCIe driver
(pcie-mediatek-gen3)

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Tested-by: Zhengping Zhang <zhengping.zhang@airoha.com>
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/clk/clk-en7523.c | 41 ++--------------------------------------
 1 file changed, 2 insertions(+), 39 deletions(-)

diff --git a/drivers/clk/clk-en7523.c b/drivers/clk/clk-en7523.c
index 00638714fe08..ad8a4d1ad62c 100644
--- a/drivers/clk/clk-en7523.c
+++ b/drivers/clk/clk-en7523.c
@@ -361,9 +361,8 @@ static struct clk_hw *en7523_register_pcie_clk(struct device *dev,
 	cg->base = np_base;
 	cg->hw.init = &init;
 
-	if (init.ops->disable)
-		init.ops->disable(&cg->hw);
-	init.ops->unprepare(&cg->hw);
+	if (init.ops->unprepare)
+		init.ops->unprepare(&cg->hw);
 
 	if (clk_hw_register(dev, &cg->hw))
 		return NULL;
@@ -381,23 +380,6 @@ static int en7581_pci_is_enabled(struct clk_hw *hw)
 	return (val & mask) == mask;
 }
 
-static int en7581_pci_prepare(struct clk_hw *hw)
-{
-	struct en_clk_gate *cg = container_of(hw, struct en_clk_gate, hw);
-	void __iomem *np_base = cg->base;
-	u32 val, mask;
-
-	mask = REG_RESET_CONTROL_PCIE1 | REG_RESET_CONTROL_PCIE2 |
-	       REG_RESET_CONTROL_PCIEHB;
-	val = readl(np_base + REG_RESET_CONTROL1);
-	writel(val & ~mask, np_base + REG_RESET_CONTROL1);
-	val = readl(np_base + REG_RESET_CONTROL2);
-	writel(val & ~REG_RESET2_CONTROL_PCIE2, np_base + REG_RESET_CONTROL2);
-	usleep_range(5000, 10000);
-
-	return 0;
-}
-
 static int en7581_pci_enable(struct clk_hw *hw)
 {
 	struct en_clk_gate *cg = container_of(hw, struct en_clk_gate, hw);
@@ -414,23 +396,6 @@ static int en7581_pci_enable(struct clk_hw *hw)
 	return 0;
 }
 
-static void en7581_pci_unprepare(struct clk_hw *hw)
-{
-	struct en_clk_gate *cg = container_of(hw, struct en_clk_gate, hw);
-	void __iomem *np_base = cg->base;
-	u32 val, mask;
-
-	mask = REG_RESET_CONTROL_PCIE1 | REG_RESET_CONTROL_PCIE2 |
-	       REG_RESET_CONTROL_PCIEHB;
-	val = readl(np_base + REG_RESET_CONTROL1);
-	writel(val | mask, np_base + REG_RESET_CONTROL1);
-	mask = REG_RESET_CONTROL_PCIE1 | REG_RESET_CONTROL_PCIE2;
-	writel(val | mask, np_base + REG_RESET_CONTROL1);
-	val = readl(np_base + REG_RESET_CONTROL2);
-	writel(val | REG_RESET_CONTROL_PCIE2, np_base + REG_RESET_CONTROL2);
-	msleep(100);
-}
-
 static void en7581_pci_disable(struct clk_hw *hw)
 {
 	struct en_clk_gate *cg = container_of(hw, struct en_clk_gate, hw);
@@ -663,9 +628,7 @@ static const struct en_clk_soc_data en7523_data = {
 static const struct en_clk_soc_data en7581_data = {
 	.pcie_ops = {
 		.is_enabled = en7581_pci_is_enabled,
-		.prepare = en7581_pci_prepare,
 		.enable = en7581_pci_enable,
-		.unprepare = en7581_pci_unprepare,
 		.disable = en7581_pci_disable,
 	},
 	.reset = {
-- 
2.45.1

[PATCH v3 4/4] clk: en7523: remove pcie reset open drain configuration for EN7581

[Lorenzo Bianconi]

PCIE reset open drain configuration will be managed by pinctrl driver.

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/clk/clk-en7523.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/clk/clk-en7523.c b/drivers/clk/clk-en7523.c
index ad8a4d1ad62c..9757023601c5 100644
--- a/drivers/clk/clk-en7523.c
+++ b/drivers/clk/clk-en7523.c
@@ -37,8 +37,6 @@
 #define REG_PCIE1_MEM_MASK		0x0c
 #define REG_PCIE2_MEM			0x10
 #define REG_PCIE2_MEM_MASK		0x14
-#define REG_PCIE_RESET_OPEN_DRAIN	0x018c
-#define REG_PCIE_RESET_OPEN_DRAIN_MASK	GENMASK(2, 0)
 #define REG_NP_SCU_PCIC			0x88
 #define REG_NP_SCU_SSTR			0x9c
 #define REG_PCIE_XSI0_SEL_MASK		GENMASK(14, 13)
@@ -85,8 +83,7 @@ struct en_clk_soc_data {
 		const u16 *idx_map;
 		u16 idx_map_nr;
 	} reset;
-	int (*hw_init)(struct platform_device *pdev, void __iomem *base,
-		       void __iomem *np_base);
+	int (*hw_init)(struct platform_device *pdev, void __iomem *np_base);
 };
 
 static const u32 gsw_base[] = { 400000000, 500000000 };
@@ -411,7 +408,6 @@ static void en7581_pci_disable(struct clk_hw *hw)
 }
 
 static int en7581_clk_hw_init(struct platform_device *pdev,
-			      void __iomem *base,
 			      void __iomem *np_base)
 {
 	void __iomem *pb_base;
@@ -434,10 +430,6 @@ static int en7581_clk_hw_init(struct platform_device *pdev,
 	writel(0x28000000, pb_base + REG_PCIE2_MEM);
 	writel(0xfc000000, pb_base + REG_PCIE2_MEM_MASK);
 
-	val = readl(base + REG_PCIE_RESET_OPEN_DRAIN);
-	writel(val | REG_PCIE_RESET_OPEN_DRAIN_MASK,
-	       base + REG_PCIE_RESET_OPEN_DRAIN);
-
 	return 0;
 }
 
@@ -584,7 +576,7 @@ static int en7523_clk_probe(struct platform_device *pdev)
 
 	soc_data = device_get_match_data(&pdev->dev);
 	if (soc_data->hw_init) {
-		r = soc_data->hw_init(pdev, base, np_base);
+		r = soc_data->hw_init(pdev, np_base);
 		if (r)
 			return r;
 	}
-- 
2.45.1

Re: [PATCH v3 1/4] dt-bindings: clock: airoha: Add reset support to EN7581 clock binding

[Rob Herring (Arm)]

On Thu, 13 Jun 2024 14:47:03 +0200, Lorenzo Bianconi wrote:
> Introduce reset capability to EN7581 device-tree clock binding
> documentation.
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
>  .../bindings/clock/airoha,en7523-scu.yaml     | 25 ++++++-
>  .../dt-bindings/reset/airoha,en7581-reset.h   | 66 +++++++++++++++++++
>  2 files changed, 90 insertions(+), 1 deletion(-)
>  create mode 100644 include/dt-bindings/reset/airoha,en7581-reset.h
> 

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

Re: [PATCH v3 2/4] clk: en7523: Add reset-controller support for EN7581 SoC

[AngeloGioacchino Del Regno]

Il 13/06/24 14:47, Lorenzo Bianconi ha scritto:
> Introduce reset API support to EN7581 clock driver.
> 
> Tested-by: Zhengping Zhang <zhengping.zhang@airoha.com>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
>   drivers/clk/clk-en7523.c | 200 ++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 197 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clk/clk-en7523.c b/drivers/clk/clk-en7523.c
> index ccc394692671..00638714fe08 100644
> --- a/drivers/clk/clk-en7523.c
> +++ b/drivers/clk/clk-en7523.c

..snip..

> @@ -423,6 +505,102 @@ static void en7523_register_clocks(struct device *dev, struct clk_hw_onecell_dat
>   	clk_data->num = EN7523_NUM_CLOCKS;
>   }
>   
> +static int en7523_reset_update(struct reset_controller_dev *rcdev,
> +			       unsigned long id, bool assert)
> +{
> +	struct en_rst_data *rst_data;
> +	void __iomem *addr;

struct en_rst_data *rst_data = container_of(rcdev, struct en_rst_data, rcdev);
void __iomem *addr = rst_data->base + rst_data->bank_ofs[id / RST_NR_PER_BANK];
u32 val;

val = readl(addr);
etc etc

> +	u32 val;
> +
> +	rst_data = container_of(rcdev, struct en_rst_data, rcdev);
> +	addr = rst_data->base + rst_data->bank_ofs[id / RST_NR_PER_BANK];
> +
> +	val = readl(addr);
> +	if (assert)
> +		val |= BIT(id % RST_NR_PER_BANK);
> +	else
> +		val &= ~BIT(id % RST_NR_PER_BANK);
> +	writel(val, addr);
> +
> +	return 0;
> +}
> +
> +static int en7523_reset_assert(struct reset_controller_dev *rcdev,
> +			       unsigned long id)
> +{
> +	return en7523_reset_update(rcdev, id, true);
> +}
> +
> +static int en7523_reset_deassert(struct reset_controller_dev *rcdev,
> +				 unsigned long id)
> +{
> +	return en7523_reset_update(rcdev, id, false);
> +}
> +
> +static int en7523_reset_status(struct reset_controller_dev *rcdev,
> +			       unsigned long id)
> +{
> +	struct en_rst_data *rst_data;
> +	void __iomem *addr;
> +
> +	rst_data = container_of(rcdev, struct en_rst_data, rcdev);
> +	addr = rst_data->base + rst_data->bank_ofs[id / RST_NR_PER_BANK];

same here, just init while declaring.

> +
> +	return !!(readl(addr) & BIT(id % RST_NR_PER_BANK));
> +}
> +
> +static int en7523_reset_xlate(struct reset_controller_dev *rcdev,
> +			      const struct of_phandle_args *reset_spec)
> +{
> +	struct en_rst_data *rst_data;
> +
> +	rst_data = container_of(rcdev, struct en_rst_data, rcdev);

ditto

> +	if (reset_spec->args[0] >= rcdev->nr_resets)
> +		return -EINVAL;
> +
> +	return rst_data->idx_map[reset_spec->args[0]];
> +}
> +
> +static const struct reset_control_ops en7523_reset_ops = {
> +	.assert = en7523_reset_assert,
> +	.deassert = en7523_reset_deassert,
> +	.status = en7523_reset_status,
> +};
> +
> +static int en7523_reset_register(struct platform_device *pdev,
> +				 const struct en_clk_soc_data *soc_data)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct en_rst_data *rst_data;
> +	void __iomem *base;
> +
> +	/* no reset lines available */
> +	if (!soc_data->reset.idx_map_nr)
> +		return 0;
> +
> +	base = devm_platform_ioremap_resource(pdev, 2);
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);
> +
> +	rst_data = devm_kzalloc(dev, sizeof(*rst_data), GFP_KERNEL);
> +	if (!rst_data)
> +		return -ENOMEM;
> +
> +	rst_data->bank_ofs = soc_data->reset.bank_ofs;
> +	rst_data->idx_map = soc_data->reset.idx_map;
> +	rst_data->base = base;
> +
> +	rst_data->rcdev.nr_resets = soc_data->reset.idx_map_nr;
> +	rst_data->rcdev.of_xlate = en7523_reset_xlate;
> +	rst_data->rcdev.ops = &en7523_reset_ops;
> +	rst_data->rcdev.of_node = dev->of_node;
> +	rst_data->rcdev.of_reset_n_cells = 1;
> +	rst_data->rcdev.owner = THIS_MODULE;
> +	rst_data->rcdev.dev = dev;
> +
> +	return devm_reset_controller_register(dev, &rst_data->rcdev);
> +}
> +
>   static int en7523_clk_probe(struct platform_device *pdev)
>   {
>   	struct device_node *node = pdev->dev.of_node;
> @@ -455,12 +633,23 @@ static int en7523_clk_probe(struct platform_device *pdev)
>   	en7523_register_clocks(&pdev->dev, clk_data, base, np_base);
>   
>   	r = of_clk_add_hw_provider(node, of_clk_hw_onecell_get, clk_data);
> -	if (r)
> +	if (r) {

it's simpler if you just use dev_err_probe() at this point...

if (r)
	return dev_err_probe(.......)

>   		dev_err(&pdev->dev,
>   			"could not register clock provider: %s: %d\n",
>   			pdev->name, r);
> +		return r;
> +	}
>   
> -	return r;
> +	r = en7523_reset_register(pdev, soc_data);
> +	if (r) {

if (r) {
	of_clk_del_provider....
	return dev_err_probe(......)
};

after which

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH v3 4/4] clk: en7523: remove pcie reset open drain configuration for EN7581

[AngeloGioacchino Del Regno]

Il 13/06/24 14:47, Lorenzo Bianconi ha scritto:
> PCIE reset open drain configuration will be managed by pinctrl driver.
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH v3 1/4] dt-bindings: clock: airoha: Add reset support to EN7581 clock binding

[AngeloGioacchino Del Regno]

Il 13/06/24 14:47, Lorenzo Bianconi ha scritto:
> Introduce reset capability to EN7581 device-tree clock binding
> documentation.
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
>   .../bindings/clock/airoha,en7523-scu.yaml     | 25 ++++++-
>   .../dt-bindings/reset/airoha,en7581-reset.h   | 66 +++++++++++++++++++
>   2 files changed, 90 insertions(+), 1 deletion(-)
>   create mode 100644 include/dt-bindings/reset/airoha,en7581-reset.h
> 
> diff --git a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
> index 3f4266637733..84353fd09428 100644
> --- a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
> +++ b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
> @@ -35,7 +35,7 @@ properties:
>   
>     reg:
>       minItems: 2
> -    maxItems: 3
> +    maxItems: 4
>   
>     "#clock-cells":
>       description:
> @@ -43,6 +43,10 @@ properties:
>         clocks.
>       const: 1
>   
> +  '#reset-cells':
> +    description: ID of the controller reset line
> +    const: 1
> +
>   required:
>     - compatible
>     - reg
> @@ -60,6 +64,8 @@ allOf:
>               - description: scu base address
>               - description: misc scu base address
>   
> +        '#reset-cells': false
> +
>     - if:
>         properties:
>           compatible:
> @@ -70,6 +76,7 @@ allOf:
>             items:
>               - description: scu base address
>               - description: misc scu base address
> +            - description: reset base address

Are you sure that the indentation is correct? :-)

After fixing the indentation,

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

>               - description: pb scu base address

Re: [PATCH v3 1/4] dt-bindings: clock: airoha: Add reset support to EN7581 clock binding

[Conor Dooley]

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

On Thu, Jun 27, 2024 at 11:33:47AM +0200, AngeloGioacchino Del Regno wrote:
> Il 13/06/24 14:47, Lorenzo Bianconi ha scritto:
> > Introduce reset capability to EN7581 device-tree clock binding
> > documentation.
> > 
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > ---
> >   .../bindings/clock/airoha,en7523-scu.yaml     | 25 ++++++-
> >   .../dt-bindings/reset/airoha,en7581-reset.h   | 66 +++++++++++++++++++
> >   2 files changed, 90 insertions(+), 1 deletion(-)
> >   create mode 100644 include/dt-bindings/reset/airoha,en7581-reset.h
> > 
> > diff --git a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
> > index 3f4266637733..84353fd09428 100644
> > --- a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
> > +++ b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
> > @@ -35,7 +35,7 @@ properties:
> >     reg:
> >       minItems: 2
> > -    maxItems: 3
> > +    maxItems: 4
> >     "#clock-cells":
> >       description:
> > @@ -43,6 +43,10 @@ properties:
> >         clocks.
> >       const: 1
> > +  '#reset-cells':
> > +    description: ID of the controller reset line
> > +    const: 1
> > +
> >   required:
> >     - compatible
> >     - reg
> > @@ -60,6 +64,8 @@ allOf:
> >               - description: scu base address
> >               - description: misc scu base address
> > +        '#reset-cells': false
> > +
> >     - if:
> >         properties:
> >           compatible:
> > @@ -70,6 +76,7 @@ allOf:
> >             items:
> >               - description: scu base address
> >               - description: misc scu base address
> > +            - description: reset base address
> 
> Are you sure that the indentation is correct? :-)
> 
> After fixing the indentation,
> 
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> 
> >               - description: pb scu base address

The indentation actually looks okay when I apply this locally, but how is
it backwards compatible to add this register in the middle of the list??

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

Re: [PATCH v3 1/4] dt-bindings: clock: airoha: Add reset support to EN7581 clock binding

[AngeloGioacchino Del Regno]

Il 27/06/24 11:47, Conor Dooley ha scritto:
> On Thu, Jun 27, 2024 at 11:33:47AM +0200, AngeloGioacchino Del Regno wrote:
>> Il 13/06/24 14:47, Lorenzo Bianconi ha scritto:
>>> Introduce reset capability to EN7581 device-tree clock binding
>>> documentation.
>>>
>>> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
>>> ---
>>>    .../bindings/clock/airoha,en7523-scu.yaml     | 25 ++++++-
>>>    .../dt-bindings/reset/airoha,en7581-reset.h   | 66 +++++++++++++++++++
>>>    2 files changed, 90 insertions(+), 1 deletion(-)
>>>    create mode 100644 include/dt-bindings/reset/airoha,en7581-reset.h
>>>
>>> diff --git a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
>>> index 3f4266637733..84353fd09428 100644
>>> --- a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
>>> +++ b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
>>> @@ -35,7 +35,7 @@ properties:
>>>      reg:
>>>        minItems: 2
>>> -    maxItems: 3
>>> +    maxItems: 4
>>>      "#clock-cells":
>>>        description:
>>> @@ -43,6 +43,10 @@ properties:
>>>          clocks.
>>>        const: 1
>>> +  '#reset-cells':
>>> +    description: ID of the controller reset line
>>> +    const: 1
>>> +
>>>    required:
>>>      - compatible
>>>      - reg
>>> @@ -60,6 +64,8 @@ allOf:
>>>                - description: scu base address
>>>                - description: misc scu base address
>>> +        '#reset-cells': false
>>> +
>>>      - if:
>>>          properties:
>>>            compatible:
>>> @@ -70,6 +76,7 @@ allOf:
>>>              items:
>>>                - description: scu base address
>>>                - description: misc scu base address
>>> +            - description: reset base address
>>
>> Are you sure that the indentation is correct? :-)
>>
>> After fixing the indentation,
>>
>> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>>
>>>                - description: pb scu base address
> 
> The indentation actually looks okay when I apply this locally, but how is
> it backwards compatible to add this register in the middle of the list??

It's not, and this is actually something done on purpose - there is no DT using
this binding yet (here, nor uboot), and Lorenzo acknowledged the mistake before
it was too late...

At least this time, it wasn't a misattention :-P

Btw, as far as I know, the reset base address is in between misc scu and pb scu,
that's why it was put there in the middle.

Cheers!
Angelo

Re: [PATCH v3 1/4] dt-bindings: clock: airoha: Add reset support to EN7581 clock binding

[Conor Dooley]

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

On Thu, Jun 27, 2024 at 11:53:00AM +0200, AngeloGioacchino Del Regno wrote:
> Il 27/06/24 11:47, Conor Dooley ha scritto:
> > On Thu, Jun 27, 2024 at 11:33:47AM +0200, AngeloGioacchino Del Regno wrote:
> > > Il 13/06/24 14:47, Lorenzo Bianconi ha scritto:
> > > > Introduce reset capability to EN7581 device-tree clock binding
> > > > documentation.
> > > > 
> > > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > > > ---
> > > >    .../bindings/clock/airoha,en7523-scu.yaml     | 25 ++++++-
> > > >    .../dt-bindings/reset/airoha,en7581-reset.h   | 66 +++++++++++++++++++
> > > >    2 files changed, 90 insertions(+), 1 deletion(-)
> > > >    create mode 100644 include/dt-bindings/reset/airoha,en7581-reset.h
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
> > > > index 3f4266637733..84353fd09428 100644
> > > > --- a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
> > > > +++ b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
> > > > @@ -35,7 +35,7 @@ properties:
> > > >      reg:
> > > >        minItems: 2
> > > > -    maxItems: 3
> > > > +    maxItems: 4
> > > >      "#clock-cells":
> > > >        description:
> > > > @@ -43,6 +43,10 @@ properties:
> > > >          clocks.
> > > >        const: 1
> > > > +  '#reset-cells':
> > > > +    description: ID of the controller reset line
> > > > +    const: 1
> > > > +
> > > >    required:
> > > >      - compatible
> > > >      - reg
> > > > @@ -60,6 +64,8 @@ allOf:
> > > >                - description: scu base address
> > > >                - description: misc scu base address
> > > > +        '#reset-cells': false
> > > > +
> > > >      - if:
> > > >          properties:
> > > >            compatible:
> > > > @@ -70,6 +76,7 @@ allOf:
> > > >              items:
> > > >                - description: scu base address
> > > >                - description: misc scu base address
> > > > +            - description: reset base address
> > > 
> > > Are you sure that the indentation is correct? :-)
> > > 
> > > After fixing the indentation,
> > > 
> > > Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> > > 
> > > >                - description: pb scu base address
> > 
> > The indentation actually looks okay when I apply this locally, but how is
> > it backwards compatible to add this register in the middle of the list??
> 
> It's not, and this is actually something done on purpose - there is no DT using
> this binding yet (here, nor uboot), and Lorenzo acknowledged the mistake before
> it was too late...
> 
> At least this time, it wasn't a misattention :-P

The rationale for this being okay really should be in the commit
message...

> Btw, as far as I know, the reset base address is in between misc scu and pb scu,
> that's why it was put there in the middle.

...but I don't really see why this needs to be done incompatibly at all
in the first place. Just put it at the end of the list, there's no rule
that the order of reg properties needs to match the MMIO addresses.


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

Re: [PATCH v3 1/4] dt-bindings: clock: airoha: Add reset support to EN7581 clock binding

[AngeloGioacchino Del Regno]

Il 27/06/24 11:57, Conor Dooley ha scritto:
> On Thu, Jun 27, 2024 at 11:53:00AM +0200, AngeloGioacchino Del Regno wrote:
>> Il 27/06/24 11:47, Conor Dooley ha scritto:
>>> On Thu, Jun 27, 2024 at 11:33:47AM +0200, AngeloGioacchino Del Regno wrote:
>>>> Il 13/06/24 14:47, Lorenzo Bianconi ha scritto:
>>>>> Introduce reset capability to EN7581 device-tree clock binding
>>>>> documentation.
>>>>>
>>>>> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
>>>>> ---
>>>>>     .../bindings/clock/airoha,en7523-scu.yaml     | 25 ++++++-
>>>>>     .../dt-bindings/reset/airoha,en7581-reset.h   | 66 +++++++++++++++++++
>>>>>     2 files changed, 90 insertions(+), 1 deletion(-)
>>>>>     create mode 100644 include/dt-bindings/reset/airoha,en7581-reset.h
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
>>>>> index 3f4266637733..84353fd09428 100644
>>>>> --- a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
>>>>> +++ b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
>>>>> @@ -35,7 +35,7 @@ properties:
>>>>>       reg:
>>>>>         minItems: 2
>>>>> -    maxItems: 3
>>>>> +    maxItems: 4
>>>>>       "#clock-cells":
>>>>>         description:
>>>>> @@ -43,6 +43,10 @@ properties:
>>>>>           clocks.
>>>>>         const: 1
>>>>> +  '#reset-cells':
>>>>> +    description: ID of the controller reset line
>>>>> +    const: 1
>>>>> +
>>>>>     required:
>>>>>       - compatible
>>>>>       - reg
>>>>> @@ -60,6 +64,8 @@ allOf:
>>>>>                 - description: scu base address
>>>>>                 - description: misc scu base address
>>>>> +        '#reset-cells': false
>>>>> +
>>>>>       - if:
>>>>>           properties:
>>>>>             compatible:
>>>>> @@ -70,6 +76,7 @@ allOf:
>>>>>               items:
>>>>>                 - description: scu base address
>>>>>                 - description: misc scu base address
>>>>> +            - description: reset base address
>>>>
>>>> Are you sure that the indentation is correct? :-)
>>>>
>>>> After fixing the indentation,
>>>>
>>>> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>>>>
>>>>>                 - description: pb scu base address
>>>
>>> The indentation actually looks okay when I apply this locally, but how is
>>> it backwards compatible to add this register in the middle of the list??
>>
>> It's not, and this is actually something done on purpose - there is no DT using
>> this binding yet (here, nor uboot), and Lorenzo acknowledged the mistake before
>> it was too late...
>>
>> At least this time, it wasn't a misattention :-P
> 
> The rationale for this being okay really should be in the commit
> message...
> 
>> Btw, as far as I know, the reset base address is in between misc scu and pb scu,
>> that's why it was put there in the middle.
> 
> ...but I don't really see why this needs to be done incompatibly at all
> in the first place. Just put it at the end of the list, there's no rule
> that the order of reg properties needs to match the MMIO addresses.
> 

It's just a perfection thing... but whatever, if that's unacceptable for you,
putting it at the end of the list isn't a huge deal anyway.

Your call - it's okay for me either way.

Cheers,
Angelo

Re: [PATCH v3 1/4] dt-bindings: clock: airoha: Add reset support to EN7581 clock binding

[Conor Dooley]

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

On Thu, Jun 27, 2024 at 12:03:38PM +0200, AngeloGioacchino Del Regno wrote:
> Il 27/06/24 11:57, Conor Dooley ha scritto:
> > On Thu, Jun 27, 2024 at 11:53:00AM +0200, AngeloGioacchino Del Regno wrote:
> > > Il 27/06/24 11:47, Conor Dooley ha scritto:
> > > > On Thu, Jun 27, 2024 at 11:33:47AM +0200, AngeloGioacchino Del Regno wrote:
> > > > > Il 13/06/24 14:47, Lorenzo Bianconi ha scritto:
> > > > > > Introduce reset capability to EN7581 device-tree clock binding
> > > > > > documentation.
> > > > > > 
> > > > > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > > > > > ---
> > > > > >     .../bindings/clock/airoha,en7523-scu.yaml     | 25 ++++++-
> > > > > >     .../dt-bindings/reset/airoha,en7581-reset.h   | 66 +++++++++++++++++++
> > > > > >     2 files changed, 90 insertions(+), 1 deletion(-)
> > > > > >     create mode 100644 include/dt-bindings/reset/airoha,en7581-reset.h
> > > > > > 
> > > > > > diff --git a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
> > > > > > index 3f4266637733..84353fd09428 100644
> > > > > > --- a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
> > > > > > +++ b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
> > > > > > @@ -35,7 +35,7 @@ properties:
> > > > > >       reg:
> > > > > >         minItems: 2
> > > > > > -    maxItems: 3
> > > > > > +    maxItems: 4
> > > > > >       "#clock-cells":
> > > > > >         description:
> > > > > > @@ -43,6 +43,10 @@ properties:
> > > > > >           clocks.
> > > > > >         const: 1
> > > > > > +  '#reset-cells':
> > > > > > +    description: ID of the controller reset line
> > > > > > +    const: 1
> > > > > > +
> > > > > >     required:
> > > > > >       - compatible
> > > > > >       - reg
> > > > > > @@ -60,6 +64,8 @@ allOf:
> > > > > >                 - description: scu base address
> > > > > >                 - description: misc scu base address
> > > > > > +        '#reset-cells': false
> > > > > > +
> > > > > >       - if:
> > > > > >           properties:
> > > > > >             compatible:
> > > > > > @@ -70,6 +76,7 @@ allOf:
> > > > > >               items:
> > > > > >                 - description: scu base address
> > > > > >                 - description: misc scu base address
> > > > > > +            - description: reset base address
> > > > > 
> > > > > Are you sure that the indentation is correct? :-)
> > > > > 
> > > > > After fixing the indentation,
> > > > > 
> > > > > Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> > > > > 
> > > > > >                 - description: pb scu base address
> > > > 
> > > > The indentation actually looks okay when I apply this locally, but how is
> > > > it backwards compatible to add this register in the middle of the list??
> > > 
> > > It's not, and this is actually something done on purpose - there is no DT using
> > > this binding yet (here, nor uboot), and Lorenzo acknowledged the mistake before
> > > it was too late...
> > > 
> > > At least this time, it wasn't a misattention :-P
> > 
> > The rationale for this being okay really should be in the commit
> > message...
> > 
> > > Btw, as far as I know, the reset base address is in between misc scu and pb scu,
> > > that's why it was put there in the middle.
> > 
> > ...but I don't really see why this needs to be done incompatibly at all
> > in the first place. Just put it at the end of the list, there's no rule
> > that the order of reg properties needs to match the MMIO addresses.
> > 
> 
> It's just a perfection thing... but whatever, if that's unacceptable for you,
> putting it at the end of the list isn't a huge deal anyway.
> 
> Your call - it's okay for me either way.

It has not been in a release yet, so it's not a hard block for me,
however, I would want to see a commit message update and a Fixes: tag.
The patch should also appear in 6.10, rather than being 6.11 material.

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