[PATCH v2 0/2] Add master clock handling for nau8824

[PATCH v2 0/2] Add master clock handling for nau8824

[Maxim Kochetkov]

nau8824 has external MCLK pin. So add enable/disable external clock
management.

Signed-off-by: Maxim Kochetkov <fido_max@inbox.ru>
---
Changes in v2:
- move the devm_clk_get() to the nau8824_read_device()

---
Maxim Kochetkov (2):
  ASoC: codecs: nau8824: Add master clock handling
  ASoC: dt-bindings: nau8824: Add master clock handling

 .../bindings/sound/nuvoton,nau8824.yaml       |  8 +++++++
 sound/soc/codecs/nau8824.c                    | 21 +++++++++++++++++--
 sound/soc/codecs/nau8824.h                    |  1 +
 3 files changed, 28 insertions(+), 2 deletions(-)

-- 
2.45.2

[PATCH v2 1/2] ASoC: codecs: nau8824: Add master clock handling

[Maxim Kochetkov]

Use master clock "mclk" if provided through device tree.

Signed-off-by: Maxim Kochetkov <fido_max@inbox.ru>
---
 sound/soc/codecs/nau8824.c | 21 +++++++++++++++++++--
 sound/soc/codecs/nau8824.h |  1 +
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/nau8824.c b/sound/soc/codecs/nau8824.c
index f92b95b21cae..7153b746f4b1 100644
--- a/sound/soc/codecs/nau8824.c
+++ b/sound/soc/codecs/nau8824.c
@@ -520,8 +520,15 @@ static int system_clock_control(struct snd_soc_dapm_widget *w,
 		} else {
 			nau8824_config_sysclk(nau8824, NAU8824_CLK_DIS, 0);
 		}
+
+		if (!IS_ERR(nau8824->mclk))
+			clk_disable_unprepare(nau8824->mclk);
 	} else {
 		dev_dbg(nau8824->dev, "system clock control : POWER ON\n");
+
+		if (!IS_ERR(nau8824->mclk))
+			clk_prepare_enable(nau8824->mclk);
+
 		/* Check the clock source setting is proper or not
 		 * no matter the source is from FLL or MCLK.
 		 */
@@ -563,16 +570,22 @@ static int dmic_clock_control(struct snd_soc_dapm_widget *w,
 	struct snd_soc_component *component = snd_soc_dapm_to_component(w->dapm);
 	struct nau8824 *nau8824 = snd_soc_component_get_drvdata(component);
 	int src;
+	unsigned int freq;
+
+	if (!IS_ERR(nau8824->mclk))
+		freq = clk_get_rate(nau8824->mclk);
+	else
+		freq = nau8824->fs * 256;
 
 	/* The DMIC clock is gotten from system clock (256fs) divided by
 	 * DMIC_SRC (1, 2, 4, 8, 16, 32). The clock has to be equal or
 	 * less than 3.072 MHz.
 	 */
 	for (src = 0; src < 5; src++) {
-		if ((0x1 << (8 - src)) * nau8824->fs <= DMIC_CLK)
+		if (freq / (0x1 << src) <= DMIC_CLK)
 			break;
 	}
-	dev_dbg(nau8824->dev, "dmic src %d for mclk %d\n", src, nau8824->fs * 256);
+	dev_dbg(nau8824->dev, "dmic src %d for mclk %d\n", src, freq);
 	regmap_update_bits(nau8824->regmap, NAU8824_REG_CLK_DIVIDER,
 		NAU8824_CLK_DMIC_SRC_MASK, (src << NAU8824_CLK_DMIC_SRC_SFT));
 
@@ -1871,6 +1884,10 @@ static int nau8824_read_device_properties(struct device *dev,
 	if (ret)
 		nau8824->jack_eject_debounce = 1;
 
+	nau8824->mclk = devm_clk_get(dev, "mclk");
+	if (PTR_ERR(nau8824->mclk) == -EPROBE_DEFER)
+		return -EPROBE_DEFER;
+
 	return 0;
 }
 
diff --git a/sound/soc/codecs/nau8824.h b/sound/soc/codecs/nau8824.h
index 5fcfc43dfc85..d8e19515133c 100644
--- a/sound/soc/codecs/nau8824.h
+++ b/sound/soc/codecs/nau8824.h
@@ -434,6 +434,7 @@ struct nau8824 {
 	struct snd_soc_jack *jack;
 	struct work_struct jdet_work;
 	struct semaphore jd_sem;
+	struct clk *mclk;
 	int fs;
 	int irq;
 	int resume_lock;
-- 
2.45.2

Re: [PATCH v2 1/2] ASoC: codecs: nau8824: Add master clock handling

[Krzysztof Kozlowski]

On 28/06/2024 08:17, Maxim Kochetkov wrote:
> Use master clock "mclk" if provided through device tree.
> 
> Signed-off-by: Maxim Kochetkov <fido_max@inbox.ru>
> ---
>  sound/soc/codecs/nau8824.c | 21 +++++++++++++++++++--
>  sound/soc/codecs/nau8824.h |  1 +
>  2 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/soc/codecs/nau8824.c b/sound/soc/codecs/nau8824.c
> index f92b95b21cae..7153b746f4b1 100644
> --- a/sound/soc/codecs/nau8824.c
> +++ b/sound/soc/codecs/nau8824.c
> @@ -520,8 +520,15 @@ static int system_clock_control(struct snd_soc_dapm_widget *w,
>  		} else {
>  			nau8824_config_sysclk(nau8824, NAU8824_CLK_DIS, 0);
>  		}
> +
> +		if (!IS_ERR(nau8824->mclk))
> +			clk_disable_unprepare(nau8824->mclk);
>  	} else {
>  		dev_dbg(nau8824->dev, "system clock control : POWER ON\n");
> +
> +		if (!IS_ERR(nau8824->mclk))

Nah, clock API is not used like this. Open clk_prepare_enable() and look
how it handles NULL ptr.

> +			clk_prepare_enable(nau8824->mclk);
> +
>  		/* Check the clock source setting is proper or not
>  		 * no matter the source is from FLL or MCLK.
>  		 */
> @@ -563,16 +570,22 @@ static int dmic_clock_control(struct snd_soc_dapm_widget *w,
>  	struct snd_soc_component *component = snd_soc_dapm_to_component(w->dapm);
>  	struct nau8824 *nau8824 = snd_soc_component_get_drvdata(component);
>  	int src;
> +	unsigned int freq;
> +
> +	if (!IS_ERR(nau8824->mclk))
> +		freq = clk_get_rate(nau8824->mclk);
> +	else
> +		freq = nau8824->fs * 256;
>  
>  	/* The DMIC clock is gotten from system clock (256fs) divided by
>  	 * DMIC_SRC (1, 2, 4, 8, 16, 32). The clock has to be equal or
>  	 * less than 3.072 MHz.
>  	 */
>  	for (src = 0; src < 5; src++) {
> -		if ((0x1 << (8 - src)) * nau8824->fs <= DMIC_CLK)
> +		if (freq / (0x1 << src) <= DMIC_CLK)
>  			break;
>  	}
> -	dev_dbg(nau8824->dev, "dmic src %d for mclk %d\n", src, nau8824->fs * 256);
> +	dev_dbg(nau8824->dev, "dmic src %d for mclk %d\n", src, freq);
>  	regmap_update_bits(nau8824->regmap, NAU8824_REG_CLK_DIVIDER,
>  		NAU8824_CLK_DMIC_SRC_MASK, (src << NAU8824_CLK_DMIC_SRC_SFT));
>  
> @@ -1871,6 +1884,10 @@ static int nau8824_read_device_properties(struct device *dev,
>  	if (ret)
>  		nau8824->jack_eject_debounce = 1;
>  
> +	nau8824->mclk = devm_clk_get(dev, "mclk");
> +	if (PTR_ERR(nau8824->mclk) == -EPROBE_DEFER)
> +		return -EPROBE_DEFER;

Aren't you open-coding getting optional clock?

Best regards,
Krzysztof

[PATCH v2 2/2] ASoC: dt-bindings: nau8824: Add master clock handling

[Maxim Kochetkov]

Use master clock "mclk" if provided through device tree.

Signed-off-by: Maxim Kochetkov <fido_max@inbox.ru>
---
 .../devicetree/bindings/sound/nuvoton,nau8824.yaml        | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/nuvoton,nau8824.yaml b/Documentation/devicetree/bindings/sound/nuvoton,nau8824.yaml
index 3dbf438c3841..232dc16a94a3 100644
--- a/Documentation/devicetree/bindings/sound/nuvoton,nau8824.yaml
+++ b/Documentation/devicetree/bindings/sound/nuvoton,nau8824.yaml
@@ -23,6 +23,14 @@ properties:
   '#sound-dai-cells':
     const: 0
 
+  clocks:
+    items:
+      - description: The phandle of the master clock to the CODEC
+
+  clock-names:
+    items:
+      - const: mclk
+
   interrupts:
     maxItems: 1
 
-- 
2.45.2

Re: [PATCH v2 2/2] ASoC: dt-bindings: nau8824: Add master clock handling

[Krzysztof Kozlowski]

On 28/06/2024 08:17, Maxim Kochetkov wrote:
> Use master clock "mclk" if provided through device tree.
> 
> Signed-off-by: Maxim Kochetkov <fido_max@inbox.ru>
> ---
>  .../devicetree/bindings/sound/nuvoton,nau8824.yaml        | 8 ++++++++
>  1 file changed, 8 insertions(+)

Bindings are before their users. Order the patches correctly.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


---

<form letter>
This is an automated instruction, just in case, because many review tags
are being ignored. If you know the process, you can skip it (please do
not feel offended by me posting it here - no bad intentions intended).
If you do not know the process, here is a short explanation:

Please add Acked-by/Reviewed-by/Tested-by tags when posting new
versions, under or above your Signed-off-by tag. Tag is "received", when
provided in a message replied to you on the mailing list. Tools like b4
can help here. However, there's no need to repost patches *only* to add
the tags. The upstream maintainer will do that for tags received on the
version they apply.

https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst#L577
</form letter>

Best regards,
Krzysztof