Re: [PATCH net-next v10 1/8] hinic3: Async Event Queue interfaces

[Simon Horman <horms@kernel.org>]

On Tue, Jul 22, 2025 at 03:18:40PM +0800, Fan Gong wrote:
> Add async event queue interfaces initialization.
> It allows driver to handle async events reported by HW.
> 
> Co-developed-by: Xin Guo <guoxin09@huawei.com>
> Signed-off-by: Xin Guo <guoxin09@huawei.com>
> Co-developed-by: Zhu Yikai <zhuyikai1@h-partners.com>
> Signed-off-by: Zhu Yikai <zhuyikai1@h-partners.com>
> Signed-off-by: Fan Gong <gongfan1@huawei.com>

...

> diff --git a/drivers/net/ethernet/huawei/hinic3/hinic3_common.c b/drivers/net/ethernet/huawei/hinic3/hinic3_common.c
> index 0aa42068728c..a5aaf6febba9 100644
> --- a/drivers/net/ethernet/huawei/hinic3/hinic3_common.c
> +++ b/drivers/net/ethernet/huawei/hinic3/hinic3_common.c
> @@ -51,3 +51,16 @@ void hinic3_dma_free_coherent_align(struct device *dev,
>  	dma_free_coherent(dev, mem_align->real_size,
>  			  mem_align->ori_vaddr, mem_align->ori_paddr);
>  }
> +
> +/* Data provided to/by cmdq is arranged in structs with little endian fields but
> + * every dword (32bits) should be swapped since HW swaps it again when it
> + * copies it from/to host memory.
> + */

This scheme may work on little endian hosts.
But if so it seems unlikely to work on big endian hosts.

I expect you want be32_to_cpu_array() for data coming from hw,
with a source buffer as an array of __be32 while
the destination buffer is an array of u32.

And cpu_to_be32_array() for data going to the hw,
with the types of the source and destination buffers reversed.

If those types don't match your data, then we have
a framework to have that discussion.


That said, it is more usual for drivers to keep structures in the byte
order they are received. Stored in structures with members with types, in
this case it seems that would be __be32, and accessed using a combination
of BIT/GENMASK, FIELD_PREP/FIELD_GET, and cpu_to_be*/be*_to_cpu (in this
case cpu_to_be32/be32_to_cpu).

An advantage of this approach is that the byte order of
data is only changed when needed. Another is that it is clear
what the byte order of data is.

> +void hinic3_cmdq_buf_swab32(void *data, int len)
> +{
> +	u32 *mem = data;
> +	u32 i;
> +
> +	for (i = 0; i < len / sizeof(u32); i++)
> +		mem[i] = swab32(mem[i]);
> +}

This seems to open code swab32_array().

...