Re: [PATCH v7 03/21] dt-bindings: pinctrl: Add starfive,jhb100-sys0-pinctrl

From: Conor Dooley <conor@kernel.org>
To: Changhuang Liang <changhuang.liang@starfivetech.com>
Cc: Linus Walleij <linusw@kernel.org>, Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Emil Renner Berthing <kernel@esmil.dk>,
	Paul Walmsley <pjw@kernel.org>, Albert Ou <aou@eecs.berkeley.edu>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Alexandre Ghiti <alex@ghiti.fr>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Bartosz Golaszewski <brgl@kernel.org>,
	linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org,
	devicetree@vger.kernel.org, linux-riscv@lists.infradead.org,
	Lianfeng Ouyang <lianfeng.ouyang@starfivetech.com>
Subject: Re: [PATCH v7 03/21] dt-bindings: pinctrl: Add starfive,jhb100-sys0-pinctrl
Date: Tue, 8 Sep 2026 20:57:39 +0100	[thread overview]
Message-ID: <20260908-annually-catfight-39e3a1543274@spud> (raw)
In-Reply-To: <20260831113514.66382-4-changhuang.liang@starfivetech.com>

[-- Attachment #1: Type: text/plain, Size: 2183 bytes --]

On Mon, Aug 31, 2026 at 04:34:56AM -0700, Changhuang Liang wrote:
> diff --git a/include/dt-bindings/pinctrl/starfive,jhb100-pinctrl.h b/include/dt-bindings/pinctrl/starfive,jhb100-pinctrl.h
> new file mode 100644
> index 000000000000..6d8f5516a178
> --- /dev/null
> +++ b/include/dt-bindings/pinctrl/starfive,jhb100-pinctrl.h
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: GPL-2.0 OR MIT */
> +/*
> + * Copyright (C) 2022 StarFive Technology Co., Ltd.
> + *
> + * Author: Changhuang Liang <changhuang.liang@starfivetech.com>
> + */
> +
> +#ifndef __DT_BINDINGS_PINCTRL_STARFIVE_JHB100_H__
> +#define __DT_BINDINGS_PINCTRL_STARFIVE_JHB100_H__
> +
> +/* sys0 pad numbers */
> +#define PADNUM_SYS0_GPIO_A0				0
> +#define PADNUM_SYS0_GPIO_A1				1
> +#define PADNUM_SYS0_GPIO_A2				2
> +#define PADNUM_SYS0_GPIO_A3				3

Krzysztof's point [1] about these still stands. Pad indices aren't bindings.
Sure, your driver and your dts both might want to use these but that
doesn't make them a binding. For that to be the case, they need to
effectively be made up numbers - like how clocks are often listed with
numbers from 0 into the dozens or hundreds, when that may or may not
correlate with actual bits in registers, e.g. indices 0-31 in a clock
binding might be in register 1 and then 32-63 are in register 2. There's
no need for a binding here to assign meanings to numbers, because the
meanings are assigned by the hardware itself - index 0 for SYS0 *is* A0,
because that's how the hardware is designed.

Were the numbers to run continuously, so that we had

#define JHB100_PADNUM_A0	0
#define JHB100_PADNUM_A1	1
#define JHB100_PADNUM_A2	2
#define JHB100_PADNUM_A3	3
#define JHB100_PADNUM_A4	4
and so on down to
#define JHB100_PADNUM__D0	1234

then it would be a binding, because we're assigning a meaning to 1234
that's not something determined by the hardware.

FWIW, I'm happy to have the unchanged starfive,jhb100-pinctrl.h sit in
arch/riscv/boot/dts/starfive, because the defines are helpful - but as
things stand I think Krzysztof is right.

Cheers,
Conor.

1 - https://lore.kernel.org/all/ZQ4PR01MB1202123864865439817E1549F2AA2@ZQ4PR01MB1202.CHNPR01.prod.partner.outlook.cn/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]