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

[Krzysztof Kozlowski <krzk@kernel.org>]

On 30/08/2026 10:57, Changhuang Liang wrote:
> Hi, Krzysztof
> 
> Thanks for the review.
> 
>> 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?
> 
> Here, it is also provided for use by the device tree later. For example:
> 
> 	auxpowergood0_pins: auxpowergood0-grp {
> 		auxpowergood0-pins {
> 			pins = <PADNUM_SYS0_GPIO_A0>;
> 			function = "auxpwrgood";
> 			input-enable;
> 		};
> 	};
> 
> 	auxpowergood1_pins: auxpowergood1-grp {
> 		auxpowergood1-pins {
> 			pins = <PADNUM_SYS0_GPIO_A1>;
> 			function = "auxpwrgood";
> 			input-enable;
> 		};
> 	};
> 
> The pins property corresponds to the specific GPIO offset, so PADNUM_SYS0_GPIO_A2 cannot start at index 0.

Did you even read my reply? Or you just respond whatever to tick the
checkbox of responding to maintainer?

AGAIN:
"You do not need to store there hardware values"

Do you understand the term ABI between software and DTS? Is in this term
anything about your actual hardware numbers? No.

Best regards,
Krzysztof