On 2023/8/21 18:26, Will Deacon wrote:
> On Mon, Aug 21, 2023 at 04:19:54PM +0800, Herbert Xu wrote:
>> On Sat, Aug 19, 2023 at 09:33:18AM +0200, Ard Biesheuvel wrote:
>>>
>>> No, that otx2_write128() routine looks buggy, actually, The ! at the
>>> end means writeback, and so the register holding addr will be
>>> modified, which is not reflect in the asm constraints. It also lacks a
>>> barrier.
>>
>> OK. But at least having a helper called write128 looks a lot
>> cleaner than just having unexplained assembly in the code.
>
> I guess we want something similar to how writeq() is handled on 32-bit
> architectures (see include/linux/io-64-nonatomic-{hi-lo,lo-hi}.h.
>
> It's then CPU-dependent on whether you get atomicity.
>
> Will
> .
>
Thanks for the review.
Since the HiSilicon accelerator devices are supported only on the ARM64 platform,
the following 128bit read and write interfaces are added to the driver, is this OK?
#if defined(CONFIG_ARM64)
static void qm_write128(void __iomem *addr, const void *buffer)
{
unsigned long tmp0 = 0, tmp1 = 0;
asm volatile("ldp %0, %1, %3\n"
"stp %0, %1, %2\n"
"dmb oshst\n"
: "=&r" (tmp0),
"=&r" (tmp1),
"+Q" (*((char __iomem *)addr))
: "Q" (*((char *)buffer))
: "memory");
}
static void qm_read128(void *buffer, const void __iomem *addr)
{
unsigned long tmp0 = 0, tmp1 = 0;
asm volatile("ldp %0, %1, %3\n"
"stp %0, %1, %2\n"
"dmb oshst\n"
: "=&r" (tmp0),
"=&r" (tmp1),
"+Q" (*((char *)buffer))
: "Q" (*((char __iomem *)addr))
: "memory");
}
#else
static void qm_write128(void __iomem *addr, const void *buffer)
{
}
static void qm_read128(void *buffer, const void __iomem *addr)
{
}
#endif
Thanks,
Weili
