* [PATCH 01/14] fsi: Move fsi_slave structure definition to header
2023-06-12 19:56 [PATCH 00/14] fsi: Miscellaneous fixes and I2C Responder driver Eddie James
@ 2023-06-12 19:56 ` Eddie James
2023-06-12 19:56 ` [PATCH 02/14] fsi: Add aliased device numbering Eddie James
` (12 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Eddie James @ 2023-06-12 19:56 UTC (permalink / raw)
To: linux-fsi
Cc: linux-kernel, jk, joel, alistair, andrew, robh+dt,
krzysztof.kozlowski+dt, conor+dt, devicetree
Some FSI drivers may have need of the slave definition, so
move it to a header file. Also use one macro for obtaining a
pointer to the fsi_master structure.
Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
drivers/fsi/fsi-core.c | 24 ++++--------------------
drivers/fsi/fsi-master-aspeed.c | 2 +-
drivers/fsi/fsi-master-ast-cf.c | 2 +-
drivers/fsi/fsi-master-gpio.c | 2 +-
drivers/fsi/fsi-master-hub.c | 2 +-
drivers/fsi/fsi-master.h | 2 +-
drivers/fsi/fsi-slave.h | 28 ++++++++++++++++++++++++++++
7 files changed, 37 insertions(+), 25 deletions(-)
create mode 100644 drivers/fsi/fsi-slave.h
diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index 0b927c9f4267..d591e68afd11 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -23,6 +23,10 @@
#include <linux/uaccess.h>
#include "fsi-master.h"
+#include "fsi-slave.h"
+
+#define CREATE_TRACE_POINTS
+#include <trace/events/fsi.h>
#define FSI_SLAVE_CONF_NEXT_MASK GENMASK(31, 31)
#define FSI_SLAVE_CONF_SLOTS_MASK GENMASK(23, 16)
@@ -78,26 +82,6 @@ static const int engine_page_size = 0x400;
static DEFINE_IDA(master_ida);
-struct fsi_slave {
- struct device dev;
- struct fsi_master *master;
- struct cdev cdev;
- int cdev_idx;
- int id; /* FSI address */
- int link; /* FSI link# */
- u32 cfam_id;
- int chip_id;
- uint32_t size; /* size of slave address space */
- u8 t_send_delay;
- u8 t_echo_delay;
-};
-
-#define CREATE_TRACE_POINTS
-#include <trace/events/fsi.h>
-
-#define to_fsi_master(d) container_of(d, struct fsi_master, dev)
-#define to_fsi_slave(d) container_of(d, struct fsi_slave, dev)
-
static const int slave_retries = 2;
static int discard_errors;
diff --git a/drivers/fsi/fsi-master-aspeed.c b/drivers/fsi/fsi-master-aspeed.c
index 7cec1772820d..437f87b4a6a3 100644
--- a/drivers/fsi/fsi-master-aspeed.c
+++ b/drivers/fsi/fsi-master-aspeed.c
@@ -376,7 +376,7 @@ static int aspeed_master_break(struct fsi_master *master, int link)
static void aspeed_master_release(struct device *dev)
{
struct fsi_master_aspeed *aspeed =
- to_fsi_master_aspeed(dev_to_fsi_master(dev));
+ to_fsi_master_aspeed(to_fsi_master(dev));
kfree(aspeed);
}
diff --git a/drivers/fsi/fsi-master-ast-cf.c b/drivers/fsi/fsi-master-ast-cf.c
index 5f608ef8b53c..6124978305eb 100644
--- a/drivers/fsi/fsi-master-ast-cf.c
+++ b/drivers/fsi/fsi-master-ast-cf.c
@@ -1190,7 +1190,7 @@ static int fsi_master_acf_gpio_release(void *data)
static void fsi_master_acf_release(struct device *dev)
{
- struct fsi_master_acf *master = to_fsi_master_acf(dev_to_fsi_master(dev));
+ struct fsi_master_acf *master = to_fsi_master_acf(to_fsi_master(dev));
/* Cleanup, stop coprocessor */
mutex_lock(&master->lock);
diff --git a/drivers/fsi/fsi-master-gpio.c b/drivers/fsi/fsi-master-gpio.c
index 7d5f29b4b595..ed03da4f2447 100644
--- a/drivers/fsi/fsi-master-gpio.c
+++ b/drivers/fsi/fsi-master-gpio.c
@@ -761,7 +761,7 @@ static DEVICE_ATTR(external_mode, 0664,
static void fsi_master_gpio_release(struct device *dev)
{
- struct fsi_master_gpio *master = to_fsi_master_gpio(dev_to_fsi_master(dev));
+ struct fsi_master_gpio *master = to_fsi_master_gpio(to_fsi_master(dev));
of_node_put(dev_of_node(master->dev));
diff --git a/drivers/fsi/fsi-master-hub.c b/drivers/fsi/fsi-master-hub.c
index 01f0a796111e..6d8b6e8854e5 100644
--- a/drivers/fsi/fsi-master-hub.c
+++ b/drivers/fsi/fsi-master-hub.c
@@ -105,7 +105,7 @@ static int hub_master_link_enable(struct fsi_master *master, int link,
static void hub_master_release(struct device *dev)
{
- struct fsi_master_hub *hub = to_fsi_master_hub(dev_to_fsi_master(dev));
+ struct fsi_master_hub *hub = to_fsi_master_hub(to_fsi_master(dev));
kfree(hub);
}
diff --git a/drivers/fsi/fsi-master.h b/drivers/fsi/fsi-master.h
index 4762315a46ba..967622c1cabf 100644
--- a/drivers/fsi/fsi-master.h
+++ b/drivers/fsi/fsi-master.h
@@ -136,7 +136,7 @@ struct fsi_master {
u8 t_send_delay, u8 t_echo_delay);
};
-#define dev_to_fsi_master(d) container_of(d, struct fsi_master, dev)
+#define to_fsi_master(d) container_of(d, struct fsi_master, dev)
/**
* fsi_master registration & lifetime: the fsi_master_register() and
diff --git a/drivers/fsi/fsi-slave.h b/drivers/fsi/fsi-slave.h
new file mode 100644
index 000000000000..1d63a585829d
--- /dev/null
+++ b/drivers/fsi/fsi-slave.h
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* Copyright (C) IBM Corporation 2023 */
+
+#ifndef DRIVERS_FSI_SLAVE_H
+#define DRIVERS_FSI_SLAVE_H
+
+#include <linux/cdev.h>
+#include <linux/device.h>
+
+struct fsi_master;
+
+struct fsi_slave {
+ struct device dev;
+ struct fsi_master *master;
+ struct cdev cdev;
+ int cdev_idx;
+ int id; /* FSI address */
+ int link; /* FSI link# */
+ u32 cfam_id;
+ int chip_id;
+ uint32_t size; /* size of slave address space */
+ u8 t_send_delay;
+ u8 t_echo_delay;
+};
+
+#define to_fsi_slave(d) container_of(d, struct fsi_slave, dev)
+
+#endif /* DRIVERS_FSI_SLAVE_H */
--
2.31.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 02/14] fsi: Add aliased device numbering
2023-06-12 19:56 [PATCH 00/14] fsi: Miscellaneous fixes and I2C Responder driver Eddie James
2023-06-12 19:56 ` [PATCH 01/14] fsi: Move fsi_slave structure definition to header Eddie James
@ 2023-06-12 19:56 ` Eddie James
2023-06-12 19:56 ` [PATCH 03/14] fsi: Use of_match_table for bus matching if specified Eddie James
` (11 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Eddie James @ 2023-06-12 19:56 UTC (permalink / raw)
To: linux-fsi
Cc: linux-kernel, jk, joel, alistair, andrew, robh+dt,
krzysztof.kozlowski+dt, conor+dt, devicetree
The I2C and SPI subsystems can use an aliased name to number the device.
Add similar support to the FSI subsystem for any device type.
Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
drivers/fsi/fsi-core.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index d591e68afd11..b77013b9d8a7 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -955,9 +955,34 @@ static int __fsi_get_new_minor(struct fsi_slave *slave, enum fsi_dev_type type,
return 0;
}
+static const char *const fsi_dev_type_names[] = {
+ "cfam",
+ "sbefifo",
+ "scom",
+ "occ",
+};
+
int fsi_get_new_minor(struct fsi_device *fdev, enum fsi_dev_type type,
dev_t *out_dev, int *out_index)
{
+ if (fdev->dev.of_node) {
+ int aid = of_alias_get_id(fdev->dev.of_node, fsi_dev_type_names[type]);
+
+ if (aid >= 0) {
+ int id = (aid << 4) | type;
+
+ id = ida_simple_get(&fsi_minor_ida, id, id + 1, GFP_KERNEL);
+ if (id >= 0) {
+ *out_index = aid;
+ *out_dev = fsi_base_dev + id;
+ return 0;
+ }
+
+ if (id != -ENOSPC)
+ return id;
+ }
+ }
+
return __fsi_get_new_minor(fdev->slave, type, out_dev, out_index);
}
EXPORT_SYMBOL_GPL(fsi_get_new_minor);
--
2.31.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 03/14] fsi: Use of_match_table for bus matching if specified
2023-06-12 19:56 [PATCH 00/14] fsi: Miscellaneous fixes and I2C Responder driver Eddie James
2023-06-12 19:56 ` [PATCH 01/14] fsi: Move fsi_slave structure definition to header Eddie James
2023-06-12 19:56 ` [PATCH 02/14] fsi: Add aliased device numbering Eddie James
@ 2023-06-12 19:56 ` Eddie James
2023-08-17 15:46 ` Rob Herring
2023-06-12 19:56 ` [PATCH 04/14] fsi: sbefifo: Don't check status during probe Eddie James
` (10 subsequent siblings)
13 siblings, 1 reply; 21+ messages in thread
From: Eddie James @ 2023-06-12 19:56 UTC (permalink / raw)
To: linux-fsi
Cc: linux-kernel, jk, joel, alistair, andrew, robh+dt,
krzysztof.kozlowski+dt, conor+dt, devicetree
Since we have two scom drivers, use the standard of matching if
the driver specifies a table so that the right devices go to the
right driver.
Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
drivers/fsi/fsi-core.c | 11 +++++++++--
drivers/fsi/fsi-scom.c | 8 ++++++++
2 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index b77013b9d8a7..ca4a9634fbc3 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -16,6 +16,7 @@
#include <linux/idr.h>
#include <linux/module.h>
#include <linux/of.h>
+#include <linux/of_device.h>
#include <linux/slab.h>
#include <linux/bitops.h>
#include <linux/cdev.h>
@@ -1375,8 +1376,14 @@ static int fsi_bus_match(struct device *dev, struct device_driver *drv)
if (id->engine_type != fsi_dev->engine_type)
continue;
if (id->version == FSI_VERSION_ANY ||
- id->version == fsi_dev->version)
- return 1;
+ id->version == fsi_dev->version) {
+ if (drv->of_match_table) {
+ if (of_driver_match_device(dev, drv))
+ return 1;
+ } else {
+ return 1;
+ }
+ }
}
return 0;
diff --git a/drivers/fsi/fsi-scom.c b/drivers/fsi/fsi-scom.c
index bcb756dc9866..61dbda9dbe2b 100644
--- a/drivers/fsi/fsi-scom.c
+++ b/drivers/fsi/fsi-scom.c
@@ -10,6 +10,7 @@
#include <linux/cdev.h>
#include <linux/delay.h>
#include <linux/fs.h>
+#include <linux/mod_devicetable.h>
#include <linux/uaccess.h>
#include <linux/slab.h>
#include <linux/list.h>
@@ -587,6 +588,12 @@ static int scom_remove(struct device *dev)
return 0;
}
+static const struct of_device_id scom_of_ids[] = {
+ { .compatible = "ibm,fsi2pib" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, scom_of_ids);
+
static const struct fsi_device_id scom_ids[] = {
{
.engine_type = FSI_ENGID_SCOM,
@@ -600,6 +607,7 @@ static struct fsi_driver scom_drv = {
.drv = {
.name = "scom",
.bus = &fsi_bus_type,
+ .of_match_table = scom_of_ids,
.probe = scom_probe,
.remove = scom_remove,
}
--
2.31.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 03/14] fsi: Use of_match_table for bus matching if specified
2023-06-12 19:56 ` [PATCH 03/14] fsi: Use of_match_table for bus matching if specified Eddie James
@ 2023-08-17 15:46 ` Rob Herring
0 siblings, 0 replies; 21+ messages in thread
From: Rob Herring @ 2023-08-17 15:46 UTC (permalink / raw)
To: Eddie James
Cc: linux-fsi, linux-kernel, jk, joel, alistair, andrew,
krzysztof.kozlowski+dt, conor+dt, devicetree
On Mon, Jun 12, 2023 at 02:56:46PM -0500, Eddie James wrote:
> Since we have two scom drivers, use the standard of matching if
> the driver specifies a table so that the right devices go to the
> right driver.
>
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> ---
> drivers/fsi/fsi-core.c | 11 +++++++++--
> drivers/fsi/fsi-scom.c | 8 ++++++++
> 2 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
> index b77013b9d8a7..ca4a9634fbc3 100644
> --- a/drivers/fsi/fsi-core.c
> +++ b/drivers/fsi/fsi-core.c
> @@ -16,6 +16,7 @@
> #include <linux/idr.h>
> #include <linux/module.h>
> #include <linux/of.h>
> +#include <linux/of_device.h>
> #include <linux/slab.h>
> #include <linux/bitops.h>
> #include <linux/cdev.h>
> @@ -1375,8 +1376,14 @@ static int fsi_bus_match(struct device *dev, struct device_driver *drv)
> if (id->engine_type != fsi_dev->engine_type)
> continue;
> if (id->version == FSI_VERSION_ANY ||
> - id->version == fsi_dev->version)
> - return 1;
> + id->version == fsi_dev->version) {
> + if (drv->of_match_table) {
> + if (of_driver_match_device(dev, drv))
> + return 1;
> + } else {
> + return 1;
> + }
> + }
> }
>
> return 0;
> diff --git a/drivers/fsi/fsi-scom.c b/drivers/fsi/fsi-scom.c
> index bcb756dc9866..61dbda9dbe2b 100644
> --- a/drivers/fsi/fsi-scom.c
> +++ b/drivers/fsi/fsi-scom.c
> @@ -10,6 +10,7 @@
> #include <linux/cdev.h>
> #include <linux/delay.h>
> #include <linux/fs.h>
> +#include <linux/mod_devicetable.h>
> #include <linux/uaccess.h>
> #include <linux/slab.h>
> #include <linux/list.h>
> @@ -587,6 +588,12 @@ static int scom_remove(struct device *dev)
> return 0;
> }
>
> +static const struct of_device_id scom_of_ids[] = {
> + { .compatible = "ibm,fsi2pib" },
This is not documented. Please add a binding.
Rob
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 04/14] fsi: sbefifo: Don't check status during probe
2023-06-12 19:56 [PATCH 00/14] fsi: Miscellaneous fixes and I2C Responder driver Eddie James
` (2 preceding siblings ...)
2023-06-12 19:56 ` [PATCH 03/14] fsi: Use of_match_table for bus matching if specified Eddie James
@ 2023-06-12 19:56 ` Eddie James
2023-06-12 19:56 ` [PATCH 05/14] fsi: sbefifo: Add configurable in-command timeout Eddie James
` (9 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Eddie James @ 2023-06-12 19:56 UTC (permalink / raw)
To: linux-fsi
Cc: linux-kernel, jk, joel, alistair, andrew, robh+dt,
krzysztof.kozlowski+dt, conor+dt, devicetree
The status check during probe doesn't serve any purpose. Any attempt
to use the SBEFIFO will result in the same check and cleanup.
Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
drivers/fsi/fsi-sbefifo.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/drivers/fsi/fsi-sbefifo.c b/drivers/fsi/fsi-sbefifo.c
index 9912b7a6a4b9..42d7c95528d1 100644
--- a/drivers/fsi/fsi-sbefifo.c
+++ b/drivers/fsi/fsi-sbefifo.c
@@ -1027,14 +1027,6 @@ static int sbefifo_probe(struct device *dev)
mutex_init(&sbefifo->lock);
sbefifo->timeout_start_rsp_ms = SBEFIFO_TIMEOUT_START_RSP;
- /*
- * Try cleaning up the FIFO. If this fails, we still register the
- * driver and will try cleaning things up again on the next access.
- */
- rc = sbefifo_cleanup_hw(sbefifo);
- if (rc && rc != -ESHUTDOWN)
- dev_err(dev, "Initial HW cleanup failed, will retry later\n");
-
/* Create chardev for userspace access */
sbefifo->dev.type = &fsi_cdev_type;
sbefifo->dev.parent = dev;
--
2.31.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 05/14] fsi: sbefifo: Add configurable in-command timeout
2023-06-12 19:56 [PATCH 00/14] fsi: Miscellaneous fixes and I2C Responder driver Eddie James
` (3 preceding siblings ...)
2023-06-12 19:56 ` [PATCH 04/14] fsi: sbefifo: Don't check status during probe Eddie James
@ 2023-06-12 19:56 ` Eddie James
2023-06-12 19:56 ` [PATCH 06/14] fsi: sbefifo: Remove limits on user-specified read timeout Eddie James
` (8 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Eddie James @ 2023-06-12 19:56 UTC (permalink / raw)
To: linux-fsi
Cc: linux-kernel, jk, joel, alistair, andrew, robh+dt,
krzysztof.kozlowski+dt, conor+dt, devicetree
From: Eddie James <eajames@us.ibm.com>
A new use case for the SBEFIFO requires a long in-command timeout
as the SBE processes each part of the command before clearing the
upstream FIFO for the next part of the command.
Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
drivers/fsi/fsi-sbefifo.c | 30 +++++++++++++++++++++++++++++-
include/uapi/linux/fsi.h | 10 ++++++++++
2 files changed, 39 insertions(+), 1 deletion(-)
diff --git a/drivers/fsi/fsi-sbefifo.c b/drivers/fsi/fsi-sbefifo.c
index 42d7c95528d1..5e6a9e08a92d 100644
--- a/drivers/fsi/fsi-sbefifo.c
+++ b/drivers/fsi/fsi-sbefifo.c
@@ -127,6 +127,7 @@ struct sbefifo {
bool dead;
bool async_ffdc;
bool timed_out;
+ u32 timeout_in_cmd_ms;
u32 timeout_start_rsp_ms;
};
@@ -136,6 +137,7 @@ struct sbefifo_user {
void *cmd_page;
void *pending_cmd;
size_t pending_len;
+ u32 cmd_timeout_ms;
u32 read_timeout_ms;
};
@@ -508,7 +510,7 @@ static int sbefifo_send_command(struct sbefifo *sbefifo,
rc = sbefifo_wait(sbefifo, true, &status, timeout);
if (rc < 0)
return rc;
- timeout = msecs_to_jiffies(SBEFIFO_TIMEOUT_IN_CMD);
+ timeout = msecs_to_jiffies(sbefifo->timeout_in_cmd_ms);
vacant = sbefifo_vacant(status);
len = chunk = min(vacant, remaining);
@@ -802,6 +804,7 @@ static int sbefifo_user_open(struct inode *inode, struct file *file)
return -ENOMEM;
}
mutex_init(&user->file_lock);
+ user->cmd_timeout_ms = SBEFIFO_TIMEOUT_IN_CMD;
user->read_timeout_ms = SBEFIFO_TIMEOUT_START_RSP;
return 0;
@@ -845,9 +848,11 @@ static ssize_t sbefifo_user_read(struct file *file, char __user *buf,
rc = mutex_lock_interruptible(&sbefifo->lock);
if (rc)
goto bail;
+ sbefifo->timeout_in_cmd_ms = user->cmd_timeout_ms;
sbefifo->timeout_start_rsp_ms = user->read_timeout_ms;
rc = __sbefifo_submit(sbefifo, user->pending_cmd, cmd_len, &resp_iter);
sbefifo->timeout_start_rsp_ms = SBEFIFO_TIMEOUT_START_RSP;
+ sbefifo->timeout_in_cmd_ms = SBEFIFO_TIMEOUT_IN_CMD;
mutex_unlock(&sbefifo->lock);
if (rc < 0)
goto bail;
@@ -937,6 +942,25 @@ static int sbefifo_user_release(struct inode *inode, struct file *file)
return 0;
}
+static int sbefifo_cmd_timeout(struct sbefifo_user *user, void __user *argp)
+{
+ struct device *dev = &user->sbefifo->dev;
+ u32 timeout;
+
+ if (get_user(timeout, (__u32 __user *)argp))
+ return -EFAULT;
+
+ if (timeout == 0) {
+ user->cmd_timeout_ms = SBEFIFO_TIMEOUT_IN_CMD;
+ dev_dbg(dev, "Command timeout reset to %us\n", user->cmd_timeout_ms / 1000);
+ return 0;
+ }
+
+ user->cmd_timeout_ms = timeout * 1000; /* user timeout is in sec */
+ dev_dbg(dev, "Command timeout set to %us\n", timeout);
+ return 0;
+}
+
static int sbefifo_read_timeout(struct sbefifo_user *user, void __user *argp)
{
struct device *dev = &user->sbefifo->dev;
@@ -971,6 +995,9 @@ static long sbefifo_user_ioctl(struct file *file, unsigned int cmd, unsigned lon
mutex_lock(&user->file_lock);
switch (cmd) {
+ case FSI_SBEFIFO_CMD_TIMEOUT_SECONDS:
+ rc = sbefifo_cmd_timeout(user, (void __user *)arg);
+ break;
case FSI_SBEFIFO_READ_TIMEOUT_SECONDS:
rc = sbefifo_read_timeout(user, (void __user *)arg);
break;
@@ -1025,6 +1052,7 @@ static int sbefifo_probe(struct device *dev)
sbefifo->fsi_dev = fsi_dev;
dev_set_drvdata(dev, sbefifo);
mutex_init(&sbefifo->lock);
+ sbefifo->timeout_in_cmd_ms = SBEFIFO_TIMEOUT_IN_CMD;
sbefifo->timeout_start_rsp_ms = SBEFIFO_TIMEOUT_START_RSP;
/* Create chardev for userspace access */
diff --git a/include/uapi/linux/fsi.h b/include/uapi/linux/fsi.h
index b2f1977378c7..a2e730fc6309 100644
--- a/include/uapi/linux/fsi.h
+++ b/include/uapi/linux/fsi.h
@@ -59,6 +59,16 @@ struct scom_access {
* /dev/sbefifo* ioctl interface
*/
+/**
+ * FSI_SBEFIFO_CMD_TIMEOUT sets the timeout for writing data to the SBEFIFO.
+ *
+ * The command timeout is specified in seconds. The minimum value of command
+ * timeout is 1 seconds (default) and the maximum value of command timeout is
+ * 120 seconds. A command timeout of 0 will reset the value to the default of
+ * 1 seconds.
+ */
+#define FSI_SBEFIFO_CMD_TIMEOUT_SECONDS _IOW('s', 0x01, __u32)
+
/**
* FSI_SBEFIFO_READ_TIMEOUT sets the read timeout for response from SBE.
*
--
2.31.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 06/14] fsi: sbefifo: Remove limits on user-specified read timeout
2023-06-12 19:56 [PATCH 00/14] fsi: Miscellaneous fixes and I2C Responder driver Eddie James
` (4 preceding siblings ...)
2023-06-12 19:56 ` [PATCH 05/14] fsi: sbefifo: Add configurable in-command timeout Eddie James
@ 2023-06-12 19:56 ` Eddie James
2023-06-12 19:56 ` [PATCH 07/14] fsi: aspeed: Reset master errors after CFAM reset Eddie James
` (7 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Eddie James @ 2023-06-12 19:56 UTC (permalink / raw)
To: linux-fsi
Cc: linux-kernel, jk, joel, alistair, andrew, robh+dt,
krzysztof.kozlowski+dt, conor+dt, devicetree
There's no reason to limit the user here. The way the driver is
designed, extremely large transfers require extremely long timeouts.
Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
drivers/fsi/fsi-sbefifo.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/fsi/fsi-sbefifo.c b/drivers/fsi/fsi-sbefifo.c
index 5e6a9e08a92d..3364ddd90ee0 100644
--- a/drivers/fsi/fsi-sbefifo.c
+++ b/drivers/fsi/fsi-sbefifo.c
@@ -971,17 +971,12 @@ static int sbefifo_read_timeout(struct sbefifo_user *user, void __user *argp)
if (timeout == 0) {
user->read_timeout_ms = SBEFIFO_TIMEOUT_START_RSP;
- dev_dbg(dev, "Timeout reset to %d\n", user->read_timeout_ms);
+ dev_dbg(dev, "Timeout reset to %us\n", user->read_timeout_ms / 1000);
return 0;
}
- if (timeout < 10 || timeout > 120)
- return -EINVAL;
-
user->read_timeout_ms = timeout * 1000; /* user timeout is in sec */
-
- dev_dbg(dev, "Timeout set to %d\n", user->read_timeout_ms);
-
+ dev_dbg(dev, "Timeout set to %us\n", timeout);
return 0;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 07/14] fsi: aspeed: Reset master errors after CFAM reset
2023-06-12 19:56 [PATCH 00/14] fsi: Miscellaneous fixes and I2C Responder driver Eddie James
` (5 preceding siblings ...)
2023-06-12 19:56 ` [PATCH 06/14] fsi: sbefifo: Remove limits on user-specified read timeout Eddie James
@ 2023-06-12 19:56 ` Eddie James
2023-06-12 19:56 ` [PATCH 08/14] fsi: core: Add trace events for scan and unregister Eddie James
` (6 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Eddie James @ 2023-06-12 19:56 UTC (permalink / raw)
To: linux-fsi
Cc: linux-kernel, jk, joel, alistair, andrew, robh+dt,
krzysztof.kozlowski+dt, conor+dt, devicetree
It has been observed that sometimes the FSI master will return all 0xffs
after a CFAM has been taken out of reset, without presenting any error.
Resetting the FSI master errors resolves the issue.
Fixes: 4a851d714ead ("fsi: aspeed: Support CFAM reset GPIO")
Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
drivers/fsi/fsi-master-aspeed.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/fsi/fsi-master-aspeed.c b/drivers/fsi/fsi-master-aspeed.c
index 437f87b4a6a3..f0a19cd451a0 100644
--- a/drivers/fsi/fsi-master-aspeed.c
+++ b/drivers/fsi/fsi-master-aspeed.c
@@ -454,6 +454,8 @@ static ssize_t cfam_reset_store(struct device *dev, struct device_attribute *att
gpiod_set_value(aspeed->cfam_reset_gpio, 1);
usleep_range(900, 1000);
gpiod_set_value(aspeed->cfam_reset_gpio, 0);
+ usleep_range(900, 1000);
+ opb_writel(aspeed, ctrl_base + FSI_MRESP0, cpu_to_be32(FSI_MRESP_RST_ALL_MASTER));
mutex_unlock(&aspeed->lock);
trace_fsi_master_aspeed_cfam_reset(false);
--
2.31.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 08/14] fsi: core: Add trace events for scan and unregister
2023-06-12 19:56 [PATCH 00/14] fsi: Miscellaneous fixes and I2C Responder driver Eddie James
` (6 preceding siblings ...)
2023-06-12 19:56 ` [PATCH 07/14] fsi: aspeed: Reset master errors after CFAM reset Eddie James
@ 2023-06-12 19:56 ` Eddie James
2023-06-12 19:56 ` [PATCH 09/14] fsi: core: Fix legacy minor numbering Eddie James
` (5 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Eddie James @ 2023-06-12 19:56 UTC (permalink / raw)
To: linux-fsi
Cc: linux-kernel, jk, joel, alistair, andrew, robh+dt,
krzysztof.kozlowski+dt, conor+dt, devicetree
Add more trace events for the scanning and unregistration
functions for debug purposes.
Signed-off-by: Eddie James <eajames@linux.ibm.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
drivers/fsi/fsi-core.c | 4 ++++
include/trace/events/fsi.h | 31 +++++++++++++++++++++++++++++++
2 files changed, 35 insertions(+)
diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index ca4a9634fbc3..47e47c16d5ed 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -1220,6 +1220,7 @@ static int fsi_master_scan(struct fsi_master *master)
{
int link, rc;
+ trace_fsi_master_scan(master, true);
for (link = 0; link < master->n_links; link++) {
rc = fsi_master_link_enable(master, link);
if (rc) {
@@ -1261,6 +1262,7 @@ static int fsi_master_remove_slave(struct device *dev, void *arg)
static void fsi_master_unscan(struct fsi_master *master)
{
+ trace_fsi_master_scan(master, false);
device_for_each_child(&master->dev, NULL, fsi_master_remove_slave);
}
@@ -1349,6 +1351,8 @@ EXPORT_SYMBOL_GPL(fsi_master_register);
void fsi_master_unregister(struct fsi_master *master)
{
+ trace_fsi_master_unregister(master);
+
if (master->idx >= 0) {
ida_simple_remove(&master_ida, master->idx);
master->idx = -1;
diff --git a/include/trace/events/fsi.h b/include/trace/events/fsi.h
index c9a72e8432b8..5ff15126ad9d 100644
--- a/include/trace/events/fsi.h
+++ b/include/trace/events/fsi.h
@@ -122,6 +122,37 @@ TRACE_EVENT(fsi_master_break,
)
);
+TRACE_EVENT(fsi_master_scan,
+ TP_PROTO(const struct fsi_master *master, bool scan),
+ TP_ARGS(master, scan),
+ TP_STRUCT__entry(
+ __field(int, master_idx)
+ __field(int, n_links)
+ __field(bool, scan)
+ ),
+ TP_fast_assign(
+ __entry->master_idx = master->idx;
+ __entry->n_links = master->n_links;
+ __entry->scan = scan;
+ ),
+ TP_printk("fsi%d (%d links) %s", __entry->master_idx, __entry->n_links,
+ __entry->scan ? "scan" : "unscan")
+);
+
+TRACE_EVENT(fsi_master_unregister,
+ TP_PROTO(const struct fsi_master *master),
+ TP_ARGS(master),
+ TP_STRUCT__entry(
+ __field(int, master_idx)
+ __field(int, n_links)
+ ),
+ TP_fast_assign(
+ __entry->master_idx = master->idx;
+ __entry->n_links = master->n_links;
+ ),
+ TP_printk("fsi%d (%d links)", __entry->master_idx, __entry->n_links)
+);
+
TRACE_EVENT(fsi_slave_init,
TP_PROTO(const struct fsi_slave *slave),
TP_ARGS(slave),
--
2.31.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 09/14] fsi: core: Fix legacy minor numbering
2023-06-12 19:56 [PATCH 00/14] fsi: Miscellaneous fixes and I2C Responder driver Eddie James
` (7 preceding siblings ...)
2023-06-12 19:56 ` [PATCH 08/14] fsi: core: Add trace events for scan and unregister Eddie James
@ 2023-06-12 19:56 ` Eddie James
2023-06-12 19:56 ` [PATCH 10/14] fsi: core: Switch to ida_alloc/free Eddie James
` (4 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Eddie James @ 2023-06-12 19:56 UTC (permalink / raw)
To: linux-fsi
Cc: linux-kernel, jk, joel, alistair, andrew, robh+dt,
krzysztof.kozlowski+dt, conor+dt, devicetree
From: Eddie James <eajames@us.ibm.com>
The legacy minor numbering shifts the chip id too much,
resulting in ids that overlap with regular ids. Since there
are only 2 bits for 4 types, only shift the chip id by 2
to fit the legacy ids in their reserved space.
Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
drivers/fsi/fsi-core.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index 47e47c16d5ed..ea280f149b3f 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -934,8 +934,12 @@ static int __fsi_get_new_minor(struct fsi_slave *slave, enum fsi_dev_type type,
/* Check if we qualify for legacy numbering */
if (cid >= 0 && cid < 16 && type < 4) {
- /* Try reserving the legacy number */
- id = (cid << 4) | type;
+ /*
+ * Try reserving the legacy number, which has 0 - 0x3f reserved
+ * in the ida range. cid goes up to 0xf and type contains two
+ * bits, so construct the id with the below two bit shift.
+ */
+ id = (cid << 2) | type;
id = ida_simple_get(&fsi_minor_ida, id, id + 1, GFP_KERNEL);
if (id >= 0) {
*out_index = fsi_adjust_index(cid);
@@ -970,7 +974,8 @@ int fsi_get_new_minor(struct fsi_device *fdev, enum fsi_dev_type type,
int aid = of_alias_get_id(fdev->dev.of_node, fsi_dev_type_names[type]);
if (aid >= 0) {
- int id = (aid << 4) | type;
+ /* Use the same scheme as the legacy numbers. */
+ int id = (aid << 2) | type;
id = ida_simple_get(&fsi_minor_ida, id, id + 1, GFP_KERNEL);
if (id >= 0) {
--
2.31.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 10/14] fsi: core: Switch to ida_alloc/free
2023-06-12 19:56 [PATCH 00/14] fsi: Miscellaneous fixes and I2C Responder driver Eddie James
` (8 preceding siblings ...)
2023-06-12 19:56 ` [PATCH 09/14] fsi: core: Fix legacy minor numbering Eddie James
@ 2023-06-12 19:56 ` Eddie James
2023-06-12 19:56 ` [PATCH 11/14] fsi: Improve master indexing Eddie James
` (3 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Eddie James @ 2023-06-12 19:56 UTC (permalink / raw)
To: linux-fsi
Cc: linux-kernel, jk, joel, alistair, andrew, robh+dt,
krzysztof.kozlowski+dt, conor+dt, devicetree
ida_simple_get/remove are deprecated, so switch to ida_alloc/free.
Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
drivers/fsi/fsi-core.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index ea280f149b3f..ec4d02264391 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -940,7 +940,7 @@ static int __fsi_get_new_minor(struct fsi_slave *slave, enum fsi_dev_type type,
* bits, so construct the id with the below two bit shift.
*/
id = (cid << 2) | type;
- id = ida_simple_get(&fsi_minor_ida, id, id + 1, GFP_KERNEL);
+ id = ida_alloc_range(&fsi_minor_ida, id, id, GFP_KERNEL);
if (id >= 0) {
*out_index = fsi_adjust_index(cid);
*out_dev = fsi_base_dev + id;
@@ -951,8 +951,8 @@ static int __fsi_get_new_minor(struct fsi_slave *slave, enum fsi_dev_type type,
return id;
/* Fallback to non-legacy allocation */
}
- id = ida_simple_get(&fsi_minor_ida, FSI_CHAR_LEGACY_TOP,
- FSI_CHAR_MAX_DEVICES, GFP_KERNEL);
+ id = ida_alloc_range(&fsi_minor_ida, FSI_CHAR_LEGACY_TOP,
+ FSI_CHAR_MAX_DEVICES - 1, GFP_KERNEL);
if (id < 0)
return id;
*out_index = fsi_adjust_index(id);
@@ -977,7 +977,7 @@ int fsi_get_new_minor(struct fsi_device *fdev, enum fsi_dev_type type,
/* Use the same scheme as the legacy numbers. */
int id = (aid << 2) | type;
- id = ida_simple_get(&fsi_minor_ida, id, id + 1, GFP_KERNEL);
+ id = ida_alloc_range(&fsi_minor_ida, id, id, GFP_KERNEL);
if (id >= 0) {
*out_index = aid;
*out_dev = fsi_base_dev + id;
@@ -995,7 +995,7 @@ EXPORT_SYMBOL_GPL(fsi_get_new_minor);
void fsi_free_minor(dev_t dev)
{
- ida_simple_remove(&fsi_minor_ida, MINOR(dev));
+ ida_free(&fsi_minor_ida, MINOR(dev));
}
EXPORT_SYMBOL_GPL(fsi_free_minor);
@@ -1330,7 +1330,7 @@ int fsi_master_register(struct fsi_master *master)
struct device_node *np;
mutex_init(&master->scan_lock);
- master->idx = ida_simple_get(&master_ida, 0, INT_MAX, GFP_KERNEL);
+ master->idx = ida_alloc(&master_ida, GFP_KERNEL);
if (master->idx < 0)
return master->idx;
@@ -1339,7 +1339,7 @@ int fsi_master_register(struct fsi_master *master)
rc = device_register(&master->dev);
if (rc) {
- ida_simple_remove(&master_ida, master->idx);
+ ida_free(&master_ida, master->idx);
return rc;
}
@@ -1359,7 +1359,7 @@ void fsi_master_unregister(struct fsi_master *master)
trace_fsi_master_unregister(master);
if (master->idx >= 0) {
- ida_simple_remove(&master_ida, master->idx);
+ ida_free(&master_ida, master->idx);
master->idx = -1;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 11/14] fsi: Improve master indexing
2023-06-12 19:56 [PATCH 00/14] fsi: Miscellaneous fixes and I2C Responder driver Eddie James
` (9 preceding siblings ...)
2023-06-12 19:56 ` [PATCH 10/14] fsi: core: Switch to ida_alloc/free Eddie James
@ 2023-06-12 19:56 ` Eddie James
2023-08-09 7:08 ` Joel Stanley
2023-06-12 19:56 ` [PATCH 12/14] dt-bindings: fsi: Document the IBM I2C Responder virtual FSI master Eddie James
` (2 subsequent siblings)
13 siblings, 1 reply; 21+ messages in thread
From: Eddie James @ 2023-06-12 19:56 UTC (permalink / raw)
To: linux-fsi
Cc: linux-kernel, jk, joel, alistair, andrew, robh+dt,
krzysztof.kozlowski+dt, conor+dt, devicetree
Master indexing is problematic if a hub is rescanned while the
root master is being rescanned. Move the IDA free below the device
unregistration, lock the scan mutex in the probe function, and
request a specific idx in the hub driver.
Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
drivers/fsi/fsi-core.c | 41 ++++++++++++++++++++++--------------
drivers/fsi/fsi-master-hub.c | 2 ++
2 files changed, 27 insertions(+), 16 deletions(-)
diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index ec4d02264391..503061a6740b 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -1327,46 +1327,55 @@ 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);
- master->idx = ida_alloc(&master_ida, GFP_KERNEL);
+
+ if (master->idx) {
+ master->idx = ida_alloc_range(&master_ida, master->idx,
+ master->idx, GFP_KERNEL);
+ if (master->idx < 0)
+ master->idx = ida_alloc(&master_ida, GFP_KERNEL);
+ } else {
+ master->idx = ida_alloc(&master_ida, GFP_KERNEL);
+ }
+
if (master->idx < 0)
return master->idx;
- dev_set_name(&master->dev, "fsi%d", master->idx);
+ if (!dev_name(&master->dev))
+ dev_set_name(&master->dev, "fsi%d", master->idx);
+
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);
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-hub.c b/drivers/fsi/fsi-master-hub.c
index 6d8b6e8854e5..36da643b3201 100644
--- a/drivers/fsi/fsi-master-hub.c
+++ b/drivers/fsi/fsi-master-hub.c
@@ -12,6 +12,7 @@
#include <linux/slab.h>
#include "fsi-master.h"
+#include "fsi-slave.h"
#define FSI_ENGID_HUB_MASTER 0x1c
@@ -229,6 +230,7 @@ static int hub_master_probe(struct device *dev)
hub->master.dev.release = hub_master_release;
hub->master.dev.of_node = of_node_get(dev_of_node(dev));
+ hub->master.idx = fsi_dev->slave->link + 1;
hub->master.n_links = links;
hub->master.read = hub_master_read;
hub->master.write = hub_master_write;
--
2.31.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 11/14] fsi: Improve master indexing
2023-06-12 19:56 ` [PATCH 11/14] fsi: Improve master indexing Eddie James
@ 2023-08-09 7:08 ` Joel Stanley
2023-08-09 11:55 ` Joel Stanley
2023-08-09 16:21 ` Eddie James
0 siblings, 2 replies; 21+ messages in thread
From: Joel Stanley @ 2023-08-09 7:08 UTC (permalink / raw)
To: Eddie James
Cc: linux-fsi, linux-kernel, jk, alistair, andrew, robh+dt,
krzysztof.kozlowski+dt, conor+dt, devicetree
On Mon, 12 Jun 2023 at 19:57, Eddie James <eajames@linux.ibm.com> wrote:
>
> Master indexing is problematic if a hub is rescanned while the
> root master is being rescanned. Move the IDA free below the device
> unregistration, lock the scan mutex in the probe function, and
> request a specific idx in the hub driver.
I've applied this series, but taking a closer look at this patch I
think it can be improved. If you resend, just send this patch.
>
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> ---
> drivers/fsi/fsi-core.c | 41 ++++++++++++++++++++++--------------
> drivers/fsi/fsi-master-hub.c | 2 ++
> 2 files changed, 27 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
> index ec4d02264391..503061a6740b 100644
> --- a/drivers/fsi/fsi-core.c
> +++ b/drivers/fsi/fsi-core.c
> @@ -1327,46 +1327,55 @@ 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);
> - master->idx = ida_alloc(&master_ida, GFP_KERNEL);
> +
> + if (master->idx) {
Why do we allocate a new idx if there's already one?
> + master->idx = ida_alloc_range(&master_ida, master->idx,
> + master->idx, GFP_KERNEL);
If we can't get one in the range we want, we ask for any? Should this
print a warning?
> + if (master->idx < 0)
> + master->idx = ida_alloc(&master_ida, GFP_KERNEL);
> + } else {
If ixd was zero, we create one. This is the "normal" case?
> + master->idx = ida_alloc(&master_ida, GFP_KERNEL);
> + }
> +
We check the same error condition again.
> if (master->idx < 0)
> return master->idx;
>
> - dev_set_name(&master->dev, "fsi%d", master->idx);
> + if (!dev_name(&master->dev))
> + dev_set_name(&master->dev, "fsi%d", master->idx);
> +
> 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);
This change looks a bit different to the idx changes. What's happening here?
>
> - 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);
>
> 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-hub.c b/drivers/fsi/fsi-master-hub.c
> index 6d8b6e8854e5..36da643b3201 100644
> --- a/drivers/fsi/fsi-master-hub.c
> +++ b/drivers/fsi/fsi-master-hub.c
> @@ -12,6 +12,7 @@
> #include <linux/slab.h>
>
> #include "fsi-master.h"
> +#include "fsi-slave.h"
>
> #define FSI_ENGID_HUB_MASTER 0x1c
>
> @@ -229,6 +230,7 @@ static int hub_master_probe(struct device *dev)
> hub->master.dev.release = hub_master_release;
> hub->master.dev.of_node = of_node_get(dev_of_node(dev));
>
> + hub->master.idx = fsi_dev->slave->link + 1;
> hub->master.n_links = links;
> hub->master.read = hub_master_read;
> hub->master.write = hub_master_write;
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 11/14] fsi: Improve master indexing
2023-08-09 7:08 ` Joel Stanley
@ 2023-08-09 11:55 ` Joel Stanley
2023-08-09 16:08 ` Eddie James
2023-08-09 16:21 ` Eddie James
1 sibling, 1 reply; 21+ messages in thread
From: Joel Stanley @ 2023-08-09 11:55 UTC (permalink / raw)
To: Eddie James
Cc: linux-fsi, linux-kernel, jk, alistair, andrew, robh+dt,
krzysztof.kozlowski+dt, conor+dt, devicetree
On Wed, 9 Aug 2023 at 07:08, Joel Stanley <joel@jms.id.au> wrote:
>
> On Mon, 12 Jun 2023 at 19:57, Eddie James <eajames@linux.ibm.com> wrote:
> >
> > Master indexing is problematic if a hub is rescanned while the
> > root master is being rescanned. Move the IDA free below the device
> > unregistration, lock the scan mutex in the probe function, and
> > request a specific idx in the hub driver.
>
> I've applied this series, but taking a closer look at this patch I
> think it can be improved. If you resend, just send this patch.
On hardware, it did this at FSI scan time:
WARNING: CPU: 0 PID: 761 at /lib/idr.c:525 ida_free+0x140/0x154
ida_free called for id=1 which is not allocated.
CPU: 0 PID: 761 Comm: openpower-proc- Not tainted 6.1.34-d42f59e #1
Hardware name: Generic DT based system
unwind_backtrace from show_stack+0x18/0x1c
show_stack from dump_stack_lvl+0x24/0x2c
dump_stack_lvl from __warn+0x74/0xf0
__warn from warn_slowpath_fmt+0x9c/0xd8
warn_slowpath_fmt from ida_free+0x140/0x154
ida_free from fsi_master_register+0xd0/0xf0
fsi_master_register from hub_master_probe+0x11c/0x358
hub_master_probe from really_probe+0xd4/0x3f0
really_probe from driver_probe_device+0x38/0xd0
driver_probe_device from __device_attach_driver+0xc8/0x148
__device_attach_driver from bus_for_each_drv+0x90/0xdc
bus_for_each_drv from __device_attach+0x114/0x1a4
__device_attach from bus_probe_device+0x8c/0x94
bus_probe_device from device_add+0x3a8/0x7fc
device_add from fsi_master_scan+0x4e0/0x950
fsi_master_scan from fsi_master_rescan+0x38/0x88
fsi_master_rescan from master_rescan_store+0x14/0x20
master_rescan_store from kernfs_fop_write_iter+0x114/0x200
kernfs_fop_write_iter from vfs_write+0x1d0/0x374
vfs_write from ksys_write+0x78/0x100
ksys_write from ret_fast_syscall+0x0/0x54
Exception stack(0x9fc51fa8 to 0x9fc51ff0)
1fa0: 00000001 01a01c78 00000003 01a01c78 00000001 00000001
1fc0: 00000001 01a01c78 00000001 00000004 7eeb4ab0 7eeb4b3c 7eeb4ab4 7eeb499c
1fe0: 76985abc 7eeb4928 76848af8 766f176c
>
> >
> > Signed-off-by: Eddie James <eajames@linux.ibm.com>
> > ---
> > drivers/fsi/fsi-core.c | 41 ++++++++++++++++++++++--------------
> > drivers/fsi/fsi-master-hub.c | 2 ++
> > 2 files changed, 27 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
> > index ec4d02264391..503061a6740b 100644
> > --- a/drivers/fsi/fsi-core.c
> > +++ b/drivers/fsi/fsi-core.c
> > @@ -1327,46 +1327,55 @@ 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);
> > - master->idx = ida_alloc(&master_ida, GFP_KERNEL);
> > +
> > + if (master->idx) {
>
> Why do we allocate a new idx if there's already one?
>
> > + master->idx = ida_alloc_range(&master_ida, master->idx,
> > + master->idx, GFP_KERNEL);
>
> If we can't get one in the range we want, we ask for any? Should this
> print a warning?
>
> > + if (master->idx < 0)
> > + master->idx = ida_alloc(&master_ida, GFP_KERNEL);
> > + } else {
>
> If ixd was zero, we create one. This is the "normal" case?
>
> > + master->idx = ida_alloc(&master_ida, GFP_KERNEL);
> > + }
> > +
>
> We check the same error condition again.
>
> > if (master->idx < 0)
> > return master->idx;
>
> >
> > - dev_set_name(&master->dev, "fsi%d", master->idx);
> > + if (!dev_name(&master->dev))
> > + dev_set_name(&master->dev, "fsi%d", master->idx);
> > +
> > 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);
>
> This change looks a bit different to the idx changes. What's happening here?
> >
> > - 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);
> >
> > 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-hub.c b/drivers/fsi/fsi-master-hub.c
> > index 6d8b6e8854e5..36da643b3201 100644
> > --- a/drivers/fsi/fsi-master-hub.c
> > +++ b/drivers/fsi/fsi-master-hub.c
> > @@ -12,6 +12,7 @@
> > #include <linux/slab.h>
> >
> > #include "fsi-master.h"
> > +#include "fsi-slave.h"
> >
> > #define FSI_ENGID_HUB_MASTER 0x1c
> >
> > @@ -229,6 +230,7 @@ static int hub_master_probe(struct device *dev)
> > hub->master.dev.release = hub_master_release;
> > hub->master.dev.of_node = of_node_get(dev_of_node(dev));
> >
> > + hub->master.idx = fsi_dev->slave->link + 1;
> > hub->master.n_links = links;
> > hub->master.read = hub_master_read;
> > hub->master.write = hub_master_write;
> > --
> > 2.31.1
> >
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 11/14] fsi: Improve master indexing
2023-08-09 11:55 ` Joel Stanley
@ 2023-08-09 16:08 ` Eddie James
0 siblings, 0 replies; 21+ messages in thread
From: Eddie James @ 2023-08-09 16:08 UTC (permalink / raw)
To: Joel Stanley
Cc: linux-fsi, linux-kernel, jk, alistair, andrew, robh+dt,
krzysztof.kozlowski+dt, conor+dt, devicetree
On 8/9/23 06:55, Joel Stanley wrote:
> On Wed, 9 Aug 2023 at 07:08, Joel Stanley <joel@jms.id.au> wrote:
>> On Mon, 12 Jun 2023 at 19:57, Eddie James <eajames@linux.ibm.com> wrote:
>>> Master indexing is problematic if a hub is rescanned while the
>>> root master is being rescanned. Move the IDA free below the device
>>> unregistration, lock the scan mutex in the probe function, and
>>> request a specific idx in the hub driver.
>> I've applied this series, but taking a closer look at this patch I
>> think it can be improved. If you resend, just send this patch.
> On hardware, it did this at FSI scan time:
Well this backtrace is without this patch, right? The hub master changes
that went in are dependent on this patch. master->idx is 1 for the hub
but it's not being allocated in the ida and the device name isn't
getting set. So device registration fails and then trying to free the
index in the ida causes this warning.
I'll reply to your comments in your other email and rebase and resend.
>
> WARNING: CPU: 0 PID: 761 at /lib/idr.c:525 ida_free+0x140/0x154
> ida_free called for id=1 which is not allocated.
> CPU: 0 PID: 761 Comm: openpower-proc- Not tainted 6.1.34-d42f59e #1
> Hardware name: Generic DT based system
> unwind_backtrace from show_stack+0x18/0x1c
> show_stack from dump_stack_lvl+0x24/0x2c
> dump_stack_lvl from __warn+0x74/0xf0
> __warn from warn_slowpath_fmt+0x9c/0xd8
> warn_slowpath_fmt from ida_free+0x140/0x154
> ida_free from fsi_master_register+0xd0/0xf0
> fsi_master_register from hub_master_probe+0x11c/0x358
> hub_master_probe from really_probe+0xd4/0x3f0
> really_probe from driver_probe_device+0x38/0xd0
> driver_probe_device from __device_attach_driver+0xc8/0x148
> __device_attach_driver from bus_for_each_drv+0x90/0xdc
> bus_for_each_drv from __device_attach+0x114/0x1a4
> __device_attach from bus_probe_device+0x8c/0x94
> bus_probe_device from device_add+0x3a8/0x7fc
> device_add from fsi_master_scan+0x4e0/0x950
> fsi_master_scan from fsi_master_rescan+0x38/0x88
> fsi_master_rescan from master_rescan_store+0x14/0x20
> master_rescan_store from kernfs_fop_write_iter+0x114/0x200
> kernfs_fop_write_iter from vfs_write+0x1d0/0x374
> vfs_write from ksys_write+0x78/0x100
> ksys_write from ret_fast_syscall+0x0/0x54
> Exception stack(0x9fc51fa8 to 0x9fc51ff0)
> 1fa0: 00000001 01a01c78 00000003 01a01c78 00000001 00000001
> 1fc0: 00000001 01a01c78 00000001 00000004 7eeb4ab0 7eeb4b3c 7eeb4ab4 7eeb499c
> 1fe0: 76985abc 7eeb4928 76848af8 766f176c
>
>
>>> Signed-off-by: Eddie James <eajames@linux.ibm.com>
>>> ---
>>> drivers/fsi/fsi-core.c | 41 ++++++++++++++++++++++--------------
>>> drivers/fsi/fsi-master-hub.c | 2 ++
>>> 2 files changed, 27 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
>>> index ec4d02264391..503061a6740b 100644
>>> --- a/drivers/fsi/fsi-core.c
>>> +++ b/drivers/fsi/fsi-core.c
>>> @@ -1327,46 +1327,55 @@ 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);
>>> - master->idx = ida_alloc(&master_ida, GFP_KERNEL);
>>> +
>>> + if (master->idx) {
>> Why do we allocate a new idx if there's already one?
>>
>>> + master->idx = ida_alloc_range(&master_ida, master->idx,
>>> + master->idx, GFP_KERNEL);
>> If we can't get one in the range we want, we ask for any? Should this
>> print a warning?
>>
>>> + if (master->idx < 0)
>>> + master->idx = ida_alloc(&master_ida, GFP_KERNEL);
>>> + } else {
>> If ixd was zero, we create one. This is the "normal" case?
>>
>>> + master->idx = ida_alloc(&master_ida, GFP_KERNEL);
>>> + }
>>> +
>> We check the same error condition again.
>>
>>> if (master->idx < 0)
>>> return master->idx;
>>> - dev_set_name(&master->dev, "fsi%d", master->idx);
>>> + if (!dev_name(&master->dev))
>>> + dev_set_name(&master->dev, "fsi%d", master->idx);
>>> +
>>> 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);
>> This change looks a bit different to the idx changes. What's happening here?
>>> - 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);
>>>
>>> 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-hub.c b/drivers/fsi/fsi-master-hub.c
>>> index 6d8b6e8854e5..36da643b3201 100644
>>> --- a/drivers/fsi/fsi-master-hub.c
>>> +++ b/drivers/fsi/fsi-master-hub.c
>>> @@ -12,6 +12,7 @@
>>> #include <linux/slab.h>
>>>
>>> #include "fsi-master.h"
>>> +#include "fsi-slave.h"
>>>
>>> #define FSI_ENGID_HUB_MASTER 0x1c
>>>
>>> @@ -229,6 +230,7 @@ static int hub_master_probe(struct device *dev)
>>> hub->master.dev.release = hub_master_release;
>>> hub->master.dev.of_node = of_node_get(dev_of_node(dev));
>>>
>>> + hub->master.idx = fsi_dev->slave->link + 1;
>>> hub->master.n_links = links;
>>> hub->master.read = hub_master_read;
>>> hub->master.write = hub_master_write;
>>> --
>>> 2.31.1
>>>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 11/14] fsi: Improve master indexing
2023-08-09 7:08 ` Joel Stanley
2023-08-09 11:55 ` Joel Stanley
@ 2023-08-09 16:21 ` Eddie James
1 sibling, 0 replies; 21+ messages in thread
From: Eddie James @ 2023-08-09 16:21 UTC (permalink / raw)
To: Joel Stanley
Cc: linux-fsi, linux-kernel, jk, alistair, andrew, robh+dt,
krzysztof.kozlowski+dt, conor+dt, devicetree
On 8/9/23 02:08, Joel Stanley wrote:
> On Mon, 12 Jun 2023 at 19:57, Eddie James <eajames@linux.ibm.com> wrote:
>> Master indexing is problematic if a hub is rescanned while the
>> root master is being rescanned. Move the IDA free below the device
>> unregistration, lock the scan mutex in the probe function, and
>> request a specific idx in the hub driver.
> I've applied this series, but taking a closer look at this patch I
> think it can be improved. If you resend, just send this patch.
>
>> Signed-off-by: Eddie James <eajames@linux.ibm.com>
>> ---
>> drivers/fsi/fsi-core.c | 41 ++++++++++++++++++++++--------------
>> drivers/fsi/fsi-master-hub.c | 2 ++
>> 2 files changed, 27 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
>> index ec4d02264391..503061a6740b 100644
>> --- a/drivers/fsi/fsi-core.c
>> +++ b/drivers/fsi/fsi-core.c
>> @@ -1327,46 +1327,55 @@ 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);
>> - master->idx = ida_alloc(&master_ida, GFP_KERNEL);
>> +
>> + if (master->idx) {
> Why do we allocate a new idx if there's already one?
At this point, the master driver (aspeed, hub, i2cr, whatever) might
just be requesting a certain index. It's not allocated yet, so it needs
to be allocated so that we don't get overlap.
>
>> + master->idx = ida_alloc_range(&master_ida, master->idx,
>> + master->idx, GFP_KERNEL);
> If we can't get one in the range we want, we ask for any? Should this
> print a warning?
Perhaps, we could also return error here if we decide that the requested
index is needed and not just wanted.
>
>> + if (master->idx < 0)
>> + master->idx = ida_alloc(&master_ida, GFP_KERNEL);
>> + } else {
> If ixd was zero, we create one. This is the "normal" case?
Yes, the assumption is: zero is the default due to zero-alloc'd master
structures.
>
>> + master->idx = ida_alloc(&master_ida, GFP_KERNEL);
>> + }
>> +
> We check the same error condition again.
Yes I might be able to make this cleaner...
>
>> if (master->idx < 0)
>> return master->idx;
>> - dev_set_name(&master->dev, "fsi%d", master->idx);
>> + if (!dev_name(&master->dev))
>> + dev_set_name(&master->dev, "fsi%d", master->idx);
>> +
>> 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);
> This change looks a bit different to the idx changes. What's happening here?
This is just restructuring to get the lock before the scan. It could be
a separate commit, yea.
Thanks for the review!
Eddie
>> - 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);
>>
>> 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-hub.c b/drivers/fsi/fsi-master-hub.c
>> index 6d8b6e8854e5..36da643b3201 100644
>> --- a/drivers/fsi/fsi-master-hub.c
>> +++ b/drivers/fsi/fsi-master-hub.c
>> @@ -12,6 +12,7 @@
>> #include <linux/slab.h>
>>
>> #include "fsi-master.h"
>> +#include "fsi-slave.h"
>>
>> #define FSI_ENGID_HUB_MASTER 0x1c
>>
>> @@ -229,6 +230,7 @@ static int hub_master_probe(struct device *dev)
>> hub->master.dev.release = hub_master_release;
>> hub->master.dev.of_node = of_node_get(dev_of_node(dev));
>>
>> + hub->master.idx = fsi_dev->slave->link + 1;
>> hub->master.n_links = links;
>> hub->master.read = hub_master_read;
>> hub->master.write = hub_master_write;
>> --
>> 2.31.1
>>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 12/14] dt-bindings: fsi: Document the IBM I2C Responder virtual FSI master
2023-06-12 19:56 [PATCH 00/14] fsi: Miscellaneous fixes and I2C Responder driver Eddie James
` (10 preceding siblings ...)
2023-06-12 19:56 ` [PATCH 11/14] fsi: Improve master indexing Eddie James
@ 2023-06-12 19:56 ` Eddie James
2023-06-12 19:56 ` [PATCH 13/14] fsi: Add " Eddie James
2023-06-12 19:56 ` [PATCH 14/14] fsi: Add I2C Responder SCOM driver Eddie James
13 siblings, 0 replies; 21+ messages in thread
From: Eddie James @ 2023-06-12 19:56 UTC (permalink / raw)
To: linux-fsi
Cc: linux-kernel, jk, joel, alistair, andrew, robh+dt,
krzysztof.kozlowski+dt, conor+dt, devicetree
The I2C Responder translates I2C commands to CFAM or SCOM operations,
effectively implementing an FSI master.
Signed-off-by: Eddie James <eajames@linux.ibm.com>
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
.../bindings/fsi/ibm,i2cr-fsi-master.yaml | 41 +++++++++++++++++++
1 file changed, 41 insertions(+)
create mode 100644 Documentation/devicetree/bindings/fsi/ibm,i2cr-fsi-master.yaml
diff --git a/Documentation/devicetree/bindings/fsi/ibm,i2cr-fsi-master.yaml b/Documentation/devicetree/bindings/fsi/ibm,i2cr-fsi-master.yaml
new file mode 100644
index 000000000000..442cecdc57cb
--- /dev/null
+++ b/Documentation/devicetree/bindings/fsi/ibm,i2cr-fsi-master.yaml
@@ -0,0 +1,41 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/fsi/ibm,i2cr-fsi-master.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: IBM I2C Responder virtual FSI master
+
+maintainers:
+ - Eddie James <eajames@linux.ibm.com>
+
+description: |
+ The I2C Responder (I2CR) is a an I2C device that's connected to an FSI CFAM
+ (see fsi.txt). The I2CR translates I2C bus operations to FSI CFAM reads and
+ writes or SCOM operations, thereby acting as an FSI master.
+
+properties:
+ compatible:
+ enum:
+ - ibm,i2cr-fsi-master
+
+ reg:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ i2cr@20 {
+ compatible = "ibm,i2cr-fsi-master";
+ reg = <0x20>;
+ };
+ };
--
2.31.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 13/14] fsi: Add IBM I2C Responder virtual FSI master
2023-06-12 19:56 [PATCH 00/14] fsi: Miscellaneous fixes and I2C Responder driver Eddie James
` (11 preceding siblings ...)
2023-06-12 19:56 ` [PATCH 12/14] dt-bindings: fsi: Document the IBM I2C Responder virtual FSI master Eddie James
@ 2023-06-12 19:56 ` Eddie James
2023-06-12 19:56 ` [PATCH 14/14] fsi: Add I2C Responder SCOM driver Eddie James
13 siblings, 0 replies; 21+ messages in thread
From: Eddie James @ 2023-06-12 19:56 UTC (permalink / raw)
To: linux-fsi
Cc: linux-kernel, jk, joel, alistair, andrew, robh+dt,
krzysztof.kozlowski+dt, conor+dt, devicetree
The I2C Responder (I2CR) is an I2C device that translates I2C commands
to CFAM or SCOM operations, effectively implementing an FSI master and
bus.
Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
drivers/fsi/Kconfig | 9 +
drivers/fsi/Makefile | 1 +
drivers/fsi/fsi-master-i2cr.c | 316 +++++++++++++++++++++++++
drivers/fsi/fsi-master-i2cr.h | 33 +++
include/trace/events/fsi_master_i2cr.h | 107 +++++++++
5 files changed, 466 insertions(+)
create mode 100644 drivers/fsi/fsi-master-i2cr.c
create mode 100644 drivers/fsi/fsi-master-i2cr.h
create mode 100644 include/trace/events/fsi_master_i2cr.h
diff --git a/drivers/fsi/Kconfig b/drivers/fsi/Kconfig
index e6668a869913..999be82720c5 100644
--- a/drivers/fsi/Kconfig
+++ b/drivers/fsi/Kconfig
@@ -62,6 +62,15 @@ config FSI_MASTER_ASPEED
Enable it for your BMC kernel in an OpenPower or IBM Power system.
+config FSI_MASTER_I2CR
+ tristate "IBM I2C Responder virtual FSI master"
+ depends on I2C
+ help
+ This option enables a virtual FSI master in order to access a CFAM
+ behind an IBM I2C Responder (I2CR) chip. The I2CR is an I2C device
+ that translates I2C commands to CFAM or SCOM operations, effectively
+ implementing an FSI master and bus.
+
config FSI_SCOM
tristate "SCOM FSI client device driver"
help
diff --git a/drivers/fsi/Makefile b/drivers/fsi/Makefile
index da218a1ad8e1..34dbaa1c452e 100644
--- a/drivers/fsi/Makefile
+++ b/drivers/fsi/Makefile
@@ -4,6 +4,7 @@ obj-$(CONFIG_FSI) += fsi-core.o
obj-$(CONFIG_FSI_MASTER_HUB) += fsi-master-hub.o
obj-$(CONFIG_FSI_MASTER_ASPEED) += fsi-master-aspeed.o
obj-$(CONFIG_FSI_MASTER_GPIO) += fsi-master-gpio.o
+obj-$(CONFIG_FSI_MASTER_I2CR) += fsi-master-i2cr.o
obj-$(CONFIG_FSI_MASTER_AST_CF) += fsi-master-ast-cf.o
obj-$(CONFIG_FSI_SCOM) += fsi-scom.o
obj-$(CONFIG_FSI_SBEFIFO) += fsi-sbefifo.o
diff --git a/drivers/fsi/fsi-master-i2cr.c b/drivers/fsi/fsi-master-i2cr.c
new file mode 100644
index 000000000000..61659c27a973
--- /dev/null
+++ b/drivers/fsi/fsi-master-i2cr.c
@@ -0,0 +1,316 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (C) IBM Corporation 2023 */
+
+#include <linux/device.h>
+#include <linux/fsi.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/mutex.h>
+
+#include "fsi-master-i2cr.h"
+
+#define CREATE_TRACE_POINTS
+#include <trace/events/fsi_master_i2cr.h>
+
+#define I2CR_ADDRESS_CFAM(a) ((a) >> 2)
+#define I2CR_INITIAL_PARITY true
+
+#define I2CR_STATUS_CMD 0x60002
+#define I2CR_STATUS_ERR BIT_ULL(61)
+#define I2CR_ERROR_CMD 0x60004
+#define I2CR_LOG_CMD 0x60008
+
+static const u8 i2cr_cfam[] = {
+ 0xc0, 0x02, 0x0d, 0xa6,
+ 0x80, 0x01, 0x10, 0x02,
+ 0x80, 0x01, 0x10, 0x02,
+ 0x80, 0x01, 0x10, 0x02,
+ 0x80, 0x01, 0x80, 0x52,
+ 0x80, 0x01, 0x10, 0x02,
+ 0x80, 0x01, 0x10, 0x02,
+ 0x80, 0x01, 0x10, 0x02,
+ 0x80, 0x01, 0x10, 0x02,
+ 0x80, 0x01, 0x22, 0x2d,
+ 0x00, 0x00, 0x00, 0x00,
+ 0xde, 0xad, 0xc0, 0xde
+};
+
+static bool i2cr_check_parity32(u32 v, bool parity)
+{
+ u32 i;
+
+ for (i = 0; i < 32; ++i) {
+ if (v & (1u << i))
+ parity = !parity;
+ }
+
+ return parity;
+}
+
+static bool i2cr_check_parity64(u64 v)
+{
+ u32 i;
+ bool parity = I2CR_INITIAL_PARITY;
+
+ for (i = 0; i < 64; ++i) {
+ if (v & (1llu << i))
+ parity = !parity;
+ }
+
+ return parity;
+}
+
+static u32 i2cr_get_command(u32 address, bool parity)
+{
+ address <<= 1;
+
+ if (i2cr_check_parity32(address, parity))
+ address |= 1;
+
+ return address;
+}
+
+static int i2cr_transfer(struct i2c_client *client, u32 command, u64 *data)
+{
+ struct i2c_msg msgs[2];
+ int ret;
+
+ msgs[0].addr = client->addr;
+ msgs[0].flags = 0;
+ msgs[0].len = sizeof(command);
+ msgs[0].buf = (__u8 *)&command;
+ msgs[1].addr = client->addr;
+ msgs[1].flags = I2C_M_RD;
+ msgs[1].len = sizeof(*data);
+ msgs[1].buf = (__u8 *)data;
+
+ ret = i2c_transfer(client->adapter, msgs, 2);
+ if (ret == 2)
+ return 0;
+
+ trace_i2cr_i2c_error(client, command, ret);
+
+ if (ret < 0)
+ return ret;
+
+ return -EIO;
+}
+
+static int i2cr_check_status(struct i2c_client *client)
+{
+ u64 status;
+ int ret;
+
+ ret = i2cr_transfer(client, I2CR_STATUS_CMD, &status);
+ if (ret)
+ return ret;
+
+ if (status & I2CR_STATUS_ERR) {
+ u32 buf[3] = { 0, 0, 0 };
+ u64 error;
+ u64 log;
+
+ i2cr_transfer(client, I2CR_ERROR_CMD, &error);
+ i2cr_transfer(client, I2CR_LOG_CMD, &log);
+
+ trace_i2cr_status_error(client, status, error, log);
+
+ buf[0] = I2CR_STATUS_CMD;
+ i2c_master_send(client, (const char *)buf, sizeof(buf));
+
+ buf[0] = I2CR_ERROR_CMD;
+ i2c_master_send(client, (const char *)buf, sizeof(buf));
+
+ buf[0] = I2CR_LOG_CMD;
+ i2c_master_send(client, (const char *)buf, sizeof(buf));
+
+ dev_err(&client->dev, "status:%016llx error:%016llx log:%016llx\n", status, error,
+ log);
+ return -EREMOTEIO;
+ }
+
+ trace_i2cr_status(client, status);
+ return 0;
+}
+
+int fsi_master_i2cr_read(struct fsi_master_i2cr *i2cr, u32 addr, u64 *data)
+{
+ u32 command = i2cr_get_command(addr, I2CR_INITIAL_PARITY);
+ int ret;
+
+ mutex_lock(&i2cr->lock);
+
+ ret = i2cr_transfer(i2cr->client, command, data);
+ if (ret)
+ goto unlock;
+
+ ret = i2cr_check_status(i2cr->client);
+ if (ret)
+ goto unlock;
+
+ trace_i2cr_read(i2cr->client, command, data);
+
+unlock:
+ mutex_unlock(&i2cr->lock);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(fsi_master_i2cr_read);
+
+int fsi_master_i2cr_write(struct fsi_master_i2cr *i2cr, u32 addr, u64 data)
+{
+ u32 buf[3] = { 0 };
+ int ret;
+
+ buf[0] = i2cr_get_command(addr, i2cr_check_parity64(data));
+ memcpy(&buf[1], &data, sizeof(data));
+
+ mutex_lock(&i2cr->lock);
+
+ ret = i2c_master_send(i2cr->client, (const char *)buf, sizeof(buf));
+ if (ret == sizeof(buf)) {
+ ret = i2cr_check_status(i2cr->client);
+ if (!ret)
+ trace_i2cr_write(i2cr->client, buf[0], data);
+ } else {
+ trace_i2cr_i2c_error(i2cr->client, buf[0], ret);
+
+ if (ret >= 0)
+ ret = -EIO;
+ }
+
+ mutex_unlock(&i2cr->lock);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(fsi_master_i2cr_write);
+
+static int i2cr_read(struct fsi_master *master, int link, uint8_t id, uint32_t addr, void *val,
+ size_t size)
+{
+ struct fsi_master_i2cr *i2cr = container_of(master, struct fsi_master_i2cr, master);
+ u64 data;
+ size_t i;
+ int ret;
+
+ if (link || id || (addr & 0xffff0000) || !(size == 1 || size == 2 || size == 4))
+ return -EINVAL;
+
+ /*
+ * The I2CR doesn't have CFAM or FSI slave address space - only the
+ * engines. In order for this to work with the FSI core, we need to
+ * emulate at minimum the CFAM config table so that the appropriate
+ * engines are discovered.
+ */
+ if (addr < 0xc00) {
+ if (addr > sizeof(i2cr_cfam) - 4)
+ addr = (addr & 0x3) + (sizeof(i2cr_cfam) - 4);
+
+ memcpy(val, &i2cr_cfam[addr], size);
+ return 0;
+ }
+
+ ret = fsi_master_i2cr_read(i2cr, I2CR_ADDRESS_CFAM(addr), &data);
+ if (ret)
+ return ret;
+
+ /*
+ * FSI core expects up to 4 bytes BE back, while I2CR replied with LE
+ * bytes on the wire.
+ */
+ for (i = 0; i < size; ++i)
+ ((u8 *)val)[i] = ((u8 *)&data)[7 - i];
+
+ return 0;
+}
+
+static int i2cr_write(struct fsi_master *master, int link, uint8_t id, uint32_t addr,
+ const void *val, size_t size)
+{
+ struct fsi_master_i2cr *i2cr = container_of(master, struct fsi_master_i2cr, master);
+ u64 data = 0;
+ size_t i;
+
+ if (link || id || (addr & 0xffff0000) || !(size == 1 || size == 2 || size == 4))
+ return -EINVAL;
+
+ /* I2CR writes to CFAM or FSI slave address are a successful no-op. */
+ if (addr < 0xc00)
+ return 0;
+
+ /*
+ * FSI core passes up to 4 bytes BE, while the I2CR expects LE bytes on
+ * the wire.
+ */
+ for (i = 0; i < size; ++i)
+ ((u8 *)&data)[7 - i] = ((u8 *)val)[i];
+
+ return fsi_master_i2cr_write(i2cr, I2CR_ADDRESS_CFAM(addr), data);
+}
+
+static void i2cr_release(struct device *dev)
+{
+ struct fsi_master_i2cr *i2cr = to_fsi_master_i2cr(to_fsi_master(dev));
+
+ of_node_put(dev->of_node);
+
+ kfree(i2cr);
+}
+
+static int i2cr_probe(struct i2c_client *client)
+{
+ struct fsi_master_i2cr *i2cr;
+ int ret;
+
+ i2cr = kzalloc(sizeof(*i2cr), GFP_KERNEL);
+ if (!i2cr)
+ return -ENOMEM;
+
+ /* 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);
+ 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.read = i2cr_read;
+ i2cr->master.write = i2cr_write;
+
+ mutex_init(&i2cr->lock);
+ i2cr->client = client;
+
+ ret = fsi_master_register(&i2cr->master);
+ if (ret)
+ return ret;
+
+ i2c_set_clientdata(client, i2cr);
+ return 0;
+}
+
+static void i2cr_remove(struct i2c_client *client)
+{
+ struct fsi_master_i2cr *i2cr = i2c_get_clientdata(client);
+
+ fsi_master_unregister(&i2cr->master);
+}
+
+static const struct of_device_id i2cr_ids[] = {
+ { .compatible = "ibm,i2cr-fsi-master" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, i2cr_ids);
+
+static struct i2c_driver i2cr_driver = {
+ .probe_new = i2cr_probe,
+ .remove = i2cr_remove,
+ .driver = {
+ .name = "fsi-master-i2cr",
+ .of_match_table = i2cr_ids,
+ },
+};
+
+module_i2c_driver(i2cr_driver)
+
+MODULE_AUTHOR("Eddie James <eajames@linux.ibm.com>");
+MODULE_DESCRIPTION("IBM I2C Responder virtual FSI master driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/fsi/fsi-master-i2cr.h b/drivers/fsi/fsi-master-i2cr.h
new file mode 100644
index 000000000000..96636bf28cac
--- /dev/null
+++ b/drivers/fsi/fsi-master-i2cr.h
@@ -0,0 +1,33 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/* Copyright (C) IBM Corporation 2023 */
+
+#ifndef DRIVERS_FSI_MASTER_I2CR_H
+#define DRIVERS_FSI_MASTER_I2CR_H
+
+#include <linux/i2c.h>
+#include <linux/mutex.h>
+
+#include "fsi-master.h"
+
+struct i2c_client;
+
+struct fsi_master_i2cr {
+ struct fsi_master master;
+ struct mutex lock; /* protect HW access */
+ struct i2c_client *client;
+};
+
+#define to_fsi_master_i2cr(m) container_of(m, struct fsi_master_i2cr, master)
+
+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/include/trace/events/fsi_master_i2cr.h b/include/trace/events/fsi_master_i2cr.h
new file mode 100644
index 000000000000..c33eba130049
--- /dev/null
+++ b/include/trace/events/fsi_master_i2cr.h
@@ -0,0 +1,107 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM fsi_master_i2cr
+
+#if !defined(_TRACE_FSI_MASTER_I2CR_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_FSI_MASTER_I2CR_H
+
+#include <linux/tracepoint.h>
+
+TRACE_EVENT(i2cr_i2c_error,
+ TP_PROTO(const struct i2c_client *client, uint32_t command, int rc),
+ TP_ARGS(client, command, rc),
+ TP_STRUCT__entry(
+ __field(int, bus)
+ __field(int, rc)
+ __array(unsigned char, command, sizeof(uint32_t))
+ __field(unsigned short, addr)
+ ),
+ TP_fast_assign(
+ __entry->bus = client->adapter->nr;
+ __entry->rc = rc;
+ memcpy(__entry->command, &command, sizeof(uint32_t));
+ __entry->addr = client->addr;
+ ),
+ TP_printk("%d-%02x command:{ %*ph } rc:%d", __entry->bus, __entry->addr,
+ (int)sizeof(uint32_t), __entry->command, __entry->rc)
+);
+
+TRACE_EVENT(i2cr_read,
+ TP_PROTO(const struct i2c_client *client, uint32_t command, uint64_t *data),
+ TP_ARGS(client, command, data),
+ TP_STRUCT__entry(
+ __field(int, bus)
+ __array(unsigned char, data, sizeof(uint64_t))
+ __array(unsigned char, command, sizeof(uint32_t))
+ __field(unsigned short, addr)
+ ),
+ TP_fast_assign(
+ __entry->bus = client->adapter->nr;
+ memcpy(__entry->data, data, sizeof(uint64_t));
+ memcpy(__entry->command, &command, sizeof(uint32_t));
+ __entry->addr = client->addr;
+ ),
+ TP_printk("%d-%02x command:{ %*ph } { %*ph }", __entry->bus, __entry->addr,
+ (int)sizeof(uint32_t), __entry->command, (int)sizeof(uint64_t), __entry->data)
+);
+
+TRACE_EVENT(i2cr_status,
+ TP_PROTO(const struct i2c_client *client, uint64_t status),
+ TP_ARGS(client, status),
+ TP_STRUCT__entry(
+ __field(uint64_t, status)
+ __field(int, bus)
+ __field(unsigned short, addr)
+ ),
+ TP_fast_assign(
+ __entry->status = status;
+ __entry->bus = client->adapter->nr;
+ __entry->addr = client->addr;
+ ),
+ TP_printk("%d-%02x %016llx", __entry->bus, __entry->addr, __entry->status)
+);
+
+TRACE_EVENT(i2cr_status_error,
+ TP_PROTO(const struct i2c_client *client, uint64_t status, uint64_t error, uint64_t log),
+ TP_ARGS(client, status, error, log),
+ TP_STRUCT__entry(
+ __field(uint64_t, error)
+ __field(uint64_t, log)
+ __field(uint64_t, status)
+ __field(int, bus)
+ __field(unsigned short, addr)
+ ),
+ TP_fast_assign(
+ __entry->error = error;
+ __entry->log = log;
+ __entry->status = status;
+ __entry->bus = client->adapter->nr;
+ __entry->addr = client->addr;
+ ),
+ TP_printk("%d-%02x status:%016llx error:%016llx log:%016llx", __entry->bus, __entry->addr,
+ __entry->status, __entry->error, __entry->log)
+);
+
+TRACE_EVENT(i2cr_write,
+ TP_PROTO(const struct i2c_client *client, uint32_t command, uint64_t data),
+ TP_ARGS(client, command, data),
+ TP_STRUCT__entry(
+ __field(int, bus)
+ __array(unsigned char, data, sizeof(uint64_t))
+ __array(unsigned char, command, sizeof(uint32_t))
+ __field(unsigned short, addr)
+ ),
+ TP_fast_assign(
+ __entry->bus = client->adapter->nr;
+ memcpy(__entry->data, &data, sizeof(uint64_t));
+ memcpy(__entry->command, &command, sizeof(uint32_t));
+ __entry->addr = client->addr;
+ ),
+ TP_printk("%d-%02x command:{ %*ph } { %*ph }", __entry->bus, __entry->addr,
+ (int)sizeof(uint32_t), __entry->command, (int)sizeof(uint64_t), __entry->data)
+);
+
+#endif
+
+#include <trace/define_trace.h>
--
2.31.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 14/14] fsi: Add I2C Responder SCOM driver
2023-06-12 19:56 [PATCH 00/14] fsi: Miscellaneous fixes and I2C Responder driver Eddie James
` (12 preceding siblings ...)
2023-06-12 19:56 ` [PATCH 13/14] fsi: Add " Eddie James
@ 2023-06-12 19:56 ` Eddie James
2023-08-17 15:56 ` Rob Herring
13 siblings, 1 reply; 21+ messages in thread
From: Eddie James @ 2023-06-12 19:56 UTC (permalink / raw)
To: linux-fsi
Cc: linux-kernel, jk, joel, alistair, andrew, robh+dt,
krzysztof.kozlowski+dt, conor+dt, devicetree
The I2CR has the capability to directly perform SCOM operations,
circumventing the need to drive the FSI2PIB engine. Add a new
driver to perform SCOM operations through the I2CR.
Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
drivers/fsi/Kconfig | 8 +++
drivers/fsi/Makefile | 1 +
drivers/fsi/i2cr-scom.c | 154 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 163 insertions(+)
create mode 100644 drivers/fsi/i2cr-scom.c
diff --git a/drivers/fsi/Kconfig b/drivers/fsi/Kconfig
index 999be82720c5..79a31593618a 100644
--- a/drivers/fsi/Kconfig
+++ b/drivers/fsi/Kconfig
@@ -94,4 +94,12 @@ config FSI_OCC
provide the raw sensor data as well as perform thermal and power
management on the system.
+config I2CR_SCOM
+ tristate "IBM I2C Responder SCOM driver"
+ depends on FSI_MASTER_I2CR
+ help
+ This option enables an I2C Responder based SCOM device driver. The
+ I2CR has the capability to directly perform SCOM operations instead
+ of using the FSI2PIB engine.
+
endif
diff --git a/drivers/fsi/Makefile b/drivers/fsi/Makefile
index 34dbaa1c452e..5550aa15e0b1 100644
--- a/drivers/fsi/Makefile
+++ b/drivers/fsi/Makefile
@@ -9,3 +9,4 @@ obj-$(CONFIG_FSI_MASTER_AST_CF) += fsi-master-ast-cf.o
obj-$(CONFIG_FSI_SCOM) += fsi-scom.o
obj-$(CONFIG_FSI_SBEFIFO) += fsi-sbefifo.o
obj-$(CONFIG_FSI_OCC) += fsi-occ.o
+obj-$(CONFIG_I2CR_SCOM) += i2cr-scom.o
diff --git a/drivers/fsi/i2cr-scom.c b/drivers/fsi/i2cr-scom.c
new file mode 100644
index 000000000000..cb7e02213032
--- /dev/null
+++ b/drivers/fsi/i2cr-scom.c
@@ -0,0 +1,154 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (C) IBM Corporation 2023 */
+
+#include <linux/cdev.h>
+#include <linux/device.h>
+#include <linux/fs.h>
+#include <linux/fsi.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+
+#include "fsi-master-i2cr.h"
+#include "fsi-slave.h"
+
+struct i2cr_scom {
+ struct device dev;
+ struct cdev cdev;
+ struct fsi_master_i2cr *i2cr;
+};
+
+static loff_t i2cr_scom_llseek(struct file *file, loff_t offset, int whence)
+{
+ switch (whence) {
+ case SEEK_CUR:
+ break;
+ case SEEK_SET:
+ file->f_pos = offset;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return offset;
+}
+
+static ssize_t i2cr_scom_read(struct file *filep, char __user *buf, size_t len, loff_t *offset)
+{
+ struct i2cr_scom *scom = filep->private_data;
+ u64 data;
+ int ret;
+
+ if (len != sizeof(data))
+ return -EINVAL;
+
+ ret = fsi_master_i2cr_read(scom->i2cr, (u32)*offset, &data);
+ if (ret)
+ return ret;
+
+ ret = copy_to_user(buf, &data, len);
+ if (ret)
+ return ret;
+
+ return len;
+}
+
+static ssize_t i2cr_scom_write(struct file *filep, const char __user *buf, size_t len,
+ loff_t *offset)
+{
+ struct i2cr_scom *scom = filep->private_data;
+ u64 data;
+ int ret;
+
+ if (len != sizeof(data))
+ return -EINVAL;
+
+ ret = copy_from_user(&data, buf, len);
+ if (ret)
+ return ret;
+
+ ret = fsi_master_i2cr_write(scom->i2cr, (u32)*offset, data);
+ if (ret)
+ return ret;
+
+ return len;
+}
+
+static const struct file_operations i2cr_scom_fops = {
+ .owner = THIS_MODULE,
+ .open = simple_open,
+ .llseek = i2cr_scom_llseek,
+ .read = i2cr_scom_read,
+ .write = i2cr_scom_write,
+};
+
+static int i2cr_scom_probe(struct device *dev)
+{
+ struct fsi_device *fsi_dev = to_fsi_dev(dev);
+ struct i2cr_scom *scom;
+ int didx;
+ int ret;
+
+ if (!is_fsi_master_i2cr(fsi_dev->slave->master))
+ return -ENODEV;
+
+ scom = devm_kzalloc(dev, sizeof(*scom), GFP_KERNEL);
+ if (!scom)
+ return -ENOMEM;
+
+ scom->i2cr = to_fsi_master_i2cr(fsi_dev->slave->master);
+ dev_set_drvdata(dev, scom);
+
+ scom->dev.type = &fsi_cdev_type;
+ scom->dev.parent = dev;
+ device_initialize(&scom->dev);
+
+ ret = fsi_get_new_minor(fsi_dev, fsi_dev_scom, &scom->dev.devt, &didx);
+ if (ret)
+ return ret;
+
+ dev_set_name(&scom->dev, "scom%d", didx);
+ cdev_init(&scom->cdev, &i2cr_scom_fops);
+ ret = cdev_device_add(&scom->cdev, &scom->dev);
+ if (ret)
+ fsi_free_minor(scom->dev.devt);
+
+ return ret;
+}
+
+static int i2cr_scom_remove(struct device *dev)
+{
+ struct i2cr_scom *scom = dev_get_drvdata(dev);
+
+ cdev_device_del(&scom->cdev, &scom->dev);
+ fsi_free_minor(scom->dev.devt);
+
+ return 0;
+}
+
+static const struct of_device_id i2cr_scom_of_ids[] = {
+ { .compatible = "ibm,i2cr-scom" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, i2cr_scom_of_ids);
+
+static const struct fsi_device_id i2cr_scom_ids[] = {
+ { 0x5, FSI_VERSION_ANY },
+ { }
+};
+
+static struct fsi_driver i2cr_scom_driver = {
+ .id_table = i2cr_scom_ids,
+ .drv = {
+ .name = "i2cr_scom",
+ .bus = &fsi_bus_type,
+ .of_match_table = i2cr_scom_of_ids,
+ .probe = i2cr_scom_probe,
+ .remove = i2cr_scom_remove,
+ }
+};
+
+module_fsi_driver(i2cr_scom_driver);
+
+MODULE_AUTHOR("Eddie James <eajames@linux.ibm.com>");
+MODULE_DESCRIPTION("IBM I2C Responder SCOM driver");
+MODULE_LICENSE("GPL");
--
2.31.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 14/14] fsi: Add I2C Responder SCOM driver
2023-06-12 19:56 ` [PATCH 14/14] fsi: Add I2C Responder SCOM driver Eddie James
@ 2023-08-17 15:56 ` Rob Herring
0 siblings, 0 replies; 21+ messages in thread
From: Rob Herring @ 2023-08-17 15:56 UTC (permalink / raw)
To: Eddie James
Cc: linux-fsi, linux-kernel, jk, joel, alistair, andrew,
krzysztof.kozlowski+dt, conor+dt, devicetree
On Mon, Jun 12, 2023 at 02:56:57PM -0500, Eddie James wrote:
> The I2CR has the capability to directly perform SCOM operations,
> circumventing the need to drive the FSI2PIB engine. Add a new
> driver to perform SCOM operations through the I2CR.
>
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> ---
> drivers/fsi/Kconfig | 8 +++
> drivers/fsi/Makefile | 1 +
> drivers/fsi/i2cr-scom.c | 154 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 163 insertions(+)
> create mode 100644 drivers/fsi/i2cr-scom.c
>
> diff --git a/drivers/fsi/Kconfig b/drivers/fsi/Kconfig
> index 999be82720c5..79a31593618a 100644
> --- a/drivers/fsi/Kconfig
> +++ b/drivers/fsi/Kconfig
> @@ -94,4 +94,12 @@ config FSI_OCC
> provide the raw sensor data as well as perform thermal and power
> management on the system.
>
> +config I2CR_SCOM
> + tristate "IBM I2C Responder SCOM driver"
> + depends on FSI_MASTER_I2CR
> + help
> + This option enables an I2C Responder based SCOM device driver. The
> + I2CR has the capability to directly perform SCOM operations instead
> + of using the FSI2PIB engine.
> +
> endif
> diff --git a/drivers/fsi/Makefile b/drivers/fsi/Makefile
> index 34dbaa1c452e..5550aa15e0b1 100644
> --- a/drivers/fsi/Makefile
> +++ b/drivers/fsi/Makefile
> @@ -9,3 +9,4 @@ obj-$(CONFIG_FSI_MASTER_AST_CF) += fsi-master-ast-cf.o
> obj-$(CONFIG_FSI_SCOM) += fsi-scom.o
> obj-$(CONFIG_FSI_SBEFIFO) += fsi-sbefifo.o
> obj-$(CONFIG_FSI_OCC) += fsi-occ.o
> +obj-$(CONFIG_I2CR_SCOM) += i2cr-scom.o
> diff --git a/drivers/fsi/i2cr-scom.c b/drivers/fsi/i2cr-scom.c
> new file mode 100644
> index 000000000000..cb7e02213032
> --- /dev/null
> +++ b/drivers/fsi/i2cr-scom.c
> @@ -0,0 +1,154 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (C) IBM Corporation 2023 */
> +
> +#include <linux/cdev.h>
> +#include <linux/device.h>
> +#include <linux/fs.h>
> +#include <linux/fsi.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +
> +#include "fsi-master-i2cr.h"
> +#include "fsi-slave.h"
> +
> +struct i2cr_scom {
> + struct device dev;
> + struct cdev cdev;
> + struct fsi_master_i2cr *i2cr;
> +};
> +
> +static loff_t i2cr_scom_llseek(struct file *file, loff_t offset, int whence)
> +{
> + switch (whence) {
> + case SEEK_CUR:
> + break;
> + case SEEK_SET:
> + file->f_pos = offset;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return offset;
> +}
> +
> +static ssize_t i2cr_scom_read(struct file *filep, char __user *buf, size_t len, loff_t *offset)
> +{
> + struct i2cr_scom *scom = filep->private_data;
> + u64 data;
> + int ret;
> +
> + if (len != sizeof(data))
> + return -EINVAL;
> +
> + ret = fsi_master_i2cr_read(scom->i2cr, (u32)*offset, &data);
> + if (ret)
> + return ret;
> +
> + ret = copy_to_user(buf, &data, len);
> + if (ret)
> + return ret;
> +
> + return len;
> +}
> +
> +static ssize_t i2cr_scom_write(struct file *filep, const char __user *buf, size_t len,
> + loff_t *offset)
> +{
> + struct i2cr_scom *scom = filep->private_data;
> + u64 data;
> + int ret;
> +
> + if (len != sizeof(data))
> + return -EINVAL;
> +
> + ret = copy_from_user(&data, buf, len);
> + if (ret)
> + return ret;
> +
> + ret = fsi_master_i2cr_write(scom->i2cr, (u32)*offset, data);
> + if (ret)
> + return ret;
> +
> + return len;
> +}
> +
> +static const struct file_operations i2cr_scom_fops = {
> + .owner = THIS_MODULE,
> + .open = simple_open,
> + .llseek = i2cr_scom_llseek,
> + .read = i2cr_scom_read,
> + .write = i2cr_scom_write,
> +};
> +
> +static int i2cr_scom_probe(struct device *dev)
> +{
> + struct fsi_device *fsi_dev = to_fsi_dev(dev);
> + struct i2cr_scom *scom;
> + int didx;
> + int ret;
> +
> + if (!is_fsi_master_i2cr(fsi_dev->slave->master))
> + return -ENODEV;
> +
> + scom = devm_kzalloc(dev, sizeof(*scom), GFP_KERNEL);
> + if (!scom)
> + return -ENOMEM;
> +
> + scom->i2cr = to_fsi_master_i2cr(fsi_dev->slave->master);
> + dev_set_drvdata(dev, scom);
> +
> + scom->dev.type = &fsi_cdev_type;
> + scom->dev.parent = dev;
> + device_initialize(&scom->dev);
> +
> + ret = fsi_get_new_minor(fsi_dev, fsi_dev_scom, &scom->dev.devt, &didx);
> + if (ret)
> + return ret;
> +
> + dev_set_name(&scom->dev, "scom%d", didx);
> + cdev_init(&scom->cdev, &i2cr_scom_fops);
> + ret = cdev_device_add(&scom->cdev, &scom->dev);
> + if (ret)
> + fsi_free_minor(scom->dev.devt);
> +
> + return ret;
> +}
> +
> +static int i2cr_scom_remove(struct device *dev)
> +{
> + struct i2cr_scom *scom = dev_get_drvdata(dev);
> +
> + cdev_device_del(&scom->cdev, &scom->dev);
> + fsi_free_minor(scom->dev.devt);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id i2cr_scom_of_ids[] = {
> + { .compatible = "ibm,i2cr-scom" },
This is not documented. Please add a binding.
Rob
^ permalink raw reply [flat|nested] 21+ messages in thread