Re: [PATCH 2/2] - SH/Dreamcast - fix maple bus bugs

2008-02-06 Thread Greg KH
On Wed, Feb 06, 2008 at 11:51:21PM +, Adrian McMenamin wrote:
> Replacement second-in-series patch:
> 
> This patch fixes up memory leaks and, by delaying initialisation, makes
> device detection more robust.
> 
> It also makes clearer the difference between struct maple_device and
> struct device, as well as cleaning up the interrupt request code
> (without changing its function in any way).
> 
> Also now removes redundant registration checking.

Thanks for changing this.

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/2] - SH/Dreamcast - fix maple bus bugs

2008-02-06 Thread Adrian McMenamin
Replacement second-in-series patch:

This patch fixes up memory leaks and, by delaying initialisation, makes
device detection more robust.

It also makes clearer the difference between struct maple_device and
struct device, as well as cleaning up the interrupt request code
(without changing its function in any way).

Also now removes redundant registration checking.

Signed-off by: Adrian McMenamin <[EMAIL PROTECTED]>



diff --git a/drivers/sh/maple/maple.c b/drivers/sh/maple/maple.c
index 3f341dc..fbca7f8 100644
--- a/drivers/sh/maple/maple.c
+++ b/drivers/sh/maple/maple.c
@@ -30,6 +30,7 @@
 #include 
 #include 
 #include 
+#include 
 
 MODULE_AUTHOR("Yaegshi Takeshi, Paul Mundt, M.R. Brown, Adrian McMenamin");
 MODULE_DESCRIPTION("Maple bus driver for Dreamcast");
@@ -52,7 +53,7 @@ static struct device maple_bus;
 static int subdevice_map[MAPLE_PORTS];
 static unsigned long *maple_sendbuf, *maple_sendptr, *maple_lastptr;
 static unsigned long maple_pnp_time;
-static int started, scanning, liststatus;
+static int started, scanning, liststatus, realscan;
 static struct kmem_cache *maple_queue_cache;
 
 struct maple_device_specify {
@@ -72,7 +73,6 @@ int maple_driver_register(struct device_driver *drv)
drv->bus = _bus_type;
return driver_register(drv);
 }
-
 EXPORT_SYMBOL_GPL(maple_driver_register);
 
 /* set hardware registers to enable next round of dma */
@@ -94,15 +94,14 @@ static void maplebus_dma_reset(void)
  * @function: the function code for the device
  */
 void maple_getcond_callback(struct maple_device *dev,
-   void (*callback) (struct mapleq * mq),
-   unsigned long interval, unsigned long function)
+   void (*callback) (struct mapleq *mq),
+   unsigned long interval, unsigned long function)
 {
dev->callback = callback;
dev->interval = interval;
dev->function = cpu_to_be32(function);
dev->when = jiffies;
 }
-
 EXPORT_SYMBOL_GPL(maple_getcond_callback);
 
 static int maple_dma_done(void)
@@ -112,10 +111,19 @@ static int maple_dma_done(void)
 
 static void maple_release_device(struct device *dev)
 {
-   if (dev->type) {
-   kfree(dev->type->name);
-   kfree(dev->type);
+   struct maple_device *mdev;
+   struct mapleq *mq;
+   if (!dev)
+   return;
+   mdev = to_maple_dev(dev);
+   mq = mdev->mq;
+   if (mq) {
+   if (mq->recvbufdcsp)
+   kmem_cache_free(maple_queue_cache, mq->recvbufdcsp);
+   kfree(mq);
+   mq = NULL;
}
+   kfree(mdev);
 }
 
 /**
@@ -128,10 +136,9 @@ void maple_add_packet(struct mapleq *mq)
list_add(>list, _waitq);
mutex_unlock(_list_lock);
 }
-
 EXPORT_SYMBOL_GPL(maple_add_packet);
 
-static struct mapleq *maple_allocq(struct maple_device *dev)
+static struct mapleq *maple_allocq(struct maple_device *mdev)
 {
struct mapleq *mq;
 
@@ -139,7 +146,7 @@ static struct mapleq *maple_allocq(struct maple_device *dev)
if (!mq)
return NULL;
 
-   mq->dev = dev;
+   mq->dev = mdev;
mq->recvbufdcsp = kmem_cache_zalloc(maple_queue_cache, GFP_KERNEL);
mq->recvbuf = (void *) P2SEGADDR(mq->recvbufdcsp);
if (!mq->recvbuf) {
@@ -152,22 +159,24 @@ static struct mapleq *maple_allocq(struct maple_device 
*dev)
 
 static struct maple_device *maple_alloc_dev(int port, int unit)
 {
-   struct maple_device *dev;
+   struct maple_device *mdev;
 
-   dev = kzalloc(sizeof(*dev), GFP_KERNEL);
-   if (!dev)
+   mdev = kzalloc(sizeof(*mdev), GFP_KERNEL);
+   if (!mdev)
return NULL;
 
-   dev->port = port;
-   dev->unit = unit;
-   dev->mq = maple_allocq(dev);
+   mdev->port = port;
+   mdev->unit = unit;
+   mdev->mq = maple_allocq(mdev);
 
-   if (!dev->mq) {
-   kfree(dev);
+   if (!mdev->mq) {
+   kfree(mdev);
return NULL;
}
-
-   return dev;
+   mdev->dev.bus = _bus_type;
+   mdev->dev.parent = _bus;
+   mdev->function = 0;
+   return mdev;
 }
 
 static void maple_free_dev(struct maple_device *mdev)
@@ -175,7 +184,9 @@ static void maple_free_dev(struct maple_device *mdev)
if (!mdev)
return;
if (mdev->mq) {
-   kmem_cache_free(maple_queue_cache, mdev->mq->recvbufdcsp);
+   if (mdev->mq->recvbufdcsp)
+   kmem_cache_free(maple_queue_cache,
+   mdev->mq->recvbufdcsp);
kfree(mdev->mq);
}
kfree(mdev);
@@ -259,80 +270,89 @@ static void maple_detach_driver(struct maple_device *mdev)
mdev->driver->disconnect(mdev);
}
mdev->driver = NULL;
-   if (mdev->registered) {
-   maple_release_device(>dev);
-   device_unregister(>dev);
-  

Re: [PATCH 2/2] - SH/Dreamcast - fix maple bus bugs

2008-02-06 Thread Greg KH
On Wed, Feb 06, 2008 at 11:05:14PM +, Adrian McMenamin wrote:
> 
> On Wed, 2008-02-06 at 15:01 -0800, Greg KH wrote:
> > On Wed, Feb 06, 2008 at 10:53:51PM +, Adrian McMenamin wrote:
> > > - dev->function = function;
> > > - dev->dev.bus = _bus_type;
> > > - dev->dev.parent = _bus;
> > > - dev->dev.release = _release_device;
> > > - retval = device_register(>dev);
> > > - if (retval) {
> > > - printk(KERN_INFO
> > > -"Maple bus: Attempt to register device (%x, %x) 
> > > failed.\n",
> > > -dev->port, dev->unit);
> > > - maple_free_dev(dev);
> > > + mdev->function = function;
> > > + mdev->dev.release = _release_device;
> > > + if (mdev->registered == 0) {
> > > + retval = device_register(>dev);
> > > + if (retval) {
> > > + printk(KERN_INFO
> > > + "Maple bus: Attempt to register device"
> > > + " (%x, %x) failed.\n",
> > > + mdev->port, mdev->unit);
> > > + maple_free_dev(mdev);
> > > + mdev = NULL;
> > > + return;
> > > + }
> > > + mdev->registered = 1;
> > >   }
> > > - dev->registered = 1;
> > 
> > I think you are still papering over the real problem here of trying to
> > register a device twice.  That's something you should never be doing,
> > and your bus walking logic must be incorrect...
> > 
> I just left that in as a sanity check. It should never be acted on. I
> can remove it if you prefer.

Please do, that way, and real problems will show up with that warning
you were getting that started all of this work :)

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/2] - SH/Dreamcast - fix maple bus bugs

2008-02-06 Thread Adrian McMenamin

On Wed, 2008-02-06 at 15:01 -0800, Greg KH wrote:
> On Wed, Feb 06, 2008 at 10:53:51PM +, Adrian McMenamin wrote:
> > -   dev->function = function;
> > -   dev->dev.bus = _bus_type;
> > -   dev->dev.parent = _bus;
> > -   dev->dev.release = _release_device;
> > -   retval = device_register(>dev);
> > -   if (retval) {
> > -   printk(KERN_INFO
> > -  "Maple bus: Attempt to register device (%x, %x) 
> > failed.\n",
> > -  dev->port, dev->unit);
> > -   maple_free_dev(dev);
> > +   mdev->function = function;
> > +   mdev->dev.release = _release_device;
> > +   if (mdev->registered == 0) {
> > +   retval = device_register(>dev);
> > +   if (retval) {
> > +   printk(KERN_INFO
> > +   "Maple bus: Attempt to register device"
> > +   " (%x, %x) failed.\n",
> > +   mdev->port, mdev->unit);
> > +   maple_free_dev(mdev);
> > +   mdev = NULL;
> > +   return;
> > +   }
> > +   mdev->registered = 1;
> > }
> > -   dev->registered = 1;
> 
> I think you are still papering over the real problem here of trying to
> register a device twice.  That's something you should never be doing,
> and your bus walking logic must be incorrect...
> 
I just left that in as a sanity check. It should never be acted on. I
can remove it if you prefer.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/2] - SH/Dreamcast - fix maple bus bugs

2008-02-06 Thread Greg KH
On Wed, Feb 06, 2008 at 10:53:51PM +, Adrian McMenamin wrote:
> - dev->function = function;
> - dev->dev.bus = _bus_type;
> - dev->dev.parent = _bus;
> - dev->dev.release = _release_device;
> - retval = device_register(>dev);
> - if (retval) {
> - printk(KERN_INFO
> -"Maple bus: Attempt to register device (%x, %x) 
> failed.\n",
> -dev->port, dev->unit);
> - maple_free_dev(dev);
> + mdev->function = function;
> + mdev->dev.release = _release_device;
> + if (mdev->registered == 0) {
> + retval = device_register(>dev);
> + if (retval) {
> + printk(KERN_INFO
> + "Maple bus: Attempt to register device"
> + " (%x, %x) failed.\n",
> + mdev->port, mdev->unit);
> + maple_free_dev(mdev);
> + mdev = NULL;
> + return;
> + }
> + mdev->registered = 1;
>   }
> - dev->registered = 1;

I think you are still papering over the real problem here of trying to
register a device twice.  That's something you should never be doing,
and your bus walking logic must be incorrect...

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 2/2] - SH/Dreamcast - fix maple bus bugs

2008-02-06 Thread Adrian McMenamin
This patch fixes up memory leaks and, by delaying initialisation, makes
device detection more robust.

It also makes clearer the difference between struct maple_device and
struct device, as well as cleaning up the interrupt request code
(without changing its function in any way).

Signed-off by: Adrian McMenamin <[EMAIL PROTECTED]>


diff --git a/drivers/sh/maple/maple.c b/drivers/sh/maple/maple.c
index 3f341dc..60eeb92 100644
--- a/drivers/sh/maple/maple.c
+++ b/drivers/sh/maple/maple.c
@@ -30,6 +30,7 @@
 #include 
 #include 
 #include 
+#include 
 
 MODULE_AUTHOR("Yaegshi Takeshi, Paul Mundt, M.R. Brown, Adrian McMenamin");
 MODULE_DESCRIPTION("Maple bus driver for Dreamcast");
@@ -52,7 +53,7 @@ static struct device maple_bus;
 static int subdevice_map[MAPLE_PORTS];
 static unsigned long *maple_sendbuf, *maple_sendptr, *maple_lastptr;
 static unsigned long maple_pnp_time;
-static int started, scanning, liststatus;
+static int started, scanning, liststatus, realscan;
 static struct kmem_cache *maple_queue_cache;
 
 struct maple_device_specify {
@@ -72,7 +73,6 @@ int maple_driver_register(struct device_driver *drv)
drv->bus = _bus_type;
return driver_register(drv);
 }
-
 EXPORT_SYMBOL_GPL(maple_driver_register);
 
 /* set hardware registers to enable next round of dma */
@@ -94,15 +94,14 @@ static void maplebus_dma_reset(void)
  * @function: the function code for the device
  */
 void maple_getcond_callback(struct maple_device *dev,
-   void (*callback) (struct mapleq * mq),
-   unsigned long interval, unsigned long function)
+   void (*callback) (struct mapleq *mq),
+   unsigned long interval, unsigned long function)
 {
dev->callback = callback;
dev->interval = interval;
dev->function = cpu_to_be32(function);
dev->when = jiffies;
 }
-
 EXPORT_SYMBOL_GPL(maple_getcond_callback);
 
 static int maple_dma_done(void)
@@ -112,10 +111,19 @@ static int maple_dma_done(void)
 
 static void maple_release_device(struct device *dev)
 {
-   if (dev->type) {
-   kfree(dev->type->name);
-   kfree(dev->type);
+   struct maple_device *mdev;
+   struct mapleq *mq;
+   if (!dev)
+   return;
+   mdev = to_maple_dev(dev);
+   mq = mdev->mq;
+   if (mq) {
+   if (mq->recvbufdcsp)
+   kmem_cache_free(maple_queue_cache, mq->recvbufdcsp);
+   kfree(mq);
+   mq = NULL;
}
+   kfree(mdev);
 }
 
 /**
@@ -128,10 +136,9 @@ void maple_add_packet(struct mapleq *mq)
list_add(>list, _waitq);
mutex_unlock(_list_lock);
 }
-
 EXPORT_SYMBOL_GPL(maple_add_packet);
 
-static struct mapleq *maple_allocq(struct maple_device *dev)
+static struct mapleq *maple_allocq(struct maple_device *mdev)
 {
struct mapleq *mq;
 
@@ -139,7 +146,7 @@ static struct mapleq *maple_allocq(struct maple_device *dev)
if (!mq)
return NULL;
 
-   mq->dev = dev;
+   mq->dev = mdev;
mq->recvbufdcsp = kmem_cache_zalloc(maple_queue_cache, GFP_KERNEL);
mq->recvbuf = (void *) P2SEGADDR(mq->recvbufdcsp);
if (!mq->recvbuf) {
@@ -152,22 +159,24 @@ static struct mapleq *maple_allocq(struct maple_device 
*dev)
 
 static struct maple_device *maple_alloc_dev(int port, int unit)
 {
-   struct maple_device *dev;
+   struct maple_device *mdev;
 
-   dev = kzalloc(sizeof(*dev), GFP_KERNEL);
-   if (!dev)
+   mdev = kzalloc(sizeof(*mdev), GFP_KERNEL);
+   if (!mdev)
return NULL;
 
-   dev->port = port;
-   dev->unit = unit;
-   dev->mq = maple_allocq(dev);
+   mdev->port = port;
+   mdev->unit = unit;
+   mdev->mq = maple_allocq(mdev);
 
-   if (!dev->mq) {
-   kfree(dev);
+   if (!mdev->mq) {
+   kfree(mdev);
return NULL;
}
-
-   return dev;
+   mdev->dev.bus = _bus_type;
+   mdev->dev.parent = _bus;
+   mdev->function = 0;
+   return mdev;
 }
 
 static void maple_free_dev(struct maple_device *mdev)
@@ -175,7 +184,9 @@ static void maple_free_dev(struct maple_device *mdev)
if (!mdev)
return;
if (mdev->mq) {
-   kmem_cache_free(maple_queue_cache, mdev->mq->recvbufdcsp);
+   if (mdev->mq->recvbufdcsp)
+   kmem_cache_free(maple_queue_cache,
+   mdev->mq->recvbufdcsp);
kfree(mdev->mq);
}
kfree(mdev);
@@ -259,80 +270,93 @@ static void maple_detach_driver(struct maple_device *mdev)
mdev->driver->disconnect(mdev);
}
mdev->driver = NULL;
-   if (mdev->registered) {
-   maple_release_device(>dev);
+   if (mdev->registered)
device_unregister(>dev);
-   }
-   mdev->registered = 0;
-   

[PATCH 2/2] - SH/Dreamcast - fix maple bus bugs

2008-02-06 Thread Adrian McMenamin
This patch fixes up memory leaks and, by delaying initialisation, makes
device detection more robust.

It also makes clearer the difference between struct maple_device and
struct device, as well as cleaning up the interrupt request code
(without changing its function in any way).

Signed-off by: Adrian McMenamin [EMAIL PROTECTED]


diff --git a/drivers/sh/maple/maple.c b/drivers/sh/maple/maple.c
index 3f341dc..60eeb92 100644
--- a/drivers/sh/maple/maple.c
+++ b/drivers/sh/maple/maple.c
@@ -30,6 +30,7 @@
 #include asm/mach/dma.h
 #include asm/mach/sysasic.h
 #include asm/mach/maple.h
+#include linux/delay.h
 
 MODULE_AUTHOR(Yaegshi Takeshi, Paul Mundt, M.R. Brown, Adrian McMenamin);
 MODULE_DESCRIPTION(Maple bus driver for Dreamcast);
@@ -52,7 +53,7 @@ static struct device maple_bus;
 static int subdevice_map[MAPLE_PORTS];
 static unsigned long *maple_sendbuf, *maple_sendptr, *maple_lastptr;
 static unsigned long maple_pnp_time;
-static int started, scanning, liststatus;
+static int started, scanning, liststatus, realscan;
 static struct kmem_cache *maple_queue_cache;
 
 struct maple_device_specify {
@@ -72,7 +73,6 @@ int maple_driver_register(struct device_driver *drv)
drv-bus = maple_bus_type;
return driver_register(drv);
 }
-
 EXPORT_SYMBOL_GPL(maple_driver_register);
 
 /* set hardware registers to enable next round of dma */
@@ -94,15 +94,14 @@ static void maplebus_dma_reset(void)
  * @function: the function code for the device
  */
 void maple_getcond_callback(struct maple_device *dev,
-   void (*callback) (struct mapleq * mq),
-   unsigned long interval, unsigned long function)
+   void (*callback) (struct mapleq *mq),
+   unsigned long interval, unsigned long function)
 {
dev-callback = callback;
dev-interval = interval;
dev-function = cpu_to_be32(function);
dev-when = jiffies;
 }
-
 EXPORT_SYMBOL_GPL(maple_getcond_callback);
 
 static int maple_dma_done(void)
@@ -112,10 +111,19 @@ static int maple_dma_done(void)
 
 static void maple_release_device(struct device *dev)
 {
-   if (dev-type) {
-   kfree(dev-type-name);
-   kfree(dev-type);
+   struct maple_device *mdev;
+   struct mapleq *mq;
+   if (!dev)
+   return;
+   mdev = to_maple_dev(dev);
+   mq = mdev-mq;
+   if (mq) {
+   if (mq-recvbufdcsp)
+   kmem_cache_free(maple_queue_cache, mq-recvbufdcsp);
+   kfree(mq);
+   mq = NULL;
}
+   kfree(mdev);
 }
 
 /**
@@ -128,10 +136,9 @@ void maple_add_packet(struct mapleq *mq)
list_add(mq-list, maple_waitq);
mutex_unlock(maple_list_lock);
 }
-
 EXPORT_SYMBOL_GPL(maple_add_packet);
 
-static struct mapleq *maple_allocq(struct maple_device *dev)
+static struct mapleq *maple_allocq(struct maple_device *mdev)
 {
struct mapleq *mq;
 
@@ -139,7 +146,7 @@ static struct mapleq *maple_allocq(struct maple_device *dev)
if (!mq)
return NULL;
 
-   mq-dev = dev;
+   mq-dev = mdev;
mq-recvbufdcsp = kmem_cache_zalloc(maple_queue_cache, GFP_KERNEL);
mq-recvbuf = (void *) P2SEGADDR(mq-recvbufdcsp);
if (!mq-recvbuf) {
@@ -152,22 +159,24 @@ static struct mapleq *maple_allocq(struct maple_device 
*dev)
 
 static struct maple_device *maple_alloc_dev(int port, int unit)
 {
-   struct maple_device *dev;
+   struct maple_device *mdev;
 
-   dev = kzalloc(sizeof(*dev), GFP_KERNEL);
-   if (!dev)
+   mdev = kzalloc(sizeof(*mdev), GFP_KERNEL);
+   if (!mdev)
return NULL;
 
-   dev-port = port;
-   dev-unit = unit;
-   dev-mq = maple_allocq(dev);
+   mdev-port = port;
+   mdev-unit = unit;
+   mdev-mq = maple_allocq(mdev);
 
-   if (!dev-mq) {
-   kfree(dev);
+   if (!mdev-mq) {
+   kfree(mdev);
return NULL;
}
-
-   return dev;
+   mdev-dev.bus = maple_bus_type;
+   mdev-dev.parent = maple_bus;
+   mdev-function = 0;
+   return mdev;
 }
 
 static void maple_free_dev(struct maple_device *mdev)
@@ -175,7 +184,9 @@ static void maple_free_dev(struct maple_device *mdev)
if (!mdev)
return;
if (mdev-mq) {
-   kmem_cache_free(maple_queue_cache, mdev-mq-recvbufdcsp);
+   if (mdev-mq-recvbufdcsp)
+   kmem_cache_free(maple_queue_cache,
+   mdev-mq-recvbufdcsp);
kfree(mdev-mq);
}
kfree(mdev);
@@ -259,80 +270,93 @@ static void maple_detach_driver(struct maple_device *mdev)
mdev-driver-disconnect(mdev);
}
mdev-driver = NULL;
-   if (mdev-registered) {
-   maple_release_device(mdev-dev);
+   if (mdev-registered)
device_unregister(mdev-dev);
-   }
-   

Re: [PATCH 2/2] - SH/Dreamcast - fix maple bus bugs

2008-02-06 Thread Greg KH
On Wed, Feb 06, 2008 at 10:53:51PM +, Adrian McMenamin wrote:
 - dev-function = function;
 - dev-dev.bus = maple_bus_type;
 - dev-dev.parent = maple_bus;
 - dev-dev.release = maple_release_device;
 - retval = device_register(dev-dev);
 - if (retval) {
 - printk(KERN_INFO
 -Maple bus: Attempt to register device (%x, %x) 
 failed.\n,
 -dev-port, dev-unit);
 - maple_free_dev(dev);
 + mdev-function = function;
 + mdev-dev.release = maple_release_device;
 + if (mdev-registered == 0) {
 + retval = device_register(mdev-dev);
 + if (retval) {
 + printk(KERN_INFO
 + Maple bus: Attempt to register device
 +  (%x, %x) failed.\n,
 + mdev-port, mdev-unit);
 + maple_free_dev(mdev);
 + mdev = NULL;
 + return;
 + }
 + mdev-registered = 1;
   }
 - dev-registered = 1;

I think you are still papering over the real problem here of trying to
register a device twice.  That's something you should never be doing,
and your bus walking logic must be incorrect...

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/2] - SH/Dreamcast - fix maple bus bugs

2008-02-06 Thread Adrian McMenamin

On Wed, 2008-02-06 at 15:01 -0800, Greg KH wrote:
 On Wed, Feb 06, 2008 at 10:53:51PM +, Adrian McMenamin wrote:
  -   dev-function = function;
  -   dev-dev.bus = maple_bus_type;
  -   dev-dev.parent = maple_bus;
  -   dev-dev.release = maple_release_device;
  -   retval = device_register(dev-dev);
  -   if (retval) {
  -   printk(KERN_INFO
  -  Maple bus: Attempt to register device (%x, %x) 
  failed.\n,
  -  dev-port, dev-unit);
  -   maple_free_dev(dev);
  +   mdev-function = function;
  +   mdev-dev.release = maple_release_device;
  +   if (mdev-registered == 0) {
  +   retval = device_register(mdev-dev);
  +   if (retval) {
  +   printk(KERN_INFO
  +   Maple bus: Attempt to register device
  +(%x, %x) failed.\n,
  +   mdev-port, mdev-unit);
  +   maple_free_dev(mdev);
  +   mdev = NULL;
  +   return;
  +   }
  +   mdev-registered = 1;
  }
  -   dev-registered = 1;
 
 I think you are still papering over the real problem here of trying to
 register a device twice.  That's something you should never be doing,
 and your bus walking logic must be incorrect...
 
I just left that in as a sanity check. It should never be acted on. I
can remove it if you prefer.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/2] - SH/Dreamcast - fix maple bus bugs

2008-02-06 Thread Greg KH
On Wed, Feb 06, 2008 at 11:05:14PM +, Adrian McMenamin wrote:
 
 On Wed, 2008-02-06 at 15:01 -0800, Greg KH wrote:
  On Wed, Feb 06, 2008 at 10:53:51PM +, Adrian McMenamin wrote:
   - dev-function = function;
   - dev-dev.bus = maple_bus_type;
   - dev-dev.parent = maple_bus;
   - dev-dev.release = maple_release_device;
   - retval = device_register(dev-dev);
   - if (retval) {
   - printk(KERN_INFO
   -Maple bus: Attempt to register device (%x, %x) 
   failed.\n,
   -dev-port, dev-unit);
   - maple_free_dev(dev);
   + mdev-function = function;
   + mdev-dev.release = maple_release_device;
   + if (mdev-registered == 0) {
   + retval = device_register(mdev-dev);
   + if (retval) {
   + printk(KERN_INFO
   + Maple bus: Attempt to register device
   +  (%x, %x) failed.\n,
   + mdev-port, mdev-unit);
   + maple_free_dev(mdev);
   + mdev = NULL;
   + return;
   + }
   + mdev-registered = 1;
 }
   - dev-registered = 1;
  
  I think you are still papering over the real problem here of trying to
  register a device twice.  That's something you should never be doing,
  and your bus walking logic must be incorrect...
  
 I just left that in as a sanity check. It should never be acted on. I
 can remove it if you prefer.

Please do, that way, and real problems will show up with that warning
you were getting that started all of this work :)

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 2/2] - SH/Dreamcast - fix maple bus bugs

2008-02-06 Thread Greg KH
On Wed, Feb 06, 2008 at 11:51:21PM +, Adrian McMenamin wrote:
 Replacement second-in-series patch:
 
 This patch fixes up memory leaks and, by delaying initialisation, makes
 device detection more robust.
 
 It also makes clearer the difference between struct maple_device and
 struct device, as well as cleaning up the interrupt request code
 (without changing its function in any way).
 
 Also now removes redundant registration checking.

Thanks for changing this.

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/