drivers/fsi: Remove hub devices during master unregister

PATCH In review Christopher Bostic 3 participants 4 messages Mar 7, 2017 – Mar 9, 2017 FSI

Share & tools

Christopher Bostic

author

2017-03-07 16:46 UTC #0 lore raw

PATCH v2 1/1 drivers/fsi: Remove hub devices during master unregister

All hub devices must be removed when the primary FSI master is being unregistered. This ensures that the system is in a clean state when another scan is attempted. Failure to do so will result in duplicate /sys device name errors.

Signed-off-by Christopher Bostic <[email protected]>

---
v2: Fix missing return in non void function warning

---
 drivers/fsi/fsi-core.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

drivers/fsi/fsi-core.c +18 -0

index bd57b41..bbdf6b8 100644
@@ -84,6 +84,7 @@ struct fsi_master_hub {
 #define to_fsi_master_hub(d) container_of(d, struct fsi_master_hub, master)
 #define to_fsi_slave(d) container_of(d, struct fsi_slave, dev)
 
+static void fsi_master_unscan(struct fsi_master *master);
 static int fsi_slave_read(struct fsi_slave *slave, uint32_t addr,
 		void *val, size_t size);
 static int fsi_slave_write(struct fsi_slave *slave, uint32_t addr,

@@ -441,6 +442,7 @@ static int fsi_slave_scan(struct fsi_slave *slave)
 			hub->dev.release = hub_master_release;
 			hub->master.dev = &hub->dev;
 			hub->master.dev->parent = &slave->dev;
+			dev_set_drvdata(&hub->dev, hub);
 			rc = device_add(&hub->dev);
 			if (rc)
 				return rc;

@@ -739,6 +741,20 @@ static int fsi_master_scan(struct fsi_master *master)
 	return 0;
 }
 
+static int fsi_unregister_hubs(struct device *dev, void *data)
+{
+	struct fsi_master_hub *hub =
+				(struct fsi_master_hub *)dev_get_drvdata(dev);
+
+	if (!hub)
+		return 0;
+
+	device_del(dev);
+	fsi_master_unscan(&hub->master);
+
+	return 0;
+}
+
 static void fsi_master_unscan(struct fsi_master *master)
 {
 	struct fsi_slave *slave, *slave_tmp;

@@ -756,6 +772,8 @@ static void fsi_master_unscan(struct fsi_master *master)
 			device_del(&fsi_dev->dev);
 			put_device(&fsi_dev->dev);
 		}
+		/* Remove any hub masters */
+		device_for_each_child(&slave->dev, NULL, fsi_unregister_hubs);
 		device_unregister(&slave->dev);
 	}
 	master->slave_list = false;

Signature · 1 line

1.8.2.2

Eddie James

Acked-by on v2 1/1

2017-03-08 15:06 UTC #1 lore raw

Quoted 54 lines · Christopher Bostic

Signed-off-by Christopher Bostic <[email protected]>

---
v2: Fix missing return in non void function warning

---
  drivers/fsi/fsi-core.c | 18 ++++++++++++++++++
  1 file changed, 18 insertions(+)

drivers/fsi/fsi-core.c +16 -0

index bd57b41..bbdf6b8 100644
@@ -84,6 +84,7 @@ struct fsi_master_hub {
  #define to_fsi_master_hub(d) container_of(d, struct fsi_master_hub, master)
  #define to_fsi_slave(d) container_of(d, struct fsi_slave, dev)
 
+static void fsi_master_unscan(struct fsi_master *master);
  static int fsi_slave_read(struct fsi_slave *slave, uint32_t addr,
  		void *val, size_t size);
  static int fsi_slave_write(struct fsi_slave *slave, uint32_t addr,

@@ -441,6 +442,7 @@ static int fsi_slave_scan(struct fsi_slave *slave)
  			hub->dev.release = hub_master_release;
  			hub->master.dev = &hub->dev;
  			hub->master.dev->parent = &slave->dev;
+			dev_set_drvdata(&hub->dev, hub);
  			rc = device_add(&hub->dev);
  			if (rc)
  				return rc;

@@ -739,6 +741,20 @@ static int fsi_master_scan(struct fsi_master *master)
  	return 0;
  }
 
+static int fsi_unregister_hubs(struct device *dev, void *data)
+{
+	struct fsi_master_hub *hub =
+				(struct fsi_master_hub *)dev_get_drvdata(dev);
+
+	if (!hub)
+		return 0;
+
+	device_del(dev);
+	fsi_master_unscan(&hub->master);
+
+	return 0;
+}
+
  static void fsi_master_unscan(struct fsi_master *master)
  {
  	struct fsi_slave *slave, *slave_tmp;

drivers/fsi/fsi-core.c static void fsi_master_unscan(struct fsi_master *master) · line 779

Quoted 8 lines

drivers/fsi/fsi-core.c +2 -0

@@ -756,6 +772,8 @@ static void fsi_master_unscan(struct fsi_master *master)
  			device_del(&fsi_dev->dev);
  			put_device(&fsi_dev->dev);
  		}
+		/* Remove any hub masters */
+		device_for_each_child(&slave->dev, NULL, fsi_unregister_hubs);
  		device_unregister(&slave->dev);
  	}
  	master->slave_list = false;

Looks good.

Acked-by Eddie James <[email protected]>

Joel Stanley

2017-03-09 10:54 UTC #2 lore raw

In reply to Christopher Bostic

Quoted 33 lines · Christopher Bostic

Signed-off-by Christopher Bostic <[email protected]>

---
v2: Fix missing return in non void function warning

---
 drivers/fsi/fsi-core.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

drivers/fsi/fsi-core.c +2 -0

index bd57b41..bbdf6b8 100644
@@ -84,6 +84,7 @@ struct fsi_master_hub {
 #define to_fsi_master_hub(d) container_of(d, struct fsi_master_hub, master)
 #define to_fsi_slave(d) container_of(d, struct fsi_slave, dev)
 
+static void fsi_master_unscan(struct fsi_master *master);
 static int fsi_slave_read(struct fsi_slave *slave, uint32_t addr,
                void *val, size_t size);
 static int fsi_slave_write(struct fsi_slave *slave, uint32_t addr,

@@ -441,6 +442,7 @@ static int fsi_slave_scan(struct fsi_slave *slave)
                        hub->dev.release = hub_master_release;
                        hub->master.dev = &hub->dev;
                        hub->master.dev->parent = &slave->dev;
+                       dev_set_drvdata(&hub->dev, hub);
                        rc = device_add(&hub->dev);
                        if (rc)
                                return rc;

drivers/fsi/fsi-core.c static int fsi_master_scan(struct fsi_master *master) · line 747

Quoted 7 lines

drivers/fsi/fsi-core.c +4 -0

@@ -739,6 +741,20 @@ static int fsi_master_scan(struct fsi_master *master)
        return 0;
 }
 
+static int fsi_unregister_hubs(struct device *dev, void *data)
+{
+       struct fsi_master_hub *hub =
+                               (struct fsi_master_hub *)dev_get_drvdata(dev);

dev_get_drvdata returns a void *, so there is no need to cast.

I fixed this up when applying.

drivers/fsi/fsi-core.c static int fsi_master_scan(struct fsi_master *master) · line 749

Quoted 2 lines

drivers/fsi/fsi-core.c +2 -0

+       if (!hub)
+               return 0;

Is this expected? Should it return an error?

Quoted 22 lines

drivers/fsi/fsi-core.c +9 -1

+
+       device_del(dev);
+       fsi_master_unscan(&hub->master);
+
+       return 0;
+}
+
 static void fsi_master_unscan(struct fsi_master *master)
 {
        struct fsi_slave *slave, *slave_tmp;

@@ -756,6 +772,8 @@ static void fsi_master_unscan(struct fsi_master *master)
                        device_del(&fsi_dev->dev);
                        put_device(&fsi_dev->dev);
                }
+               /* Remove any hub masters */
+               device_for_each_child(&slave->dev, NULL, fsi_unregister_hubs);
                device_unregister(&slave->dev);
        }
        master->slave_list = false;
--

1.8.2.2

Christopher Bostic

author

2017-03-09 14:06 UTC #3 lore raw

Quoted 44 lines · Joel Stanley

Quoted 33 lines · Christopher Bostic

Signed-off-by Christopher Bostic <[email protected]>

---
v2: Fix missing return in non void function warning

---
  drivers/fsi/fsi-core.c | 18 ++++++++++++++++++
  1 file changed, 18 insertions(+)

drivers/fsi/fsi-core.c +2 -0

index bd57b41..bbdf6b8 100644
@@ -84,6 +84,7 @@ struct fsi_master_hub {
  #define to_fsi_master_hub(d) container_of(d, struct fsi_master_hub, master)
  #define to_fsi_slave(d) container_of(d, struct fsi_slave, dev)
 
+static void fsi_master_unscan(struct fsi_master *master);
  static int fsi_slave_read(struct fsi_slave *slave, uint32_t addr,
                 void *val, size_t size);
  static int fsi_slave_write(struct fsi_slave *slave, uint32_t addr,

@@ -441,6 +442,7 @@ static int fsi_slave_scan(struct fsi_slave *slave)
                         hub->dev.release = hub_master_release;
                         hub->master.dev = &hub->dev;
                         hub->master.dev->parent = &slave->dev;
+                       dev_set_drvdata(&hub->dev, hub);
                         rc = device_add(&hub->dev);
                         if (rc)
                                 return rc;

drivers/fsi/fsi-core.c static int fsi_master_scan(struct fsi_master *master) · line 747

Quoted 7 lines

drivers/fsi/fsi-core.c +4 -0

@@ -739,6 +741,20 @@ static int fsi_master_scan(struct fsi_master *master)
         return 0;
  }
 
+static int fsi_unregister_hubs(struct device *dev, void *data)
+{
+       struct fsi_master_hub *hub =
+                               (struct fsi_master_hub *)dev_get_drvdata(dev);

dev_get_drvdata returns a void *, so there is no need to cast.

Quoted 5 lines

I fixed this up when applying.
drivers/fsi/fsi-core.c static int fsi_master_scan(struct fsi_master *master) · line 749
Quoted 2 lines
drivers/fsi/fsi-core.c +2 -0
+       if (!hub)
+               return 0;
Is this expected? Should it return an error?

Hi Joel,

Yes it would be expected in cases where the child dev being processed is not a hub. Only hub master has its driver data set. In effect !hub means this child is not a hub master.

Thanks
-Chris

Quoted 22 lines

drivers/fsi/fsi-core.c +9 -1

+
+       device_del(dev);
+       fsi_master_unscan(&hub->master);
+
+       return 0;
+}
+
  static void fsi_master_unscan(struct fsi_master *master)
  {
         struct fsi_slave *slave, *slave_tmp;

@@ -756,6 +772,8 @@ static void fsi_master_unscan(struct fsi_master *master)
                         device_del(&fsi_dev->dev);
                         put_device(&fsi_dev->dev);
                 }
+               /* Remove any hub masters */
+               device_for_each_child(&slave->dev, NULL, fsi_unregister_hubs);
                 device_unregister(&slave->dev);
         }
         master->slave_list = false;
--

1.8.2.2