Re: [PATCH 1/3] arm64: dts: imx93-11x11-evk: add bt-sco sound card support

[Shengjiu Wang]

On Thu, Jul 25, 2024 at 7:32 PM Krzysztof Kozlowski
<krzysztof.kozlowski@xxxxxxxxxx> wrote:
>
> On 25/07/2024 10:59, Shengjiu Wang wrote:
> > Add bt-sco sound card, which is used by BT HFP case.
> > It supports wb profile as default
>
> <form letter>
> Please use scripts/get_maintainers.pl to get a list of necessary people
> and lists to CC (and consider --no-git-fallback argument). It might
> happen, that command when run on an older kernel, gives you outdated
> entries. Therefore please be sure you base your patches on recent Linux
> kernel.
>
> Tools like b4 or scripts/get_maintainer.pl provide you proper list of
> people, so fix your workflow. Tools might also fail if you work on some
> ancient tree (don't, instead use mainline) or work on fork of kernel
> (don't, instead use mainline). Just use b4 and everything should be
> fine, although remember about `b4 prep --auto-to-cc` if you added new
> patches to the patchset.
> </form letter>

Sorry,  I don't get the point. I used the scripts/get_maintainer.pl to get
the list of people.   Anything wrong?

>
> >
> > Signed-off-by: Shengjiu Wang <shengjiu.wang@xxxxxxx>
> > ---
> >  .../boot/dts/freescale/imx93-11x11-evk.dts    | 54 +++++++++++++++++++
> >  1 file changed, 54 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx93-11x11-evk.dts b/arch/arm64/boot/dts/freescale/imx93-11x11-evk.dts
> > index a15987f49e8d..7ed75fb287df 100644
> > --- a/arch/arm64/boot/dts/freescale/imx93-11x11-evk.dts
> > +++ b/arch/arm64/boot/dts/freescale/imx93-11x11-evk.dts
> > @@ -80,6 +80,30 @@ reg_usdhc2_vmmc: regulator-usdhc2 {
> >               off-on-delay-us = <12000>;
> >               enable-active-high;
> >       };
> > +
> > +     bt_sco_codec: bt_sco_codec {
>
> No underscores, generic node names. Please follow DTS coding style.

Ok, will update it.
>
> > +             #sound-dai-cells = <1>;
>
> Order properties and nodes according to DTS coding style.

ok, will update it.

>
> > +             compatible = "linux,bt-sco";
> > +     };
> > +
> > +     sound-bt-sco {
>
> Why this is not "sound"?
>
> How many sound cards do you have there?

We will have 4 sound cards.

>
> > +             compatible = "simple-audio-card";
> > +             simple-audio-card,name = "bt-sco-audio";
> > +             simple-audio-card,format = "dsp_a";
> > +             simple-audio-card,bitclock-inversion;
> > +             simple-audio-card,frame-master = <&btcpu>;
> > +             simple-audio-card,bitclock-master = <&btcpu>;
> > +
> > +             btcpu: simple-audio-card,cpu {
> > +                     sound-dai = <&sai1>;
> > +                     dai-tdm-slot-num = <2>;
> > +                     dai-tdm-slot-width = <16>;
> > +             };
> > +
> > +             simple-audio-card,codec {
> > +                     sound-dai = <&bt_sco_codec 1>;
> > +             };
> > +     };
> >  };
> >
> >  &adc1 {
> > @@ -345,6 +369,18 @@ &mu2 {
> >       status = "okay";
> >  };
> >
> > +&sai1 {
> > +     #sound-dai-cells = <0>;
> > +     pinctrl-names = "default", "sleep";
> > +     pinctrl-0 = <&pinctrl_sai1>;
> > +     pinctrl-1 = <&pinctrl_sai1_sleep>;
> > +     assigned-clocks = <&clk IMX93_CLK_SAI1>;
> > +     assigned-clock-parents = <&clk IMX93_CLK_AUDIO_PLL>;
> > +     assigned-clock-rates = <12288000>;
> > +     fsl,sai-mclk-direction-output;
> > +     status = "okay";
> > +};
> > +
> >  &usbotg1 {
> >       dr_mode = "otg";
> >       hnp-disable;
> > @@ -528,6 +564,24 @@ MX93_PAD_CCM_CLKO2__GPIO3_IO27                   0x31e
> >               >;
> >       };
> >
> > +     pinctrl_sai1: sai1grp {
> > +             fsl,pins = <
> > +                     MX93_PAD_SAI1_TXC__SAI1_TX_BCLK                 0x31e
> > +                     MX93_PAD_SAI1_TXFS__SAI1_TX_SYNC                0x31e
> > +                     MX93_PAD_SAI1_TXD0__SAI1_TX_DATA00              0x31e
> > +                     MX93_PAD_SAI1_RXD0__SAI1_RX_DATA00              0x31e
> > +             >;
> > +     };
> > +
> > +     pinctrl_sai1_sleep: sai1grpsleep {
>
> Does not look like tested. Srsly, NXP, start doing basic tests before
> posting your code.

Did the test. but maybe my test method is not correct,  will double check.

Best regards
Shengjiu wang
>
> It does not look like you tested the DTS against bindings. Please run
> `make dtbs_check W=1` (see
> Documentation/devicetree/bindings/writing-schema.rst or
> https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
> for instructions).
>
>
> Best regards,
> Krzysztof
>