All of lore.kernel.org
 help / color / mirror / Atom feed
From: Cristian Marussi <cristian.marussi@arm.com>
To: linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Cc: sudeep.holla@arm.com, james.quinlan@broadcom.com,
	Jonathan.Cameron@Huawei.com, f.fainelli@gmail.com,
	etienne.carriere@linaro.org, vincent.guittot@linaro.org,
	souvik.chakravarty@arm.com, cristian.marussi@arm.com
Subject: [PATCH v4 02/12] firmware: arm_scmi: Add configurable polling mode for transports
Date: Tue, 24 Aug 2021 14:59:31 +0100	[thread overview]
Message-ID: <20210824135941.38656-3-cristian.marussi@arm.com> (raw)
In-Reply-To: <20210824135941.38656-1-cristian.marussi@arm.com>

SCMI communications along TX channels can optionally be provided of a
completion interrupt; when such interrupt is not available, command
transactions should rely on polling, where the SCMI core takes care to
repeatedly evaluate the transport-specific .poll_done() function to
determine if and when a request was fully completed or timed out.

Such mechanism is already present and working on a single transfer base:
SCMI protocols can indeed enable hdr.poll_completion on specific commands
ahead of each transfer and cause that transaction to be handled with
polling.

Introduce a couple of flags to be able to enforce such polling behaviour
globally at will:

 - scmi_desc.force_polling: to statically switch the whole transport to
   polling mode.

 - scmi_chan_info.no_completion_irq: to switch a single channel dynamically
   to polling mode if at runtime is determined that no completion interrupt
   was available for such channel.

Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
v3 --> v4:
- renamed .needs_polling flag to .no_completion_irq
- refactored error path when polling needed but not supported
---
 drivers/firmware/arm_scmi/common.h | 11 +++++++++++
 drivers/firmware/arm_scmi/driver.c | 29 +++++++++++++++++++++++------
 2 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index dea1bfbe1052..67c761141a48 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -339,11 +339,19 @@ void scmi_protocol_release(const struct scmi_handle *handle, u8 protocol_id);
  * @dev: Reference to device in the SCMI hierarchy corresponding to this
  *	 channel
  * @handle: Pointer to SCMI entity handle
+ * @no_completion_irq: Flag to indicate that this channel has no completion
+ *		       interrupt mechanism, so it needs SCMI core to poll,
+ *		       using .poll_done(), to determine when a command has
+ *		       completed.
+ *		       This can be dynamically set by transports at run-time
+ *		       inside their provided .chan_setup() when they determine
+ *		       no completion interrupt is available.
  * @transport_info: Transport layer related information
  */
 struct scmi_chan_info {
 	struct device *dev;
 	struct scmi_handle *handle;
+	bool no_completion_irq;
 	void *transport_info;
 };
 
@@ -402,6 +410,8 @@ struct scmi_device *scmi_child_dev_find(struct device *parent,
  *	be pending simultaneously in the system. May be overridden by the
  *	get_max_msg op.
  * @max_msg_size: Maximum size of data per message that can be handled.
+ * @force_polling: Flag to force this whole transport to use SCMI core polling
+ *		   mechanism instead of completion interrupts even if available.
  */
 struct scmi_desc {
 	int (*transport_init)(void);
@@ -410,6 +420,7 @@ struct scmi_desc {
 	int max_rx_timeout_ms;
 	int max_msg;
 	int max_msg_size;
+	bool force_polling;
 };
 
 #ifdef CONFIG_ARM_SCMI_TRANSPORT_MAILBOX
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 01c79a8fa77f..a3700f49e8ac 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -656,6 +656,15 @@ static void scmi_handle_response(struct scmi_chan_info *cinfo,
 		return;
 	}
 
+	/* Discard unexpected messages when polling is active. */
+	if (xfer->hdr.type != MSG_TYPE_DELAYED_RESP &&
+	    xfer->hdr.poll_completion) {
+		WARN_ON_ONCE(1);
+		dev_dbg(cinfo->dev,
+			"Completion IRQ received but using polling. Ignore.\n");
+		return;
+	}
+
 	/* rx.len could be shrunk in the sync do_xfer, so reset to maxsz */
 	if (xfer->hdr.type == MSG_TYPE_DELAYED_RESP)
 		xfer->rx.len = info->desc->max_msg_size;
@@ -760,16 +769,19 @@ static int do_xfer(const struct scmi_protocol_handle *ph,
 	struct device *dev = info->dev;
 	struct scmi_chan_info *cinfo;
 
-	if (xfer->hdr.poll_completion && !info->desc->ops->poll_done) {
-		dev_warn_once(dev,
-			      "Polling mode is not supported by transport.\n");
-		return -EINVAL;
-	}
-
 	cinfo = idr_find(&info->tx_idr, pi->proto->id);
 	if (unlikely(!cinfo))
 		return -EINVAL;
 
+	if (info->desc->force_polling || cinfo->no_completion_irq) {
+		if (!info->desc->ops->poll_done) {
+			dev_warn_once(dev,
+				      "Polling mode is not supported by transport.\n");
+			return -EINVAL;
+		}
+		xfer->hdr.poll_completion = true;
+	}
+
 	/*
 	 * Initialise protocol id now from protocol handle to avoid it being
 	 * overridden by mistake (or malice) by the protocol code mangling with
@@ -1499,6 +1511,11 @@ static int scmi_chan_setup(struct scmi_info *info, struct device *dev,
 	if (ret)
 		return ret;
 
+	if (tx && (cinfo->no_completion_irq || info->desc->force_polling))
+		dev_info(dev,
+			 "Enabled polling mode for TX channel - prot_id:%d\n",
+			 prot_id);
+
 idr_alloc:
 	ret = idr_alloc(idr, cinfo, prot_id, prot_id + 1, GFP_KERNEL);
 	if (ret != prot_id) {
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: Cristian Marussi <cristian.marussi@arm.com>
To: linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Cc: sudeep.holla@arm.com, james.quinlan@broadcom.com,
	Jonathan.Cameron@Huawei.com, f.fainelli@gmail.com,
	etienne.carriere@linaro.org, vincent.guittot@linaro.org,
	souvik.chakravarty@arm.com, cristian.marussi@arm.com
Subject: [PATCH v4 02/12] firmware: arm_scmi: Add configurable polling mode for transports
Date: Tue, 24 Aug 2021 14:59:31 +0100	[thread overview]
Message-ID: <20210824135941.38656-3-cristian.marussi@arm.com> (raw)
In-Reply-To: <20210824135941.38656-1-cristian.marussi@arm.com>

SCMI communications along TX channels can optionally be provided of a
completion interrupt; when such interrupt is not available, command
transactions should rely on polling, where the SCMI core takes care to
repeatedly evaluate the transport-specific .poll_done() function to
determine if and when a request was fully completed or timed out.

Such mechanism is already present and working on a single transfer base:
SCMI protocols can indeed enable hdr.poll_completion on specific commands
ahead of each transfer and cause that transaction to be handled with
polling.

Introduce a couple of flags to be able to enforce such polling behaviour
globally at will:

 - scmi_desc.force_polling: to statically switch the whole transport to
   polling mode.

 - scmi_chan_info.no_completion_irq: to switch a single channel dynamically
   to polling mode if at runtime is determined that no completion interrupt
   was available for such channel.

Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
v3 --> v4:
- renamed .needs_polling flag to .no_completion_irq
- refactored error path when polling needed but not supported
---
 drivers/firmware/arm_scmi/common.h | 11 +++++++++++
 drivers/firmware/arm_scmi/driver.c | 29 +++++++++++++++++++++++------
 2 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index dea1bfbe1052..67c761141a48 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -339,11 +339,19 @@ void scmi_protocol_release(const struct scmi_handle *handle, u8 protocol_id);
  * @dev: Reference to device in the SCMI hierarchy corresponding to this
  *	 channel
  * @handle: Pointer to SCMI entity handle
+ * @no_completion_irq: Flag to indicate that this channel has no completion
+ *		       interrupt mechanism, so it needs SCMI core to poll,
+ *		       using .poll_done(), to determine when a command has
+ *		       completed.
+ *		       This can be dynamically set by transports at run-time
+ *		       inside their provided .chan_setup() when they determine
+ *		       no completion interrupt is available.
  * @transport_info: Transport layer related information
  */
 struct scmi_chan_info {
 	struct device *dev;
 	struct scmi_handle *handle;
+	bool no_completion_irq;
 	void *transport_info;
 };
 
@@ -402,6 +410,8 @@ struct scmi_device *scmi_child_dev_find(struct device *parent,
  *	be pending simultaneously in the system. May be overridden by the
  *	get_max_msg op.
  * @max_msg_size: Maximum size of data per message that can be handled.
+ * @force_polling: Flag to force this whole transport to use SCMI core polling
+ *		   mechanism instead of completion interrupts even if available.
  */
 struct scmi_desc {
 	int (*transport_init)(void);
@@ -410,6 +420,7 @@ struct scmi_desc {
 	int max_rx_timeout_ms;
 	int max_msg;
 	int max_msg_size;
+	bool force_polling;
 };
 
 #ifdef CONFIG_ARM_SCMI_TRANSPORT_MAILBOX
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 01c79a8fa77f..a3700f49e8ac 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -656,6 +656,15 @@ static void scmi_handle_response(struct scmi_chan_info *cinfo,
 		return;
 	}
 
+	/* Discard unexpected messages when polling is active. */
+	if (xfer->hdr.type != MSG_TYPE_DELAYED_RESP &&
+	    xfer->hdr.poll_completion) {
+		WARN_ON_ONCE(1);
+		dev_dbg(cinfo->dev,
+			"Completion IRQ received but using polling. Ignore.\n");
+		return;
+	}
+
 	/* rx.len could be shrunk in the sync do_xfer, so reset to maxsz */
 	if (xfer->hdr.type == MSG_TYPE_DELAYED_RESP)
 		xfer->rx.len = info->desc->max_msg_size;
@@ -760,16 +769,19 @@ static int do_xfer(const struct scmi_protocol_handle *ph,
 	struct device *dev = info->dev;
 	struct scmi_chan_info *cinfo;
 
-	if (xfer->hdr.poll_completion && !info->desc->ops->poll_done) {
-		dev_warn_once(dev,
-			      "Polling mode is not supported by transport.\n");
-		return -EINVAL;
-	}
-
 	cinfo = idr_find(&info->tx_idr, pi->proto->id);
 	if (unlikely(!cinfo))
 		return -EINVAL;
 
+	if (info->desc->force_polling || cinfo->no_completion_irq) {
+		if (!info->desc->ops->poll_done) {
+			dev_warn_once(dev,
+				      "Polling mode is not supported by transport.\n");
+			return -EINVAL;
+		}
+		xfer->hdr.poll_completion = true;
+	}
+
 	/*
 	 * Initialise protocol id now from protocol handle to avoid it being
 	 * overridden by mistake (or malice) by the protocol code mangling with
@@ -1499,6 +1511,11 @@ static int scmi_chan_setup(struct scmi_info *info, struct device *dev,
 	if (ret)
 		return ret;
 
+	if (tx && (cinfo->no_completion_irq || info->desc->force_polling))
+		dev_info(dev,
+			 "Enabled polling mode for TX channel - prot_id:%d\n",
+			 prot_id);
+
 idr_alloc:
 	ret = idr_alloc(idr, cinfo, prot_id, prot_id + 1, GFP_KERNEL);
 	if (ret != prot_id) {
-- 
2.17.1


  parent reply	other threads:[~2021-08-24 14:03 UTC|newest]

Thread overview: 62+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-24 13:59 [PATCH v4 0/12] Introduce atomic support for SCMI transports Cristian Marussi
2021-08-24 13:59 ` Cristian Marussi
2021-08-24 13:59 ` [PATCH v4 01/12] firmware: arm_scmi: Perform earlier cinfo lookup call in do_xfer Cristian Marussi
2021-08-24 13:59   ` Cristian Marussi
2021-08-25 16:28   ` Florian Fainelli
2021-08-25 16:28     ` Florian Fainelli
2021-08-24 13:59 ` Cristian Marussi [this message]
2021-08-24 13:59   ` [PATCH v4 02/12] firmware: arm_scmi: Add configurable polling mode for transports Cristian Marussi
2021-08-25 16:29   ` Florian Fainelli
2021-08-25 16:29     ` Florian Fainelli
2021-08-24 13:59 ` [PATCH v4 03/12] firmware: arm_scmi: Add support for atomic transports Cristian Marussi
2021-08-24 13:59   ` Cristian Marussi
2021-08-25 16:18   ` Jim Quinlan
2021-08-25 16:18     ` Jim Quinlan
2021-08-25 17:50     ` Cristian Marussi
2021-08-25 17:50       ` Cristian Marussi
2021-08-24 13:59 ` [PATCH v4 04/12] include: trace: Add new scmi_xfer_response_wait event Cristian Marussi
2021-08-24 13:59   ` Cristian Marussi
2021-08-25 16:30   ` Florian Fainelli
2021-08-25 16:30     ` Florian Fainelli
2021-08-24 13:59 ` [PATCH v4 05/12] firmware: arm_scmi: Use new trace event scmi_xfer_response_wait Cristian Marussi
2021-08-24 13:59   ` Cristian Marussi
2021-08-25 16:31   ` Florian Fainelli
2021-08-25 16:31     ` Florian Fainelli
2021-08-25 17:52     ` Cristian Marussi
2021-08-25 17:52       ` Cristian Marussi
2021-08-24 13:59 ` [PATCH v4 06/12] firmware: arm_scmi: Add is_transport_atomic() handle method Cristian Marussi
2021-08-24 13:59   ` Cristian Marussi
2021-08-25 16:32   ` Florian Fainelli
2021-08-25 16:32     ` Florian Fainelli
2021-08-24 13:59 ` [PATCH v4 07/12] clk: scmi: Support atomic enable/disable API Cristian Marussi
2021-08-24 13:59   ` Cristian Marussi
2021-08-25 16:33   ` Florian Fainelli
2021-08-25 16:33     ` Florian Fainelli
2021-08-24 13:59 ` [PATCH v4 08/12] firmware: arm_scmi: Declare virtio transport .atomic_capable Cristian Marussi
2021-08-24 13:59   ` Cristian Marussi
2021-08-24 13:59 ` [PATCH v4 09/12] [RFC] firmware: arm_scmi: Make smc transport use common completions Cristian Marussi
2021-08-24 13:59   ` Cristian Marussi
2021-08-25 16:35   ` Florian Fainelli
2021-08-25 16:35     ` Florian Fainelli
2021-08-24 13:59 ` [PATCH v4 10/12] [RFC] firmware: arm_scmi: Make smc transport atomic Cristian Marussi
2021-08-24 13:59   ` Cristian Marussi
2021-08-24 13:59 ` [PATCH v4 11/12] [RFC] firmware: arm_scmi: Add sync_cmds_atomic_replies transport flag Cristian Marussi
2021-08-24 13:59   ` Cristian Marussi
2021-08-25 16:38   ` Florian Fainelli
2021-08-25 16:38     ` Florian Fainelli
2021-08-25 17:17     ` Jim Quinlan
2021-08-25 17:17       ` Jim Quinlan
2021-08-25 18:49       ` Cristian Marussi
2021-08-25 18:49         ` Cristian Marussi
2021-08-26 18:29         ` Jim Quinlan
2021-08-26 18:29           ` Jim Quinlan
2021-08-31  5:56           ` Cristian Marussi
2021-08-31  5:56             ` Cristian Marussi
2021-09-23 15:03           ` Cristian Marussi
2021-09-23 15:03             ` Cristian Marussi
2021-10-04 17:50             ` Jim Quinlan
2021-10-04 17:50               ` Jim Quinlan
2021-10-04 18:00               ` Cristian Marussi
2021-10-04 18:00                 ` Cristian Marussi
2021-08-24 13:59 ` [PATCH v4 12/12] [RFC] firmware: arm_scmi: Make smc support atomic commands replies Cristian Marussi
2021-08-24 13:59   ` Cristian Marussi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210824135941.38656-3-cristian.marussi@arm.com \
    --to=cristian.marussi@arm.com \
    --cc=Jonathan.Cameron@Huawei.com \
    --cc=etienne.carriere@linaro.org \
    --cc=f.fainelli@gmail.com \
    --cc=james.quinlan@broadcom.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=souvik.chakravarty@arm.com \
    --cc=sudeep.holla@arm.com \
    --cc=vincent.guittot@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.