LKML Archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] PNP: cleanup pnp_fixup_device()
@ 2008-05-06  1:08 Rene Herman
  2008-05-06  1:14 ` Rene Herman
  2008-05-06 17:15 ` Bjorn Helgaas
  0 siblings, 2 replies; 7+ messages in thread
From: Rene Herman @ 2008-05-06  1:08 UTC (permalink / raw
  To: Bjorn Helgaas
  Cc: Uwe Bugla, Takashi Iwai, Len Brown, Andrew Morton, Linux Kernel

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

Hi Bjorn.

In response to off-list AD181x thread, small cleanup to quirk handling.

Patch 3/3 does the actual work. Mind if I feed this stuff through you? And,
mind this, period? Not wed to it or anything...

Signed-off-by: Rene Herman <rene.herman@gmail.com>


[-- Attachment #2: pnp-cleanup-pnp_fixup_device.diff --]
[-- Type: text/plain, Size: 1067 bytes --]

commit 4ed0cb4cef2b45a257b6d7d6cdc987e3511ddf58
Author: Rene Herman <rene.herman@gmail.com>
Date:   Tue May 6 01:51:47 2008 +0200

    PNP: cleanup pnp_fixup_device()
    
    Signed-off-by: Rene Herman <rene.herman@gmail.com>

diff --git a/drivers/pnp/quirks.c b/drivers/pnp/quirks.c
index 3f2d34a..10b1295 100644
--- a/drivers/pnp/quirks.c
+++ b/drivers/pnp/quirks.c
@@ -209,20 +209,12 @@ static struct pnp_fixup pnp_fixups[] = {
 
 void pnp_fixup_device(struct pnp_dev *dev)
 {
-	int i = 0;
-	void (*quirk)(struct pnp_dev *);
-
-	while (*pnp_fixups[i].id) {
-		if (compare_pnp_id(dev->id, pnp_fixups[i].id)) {
-			quirk = pnp_fixups[i].quirk_function;
-
-#ifdef DEBUG
-			dev_dbg(&dev->dev, "calling quirk 0x%p", quirk);
-			print_fn_descriptor_symbol(": %s()\n",
-				(unsigned long) *quirk);
-#endif
-			(*quirk)(dev);
-		}
-		i++;
+	int i;
+
+	for (i = 0; *pnp_fixups[i].id; i++) {
+		if (!compare_pnp_id(dev->id, pnp_fixups[i].id))
+			continue;
+		dev_dbg(&dev->dev, "calling quirk for %s\n", pnp_fixups[i].id);
+		pnp_fixups[i].quirk_function(dev);
 	}
 }



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/3] PNP: cleanup pnp_fixup_device()
  2008-05-06  1:08 [PATCH 1/3] PNP: cleanup pnp_fixup_device() Rene Herman
@ 2008-05-06  1:14 ` Rene Herman
  2008-05-06 17:15 ` Bjorn Helgaas
  1 sibling, 0 replies; 7+ messages in thread
From: Rene Herman @ 2008-05-06  1:14 UTC (permalink / raw
  To: Bjorn Helgaas
  Cc: Uwe Bugla, Takashi Iwai, Len Brown, Andrew Morton, Linux Kernel

On 06-05-08 03:08, Rene Herman wrote:

> In response to off-list AD181x thread, small cleanup to quirk handling.
> 
> Patch 3/3 does the actual work. Mind if I feed this stuff through you? And,
> mind this, period? Not wed to it or anything...

This by the way is still against your v5. I see there are changes in current 
mainline again. If you need me to regenerate against mainline, please say.

Rene.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/3] PNP: cleanup pnp_fixup_device()
  2008-05-06  1:08 [PATCH 1/3] PNP: cleanup pnp_fixup_device() Rene Herman
  2008-05-06  1:14 ` Rene Herman
@ 2008-05-06 17:15 ` Bjorn Helgaas
  2008-05-10 21:48   ` Rene Herman
       [not found]   ` <482614B3.4000908@gmail.com>
  1 sibling, 2 replies; 7+ messages in thread
From: Bjorn Helgaas @ 2008-05-06 17:15 UTC (permalink / raw
  To: Rene Herman
  Cc: Uwe Bugla, Takashi Iwai, Len Brown, Andrew Morton, Linux Kernel

On Monday 05 May 2008 07:08:09 pm Rene Herman wrote:

[It's easier to comment if the patch is inline rather than attached.]

> PNP: cleanup pnp_fixup_device()
>    
>     Signed-off-by: Rene Herman <rene.herman@gmail.com>
>
> diff --git a/drivers/pnp/quirks.c b/drivers/pnp/quirks.c
> index 3f2d34a..10b1295 100644
> --- a/drivers/pnp/quirks.c
> +++ b/drivers/pnp/quirks.c
> @@ -209,20 +209,12 @@ static struct pnp_fixup pnp_fixups[] = {
>  
>  void pnp_fixup_device(struct pnp_dev *dev)
>  {
> -       int i = 0;
> -       void (*quirk)(struct pnp_dev *);
> -
> -       while (*pnp_fixups[i].id) {
> -               if (compare_pnp_id(dev->id, pnp_fixups[i].id)) {
> -                       quirk = pnp_fixups[i].quirk_function;
> -
> -#ifdef DEBUG
> -                       dev_dbg(&dev->dev, "calling quirk 0x%p", quirk);
> -                       print_fn_descriptor_symbol(": %s()\n",
> -                               (unsigned long) *quirk); 
> -#endif
> -                       (*quirk)(dev);
> -               }
> -               i++;
> +       int i;
> +
> +       for (i = 0; *pnp_fixups[i].id; i++) {
> +               if (!compare_pnp_id(dev->id, pnp_fixups[i].id))
> +                       continue;
> +               dev_dbg(&dev->dev, "calling quirk for %s\n", pnp_fixups[i].id);
> +               pnp_fixups[i].quirk_function(dev);
>         }
>  }

I'm sort of attached to keeping this similar to pci_do_fixups()
and keeping the symbol, even though the #ifdef is ugly.  I do
like getting rid of the "0x%p" and adding the PNP ID.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/3] PNP: cleanup pnp_fixup_device()
  2008-05-06 17:15 ` Bjorn Helgaas
@ 2008-05-10 21:48   ` Rene Herman
       [not found]   ` <482614B3.4000908@gmail.com>
  1 sibling, 0 replies; 7+ messages in thread
From: Rene Herman @ 2008-05-10 21:48 UTC (permalink / raw
  To: Bjorn Helgaas
  Cc: Uwe Bugla, Takashi Iwai, Len Brown, Andrew Morton, Linux Kernel

On 06-05-08 19:15, Bjorn Helgaas wrote:

> It's easier to comment if the patch is inline rather than attached.

I hope that works...

> I'm sort of attached to keeping this similar to pci_do_fixups() and
> keeping the symbol, even though the #ifdef is ugly. I do like getting
> rid of the "0x%p" and adding the PNP ID.

Okay. This better? Makes it a bit more similar even.

===
PNP: cleanup pnp_fixup_device()

Make it look a bit more like pci_fixup_device/pci_do_fixups. Also
print the PnP ID and delete the () from the "foo+0x0/0x1234()".

Signed-off-by: Rene Herman <rene.herman@gmail.com>
---
drivers/pnp/quirks.c |   20 ++++++++------------
1 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/pnp/quirks.c b/drivers/pnp/quirks.c
index d049a22..a1af2f9 100644
--- a/drivers/pnp/quirks.c
+++ b/drivers/pnp/quirks.c
@@ -212,20 +212,16 @@ static struct pnp_fixup pnp_fixups[] = {

void pnp_fixup_device(struct pnp_dev *dev)
{
-	int i = 0;
-	void (*quirk)(struct pnp_dev *);
-
-	while (*pnp_fixups[i].id) {
-		if (compare_pnp_id(dev->id, pnp_fixups[i].id)) {
-			quirk = pnp_fixups[i].quirk_function;
+	struct pnp_fixup *f;

+	for (f = pnp_fixups; *f->id; f++) {
+		if (!compare_pnp_id(dev->id, f->id))
+			continue;
#ifdef DEBUG
-			dev_dbg(&dev->dev, "calling ");
-			print_fn_descriptor_symbol("%s()\n",
-				(unsigned long) *quirk);
+		dev_dbg(&dev->dev, "%s: calling ", f->id);
+		print_fn_descriptor_symbol("%s\n",
+				(unsigned long) f->quirk_function);
#endif
-			(*quirk)(dev);
-		}
-		i++;
+		f->quirk_function(dev);
	}
}



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/3] PNP: add pnp_build_option() to the API
       [not found]   ` <482614B3.4000908@gmail.com>
@ 2008-05-10 21:48     ` Rene Herman
  2008-05-10 21:48     ` [PATCH 3/3] PNP: add ISAPnP MPU option quirks Rene Herman
  1 sibling, 0 replies; 7+ messages in thread
From: Rene Herman @ 2008-05-10 21:48 UTC (permalink / raw
  To: Bjorn Helgaas
  Cc: Uwe Bugla, Takashi Iwai, Len Brown, Andrew Morton, Linux Kernel

Hi Bjorn,

===
PNP: add pnp_build_option() to the API

The subsequent ISAPnP MPU quirk patch would like this as part of the
API. pnp_register_dependent_option() adds to the same dependent chain
the quirk is walking which is fairly unclean. This enables a private
option chain build which it can then just add onto the end when done.

Signed-off-by: Rene Herman <rene.herman@gmail.com>
---
drivers/pnp/base.h     |    1 +
drivers/pnp/resource.c |    2 +-
2 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/pnp/base.h b/drivers/pnp/base.h
index 4fe7c58..886dac8 100644
--- a/drivers/pnp/base.h
+++ b/drivers/pnp/base.h
@@ -19,6 +19,7 @@ void pnp_remove_card(struct pnp_card *card);
int pnp_add_card_device(struct pnp_card *card, struct pnp_dev *dev);
void pnp_remove_card_device(struct pnp_dev *dev);

+struct pnp_option *pnp_build_option(int priority);
struct pnp_option *pnp_register_independent_option(struct pnp_dev *dev);
struct pnp_option *pnp_register_dependent_option(struct pnp_dev *dev,
						 int priority);
diff --git a/drivers/pnp/resource.c b/drivers/pnp/resource.c
index 2041620..390b500 100644
--- a/drivers/pnp/resource.c
+++ b/drivers/pnp/resource.c
@@ -28,7 +28,7 @@ static int pnp_reserve_mem[16] = {[0 ... 15] = -1 };	/* reserve (don't use) some
* option registration
*/

-static struct pnp_option *pnp_build_option(int priority)
+struct pnp_option *pnp_build_option(int priority)
{
	struct pnp_option *option = pnp_alloc(sizeof(struct pnp_option));

-- 
1.5.2.2




^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/3] PNP: add ISAPnP MPU option quirks
       [not found]   ` <482614B3.4000908@gmail.com>
  2008-05-10 21:48     ` [PATCH 2/3] PNP: add pnp_build_option() to the API Rene Herman
@ 2008-05-10 21:48     ` Rene Herman
  2008-05-10 21:56       ` Rene Herman
  1 sibling, 1 reply; 7+ messages in thread
From: Rene Herman @ 2008-05-10 21:48 UTC (permalink / raw
  To: Bjorn Helgaas
  Cc: Uwe Bugla, Takashi Iwai, Len Brown, Andrew Morton, Linux Kernel

On 06-05-08 23:27, Bjorn Helgaas wrote:

> OK, thanks a lot for the explanation. It makes a lot more sense now,
> and I'm happy with your approach now. It shouldn't make anything 
> worse, and it should make the card useful in some cases where it 
> wasn't before.

Or at least allow one less shared PCI interrupt...

With respect to the last posting, this also grew the AZT0002 ID (which
just shares the same code) which the AZT2320 exports for its MPU401 due
to it sharing the same mandatory-IRQ setup as AD1816.

===
PNP: add ISAPnP MPU option quirks.

The AD181x and AZT230 chips don't support an IRQ-less MPU401 option
but work fine without one. This adds (priority functional) IRQ-less
options for each port option to help systems with few available IRQs.

The AD1815 quirk can't use pnp_register_irq_resource() due to doubly
penalizing the IRQ. Also, while not a practical issue due to no IRQ
option being present for the dependents, this needs to add in front,
not back.

Doesn't use pnp_register_port_resource() for symetry with above.

This does not delete the AD1815 independent option even though it
should be empty after the IRQ transfer due to AD1816 coming with an
empty but still present independent option by default.

Was tested on AD1815, AD1816 and AZT230. The ALSA snd-ad1818a driver
also support the AZT2002 ID for MPU401 but this doesn't as I was
unable to test it.

Signed-off-by: Rene Herman <rene.herman@gmail.com>
---
drivers/pnp/quirks.c |  112 ++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 112 insertions(+), 0 deletions(-)

diff --git a/drivers/pnp/quirks.c b/drivers/pnp/quirks.c
index a1af2f9..ffdb12a 100644
--- a/drivers/pnp/quirks.c
+++ b/drivers/pnp/quirks.c
@@ -111,6 +111,113 @@ static void quirk_sb16audio_resources(struct pnp_dev *dev)
		dev_info(&dev->dev, "SB audio device quirk - increased port range\n");
}

+static struct pnp_option *quirk_isapnp_mpu_options(struct pnp_dev *dev)
+{
+	struct pnp_option *head = NULL;
+	struct pnp_option *prev = NULL;
+	struct pnp_option *res;
+
+	/*
+	 * Build a functional IRQ-less variant of each MPU option.
+	 */
+
+	for (res = dev->dependent; res; res = res->next) {
+		struct pnp_option *curr;
+		struct pnp_port *port;
+		struct pnp_port *copy;
+
+		port = res->port;
+		if (!port || !res->irq)
+			continue;
+
+		copy = pnp_alloc(sizeof *copy);
+		if (!copy)
+			break;
+
+		copy->min = port->min;
+		copy->max = port->max;
+		copy->align = port->align;
+		copy->size = port->size;
+		copy->flags = port->flags;
+
+		curr = pnp_build_option(PNP_RES_PRIORITY_FUNCTIONAL);
+		if (!curr) {
+			kfree(copy);
+			break;
+		}
+		curr->port = copy;
+
+		if (prev)
+			prev->next = curr;
+		else
+			head = curr;
+		prev = curr;
+	}
+	if (head)
+		dev_info(&dev->dev, "adding IRQ-less MPU options\n");
+
+	return head;
+}
+
+static void quirk_ad1815_mpu_resources(struct pnp_dev *dev)
+{
+	struct pnp_option *res;
+	struct pnp_irq *irq;
+
+	/*
+	 * Distribute the independent IRQ over the dependent options
+	 */
+
+	res = dev->independent;
+	if (!res)
+		return;
+
+	irq = res->irq;
+	if (!irq || irq->next)
+		return;
+
+	res = dev->dependent;
+	if (!res)
+		return;
+
+	while (1) {
+		struct pnp_irq *copy;
+
+		copy = pnp_alloc(sizeof *copy);
+		if (!copy)
+			break;
+
+		memcpy(copy->map, irq->map, sizeof copy->map);
+		copy->flags = irq->flags;
+
+		copy->next = res->irq; /* Yes, this is NULL */
+		res->irq = copy;
+
+		if (!res->next)
+			break;
+		res = res->next;
+	}
+	kfree(irq);
+
+	res->next = quirk_isapnp_mpu_options(dev);
+
+	res = dev->independent;
+	res->irq = NULL;
+}
+
+static void quirk_isapnp_mpu_resources(struct pnp_dev *dev)
+{
+	struct pnp_option *res;
+
+	res = dev->dependent;
+	if (!res)
+		return;
+
+	while (res->next)
+		res = res->next;
+
+	res->next = quirk_isapnp_mpu_options(dev);
+}

#include <linux/pci.h>

@@ -205,6 +312,11 @@ static struct pnp_fixup pnp_fixups[] = {
	{"CTL0043", quirk_sb16audio_resources},
	{"CTL0044", quirk_sb16audio_resources},
	{"CTL0045", quirk_sb16audio_resources},
+	/* Add IRQ-less MPU options */
+	{"ADS7151", quirk_ad1815_mpu_resources},
+	{"ADS7181", quirk_isapnp_mpu_resources},
+	{"AZT0002", quirk_isapnp_mpu_resources},
+	/* PnP resources that might overlap PCI BARs */
	{"PNP0c01", quirk_system_pci_resources},
	{"PNP0c02", quirk_system_pci_resources},
	{""}
-- 
1.5.2.2



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/3] PNP: add ISAPnP MPU option quirks
  2008-05-10 21:48     ` [PATCH 3/3] PNP: add ISAPnP MPU option quirks Rene Herman
@ 2008-05-10 21:56       ` Rene Herman
  0 siblings, 0 replies; 7+ messages in thread
From: Rene Herman @ 2008-05-10 21:56 UTC (permalink / raw
  To: Bjorn Helgaas
  Cc: Uwe Bugla, Takashi Iwai, Len Brown, Andrew Morton, Linux Kernel

On 10-05-08 23:48, Rene Herman wrote:

> PNP: add ISAPnP MPU option quirks.
> 
> The AD181x and AZT230 chips don't support an IRQ-less MPU401 option
> but work fine without one. This adds (priority functional) IRQ-less
> options for each port option to help systems with few available IRQs.
> 
> The AD1815 quirk can't use pnp_register_irq_resource() due to doubly
> penalizing the IRQ. Also, while not a practical issue due to no IRQ
> option being present for the dependents, this needs to add in front,
> not back.
> 
> Doesn't use pnp_register_port_resource() for symetry with above.
> 
> This does not delete the AD1815 independent option even though it
> should be empty after the IRQ transfer due to AD1816 coming with an
> empty but still present independent option by default.
> 
> Was tested on AD1815, AD1816 and AZT230. The ALSA snd-ad1818a driver

Typo: AZT2320

> also support the AZT2002 ID for MPU401 but this doesn't as I was
> unable to test it.
> 
> Signed-off-by: Rene Herman <rene.herman@gmail.com>

Rene.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2008-05-10 21:55 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-05-06  1:08 [PATCH 1/3] PNP: cleanup pnp_fixup_device() Rene Herman
2008-05-06  1:14 ` Rene Herman
2008-05-06 17:15 ` Bjorn Helgaas
2008-05-10 21:48   ` Rene Herman
     [not found]   ` <482614B3.4000908@gmail.com>
2008-05-10 21:48     ` [PATCH 2/3] PNP: add pnp_build_option() to the API Rene Herman
2008-05-10 21:48     ` [PATCH 3/3] PNP: add ISAPnP MPU option quirks Rene Herman
2008-05-10 21:56       ` Rene Herman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).