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 E8023F532EE for ; Tue, 24 Mar 2026 08:17:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=VIRFr+vTpbz+Bqzxl07zXOH5KY7QuZAdEr+yCdmlHbU=; b=j+PzotclWdhbl1GAu4knwUYe0o JjFLdORbIr7xGmYdlgNgLfCJkUtsPzki3ooUoB9ICr8oUsg/9VXrDZl5ApRXHWiSkCpEWKYfstESU /jWM+v0tcs+EAygHu/JyKfkN7j2Hr51l8WguBbSMtvq+XAUNM+8Ms9LfhR4fqtS8TihYyp88vWh2h 3+DR2bOeqtUVYqzxuy9ZwyXGBL5Eq8KIBZlyQlinqDYCscPCtf2kr8kPWccQ6QMcrDkm/S9Jwpxd/ H/5tLd/MAm9RtzwQp11gdBnmfc1MZIQclHx+9X7BW+i57U4nVtbFIyaPHqqKvSlpg6+waMllPQaoi VOfjZUhg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1w4wwc-00000000ySD-25Bn; Tue, 24 Mar 2026 08:16:58 +0000 Received: from lahtoruutu.iki.fi ([2a0b:5c81:1c1::37]) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1w4wwY-00000000yR8-2ZK0; Tue, 24 Mar 2026 08:16:57 +0000 Received: from hillosipuli.retiisi.eu (n18ws8cotq5gnfn8-1.v6.elisa-laajakaista.fi [IPv6:2001:99a:0:19f:4ce7:0:938c:d2f4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: sailus) by lahtoruutu.iki.fi (Postfix) with ESMTPSA id 4fg2v32Nrbz49Q4j; Tue, 24 Mar 2026 10:16:47 +0200 (EET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iki.fi; s=lahtoruutu; t=1774340208; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=VIRFr+vTpbz+Bqzxl07zXOH5KY7QuZAdEr+yCdmlHbU=; b=TgcYZ0ZXw3h6MLj2WcJ6Hfu5eh5gD/Fi1orEgGCkCwkRjZwOHtfQIv2SvUvLIMiVbokkIA A2pVSC7dfQkQ2c8cNRn4DI6gQt3PmrreqC5+dD5dJCkEsirL44vnR/xgEfXLcyGf4UKi5e oGxZ3Q3s0T81FYfFahmRabACiWRRy/dkhBdGktAK6lzzcXnwSquapbUswOfJHTZCiFhc+Y 50JXpwe10kHyAzMwAeG6zch5RorXOHlQ/SXf0H60BoFHUF0f7P+UE9FwYfef0LuJCdSfug jgpijiuz/AG0LtE/btoaUqGIeakMYQ5bHiVNpVQzMrRGn5XaQOS2ILQZDBG1uA== ARC-Seal: i=1; a=rsa-sha256; d=iki.fi; s=lahtoruutu; cv=none; t=1774340208; b=D6LEbkeVJYw1cBKR8+mTzHjdw3FM3apl2w2BHODVxEYWQuCxmw6sa3DWGm0FxD4pDXQheR 6KBtDrbxhScpHh/QGReYpuPkz+0WuhAo/ripVEwuUkTQkTSKdeWh2VyruKcCaT6munw30c uaBiM2hJQgYt3HUnt8j67jfTgfnGGrz9EgzJY6ApCEzcfB5MjYP4+5I2l5gBj3CkCvcBuz 5GzIaNKKJ3pCidvjYxL73b9p+7z321mhDEIiD9T9EorqIuljRM5fnrmXqjBgXQtOBV+MIV oDF5Y0nN8+8OMrqWh5fKThgY9C5F1BuYpI14RgFBlXb3XP229YSS+h1GTuSoug== ARC-Authentication-Results: i=1; ORIGINATING; auth=pass smtp.auth=sailus smtp.mailfrom=sakari.ailus@iki.fi ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=iki.fi; s=lahtoruutu; t=1774340208; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=VIRFr+vTpbz+Bqzxl07zXOH5KY7QuZAdEr+yCdmlHbU=; b=FC2FR84W50DqmnVdifULU5w8cNSruG+U48ilxjw+UkdDmqevvz+lUoeylCJic5YeV3LD34 6cO9JvPEOfhiktZjz9oNTw60kBmyeHLmTL6gxqJoQq7zXxVEyrr7dCVxBwi3BYI0w2OMq8 aOGGNy382Vq2AN3/3BuXSIo7ncbWp9jQksfGe7RwqUPU207bumq2TScT1+Og7avGd0lF18 VrSC6eq3915oPUNp4sw6bOGOrGho6NR0AQEJQXZ0f2R74vaqHyBVouWmz0BsYzivc10/Ci PBue0TvkHOvH2VuhjTjEDiTpHS7Uhl1uF0JTD1igLvw7IzAD2CiiAah6CgCreQ== Received: from valkosipuli.retiisi.eu (valkosipuli.local [192.168.4.2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange secp256r1 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by hillosipuli.retiisi.eu (Postfix) with ESMTPS id 91F4C634C51; Tue, 24 Mar 2026 10:16:46 +0200 (EET) Date: Tue, 24 Mar 2026 10:16:46 +0200 From: Sakari Ailus To: Frank Li Cc: Michael Riesch , Mauro Carvalho Chehab , Philipp Zabel , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Heiko Stuebner , linux-media@vger.kernel.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-rockchip@lists.infradead.org, Guoniu Zhou , Laurent Pinchart , imx@lists.linux.dev Subject: Re: [PATCH 4/6] media: synopsys: use struct dw_mipi_csi2rx_regs to describe register offsets Message-ID: References: <20260210-imx93-dw-csi2-v1-0-69667bb86bfa@nxp.com> <20260210-imx93-dw-csi2-v1-4-69667bb86bfa@nxp.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260210-imx93-dw-csi2-v1-4-69667bb86bfa@nxp.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20260324_011654_839512_E3249B08 X-CRM114-Status: GOOD ( 26.60 ) 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: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Frank, On Tue, Feb 10, 2026 at 12:11:11PM -0500, Frank Li wrote: > Use struct dw_mipi_csi2rx_regs to describe register offsets and support > new IP versions with differing register layouts. > > Add rk3568_regs, matching the previous macro definitions, and pass it as > driver data retrieved during probe. > > No functional change. > > Signed-off-by: Frank Li > --- > drivers/media/platform/synopsys/dw-mipi-csi2rx.c | 96 +++++++++++++++++------- > 1 file changed, 69 insertions(+), 27 deletions(-) > > diff --git a/drivers/media/platform/synopsys/dw-mipi-csi2rx.c b/drivers/media/platform/synopsys/dw-mipi-csi2rx.c > index 4ad4e3b23448affeeaa932a706653818ba4019ba..6a2966c9e3a2eac661fa1f8610c9f021d6e26cf8 100644 > --- a/drivers/media/platform/synopsys/dw-mipi-csi2rx.c > +++ b/drivers/media/platform/synopsys/dw-mipi-csi2rx.c > @@ -24,14 +24,39 @@ > #include > #include > > -#define DW_MIPI_CSI2RX_N_LANES 0x04 > -#define DW_MIPI_CSI2RX_RESETN 0x10 > -#define DW_MIPI_CSI2RX_PHY_STATE 0x14 > -#define DW_MIPI_CSI2RX_ERR1 0x20 > -#define DW_MIPI_CSI2RX_ERR2 0x24 > -#define DW_MIPI_CSI2RX_MSK1 0x28 > -#define DW_MIPI_CSI2RX_MSK2 0x2c > -#define DW_MIPI_CSI2RX_CONTROL 0x40 > +struct dw_mipi_csi2rx_regs { > + u32 n_lanes; > + u32 resetn; > + u32 phy_state; > + u32 err1; > + u32 err2; > + u32 msk1; > + u32 msk2; > + u32 control; > +}; > + > +struct dw_mipi_csi2rx_drvdata { > + const struct dw_mipi_csi2rx_regs *regs; > +}; > + > +/* Help check wrong access unexisted register at difference IP version */ > +#define DW_REG_EXIST BIT(31) > +#define DW_REG(x) (DW_REG_EXIST | (x)) > + > +static const struct dw_mipi_csi2rx_regs rk3568_regs = { > + .n_lanes = DW_REG(0x4), > + .resetn = DW_REG(0x10), > + .phy_state = DW_REG(0x14), > + .err1 = DW_REG(0x20), > + .err2 = DW_REG(0x24), > + .msk1 = DW_REG(0x28), > + .msk2 = DW_REG(0x2c), > + .control = DW_REG(0x40), > +}; > + > +static const struct dw_mipi_csi2rx_drvdata rk3568_drvdata = { "drvdata" is typically used of driver's context struct related to a given device. This is something else. How about calling it rk3568_devinfo for instance? > + .regs = &rk3568_regs, > +}; > > #define SW_CPHY_EN(x) ((x) << 0) > #define SW_DSI_EN(x) ((x) << 4) > @@ -74,8 +99,35 @@ struct dw_mipi_csi2rx_device { > > enum v4l2_mbus_type bus_type; > u32 lanes_num; > + > + const struct dw_mipi_csi2rx_drvdata *drvdata; > }; > > +static int > +dw_mipi_csi2rx_reg_err(struct dw_mipi_csi2rx_device *csi2, const char *name) > +{ > + dev_err_once(csi2->dev, "access to non-existent register: %s\n", name); > + return 0; > +} > + > +#define __dw_reg_exist(offset) ((offset) & DW_REG_EXIST) > + > +#define dw_reg_exist(csi2, __name) __dw_reg_exist((csi2)->drvdata->regs->__name) Can you use the same dw_mipi_csi2rx prefix here? > + > +#define dw_mipi_csi2rx_write(csi2, __name, value) \ > +({ auto __csi2 = csi2; \ Are there different types being used for csi2? As this there's a single driver here I'd suppose no? > + u32 offset = __csi2->drvdata->regs->__name; \ > + __dw_reg_exist(offset) ? \ > + writel(value, __csi2->base_addr + (offset & ~DW_REG_EXIST)) : \ > + dw_mipi_csi2rx_reg_err(__csi2, #__name); }) > + > +#define dw_mipi_csi2rx_read(csi2, __name) \ > +({ auto __csi2 = csi2; \ > + u32 offset = __csi2->drvdata->regs->__name; \ > + __dw_reg_exist(offset) ? \ > + readl(__csi2->base_addr + (offset & ~DW_REG_EXIST)) : \ > + dw_mipi_csi2rx_reg_err(__csi2, #__name); }) > + > static const struct v4l2_mbus_framefmt default_format = { > .width = 3840, > .height = 2160, > @@ -188,18 +240,6 @@ static inline struct dw_mipi_csi2rx_device *to_csi2(struct v4l2_subdev *sd) > return container_of(sd, struct dw_mipi_csi2rx_device, sd); > } > > -static inline void dw_mipi_csi2rx_write(struct dw_mipi_csi2rx_device *csi2, > - unsigned int addr, u32 val) > -{ > - writel(val, csi2->base_addr + addr); > -} > - > -static inline u32 dw_mipi_csi2rx_read(struct dw_mipi_csi2rx_device *csi2, > - unsigned int addr) > -{ > - return readl(csi2->base_addr + addr); > -} > - > static const struct dw_mipi_csi2rx_format * > dw_mipi_csi2rx_find_format(struct dw_mipi_csi2rx_device *csi2, u32 mbus_code) > { > @@ -265,9 +305,9 @@ static int dw_mipi_csi2rx_start(struct dw_mipi_csi2rx_device *csi2) > control |= SW_DATATYPE_FS(0x00) | SW_DATATYPE_FE(0x01) | > SW_DATATYPE_LS(0x02) | SW_DATATYPE_LE(0x03); > > - dw_mipi_csi2rx_write(csi2, DW_MIPI_CSI2RX_N_LANES, lanes - 1); > - dw_mipi_csi2rx_write(csi2, DW_MIPI_CSI2RX_CONTROL, control); > - dw_mipi_csi2rx_write(csi2, DW_MIPI_CSI2RX_RESETN, 1); > + dw_mipi_csi2rx_write(csi2, n_lanes, lanes - 1); > + dw_mipi_csi2rx_write(csi2, control, control); > + dw_mipi_csi2rx_write(csi2, resetn, 1); > > return phy_power_on(csi2->phy); > } > @@ -276,9 +316,9 @@ static void dw_mipi_csi2rx_stop(struct dw_mipi_csi2rx_device *csi2) > { > phy_power_off(csi2->phy); > > - dw_mipi_csi2rx_write(csi2, DW_MIPI_CSI2RX_RESETN, 0); > - dw_mipi_csi2rx_write(csi2, DW_MIPI_CSI2RX_MSK1, ~0); > - dw_mipi_csi2rx_write(csi2, DW_MIPI_CSI2RX_MSK2, ~0); > + dw_mipi_csi2rx_write(csi2, resetn, 0); > + dw_mipi_csi2rx_write(csi2, msk1, ~0); > + dw_mipi_csi2rx_write(csi2, msk2, ~0); > } > > static const struct media_entity_operations dw_mipi_csi2rx_media_ops = { > @@ -632,7 +672,7 @@ static void dw_mipi_csi2rx_unregister(struct dw_mipi_csi2rx_device *csi2) > > static const struct of_device_id dw_mipi_csi2rx_of_match[] = { > { > - .compatible = "rockchip,rk3568-mipi-csi2", > + .compatible = "rockchip,rk3568-mipi-csi2", .data = &rk3568_drvdata, > }, > {} > }; > @@ -654,6 +694,8 @@ static int dw_mipi_csi2rx_probe(struct platform_device *pdev) > if (IS_ERR(csi2->base_addr)) > return PTR_ERR(csi2->base_addr); > > + csi2->drvdata = device_get_match_data(dev); > + > ret = devm_clk_bulk_get_all(dev, &csi2->clks); > if (ret < 0) > return dev_err_probe(dev, -ENODEV, "failed to get clocks\n"); > -- Regards, Sakari Ailus