[PATCH v2] ASoC: bytcr_rt5640: Allow quirk set via module option

[PATCH v2] ASoC: bytcr_rt5640: Allow quirk set via module option

[Takashi Iwai]

The bytcr-rt5640 driver has a few quirk setups depending on the board,
where the quirk value is set by DMI matching.  When you have a new
device to add the support, you often experience to try the different
quirk by trial-and-error.  Or, you may have a development model that
still has no proper DMI string.  In either case, you'd need to compile
the driver at each time.

This patch introduces a module option to override the quirk value on
the fly.  User can boot like snd-soc-sst-bytcr-rt5640.quirk=0x4004 to
override the default value without recompilation.  It's a raw value,
so user needs to check the source code for the meaning of each bit.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
v1->v2: override the quirk value completely

 sound/soc/intel/boards/bytcr_rt5640.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c
index 5c7219fb3aa8..23f5ffdb3731 100644
--- a/sound/soc/intel/boards/bytcr_rt5640.c
+++ b/sound/soc/intel/boards/bytcr_rt5640.c
@@ -19,6 +19,7 @@
 
 #include <linux/init.h>
 #include <linux/module.h>
+#include <linux/moduleparam.h>
 #include <linux/platform_device.h>
 #include <linux/acpi.h>
 #include <linux/device.h>
@@ -58,6 +59,9 @@ struct byt_rt5640_private {
 };
 
 static unsigned long byt_rt5640_quirk = BYT_RT5640_MCLK_EN;
+static unsigned int quirk_override;
+module_param_named(quirk, quirk_override, int, 0444);
+MODULE_PARM_DESC(quirk, "Board-specific quirk override");
 
 static void log_quirks(struct device *dev)
 {
@@ -806,6 +810,8 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev)
 
 	/* check quirks before creating card */
 	dmi_check_system(byt_rt5640_quirk_table);
+	if (quirk_override)
+		byt_rt5640_quirk = quirk_override;
 	log_quirks(&pdev->dev);
 
 	if ((byt_rt5640_quirk & BYT_RT5640_SSP2_AIF2) ||
-- 
2.11.1

Re: [PATCH v2] ASoC: bytcr_rt5640: Allow quirk set via module option

[Pierre-Louis Bossart]

On 04/21/2017 03:38 PM, Takashi Iwai wrote:
> The bytcr-rt5640 driver has a few quirk setups depending on the board,
> where the quirk value is set by DMI matching.  When you have a new
> device to add the support, you often experience to try the different
> quirk by trial-and-error.  Or, you may have a development model that
> still has no proper DMI string.  In either case, you'd need to compile
> the driver at each time.
>
> This patch introduces a module option to override the quirk value on
> the fly.  User can boot like snd-soc-sst-bytcr-rt5640.quirk=0x4004 to
> override the default value without recompilation.  It's a raw value,
> so user needs to check the source code for the meaning of each bit.
>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
> v1->v2: override the quirk value completely
>
>   sound/soc/intel/boards/bytcr_rt5640.c | 6 ++++++
>   1 file changed, 6 insertions(+)
>
> diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c
> index 5c7219fb3aa8..23f5ffdb3731 100644
> --- a/sound/soc/intel/boards/bytcr_rt5640.c
> +++ b/sound/soc/intel/boards/bytcr_rt5640.c
> @@ -19,6 +19,7 @@
>   
>   #include <linux/init.h>
>   #include <linux/module.h>
> +#include <linux/moduleparam.h>
>   #include <linux/platform_device.h>
>   #include <linux/acpi.h>
>   #include <linux/device.h>
> @@ -58,6 +59,9 @@ struct byt_rt5640_private {
>   };
>   
>   static unsigned long byt_rt5640_quirk = BYT_RT5640_MCLK_EN;
> +static unsigned int quirk_override;
> +module_param_named(quirk, quirk_override, int, 0444);
> +MODULE_PARM_DESC(quirk, "Board-specific quirk override");
>   
>   static void log_quirks(struct device *dev)
>   {
> @@ -806,6 +810,8 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev)
>   
>   	/* check quirks before creating card */
>   	dmi_check_system(byt_rt5640_quirk_table);
> +	if (quirk_override)
> +		byt_rt5640_quirk = quirk_override;
>   	log_quirks(&pdev->dev);
sounds fine.
I would also change this log_quirks function to signal if the bits set 
by the user aren't aligned with the driver known quirks or are inconsistent.
I can add a patch if you want.

>   
>   	if ((byt_rt5640_quirk & BYT_RT5640_SSP2_AIF2) ||

Re: [PATCH v2] ASoC: bytcr_rt5640: Allow quirk set via module option

[Takashi Iwai]

On Fri, 21 Apr 2017 22:45:29 +0200,
Pierre-Louis Bossart wrote:
> 
> 
> 
> On 04/21/2017 03:38 PM, Takashi Iwai wrote:
> > The bytcr-rt5640 driver has a few quirk setups depending on the board,
> > where the quirk value is set by DMI matching.  When you have a new
> > device to add the support, you often experience to try the different
> > quirk by trial-and-error.  Or, you may have a development model that
> > still has no proper DMI string.  In either case, you'd need to compile
> > the driver at each time.
> >
> > This patch introduces a module option to override the quirk value on
> > the fly.  User can boot like snd-soc-sst-bytcr-rt5640.quirk=0x4004 to
> > override the default value without recompilation.  It's a raw value,
> > so user needs to check the source code for the meaning of each bit.
> >
> > Signed-off-by: Takashi Iwai <tiwai@suse.de>
> > ---
> > v1->v2: override the quirk value completely
> >
> >   sound/soc/intel/boards/bytcr_rt5640.c | 6 ++++++
> >   1 file changed, 6 insertions(+)
> >
> > diff --git a/sound/soc/intel/boards/bytcr_rt5640.c b/sound/soc/intel/boards/bytcr_rt5640.c
> > index 5c7219fb3aa8..23f5ffdb3731 100644
> > --- a/sound/soc/intel/boards/bytcr_rt5640.c
> > +++ b/sound/soc/intel/boards/bytcr_rt5640.c
> > @@ -19,6 +19,7 @@
> >     #include <linux/init.h>
> >   #include <linux/module.h>
> > +#include <linux/moduleparam.h>
> >   #include <linux/platform_device.h>
> >   #include <linux/acpi.h>
> >   #include <linux/device.h>
> > @@ -58,6 +59,9 @@ struct byt_rt5640_private {
> >   };
> >     static unsigned long byt_rt5640_quirk = BYT_RT5640_MCLK_EN;
> > +static unsigned int quirk_override;
> > +module_param_named(quirk, quirk_override, int, 0444);
> > +MODULE_PARM_DESC(quirk, "Board-specific quirk override");
> >     static void log_quirks(struct device *dev)
> >   {
> > @@ -806,6 +810,8 @@ static int snd_byt_rt5640_mc_probe(struct platform_device *pdev)
> >     	/* check quirks before creating card */
> >   	dmi_check_system(byt_rt5640_quirk_table);
> > +	if (quirk_override)
> > +		byt_rt5640_quirk = quirk_override;
> >   	log_quirks(&pdev->dev);
> sounds fine.
> I would also change this log_quirks function to signal if the bits set
> by the user aren't aligned with the driver known quirks or are
> inconsistent.

Well, we can just add a dev_info() around "if (quirk_override)"
block.

I'm going to respin the v3 patch tomorrow with that change.
Could you give your ack or review tag I can add to v3?


thanks,

Takashi