From: David Brownell <[email protected]>
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 <[email protected]>
---
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];
On Mon, Apr 27, 2009 at 07:43:40PM -0700, David Brownell wrote:
> From: David Brownell <[email protected]>
>
> 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
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
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
On Mon, Apr 27, 2009 at 07:43:40PM -0700, David Brownell wrote:
> From: David Brownell <[email protected]>
>
> 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 <[email protected]>
> ---
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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
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
>
>
2009/4/29 Greg KH <[email protected]>:
>
> 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
On Wed, Apr 29, 2009 at 08:08:10PM +0800, Ming Lei wrote:
> 2009/4/29 Greg KH <[email protected]>:
> >
> > 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
2009/4/30 Greg KH <[email protected]>:
> On Wed, Apr 29, 2009 at 08:08:10PM +0800, Ming Lei wrote:
>> 2009/4/29 Greg KH <[email protected]>:
>> >
>> > 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