From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 65706C77B75 for ; Tue, 18 Apr 2023 03:46:59 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=h+AE1L3NrKdZFNOgTLD5YrN44GKMH1np1h6RbJneqXw=; b=xIL3rLlazwl0sQ XO+cnbEyRZHkgOmmX5XkYMHQYRHkpDlYla1RcTN/vryz+Xy29Cz8huDRtBxRgUDubg/55Pk19cbXC zrrEV5Fpehb80M9SLZPA9PUWkjb+lModpA4rrwmyPP4mSMoIYO12IB3Rw78yIB2GRjR12en4drfIz xx8ovNC/GppcodF3fhjoLePIKgj8qdb3T+ysEMId9gKS7ThEd335LWEueqFON+G/V+kNELbgJ18r2 Gwef/I1QVAjJdXG8MrY3fOgHzzHzWtPUGbXx7ayiuOhG4pbaKicIk68ESuEzvcqzDVYFb583y1jW/ flm3La0tM4Y434SKOZFA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1pocHZ-000lC6-0j; Tue, 18 Apr 2023 03:45:29 +0000 Received: from desiato.infradead.org ([2001:8b0:10b:1:d65d:64ff:fe57:4e05]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1pocHX-000lBw-1p for linux-arm-kernel@bombadil.infradead.org; Tue, 18 Apr 2023 03:45:27 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=desiato.20200630; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=miEXN68ovrMu6JgphcIBAob5hnYvrZQwT3YJSjN6EJA=; b=OBUjuk4EZJZqfifoxtdNSqQ1tr 2cXO1io/IkQHM/Gi7Feu9lVt2QKxRHgwvl4gKZ25jIcQ8SVHeU7LIs9SqbBrF3WNzdUm9HaYhW5Ye Ck+FdlDjPVInIvFVdhKt9ZieN4jeO4Qj/91Y3cSE+6cqzQ6Mapzo7yvp+bYAN5wmbxvDXy4jyAL2v d31wh72ErPl1BdoWZVXPEpdqe87INKeO7/RMpZdMzasfMIux1LLWk98/NshQ4kd19pKFaIfdc8/dQ VxJfFivmuvIG0sTViR5JwQxU7+xSstjae5/zbri7BmI/94fSunZjUy6tTJFxkTZ66s+CneecsxlPx oEcmBJVQ==; Received: from perceval.ideasonboard.com ([213.167.242.64]) by desiato.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1pobuB-000JZC-2U for linux-arm-kernel@lists.infradead.org; Tue, 18 Apr 2023 03:21:24 +0000 Received: from pendragon.ideasonboard.com (133-32-181-51.west.xps.vectant.ne.jp [133.32.181.51]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id A0611802; Tue, 18 Apr 2023 05:20:12 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1681788013; bh=FmAPaG5Rw0O5ZC4OZJehoo+7UlYBrnknsOaW1shgmlA=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=cdddtK3VDB1vHQjDBJSV1C0mSmkEMvWLS37C2/t0C+mSJ3FEQlINGW92zX8g6ngNo 4bI8hCZeNAq5YKFpLAMwO5uk77TaOYw0QvQUs54oUTZzI6ZF3PD1ILrKQjc9jh4dCo r7NdV+qQAjm+2SUOozb83kPCTI2JEt6P7QZ4PlY4= Date: Tue, 18 Apr 2023 06:20:31 +0300 From: Laurent Pinchart To: Arnd Bergmann Cc: Mauro Carvalho Chehab , Shawn Guo , Sascha Hauer , Christian Hemp , Dong Aisheng , Stefan Riedmueller , Arnd Bergmann , Pengutronix Kernel Team , Fabio Estevam , NXP Linux Team , Jacopo Mondi , linux-media@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] media: nxp: imx8-isi: fix buiding on 32-bit Message-ID: <20230418032031.GA4703@pendragon.ideasonboard.com> References: <20230417223738.1811110-1-arnd@kernel.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20230417223738.1811110-1-arnd@kernel.org> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230418_042122_218165_BA139B08 X-CRM114-Status: GOOD ( 25.90 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Arnd, Thank you for the patch. On Tue, Apr 18, 2023 at 12:37:27AM +0200, Arnd Bergmann wrote: > From: Arnd Bergmann > > The #if check is wrong, leading to a build failure: > > drivers/media/platform/nxp/imx8-isi/imx8-isi-hw.c: In function 'mxc_isi_channel_set_inbuf': > drivers/media/platform/nxp/imx8-isi/imx8-isi-hw.c:33:5: error: "CONFIG_ARCH_DMA_ADDR_T_64BIT" is not defined, evaluates to 0 [-Werror=undef] > 33 | #if CONFIG_ARCH_DMA_ADDR_T_64BIT > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > This could just be an #ifdef, but it seems nicer to just remove > the check entirely. Apparently the only reason for the #ifdef > is to avoid another warning: > > 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. > Fixes: cf21f328fcaf ("media: nxp: Add i.MX8 ISI driver") > Signed-off-by: Arnd Bergmann > --- > .../media/platform/nxp/imx8-isi/imx8-isi-hw.c | 41 ++++++++++--------- > 1 file changed, 21 insertions(+), 20 deletions(-) > > diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-hw.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-hw.c > index db538f3d88ec..f6112b83866a 100644 > --- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-hw.c > +++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-hw.c > @@ -29,11 +29,10 @@ static inline void mxc_isi_write(struct mxc_isi_pipe *pipe, u32 reg, u32 val) > > void mxc_isi_channel_set_inbuf(struct mxc_isi_pipe *pipe, dma_addr_t dma_addr) > { > - mxc_isi_write(pipe, CHNL_IN_BUF_ADDR, dma_addr); > -#if CONFIG_ARCH_DMA_ADDR_T_64BIT > + mxc_isi_write(pipe, CHNL_IN_BUF_ADDR, lower_32_bits(dma_addr)); > if (pipe->isi->pdata->has_36bit_dma) > - mxc_isi_write(pipe, CHNL_IN_BUF_XTND_ADDR, dma_addr >> 32); > -#endif > + mxc_isi_write(pipe, CHNL_IN_BUF_XTND_ADDR, > + upper_32_bits(dma_addr)); > } > > void mxc_isi_channel_set_outbuf(struct mxc_isi_pipe *pipe, > @@ -45,34 +44,36 @@ void mxc_isi_channel_set_outbuf(struct mxc_isi_pipe *pipe, > val = mxc_isi_read(pipe, CHNL_OUT_BUF_CTRL); > > 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 > + mxc_isi_write(pipe, CHNL_OUT_BUF1_ADDR_U, > + lower_32_bits(dma_addrs[1])); > + mxc_isi_write(pipe, CHNL_OUT_BUF1_ADDR_V, > + lower_32_bits(dma_addrs[2])); > if (pipe->isi->pdata->has_36bit_dma) { > mxc_isi_write(pipe, CHNL_Y_BUF1_XTND_ADDR, > - dma_addrs[0] >> 32); > + upper_32_bits(dma_addrs[0])); > mxc_isi_write(pipe, CHNL_U_BUF1_XTND_ADDR, > - dma_addrs[1] >> 32); > + upper_32_bits(dma_addrs[1])); > mxc_isi_write(pipe, CHNL_V_BUF1_XTND_ADDR, > - dma_addrs[2] >> 32); > + upper_32_bits(dma_addrs[2])); > } > -#endif > val ^= CHNL_OUT_BUF_CTRL_LOAD_BUF1_ADDR; > } else { > - mxc_isi_write(pipe, CHNL_OUT_BUF2_ADDR_Y, dma_addrs[0]); > - mxc_isi_write(pipe, CHNL_OUT_BUF2_ADDR_U, dma_addrs[1]); > - mxc_isi_write(pipe, CHNL_OUT_BUF2_ADDR_V, dma_addrs[2]); > -#if CONFIG_ARCH_DMA_ADDR_T_64BIT > + mxc_isi_write(pipe, CHNL_OUT_BUF2_ADDR_Y, > + lower_32_bits(dma_addrs[0])); > + mxc_isi_write(pipe, CHNL_OUT_BUF2_ADDR_U, > + lower_32_bits(dma_addrs[1])); > + mxc_isi_write(pipe, CHNL_OUT_BUF2_ADDR_V, > + lower_32_bits(dma_addrs[2])); > if (pipe->isi->pdata->has_36bit_dma) { > mxc_isi_write(pipe, CHNL_Y_BUF2_XTND_ADDR, > - dma_addrs[0] >> 32); > + upper_32_bits(dma_addrs[0])); > mxc_isi_write(pipe, CHNL_U_BUF2_XTND_ADDR, > - dma_addrs[1] >> 32); > + upper_32_bits(dma_addrs[1])); > mxc_isi_write(pipe, CHNL_V_BUF2_XTND_ADDR, > - dma_addrs[2] >> 32); > + upper_32_bits(dma_addrs[2])); > } > -#endif > val ^= CHNL_OUT_BUF_CTRL_LOAD_BUF2_ADDR; > } > -- Regards, Laurent Pinchart _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel