Fix some issues with FSI scanning and device registration.
Eddie James (2):
fsi: Improve master indexing
fsi: Lock mutex for master device registration
drivers/fsi/fsi-core.c | 40 +++++++++++++++++++----------------
drivers/fsi/fsi-master-i2cr.c | 4 +---
drivers/fsi/fsi-master-i2cr.h | 9 ++++++++
drivers/fsi/fsi-master.h | 1 -
drivers/fsi/i2cr-scom.c | 2 +-
5 files changed, 33 insertions(+), 23 deletions(-)
--
2.39.3
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 <[email protected]>
---
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 {
+ 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
Master indexing is problematic if a hub is rescanned while the
root master is being rescanned. Always allocate an index for the
FSI master, and set the device name if it hasn't already been set.
Move the call to ida_free to the bottom of master unregistration
and set the number of links to 0 in case another call to scan
comes in before the device is removed.
Signed-off-by: Eddie James <[email protected]>
---
drivers/fsi/fsi-core.c | 24 +++++++++++++++---------
drivers/fsi/fsi-master-i2cr.c | 4 +---
drivers/fsi/fsi-master-i2cr.h | 9 +++++++++
drivers/fsi/fsi-master.h | 1 -
drivers/fsi/i2cr-scom.c | 2 +-
5 files changed, 26 insertions(+), 14 deletions(-)
diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index 86b3108ea753..c7a002076292 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -1310,13 +1310,19 @@ int fsi_master_register(struct fsi_master *master)
mutex_init(&master->scan_lock);
- if (!master->idx) {
+ /* Alloc the requested index if it's non-zero */
+ if (master->idx) {
+ master->idx = ida_alloc_range(&master_ida, master->idx,
+ master->idx, GFP_KERNEL);
+ } else {
master->idx = ida_alloc(&master_ida, GFP_KERNEL);
- if (master->idx < 0)
- return master->idx;
+ }
+
+ if (master->idx < 0)
+ return master->idx;
+ if (!dev_name(&master->dev))
dev_set_name(&master->dev, "fsi%d", master->idx);
- }
master->dev.class = &fsi_master_class;
@@ -1339,17 +1345,17 @@ EXPORT_SYMBOL_GPL(fsi_master_register);
void fsi_master_unregister(struct fsi_master *master)
{
- trace_fsi_master_unregister(master);
+ int idx = master->idx;
- if (master->idx >= 0) {
- ida_free(&master_ida, master->idx);
- master->idx = -1;
- }
+ trace_fsi_master_unregister(master);
mutex_lock(&master->scan_lock);
fsi_master_unscan(master);
+ master->n_links = 0;
mutex_unlock(&master->scan_lock);
+
device_unregister(&master->dev);
+ ida_free(&master_ida, idx);
}
EXPORT_SYMBOL_GPL(fsi_master_unregister);
diff --git a/drivers/fsi/fsi-master-i2cr.c b/drivers/fsi/fsi-master-i2cr.c
index e4e8a5931ca3..61659c27a973 100644
--- a/drivers/fsi/fsi-master-i2cr.c
+++ b/drivers/fsi/fsi-master-i2cr.c
@@ -267,13 +267,12 @@ static int i2cr_probe(struct i2c_client *client)
/* Only one I2CR on any given I2C bus (fixed I2C device address) */
i2cr->master.idx = client->adapter->nr;
- dev_set_name(&i2cr->master.dev, "i2cr%d",i2cr->master.idx);
+ dev_set_name(&i2cr->master.dev, "i2cr%d", i2cr->master.idx);
i2cr->master.dev.parent = &client->dev;
i2cr->master.dev.of_node = of_node_get(dev_of_node(&client->dev));
i2cr->master.dev.release = i2cr_release;
i2cr->master.n_links = 1;
- i2cr->master.flags = FSI_MASTER_FLAG_I2CR;
i2cr->master.read = i2cr_read;
i2cr->master.write = i2cr_write;
@@ -292,7 +291,6 @@ static void i2cr_remove(struct i2c_client *client)
{
struct fsi_master_i2cr *i2cr = i2c_get_clientdata(client);
- i2cr->master.idx = -1;
fsi_master_unregister(&i2cr->master);
}
diff --git a/drivers/fsi/fsi-master-i2cr.h b/drivers/fsi/fsi-master-i2cr.h
index 929d63995c7b..96636bf28cac 100644
--- a/drivers/fsi/fsi-master-i2cr.h
+++ b/drivers/fsi/fsi-master-i2cr.h
@@ -4,6 +4,7 @@
#ifndef DRIVERS_FSI_MASTER_I2CR_H
#define DRIVERS_FSI_MASTER_I2CR_H
+#include <linux/i2c.h>
#include <linux/mutex.h>
#include "fsi-master.h"
@@ -21,4 +22,12 @@ struct fsi_master_i2cr {
int fsi_master_i2cr_read(struct fsi_master_i2cr *i2cr, u32 addr, u64 *data);
int fsi_master_i2cr_write(struct fsi_master_i2cr *i2cr, u32 addr, u64 data);
+static inline bool is_fsi_master_i2cr(struct fsi_master *master)
+{
+ if (master->dev.parent && master->dev.parent->type == &i2c_client_type)
+ return true;
+
+ return false;
+}
+
#endif /* DRIVERS_FSI_MASTER_I2CR_H */
diff --git a/drivers/fsi/fsi-master.h b/drivers/fsi/fsi-master.h
index a1fa315849d2..967622c1cabf 100644
--- a/drivers/fsi/fsi-master.h
+++ b/drivers/fsi/fsi-master.h
@@ -111,7 +111,6 @@
/* fsi-master definition and flags */
#define FSI_MASTER_FLAG_SWCLOCK 0x1
-#define FSI_MASTER_FLAG_I2CR 0x2
/*
* Structures and function prototypes
diff --git a/drivers/fsi/i2cr-scom.c b/drivers/fsi/i2cr-scom.c
index 63b548bdef3e..cb7e02213032 100644
--- a/drivers/fsi/i2cr-scom.c
+++ b/drivers/fsi/i2cr-scom.c
@@ -88,7 +88,7 @@ static int i2cr_scom_probe(struct device *dev)
int didx;
int ret;
- if (!(fsi_dev->slave->master->flags & FSI_MASTER_FLAG_I2CR))
+ if (!is_fsi_master_i2cr(fsi_dev->slave->master))
return -ENODEV;
scom = devm_kzalloc(dev, sizeof(*scom), GFP_KERNEL);
--
2.39.3
On Wed, 9 Aug 2023 at 18:08, Eddie James <[email protected]> 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 <[email protected]>
> ---
> 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
>