Re: [PATCH v11 2/3] ASoC: qcom: dt-bindings: Add sc7180 machine bindings

Sorry for resending this mail.
I forgot to use plain text mode in the previous mail.
On Thu, Oct 22, 2020 at 6:12 PM Srinivas Kandagatla
[off-list ref] wrote:

On 22/10/2020 04:29, Tzung-Bi Shih wrote:

quoted

Hi, sorry for jumping into your discussion but I am trying to
summarize them to make sure we are on the same page.  Pardon me to
manually copy-and-paste partial sentences to quote.

ACK:
- Don't expose DAI connections in compatible strings.
- Use "model" DT property to make the card more UCM2-friendly.
- Expose new DT properties to distinguish different DMIC models.

NACK:
- All the board variations using exactly the same compatible string.
=> This is less realistic.  Although the CODECS information can be
retrieved from DT, it is inevitable to have some custom code for each
CODEC.

Per Mark's words:

quoted

a different CODEC is something that often justifies a separate compatible

I think we should use different compatible strings for new CODECS
combinations.  And we should try to reuse the machine driver if they
share the most code.  In the worst case, introduce a new machine
driver for the new CODECS combinations.

- Srinivas's suggestion to set driver_name.
e.g. card->driver_name = "SM8250";
=> This sounds like a new DT property should be parsed in
sound/soc/qcom/common.c.  For example: "qcom,family"?  But as we do
less care about UCM2 for now, I would prefer to just leave it as is.

No, you can just hardcode this driver_name in your machine driver rather
than getting it from DT, this is how everyone does!.
So need of adding anything to common.c

ACK

The thing that I suggested to add to common.c is setting card->long_name
from "model" property.

NACK
I found that I don't need to set card->long_name in common.c because
soc-core.c already sets longname using card->name if
card->long_name is NULL.

        soc_setup_card_name(card->snd_card->longname,
                            card->long_name, card->name, 0);

So we can leave common.c as it is and still get long name.

quoted

I would expect the following variants in DTS (just for example):

sound {
   compatible = "qcom,sc7180-trogdor";

Make sure that vendor name is correct here, am not sure if trogdor is
qcom board or Google own board!

ACK
I should use "google,sc7180-trogdor" because google is the vendor.

quoted

   model = "sc7180-rt5682-max98357a-1mic";
}

sound {
   compatible = "qcom,sc7180-trogdor";
   model = "sc7180-rt5682-max98357a-2mic";
   dmic-gpio = ...
}

sound {
   compatible = "qcom,sc7180-pompom";
   model = "sc7180-adau7002-max98357a";
}


Please correct me if there is any misunderstanding.

Looks good to me!
thanks for doing this!

Thank you. I will collect the discussion result to send a v12, and
sync with variant board partners to submit following machine driver
changes.
We will make sure future projects follow this approach

--srini

quoted

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel