[alsa-devel] [PATCH] ASoC: Intel: bytcht_es8316: fix compilation warning and quirk handling
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; }
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;@@ -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);
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.
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
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.
participants (2)
-
Hans de Goede -
Pierre-Louis Bossart