Merge tag 'fsi-for-v5.18' of git://git.kernel.org/pub/scm/linux/kernel/git/joel/fsi into char-misc-next

Joel writes:

FSI changes for v5.18

* Improvements in SCOM and OCC drivers for error handling and retries

* Addition of tracepoints for initialisation path

* API for setting long running SBE FIFO operations

* tag 'fsi-for-v5.18' of git://git.kernel.org/pub/scm/linux/kernel/git/joel/fsi:
fsi: Add trace events in initialization path
fsi: sbefifo: Implement FSI_SBEFIFO_READ_TIMEOUT_SECONDS ioctl
fsi: sbefifo: Use specified value of start of response timeout
fsi: occ: Improve response status checking
fsi: scom: Remove retries in indirect scoms
fsi: scom: Fix error handling

drivers/fsi/fsi-core.c

···
 
 #include "fsi-master.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)
 #define FSI_SLAVE_CONF_SLOTS_SHIFT	16
···
 	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)
···
 			dev->unit = i;
 			dev->addr = engine_addr;
 			dev->size = slots * engine_page_size;
+
+			trace_fsi_dev_init(dev);
 
 			dev_dbg(&slave->dev,
 			"engine[%i]: type %x, version %x, addr %x size %x\n",
···
 
 	crc = crc4(0, cfam_id, 32);
 	if (crc) {
+		trace_fsi_slave_invalid_cfam(master, link, cfam_id);
 		dev_warn(&master->dev, "slave %02x:%02x invalid cfam id CRC!\n",
 				link, id);
 		return -EIO;
···
 				 &slave->cdev_idx);
 	if (rc)
 		goto err_free;
+
+	trace_fsi_slave_init(slave);
 
 	/* Create chardev for userspace access */
 	cdev_init(&slave->cdev, &cfam_fops);

drivers/fsi/fsi-master-aspeed.c

···
 {
 	struct fsi_master_aspeed *aspeed = dev_get_drvdata(dev);
 
+	trace_fsi_master_aspeed_cfam_reset(true);
 	mutex_lock(&aspeed->lock);
 	gpiod_set_value(aspeed->cfam_reset_gpio, 1);
 	usleep_range(900, 1000);
 	gpiod_set_value(aspeed->cfam_reset_gpio, 0);
 	mutex_unlock(&aspeed->lock);
+	trace_fsi_master_aspeed_cfam_reset(false);
 
 	return count;
 }

drivers/fsi/fsi-occ.c

···
 	return rc;
 }
 
+static bool fsi_occ_response_not_ready(struct occ_response *resp, u8 seq_no,
+				       u8 cmd_type)
+{
+	return resp->return_status == OCC_RESP_CMD_IN_PRG ||
+		resp->return_status == OCC_RESP_CRIT_INIT ||
+		resp->seq_no != seq_no || resp->cmd_type != cmd_type;
+}
+
 int fsi_occ_submit(struct device *dev, const void *request, size_t req_len,
 		   void *response, size_t *resp_len)
 {
···
 	struct occ_response *resp = response;
 	size_t user_resp_len = *resp_len;
 	u8 seq_no;
+	u8 cmd_type;
 	u16 checksum = 0;
 	u16 resp_data_length;
 	const u8 *byte_request = (const u8 *)request;
-	unsigned long start;
+	unsigned long end;
 	int rc;
 	size_t i;
 
···
 		dev_dbg(dev, "Bad resplen %zd\n", user_resp_len);
 		return -EINVAL;
 	}
+
+	cmd_type = byte_request[1];
 
 	/* Checksum the request, ignoring first byte (sequence number). */
 	for (i = 1; i < req_len - 2; ++i)
···
 	if (rc)
 		goto done;
 
-	/* Read occ response header */
-	start = jiffies;
-	do {
+	end = jiffies + timeout;
+	while (true) {
+		/* Read occ response header */
 		rc = occ_getsram(occ, 0, resp, 8);
 		if (rc)
 			goto done;
 
-		if (resp->return_status == OCC_RESP_CMD_IN_PRG ||
-		    resp->return_status == OCC_RESP_CRIT_INIT ||
-		    resp->seq_no != seq_no) {
-			rc = -ETIMEDOUT;
-
-			if (time_after(jiffies, start + timeout)) {
-				dev_err(occ->dev, "resp timeout status=%02x "
-					"resp seq_no=%d our seq_no=%d\n",
+		if (fsi_occ_response_not_ready(resp, seq_no, cmd_type)) {
+			if (time_after(jiffies, end)) {
+				dev_err(occ->dev,
+					"resp timeout status=%02x seq=%d cmd=%d, our seq=%d cmd=%d\n",
 					resp->return_status, resp->seq_no,
-					seq_no);
+					resp->cmd_type, seq_no, cmd_type);
+				rc = -ETIMEDOUT;
 				goto done;
 			}
 
 			set_current_state(TASK_UNINTERRUPTIBLE);
 			schedule_timeout(wait_time);
+		} else {
+			/* Extract size of response data */
+			resp_data_length =
+				get_unaligned_be16(&resp->data_length);
+
+			/*
+			 * Message size is data length + 5 bytes header + 2
+			 * bytes checksum
+			 */
+			if ((resp_data_length + 7) > user_resp_len) {
+				rc = -EMSGSIZE;
+				goto done;
+			}
+
+			/*
+			 * Get the entire response including the header again,
+			 * in case it changed
+			 */
+			if (resp_data_length > 1) {
+				rc = occ_getsram(occ, 0, resp,
+						 resp_data_length + 7);
+				if (rc)
+					goto done;
+
+				if (!fsi_occ_response_not_ready(resp, seq_no,
+								cmd_type))
+					break;
+			} else {
+				break;
+			}
 		}
-	} while (rc);
-
-	/* Extract size of response data */
-	resp_data_length = get_unaligned_be16(&resp->data_length);
-
-	/* Message size is data length + 5 bytes header + 2 bytes checksum */
-	if ((resp_data_length + 7) > user_resp_len) {
-		rc = -EMSGSIZE;
-		goto done;
 	}
 
 	dev_dbg(dev, "resp_status=%02x resp_data_len=%d\n",
 		resp->return_status, resp_data_length);
-
-	/* Grab the rest */
-	if (resp_data_length > 1) {
-		/* already got 3 bytes resp, also need 2 bytes checksum */
-		rc = occ_getsram(occ, 8, &resp->data[3], resp_data_length - 1);
-		if (rc)
-			goto done;
-	}
 
 	occ->client_response_size = resp_data_length + 7;
 	rc = occ_verify_checksum(occ, resp, resp_data_length);
···
 	occ->version = (uintptr_t)of_device_get_match_data(dev);
 	occ->dev = dev;
 	occ->sbefifo = dev->parent;
-	occ->sequence_number = 1;
+	/*
+	 * Quickly derive a pseudo-random number from jiffies so that
+	 * re-probing the driver doesn't accidentally overlap sequence numbers.
+	 */
+	occ->sequence_number = (u8)((jiffies % 0xff) + 1);
 	mutex_init(&occ->occ_lock);
 
 	if (dev->of_node) {

drivers/fsi/fsi-sbefifo.c

···
 #include <linux/vmalloc.h>
 #include <linux/mm.h>
 
+#include <uapi/linux/fsi.h>
+
 /*
  * The SBEFIFO is a pipe-like FSI device for communicating with
  * the self boot engine on POWER processors.
···
 	bool			dead;
 	bool			async_ffdc;
 	bool			timed_out;
+	u32			timeout_start_rsp_ms;
 };
 
 struct sbefifo_user {
···
 	void			*cmd_page;
 	void			*pending_cmd;
 	size_t			pending_len;
+	u32			read_timeout_ms;
 };
 
 static DEFINE_MUTEX(sbefifo_ffdc_mutex);
···
 
 	dev_vdbg(dev, "reading response, buflen = %zd\n", iov_iter_count(response));
 
-	timeout = msecs_to_jiffies(SBEFIFO_TIMEOUT_START_RSP);
+	timeout = msecs_to_jiffies(sbefifo->timeout_start_rsp_ms);
 	for (;;) {
 		/* Grab FIFO status (this will handle parity errors) */
 		rc = sbefifo_wait(sbefifo, false, &status, timeout);
···
 		return -ENOMEM;
 	}
 	mutex_init(&user->file_lock);
+	user->read_timeout_ms = SBEFIFO_TIMEOUT_START_RSP;
 
 	return 0;
 }
···
 	rc = mutex_lock_interruptible(&sbefifo->lock);
 	if (rc)
 		goto bail;
+	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;
 	mutex_unlock(&sbefifo->lock);
 	if (rc < 0)
 		goto bail;
···
 	return 0;
 }
 
+static int sbefifo_read_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->read_timeout_ms = SBEFIFO_TIMEOUT_START_RSP;
+		dev_dbg(dev, "Timeout reset to %d\n", user->read_timeout_ms);
+		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);
+
+	return 0;
+}
+
+static long sbefifo_user_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+{
+	struct sbefifo_user *user = file->private_data;
+	int rc = -ENOTTY;
+
+	if (!user)
+		return -EINVAL;
+
+	mutex_lock(&user->file_lock);
+	switch (cmd) {
+	case FSI_SBEFIFO_READ_TIMEOUT_SECONDS:
+		rc = sbefifo_read_timeout(user, (void __user *)arg);
+		break;
+	}
+	mutex_unlock(&user->file_lock);
+	return rc;
+}
+
 static const struct file_operations sbefifo_fops = {
 	.owner		= THIS_MODULE,
 	.open		= sbefifo_user_open,
 	.read		= sbefifo_user_read,
 	.write		= sbefifo_user_write,
 	.release	= sbefifo_user_release,
+	.unlocked_ioctl = sbefifo_user_ioctl,
 };
 
 static void sbefifo_free(struct device *dev)
···
 	sbefifo->fsi_dev = fsi_dev;
 	dev_set_drvdata(dev, sbefifo);
 	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

drivers/fsi/fsi-scom.c

···
 				   uint64_t addr, uint32_t *status)
 {
 	uint64_t ind_data, ind_addr;
-	int rc, retries, err = 0;
+	int rc, err;
 
 	if (value & ~XSCOM_DATA_IND_DATA)
 		return -EINVAL;
···
 	if (rc || (*status & SCOM_STATUS_ANY_ERR))
 		return rc;
 
-	for (retries = 0; retries < SCOM_MAX_IND_RETRIES; retries++) {
-		rc = __get_scom(scom, &ind_data, addr, status);
-		if (rc || (*status & SCOM_STATUS_ANY_ERR))
-			return rc;
+	rc = __get_scom(scom, &ind_data, addr, status);
+	if (rc || (*status & SCOM_STATUS_ANY_ERR))
+		return rc;
 
-		err = (ind_data & XSCOM_DATA_IND_ERR_MASK) >> XSCOM_DATA_IND_ERR_SHIFT;
-		*status = err << SCOM_STATUS_PIB_RESP_SHIFT;
-		if ((ind_data & XSCOM_DATA_IND_COMPLETE) || (err != SCOM_PIB_BLOCKED))
-			return 0;
+	err = (ind_data & XSCOM_DATA_IND_ERR_MASK) >> XSCOM_DATA_IND_ERR_SHIFT;
+	*status = err << SCOM_STATUS_PIB_RESP_SHIFT;
 
-		msleep(1);
-	}
-	return rc;
+	return 0;
 }
 
 static int put_indirect_scom_form1(struct scom_device *scom, uint64_t value,
···
 				   uint64_t addr, uint32_t *status)
 {
 	uint64_t ind_data, ind_addr;
-	int rc, retries, err = 0;
+	int rc, err;
 
 	ind_addr = addr & XSCOM_ADDR_DIRECT_PART;
 	ind_data = (addr & XSCOM_ADDR_INDIRECT_PART) | XSCOM_DATA_IND_READ;
···
 	if (rc || (*status & SCOM_STATUS_ANY_ERR))
 		return rc;
 
-	for (retries = 0; retries < SCOM_MAX_IND_RETRIES; retries++) {
-		rc = __get_scom(scom, &ind_data, addr, status);
-		if (rc || (*status & SCOM_STATUS_ANY_ERR))
-			return rc;
+	rc = __get_scom(scom, &ind_data, addr, status);
+	if (rc || (*status & SCOM_STATUS_ANY_ERR))
+		return rc;
 
-		err = (ind_data & XSCOM_DATA_IND_ERR_MASK) >> XSCOM_DATA_IND_ERR_SHIFT;
-		*status = err << SCOM_STATUS_PIB_RESP_SHIFT;
-		*value = ind_data & XSCOM_DATA_IND_DATA;
+	err = (ind_data & XSCOM_DATA_IND_ERR_MASK) >> XSCOM_DATA_IND_ERR_SHIFT;
+	*status = err << SCOM_STATUS_PIB_RESP_SHIFT;
+	*value = ind_data & XSCOM_DATA_IND_DATA;
 
-		if ((ind_data & XSCOM_DATA_IND_COMPLETE) || (err != SCOM_PIB_BLOCKED))
-			return 0;
-
-		msleep(1);
-	}
-	return rc;
+	return 0;
 }
 
 static int raw_put_scom(struct scom_device *scom, uint64_t value,
···
 	int rc;
 
 	rc = raw_put_scom(scom, value, addr, &status);
-	if (rc == -ENODEV)
+	if (rc)
 		return rc;
 
 	rc = handle_fsi2pib_status(scom, status);
···
 	int rc;
 
 	rc = raw_get_scom(scom, value, addr, &status);
-	if (rc == -ENODEV)
+	if (rc)
 		return rc;
 
 	rc = handle_fsi2pib_status(scom, status);

include/trace/events/fsi.h

···
 	)
 );
 
+TRACE_EVENT(fsi_slave_init,
+	TP_PROTO(const struct fsi_slave *slave),
+	TP_ARGS(slave),
+	TP_STRUCT__entry(
+		__field(int,	master_idx)
+		__field(int,	master_n_links)
+		__field(int,	idx)
+		__field(int,	link)
+		__field(int,	chip_id)
+		__field(__u32,	cfam_id)
+		__field(__u32,	size)
+	),
+	TP_fast_assign(
+		__entry->master_idx = slave->master->idx;
+		__entry->master_n_links = slave->master->n_links;
+		__entry->idx = slave->cdev_idx;
+		__entry->link = slave->link;
+		__entry->chip_id = slave->chip_id;
+		__entry->cfam_id = slave->cfam_id;
+		__entry->size = slave->size;
+	),
+	TP_printk("fsi%d: idx:%d link:%d/%d cid:%d cfam:%08x %08x",
+		__entry->master_idx,
+		__entry->idx,
+		__entry->link,
+		__entry->master_n_links,
+		__entry->chip_id,
+		__entry->cfam_id,
+		__entry->size
+	)
+);
+
+TRACE_EVENT(fsi_slave_invalid_cfam,
+	TP_PROTO(const struct fsi_master *master, int link, uint32_t cfam_id),
+	TP_ARGS(master, link, cfam_id),
+	TP_STRUCT__entry(
+		__field(int,	master_idx)
+		__field(int,	master_n_links)
+		__field(int,	link)
+		__field(__u32,	cfam_id)
+	),
+	TP_fast_assign(
+		__entry->master_idx = master->idx;
+		__entry->master_n_links = master->n_links;
+		__entry->link = link;
+		__entry->cfam_id = cfam_id;
+	),
+	TP_printk("fsi%d: cfam:%08x link:%d/%d",
+		__entry->master_idx,
+		__entry->cfam_id,
+		__entry->link,
+		__entry->master_n_links
+	)
+);
+
+TRACE_EVENT(fsi_dev_init,
+	TP_PROTO(const struct fsi_device *dev),
+	TP_ARGS(dev),
+	TP_STRUCT__entry(
+		__field(int,	master_idx)
+		__field(int,	link)
+		__field(int,	type)
+		__field(int,	unit)
+		__field(int,	version)
+		__field(__u32,	addr)
+		__field(__u32,	size)
+	),
+	TP_fast_assign(
+		__entry->master_idx = dev->slave->master->idx;
+		__entry->link = dev->slave->link;
+		__entry->type = dev->engine_type;
+		__entry->unit = dev->unit;
+		__entry->version = dev->version;
+		__entry->addr = dev->addr;
+		__entry->size = dev->size;
+	),
+	TP_printk("fsi%d: slv%d: t:%02x u:%02x v:%02x %08x@%08x",
+		__entry->master_idx,
+		__entry->link,
+		__entry->type,
+		__entry->unit,
+		__entry->version,
+		__entry->size,
+		__entry->addr
+	)
+);
 
 #endif /* _TRACE_FSI_H */
 

include/trace/events/fsi_master_aspeed.h

···
 		)
 	);
 
+TRACE_EVENT(fsi_master_aspeed_cfam_reset,
+	TP_PROTO(bool start),
+	TP_ARGS(start),
+	TP_STRUCT__entry(
+		__field(bool,	start)
+	),
+	TP_fast_assign(
+		__entry->start = start;
+	),
+	TP_printk("%s", __entry->start ? "start" : "end")
+);
+
 #endif
 
 #include <trace/define_trace.h>

include/uapi/linux/fsi.h

···
 #define FSI_SCOM_WRITE	_IOWR('s', 0x02, struct scom_access)
 #define FSI_SCOM_RESET	_IOW('s', 0x03, __u32)
 
+/*
+ * /dev/sbefifo* ioctl interface
+ */
+
+/**
+ * FSI_SBEFIFO_READ_TIMEOUT sets the read timeout for response from SBE.
+ *
+ * The read timeout is specified in seconds.  The minimum value of read
+ * timeout is 10 seconds (default) and the maximum value of read timeout is
+ * 120 seconds.  A read timeout of 0 will reset the value to the default of
+ * (10 seconds).
+ */
+#define FSI_SBEFIFO_READ_TIMEOUT_SECONDS	_IOW('s', 0x00, __u32)
+
 #endif /* _UAPI_LINUX_FSI_H */