linux-kernel - Re: [PATCH 1/2] riscv: dts: spacemit: add pinctrl support for K1 SoC

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]

Message-ID: <b7a01cba-9f68-4a6f-9795-b9103ee81d8b@kernel.org>
Date: Tue, 23 Jul 2024 16:24:58 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Yixun Lan <dlan@...too.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Paul Walmsley <paul.walmsley@...ive.com>,
 Palmer Dabbelt <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>,
 Conor Dooley <conor@...nel.org>
Cc: Yangyu Chen <cyy@...self.name>, Jesse Taube <jesse@...osinc.com>,
 Jisheng Zhang <jszhang@...nel.org>, Inochi Amaoto <inochiama@...look.com>,
 Icenowy Zheng <uwu@...nowy.me>, Meng Zhang <zhangmeng.kevin@...cemit.com>,
 Meng Zhang <kevin.z.m@...mail.com>, devicetree@...r.kernel.org,
 linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] riscv: dts: spacemit: add pinctrl support for K1 SoC

On 19/07/2024 14:21, Yixun Lan wrote:
> SpacemiT's K1 SoC has a pinctrl controller which can be
> modeled using the pinctrl-single driver.
> 
> Signed-off-by: Yixun Lan <dlan@...too.org>
> ---
>  arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi | 1746 ++++++++++++++++++++++++++
>  arch/riscv/boot/dts/spacemit/k1-pinctrl.h    |  220 ++++
>  arch/riscv/boot/dts/spacemit/k1.dtsi         |   14 +
>  3 files changed, 1980 insertions(+)
> 
> diff --git a/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi b/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
> new file mode 100644
> index 0000000000000..ad6f171d3ec1f
> --- /dev/null
> +++ b/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
> @@ -0,0 +1,1746 @@
> +// SPDX-License-Identifier: GPL-2.0 OR MIT
> +/*
> + * Copyright (c) 2022 SpacemiT (Hangzhou) Technology Co. Ltd
> + * Copyright (c) 2024 Yixun Lan <dlan@...too.org>
> + */
> +
> +#include "k1-pinctrl.h"
> +
> +&pinctrl {
> +	camera0_pin: camera0-pin {
> +		pinctrl-single,pins = <
> +			K1_PADCONF0(53, 1) /* cam_mclk0 */
> +		>;
> +		pinctrl-single,bias-pullup = PINCTRL_PULLUP(0);
> +		pinctrl-single,bias-pulldown = PINCTRL_PULLDN(1);
> +		pinctrl-single,drive-strength = PINCTRL_1V8_DS(2);
> +	};
> +
> +	camera1_pin: camera1-pin {
> +		pinctrl-single,pins = <
> +			K1_PADCONF0(58, 1) /* cam_mclk1 */
> +		>;
> +		pinctrl-single,bias-pullup = PINCTRL_PULLUP(0);
> +		pinctrl-single,bias-pulldown = PINCTRL_PULLDN(1);
> +		pinctrl-single,drive-strength = PINCTRL_1V8_DS(2);
> +	};

...

> +
> +#endif /* __DT_BINDINGS_K1_PINCTRL_H */
> diff --git a/arch/riscv/boot/dts/spacemit/k1.dtsi b/arch/riscv/boot/dts/spacemit/k1.dtsi
> index 0777bf9e01183..2f0c56b06d413 100644
> --- a/arch/riscv/boot/dts/spacemit/k1.dtsi
> +++ b/arch/riscv/boot/dts/spacemit/k1.dtsi
> @@ -416,6 +416,20 @@ uart9: serial@...17800 {
>  			status = "disabled";
>  		};
>  
> +		pinctrl: pinctrl@...1e000 {
> +			compatible = "pinconf-single";

Please don't. This results in huge code in DTS mapping hardware
registers, basically what once SoC platform did some time ago for clocks
(one clock per node). That is not maintainable, not readable and does
not reduce amount of work. Quite contrary - due to nature of DTS
language, you have huge duplication of code.

Define proper pin controller driver.

Best regards,
Krzysztof