lkml.org 
[lkml]   [2022]   [Dec]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v3 2/2] mailbox: mpfs: read the system controller's status
From
On Wed, 23 Nov 2022 09:56:52 PST (-0800), Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
>
> Some services explicitly return an error code in their response, but
> others rely on the system controller to set a status in its status
> register. The meaning of the bits varies based on what service is
> requested, so pass it back up to the driver that requested the service
> in the first place. The field in the message struct already existed, but
> was unused until now.
>
> If the system controller is busy, in which case we should never actually
> be in the interrupt handler, or if the service fails the mailbox itself
> should not be read. Callers should check the status before operating on
> the response.
>
> There's an existing, but unused, #define for the mailbox mask - but it
> was incorrect. It was doing a GENMASK_ULL(32, 16) which should've just
> been a GENMASK(31, 16), so fix that up and start using it.
>
> Fixes: 83d7b1560810 ("mbox: add polarfire soc system controller mailbox")
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> drivers/mailbox/mailbox-mpfs.c | 31 ++++++++++++++++++++++++++++---
> 1 file changed, 28 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mailbox/mailbox-mpfs.c b/drivers/mailbox/mailbox-mpfs.c
> index cfacb3f320a6..853901acaeec 100644
> --- a/drivers/mailbox/mailbox-mpfs.c
> +++ b/drivers/mailbox/mailbox-mpfs.c
> @@ -2,7 +2,7 @@
> /*
> * Microchip PolarFire SoC (MPFS) system controller/mailbox controller driver
> *
> - * Copyright (c) 2020 Microchip Corporation. All rights reserved.
> + * Copyright (c) 2020-2022 Microchip Corporation. All rights reserved.
> *
> * Author: Conor Dooley <conor.dooley@microchip.com>
> *
> @@ -56,7 +56,7 @@
> #define SCB_STATUS_NOTIFY_MASK BIT(SCB_STATUS_NOTIFY)
>
> #define SCB_STATUS_POS (16)
> -#define SCB_STATUS_MASK GENMASK_ULL(SCB_STATUS_POS + SCB_MASK_WIDTH, SCB_STATUS_POS)
> +#define SCB_STATUS_MASK GENMASK(SCB_STATUS_POS + SCB_MASK_WIDTH - 1, SCB_STATUS_POS)
>
> struct mpfs_mbox {
> struct mbox_controller controller;
> @@ -130,13 +130,38 @@ static void mpfs_mbox_rx_data(struct mbox_chan *chan)
> struct mpfs_mbox *mbox = (struct mpfs_mbox *)chan->con_priv;
> struct mpfs_mss_response *response = mbox->response;
> u16 num_words = ALIGN((response->resp_size), (4)) / 4U;
> - u32 i;
> + u32 i, status;
>
> if (!response->resp_msg) {
> dev_err(mbox->dev, "failed to assign memory for response %d\n", -ENOMEM);
> return;
> }
>
> + /*
> + * The status is stored in bits 31:16 of the SERVICES_SR register.
> + * It is only valid when BUSY == 0.
> + * We should *never* get an interrupt while the controller is
> + * still in the busy state. If we do, something has gone badly
> + * wrong & the content of the mailbox would not be valid.
> + */
> + if (mpfs_mbox_busy(mbox)) {
> + dev_err(mbox->dev, "got an interrupt but system controller is busy\n");
> + response->resp_status = 0xDEAD;
> + return;
> + }
> +
> + status = readl_relaxed(mbox->ctrl_base + SERVICES_SR_OFFSET);
> +
> + /*
> + * If the status of the individual servers is non-zero, the service has
> + * failed. The contents of the mailbox at this point are not be valid,
> + * so don't bother reading them. Set the status so that the driver
> + * implementing the service can handle the result.
> + */
> + response->resp_status = (status & SCB_STATUS_MASK) >> SCB_STATUS_POS;
> + if (response->resp_status)
> + return;
> +
> if (!mpfs_mbox_busy(mbox)) {
> for (i = 0; i < num_words; i++) {
> response->resp_msg[i] =

Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com>

I'm assuming this is aimed at some other tree.

\
 
 \ /
  Last update: 2022-12-09 20:15    [W:0.164 / U:0.976 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site