[PATCH] driver core: Silent meaningless error message

[PATCH] driver core: Silent meaningless error message

[Jean Delvare]

I see the following error message in my kernel logs:

w83627ehf.2576: use which platform_data?

This is caused by platform_device_add_data() setting both
pdev->platform_data and pdev->dev.platform_data, and then
platform_device_add() complaining if both pointers are set.

As platform_device_add() is now making sure that both pointers are
set, there is no point in setting both in platform_device_add_data().

Additionally, we should only issue the error message when there is a
real problem, that is: both data pointers are set and differ. And, in
this case, we should fail right away, as something is seriously wrong.

Also free the memory through pa->pdev.platform_data rather than
pa->pdev.dev.platform_data. We will have to do that someday anyway, so
we might as well start now to make the intents clearer.

Signed-off-by: Jean Delvare <khali@linux-fr.org>
Cc: Ming Lei <tom.leiming@gmail.com>
Cc: Greg Kroah-Hartman <gregkh@suse.de>
---
 drivers/base/platform.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

--- linux-2.6.30-rc4.orig/drivers/base/platform.c	2009-04-22 09:51:01.000000000 +0200
+++ linux-2.6.30-rc4/drivers/base/platform.c	2009-05-01 14:09:16.000000000 +0200
@@ -143,7 +143,7 @@ static void platform_device_release(stru
 	struct platform_object *pa = container_of(dev, struct platform_object,
 						  pdev.dev);
 
-	kfree(pa->pdev.dev.platform_data);
+	kfree(pa->pdev.platform_data);
 	kfree(pa->pdev.resource);
 	kfree(pa);
 }
@@ -216,7 +216,6 @@ int platform_device_add_data(struct plat
 	d = kmalloc(size, GFP_KERNEL);
 	if (d) {
 		memcpy(d, data, size);
-		pdev->dev.platform_data = d;
 		pdev->platform_data = d;
 	}
 	return d ? 0 : -ENOMEM;
@@ -253,9 +252,12 @@ int platform_device_add(struct platform_
 	* long time, so we allow the two cases coexist to make
 	* this kind of fix more easily*/
 	if (pdev->platform_data && pdev->dev.platform_data) {
-		printk(KERN_ERR
+		if (pdev->platform_data != pdev->dev.platform_data) {
+			printk(KERN_ERR
 			       "%s: use which platform_data?\n",
 			       dev_name(&pdev->dev));
+			return -EINVAL;
+		}
 	} else if (pdev->platform_data) {
 		pdev->dev.platform_data = pdev->platform_data;
 	} else if (pdev->dev.platform_data) {


-- 
Jean Delvare

Re: [PATCH] driver core: Silent meaningless error message

[Greg KH]

On Fri, May 01, 2009 at 02:51:51PM +0200, Jean Delvare wrote:
> I see the following error message in my kernel logs:
> 
> w83627ehf.2576: use which platform_data?
> 
> This is caused by platform_device_add_data() setting both
> pdev->platform_data and pdev->dev.platform_data, and then
> platform_device_add() complaining if both pointers are set.
> 

The patch that causes this is about to be reverted later today (it's
already reverted in my trees), as it was the wrong thing to do, as you
point out.

So this should be solved soon, no need for your patch.

thanks,

greg k-h

Re: [PATCH] driver core: Silent meaningless error message

[Jean Delvare]

Hi Greg,

On Fri, 1 May 2009 08:14:35 -0700, Greg KH wrote:
> On Fri, May 01, 2009 at 02:51:51PM +0200, Jean Delvare wrote:
> > I see the following error message in my kernel logs:
> > 
> > w83627ehf.2576: use which platform_data?
> > 
> > This is caused by platform_device_add_data() setting both
> > pdev->platform_data and pdev->dev.platform_data, and then
> > platform_device_add() complaining if both pointers are set.
> > 
> 
> The patch that causes this is about to be reverted later today (it's
> already reverted in my trees), as it was the wrong thing to do, as you
> point out.
> 
> So this should be solved soon, no need for your patch.

Any news on this? I can't see any fix in kernel 2.6.30-rc5.

-- 
Jean Delvare

Re: [PATCH] driver core: Silent meaningless error message

[Greg KH]

On Sat, May 09, 2009 at 03:25:14PM +0200, Jean Delvare wrote:
> Hi Greg,
> 
> On Fri, 1 May 2009 08:14:35 -0700, Greg KH wrote:
> > On Fri, May 01, 2009 at 02:51:51PM +0200, Jean Delvare wrote:
> > > I see the following error message in my kernel logs:
> > > 
> > > w83627ehf.2576: use which platform_data?
> > > 
> > > This is caused by platform_device_add_data() setting both
> > > pdev->platform_data and pdev->dev.platform_data, and then
> > > platform_device_add() complaining if both pointers are set.
> > > 
> > 
> > The patch that causes this is about to be reverted later today (it's
> > already reverted in my trees), as it was the wrong thing to do, as you
> > point out.
> > 
> > So this should be solved soon, no need for your patch.
> 
> Any news on this? I can't see any fix in kernel 2.6.30-rc5.

It was sent to Linus about 8 hours ago, right after 2.6.30-rc5 came out.
See the patches on lkml if you are interested.

thanks,

greg k-h