| Message ID | 1311176068-9934-1-git-send-email-wanlong.gao@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jul 20, 2011 at 11:34:28PM +0800, Wanlong Gao wrote: > It's better to set the device's driver data to NULL > when remove it. > I'd rather have platform devices core clean up this pointer, then we could stop caring about it in all drivers... Thanks.
On 07/25/2011 04:30 PM, Dmitry Torokhov wrote: > On Wed, Jul 20, 2011 at 11:34:28PM +0800, Wanlong Gao wrote: >> It's better to set the device's driver data to NULL >> when remove it. >> > > I'd rather have platform devices core clean up this pointer, then we > could stop caring about it in all drivers... But the platform devices core just call the method of each own. And don't care about the details like pdata, etc. Meanwhile, I think the platform core need not care about these details. and Greg, what do you think about this?
On Mon, Jul 25, 2011 at 05:14:11PM +0800, Wanlong Gao wrote: > On 07/25/2011 04:30 PM, Dmitry Torokhov wrote: > >On Wed, Jul 20, 2011 at 11:34:28PM +0800, Wanlong Gao wrote: > >>It's better to set the device's driver data to NULL > >>when remove it. > >> > > > >I'd rather have platform devices core clean up this pointer, then we > >could stop caring about it in all drivers... > But the platform devices core just call the method of each own. > And don't care about the details like pdata, etc. > > Meanwhile, I think the platform core need not care about these details. > > and Greg, what do you think about this? I don't understand what you are asking me. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2011-07-25 at 08:21 -0700, Greg KH wrote: > On Mon, Jul 25, 2011 at 05:14:11PM +0800, Wanlong Gao wrote: > > On 07/25/2011 04:30 PM, Dmitry Torokhov wrote: > > >On Wed, Jul 20, 2011 at 11:34:28PM +0800, Wanlong Gao wrote: > > >>It's better to set the device's driver data to NULL > > >>when remove it. > > >> > > > > > >I'd rather have platform devices core clean up this pointer, then we > > >could stop caring about it in all drivers... > > But the platform devices core just call the method of each own. > > And don't care about the details like pdata, etc. > > > > Meanwhile, I think the platform core need not care about these details. > > > > and Greg, what do you think about this? > > I don't understand what you are asking me. Sorry, that's below: firt for this patch: > It's better to set the device's driver data to NULL > when remove it. > > Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com> > --- > drivers/input/misc/pcap_keys.c | 2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) > > diff --git a/drivers/input/misc/pcap_keys.c > b/drivers/input/misc/pcap_keys.c > index 99335c2..6c670f5 100644 > --- a/drivers/input/misc/pcap_keys.c > +++ b/drivers/input/misc/pcap_keys.c > @@ -114,6 +114,8 @@ static int __devexit pcap_keys_remove(struct > platform_device *pdev) > input_unregister_device(pcap_keys->input); > kfree(pcap_keys); > > + platform_set_drvdata(pdev, NULL); > + > return 0; > } > then, Dmitry said that: > I'd rather have platform devices core clean up this pointer, then we > could stop caring about it in all drivers... > then I said: > But the platform devices core just call the method of each own. > And don't care about the details like pdata, etc. > > Meanwhile, I think the platform core need not care about these > details. > > and Greg, what do you think about this? > this is the thread, Greg, understand now? For short, it's all about the platform driver data. Since many drivers set the platform driver data to NULL when it is removed, then Dmitry think it should be done in the platform driver core instead. So, I asked you for this. It's clear now? Thanks a lot -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jul 25, 2011 at 11:34:29PM +0800, Wanlong Gao wrote: > > > On 07/25/2011 04:30 PM, Dmitry Torokhov wrote: > > > I'd rather have platform devices core clean up this pointer, then we > > could stop caring about it in all drivers... > > > > then I said: > > > But the platform devices core just call the method of each own. > > And don't care about the details like pdata, etc. > > > > Meanwhile, I think the platform core need not care about these > > details. > > > > and Greg, what do you think about this? > > > this is the thread, Greg, understand now? > > For short, it's all about the platform driver data. Since many drivers > set the platform driver data to NULL when it is removed, then Dmitry > think it should be done in the platform driver core instead. Right, like i2c bus we could just have platform core clean up platform drvdata pointer after calling ->remove() and also if ->probe() errors out. Then individual drivers do not have to care about cleaning up this pointer. Thanks.
On Mon, Jul 25, 2011 at 08:21:06AM -0700, Greg KH wrote: > On Mon, Jul 25, 2011 at 05:14:11PM +0800, Wanlong Gao wrote: > > On 07/25/2011 04:30 PM, Dmitry Torokhov wrote: > > >I'd rather have platform devices core clean up this pointer, then we > > >could stop caring about it in all drivers... > > But the platform devices core just call the method of each own. > > And don't care about the details like pdata, etc. > > Meanwhile, I think the platform core need not care about these details. > > and Greg, what do you think about this? > I don't understand what you are asking me. They're asking if drivers should set driver_data to NULL while unbinding, which always struck me as a waste of time given that nothing except a currently bound driver should be using driver_data. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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). > Then individual drivers do not have to care about cleaning up this > pointer. They don't have to worry about it anyway, the only thing that is allowed to use the pointer is a currently bound driver and it's only allowed to rely on is that while it's bound it'll get back the same driver_data that it put in. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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(). > > Then individual drivers do not have to care about cleaning up this > > pointer. > > They don't have to worry about it anyway, the only thing that is allowed > to use the pointer is a currently bound driver and it's only allowed to > rely on is that while it's bound it'll get back the same driver_data > that it put in. Right, except that some people trying to use this pointers to pass platform data to the driver... Resetting the pointer to NULL on unbind will hopefully show them their mistake.
On Mon, Jul 25, 2011 at 07:26:28PM +0100, Mark Brown wrote: > On Mon, Jul 25, 2011 at 08:21:06AM -0700, Greg KH wrote: > > On Mon, Jul 25, 2011 at 05:14:11PM +0800, Wanlong Gao wrote: > > > On 07/25/2011 04:30 PM, Dmitry Torokhov wrote: > > > > >I'd rather have platform devices core clean up this pointer, then we > > > >could stop caring about it in all drivers... > > > > But the platform devices core just call the method of each own. > > > And don't care about the details like pdata, etc. > > > > Meanwhile, I think the platform core need not care about these details. > > > > and Greg, what do you think about this? > > > I don't understand what you are asking me. > > They're asking if drivers should set driver_data to NULL while > unbinding, which always struck me as a waste of time given that > nothing except a currently bound driver should be using driver_data. Yeah, it's not needed, as nothing should rely on that. However it's also not hurting anything either. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jul 25, 2011 at 04:25:33PM -0700, Greg KH wrote: > On Mon, Jul 25, 2011 at 07:26:28PM +0100, Mark Brown wrote: > > They're asking if drivers should set driver_data to NULL while > > unbinding, which always struck me as a waste of time given that > > nothing except a currently bound driver should be using driver_data. > Yeah, it's not needed, as nothing should rely on that. However it's > also not hurting anything either. Every time I see it it always sets off a red flag, partly because we shouldn't need to bother at all and partly because to the extent it's valuable it's not something we should be doing on a driver by driver or even subsystem by subsystem basis. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/input/misc/pcap_keys.c b/drivers/input/misc/pcap_keys.c index 99335c2..6c670f5 100644 --- a/drivers/input/misc/pcap_keys.c +++ b/drivers/input/misc/pcap_keys.c @@ -114,6 +114,8 @@ static int __devexit pcap_keys_remove(struct platform_device *pdev) input_unregister_device(pcap_keys->input); kfree(pcap_keys); + platform_set_drvdata(pdev, NULL); + return 0; }
It's better to set the device's driver data to NULL when remove it. Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com> --- drivers/input/misc/pcap_keys.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)