dt-bindings: clock: fsl-sai: Document i.MX8M support

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 <[email protected]>

---
Cc: Brian Masney <[email protected]>
Cc: Conor Dooley <[email protected]>
Cc: Krzysztof Kozlowski <[email protected]>
Cc: Michael Turquette <[email protected]>
Cc: Michael Walle <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Stephen Boyd <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
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(-)

drivers/clk/clk-fsl-sai.c +23 -2

index 27925893c4c27..01c5e26f55517 100644
@@ -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;

drivers/clk/clk-fsl-sai.c +7 -0

+	if (!cname)
+		return -ENOMEM;
+
+	chw = devm_clk_hw_register_composite_pdata(dev, cname,
+						   &pdata, 1, NULL, NULL,
+						   &div->hw,
+						   &clk_divider_ops,

Quoted 12 lines

drivers/clk/clk-fsl-sai.c +12 -0

+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;

drivers/clk/clk-fsl-sai.c struct fsl_sai_data { · line 64

Quoted 10 lines

drivers/clk/clk-fsl-sai.c +10 -0

+	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?

Quoted 7 lines

drivers/clk/clk-fsl-sai.c +7 -0

+	if (!cname)
+		return -ENOMEM;
+
+	chw = devm_clk_hw_register_composite_pdata(dev, cname,
+						   &pdata, 1, NULL, NULL,
+						   &div->hw,
+						   &clk_divider_ops,

drivers/clk/clk-fsl-sai.c struct fsl_sai_data { · line 81

Quoted 10 lines

drivers/clk/clk-fsl-sai.c +10 -0

+						   &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?

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 <[email protected]>

---
Cc: Brian Masney <[email protected]>
Cc: Conor Dooley <[email protected]>
Cc: Krzysztof Kozlowski <[email protected]>
Cc: Michael Turquette <[email protected]>
Cc: Michael Walle <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Stephen Boyd <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
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(-)

drivers/clk/clk-fsl-sai.c +55 -21

index 27925893c4c27..01c5e26f55517 100644
@@ -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,

Quoted 5 lines

drivers/clk/clk-fsl-sai.c +0 -5

-	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,

drivers/clk/clk-fsl-sai.c static int fsl_sai_clk_probe(struct platform_device *pdev) · line 112

Quoted 10 lines

drivers/clk/clk-fsl-sai.c +4 -6

-						  &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.