Re: [PATCH] ASoC: Intel: add bytcr-wm5102 machine driver

[Paulo Sergio <pstglia@gmail.com>]

Hi Pierre,how are you?

>> +static int is_byt_cr(struct device *dev, struct sst_acpi_mach *mach, bool
>> *bytcr)
>>   {
>>         int status = 0;
>>   +     /* Lenovo Yoga2 exception - acpi_ipc_irq_index is the 1st index,
>> +          but iosf_mbi_read (bios_status) returns 0 for bits 26:27 */
>> +       if (!strcmp(mach->drv_name, "bytcr_wm5102")) {
>> +               *bytcr = true;
>> +               return status;
>> +       }
>
>
> well no, this isn't the right way to do this. quirks need to be based on
> real hardware information (e.g. DMI) and not a driver name.

Thanks for the review. Could I use dmi_match function inside is_byt_cr?

git diff HEAD sound/soc/intel/atom/sst/sst_acpi.c
diff --git a/sound/soc/intel/atom/sst/sst_acpi.c
b/sound/soc/intel/atom/sst/sst_acpi.c
index dd250b8..a99d5a5 100644
--- a/sound/soc/intel/atom/sst/sst_acpi.c
+++ b/sound/soc/intel/atom/sst/sst_acpi.c
@@ -244,6 +244,13 @@ static int is_byt_cr(struct device *dev, bool *bytcr)
 {
        int status = 0;

+       /* Lenovo Yoga2 exception - acpi_ipc_irq_index is the 1st index,
+          but iosf_mbi_read (bios_status) returns 0 for bits 26:27 */
+       if (dmi_match(DMI_SYS_VENDOR, "LENOVO") &&
dmi_match(DMI_CHASSIS_VERSION, "1051F")) {
+               *bytcr = true;
+               return status;
+       }
+
        if (IS_ENABLED(CONFIG_IOSF_MBI)) {
...


>
> The rest looks more of less ok, it's not clear to me if all the quirks are
> needed and make sense.

Inside machine driver code, I can confirm these as necessary:

- SSP0 usage
- 25Mhz Clock

About the other quirks: To be honest, as I started using bycr_rt5640
as base, I avoided touching other parts of the code.

I'll understand if this is not adequated to be pushed upstream.
Besides, the codec side changes (arizona/wm5102 - not sent with this
patch)
are not ready to be pushed due some hardcoding & an issue with
regulator part (arizona-code will only detected codec chip if
arizona-ldo1 is marked as built-in; as a module it is not working)

Regards
Paulo Sérgio Travaglia (Pstglia)
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel