Skip to contentSkip to conversation

Balance alloc/free and ioremap/iounmap in gpio_mdio_probe (powerpc/platforms/pasemi/gpio_mdio.c)

PATCH In review Roel Kluin 4 participants 8 messages Nov 4, 2007 – Nov 5, 2007 PowerPC

lore
Share & tools
lore

Roel Kluin

author
#0 lore raw

PATCH 1/1 Balance alloc/free and ioremap/iounmap in gpio_mdio_probe (powerpc/platforms/pasemi/gpio_mdio.c)

I think this is how it should be done, but
please review: it was not tested.
--
Balance alloc/free and ioremap/iounmap

Signed-off-by Roel Kluin <[email protected]>

---

arch/powerpc/platforms/pasemi/gpio_mdio.c +47 -26
index dae9f65..f250ba4 100644
@@ -208,95 +208,116 @@ static int gpio_mdio_write(struct mii_bus *bus, int phy_id, int location, u16 va
 }
 
 static int gpio_mdio_reset(struct mii_bus *bus)
 {
 	/*nothing here - dunno how to reset it*/
 	return 0;
 }
 
-
-static int __devinit gpio_mdio_probe(struct of_device *ofdev,
-				     const struct of_device_id *match)
+static int __devinit __gpio_mdio_register_bus(struct of_device *ofdev,
+					const struct of_device_id *match)
 {
 	struct device *dev = &ofdev->dev;
 	struct device_node *np = ofdev->node;
-	struct device_node *gpio_np;
 	struct mii_bus *new_bus;
 	struct resource res;
 	struct gpio_priv *priv;
 	const unsigned int *prop;
-	int err = 0;
 	int i;
 
-	gpio_np = of_find_compatible_node(NULL, "gpio", "1682m-gpio");
-
-	if (!gpio_np)
-		return -ENODEV;
-
-	err = of_address_to_resource(gpio_np, 0, &res);
-	of_node_put(gpio_np);
-
-	if (err)
-		return -EINVAL;
-
-	if (!gpio_regs)
-		gpio_regs = ioremap(res.start, 0x100);
-
-	if (!gpio_regs)
-		return -EPERM;
-
 	priv = kzalloc(sizeof(struct gpio_priv), GFP_KERNEL);
-	if (priv == NULL)
+	if (unlikely(priv == NULL))
 		return -ENOMEM;
 
 	new_bus = kzalloc(sizeof(struct mii_bus), GFP_KERNEL);
 
-	if (new_bus == NULL)
-		return -ENOMEM;
+	if (unlikely(new_bus == NULL))
+		goto free_priv;
 
 	new_bus->name = "pasemi gpio mdio bus",
 	new_bus->read = &gpio_mdio_read,
 	new_bus->write = &gpio_mdio_write,
 	new_bus->reset = &gpio_mdio_reset,
 
 	prop = of_get_property(np, "reg", NULL);
 	new_bus->id = *prop;
 	new_bus->priv = priv;
 
 	new_bus->phy_mask = 0;
 
 	new_bus->irq = kmalloc(sizeof(int)*PHY_MAX_ADDR, GFP_KERNEL);
+	if (unlikely(new_bus->irq == NULL))
+		goto free_new_bus;
+
 	for(i = 0; i < PHY_MAX_ADDR; ++i)
 		new_bus->irq[i] = irq_create_mapping(NULL, 10);
 
 
 	prop = of_get_property(np, "mdc-pin", NULL);
 	priv->mdc_pin = *prop;
 
 	prop = of_get_property(np, "mdio-pin", NULL);
 	priv->mdio_pin = *prop;
 
 	new_bus->dev = dev;
 	dev_set_drvdata(dev, new_bus);
 
 	err = mdiobus_register(new_bus);
 
-	if (0 != err) {
+	if (unlikely(0 != err)) {
 		printk(KERN_ERR "%s: Cannot register as MDIO bus, err %d\n",
 				new_bus->name, err);
 		goto bus_register_fail;
 	}
 
 	return 0;
 
 bus_register_fail:
+	kfree(new_bus->irq);
+free_new_bus:
 	kfree(new_bus);
+free_priv:
+	kfree(priv);
+
+	return -ENOMEM;
+}
+
+
+static int __devinit gpio_mdio_probe(struct of_device *ofdev,
+				     const struct of_device_id *match)
+{
+	struct device_node *gpio_np;
+	int err;
+
+	gpio_np = of_find_compatible_node(NULL, "gpio", "1682m-gpio");
+
+	if (!gpio_np)
+		return -ENODEV;
+
+	err = of_address_to_resource(gpio_np, 0, &res);
+	of_node_put(gpio_np);
+
+	if (err)
+		return -EINVAL;
+
+	if (!gpio_regs) {
+
+		gpio_regs = ioremap(res.start, 0x100);
+		if (unlikely(!gpio_regs))
+			return -EPERM;
+
+		err = __gpio_mdio_register_bus(ofdev, match);
+		if (err < 0)
+			iounmap(gpio_regs);
+	} else
+		err = __gpio_mdio_register_bus(ofdev, match);
 
 	return err;
+
 }
 
 
 static int gpio_mdio_remove(struct of_device *dev)
 {
 	struct mii_bus *bus = dev_get_drvdata(&dev->dev);
 
 	mdiobus_unregister(bus);
#1 lore raw

Hi,

Quoted 4 lines · Roel Kluin

I think this is how it should be done, but
please review: it was not tested.
--
Balance alloc/free and ioremap/iounmap

It would be more helpful if your changelog identified the objects which could be leaked. More comments below.

Quoted 26 lines
arch/powerpc/platforms/pasemi/gpio_mdio.c +2 -15
-static int __devinit gpio_mdio_probe(struct of_device *ofdev,
-				     const struct of_device_id *match)
+static int __devinit __gpio_mdio_register_bus(struct of_device *ofdev,
+					const struct of_device_id *match)
 {
 	struct device *dev = &ofdev->dev;
 	struct device_node *np = ofdev->node;
-	struct device_node *gpio_np;
 	struct mii_bus *new_bus;
 	struct resource res;
 	struct gpio_priv *priv;
 	const unsigned int *prop;
-	int err = 0;
 	int i;
 
-	gpio_np = of_find_compatible_node(NULL, "gpio", "1682m-gpio");
-
-	if (!gpio_np)
-		return -ENODEV;
-
-	err = of_address_to_resource(gpio_np, 0, &res);
-	of_node_put(gpio_np);
-
-	if (err)
-		return -EINVAL;
-

arch/powerpc/platforms/pasemi/gpio_mdio.c static int gpio_mdio_write(struct mii_bus *bus, int phy_id, int location, u16 va · line 229

Quoted 10 lines
arch/powerpc/platforms/pasemi/gpio_mdio.c +1 -7
-	if (!gpio_regs)
-		gpio_regs = ioremap(res.start, 0x100);
-
-	if (!gpio_regs)
-		return -EPERM;
-
 	priv = kzalloc(sizeof(struct gpio_priv), GFP_KERNEL);
-	if (priv == NULL)
+	if (unlikely(priv == NULL))
 		return -ENOMEM;

Please don't add likely/unlikely to non-hot paths such as this.

arch/powerpc/platforms/pasemi/gpio_mdio.c static int gpio_mdio_write(struct mii_bus *bus, int phy_id, int location, u16 va · line 234

Quoted 6 lines
arch/powerpc/platforms/pasemi/gpio_mdio.c +2 -2
 	new_bus = kzalloc(sizeof(struct mii_bus), GFP_KERNEL);
 
-	if (new_bus == NULL)
-		return -ENOMEM;
+	if (unlikely(new_bus == NULL))
+		goto free_priv;

again

Quoted 6 lines
arch/powerpc/platforms/pasemi/gpio_mdio.c +0 -0
 	new_bus->name = "pasemi gpio mdio bus",
 	new_bus->read = &gpio_mdio_read,
 	new_bus->write = &gpio_mdio_write,
 	new_bus->reset = &gpio_mdio_reset,

arch/powerpc/platforms/pasemi/gpio_mdio.c static int gpio_mdio_write(struct mii_bus *bus, int phy_id, int location, u16 va · line 250

Quoted 10 lines
arch/powerpc/platforms/pasemi/gpio_mdio.c +3 -0
 	prop = of_get_property(np, "reg", NULL);
 	new_bus->id = *prop;
 	new_bus->priv = priv;
 
 	new_bus->phy_mask = 0;
 
 	new_bus->irq = kmalloc(sizeof(int)*PHY_MAX_ADDR, GFP_KERNEL);
+	if (unlikely(new_bus->irq == NULL))
+		goto free_new_bus;
+

again

Quoted 7 lines
arch/powerpc/platforms/pasemi/gpio_mdio.c +0 -0
 	for(i = 0; i < PHY_MAX_ADDR; ++i)
 		new_bus->irq[i] = irq_create_mapping(NULL, 10);
 
 
 	prop = of_get_property(np, "mdc-pin", NULL);
 	priv->mdc_pin = *prop;

arch/powerpc/platforms/pasemi/gpio_mdio.c static int gpio_mdio_write(struct mii_bus *bus, int phy_id, int location, u16 va · line 266

Quoted 10 lines
arch/powerpc/platforms/pasemi/gpio_mdio.c +1 -1
 	prop = of_get_property(np, "mdio-pin", NULL);
 	priv->mdio_pin = *prop;
 
 	new_bus->dev = dev;
 	dev_set_drvdata(dev, new_bus);
 
 	err = mdiobus_register(new_bus);
 
-	if (0 != err) {
+	if (unlikely(0 != err)) {

again

Quoted 20 lines
arch/powerpc/platforms/pasemi/gpio_mdio.c +11 -0
 		printk(KERN_ERR "%s: Cannot register as MDIO bus, err %d\n",
 				new_bus->name, err);
 		goto bus_register_fail;
 	}
 
 	return 0;
 
 bus_register_fail:
+	kfree(new_bus->irq);
+free_new_bus:
 	kfree(new_bus);
+free_priv:
+	kfree(priv);
+
+	return -ENOMEM;
+}
+
+
+static int __devinit gpio_mdio_probe(struct of_device *ofdev,
+				     const struct of_device_id *match)

arch/powerpc/platforms/pasemi/gpio_mdio.c static int gpio_mdio_write(struct mii_bus *bus, int phy_id, int location, u16 va · line 296

Quoted 10 lines
arch/powerpc/platforms/pasemi/gpio_mdio.c +10 -0
+{
+	struct device_node *gpio_np;
+	int err;
+
+	gpio_np = of_find_compatible_node(NULL, "gpio", "1682m-gpio");
+
+	if (!gpio_np)
+		return -ENODEV;
+
+	err = of_address_to_resource(gpio_np, 0, &res);

Hmm, where is "res" defined?

arch/powerpc/platforms/pasemi/gpio_mdio.c static int gpio_mdio_write(struct mii_bus *bus, int phy_id, int location, u16 va · line 303

Quoted 7 lines
arch/powerpc/platforms/pasemi/gpio_mdio.c +7 -0
+	of_node_put(gpio_np);
+
+	if (err)
+		return -EINVAL;
+
+	if (!gpio_regs) {
+

Unneeded newline

arch/powerpc/platforms/pasemi/gpio_mdio.c static int gpio_mdio_write(struct mii_bus *bus, int phy_id, int location, u16 va · line 315

Quoted 12 lines
arch/powerpc/platforms/pasemi/gpio_mdio.c +10 -0
+		gpio_regs = ioremap(res.start, 0x100);
+		if (unlikely(!gpio_regs))
+			return -EPERM;
+
+		err = __gpio_mdio_register_bus(ofdev, match);
+		if (err < 0)
+			iounmap(gpio_regs);
+	} else
+		err = __gpio_mdio_register_bus(ofdev, match);
 
 	return err;
+

again

Quoted 9 lines

}

static int gpio_mdio_remove(struct of_device *dev) {

arch/powerpc/platforms/pasemi/gpio_mdio.c +0 -0
 	struct mii_bus *bus = dev_get_drvdata(&dev->dev);
 
 	mdiobus_unregister(bus);
author
#2 lore raw
Quoted 1 line · Nathan Lynch

Hi,

I moved res to the wrong function, that's fixed as well as the unlikely's and the extra new lines.

Thanks for your quick response. Here is an updated version:
--
Upon errors gpio_regs was not iounmapped, and later priv nor new_bus->irq was freed. Testing succes of the allocation of new_bus->irq was missing as well.

This patch creates a new function __gpio_mdio_register_bus to allow the iounmapping after an ioremap.

Signed-off-by Roel Kluin <[email protected]>

---

arch/powerpc/platforms/pasemi/gpio_mdio.c +43 -24
index dae9f65..af5b241 100644
@@ -208,68 +208,50 @@ static int gpio_mdio_write(struct mii_bus *bus, int phy_id, int location, u16 va
 }
 
 static int gpio_mdio_reset(struct mii_bus *bus)
 {
 	/*nothing here - dunno how to reset it*/
 	return 0;
 }
 
-
-static int __devinit gpio_mdio_probe(struct of_device *ofdev,
-				     const struct of_device_id *match)
+static int __devinit __gpio_mdio_register_bus(struct of_device *ofdev,
+					const struct of_device_id *match)
 {
 	struct device *dev = &ofdev->dev;
 	struct device_node *np = ofdev->node;
-	struct device_node *gpio_np;
 	struct mii_bus *new_bus;
-	struct resource res;
 	struct gpio_priv *priv;
 	const unsigned int *prop;
-	int err = 0;
 	int i;
 
-	gpio_np = of_find_compatible_node(NULL, "gpio", "1682m-gpio");
-
-	if (!gpio_np)
-		return -ENODEV;
-
-	err = of_address_to_resource(gpio_np, 0, &res);
-	of_node_put(gpio_np);
-
-	if (err)
-		return -EINVAL;
-
-	if (!gpio_regs)
-		gpio_regs = ioremap(res.start, 0x100);
-
-	if (!gpio_regs)
-		return -EPERM;
-
 	priv = kzalloc(sizeof(struct gpio_priv), GFP_KERNEL);
 	if (priv == NULL)
 		return -ENOMEM;
 
 	new_bus = kzalloc(sizeof(struct mii_bus), GFP_KERNEL);
 
 	if (new_bus == NULL)
-		return -ENOMEM;
+		goto free_priv;
 
 	new_bus->name = "pasemi gpio mdio bus",
 	new_bus->read = &gpio_mdio_read,
 	new_bus->write = &gpio_mdio_write,
 	new_bus->reset = &gpio_mdio_reset,
 
 	prop = of_get_property(np, "reg", NULL);
 	new_bus->id = *prop;
 	new_bus->priv = priv;
 
 	new_bus->phy_mask = 0;
 
 	new_bus->irq = kmalloc(sizeof(int)*PHY_MAX_ADDR, GFP_KERNEL);
+	if (new_bus->irq == NULL)
+		goto free_new_bus;
+
 	for(i = 0; i < PHY_MAX_ADDR; ++i)
 		new_bus->irq[i] = irq_create_mapping(NULL, 10);
 
 
 	prop = of_get_property(np, "mdc-pin", NULL);
 	priv->mdc_pin = *prop;
 
 	prop = of_get_property(np, "mdio-pin", NULL);

@@ -284,17 +266,54 @@ static int __devinit gpio_mdio_probe(struct of_device *ofdev,
 		printk(KERN_ERR "%s: Cannot register as MDIO bus, err %d\n",
 				new_bus->name, err);
 		goto bus_register_fail;
 	}
 
 	return 0;
 
 bus_register_fail:
+	kfree(new_bus->irq);
+free_new_bus:
 	kfree(new_bus);
+free_priv:
+	kfree(priv);
+
+	return -ENOMEM;
+}
+
+
+static int __devinit gpio_mdio_probe(struct of_device *ofdev,
+				     const struct of_device_id *match)
+{
+	struct device_node *gpio_np;
+	struct resource res;
+	int err;
+
+	gpio_np = of_find_compatible_node(NULL, "gpio", "1682m-gpio");
+
+	if (!gpio_np)
+		return -ENODEV;
+
+	err = of_address_to_resource(gpio_np, 0, &res);
+	of_node_put(gpio_np);
+
+	if (err)
+		return -EINVAL;
+
+	if (!gpio_regs) {
+		gpio_regs = ioremap(res.start, 0x100);
+		if (unlikely(!gpio_regs))
+			return -EPERM;
+
+		err = __gpio_mdio_register_bus(ofdev, match);
+		if (err < 0)
+			iounmap(gpio_regs);
+	} else
+		err = __gpio_mdio_register_bus(ofdev, match);
 
 	return err;
 }
 
 
 static int gpio_mdio_remove(struct of_device *dev)
 {
 	struct mii_bus *bus = dev_get_drvdata(&dev->dev);
#3 lore raw
In reply to Roel Kluin
Quoted 2 lines · Roel Kluin

I think this is how it should be done, but
please review: it was not tested.

Hi,

Thanks for the bug report. The mdio driver needs a set of other cleanups as well. I have a more comprehensive patch that I'll post tomorrow after I have a chance to test them.

-Olof

author
#4 lore raw
Quoted 9 lines · Olof Johansson
Quoted 2 lines · Roel Kluin

I think this is how it should be done, but
please review: it was not tested.

Hi,

Thanks for the bug report. The mdio driver needs a set of other cleanups as well. I have a more comprehensive patch that I'll post tomorrow after I have a chance to test them.

Ok, that's fine with me,

Roel

#5 lore raw

PATCH 1/1 pasemi: clean up gpio_mdio init

pasemi: clean up mdio_gpio driver init

Misc cleanups of mdio_gpio:
* Better error handling/unrolling in case of init/alloc failures
* Go through child nodes and get their interrupts instead of using hardcoded values
* Remap the GPIO registers at module load/driver init instead of during probe
* Coding style and other misc cleanups

Signed-off-by Olof Johansson <[email protected]>

---

Quoted 5 lines · Roel Kluin
Quoted 4 lines · Olof Johansson
Quoted 2 lines · Roel Kluin

I think this is how it should be done, but
please review: it was not tested.

Quoted 7 lines
Quoted 5 lines

Hi,

Thanks for the bug report. The mdio driver needs a set of other cleanups as well. I have a more comprehensive patch that I'll post tomorrow after I have a chance to test them.

Ok, that's fine with me,

This is what I'll queue up for 2.6.25.

-Olof

arch/powerpc/platforms/pasemi/gpio_mdio.c +49 -37
index dae9f65..2254091 100644
@@ -218,45 +218,27 @@ static int __devinit gpio_mdio_probe(struct of_device *ofdev,
 				     const struct of_device_id *match)
 {
 	struct device *dev = &ofdev->dev;
-	struct device_node *np = ofdev->node;
-	struct device_node *gpio_np;
+	struct device_node *phy_dn, *np = ofdev->node;
 	struct mii_bus *new_bus;
-	struct resource res;
 	struct gpio_priv *priv;
 	const unsigned int *prop;
-	int err = 0;
+	int err;
 	int i;
 
-	gpio_np = of_find_compatible_node(NULL, "gpio", "1682m-gpio");
-
-	if (!gpio_np)
-		return -ENODEV;
-
-	err = of_address_to_resource(gpio_np, 0, &res);
-	of_node_put(gpio_np);
-
-	if (err)
-		return -EINVAL;
-
-	if (!gpio_regs)
-		gpio_regs = ioremap(res.start, 0x100);
-
-	if (!gpio_regs)
-		return -EPERM;
-
+	err = -ENOMEM;
 	priv = kzalloc(sizeof(struct gpio_priv), GFP_KERNEL);
-	if (priv == NULL)
-		return -ENOMEM;
+	if (!priv)
+		goto out;
 
 	new_bus = kzalloc(sizeof(struct mii_bus), GFP_KERNEL);
 
-	if (new_bus == NULL)
-		return -ENOMEM;
+	if (!new_bus)
+		goto out_free_priv;
 
-	new_bus->name = "pasemi gpio mdio bus",
-	new_bus->read = &gpio_mdio_read,
-	new_bus->write = &gpio_mdio_write,
-	new_bus->reset = &gpio_mdio_reset,
+	new_bus->name = "pasemi gpio mdio bus";
+	new_bus->read = &gpio_mdio_read;
+	new_bus->write = &gpio_mdio_write;
+	new_bus->reset = &gpio_mdio_reset;
 
 	prop = of_get_property(np, "reg", NULL);
 	new_bus->id = *prop;

@@ -265,9 +247,24 @@ static int __devinit gpio_mdio_probe(struct of_device *ofdev,
 	new_bus->phy_mask = 0;
 
 	new_bus->irq = kmalloc(sizeof(int)*PHY_MAX_ADDR, GFP_KERNEL);
-	for(i = 0; i < PHY_MAX_ADDR; ++i)
-		new_bus->irq[i] = irq_create_mapping(NULL, 10);
 
+	if (!new_bus->irq)
+		goto out_free_bus;
+
+	for (i = 0; i < PHY_MAX_ADDR; i++)
+		new_bus->irq[i] = NO_IRQ;
+
+	for (phy_dn = of_get_next_child(np, NULL);
+	     phy_dn != NULL;
+	     phy_dn = of_get_next_child(np, phy_dn)) {
+		const int *ip, *regp;
+
+		ip = of_get_property(phy_dn, "interrupts", NULL);
+		regp = of_get_property(phy_dn, "reg", NULL);
+		if (!ip || !regp)
+			continue;
+		new_bus->irq[*regp] = irq_create_mapping(NULL, *ip);
+	}
 
 	prop = of_get_property(np, "mdc-pin", NULL);
 	priv->mdc_pin = *prop;

@@ -280,17 +277,21 @@ static int __devinit gpio_mdio_probe(struct of_device *ofdev,
 
 	err = mdiobus_register(new_bus);
 
-	if (0 != err) {
+	if (err != 0) {
 		printk(KERN_ERR "%s: Cannot register as MDIO bus, err %d\n",
 				new_bus->name, err);
-		goto bus_register_fail;
+		goto out_free_irq;
 	}
 
 	return 0;
 
-bus_register_fail:
+out_free_irq:
+	kfree(new_bus->irq);
+out_free_bus:
 	kfree(new_bus);
-
+out_free_priv:
+	kfree(priv);
+out:
 	return err;
 }
 

@@ -330,12 +331,23 @@ static struct of_platform_driver gpio_mdio_driver =
 
 int gpio_mdio_init(void)
 {
+	struct device_node *np;
+
+	np = of_find_compatible_node(NULL, "gpio", "1682m-gpio");
+	if (!np)
+		return -ENODEV;
+	gpio_regs = of_iomap(np, 0);
+	if (!gpio_regs)
+		return -ENODEV;
+
 	return of_register_platform_driver(&gpio_mdio_driver);
 }
+module_init(gpio_mdio_init);
 
 void gpio_mdio_exit(void)
 {
 	of_unregister_platform_driver(&gpio_mdio_driver);
+	if (gpio_regs)
+		iounmap(gpio_regs);
 }
-device_initcall(gpio_mdio_init);
-
+module_exit(gpio_mdio_exit);
#6 lore raw

Re: [PATCH] pasemi: clean up gpio_mdio init

Hi Olof,

Just a couple of small things ...

arch/powerpc/platforms/pasemi/gpio_mdio.c static int __devinit gpio_mdio_probe(struct of_device *ofdev, · line 266

Quoted 6 lines · On Sun, 4 Nov 2007 15:37:09 -0600 Olof Johansson <[email protected]> wrote:
arch/powerpc/platforms/pasemi/gpio_mdio.c +5 -0
+		ip = of_get_property(phy_dn, "interrupts", NULL);
+		regp = of_get_property(phy_dn, "reg", NULL);
+		if (!ip || !regp)
+			continue;
+		new_bus->irq[*regp] = irq_create_mapping(NULL, *ip);

Paranoid me says to check that *regp is in range.

arch/powerpc/platforms/pasemi/gpio_mdio.c static struct of_platform_driver gpio_mdio_driver = · line 339

Quoted 8 lines

int gpio_mdio_init(void)
{

arch/powerpc/platforms/pasemi/gpio_mdio.c +6 -0
+	struct device_node *np;
+
+	np = of_find_compatible_node(NULL, "gpio", "1682m-gpio");
+	if (!np)
+		return -ENODEV;
+	gpio_regs = of_iomap(np, 0);

	of_node_put(np);		?
Quoted 5 lines
arch/powerpc/platforms/pasemi/gpio_mdio.c +3 -0
+	if (!gpio_regs)
+		return -ENODEV;
+
 	return of_register_platform_driver(&gpio_mdio_driver);
 }
Signature · 3 lines
Cheers,
Stephen Rothwell                    [email protected]
http://www.canb.auug.org.au/~sfr/
#7 lore raw

Re: [PATCH] pasemi: clean up gpio_mdio init

Hi,

Quoted 1 line · Stephen Rothwell

Just a couple of small things ...

Thanks! Unless someone finds something bigger, I'll update this before I push it out through git, but I won't repost.

Quoted 9 lines · On Sun, 4 Nov 2007 15:37:09 -0600 Olof Johansson <[email protected]> wrote:

arch/powerpc/platforms/pasemi/gpio_mdio.c static int __devinit gpio_mdio_probe(struct of_device *ofdev, · line 266

Quoted 6 lines
arch/powerpc/platforms/pasemi/gpio_mdio.c +5 -0
+		ip = of_get_property(phy_dn, "interrupts", NULL);
+		regp = of_get_property(phy_dn, "reg", NULL);
+		if (!ip || !regp)
+			continue;
+		new_bus->irq[*regp] = irq_create_mapping(NULL, *ip);

Paranoid me says to check that *regp is in range.

Yeah, good point.

Quoted 11 lines

arch/powerpc/platforms/pasemi/gpio_mdio.c static struct of_platform_driver gpio_mdio_driver = · line 339

Quoted 9 lines

int gpio_mdio_init(void)
{

arch/powerpc/platforms/pasemi/gpio_mdio.c +6 -0
+	struct device_node *np;
+
+	np = of_find_compatible_node(NULL, "gpio", "1682m-gpio");
+	if (!np)
+		return -ENODEV;
+	gpio_regs = of_iomap(np, 0);

	of_node_put(np);		?

Yup.

Thanks for the review,

-Olof