Message ID | f37bfdccfcf8a8d1bdcbf668b802df13253ed3fe.1650556911.git.christophe.jaillet@wanadoo.fr |
---|---|
State | New |
Headers | show |
Series | fsi: Fix error handling in fsi_master_register() and its callers | expand |
Le 21/04/2022 à 18:02, Christophe JAILLET a écrit : > fsi_master_register() calls device_register(). > When device_register() fails, put_device() still needs to be called. > > Up to now, there is no put_device() in fsi_master_register(). > Some callers of fsi_master_register() have it, some have not. > - fsi_master_acf_probe() call put_device() if fsi_master_register() fails > - fsi_master_gpio_probe() call put_device() if fsi_master_register() fails > - fsi_master_aspeed_probe() doesn't > - hub_master_probe() doesn't > > In order to fix it and be consistent with the different callers, add the > missing put_device() in the error handling path of fsi_master_register() > and remove it from 2 callers that were handling it by themselves. > > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > --- > This patch is speculative and compile tested only. > Review with care. > > Another alternative would be to add the put_device() call in > fsi_master_aspeed_probe() and hub_master_probe() if it makes more sense. > > Having one or more Fixes tag is a bit hard because of the relations and > log history of the 5 files involved in this bug fix. > --- > drivers/fsi/fsi-core.c | 1 + > drivers/fsi/fsi-master-ast-cf.c | 1 - > drivers/fsi/fsi-master-gpio.c | 1 - > 3 files changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c > index 3a7b78e36701..640692e5400f 100644 > --- a/drivers/fsi/fsi-core.c > +++ b/drivers/fsi/fsi-core.c > @@ -1319,6 +1319,7 @@ int fsi_master_register(struct fsi_master *master) > > rc = device_register(&master->dev); > if (rc) { > + put_device(&master->dev); > ida_simple_remove(&master_ida, master->idx); > return rc; > } > diff --git a/drivers/fsi/fsi-master-ast-cf.c b/drivers/fsi/fsi-master-ast-cf.c > index 24292acdbaf8..dde63d703ea1 100644 > --- a/drivers/fsi/fsi-master-ast-cf.c > +++ b/drivers/fsi/fsi-master-ast-cf.c > @@ -1395,7 +1395,6 @@ static int fsi_master_acf_probe(struct platform_device *pdev) > return 0; > > device_remove_file(master->dev, &dev_attr_external_mode); > - put_device(&master->master.dev); > return rc; > > stop_copro: > diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c > index 7d5f29b4b595..1ae3c164d7fd 100644 > --- a/drivers/fsi/fsi-master-gpio.c > +++ b/drivers/fsi/fsi-master-gpio.c > @@ -856,7 +856,6 @@ static int fsi_master_gpio_probe(struct platform_device *pdev) > rc = fsi_master_register(&master->master); > if (rc) { > device_remove_file(&pdev->dev, &dev_attr_external_mode); > - put_device(&master->master.dev); > return rc; > } > return 0; NACK After additional reading of the code, it looks good to me as-is. Usage in fsi_master_aspeed_probe() and hub_master_probe() seem to have no release function. CJ
diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c index 3a7b78e36701..640692e5400f 100644 --- a/drivers/fsi/fsi-core.c +++ b/drivers/fsi/fsi-core.c @@ -1319,6 +1319,7 @@ int fsi_master_register(struct fsi_master *master) rc = device_register(&master->dev); if (rc) { + put_device(&master->dev); ida_simple_remove(&master_ida, master->idx); return rc; } diff --git a/drivers/fsi/fsi-master-ast-cf.c b/drivers/fsi/fsi-master-ast-cf.c index 24292acdbaf8..dde63d703ea1 100644 --- a/drivers/fsi/fsi-master-ast-cf.c +++ b/drivers/fsi/fsi-master-ast-cf.c @@ -1395,7 +1395,6 @@ static int fsi_master_acf_probe(struct platform_device *pdev) return 0; device_remove_file(master->dev, &dev_attr_external_mode); - put_device(&master->master.dev); return rc; stop_copro: diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c index 7d5f29b4b595..1ae3c164d7fd 100644 --- a/drivers/fsi/fsi-master-gpio.c +++ b/drivers/fsi/fsi-master-gpio.c @@ -856,7 +856,6 @@ static int fsi_master_gpio_probe(struct platform_device *pdev) rc = fsi_master_register(&master->master); if (rc) { device_remove_file(&pdev->dev, &dev_attr_external_mode); - put_device(&master->master.dev); return rc; } return 0;
fsi_master_register() calls device_register(). When device_register() fails, put_device() still needs to be called. Up to now, there is no put_device() in fsi_master_register(). Some callers of fsi_master_register() have it, some have not. - fsi_master_acf_probe() call put_device() if fsi_master_register() fails - fsi_master_gpio_probe() call put_device() if fsi_master_register() fails - fsi_master_aspeed_probe() doesn't - hub_master_probe() doesn't In order to fix it and be consistent with the different callers, add the missing put_device() in the error handling path of fsi_master_register() and remove it from 2 callers that were handling it by themselves. Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> --- This patch is speculative and compile tested only. Review with care. Another alternative would be to add the put_device() call in fsi_master_aspeed_probe() and hub_master_probe() if it makes more sense. Having one or more Fixes tag is a bit hard because of the relations and log history of the 5 files involved in this bug fix. --- drivers/fsi/fsi-core.c | 1 + drivers/fsi/fsi-master-ast-cf.c | 1 - drivers/fsi/fsi-master-gpio.c | 1 - 3 files changed, 1 insertion(+), 2 deletions(-)