[PATCH] AC97 bus interface for ad-hoc drivers

[PATCH] AC97 bus interface for ad-hoc drivers

[Liam Girdwood]

[-- Attachment #1: Type: text/plain, Size: 446 bytes --]

This patch adds support for ad-hoc AC97 device drivers (e.g WM97xx and
UCB touchscreen drivers) and was originally posted to the list by
Nicolas Pitre as an RFC. 

Changes from RFC version :-

  o Now matches codec name within codec group.
  o Added ac97_dev_release() to stop kernel complaining about no release
method for device.

Liam

Signed-off-by: Liam Girdwood <liam.girdwood@wolfsonmicro.com>
Signed-off-by: Nicolas Pitre <nico@cam.org>


[-- Attachment #2: ac97-bus.diff --]
[-- Type: text/x-patch, Size: 4501 bytes --]

diff --git a/include/sound/ac97_codec.h b/include/sound/ac97_codec.h
--- a/include/sound/ac97_codec.h
+++ b/include/sound/ac97_codec.h
@@ -26,6 +26,7 @@
  */
 
 #include <linux/bitops.h>
+#include <linux/device.h>
 #include "pcm.h"
 #include "control.h"
 #include "info.h"
@@ -520,6 +521,7 @@ struct _snd_ac97 {
 	/* jack-sharing info */
 	unsigned char indep_surround;
 	unsigned char channel_mode;
+	struct device dev;
 };
 
 /* conditions */
@@ -599,4 +601,8 @@ struct ac97_enum {
 	unsigned short mask;
 	const char **texts;
 };
+
+/* ad hoc AC97 device driver access */
+extern struct bus_type ac97_bus_type;
+
 #endif /* __SOUND_AC97_CODEC_H */
diff --git a/sound/pci/ac97/Makefile b/sound/pci/ac97/Makefile
--- a/sound/pci/ac97/Makefile
+++ b/sound/pci/ac97/Makefile
@@ -12,7 +12,7 @@ endif
 snd-ak4531-codec-objs := ak4531_codec.o
 
 # Toplevel Module Dependency
-obj-$(CONFIG_SND_AC97_CODEC) += snd-ac97-codec.o
+obj-$(CONFIG_SND_AC97_CODEC) += snd-ac97-codec.o ac97_bus.o
 obj-$(CONFIG_SND_ENS1370) += snd-ak4531-codec.o
 
 obj-m := $(sort $(obj-m))
diff --git a/sound/pci/ac97/ac97_bus.c b/sound/pci/ac97/ac97_bus.c
new file mode 100644
--- /dev/null
+++ b/sound/pci/ac97/ac97_bus.c
@@ -0,0 +1,83 @@
+/*
+ * Linux driver model AC97 bus interface
+ *
+ * Author:	Nicolas Pitre
+ * Created:	Jan 14, 2005
+ * Copyright:	(C) MontaVista Software Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/device.h>
+#include <linux/string.h>
+
+#include <sound/driver.h>
+#include <sound/core.h>
+#include <sound/ac97_codec.h>
+
+/*
+ * Codec families have names seperated by commas, so we search for an
+ * individual codec name within the family string. 
+ */
+static int ac97_bus_match(struct device *dev, struct device_driver *drv)
+{
+	return (strstr(dev->bus_id, drv->name) != NULL);
+}
+
+static int ac97_bus_suspend(struct device *dev, pm_message_t state)
+{
+	int ret = 0;
+
+	if (dev->driver && dev->driver->suspend) {
+		ret = dev->driver->suspend(dev, state, SUSPEND_DISABLE);
+		if (ret == 0)
+			ret = dev->driver->suspend(dev, state, SUSPEND_SAVE_STATE);
+		if (ret == 0)
+			ret = dev->driver->suspend(dev, state, SUSPEND_POWER_DOWN);
+	}
+	return ret;
+}
+
+static int ac97_bus_resume(struct device *dev)
+{
+	int ret = 0;
+
+	if (dev->driver && dev->driver->resume) {
+		ret = dev->driver->resume(dev, RESUME_POWER_ON);
+		if (ret == 0)
+			ret = dev->driver->resume(dev, RESUME_RESTORE_STATE);
+		if (ret == 0)
+			ret = dev->driver->resume(dev, RESUME_ENABLE);
+	}
+	return ret;
+}
+
+struct bus_type ac97_bus_type = {
+	.name		= "ac97",
+	.match		= ac97_bus_match,
+	.suspend	= ac97_bus_suspend,
+	.resume		= ac97_bus_resume,
+};
+
+static int __init ac97_bus_init(void)
+{
+	return bus_register(&ac97_bus_type);
+}
+
+subsys_initcall(ac97_bus_init);
+
+static void __exit ac97_bus_exit(void)
+{
+	bus_unregister(&ac97_bus_type);
+}
+
+module_exit(ac97_bus_exit);
+
+EXPORT_SYMBOL(ac97_bus_type);
+
+MODULE_LICENSE("GPL");
diff --git a/sound/pci/ac97/ac97_codec.c b/sound/pci/ac97/ac97_codec.c
--- a/sound/pci/ac97/ac97_codec.c
+++ b/sound/pci/ac97/ac97_codec.c
@@ -992,6 +992,8 @@ static int snd_ac97_free(ac97_t *ac97)
 {
 	if (ac97) {
 		snd_ac97_proc_done(ac97);
+		if (ac97->dev.bus)
+			device_unregister(&ac97->dev);
 		if (ac97->bus) {
 			ac97->bus->codec[ac97->num] = NULL;
 			if (ac97->bus->shared_type) {
@@ -1007,6 +1009,11 @@ static int snd_ac97_free(ac97_t *ac97)
 	return 0;
 }
 
+/* Stops no dev release warning */
+static void ac97_dev_release(struct device * dev)
+{
+}
+
 static int snd_ac97_dev_free(snd_device_t *device)
 {
 	ac97_t *ac97 = device->device_data;
@@ -2117,6 +2124,14 @@ int snd_ac97_mixer(ac97_bus_t *bus, ac97
 		snd_ac97_free(ac97);
 		return err;
 	}
+
+	ac97->dev.bus = &ac97_bus_type;
+	ac97->dev.parent = ac97->bus->card->dev;
+	ac97->dev.platform_data = ac97;
+	ac97->dev.release = ac97_dev_release;
+	strncpy(ac97->dev.bus_id, snd_ac97_get_short_name(ac97), BUS_ID_SIZE);
+	if ((err = device_register(&ac97->dev)) < 0)
+		return err;
 	*rac97 = ac97;
 
 	if (bus->shared_type) {

Re: [PATCH] AC97 bus interface for ad-hoc drivers

[Takashi Iwai]

At Tue, 19 Jul 2005 13:20:24 +0100,
Liam Girdwood wrote:
> 
> This patch adds support for ad-hoc AC97 device drivers (e.g WM97xx and
> UCB touchscreen drivers) and was originally posted to the list by
> Nicolas Pitre as an RFC. 
> 
> Changes from RFC version :-
> 
>   o Now matches codec name within codec group.
>   o Added ac97_dev_release() to stop kernel complaining about no release
> method for device.

Thanks for the patch.  The idea is fine, and the code looks almost
OK except for the below.

Could you describe the full changelog (or paste Nicolas' RFC) ?


> diff --git a/sound/pci/ac97/Makefile b/sound/pci/ac97/Makefile
> --- a/sound/pci/ac97/Makefile
> +++ b/sound/pci/ac97/Makefile
> @@ -12,7 +12,7 @@ endif
>  snd-ak4531-codec-objs := ak4531_codec.o
>  
>  # Toplevel Module Dependency
> -obj-$(CONFIG_SND_AC97_CODEC) += snd-ac97-codec.o
> +obj-$(CONFIG_SND_AC97_CODEC) += snd-ac97-codec.o ac97_bus.o

Better to keep snd- prefix for ac97_bus module (although it can be
used independently from ALSA)...?  It's an open question.

> diff --git a/sound/pci/ac97/ac97_bus.c b/sound/pci/ac97/ac97_bus.c
> new file mode 100644
> --- /dev/null
> +++ b/sound/pci/ac97/ac97_bus.c
(snip)
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/device.h>
> +#include <linux/string.h>
> +
> +#include <sound/driver.h>
> +#include <sound/core.h>
> +#include <sound/ac97_codec.h>

Don't include unnecessary header files.

> diff --git a/sound/pci/ac97/ac97_codec.c b/sound/pci/ac97/ac97_codec.c
> --- a/sound/pci/ac97/ac97_codec.c
> +++ b/sound/pci/ac97/ac97_codec.c
(snip)
> @@ -2117,6 +2124,14 @@ int snd_ac97_mixer(ac97_bus_t *bus, ac97
>  		snd_ac97_free(ac97);
>  		return err;
>  	}
> +
> +	ac97->dev.bus = &ac97_bus_type;
> +	ac97->dev.parent = ac97->bus->card->dev;
> +	ac97->dev.platform_data = ac97;
> +	ac97->dev.release = ac97_dev_release;
> +	strncpy(ac97->dev.bus_id, snd_ac97_get_short_name(ac97), BUS_ID_SIZE);
> +	if ((err = device_register(&ac97->dev)) < 0)
> +		return err;

Need to call snd_ac97_free() in the error path (but reset dev.bus to
NULL to avoid unregister).


Takashi


-------------------------------------------------------
SF.Net email is sponsored by: Discover Easy Linux Migration Strategies
from IBM. Find simple to follow Roadmaps, straightforward articles,
informative Webcasts and more! Get everything you need to get up to
speed, fast. http://ads.osdn.com/?ad_id=7477&alloc_id=16492&op=click

Re: [PATCH] AC97 bus interface for ad-hoc drivers

[Nicolas Pitre]

On Wed, 27 Jul 2005, Takashi Iwai wrote:

> At Tue, 19 Jul 2005 13:20:24 +0100,
> Liam Girdwood wrote:
> > 
> > This patch adds support for ad-hoc AC97 device drivers (e.g WM97xx and
> > UCB touchscreen drivers) and was originally posted to the list by
> > Nicolas Pitre as an RFC. 
> > 
> > Changes from RFC version :-
> > 
> >   o Now matches codec name within codec group.
> >   o Added ac97_dev_release() to stop kernel complaining about no release
> > method for device.
> 
> Thanks for the patch.  The idea is fine, and the code looks almost
> OK except for the below.
> 
> Could you describe the full changelog (or paste Nicolas' RFC) ?
> 
> 
> > diff --git a/sound/pci/ac97/Makefile b/sound/pci/ac97/Makefile
> > --- a/sound/pci/ac97/Makefile
> > +++ b/sound/pci/ac97/Makefile
> > @@ -12,7 +12,7 @@ endif
> >  snd-ak4531-codec-objs := ak4531_codec.o
> >  
> >  # Toplevel Module Dependency
> > -obj-$(CONFIG_SND_AC97_CODEC) += snd-ac97-codec.o
> > +obj-$(CONFIG_SND_AC97_CODEC) += snd-ac97-codec.o ac97_bus.o
> 
> Better to keep snd- prefix for ac97_bus module (although it can be
> used independently from ALSA)...?  It's an open question.

The idea is to be able to use ALSA as modules and the extra function 
driver linked in, or vice versa.  Therefore ac97_bus.o should probably 
be selected with its own config symbol allowing for things like:

config UCB1400_TS
	tristate "UCB1400 touchscreen interface"
	select AC97_BUS


And similarly:

config SND_AC97_CODEC
	...
	select AC97_BUS

This way there is no hard linkage depedency between ALSA and any ad-hoc 
codec function driver.


Nicolas


-------------------------------------------------------
SF.Net email is sponsored by: Discover Easy Linux Migration Strategies
from IBM. Find simple to follow Roadmaps, straightforward articles,
informative Webcasts and more! Get everything you need to get up to
speed, fast. http://ads.osdn.com/?ad_id=7477&alloc_id=16492&op=click

Re: [PATCH] AC97 bus interface for ad-hoc drivers

[Takashi Iwai]

At Wed, 27 Jul 2005 09:05:33 -0400 (EDT),
Nicolas Pitre wrote:
> 
> On Wed, 27 Jul 2005, Takashi Iwai wrote:
> 
> > At Tue, 19 Jul 2005 13:20:24 +0100,
> > Liam Girdwood wrote:
> > > 
> > > This patch adds support for ad-hoc AC97 device drivers (e.g WM97xx and
> > > UCB touchscreen drivers) and was originally posted to the list by
> > > Nicolas Pitre as an RFC. 
> > > 
> > > Changes from RFC version :-
> > > 
> > >   o Now matches codec name within codec group.
> > >   o Added ac97_dev_release() to stop kernel complaining about no release
> > > method for device.
> > 
> > Thanks for the patch.  The idea is fine, and the code looks almost
> > OK except for the below.
> > 
> > Could you describe the full changelog (or paste Nicolas' RFC) ?
> > 
> > 
> > > diff --git a/sound/pci/ac97/Makefile b/sound/pci/ac97/Makefile
> > > --- a/sound/pci/ac97/Makefile
> > > +++ b/sound/pci/ac97/Makefile
> > > @@ -12,7 +12,7 @@ endif
> > >  snd-ak4531-codec-objs := ak4531_codec.o
> > >  
> > >  # Toplevel Module Dependency
> > > -obj-$(CONFIG_SND_AC97_CODEC) += snd-ac97-codec.o
> > > +obj-$(CONFIG_SND_AC97_CODEC) += snd-ac97-codec.o ac97_bus.o
> > 
> > Better to keep snd- prefix for ac97_bus module (although it can be
> > used independently from ALSA)...?  It's an open question.
> 
> The idea is to be able to use ALSA as modules and the extra function 
> driver linked in, or vice versa.  Therefore ac97_bus.o should probably 
> be selected with its own config symbol allowing for things like:
> 
> config UCB1400_TS
> 	tristate "UCB1400 touchscreen interface"
> 	select AC97_BUS
> 
> 
> And similarly:
> 
> config SND_AC97_CODEC
> 	...
> 	select AC97_BUS
> 
> This way there is no hard linkage depedency between ALSA and any ad-hoc 
> codec function driver.

Yes, I understand the purpose.  My concern was whether we keep a
consistent module name rule for the modules in sound directory, or
simply use as it is.  I have no special preference here, and would
like to hear from others.


Takashi


-------------------------------------------------------
SF.Net email is sponsored by: Discover Easy Linux Migration Strategies
from IBM. Find simple to follow Roadmaps, straightforward articles,
informative Webcasts and more! Get everything you need to get up to
speed, fast. http://ads.osdn.com/?ad_id=7477&alloc_id=16492&op=click

Re: [PATCH] AC97 bus interface for ad-hoc drivers

[Liam Girdwood]

On Wed, 2005-07-27 at 15:18 +0200, Takashi Iwai wrote:
> At Wed, 27 Jul 2005 09:05:33 -0400 (EDT),
> Nicolas Pitre wrote:
> > 
> > On Wed, 27 Jul 2005, Takashi Iwai wrote:
> > 
> > > At Tue, 19 Jul 2005 13:20:24 +0100,
> > > Liam Girdwood wrote:
> > > > 
> > > > This patch adds support for ad-hoc AC97 device drivers (e.g WM97xx and
> > > > UCB touchscreen drivers) and was originally posted to the list by
> > > > Nicolas Pitre as an RFC. 
> > > > 
> > > > Changes from RFC version :-
> > > > 
> > > >   o Now matches codec name within codec group.
> > > >   o Added ac97_dev_release() to stop kernel complaining about no release
> > > > method for device.
> > > 
> > > Thanks for the patch.  The idea is fine, and the code looks almost
> > > OK except for the below.
> > > 
> > > Could you describe the full changelog (or paste Nicolas' RFC) ?
> > > 
> > > 
> > > > diff --git a/sound/pci/ac97/Makefile b/sound/pci/ac97/Makefile
> > > > --- a/sound/pci/ac97/Makefile
> > > > +++ b/sound/pci/ac97/Makefile
> > > > @@ -12,7 +12,7 @@ endif
> > > >  snd-ak4531-codec-objs := ak4531_codec.o
> > > >  
> > > >  # Toplevel Module Dependency
> > > > -obj-$(CONFIG_SND_AC97_CODEC) += snd-ac97-codec.o
> > > > +obj-$(CONFIG_SND_AC97_CODEC) += snd-ac97-codec.o ac97_bus.o
> > > 
> > > Better to keep snd- prefix for ac97_bus module (although it can be
> > > used independently from ALSA)...?  It's an open question.
> > 
> > The idea is to be able to use ALSA as modules and the extra function 
> > driver linked in, or vice versa.  Therefore ac97_bus.o should probably 
> > be selected with its own config symbol allowing for things like:
> > 
> > config UCB1400_TS
> > 	tristate "UCB1400 touchscreen interface"
> > 	select AC97_BUS
> > 
> > 
> > And similarly:
> > 
> > config SND_AC97_CODEC
> > 	...
> > 	select AC97_BUS
> > 
> > This way there is no hard linkage depedency between ALSA and any ad-hoc 
> > codec function driver.
> 
> Yes, I understand the purpose.  My concern was whether we keep a
> consistent module name rule for the modules in sound directory, or
> simply use as it is.  I have no special preference here, and would
> like to hear from others.
> 

We should probably stick with the snd- naming convention because the
ad-hoc bus module does require the sound drivers to be loaded before any
ad-hoc driver will work. This does not mean the sound modules or bus
module needs to be loaded before the ad-hoc driver, it just means the
ad-hoc driver has a functional dependency on the others. 

Liam



-------------------------------------------------------
SF.Net email is sponsored by: Discover Easy Linux Migration Strategies
from IBM. Find simple to follow Roadmaps, straightforward articles,
informative Webcasts and more! Get everything you need to get up to
speed, fast. http://ads.osdn.com/?ad_id=7477&alloc_id=16492&op=click

Re: [PATCH] AC97 bus interface for ad-hoc drivers

[Liam Girdwood]

On Wed, 2005-07-27 at 09:05 -0400, Nicolas Pitre wrote:
> On Wed, 27 Jul 2005, Takashi Iwai wrote:
> > 
> > Better to keep snd- prefix for ac97_bus module (although it can be
> > used independently from ALSA)...?  It's an open question.
> 
> The idea is to be able to use ALSA as modules and the extra function 
> driver linked in, or vice versa.  Therefore ac97_bus.o should probably 
> be selected with its own config symbol allowing for things like:
> 
> config UCB1400_TS
> 	tristate "UCB1400 touchscreen interface"
> 	select AC97_BUS
> 
> 
> And similarly:
> 
> config SND_AC97_CODEC
> 	...
> 	select AC97_BUS
> 
> This way there is no hard linkage depedency between ALSA and any ad-hoc 
> codec function driver.

I've just unsuccessfully tried to create this dependency in the Kconfig
system. It looks like you cannot do:-

config SND_SOME_DRIVER
	tristate "some driver"
	select SND_AC97_CODEC

config SND_AC97_CODEC
	tristate 
	select SND_PCM
	select SND_AC97_BUS

config SND_AC97_BUS
	tristate

In this scenario SND_SOME_DRIVER selects SND_AC97_CODEC selects
SND_AC97_BUS and I could never get a SND_AC97_BUS=m or otherwise in
my .config. I was only ever successful by adding the select SND_AC97_BUS
line to SND_SOME_DRIVER.

I now don't think that the "select SND_PCM" is having any effect in the
AC97_CODEC config, although it still builds fine when remove it.

With this in mind (I'm not sure if this is intentional in the build
system or not), it might be better to stick with the original Makefile
and rename the module with a snd- prefix.

Liam

 

  



-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO September
19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf

Re: [PATCH] AC97 bus interface for ad-hoc drivers

[Takashi Iwai]

At Thu, 28 Jul 2005 21:26:39 +0100,
Liam Girdwood wrote:
> 
> On Wed, 2005-07-27 at 09:05 -0400, Nicolas Pitre wrote:
> > On Wed, 27 Jul 2005, Takashi Iwai wrote:
> > > 
> > > Better to keep snd- prefix for ac97_bus module (although it can be
> > > used independently from ALSA)...?  It's an open question.
> > 
> > The idea is to be able to use ALSA as modules and the extra function 
> > driver linked in, or vice versa.  Therefore ac97_bus.o should probably 
> > be selected with its own config symbol allowing for things like:
> > 
> > config UCB1400_TS
> > 	tristate "UCB1400 touchscreen interface"
> > 	select AC97_BUS
> > 
> > 
> > And similarly:
> > 
> > config SND_AC97_CODEC
> > 	...
> > 	select AC97_BUS
> > 
> > This way there is no hard linkage depedency between ALSA and any ad-hoc 
> > codec function driver.
> 
> I've just unsuccessfully tried to create this dependency in the Kconfig
> system. It looks like you cannot do:-
> 
> config SND_SOME_DRIVER
> 	tristate "some driver"
> 	select SND_AC97_CODEC
> 
> config SND_AC97_CODEC
> 	tristate 
> 	select SND_PCM
> 	select SND_AC97_BUS
> 
> config SND_AC97_BUS
> 	tristate
> 
> In this scenario SND_SOME_DRIVER selects SND_AC97_CODEC selects
> SND_AC97_BUS and I could never get a SND_AC97_BUS=m or otherwise in
> my .config. I was only ever successful by adding the select SND_AC97_BUS
> line to SND_SOME_DRIVER.

Is it a PCI driver?  Since SND_AC97_CODEC and SND_AC97_BUS is in
pci/Kconfig, it might the case that it's be selected only when
PCI!=n.


> I now don't think that the "select SND_PCM" is having any effect in the
> AC97_CODEC config, although it still builds fine when remove it.

Hmm, we didn't have problems with other similar cases...



Takashi


-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO September
19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf

Re: [PATCH] AC97 bus interface for ad-hoc drivers

[Liam Girdwood]

On Fri, 2005-07-29 at 12:45 +0200, Takashi Iwai wrote:
> At Thu, 28 Jul 2005 21:26:39 +0100,
> Liam Girdwood wrote:

> > 
> > In this scenario SND_SOME_DRIVER selects SND_AC97_CODEC selects
> > SND_AC97_BUS and I could never get a SND_AC97_BUS=m or otherwise in
> > my .config. I was only ever successful by adding the select SND_AC97_BUS
> > line to SND_SOME_DRIVER.
> 
> Is it a PCI driver?  Since SND_AC97_CODEC and SND_AC97_BUS is in
> pci/Kconfig, it might the case that it's be selected only when
> PCI!=n.
> 
> 

It's an Arm (PXA27x) based driver, hence PCI=n.

You are correct. I had unsuccessfully tried the config SND_AC97_BUS out
with the PCI=n clause. It only works when _both_ SND_AC97_CODEC and
SND_AC97_BUS are out with PCI=n.

If nobody objects, I can resubmit with the SND_AC97_CODEC out with the
PCI=n in sound/pci/Kconfig. Will this cause any problems for other
cards ? 

> > I now don't think that the "select SND_PCM" is having any effect in the
> > AC97_CODEC config, although it still builds fine when remove it.
> 
> Hmm, we didn't have problems with other similar cases...

It was being included by the other Arm drivers.

Liam 



-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO September
19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf

Re: [PATCH] AC97 bus interface for ad-hoc drivers

[Takashi Iwai]

At Fri, 29 Jul 2005 12:20:40 +0100,
Liam Girdwood wrote:
> 
> On Fri, 2005-07-29 at 12:45 +0200, Takashi Iwai wrote:
> > At Thu, 28 Jul 2005 21:26:39 +0100,
> > Liam Girdwood wrote:
> 
> > > 
> > > In this scenario SND_SOME_DRIVER selects SND_AC97_CODEC selects
> > > SND_AC97_BUS and I could never get a SND_AC97_BUS=m or otherwise in
> > > my .config. I was only ever successful by adding the select SND_AC97_BUS
> > > line to SND_SOME_DRIVER.
> > 
> > Is it a PCI driver?  Since SND_AC97_CODEC and SND_AC97_BUS is in
> > pci/Kconfig, it might the case that it's be selected only when
> > PCI!=n.
> > 
> > 
> 
> It's an Arm (PXA27x) based driver, hence PCI=n.
> 
> You are correct. I had unsuccessfully tried the config SND_AC97_BUS out
> with the PCI=n clause. It only works when _both_ SND_AC97_CODEC and
> SND_AC97_BUS are out with PCI=n.
> 
> If nobody objects, I can resubmit with the SND_AC97_CODEC out with the
> PCI=n in sound/pci/Kconfig. Will this cause any problems for other
> cards ? 

Please post it.  That change should be harmless.


Takashi


-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO September
19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf

Re: [PATCH] AC97 bus interface for ad-hoc drivers

[Liam Girdwood]

[-- Attachment #1: Type: text/plain, Size: 1187 bytes --]

I've made the review changes and as requested I've pasted the RFC by
Nicolas below:-

"I would like to know what people think of the following patch.  It 
allows for a codec on an AC97 bus to be shared with other drivers which 
are completely unrelated to audio.  It registers a new bus type, and 
whenever a codec instance is created then a device for it is also 
registered with the driver model using that bus type.  This allows, for 
example, to use the extra features of the UCB1400 like the touchscreen 
interface and the additional GPIOs and ADCs available on that chip for 
battery monitoring.  I have a working UCB1400 touchscreen driver here 
that simply registers with the driver model happily working alongside 
with audio features using this."

Changes over RFC:-
 
  o Now matches codec name within codec group.
  o Added ac97_dev_release() to stop kernel complaining about no release
method for device.
  o Added "config SND_AC97_BUS" to sound/pci/Kconfig and moved "config
SND_AC97_CODEC" out with the PCI=n statement.
  o module is now called snd-ac97-bus

Signed-off-by: Liam Girdwood <liam.girdwood@wolfsonmicro.com>
Signed-off-by: Nicolas Pitre <nico@cam.org>

Liam 

[-- Attachment #2: ac97_bus.diff --]
[-- Type: text/x-patch, Size: 4770 bytes --]

diff --git a/sound/pci/Kconfig b/sound/pci/Kconfig
--- a/sound/pci/Kconfig
+++ b/sound/pci/Kconfig
@@ -1,11 +1,15 @@
 # ALSA PCI drivers
 
-menu "PCI devices"
-	depends on SND!=n && PCI
-
 config SND_AC97_CODEC
 	tristate
 	select SND_PCM
+	select SND_AC97_BUS
+
+config SND_AC97_BUS
+	tristate
+
+menu "PCI devices"
+	depends on SND!=n && PCI
 
 config SND_ALI5451
 	tristate "ALi M5451 PCI Audio Controller"
diff --git a/sound/pci/ac97/ac97_bus.c b/sound/pci/ac97/ac97_bus.c
new file mode 100644
--- /dev/null
+++ b/sound/pci/ac97/ac97_bus.c
@@ -0,0 +1,79 @@
+/*
+ * Linux driver model AC97 bus interface
+ *
+ * Author:	Nicolas Pitre
+ * Created:	Jan 14, 2005
+ * Copyright:	(C) MontaVista Software Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/device.h>
+#include <linux/string.h>
+
+/*
+ * Codec families have names seperated by commas, so we search for an
+ * individual codec name within the family string. 
+ */
+static int ac97_bus_match(struct device *dev, struct device_driver *drv)
+{
+	return (strstr(dev->bus_id, drv->name) != NULL);
+}
+
+static int ac97_bus_suspend(struct device *dev, pm_message_t state)
+{
+	int ret = 0;
+
+	if (dev->driver && dev->driver->suspend) {
+		ret = dev->driver->suspend(dev, state, SUSPEND_DISABLE);
+		if (ret == 0)
+			ret = dev->driver->suspend(dev, state, SUSPEND_SAVE_STATE);
+		if (ret == 0)
+			ret = dev->driver->suspend(dev, state, SUSPEND_POWER_DOWN);
+	}
+	return ret;
+}
+
+static int ac97_bus_resume(struct device *dev)
+{
+	int ret = 0;
+
+	if (dev->driver && dev->driver->resume) {
+		ret = dev->driver->resume(dev, RESUME_POWER_ON);
+		if (ret == 0)
+			ret = dev->driver->resume(dev, RESUME_RESTORE_STATE);
+		if (ret == 0)
+			ret = dev->driver->resume(dev, RESUME_ENABLE);
+	}
+	return ret;
+}
+
+struct bus_type ac97_bus_type = {
+	.name		= "ac97",
+	.match		= ac97_bus_match,
+	.suspend	= ac97_bus_suspend,
+	.resume		= ac97_bus_resume,
+};
+
+static int __init ac97_bus_init(void)
+{
+	return bus_register(&ac97_bus_type);
+}
+
+subsys_initcall(ac97_bus_init);
+
+static void __exit ac97_bus_exit(void)
+{
+	bus_unregister(&ac97_bus_type);
+}
+
+module_exit(ac97_bus_exit);
+
+EXPORT_SYMBOL(ac97_bus_type);
+
+MODULE_LICENSE("GPL");
diff --git a/sound/pci/ac97/ac97_codec.c b/sound/pci/ac97/ac97_codec.c
--- a/sound/pci/ac97/ac97_codec.c
+++ b/sound/pci/ac97/ac97_codec.c
@@ -992,6 +992,8 @@ static int snd_ac97_free(ac97_t *ac97)
 {
 	if (ac97) {
 		snd_ac97_proc_done(ac97);
+		if (ac97->dev.bus)
+			device_unregister(&ac97->dev);
 		if (ac97->bus) {
 			ac97->bus->codec[ac97->num] = NULL;
 			if (ac97->bus->shared_type) {
@@ -1007,6 +1009,11 @@ static int snd_ac97_free(ac97_t *ac97)
 	return 0;
 }
 
+/* stop no dev release warning */
+static void ac97_dev_release(struct device * dev)
+{
+}
+
 static int snd_ac97_dev_free(snd_device_t *device)
 {
 	ac97_t *ac97 = device->device_data;
@@ -2117,6 +2136,17 @@ int snd_ac97_mixer(ac97_bus_t *bus, ac97
 		snd_ac97_free(ac97);
 		return err;
 	}
+
+	ac97->dev.bus = &ac97_bus_type;
+	ac97->dev.parent = ac97->bus->card->dev;
+	ac97->dev.platform_data = ac97;
+	ac97->dev.release = ac97_dev_release;
+	strncpy(ac97->dev.bus_id, snd_ac97_get_short_name(ac97), BUS_ID_SIZE);
+	if ((err = device_register(&ac97->dev)) < 0) {
+		ac97->dev.bus = NULL;
+		snd_ac97_free(ac97);
+		return err;
+	}
 	*rac97 = ac97;
 
 	if (bus->shared_type) {
diff --git a/include/sound/ac97_codec.h b/include/sound/ac97_codec.h
--- a/include/sound/ac97_codec.h
+++ b/include/sound/ac97_codec.h
@@ -26,6 +26,7 @@
  */
 
 #include <linux/bitops.h>
+#include <linux/device.h>
 #include "pcm.h"
 #include "control.h"
 #include "info.h"
@@ -520,6 +524,7 @@ struct _snd_ac97 {
 	/* jack-sharing info */
 	unsigned char indep_surround;
 	unsigned char channel_mode;
+	struct device dev;
 };
 
 /* conditions */
@@ -599,4 +604,8 @@ struct ac97_enum {
 	unsigned short mask;
 	const char **texts;
 };
+
+/* ad hoc AC97 device driver access */
+extern struct bus_type ac97_bus_type;
+
 #endif /* __SOUND_AC97_CODEC_H */
diff --git a/sound/pci/ac97/Makefile b/sound/pci/ac97/Makefile
--- a/sound/pci/ac97/Makefile
+++ b/sound/pci/ac97/Makefile
@@ -10,9 +10,11 @@ snd-ac97-codec-objs += ac97_proc.o
 endif
 
 snd-ak4531-codec-objs := ak4531_codec.o
+snd-ac97-bus-objs := ac97_bus.o
 
 # Toplevel Module Dependency
 obj-$(CONFIG_SND_AC97_CODEC) += snd-ac97-codec.o
 obj-$(CONFIG_SND_ENS1370) += snd-ak4531-codec.o
+obj-$(CONFIG_SND_AC97_BUS) += snd-ac97-bus.o
 
 obj-m := $(sort $(obj-m))

Re: [PATCH] AC97 bus interface for ad-hoc drivers

[Takashi Iwai]

At Fri, 29 Jul 2005 13:51:56 +0100,
Liam Girdwood wrote:
> 
> I've made the review changes and as requested I've pasted the RFC by
> Nicolas below:-
> 
> "I would like to know what people think of the following patch.  It 
> allows for a codec on an AC97 bus to be shared with other drivers which 
> are completely unrelated to audio.  It registers a new bus type, and 
> whenever a codec instance is created then a device for it is also 
> registered with the driver model using that bus type.  This allows, for 
> example, to use the extra features of the UCB1400 like the touchscreen 
> interface and the additional GPIOs and ADCs available on that chip for 
> battery monitoring.  I have a working UCB1400 touchscreen driver here 
> that simply registers with the driver model happily working alongside 
> with audio features using this."
> 
> Changes over RFC:-
>  
>   o Now matches codec name within codec group.
>   o Added ac97_dev_release() to stop kernel complaining about no release
> method for device.
>   o Added "config SND_AC97_BUS" to sound/pci/Kconfig and moved "config
> SND_AC97_CODEC" out with the PCI=n statement.
>   o module is now called snd-ac97-bus
> 
> Signed-off-by: Liam Girdwood <liam.girdwood@wolfsonmicro.com>
> Signed-off-by: Nicolas Pitre <nico@cam.org>

Thanks.  I applied it to CVS with some modifications.

- the registration and unregistration to ac97_bus is done in
  dev_register and dev_unregister callbacks.  This assures that
  card->dev is set.

- 2.2/2.4 workaronds in alsa-driver tree.

Please check whether it's OK (later, after cvs tree is sync'ed)


Takashi


-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO September
19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf

Re: [PATCH] AC97 bus interface for ad-hoc drivers

[Liam Girdwood]

On Fri, 2005-07-29 at 17:19 +0200, Takashi Iwai wrote:
> 
> Thanks.  I applied it to CVS with some modifications.
> 
> - the registration and unregistration to ac97_bus is done in
>   dev_register and dev_unregister callbacks.  This assures that
>   card->dev is set.
> 
> - 2.2/2.4 workaronds in alsa-driver tree.
> 
> Please check whether it's OK (later, after cvs tree is sync'ed)

Works as expected.

Thanks

Liam



-------------------------------------------------------
SF.Net email is sponsored by: Discover Easy Linux Migration Strategies
from IBM. Find simple to follow Roadmaps, straightforward articles,
informative Webcasts and more! Get everything you need to get up to
speed, fast. http://ads.osdn.com/?ad_id=7477&alloc_id=16492&op=click