[PATCH v1 0/2] ASoC: sgtl5000: Introduce "mclk-rate" Property for Device Tree Configuration

[PATCH v1 0/2] ASoC: sgtl5000: Introduce "mclk-rate" Property for Device Tree Configuration

[Prasad Kumpatla]

Introduce the "mclk-rate" property, enabling the configuration of the
Master Clock (MCLK) via the device tree. The Codec Slave supports
a defined range of frequencies, and users can request the desired rate
from the Master using `clk_set_rate()`. If the property is not specified,
the default MCLK frequency will be applied.

Prasad Kumpatla (2):
  ASoC: dt-bindings: Add support for master clock frequency
  ASoC: sgtl5000: Introduce "mclk-rate" Property for Device Tree
    Configuration

 .../devicetree/bindings/sound/fsl,sgtl5000.yaml | 17 +++++++++++++----
 sound/soc/codecs/sgtl5000.c                     | 10 ++++++++++
 2 files changed, 23 insertions(+), 4 deletions(-)


base-commit: 5757b31666277e2b177b406e48878dc48d587a46
-- 
2.17.1

[PATCH v1 1/2] ASoC: dt-bindings: Add support for master clock frequency

[Prasad Kumpatla]

Introduce a property specifies the frequency of the master clock
provided to the codec slave. The slave has the capability to adjust
the frequency according to user needs in the defined range.

Also fixed yamllint check errors.

Signed-off-by: Prasad Kumpatla <quic_pkumpatl@quicinc.com>
---
 .../devicetree/bindings/sound/fsl,sgtl5000.yaml | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/fsl,sgtl5000.yaml b/Documentation/devicetree/bindings/sound/fsl,sgtl5000.yaml
index c6ab1ca16763..2512cad10961 100644
--- a/Documentation/devicetree/bindings/sound/fsl,sgtl5000.yaml
+++ b/Documentation/devicetree/bindings/sound/fsl,sgtl5000.yaml
@@ -44,14 +44,14 @@ properties:
       values of 2k, 4k or 8k. If set to 0 it will be off. If this node is not
       mentioned or if the value is unknown, then micbias resistor is set to
       4k.
-    enum: [ 0, 2, 4, 8 ]
+    enum: [0, 2, 4, 8]
 
   micbias-voltage-m-volts:
     description: The bias voltage to be used in mVolts. The voltage can take
       values from 1.25V to 3V by 250mV steps. If this node is not mentioned
       or the value is unknown, then the value is set to 1.25V.
     $ref: /schemas/types.yaml#/definitions/uint32
-    enum: [ 1250, 1500, 1750, 2000, 2250, 2500, 2750, 3000 ]
+    enum: [1250, 1500, 1750, 2000, 2250, 2500, 2750, 3000]
 
   lrclk-strength:
     description: |
@@ -64,7 +64,7 @@ properties:
         2 =		3.33 mA		5.74 mA		8.03  mA
         3 =		4.99 mA		8.61 mA		12.05 mA
     $ref: /schemas/types.yaml#/definitions/uint32
-    enum: [ 0, 1, 2, 3 ]
+    enum: [0, 1, 2, 3]
 
   sclk-strength:
     description: |
@@ -77,12 +77,20 @@ properties:
         2 =		3.33 mA		5.74 mA		8.03  mA
         3 =		4.99 mA		8.61 mA		12.05 mA
     $ref: /schemas/types.yaml#/definitions/uint32
-    enum: [ 0, 1, 2, 3 ]
+    enum: [0, 1, 2, 3]
 
   port:
     $ref: audio-graph-port.yaml#
     unevaluatedProperties: false
 
+  mclk-rate:
+    description: |
+      Master clock rate in Hz. This property specifies the frequency
+      of the master clock provided to the codec slave.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 8000000
+    maximum: 27000000
+
 required:
   - compatible
   - reg
@@ -108,6 +116,7 @@ examples:
             micbias-voltage-m-volts = <2250>;
             VDDA-supply = <&reg_3p3v>;
             VDDIO-supply = <&reg_3p3v>;
+            mclk-rate = <12288000>;
         };
     };
 ...
-- 
2.17.1

Re: [PATCH v1 1/2] ASoC: dt-bindings: Add support for master clock frequency

[Fabio Estevam]

On Mon, Dec 16, 2024 at 8:04 AM Prasad Kumpatla
<quic_pkumpatl@quicinc.com> wrote:
>
> Introduce a property specifies the frequency of the master clock

"a property that specifies"

> -    enum: [ 0, 2, 4, 8 ]
> +    enum: [0, 2, 4, 8]
>
>    micbias-voltage-m-volts:
>      description: The bias voltage to be used in mVolts. The voltage can take
>        values from 1.25V to 3V by 250mV steps. If this node is not mentioned
>        or the value is unknown, then the value is set to 1.25V.
>      $ref: /schemas/types.yaml#/definitions/uint32
> -    enum: [ 1250, 1500, 1750, 2000, 2250, 2500, 2750, 3000 ]
> +    enum: [1250, 1500, 1750, 2000, 2250, 2500, 2750, 3000]
>
>    lrclk-strength:
>      description: |
> @@ -64,7 +64,7 @@ properties:
>          2 =            3.33 mA         5.74 mA         8.03  mA
>          3 =            4.99 mA         8.61 mA         12.05 mA
>      $ref: /schemas/types.yaml#/definitions/uint32
> -    enum: [ 0, 1, 2, 3 ]
> +    enum: [0, 1, 2, 3]
>
>    sclk-strength:
>      description: |
> @@ -77,12 +77,20 @@ properties:
>          2 =            3.33 mA         5.74 mA         8.03  mA
>          3 =            4.99 mA         8.61 mA         12.05 mA
>      $ref: /schemas/types.yaml#/definitions/uint32
> -    enum: [ 0, 1, 2, 3 ]
> +    enum: [0, 1, 2, 3]

All these enum changes are unrelated.

Re: [PATCH v1 1/2] ASoC: dt-bindings: Add support for master clock frequency

[Krzysztof Kozlowski]

On 16/12/2024 12:03, Prasad Kumpatla wrote:
> Introduce a property specifies the frequency of the master clock
> provided to the codec slave. The slave has the capability to adjust
> the frequency according to user needs in the defined range.


You are duplicating existing assigned-clocks properties.

> 
> Also fixed yamllint check errors.

Which ones?

Best regards,
Krzysztof

Re: [PATCH v1 1/2] ASoC: dt-bindings: Add support for master clock frequency

[Rob Herring]

On Mon, Dec 16, 2024 at 04:33:40PM +0530, Prasad Kumpatla wrote:
> Introduce a property specifies the frequency of the master clock
> provided to the codec slave. The slave has the capability to adjust
> the frequency according to user needs in the defined range.
> 
> Also fixed yamllint check errors.

'Also' is the red flag for something that should be a separate commit.

If you see errors with these, you are not using the right config which 
is in Documentation/devicetree/bindings/.yamllint.

[PATCH v1 2/2] ASoC: sgtl5000: Introduce "mclk-rate" Property for Device Tree Configuration

[Prasad Kumpatla]

Introduce the "mclk-rate" property, enabling the configuration of the
Master Clock (MCLK) via the device tree. The Codec Slave supports
a defined range of frequencies, and users can request the desired rate
from the Master, using `clk_set_rate()`. If the property is not specified,
the default MCLK frequency will be applied.

Signed-off-by: Prasad Kumpatla <quic_pkumpatl@quicinc.com>
---
 sound/soc/codecs/sgtl5000.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/sound/soc/codecs/sgtl5000.c b/sound/soc/codecs/sgtl5000.c
index 7aa89e34657e..7e134a3219c7 100644
--- a/sound/soc/codecs/sgtl5000.c
+++ b/sound/soc/codecs/sgtl5000.c
@@ -1615,6 +1615,16 @@ static int sgtl5000_i2c_probe(struct i2c_client *client)
 		goto disable_regs;
 	}
 
+	if (!of_property_read_u32(np, "mclk-rate", &value)) {
+		if (value > 0) {
+			ret = clk_set_rate(sgtl5000->mclk, value);
+			if (ret) {
+				dev_err(&client->dev, "Failed to set mclock\n");
+				goto disable_regs;
+			}
+		}
+	}
+
 	ret = clk_prepare_enable(sgtl5000->mclk);
 	if (ret) {
 		dev_err(&client->dev, "Error enabling clock %d\n", ret);
-- 
2.17.1

Re: [PATCH v1 2/2] ASoC: sgtl5000: Introduce "mclk-rate" Property for Device Tree Configuration

[Krzysztof Kozlowski]

On 16/12/2024 12:03, Prasad Kumpatla wrote:
> Introduce the "mclk-rate" property, enabling the configuration of the
> Master Clock (MCLK) via the device tree. The Codec Slave supports
> a defined range of frequencies, and users can request the desired rate
> from the Master, using `clk_set_rate()`. If the property is not specified,
> the default MCLK frequency will be applied.
> 
> Signed-off-by: Prasad Kumpatla <quic_pkumpatl@quicinc.com>
> ---
>  sound/soc/codecs/sgtl5000.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/sound/soc/codecs/sgtl5000.c b/sound/soc/codecs/sgtl5000.c
> index 7aa89e34657e..7e134a3219c7 100644
> --- a/sound/soc/codecs/sgtl5000.c
> +++ b/sound/soc/codecs/sgtl5000.c
> @@ -1615,6 +1615,16 @@ static int sgtl5000_i2c_probe(struct i2c_client *client)
>  		goto disable_regs;
>  	}
>  
> +	if (!of_property_read_u32(np, "mclk-rate", &value)) {

No, NAK, you just reimplemented legacy clock-frequency property
duplicating existing assigned-clocks properties.

Best regards,
Krzysztof