[PATCH v5 1/6] dt-bindings: clock: fsl-sai: Document i.MX8M support

[PATCH v5 1/6] dt-bindings: clock: fsl-sai: Document i.MX8M support

[Marek Vasut]

The i.MX8M/Mini/Nano/Plus variant of the SAI IP has control registers
shifted by +8 bytes and requires additional bus clock. Document support
for the i.MX8M variant of the IP with this register shift and additional
clock. Update the description slightly.

Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Marek Vasut <marex@nabladev.com>
---
Cc: Brian Masney <bmasney@redhat.com>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Michael Walle <michael@walle.cc>
Cc: Rob Herring <robh@kernel.org>
Cc: Stephen Boyd <sboyd@kernel.org>
Cc: devicetree@vger.kernel.org
Cc: linux-clk@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
V2: No change
V3: - Rebase on current next, update mail address
    - Pick ancient AB from Conor, although this may be outdated
      https://patchwork.kernel.org/project/alsa-devel/patch/20241226162234.40141-1-marex@denx.de/
    - Invert the allOf conditional to match on VF610 and limit
      the clocks/clock-names there. MX8M can have one or two
      input clock, "bus" is mandatory and "mclk1" is optional.
      The "mclk1" are used by the driver in 4/4 .
V4: No change
V5: No change
---
 .../bindings/clock/fsl,sai-clock.yaml         | 41 ++++++++++++++++---
 1 file changed, 35 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/fsl,sai-clock.yaml b/Documentation/devicetree/bindings/clock/fsl,sai-clock.yaml
index 3bca9d11c148f..90799b3b505ee 100644
--- a/Documentation/devicetree/bindings/clock/fsl,sai-clock.yaml
+++ b/Documentation/devicetree/bindings/clock/fsl,sai-clock.yaml
@@ -10,10 +10,10 @@ maintainers:
   - Michael Walle <michael@walle.cc>
 
 description: |
-  It is possible to use the BCLK pin of a SAI module as a generic clock
-  output. Some SoC are very constrained in their pin multiplexer
-  configuration. Eg. pins can only be changed groups. For example, on the
-  LS1028A SoC you can only enable SAIs in pairs. If you use only one SAI,
+  It is possible to use the BCLK pin of a SAI module as a generic
+  clock output. Some SoC are very constrained in their pin multiplexer
+  configuration. E.g. pins can only be changed in groups. For example, on
+  the LS1028A SoC you can only enable SAIs in pairs. If you use only one SAI,
   the second pins are wasted. Using this binding it is possible to use the
   clock of the second SAI as a MCLK clock for an audio codec, for example.
 
@@ -21,17 +21,46 @@ description: |
 
 properties:
   compatible:
-    const: fsl,vf610-sai-clock
+    oneOf:
+      - items:
+          - enum:
+              - fsl,imx8mm-sai-clock
+              - fsl,imx8mn-sai-clock
+              - fsl,imx8mp-sai-clock
+          - const: fsl,imx8mq-sai-clock
+      - items:
+          - enum:
+              - fsl,imx8mq-sai-clock
+              - fsl,vf610-sai-clock
 
   reg:
     maxItems: 1
 
   clocks:
-    maxItems: 1
+    minItems: 1
+    maxItems: 2
+
+  clock-names:
+    minItems: 1
+    items:
+      - const: bus
+      - const: mclk1
 
   '#clock-cells':
     const: 0
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: fsl,vf610-sai-clock
+    then:
+      properties:
+        clocks:
+          maxItems: 1
+        clock-names: false
+
 required:
   - compatible
   - reg
-- 
2.53.0

[PATCH v5 2/6] clk: fsl-sai: Sort the headers

[Marek Vasut]

Sort the headers. No functional change.

Reviewed-by: Brian Masney <bmasney@redhat.com>
Signed-off-by: Marek Vasut <marex@nabladev.com>
---
Cc: Brian Masney <bmasney@redhat.com>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Michael Walle <michael@walle.cc>
Cc: Rob Herring <robh@kernel.org>
Cc: Stephen Boyd <sboyd@kernel.org>
Cc: devicetree@vger.kernel.org
Cc: linux-clk@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
V4: New patch
V5: - Reorder this into 2/6 position
    - Add RB from Brian
---
 drivers/clk/clk-fsl-sai.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/clk-fsl-sai.c b/drivers/clk/clk-fsl-sai.c
index cba45e07562da..c59ddd519f9f5 100644
--- a/drivers/clk/clk-fsl-sai.c
+++ b/drivers/clk/clk-fsl-sai.c
@@ -5,12 +5,12 @@
  * Copyright 2020 Michael Walle <michael@walle.cc>
  */
 
-#include <linux/module.h>
-#include <linux/platform_device.h>
 #include <linux/clk-provider.h>
 #include <linux/err.h>
+#include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
+#include <linux/platform_device.h>
 #include <linux/slab.h>
 
 #define I2S_CSR		0x00
-- 
2.53.0

[PATCH v5 3/6] clk: fsl-sai: Add i.MX8M support with 8 byte register offset

[Marek Vasut]

The i.MX8M/Mini/Nano/Plus variant of the SAI IP has control registers
shifted by +8 bytes and requires additional bus clock. Add support for
the i.MX8M variant of the IP with this register shift and additional
clock.

Reviewed-by: Brian Masney <bmasney@redhat.com>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
Signed-off-by: Marek Vasut <marex@nabladev.com>
---
Cc: Brian Masney <bmasney@redhat.com>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Michael Walle <michael@walle.cc>
Cc: Rob Herring <robh@kernel.org>
Cc: Stephen Boyd <sboyd@kernel.org>
Cc: devicetree@vger.kernel.org
Cc: linux-clk@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
V2: Update commit message, align it with the bindings one
V3: - Rebase on current next, update mail address
    - Pick ancient RB from Peng, although this may be outdated
      https://patchwork.kernel.org/project/alsa-devel/patch/20241226162234.40141-2-marex@denx.de/
    - Optionally enable "bus" clock, which are needed on MX8M to operate
      register file
V4: Add RB from Brian
V5: Include clk.h
---
 drivers/clk/Kconfig       |  2 +-
 drivers/clk/clk-fsl-sai.c | 28 ++++++++++++++++++++++++----
 2 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index cc8743b11bb1f..9f7f391a5615a 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -255,7 +255,7 @@ config COMMON_CLK_FSL_FLEXSPI
 
 config COMMON_CLK_FSL_SAI
 	bool "Clock driver for BCLK of Freescale SAI cores"
-	depends on ARCH_LAYERSCAPE || COMPILE_TEST
+	depends on ARCH_LAYERSCAPE || ARCH_MXC || COMPILE_TEST
 	help
 	  This driver supports the Freescale SAI (Synchronous Audio Interface)
 	  to be used as a generic clock output. Some SoCs have restrictions
diff --git a/drivers/clk/clk-fsl-sai.c b/drivers/clk/clk-fsl-sai.c
index c59ddd519f9f5..27925893c4c27 100644
--- a/drivers/clk/clk-fsl-sai.c
+++ b/drivers/clk/clk-fsl-sai.c
@@ -6,6 +6,7 @@
  */
 
 #include <linux/clk-provider.h>
+#include <linux/clk.h>
 #include <linux/err.h>
 #include <linux/module.h>
 #include <linux/of.h>
@@ -20,6 +21,10 @@
 #define CR2_DIV_SHIFT	0
 #define CR2_DIV_WIDTH	8
 
+struct fsl_sai_data {
+	unsigned int	offset;	/* Register offset */
+};
+
 struct fsl_sai_clk {
 	struct clk_divider div;
 	struct clk_gate gate;
@@ -29,8 +34,10 @@ struct fsl_sai_clk {
 static int fsl_sai_clk_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
+	const struct fsl_sai_data *data = device_get_match_data(dev);
 	struct fsl_sai_clk *sai_clk;
 	struct clk_parent_data pdata = { .index = 0 };
+	struct clk *clk_bus;
 	void __iomem *base;
 	struct clk_hw *hw;
 
@@ -42,19 +49,23 @@ static int fsl_sai_clk_probe(struct platform_device *pdev)
 	if (IS_ERR(base))
 		return PTR_ERR(base);
 
+	clk_bus = devm_clk_get_optional_enabled(dev, "bus");
+	if (IS_ERR(clk_bus))
+		return PTR_ERR(clk_bus);
+
 	spin_lock_init(&sai_clk->lock);
 
-	sai_clk->gate.reg = base + I2S_CSR;
+	sai_clk->gate.reg = base + data->offset + I2S_CSR;
 	sai_clk->gate.bit_idx = CSR_BCE_BIT;
 	sai_clk->gate.lock = &sai_clk->lock;
 
-	sai_clk->div.reg = base + I2S_CR2;
+	sai_clk->div.reg = base + data->offset + I2S_CR2;
 	sai_clk->div.shift = CR2_DIV_SHIFT;
 	sai_clk->div.width = CR2_DIV_WIDTH;
 	sai_clk->div.lock = &sai_clk->lock;
 
 	/* set clock direction, we are the BCLK master */
-	writel(CR2_BCD, base + I2S_CR2);
+	writel(CR2_BCD, base + data->offset + I2S_CR2);
 
 	hw = devm_clk_hw_register_composite_pdata(dev, dev->of_node->name,
 						  &pdata, 1, NULL, NULL,
@@ -69,8 +80,17 @@ static int fsl_sai_clk_probe(struct platform_device *pdev)
 	return devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get, hw);
 }
 
+static const struct fsl_sai_data fsl_sai_vf610_data = {
+	.offset	= 0,
+};
+
+static const struct fsl_sai_data fsl_sai_imx8mq_data = {
+	.offset	= 8,
+};
+
 static const struct of_device_id of_fsl_sai_clk_ids[] = {
-	{ .compatible = "fsl,vf610-sai-clock" },
+	{ .compatible = "fsl,vf610-sai-clock", .data = &fsl_sai_vf610_data },
+	{ .compatible = "fsl,imx8mq-sai-clock", .data = &fsl_sai_imx8mq_data },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, of_fsl_sai_clk_ids);
-- 
2.53.0

[PATCH v5 4/6] dt-bindings: clock: fsl-sai: Document clock-cells = <1> support

[Marek Vasut]

The driver now supports generation of both BCLK and MCLK, document
support for #clock-cells = <0> for legacy case and #clock-cells = <1>
for the new case which can differentiate between BCLK and MCLK.

Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Marek Vasut <marex@nabladev.com>
---
Cc: Brian Masney <bmasney@redhat.com>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Michael Walle <michael@walle.cc>
Cc: Rob Herring <robh@kernel.org>
Cc: Stephen Boyd <sboyd@kernel.org>
Cc: devicetree@vger.kernel.org
Cc: linux-clk@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
V2: Update commit message, align it with the bindings one
V3: - Rebase on current next, update mail address
    - Pick ancient AB from Conor, although this may be outdated
      https://patchwork.kernel.org/project/alsa-devel/patch/20241226162234.40141-3-marex@denx.de/
V4: No change
V5: No change
---
 Documentation/devicetree/bindings/clock/fsl,sai-clock.yaml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/fsl,sai-clock.yaml b/Documentation/devicetree/bindings/clock/fsl,sai-clock.yaml
index 90799b3b505ee..041a63fa2d2b0 100644
--- a/Documentation/devicetree/bindings/clock/fsl,sai-clock.yaml
+++ b/Documentation/devicetree/bindings/clock/fsl,sai-clock.yaml
@@ -10,7 +10,7 @@ maintainers:
   - Michael Walle <michael@walle.cc>
 
 description: |
-  It is possible to use the BCLK pin of a SAI module as a generic
+  It is possible to use the BCLK or MCLK pin of a SAI module as a generic
   clock output. Some SoC are very constrained in their pin multiplexer
   configuration. E.g. pins can only be changed in groups. For example, on
   the LS1028A SoC you can only enable SAIs in pairs. If you use only one SAI,
@@ -47,7 +47,7 @@ properties:
       - const: mclk1
 
   '#clock-cells':
-    const: 0
+    maximum: 1
 
 allOf:
   - if:
-- 
2.53.0

[PATCH v5 5/6] clk: fsl-sai: Extract clock setup into fsl_sai_clk_register()

[Marek Vasut]

Create helper function fsl_sai_clk_register() to set up and register
SAI clock. Rename BCLK specific struct fsl_sai_clk members with bclk_
prefix. Use of_node_full_name(dev->of_node) and clock name to register
uniquely named clock. This is done in preparation for the follow up
patch, which adds MCLK support.

Signed-off-by: Marek Vasut <marex@nabladev.com>
---
Cc: Brian Masney <bmasney@redhat.com>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Michael Walle <michael@walle.cc>
Cc: Rob Herring <robh@kernel.org>
Cc: Stephen Boyd <sboyd@kernel.org>
Cc: devicetree@vger.kernel.org
Cc: linux-clk@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
V4: New patch
V5: - Include fsl_sai_of_clk_get() which returns only BCLK in here already
    - s/hw/chw/ in fsl_sai_clk_register
---
 drivers/clk/clk-fsl-sai.c | 90 +++++++++++++++++++++++++++------------
 1 file changed, 63 insertions(+), 27 deletions(-)

diff --git a/drivers/clk/clk-fsl-sai.c b/drivers/clk/clk-fsl-sai.c
index 27925893c4c27..01c5e26f55517 100644
--- a/drivers/clk/clk-fsl-sai.c
+++ b/drivers/clk/clk-fsl-sai.c
@@ -26,20 +26,71 @@ struct fsl_sai_data {
 };
 
 struct fsl_sai_clk {
-	struct clk_divider div;
-	struct clk_gate gate;
+	struct clk_divider bclk_div;
+	struct clk_gate bclk_gate;
+	struct clk_hw *bclk_hw;
 	spinlock_t lock;
 };
 
+static struct clk_hw *
+fsl_sai_of_clk_get(struct of_phandle_args *clkspec, void *data)
+{
+	struct fsl_sai_clk *sai_clk = data;
+
+	return sai_clk->bclk_hw;
+}
+
+static int fsl_sai_clk_register(struct device *dev, void __iomem *base,
+				spinlock_t *lock, struct clk_divider *div,
+				struct clk_gate *gate, struct clk_hw **hw,
+				const int gate_bit, const int dir_bit,
+				const int div_reg, char *name)
+{
+	const struct fsl_sai_data *data = device_get_match_data(dev);
+	struct clk_parent_data pdata = { .index = 0 };
+	struct clk_hw *chw;
+	char *cname;
+
+	gate->reg = base + data->offset + I2S_CSR;
+	gate->bit_idx = gate_bit;
+	gate->lock = lock;
+
+	div->reg = base + div_reg;
+	div->shift = CR2_DIV_SHIFT;
+	div->width = CR2_DIV_WIDTH;
+	div->lock = lock;
+
+	cname = devm_kasprintf(dev, GFP_KERNEL, "%s.%s",
+			       of_node_full_name(dev->of_node), name);
+	if (!cname)
+		return -ENOMEM;
+
+	chw = devm_clk_hw_register_composite_pdata(dev, cname,
+						   &pdata, 1, NULL, NULL,
+						   &div->hw,
+						   &clk_divider_ops,
+						   &gate->hw,
+						   &clk_gate_ops,
+						   CLK_SET_RATE_GATE);
+	if (IS_ERR(chw))
+		return PTR_ERR(chw);
+
+	*hw = chw;
+
+	/* Set clock direction */
+	writel(dir_bit, base + div_reg);
+
+	return 0;
+}
+
 static int fsl_sai_clk_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	const struct fsl_sai_data *data = device_get_match_data(dev);
 	struct fsl_sai_clk *sai_clk;
-	struct clk_parent_data pdata = { .index = 0 };
 	struct clk *clk_bus;
 	void __iomem *base;
-	struct clk_hw *hw;
+	int ret;
 
 	sai_clk = devm_kzalloc(dev, sizeof(*sai_clk), GFP_KERNEL);
 	if (!sai_clk)
@@ -55,29 +106,14 @@ static int fsl_sai_clk_probe(struct platform_device *pdev)
 
 	spin_lock_init(&sai_clk->lock);
 
-	sai_clk->gate.reg = base + data->offset + I2S_CSR;
-	sai_clk->gate.bit_idx = CSR_BCE_BIT;
-	sai_clk->gate.lock = &sai_clk->lock;
-
-	sai_clk->div.reg = base + data->offset + I2S_CR2;
-	sai_clk->div.shift = CR2_DIV_SHIFT;
-	sai_clk->div.width = CR2_DIV_WIDTH;
-	sai_clk->div.lock = &sai_clk->lock;
-
-	/* set clock direction, we are the BCLK master */
-	writel(CR2_BCD, base + data->offset + I2S_CR2);
-
-	hw = devm_clk_hw_register_composite_pdata(dev, dev->of_node->name,
-						  &pdata, 1, NULL, NULL,
-						  &sai_clk->div.hw,
-						  &clk_divider_ops,
-						  &sai_clk->gate.hw,
-						  &clk_gate_ops,
-						  CLK_SET_RATE_GATE);
-	if (IS_ERR(hw))
-		return PTR_ERR(hw);
-
-	return devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get, hw);
+	ret = fsl_sai_clk_register(dev, base, &sai_clk->lock,
+				   &sai_clk->bclk_div, &sai_clk->bclk_gate,
+				   &sai_clk->bclk_hw, CSR_BCE_BIT, CR2_BCD,
+				   data->offset + I2S_CR2, "BCLK");
+	if (ret)
+		return ret;
+
+	return devm_of_clk_add_hw_provider(dev, fsl_sai_of_clk_get, sai_clk);
 }
 
 static const struct fsl_sai_data fsl_sai_vf610_data = {
-- 
2.53.0

[PATCH v5 6/6] clk: fsl-sai: Add MCLK generation support

[Marek Vasut]

The driver currently supports generating BCLK. There are systems which
require generation of MCLK instead. Register new MCLK clock and handle
clock-cells = <1> to differentiate between BCLK and MCLK. In case of a
legacy system with clock-cells = <0>, the driver behaves as before, i.e.
always returns BCLK.

Note that it is not possible re-use the current SAI audio driver to
generate MCLK and correctly enable and disable the MCLK.

If SAI (audio driver) is used to control the MCLK enablement, then MCLK
clock is not always enabled, and it is not necessarily enabled when the
codec may need the clock to be enabled. There is also no way for the
codec node to specify phandle to clock provider in DT, because the SAI
(audio driver) is not clock provider.

If SAI (clock driver) is used to control the MCLK enablement, then MCLK
clock is enabled when the codec needs the clock enabled, because the
codec is the clock consumer and the SAI (clock driver) is the clock
provider, and the codec driver can request the clock to be enabled when
needed. There is also the usual phandle to clock provider in DT, because
the SAI (clock driver) is clock provider.

Acked-by: Michael Walle <mwalle@kernel.org>
Signed-off-by: Marek Vasut <marex@nabladev.com>
---
Cc: Brian Masney <bmasney@redhat.com>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Michael Walle <michael@walle.cc>
Cc: Rob Herring <robh@kernel.org>
Cc: Stephen Boyd <sboyd@kernel.org>
Cc: devicetree@vger.kernel.org
Cc: linux-clk@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
V2: No change
V3: - Rebase on current next, update mail address
    - Update commit message according to clarify the difference between
      SAI audio and SAI clock driver
    - Pick ancient AB from Michael, although this may be outdated
      https://patchwork.kernel.org/project/alsa-devel/patch/20241226162234.40141-4-marex@denx.de/
V4: Use the fsl_sai_clk_register() helper.
V5: - Move include clk.h into 3/6
    - Validate clock cells in fsl_sai_of_clk_get()
---
 drivers/clk/clk-fsl-sai.c | 34 +++++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/clk-fsl-sai.c b/drivers/clk/clk-fsl-sai.c
index 01c5e26f55517..97db34358f62b 100644
--- a/drivers/clk/clk-fsl-sai.c
+++ b/drivers/clk/clk-fsl-sai.c
@@ -16,19 +16,27 @@
 
 #define I2S_CSR		0x00
 #define I2S_CR2		0x08
+#define I2S_MCR		0x100
 #define CSR_BCE_BIT	28
+#define CSR_TE_BIT	31
 #define CR2_BCD		BIT(24)
 #define CR2_DIV_SHIFT	0
 #define CR2_DIV_WIDTH	8
+#define MCR_MOE		BIT(30)
 
 struct fsl_sai_data {
 	unsigned int	offset;	/* Register offset */
+	bool		have_mclk; /* Have MCLK control */
 };
 
 struct fsl_sai_clk {
+	const struct fsl_sai_data *data;
 	struct clk_divider bclk_div;
+	struct clk_divider mclk_div;
 	struct clk_gate bclk_gate;
+	struct clk_gate mclk_gate;
 	struct clk_hw *bclk_hw;
+	struct clk_hw *mclk_hw;
 	spinlock_t lock;
 };
 
@@ -37,7 +45,17 @@ fsl_sai_of_clk_get(struct of_phandle_args *clkspec, void *data)
 {
 	struct fsl_sai_clk *sai_clk = data;
 
-	return sai_clk->bclk_hw;
+	if (clkspec->args_count == 0)
+		return sai_clk->bclk_hw;
+
+	if (clkspec->args_count == 1) {
+		if (clkspec->args[0] == 0)
+			return sai_clk->bclk_hw;
+		if (sai_clk->data->have_mclk && clkspec->args[0] == 1)
+			return sai_clk->mclk_hw;
+	}
+
+	return ERR_PTR(-EINVAL);
 }
 
 static int fsl_sai_clk_register(struct device *dev, void __iomem *base,
@@ -104,6 +122,7 @@ static int fsl_sai_clk_probe(struct platform_device *pdev)
 	if (IS_ERR(clk_bus))
 		return PTR_ERR(clk_bus);
 
+	sai_clk->data = data;
 	spin_lock_init(&sai_clk->lock);
 
 	ret = fsl_sai_clk_register(dev, base, &sai_clk->lock,
@@ -113,15 +132,28 @@ static int fsl_sai_clk_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	if (data->have_mclk) {
+		ret = fsl_sai_clk_register(dev, base, &sai_clk->lock,
+					   &sai_clk->mclk_div,
+					   &sai_clk->mclk_gate,
+					   &sai_clk->mclk_hw,
+					   CSR_TE_BIT, MCR_MOE, I2S_MCR,
+					   "MCLK");
+		if (ret)
+			return ret;
+	}
+
 	return devm_of_clk_add_hw_provider(dev, fsl_sai_of_clk_get, sai_clk);
 }
 
 static const struct fsl_sai_data fsl_sai_vf610_data = {
 	.offset	= 0,
+	.have_mclk = false,
 };
 
 static const struct fsl_sai_data fsl_sai_imx8mq_data = {
 	.offset	= 8,
+	.have_mclk = true,
 };
 
 static const struct of_device_id of_fsl_sai_clk_ids[] = {
-- 
2.53.0

Re: [PATCH v5 5/6] clk: fsl-sai: Extract clock setup into fsl_sai_clk_register()

[Brian Masney]

Hi Marek,

On Tue, Apr 07, 2026 at 11:10:02PM +0200, Marek Vasut wrote:
> Create helper function fsl_sai_clk_register() to set up and register
> SAI clock. Rename BCLK specific struct fsl_sai_clk members with bclk_
> prefix. Use of_node_full_name(dev->of_node) and clock name to register
> uniquely named clock. This is done in preparation for the follow up
> patch, which adds MCLK support.
> 
> Signed-off-by: Marek Vasut <marex@nabladev.com>
> ---
> Cc: Brian Masney <bmasney@redhat.com>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Michael Walle <michael@walle.cc>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Stephen Boyd <sboyd@kernel.org>
> Cc: devicetree@vger.kernel.org
> Cc: linux-clk@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
> V4: New patch
> V5: - Include fsl_sai_of_clk_get() which returns only BCLK in here already
>     - s/hw/chw/ in fsl_sai_clk_register
> ---
>  drivers/clk/clk-fsl-sai.c | 90 +++++++++++++++++++++++++++------------
>  1 file changed, 63 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/clk/clk-fsl-sai.c b/drivers/clk/clk-fsl-sai.c
> index 27925893c4c27..01c5e26f55517 100644
> --- a/drivers/clk/clk-fsl-sai.c
> +++ b/drivers/clk/clk-fsl-sai.c
> @@ -26,20 +26,71 @@ struct fsl_sai_data {
>  };
>  
>  struct fsl_sai_clk {
> -	struct clk_divider div;
> -	struct clk_gate gate;
> +	struct clk_divider bclk_div;
> +	struct clk_gate bclk_gate;
> +	struct clk_hw *bclk_hw;
>  	spinlock_t lock;
>  };
>  
> +static struct clk_hw *
> +fsl_sai_of_clk_get(struct of_phandle_args *clkspec, void *data)
> +{
> +	struct fsl_sai_clk *sai_clk = data;
> +
> +	return sai_clk->bclk_hw;
> +}
> +
> +static int fsl_sai_clk_register(struct device *dev, void __iomem *base,
> +				spinlock_t *lock, struct clk_divider *div,
> +				struct clk_gate *gate, struct clk_hw **hw,
> +				const int gate_bit, const int dir_bit,
> +				const int div_reg, char *name)
> +{
> +	const struct fsl_sai_data *data = device_get_match_data(dev);
> +	struct clk_parent_data pdata = { .index = 0 };
> +	struct clk_hw *chw;
> +	char *cname;
> +
> +	gate->reg = base + data->offset + I2S_CSR;
> +	gate->bit_idx = gate_bit;
> +	gate->lock = lock;
> +
> +	div->reg = base + div_reg;
> +	div->shift = CR2_DIV_SHIFT;
> +	div->width = CR2_DIV_WIDTH;
> +	div->lock = lock;
> +
> +	cname = devm_kasprintf(dev, GFP_KERNEL, "%s.%s",
> +			       of_node_full_name(dev->of_node), name);

Sashiko has the following feedback:
https://sashiko.dev/#/patchset/20260407211123.77602-1-marex%40nabladev.com

    Does using of_node_full_name() break debugfs directory creation? 
    
    The full node name usually contains slashes (e.g., soc/bus@1000/sai@2000),
    which causes lookup_one_len() to reject the name with -EACCES when the CCF
    tries to create the directories.
    
    Also, if the device is instantiated without Device Tree, this evaluates to
    "<no-node>", potentially causing collisions if multiple instances exist.
    Would dev_name(dev) be more appropriate here?

> +	if (!cname)
> +		return -ENOMEM;
> +
> +	chw = devm_clk_hw_register_composite_pdata(dev, cname,
> +						   &pdata, 1, NULL, NULL,
> +						   &div->hw,
> +						   &clk_divider_ops,
> +						   &gate->hw,
> +						   &clk_gate_ops,
> +						   CLK_SET_RATE_GATE);
> +	if (IS_ERR(chw))
> +		return PTR_ERR(chw);
> +
> +	*hw = chw;
> +
> +	/* Set clock direction */
> +	writel(dir_bit, base + div_reg);

Sashiko also has the following feedback:

    Is it safe to initialize the hardware register after registering the clock
    with the CCF?
    
    During devm_clk_hw_register_composite_pdata(), the Common Clock Framework
    inspects the hardware to calculate and cache the initial clock rate based on
    the existing divider value.
    
    The subsequent writel() completely overwrites the 32-bit register, setting
    the direction bit and zeroing out the divider bits. Because this occurs
    after registration, the CCF is completely unaware of the change, leaving its
    cached rate stale and mismatched with the hardware.
    
    Additionally, since the clock is already exposed to the system, could this
    lockless writel() race with concurrent clock operations like clk_set_rate()
    and clobber new divider configurations?
    
    The original code safely performed this initialization before registration.

Brian

Re: [PATCH v5 5/6] clk: fsl-sai: Extract clock setup into fsl_sai_clk_register()

[Brian Masney]

Hi Marek,

On Tue, Apr 07, 2026 at 11:10:02PM +0200, Marek Vasut wrote:
> Create helper function fsl_sai_clk_register() to set up and register
> SAI clock. Rename BCLK specific struct fsl_sai_clk members with bclk_
> prefix. Use of_node_full_name(dev->of_node) and clock name to register
> uniquely named clock. This is done in preparation for the follow up
> patch, which adds MCLK support.
> 
> Signed-off-by: Marek Vasut <marex@nabladev.com>
> ---
> Cc: Brian Masney <bmasney@redhat.com>
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Michael Walle <michael@walle.cc>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Stephen Boyd <sboyd@kernel.org>
> Cc: devicetree@vger.kernel.org
> Cc: linux-clk@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
> V4: New patch
> V5: - Include fsl_sai_of_clk_get() which returns only BCLK in here already
>     - s/hw/chw/ in fsl_sai_clk_register
> ---
>  drivers/clk/clk-fsl-sai.c | 90 +++++++++++++++++++++++++++------------
>  1 file changed, 63 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/clk/clk-fsl-sai.c b/drivers/clk/clk-fsl-sai.c
> index 27925893c4c27..01c5e26f55517 100644
> --- a/drivers/clk/clk-fsl-sai.c
> +++ b/drivers/clk/clk-fsl-sai.c
> @@ -26,20 +26,71 @@ struct fsl_sai_data {
>  };
>  
>  struct fsl_sai_clk {
> -	struct clk_divider div;
> -	struct clk_gate gate;
> +	struct clk_divider bclk_div;
> +	struct clk_gate bclk_gate;
> +	struct clk_hw *bclk_hw;
>  	spinlock_t lock;
>  };
>  
> +static struct clk_hw *
> +fsl_sai_of_clk_get(struct of_phandle_args *clkspec, void *data)
> +{
> +	struct fsl_sai_clk *sai_clk = data;
> +
> +	return sai_clk->bclk_hw;
> +}
> +
> +static int fsl_sai_clk_register(struct device *dev, void __iomem *base,
> +				spinlock_t *lock, struct clk_divider *div,
> +				struct clk_gate *gate, struct clk_hw **hw,
> +				const int gate_bit, const int dir_bit,
> +				const int div_reg, char *name)
> +{
> +	const struct fsl_sai_data *data = device_get_match_data(dev);
> +	struct clk_parent_data pdata = { .index = 0 };
> +	struct clk_hw *chw;
> +	char *cname;
> +
> +	gate->reg = base + data->offset + I2S_CSR;
> +	gate->bit_idx = gate_bit;
> +	gate->lock = lock;
> +
> +	div->reg = base + div_reg;
> +	div->shift = CR2_DIV_SHIFT;
> +	div->width = CR2_DIV_WIDTH;
> +	div->lock = lock;
> +
> +	cname = devm_kasprintf(dev, GFP_KERNEL, "%s.%s",
> +			       of_node_full_name(dev->of_node), name);
> +	if (!cname)
> +		return -ENOMEM;
> +
> +	chw = devm_clk_hw_register_composite_pdata(dev, cname,
> +						   &pdata, 1, NULL, NULL,
> +						   &div->hw,
> +						   &clk_divider_ops,
> +						   &gate->hw,
> +						   &clk_gate_ops,
> +						   CLK_SET_RATE_GATE);
> +	if (IS_ERR(chw))
> +		return PTR_ERR(chw);
> +
> +	*hw = chw;
> +
> +	/* Set clock direction */
> +	writel(dir_bit, base + div_reg);
> +
> +	return 0;
> +}
> +
>  static int fsl_sai_clk_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	const struct fsl_sai_data *data = device_get_match_data(dev);
>  	struct fsl_sai_clk *sai_clk;
> -	struct clk_parent_data pdata = { .index = 0 };
>  	struct clk *clk_bus;
>  	void __iomem *base;
> -	struct clk_hw *hw;
> +	int ret;
>  
>  	sai_clk = devm_kzalloc(dev, sizeof(*sai_clk), GFP_KERNEL);
>  	if (!sai_clk)
> @@ -55,29 +106,14 @@ static int fsl_sai_clk_probe(struct platform_device *pdev)
>  
>  	spin_lock_init(&sai_clk->lock);
>  
> -	sai_clk->gate.reg = base + data->offset + I2S_CSR;
> -	sai_clk->gate.bit_idx = CSR_BCE_BIT;
> -	sai_clk->gate.lock = &sai_clk->lock;
> -
> -	sai_clk->div.reg = base + data->offset + I2S_CR2;
> -	sai_clk->div.shift = CR2_DIV_SHIFT;
> -	sai_clk->div.width = CR2_DIV_WIDTH;
> -	sai_clk->div.lock = &sai_clk->lock;
> -
> -	/* set clock direction, we are the BCLK master */
> -	writel(CR2_BCD, base + data->offset + I2S_CR2);
> -
> -	hw = devm_clk_hw_register_composite_pdata(dev, dev->of_node->name,
> -						  &pdata, 1, NULL, NULL,
> -						  &sai_clk->div.hw,
> -						  &clk_divider_ops,
> -						  &sai_clk->gate.hw,
> -						  &clk_gate_ops,
> -						  CLK_SET_RATE_GATE);
> -	if (IS_ERR(hw))
> -		return PTR_ERR(hw);
> -
> -	return devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get, hw);
> +	ret = fsl_sai_clk_register(dev, base, &sai_clk->lock,
> +				   &sai_clk->bclk_div, &sai_clk->bclk_gate,
> +				   &sai_clk->bclk_hw, CSR_BCE_BIT, CR2_BCD,
> +				   data->offset + I2S_CR2, "BCLK");
                                   ^^^^^^^^^^^^^^^^^^^^^^
Sashiko reports the following:
https://sashiko.dev/#/patchset/20260407211123.77602-1-marex%40nabladev.com

    For MCLK, this evaluates to writel(MCR_MOE, base + I2S_MCR). Does this
    overwrite the entire register and clear all other bits to 0? This could zero
    out fields like the MCLK Divider Enable and Clock Source Select, breaking
    the MCLK hardware divider.

It's the same behavior as what was there previously. There's a lot more
to the comment. Can you look and see if there's anything else valid in
Sashiko's comment that should be addressed while changes are being made
here?

Otherwise, based on what was there previously, this looks good to me.

Brian

Re: [PATCH v5 5/6] clk: fsl-sai: Extract clock setup into fsl_sai_clk_register()

[Marek Vasut]

On 4/8/26 11:58 PM, Brian Masney wrote:

Hi,

>> +static int fsl_sai_clk_register(struct device *dev, void __iomem *base,
>> +				spinlock_t *lock, struct clk_divider *div,
>> +				struct clk_gate *gate, struct clk_hw **hw,
>> +				const int gate_bit, const int dir_bit,
>> +				const int div_reg, char *name)
>> +{
>> +	const struct fsl_sai_data *data = device_get_match_data(dev);
>> +	struct clk_parent_data pdata = { .index = 0 };
>> +	struct clk_hw *chw;
>> +	char *cname;
>> +
>> +	gate->reg = base + data->offset + I2S_CSR;
>> +	gate->bit_idx = gate_bit;
>> +	gate->lock = lock;
>> +
>> +	div->reg = base + div_reg;
>> +	div->shift = CR2_DIV_SHIFT;
>> +	div->width = CR2_DIV_WIDTH;
>> +	div->lock = lock;
>> +
>> +	cname = devm_kasprintf(dev, GFP_KERNEL, "%s.%s",
>> +			       of_node_full_name(dev->of_node), name);
> 
> Sashiko has the following feedback:
> https://sashiko.dev/#/patchset/20260407211123.77602-1-marex%40nabladev.com
> 
>      Does using of_node_full_name() break debugfs directory creation?
>      
>      The full node name usually contains slashes (e.g., soc/bus@1000/sai@2000),
>      which causes lookup_one_len() to reject the name with -EACCES when the CCF
>      tries to create the directories.

This is nonsense, the node name is clock-controller@12340000 or 
something like that .

>      Also, if the device is instantiated without Device Tree, this evaluates to
>      "<no-node>", potentially causing collisions if multiple instances exist.
>      Would dev_name(dev) be more appropriate here?

This driver is always instantiated from DT.

>> +	if (!cname)
>> +		return -ENOMEM;
>> +
>> +	chw = devm_clk_hw_register_composite_pdata(dev, cname,
>> +						   &pdata, 1, NULL, NULL,
>> +						   &div->hw,
>> +						   &clk_divider_ops,
>> +						   &gate->hw,
>> +						   &clk_gate_ops,
>> +						   CLK_SET_RATE_GATE);
>> +	if (IS_ERR(chw))
>> +		return PTR_ERR(chw);
>> +
>> +	*hw = chw;
>> +
>> +	/* Set clock direction */
>> +	writel(dir_bit, base + div_reg);
> 
> Sashiko also has the following feedback:
> 
>      Is it safe to initialize the hardware register after registering the clock
>      with the CCF?
The BCD/MOE/DIVEN/DIV fields of the CR2/MCR registers are all zeroes, so 
yes, either order is safe.

Re: [PATCH v5 5/6] clk: fsl-sai: Extract clock setup into fsl_sai_clk_register()

[Marek Vasut]

On 4/9/26 12:12 AM, Brian Masney wrote:

Hi,

>> -	hw = devm_clk_hw_register_composite_pdata(dev, dev->of_node->name,
>> -						  &pdata, 1, NULL, NULL,
>> -						  &sai_clk->div.hw,
>> -						  &clk_divider_ops,
>> -						  &sai_clk->gate.hw,
>> -						  &clk_gate_ops,
>> -						  CLK_SET_RATE_GATE);
>> -	if (IS_ERR(hw))
>> -		return PTR_ERR(hw);
>> -
>> -	return devm_of_clk_add_hw_provider(dev, of_clk_hw_simple_get, hw);
>> +	ret = fsl_sai_clk_register(dev, base, &sai_clk->lock,
>> +				   &sai_clk->bclk_div, &sai_clk->bclk_gate,
>> +				   &sai_clk->bclk_hw, CSR_BCE_BIT, CR2_BCD,
>> +				   data->offset + I2S_CR2, "BCLK");
>                                     ^^^^^^^^^^^^^^^^^^^^^^
> Sashiko reports the following:
> https://sashiko.dev/#/patchset/20260407211123.77602-1-marex%40nabladev.com
> 
>      For MCLK, this evaluates to writel(MCR_MOE, base + I2S_MCR). Does this
>      overwrite the entire register and clear all other bits to 0? This could zero
>      out fields like the MCLK Divider Enable and Clock Source Select, breaking
>      the MCLK hardware divider.
> 
> It's the same behavior as what was there previously. There's a lot more
> to the comment. Can you look and see if there's anything else valid in
> Sashiko's comment that should be addressed while changes are being made
> here?
> 
> Otherwise, based on what was there previously, this looks good to me.
I'll move the writel() before devm_clk_hw_register_composite_pdata() 
even if it makes no difference in this particular case.