diff mbox series

[2/2] fsi: Lock mutex for master device registration

Message ID 20230809180814.151984-3-eajames@linux.ibm.com
State New
Headers
Series fsi: Improve master indexing |

Commit Message

Eddie James Aug. 9, 2023, 6:08 p.m. UTC 
  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(-)

Comments

Joel Stanley Aug. 10, 2023, 5:36 a.m. UTC | #1 
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
>
diff mbox series

Patch

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 {
+		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);