Message ID | 20231113005702.2467-2-jszhang@kernel.org |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b909:0:b0:403:3b70:6f57 with SMTP id t9csp937545vqg; Sun, 12 Nov 2023 17:09:34 -0800 (PST) X-Google-Smtp-Source: AGHT+IHlEqC7A6axg10odA7BclHNNW9i566uHPlQY83bdEYrfOabL3jDdeIvQKjPFOVV/Er2sAtr X-Received: by 2002:a17:90b:3847:b0:27c:f309:f381 with SMTP id nl7-20020a17090b384700b0027cf309f381mr5939542pjb.6.1699837774595; Sun, 12 Nov 2023 17:09:34 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1699837774; cv=none; d=google.com; s=arc-20160816; b=CWw4PmCWJLusA3YTUJEzmJg2TXogMO7oKTosVSLXs2w4cyDEGuiZUYq7q9T0mmJRAf CqzaeuGFPdAPzEdu9KNF4fSV6/Fua/QssBvHsnZOi/mPb1F+EPvNU7rCYEh92pQl4CQW g/fmqPHe7efnGZYtv7YpCd7tvyDnjkqCwbu4lHLR/BoyYri39XgpLtuuzw+OGLATpHwx /hTSIzL5RiDBHp5EUw1A/pf9ZZTE81tynlkI96TVyljYcwJfwL9GC8j5o5U3pqVbLcwt xEBpjLiDwHFxgHY1oPFEbuHBlGffEaNoqXWGSFBFvLu/MHe0Dyrkh08M1dw4GmZ/GA3z v1Mw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=ciLaCnJxMtnoZAc5xUN+L/PUVeCVk4sAPW0V93MHkNg=; fh=SEnrj8kK8OIRId3/FYvxkia4waswDsqs7tUaPlsk3E8=; b=epF3KuenVooBxhPzzZ1w1TIYAs++PGxpGEBhcXUNjBJjKdUnMB7lvPkzyaAGMZyX3w /Iu8jP6vRGigV4mSDbVpeMLglHdwW7fUMD1eK5gKinMOMZP7UOUAQLPnx2r/D15Q/9Oj TuuZrqW8w5N4AI8GH5N4yUH4hJP4wdyYDLi4Mo4cBQ9VGHxivCEBK+PcyTc24parvl/L vxmZSkNTGZTkZwqtbPRUH7tvDHuZH7qBWFJlSMgPIULj9AGmLGY/QPEg2gcALnopIaWq QHWDW9GSo97Hx1AeJaJFN+AVjRD9eJFBvKOpJ+PlVrjRXPVG8xHU1xhPlCmft2Lji2eQ 4sMQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=O6rb0rqu; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: from snail.vger.email (snail.vger.email. [2620:137:e000::3:7]) by mx.google.com with ESMTPS id w10-20020a17090abc0a00b00280cc652dd6si9431423pjr.173.2023.11.12.17.09.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 12 Nov 2023 17:09:34 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=O6rb0rqu; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by snail.vger.email (Postfix) with ESMTP id D778280A5F5B; Sun, 12 Nov 2023 17:09:33 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at snail.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232721AbjKMBJa (ORCPT <rfc822;lhua1029@gmail.com> + 30 others); Sun, 12 Nov 2023 20:09:30 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50024 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232698AbjKMBJ2 (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Sun, 12 Nov 2023 20:09:28 -0500 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4F0691BC0 for <linux-kernel@vger.kernel.org>; Sun, 12 Nov 2023 17:09:25 -0800 (PST) Received: by smtp.kernel.org (Postfix) with ESMTPSA id D71F6C433CB; Mon, 13 Nov 2023 01:09:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1699837765; bh=eM8d9IMh9iQd7b2YIyNC18c8crry0l3aFLLjrqGpruk=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=O6rb0rquzaUvuzBIP52vidXntbBi5DW5ejXnhTOUBEwOPsf8ZN5BMhV8fR34Lx5ip 5JkRoKxH41DzNnuB04HSXBupca8B9hDi3cuIWjczJ6JO9eqnJQjjOKE5KAzpzEwBuE qdHLIdJgAiRr+fpy1YJIYi9/snJYD66jf17nwDh0zfWscpB/IrbuKSo0AqTX7GqCZT mfPbCUYYhl+XKOfLGNeWMs4YC3IJh+ZPH27RTLGyIJ+H5rB9fan8N90hM5IGyKWU45 0DiSQnTobBsQZMU/NTDqwFFaG2iLFJdsVVA/szOWrjgG6Jx3PKHUTUWDkV7EpLP0Tu epwC5wBII2xww== From: Jisheng Zhang <jszhang@kernel.org> To: Rob Herring <robh+dt@kernel.org>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Conor Dooley <conor+dt@kernel.org>, Paul Walmsley <paul.walmsley@sifive.com>, Palmer Dabbelt <palmer@dabbelt.com>, Albert Ou <aou@eecs.berkeley.edu>, Chao Wei <chao.wei@sophgo.com>, Chen Wang <unicorn_wang@outlook.com> Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org Subject: [PATCH 1/2] riscv: dts: cv1800b: add pinctrl node for cv1800b Date: Mon, 13 Nov 2023 08:57:01 +0800 Message-Id: <20231113005702.2467-2-jszhang@kernel.org> X-Mailer: git-send-email 2.40.0 In-Reply-To: <20231113005702.2467-1-jszhang@kernel.org> References: <20231113005702.2467-1-jszhang@kernel.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF, RCVD_IN_DNSWL_BLOCKED,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Sun, 12 Nov 2023 17:09:33 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1782409094304609857 X-GMAIL-MSGID: 1782409094304609857 |
Series |
riscv: sophgo: add pinctrl support for cv1800b
|
|
Commit Message
Jisheng Zhang
Nov. 13, 2023, 12:57 a.m. UTC
Add the reset device tree node to cv1800b SoC reusing the
pinctrl-single driver.
Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
---
arch/riscv/boot/dts/sophgo/cv-pinctrl.h | 19 +++++++++++++++++++
arch/riscv/boot/dts/sophgo/cv1800b.dtsi | 10 ++++++++++
2 files changed, 29 insertions(+)
create mode 100644 arch/riscv/boot/dts/sophgo/cv-pinctrl.h
Comments
On 2023-11-12 6:57 PM, Jisheng Zhang wrote: > Add the reset device tree node to cv1800b SoC reusing the ^^^^^ I assume you mean pinctrl here? > pinctrl-single driver. > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > --- > arch/riscv/boot/dts/sophgo/cv-pinctrl.h | 19 +++++++++++++++++++ > arch/riscv/boot/dts/sophgo/cv1800b.dtsi | 10 ++++++++++ > 2 files changed, 29 insertions(+) > create mode 100644 arch/riscv/boot/dts/sophgo/cv-pinctrl.h > > diff --git a/arch/riscv/boot/dts/sophgo/cv-pinctrl.h b/arch/riscv/boot/dts/sophgo/cv-pinctrl.h > new file mode 100644 > index 000000000000..ed78b6fb3142 > --- /dev/null > +++ b/arch/riscv/boot/dts/sophgo/cv-pinctrl.h A couple of questions: Should this go in include/dt-bindings? And is it worth including macros for the actual function mappings, like in the vendor source[1]? [1]: https://github.com/milkv-duo/duo-buildroot-sdk/blob/develop/linux_5.10/drivers/pinctrl/cvitek/cv180x_pinlist_swconfig.h > @@ -0,0 +1,19 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * This header provides constants for pinctrl bindings for Sophgo CV* SoC. > + * > + * Copyright (C) 2023 Jisheng Zhang <jszhang@kernel.org> > + */ > +#ifndef _DTS_RISCV_SOPHGO_CV_PINCTRL_H > +#define _DTS_RISCV_SOPHGO_CV_PINCTRL_H > + > +#define MUX_M0 0 > +#define MUX_M1 1 > +#define MUX_M2 2 > +#define MUX_M3 3 > +#define MUX_M4 4 > +#define MUX_M5 5 > +#define MUX_M6 6 > +#define MUX_M7 7 > + > +#endif > diff --git a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi > index e04df04a91c0..7a44d8e8672b 100644 > --- a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi > +++ b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi > @@ -6,6 +6,8 @@ > #include <dt-bindings/interrupt-controller/irq.h> > #include <dt-bindings/reset/sophgo,cv1800b-reset.h> > > +#include "cv-pinctrl.h" > + > / { > compatible = "sophgo,cv1800b"; > #address-cells = <1>; > @@ -55,6 +57,14 @@ soc { > dma-noncoherent; > ranges; > > + pinctrl0: pinctrl@3001000 { > + compatible = "pinctrl-single"; > + reg = <0x3001000 0x130>; > + #pinctrl-cells = <1>; > + pinctrl-single,register-width = <32>; > + pinctrl-single,function-mask = <0x00000007>; > + }; From the vendor driver[2], it looks like this peripheral block only handles pinmuxing, so indeed this looks like a good use of pinctrl-single. [2]: https://github.com/milkv-duo/duo-buildroot-sdk/blob/develop/linux_5.10/drivers/pinctrl/cvitek/pinctrl-cv180x.h > + > rst: reset-controller@3003000 { > compatible = "sophgo,cv1800b-reset"; > reg = <0x03003000 0x1000>;
On Sun, Nov 12, 2023 at 08:51:20PM -0500, Samuel Holland wrote: > On 2023-11-12 6:57 PM, Jisheng Zhang wrote: > > Add the reset device tree node to cv1800b SoC reusing the > ^^^^^ > I assume you mean pinctrl here? oops copy and paste the commit msg ;) thanks > > > pinctrl-single driver. > > > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > > --- > > arch/riscv/boot/dts/sophgo/cv-pinctrl.h | 19 +++++++++++++++++++ > > arch/riscv/boot/dts/sophgo/cv1800b.dtsi | 10 ++++++++++ > > 2 files changed, 29 insertions(+) > > create mode 100644 arch/riscv/boot/dts/sophgo/cv-pinctrl.h > > > > diff --git a/arch/riscv/boot/dts/sophgo/cv-pinctrl.h b/arch/riscv/boot/dts/sophgo/cv-pinctrl.h > > new file mode 100644 > > index 000000000000..ed78b6fb3142 > > --- /dev/null > > +++ b/arch/riscv/boot/dts/sophgo/cv-pinctrl.h > > A couple of questions: Should this go in include/dt-bindings? And is it worth When I cooked this series two weeks ago, I did put it in dt-binding, but then I found commit fe49f2d776f799 ("arm64: dts: ti: Use local header for pinctrl register values"), "These definitions were previously put in the bindings header to avoid code duplication and to provide some context meaning (name), but they do not fit the purpose of bindings." which is suggested and acked by Krzysztof, so I just want to follow the style here. > including macros for the actual function mappings, like in the vendor source[1]? Do you want something as the following? #define UART0_TX 0 #define CAM_MCLK1 1 ... #define REG_UART0_TX 0x24 ... pinctrl-single,pins = <REG_UART0_TX UART0_TX>; Other pinctl-single users just uses the register value directly, I have no preference. But I'd like to get suggestions from DT and pinctl-single maintainers. Hi Rob, Krzysztof, Conor, Tony, what's your opinion? > > [1]: > https://github.com/milkv-duo/duo-buildroot-sdk/blob/develop/linux_5.10/drivers/pinctrl/cvitek/cv180x_pinlist_swconfig.h > > > @@ -0,0 +1,19 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +/* > > + * This header provides constants for pinctrl bindings for Sophgo CV* SoC. > > + * > > + * Copyright (C) 2023 Jisheng Zhang <jszhang@kernel.org> > > + */ > > +#ifndef _DTS_RISCV_SOPHGO_CV_PINCTRL_H > > +#define _DTS_RISCV_SOPHGO_CV_PINCTRL_H > > + > > +#define MUX_M0 0 > > +#define MUX_M1 1 > > +#define MUX_M2 2 > > +#define MUX_M3 3 > > +#define MUX_M4 4 > > +#define MUX_M5 5 > > +#define MUX_M6 6 > > +#define MUX_M7 7 > > + > > +#endif > > diff --git a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi > > index e04df04a91c0..7a44d8e8672b 100644 > > --- a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi > > +++ b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi > > @@ -6,6 +6,8 @@ > > #include <dt-bindings/interrupt-controller/irq.h> > > #include <dt-bindings/reset/sophgo,cv1800b-reset.h> > > > > +#include "cv-pinctrl.h" > > + > > / { > > compatible = "sophgo,cv1800b"; > > #address-cells = <1>; > > @@ -55,6 +57,14 @@ soc { > > dma-noncoherent; > > ranges; > > > > + pinctrl0: pinctrl@3001000 { > > + compatible = "pinctrl-single"; > > + reg = <0x3001000 0x130>; > > + #pinctrl-cells = <1>; > > + pinctrl-single,register-width = <32>; > > + pinctrl-single,function-mask = <0x00000007>; > > + }; > > From the vendor driver[2], it looks like this peripheral block only handles > pinmuxing, so indeed this looks like a good use of pinctrl-single. This is deffinitely pinctrl-single style pinmux controller ;) > > [2]: > https://github.com/milkv-duo/duo-buildroot-sdk/blob/develop/linux_5.10/drivers/pinctrl/cvitek/pinctrl-cv180x.h > > > + > > rst: reset-controller@3003000 { > > compatible = "sophgo,cv1800b-reset"; > > reg = <0x03003000 0x1000>; >
On Mon, Nov 13, 2023 at 09:03:11PM +0800, Jisheng Zhang wrote: > On Sun, Nov 12, 2023 at 08:51:20PM -0500, Samuel Holland wrote: > > On 2023-11-12 6:57 PM, Jisheng Zhang wrote: > > > Add the reset device tree node to cv1800b SoC reusing the > > ^^^^^ > > I assume you mean pinctrl here? > > oops copy and paste the commit msg ;) thanks > > > > > pinctrl-single driver. > > > > > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > > > --- > > > arch/riscv/boot/dts/sophgo/cv-pinctrl.h | 19 +++++++++++++++++++ > > > arch/riscv/boot/dts/sophgo/cv1800b.dtsi | 10 ++++++++++ > > > 2 files changed, 29 insertions(+) > > > create mode 100644 arch/riscv/boot/dts/sophgo/cv-pinctrl.h > > > > > > diff --git a/arch/riscv/boot/dts/sophgo/cv-pinctrl.h b/arch/riscv/boot/dts/sophgo/cv-pinctrl.h > > > new file mode 100644 > > > index 000000000000..ed78b6fb3142 > > > --- /dev/null > > > +++ b/arch/riscv/boot/dts/sophgo/cv-pinctrl.h > > > > A couple of questions: Should this go in include/dt-bindings? And is it worth > > When I cooked this series two weeks ago, I did put it in dt-binding, but > then I found commit fe49f2d776f799 ("arm64: dts: ti: Use local header for > pinctrl register values"), "These definitions were previously put in the > bindings header to avoid code duplication and to provide some context > meaning (name), but they do not fit the purpose of bindings." which is > suggested and acked by Krzysztof, so I just want to follow the style > here. > > > > including macros for the actual function mappings, like in the vendor source[1]? > > Do you want something as the following? > > #define UART0_TX 0 > #define CAM_MCLK1 1 > ... > > #define REG_UART0_TX 0x24 > ... > > pinctrl-single,pins = <REG_UART0_TX UART0_TX>; > > Other pinctl-single users just uses the register value directly, I have > no preference. But I'd like to get suggestions from DT and pinctl-single > maintainers. Hi Rob, Krzysztof, Conor, Tony, what's your opinion? Basically, if the definitions map directly to registers and are just used to make writing your devicetree easier then they do not belong in a binding. This differs from clock or reset indices, where we essentially make up a set of indices that may or may not correlate to offsets in the hardware as using the register values without any sort of abstraction is not defining an ABI. Cheers, Conor. > > > > > [1]: > > https://github.com/milkv-duo/duo-buildroot-sdk/blob/develop/linux_5.10/drivers/pinctrl/cvitek/cv180x_pinlist_swconfig.h > > > > > @@ -0,0 +1,19 @@ > > > +/* SPDX-License-Identifier: GPL-2.0 */ > > > +/* > > > + * This header provides constants for pinctrl bindings for Sophgo CV* SoC. > > > + * > > > + * Copyright (C) 2023 Jisheng Zhang <jszhang@kernel.org> > > > + */ > > > +#ifndef _DTS_RISCV_SOPHGO_CV_PINCTRL_H > > > +#define _DTS_RISCV_SOPHGO_CV_PINCTRL_H > > > + > > > +#define MUX_M0 0 > > > +#define MUX_M1 1 > > > +#define MUX_M2 2 > > > +#define MUX_M3 3 > > > +#define MUX_M4 4 > > > +#define MUX_M5 5 > > > +#define MUX_M6 6 > > > +#define MUX_M7 7 > > > + > > > +#endif > > > diff --git a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi > > > index e04df04a91c0..7a44d8e8672b 100644 > > > --- a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi > > > +++ b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi > > > @@ -6,6 +6,8 @@ > > > #include <dt-bindings/interrupt-controller/irq.h> > > > #include <dt-bindings/reset/sophgo,cv1800b-reset.h> > > > > > > +#include "cv-pinctrl.h" > > > + > > > / { > > > compatible = "sophgo,cv1800b"; > > > #address-cells = <1>; > > > @@ -55,6 +57,14 @@ soc { > > > dma-noncoherent; > > > ranges; > > > > > > + pinctrl0: pinctrl@3001000 { > > > + compatible = "pinctrl-single"; > > > + reg = <0x3001000 0x130>; > > > + #pinctrl-cells = <1>; > > > + pinctrl-single,register-width = <32>; > > > + pinctrl-single,function-mask = <0x00000007>; > > > + }; > > > > From the vendor driver[2], it looks like this peripheral block only handles > > pinmuxing, so indeed this looks like a good use of pinctrl-single. > > This is deffinitely pinctrl-single style pinmux controller ;) > > > > > [2]: > > https://github.com/milkv-duo/duo-buildroot-sdk/blob/develop/linux_5.10/drivers/pinctrl/cvitek/pinctrl-cv180x.h > > > > > + > > > rst: reset-controller@3003000 { > > > compatible = "sophgo,cv1800b-reset"; > > > reg = <0x03003000 0x1000>; > >
Hi Conor, On 2023-11-13 7:29 AM, Conor Dooley wrote: > On Mon, Nov 13, 2023 at 09:03:11PM +0800, Jisheng Zhang wrote: >> On Sun, Nov 12, 2023 at 08:51:20PM -0500, Samuel Holland wrote: >>> On 2023-11-12 6:57 PM, Jisheng Zhang wrote: >>>> Add the reset device tree node to cv1800b SoC reusing the >>> ^^^^^ >>> I assume you mean pinctrl here? >> >> oops copy and paste the commit msg ;) thanks >>> >>>> pinctrl-single driver. >>>> >>>> Signed-off-by: Jisheng Zhang <jszhang@kernel.org> >>>> --- >>>> arch/riscv/boot/dts/sophgo/cv-pinctrl.h | 19 +++++++++++++++++++ >>>> arch/riscv/boot/dts/sophgo/cv1800b.dtsi | 10 ++++++++++ >>>> 2 files changed, 29 insertions(+) >>>> create mode 100644 arch/riscv/boot/dts/sophgo/cv-pinctrl.h >>>> >>>> diff --git a/arch/riscv/boot/dts/sophgo/cv-pinctrl.h b/arch/riscv/boot/dts/sophgo/cv-pinctrl.h >>>> new file mode 100644 >>>> index 000000000000..ed78b6fb3142 >>>> --- /dev/null >>>> +++ b/arch/riscv/boot/dts/sophgo/cv-pinctrl.h >>> >>> A couple of questions: Should this go in include/dt-bindings? And is it worth >> >> When I cooked this series two weeks ago, I did put it in dt-binding, but >> then I found commit fe49f2d776f799 ("arm64: dts: ti: Use local header for >> pinctrl register values"), "These definitions were previously put in the >> bindings header to avoid code duplication and to provide some context >> meaning (name), but they do not fit the purpose of bindings." which is >> suggested and acked by Krzysztof, so I just want to follow the style >> here. >> >> >>> including macros for the actual function mappings, like in the vendor source[1]? >> >> Do you want something as the following? >> >> #define UART0_TX 0 >> #define CAM_MCLK1 1 >> ... >> >> #define REG_UART0_TX 0x24 >> ... >> >> pinctrl-single,pins = <REG_UART0_TX UART0_TX>; >> >> Other pinctl-single users just uses the register value directly, I have >> no preference. But I'd like to get suggestions from DT and pinctl-single >> maintainers. Hi Rob, Krzysztof, Conor, Tony, what's your opinion? > > Basically, if the definitions map directly to registers and are just > used to make writing your devicetree easier then they do not belong > in a binding. This differs from clock or reset indices, where we > essentially make up a set of indices that may or may not correlate to > offsets in the hardware as using the register values without any sort of > abstraction is not defining an ABI. Right. I should have remembered this policy :) Regards, Samuel
On 2023/11/13 8:57, Jisheng Zhang wrote: > Add the reset device tree node to cv1800b SoC reusing the > pinctrl-single driver. > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > --- > arch/riscv/boot/dts/sophgo/cv-pinctrl.h | 19 +++++++++++++++++++ > arch/riscv/boot/dts/sophgo/cv1800b.dtsi | 10 ++++++++++ > 2 files changed, 29 insertions(+) > create mode 100644 arch/riscv/boot/dts/sophgo/cv-pinctrl.h I'm afraid there will not be more cv* chip in coming days. So I would suggest you use "cv1800b-pinctrl.h" first here. If we see more cv* coming, and if they will reuse the definition here, we can consider optimize the filename, what do you think? BTW, how about defining the file name as "cv1800b.h" and I'm not sure if you will have more macro const definition for other modules? > diff --git a/arch/riscv/boot/dts/sophgo/cv-pinctrl.h b/arch/riscv/boot/dts/sophgo/cv-pinctrl.h > new file mode 100644 > index 000000000000..ed78b6fb3142 > --- /dev/null > +++ b/arch/riscv/boot/dts/sophgo/cv-pinctrl.h > @@ -0,0 +1,19 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * This header provides constants for pinctrl bindings for Sophgo CV* SoC. > + * > + * Copyright (C) 2023 Jisheng Zhang <jszhang@kernel.org> > + */ > +#ifndef _DTS_RISCV_SOPHGO_CV_PINCTRL_H > +#define _DTS_RISCV_SOPHGO_CV_PINCTRL_H > + > +#define MUX_M0 0 > +#define MUX_M1 1 > +#define MUX_M2 2 > +#define MUX_M3 3 > +#define MUX_M4 4 > +#define MUX_M5 5 > +#define MUX_M6 6 > +#define MUX_M7 7 > + > +#endif > diff --git a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi > index e04df04a91c0..7a44d8e8672b 100644 > --- a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi > +++ b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi > @@ -6,6 +6,8 @@ > #include <dt-bindings/interrupt-controller/irq.h> > #include <dt-bindings/reset/sophgo,cv1800b-reset.h> > > +#include "cv-pinctrl.h" > + > / { > compatible = "sophgo,cv1800b"; > #address-cells = <1>; > @@ -55,6 +57,14 @@ soc { > dma-noncoherent; > ranges; > > + pinctrl0: pinctrl@3001000 { > + compatible = "pinctrl-single"; > + reg = <0x3001000 0x130>; > + #pinctrl-cells = <1>; > + pinctrl-single,register-width = <32>; > + pinctrl-single,function-mask = <0x00000007>; > + }; > + > rst: reset-controller@3003000 { > compatible = "sophgo,cv1800b-reset"; > reg = <0x03003000 0x1000>;
On 2023/11/13 8:57, Jisheng Zhang wrote: > Add the reset device tree node to cv1800b SoC reusing the > pinctrl-single driver. > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > --- > arch/riscv/boot/dts/sophgo/cv-pinctrl.h | 19 +++++++++++++++++++ > arch/riscv/boot/dts/sophgo/cv1800b.dtsi | 10 ++++++++++ > 2 files changed, 29 insertions(+) > create mode 100644 arch/riscv/boot/dts/sophgo/cv-pinctrl.h > > diff --git a/arch/riscv/boot/dts/sophgo/cv-pinctrl.h b/arch/riscv/boot/dts/sophgo/cv-pinctrl.h > new file mode 100644 > index 000000000000..ed78b6fb3142 > --- /dev/null > +++ b/arch/riscv/boot/dts/sophgo/cv-pinctrl.h > @@ -0,0 +1,19 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * This header provides constants for pinctrl bindings for Sophgo CV* SoC. > + * > + * Copyright (C) 2023 Jisheng Zhang <jszhang@kernel.org> > + */ > +#ifndef _DTS_RISCV_SOPHGO_CV_PINCTRL_H > +#define _DTS_RISCV_SOPHGO_CV_PINCTRL_H > + > +#define MUX_M0 0 > +#define MUX_M1 1 > +#define MUX_M2 2 > +#define MUX_M3 3 > +#define MUX_M4 4 > +#define MUX_M5 5 > +#define MUX_M6 6 > +#define MUX_M7 7 > + > +#endif > diff --git a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi > index e04df04a91c0..7a44d8e8672b 100644 > --- a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi > +++ b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi > @@ -6,6 +6,8 @@ > #include <dt-bindings/interrupt-controller/irq.h> > #include <dt-bindings/reset/sophgo,cv1800b-reset.h> Another question: Does this patch depend on anther patchset "riscv: sophgo: add reset support for cv1800b"? If so, you may need to add comments in the cover-letter to clarify this. But I'm afraid it does not. > > +#include "cv-pinctrl.h" > + > / { > compatible = "sophgo,cv1800b"; > #address-cells = <1>; > @@ -55,6 +57,14 @@ soc { > dma-noncoherent; > ranges; > > + pinctrl0: pinctrl@3001000 { > + compatible = "pinctrl-single"; > + reg = <0x3001000 0x130>; > + #pinctrl-cells = <1>; > + pinctrl-single,register-width = <32>; > + pinctrl-single,function-mask = <0x00000007>; > + }; > + > rst: reset-controller@3003000 { > compatible = "sophgo,cv1800b-reset"; > reg = <0x03003000 0x1000>;
On Tue, Nov 14, 2023 at 09:38:47AM +0800, Chen Wang wrote: > > On 2023/11/13 8:57, Jisheng Zhang wrote: > > Add the reset device tree node to cv1800b SoC reusing the > > pinctrl-single driver. > > > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > > --- > > arch/riscv/boot/dts/sophgo/cv-pinctrl.h | 19 +++++++++++++++++++ > > arch/riscv/boot/dts/sophgo/cv1800b.dtsi | 10 ++++++++++ > > 2 files changed, 29 insertions(+) > > create mode 100644 arch/riscv/boot/dts/sophgo/cv-pinctrl.h > > > I'm afraid there will not be more cv* chip in coming days. So I would > suggest you use "cv1800b-pinctrl.h" first here. If we see more cv* coming, > and if they will reuse the definition here, we can consider optimize the > filename, what do you think? > > BTW, how about defining the file name as "cv1800b.h" and I'm not sure if you hmm, cv1800b-pinctrl.h is fine. Only pinctrl related stuff will be put there. > will have more macro const definition for other modules? > > > diff --git a/arch/riscv/boot/dts/sophgo/cv-pinctrl.h b/arch/riscv/boot/dts/sophgo/cv-pinctrl.h > > new file mode 100644 > > index 000000000000..ed78b6fb3142 > > --- /dev/null > > +++ b/arch/riscv/boot/dts/sophgo/cv-pinctrl.h > > @@ -0,0 +1,19 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +/* > > + * This header provides constants for pinctrl bindings for Sophgo CV* SoC. > > + * > > + * Copyright (C) 2023 Jisheng Zhang <jszhang@kernel.org> > > + */ > > +#ifndef _DTS_RISCV_SOPHGO_CV_PINCTRL_H > > +#define _DTS_RISCV_SOPHGO_CV_PINCTRL_H > > + > > +#define MUX_M0 0 > > +#define MUX_M1 1 > > +#define MUX_M2 2 > > +#define MUX_M3 3 > > +#define MUX_M4 4 > > +#define MUX_M5 5 > > +#define MUX_M6 6 > > +#define MUX_M7 7 > > + > > +#endif > > diff --git a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi > > index e04df04a91c0..7a44d8e8672b 100644 > > --- a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi > > +++ b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi > > @@ -6,6 +6,8 @@ > > #include <dt-bindings/interrupt-controller/irq.h> > > #include <dt-bindings/reset/sophgo,cv1800b-reset.h> > > +#include "cv-pinctrl.h" > > + > > / { > > compatible = "sophgo,cv1800b"; > > #address-cells = <1>; > > @@ -55,6 +57,14 @@ soc { > > dma-noncoherent; > > ranges; > > + pinctrl0: pinctrl@3001000 { > > + compatible = "pinctrl-single"; > > + reg = <0x3001000 0x130>; > > + #pinctrl-cells = <1>; > > + pinctrl-single,register-width = <32>; > > + pinctrl-single,function-mask = <0x00000007>; > > + }; > > + > > rst: reset-controller@3003000 { > > compatible = "sophgo,cv1800b-reset"; > > reg = <0x03003000 0x1000>;
On Mon, Nov 13, 2023 at 08:57:01AM +0800, Jisheng Zhang wrote: > Add the reset device tree node to cv1800b SoC reusing the > pinctrl-single driver. > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > --- > arch/riscv/boot/dts/sophgo/cv-pinctrl.h | 19 +++++++++++++++++++ > arch/riscv/boot/dts/sophgo/cv1800b.dtsi | 10 ++++++++++ > 2 files changed, 29 insertions(+) > create mode 100644 arch/riscv/boot/dts/sophgo/cv-pinctrl.h > > diff --git a/arch/riscv/boot/dts/sophgo/cv-pinctrl.h b/arch/riscv/boot/dts/sophgo/cv-pinctrl.h > new file mode 100644 > index 000000000000..ed78b6fb3142 > --- /dev/null > +++ b/arch/riscv/boot/dts/sophgo/cv-pinctrl.h > @@ -0,0 +1,19 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ Please match the licensing of the file(s) that include this. Not that there really anything > +/* > + * This header provides constants for pinctrl bindings for Sophgo CV* SoC. > + * > + * Copyright (C) 2023 Jisheng Zhang <jszhang@kernel.org> > + */ > +#ifndef _DTS_RISCV_SOPHGO_CV_PINCTRL_H > +#define _DTS_RISCV_SOPHGO_CV_PINCTRL_H > + > +#define MUX_M0 0 > +#define MUX_M1 1 > +#define MUX_M2 2 > +#define MUX_M3 3 > +#define MUX_M4 4 > +#define MUX_M5 5 > +#define MUX_M6 6 > +#define MUX_M7 7 I find defines with the number in the name to be somewhat pointless. > + > +#endif > diff --git a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi > index e04df04a91c0..7a44d8e8672b 100644 > --- a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi > +++ b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi > @@ -6,6 +6,8 @@ > #include <dt-bindings/interrupt-controller/irq.h> > #include <dt-bindings/reset/sophgo,cv1800b-reset.h> > > +#include "cv-pinctrl.h" > + > / { > compatible = "sophgo,cv1800b"; > #address-cells = <1>; > @@ -55,6 +57,14 @@ soc { > dma-noncoherent; > ranges; > > + pinctrl0: pinctrl@3001000 { > + compatible = "pinctrl-single"; > + reg = <0x3001000 0x130>; > + #pinctrl-cells = <1>; > + pinctrl-single,register-width = <32>; > + pinctrl-single,function-mask = <0x00000007>; > + }; Even more pointless is the defines are not even used. > + > rst: reset-controller@3003000 { > compatible = "sophgo,cv1800b-reset"; > reg = <0x03003000 0x1000>; > -- > 2.42.0 >
diff --git a/arch/riscv/boot/dts/sophgo/cv-pinctrl.h b/arch/riscv/boot/dts/sophgo/cv-pinctrl.h new file mode 100644 index 000000000000..ed78b6fb3142 --- /dev/null +++ b/arch/riscv/boot/dts/sophgo/cv-pinctrl.h @@ -0,0 +1,19 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * This header provides constants for pinctrl bindings for Sophgo CV* SoC. + * + * Copyright (C) 2023 Jisheng Zhang <jszhang@kernel.org> + */ +#ifndef _DTS_RISCV_SOPHGO_CV_PINCTRL_H +#define _DTS_RISCV_SOPHGO_CV_PINCTRL_H + +#define MUX_M0 0 +#define MUX_M1 1 +#define MUX_M2 2 +#define MUX_M3 3 +#define MUX_M4 4 +#define MUX_M5 5 +#define MUX_M6 6 +#define MUX_M7 7 + +#endif diff --git a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi index e04df04a91c0..7a44d8e8672b 100644 --- a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi +++ b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi @@ -6,6 +6,8 @@ #include <dt-bindings/interrupt-controller/irq.h> #include <dt-bindings/reset/sophgo,cv1800b-reset.h> +#include "cv-pinctrl.h" + / { compatible = "sophgo,cv1800b"; #address-cells = <1>; @@ -55,6 +57,14 @@ soc { dma-noncoherent; ranges; + pinctrl0: pinctrl@3001000 { + compatible = "pinctrl-single"; + reg = <0x3001000 0x130>; + #pinctrl-cells = <1>; + pinctrl-single,register-width = <32>; + pinctrl-single,function-mask = <0x00000007>; + }; + rst: reset-controller@3003000 { compatible = "sophgo,cv1800b-reset"; reg = <0x03003000 0x1000>;