From: jiebing chen <jiebing.chen@amlogic.com>
Add the audio tocodec for s4, add the 8 lane support,
add the mclk and sclk enable event when start data enable auto switch
Signed-off-by: jiebing chen <jiebing.chen@amlogic.com>
---
sound/soc/meson/axg-card.c | 3 +-
sound/soc/meson/g12a-toacodec.c | 64 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 66 insertions(+), 1 deletion(-)
diff --git a/sound/soc/meson/axg-card.c b/sound/soc/meson/axg-card.c
index a2dfccb7990f3a53f508fc6724b21de53b4494d8..5cef069c3370257d4aaf24d7270482651babcfe1 100644
--- a/sound/soc/meson/axg-card.c
+++ b/sound/soc/meson/axg-card.c
@@ -303,7 +303,8 @@ static int axg_card_cpu_is_tdm_iface(struct device_node *np)
static int axg_card_cpu_is_codec(struct device_node *np)
{
return of_device_is_compatible(np, DT_PREFIX "g12a-tohdmitx") ||
- of_device_is_compatible(np, DT_PREFIX "g12a-toacodec");
+ of_device_is_compatible(np, DT_PREFIX "g12a-toacodec") ||
+ of_device_is_compatible(np, DT_PREFIX "s4-toacodec");
}
static int axg_card_add_link(struct snd_soc_card *card, struct device_node *np,
diff --git a/sound/soc/meson/g12a-toacodec.c b/sound/soc/meson/g12a-toacodec.c
index 531bb8707a3ec4c47814d6a0676d5c62c705da75..a93a91136e8ea00e856c3981b9c1e7e08d927a3b 100644
--- a/sound/soc/meson/g12a-toacodec.c
+++ b/sound/soc/meson/g12a-toacodec.c
@@ -41,6 +41,9 @@
#define CTRL0_BCLK_SEL_LSB 4
#define CTRL0_MCLK_SEL GENMASK(2, 0)
+#define CTRL0_BCLK_ENABLE_SHIFT 30
+#define CTRL0_MCLK_ENABLE_SHIFT 29
+
#define TOACODEC_OUT_CHMAX 2
struct g12a_toacodec {
@@ -107,6 +110,33 @@ static int g12a_toacodec_mux_put_enum(struct snd_kcontrol *kcontrol,
return 1;
}
+static int tocodec_clk_enable(struct snd_soc_dapm_widget *w,
+ struct snd_kcontrol *control,
+ int event)
+{
+ int ret = 0;
+ unsigned int mask = 0, val = 0;
+ struct snd_soc_component *component = snd_soc_dapm_to_component(w->dapm);
+
+ switch (event) {
+ case SND_SOC_DAPM_PRE_PMU:
+ mask = 1 << CTRL0_MCLK_ENABLE_SHIFT | 1 << CTRL0_BCLK_ENABLE_SHIFT;
+ val = 1 << CTRL0_MCLK_ENABLE_SHIFT | 1 << CTRL0_BCLK_ENABLE_SHIFT;
+ ret = snd_soc_component_update_bits(component, TOACODEC_CTRL0, mask, val);
+ break;
+ case SND_SOC_DAPM_PRE_PMD:
+ mask = 1 << CTRL0_MCLK_ENABLE_SHIFT | 1 << CTRL0_BCLK_ENABLE_SHIFT;
+ val = 0 << CTRL0_MCLK_ENABLE_SHIFT | 0 << CTRL0_BCLK_ENABLE_SHIFT;
+ ret = snd_soc_component_update_bits(component, TOACODEC_CTRL0, mask, val);
+ break;
+ default:
+ dev_err(component->dev, "Unexpected event %d\n", event);
+ return -EINVAL;
+ }
+
+ return ret;
+}
+
static SOC_ENUM_SINGLE_DECL(g12a_toacodec_mux_enum, TOACODEC_CTRL0,
CTRL0_DAT_SEL_LSB,
g12a_toacodec_mux_texts);
@@ -143,6 +173,14 @@ static const struct snd_soc_dapm_widget sm1_toacodec_widgets[] = {
&g12a_toacodec_out_enable),
};
+static const struct snd_soc_dapm_widget s4_toacodec_widgets[] = {
+ SND_SOC_DAPM_MUX("SRC", SND_SOC_NOPM, 0, 0,
+ &sm1_toacodec_mux),
+ SND_SOC_DAPM_SWITCH_E("OUT EN", SND_SOC_NOPM, 0, 0,
+ &g12a_toacodec_out_enable, tocodec_clk_enable,
+ (SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_PRE_PMD)),
+};
+
static int g12a_toacodec_input_hw_params(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *params,
struct snd_soc_dai *dai)
@@ -236,6 +274,10 @@ static const struct snd_kcontrol_new sm1_toacodec_controls[] = {
SOC_SINGLE("Lane Select", TOACODEC_CTRL0, CTRL0_LANE_SEL_SM1, 3, 0),
};
+static const struct snd_kcontrol_new s4_toacodec_controls[] = {
+ SOC_SINGLE("Lane Select", TOACODEC_CTRL0, CTRL0_LANE_SEL_SM1, 7, 0),
+};
+
static const struct snd_soc_component_driver g12a_toacodec_component_drv = {
.probe = g12a_toacodec_component_probe,
.controls = g12a_toacodec_controls,
@@ -258,6 +300,17 @@ static const struct snd_soc_component_driver sm1_toacodec_component_drv = {
.endianness = 1,
};
+static const struct snd_soc_component_driver s4_toacodec_component_drv = {
+ .probe = sm1_toacodec_component_probe,
+ .controls = s4_toacodec_controls,
+ .num_controls = ARRAY_SIZE(s4_toacodec_controls),
+ .dapm_widgets = s4_toacodec_widgets,
+ .num_dapm_widgets = ARRAY_SIZE(s4_toacodec_widgets),
+ .dapm_routes = g12a_toacodec_routes,
+ .num_dapm_routes = ARRAY_SIZE(g12a_toacodec_routes),
+ .endianness = 1,
+};
+
static const struct regmap_config g12a_toacodec_regmap_cfg = {
.reg_bits = 32,
.val_bits = 32,
@@ -278,6 +331,13 @@ static const struct g12a_toacodec_match_data sm1_toacodec_match_data = {
.field_bclk_sel = REG_FIELD(TOACODEC_CTRL0, 4, 6),
};
+static const struct g12a_toacodec_match_data s4_toacodec_match_data = {
+ .component_drv = &s4_toacodec_component_drv,
+ .field_dat_sel = REG_FIELD(TOACODEC_CTRL0, 19, 20),
+ .field_lrclk_sel = REG_FIELD(TOACODEC_CTRL0, 12, 14),
+ .field_bclk_sel = REG_FIELD(TOACODEC_CTRL0, 4, 6),
+};
+
static const struct of_device_id g12a_toacodec_of_match[] = {
{
.compatible = "amlogic,g12a-toacodec",
@@ -287,6 +347,10 @@ static const struct of_device_id g12a_toacodec_of_match[] = {
.compatible = "amlogic,sm1-toacodec",
.data = &sm1_toacodec_match_data,
},
+ {
+ .compatible = "amlogic,s4-toacodec",
+ .data = &s4_toacodec_match_data,
+ },
{}
};
MODULE_DEVICE_TABLE(of, g12a_toacodec_of_match);
--
2.43.0On Fri 14 Feb 2025 at 10:13, jiebing chen via B4 Relay <devnull+jiebing.chen.amlogic.com@kernel.org> wrote:
> From: jiebing chen <jiebing.chen@amlogic.com>
>
> Add the audio tocodec for s4, add the 8 lane support,
> add the mclk and sclk enable event when start data enable auto switch
Again, incomplete description and mixing things together.
>
> Signed-off-by: jiebing chen <jiebing.chen@amlogic.com>
> ---
> sound/soc/meson/axg-card.c | 3 +-
> sound/soc/meson/g12a-toacodec.c | 64 +++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 66 insertions(+), 1 deletion(-)
>
> diff --git a/sound/soc/meson/axg-card.c b/sound/soc/meson/axg-card.c
> index a2dfccb7990f3a53f508fc6724b21de53b4494d8..5cef069c3370257d4aaf24d7270482651babcfe1 100644
> --- a/sound/soc/meson/axg-card.c
> +++ b/sound/soc/meson/axg-card.c
> @@ -303,7 +303,8 @@ static int axg_card_cpu_is_tdm_iface(struct device_node *np)
> static int axg_card_cpu_is_codec(struct device_node *np)
> {
> return of_device_is_compatible(np, DT_PREFIX "g12a-tohdmitx") ||
> - of_device_is_compatible(np, DT_PREFIX "g12a-toacodec");
> + of_device_is_compatible(np, DT_PREFIX "g12a-toacodec") ||
> + of_device_is_compatible(np, DT_PREFIX "s4-toacodec");
There is no need to extend that indefinitely, use fall-back
> }
>
> static int axg_card_add_link(struct snd_soc_card *card, struct device_node *np,
> diff --git a/sound/soc/meson/g12a-toacodec.c b/sound/soc/meson/g12a-toacodec.c
> index 531bb8707a3ec4c47814d6a0676d5c62c705da75..a93a91136e8ea00e856c3981b9c1e7e08d927a3b 100644
> --- a/sound/soc/meson/g12a-toacodec.c
> +++ b/sound/soc/meson/g12a-toacodec.c
> @@ -41,6 +41,9 @@
> #define CTRL0_BCLK_SEL_LSB 4
> #define CTRL0_MCLK_SEL GENMASK(2, 0)
>
> +#define CTRL0_BCLK_ENABLE_SHIFT 30
> +#define CTRL0_MCLK_ENABLE_SHIFT 29
> +
> #define TOACODEC_OUT_CHMAX 2
>
> struct g12a_toacodec {
> @@ -107,6 +110,33 @@ static int g12a_toacodec_mux_put_enum(struct snd_kcontrol *kcontrol,
> return 1;
> }
>
> +static int tocodec_clk_enable(struct snd_soc_dapm_widget *w,
> + struct snd_kcontrol *control,
> + int event)
> +{
> + int ret = 0;
> + unsigned int mask = 0, val = 0;
> + struct snd_soc_component *component = snd_soc_dapm_to_component(w->dapm);
> +
Over complicated for no reason
> + switch (event) {
> + case SND_SOC_DAPM_PRE_PMU:
> + mask = 1 << CTRL0_MCLK_ENABLE_SHIFT | 1 << CTRL0_BCLK_ENABLE_SHIFT;
> + val = 1 << CTRL0_MCLK_ENABLE_SHIFT | 1 << CTRL0_BCLK_ENABLE_SHIFT;
> + ret = snd_soc_component_update_bits(component, TOACODEC_CTRL0, mask, val);
All this could be done in one line and be actually readable if you
properly used the BIT() macro.
> + break;
> + case SND_SOC_DAPM_PRE_PMD:
> + mask = 1 << CTRL0_MCLK_ENABLE_SHIFT | 1 << CTRL0_BCLK_ENABLE_SHIFT;
> + val = 0 << CTRL0_MCLK_ENABLE_SHIFT | 0 << CTRL0_BCLK_ENABLE_SHIFT;
> + ret = snd_soc_component_update_bits(component, TOACODEC_CTRL0, mask, val);
> + break;
> + default:
> + dev_err(component->dev, "Unexpected event %d\n", event);
> + return -EINVAL;
> + }
... and nothing explains what is being done and why ...
> +
> + return ret;
> +}
> +
> static SOC_ENUM_SINGLE_DECL(g12a_toacodec_mux_enum, TOACODEC_CTRL0,
> CTRL0_DAT_SEL_LSB,
> g12a_toacodec_mux_texts);
> @@ -143,6 +173,14 @@ static const struct snd_soc_dapm_widget sm1_toacodec_widgets[] = {
> &g12a_toacodec_out_enable),
> };
>
> +static const struct snd_soc_dapm_widget s4_toacodec_widgets[] = {
> + SND_SOC_DAPM_MUX("SRC", SND_SOC_NOPM, 0, 0,
> + &sm1_toacodec_mux),
> + SND_SOC_DAPM_SWITCH_E("OUT EN", SND_SOC_NOPM, 0, 0,
> + &g12a_toacodec_out_enable, tocodec_clk_enable,
> + (SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_PRE_PMD)),
Drops the .autodisable without a even comment
AFAICT, could be done like the other SoC with SOC_SINGLE_AUTODISABLE()
with properly chosen values.
> +};
> +
> static int g12a_toacodec_input_hw_params(struct snd_pcm_substream *substream,
> struct snd_pcm_hw_params *params,
> struct snd_soc_dai *dai)
> @@ -236,6 +274,10 @@ static const struct snd_kcontrol_new sm1_toacodec_controls[] = {
> SOC_SINGLE("Lane Select", TOACODEC_CTRL0, CTRL0_LANE_SEL_SM1, 3, 0),
> };
>
> +static const struct snd_kcontrol_new s4_toacodec_controls[] = {
> + SOC_SINGLE("Lane Select", TOACODEC_CTRL0, CTRL0_LANE_SEL_SM1, 7, 0),
> +};
> +
No. there is no reason to add that for s4 and not for sm1 which has 8
line HW support too. That clearly shows up with #define used.
If you must do that, please do it correctly without leaving the other
platforms behind.
> static const struct snd_soc_component_driver g12a_toacodec_component_drv = {
> .probe = g12a_toacodec_component_probe,
> .controls = g12a_toacodec_controls,
> @@ -258,6 +300,17 @@ static const struct snd_soc_component_driver sm1_toacodec_component_drv = {
> .endianness = 1,
> };
>
> +static const struct snd_soc_component_driver s4_toacodec_component_drv = {
> + .probe = sm1_toacodec_component_probe,
> + .controls = s4_toacodec_controls,
> + .num_controls = ARRAY_SIZE(s4_toacodec_controls),
> + .dapm_widgets = s4_toacodec_widgets,
> + .num_dapm_widgets = ARRAY_SIZE(s4_toacodec_widgets),
> + .dapm_routes = g12a_toacodec_routes,
> + .num_dapm_routes = ARRAY_SIZE(g12a_toacodec_routes),
> + .endianness = 1,
> +};
> +
> static const struct regmap_config g12a_toacodec_regmap_cfg = {
> .reg_bits = 32,
> .val_bits = 32,
> @@ -278,6 +331,13 @@ static const struct g12a_toacodec_match_data sm1_toacodec_match_data = {
> .field_bclk_sel = REG_FIELD(TOACODEC_CTRL0, 4, 6),
> };
>
> +static const struct g12a_toacodec_match_data s4_toacodec_match_data = {
> + .component_drv = &s4_toacodec_component_drv,
> + .field_dat_sel = REG_FIELD(TOACODEC_CTRL0, 19, 20),
> + .field_lrclk_sel = REG_FIELD(TOACODEC_CTRL0, 12, 14),
> + .field_bclk_sel = REG_FIELD(TOACODEC_CTRL0, 4, 6),
> +};
> +
> static const struct of_device_id g12a_toacodec_of_match[] = {
> {
> .compatible = "amlogic,g12a-toacodec",
> @@ -287,6 +347,10 @@ static const struct of_device_id g12a_toacodec_of_match[] = {
> .compatible = "amlogic,sm1-toacodec",
> .data = &sm1_toacodec_match_data,
> },
> + {
> + .compatible = "amlogic,s4-toacodec",
> + .data = &s4_toacodec_match_data,
> + },
> {}
> };
> MODULE_DEVICE_TABLE(of, g12a_toacodec_of_match);
--
Jerome
在 2025/2/14 17:51, Jerome Brunet 写道:
> [ EXTERNAL EMAIL ]
>
> On Fri 14 Feb 2025 at 10:13, jiebing chen via B4 Relay <devnull+jiebing.chen.amlogic.com@kernel.org> wrote:
>
>> From: jiebing chen <jiebing.chen@amlogic.com>
>>
>> Add the audio tocodec for s4, add the 8 lane support,
>> add the mclk and sclk enable event when start data enable auto switch
> Again, incomplete description and mixing things together.
>
>> Signed-off-by: jiebing chen <jiebing.chen@amlogic.com>
>> ---
>> sound/soc/meson/axg-card.c | 3 +-
>> sound/soc/meson/g12a-toacodec.c | 64 +++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 66 insertions(+), 1 deletion(-)
>>
>> diff --git a/sound/soc/meson/axg-card.c b/sound/soc/meson/axg-card.c
>> index a2dfccb7990f3a53f508fc6724b21de53b4494d8..5cef069c3370257d4aaf24d7270482651babcfe1 100644
>> --- a/sound/soc/meson/axg-card.c
>> +++ b/sound/soc/meson/axg-card.c
>> @@ -303,7 +303,8 @@ static int axg_card_cpu_is_tdm_iface(struct device_node *np)
>> static int axg_card_cpu_is_codec(struct device_node *np)
>> {
>> return of_device_is_compatible(np, DT_PREFIX "g12a-tohdmitx") ||
>> - of_device_is_compatible(np, DT_PREFIX "g12a-toacodec");
>> + of_device_is_compatible(np, DT_PREFIX "g12a-toacodec") ||
>> + of_device_is_compatible(np, DT_PREFIX "s4-toacodec");
> There is no need to extend that indefinitely, use fall-back
>
>> }
>>
>> static int axg_card_add_link(struct snd_soc_card *card, struct device_node *np,
>> diff --git a/sound/soc/meson/g12a-toacodec.c b/sound/soc/meson/g12a-toacodec.c
>> index 531bb8707a3ec4c47814d6a0676d5c62c705da75..a93a91136e8ea00e856c3981b9c1e7e08d927a3b 100644
>> --- a/sound/soc/meson/g12a-toacodec.c
>> +++ b/sound/soc/meson/g12a-toacodec.c
>> @@ -41,6 +41,9 @@
>> #define CTRL0_BCLK_SEL_LSB 4
>> #define CTRL0_MCLK_SEL GENMASK(2, 0)
>>
>> +#define CTRL0_BCLK_ENABLE_SHIFT 30
>> +#define CTRL0_MCLK_ENABLE_SHIFT 29
>> +
>> #define TOACODEC_OUT_CHMAX 2
>>
>> struct g12a_toacodec {
>> @@ -107,6 +110,33 @@ static int g12a_toacodec_mux_put_enum(struct snd_kcontrol *kcontrol,
>> return 1;
>> }
>>
>> +static int tocodec_clk_enable(struct snd_soc_dapm_widget *w,
>> + struct snd_kcontrol *control,
>> + int event)
>> +{
>> + int ret = 0;
>> + unsigned int mask = 0, val = 0;
>> + struct snd_soc_component *component = snd_soc_dapm_to_component(w->dapm);
>> +
> Over complicated for no reason
>
>> + switch (event) {
>> + case SND_SOC_DAPM_PRE_PMU:
>> + mask = 1 << CTRL0_MCLK_ENABLE_SHIFT | 1 << CTRL0_BCLK_ENABLE_SHIFT;
>> + val = 1 << CTRL0_MCLK_ENABLE_SHIFT | 1 << CTRL0_BCLK_ENABLE_SHIFT;
>> + ret = snd_soc_component_update_bits(component, TOACODEC_CTRL0, mask, val);
> All this could be done in one line and be actually readable if you
> properly used the BIT() macro.
>
>> + break;
>> + case SND_SOC_DAPM_PRE_PMD:
>> + mask = 1 << CTRL0_MCLK_ENABLE_SHIFT | 1 << CTRL0_BCLK_ENABLE_SHIFT;
>> + val = 0 << CTRL0_MCLK_ENABLE_SHIFT | 0 << CTRL0_BCLK_ENABLE_SHIFT;
>> + ret = snd_soc_component_update_bits(component, TOACODEC_CTRL0, mask, val);
>> + break;
>> + default:
>> + dev_err(component->dev, "Unexpected event %d\n", event);
>> + return -EINVAL;
>> + }
> ... and nothing explains what is being done and why ...
>
>> +
>> + return ret;
>> +}
>> +
>> static SOC_ENUM_SINGLE_DECL(g12a_toacodec_mux_enum, TOACODEC_CTRL0,
>> CTRL0_DAT_SEL_LSB,
>> g12a_toacodec_mux_texts);
>> @@ -143,6 +173,14 @@ static const struct snd_soc_dapm_widget sm1_toacodec_widgets[] = {
>> &g12a_toacodec_out_enable),
>> };
>>
>> +static const struct snd_soc_dapm_widget s4_toacodec_widgets[] = {
>> + SND_SOC_DAPM_MUX("SRC", SND_SOC_NOPM, 0, 0,
>> + &sm1_toacodec_mux),
>> + SND_SOC_DAPM_SWITCH_E("OUT EN", SND_SOC_NOPM, 0, 0,
>> + &g12a_toacodec_out_enable, tocodec_clk_enable,
>> + (SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_PRE_PMD)),
> Drops the .autodisable without a even comment
>
> AFAICT, could be done like the other SoC with SOC_SINGLE_AUTODISABLE()
> with properly chosen values.
you mean we can add the New switch for auto enable tocodec mclk bit[29]
and tocodec bclk bit[30]
static const struct snd_kcontrol_new s4_toacodec_mclk_enable =
SOC_DAPM_SINGLE_AUTODISABLE("Switch", TOACODEC_CTRL0,
CTRL0_ENABLE_BCLK_SHIFT, 1, 0);
static const struct snd_kcontrol_new s4_toacodec_bclk_enable =
SOC_DAPM_SINGLE_AUTODISABLE("Switch", TOACODEC_CTRL0,
CTRL0_ENABLE_MCLK_SHIFT, 1, 0);
add the route path
static const struct snd_soc_dapm_widget s4_toacodec_widgets[] = {
SND_SOC_DAPM_MUX("SRC", SND_SOC_NOPM, 0, 0,
&sm1_toacodec_mux),
SND_SOC_DAPM_SWITCH("OUT EN", SND_SOC_NOPM, 0, 0,
&g12a_toacodec_out_enable),
SND_SOC_DAPM_SWITCH("MCLK OUT EN", SND_SOC_NOPM, 0, 0,
&s4_toacodec_mclk_enable),
SND_SOC_DAPM_SWITCH("BCLK OUT EN", SND_SOC_NOPM, 0, 0,
&s4_toacodec_mclk_enable),
};
static const struct snd_soc_dapm_route s4_toacodec_routes[] = {
{ "SRC", "I2S A", "IN A Playback" },
{ "SRC", "I2S B", "IN B Playback" },
{ "SRC", "I2S C", "IN C Playback" },
{ "OUT EN", "Switch", "SRC" },
{"MCLK OUT EN", "Switch","OUT EN"},
{"BCLK OUT EN", "Switch","MCLK OUT EN"},
{ "OUT Capture", NULL, "BCLK OUT EN" },
};
add it replace auto switch event, like it ?
>> +};
>> +
>> static int g12a_toacodec_input_hw_params(struct snd_pcm_substream *substream,
>> struct snd_pcm_hw_params *params,
>> struct snd_soc_dai *dai)
>> @@ -236,6 +274,10 @@ static const struct snd_kcontrol_new sm1_toacodec_controls[] = {
>> SOC_SINGLE("Lane Select", TOACODEC_CTRL0, CTRL0_LANE_SEL_SM1, 3, 0),
>> };
>>
>> +static const struct snd_kcontrol_new s4_toacodec_controls[] = {
>> + SOC_SINGLE("Lane Select", TOACODEC_CTRL0, CTRL0_LANE_SEL_SM1, 7, 0),
>> +};
>> +
> No. there is no reason to add that for s4 and not for sm1 which has 8
> line HW support too. That clearly shows up with #define used.
>
> If you must do that, please do it correctly without leaving the other
> platforms behind.
>
>> static const struct snd_soc_component_driver g12a_toacodec_component_drv = {
>> .probe = g12a_toacodec_component_probe,
>> .controls = g12a_toacodec_controls,
>> @@ -258,6 +300,17 @@ static const struct snd_soc_component_driver sm1_toacodec_component_drv = {
>> .endianness = 1,
>> };
>>
>> +static const struct snd_soc_component_driver s4_toacodec_component_drv = {
>> + .probe = sm1_toacodec_component_probe,
>> + .controls = s4_toacodec_controls,
>> + .num_controls = ARRAY_SIZE(s4_toacodec_controls),
>> + .dapm_widgets = s4_toacodec_widgets,
>> + .num_dapm_widgets = ARRAY_SIZE(s4_toacodec_widgets),
>> + .dapm_routes = g12a_toacodec_routes,
>> + .num_dapm_routes = ARRAY_SIZE(g12a_toacodec_routes),
>> + .endianness = 1,
>> +};
>> +
>> static const struct regmap_config g12a_toacodec_regmap_cfg = {
>> .reg_bits = 32,
>> .val_bits = 32,
>> @@ -278,6 +331,13 @@ static const struct g12a_toacodec_match_data sm1_toacodec_match_data = {
>> .field_bclk_sel = REG_FIELD(TOACODEC_CTRL0, 4, 6),
>> };
>>
>> +static const struct g12a_toacodec_match_data s4_toacodec_match_data = {
>> + .component_drv = &s4_toacodec_component_drv,
>> + .field_dat_sel = REG_FIELD(TOACODEC_CTRL0, 19, 20),
>> + .field_lrclk_sel = REG_FIELD(TOACODEC_CTRL0, 12, 14),
>> + .field_bclk_sel = REG_FIELD(TOACODEC_CTRL0, 4, 6),
>> +};
>> +
>> static const struct of_device_id g12a_toacodec_of_match[] = {
>> {
>> .compatible = "amlogic,g12a-toacodec",
>> @@ -287,6 +347,10 @@ static const struct of_device_id g12a_toacodec_of_match[] = {
>> .compatible = "amlogic,sm1-toacodec",
>> .data = &sm1_toacodec_match_data,
>> },
>> + {
>> + .compatible = "amlogic,s4-toacodec",
>> + .data = &s4_toacodec_match_data,
>> + },
>> {}
>> };
>> MODULE_DEVICE_TABLE(of, g12a_toacodec_of_match);
> --
> Jerome
在 2025/2/14 17:51, Jerome Brunet 写道:
> [ EXTERNAL EMAIL ]
>
> On Fri 14 Feb 2025 at 10:13, jiebing chen via B4 Relay <devnull+jiebing.chen.amlogic.com@kernel.org> wrote:
>
>> From: jiebing chen <jiebing.chen@amlogic.com>
>>
>> Add the audio tocodec for s4, add the 8 lane support,
>> add the mclk and sclk enable event when start data enable auto switch
> Again, incomplete description and mixing things together.
>
>> Signed-off-by: jiebing chen <jiebing.chen@amlogic.com>
>> ---
>> sound/soc/meson/axg-card.c | 3 +-
>> sound/soc/meson/g12a-toacodec.c | 64 +++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 66 insertions(+), 1 deletion(-)
>>
>> diff --git a/sound/soc/meson/axg-card.c b/sound/soc/meson/axg-card.c
>> index a2dfccb7990f3a53f508fc6724b21de53b4494d8..5cef069c3370257d4aaf24d7270482651babcfe1 100644
>> --- a/sound/soc/meson/axg-card.c
>> +++ b/sound/soc/meson/axg-card.c
>> @@ -303,7 +303,8 @@ static int axg_card_cpu_is_tdm_iface(struct device_node *np)
>> static int axg_card_cpu_is_codec(struct device_node *np)
>> {
>> return of_device_is_compatible(np, DT_PREFIX "g12a-tohdmitx") ||
>> - of_device_is_compatible(np, DT_PREFIX "g12a-toacodec");
>> + of_device_is_compatible(np, DT_PREFIX "g12a-toacodec") ||
>> + of_device_is_compatible(np, DT_PREFIX "s4-toacodec");
> There is no need to extend that indefinitely, use fall-back
>
>> }
>>
>> static int axg_card_add_link(struct snd_soc_card *card, struct device_node *np,
>> diff --git a/sound/soc/meson/g12a-toacodec.c b/sound/soc/meson/g12a-toacodec.c
>> index 531bb8707a3ec4c47814d6a0676d5c62c705da75..a93a91136e8ea00e856c3981b9c1e7e08d927a3b 100644
>> --- a/sound/soc/meson/g12a-toacodec.c
>> +++ b/sound/soc/meson/g12a-toacodec.c
>> @@ -41,6 +41,9 @@
>> #define CTRL0_BCLK_SEL_LSB 4
>> #define CTRL0_MCLK_SEL GENMASK(2, 0)
>>
>> +#define CTRL0_BCLK_ENABLE_SHIFT 30
>> +#define CTRL0_MCLK_ENABLE_SHIFT 29
>> +
>> #define TOACODEC_OUT_CHMAX 2
>>
>> struct g12a_toacodec {
>> @@ -107,6 +110,33 @@ static int g12a_toacodec_mux_put_enum(struct snd_kcontrol *kcontrol,
>> return 1;
>> }
>>
>> +static int tocodec_clk_enable(struct snd_soc_dapm_widget *w,
>> + struct snd_kcontrol *control,
>> + int event)
>> +{
>> + int ret = 0;
>> + unsigned int mask = 0, val = 0;
>> + struct snd_soc_component *component = snd_soc_dapm_to_component(w->dapm);
>> +
> Over complicated for no reason
>
>> + switch (event) {
>> + case SND_SOC_DAPM_PRE_PMU:
>> + mask = 1 << CTRL0_MCLK_ENABLE_SHIFT | 1 << CTRL0_BCLK_ENABLE_SHIFT;
>> + val = 1 << CTRL0_MCLK_ENABLE_SHIFT | 1 << CTRL0_BCLK_ENABLE_SHIFT;
>> + ret = snd_soc_component_update_bits(component, TOACODEC_CTRL0, mask, val);
> All this could be done in one line and be actually readable if you
> properly used the BIT() macro.
>
>> + break;
>> + case SND_SOC_DAPM_PRE_PMD:
>> + mask = 1 << CTRL0_MCLK_ENABLE_SHIFT | 1 << CTRL0_BCLK_ENABLE_SHIFT;
>> + val = 0 << CTRL0_MCLK_ENABLE_SHIFT | 0 << CTRL0_BCLK_ENABLE_SHIFT;
>> + ret = snd_soc_component_update_bits(component, TOACODEC_CTRL0, mask, val);
>> + break;
>> + default:
>> + dev_err(component->dev, "Unexpected event %d\n", event);
>> + return -EINVAL;
>> + }
> ... and nothing explains what is being done and why ...
>
>> +
>> + return ret;
>> +}
>> +
>> static SOC_ENUM_SINGLE_DECL(g12a_toacodec_mux_enum, TOACODEC_CTRL0,
>> CTRL0_DAT_SEL_LSB,
>> g12a_toacodec_mux_texts);
>> @@ -143,6 +173,14 @@ static const struct snd_soc_dapm_widget sm1_toacodec_widgets[] = {
>> &g12a_toacodec_out_enable),
>> };
>>
>> +static const struct snd_soc_dapm_widget s4_toacodec_widgets[] = {
>> + SND_SOC_DAPM_MUX("SRC", SND_SOC_NOPM, 0, 0,
>> + &sm1_toacodec_mux),
>> + SND_SOC_DAPM_SWITCH_E("OUT EN", SND_SOC_NOPM, 0, 0,
>> + &g12a_toacodec_out_enable, tocodec_clk_enable,
>> + (SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_PRE_PMD)),
> Drops the .autodisable without a even comment
>
> AFAICT, could be done like the other SoC with SOC_SINGLE_AUTODISABLE()
> with properly chosen values.
>
>> +};
>> +
>> static int g12a_toacodec_input_hw_params(struct snd_pcm_substream *substream,
>> struct snd_pcm_hw_params *params,
>> struct snd_soc_dai *dai)
>> @@ -236,6 +274,10 @@ static const struct snd_kcontrol_new sm1_toacodec_controls[] = {
>> SOC_SINGLE("Lane Select", TOACODEC_CTRL0, CTRL0_LANE_SEL_SM1, 3, 0),
>> };
>>
>> +static const struct snd_kcontrol_new s4_toacodec_controls[] = {
>> + SOC_SINGLE("Lane Select", TOACODEC_CTRL0, CTRL0_LANE_SEL_SM1, 7, 0),
>> +};
>> +
> No. there is no reason to add that for s4 and not for sm1 which has 8
> line HW support too. That clearly shows up with #define used.
>
> If you must do that, please do it correctly without leaving the other
> platforms behind.
Hi，jerome
for sm1 tocdec reg, tocodec only support 4 lane
bit [19:16]
0: tdmout_a_dat[0];
1: tdmout_a_dat[1];
2: tdmout_a_dat[2];
3: tdmout_a_dat[3];
4: tdmout_b_dat[0];
5: tdmout_b_dat[1];
6: tdmout_b_dat[2];
7: tdmout_b_dat[3];
8: tdmout_c_dat[0];
9: tdmout_c_dat[1];
10: tdmout_c_dat[2];
11: tdmout_c_dat[3];
for s4 tocodec reg, tocodec support 8 lane
bit[20:16]
0: tdmout_a_dat[0];
1: tdmout_a_dat[1];
2: tdmout_a_dat[2];
3: tdmout_a_dat[3];
4: tdmout_a_dat[4];
5: tdmout_a_dat[5];
6: tdmout_a_dat[6];
7: tdmout_a_dat[7];
8: tdmout_b_dat[0];
9: tdmout_b_dat[1];
10: tdmout_b_dat[2];
11: tdmout_b_dat[3];
12: tdmout_b_dat[4];
13: tdmout_b_dat[5];
14: tdmout_b_dat[6];
15: tdmout_b_dat[7];
16: tdmout_c_dat[0];
17: tdmout_c_dat[1];
18: tdmout_c_dat[2];
19: tdmout_c_dat[3];
20: tdmout_c_dat[4];
21: tdmout_c_dat[5];
22: tdmout_c_dat[6];
23: tdmout_c_dat[7];
reg offset ,and mask is differnet
in addition to this
add bit[29] : enable tocodec mclk , bit[30]: enable tocodec bclk
we want to auto enable bit[30] and enable bit [29] when bit[31] enable
if not add the autodisable event, we can enable it on startup api,
disable on shutdow api ?
>
>> static const struct snd_soc_component_driver g12a_toacodec_component_drv = {
>> .probe = g12a_toacodec_component_probe,
>> .controls = g12a_toacodec_controls,
>> @@ -258,6 +300,17 @@ static const struct snd_soc_component_driver sm1_toacodec_component_drv = {
>> .endianness = 1,
>> };
>>
>> +static const struct snd_soc_component_driver s4_toacodec_component_drv = {
>> + .probe = sm1_toacodec_component_probe,
>> + .controls = s4_toacodec_controls,
>> + .num_controls = ARRAY_SIZE(s4_toacodec_controls),
>> + .dapm_widgets = s4_toacodec_widgets,
>> + .num_dapm_widgets = ARRAY_SIZE(s4_toacodec_widgets),
>> + .dapm_routes = g12a_toacodec_routes,
>> + .num_dapm_routes = ARRAY_SIZE(g12a_toacodec_routes),
>> + .endianness = 1,
>> +};
>> +
>> static const struct regmap_config g12a_toacodec_regmap_cfg = {
>> .reg_bits = 32,
>> .val_bits = 32,
>> @@ -278,6 +331,13 @@ static const struct g12a_toacodec_match_data sm1_toacodec_match_data = {
>> .field_bclk_sel = REG_FIELD(TOACODEC_CTRL0, 4, 6),
>> };
>>
>> +static const struct g12a_toacodec_match_data s4_toacodec_match_data = {
>> + .component_drv = &s4_toacodec_component_drv,
>> + .field_dat_sel = REG_FIELD(TOACODEC_CTRL0, 19, 20),
>> + .field_lrclk_sel = REG_FIELD(TOACODEC_CTRL0, 12, 14),
>> + .field_bclk_sel = REG_FIELD(TOACODEC_CTRL0, 4, 6),
>> +};
>> +
>> static const struct of_device_id g12a_toacodec_of_match[] = {
>> {
>> .compatible = "amlogic,g12a-toacodec",
>> @@ -287,6 +347,10 @@ static const struct of_device_id g12a_toacodec_of_match[] = {
>> .compatible = "amlogic,sm1-toacodec",
>> .data = &sm1_toacodec_match_data,
>> },
>> + {
>> + .compatible = "amlogic,s4-toacodec",
>> + .data = &s4_toacodec_match_data,
>> + },
>> {}
>> };
>> MODULE_DEVICE_TABLE(of, g12a_toacodec_of_match);
> --
> Jerome
© 2016 - 2025 Red Hat, Inc.