Re: [PATCH] media: nxp: imx8-isi: fix buiding on 32-bit

[Arnd Bergmann]

On Tue, Apr 18, 2023, at 05:20, Laurent Pinchart wrote:
> On Tue, Apr 18, 2023 at 12:37:27AM +0200, Arnd Bergmann wrote:
>> drivers/media/platform/nxp/imx8-isi/imx8-isi-hw.c:55:24: error: right shift count >= width of type [-Werror=shift-count-overflow]
>> 
>> But this is best avoided by using the lower_32_bits()/upper_32_bits()
>> helpers.
>
> I've submitted a patch yesterday that uses #ifdef, but I like this one
> better. One could argue that it leads to dead code on 32-bit platforms,
> but the ISI is only present in 64-bit SoCs, so that's not an issue.

I think there are actually some people using 32-bit kernels on imx8,
but they tend to do it for the wrong reasons, and I'm not worried
about wasting a few bytes in those setups.

My first draft kept an IS_ENABLED(CONFIG_ARCH_DMA_ADDR_T_64BIT) inside
each of the three if(), but I dropped that for readability.

>>  	if (buf_id == MXC_ISI_BUF1) {
>> -		mxc_isi_write(pipe, CHNL_OUT_BUF1_ADDR_Y, dma_addrs[0]);
>> -		mxc_isi_write(pipe, CHNL_OUT_BUF1_ADDR_U, dma_addrs[1]);
>> -		mxc_isi_write(pipe, CHNL_OUT_BUF1_ADDR_V, dma_addrs[2]);
>> -#if CONFIG_ARCH_DMA_ADDR_T_64BIT
>> +		mxc_isi_write(pipe, CHNL_OUT_BUF1_ADDR_Y,
>> +			lower_32_bits(dma_addrs[0]));
>
> Could you please align this with the ( ? I can also do so in my tree,
> but I expect Mauro to pick this patch directly from the list, so a v2
> would make it easier. You can then add my
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>

Ok, I sent that out now, but now I see I forgot to add your
Reviewed-by.

     Arnd