Hi Krzysztof,
Thanks for the review.
>-----Original Message-----
>From: Krzysztof Kozlowski <krzk@xxxxxxxxxx>
>Sent: Friday, January 3, 2025 12:00 AM
>To: Visavalia, Rohit <rohit.visavalia@xxxxxxx>; mturquette@xxxxxxxxxxxx;
>sboyd@xxxxxxxxxx; robh@xxxxxxxxxx; krzk+dt@xxxxxxxxxx; conor+dt@xxxxxxxxxx
>Cc: linux-clk@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-
>kernel@xxxxxxxxxxxxxxx
>Subject: Re: [PATCH 3/3] dt-bindings: clock: xilinx: Update VCU bindings for reset
>GPIO
>
>On 02/01/2025 17:37, Rohit Visavalia wrote:
>> Updated VCU binding for reset GPIO pin as optional property.
>
>Subject and here: everything is an update. Be specific and drop all redundant
>things making this unnecessary long: add reset GPIO
Sure. I will take care in v2.
>
>> It is marked as optional as some of the ZynqMP designs are having
>> vcu_reset (reset pin of VCU IP) is driven by proc_sys_reset,
>> proc_sys_reset is another
>
>"are having is" looks like two verbs.
I will correct in v2 patch.
>
>I don't get here mainly why SoC has something driven by its own GPIO.
>That's unusual pattern.
VCU IP is in PL not part of PS.
>
>
>> PL IP driven by the PS pl_reset. So, here the VCU reset is not driven
>> by axi_gpio or PS GPIO so there will be no GPIO entry.
>
>Anyway, this has to be constrained per SoC.
As VCU IP is in PL, it depends on PL design creator how connection of VCU reset pin is done.
>
>>
>> Signed-off-by: Rohit Visavalia <rohit.visavalia@xxxxxxx>
>> ---
>> Documentation/devicetree/bindings/clock/xlnx,vcu.yaml | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/clock/xlnx,vcu.yaml
>> b/Documentation/devicetree/bindings/clock/xlnx,vcu.yaml
>> index bdb14594c40b..b3061309f8dd 100644
>> --- a/Documentation/devicetree/bindings/clock/xlnx,vcu.yaml
>> +++ b/Documentation/devicetree/bindings/clock/xlnx,vcu.yaml
>> @@ -36,6 +36,11 @@ properties:
>> - const: pll_ref
>> - const: aclk
>>
>> + reset-gpios:
>> + description: Optional GPIO used to reset the VCU, if available.
>> + Need use this
>
>Drop redundant parts. Not being part of required defines "optional"
>already. Don't repeat the schema but say something which we cannot deduce from
>this.
Got it.
>
>> + reset gpio when in design 'vcu_resetn' is driven by gpio.
>> + maxItems: 1
>> +
>> required:
>> - reg
>> - clocks
>> @@ -52,6 +57,7 @@ examples:
>> xlnx_vcu: vcu@a0040000 {
>> compatible = "xlnx,vcu-logicoreip-1.0";
>> reg = <0x0 0xa0040000 0x0 0x1000>;
>> + reset-gpios = <&gpio 0x4e GPIO_ACTIVE_HIGH>;
>
>GPIO numbers are not hex... unless this is not GPIO :/
Sure, I will update in v2 patch series.
>
>> clocks = <&si570_1>, <&clkc 71>;
>> clock-names = "pll_ref", "aclk";
>> };
>
>
>Best regards,
>Krzysztof
Thanks
Rohit
