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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 18 Apr 2023 08:13:05 +0200
From:   "Arnd Bergmann" <arnd@...db.de>
To:     "laurent.pinchart" <laurent.pinchart@...asonboard.com>,
        "Arnd Bergmann" <arnd@...nel.org>
Cc:     "Mauro Carvalho Chehab" <mchehab@...nel.org>,
        "Shawn Guo" <shawnguo@...nel.org>,
        "Sascha Hauer" <s.hauer@...gutronix.de>,
        "Christian Hemp" <c.hemp@...tec.de>,
        "Dong Aisheng" <aisheng.dong@....com>,
        "Stefan Riedmueller" <s.riedmueller@...tec.de>,
        "Pengutronix Kernel Team" <kernel@...gutronix.de>,
        "Fabio Estevam" <festevam@...il.com>,
        "NXP Linux Team" <linux-imx@....com>,
        "Jacopo Mondi" <jacopo.mondi@...asonboard.com>,
        linux-media@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] media: nxp: imx8-isi: fix buiding on 32-bit

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@...asonboard.com>

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

     Arnd