[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <80a257e8-65fb-4545-a00d-9cb793d1b120@broadcom.com>
Date: Sat, 10 Aug 2024 19:42:59 -0700
From: Florian Fainelli <florian.fainelli@...adcom.com>
To: linux-arm-kernel@...ts.infradead.org
Cc: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Sudeep Holla <sudeep.holla@....com>,
Cristian Marussi <cristian.marussi@....com>,
"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
<devicetree@...r.kernel.org>, open list <linux-kernel@...r.kernel.org>,
arm-scmi@...r.kernel.org, james.quinlan@...adcom.com,
justin.chen@...adcom.com, kapil.hali@...adcom.com,
bcm-kernel-feedback-list@...adcom.com
Subject: Re: [PATCH 2/2] firmware: arm_scmi: Support 'reg-io-width' property
for shared memory
On 8/10/2024 2:46 PM, Florian Fainelli wrote:
> Some shared memory areas might only support a certain access width,
> (e.g.: 32 bits accesses only). Update the shmem layer to support
> reading from and writing to such shared memory area using the specified
> I/O width in the Device Tree. The various transport layers making use of
> the shmem.c code are updated accordingly to pass the I/O width to the
> routines that need it.
>
> Signed-off-by: Florian Fainelli <florian.fainelli@...adcom.com>
Doing a self review, though I will only resend after collecting
additional feedback.
> ---
> drivers/firmware/arm_scmi/common.h | 14 +++-
> .../arm_scmi/scmi_transport_mailbox.c | 12 ++-
> .../firmware/arm_scmi/scmi_transport_optee.c | 7 +-
> .../firmware/arm_scmi/scmi_transport_smc.c | 10 ++-
> drivers/firmware/arm_scmi/shmem.c | 77 ++++++++++++++++---
> 5 files changed, 96 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
> index 69928bbd01c2..97dae844a190 100644
> --- a/drivers/firmware/arm_scmi/common.h
> +++ b/drivers/firmware/arm_scmi/common.h
> @@ -170,6 +170,7 @@ void scmi_protocol_release(const struct scmi_handle *handle, u8 protocol_id);
> * This can be dynamically set by transports at run-time
> * inside their provided .chan_setup().
> * @transport_info: Transport layer related information
> + * @shmem_io_width: I/O width in bytes of the shared memory area
> */
> struct scmi_chan_info {
> int id;
> @@ -178,6 +179,7 @@ struct scmi_chan_info {
> struct scmi_handle *handle;
> bool no_completion_irq;
> void *transport_info;
> + u32 shmem_io_width;
This is not used because the individual transports store the
shmem_io_width in their own transport_info structure.
[snip]
>
> +#define __shmem_copy_toio_tpl(s) \
> + for (unsigned int i = 0; i < xfer->tx.len; i += shmem_io_width) \
> + iowrite##s(((u##s *)(xfer->tx.buf))[i / shmem_io_width], \
> + shmem->msg_payload + i);
> +
> +#define __shmem_copy_fromio_tpl(s) \
> + for (unsigned int i = 0; i < xfer->rx.len; i += shmem_io_width) \
> + ((u##s *)(xfer->rx.buf))[i / shmem_io_width] = \
> + ioread##s(shmem->msg_payload + shmem_io_width + i);
This needs to be shmem->msg_payload + 4 + i. This worked in my testing
because I was precisely tseting with 'reg-io-width = <4>'.
[snip]
>
> static u32 shmem_read_header(struct scmi_shared_mem __iomem *shmem)
> @@ -81,8 +109,34 @@ static u32 shmem_read_header(struct scmi_shared_mem __iomem *shmem)
> return ioread32(&shmem->msg_header);
> }
>
> +static void __shmem_fetch_resp_notif_data(struct scmi_xfer *xfer,
> + struct scmi_shared_mem __iomem *shmem,
> + u32 shmem_io_width)
> +{
> + /* Take a copy to the rx buffer.. */
> + switch (shmem_io_width) {
> + case 1:
> + __shmem_copy_fromio_tpl(8);
> + break;
> + case 2:
> + __shmem_copy_fromio_tpl(16);
> + break;
> + case 4:
> + __shmem_copy_fromio_tpl(32);
> + break;
> + case 8:
> + __shmem_copy_fromio_tpl(32);
This should be 64 and it looks like ioread64/iowrite64 is not
necessarily defined for 32-bit configurations, so this needs to be gated
with a CONFIG_64BIT define here since ioread64/iowrite64 in
include/asm-generic/io.h is defined that way.
--
Florian
Powered by blists - more mailing lists