Messages in this thread Patch in this message | | | From | Joel Stanley <> | Date | Thu, 10 Aug 2023 05:36:39 +0000 | Subject | Re: [PATCH 2/2] fsi: Lock mutex for master device registration |
| |
On Wed, 9 Aug 2023 at 18:08, Eddie James <eajames@linux.ibm.com> wrote: > > Because master device registration may cause hub master scans, or > user scans may begin before device registration has ended, so the > master scan lock must be held while registering the device. > > Signed-off-by: Eddie James <eajames@linux.ibm.com> > --- > drivers/fsi/fsi-core.c | 16 +++++++--------- > 1 file changed, 7 insertions(+), 9 deletions(-) > > diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c > index c7a002076292..ad50cdaf8421 100644 > --- a/drivers/fsi/fsi-core.c > +++ b/drivers/fsi/fsi-core.c > @@ -1306,7 +1306,6 @@ static struct class fsi_master_class = { > int fsi_master_register(struct fsi_master *master) > { > int rc; > - struct device_node *np; > > mutex_init(&master->scan_lock); > > @@ -1326,20 +1325,19 @@ int fsi_master_register(struct fsi_master *master) > > master->dev.class = &fsi_master_class; > > + mutex_lock(&master->scan_lock); > rc = device_register(&master->dev); > if (rc) { > ida_free(&master_ida, master->idx); > - return rc; > - } > + } else {
I kept getting stuck on the "if else" behaviour, but really the else is just the non-error path following the device_register.
I reworked the patch to be like this:
--- a/drivers/fsi/fsi-core.c +++ b/drivers/fsi/fsi-core.c @@ -1326,20 +1326,21 @@ int fsi_master_register(struct fsi_master *master)
master->dev.class = &fsi_master_class;
+ mutex_lock(&master->scan_lock); rc = device_register(&master->dev); if (rc) { ida_free(&master_ida, master->idx); - return rc; + goto out; }
np = dev_of_node(&master->dev); if (!of_property_read_bool(np, "no-scan-on-init")) { - mutex_lock(&master->scan_lock); fsi_master_scan(master); - mutex_unlock(&master->scan_lock); }
- return 0; +out: + mutex_unlock(&master->scan_lock); + return rc; } EXPORT_SYMBOL_GPL(fsi_master_register);
If you don't mind that style, can you send a new version with that? > + struct device_node *np = dev_of_node(&master->dev); > > - np = dev_of_node(&master->dev); > - if (!of_property_read_bool(np, "no-scan-on-init")) { > - mutex_lock(&master->scan_lock); > - fsi_master_scan(master); > - mutex_unlock(&master->scan_lock); > + if (!of_property_read_bool(np, "no-scan-on-init")) > + fsi_master_scan(master); > } > > - return 0; > + mutex_unlock(&master->scan_lock); > + return rc; > } > EXPORT_SYMBOL_GPL(fsi_master_register); > > -- > 2.39.3 >
| |