Re: [v3 15/19] ASoC: mediatek: mt8186: add machine driver with mt6366, da7219 and max98357

From: Jiaxin Yu
Date: Tue Apr 05 2022 - 00:06:53 EST


On Mon, 2022-03-14 at 11:44 +0100, AngeloGioacchino Del Regno wrote:
> Il 13/03/22 16:10, Jiaxin Yu ha scritto:
> > Add support for mt8186 board with mt6366, da7219 and max98357.
> >
> > Signed-off-by: Jiaxin Yu <jiaxin.yu@xxxxxxxxxxxx>
> > ---
> > .../mt8186/mt8186-mt6366-da7219-max98357.c | 924
> > ++++++++++++++++++
> > 1 file changed, 924 insertions(+)
> > create mode 100644 sound/soc/mediatek/mt8186/mt8186-mt6366-
> > da7219-max98357.c
> >
>
> Hello Jiaxin,
>
> I see some duplication between this one and the mt6366-rt1019-
> rt5682s....
> ....for this reason, I would propose to split out the MT6366 bits
> into a
> common file, something like mt8186-mt6366-common.c, as to reduce the
> duplication.
>

Hello Angelo,

I'm sorry to reply so later about this comment. I've been thinking
about the repetition of these two machine driver recently. The biggest
difference between them are the .init .ops and .be_hw_params_fixup
callback functions of BE's dai_link. So I want break them down into
rt1019-rt5682s.c and da7219-max98357.c, the rest becomes mt8186-
mt6366.c.

SND_SOC_MT8186_MT6366 ==> mt8186-mt6366.c
SND_SOC_RT1019_RT5682S ==> rt1019-rt5682s.c
SND_SOC_DA7219_MAX98357 ==> da7219-max98357.c

Or put these three files in the same mt8186-mt6366.c, then distinguish
by different compatible string.

If it is expected to see MT8186 machines with DA7219 or MAX98357,
> then it'd be a
> good idea to also do something about preventively commonizing these
> ones, like
> it is being done in ... MT8192, if I remember correctly.
>
> Regards,
> Angelo

Yes, I will change this part that being done in MT8192 to simplify the
code. But the part of mt8192 is being reviewed. I'm not sure if you
have any comments about this series.

Link:
https://lore.kernel.org/linux-arm-kernel/20220402051754.17513-1-jiaxin.yu@xxxxxxxxxxxx/T/

Jiaxin.Yu
Thanks.