From: Paolo 'Blaisorblade' Giarrusso <[email protected]>
platform_device_add calls device_register which calls then again
device_initialize, redundantly.
Cc: Greg Kroah-Hartman <[email protected]>
Signed-off-by: Paolo 'Blaisorblade' Giarrusso <[email protected]>
---
drivers/base/platform.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 83f5c59..b0d9bd4 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -317,7 +317,6 @@ EXPORT_SYMBOL_GPL(platform_device_del);
*/
int platform_device_register(struct platform_device * pdev)
{
- device_initialize(&pdev->dev);
return platform_device_add(pdev);
}
EXPORT_SYMBOL_GPL(platform_device_register);
On Fri, May 05, 2006 at 05:39:08PM +0200, Paolo 'Blaisorblade' Giarrusso wrote:
> From: Paolo 'Blaisorblade' Giarrusso <[email protected]>
>
> platform_device_add calls device_register which calls then again
> device_initialize, redundantly.
platform_device_add should be using device_add not device_register.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
On Fri, 05 May 2006 17:39:08 +0200
"Paolo 'Blaisorblade' Giarrusso" <[email protected]> wrote:
> From: Paolo 'Blaisorblade' Giarrusso <[email protected]>
>
> platform_device_add calls device_register which calls then again
> device_initialize, redundantly.
>
> Cc: Greg Kroah-Hartman <[email protected]>
> Signed-off-by: Paolo 'Blaisorblade' Giarrusso <[email protected]>
> ---
>
> drivers/base/platform.c | 1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index 83f5c59..b0d9bd4 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -317,7 +317,6 @@ EXPORT_SYMBOL_GPL(platform_device_del);
> */
> int platform_device_register(struct platform_device * pdev)
> {
> - device_initialize(&pdev->dev);
> return platform_device_add(pdev);
> }
> EXPORT_SYMBOL_GPL(platform_device_register);
platform_device_add() initialises a bunch of things in pdev->dev and _then_
calls device_register(&pdev->dev) which calls device_initialize() to "init
device structure". This happens after we've already done some
initialisation on the thing. This is not nice.
A better design would be to rip out all the device_initialize() calls and
require that the caller run device_initialize() before "add"ing or
"register"ing the platform_device.
And indeed platform_device_alloc() already does that. If that is
sufficient then we're in good shape.
If it is not sufficient then more thought would be needed. We could at
least run device_initialize() at the _start_ of platform_device_add(),
rather than towards the end.
icky stuff.
On Fri, May 05, 2006 at 07:35:42PM -0700, Andrew Morton wrote:
> A better design would be to rip out all the device_initialize() calls and
> require that the caller run device_initialize() before "add"ing or
> "register"ing the platform_device.
No. The caller must not call device_initialise(). They either use
platform_device_alloc() and platform_device_add(), or
platform_device_register(). Same rules apply to these as they
do for device_add() vs device_register().
> And indeed platform_device_alloc() already does that. If that is
> sufficient then we're in good shape.
>
> If it is not sufficient then more thought would be needed. We could at
> least run device_initialize() at the _start_ of platform_device_add(),
> rather than towards the end.
Just remove the call to device_initialise() in platform_device_add() -
that's something I missed when I renamed platform_device_register to
platform_device_add().
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
On Sat, May 06, 2006 at 08:10:36AM +0100, Russell King wrote:
> On Fri, May 05, 2006 at 07:35:42PM -0700, Andrew Morton wrote:
> > And indeed platform_device_alloc() already does that. If that is
> > sufficient then we're in good shape.
> >
> > If it is not sufficient then more thought would be needed. We could at
> > least run device_initialize() at the _start_ of platform_device_add(),
> > rather than towards the end.
>
> Just remove the call to device_initialise() in platform_device_add() -
> that's something I missed when I renamed platform_device_register to
> platform_device_add().
My email last night on the subject was more accurate than this - ETOOEARLY.
Have a patch instead.
# Base git commit: 5528e568a760442e0ec8fd2dea1f0791875a066b
# ([TCP]: Fix snd_cwnd adjustments in tcp_highspeed.c)
#
# Author: Russell King (Sat May 6 08:13:02 BST 2006)
# Committer: Russell King (Sat May 6 08:13:02 BST 2006)
#
# [DRVMODEL] Fix platform_device_add to use device_add
#
# platform_device_add() should be using device_add() rather
# than device_register() - any platform device passed to
# platform_device_add() should have already been initialised,
# either by platform_device_alloc() or platform_device_register().
#
# Signed-off-by: Russell King
#
# drivers/base/platform.c | 2 +-
# 1 files changed, 1 insertions(+), 1 deletions(-)
#
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -275,7 +275,7 @@ int platform_device_add(struct platform_
pr_debug("Registering platform device '%s'. Parent at %s\n",
pdev->dev.bus_id, pdev->dev.parent->bus_id);
- ret = device_register(&pdev->dev);
+ ret = device_add(&pdev->dev);
if (ret == 0)
return ret;
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core