* [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).