| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 2 Mar 2022 12:07:39 +0800
From: Jiaxin Yu <jiaxin.yu@...iatek.com>
To: Tzung-Bi Shih <tzungbi@...nel.org>
CC: <broonie@...nel.org>, <robh+dt@...nel.org>,
<matthias.bgg@...il.com>, <trevor.wu@...iatek.com>,
<alsa-devel@...a-project.org>, <devicetree@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>,
<linux-mediatek@...ts.infradead.org>,
<linux-kernel@...r.kernel.org>,
<Project_Global_Chrome_Upstream_Group@...iatek.com>,
Jiaxin Yu <jiaxin.yu@...iatek.corp-partner.google.com>
Subject: Re: [PATCH 2/2] ASoC: mediatek: mt8192: support rt1015p_rt5682s
On Tue, 2022-03-01 at 17:03 +0800, Tzung-Bi Shih wrote:
> On Tue, Mar 01, 2022 at 03:29:24PM +0800, Jiaxin Yu wrote:
> > From: Jiaxin Yu <jiaxin.yu@...iatek.corp-partner.google.com>
>
> The environment didn't configure properly so that the header showed
> up.
> See [1].
>
> [1]:
> https://git-scm.com/docs/git-send-email#Documentation/git-send-email.txt---fromltaddressgt
>
Ok, I will take a look at the "from: " header.
> > diff --git a/sound/soc/mediatek/mt8192/mt8192-mt6359-rt1015-
> > rt5682.c b/sound/soc/mediatek/mt8192/mt8192-mt6359-rt1015-rt5682.c
>
> [...]
> > +static struct snd_soc_card mt8192_mt6359_rt1015p_rt5682s_card = {
> > + .name = "mt8192_mt6359_rt1015p_rt5682s",
> > + .owner = THIS_MODULE,
> > + .dai_link = mt8192_mt6359_dai_links,
> > + .num_links = ARRAY_SIZE(mt8192_mt6359_dai_links),
> > + .controls = mt8192_mt6359_rt1015p_rt5682_controls,
> > + .num_controls =
> > ARRAY_SIZE(mt8192_mt6359_rt1015p_rt5682_controls),
> > + .dapm_widgets = mt8192_mt6359_rt1015p_rt5682_widgets,
> > + .num_dapm_widgets =
> > ARRAY_SIZE(mt8192_mt6359_rt1015p_rt5682_widgets),
> > + .dapm_routes = mt8192_mt6359_rt1015p_rt5682_routes,
> > + .num_dapm_routes =
> > ARRAY_SIZE(mt8192_mt6359_rt1015p_rt5682_routes),
> > +};
>
> Are the two cards only different from names
> (mt8192_mt6359_rt1015p_rt5682_card vs.
> mt8192_mt6359_rt1015p_rt5682s_card)?
>
Yes, they are only different form names.
> > @@ -1150,6 +1177,52 @@ static int mt8192_mt6359_dev_probe(struct
> > platform_device *pdev)
> > dai_link->num_platforms =
> > ARRAY_SIZE(i2s3_rt1015p_platfor
> > ms);
> > }
> > + } else if (strcmp(dai_link->name, "I2S8") == 0) {
> > + if (card == &mt8192_mt6359_rt1015_rt5682_card
> > ||
> > + card == &mt8192_mt6359_rt1015p_rt5682_card)
> > {
> > + dai_link->cpus = i2s8_rt5682_cpus;
> > + dai_link->num_cpus =
> > + ARRAY_SIZE(i2s8_rt5682_cpus);
> > + dai_link->codecs = i2s8_rt5682_codecs;
> > + dai_link->num_codecs =
> > + ARRAY_SIZE(i2s8_rt5682_codecs);
> > + dai_link->platforms =
> > i2s8_rt5682_platforms;
> > + dai_link->num_platforms =
> > + ARRAY_SIZE(i2s8_rt5682_platform
> > s);
> > + } else if (card ==
> > &mt8192_mt6359_rt1015p_rt5682s_card) {
> > + dai_link->cpus = i2s8_rt5682s_cpus;
> > + dai_link->num_cpus =
> > + ARRAY_SIZE(i2s8_rt5682s_cpus);
> > + dai_link->codecs = i2s8_rt5682s_codecs;
> > + dai_link->num_codecs =
> > + ARRAY_SIZE(i2s8_rt5682s_codecs)
> > ;
> > + dai_link->platforms =
> > i2s8_rt5682s_platforms;
> > + dai_link->num_platforms =
> > + ARRAY_SIZE(i2s8_rt5682s_platfor
> > ms);
> > + }
> > + } else if (strcmp(dai_link->name, "I2S9") == 0) {
> > + if (card == &mt8192_mt6359_rt1015_rt5682_card
> > ||
> > + card == &mt8192_mt6359_rt1015p_rt5682_card)
> > {
> > + dai_link->cpus = i2s9_rt5682_cpus;
> > + dai_link->num_cpus =
> > + ARRAY_SIZE(i2s9_rt5682_cpus);
> > + dai_link->codecs = i2s9_rt5682_codecs;
> > + dai_link->num_codecs =
> > + ARRAY_SIZE(i2s9_rt5682_codecs);
> > + dai_link->platforms =
> > i2s9_rt5682_platforms;
> > + dai_link->num_platforms =
> > + ARRAY_SIZE(i2s9_rt5682_platform
> > s);
> > + } else if (card ==
> > &mt8192_mt6359_rt1015p_rt5682s_card) {
> > + dai_link->cpus = i2s9_rt5682s_cpus;
> > + dai_link->num_cpus =
> > + ARRAY_SIZE(i2s9_rt5682s_cpus);
> > + dai_link->codecs = i2s9_rt5682s_codecs;
> > + dai_link->num_codecs =
> > + ARRAY_SIZE(i2s9_rt5682s_codecs)
> > ;
> > + dai_link->platforms =
> > i2s9_rt5682s_platforms;
> > + dai_link->num_platforms =
> > + ARRAY_SIZE(i2s9_rt5682s_platfor
> > ms);
> > + }
>
> After seeing the code, I am starting to wonder if the reuse is
> overkill. If
> they (RT5682 vs. RT5682S) only have some minor differences, probably
> it could
> reuse more by:
>
> SND_SOC_DAILINK_DEFS(i2s8, ...
> SND_SOC_DAILINK_DEFS(i2s9, ...
>
> ...
>
> if (card == &mt8192_mt6359_rt1015p_rt5682s_card) {
> i2s8_codecs.name = RT5682S_DEV0_NAME;
> i2s8_codecs.dai_name = RT5682S_CODEC_DAI;
> ...
> }
>
> Or even uses of_device_is_compatible() if it would like to reuse the
> struct
> snd_soc_card.
If we reuse the struct snd_soc_card, the card .name will be same.
Should I change the card .name through of_device_is_compatible()?
Maybe like below:
Remove rt5682x related words for snd_soc_card.
static struct snd_soc_card mt8192_mt6359_rt1015p_card = {
.owner = THIS_MODULE,
.dai_link = mt8192_mt6359_dai_links,
.num_links = ARRAY_SIZE(mt8192_mt6359_dai_links),
.controls = mt8192_mt6359_rt1015p_controls,
.num_controls = ARRAY_SIZE(mt8192_mt6359_rt1015p_controls),
.dapm_widgets = mt8192_mt6359_rt1015p_widgets,
.num_dapm_widgets = ARRAY_SIZE(mt8192_mt6359_rt1015p_widgets),
.dapm_routes = mt8192_mt6359_rt1015p_routes,
.num_dapm_routes = ARRAY_SIZE(mt8192_mt6359_rt1015p_routes),
};
static const struct of_device_id mt8192_mt6359_dt_match[] = {
{
.compatible = "mediatek,mt8192_mt6359_rt1015_rt5682",
.data = &mt8192_mt6359_rt1015_rt5682_card,
},
{
.compatible = "mediatek,mt8192_mt6359_rt1015p_rt5682",
.data = &mt8192_mt6359_rt1015p_card,
},
{
.compatible = "mediatek,mt8192_mt6359_rt1015p_rt5682s",
.data = &mt8192_mt6359_rt1015p_card,
},
{}
};
if (of_device_is_compatible(np, "mediatek,
mt8192_mt6359_rt1015p_rt5682")) {
card.name = "mt8192_mt6359_rt1015p_rt5682";
...
} else if (of_device_is_compatible(np, "mediatek,
mt8192_mt6359_rt1015p_rt5682s")) {
card.name = "mt8192_mt6359_rt1015p_rt5682s";
...
}
Powered by blists - more mailing lists