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

[Changhuang Liang]

Hi, Conor

> On Wed, Sep 09, 2026 at 01:43:20AM +0000, Changhuang Liang wrote:
> > 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?
> 
> 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.

Understood. I'll define the ones that are actually used by the driver directly
in the corresponding domain.c files in the next version.

Best Regards,
Changhuang