[PATCH v1 0/2] clk: zynqmp: Add versal-net compatible string

[PATCH v1 0/2] clk: zynqmp: Add versal-net compatible string

[Shubhrajyoti Datta]

Add a compatible string for versal-net.

Jay Buddhabhatti (2):
  dt-bindings: clock: versal: Add versal-net compatible string
  drivers: clk: zynqmp: Add versal-net compatible string

 .../devicetree/bindings/clock/xlnx,versal-clk.yaml         | 7 ++++++-
 drivers/clk/zynqmp/clkc.c                                  | 1 +
 2 files changed, 7 insertions(+), 1 deletion(-)

-- 
2.17.1

[PATCH v1 1/2] dt-bindings: clock: versal: Add versal-net compatible string

[Shubhrajyoti Datta]

From: Jay Buddhabhatti <jay.buddhabhatti@xilinx.com>

Add dt-binding documentation for Versal NET platforms.

Signed-off-by: Jay Buddhabhatti <jay.buddhabhatti@xilinx.com>
Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>
---

 .../devicetree/bindings/clock/xlnx,versal-clk.yaml         | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/clock/xlnx,versal-clk.yaml b/Documentation/devicetree/bindings/clock/xlnx,versal-clk.yaml
index 4bbf96484b56..ee1924acebaf 100644
--- a/Documentation/devicetree/bindings/clock/xlnx,versal-clk.yaml
+++ b/Documentation/devicetree/bindings/clock/xlnx,versal-clk.yaml
@@ -20,7 +20,12 @@ select: false
 
 properties:
   compatible:
-    const: xlnx,versal-clk
+    oneOf:
+      - description: For implementations complying for Versal.
+        const: xlnx,versal-clk
+
+      - description: For implementations complying for Versal NET.
+        const: xlnx,versal-net-clk
 
   "#clock-cells":
     const: 1
-- 
2.17.1
 

[PATCH v1 2/2] drivers: clk: zynqmp: Add versal-net compatible string

[Shubhrajyoti Datta]

From: Jay Buddhabhatti <jay.buddhabhatti@xilinx.com>

Add compatible string for versal-net.

Signed-off-by: Jay Buddhabhatti <jay.buddhabhatti@xilinx.com>
Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>
---

 drivers/clk/zynqmp/clkc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/zynqmp/clkc.c b/drivers/clk/zynqmp/clkc.c
index 5636ff1ce552..1ea5fba20d91 100644
--- a/drivers/clk/zynqmp/clkc.c
+++ b/drivers/clk/zynqmp/clkc.c
@@ -789,6 +789,7 @@ static int zynqmp_clock_probe(struct platform_device *pdev)
 static const struct of_device_id zynqmp_clock_of_match[] = {
 	{.compatible = "xlnx,zynqmp-clk"},
 	{.compatible = "xlnx,versal-clk"},
+	{.compatible = "xlnx,versal-net-clk"},
 	{},
 };
 MODULE_DEVICE_TABLE(of, zynqmp_clock_of_match);
-- 
2.17.1

Re: [PATCH v1 1/2] dt-bindings: clock: versal: Add versal-net compatible string

[Krzysztof Kozlowski]

On 30/03/2023 11:13, Shubhrajyoti Datta wrote:
> From: Jay Buddhabhatti <jay.buddhabhatti@xilinx.com>
> 
> Add dt-binding documentation for Versal NET platforms.
> 
> Signed-off-by: Jay Buddhabhatti <jay.buddhabhatti@xilinx.com>
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>
> ---
> 
>  .../devicetree/bindings/clock/xlnx,versal-clk.yaml         | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/xlnx,versal-clk.yaml b/Documentation/devicetree/bindings/clock/xlnx,versal-clk.yaml
> index 4bbf96484b56..ee1924acebaf 100644
> --- a/Documentation/devicetree/bindings/clock/xlnx,versal-clk.yaml
> +++ b/Documentation/devicetree/bindings/clock/xlnx,versal-clk.yaml
> @@ -20,7 +20,12 @@ select: false
>  
>  properties:
>    compatible:
> -    const: xlnx,versal-clk
> +    oneOf:


Just enum. Your descriptions are not saying anything more than
compatible is.

Best regards,
Krzysztof

Re: [PATCH v1 2/2] drivers: clk: zynqmp: Add versal-net compatible string

[Krzysztof Kozlowski]

On 30/03/2023 11:13, Shubhrajyoti Datta wrote:
> From: Jay Buddhabhatti <jay.buddhabhatti@xilinx.com>
> 
> Add compatible string for versal-net.
> 
> Signed-off-by: Jay Buddhabhatti <jay.buddhabhatti@xilinx.com>
> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>
> ---
> 
>  drivers/clk/zynqmp/clkc.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/clk/zynqmp/clkc.c b/drivers/clk/zynqmp/clkc.c
> index 5636ff1ce552..1ea5fba20d91 100644
> --- a/drivers/clk/zynqmp/clkc.c
> +++ b/drivers/clk/zynqmp/clkc.c
> @@ -789,6 +789,7 @@ static int zynqmp_clock_probe(struct platform_device *pdev)
>  static const struct of_device_id zynqmp_clock_of_match[] = {
>  	{.compatible = "xlnx,zynqmp-clk"},
>  	{.compatible = "xlnx,versal-clk"},
> +	{.compatible = "xlnx,versal-net-clk"},

Why no driver data? Why do you create new driver matchings if devices
are compatible?

Best regards,
Krzysztof

Re: [PATCH v1 2/2] drivers: clk: zynqmp: Add versal-net compatible string

[Michal Simek]

On 3/30/23 13:08, Krzysztof Kozlowski wrote:
> On 30/03/2023 11:13, Shubhrajyoti Datta wrote:
>> From: Jay Buddhabhatti <jay.buddhabhatti@xilinx.com>
>>
>> Add compatible string for versal-net.
>>
>> Signed-off-by: Jay Buddhabhatti <jay.buddhabhatti@xilinx.com>
>> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>
>> ---
>>
>>   drivers/clk/zynqmp/clkc.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/clk/zynqmp/clkc.c b/drivers/clk/zynqmp/clkc.c
>> index 5636ff1ce552..1ea5fba20d91 100644
>> --- a/drivers/clk/zynqmp/clkc.c
>> +++ b/drivers/clk/zynqmp/clkc.c
>> @@ -789,6 +789,7 @@ static int zynqmp_clock_probe(struct platform_device *pdev)
>>   static const struct of_device_id zynqmp_clock_of_match[] = {
>>   	{.compatible = "xlnx,zynqmp-clk"},
>>   	{.compatible = "xlnx,versal-clk"},
>> +	{.compatible = "xlnx,versal-net-clk"},
> 
> Why no driver data? Why do you create new driver matchings if devices
> are compatible?

It is the same discussion as we had recently in connection to emmc.
versal-net is new SOC and if any issue happens we would apply workaround based 
on DT compatible string. And by using special compatible string directly from 
beginning will allow us to do changes without change DT.

We actually don't need to update the driver for this. We can just simply list 
new dt binding and use both compatible string in DT like this.
"xlnx,versal-net-clk", "xlnx,versal-clk"

Thanks,
Michal

Re: [PATCH v1 2/2] drivers: clk: zynqmp: Add versal-net compatible string

[Krzysztof Kozlowski]

On 30/03/2023 14:00, Michal Simek wrote:
> 
> 
> On 3/30/23 13:08, Krzysztof Kozlowski wrote:
>> On 30/03/2023 11:13, Shubhrajyoti Datta wrote:
>>> From: Jay Buddhabhatti <jay.buddhabhatti@xilinx.com>
>>>
>>> Add compatible string for versal-net.
>>>
>>> Signed-off-by: Jay Buddhabhatti <jay.buddhabhatti@xilinx.com>
>>> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>
>>> ---
>>>
>>>   drivers/clk/zynqmp/clkc.c | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/clk/zynqmp/clkc.c b/drivers/clk/zynqmp/clkc.c
>>> index 5636ff1ce552..1ea5fba20d91 100644
>>> --- a/drivers/clk/zynqmp/clkc.c
>>> +++ b/drivers/clk/zynqmp/clkc.c
>>> @@ -789,6 +789,7 @@ static int zynqmp_clock_probe(struct platform_device *pdev)
>>>   static const struct of_device_id zynqmp_clock_of_match[] = {
>>>   	{.compatible = "xlnx,zynqmp-clk"},
>>>   	{.compatible = "xlnx,versal-clk"},
>>> +	{.compatible = "xlnx,versal-net-clk"},
>>
>> Why no driver data? Why do you create new driver matchings if devices
>> are compatible?
> 
> It is the same discussion as we had recently in connection to emmc.
> versal-net is new SOC and if any issue happens we would apply workaround based 
> on DT compatible string. And by using special compatible string directly from 
> beginning will allow us to do changes without change DT.

None of these arguments require growing match data. My comment was not
about bindings. My comment was why do you need to grow of_device_id
needlessly?

> 
> We actually don't need to update the driver for this. We can just simply list 
> new dt binding and use both compatible string in DT like this.
> "xlnx,versal-net-clk", "xlnx,versal-clk"


Best regards,
Krzysztof

Re: [PATCH v1 2/2] drivers: clk: zynqmp: Add versal-net compatible string

[Michal Simek]

On 3/30/23 15:32, Krzysztof Kozlowski wrote:
> On 30/03/2023 14:00, Michal Simek wrote:
>>
>>
>> On 3/30/23 13:08, Krzysztof Kozlowski wrote:
>>> On 30/03/2023 11:13, Shubhrajyoti Datta wrote:
>>>> From: Jay Buddhabhatti <jay.buddhabhatti@xilinx.com>
>>>>
>>>> Add compatible string for versal-net.
>>>>
>>>> Signed-off-by: Jay Buddhabhatti <jay.buddhabhatti@xilinx.com>
>>>> Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>
>>>> ---
>>>>
>>>>    drivers/clk/zynqmp/clkc.c | 1 +
>>>>    1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/drivers/clk/zynqmp/clkc.c b/drivers/clk/zynqmp/clkc.c
>>>> index 5636ff1ce552..1ea5fba20d91 100644
>>>> --- a/drivers/clk/zynqmp/clkc.c
>>>> +++ b/drivers/clk/zynqmp/clkc.c
>>>> @@ -789,6 +789,7 @@ static int zynqmp_clock_probe(struct platform_device *pdev)
>>>>    static const struct of_device_id zynqmp_clock_of_match[] = {
>>>>    	{.compatible = "xlnx,zynqmp-clk"},
>>>>    	{.compatible = "xlnx,versal-clk"},
>>>> +	{.compatible = "xlnx,versal-net-clk"},
>>>
>>> Why no driver data? Why do you create new driver matchings if devices
>>> are compatible?
>>
>> It is the same discussion as we had recently in connection to emmc.
>> versal-net is new SOC and if any issue happens we would apply workaround based
>> on DT compatible string. And by using special compatible string directly from
>> beginning will allow us to do changes without change DT.
> 
> None of these arguments require growing match data. My comment was not
> about bindings. My comment was why do you need to grow of_device_id
> needlessly?

Right.
Shubhrajyoti: Please update only dt binding and for versal-net we will use
new compatible string followed by old one.

Thanks,
Michal