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

[Changhuang Liang]

Hi, Conor

Thanks for the review.

> 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@xxxxxxxxxxxxxxxx>
> > + */
> > +
> > +#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?

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

Best Regards,
Changhuang