[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <b2ce3e0b-a639-4e70-a5e7-ffbdc855153e@app.fastmail.com>
Date: Thu, 25 Sep 2025 14:14:39 +0200
From: "Arnd Bergmann" <arnd@...db.de>
To: "Manikanta Guntupalli" <manikanta.guntupalli@....com>,
"git (AMD-Xilinx)" <git@....com>, "Michal Simek" <michal.simek@....com>,
"Alexandre Belloni" <alexandre.belloni@...tlin.com>,
"Frank Li" <Frank.Li@....com>, "Rob Herring" <robh@...nel.org>,
"krzk+dt@...nel.org" <krzk+dt@...nel.org>,
"Conor Dooley" <conor+dt@...nel.org>,
Przemysław Gaj <pgaj@...ence.com>,
"Wolfram Sang" <wsa+renesas@...g-engineering.com>,
"tommaso.merciai.xr@...renesas.com" <tommaso.merciai.xr@...renesas.com>,
"quic_msavaliy@...cinc.com" <quic_msavaliy@...cinc.com>, "S-k,
Shyam-sundar" <Shyam-sundar.S-k@....com>,
"Sakari Ailus" <sakari.ailus@...ux.intel.com>,
"'billy_tsai@...eedtech.com'" <billy_tsai@...eedtech.com>,
"Kees Cook" <kees@...nel.org>,
"Gustavo A. R. Silva" <gustavoars@...nel.org>,
"Jarkko Nikula" <jarkko.nikula@...ux.intel.com>,
"Jorge Marques" <jorge.marques@...log.com>,
"linux-i3c@...ts.infradead.org" <linux-i3c@...ts.infradead.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Linux-Arch <linux-arch@...r.kernel.org>,
"linux-hardening@...r.kernel.org" <linux-hardening@...r.kernel.org>
Cc: "Pandey, Radhey Shyam" <radhey.shyam.pandey@....com>,
"Goud, Srinivas" <srinivas.goud@....com>,
"Datta, Shubhrajyoti" <shubhrajyoti.datta@....com>,
"manion05gk@...il.com" <manion05gk@...il.com>
Subject: Re: [PATCH V7 3/4] i3c: master: Add endianness support for i3c_readl_fifo()
and i3c_writel_fifo()
On Thu, Sep 25, 2025, at 11:26, Guntupalli, Manikanta wrote:
>> Can you explain how that works? What I see is that your
>> readsl_be()/writesl_be() functions do a byteswap on every four bytes, so the
>> bytestream that gets copied to/from the FIFO gets garbled, in particular the final
>> (unaligned) bytes of the kernel buffer end up in the higher bytes of the FIFO register
>> rather than the first bytes as they do on a big-endian kernel.
>>
>> Are both the big-endian and little-endian kernels in your tests on microblaze, using
>> the upstream version of asm/io.h? Is there a hardware byteswap between the CPU
>> local bus and the i3c controller? If there is one, is it set the same way for both
>> kernels?
>>
> To clarify, my testing was performed on the latest upstream kernel on a
> ZCU102 (Zynq UltraScale+ MPSoC, Cortex-A53, little-endian) with
> big-endian FIFOs and no bus-level byteswap. For more details, please
> refer to my reply in Re: [PATCH] [v2] i3c: fix big-endian FIFO
> transfers.
Ok, thanks fro the clarification. I got confused by your description
mentioning big-endian in [PATCH V7 3/4] and assumed this would be on
a big-endian microblaze CPU, after I saw that the original i3c_readl_fifo()
had a bug in that configuration that your patch addressed and this
being a driver for a xilinx design. That fix just turned out unrelated
to what you were actually trying to do ;-)
> Please don't take this as negative or aggressive-my intention is purely
> to learn and ensure it works correctly in all cases.
No worries, I should not have jumped to conclusions myself based
on what I saw in your implementation and assumed that fixing the
one bug would address your problem as well.
I do understand that your driver clearly needs a special case,
we just need to come to a conclusion what exactly the hardware
does and how to best deal with it. This is partly about whether
you may be able to use an external DMA engine such as
xlnx,zynqmp-dma-1.0 or xlnx,zynqmp-dpdma, and whether that would
need the same byteswap.
If you already plan to add that support, you likely need to
allocate a bounce buffer and byteswap the data in place
to have it copied in and out of the FIFO, and when you have
that, you can use the regular i3c_readl_fifo() unchanged.
If you are sure that the i3c controller is not going to be
used with DMA, or if the FIFO register as seen by the DMA
master does not require a byteswap, having a local helper
for the transfer is likely easier.
Arnd
Powered by blists - more mailing lists