On Fri, Aug 17, 2012 at 02:16:28AM -0700, Bill Huang wrote: [...] > diff --git a/drivers/mfd/tps6586x.c b/drivers/mfd/tps6586x.c [...] > @@ -505,6 +519,11 @@ static int __devinit tps6586x_i2c_probe(struct i2c_client *client, > goto err_add_devs; > } > > + tps6586x_dev = &client->dev; > + > + if (pdata->pm_off && !pm_power_off) > + pm_power_off = tps6586x_power_off; > + I think the assignment of tps6586x_dev needs to go inside the if block as well. Otherwise it might be overwritten by another instance for systems that actually have more than one tps6586x. Since currently the driver can't be built as a module it probably makes little sense to clean this up in .remove(), but it might still be worth adding so it isn't forgotten if and when somebody tries to convert the driver to a module. I should note that I don't like very much how the pm_power_off works. Maybe this should really be changed to allow passing a context for the function to work from. Something like: pm_power_off = tps6586x_power_off; pm_power_off_data = &client->dev; Where pm_power_off() would receive pm_power_off_data as an argument. Even that's not very pretty. On the other hand this doesn't really buy us much because only the storage location of the variable would change and nothing else. But it would still make the association of the data clearer. Thierry
Attachment:
pgp4YIJFfJkGu.pgp
Description: PGP signature
