diff mbox

ASoC: Intel: add bytcr-wm5102 machine driver

Message ID CAJcYhpbxWicEAOheSw9mdHibxQ2RPVQfZ4ncDvtQ5YEwtY=xWg@mail.gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Paulo Sergio June 5, 2017, 2:25 p.m. UTC
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


>
> 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)

Comments

Pierre-Louis Bossart June 5, 2017, 4:37 p.m. UTC | #1
On 06/05/2017 09:25 AM, Paulo Sergio wrote:
> 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?

Yes, however it's the first time I see a quirk based on 
DMI_CHASSIS_VERSION. we use PRODUCT or BOARD usually.

>
> 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)
Paulo Sergio June 5, 2017, 5:01 p.m. UTC | #2
>>> 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?
>
>
> Yes, however it's the first time I see a quirk based on DMI_CHASSIS_VERSION.
> we use PRODUCT or BOARD usually.

Not a problem. I can change it. I have these info available from dmi:

bios_date: 08/01/2014
bios_vendor: LENOVO
bios_version: 01WT17WW
board_asset_tag: NO Asset Tag
board_name: INVALID
board_serial: HA07YER9
board_vendor: LENOVO
board_version: SDK9A6N2HVWIN
chassis_asset_tag: NO Asset Tag
chassis_serial: HA07YER9
chassis_type: 11
chassis_vendor: LENOVO
chassis_version: 1051F
modalias: dmi:bvnLENOVO:bvr01WT17WW:bd08/01/2014:svnLENOVO:pn60073:pvr1051F:rvnLENOVO:rnINVALID:rvrSDK9A6N2HVWIN:cvnLENOVO:ct11:cvr1051F:
product_name: 60073
product_serial: HA07YER9
product_uuid: 39FFE49E-1918-884A-9499-CC7705A289C9
product_version: 1051F
sys_vendor: LENOVO
uevent: MODALIAS=dmi:bvnLENOVO:bvr01WT17WW:bd08/01/2014:svnLENOVO:pn60073:pvr1051F:rvnLENOVO:rnINVALID:rvrSDK9A6N2HVWIN:cvnLENOVO:ct11:cvr1051F:
diff mbox

Patch

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)) {
...