From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 03F23C32793 for ; Wed, 18 Jan 2023 13:54:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:Content-Type: List-Subscribe:List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id: In-Reply-To:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date: Reply-To:Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date :Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=46uJaCykpPq8wz5mUOnZnOQsVVxBb1jryTzHNK5kfsI=; b=p3S81LrF565+8LJ9C+8arsCRZ5 PvJLfvT52+uRruPieV3UtIWU5u/XWuM0sSYzarBM834xaxkDUEZcSTUGvXhaim+2g9ypSn2myP4Dz mJXgAen7ldkIuKDcbw2Y//TWrsrHurfqlFTZXh7WPLuoa6LXW546H9gaklDkhnJytOTl3DXH8n7wV D+/RHREwNrONpEb1q7MjM1QpC6qO8ZqE6v9m5+ynnuu4JI9lGSiC5fIndn8M5KqugB4Wpxh7XgUV1 QSButkaKQlf/oM/5/6A1rhPuZKHxQWcELhj8EM2uSDM17W87CVxUcyO71lmGggWZyo0JSJtidsyx4 4K63JMxQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1pI8t4-001Dlx-RD; Wed, 18 Jan 2023 13:53:58 +0000 Received: from dfw.source.kernel.org ([2604:1380:4641:c500::1]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1pI8t1-001DlL-6K for linux-riscv@lists.infradead.org; Wed, 18 Jan 2023 13:53:56 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id AD672617F3; Wed, 18 Jan 2023 13:53:54 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id E6B1CC433EF; Wed, 18 Jan 2023 13:53:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1674050034; bh=w5r9sYL2RG2NQJE13OljQZOKM5j8E2WL4wRHuMh3tTk=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=DoZa7S5Bv0ocCl/sR+1FXm3srL0KZoiUzA6xDqSnJY1xn3gB0EBIC9xbJBfzPZiou XOsLjDM+alD7jblgSJo2QDzfzdqn0z4yzHE9PyiBSnXZ5AJQn4oqH7IQyaA5TGb2G3 gE/1/9byCcSOGQrXWyQqDlVgygR1wiDevmDoc/+sI0FKnk8j9Lg6jyXnVEUv6MAeX0 aQoDFannscnQMUkEeUL4V3taDKVOHEjKrWk+zh8ckmuB1z+W5Vb+9qw08+FeAbvCdw Q+7ADmLOh2rXDg5QfSxmkC9aQKq7C1SB3FYJMOQMVGyKChM+feNfZSXAFdAQfz/vXD Z6L//5QAxTang== Date: Wed, 18 Jan 2023 13:53:50 +0000 From: Conor Dooley To: Conor Dooley Cc: Jassi Brar , Daire McNamara , linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v1 0/7] MPFS system controller/mailbox fixes Message-ID: References: <20230111134513.2495510-1-conor.dooley@microchip.com> MIME-Version: 1.0 In-Reply-To: <20230111134513.2495510-1-conor.dooley@microchip.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230118_055355_338943_A9421BD8 X-CRM114-Status: GOOD ( 46.67 ) X-BeenThere: linux-riscv@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: multipart/mixed; boundary="===============3402176557196212070==" Sender: "linux-riscv" Errors-To: linux-riscv-bounces+linux-riscv=archiver.kernel.org@lists.infradead.org --===============3402176557196212070== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="+7I49tIPzidPH2wa" Content-Disposition: inline --+7I49tIPzidPH2wa Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Jan 11, 2023 at 01:45:06PM +0000, Conor Dooley wrote: > Hey Jassi, all, >=20 > Here are some fixes for the system controller on PolarFire SoC that I > ran into while implementing support for using the system controller to > re-program the FPGA. A few are just minor bits that I fixed in passing, > but the bulk of the patchset is changes to how the mailbox figures out > if a "service" has completed. >=20 > Prior to implementing this particular functionality, the services > requested from the system controller, via its mailbox interface, always > triggered an interrupt when the system controller was finished with > the service. >=20 > Unfortunately some of the services used to validate the FPGA images > before programming them do not trigger an interrupt if they fail. > For example, the service that checks whether an FPGA image is actually > a newer version than what is already programmed, does not trigger an > interrupt, unless the image is actually newer than the one currently > programmed. If it has an earlier version, no interrupt is triggered > and a status is set in the system controller's status register to > signify the reason for the failure. I think, with how things currently are, the timeout is insufficient. I've noticed some services taking longer (significantly) longer than what I have provisioned for. I'll try to find an upper bound and respin a v2. Questions below are still valid either way! Thanks, Conor. > In order to differentiate between the service succeeding & the system > controller being inoperative or otherwise unable to function, I had to > switch the controller to poll a busy bit in the system controller's > registers to see if it has completed a service. > This makes sense anyway, as the interrupt corresponds to "data ready" > rather than "tx done", so I have changed the mailbox controller driver > to do that & left the interrupt solely for signalling data ready. > It just so happened that all of the services that I had worked with and > tested up to this point were "infallible" & did not set a status, so the > particular code paths were never tested. >=20 > Jassi, the mailbox and soc patches depend on each other, as the change > in what the interrupt is used for requires changing the client driver's > behaviour too, as mbox_send_message() will now return when the system > controller is no longer busy rather than when the data is ready. > I'm happy to send the lot via the soc tree with your Ack and/or reivew, > if that also works you? >=20 > Secondly, I have a question about what to do if a service does fail, but > not due to a timeout - eg the above example where the "new" image for > the FPGA is actually older than the one that currently exists. > Ideally, if a service fails due to something other than the transaction > timing out, I would go and read the status registers to see what the > cause of failure was. > I could not find a function in the mailbox framework that allows the > client to request that sort of information from the client. Trying to > do something with the auxiliary bus, or exporting some function to a > device specific header seemed like a circumvention of the mailbox > framework. > Do you think it would be a good idea to implement something like > mbox_client_peek_status(struct mbox_chan *chan, void *data) to allow > clients to request this type of information? >=20 > It'd certainly allow me to report the actual errors to the drivers > implementing the service & make better decisions there about how to > proceed. > Perhaps I have missed some way of doing this kind of thing that (should > have been) staring me in the face! >=20 > Thanks, > Conor. >=20 > CC: Conor Dooley > CC: Daire McNamara > CC: Jassi Brar > CC: linux-riscv@lists.infradead.org > CC: linux-kernel@vger.kernel.org >=20 > Conor Dooley (7): > mailbox: mpfs: fix an incorrect mask width > mailbox: mpfs: switch to txdone_poll > mailbox: mpfs: ditch a useless busy check > soc: microchip: mpfs: fix some horrible alignment > soc: microchip: mpfs: use a consistent completion timeout > soc: microchip: mpfs: simplify error handling in > mpfs_blocking_transaction() > soc: microchip: mpfs: handle timeouts and failed services differently >=20 > drivers/mailbox/mailbox-mpfs.c | 25 +++++++---- > drivers/soc/microchip/mpfs-sys-controller.c | 48 +++++++++++++-------- > 2 files changed, 47 insertions(+), 26 deletions(-) >=20 >=20 > base-commit: 88603b6dc419445847923fcb7fe5080067a30f98 > --=20 > 2.39.0 >=20 --+7I49tIPzidPH2wa Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEABYIAB0WIQRh246EGq/8RLhDjO14tDGHoIJi0gUCY8f5zQAKCRB4tDGHoIJi 0uQ1AP9fSIWdrEuF5cYVtH2JW2+2CBmFvbK0cmJCHDzavG/SpgD/fsXfHn5TSM+p JJGfXx5N8gomdWbZkP0hBAwHcacvvAw= =liwu -----END PGP SIGNATURE----- --+7I49tIPzidPH2wa-- --===============3402176557196212070== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv --===============3402176557196212070==--