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

[Damien Le Moal]

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 ?

> 
> Best regards,
> Krzysztof
> 

-- 
Damien Le Moal
Western Digital Research