Re: [PATCH v2 4/5] ASoC: meson: s4:support audio tocodec

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]




在 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@xxxxxxxxxx> wrote:

From: jiebing chen <jiebing.chen@xxxxxxxxxxx>

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@xxxxxxxxxxx>
---
  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




[Index of Archives]     [Pulseaudio]     [Linux Audio Users]     [ALSA Devel]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]

  Powered by Linux