Re: [PATCH] alsa soc wm8750 fix 2.6.20-rc1-mm1

[Takashi Iwai <tiwai@suse.de>]

At Wed, 20 Dec 2006 21:47:44 -0800,
Andrew Morton wrote:
> 
> On Wed, 20 Dec 2006 20:20:45 +0300
> "Eugene Ilkov" <e.ilkov@gmail.com> wrote:
> 
> >  There was some INIT_WORK related changes, here is patch against
> > wm8750 codec driver. Tested on sharp sl-c1000
> > 
> > 
> > --- linux-2.6.20-rc1-mm1/sound/soc/codecs/wm8750.c	2006-12-20
> > 19:23:27.000000000 +0300
> > +++ linux-2.6.20-rc1-mm.z1/sound/soc/codecs/wm8750.c	2006-12-20
> > 19:27:28.000000000 +0300
> > @@ -52,7 +52,6 @@
> >  	printk(KERN_WARNING AUDIO_NAME ": " format "\n" , ## arg)
> >   static struct workqueue_struct *wm8750_workq = NULL;
> > -static struct work_struct wm8750_dapm_work;
> >   /*
> >   * wm8750 register cache
> > @@ -1001,9 +1000,11 @@
> >  };
> >  EXPORT_SYMBOL_GPL(wm8750_dai);
> >  -static void wm8750_work(void *data)
> > +static void wm8750_work(struct work_struct *work)
> >  {
> > -	struct snd_soc_codec *codec = (struct snd_soc_codec *)data;
> > +	struct snd_soc_device *socdev =
> > +		container_of(work, struct snd_soc_device, delayed_work.work);
> > +	struct snd_soc_codec *codec = socdev->codec;
> >  	wm8750_dapm_event(codec, codec->dapm_state);
> >  }
> >  @@ -1039,7 +1040,7 @@
> >  	if (codec->suspend_dapm_state == SNDRV_CTL_POWER_D0) {
> >  		wm8750_dapm_event(codec, SNDRV_CTL_POWER_D2);
> >  		codec->dapm_state = SNDRV_CTL_POWER_D0;
> > -		queue_delayed_work(wm8750_workq, &wm8750_dapm_work,
> > +		queue_delayed_work(wm8750_workq, &socdev->delayed_work,
> >  			 msecs_to_jiffies(1000));
> >  	}
> >  @@ -1084,7 +1085,7 @@
> >  	/* charge output caps */
> >  	wm8750_dapm_event(codec, SNDRV_CTL_POWER_D2);
> >  	codec->dapm_state = SNDRV_CTL_POWER_D3hot;
> > -	queue_delayed_work(wm8750_workq, &wm8750_dapm_work,
> > +	queue_delayed_work(wm8750_workq, &socdev->delayed_work,
> >  		msecs_to_jiffies(1000));
> >   	/* set the update bits */
> > @@ -1227,7 +1228,7 @@
> >  	INIT_LIST_HEAD(&codec->dapm_widgets);
> >  	INIT_LIST_HEAD(&codec->dapm_paths);
> >  	wm8750_socdev = socdev;
> > -	INIT_WORK(&wm8750_dapm_work, wm8750_work, codec);
> > +	INIT_DELAYED_WORK(&socdev->delayed_work, wm8750_work);
> >  	wm8750_workq = create_workqueue("wm8750");
> >  	if (wm8750_workq == NULL) {
> >  		kfree(codec);
> > 
> 
> I'm really not sure what's going on here.  Your patch appears to be against
> a version of the driver which someone had attempted to fix up.  But the
> version of the driver which I see in today's alsa git tree doesn't have
> even those fixes.
> 
> Shrug.  Oh well, I queued up something which hopefuly will work, but please
> retest next -mm, thanks.

Oh yeah, there are conflicts in ALSA git tree and this change.
The problem is that the ALSA patches haven't been merged to 2.6.20
tree, and the development went parallel.

Anyway, the patch looks wrong, too.  socdev->delayed_work is used for
another thing at closing PCM stream in soc-core.c, thus this
conflicts.  The codec code should have its own work struct.

The below is the patch I applied to ALSA tree.  The alsa-git will be
updated soon with this, too.

I'll try to collect the necessary patches from alsa-git to make these
things consistent, and pass to Jaroslav to push to upstream.


thanks,

Takashi

---
diff -r e276b2632752 include/sound/soc.h
--- a/include/sound/soc.h	Wed Dec 20 19:20:07 2006 +0100
+++ b/include/sound/soc.h	Thu Dec 21 10:44:06 2006 +0100
@@ -374,6 +374,7 @@ struct snd_soc_codec {
 	struct list_head dapm_paths;
 	unsigned int dapm_state;
 	unsigned int suspend_dapm_state;
+	struct delayed_work delayed_work;
 
 	/* codec DAI's */
 	struct snd_soc_codec_dai *dai;
diff -r e276b2632752 soc/codecs/wm8750.c
--- a/sound/soc/codecs/wm8750.c	Wed Dec 20 19:20:07 2006 +0100
+++ b/sound/soc/codecs/wm8750.c	Thu Dec 21 10:45:28 2006 +0100
@@ -50,8 +50,6 @@
 	printk(KERN_INFO AUDIO_NAME ": " format "\n" , ## arg)
 #define warn(format, arg...) \
 	printk(KERN_WARNING AUDIO_NAME ": " format "\n" , ## arg)
-
-static struct work_struct wm8750_dapm_work;
 
 /*
  * wm8750 register cache
@@ -1000,9 +998,10 @@ struct snd_soc_codec_dai wm8750_dai = {
 };
 EXPORT_SYMBOL_GPL(wm8750_dai);
 
-static void wm8750_work(void *data)
-{
-	struct snd_soc_codec *codec = (struct snd_soc_codec *)data;
+static void wm8750_work(struct work_struct *work)
+{
+	struct snd_soc_codec *codec =
+		container_of(work, struct snd_soc_codec, delayed_work.work);
 	wm8750_dapm_event(codec, codec->dapm_state);
 }
 
@@ -1038,7 +1037,7 @@ static int wm8750_resume(struct platform
 	if (codec->suspend_dapm_state == SNDRV_CTL_POWER_D0) {
 		wm8750_dapm_event(codec, SNDRV_CTL_POWER_D2);
 		codec->dapm_state = SNDRV_CTL_POWER_D0;
-		schedule_delayed_work(&wm8750_dapm_work,
+		schedule_delayed_work(&codec->delayed_work,
 			 msecs_to_jiffies(1000));
 	}
 
@@ -1083,7 +1082,7 @@ static int wm8750_init(struct snd_soc_de
 	/* charge output caps */
 	wm8750_dapm_event(codec, SNDRV_CTL_POWER_D2);
 	codec->dapm_state = SNDRV_CTL_POWER_D3hot;
-	schedule_delayed_work(&wm8750_dapm_work, msecs_to_jiffies(1000));
+	schedule_delayed_work(&codec->delayed_work, msecs_to_jiffies(1000));
 
 	/* set the update bits */
 	reg = wm8750_read_reg_cache(codec, WM8750_LDAC);
@@ -1225,7 +1224,7 @@ static int wm8750_probe(struct platform_
 	INIT_LIST_HEAD(&codec->dapm_widgets);
 	INIT_LIST_HEAD(&codec->dapm_paths);
 	wm8750_socdev = socdev;
-	INIT_WORK(&wm8750_dapm_work, wm8750_work, codec);
+	INIT_DELAYED_WORK(&codec->delayed_work, wm8750_work);
 #if defined (CONFIG_I2C) || defined (CONFIG_I2C_MODULE)
 	if (setup->i2c_address) {
 		normal_i2c[0] = setup->i2c_address;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/