On Tue, Jul 26, 2011 at 09:29:59AM +0800, Wanlong Gao wrote:
> On 07/26/2011 02:37 AM, Dmitry Torokhov wrote:
> >On Mon, Jul 25, 2011 at 07:29:28PM +0100, Mark Brown wrote:
> >>On Mon, Jul 25, 2011 at 11:19:16AM -0700, Dmitry Torokhov wrote:
> >>
> >>>Right, like i2c bus we could just have platform core clean up platform
> >>>drvdata pointer after calling ->remove() and also if ->probe() errors
> >>>out.
> >>
> >>I2C doesn't do this (at least not any more).
> >>
> >
> >Sure does. See drivers/i2c/i2c-core.c::i2c_device_probe() and
> >i2c_device_remove().
> Yeah, I see it. Sure i2c does.
> Let the Core to do the pointer's clean up is very good idea.
No it isn't.
> So, Dmitry, do you means this ?
>
> Signed-off-by: Wanlong Gao <gaowanlong@xxxxxxxxxxxxxx>
> ---
> drivers/base/platform.c | 30 ++++++++++++++++++++++++++++--
> 1 files changed, 28 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index 6040717..349e71b 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -405,8 +405,21 @@ static int platform_drv_probe(struct device *_dev)
> {
> struct platform_driver *drv = to_platform_driver(_dev->driver);
> struct platform_device *dev = to_platform_device(_dev);
> + int status;
>
> - return drv->probe(dev);
> + if (!dev)
> + return 0;
When would dev ever be NULL? Wouldn't that be an error?
> +
> + if (!drv->probe)
> + return -ENODEV;
That's not a valid error for this. And when would probe ever be NULL?
> +
> + dev_dbg(_dev, "probe\n");
Why add this noise?
> +
> + status = drv->probe(dev);
> + if (status)
> + platform_set_drvdata(dev, NULL);
No, not ok.
> +
> + return status;
> }
>
> static int platform_drv_probe_fail(struct device *_dev)
> @@ -418,8 +431,21 @@ static int platform_drv_remove(struct device *_dev)
> {
> struct platform_driver *drv = to_platform_driver(_dev->driver);
> struct platform_device *dev = to_platform_device(_dev);
> + int status;
> +
> + if (!dev)
> + return 0;
Again, when would that ever happen?
> +
> + if (drv->remove) {
> + dev_dbg(_dev, "remove\n");
> + status = drv->remove(dev);
> + } else {
> + status = 0;
> + }
Again, why would remove ever be NULL?
This whole thing isn't needed at all.
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
