On Wed, 2023-05-17 at 13:48 +0200, AngeloGioacchino Del Regno wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>
>
> Il 17/05/23 13:15, Trevor Wu ha scritto:
> > There are two changes included in the patch.
> >
> > First, add set_dailink_daifmt() function, so dai_fmt can be updated
> > by
> > the configuration in dai-link sub node.
> >
> > Second, remove codec phandle from required property in dai-link sub
> > node.
> > For example, user possibly needs to update dai-format for all etdm
> > co-clock dai-links, but codec doesn't need to be specified in
> > capture
> > dai-link for a speaker amp.
> >
> > Signed-off-by: Trevor Wu <trevor.wu@xxxxxxxxxxxx>
> > ---
> > .../mediatek/common/mtk-soundcard-driver.c | 49
> > ++++++++++++++++++-
> > 1 file changed, 48 insertions(+), 1 deletion(-)
> >
> > diff --git a/sound/soc/mediatek/common/mtk-soundcard-driver.c
> > b/sound/soc/mediatek/common/mtk-soundcard-driver.c
> > index 738093451ccb..5e291092046b 100644
> > --- a/sound/soc/mediatek/common/mtk-soundcard-driver.c
> > +++ b/sound/soc/mediatek/common/mtk-soundcard-driver.c
> > @@ -22,7 +22,7 @@ static int set_card_codec_info(struct
> > snd_soc_card *card,
> >
> > codec_node = of_get_child_by_name(sub_node, "codec");
> > if (!codec_node)
> > - return -EINVAL;
> > + return 0;
> >
> > /* set card codec info */
> > ret = snd_soc_of_get_dai_link_codecs(dev, codec_node,
> > dai_link);
> > @@ -36,6 +36,47 @@ static int set_card_codec_info(struct
> > snd_soc_card *card,
> > return 0;
> > }
> >
> > +static int set_dailink_daifmt(struct snd_soc_card *card,
> > + struct device_node *sub_node,
> > + struct snd_soc_dai_link *dai_link)
> > +{
> > + unsigned int daifmt;
> > + const char *str;
> > + int ret;
> > + struct {
> > + char *name;
> > + unsigned int val;
> > + } of_clk_table[] = {
> > + { "cpu", SND_SOC_DAIFMT_CBC_CFC },
> > + { "codec", SND_SOC_DAIFMT_CBP_CFP },
> > + };
> > +
>
> This is an optional property and this function should not do anything
> if that
> property is not found, so....
>
> /*
> * Check "mediatek,clk-provider" to select the clock provider
> * in SND_SOC_DAIFMT_CLOCK_PROVIDER_MASK area, if specified.
> */
> if (!of_property_read_string(sub_node, "mediatek,clk-
> provider", &str))
> return 0;
>
> ...this allows you to reduce indentation as well.
Hi Angelo,
Actually, there are two properties handled by the function.
"dai-format" is parsed at snd_soc_daifmt_parse_format(), and
"mediatek,clk-provider" is parsed by of_property_read_string().
I have to go through both properties in the function, so it can't
return directly if "mediatek,clk-provider" is not found.
>
> > + daifmt = snd_soc_daifmt_parse_format(sub_node, NULL);
> > + if (daifmt) {
> > + dai_link->dai_fmt &=
> > SND_SOC_DAIFMT_CLOCK_PROVIDER_MASK;
> > + dai_link->dai_fmt |= daifmt;
> > + }
> > +
> > + /*
> > + * check "mediatek,clk-provider = xxx"
> > + * SND_SOC_DAIFMT_CLOCK_PROVIDER_MASK area
> > + */
> > + ret = of_property_read_string(sub_node, "mediatek,clk-
> > provider", &str);
> > + if (ret == 0) {
> > + int i;
> > +
> > + for (i = 0; i < ARRAY_SIZE(of_clk_table); i++) {
> > + if (strcmp(str, of_clk_table[i].name) == 0) {
> > + dai_link->dai_fmt &=
> > ~SND_SOC_DAIFMT_CLOCK_PROVIDER_MASK;
> > + dai_link->dai_fmt |=
> > of_clk_table[i].val;
> > + break;
> > + }
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
> > int parse_dai_link_info(struct snd_soc_card *card)
> > {
> > struct device *dev = card->dev;
> > @@ -67,6 +108,12 @@ int parse_dai_link_info(struct snd_soc_card
> > *card)
> > of_node_put(sub_node);
> > return ret;
> > }
> > +
> > + ret = set_dailink_daifmt(card, sub_node, dai_link);
> > + if (ret < 0) {
>
> if (ret) {
> ...
> }
>
Currently, this function only returns 0, but I'm not sure if it will
return a positive value for some other usage in the future.
Because a negative value is always returned for an error and the
preceding rule is also "ret < 0", I prefer "ret < 0" here.
Thanks,
Trevor
> > + of_node_put(sub_node);
> > + return ret;
> > + }
>
> Regards,
> Angelo
>
