[PATCH v2 17/18] dt-bindings: pci: rockchip,rk3399-pcie-ep: Add ep-gpios property

[Krzysztof Kozlowski]

On 02/04/2024 01:36, Damien Le Moal wrote:
> On 4/1/24 18:57, Krzysztof Kozlowski wrote:
>> On 01/04/2024 01:06, Damien Le Moal wrote:
>>> On 3/30/24 18:16, Krzysztof Kozlowski wrote:
>>>> On 30/03/2024 05:19, Damien Le Moal wrote:
>>>>> From: Wilfred Mallawa <wilfred.mallawa at wdc.com>
>>>>>
>>>>> Describe the `ep-gpios` property which is used to map the PERST# input
>>>>> signal for endpoint mode.
>>>>>
>>>>> Signed-off-by: Wilfred Mallawa <wilfred.mallawa at wdc.com>
>>>>> Signed-off-by: Damien Le Moal <dlemoal at kernel.org>
>>>>> ---
>>>>>  .../devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml       | 3 +++
>>>>>  1 file changed, 3 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml b/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml
>>>>> index 6b62f6f58efe..9331d44d6963 100644
>>>>> --- a/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml
>>>>> +++ b/Documentation/devicetree/bindings/pci/rockchip,rk3399-pcie-ep.yaml
>>>>> @@ -30,6 +30,9 @@ properties:
>>>>>      maximum: 32
>>>>>      default: 32
>>>>>  
>>>>> +  ep-gpios:
>>>>> +    description: Input GPIO configured for the PERST# signal.
>>>>
>>>> Missing maxItems. But more important: why existing property perst-gpios,
>>>> which you already have there in common schema, is not correct for this case?
>>>
>>> I am confused... Where do you find perst-gpios defined for the rk3399 ?
>>> Under Documentation/devicetree/bindings/pci/, the only schema I see using
>>> perst-gpios property are for the qcom (Qualcomm) controllers.
>>
>> You are right, it's so far only in Qualcomm.
>>
>>> The RC bindings for the rockchip rk3399 PCIe controller
>>> (pci/rockchip,rk3399-pcie.yaml) already define the ep-gpios property. So if
>>
>> Any reason why this cannot be named like GPIO? Is there already a user
>> of this in Linux kernel? Commit msg says nothing about this, so that's
>> why I would expect name matching the signal.
> 
> The RC-mode PCIe controller node of the rk3399 DTS already defines the ep-gpios
> property for RC side PERST# signal handling. So we simply reused the exact same
> name to be consistent between RC and EP. I personnally have no preferences. If
> there is an effort to rename such signal with some preferred pattern, I will
> follow. For the EP node, there was no PERST signal handling in the driver and
> no property defined for it, so any name is fine. "perst-gpios" would indeed be
> a better name, but again, given that the RC controller node has ep-gpios, we
> reused that. What is your recommendation here ?

Actually I don't know, perst and ep would work for me. If you do not
have code for this in the driver yet (nothing is shared between ep and
host), then maybe let's go with perst to match the actual name.

Anyway, you need maxItems. I sent a patch for the other binding:
https://lore.kernel.org/all/20240401100058.15749-1-krzysztof.kozlowski@linaro.org/

Best regards,
Krzysztof