Re: [PATCH 2/2] ASoC: intel: sof_rt5682: Add quirk for Dooly

[Pierre-Louis Bossart]

On 10/30/20 11:44 AM, Lu, Brent wrote:
> , Brent Lu wrote:
> > > This DMI product family string of this board is "Google_Hatch" so the
> > > DMI quirk will take place. However, this board is using rt1015 speaker
> > > amp instead of max98357a specified in the quirk. Therefore, we need an
> > > new DMI quirk for this board.
> >
> > Do you actually need a DMI quirk for this platform?
> >
> > the .driver_data below uses the exact same settings as what you would use
> > with the generic solution based on ACPI IDs, see below.
> >
> > Wondering if patch1 would be enough?
>
> Dooly has DMI family string " Google_Hatch" so the DMI quirk will overwrite the
> driver_data. I asked google but they prefer not removing this string so it seems to
> me that one extra DMI quirk is needed.

I find this pretty funny. The PRODUCT_FAMILY was added to reduce the 
number of quirks, but of course there's a variant that has nothing to do 
with this 'FAMILY'.

You should add a comment on this, to make sure this information remains 
in the code and we don't lose it during code cleanups.

>                  {
>                                  .callback = sof_rt5682_quirk_cb,
>                                  .matches = {
>                                                  DMI_MATCH(DMI_PRODUCT_FAMILY, "Google_Hatch"),
>                                  },
>                                  .driver_data = (void *)(SOF_RT5682_MCLK_EN |
>                                                                                  SOF_RT5682_MCLK_24MHZ |
>                                                                                  SOF_RT5682_SSP_CODEC(0) |
>                                                                                  SOF_SPEAKER_AMP_PRESENT |
>                                                                                  SOF_RT5682_SSP_AMP(1)),
>                  },
>
> The other way is using acpi_dev_present() in probe to update the quirk with correct
> codec setting. Which one do you think is better?

The DMI quirk you added is probably better for now, I don't know if the 
odds of getting things right with acpi_dev_present() are that high or if 
we are going to get even more variants on top of this variant (e.g. 
tweeter/booster cases...).
If we get too many quirks we'll see later if we can simplify.

So if you don't mind adding a comment on the 'Dooly' quirk in a v3 that 
series is good to go.  Thank you!