[patch 2.6.30-rc3] platform_bus: remove "which platform_data?" confusion

[patch 2.6.30-rc3] platform_bus: remove "which platform_data?" confusion

[David Brownell]

From: David Brownell <dbrownell@users.sourceforge.net>

Recent patches have caused platform_device_add_pdata() users
to trigger confused "use which platform_data?" error messages
from the kernel.  ("The _only_ one you were given, dummy!" is
the correct answer.)

This patch fixes those messages so they only appear if there's
reason to be confused.  (The call should probably fail too...)

Those patches seem to support what I think is a misguided
notion:  that somehow device.platform_data might move into
the platform_device.  The problem with that idea is that it's
a general purpose hook, and is used by other busses to provide
board-specific configuration data ... not just for platform_bus.

Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
---
For 2.6.30, which recently acquired this confusion.

 drivers/base/platform.c |   22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -251,16 +251,20 @@ int platform_device_add(struct platform_
 	* if all platform devices pass its platform specific data
 	* from platform_device. The conversion is going to be a
 	* 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
-			       "%s: use which platform_data?\n",
-			       dev_name(&pdev->dev));
-	} else if (pdev->platform_data) {
-		pdev->dev.platform_data = pdev->platform_data;
-	} else if (pdev->dev.platform_data) {
+	* this kind of fix more easily
+	*
+	* REVISIT platform_data is used by more than "platform_bus".
+	* It's a generic hook for data that's specific to a given
+	* board.  Removing it seems rather impractical...
+	*/
+	if (!pdev->platform_data)
 		pdev->platform_data = pdev->dev.platform_data;
-	}
+	else if (!pdev->dev.platform_data)
+		pdev->dev.platform_data = pdev->platform_data;
+
+	if (pdev->platform_data != pdev->dev.platform_data)
+		pr_err("%s: use which platform_data?\n",
+			       dev_name(&pdev->dev));
 
 	for (i = 0; i < pdev->num_resources; i++) {
 		struct resource *p, *r = &pdev->resource[i];

Re: [patch 2.6.30-rc3] platform_bus: remove "which platform_data?" confusion

[Greg KH]

On Mon, Apr 27, 2009 at 07:43:40PM -0700, David Brownell wrote:
> From: David Brownell <dbrownell@users.sourceforge.net>
> 
> Recent patches have caused platform_device_add_pdata() users
> to trigger confused "use which platform_data?" error messages
> from the kernel.  ("The _only_ one you were given, dummy!" is
> the correct answer.)
> 
> This patch fixes those messages so they only appear if there's
> reason to be confused.  (The call should probably fail too...)
> 
> Those patches seem to support what I think is a misguided
> notion:  that somehow device.platform_data might move into
> the platform_device.  The problem with that idea is that it's
> a general purpose hook, and is used by other busses to provide
> board-specific configuration data ... not just for platform_bus.

It is?  What other busses do this?  And why, can't they use their own
bus private data pointers?

thanks,

greg k-h

Re: [patch 2.6.30-rc3] platform_bus: remove "which platform_data?" confusion

[David Brownell]

No comment on the bugfix part of $SUBJECT patch?


On Monday 27 April 2009, Greg KH wrote:
> 
> > Those patches seem to support what I think is a misguided
> > notion:  that somehow device.platform_data might move into
> > the platform_device.  The problem with that idea is that it's
> > a general purpose hook, and is used by other busses to provide
> > board-specific configuration data ... not just for platform_bus.
> 
> It is?  What other busses do this?

SPI and I2C come quickly to mind...

Basically, *any* bus that could ever be used on an embedded
system may need platform_data to explain how each discrete
chip has been wired up on that particular board.  Very few
such busses can self-enumerate like PCI or USB.  And most of
the chips sitting on such busses expect to interface to fairly
random external hardware.

And come to think of it, I've seen cases with PCI and USB
where board-specific config data is needed.  PCI doesn't
always wrap it up in some ACPI bytecode, and sometimes USB
devices use "transceiverless link" hookup, so the board
can just hook up using a differential pair.

SDIO/MMC doesn't tend to need it though, even for SDIO
WLAN or MMC/SD storage links (eMMC, CE-ATA, etc).


> And why, can't they use their own bus private data pointers?

ENOPATCH.  ;)

Though ... since devices on *any* bus may need this, I
don't much see the point of modifying every bus like that.

- Dave

Re: [patch 2.6.30-rc3] platform_bus: remove "which platform_data?" confusion

[Greg KH]

On Tue, Apr 28, 2009 at 02:28:07AM -0700, David Brownell wrote:
> No comment on the bugfix part of $SUBJECT patch?

Well, no, I'm assuming it is correct :)

Should I just revert the original change, if the fact that busses are
using the platform_data field?

> On Monday 27 April 2009, Greg KH wrote:
> > 
> > > Those patches seem to support what I think is a misguided
> > > notion:  that somehow device.platform_data might move into
> > > the platform_device.  The problem with that idea is that it's
> > > a general purpose hook, and is used by other busses to provide
> > > board-specific configuration data ... not just for platform_bus.
> > 
> > It is?  What other busses do this?
> 
> SPI and I2C come quickly to mind...
> 
> Basically, *any* bus that could ever be used on an embedded
> system may need platform_data to explain how each discrete
> chip has been wired up on that particular board.  Very few
> such busses can self-enumerate like PCI or USB.  And most of
> the chips sitting on such busses expect to interface to fairly
> random external hardware.
> 
> And come to think of it, I've seen cases with PCI and USB
> where board-specific config data is needed.  PCI doesn't
> always wrap it up in some ACPI bytecode, and sometimes USB
> devices use "transceiverless link" hookup, so the board
> can just hook up using a differential pair.
> 
> SDIO/MMC doesn't tend to need it though, even for SDIO
> WLAN or MMC/SD storage links (eMMC, CE-ATA, etc).
> 
> 
> > And why, can't they use their own bus private data pointers?
> 
> ENOPATCH.  ;)
> 
> Though ... since devices on *any* bus may need this, I
> don't much see the point of modifying every bus like that.

Fair enough, no objection from me.

thanks,

greg k-h

Re: [patch 2.6.30-rc3] platform_bus: remove "which platform_data?" confusion

[Greg KH]

On Mon, Apr 27, 2009 at 07:43:40PM -0700, David Brownell wrote:
> From: David Brownell <dbrownell@users.sourceforge.net>
> 
> Recent patches have caused platform_device_add_pdata() users
> to trigger confused "use which platform_data?" error messages
> from the kernel.  ("The _only_ one you were given, dummy!" is
> the correct answer.)
> 
> This patch fixes those messages so they only appear if there's
> reason to be confused.  (The call should probably fail too...)
> 
> Those patches seem to support what I think is a misguided
> notion:  that somehow device.platform_data might move into
> the platform_device.  The problem with that idea is that it's
> a general purpose hook, and is used by other busses to provide
> board-specific configuration data ... not just for platform_bus.
> 
> Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
> ---

Ming, what do you think of this?

thanks,

greg k-h


> For 2.6.30, which recently acquired this confusion.
> 
>  drivers/base/platform.c |   22 +++++++++++++---------
>  1 file changed, 13 insertions(+), 9 deletions(-)
> 
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -251,16 +251,20 @@ int platform_device_add(struct platform_
>  	* if all platform devices pass its platform specific data
>  	* from platform_device. The conversion is going to be a
>  	* 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
> -			       "%s: use which platform_data?\n",
> -			       dev_name(&pdev->dev));
> -	} else if (pdev->platform_data) {
> -		pdev->dev.platform_data = pdev->platform_data;
> -	} else if (pdev->dev.platform_data) {
> +	* this kind of fix more easily
> +	*
> +	* REVISIT platform_data is used by more than "platform_bus".
> +	* It's a generic hook for data that's specific to a given
> +	* board.  Removing it seems rather impractical...
> +	*/
> +	if (!pdev->platform_data)
>  		pdev->platform_data = pdev->dev.platform_data;
> -	}
> +	else if (!pdev->dev.platform_data)
> +		pdev->dev.platform_data = pdev->platform_data;
> +
> +	if (pdev->platform_data != pdev->dev.platform_data)
> +		pr_err("%s: use which platform_data?\n",
> +			       dev_name(&pdev->dev));
>  
>  	for (i = 0; i < pdev->num_resources; i++) {
>  		struct resource *p, *r = &pdev->resource[i];
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [patch 2.6.30-rc3] platform_bus: remove "which platform_data?" confusion

[David Brownell]

On Tuesday 28 April 2009, Greg KH wrote:
> On Tue, Apr 28, 2009 at 02:28:07AM -0700, David Brownell wrote:
> > No comment on the bugfix part of $SUBJECT patch?
> 
> Well, no, I'm assuming it is correct :)
> 
> Should I just revert the original change, if the fact that busses are
> using the platform_data field?

That would be my inclination.


> > On Monday 27 April 2009, Greg KH wrote:
> > > 
> > > > Those patches seem to support what I think is a misguided
> > > > notion:  that somehow device.platform_data might move into
> > > > the platform_device.  The problem with that idea is that it's
> > > > a general purpose hook, and is used by other busses to provide
> > > > board-specific configuration data ... not just for platform_bus.
> > > 
> > > It is?  What other busses do this?
> > 
> > SPI and I2C come quickly to mind...
> > 
> > Basically, *any* bus that could ever be used on an embedded
> > system may need platform_data to explain how each discrete
> > chip has been wired up on that particular board.  Very few
> > such busses can self-enumerate like PCI or USB.  And most of
> > the chips sitting on such busses expect to interface to fairly
> > random external hardware.
> > 
> > And come to think of it, I've seen cases with PCI and USB
> > where board-specific config data is needed.  PCI doesn't
> > always wrap it up in some ACPI bytecode, and sometimes USB
> > devices use "transceiverless link" hookup, so the board
> > can just hook up using a differential pair.
> > 
> > SDIO/MMC doesn't tend to need it though, even for SDIO
> > WLAN or MMC/SD storage links (eMMC, CE-ATA, etc).
> > 
> > 
> > > And why, can't they use their own bus private data pointers?
> > 
> > ENOPATCH.  ;)
> > 
> > Though ... since devices on *any* bus may need this, I
> > don't much see the point of modifying every bus like that.
> 
> Fair enough, no objection from me.
> 
> thanks,
> 
> greg k-h
> 
> 

Re: [patch 2.6.30-rc3] platform_bus: remove "which platform_data?" confusion

[Ming Lei]

2009/4/29 Greg KH <greg@kroah.com>:
>
> Ming, what do you think of this?

I did not know platform_data field can be used in other bus , so please
revert the previous patches.

Thanks!

-- 
Lei Ming

Re: [patch 2.6.30-rc3] platform_bus: remove "which platform_data?" confusion

[Greg KH]

On Wed, Apr 29, 2009 at 08:08:10PM +0800, Ming Lei wrote:
> 2009/4/29 Greg KH <greg@kroah.com>:
> >
> > Ming, what do you think of this?
> 
> I did not know platform_data field can be used in other bus , so please
> revert the previous patches.

So that would be the following 2 commits, right:
	ce21c7bcd796fc4f45d48781b7e85f493cc55ee5
	006f4571a15fae3a0575f2a0f9e9b63b3d1012f8
anything else I need to revert as well?

thanks,

greg k-h

Re: [patch 2.6.30-rc3] platform_bus: remove "which platform_data?" confusion

[Ming Lei]

2009/4/30 Greg KH <greg@kroah.com>:
> On Wed, Apr 29, 2009 at 08:08:10PM +0800, Ming Lei wrote:
>> 2009/4/29 Greg KH <greg@kroah.com>:
>> >
>> > Ming, what do you think of this?
>>
>> I did not know platform_data field can be used in other bus , so please
>> revert the previous patches.
>
> So that would be the following 2 commits, right:
>        ce21c7bcd796fc4f45d48781b7e85f493cc55ee5
>        006f4571a15fae3a0575f2a0f9e9b63b3d1012f8
> anything else I need to revert as well?

Also you need to drop
http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/patches/driver-core.current/driver-core-platform-do-not-complain-for-platform_data-added-by-platform_device_add_data.patch.

Thanks!

-- 
Lei Ming