Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] clk: imx: Add audio PLL debugfs for K-divider control
@ 2026-05-19  9:19 Jacky Bai
  2026-05-25  1:44 ` Peng Fan
  0 siblings, 1 reply; 3+ messages in thread
From: Jacky Bai @ 2026-05-19  9:19 UTC (permalink / raw)
  To: Abel Vesa, Peng Fan, Michael Turquette, Stephen Boyd,
	Brian Masney, Frank Li, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam
  Cc: linux-clk, imx, linux-arm-kernel, Jacky Bai

Add debugfs support for runtime tuning of the audio PLL K divider,
which enables fine-grained frequency adjustments for audio PLL.
This is used for:
  - Audio clock calibration and testing
  - Debugging audio synchronization issues

Two debug interfaces are exported to userspace:
  - delta_k: It is used to adjust the K divider in PLL based on small
    steps
  - pll_parameter: It is used for get PLL's current M-divider,
    P-divider, S-divider & K-divider setting in PLL register

Signed-off-by: Jacky Bai <ping.bai@nxp.com>
---
Changes in v2:
- remove the examples from commit log.
- refine the comments blocks
- add delta_k read back support
- resolve the comments from Sashiko AI
- add prefix to audio_pll_debug_init API
- Link to v1: https://lore.kernel.org/r/20260512-imx8m_pll_debugfs-v1-1-e1e44b21be90@nxp.com
---
 drivers/clk/imx/clk-imx8mm.c  |   6 +++
 drivers/clk/imx/clk-pll14xx.c | 107 ++++++++++++++++++++++++++++++++++++++++++
 drivers/clk/imx/clk.h         |   1 +
 3 files changed, 114 insertions(+)

diff --git a/drivers/clk/imx/clk-imx8mm.c b/drivers/clk/imx/clk-imx8mm.c
index 319af4deec01c188524807d39dff92bbd08f3601..f080a4dcead359f9494b289ebd277ef2dfdf72cd 100644
--- a/drivers/clk/imx/clk-imx8mm.c
+++ b/drivers/clk/imx/clk-imx8mm.c
@@ -298,6 +298,7 @@ static struct clk_hw **hws;
 
 static int imx8mm_clocks_probe(struct platform_device *pdev)
 {
+	struct clk_hw *audio_pll_hws[2];
 	struct device *dev = &pdev->dev;
 	struct device_node *np = dev->of_node;
 	void __iomem *base;
@@ -610,6 +611,11 @@ static int imx8mm_clocks_probe(struct platform_device *pdev)
 
 	imx_register_uart_clocks();
 
+	/* Add debug interface for audio PLLs */
+	audio_pll_hws[0] = hws[IMX8MM_AUDIO_PLL1];
+	audio_pll_hws[1] = hws[IMX8MM_AUDIO_PLL2];
+	imx_audio_pll_debug_init(audio_pll_hws, ARRAY_SIZE(audio_pll_hws));
+
 	return 0;
 
 unregister_hws:
diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c
index 39600ee22be3066683b562aa6f2a8b750d19c4cc..b00529cb196a22ad0444b1efdbc3b53d0e11c20b 100644
--- a/drivers/clk/imx/clk-pll14xx.c
+++ b/drivers/clk/imx/clk-pll14xx.c
@@ -8,11 +8,13 @@
 #include <linux/bitfield.h>
 #include <linux/bits.h>
 #include <linux/clk-provider.h>
+#include <linux/debugfs.h>
 #include <linux/err.h>
 #include <linux/export.h>
 #include <linux/io.h>
 #include <linux/iopoll.h>
 #include <linux/slab.h>
+#include <linux/spinlock.h>
 #include <linux/jiffies.h>
 
 #include "clk.h"
@@ -40,6 +42,8 @@ struct clk_pll14xx {
 	enum imx_pll14xx_type		type;
 	const struct imx_pll14xx_rate_table *rate_table;
 	int rate_count;
+	s16 delta_k;
+	spinlock_t lock;
 };
 
 #define to_clk_pll14xx(_hw) container_of(_hw, struct clk_pll14xx, hw)
@@ -361,6 +365,7 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
 {
 	struct clk_pll14xx *pll = to_clk_pll14xx(hw);
 	struct imx_pll14xx_rate_table rate;
+	unsigned long flags;
 	u32 gnrl_ctl, div_ctl0;
 	int ret;
 
@@ -374,9 +379,13 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
 		div_ctl0 |= FIELD_PREP(SDIV_MASK, rate.sdiv);
 		writel_relaxed(div_ctl0, pll->base + DIV_CTL0);
 
+		spin_lock_irqsave(&pll->lock, flags);
+
 		writel_relaxed(FIELD_PREP(KDIV_MASK, rate.kdiv),
 			       pll->base + DIV_CTL1);
 
+		spin_unlock_irqrestore(&pll->lock, flags);
+
 		return 0;
 	}
 
@@ -394,8 +403,12 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
 		   FIELD_PREP(SDIV_MASK, rate.sdiv);
 	writel_relaxed(div_ctl0, pll->base + DIV_CTL0);
 
+	spin_lock_irqsave(&pll->lock, flags);
+
 	writel_relaxed(FIELD_PREP(KDIV_MASK, rate.kdiv), pll->base + DIV_CTL1);
 
+	spin_unlock_irqrestore(&pll->lock, flags);
+
 	/*
 	 * According to SPEC, t3 - t2 need to be greater than
 	 * 1us and 1/FREF, respectively.
@@ -508,6 +521,8 @@ struct clk_hw *imx_dev_clk_hw_pll14xx(struct device *dev, const char *name,
 	if (!pll)
 		return ERR_PTR(-ENOMEM);
 
+	spin_lock_init(&pll->lock);
+
 	init.name = name;
 	init.flags = pll_clk->flags;
 	init.parent_names = &parent_name;
@@ -551,3 +566,95 @@ struct clk_hw *imx_dev_clk_hw_pll14xx(struct device *dev, const char *name,
 	return hw;
 }
 EXPORT_SYMBOL_GPL(imx_dev_clk_hw_pll14xx);
+
+/*
+ * Debugfs interface for Audio PLL runtime monitoring and control
+ *
+ * This interface allows dynamic adjustment of the Audio PLL
+ * K-divider for precise frequency tuning, particularly useful
+ * for audio applications.
+ *
+ * examples for the usage of the two interfaces:
+ *   1): Get the current PLL setting of dividers
+ *     cat /sys/kernel/debug/audio_pll_monitor/audio_pll1/pll_parameter
+ *
+ *   2): Adjust the K-divider by a small delta_k
+ *     echo 1 > /sys/kernel/debug/audio_pll_monitor/audio_pll1/delta_k;
+ */
+#ifdef CONFIG_DEBUG_FS
+static int pll_delta_k_get(void *data, u64 *val)
+{
+	struct clk_pll14xx *pll = to_clk_pll14xx(data);
+	*val = pll->delta_k;
+	return 0;
+}
+
+static int pll_delta_k_set(void *data, u64 val)
+{
+	struct clk_pll14xx *pll = to_clk_pll14xx(data);
+	unsigned long flags;
+	u32 div_ctl1;
+	s16 kdiv, delta_k;
+
+	delta_k = (s16)clamp_t(long, val, KDIV_MIN, KDIV_MAX);
+	pll->delta_k = delta_k;
+
+	spin_lock_irqsave(&pll->lock, flags);
+
+	div_ctl1 = readl_relaxed(pll->base + DIV_CTL1);
+	kdiv = (s16)FIELD_GET(KDIV_MASK, div_ctl1);
+	kdiv = (s16)clamp_t(s32, (s32)kdiv + delta_k, KDIV_MIN, KDIV_MAX);
+	writel_relaxed(FIELD_PREP(KDIV_MASK, kdiv), pll->base + DIV_CTL1);
+
+	spin_unlock_irqrestore(&pll->lock, flags);
+
+	return 0;
+}
+DEFINE_DEBUGFS_ATTRIBUTE_SIGNED(delta_k_fops, pll_delta_k_get, pll_delta_k_set, "%lld\n");
+
+static int pll_setting_show(struct seq_file *s, void *data)
+{
+	struct clk_pll14xx *pll = to_clk_pll14xx(s->private);
+	u32 div_ctl0, div_ctl1;
+	u32 mdiv, pdiv, sdiv, kdiv;
+
+	div_ctl0 = readl_relaxed(pll->base + DIV_CTL0);
+	div_ctl1 = readl_relaxed(pll->base + DIV_CTL1);
+
+	mdiv = FIELD_GET(MDIV_MASK, div_ctl0);
+	pdiv = FIELD_GET(PDIV_MASK, div_ctl0);
+	sdiv = FIELD_GET(SDIV_MASK, div_ctl0);
+	kdiv = FIELD_GET(KDIV_MASK, div_ctl1);
+
+	seq_printf(s, "Mdiv: 0x%x; Pdiv: 0x%x; Sdiv: 0x%x; Kdiv: 0x%x\n",
+		   mdiv, pdiv, sdiv, kdiv);
+
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(pll_setting);
+
+void imx_audio_pll_debug_init(struct clk_hw *hws[], unsigned int num_plls)
+{
+	struct dentry *rootdir, *audio_pll_dir;
+	const char *pll_name;
+	int i;
+
+	rootdir = debugfs_create_dir("audio_pll_monitor", NULL);
+
+	for (i = 0; i < num_plls; i++) {
+		if (!IS_ERR_OR_NULL(hws[i])) {
+			pll_name = clk_hw_get_name(hws[i]);
+			audio_pll_dir = debugfs_create_dir(pll_name, rootdir);
+			debugfs_create_file_unsafe("delta_k", 0600, audio_pll_dir,
+					hws[i], &delta_k_fops);
+			debugfs_create_file("pll_parameter", 0444, audio_pll_dir,
+					hws[i], &pll_setting_fops);
+		}
+	}
+}
+#else /* !CONFIG_DEBUG_FS */
+void imx_audio_pll_debug_init(struct clk_hw *hws[], unsigned int num_plls)
+{
+}
+#endif /* CONFIG_DEBUG_FS */
+EXPORT_SYMBOL_GPL(imx_audio_pll_debug_init);
diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h
index aa5202f284f3d1b7c1b4bf65e2329831832b43a5..16e0bafa0c9b99cb4eee37a216e0a274d27f11fc 100644
--- a/drivers/clk/imx/clk.h
+++ b/drivers/clk/imx/clk.h
@@ -487,4 +487,5 @@ struct clk_hw *imx_clk_gpr_mux(const char *name, const char *compatible,
 			       u32 reg, const char **parent_names,
 			       u8 num_parents, const u32 *mux_table, u32 mask);
 
+void imx_audio_pll_debug_init(struct clk_hw **hws, unsigned int num_plls);
 #endif

---
base-commit: e98d21c170b01ddef366f023bbfcf6b31509fa83
change-id: 20260421-imx8m_pll_debugfs-246d0fbbb617

Best regards,
-- 
Jacky Bai <ping.bai@nxp.com>



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] clk: imx: Add audio PLL debugfs for K-divider control
  2026-05-19  9:19 [PATCH v2] clk: imx: Add audio PLL debugfs for K-divider control Jacky Bai
@ 2026-05-25  1:44 ` Peng Fan
  2026-05-25  7:20   ` Jacky Bai
  0 siblings, 1 reply; 3+ messages in thread
From: Peng Fan @ 2026-05-25  1:44 UTC (permalink / raw)
  To: Jacky Bai
  Cc: Abel Vesa, Peng Fan, Michael Turquette, Stephen Boyd,
	Brian Masney, Frank Li, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, linux-clk, imx, linux-arm-kernel

On Tue, May 19, 2026 at 05:19:18PM +0800, Jacky Bai wrote:
>Add debugfs support for runtime tuning of the audio PLL K divider,
>which enables fine-grained frequency adjustments for audio PLL.
>This is used for:
>  - Audio clock calibration and testing
>  - Debugging audio synchronization issues
>
>Two debug interfaces are exported to userspace:
>  - delta_k: It is used to adjust the K divider in PLL based on small
>    steps
>  - pll_parameter: It is used for get PLL's current M-divider,
>    P-divider, S-divider & K-divider setting in PLL register
>
>Signed-off-by: Jacky Bai <ping.bai@nxp.com>
>---
>Changes in v2:
>- remove the examples from commit log.
>- refine the comments blocks
>- add delta_k read back support
>- resolve the comments from Sashiko AI
>- add prefix to audio_pll_debug_init API
>- Link to v1: https://lore.kernel.org/r/20260512-imx8m_pll_debugfs-v1-1-e1e44b21be90@nxp.com
>---
> drivers/clk/imx/clk-imx8mm.c  |   6 +++
> drivers/clk/imx/clk-pll14xx.c | 107 ++++++++++++++++++++++++++++++++++++++++++
> drivers/clk/imx/clk.h         |   1 +
> 3 files changed, 114 insertions(+)
>
>diff --git a/drivers/clk/imx/clk-imx8mm.c b/drivers/clk/imx/clk-imx8mm.c
>index 319af4deec01c188524807d39dff92bbd08f3601..f080a4dcead359f9494b289ebd277ef2dfdf72cd 100644
>--- a/drivers/clk/imx/clk-imx8mm.c
>+++ b/drivers/clk/imx/clk-imx8mm.c
>@@ -298,6 +298,7 @@ static struct clk_hw **hws;
> 
> static int imx8mm_clocks_probe(struct platform_device *pdev)
> {
>+	struct clk_hw *audio_pll_hws[2];
> 	struct device *dev = &pdev->dev;
> 	struct device_node *np = dev->of_node;
> 	void __iomem *base;
>@@ -610,6 +611,11 @@ static int imx8mm_clocks_probe(struct platform_device *pdev)
> 
> 	imx_register_uart_clocks();
> 
>+	/* Add debug interface for audio PLLs */
>+	audio_pll_hws[0] = hws[IMX8MM_AUDIO_PLL1];
>+	audio_pll_hws[1] = hws[IMX8MM_AUDIO_PLL2];
>+	imx_audio_pll_debug_init(audio_pll_hws, ARRAY_SIZE(audio_pll_hws));
>+
> 	return 0;
> 
> unregister_hws:
>diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c
>index 39600ee22be3066683b562aa6f2a8b750d19c4cc..b00529cb196a22ad0444b1efdbc3b53d0e11c20b 100644
>--- a/drivers/clk/imx/clk-pll14xx.c
>+++ b/drivers/clk/imx/clk-pll14xx.c
>@@ -8,11 +8,13 @@
> #include <linux/bitfield.h>
> #include <linux/bits.h>
> #include <linux/clk-provider.h>
>+#include <linux/debugfs.h>
> #include <linux/err.h>
> #include <linux/export.h>
> #include <linux/io.h>
> #include <linux/iopoll.h>
> #include <linux/slab.h>
>+#include <linux/spinlock.h>
> #include <linux/jiffies.h>
> 
> #include "clk.h"
>@@ -40,6 +42,8 @@ struct clk_pll14xx {
> 	enum imx_pll14xx_type		type;
> 	const struct imx_pll14xx_rate_table *rate_table;
> 	int rate_count;
>+	s16 delta_k;
>+	spinlock_t lock;
> };
> 
> #define to_clk_pll14xx(_hw) container_of(_hw, struct clk_pll14xx, hw)
>@@ -361,6 +365,7 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
> {
> 	struct clk_pll14xx *pll = to_clk_pll14xx(hw);
> 	struct imx_pll14xx_rate_table rate;
>+	unsigned long flags;
> 	u32 gnrl_ctl, div_ctl0;
> 	int ret;
> 
>@@ -374,9 +379,13 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
> 		div_ctl0 |= FIELD_PREP(SDIV_MASK, rate.sdiv);
> 		writel_relaxed(div_ctl0, pll->base + DIV_CTL0);
> 
>+		spin_lock_irqsave(&pll->lock, flags);

Should this lock also protect the setting to DIV_CTL0?

>+
> 		writel_relaxed(FIELD_PREP(KDIV_MASK, rate.kdiv),
> 			       pll->base + DIV_CTL1);
> 
>+		spin_unlock_irqrestore(&pll->lock, flags);
>+
> 		return 0;
> 	}
> 
>@@ -394,8 +403,12 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
> 		   FIELD_PREP(SDIV_MASK, rate.sdiv);
> 	writel_relaxed(div_ctl0, pll->base + DIV_CTL0);
> 
>+	spin_lock_irqsave(&pll->lock, flags);

Ditto.

>+
> 	writel_relaxed(FIELD_PREP(KDIV_MASK, rate.kdiv), pll->base + DIV_CTL1);
> 
>+	spin_unlock_irqrestore(&pll->lock, flags);
>+
> 	/*
> 	 * According to SPEC, t3 - t2 need to be greater than
> 	 * 1us and 1/FREF, respectively.
>@@ -508,6 +521,8 @@ struct clk_hw *imx_dev_clk_hw_pll14xx(struct device *dev, const char *name,
> 	if (!pll)
> 		return ERR_PTR(-ENOMEM);
> 
>+	spin_lock_init(&pll->lock);
>+
> 	init.name = name;
> 	init.flags = pll_clk->flags;
> 	init.parent_names = &parent_name;
>@@ -551,3 +566,95 @@ struct clk_hw *imx_dev_clk_hw_pll14xx(struct device *dev, const char *name,
> 	return hw;
> }
> EXPORT_SYMBOL_GPL(imx_dev_clk_hw_pll14xx);
>+
>+/*
>+ * Debugfs interface for Audio PLL runtime monitoring and control
>+ *
>+ * This interface allows dynamic adjustment of the Audio PLL
>+ * K-divider for precise frequency tuning, particularly useful
>+ * for audio applications.
>+ *
>+ * examples for the usage of the two interfaces:
>+ *   1): Get the current PLL setting of dividers
>+ *     cat /sys/kernel/debug/audio_pll_monitor/audio_pll1/pll_parameter
>+ *
>+ *   2): Adjust the K-divider by a small delta_k
>+ *     echo 1 > /sys/kernel/debug/audio_pll_monitor/audio_pll1/delta_k;
>+ */
>+#ifdef CONFIG_DEBUG_FS
>+static int pll_delta_k_get(void *data, u64 *val)
>+{
>+	struct clk_pll14xx *pll = to_clk_pll14xx(data);
>+	*val = pll->delta_k;
>+	return 0;
>+}
>+
>+static int pll_delta_k_set(void *data, u64 val)
>+{
>+	struct clk_pll14xx *pll = to_clk_pll14xx(data);
>+	unsigned long flags;
>+	u32 div_ctl1;
>+	s16 kdiv, delta_k;
>+
>+	delta_k = (s16)clamp_t(long, val, KDIV_MIN, KDIV_MAX);
>+	pll->delta_k = delta_k;
>+
>+	spin_lock_irqsave(&pll->lock, flags);
>+
>+	div_ctl1 = readl_relaxed(pll->base + DIV_CTL1);
>+	kdiv = (s16)FIELD_GET(KDIV_MASK, div_ctl1);
>+	kdiv = (s16)clamp_t(s32, (s32)kdiv + delta_k, KDIV_MIN, KDIV_MAX);
>+	writel_relaxed(FIELD_PREP(KDIV_MASK, kdiv), pll->base + DIV_CTL1);
>+
>+	spin_unlock_irqrestore(&pll->lock, flags);
>+
>+	return 0;
>+}
>+DEFINE_DEBUGFS_ATTRIBUTE_SIGNED(delta_k_fops, pll_delta_k_get, pll_delta_k_set, "%lld\n");
>+
>+static int pll_setting_show(struct seq_file *s, void *data)
>+{
>+	struct clk_pll14xx *pll = to_clk_pll14xx(s->private);
>+	u32 div_ctl0, div_ctl1;
>+	u32 mdiv, pdiv, sdiv, kdiv;
>+
>+	div_ctl0 = readl_relaxed(pll->base + DIV_CTL0);
>+	div_ctl1 = readl_relaxed(pll->base + DIV_CTL1);

sashiko-bot brings a valid concern, you may need to protect the read operation.

[1]https://lore.kernel.org/all/20260519095044.996B7C2BCB3@smtp.kernel.org/


Regards
Peng

>+
>+	mdiv = FIELD_GET(MDIV_MASK, div_ctl0);
>+	pdiv = FIELD_GET(PDIV_MASK, div_ctl0);
>+	sdiv = FIELD_GET(SDIV_MASK, div_ctl0);
>+	kdiv = FIELD_GET(KDIV_MASK, div_ctl1);
>+
>+	seq_printf(s, "Mdiv: 0x%x; Pdiv: 0x%x; Sdiv: 0x%x; Kdiv: 0x%x\n",
>+		   mdiv, pdiv, sdiv, kdiv);
>+
>+	return 0;
>+}
>+DEFINE_SHOW_ATTRIBUTE(pll_setting);
>+
>+void imx_audio_pll_debug_init(struct clk_hw *hws[], unsigned int num_plls)
>+{
>+	struct dentry *rootdir, *audio_pll_dir;
>+	const char *pll_name;
>+	int i;
>+
>+	rootdir = debugfs_create_dir("audio_pll_monitor", NULL);
>+
>+	for (i = 0; i < num_plls; i++) {
>+		if (!IS_ERR_OR_NULL(hws[i])) {
>+			pll_name = clk_hw_get_name(hws[i]);
>+			audio_pll_dir = debugfs_create_dir(pll_name, rootdir);
>+			debugfs_create_file_unsafe("delta_k", 0600, audio_pll_dir,
>+					hws[i], &delta_k_fops);
>+			debugfs_create_file("pll_parameter", 0444, audio_pll_dir,
>+					hws[i], &pll_setting_fops);
>+		}
>+	}
>+}
>+#else /* !CONFIG_DEBUG_FS */
>+void imx_audio_pll_debug_init(struct clk_hw *hws[], unsigned int num_plls)
>+{
>+}
>+#endif /* CONFIG_DEBUG_FS */
>+EXPORT_SYMBOL_GPL(imx_audio_pll_debug_init);
>diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h
>index aa5202f284f3d1b7c1b4bf65e2329831832b43a5..16e0bafa0c9b99cb4eee37a216e0a274d27f11fc 100644
>--- a/drivers/clk/imx/clk.h
>+++ b/drivers/clk/imx/clk.h
>@@ -487,4 +487,5 @@ struct clk_hw *imx_clk_gpr_mux(const char *name, const char *compatible,
> 			       u32 reg, const char **parent_names,
> 			       u8 num_parents, const u32 *mux_table, u32 mask);
> 
>+void imx_audio_pll_debug_init(struct clk_hw **hws, unsigned int num_plls);
> #endif
>
>---
>base-commit: e98d21c170b01ddef366f023bbfcf6b31509fa83
>change-id: 20260421-imx8m_pll_debugfs-246d0fbbb617
>
>Best regards,
>-- 
>Jacky Bai <ping.bai@nxp.com>
>


^ permalink raw reply	[flat|nested] 3+ messages in thread

* RE: [PATCH v2] clk: imx: Add audio PLL debugfs for K-divider control
  2026-05-25  1:44 ` Peng Fan
@ 2026-05-25  7:20   ` Jacky Bai
  0 siblings, 0 replies; 3+ messages in thread
From: Jacky Bai @ 2026-05-25  7:20 UTC (permalink / raw)
  To: Peng Fan (OSS)
  Cc: Abel Vesa, Peng Fan, Michael Turquette, Stephen Boyd,
	Brian Masney, Frank Li, Sascha Hauer, Pengutronix Kernel Team,
	Fabio Estevam, linux-clk@vger.kernel.org, imx@lists.linux.dev,
	linux-arm-kernel@lists.infradead.org

> Subject: Re: [PATCH v2] clk: imx: Add audio PLL debugfs for K-divider control
> 
[..]

> >@@ -374,9 +379,13 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw,
> unsigned long drate,
> > 		div_ctl0 |= FIELD_PREP(SDIV_MASK, rate.sdiv);
> > 		writel_relaxed(div_ctl0, pll->base + DIV_CTL0);
> >
> >+		spin_lock_irqsave(&pll->lock, flags);
> 
> Should this lock also protect the setting to DIV_CTL0?
>

I can add the DIV_CTL0 into the lock scope in next version.

The AI review agent's comment for the spinlock stuff is just let the code looks
like correct and useful, but useless in this audio pll debugfs real use case.

BR

> >+
> > 		writel_relaxed(FIELD_PREP(KDIV_MASK, rate.kdiv),
> > 			       pll->base + DIV_CTL1);
> >
> >+		spin_unlock_irqrestore(&pll->lock, flags);
> >+
> > 		return 0;
> > 	}
> >
> >@@ -394,8 +403,12 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw,
> unsigned long drate,
> > 		   FIELD_PREP(SDIV_MASK, rate.sdiv);
> > 	writel_relaxed(div_ctl0, pll->base + DIV_CTL0);
> >
> >+	spin_lock_irqsave(&pll->lock, flags);
> 
> Ditto.
> 
> >+
> > 	writel_relaxed(FIELD_PREP(KDIV_MASK, rate.kdiv), pll->base +
> > DIV_CTL1);
> >
> >+	spin_unlock_irqrestore(&pll->lock, flags);
> >+
> > 	/*
> > 	 * According to SPEC, t3 - t2 need to be greater than
> > 	 * 1us and 1/FREF, respectively.
> >@@ -508,6 +521,8 @@ struct clk_hw *imx_dev_clk_hw_pll14xx(struct
> device *dev, const char *name,
> > 	if (!pll)
> > 		return ERR_PTR(-ENOMEM);
> >
> >+	spin_lock_init(&pll->lock);
> >+
> > 	init.name = name;
> > 	init.flags = pll_clk->flags;
> > 	init.parent_names = &parent_name;
> >@@ -551,3 +566,95 @@ struct clk_hw *imx_dev_clk_hw_pll14xx(struct
> device *dev, const char *name,
> > 	return hw;
> > }
> > EXPORT_SYMBOL_GPL(imx_dev_clk_hw_pll14xx);
> >+
> >+/*
> >+ * Debugfs interface for Audio PLL runtime monitoring and control
> >+ *
> >+ * This interface allows dynamic adjustment of the Audio PLL
> >+ * K-divider for precise frequency tuning, particularly useful
> >+ * for audio applications.
> >+ *
> >+ * examples for the usage of the two interfaces:
> >+ *   1): Get the current PLL setting of dividers
> >+ *     cat
> /sys/kernel/debug/audio_pll_monitor/audio_pll1/pll_parameter
> >+ *
> >+ *   2): Adjust the K-divider by a small delta_k
> >+ *     echo 1 > /sys/kernel/debug/audio_pll_monitor/audio_pll1/delta_k;
> >+ */
> >+#ifdef CONFIG_DEBUG_FS
> >+static int pll_delta_k_get(void *data, u64 *val) {
> >+	struct clk_pll14xx *pll = to_clk_pll14xx(data);
> >+	*val = pll->delta_k;
> >+	return 0;
> >+}
> >+
> >+static int pll_delta_k_set(void *data, u64 val) {
> >+	struct clk_pll14xx *pll = to_clk_pll14xx(data);
> >+	unsigned long flags;
> >+	u32 div_ctl1;
> >+	s16 kdiv, delta_k;
> >+
> >+	delta_k = (s16)clamp_t(long, val, KDIV_MIN, KDIV_MAX);
> >+	pll->delta_k = delta_k;
> >+
> >+	spin_lock_irqsave(&pll->lock, flags);
> >+
> >+	div_ctl1 = readl_relaxed(pll->base + DIV_CTL1);
> >+	kdiv = (s16)FIELD_GET(KDIV_MASK, div_ctl1);
> >+	kdiv = (s16)clamp_t(s32, (s32)kdiv + delta_k, KDIV_MIN, KDIV_MAX);
> >+	writel_relaxed(FIELD_PREP(KDIV_MASK, kdiv), pll->base + DIV_CTL1);
> >+
> >+	spin_unlock_irqrestore(&pll->lock, flags);
> >+
> >+	return 0;
> >+}
> >+DEFINE_DEBUGFS_ATTRIBUTE_SIGNED(delta_k_fops, pll_delta_k_get,
> >+pll_delta_k_set, "%lld\n");
> >+
> >+static int pll_setting_show(struct seq_file *s, void *data) {
> >+	struct clk_pll14xx *pll = to_clk_pll14xx(s->private);
> >+	u32 div_ctl0, div_ctl1;
> >+	u32 mdiv, pdiv, sdiv, kdiv;
> >+
> >+	div_ctl0 = readl_relaxed(pll->base + DIV_CTL0);
> >+	div_ctl1 = readl_relaxed(pll->base + DIV_CTL1);
> 
> sashiko-bot brings a valid concern, you may need to protect the read
> operation.
> 
> [1]https://lore.kernel.org/all/20260519095044.996B7C2BCB3@smtp.kernel.
> org/
> 
> 
> Regards
> Peng
> 
> >+
> >+	mdiv = FIELD_GET(MDIV_MASK, div_ctl0);
> >+	pdiv = FIELD_GET(PDIV_MASK, div_ctl0);
> >+	sdiv = FIELD_GET(SDIV_MASK, div_ctl0);
> >+	kdiv = FIELD_GET(KDIV_MASK, div_ctl1);
> >+
> >+	seq_printf(s, "Mdiv: 0x%x; Pdiv: 0x%x; Sdiv: 0x%x; Kdiv: 0x%x\n",
> >+		   mdiv, pdiv, sdiv, kdiv);
> >+
> >+	return 0;
> >+}
> >+DEFINE_SHOW_ATTRIBUTE(pll_setting);
> >+
> >+void imx_audio_pll_debug_init(struct clk_hw *hws[], unsigned int
> >+num_plls) {
> >+	struct dentry *rootdir, *audio_pll_dir;
> >+	const char *pll_name;
> >+	int i;
> >+
> >+	rootdir = debugfs_create_dir("audio_pll_monitor", NULL);
> >+
> >+	for (i = 0; i < num_plls; i++) {
> >+		if (!IS_ERR_OR_NULL(hws[i])) {
> >+			pll_name = clk_hw_get_name(hws[i]);
> >+			audio_pll_dir = debugfs_create_dir(pll_name, rootdir);
> >+			debugfs_create_file_unsafe("delta_k", 0600, audio_pll_dir,
> >+					hws[i], &delta_k_fops);
> >+			debugfs_create_file("pll_parameter", 0444, audio_pll_dir,
> >+					hws[i], &pll_setting_fops);
> >+		}
> >+	}
> >+}
> >+#else /* !CONFIG_DEBUG_FS */
> >+void imx_audio_pll_debug_init(struct clk_hw *hws[], unsigned int
> >+num_plls) { } #endif /* CONFIG_DEBUG_FS */
> >+EXPORT_SYMBOL_GPL(imx_audio_pll_debug_init);
> >diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h index
> >aa5202f284f3d1b7c1b4bf65e2329831832b43a5..16e0bafa0c9b99cb4eee37
> a216e0a
> >274d27f11fc 100644
> >--- a/drivers/clk/imx/clk.h
> >+++ b/drivers/clk/imx/clk.h
> >@@ -487,4 +487,5 @@ struct clk_hw *imx_clk_gpr_mux(const char *name,
> const char *compatible,
> > 			       u32 reg, const char **parent_names,
> > 			       u8 num_parents, const u32 *mux_table, u32 mask);
> >
> >+void imx_audio_pll_debug_init(struct clk_hw **hws, unsigned int
> >+num_plls);
> > #endif
> >
> >---
> >base-commit: e98d21c170b01ddef366f023bbfcf6b31509fa83
> >change-id: 20260421-imx8m_pll_debugfs-246d0fbbb617
> >
> >Best regards,
> >--
> >Jacky Bai <ping.bai@nxp.com>
> >


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-05-25  7:20 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-19  9:19 [PATCH v2] clk: imx: Add audio PLL debugfs for K-divider control Jacky Bai
2026-05-25  1:44 ` Peng Fan
2026-05-25  7:20   ` Jacky Bai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox