diff mbox

[1/4] amba: add name based matching

Message ID 1300200116-12185-2-git-send-email-philippe.langlais@stericsson.com
State New
Headers show

Commit Message

Philippe Langlais March 15, 2011, 2:41 p.m. UTC 
From: Rabin Vincent <rabin.vincent@stericsson.com>

Some peripherals on the DBx500 family of SoCs have changes in their
functionality and registers between different variants in the family but
retain the same AMBA peripheral ID, making it impossible to distinguish
between them in AMBA drivers with the current AMBA id_table.

To support this, add a name parameter to the amba_device and the amba_id
and allow name based matching as a second level filter after the
periphid match.

Acked-by: Linus Walleij <linus.walleij@stericsson.com>
Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
---
 drivers/amba/bus.c       |    6 ++++++
 include/linux/amba/bus.h |    2 ++
 2 files changed, 8 insertions(+), 0 deletions(-)

Comments

Russell King - ARM Linux March 17, 2011, 8:48 a.m. UTC | #1 
On Thu, Mar 17, 2011 at 09:47:35AM +0100, Linus Walleij wrote:
> 2011/3/15 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> 
> > I really hate the idea of matching AMBA stuff by name.  Isn't there any
> > other solution to this?
> 
> What do you think about this:
> 
> diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
> index 6d2bb25..fcf355c 100644
> --- a/drivers/amba/bus.c
> +++ b/drivers/amba/bus.c
> @@ -603,6 +603,10 @@ int amba_device_register(struct amba_device *dev,
> struct resource *parent)
>         if (ret)
>                 goto err_out;
> 
> +       /* Hard-coded primecell ID instead of plug-n-play */
> +       if (dev->periphid != 0)
> +               goto skip_probe;
> +
>         /*
>          * Dynamically calculate the size of the resource
>          * and use this for iomap
> @@ -643,6 +647,7 @@ int amba_device_register(struct amba_device *dev,
> struct resource *parent)
>         if (ret)
>                 goto err_release;
> 
> + skip_probe:
>         ret = device_add(&dev->dev);
>         if (ret)
>                 goto err_release;
> 
> This way, if we hardcode periphid in the platform, it will take
> precedence. Currently hardware will override such hard-codec
> values.
> 
> There are a few instances using this in the kernel I think, but
> to my memory they are all of the type where the hardcoded
> number and the ID found in hardware are actually identical,
> and eiher make no harm or should be patched away with.

This means we don't pick up on the hardware configuration when platforms
have set the ID, and since we always provide the ID that would be bad.
Linus Walleij March 17, 2011, 8:58 a.m. UTC | #2 
On Thu, Mar 17, 2011 at 9:48 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:

> This means we don't pick up on the hardware configuration when platforms
> have set the ID, and since we always provide the ID that would be bad.

Yes, so I don't put in this one patch, but also take a grep
and check through the kernel tree and try to find and eleminate those, so
that the possibility of hardcoding the ID is only used when it is proper,
i.e. when it should have been something else than what is in the hardware.

Yours,
Linus Walleij
diff mbox

Patch

diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
index 6d2bb25..7f1590f 100644
--- a/drivers/amba/bus.c
+++ b/drivers/amba/bus.c
@@ -29,6 +29,12 @@  amba_lookup(const struct amba_id *table, struct amba_device *dev)
 
 	while (table->mask) {
 		ret = (dev->periphid & table->mask) == table->id;
+		if (ret && (table->name || dev->name)) {
+			if (table->name && dev->name)
+				ret = strcmp(dev->name, table->name) == 0;
+			else
+				ret = 0;
+		}
 		if (ret)
 			break;
 		table++;
diff --git a/include/linux/amba/bus.h b/include/linux/amba/bus.h
index fcbbe71..f5f82b2 100644
--- a/include/linux/amba/bus.h
+++ b/include/linux/amba/bus.h
@@ -32,12 +32,14 @@  struct amba_device {
 	struct regulator	*vcore;
 	u64			dma_mask;
 	unsigned int		periphid;
+	const char		*name;
 	unsigned int		irq[AMBA_NR_IRQS];
 };
 
 struct amba_id {
 	unsigned int		id;
 	unsigned int		mask;
+	const char		*name;
 	void			*data;
 };