All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ASoC: Intel: bytcht_es8316: fix compilation warning and quirk handling
@ 2019-04-10 16:05 Pierre-Louis Bossart
  2019-04-10 16:16 ` Hans de Goede
  0 siblings, 1 reply; 5+ messages in thread
From: Pierre-Louis Bossart @ 2019-04-10 16:05 UTC (permalink / raw)
  To: alsa-devel; +Cc: tiwai, hdegoede, Paul Cercueil, broonie, Pierre-Louis Bossart

Remove warning and align quirk management with other machine
drivers. No need to be creative here.

bytcht_es8316.c:508:11: warning: cast from pointer to integer of
different size [-Wpointer-to-int-cast]

   quirk = (int)dmi_id->driver_data;
           ^
Fixes: a8d218f4fe811 ('ASoC: Intel: bytcht_es8316: Add quirk for the Teclast X98+ II')
Cc: Paul Cercueil <paul@crapouillou.net>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/intel/boards/bytcht_es8316.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/sound/soc/intel/boards/bytcht_es8316.c b/sound/soc/intel/boards/bytcht_es8316.c
index 38975827e276..42e9e1e50e9c 100644
--- a/sound/soc/intel/boards/bytcht_es8316.c
+++ b/sound/soc/intel/boards/bytcht_es8316.c
@@ -61,9 +61,9 @@ enum {
 #define BYT_CHT_ES8316_MONO_SPEAKER		BIT(17)
 #define BYT_CHT_ES8316_JD_INVERTED		BIT(18)
 
-static int quirk;
+static unsigned long quirk;
 
-static int quirk_override = -1;
+static int quirk_override;
 module_param_named(quirk, quirk_override, int, 0444);
 MODULE_PARM_DESC(quirk, "Board-specific quirk override");
 
@@ -505,7 +505,7 @@ static int snd_byt_cht_es8316_mc_probe(struct platform_device *pdev)
 	/* Check for BYTCR or other platform and setup quirks */
 	dmi_id = dmi_first_match(byt_cht_es8316_quirk_table);
 	if (dmi_id) {
-		quirk = (int)dmi_id->driver_data;
+		quirk = (unsigned long)dmi_id->driver_data;
 	} else if (x86_match_cpu(baytrail_cpu_ids) &&
 	    mach->mach_params.acpi_ipc_irq_index == 0) {
 		/* On BYTCR default to SSP0, internal-mic-in2-map, mono-spk */
@@ -516,8 +516,9 @@ static int snd_byt_cht_es8316_mc_probe(struct platform_device *pdev)
 		quirk = BYT_CHT_ES8316_INTMIC_IN1_MAP |
 			BYT_CHT_ES8316_MONO_SPEAKER;
 	}
-	if (quirk_override != -1) {
-		dev_info(dev, "Overriding quirk 0x%x => 0x%x\n", quirk,
+	if (quirk_override) {
+		dev_info(dev, "Overriding quirk 0x%x => 0x%x\n",
+			 (unsigned int)quirk,
 			 quirk_override);
 		quirk = quirk_override;
 	}
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH] ASoC: Intel: bytcht_es8316: fix compilation warning and quirk handling
  2019-04-10 16:05 [PATCH] ASoC: Intel: bytcht_es8316: fix compilation warning and quirk handling Pierre-Louis Bossart
@ 2019-04-10 16:16 ` Hans de Goede
  2019-04-10 17:58   ` Pierre-Louis Bossart
  0 siblings, 1 reply; 5+ messages in thread
From: Hans de Goede @ 2019-04-10 16:16 UTC (permalink / raw)
  To: Pierre-Louis Bossart, alsa-devel; +Cc: tiwai, Paul Cercueil, broonie

Hi,

On 10-04-19 18:05, Pierre-Louis Bossart wrote:
> Remove warning and align quirk management with other machine
> drivers. No need to be creative here.
> 
> bytcht_es8316.c:508:11: warning: cast from pointer to integer of
> different size [-Wpointer-to-int-cast]
> 
>     quirk = (int)dmi_id->driver_data;
>             ^
> Fixes: a8d218f4fe811 ('ASoC: Intel: bytcht_es8316: Add quirk for the Teclast X98+ II')
> Cc: Paul Cercueil <paul@crapouillou.net>
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

There is a good reason to use -1 for quirk-override not set.

The user may want to override the quirk variable to actual 0, on
CHT we default to:

                 /* Others default to internal-mic-in1-map, mono-speaker */
                 quirk = BYT_CHT_ES8316_INTMIC_IN1_MAP |
                         BYT_CHT_ES8316_MONO_SPEAKER;

Which is 0x20000, if the user now justs wants to clear the
BYT_CHT_ES8316_MONO_SPEAKER flag through the override, then the user
must be able to specify 0 as override.

So IMHO the proper fix here would be to just change the:

	(int)dmi_id->driver_data;

to:

	(long)dmi_id->driver_data;

And leave the rest of the code alone.

Regards,

Hans




> ---
>   sound/soc/intel/boards/bytcht_es8316.c | 11 ++++++-----
>   1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/sound/soc/intel/boards/bytcht_es8316.c b/sound/soc/intel/boards/bytcht_es8316.c
> index 38975827e276..42e9e1e50e9c 100644
> --- a/sound/soc/intel/boards/bytcht_es8316.c
> +++ b/sound/soc/intel/boards/bytcht_es8316.c
> @@ -61,9 +61,9 @@ enum {
>   #define BYT_CHT_ES8316_MONO_SPEAKER		BIT(17)
>   #define BYT_CHT_ES8316_JD_INVERTED		BIT(18)
>   
> -static int quirk;
> +static unsigned long quirk;
>   
> -static int quirk_override = -1;
> +static int quirk_override;
>   module_param_named(quirk, quirk_override, int, 0444);
>   MODULE_PARM_DESC(quirk, "Board-specific quirk override");
>   
> @@ -505,7 +505,7 @@ static int snd_byt_cht_es8316_mc_probe(struct platform_device *pdev)
>   	/* Check for BYTCR or other platform and setup quirks */
>   	dmi_id = dmi_first_match(byt_cht_es8316_quirk_table);
>   	if (dmi_id) {
> -		quirk = (int)dmi_id->driver_data;
> +		quirk = (unsigned long)dmi_id->driver_data;
>   	} else if (x86_match_cpu(baytrail_cpu_ids) &&
>   	    mach->mach_params.acpi_ipc_irq_index == 0) {
>   		/* On BYTCR default to SSP0, internal-mic-in2-map, mono-spk */
> @@ -516,8 +516,9 @@ static int snd_byt_cht_es8316_mc_probe(struct platform_device *pdev)
>   		quirk = BYT_CHT_ES8316_INTMIC_IN1_MAP |
>   			BYT_CHT_ES8316_MONO_SPEAKER;
>   	}
> -	if (quirk_override != -1) {
> -		dev_info(dev, "Overriding quirk 0x%x => 0x%x\n", quirk,
> +	if (quirk_override) {
> +		dev_info(dev, "Overriding quirk 0x%x => 0x%x\n",
> +			 (unsigned int)quirk,
>   			 quirk_override);
>   		quirk = quirk_override;
>   	}
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] ASoC: Intel: bytcht_es8316: fix compilation warning and quirk handling
  2019-04-10 16:16 ` Hans de Goede
@ 2019-04-10 17:58   ` Pierre-Louis Bossart
  2019-04-18 11:57     ` Hans de Goede
  0 siblings, 1 reply; 5+ messages in thread
From: Pierre-Louis Bossart @ 2019-04-10 17:58 UTC (permalink / raw)
  To: Hans de Goede, alsa-devel; +Cc: tiwai, Paul Cercueil, broonie


> There is a good reason to use -1 for quirk-override not set.
> 
> The user may want to override the quirk variable to actual 0, on
> CHT we default to:
> 
>                  /* Others default to internal-mic-in1-map, mono-speaker */
>                  quirk = BYT_CHT_ES8316_INTMIC_IN1_MAP |
>                          BYT_CHT_ES8316_MONO_SPEAKER;
> 
> Which is 0x20000, if the user now justs wants to clear the
> BYT_CHT_ES8316_MONO_SPEAKER flag through the override, then the user
> must be able to specify 0 as override.

ok, I accept the argument but then why don't we do the same in other 
machine drivers? I could make the same argument for bytcr_rt5640/51, so 
we should align all machine drivers using this mechanism.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] ASoC: Intel: bytcht_es8316: fix compilation warning and quirk handling
  2019-04-10 17:58   ` Pierre-Louis Bossart
@ 2019-04-18 11:57     ` Hans de Goede
  2019-04-18 12:57       ` Pierre-Louis Bossart
  0 siblings, 1 reply; 5+ messages in thread
From: Hans de Goede @ 2019-04-18 11:57 UTC (permalink / raw)
  To: Pierre-Louis Bossart, alsa-devel; +Cc: tiwai, Paul Cercueil, broonie

Hi,

On 10-04-19 19:58, Pierre-Louis Bossart wrote:
> 
>> There is a good reason to use -1 for quirk-override not set.
>>
>> The user may want to override the quirk variable to actual 0, on
>> CHT we default to:
>>
>>                  /* Others default to internal-mic-in1-map, mono-speaker */
>>                  quirk = BYT_CHT_ES8316_INTMIC_IN1_MAP |
>>                          BYT_CHT_ES8316_MONO_SPEAKER;
>>
>> Which is 0x20000, if the user now justs wants to clear the
>> BYT_CHT_ES8316_MONO_SPEAKER flag through the override, then the user
>> must be able to specify 0 as override.
> 
> ok, I accept the argument but then why don't we do the same in other machine drivers? I could make the same argument for bytcr_rt5640/51, so we should align all machine drivers using this mechanism.

I agree that ideally all machine drivers should be updated to use -1
for "quirk override not set". The drivers which are currently not
using -1 already had their current quirk-override implementation in
place before I started working on them.

Regards,

Hans


_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] ASoC: Intel: bytcht_es8316: fix compilation warning and quirk handling
  2019-04-18 11:57     ` Hans de Goede
@ 2019-04-18 12:57       ` Pierre-Louis Bossart
  0 siblings, 0 replies; 5+ messages in thread
From: Pierre-Louis Bossart @ 2019-04-18 12:57 UTC (permalink / raw)
  To: Hans de Goede, alsa-devel; +Cc: tiwai, Paul Cercueil, broonie

On 4/18/19 6:57 AM, Hans de Goede wrote:
> Hi,
> 
> On 10-04-19 19:58, Pierre-Louis Bossart wrote:
>>
>>> There is a good reason to use -1 for quirk-override not set.
>>>
>>> The user may want to override the quirk variable to actual 0, on
>>> CHT we default to:
>>>
>>>                  /* Others default to internal-mic-in1-map, 
>>> mono-speaker */
>>>                  quirk = BYT_CHT_ES8316_INTMIC_IN1_MAP |
>>>                          BYT_CHT_ES8316_MONO_SPEAKER;
>>>
>>> Which is 0x20000, if the user now justs wants to clear the
>>> BYT_CHT_ES8316_MONO_SPEAKER flag through the override, then the user
>>> must be able to specify 0 as override.
>>
>> ok, I accept the argument but then why don't we do the same in other 
>> machine drivers? I could make the same argument for bytcr_rt5640/51, 
>> so we should align all machine drivers using this mechanism.
> 
> I agree that ideally all machine drivers should be updated to use -1
> for "quirk override not set". The drivers which are currently not
> using -1 already had their current quirk-override implementation in
> place before I started working on them.

Thanks Hans, I'll provide an update shortly.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-04-18 12:57 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-10 16:05 [PATCH] ASoC: Intel: bytcht_es8316: fix compilation warning and quirk handling Pierre-Louis Bossart
2019-04-10 16:16 ` Hans de Goede
2019-04-10 17:58   ` Pierre-Louis Bossart
2019-04-18 11:57     ` Hans de Goede
2019-04-18 12:57       ` Pierre-Louis Bossart

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.