Re: RFC: [PATCH Upated]: snd-pmac-gpio interfaces for snd-powermac

Hi,

I have no idea if this is the latest version of this patch, but I
couldn't find any more recent one...

A few of comments inlined below.

+static struct pmf_function *get_audio_pfunc(const char *name, const char *altname)
+{
+	struct device_node *np;
+	struct pmf_function *pfunc = NULL;
+
+	if (! (np = find_devices("i2s-a")))
+		return NULL;

Can we have this function take an explicit device parameter instead? As
it is now, it can only handle the i2s-a soundcard, while the latest
powermacs have two sound cards (codecs?): i2s-a and i2s-c.

+int snd_pmac_get_gpio(const char *name, const char *altname,
+                            snd_pmac_gpio_t *gp)
+{
+        memset(gp, 0, sizeof(*gp));
+
+	gp->name = name;
+	gp->altname = altname;
+
+        /* Platform functions are prefered */
+        if ((gp->pfunc = get_audio_pfunc(name, altname)))
+                return 0;
+
+	/* Else, fallback to direct gpio */
+	return get_audio_gpio(name, altname, gp);

Maybe there ought to be a way to disable the fallback when we're on
newer chips? I don't grok the sound architecture well enough yet to
tell.

+			/* XXX: pmf_unregister_irq_client doesn't use its
+			 * first two arguments. We only need to send it
+			 * the irq_client. WATCH FOR THIS CHANGING!
+			 */
+			pmf_unregister_irq_client(NULL, NULL, &gp->irq_client);

Heh, so I'm looking at an old version of this patch. The current
pmf_unregister_irq_client makes this explicit and only takes one
parameter :)

+int snd_pmac_request_irq(snd_pmac_gpio_t *gp, void (*handler)(void *),
+			 void *data)
+{
+	int ret = -ENODEV;
+	struct device_node *np;
+
+	gp->irq_client.handler = handler;
+	gp->irq_client.data = data;
+	gp->irq_client.owner = NULL;
+
+	if (gp->pfunc) {
+		gp->irq_client.owner = THIS_MODULE;
+
+		if ((np = find_devices("i2s-a"))) {

same comment here as the first one -- the powermac needs to be able to
access i2s-c through this too, I think.

johannes