[PATCH 1/3] ASoC: dt-bindings: irondevice,sma1303: Rework binding and add missing properties

[PATCH 1/3] ASoC: dt-bindings: irondevice,sma1303: Rework binding and add missing properties

[Kiseok Jo]

Fix according to the writing-schema.rst file and tested.
Add the missing property as compatible, reg, sys-clk-id.

Signed-off-by: Kiseok Jo <kiseok.jo@irondevice.com>
---
 .../bindings/sound/irondevice,sma1303.yaml    | 35 +++++++++++++++++--
 1 file changed, 32 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/irondevice,sma1303.yaml b/Documentation/devicetree/bindings/sound/irondevice,sma1303.yaml
index 162c52606635..2c6158277aee 100644
--- a/Documentation/devicetree/bindings/sound/irondevice,sma1303.yaml
+++ b/Documentation/devicetree/bindings/sound/irondevice,sma1303.yaml
@@ -10,22 +10,51 @@ maintainers:
   - Kiseok Jo <kiseok.jo@irondevice.com>
 
 description:
-  SMA1303 digital class-D audio amplifier with an integrated boost converter.
+  SMA1303 digital class-D audio amplifier
+  with an integrated boost converter.
 
 allOf:
-  - $ref: name-prefix.yaml#
+  - $ref: dai-common.yaml#
+
+properties:
+  compatible:
+    enum:
+      - irondevice,sma1303
+
+  reg:
+    maxItems: 1
+
+  '#sound-dai-cells':
+    const: 1
+
+  sys-clk-id:
+    description:
+      select the using system clock.
+      0 = External Clock 19.2MHz
+      1 = External Clock 24.576MHz
+      2 = Using PLL in MCLK
+      3 = Using PLL in BCLK
+    $ref: "/schemas/types.yaml#/definitions/uint32"
+    maximum: 3
+    default: 3
 
 required:
   - compatible
   - reg
+  - '#sound-dai-cells'
 
 additionalProperties: false
 
 examples:
   - |
-    i2c_bus {
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
         amplifier@1e {
             compatible = "irondevice,sma1303";
             reg = <0x1e>;
+            #sound-dai-cells = <1>;
+            sys-clk-id = <3>;
         };
     };

base-commit: afdf57285d1e16a1bbdbf6cc33fb22fb6c8e7860
-- 
2.20.1

[PATCH 2/3] ASoC: SMA1303: Remove the I2C Retry property in devicetree

[Kiseok Jo]

It's necessary to set the value for each device, so remove that.

Signed-off-by: Kiseok Jo <kiseok.jo@irondevice.com>
---
 sound/soc/codecs/sma1303.c | 16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/sound/soc/codecs/sma1303.c b/sound/soc/codecs/sma1303.c
index fbedba574ff4..fdcc40f5fc61 100644
--- a/sound/soc/codecs/sma1303.c
+++ b/sound/soc/codecs/sma1303.c
@@ -1621,21 +1621,6 @@ static int sma1303_i2c_probe(struct i2c_client *client)
 	}
 
 	if (np) {
-		if (!of_property_read_u32(np, "i2c-retry", &value)) {
-			if (value > 50 || value <= 0) {
-				sma1303->retry_cnt = SMA1303_I2C_RETRY_COUNT;
-				dev_dbg(&client->dev, "%s : %s\n", __func__,
-					"i2c-retry out of range (up to 50)");
-			} else {
-				sma1303->retry_cnt = value;
-				dev_dbg(&client->dev, "%s : %s = %u\n",
-					__func__, "i2c-retry count", value);
-			}
-		} else {
-			dev_dbg(&client->dev, "%s : %s = %d\n", __func__,
-				"i2c-retry count", SMA1303_I2C_RETRY_COUNT);
-			sma1303->retry_cnt = SMA1303_I2C_RETRY_COUNT;
-		}
 		if (!of_property_read_u32(np, "tdm-slot-rx", &value)) {
 			dev_dbg(&client->dev,
 				"tdm slot rx is '%d' from DT\n", value);
@@ -1733,6 +1718,7 @@ static int sma1303_i2c_probe(struct i2c_client *client)
 	sma1303->last_ocp_val = 0x08;
 	sma1303->last_over_temp = 0xC0;
 	sma1303->tsdw_cnt = 0;
+	sma1303->retry_cnt = SMA1303_I2C_RETRY_COUNT;
 
 	sma1303->dev = &client->dev;
 	sma1303->kobj = &client->dev.kobj;
-- 
2.20.1

[PATCH 3/3] ASoC: SMA1303: Convert the TDM slot properties in devicetree to mixer

[Kiseok Jo]

It seems correct that the user changes the TDM slot as needed after
device probe.

Signed-off-by: Kiseok Jo <kiseok.jo@irondevice.com>
---
 sound/soc/codecs/sma1303.c | 100 ++++++++++++++++++++++++++++++-------
 1 file changed, 82 insertions(+), 18 deletions(-)

diff --git a/sound/soc/codecs/sma1303.c b/sound/soc/codecs/sma1303.c
index fdcc40f5fc61..9ae4e3cba3ae 100644
--- a/sound/soc/codecs/sma1303.c
+++ b/sound/soc/codecs/sma1303.c
@@ -292,6 +292,9 @@ static const char * const sma1303_aif_in_source_text[] = {
 static const char * const sma1303_aif_out_source_text[] = {
 	"Disable", "After_FmtC", "After_Mixer", "After_DSP", "After_Post",
 		"Clk_PLL", "Clk_OSC"};
+static const char * const sma1303_tdm_slot_text[] = {
+	"Slot0", "Slot1", "Slot2", "Slot3",
+	"Slot4", "Slot5", "Slot6", "Slot7"};
 
 static const struct soc_enum sma1303_aif_in_source_enum =
 	SOC_ENUM_SINGLE_EXT(ARRAY_SIZE(sma1303_aif_in_source_text),
@@ -299,6 +302,9 @@ static const struct soc_enum sma1303_aif_in_source_enum =
 static const struct soc_enum sma1303_aif_out_source_enum =
 	SOC_ENUM_SINGLE_EXT(ARRAY_SIZE(sma1303_aif_out_source_text),
 			sma1303_aif_out_source_text);
+static const struct soc_enum sma1303_tdm_slot_enum =
+	SOC_ENUM_SINGLE_EXT(ARRAY_SIZE(sma1303_tdm_slot_text),
+			sma1303_tdm_slot_text);
 
 static int sma1303_force_mute_get(struct snd_kcontrol *kcontrol,
 				struct snd_ctl_elem_value *ucontrol)
@@ -368,6 +374,76 @@ static int sma1303_postscaler_put(struct snd_kcontrol *kcontrol,
 	return change;
 }
 
+static int sma1303_tdm_slot_rx_get(struct snd_kcontrol *kcontrol,
+				struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *component =
+		snd_soc_kcontrol_component(kcontrol);
+	struct sma1303_priv *sma1303 = snd_soc_component_get_drvdata(component);
+	int val, ret;
+
+	ret = sma1303_regmap_read(sma1303, SMA1303_A5_TDM1, &val);
+	if (ret < 0)
+		return -EINVAL;
+
+	ucontrol->value.integer.value[0] = (val & 0x38) >> 3;
+	sma1303->tdm_slot_rx = ucontrol->value.integer.value[0];
+
+	return 0;
+}
+
+static int sma1303_tdm_slot_rx_put(struct snd_kcontrol *kcontrol,
+				struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *component =
+		snd_soc_kcontrol_component(kcontrol);
+	struct sma1303_priv *sma1303 = snd_soc_component_get_drvdata(component);
+	int ret, val = (int)ucontrol->value.integer.value[0];
+	bool change;
+
+	ret = sma1303_regmap_update_bits(sma1303,
+			SMA1303_A5_TDM1, 0x38, (val << 3), &change);
+	if (ret < 0)
+		return -EINVAL;
+
+	return change;
+}
+
+static int sma1303_tdm_slot_tx_get(struct snd_kcontrol *kcontrol,
+				struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *component =
+		snd_soc_kcontrol_component(kcontrol);
+	struct sma1303_priv *sma1303 = snd_soc_component_get_drvdata(component);
+	int val, ret;
+
+	ret = sma1303_regmap_read(sma1303, SMA1303_A6_TDM2, &val);
+	if (ret < 0)
+		return -EINVAL;
+
+	ucontrol->value.integer.value[0] = (val & 0x38) >> 3;
+	sma1303->tdm_slot_tx = ucontrol->value.integer.value[0];
+
+	return 0;
+}
+
+static int sma1303_tdm_slot_tx_put(struct snd_kcontrol *kcontrol,
+				struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *component =
+		snd_soc_kcontrol_component(kcontrol);
+	struct sma1303_priv *sma1303 = snd_soc_component_get_drvdata(component);
+	int ret, val = (int)ucontrol->value.integer.value[0];
+	bool change;
+
+	ret = sma1303_regmap_update_bits(sma1303,
+			SMA1303_A6_TDM2, 0x38, (val << 3), &change);
+	if (ret < 0)
+		return -EINVAL;
+
+	return change;
+}
+
 static int sma1303_startup(struct snd_soc_component *component)
 {
 	struct sma1303_priv *sma1303 = snd_soc_component_get_drvdata(component);
@@ -781,6 +857,10 @@ static const struct snd_kcontrol_new sma1303_snd_controls[] = {
 		sma1303_force_mute_get, sma1303_force_mute_put),
 	SOC_SINGLE_EXT("Postscaler Gain", SMA1303_90_POSTSCALER, 1, 0x30, 0,
 		sma1303_postscaler_get, sma1303_postscaler_put),
+	SOC_ENUM_EXT("TDM RX Slot Position", sma1303_tdm_slot_enum,
+			sma1303_tdm_slot_rx_get, sma1303_tdm_slot_rx_put),
+	SOC_ENUM_EXT("TDM TX Slot Position", sma1303_tdm_slot_enum,
+			sma1303_tdm_slot_tx_get, sma1303_tdm_slot_tx_put),
 };
 
 static const struct snd_soc_dapm_widget sma1303_dapm_widgets[] = {
@@ -1621,24 +1701,6 @@ static int sma1303_i2c_probe(struct i2c_client *client)
 	}
 
 	if (np) {
-		if (!of_property_read_u32(np, "tdm-slot-rx", &value)) {
-			dev_dbg(&client->dev,
-				"tdm slot rx is '%d' from DT\n", value);
-			sma1303->tdm_slot_rx = value;
-		} else {
-			dev_dbg(&client->dev,
-				"Default setting of tdm slot rx is '0'\n");
-			sma1303->tdm_slot_rx = 0;
-		}
-		if (!of_property_read_u32(np, "tdm-slot-tx", &value)) {
-			dev_dbg(&client->dev,
-				"tdm slot tx is '%u' from DT\n", value);
-			sma1303->tdm_slot_tx = value;
-		} else {
-			dev_dbg(&client->dev,
-				"Default setting of tdm slot tx is '0'\n");
-			sma1303->tdm_slot_tx = 0;
-		}
 		if (!of_property_read_u32(np, "sys-clk-id", &value)) {
 			switch (value) {
 			case SMA1303_EXTERNAL_CLOCK_19_2:
@@ -1719,6 +1781,8 @@ static int sma1303_i2c_probe(struct i2c_client *client)
 	sma1303->last_over_temp = 0xC0;
 	sma1303->tsdw_cnt = 0;
 	sma1303->retry_cnt = SMA1303_I2C_RETRY_COUNT;
+	sma1303->tdm_slot_rx = 0;
+	sma1303->tdm_slot_tx = 0;
 
 	sma1303->dev = &client->dev;
 	sma1303->kobj = &client->dev.kobj;
-- 
2.20.1

Re: [PATCH 1/3] ASoC: dt-bindings: irondevice,sma1303: Rework binding and add missing properties

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 715 bytes --]

On Fri, Feb 03, 2023 at 09:42:37AM +0000, Kiseok Jo wrote:

> +  sys-clk-id:
> +    description:
> +      select the using system clock.
> +      0 = External Clock 19.2MHz
> +      1 = External Clock 24.576MHz
> +      2 = Using PLL in MCLK
> +      3 = Using PLL in BCLK
> +    $ref: "/schemas/types.yaml#/definitions/uint32"
> +    maximum: 3
> +    default: 3

We really shouldn't have this as a custom property, we should be using
a combination of the clock bindings.

In the driver this would turn into using clk_get() to get the MCLK, then
we can query the rate with clk_get_rate() and use option 0 or 1 if the
rate matches, or the PLL otherwise.  If we fail to get MCLK then we can
fall back to using BCLK.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/3] ASoC: dt-bindings: irondevice,sma1303: Rework binding and add missing properties

[Rob Herring]

On Fri, Feb 03, 2023 at 09:42:37AM +0000, Kiseok Jo wrote:
> Fix according to the writing-schema.rst file and tested.
> Add the missing property as compatible, reg, sys-clk-id.
> 
> Signed-off-by: Kiseok Jo <kiseok.jo@irondevice.com>
> ---
>  .../bindings/sound/irondevice,sma1303.yaml    | 35 +++++++++++++++++--
>  1 file changed, 32 insertions(+), 3 deletions(-)

What's the output of 'make dt_binding_check'? Asking since linux-next 
has been broken in different ways with this schema.

Rob

Re: [PATCH 1/3] ASoC: dt-bindings: irondevice,sma1303: Rework binding and add missing properties

[Ki-Seok Jo]

> > Fix according to the writing-schema.rst file and tested.
> > Add the missing property as compatible, reg, sys-clk-id.
> >
> > Signed-off-by: Kiseok Jo <kiseok.jo@irondevice.com>
> > ---
> >  .../bindings/sound/irondevice,sma1303.yaml    | 35 +++++++++++++++++--
> >  1 file changed, 32 insertions(+), 3 deletions(-)
> 
> What's the output of 'make dt_binding_check'? Asking since linux-next has
> been broken in different ways with this schema.
> 
> Rob

2 logs and 2 files output below:
  DTEX  irondevice,sma1303.example.dts
  DTC_CHK  irondevice,sma1303.example.dtb

And I didn't see any other error, so I thought it was okay.
Is there anything else I need to check or how? I'll try it.

Thank you!

Best Regard,
Kiseok Jo

Re: [PATCH 1/3] ASoC: dt-bindings: irondevice,sma1303: Rework binding and add missing properties

[Ki-Seok Jo]

[-- Attachment #1: Type: text/plain, Size: 845 bytes --]

> 
> > +  sys-clk-id:
> > +    description:
> > +      select the using system clock.
> > +      0 = External Clock 19.2MHz
> > +      1 = External Clock 24.576MHz
> > +      2 = Using PLL in MCLK
> > +      3 = Using PLL in BCLK
> > +    $ref: "/schemas/types.yaml#/definitions/uint32"
> > +    maximum: 3
> > +    default: 3
> 
> We really shouldn't have this as a custom property, we should be using a
> combination of the clock bindings.
> 
> In the driver this would turn into using clk_get() to get the MCLK, then
> we can query the rate with clk_get_rate() and use option 0 or 1 if the
> rate matches, or the PLL otherwise.  If we fail to get MCLK then we can
> fall back to using BCLK.

Okay, I got it.
I'll make it based on what you sent me using other drivers.
Thanks, your feedback.

Best regards,
Kiseok Jo

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 499 bytes --]