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

On Wed, Sep 09, 2026 at 01:43:20AM +0000, Changhuang Liang wrote:

Hi, Conor

Thanks for the review.

quoted

On Mon, Aug 31, 2026 at 04:34:56AM -0700, Changhuang Liang wrote:

quoted

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.

For the current series, the drivers also use some definitions from the binding. In 
the next version, should the macro definitions be directly placed in their respective 
pinctrl-starfive-jhb100-<domain>.c files?

Only the ones that are actually used, which I think is a limited subset
of the definitions in this header. At least, none of the ones that I
looked up were actually used in the drivers, except for the sys0 ones.