Re: [PATCH] SH/Dreamcast - fix regressions, whitespace and memory leaks in Maple Bus driver
On Mon, Feb 04, 2008 at 10:37:37PM +, Adrian McMenamin wrote:
>
> On Mon, 2008-02-04 at 08:14 -0800, Greg KH wrote:
> > On Mon, Feb 04, 2008 at 08:27:55AM +, Adrian McMenamin wrote:
> > >
> > > On Sun, 2008-02-03 at 21:29 -0800, Greg KH wrote:
> > > > On Sun, Feb 03, 2008 at 08:00:47PM +, Adrian McMenamin wrote:
> > > > > From: Adrian McMenamin
> > > > >
> > > > > This patch fixes the regression noted here:
> > > > > http://lkml.org/lkml/2008/1/26/189 as well as whitespace issues in the
> > > > > previous commit of this driver and the memory leaks noted here:
> > > > > http://lkml.org/lkml/2008/2/2/143 (as well as one or two other minor
> > > > > cleanups).
> > > >
> > > > Which portion of the patch fixes the kobject WARN_ON()?
> > >
> > >
> > >
> > > + 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;
> > > + }
> > > }
> > >
> > >
> > > Specifically the check on mdev->registered
> >
> > So the code path could cause devices to be registered more than once?
> > That seems broken, as no other bus that I know of needs such a check :(
> >
> > Is there a way to fix the root problem here, instead of this type of
> > change?
> >
>
> The hardware is very flaky. If I add in delays to the bus start, it will
> detect the devices, but it's not brilliant. Registering an empty device
> got round that problem, at the price of testing for the earlier
> registration.
That sounds like you are just papering over the problem. Just delay
and let the hardware settle down if needed :)
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] SH/Dreamcast - fix regressions, whitespace and memory leaks in Maple Bus driver
On Mon, 2008-02-04 at 22:37 +, Adrian McMenamin wrote: > > The hardware is very flaky. If I add in delays to the bus start, it will > detect the devices, but it's not brilliant. Registering an empty device > got round that problem, at the price of testing for the earlier > registration. > I have found if I push it (and drivers) further down the initcall hierarchy that problem is less significant and the hardware seems to be properly detected. I'll look to post a clean up series of patches in the next few days. -- 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] SH/Dreamcast - fix regressions, whitespace and memory leaks in Maple Bus driver
On Mon, 2008-02-04 at 08:14 -0800, Greg KH wrote:
> On Mon, Feb 04, 2008 at 08:27:55AM +, Adrian McMenamin wrote:
> >
> > On Sun, 2008-02-03 at 21:29 -0800, Greg KH wrote:
> > > On Sun, Feb 03, 2008 at 08:00:47PM +, Adrian McMenamin wrote:
> > > > From: Adrian McMenamin
> > > >
> > > > This patch fixes the regression noted here:
> > > > http://lkml.org/lkml/2008/1/26/189 as well as whitespace issues in the
> > > > previous commit of this driver and the memory leaks noted here:
> > > > http://lkml.org/lkml/2008/2/2/143 (as well as one or two other minor
> > > > cleanups).
> > >
> > > Which portion of the patch fixes the kobject WARN_ON()?
> >
> >
> >
> > + 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;
> > + }
> > }
> >
> >
> > Specifically the check on mdev->registered
>
> So the code path could cause devices to be registered more than once?
> That seems broken, as no other bus that I know of needs such a check :(
>
> Is there a way to fix the root problem here, instead of this type of
> change?
>
The hardware is very flaky. If I add in delays to the bus start, it will
detect the devices, but it's not brilliant. Registering an empty device
got round that problem, at the price of testing for the earlier
registration.
--
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] SH/Dreamcast - fix regressions, whitespace and memory leaks in Maple Bus driver
On Mon, Feb 04, 2008 at 08:27:55AM +, Adrian McMenamin wrote:
>
> On Sun, 2008-02-03 at 21:29 -0800, Greg KH wrote:
> > On Sun, Feb 03, 2008 at 08:00:47PM +, Adrian McMenamin wrote:
> > > From: Adrian McMenamin
> > >
> > > This patch fixes the regression noted here:
> > > http://lkml.org/lkml/2008/1/26/189 as well as whitespace issues in the
> > > previous commit of this driver and the memory leaks noted here:
> > > http://lkml.org/lkml/2008/2/2/143 (as well as one or two other minor
> > > cleanups).
> >
> > Which portion of the patch fixes the kobject WARN_ON()?
>
>
>
> + 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;
> + }
> }
>
>
> Specifically the check on mdev->registered
So the code path could cause devices to be registered more than once?
That seems broken, as no other bus that I know of needs such a check :(
Is there a way to fix the root problem here, instead of this type of
change?
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] SH/Dreamcast - fix regressions, whitespace and memory leaks in Maple Bus driver
On Mon, February 4, 2008 9:59 am, Paul Mundt wrote: > On Mon, Feb 04, 2008 at 09:35:11AM -, Adrian McMenamin wrote: >> On Mon, February 4, 2008 9:02 am, Paul Mundt wrote: >> > My previous commit was directly from _your_ patch, given that your >> > patches have a history of whitespace damage, this doesn't seem like >> much >> > of a stretch. It's true I neglected to run it through checkpatch, I'll >> be >> > more careful with that in the future when applying patches from >> certain >> > parties. >> > >> >> I'm sorry. But that's garbage. >> > > Thanks for playing, try again. > It's a fair cop. My apologies. -- 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] SH/Dreamcast - fix regressions, whitespace and memory leaks in Maple Bus driver
On Mon, Feb 04, 2008 at 09:35:11AM -, Adrian McMenamin wrote:
> On Mon, February 4, 2008 9:02 am, Paul Mundt wrote:
> > My previous commit was directly from _your_ patch, given that your
> > patches have a history of whitespace damage, this doesn't seem like much
> > of a stretch. It's true I neglected to run it through checkpatch, I'll be
> > more careful with that in the future when applying patches from certain
> > parties.
> >
>
> I'm sorry. But that's garbage.
>
http://lkml.org/lkml/2007/9/20/179
Also in my archives, and piped through checkpatch:
ERROR: use tabs not spaces
#118: FILE: drivers/sh/maple/maplebus.c:66:
+ return -EINVAL;$
WARNING: EXPORT_SYMBOL(foo); should immediately follow its function/variable
#123: FILE: drivers/sh/maple/maplebus.c:71:
+EXPORT_SYMBOL_GPL(maple_driver_register);
ERROR: use tabs not spaces
#144: FILE: drivers/sh/maple/maplebus.c:92:
+ void (*callback) (struct mapleq * mq),$
ERROR: "foo * bar" should be "foo *bar"
#144: FILE: drivers/sh/maple/maplebus.c:92:
+ void (*callback) (struct mapleq * mq),
ERROR: use tabs not spaces
#145: FILE: drivers/sh/maple/maplebus.c:93:
+ unsigned long interval, unsigned long function)$
ERROR: use tabs not spaces
#162: FILE: drivers/sh/maple/maplebus.c:110:
+ kfree(dev->type->name);$
ERROR: use tabs not spaces
#163: FILE: drivers/sh/maple/maplebus.c:111:
+ kfree(dev->type);$
ERROR: use tabs not spaces
#185: FILE: drivers/sh/maple/maplebus.c:133:
+ return NULL;$
ERROR: use tabs not spaces
#191: FILE: drivers/sh/maple/maplebus.c:139:
+ kfree(mq);$
ERROR: use tabs not spaces
#192: FILE: drivers/sh/maple/maplebus.c:140:
+ return NULL;$
ERROR: use tabs not spaces
#204: FILE: drivers/sh/maple/maplebus.c:152:
+ return NULL;$
ERROR: use tabs not spaces
#211: FILE: drivers/sh/maple/maplebus.c:159:
+ kfree(dev);$
ERROR: use tabs not spaces
#212: FILE: drivers/sh/maple/maplebus.c:160:
+ return NULL;$
ERROR: use tabs not spaces
#221: FILE: drivers/sh/maple/maplebus.c:169:
+ return;$
ERROR: use tabs not spaces
#223: FILE: drivers/sh/maple/maplebus.c:171:
+ kmem_cache_free(maple_queue_cache, mdev->mq->recvbufdcsp);$
ERROR: use tabs not spaces
#224: FILE: drivers/sh/maple/maplebus.c:172:
+ kfree(mdev->mq);$
ERROR: use tabs not spaces
#249: FILE: drivers/sh/maple/maplebus.c:197:
+ mq->command | (to << 8) | (from << 16) | (len << 24);$
ERROR: use tabs not spaces
#252: FILE: drivers/sh/maple/maplebus.c:200:
+ *maple_sendptr++ = *lsendbuf++;$
ERROR: use tabs not spaces
#263: FILE: drivers/sh/maple/maplebus.c:211:
+ return;$
ERROR: use tabs not spaces
#265: FILE: drivers/sh/maple/maplebus.c:213:
+ return;$
ERROR: use tabs not spaces
#269: FILE: drivers/sh/maple/maplebus.c:217:
+ maple_build_block(mq);$
ERROR: use tabs not spaces
#270: FILE: drivers/sh/maple/maplebus.c:218:
+ list_move(>list, _sentq);$
ERROR: use tabs not spaces
#271: FILE: drivers/sh/maple/maplebus.c:219:
+ if (maple_packets++ > MAPLE_MAXPACKETS)$
ERROR: use tabs not spaces
#272: FILE: drivers/sh/maple/maplebus.c:220:
+ break;$
ERROR: use tabs not spaces
#275: FILE: drivers/sh/maple/maplebus.c:223:
+ for (i = 0; i < (1 << MAPLE_DMA_PAGES); i++)$
ERROR: use tabs not spaces
#276: FILE: drivers/sh/maple/maplebus.c:224:
+ dma_cache_sync(0, maple_sendbuf + i * PAGE_SIZE,$
ERROR: use tabs not spaces
#277: FILE: drivers/sh/maple/maplebus.c:225:
+ PAGE_SIZE, DMA_BIDIRECTIONAL);$
ERROR: use tabs not spaces
#282: FILE: drivers/sh/maple/maplebus.c:230:
+ void *devptr)$
ERROR: use tabs not spaces
#290: FILE: drivers/sh/maple/maplebus.c:238:
+ if (maple_drv->connect(mdev) == 0) {$
ERROR: use tabs not spaces
#291: FILE: drivers/sh/maple/maplebus.c:239:
+ mdev->driver = maple_drv;$
ERROR: use tabs not spaces
#292: FILE: drivers/sh/maple/maplebus.c:240:
+ return 1;$
ERROR: use tabs not spaces
#293: FILE: drivers/sh/maple/maplebus.c:241:
+ }$
ERROR: use tabs not spaces
#301: FILE: drivers/sh/maple/maplebus.c:249:
+ return;$
ERROR: use tabs not spaces
#303: FILE: drivers/sh/maple/maplebus.c:251:
+ if (mdev->driver->disconnect)$
ERROR: use tabs not spaces
#304: FILE: drivers/sh/maple/maplebus.c:252:
+ mdev->driver->disconnect(mdev);$
ERROR: use tabs not spaces
#308: FILE: drivers/sh/maple/maplebus.c:256:
+ maple_release_device(>dev);$
ERROR: use tabs not spaces
#309: FILE: drivers/sh/maple/maplebus.c:257:
+ device_unregister(>dev);$
ERROR: use tabs not
Re: [PATCH] SH/Dreamcast - fix regressions, whitespace and memory leaks in Maple Bus driver
On Mon, February 4, 2008 9:02 am, Paul Mundt wrote: > My previous commit was directly from _your_ patch, given that your > patches have a history of whitespace damage, this doesn't seem like much > of a stretch. It's true I neglected to run it through checkpatch, I'll be > more careful with that in the future when applying patches from certain > parties. > I'm sorry. But that's garbage. I'm prepared to admit I made a mistake. Clearly you aren't. -- 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] SH/Dreamcast - fix regressions, whitespace and memory leaks in Maple Bus driver
On Mon, Feb 04, 2008 at 08:23:44AM +, Adrian McMenamin wrote: > > On Mon, 2008-02-04 at 10:10 +0900, Paul Mundt wrote: > > On Sun, Feb 03, 2008 at 08:00:47PM +, Adrian McMenamin wrote: > > > From: Adrian McMenamin > > > > > This is useless if you are submitting the patch, especially if you're > > missing a mail address. > > > > >From Documentation/SubmittingPatches > > The canonical patch message body contains the following: > * > * - A "from" line specifying the patch author. > * > Which doesn't invalidate the missing address problem, and the fact that you are _already_ in the from line. > > > This patch fixes the regression noted here: > > > http://lkml.org/lkml/2008/1/26/189 as well as whitespace issues in the > > > previous commit of this driver and the memory leaks noted here: > > > http://lkml.org/lkml/2008/2/2/143 (as well as one or two other minor > > > cleanups). > > > > > The subject notes 3 specific things that are being addressed, but you've > > rolled this all in to one patch which makes it utterly impossible to > > figure out what you're actually fixing. At the very least, split this in > > to 3 different patches, each dealing with one of the things noted in the > > subject. The fact that regressions is plural also suggests you may want > > to split this down in to smaller patches that deal with specific > > regressions if they are not directly related. > > What would be the point of submitting patches of broken code just to > remove the whitespace your previous commit added to all the lines? > My previous commit was directly from _your_ patch, given that your patches have a history of whitespace damage, this doesn't seem like much of a stretch. It's true I neglected to run it through checkpatch, I'll be more careful with that in the future when applying patches from certain parties. The point of submitting a series of patches is so that it's obvious _what_ you are changing. Lumping it in with the whitespace changes just makes it impossible to read, as GregKH also hinted at when trying to figure out specifically what you were fixing. Since your patch splits up logically in to different components, it makes sense to split the patch up in to a series that makes it obvious. I'm not sure why this needs to be spelled out for you. -- 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] SH/Dreamcast - fix regressions, whitespace and memory leaks in Maple Bus driver
On Sun, 2008-02-03 at 21:29 -0800, Greg KH wrote:
> On Sun, Feb 03, 2008 at 08:00:47PM +, Adrian McMenamin wrote:
> > From: Adrian McMenamin
> >
> > This patch fixes the regression noted here:
> > http://lkml.org/lkml/2008/1/26/189 as well as whitespace issues in the
> > previous commit of this driver and the memory leaks noted here:
> > http://lkml.org/lkml/2008/2/2/143 (as well as one or two other minor
> > cleanups).
>
> Which portion of the patch fixes the kobject WARN_ON()?
+ 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;
+ }
}
Specifically the check on mdev->registered
Unfortunately the previous commit was completely corrupted by whitespace
everywhere so the patch essentially covers the whole dirver (I had a
choice of submitting broken code with whitespace removed or working code
with whitespace removed)
--
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] SH/Dreamcast - fix regressions, whitespace and memory leaks in Maple Bus driver
On Mon, 2008-02-04 at 10:10 +0900, Paul Mundt wrote: > On Sun, Feb 03, 2008 at 08:00:47PM +, Adrian McMenamin wrote: > > From: Adrian McMenamin > > > This is useless if you are submitting the patch, especially if you're > missing a mail address. > >From Documentation/SubmittingPatches The canonical patch message body contains the following: * * - A "from" line specifying the patch author. * > > This patch fixes the regression noted here: > > http://lkml.org/lkml/2008/1/26/189 as well as whitespace issues in the > > previous commit of this driver and the memory leaks noted here: > > http://lkml.org/lkml/2008/2/2/143 (as well as one or two other minor > > cleanups). > > > The subject notes 3 specific things that are being addressed, but you've > rolled this all in to one patch which makes it utterly impossible to > figure out what you're actually fixing. At the very least, split this in > to 3 different patches, each dealing with one of the things noted in the > subject. The fact that regressions is plural also suggests you may want > to split this down in to smaller patches that deal with specific > regressions if they are not directly related. > What would be the point of submitting patches of broken code just to remove the whitespace your previous commit added to all the lines? > > Signed off by: Adrian McMenamin <[EMAIL PROTECTED]> > > > Do not invent new sign-off tags, see Documentation/SubmittingPatches. > Scripts do end up having to parse this stuff. Yes, sorry for that. -- 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] SH/Dreamcast - fix regressions, whitespace and memory leaks in Maple Bus driver
On Sun, 2008-02-03 at 21:29 -0800, Greg KH wrote:
On Sun, Feb 03, 2008 at 08:00:47PM +, Adrian McMenamin wrote:
From: Adrian McMenamin
This patch fixes the regression noted here:
http://lkml.org/lkml/2008/1/26/189 as well as whitespace issues in the
previous commit of this driver and the memory leaks noted here:
http://lkml.org/lkml/2008/2/2/143 (as well as one or two other minor
cleanups).
Which portion of the patch fixes the kobject WARN_ON()?
+ 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;
+ }
}
Specifically the check on mdev-registered
Unfortunately the previous commit was completely corrupted by whitespace
everywhere so the patch essentially covers the whole dirver (I had a
choice of submitting broken code with whitespace removed or working code
with whitespace removed)
--
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] SH/Dreamcast - fix regressions, whitespace and memory leaks in Maple Bus driver
On Mon, 2008-02-04 at 10:10 +0900, Paul Mundt wrote: On Sun, Feb 03, 2008 at 08:00:47PM +, Adrian McMenamin wrote: From: Adrian McMenamin This is useless if you are submitting the patch, especially if you're missing a mail address. From Documentation/SubmittingPatches The canonical patch message body contains the following: * * - A from line specifying the patch author. * This patch fixes the regression noted here: http://lkml.org/lkml/2008/1/26/189 as well as whitespace issues in the previous commit of this driver and the memory leaks noted here: http://lkml.org/lkml/2008/2/2/143 (as well as one or two other minor cleanups). The subject notes 3 specific things that are being addressed, but you've rolled this all in to one patch which makes it utterly impossible to figure out what you're actually fixing. At the very least, split this in to 3 different patches, each dealing with one of the things noted in the subject. The fact that regressions is plural also suggests you may want to split this down in to smaller patches that deal with specific regressions if they are not directly related. What would be the point of submitting patches of broken code just to remove the whitespace your previous commit added to all the lines? Signed off by: Adrian McMenamin [EMAIL PROTECTED] Do not invent new sign-off tags, see Documentation/SubmittingPatches. Scripts do end up having to parse this stuff. Yes, sorry for that. -- 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] SH/Dreamcast - fix regressions, whitespace and memory leaks in Maple Bus driver
On Mon, Feb 04, 2008 at 08:23:44AM +, Adrian McMenamin wrote: On Mon, 2008-02-04 at 10:10 +0900, Paul Mundt wrote: On Sun, Feb 03, 2008 at 08:00:47PM +, Adrian McMenamin wrote: From: Adrian McMenamin This is useless if you are submitting the patch, especially if you're missing a mail address. From Documentation/SubmittingPatches The canonical patch message body contains the following: * * - A from line specifying the patch author. * Which doesn't invalidate the missing address problem, and the fact that you are _already_ in the from line. This patch fixes the regression noted here: http://lkml.org/lkml/2008/1/26/189 as well as whitespace issues in the previous commit of this driver and the memory leaks noted here: http://lkml.org/lkml/2008/2/2/143 (as well as one or two other minor cleanups). The subject notes 3 specific things that are being addressed, but you've rolled this all in to one patch which makes it utterly impossible to figure out what you're actually fixing. At the very least, split this in to 3 different patches, each dealing with one of the things noted in the subject. The fact that regressions is plural also suggests you may want to split this down in to smaller patches that deal with specific regressions if they are not directly related. What would be the point of submitting patches of broken code just to remove the whitespace your previous commit added to all the lines? My previous commit was directly from _your_ patch, given that your patches have a history of whitespace damage, this doesn't seem like much of a stretch. It's true I neglected to run it through checkpatch, I'll be more careful with that in the future when applying patches from certain parties. The point of submitting a series of patches is so that it's obvious _what_ you are changing. Lumping it in with the whitespace changes just makes it impossible to read, as GregKH also hinted at when trying to figure out specifically what you were fixing. Since your patch splits up logically in to different components, it makes sense to split the patch up in to a series that makes it obvious. I'm not sure why this needs to be spelled out for you. -- 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] SH/Dreamcast - fix regressions, whitespace and memory leaks in Maple Bus driver
On Mon, February 4, 2008 9:02 am, Paul Mundt wrote: My previous commit was directly from _your_ patch, given that your patches have a history of whitespace damage, this doesn't seem like much of a stretch. It's true I neglected to run it through checkpatch, I'll be more careful with that in the future when applying patches from certain parties. I'm sorry. But that's garbage. I'm prepared to admit I made a mistake. Clearly you aren't. -- 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] SH/Dreamcast - fix regressions, whitespace and memory leaks in Maple Bus driver
On Mon, Feb 04, 2008 at 09:35:11AM -, Adrian McMenamin wrote:
On Mon, February 4, 2008 9:02 am, Paul Mundt wrote:
My previous commit was directly from _your_ patch, given that your
patches have a history of whitespace damage, this doesn't seem like much
of a stretch. It's true I neglected to run it through checkpatch, I'll be
more careful with that in the future when applying patches from certain
parties.
I'm sorry. But that's garbage.
http://lkml.org/lkml/2007/9/20/179
Also in my archives, and piped through checkpatch:
ERROR: use tabs not spaces
#118: FILE: drivers/sh/maple/maplebus.c:66:
+ return -EINVAL;$
WARNING: EXPORT_SYMBOL(foo); should immediately follow its function/variable
#123: FILE: drivers/sh/maple/maplebus.c:71:
+EXPORT_SYMBOL_GPL(maple_driver_register);
ERROR: use tabs not spaces
#144: FILE: drivers/sh/maple/maplebus.c:92:
+ void (*callback) (struct mapleq * mq),$
ERROR: foo * bar should be foo *bar
#144: FILE: drivers/sh/maple/maplebus.c:92:
+ void (*callback) (struct mapleq * mq),
ERROR: use tabs not spaces
#145: FILE: drivers/sh/maple/maplebus.c:93:
+ unsigned long interval, unsigned long function)$
ERROR: use tabs not spaces
#162: FILE: drivers/sh/maple/maplebus.c:110:
+ kfree(dev-type-name);$
ERROR: use tabs not spaces
#163: FILE: drivers/sh/maple/maplebus.c:111:
+ kfree(dev-type);$
ERROR: use tabs not spaces
#185: FILE: drivers/sh/maple/maplebus.c:133:
+ return NULL;$
ERROR: use tabs not spaces
#191: FILE: drivers/sh/maple/maplebus.c:139:
+ kfree(mq);$
ERROR: use tabs not spaces
#192: FILE: drivers/sh/maple/maplebus.c:140:
+ return NULL;$
ERROR: use tabs not spaces
#204: FILE: drivers/sh/maple/maplebus.c:152:
+ return NULL;$
ERROR: use tabs not spaces
#211: FILE: drivers/sh/maple/maplebus.c:159:
+ kfree(dev);$
ERROR: use tabs not spaces
#212: FILE: drivers/sh/maple/maplebus.c:160:
+ return NULL;$
ERROR: use tabs not spaces
#221: FILE: drivers/sh/maple/maplebus.c:169:
+ return;$
ERROR: use tabs not spaces
#223: FILE: drivers/sh/maple/maplebus.c:171:
+ kmem_cache_free(maple_queue_cache, mdev-mq-recvbufdcsp);$
ERROR: use tabs not spaces
#224: FILE: drivers/sh/maple/maplebus.c:172:
+ kfree(mdev-mq);$
ERROR: use tabs not spaces
#249: FILE: drivers/sh/maple/maplebus.c:197:
+ mq-command | (to 8) | (from 16) | (len 24);$
ERROR: use tabs not spaces
#252: FILE: drivers/sh/maple/maplebus.c:200:
+ *maple_sendptr++ = *lsendbuf++;$
ERROR: use tabs not spaces
#263: FILE: drivers/sh/maple/maplebus.c:211:
+ return;$
ERROR: use tabs not spaces
#265: FILE: drivers/sh/maple/maplebus.c:213:
+ return;$
ERROR: use tabs not spaces
#269: FILE: drivers/sh/maple/maplebus.c:217:
+ maple_build_block(mq);$
ERROR: use tabs not spaces
#270: FILE: drivers/sh/maple/maplebus.c:218:
+ list_move(mq-list, maple_sentq);$
ERROR: use tabs not spaces
#271: FILE: drivers/sh/maple/maplebus.c:219:
+ if (maple_packets++ MAPLE_MAXPACKETS)$
ERROR: use tabs not spaces
#272: FILE: drivers/sh/maple/maplebus.c:220:
+ break;$
ERROR: use tabs not spaces
#275: FILE: drivers/sh/maple/maplebus.c:223:
+ for (i = 0; i (1 MAPLE_DMA_PAGES); i++)$
ERROR: use tabs not spaces
#276: FILE: drivers/sh/maple/maplebus.c:224:
+ dma_cache_sync(0, maple_sendbuf + i * PAGE_SIZE,$
ERROR: use tabs not spaces
#277: FILE: drivers/sh/maple/maplebus.c:225:
+ PAGE_SIZE, DMA_BIDIRECTIONAL);$
ERROR: use tabs not spaces
#282: FILE: drivers/sh/maple/maplebus.c:230:
+ void *devptr)$
ERROR: use tabs not spaces
#290: FILE: drivers/sh/maple/maplebus.c:238:
+ if (maple_drv-connect(mdev) == 0) {$
ERROR: use tabs not spaces
#291: FILE: drivers/sh/maple/maplebus.c:239:
+ mdev-driver = maple_drv;$
ERROR: use tabs not spaces
#292: FILE: drivers/sh/maple/maplebus.c:240:
+ return 1;$
ERROR: use tabs not spaces
#293: FILE: drivers/sh/maple/maplebus.c:241:
+ }$
ERROR: use tabs not spaces
#301: FILE: drivers/sh/maple/maplebus.c:249:
+ return;$
ERROR: use tabs not spaces
#303: FILE: drivers/sh/maple/maplebus.c:251:
+ if (mdev-driver-disconnect)$
ERROR: use tabs not spaces
#304: FILE: drivers/sh/maple/maplebus.c:252:
+ mdev-driver-disconnect(mdev);$
ERROR: use tabs not spaces
#308: FILE: drivers/sh/maple/maplebus.c:256:
+ maple_release_device(mdev-dev);$
ERROR: use tabs not spaces
#309: FILE: drivers/sh/maple/maplebus.c:257:
+ device_unregister(mdev-dev);$
ERROR: use tabs not spaces
#332: FILE:
Re: [PATCH] SH/Dreamcast - fix regressions, whitespace and memory leaks in Maple Bus driver
On Mon, February 4, 2008 9:59 am, Paul Mundt wrote: On Mon, Feb 04, 2008 at 09:35:11AM -, Adrian McMenamin wrote: On Mon, February 4, 2008 9:02 am, Paul Mundt wrote: My previous commit was directly from _your_ patch, given that your patches have a history of whitespace damage, this doesn't seem like much of a stretch. It's true I neglected to run it through checkpatch, I'll be more careful with that in the future when applying patches from certain parties. I'm sorry. But that's garbage. Thanks for playing, try again. It's a fair cop. My apologies. -- 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] SH/Dreamcast - fix regressions, whitespace and memory leaks in Maple Bus driver
On Mon, Feb 04, 2008 at 08:27:55AM +, Adrian McMenamin wrote:
On Sun, 2008-02-03 at 21:29 -0800, Greg KH wrote:
On Sun, Feb 03, 2008 at 08:00:47PM +, Adrian McMenamin wrote:
From: Adrian McMenamin
This patch fixes the regression noted here:
http://lkml.org/lkml/2008/1/26/189 as well as whitespace issues in the
previous commit of this driver and the memory leaks noted here:
http://lkml.org/lkml/2008/2/2/143 (as well as one or two other minor
cleanups).
Which portion of the patch fixes the kobject WARN_ON()?
+ 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;
+ }
}
Specifically the check on mdev-registered
So the code path could cause devices to be registered more than once?
That seems broken, as no other bus that I know of needs such a check :(
Is there a way to fix the root problem here, instead of this type of
change?
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] SH/Dreamcast - fix regressions, whitespace and memory leaks in Maple Bus driver
On Mon, 2008-02-04 at 22:37 +, Adrian McMenamin wrote: The hardware is very flaky. If I add in delays to the bus start, it will detect the devices, but it's not brilliant. Registering an empty device got round that problem, at the price of testing for the earlier registration. I have found if I push it (and drivers) further down the initcall hierarchy that problem is less significant and the hardware seems to be properly detected. I'll look to post a clean up series of patches in the next few days. -- 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] SH/Dreamcast - fix regressions, whitespace and memory leaks in Maple Bus driver
On Mon, 2008-02-04 at 08:14 -0800, Greg KH wrote:
On Mon, Feb 04, 2008 at 08:27:55AM +, Adrian McMenamin wrote:
On Sun, 2008-02-03 at 21:29 -0800, Greg KH wrote:
On Sun, Feb 03, 2008 at 08:00:47PM +, Adrian McMenamin wrote:
From: Adrian McMenamin
This patch fixes the regression noted here:
http://lkml.org/lkml/2008/1/26/189 as well as whitespace issues in the
previous commit of this driver and the memory leaks noted here:
http://lkml.org/lkml/2008/2/2/143 (as well as one or two other minor
cleanups).
Which portion of the patch fixes the kobject WARN_ON()?
+ 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;
+ }
}
Specifically the check on mdev-registered
So the code path could cause devices to be registered more than once?
That seems broken, as no other bus that I know of needs such a check :(
Is there a way to fix the root problem here, instead of this type of
change?
The hardware is very flaky. If I add in delays to the bus start, it will
detect the devices, but it's not brilliant. Registering an empty device
got round that problem, at the price of testing for the earlier
registration.
--
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] SH/Dreamcast - fix regressions, whitespace and memory leaks in Maple Bus driver
On Mon, Feb 04, 2008 at 10:37:37PM +, Adrian McMenamin wrote:
On Mon, 2008-02-04 at 08:14 -0800, Greg KH wrote:
On Mon, Feb 04, 2008 at 08:27:55AM +, Adrian McMenamin wrote:
On Sun, 2008-02-03 at 21:29 -0800, Greg KH wrote:
On Sun, Feb 03, 2008 at 08:00:47PM +, Adrian McMenamin wrote:
From: Adrian McMenamin
This patch fixes the regression noted here:
http://lkml.org/lkml/2008/1/26/189 as well as whitespace issues in the
previous commit of this driver and the memory leaks noted here:
http://lkml.org/lkml/2008/2/2/143 (as well as one or two other minor
cleanups).
Which portion of the patch fixes the kobject WARN_ON()?
+ 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;
+ }
}
Specifically the check on mdev-registered
So the code path could cause devices to be registered more than once?
That seems broken, as no other bus that I know of needs such a check :(
Is there a way to fix the root problem here, instead of this type of
change?
The hardware is very flaky. If I add in delays to the bus start, it will
detect the devices, but it's not brilliant. Registering an empty device
got round that problem, at the price of testing for the earlier
registration.
That sounds like you are just papering over the problem. Just delay
and let the hardware settle down if needed :)
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] SH/Dreamcast - fix regressions, whitespace and memory leaks in Maple Bus driver
On Sun, Feb 03, 2008 at 08:00:47PM +, Adrian McMenamin wrote: > From: Adrian McMenamin > > This patch fixes the regression noted here: > http://lkml.org/lkml/2008/1/26/189 as well as whitespace issues in the > previous commit of this driver and the memory leaks noted here: > http://lkml.org/lkml/2008/2/2/143 (as well as one or two other minor > cleanups). Which portion of the patch fixes the kobject WARN_ON()? 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] SH/Dreamcast - fix regressions, whitespace and memory leaks in Maple Bus driver
On Sun, Feb 03, 2008 at 08:00:47PM +, Adrian McMenamin wrote: > From: Adrian McMenamin > This is useless if you are submitting the patch, especially if you're missing a mail address. > This patch fixes the regression noted here: > http://lkml.org/lkml/2008/1/26/189 as well as whitespace issues in the > previous commit of this driver and the memory leaks noted here: > http://lkml.org/lkml/2008/2/2/143 (as well as one or two other minor > cleanups). > The subject notes 3 specific things that are being addressed, but you've rolled this all in to one patch which makes it utterly impossible to figure out what you're actually fixing. At the very least, split this in to 3 different patches, each dealing with one of the things noted in the subject. The fact that regressions is plural also suggests you may want to split this down in to smaller patches that deal with specific regressions if they are not directly related. > Signed off by: Adrian McMenamin <[EMAIL PROTECTED]> > Do not invent new sign-off tags, see Documentation/SubmittingPatches. Scripts do end up having to parse this stuff. -- 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] SH/Dreamcast - fix regressions, whitespace and memory leaks in Maple Bus driver
From: Adrian McMenamin
This patch fixes the regression noted here:
http://lkml.org/lkml/2008/1/26/189 as well as whitespace issues in the
previous commit of this driver and the memory leaks noted here:
http://lkml.org/lkml/2008/2/2/143 (as well as one or two other minor
cleanups).
Signed off by: Adrian McMenamin <[EMAIL PROTECTED]>
==
diff -ruN a/drivers/sh/maple/maple.c b/drivers/sh/maple/maple.c
--- a/drivers/sh/maple/maple.c 2008-02-03 19:32:23.0 +
+++ b/drivers/sh/maple/maple.c 2008-02-03 19:45:41.0 +
@@ -18,7 +18,6 @@
#include
#include
#include
-#include
#include
#include
#include
@@ -53,12 +52,12 @@
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 {
- int port;
- int unit;
+ int port;
+ int unit;
};
/**
@@ -68,22 +67,22 @@
*/
int maple_driver_register(struct device_driver *drv)
{
- if (!drv)
- return -EINVAL;
- drv->bus = _bus_type;
- return driver_register(drv);
+ if (!drv)
+ return -EINVAL;
+ drv->bus = _bus_type;
+ return driver_register(drv);
}
EXPORT_SYMBOL_GPL(maple_driver_register);
/* set hardware registers to enable next round of dma */
static void maplebus_dma_reset(void)
{
- ctrl_outl(MAPLE_MAGIC, MAPLE_RESET);
- /* set trig type to 0 for software trigger, 1 for hardware (VBLANK) */
- ctrl_outl(1, MAPLE_TRIGTYPE);
- ctrl_outl(MAPLE_2MBPS | MAPLE_TIMEOUT(5), MAPLE_SPEED);
- ctrl_outl(PHYSADDR(maple_sendbuf), MAPLE_DMAADDR);
- ctrl_outl(1, MAPLE_ENABLE);
+ ctrl_outl(MAPLE_MAGIC, MAPLE_RESET);
+ /* set trig type to 0 for software trigger, 1 for hardware (VBLANK) */
+ ctrl_outl(1, MAPLE_TRIGTYPE);
+ ctrl_outl(MAPLE_2MBPS | MAPLE_TIMEOUT(5), MAPLE_SPEED);
+ ctrl_outl(PHYSADDR(maple_sendbuf), MAPLE_DMAADDR);
+ ctrl_outl(1, MAPLE_ENABLE);
}
/**
@@ -94,27 +93,36 @@
* @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;
+ 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)
{
- return (ctrl_inl(MAPLE_STATE) & 1) == 0;
+ return (ctrl_inl(MAPLE_STATE) & 1) == 0;
}
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);
}
/**
@@ -123,60 +131,62 @@
*/
void maple_add_packet(struct mapleq *mq)
{
- mutex_lock(_list_lock);
- list_add(>list, _waitq);
- mutex_unlock(_list_lock);
+ mutex_lock(_list_lock);
+ 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;
+ struct mapleq *mq;
- mq = kmalloc(sizeof(*mq), GFP_KERNEL);
- if (!mq)
- return NULL;
-
- mq->dev = dev;
- mq->recvbufdcsp = kmem_cache_zalloc(maple_queue_cache, GFP_KERNEL);
- mq->recvbuf = (void *) P2SEGADDR(mq->recvbufdcsp);
- if (!mq->recvbuf) {
- kfree(mq);
- return NULL;
- }
+ mq = kmalloc(sizeof(*mq), GFP_KERNEL);
+ if (!mq)
+ return NULL;
+
+ mq->dev = mdev;
+ mq->recvbufdcsp = kmem_cache_zalloc(maple_queue_cache, GFP_KERNEL);
+ mq->recvbuf = (void *) P2SEGADDR(mq->recvbufdcsp);
+ if (!mq->recvbuf) {
+ kfree(mq);
+ return NULL;
+ }
- return mq;
+ return mq;
}
static struct maple_device *maple_alloc_dev(int port, int unit)
{
- struct maple_device *dev;
+ struct
[PATCH] SH/Dreamcast - fix regressions, whitespace and memory leaks in Maple Bus driver
From: Adrian McMenamin
This patch fixes the regression noted here:
http://lkml.org/lkml/2008/1/26/189 as well as whitespace issues in the
previous commit of this driver and the memory leaks noted here:
http://lkml.org/lkml/2008/2/2/143 (as well as one or two other minor
cleanups).
Signed off by: Adrian McMenamin [EMAIL PROTECTED]
==
diff -ruN a/drivers/sh/maple/maple.c b/drivers/sh/maple/maple.c
--- a/drivers/sh/maple/maple.c 2008-02-03 19:32:23.0 +
+++ b/drivers/sh/maple/maple.c 2008-02-03 19:45:41.0 +
@@ -18,7 +18,6 @@
#include linux/init.h
#include linux/kernel.h
#include linux/device.h
-#include linux/module.h
#include linux/interrupt.h
#include linux/list.h
#include linux/io.h
@@ -53,12 +52,12 @@
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 {
- int port;
- int unit;
+ int port;
+ int unit;
};
/**
@@ -68,22 +67,22 @@
*/
int maple_driver_register(struct device_driver *drv)
{
- if (!drv)
- return -EINVAL;
- drv-bus = maple_bus_type;
- return driver_register(drv);
+ if (!drv)
+ return -EINVAL;
+ drv-bus = maple_bus_type;
+ return driver_register(drv);
}
EXPORT_SYMBOL_GPL(maple_driver_register);
/* set hardware registers to enable next round of dma */
static void maplebus_dma_reset(void)
{
- ctrl_outl(MAPLE_MAGIC, MAPLE_RESET);
- /* set trig type to 0 for software trigger, 1 for hardware (VBLANK) */
- ctrl_outl(1, MAPLE_TRIGTYPE);
- ctrl_outl(MAPLE_2MBPS | MAPLE_TIMEOUT(5), MAPLE_SPEED);
- ctrl_outl(PHYSADDR(maple_sendbuf), MAPLE_DMAADDR);
- ctrl_outl(1, MAPLE_ENABLE);
+ ctrl_outl(MAPLE_MAGIC, MAPLE_RESET);
+ /* set trig type to 0 for software trigger, 1 for hardware (VBLANK) */
+ ctrl_outl(1, MAPLE_TRIGTYPE);
+ ctrl_outl(MAPLE_2MBPS | MAPLE_TIMEOUT(5), MAPLE_SPEED);
+ ctrl_outl(PHYSADDR(maple_sendbuf), MAPLE_DMAADDR);
+ ctrl_outl(1, MAPLE_ENABLE);
}
/**
@@ -94,27 +93,36 @@
* @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;
+ 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)
{
- return (ctrl_inl(MAPLE_STATE) 1) == 0;
+ return (ctrl_inl(MAPLE_STATE) 1) == 0;
}
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);
}
/**
@@ -123,60 +131,62 @@
*/
void maple_add_packet(struct mapleq *mq)
{
- mutex_lock(maple_list_lock);
- list_add(mq-list, maple_waitq);
- mutex_unlock(maple_list_lock);
+ mutex_lock(maple_list_lock);
+ 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;
+ struct mapleq *mq;
- mq = kmalloc(sizeof(*mq), GFP_KERNEL);
- if (!mq)
- return NULL;
-
- mq-dev = dev;
- mq-recvbufdcsp = kmem_cache_zalloc(maple_queue_cache, GFP_KERNEL);
- mq-recvbuf = (void *) P2SEGADDR(mq-recvbufdcsp);
- if (!mq-recvbuf) {
- kfree(mq);
- return NULL;
- }
+ mq = kmalloc(sizeof(*mq), GFP_KERNEL);
+ if (!mq)
+ return NULL;
+
+ mq-dev = mdev;
+ mq-recvbufdcsp = kmem_cache_zalloc(maple_queue_cache, GFP_KERNEL);
+ mq-recvbuf = (void *) P2SEGADDR(mq-recvbufdcsp);
+ if (!mq-recvbuf) {
+ kfree(mq);
+ return NULL;
+ }
- return mq;
+ return mq;
}
static struct
Re: [PATCH] SH/Dreamcast - fix regressions, whitespace and memory leaks in Maple Bus driver
On Sun, Feb 03, 2008 at 08:00:47PM +, Adrian McMenamin wrote: From: Adrian McMenamin This is useless if you are submitting the patch, especially if you're missing a mail address. This patch fixes the regression noted here: http://lkml.org/lkml/2008/1/26/189 as well as whitespace issues in the previous commit of this driver and the memory leaks noted here: http://lkml.org/lkml/2008/2/2/143 (as well as one or two other minor cleanups). The subject notes 3 specific things that are being addressed, but you've rolled this all in to one patch which makes it utterly impossible to figure out what you're actually fixing. At the very least, split this in to 3 different patches, each dealing with one of the things noted in the subject. The fact that regressions is plural also suggests you may want to split this down in to smaller patches that deal with specific regressions if they are not directly related. Signed off by: Adrian McMenamin [EMAIL PROTECTED] Do not invent new sign-off tags, see Documentation/SubmittingPatches. Scripts do end up having to parse this stuff. -- 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] SH/Dreamcast - fix regressions, whitespace and memory leaks in Maple Bus driver
On Sun, Feb 03, 2008 at 08:00:47PM +, Adrian McMenamin wrote: From: Adrian McMenamin This patch fixes the regression noted here: http://lkml.org/lkml/2008/1/26/189 as well as whitespace issues in the previous commit of this driver and the memory leaks noted here: http://lkml.org/lkml/2008/2/2/143 (as well as one or two other minor cleanups). Which portion of the patch fixes the kobject WARN_ON()? 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/
