[PATCH 0/5] clk: add support for Airoha AN7583 clock

[PATCH 0/5] clk: add support for Airoha AN7583 clock

[Christian Marangi]

This small series introduce some cleanup and support for
clock and reset of Airoha AN7583.

The implementation is similar to EN7581 but AN7583 introduce
new reset and more clock divisor support.

Christian Marangi (5):
  clk: en7523: convert driver to regmap API
  clk: en7523: generalize register clocks function
  dt-bindings: reset: add binding for Airoha AN7583 SoC reset
  dt-bindings: clock: airoha: Document support for AN7583 clock
  clk: en7523: add support for Airoha AN7583 clock

 .../bindings/clock/airoha,en7523-scu.yaml     |  16 +-
 drivers/clk/clk-en7523.c                      | 476 +++++++++++++-----
 .../dt-bindings/reset/airoha,an7583-reset.h   |  61 +++
 3 files changed, 425 insertions(+), 128 deletions(-)
 create mode 100644 include/dt-bindings/reset/airoha,an7583-reset.h

-- 
2.48.1

[PATCH 1/5] clk: en7523: convert driver to regmap API

[Christian Marangi]

Convert driver to regmap API, in preparation for support of Airoha
AN7523 as the SCU will be an MFD and the regmap will be provided in the
parent node.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 drivers/clk/clk-en7523.c | 137 ++++++++++++++++++++++-----------------
 1 file changed, 76 insertions(+), 61 deletions(-)

diff --git a/drivers/clk/clk-en7523.c b/drivers/clk/clk-en7523.c
index 15bbdeb60b8e..314e7450313f 100644
--- a/drivers/clk/clk-en7523.c
+++ b/drivers/clk/clk-en7523.c
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-only
 
+#include <linux/bitfield.h>
 #include <linux/delay.h>
 #include <linux/clk-provider.h>
 #include <linux/io.h>
@@ -34,6 +35,7 @@
 #define   REG_RESET_CONTROL_PCIE2	BIT(26)
 /* EN7581 */
 #define REG_NP_SCU_PCIC			0x88
+#define REG_PCIE_CTRL			GENMASK(7, 0)
 #define REG_NP_SCU_SSTR			0x9c
 #define REG_PCIE_XSI0_SEL_MASK		GENMASK(14, 13)
 #define REG_PCIE_XSI1_SEL_MASK		GENMASK(12, 11)
@@ -63,14 +65,14 @@ struct en_clk_desc {
 };
 
 struct en_clk_gate {
-	void __iomem *base;
+	struct regmap *map;
 	struct clk_hw hw;
 };
 
 struct en_rst_data {
 	const u16 *bank_ofs;
 	const u16 *idx_map;
-	void __iomem *base;
+	struct regmap *map;
 	struct reset_controller_dev rcdev;
 };
 
@@ -388,44 +390,44 @@ static u32 en7523_get_div(const struct en_clk_desc *desc, u32 val)
 static int en7523_pci_is_enabled(struct clk_hw *hw)
 {
 	struct en_clk_gate *cg = container_of(hw, struct en_clk_gate, hw);
+	u32 val;
 
-	return !!(readl(cg->base + REG_PCI_CONTROL) & REG_PCI_CONTROL_REFCLK_EN1);
+	regmap_read(cg->map, REG_PCI_CONTROL, &val);
+	return !!(val & REG_PCI_CONTROL_REFCLK_EN1);
 }
 
 static int en7523_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;
+	struct regmap *map = cg->map;
+	u32 mask;
 
 	/* Need to pull device low before reset */
-	val = readl(np_base + REG_PCI_CONTROL);
-	val &= ~(REG_PCI_CONTROL_PERSTOUT1 | REG_PCI_CONTROL_PERSTOUT);
-	writel(val, np_base + REG_PCI_CONTROL);
+	regmap_clear_bits(map, REG_PCI_CONTROL,
+			  REG_PCI_CONTROL_PERSTOUT1 |
+			  REG_PCI_CONTROL_PERSTOUT);
 	usleep_range(1000, 2000);
 
 	/* Enable PCIe port 1 */
-	val |= REG_PCI_CONTROL_REFCLK_EN1;
-	writel(val, np_base + REG_PCI_CONTROL);
+	regmap_set_bits(map, REG_PCI_CONTROL,
+			REG_PCI_CONTROL_REFCLK_EN1);
 	usleep_range(1000, 2000);
 
 	/* Reset to default */
-	val = readl(np_base + REG_RESET_CONTROL1);
 	mask = REG_RESET_CONTROL_PCIE1 | REG_RESET_CONTROL_PCIE2 |
 	       REG_RESET_CONTROL_PCIEHB;
-	writel(val & ~mask, np_base + REG_RESET_CONTROL1);
+	regmap_clear_bits(map, REG_RESET_CONTROL1, mask);
 	usleep_range(1000, 2000);
-	writel(val | mask, np_base + REG_RESET_CONTROL1);
+	regmap_set_bits(map, REG_RESET_CONTROL1, mask);
 	msleep(100);
-	writel(val & ~mask, np_base + REG_RESET_CONTROL1);
+	regmap_clear_bits(map, REG_RESET_CONTROL1, mask);
 	usleep_range(5000, 10000);
 
 	/* Release device */
 	mask = REG_PCI_CONTROL_PERSTOUT1 | REG_PCI_CONTROL_PERSTOUT;
-	val = readl(np_base + REG_PCI_CONTROL);
-	writel(val & ~mask, np_base + REG_PCI_CONTROL);
+	regmap_clear_bits(map, REG_PCI_CONTROL, mask);
 	usleep_range(1000, 2000);
-	writel(val | mask, np_base + REG_PCI_CONTROL);
+	regmap_set_bits(map, REG_PCI_CONTROL, mask);
 	msleep(250);
 
 	return 0;
@@ -434,16 +436,13 @@ static int en7523_pci_prepare(struct clk_hw *hw)
 static void en7523_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;
+	struct regmap *map = cg->map;
 
-	val = readl(np_base + REG_PCI_CONTROL);
-	val &= ~REG_PCI_CONTROL_REFCLK_EN1;
-	writel(val, np_base + REG_PCI_CONTROL);
+	regmap_clear_bits(map, REG_PCI_CONTROL, REG_PCI_CONTROL_REFCLK_EN1);
 }
 
 static struct clk_hw *en7523_register_pcie_clk(struct device *dev,
-					       void __iomem *np_base)
+					       struct regmap *clk_map)
 {
 	const struct en_clk_soc_data *soc_data = device_get_match_data(dev);
 	struct clk_init_data init = {
@@ -456,7 +455,7 @@ static struct clk_hw *en7523_register_pcie_clk(struct device *dev,
 	if (!cg)
 		return NULL;
 
-	cg->base = np_base;
+	cg->map = clk_map;
 	cg->hw.init = &init;
 
 	if (init.ops->unprepare)
@@ -474,21 +473,20 @@ static int en7581_pci_is_enabled(struct clk_hw *hw)
 	u32 val, mask;
 
 	mask = REG_PCI_CONTROL_REFCLK_EN0 | REG_PCI_CONTROL_REFCLK_EN1;
-	val = readl(cg->base + REG_PCI_CONTROL);
+	regmap_read(cg->map, REG_PCI_CONTROL, &val);
 	return (val & mask) == mask;
 }
 
 static int en7581_pci_enable(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;
+	struct regmap *map = cg->map;
+	u32 mask;
 
 	mask = REG_PCI_CONTROL_REFCLK_EN0 | REG_PCI_CONTROL_REFCLK_EN1 |
 	       REG_PCI_CONTROL_PERSTOUT1 | REG_PCI_CONTROL_PERSTOUT2 |
 	       REG_PCI_CONTROL_PERSTOUT;
-	val = readl(np_base + REG_PCI_CONTROL);
-	writel(val | mask, np_base + REG_PCI_CONTROL);
+	regmap_set_bits(map, REG_PCI_CONTROL, mask);
 
 	return 0;
 }
@@ -496,19 +494,18 @@ static int en7581_pci_enable(struct clk_hw *hw)
 static void en7581_pci_disable(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;
+	struct regmap *map = cg->map;
+	u32 mask;
 
 	mask = REG_PCI_CONTROL_REFCLK_EN0 | REG_PCI_CONTROL_REFCLK_EN1 |
 	       REG_PCI_CONTROL_PERSTOUT1 | REG_PCI_CONTROL_PERSTOUT2 |
 	       REG_PCI_CONTROL_PERSTOUT;
-	val = readl(np_base + REG_PCI_CONTROL);
-	writel(val & ~mask, np_base + REG_PCI_CONTROL);
+	regmap_clear_bits(map, REG_PCI_CONTROL, mask);
 	usleep_range(1000, 2000);
 }
 
 static void en7523_register_clocks(struct device *dev, struct clk_hw_onecell_data *clk_data,
-				   void __iomem *base, void __iomem *np_base)
+				   struct regmap *map, struct regmap *clk_map)
 {
 	struct clk_hw *hw;
 	u32 rate;
@@ -517,10 +514,12 @@ static void en7523_register_clocks(struct device *dev, struct clk_hw_onecell_dat
 	for (i = 0; i < ARRAY_SIZE(en7523_base_clks); i++) {
 		const struct en_clk_desc *desc = &en7523_base_clks[i];
 		u32 reg = desc->div_reg ? desc->div_reg : desc->base_reg;
-		u32 val = readl(base + desc->base_reg);
+		u32 val;
+
+		regmap_read(map, desc->base_reg, &val);
 
 		rate = en7523_get_base_rate(desc, val);
-		val = readl(base + reg);
+		regmap_read(map, reg, &val);
 		rate /= en7523_get_div(desc, val);
 
 		hw = clk_hw_register_fixed_rate(dev, desc->name, NULL, 0, rate);
@@ -533,30 +532,47 @@ static void en7523_register_clocks(struct device *dev, struct clk_hw_onecell_dat
 		clk_data->hws[desc->id] = hw;
 	}
 
-	hw = en7523_register_pcie_clk(dev, np_base);
+	hw = en7523_register_pcie_clk(dev, clk_map);
 	clk_data->hws[EN7523_CLK_PCIE] = hw;
 }
 
+static const struct regmap_config en7523_clk_regmap_config = {
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_stride = 4,
+};
+
 static int en7523_clk_hw_init(struct platform_device *pdev,
 			      struct clk_hw_onecell_data *clk_data)
 {
 	void __iomem *base, *np_base;
+	struct regmap *map, *clk_map;
 
 	base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(base))
 		return PTR_ERR(base);
 
+	map = devm_regmap_init_mmio(&pdev->dev, base,
+				    &en7523_clk_regmap_config);
+	if (IS_ERR(map))
+		return PTR_ERR(map);
+
 	np_base = devm_platform_ioremap_resource(pdev, 1);
 	if (IS_ERR(np_base))
 		return PTR_ERR(np_base);
 
-	en7523_register_clocks(&pdev->dev, clk_data, base, np_base);
+	clk_map = devm_regmap_init_mmio(&pdev->dev, np_base,
+					&en7523_clk_regmap_config);
+	if (IS_ERR(clk_map))
+		return PTR_ERR(clk_map);
+
+	en7523_register_clocks(&pdev->dev, clk_data, map, clk_map);
 
 	return 0;
 }
 
 static void en7581_register_clocks(struct device *dev, struct clk_hw_onecell_data *clk_data,
-				   struct regmap *map, void __iomem *base)
+				   struct regmap *map, struct regmap *clk_map)
 {
 	struct clk_hw *hw;
 	u32 rate;
@@ -593,7 +609,7 @@ static void en7581_register_clocks(struct device *dev, struct clk_hw_onecell_dat
 		clk_data->hws[desc->id] = hw;
 	}
 
-	hw = en7523_register_pcie_clk(dev, base);
+	hw = en7523_register_pcie_clk(dev, clk_map);
 	clk_data->hws[EN7523_CLK_PCIE] = hw;
 }
 
@@ -601,15 +617,10 @@ static int en7523_reset_update(struct reset_controller_dev *rcdev,
 			       unsigned long id, bool assert)
 {
 	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;
+	u32 addr = 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);
+	regmap_update_bits(rst_data->map, addr, BIT(id % RST_NR_PER_BANK),
+			   assert ? BIT(id % RST_NR_PER_BANK) : 0);
 
 	return 0;
 }
@@ -630,9 +641,11 @@ static int en7523_reset_status(struct reset_controller_dev *rcdev,
 			       unsigned long id)
 {
 	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 addr = rst_data->bank_ofs[id / RST_NR_PER_BANK];
+	u32 val;
 
-	return !!(readl(addr) & BIT(id % RST_NR_PER_BANK));
+	regmap_read(rst_data->map, addr, &val);
+	return !!(val & BIT(id % RST_NR_PER_BANK));
 }
 
 static int en7523_reset_xlate(struct reset_controller_dev *rcdev,
@@ -652,7 +665,7 @@ static const struct reset_control_ops en7581_reset_ops = {
 	.status = en7523_reset_status,
 };
 
-static int en7581_reset_register(struct device *dev, void __iomem *base)
+static int en7581_reset_register(struct device *dev, struct regmap *map)
 {
 	struct en_rst_data *rst_data;
 
@@ -662,7 +675,7 @@ static int en7581_reset_register(struct device *dev, void __iomem *base)
 
 	rst_data->bank_ofs = en7581_rst_ofs;
 	rst_data->idx_map = en7581_rst_map;
-	rst_data->base = base;
+	rst_data->map = map;
 
 	rst_data->rcdev.nr_resets = ARRAY_SIZE(en7581_rst_map);
 	rst_data->rcdev.of_xlate = en7523_reset_xlate;
@@ -678,9 +691,8 @@ static int en7581_reset_register(struct device *dev, void __iomem *base)
 static int en7581_clk_hw_init(struct platform_device *pdev,
 			      struct clk_hw_onecell_data *clk_data)
 {
-	struct regmap *map;
+	struct regmap *map, *clk_map;
 	void __iomem *base;
-	u32 val;
 
 	map = syscon_regmap_lookup_by_compatible("airoha,en7581-chip-scu");
 	if (IS_ERR(map))
@@ -690,15 +702,18 @@ static int en7581_clk_hw_init(struct platform_device *pdev,
 	if (IS_ERR(base))
 		return PTR_ERR(base);
 
-	en7581_register_clocks(&pdev->dev, clk_data, map, base);
+	clk_map = devm_regmap_init_mmio(&pdev->dev, base, &en7523_clk_regmap_config);
+	if (IS_ERR(clk_map))
+		return PTR_ERR(clk_map);
+
+	en7581_register_clocks(&pdev->dev, clk_data, map, clk_map);
 
-	val = readl(base + REG_NP_SCU_SSTR);
-	val &= ~(REG_PCIE_XSI0_SEL_MASK | REG_PCIE_XSI1_SEL_MASK);
-	writel(val, base + REG_NP_SCU_SSTR);
-	val = readl(base + REG_NP_SCU_PCIC);
-	writel(val | 3, base + REG_NP_SCU_PCIC);
+	regmap_clear_bits(clk_map, REG_NP_SCU_SSTR,
+			  REG_PCIE_XSI0_SEL_MASK | REG_PCIE_XSI1_SEL_MASK);
+	regmap_update_bits(clk_map, REG_NP_SCU_PCIC, REG_PCIE_CTRL,
+			   FIELD_PREP(REG_PCIE_CTRL, 3));
 
-	return en7581_reset_register(&pdev->dev, base);
+	return en7581_reset_register(&pdev->dev, clk_map);
 }
 
 static int en7523_clk_probe(struct platform_device *pdev)
-- 
2.48.1

[PATCH 2/5] clk: en7523: generalize register clocks function

[Christian Marangi]

Generalize register clocks function for Airoha EN7523 and EN7581 clocks
driver. The same logic is applied for both clock hence code can be
reduced and simplified by putting the base_clocks struct in the soc_data
and passing that to a generic register clocks function.

There is always the pattern where the clock is incremented by one to
account for the PCIe one.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 drivers/clk/clk-en7523.c | 130 ++++++++++++++++-----------------------
 1 file changed, 53 insertions(+), 77 deletions(-)

diff --git a/drivers/clk/clk-en7523.c b/drivers/clk/clk-en7523.c
index 314e7450313f..07ab5b42fd5a 100644
--- a/drivers/clk/clk-en7523.c
+++ b/drivers/clk/clk-en7523.c
@@ -78,8 +78,10 @@ struct en_rst_data {
 
 struct en_clk_soc_data {
 	u32 num_clocks;
+	const struct en_clk_desc *base_clks;
 	const struct clk_ops pcie_ops;
 	int (*hw_init)(struct platform_device *pdev,
+		       const struct en_clk_soc_data *soc_data,
 		       struct clk_hw_onecell_data *clk_data);
 };
 
@@ -467,6 +469,50 @@ static struct clk_hw *en7523_register_pcie_clk(struct device *dev,
 	return &cg->hw;
 }
 
+static void en75xx_register_clocks(struct device *dev,
+				   const struct en_clk_soc_data *soc_data,
+				   struct clk_hw_onecell_data *clk_data,
+				   struct regmap *map, struct regmap *clk_map)
+{
+	struct clk_hw *hw;
+	u32 rate;
+	int i;
+
+	for (i = 0; i < soc_data->num_clocks - 1; i++) {
+		const struct en_clk_desc *desc = &soc_data->base_clks[i];
+		u32 val, reg = desc->div_reg ? desc->div_reg : desc->base_reg;
+		int err;
+
+		err = regmap_read(map, desc->base_reg, &val);
+		if (err) {
+			pr_err("Failed reading fixed clk rate %s: %d\n",
+			       desc->name, err);
+			continue;
+		}
+		rate = en7523_get_base_rate(desc, val);
+
+		err = regmap_read(map, reg, &val);
+		if (err) {
+			pr_err("Failed reading fixed clk div %s: %d\n",
+			       desc->name, err);
+			continue;
+		}
+		rate /= en7523_get_div(desc, val);
+
+		hw = clk_hw_register_fixed_rate(dev, desc->name, NULL, 0, rate);
+		if (IS_ERR(hw)) {
+			pr_err("Failed to register clk %s: %ld\n",
+			       desc->name, PTR_ERR(hw));
+			continue;
+		}
+
+		clk_data->hws[desc->id] = hw;
+	}
+
+	hw = en7523_register_pcie_clk(dev, clk_map);
+	clk_data->hws[EN7523_CLK_PCIE] = hw;
+}
+
 static int en7581_pci_is_enabled(struct clk_hw *hw)
 {
 	struct en_clk_gate *cg = container_of(hw, struct en_clk_gate, hw);
@@ -504,38 +550,6 @@ static void en7581_pci_disable(struct clk_hw *hw)
 	usleep_range(1000, 2000);
 }
 
-static void en7523_register_clocks(struct device *dev, struct clk_hw_onecell_data *clk_data,
-				   struct regmap *map, struct regmap *clk_map)
-{
-	struct clk_hw *hw;
-	u32 rate;
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(en7523_base_clks); i++) {
-		const struct en_clk_desc *desc = &en7523_base_clks[i];
-		u32 reg = desc->div_reg ? desc->div_reg : desc->base_reg;
-		u32 val;
-
-		regmap_read(map, desc->base_reg, &val);
-
-		rate = en7523_get_base_rate(desc, val);
-		regmap_read(map, reg, &val);
-		rate /= en7523_get_div(desc, val);
-
-		hw = clk_hw_register_fixed_rate(dev, desc->name, NULL, 0, rate);
-		if (IS_ERR(hw)) {
-			pr_err("Failed to register clk %s: %ld\n",
-			       desc->name, PTR_ERR(hw));
-			continue;
-		}
-
-		clk_data->hws[desc->id] = hw;
-	}
-
-	hw = en7523_register_pcie_clk(dev, clk_map);
-	clk_data->hws[EN7523_CLK_PCIE] = hw;
-}
-
 static const struct regmap_config en7523_clk_regmap_config = {
 	.reg_bits = 32,
 	.val_bits = 32,
@@ -543,6 +557,7 @@ static const struct regmap_config en7523_clk_regmap_config = {
 };
 
 static int en7523_clk_hw_init(struct platform_device *pdev,
+			      const struct en_clk_soc_data *soc_data,
 			      struct clk_hw_onecell_data *clk_data)
 {
 	void __iomem *base, *np_base;
@@ -566,53 +581,11 @@ static int en7523_clk_hw_init(struct platform_device *pdev,
 	if (IS_ERR(clk_map))
 		return PTR_ERR(clk_map);
 
-	en7523_register_clocks(&pdev->dev, clk_data, map, clk_map);
+	en75xx_register_clocks(&pdev->dev, soc_data, clk_data, map, clk_map);
 
 	return 0;
 }
 
-static void en7581_register_clocks(struct device *dev, struct clk_hw_onecell_data *clk_data,
-				   struct regmap *map, struct regmap *clk_map)
-{
-	struct clk_hw *hw;
-	u32 rate;
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(en7581_base_clks); i++) {
-		const struct en_clk_desc *desc = &en7581_base_clks[i];
-		u32 val, reg = desc->div_reg ? desc->div_reg : desc->base_reg;
-		int err;
-
-		err = regmap_read(map, desc->base_reg, &val);
-		if (err) {
-			pr_err("Failed reading fixed clk rate %s: %d\n",
-			       desc->name, err);
-			continue;
-		}
-		rate = en7523_get_base_rate(desc, val);
-
-		err = regmap_read(map, reg, &val);
-		if (err) {
-			pr_err("Failed reading fixed clk div %s: %d\n",
-			       desc->name, err);
-			continue;
-		}
-		rate /= en7523_get_div(desc, val);
-
-		hw = clk_hw_register_fixed_rate(dev, desc->name, NULL, 0, rate);
-		if (IS_ERR(hw)) {
-			pr_err("Failed to register clk %s: %ld\n",
-			       desc->name, PTR_ERR(hw));
-			continue;
-		}
-
-		clk_data->hws[desc->id] = hw;
-	}
-
-	hw = en7523_register_pcie_clk(dev, clk_map);
-	clk_data->hws[EN7523_CLK_PCIE] = hw;
-}
-
 static int en7523_reset_update(struct reset_controller_dev *rcdev,
 			       unsigned long id, bool assert)
 {
@@ -689,6 +662,7 @@ static int en7581_reset_register(struct device *dev, struct regmap *map)
 }
 
 static int en7581_clk_hw_init(struct platform_device *pdev,
+			      const struct en_clk_soc_data *soc_data,
 			      struct clk_hw_onecell_data *clk_data)
 {
 	struct regmap *map, *clk_map;
@@ -706,7 +680,7 @@ static int en7581_clk_hw_init(struct platform_device *pdev,
 	if (IS_ERR(clk_map))
 		return PTR_ERR(clk_map);
 
-	en7581_register_clocks(&pdev->dev, clk_data, map, clk_map);
+	en75xx_register_clocks(&pdev->dev, soc_data, clk_data, map, clk_map);
 
 	regmap_clear_bits(clk_map, REG_NP_SCU_SSTR,
 			  REG_PCIE_XSI0_SEL_MASK | REG_PCIE_XSI1_SEL_MASK);
@@ -732,7 +706,7 @@ static int en7523_clk_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	clk_data->num = soc_data->num_clocks;
-	r = soc_data->hw_init(pdev, clk_data);
+	r = soc_data->hw_init(pdev, soc_data, clk_data);
 	if (r)
 		return r;
 
@@ -740,6 +714,7 @@ static int en7523_clk_probe(struct platform_device *pdev)
 }
 
 static const struct en_clk_soc_data en7523_data = {
+	.base_clks = en7523_base_clks,
 	.num_clocks = ARRAY_SIZE(en7523_base_clks) + 1,
 	.pcie_ops = {
 		.is_enabled = en7523_pci_is_enabled,
@@ -750,6 +725,7 @@ static const struct en_clk_soc_data en7523_data = {
 };
 
 static const struct en_clk_soc_data en7581_data = {
+	.base_clks = en7581_base_clks,
 	/* We increment num_clocks by 1 to account for additional PCIe clock */
 	.num_clocks = ARRAY_SIZE(en7581_base_clks) + 1,
 	.pcie_ops = {
-- 
2.48.1

[PATCH 3/5] dt-bindings: reset: add binding for Airoha AN7583 SoC reset

[Christian Marangi]

Add binding for Airoha AN7583 SoC Resets. These are very similar to
EN7581 but lack some specific reset line hence the order is different
and a dedicated binding is needed.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 .../dt-bindings/reset/airoha,an7583-reset.h   | 61 +++++++++++++++++++
 1 file changed, 61 insertions(+)
 create mode 100644 include/dt-bindings/reset/airoha,an7583-reset.h

diff --git a/include/dt-bindings/reset/airoha,an7583-reset.h b/include/dt-bindings/reset/airoha,an7583-reset.h
new file mode 100644
index 000000000000..96cfe11d2943
--- /dev/null
+++ b/include/dt-bindings/reset/airoha,an7583-reset.h
@@ -0,0 +1,61 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Copyright (c) 2024 AIROHA Inc
+ * Author: Christian Marangi <ansuelsmth@gmail.com>
+ */
+
+#ifndef __DT_BINDINGS_RESET_CONTROLLER_AIROHA_AN7583_H_
+#define __DT_BINDINGS_RESET_CONTROLLER_AIROHA_AN7583_H_
+
+/* RST_CTRL2 */
+#define AN7583_XPON_PHY_RST		 0
+#define AN7583_GPON_OLT_RST		 1
+#define AN7583_CPU_TIMER2_RST		 2
+#define AN7583_HSUART_RST		 3
+#define AN7583_UART4_RST		 4
+#define AN7583_UART5_RST		 5
+#define AN7583_I2C2_RST			 6
+#define AN7583_XSI_MAC_RST		 7
+#define AN7583_XSI_PHY_RST		 8
+#define AN7583_NPU_RST			 9
+#define AN7583_TRNG_MSTART_RST		10
+#define AN7583_DUAL_HSI0_RST		11
+#define AN7583_DUAL_HSI1_RST		12
+#define AN7583_DUAL_HSI0_MAC_RST	13
+#define AN7583_DUAL_HSI1_MAC_RST	14
+#define AN7583_WDMA_RST			15
+#define AN7583_WOE0_RST			16
+#define AN7583_HSDMA_RST		17
+#define AN7583_TDMA_RST			18
+#define AN7583_EMMC_RST			19
+#define AN7583_SOE_RST			20
+#define AN7583_XFP_MAC_RST		21
+#define AN7583_MDIO0                    22
+#define AN7583_MDIO1                    23
+/* RST_CTRL1 */
+#define AN7583_PCM1_ZSI_ISI_RST		24
+#define AN7583_FE_PDMA_RST		25
+#define AN7583_FE_QDMA_RST		26
+#define AN7583_PCM_SPIWP_RST		27
+#define AN7583_CRYPTO_RST		28
+#define AN7583_TIMER_RST		29
+#define AN7583_PCM1_RST			30
+#define AN7583_UART_RST			31
+#define AN7583_GPIO_RST			32
+#define AN7583_GDMA_RST			33
+#define AN7583_I2C_MASTER_RST		34
+#define AN7583_PCM2_ZSI_ISI_RST		35
+#define AN7583_SFC_RST			36
+#define AN7583_UART2_RST		37
+#define AN7583_GDMP_RST			38
+#define AN7583_FE_RST			39
+#define AN7583_USB_HOST_P0_RST		40
+#define AN7583_GSW_RST			41
+#define AN7583_SFC2_PCM_RST		42
+#define AN7583_PCIE0_RST		43
+#define AN7583_PCIE1_RST		44
+#define AN7583_CPU_TIMER_RST		45
+#define AN7583_PCIE_HB_RST		46
+#define AN7583_XPON_MAC_RST		47
+
+#endif /* __DT_BINDINGS_RESET_CONTROLLER_AIROHA_AN7583_H_ */
-- 
2.48.1

Re: [PATCH 3/5] dt-bindings: reset: add binding for Airoha AN7583 SoC reset

[Krzysztof Kozlowski]

On 28/05/2025 02:49, Christian Marangi wrote:
> Add binding for Airoha AN7583 SoC Resets. These are very similar to
> EN7581 but lack some specific reset line hence the order is different
> and a dedicated binding is needed.
> 
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
>  .../dt-bindings/reset/airoha,an7583-reset.h   | 61 +++++++++++++++++++
>  1 file changed, 61 insertions(+)
>  create mode 100644 include/dt-bindings/reset/airoha,an7583-reset.h

This goes with the binding for this device, not separate commit.

Best regards,
Krzysztof

[PATCH 4/5] dt-bindings: clock: airoha: Document support for AN7583 clock

[Christian Marangi]

Document support for Airoha AN7583 clock. This is based on the EN7523
clock schema but use a more specific compatible (airoha,an7583-clock) as
the SCU now also provide very different peripherals.

Airoha AN7583 clock reg handling is also different as it's inherited
by the parent node.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 .../bindings/clock/airoha,en7523-scu.yaml        | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
index fe2c5c1baf43..06c823539ba9 100644
--- a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
+++ b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
@@ -32,6 +32,7 @@ properties:
       - enum:
           - airoha,en7523-scu
           - airoha,en7581-scu
+          - airoha,an7583-clock
 
   reg:
     items:
@@ -51,7 +52,6 @@ properties:
 
 required:
   - compatible
-  - reg
   - '#clock-cells'
 
 allOf:
@@ -66,6 +66,9 @@ allOf:
 
         '#reset-cells': false
 
+      required:
+        - reg
+
   - if:
       properties:
         compatible:
@@ -75,6 +78,17 @@ allOf:
         reg:
           maxItems: 1
 
+      required:
+        - reg
+
+  - if:
+      properties:
+        compatible:
+          const: airoha,an7583-clock
+    then:
+      properties:
+        reg: false
+
 additionalProperties: false
 
 examples:
-- 
2.48.1

Re: [PATCH 4/5] dt-bindings: clock: airoha: Document support for AN7583 clock

[Krzysztof Kozlowski]

On 28/05/2025 02:49, Christian Marangi wrote:
>    - if:
>        properties:
>          compatible:
> @@ -75,6 +78,17 @@ allOf:
>          reg:
>            maxItems: 1
>  
> +      required:
> +        - reg
> +
> +  - if:
> +      properties:
> +        compatible:
> +          const: airoha,an7583-clock
> +    then:
> +      properties:
> +        reg: false


No resources here, so this should be part of parent node.

Best regards,
Krzysztof

Re: [PATCH 4/5] dt-bindings: clock: airoha: Document support for AN7583 clock

[Christian Marangi]

On Wed, May 28, 2025 at 09:30:37AM +0200, Krzysztof Kozlowski wrote:
> On 28/05/2025 02:49, Christian Marangi wrote:
> >    - if:
> >        properties:
> >          compatible:
> > @@ -75,6 +78,17 @@ allOf:
> >          reg:
> >            maxItems: 1
> >  
> > +      required:
> > +        - reg
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          const: airoha,an7583-clock
> > +    then:
> > +      properties:
> > +        reg: false
> 
> 
> No resources here, so this should be part of parent node.
> 

Ok hope you can help here. This is another case of "MFD" thing.

I was with the idea that it was O.K. to use this with very different
devices. (current scenario Clock controller and MDIO controller)

The node structure I had in mind was

		system-controller@1fa20000 {
			compatible = "airoha,an7583-scu", "syscon", "simple-mfd";
			reg = <0x0 0x1fb00000 0x0 0x970>;

			scuclk: scuclk {
				compatible = "airoha,an7583-clock";
				#clock-cells = <1>;
				#reset-cells = <1>;
			};

			mdio {
				compatible = "airoha,an7583-mdio";
				#address-cells = <1>;
				#size-cells = <0>;

				mdio_0: bus@0 {
					reg = <0>;
					resets = <&scuclk AN7583_MDIO0>;
				};

				mdio_1: bus@1 {
					reg = <1>;
					resets = <&scuclk AN7583_MDIO1>;
				};
			};
		};

But you want

system-controller@1fa20000 {
        compatible = "airoha,an7583-scu", "syscon";
        reg = <0x0 0x1fb00000 0x0 0x970>;

        #clock-cells = <1>;
        #reset-cells = <1>;

        mdio_0: bus@0 {
                reg = <0>;
                resets = <&scuclk AN7583_MDIO0>;
        };

        mdio_1: bus@1 {
                reg = <1>;
                resets = <&scuclk AN7583_MDIO1>;
        };
};

Again sorry if this question keeps coming around and I can totally
understand if you are getting annoyed by this. The reason I always ask
this is because it's a total PAIN to implement this with the driver
structure due to the old "simple-mfd" model.

(as again putting everything in a single node conflicts with the OF
principle of autoprobing stuff with compatible property)

-- 
	Ansuel

Re: [PATCH 4/5] dt-bindings: clock: airoha: Document support for AN7583 clock

[Krzysztof Kozlowski]

On 28/05/2025 10:54, Christian Marangi wrote:
> On Wed, May 28, 2025 at 09:30:37AM +0200, Krzysztof Kozlowski wrote:
>> On 28/05/2025 02:49, Christian Marangi wrote:
>>>    - if:
>>>        properties:
>>>          compatible:
>>> @@ -75,6 +78,17 @@ allOf:
>>>          reg:
>>>            maxItems: 1
>>>  
>>> +      required:
>>> +        - reg
>>> +
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          const: airoha,an7583-clock
>>> +    then:
>>> +      properties:
>>> +        reg: false
>>
>>
>> No resources here, so this should be part of parent node.
>>
> 
> Ok hope you can help here. This is another case of "MFD" thing.
> 
> I was with the idea that it was O.K. to use this with very different
> devices. (current scenario Clock controller and MDIO controller)
> 
> The node structure I had in mind was
> 
> 		system-controller@1fa20000 {
> 			compatible = "airoha,an7583-scu", "syscon", "simple-mfd";
> 			reg = <0x0 0x1fb00000 0x0 0x970>;
> 
> 			scuclk: scuclk {
> 				compatible = "airoha,an7583-clock";
> 				#clock-cells = <1>;
> 				#reset-cells = <1>;
> 			};
> 
> 			mdio {
> 				compatible = "airoha,an7583-mdio";
> 				#address-cells = <1>;
> 				#size-cells = <0>;
> 
> 				mdio_0: bus@0 {
> 					reg = <0>;
> 					resets = <&scuclk AN7583_MDIO0>;
> 				};
> 
> 				mdio_1: bus@1 {
> 					reg = <1>;
> 					resets = <&scuclk AN7583_MDIO1>;
> 				};
> 			};
> 		};
> 
> But you want
> 
> system-controller@1fa20000 {
>         compatible = "airoha,an7583-scu", "syscon";
>         reg = <0x0 0x1fb00000 0x0 0x970>;
> 
>         #clock-cells = <1>;
>         #reset-cells = <1>;
> 

mdio could be here just to group the bus (it's pretty common I think),
although not sure if compatible is useful then.

>         mdio_0: bus@0 {
>                 reg = <0>;
>                 resets = <&scuclk AN7583_MDIO0>;
>         };
> 
>         mdio_1: bus@1 {
>                 reg = <1>;
>                 resets = <&scuclk AN7583_MDIO1>;
>         };
> };
> 
> Again sorry if this question keeps coming around and I can totally
> understand if you are getting annoyed by this. The reason I always ask
> this is because it's a total PAIN to implement this with the driver
> structure due to the old "simple-mfd" model.

... and Rob was saying multiple times: be careful when adding
simple-mfd. If it bites back, then I am sorry, but everyone were warned,
weren't they?

What is exactly the pain anyway? You cannot instantiate children from
SCU driver?

> 
> (as again putting everything in a single node conflicts with the OF
> principle of autoprobing stuff with compatible property)

I am not sure if I follow. What principle? Where is this principle
expressed?

And you do not have in your second example additional compatibles, so
even if such principle exists it is not broken: everything autoprobes, I
think.

> 


Best regards,
Krzysztof

Re: [PATCH 4/5] dt-bindings: clock: airoha: Document support for AN7583 clock

[Christian Marangi]

On Wed, May 28, 2025 at 01:56:54PM +0200, Krzysztof Kozlowski wrote:
> On 28/05/2025 10:54, Christian Marangi wrote:
> > On Wed, May 28, 2025 at 09:30:37AM +0200, Krzysztof Kozlowski wrote:
> >> On 28/05/2025 02:49, Christian Marangi wrote:
> >>>    - if:
> >>>        properties:
> >>>          compatible:
> >>> @@ -75,6 +78,17 @@ allOf:
> >>>          reg:
> >>>            maxItems: 1
> >>>  
> >>> +      required:
> >>> +        - reg
> >>> +
> >>> +  - if:
> >>> +      properties:
> >>> +        compatible:
> >>> +          const: airoha,an7583-clock
> >>> +    then:
> >>> +      properties:
> >>> +        reg: false
> >>
> >>
> >> No resources here, so this should be part of parent node.
> >>
> > 
> > Ok hope you can help here. This is another case of "MFD" thing.
> > 
> > I was with the idea that it was O.K. to use this with very different
> > devices. (current scenario Clock controller and MDIO controller)
> > 
> > The node structure I had in mind was
> > 
> > 		system-controller@1fa20000 {
> > 			compatible = "airoha,an7583-scu", "syscon", "simple-mfd";
> > 			reg = <0x0 0x1fb00000 0x0 0x970>;
> > 
> > 			scuclk: scuclk {
> > 				compatible = "airoha,an7583-clock";
> > 				#clock-cells = <1>;
> > 				#reset-cells = <1>;
> > 			};
> > 
> > 			mdio {
> > 				compatible = "airoha,an7583-mdio";
> > 				#address-cells = <1>;
> > 				#size-cells = <0>;
> > 
> > 				mdio_0: bus@0 {
> > 					reg = <0>;
> > 					resets = <&scuclk AN7583_MDIO0>;
> > 				};
> > 
> > 				mdio_1: bus@1 {
> > 					reg = <1>;
> > 					resets = <&scuclk AN7583_MDIO1>;
> > 				};
> > 			};
> > 		};
> > 
> > But you want
> > 
> > system-controller@1fa20000 {
> >         compatible = "airoha,an7583-scu", "syscon";
> >         reg = <0x0 0x1fb00000 0x0 0x970>;
> > 
> >         #clock-cells = <1>;
> >         #reset-cells = <1>;
> > 
> 
> mdio could be here just to group the bus (it's pretty common I think),
> although not sure if compatible is useful then.
> 
> >         mdio_0: bus@0 {
> >                 reg = <0>;
> >                 resets = <&scuclk AN7583_MDIO0>;
> >         };
> > 
> >         mdio_1: bus@1 {
> >                 reg = <1>;
> >                 resets = <&scuclk AN7583_MDIO1>;
> >         };
> > };
> > 
> > Again sorry if this question keeps coming around and I can totally
> > understand if you are getting annoyed by this. The reason I always ask
> > this is because it's a total PAIN to implement this with the driver
> > structure due to the old "simple-mfd" model.
> 
> ... and Rob was saying multiple times: be careful when adding
> simple-mfd. If it bites back, then I am sorry, but everyone were warned,
> weren't they?
> 
> What is exactly the pain anyway? You cannot instantiate children from
> SCU driver?
>

Answering below since they are related.

> > 
> > (as again putting everything in a single node conflicts with the OF
> > principle of autoprobing stuff with compatible property)
> 
> I am not sure if I follow. What principle? Where is this principle
> expressed?
> 
> And you do not have in your second example additional compatibles, so
> even if such principle exists it is not broken: everything autoprobes, I
> think.
> 
> > 
> 
>

The principle I'm talking about is one driver for one compatible.
(to be more precise excluding syscon compatible that is actually
ignored, if a driver for the compatible is found, any other compatible
is ignored.)

This means that declaring multiple compatible as:

compatible = "airoha,clock", "airoha,mdio"

doesn't result in the clock driver and the mdio driver probed but only
one of the 2 (probably only clock since it does have priority)

The "simple-mfd" compatible is just a simple compatible that indicate to
the OF system that every child (with a compatible) should be also probed.
And then automagically the driver gets probed.

Now the ""PAIN"" explaination. Not using the "simple-mfd" way with the
child with compatible and putting everything in the node means having to
create a dedicated MFD driver that just instruct to manually probe the
clock and mdio driver. (cause the compatible system can't be used)

So it's 3 driver instead of 2 with the extra effort of MFD driver
maintainer saying "Why simple-mfd is not used?"


There is a solution for this but I always feel it's more of a workaround
since it doesn't really describe the HW with the DT node.

The workaround is:

		system-controller@1fa20000 {
                        /* The parent SCU node implement the clock driver */
                        compatible = "airoha,an7583-scu", "syscon";
                        reg = <0x0 0x1fb00000 0x0 0x970>;

                        #clock-cells = <1>;
                        #reset-cells = <1>;

                        /* Clock driver is instructed to probe child */
                        mdio {
                                compatible = "airoha,an7583-mdio";
                                #address-cells = <1>;
                                #size-cells = <0>;

                                mdio_0: bus@0 {
                                        reg = <0>;
                                        resets = <&scuclk AN7583_MDIO0>;
                                };

                                mdio_1: bus@1 {
                                        reg = <1>;
                                        resets = <&scuclk AN7583_MDIO1>;
                                };
                        };
                };


But this really moves the probe from the simple-mfd to the clock driver.

So it's either 3 solution
1. 2 driver + "simple-mfd"
2. 3 driver + PAIN (due to MFD required driver)
3. 2 driver + not very correct DT node structure

Maybe option 3. is more acceptable?

The SCU node is mainly clock + reset controller and the MDIO bus is an
expansion of it?

Hope the long explaination makes sense to you (especially about the
OF principle thing)

--
Ansuel

Re: [PATCH 4/5] dt-bindings: clock: airoha: Document support for AN7583 clock

[Krzysztof Kozlowski]

On 28/05/2025 14:57, Christian Marangi wrote:
>>> Again sorry if this question keeps coming around and I can totally
>>> understand if you are getting annoyed by this. The reason I always ask
>>> this is because it's a total PAIN to implement this with the driver
>>> structure due to the old "simple-mfd" model.
>>
>> ... and Rob was saying multiple times: be careful when adding
>> simple-mfd. If it bites back, then I am sorry, but everyone were warned,
>> weren't they?
>>
>> What is exactly the pain anyway? You cannot instantiate children from
>> SCU driver?
>>
> 
> Answering below since they are related.
> 
>>>
>>> (as again putting everything in a single node conflicts with the OF
>>> principle of autoprobing stuff with compatible property)
>>
>> I am not sure if I follow. What principle? Where is this principle
>> expressed?
>>
>> And you do not have in your second example additional compatibles, so
>> even if such principle exists it is not broken: everything autoprobes, I
>> think.
>>
>>>
>>
>>
> 
> The principle I'm talking about is one driver for one compatible.

There is no such principle. One compatible can map to many drivers and
many compatibles can map to one driver.

> (to be more precise excluding syscon compatible that is actually
> ignored, if a driver for the compatible is found, any other compatible
> is ignored.)
> 
> This means that declaring multiple compatible as:
> 
> compatible = "airoha,clock", "airoha,mdio"
> 
> doesn't result in the clock driver and the mdio driver probed but only
> one of the 2 (probably only clock since it does have priority)

I don't understand this example. It makes no sense - clock is not
compatible with mdio.

> 
> The "simple-mfd" compatible is just a simple compatible that indicate to
> the OF system that every child (with a compatible) should be also probed.
> And then automagically the driver gets probed.
> 
> Now the ""PAIN"" explaination. Not using the "simple-mfd" way with the
> child with compatible and putting everything in the node means having to
> create a dedicated MFD driver that just instruct to manually probe the
> clock and mdio driver. (cause the compatible system can't be used)

You already have that driver - SCU. No need for new MFD driver...


> 
> So it's 3 driver instead of 2 with the extra effort of MFD driver
> maintainer saying "Why simple-mfd is not used?"

Sorry, that's a wrong argument. You can use simple-mfd, iff it follows
standard practices. If it does not fit standard practices, you cannot
use an argument "now I need more complicated solution".

> 
> 
> There is a solution for this but I always feel it's more of a workaround
> since it doesn't really describe the HW with the DT node.

Really? All arguments you used here are driver arguments - that
something is a pain in drivers. Now you mention that hardware would not
match description.

Then let's change entire talk towards hardware description and send
patches matching hardware, not matching your MFD driver structure.

> 
> The workaround is:
> 
> 		system-controller@1fa20000 {
>                         /* The parent SCU node implement the clock driver */
>                         compatible = "airoha,an7583-scu", "syscon";
>                         reg = <0x0 0x1fb00000 0x0 0x970>;
> 
>                         #clock-cells = <1>;
>                         #reset-cells = <1>;
> 
>                         /* Clock driver is instructed to probe child */
>                         mdio {
>                                 compatible = "airoha,an7583-mdio";

Again, drop compatible.

>                                 #address-cells = <1>;
>                                 #size-cells = <0>;
> 
>                                 mdio_0: bus@0 {
>                                         reg = <0>;
>                                         resets = <&scuclk AN7583_MDIO0>;
>                                 };
> 
>                                 mdio_1: bus@1 {
>                                         reg = <1>;
>                                         resets = <&scuclk AN7583_MDIO1>;
>                                 };
>                         };
>                 };
> 
> 
> But this really moves the probe from the simple-mfd to the clock driver.
> 
> So it's either 3 solution
> 1. 2 driver + "simple-mfd"
> 2. 3 driver + PAIN (due to MFD required driver)
> 3. 2 driver + not very correct DT node structure

Option 4:
Describe it correctly. You have one device which is the SCU which is
clock provider and has subnode for MDIO bus. I don't care how many
drivers you have there (but I am sure one can do it in a simple way).

> 
> Maybe option 3. is more acceptable?
> 
> The SCU node is mainly clock + reset controller and the MDIO bus is an
> expansion of it?
> 
> Hope the long explaination makes sense to you (especially about the
> OF principle thing)
> 
> --
> Ansuel


Best regards,
Krzysztof

Re: [PATCH 4/5] dt-bindings: clock: airoha: Document support for AN7583 clock

[Christian Marangi]

On Thu, May 29, 2025 at 11:00:39AM +0200, Krzysztof Kozlowski wrote:
> On 28/05/2025 14:57, Christian Marangi wrote:
> >>> Again sorry if this question keeps coming around and I can totally
> >>> understand if you are getting annoyed by this. The reason I always ask
> >>> this is because it's a total PAIN to implement this with the driver
> >>> structure due to the old "simple-mfd" model.
> >>
> >> ... and Rob was saying multiple times: be careful when adding
> >> simple-mfd. If it bites back, then I am sorry, but everyone were warned,
> >> weren't they?
> >>
> >> What is exactly the pain anyway? You cannot instantiate children from
> >> SCU driver?
> >>
> > 
> > Answering below since they are related.
> > 
> >>>
> >>> (as again putting everything in a single node conflicts with the OF
> >>> principle of autoprobing stuff with compatible property)
> >>
> >> I am not sure if I follow. What principle? Where is this principle
> >> expressed?
> >>
> >> And you do not have in your second example additional compatibles, so
> >> even if such principle exists it is not broken: everything autoprobes, I
> >> think.
> >>
> >>>
> >>
> >>
> > 
> > The principle I'm talking about is one driver for one compatible.
> 
> There is no such principle. One compatible can map to many drivers and
> many compatibles can map to one driver.
>

I might be wrong but there is currently a limitation on the OF system
where if a driver gets probed for a node then it's ignored for any other
driver. (sorry for the bad explaination, hope it's understandable)

> > (to be more precise excluding syscon compatible that is actually
> > ignored, if a driver for the compatible is found, any other compatible
> > is ignored.)
> > 
> > This means that declaring multiple compatible as:
> > 
> > compatible = "airoha,clock", "airoha,mdio"
> > 
> > doesn't result in the clock driver and the mdio driver probed but only
> > one of the 2 (probably only clock since it does have priority)
> 
> I don't understand this example. It makes no sense - clock is not
> compatible with mdio.
> 

This was an example to put every properties in the oparent node.

> > 
> > The "simple-mfd" compatible is just a simple compatible that indicate to
> > the OF system that every child (with a compatible) should be also probed.
> > And then automagically the driver gets probed.
> > 
> > Now the ""PAIN"" explaination. Not using the "simple-mfd" way with the
> > child with compatible and putting everything in the node means having to
> > create a dedicated MFD driver that just instruct to manually probe the
> > clock and mdio driver. (cause the compatible system can't be used)
> 
> You already have that driver - SCU. No need for new MFD driver...
> 

The SCU driver is actually the clock driver (currently). This was done
for simplicity and because in SCU there were only some bits.

But now with AN7583 they put 2 MDIO controller in it.

> 
> > 
> > So it's 3 driver instead of 2 with the extra effort of MFD driver
> > maintainer saying "Why simple-mfd is not used?"
> 
> Sorry, that's a wrong argument. You can use simple-mfd, iff it follows
> standard practices. If it does not fit standard practices, you cannot
> use an argument "now I need more complicated solution".
> 

Then I think we are getting confused because I am following the usual
pattern.

This is what would be the ideal and easy solution. (ti what was done on
EN7581 with pinctrl and pwm)

		scu: system-controller@1fa20000 {
			compatible = "syscon", "simple-mfd";
			reg = <0x0 0x1fb00000 0x0 0x970>;

			scuclk: scuclk {
				compatible = "airoha,an7583-scu";
				#clock-cells = <1>;
				#reset-cells = <1>;
			};

			mdio {
				compatible = "airoha,an7583-mdio";
				#address-cells = <1>;
				#size-cells = <0>;

				mdio_0: bus@0 {
					reg = <0>;
					resets = <&scuclk AN7583_MDIO0>;
				};

				mdio_1: bus@1 {
					reg = <1>;
					resets = <&scuclk AN7583_MDIO1>;
				};
			};
		};



> > 
> > 
> > There is a solution for this but I always feel it's more of a workaround
> > since it doesn't really describe the HW with the DT node.
> 
> Really? All arguments you used here are driver arguments - that
> something is a pain in drivers. Now you mention that hardware would not
> match description.
> 
> Then let's change entire talk towards hardware description and send
> patches matching hardware, not matching your MFD driver structure.
> 

Ok to describe the HW for this register block

SCU register:
- clock
- mdio controller 1
- mdio controller 2

So this is why I think a good matching node block is:

parent node (SCU register):
	- child 1 (clock)
	- child 2 (mdio controller)
		- child 1 (mdio bus 1)
		- child 2 (mdio bus 2)

Is it wrong to model the DT node this way?

> > 
> > The workaround is:
> > 
> > 		system-controller@1fa20000 {
> >                         /* The parent SCU node implement the clock driver */
> >                         compatible = "airoha,an7583-scu", "syscon";
> >                         reg = <0x0 0x1fb00000 0x0 0x970>;
> > 
> >                         #clock-cells = <1>;
> >                         #reset-cells = <1>;
> > 
> >                         /* Clock driver is instructed to probe child */
> >                         mdio {
> >                                 compatible = "airoha,an7583-mdio";
> 
> Again, drop compatible.
> 

To drop the compatible a dedicated MFD driver is needed (or adding code
in the clock driver to register the MDIO controller and that is net
clean code wise)

> >                                 #address-cells = <1>;
> >                                 #size-cells = <0>;
> > 
> >                                 mdio_0: bus@0 {
> >                                         reg = <0>;
> >                                         resets = <&scuclk AN7583_MDIO0>;
> >                                 };
> > 
> >                                 mdio_1: bus@1 {
> >                                         reg = <1>;
> >                                         resets = <&scuclk AN7583_MDIO1>;
> >                                 };
> >                         };
> >                 };
> > 
> > 
> > But this really moves the probe from the simple-mfd to the clock driver.
> > 
> > So it's either 3 solution
> > 1. 2 driver + "simple-mfd"
> > 2. 3 driver + PAIN (due to MFD required driver)
> > 3. 2 driver + not very correct DT node structure
> 
> Option 4:
> Describe it correctly. You have one device which is the SCU which is
> clock provider and has subnode for MDIO bus. I don't care how many
> drivers you have there (but I am sure one can do it in a simple way).
> 

Ok extra driver is not a problem. Problem here is understand what is the
correct node structure cause there are various options.

If the register block is still not clear to you just tell me and will
try to describe it even more.

> > 
> > Maybe option 3. is more acceptable?
> > 
> > The SCU node is mainly clock + reset controller and the MDIO bus is an
> > expansion of it?
> > 
> > Hope the long explaination makes sense to you (especially about the
> > OF principle thing)
> > 
> 
> 
> Best regards,
> Krzysztof

-- 
	Ansuel

Re: [PATCH 4/5] dt-bindings: clock: airoha: Document support for AN7583 clock

[Krzysztof Kozlowski]

On 30/05/2025 17:26, Christian Marangi wrote:
> On Thu, May 29, 2025 at 11:00:39AM +0200, Krzysztof Kozlowski wrote:
>> On 28/05/2025 14:57, Christian Marangi wrote:
>>>>> Again sorry if this question keeps coming around and I can totally
>>>>> understand if you are getting annoyed by this. The reason I always ask
>>>>> this is because it's a total PAIN to implement this with the driver
>>>>> structure due to the old "simple-mfd" model.
>>>>
>>>> ... and Rob was saying multiple times: be careful when adding
>>>> simple-mfd. If it bites back, then I am sorry, but everyone were warned,
>>>> weren't they?
>>>>
>>>> What is exactly the pain anyway? You cannot instantiate children from
>>>> SCU driver?
>>>>
>>>
>>> Answering below since they are related.
>>>
>>>>>
>>>>> (as again putting everything in a single node conflicts with the OF
>>>>> principle of autoprobing stuff with compatible property)
>>>>
>>>> I am not sure if I follow. What principle? Where is this principle
>>>> expressed?
>>>>
>>>> And you do not have in your second example additional compatibles, so
>>>> even if such principle exists it is not broken: everything autoprobes, I
>>>> think.
>>>>
>>>>>
>>>>
>>>>
>>>
>>> The principle I'm talking about is one driver for one compatible.
>>
>> There is no such principle. One compatible can map to many drivers and
>> many compatibles can map to one driver.
>>
> 
> I might be wrong but there is currently a limitation on the OF system
> where if a driver gets probed for a node then it's ignored for any other
> driver. (sorry for the bad explaination, hope it's understandable)

Yes but this can be changed easily. See: depopulate. Whether you
populate or not-populate is not a reason to model hardware description
one way or another.

> 
>>> (to be more precise excluding syscon compatible that is actually
>>> ignored, if a driver for the compatible is found, any other compatible
>>> is ignored.)
>>>
>>> This means that declaring multiple compatible as:
>>>
>>> compatible = "airoha,clock", "airoha,mdio"
>>>
>>> doesn't result in the clock driver and the mdio driver probed but only
>>> one of the 2 (probably only clock since it does have priority)
>>
>> I don't understand this example. It makes no sense - clock is not
>> compatible with mdio.
>>
> 
> This was an example to put every properties in the oparent node.

So it was a bad example because it makes no sense. You move the
properties, not merge compatibles!

> 
>>>
>>> The "simple-mfd" compatible is just a simple compatible that indicate to
>>> the OF system that every child (with a compatible) should be also probed.
>>> And then automagically the driver gets probed.
>>>
>>> Now the ""PAIN"" explaination. Not using the "simple-mfd" way with the
>>> child with compatible and putting everything in the node means having to
>>> create a dedicated MFD driver that just instruct to manually probe the
>>> clock and mdio driver. (cause the compatible system can't be used)
>>
>> You already have that driver - SCU. No need for new MFD driver...
>>
> 
> The SCU driver is actually the clock driver (currently). This was done
> for simplicity and because in SCU there were only some bits.
> 
> But now with AN7583 they put 2 MDIO controller in it.
> 
>>
>>>
>>> So it's 3 driver instead of 2 with the extra effort of MFD driver
>>> maintainer saying "Why simple-mfd is not used?"
>>
>> Sorry, that's a wrong argument. You can use simple-mfd, iff it follows
>> standard practices. If it does not fit standard practices, you cannot
>> use an argument "now I need more complicated solution".
>>
> 
> Then I think we are getting confused because I am following the usual
> pattern.
> 
> This is what would be the ideal and easy solution. (ti what was done on
> EN7581 with pinctrl and pwm)
> 
> 		scu: system-controller@1fa20000 {
> 			compatible = "syscon", "simple-mfd";
> 			reg = <0x0 0x1fb00000 0x0 0x970>;
> 
> 			scuclk: scuclk {
> 				compatible = "airoha,an7583-scu";
> 				#clock-cells = <1>;
> 				#reset-cells = <1>;
> 			};
> 
> 			mdio {
> 				compatible = "airoha,an7583-mdio";
> 				#address-cells = <1>;
> 				#size-cells = <0>;
> 
> 				mdio_0: bus@0 {
> 					reg = <0>;
> 					resets = <&scuclk AN7583_MDIO0>;
> 				};
> 
> 				mdio_1: bus@1 {
> 					reg = <1>;
> 					resets = <&scuclk AN7583_MDIO1>;
> 				};
> 			};
> 		};
> 
> 

By repeating the same you will not get different answers but rather me
become annoyed.

> 
>>>
>>>
>>> There is a solution for this but I always feel it's more of a workaround
>>> since it doesn't really describe the HW with the DT node.
>>
>> Really? All arguments you used here are driver arguments - that
>> something is a pain in drivers. Now you mention that hardware would not
>> match description.
>>
>> Then let's change entire talk towards hardware description and send
>> patches matching hardware, not matching your MFD driver structure.
>>
> 
> Ok to describe the HW for this register block
> 
> SCU register:

What is here?

> - clock

Here are clocks, but what is in "SCU register"?

> - mdio controller 1
> - mdio controller 2
> 
> So this is why I think a good matching node block is:
> 
> parent node (SCU register):

So what is here?

> 	- child 1 (clock)
> 	- child 2 (mdio controller)
> 		- child 1 (mdio bus 1)
> 		- child 2 (mdio bus 2)
> 
> Is it wrong to model the DT node this way?

Yes and I explained you already why.

> 
>>>
>>> The workaround is:
>>>
>>> 		system-controller@1fa20000 {
>>>                         /* The parent SCU node implement the clock driver */
>>>                         compatible = "airoha,an7583-scu", "syscon";
>>>                         reg = <0x0 0x1fb00000 0x0 0x970>;
>>>
>>>                         #clock-cells = <1>;
>>>                         #reset-cells = <1>;
>>>
>>>                         /* Clock driver is instructed to probe child */
>>>                         mdio {
>>>                                 compatible = "airoha,an7583-mdio";
>>
>> Again, drop compatible.
>>
> 
> To drop the compatible a dedicated MFD driver is needed (or adding code
> in the clock driver to register the MDIO controller and that is net
> clean code wise)

If you need to do some driver changes, do some driver changes...


Best regards,
Krzysztof

[PATCH 5/5] clk: en7523: add support for Airoha AN7583 clock

[Christian Marangi]

Add support for Airoha AN7583 clock and reset.

Airoha AN7583 SoC have the same register address of EN7581 but implement
different bits and additional base clocks. Also reset are different with
the introduction of 2 dedicated MDIO line and drop of some reset lines.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 drivers/clk/clk-en7523.c | 231 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 231 insertions(+)

diff --git a/drivers/clk/clk-en7523.c b/drivers/clk/clk-en7523.c
index 07ab5b42fd5a..65c7b66ab78f 100644
--- a/drivers/clk/clk-en7523.c
+++ b/drivers/clk/clk-en7523.c
@@ -11,6 +11,7 @@
 #include <linux/reset-controller.h>
 #include <dt-bindings/clock/en7523-clk.h>
 #include <dt-bindings/reset/airoha,en7581-reset.h>
+#include <dt-bindings/reset/airoha,an7583-reset.h>
 
 #define RST_NR_PER_BANK			32
 
@@ -96,6 +97,14 @@ static const u32 bus7581_base[] = { 600000000, 540000000 };
 static const u32 npu7581_base[] = { 800000000, 750000000, 720000000, 600000000 };
 static const u32 crypto_base[] = { 540000000, 480000000 };
 static const u32 emmc7581_base[] = { 200000000, 150000000 };
+/* AN7583 */
+static const u32 gsw7583_base[] = { 540672000, 270336000, 400000000, 200000000 };
+static const u32 emi7583_base[] = { 540672000, 480000000, 400000000, 300000000 };
+static const u32 bus7583_base[] = { 600000000, 540672000, 480000000, 400000000 };
+static const u32 spi7583_base[] = { 100000000, 12500000 };
+static const u32 npu7583_base[] = { 666000000, 800000000, 720000000, 600000000 };
+static const u32 crypto7583_base[] = { 540672000, 400000000 };
+static const u32 emmc7583_base[] = { 150000000, 200000000 };
 
 static const struct en_clk_desc en7523_base_clks[] = {
 	{
@@ -298,6 +307,114 @@ static const struct en_clk_desc en7581_base_clks[] = {
 	}
 };
 
+static const struct en_clk_desc an7583_base_clks[] = {
+	{
+		.id = EN7523_CLK_GSW,
+		.name = "gsw",
+
+		.base_reg = REG_GSW_CLK_DIV_SEL,
+		.base_bits = 2,
+		.base_shift = 8,
+		.base_values = gsw7583_base,
+		.n_base_values = ARRAY_SIZE(gsw7583_base),
+
+		.div_bits = 3,
+		.div_shift = 0,
+		.div_step = 1,
+		.div_offset = 1,
+	}, {
+		.id = EN7523_CLK_EMI,
+		.name = "emi",
+
+		.base_reg = REG_EMI_CLK_DIV_SEL,
+		.base_bits = 2,
+		.base_shift = 8,
+		.base_values = emi7583_base,
+		.n_base_values = ARRAY_SIZE(emi7583_base),
+
+		.div_bits = 3,
+		.div_shift = 0,
+		.div_step = 1,
+		.div_offset = 1,
+	}, {
+		.id = EN7523_CLK_BUS,
+		.name = "bus",
+
+		.base_reg = REG_BUS_CLK_DIV_SEL,
+		.base_bits = 2,
+		.base_shift = 8,
+		.base_values = bus7583_base,
+		.n_base_values = ARRAY_SIZE(bus7583_base),
+
+		.div_bits = 3,
+		.div_shift = 0,
+		.div_step = 1,
+		.div_offset = 1,
+	}, {
+		.id = EN7523_CLK_SLIC,
+		.name = "slic",
+
+		.base_reg = REG_SPI_CLK_FREQ_SEL,
+		.base_bits = 1,
+		.base_shift = 0,
+		.base_values = slic_base,
+		.n_base_values = ARRAY_SIZE(slic_base),
+
+		.div_reg = REG_SPI_CLK_DIV_SEL,
+		.div_bits = 5,
+		.div_shift = 24,
+		.div_val0 = 20,
+		.div_step = 2,
+	}, {
+		.id = EN7523_CLK_SPI,
+		.name = "spi",
+
+		.base_reg = REG_SPI_CLK_FREQ_SEL,
+		.base_bits = 1,
+		.base_shift = 1,
+		.base_values = spi7583_base,
+		.n_base_values = ARRAY_SIZE(spi7583_base),
+
+		.div_reg = REG_SPI_CLK_DIV_SEL,
+		.div_bits = 5,
+		.div_shift = 8,
+		.div_val0 = 40,
+		.div_step = 2,
+	}, {
+		.id = EN7523_CLK_NPU,
+		.name = "npu",
+
+		.base_reg = REG_NPU_CLK_DIV_SEL,
+		.base_bits = 2,
+		.base_shift = 9,
+		.base_values = npu7583_base,
+		.n_base_values = ARRAY_SIZE(npu7583_base),
+
+		.div_bits = 3,
+		.div_shift = 0,
+		.div_step = 1,
+		.div_offset = 1,
+	}, {
+		.id = EN7523_CLK_CRYPTO,
+		.name = "crypto",
+
+		.base_reg = REG_CRYPTO_CLKSRC2,
+		.base_bits = 1,
+		.base_shift = 0,
+		.base_values = crypto7583_base,
+		.n_base_values = ARRAY_SIZE(crypto7583_base),
+	}, {
+		.id = EN7581_CLK_EMMC,
+		.name = "emmc",
+
+		.base_reg = REG_CRYPTO_CLKSRC2,
+		.base_bits = 1,
+		.base_shift = 13,
+		.base_values = emmc7583_base,
+		.n_base_values = ARRAY_SIZE(emmc7583_base),
+	}
+};
+
 static const u16 en7581_rst_ofs[] = {
 	REG_RST_CTRL2,
 	REG_RST_CTRL1,
@@ -361,6 +478,59 @@ static const u16 en7581_rst_map[] = {
 	[EN7581_XPON_MAC_RST]		= RST_NR_PER_BANK + 31,
 };
 
+static const u16 an7583_rst_map[] = {
+	/* RST_CTRL2 */
+	[AN7583_XPON_PHY_RST]		= 0,
+	[AN7583_GPON_OLT_RST]		= 1,
+	[AN7583_CPU_TIMER2_RST]		= 2,
+	[AN7583_HSUART_RST]		= 3,
+	[AN7583_UART4_RST]		= 4,
+	[AN7583_UART5_RST]		= 5,
+	[AN7583_I2C2_RST]		= 6,
+	[AN7583_XSI_MAC_RST]		= 7,
+	[AN7583_XSI_PHY_RST]		= 8,
+	[AN7583_NPU_RST]		= 9,
+	[AN7583_TRNG_MSTART_RST]	= 12,
+	[AN7583_DUAL_HSI0_RST]		= 13,
+	[AN7583_DUAL_HSI1_RST]		= 14,
+	[AN7583_DUAL_HSI0_MAC_RST]	= 16,
+	[AN7583_DUAL_HSI1_MAC_RST]	= 17,
+	[AN7583_WDMA_RST]		= 19,
+	[AN7583_WOE0_RST]		= 20,
+	[AN7583_HSDMA_RST]		= 22,
+	[AN7583_TDMA_RST]		= 24,
+	[AN7583_EMMC_RST]		= 25,
+	[AN7583_SOE_RST]		= 26,
+	[AN7583_XFP_MAC_RST]		= 28,
+	[AN7583_MDIO0]			= 30,
+	[AN7583_MDIO1]			= 31,
+	/* RST_CTRL1 */
+	[AN7583_PCM1_ZSI_ISI_RST]	= RST_NR_PER_BANK + 0,
+	[AN7583_FE_PDMA_RST]		= RST_NR_PER_BANK + 1,
+	[AN7583_FE_QDMA_RST]		= RST_NR_PER_BANK + 2,
+	[AN7583_PCM_SPIWP_RST]		= RST_NR_PER_BANK + 4,
+	[AN7583_CRYPTO_RST]		= RST_NR_PER_BANK + 6,
+	[AN7583_TIMER_RST]		= RST_NR_PER_BANK + 8,
+	[AN7583_PCM1_RST]		= RST_NR_PER_BANK + 11,
+	[AN7583_UART_RST]		= RST_NR_PER_BANK + 12,
+	[AN7583_GPIO_RST]		= RST_NR_PER_BANK + 13,
+	[AN7583_GDMA_RST]		= RST_NR_PER_BANK + 14,
+	[AN7583_I2C_MASTER_RST]		= RST_NR_PER_BANK + 16,
+	[AN7583_PCM2_ZSI_ISI_RST]	= RST_NR_PER_BANK + 17,
+	[AN7583_SFC_RST]		= RST_NR_PER_BANK + 18,
+	[AN7583_UART2_RST]		= RST_NR_PER_BANK + 19,
+	[AN7583_GDMP_RST]		= RST_NR_PER_BANK + 20,
+	[AN7583_FE_RST]			= RST_NR_PER_BANK + 21,
+	[AN7583_USB_HOST_P0_RST]	= RST_NR_PER_BANK + 22,
+	[AN7583_GSW_RST]		= RST_NR_PER_BANK + 23,
+	[AN7583_SFC2_PCM_RST]		= RST_NR_PER_BANK + 25,
+	[AN7583_PCIE0_RST]		= RST_NR_PER_BANK + 26,
+	[AN7583_PCIE1_RST]		= RST_NR_PER_BANK + 27,
+	[AN7583_CPU_TIMER_RST]		= RST_NR_PER_BANK + 28,
+	[AN7583_PCIE_HB_RST]		= RST_NR_PER_BANK + 29,
+	[AN7583_XPON_MAC_RST]		= RST_NR_PER_BANK + 31,
+};
+
 static u32 en7523_get_base_rate(const struct en_clk_desc *desc, u32 val)
 {
 	if (!desc->base_bits)
@@ -690,6 +860,54 @@ static int en7581_clk_hw_init(struct platform_device *pdev,
 	return en7581_reset_register(&pdev->dev, clk_map);
 }
 
+static int an7583_reset_register(struct device *dev, struct regmap *map)
+{
+	struct en_rst_data *rst_data;
+
+	rst_data = devm_kzalloc(dev, sizeof(*rst_data), GFP_KERNEL);
+	if (!rst_data)
+		return -ENOMEM;
+
+	rst_data->bank_ofs = en7581_rst_ofs;
+	rst_data->idx_map = an7583_rst_map;
+	rst_data->map = map;
+
+	rst_data->rcdev.nr_resets = ARRAY_SIZE(an7583_rst_map);
+	rst_data->rcdev.of_xlate = en7523_reset_xlate;
+	rst_data->rcdev.ops = &en7581_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 an7583_clk_hw_init(struct platform_device *pdev,
+			      const struct en_clk_soc_data *soc_data,
+			      struct clk_hw_onecell_data *clk_data)
+{
+	struct device *dev = &pdev->dev;
+	struct regmap *map, *clk_map;
+
+	map = syscon_regmap_lookup_by_compatible("airoha,en7581-chip-scu");
+	if (IS_ERR(map))
+		return PTR_ERR(map);
+
+	clk_map = device_node_to_regmap(dev->parent->of_node);
+	if (IS_ERR(clk_map))
+		return PTR_ERR(clk_map);
+
+	en75xx_register_clocks(dev, soc_data, clk_data, map, clk_map);
+
+	regmap_clear_bits(clk_map, REG_NP_SCU_SSTR,
+			  REG_PCIE_XSI0_SEL_MASK | REG_PCIE_XSI1_SEL_MASK);
+	regmap_update_bits(clk_map, REG_NP_SCU_PCIC, REG_PCIE_CTRL,
+			   FIELD_PREP(REG_PCIE_CTRL, 3));
+
+	return an7583_reset_register(dev, clk_map);
+}
+
 static int en7523_clk_probe(struct platform_device *pdev)
 {
 	struct device_node *node = pdev->dev.of_node;
@@ -736,9 +954,22 @@ static const struct en_clk_soc_data en7581_data = {
 	.hw_init = en7581_clk_hw_init,
 };
 
+static const struct en_clk_soc_data an7583_data = {
+	.base_clks = an7583_base_clks,
+	/* We increment num_clocks by 1 to account for additional PCIe clock */
+	.num_clocks = ARRAY_SIZE(an7583_base_clks) + 1,
+	.pcie_ops = {
+		.is_enabled = en7581_pci_is_enabled,
+		.enable = en7581_pci_enable,
+		.disable = en7581_pci_disable,
+	},
+	.hw_init = an7583_clk_hw_init,
+};
+
 static const struct of_device_id of_match_clk_en7523[] = {
 	{ .compatible = "airoha,en7523-scu", .data = &en7523_data },
 	{ .compatible = "airoha,en7581-scu", .data = &en7581_data },
+	{ .compatible = "airoha,an7583-clock", .data = &an7583_data },
 	{ /* sentinel */ }
 };
 
-- 
2.48.1