linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] SCMI various fixes to Response handling code
@ 2020-04-20 15:23 Cristian Marussi
  2020-04-20 15:23 ` [PATCH 1/4] firmware: arm_scmi: Rename .clear_notification() transport_ops Cristian Marussi
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Cristian Marussi @ 2020-04-20 15:23 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel; +Cc: sudeep.holla, cristian.marussi

Hi

This serie bring a few fixes related to handling of responses in some
corner cases; renaming also .clear_notification() into clear_channel(),
being indeed a method of general utility not strictly related to
notifications. (and needed by these same fixes)

Based on scmi-next 5.7 [1], on top of:

commit a2fe63248225 ("firmware: arm_scmi: Fix return error code in
							smc_send_message")

[1] git://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux.git


Cristian Marussi (4):
  firmware: arm_scmi: Rename .clear_notification() transport_ops
  firmware: arm_scmi: Clear channel on reception of unexpected responses
  firmware: arm_scmi: Clear channel for delayed responses
  firmware: arm_scmi: Fix handling of unexpected delayed responses

 drivers/firmware/arm_scmi/common.h  |  6 +++---
 drivers/firmware/arm_scmi/driver.c  | 26 ++++++++++++++++++++++----
 drivers/firmware/arm_scmi/mailbox.c |  6 +++---
 drivers/firmware/arm_scmi/shmem.c   |  2 +-
 4 files changed, 29 insertions(+), 11 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/4] firmware: arm_scmi: Rename .clear_notification() transport_ops
  2020-04-20 15:23 [PATCH 0/4] SCMI various fixes to Response handling code Cristian Marussi
@ 2020-04-20 15:23 ` Cristian Marussi
  2020-04-20 15:23 ` [PATCH 2/4] firmware: arm_scmi: Clear channel on reception of unexpected responses Cristian Marussi
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Cristian Marussi @ 2020-04-20 15:23 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel; +Cc: sudeep.holla, cristian.marussi

SCMI transport operation .clear_notification() is indeed a generic method
to clear the channel in a transport dependent way: as such it could be a
useful helper also in other contexts.

Rename such method as .clear_channel(), renaming accordingly also its
already existent call-sites.

No functional change.

Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
 drivers/firmware/arm_scmi/common.h  | 6 +++---
 drivers/firmware/arm_scmi/driver.c  | 4 ++--
 drivers/firmware/arm_scmi/mailbox.c | 6 +++---
 drivers/firmware/arm_scmi/shmem.c   | 2 +-
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index 07eb33c1576b..31fe5a22a011 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -179,7 +179,7 @@ struct scmi_chan_info {
  * @mark_txdone: Callback to mark tx as done
  * @fetch_response: Callback to fetch response
  * @fetch_notification: Callback to fetch notification
- * @clear_notification: Callback to clear a pending notification
+ * @clear_channel: Callback to clear a channel
  * @poll_done: Callback to poll transfer status
  */
 struct scmi_transport_ops {
@@ -194,7 +194,7 @@ struct scmi_transport_ops {
 			       struct scmi_xfer *xfer);
 	void (*fetch_notification)(struct scmi_chan_info *cinfo,
 				   size_t max_len, struct scmi_xfer *xfer);
-	void (*clear_notification)(struct scmi_chan_info *cinfo);
+	void (*clear_channel)(struct scmi_chan_info *cinfo);
 	bool (*poll_done)(struct scmi_chan_info *cinfo, struct scmi_xfer *xfer);
 };
 
@@ -232,6 +232,6 @@ void shmem_fetch_response(struct scmi_shared_mem __iomem *shmem,
 			  struct scmi_xfer *xfer);
 void shmem_fetch_notification(struct scmi_shared_mem __iomem *shmem,
 			      size_t max_len, struct scmi_xfer *xfer);
-void shmem_clear_notification(struct scmi_shared_mem __iomem *shmem);
+void shmem_clear_channel(struct scmi_shared_mem __iomem *shmem);
 bool shmem_poll_done(struct scmi_shared_mem __iomem *shmem,
 		     struct scmi_xfer *xfer);
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 332edf1b09fc..90c7a0bb62ef 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -213,7 +213,7 @@ static void scmi_handle_notification(struct scmi_chan_info *cinfo, u32 msg_hdr)
 	if (IS_ERR(xfer)) {
 		dev_err(dev, "failed to get free message slot (%ld)\n",
 			PTR_ERR(xfer));
-		info->desc->ops->clear_notification(cinfo);
+		info->desc->ops->clear_channel(cinfo);
 		return;
 	}
 
@@ -228,7 +228,7 @@ static void scmi_handle_notification(struct scmi_chan_info *cinfo, u32 msg_hdr)
 
 	__scmi_xfer_put(minfo, xfer);
 
-	info->desc->ops->clear_notification(cinfo);
+	info->desc->ops->clear_channel(cinfo);
 }
 
 static void scmi_handle_response(struct scmi_chan_info *cinfo,
diff --git a/drivers/firmware/arm_scmi/mailbox.c b/drivers/firmware/arm_scmi/mailbox.c
index 19ee058f9f44..6998dc86b5ce 100644
--- a/drivers/firmware/arm_scmi/mailbox.c
+++ b/drivers/firmware/arm_scmi/mailbox.c
@@ -166,11 +166,11 @@ static void mailbox_fetch_notification(struct scmi_chan_info *cinfo,
 	shmem_fetch_notification(smbox->shmem, max_len, xfer);
 }
 
-static void mailbox_clear_notification(struct scmi_chan_info *cinfo)
+static void mailbox_clear_channel(struct scmi_chan_info *cinfo)
 {
 	struct scmi_mailbox *smbox = cinfo->transport_info;
 
-	shmem_clear_notification(smbox->shmem);
+	shmem_clear_channel(smbox->shmem);
 }
 
 static bool
@@ -189,7 +189,7 @@ static struct scmi_transport_ops scmi_mailbox_ops = {
 	.mark_txdone = mailbox_mark_txdone,
 	.fetch_response = mailbox_fetch_response,
 	.fetch_notification = mailbox_fetch_notification,
-	.clear_notification = mailbox_clear_notification,
+	.clear_channel = mailbox_clear_channel,
 	.poll_done = mailbox_poll_done,
 };
 
diff --git a/drivers/firmware/arm_scmi/shmem.c b/drivers/firmware/arm_scmi/shmem.c
index a5a5d0f6bf86..0e3eaea5d852 100644
--- a/drivers/firmware/arm_scmi/shmem.c
+++ b/drivers/firmware/arm_scmi/shmem.c
@@ -77,7 +77,7 @@ void shmem_fetch_notification(struct scmi_shared_mem __iomem *shmem,
 	memcpy_fromio(xfer->rx.buf, shmem->msg_payload, xfer->rx.len);
 }
 
-void shmem_clear_notification(struct scmi_shared_mem __iomem *shmem)
+void shmem_clear_channel(struct scmi_shared_mem __iomem *shmem)
 {
 	iowrite32(SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE, &shmem->channel_status);
 }
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/4] firmware: arm_scmi: Clear channel on reception of unexpected responses
  2020-04-20 15:23 [PATCH 0/4] SCMI various fixes to Response handling code Cristian Marussi
  2020-04-20 15:23 ` [PATCH 1/4] firmware: arm_scmi: Rename .clear_notification() transport_ops Cristian Marussi
@ 2020-04-20 15:23 ` Cristian Marussi
  2020-04-20 15:23 ` [PATCH 3/4] firmware: arm_scmi: Clear channel for delayed responses Cristian Marussi
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Cristian Marussi @ 2020-04-20 15:23 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel; +Cc: sudeep.holla, cristian.marussi

When an unexpected response message is received we currently warn the user
and bail-out: ensure to also free the channel by invoking the transport
independent operation .clear_channel()

Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
 drivers/firmware/arm_scmi/driver.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 90c7a0bb62ef..31c6a89a6edd 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -242,6 +242,7 @@ static void scmi_handle_response(struct scmi_chan_info *cinfo,
 	/* Are we even expecting this? */
 	if (!test_bit(xfer_id, minfo->xfer_alloc_table)) {
 		dev_err(dev, "message for %d is not expected!\n", xfer_id);
+		info->desc->ops->clear_channel(cinfo);
 		return;
 	}
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/4] firmware: arm_scmi: Clear channel for delayed responses
  2020-04-20 15:23 [PATCH 0/4] SCMI various fixes to Response handling code Cristian Marussi
  2020-04-20 15:23 ` [PATCH 1/4] firmware: arm_scmi: Rename .clear_notification() transport_ops Cristian Marussi
  2020-04-20 15:23 ` [PATCH 2/4] firmware: arm_scmi: Clear channel on reception of unexpected responses Cristian Marussi
@ 2020-04-20 15:23 ` Cristian Marussi
  2020-04-20 15:23 ` [PATCH 4/4] firmware: arm_scmi: Fix handling of unexpected " Cristian Marussi
  2020-04-20 16:46 ` [PATCH 0/4] SCMI various fixes to Response handling code Sudeep Holla
  4 siblings, 0 replies; 6+ messages in thread
From: Cristian Marussi @ 2020-04-20 15:23 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel; +Cc: sudeep.holla, cristian.marussi

Clear channel properly when done processing a delayed response.

Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
 drivers/firmware/arm_scmi/driver.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 31c6a89a6edd..07de196f15aa 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -256,10 +256,12 @@ static void scmi_handle_response(struct scmi_chan_info *cinfo,
 			   xfer->hdr.protocol_id, xfer->hdr.seq,
 			   msg_type);
 
-	if (msg_type == MSG_TYPE_DELAYED_RESP)
+	if (msg_type == MSG_TYPE_DELAYED_RESP) {
+		info->desc->ops->clear_channel(cinfo);
 		complete(xfer->async_done);
-	else
+	} else {
 		complete(&xfer->done);
+	}
 }
 
 /**
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 4/4] firmware: arm_scmi: Fix handling of unexpected delayed responses
  2020-04-20 15:23 [PATCH 0/4] SCMI various fixes to Response handling code Cristian Marussi
                   ` (2 preceding siblings ...)
  2020-04-20 15:23 ` [PATCH 3/4] firmware: arm_scmi: Clear channel for delayed responses Cristian Marussi
@ 2020-04-20 15:23 ` Cristian Marussi
  2020-04-20 16:46 ` [PATCH 0/4] SCMI various fixes to Response handling code Sudeep Holla
  4 siblings, 0 replies; 6+ messages in thread
From: Cristian Marussi @ 2020-04-20 15:23 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel; +Cc: sudeep.holla, cristian.marussi

Upon reception of an unexpected bogus delayed response, clear the channel
and bail-out safely.

Fixes: 4d09852b6f01 ("firmware: arm_scmi: Add support for notifications message processing")
Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
---
 drivers/firmware/arm_scmi/driver.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 07de196f15aa..0146332d06a1 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -247,6 +247,21 @@ static void scmi_handle_response(struct scmi_chan_info *cinfo,
 	}
 
 	xfer = &minfo->xfer_block[xfer_id];
+	/*
+	 * Even if a response was indeed expected on this slot at this point,
+	 * a buggy platform could wrongly reply feeding us an unexpected
+	 * delayed response we're not prepared to handle: bail-out safely
+	 * blaming fw guys.
+	 */
+	if (unlikely(msg_type == MSG_TYPE_DELAYED_RESP && !xfer->async_done)) {
+		dev_err(dev,
+			"Delayed Response for %d not expected! Buggy FW ?\n",
+			xfer_id);
+		info->desc->ops->clear_channel(cinfo);
+		/* It was unexpected, so nobody will clear the xfer if not us */
+		__scmi_xfer_put(minfo, xfer);
+		return;
+	}
 
 	scmi_dump_header_dbg(dev, &xfer->hdr);
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/4] SCMI various fixes to Response handling code
  2020-04-20 15:23 [PATCH 0/4] SCMI various fixes to Response handling code Cristian Marussi
                   ` (3 preceding siblings ...)
  2020-04-20 15:23 ` [PATCH 4/4] firmware: arm_scmi: Fix handling of unexpected " Cristian Marussi
@ 2020-04-20 16:46 ` Sudeep Holla
  4 siblings, 0 replies; 6+ messages in thread
From: Sudeep Holla @ 2020-04-20 16:46 UTC (permalink / raw)
  To: Cristian Marussi; +Cc: Sudeep Holla, linux-kernel, linux-arm-kernel

On Mon, 20 Apr 2020 16:23:11 +0100, Cristian Marussi wrote:
> Hi
>
> This serie bring a few fixes related to handling of responses in some
> corner cases; renaming also .clear_notification() into clear_channel(),
> being indeed a method of general utility not strictly related to
> notifications. (and needed by these same fixes)
>
> [...]

Applied, thanks!

[1/4] firmware: arm_scmi: Rename .clear_notification() transport_ops
      commit: 87dff4e63cf2910f2e4a32d1cb3e4a1a25406eb7
[2/4] firmware: arm_scmi: Clear channel on reception of unexpected responses
      commit: b37f5cc8d243479d7572445010fb6c9a4dff6dc4
[3/4] firmware: arm_scmi: Clear channel for delayed responses
      commit: d04fb2b2ddefad7c00edd29c1ed40188ce8f12a2
[4/4] firmware: arm_scmi: Fix handling of unexpected delayed responses
      commit: c5bceb98ce0e4ae8057a386c5171a868213fe226

--
Regards,
Sudeep


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-04-20 16:46 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-20 15:23 [PATCH 0/4] SCMI various fixes to Response handling code Cristian Marussi
2020-04-20 15:23 ` [PATCH 1/4] firmware: arm_scmi: Rename .clear_notification() transport_ops Cristian Marussi
2020-04-20 15:23 ` [PATCH 2/4] firmware: arm_scmi: Clear channel on reception of unexpected responses Cristian Marussi
2020-04-20 15:23 ` [PATCH 3/4] firmware: arm_scmi: Clear channel for delayed responses Cristian Marussi
2020-04-20 15:23 ` [PATCH 4/4] firmware: arm_scmi: Fix handling of unexpected " Cristian Marussi
2020-04-20 16:46 ` [PATCH 0/4] SCMI various fixes to Response handling code Sudeep Holla

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).