linux-kernel - Re: [PATCH 3/3] pinctrl: rockchip: add rk3308b SoC support

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] [thread-next>] [day] [month] [year] [list]

Date: Wed, 15 May 2024 18:29:54 +0200
From: Luca Ceresoli <luca.ceresoli@...tlin.com>
To: Dmitry Yashin <dmt.yashin@...il.com>
Cc: Linus Walleij <linus.walleij@...aro.org>, Heiko Stuebner
 <heiko@...ech.de>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Rob Herring <robh@...nel.org>, Jianqun Xu
 <jay.xu@...k-chips.com>, devicetree@...r.kernel.org,
 linux-gpio@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-rockchip@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] pinctrl: rockchip: add rk3308b SoC support

Hello Dmitry,

On Wed, 15 May 2024 17:16:34 +0500
Dmitry Yashin <dmt.yashin@...il.com> wrote:

> Add pinctrl support for rk3308b. This pin controller much the same as
> rk3308's, but with additional iomux routes and 3bit iomuxes selected
> via gpio##_sel_src_ctrl registers. Set them up in the function
> rk3308b_soc_sel_src_init to use new 3bit iomuxes over some 2bit old ones.
> 
> Fixes: 1f3e25a06883 ("pinctrl: rockchip: fix RK3308 pinmux bits")
> Signed-off-by: Dmitry Yashin <dmt.yashin@...il.com>

Thanks for the effort! I have one high-level remark, see below.
Otherwise at a superficial look it looks good.

> @@ -3952,6 +4150,8 @@ static const struct of_device_id rockchip_pinctrl_dt_match[] = {
>  		.data = &rk3288_pin_ctrl },
>  	{ .compatible = "rockchip,rk3308-pinctrl",
>  		.data = &rk3308_pin_ctrl },
> +	{ .compatible = "rockchip,rk3308b-pinctrl",
> +		.data = &rk3308b_pin_ctrl },

I'm skeptical about this being bound to a new DT compatible. As far as I
know the RK3308 and RK3308B are mostly equivalent, so it looks as the
pinctrl implementation could be detected at runtime. This would let
products to be built with either chip version and work on any without
any DT change.

Code for reading the chip ID is in the RK3308 codec driver [0].

[0] https://lore.kernel.org/all/20240305-rk3308-audio-codec-v4-4-312acdbe628f@bootlin.com/ -> search "GRF_CHIP_ID"

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com