Skip to contentSkip to conversation

fsi: Add IBM I2C Responder virtual FSI master

PATCH New Eddie James 1 participant 9 messages Mar 6, 2023

Subsystems (4)
lore
Share & tools
lore

Eddie James

author maintainer
#0 lore raw

v8 cover fsi: Add IBM I2C Responder virtual FSI master

The I2C Responder (I2CR) is an I2C device that translates I2C commands to CFAM or SCOM operations, effectively implementing an FSI master and bus.

Changes since v7:
- Add FSI master device naming patch
- Use I2C adapter number as FSI master index with "i2cr" name
- Export the fsi_master_i2cr_read/write functions for the SCOM driver

Changes since v6:
- Drop conversions to BE except in the FSI core interface
- Emulate the CFAM config table since it doesn't exist off the I2CR
- Drop status check during probe
- Add release function for device
- Update SCOM driver with newer interfaces and drop BE conversion
- New sbefifo change to not check state during probe

Changes since v5:
- Make I2CR scom driver depend on I2CR FSI master

Changes since v4:
- Add I2CR scom driver and associated patches
- Use compatible strings for FSI drivers if specified
- Include aliased device numbering patch
- Restructure the trace events to eliminate holes

Changes since v3:
- Rework the endian-ness in i2cr_write
- Rework the tracing to include the i2c bus and device address

Changes since v2:
- Fix the bindings again, sorry for the spam

Changes since v1:
- Fix the binding document
- Change the binding name
- Clean up the size argument checking
- Reduce __force by using packed struct for the command

Eddie James (8):
fsi: Move fsi_slave structure definition to header
fsi: Make master device indexing and naming optional
dt-bindings: fsi: Document the IBM I2C Responder virtual FSI master
fsi: Add IBM I2C Responder virtual FSI master
fsi: Add I2C Responder SCOM driver
fsi: Add aliased device numbering
fsi: Use of_match_table for bus matching if specified
fsi: sbefifo: Don't check status during probe

 .../bindings/fsi/ibm,i2cr-fsi-master.yaml     |  41 +++
 drivers/fsi/Kconfig                           |  17 +
 drivers/fsi/Makefile                          |   2 +
 drivers/fsi/fsi-core.c                        |  73 ++--
 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-i2cr.c                 | 318 ++++++++++++++++++
 drivers/fsi/fsi-master-i2cr.h                 |  24 ++
 drivers/fsi/fsi-master.h                      |   3 +-
 drivers/fsi/fsi-sbefifo.c                     |   8 -
 drivers/fsi/fsi-scom.c                        |   8 +
 drivers/fsi/fsi-slave.h                       |  28 ++
 drivers/fsi/i2cr-scom.c                       | 154 +++++++++
 include/trace/events/fsi_master_i2cr.h        | 107 ++++++
 16 files changed, 752 insertions(+), 39 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/fsi/ibm,i2cr-fsi-master.yaml
 create mode 100644 drivers/fsi/fsi-master-i2cr.c
 create mode 100644 drivers/fsi/fsi-master-i2cr.h
 create mode 100644 drivers/fsi/fsi-slave.h
 create mode 100644 drivers/fsi/i2cr-scom.c
 create mode 100644 include/trace/events/fsi_master_i2cr.h
Signature · 1 line
2.31.1
author maintainer
#1 lore raw

PATCH v8 1/8 fsi: Move fsi_slave structure definition to header

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.

---
 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
drivers/fsi/fsi-core.c +4 -20
index 0b927c9f4267..d591e68afd11 100644
@@ -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;
 
drivers/fsi/fsi-master-aspeed.c +1 -1
index 7cec1772820d..437f87b4a6a3 100644
@@ -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);
 }
drivers/fsi/fsi-master-ast-cf.c +1 -1
index 5f608ef8b53c..6124978305eb 100644
@@ -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);
drivers/fsi/fsi-master-gpio.c +1 -1
index 7d5f29b4b595..ed03da4f2447 100644
@@ -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));
 
drivers/fsi/fsi-master-hub.c +1 -1
index 01f0a796111e..6d8b6e8854e5 100644
@@ -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);
 }
drivers/fsi/fsi-master.h +1 -1
index 4762315a46ba..967622c1cabf 100644
@@ -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
drivers/fsi/fsi-slave.h new file +28 -0
new file mode 100644
index 000000000000..1d63a585829d
@@ -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 */
Signature · 1 line
2.31.1
author maintainer
#2 lore raw

PATCH v8 2/8 fsi: Make master device indexing and naming optional

In reply to Eddie James

Some FSI masters need different device naming, so make the indexing and naming an optional part of FSI master registration.

---
 drivers/fsi/fsi-core.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)
drivers/fsi/fsi-core.c +9 -4
index d591e68afd11..3271e40702ec 100644
@@ -1297,11 +1297,16 @@ 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);
-	if (master->idx < 0)
-		return master->idx;
 
-	dev_set_name(&master->dev, "fsi%d", master->idx);
+	if (!master->idx) {
+		master->idx = ida_simple_get(&master_ida, 0, INT_MAX,
+					     GFP_KERNEL);
+		if (master->idx < 0)
+			return master->idx;
+
+		dev_set_name(&master->dev, "fsi%d", master->idx);
+	}
+
 	master->dev.class = &fsi_master_class;
 
 	rc = device_register(&master->dev);
Signature · 1 line
2.31.1
author maintainer
#3 lore raw

PATCH v8 3/8 dt-bindings: fsi: Document the IBM I2C Responder virtual FSI master

In reply to Eddie James

The I2C Responder translates I2C commands to CFAM or SCOM operations, effectively implementing an FSI master.

---
 .../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
Documentation/devicetree/bindings/fsi/ibm,i2cr-fsi-master.yaml new file +41 -0
new file mode 100644
index 000000000000..442cecdc57cb
@@ -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 <[email protected]>
+
+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>;
+      };
+    };
Signature · 1 line
2.31.1
author maintainer
#4 lore raw

PATCH v8 4/8 fsi: Add IBM I2C Responder virtual FSI master

In reply to Eddie James

The I2C Responder (I2CR) is an I2C device that translates I2C commands to CFAM or SCOM operations, effectively implementing an FSI master and bus.

---
 drivers/fsi/Kconfig                    |   9 +
 drivers/fsi/Makefile                   |   1 +
 drivers/fsi/fsi-master-i2cr.c          | 318 +++++++++++++++++++++++++
 drivers/fsi/fsi-master-i2cr.h          |  24 ++
 drivers/fsi/fsi-master.h               |   1 +
 include/trace/events/fsi_master_i2cr.h | 107 +++++++++
 6 files changed, 460 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
drivers/fsi/Kconfig +9 -0
index e6668a869913..999be82720c5 100644
@@ -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
drivers/fsi/Makefile +1 -0
index da218a1ad8e1..34dbaa1c452e 100644
@@ -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
drivers/fsi/fsi-master-i2cr.c new file +318 -0

318 lines not on this page · raw message

drivers/fsi/fsi-master-i2cr.h new file +24 -0
new file mode 100644
index 000000000000..929d63995c7b
@@ -0,0 +1,24 @@
+/* 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/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);
+
+#endif /* DRIVERS_FSI_MASTER_I2CR_H */
drivers/fsi/fsi-master.h +1 -0
index 967622c1cabf..a1fa315849d2 100644
@@ -111,6 +111,7 @@
 
 /* fsi-master definition and flags */
 #define FSI_MASTER_FLAG_SWCLOCK		0x1
+#define FSI_MASTER_FLAG_I2CR		0x2
 
 /*
  * Structures and function prototypes
include/trace/events/fsi_master_i2cr.h new file +107 -0

107 lines not on this page · raw message

Signature · 1 line
2.31.1
author maintainer
#5 lore raw

PATCH v8 5/8 fsi: Add I2C Responder SCOM driver

In reply to Eddie James

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.

---
 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
drivers/fsi/Kconfig +8 -0
index 999be82720c5..79a31593618a 100644
@@ -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
drivers/fsi/Makefile +1 -0
index 34dbaa1c452e..5550aa15e0b1 100644
@@ -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
drivers/fsi/i2cr-scom.c new file +154 -0

154 lines not on this page · raw message

Signature · 1 line
2.31.1
author maintainer
#6 lore raw

PATCH v8 6/8 fsi: Add aliased device numbering

In reply to Eddie James

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.

---
 drivers/fsi/fsi-core.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)
drivers/fsi/fsi-core.c +25 -0
index 3271e40702ec..48f25dc0fad7 100644
@@ -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);
Signature · 1 line
2.31.1
author maintainer
#7 lore raw

PATCH v8 7/8 fsi: Use of_match_table for bus matching if specified

In reply to Eddie James

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.

---
 drivers/fsi/fsi-core.c | 11 +++++++++--
 drivers/fsi/fsi-scom.c |  8 ++++++++
 2 files changed, 17 insertions(+), 2 deletions(-)
drivers/fsi/fsi-core.c +9 -2
index 48f25dc0fad7..5191f6979346 100644
@@ -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>
@@ -1380,8 +1381,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;
drivers/fsi/fsi-scom.c +8 -0
index bcb756dc9866..61dbda9dbe2b 100644
@@ -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,
 	}
Signature · 1 line
2.31.1
author maintainer
#8 lore raw

PATCH v8 8/8 fsi: sbefifo: Don't check status during probe

In reply to Eddie James

The status check during probe doesn't serve any purpose. Any attempt to use the SBEFIFO will result in the same check and cleanup.

---
 drivers/fsi/fsi-sbefifo.c | 8 --------
 1 file changed, 8 deletions(-)
drivers/fsi/fsi-sbefifo.c +0 -8
index 9912b7a6a4b9..42d7c95528d1 100644
@@ -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;
Signature · 1 line
2.31.1