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

[Krzysztof Kozlowski <krzk@kernel.org>]

On 30/08/2026 10:42, Changhuang Liang wrote:
> Hi, Krzysztof
> 
> Thanks for the review.
> 
>> On 30/08/2026 03:29, Changhuang Liang wrote:
>>> Hi, Krzysztof
>>>
>>> Thanks for the review.
>>>
>>>> On 29/08/2026 04:57, Changhuang Liang wrote:
>>>>> +#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
>>>>
>>>> I could not find how your driver uses these, so are you sure it's a
>>>> binding constant for the SW?
>>>
>>> Yes, these are not used in the driver, but in PATCH 6/8/10/12, etc.,
>>> some definitions for other GPIOs are still used. Therefore, I have
>>> uniformly defined them in the binding, and they will be provided to the
>> device tree for use later.
>>
>> I am confused. The driver in these patches 6/8/10/12 does include the header
>> but does not use them. Why do you include unused headers? Or what is
>> exactly the use?
> 
> I'm sorry, I missed one thing. Actually, the current patches 4/6/8/10/12/14/16/18/20 all use this:
> 
> For example, in patch 4:
> 
> static const struct jhb100_pinctrl_func_maps jhb100_func_maps_sys0[] = {
> 	{ .func = "auxpwrgood",		.val = 1 },
> 	{ .func = "gpio",		.val = 0,
> 	  .max_pin = JHB100_FUNC_MAPS_MAX_PIN(PADNUM_SYS0_GPIO_A2) },
> 	{ .func = "gpio",		.val = 1,
> 	  .max_pin = JHB100_FUNC_MAPS_MAX_PIN(PADNUM_SYS0_GPIO_A3) },
> 	{ .func = "hbled",		.val = 1 },
> 	{ .func = "pe2rst_out",		.val = 0 },
> };
> 
> These definitions are used in `struct jhb100_pinctrl_func_maps` or `struct pinvref_desc`.

OK, I was looking for PADNUM_SYS0_GPIO_A0.

Why do you need unused constants? The constants in binding headers are
interface between DTS and drivers, not hardware values. You do not need
to store there hardware values, so what is the benefit of defining
PADNUM_SYS0_GPIO_A0? Why can't PADNUM_SYS0_GPIO_A2 start at index 0?

Indices of pins are simply not bindings.

Best regards,
Krzysztof