2023-03-24 07:40:30

by Potthuri, Sai Krishna

[permalink] [raw]
Subject: [PATCH 1/2] dt-bindings: mmc: arasan,sdci: Add Xilinx Versal Net compatible

Add Xilinx Versal Net compatible to support eMMC 5.1 PHY.

Signed-off-by: Sai Krishna Potthuri <[email protected]>
---
Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml b/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml
index 8296c34cfa00..cf44a4b988a7 100644
--- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml
+++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml
@@ -27,6 +27,7 @@ allOf:
enum:
- xlnx,zynqmp-8.9a
- xlnx,versal-8.9a
+ - xlnx,versal-net-5.1-emmc
then:
properties:
clock-output-names:
@@ -62,6 +63,11 @@ properties:
description:
For this device it is strongly suggested to include
clock-output-names and '#clock-cells'.
+ - items:
+ - const: xlnx,versal-net-5.1-emmc # Versal Net eMMC PHY
+ description:
+ For this device it is strongly suggested to include
+ clock-output-names and '#clock-cells'.
- items:
- const: intel,lgm-sdhci-5.1-emmc # Intel LGM eMMC PHY
- const: arasan,sdhci-5.1
--
2.25.1


2023-03-24 11:45:14

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: mmc: arasan,sdci: Add Xilinx Versal Net compatible

On 24/03/2023 08:36, Sai Krishna Potthuri wrote:
> Add Xilinx Versal Net compatible to support eMMC 5.1 PHY.
>
> Signed-off-by: Sai Krishna Potthuri <[email protected]>
> ---
> Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml b/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml
> index 8296c34cfa00..cf44a4b988a7 100644
> --- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml
> +++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml
> @@ -27,6 +27,7 @@ allOf:
> enum:
> - xlnx,zynqmp-8.9a
> - xlnx,versal-8.9a
> + - xlnx,versal-net-5.1-emmc

v5.1 is eMMC standard or Versal block version? If the first, it's not
suitable for compatibles.

Also, what's the difference from xlnx,versal-8.9a?

> then:
> properties:
> clock-output-names:
> @@ -62,6 +63,11 @@ properties:
> description:
> For this device it is strongly suggested to include
> clock-output-names and '#clock-cells'.
> + - items:
> + - const: xlnx,versal-net-5.1-emmc # Versal Net eMMC PHY
> + description:

Best regards,
Krzysztof

2023-03-27 10:12:03

by Potthuri, Sai Krishna

[permalink] [raw]
Subject: RE: [PATCH 1/2] dt-bindings: mmc: arasan,sdci: Add Xilinx Versal Net compatible

Hi Krzysztof,

> -----Original Message-----
> From: Krzysztof Kozlowski <[email protected]>
> Sent: Friday, March 24, 2023 5:14 PM
> To: Potthuri, Sai Krishna <[email protected]>; Ulf Hansson
> <[email protected]>; Rob Herring <[email protected]>; Krzysztof
> Kozlowski <[email protected]>; Michal Simek
> <[email protected]>; Adrian Hunter <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; git (AMD-
> Xilinx) <[email protected]>; [email protected]
> Subject: Re: [PATCH 1/2] dt-bindings: mmc: arasan,sdci: Add Xilinx Versal Net
> compatible
>
> On 24/03/2023 08:36, Sai Krishna Potthuri wrote:
> > Add Xilinx Versal Net compatible to support eMMC 5.1 PHY.
> >
> > Signed-off-by: Sai Krishna Potthuri <[email protected]>
> > ---
> > Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml
> > b/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml
> > index 8296c34cfa00..cf44a4b988a7 100644
> > --- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml
> > +++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml
> > @@ -27,6 +27,7 @@ allOf:
> > enum:
> > - xlnx,zynqmp-8.9a
> > - xlnx,versal-8.9a
> > + - xlnx,versal-net-5.1-emmc
>
> v5.1 is eMMC standard or Versal block version? If the first, it's not suitable for
> compatibles.
>
> Also, what's the difference from xlnx,versal-8.9a?
V5.1 is an eMMC standard and this compatible is defined based on sdhci arasan
eMMC5.1 Host Controller(arasan,sdhci-5.1), where as in Versal, it’s a different
controller and it is based on 4.51 Host Controller(arasan,sdhci-8.9a).
Versal Net Compatible is defined it this way to make it inline with the other
existing SoC compatibles like "intel,keembay-sdhci-5.1-emmc".
Please suggest if the compatible need to be renamed to "xlnx,versal-net-emmc"?

Regards
Sai Krishna

>
> > then:
> > properties:
> > clock-output-names:
> > @@ -62,6 +63,11 @@ properties:
> > description:
> > For this device it is strongly suggested to include
> > clock-output-names and '#clock-cells'.
> > + - items:
> > + - const: xlnx,versal-net-5.1-emmc # Versal Net eMMC PHY
> > + description:
>
> Best regards,
> Krzysztof

2023-03-28 07:16:36

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: mmc: arasan,sdci: Add Xilinx Versal Net compatible

On 27/03/2023 11:58, Potthuri, Sai Krishna wrote:
> Hi Krzysztof,
>
>> -----Original Message-----
>> From: Krzysztof Kozlowski <[email protected]>
>> Sent: Friday, March 24, 2023 5:14 PM
>> To: Potthuri, Sai Krishna <[email protected]>; Ulf Hansson
>> <[email protected]>; Rob Herring <[email protected]>; Krzysztof
>> Kozlowski <[email protected]>; Michal Simek
>> <[email protected]>; Adrian Hunter <[email protected]>
>> Cc: [email protected]; [email protected];
>> [email protected]; [email protected]; git (AMD-
>> Xilinx) <[email protected]>; [email protected]
>> Subject: Re: [PATCH 1/2] dt-bindings: mmc: arasan,sdci: Add Xilinx Versal Net
>> compatible
>>
>> On 24/03/2023 08:36, Sai Krishna Potthuri wrote:
>>> Add Xilinx Versal Net compatible to support eMMC 5.1 PHY.
>>>
>>> Signed-off-by: Sai Krishna Potthuri <[email protected]>
>>> ---
>>> Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml | 6 ++++++
>>> 1 file changed, 6 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml
>>> b/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml
>>> index 8296c34cfa00..cf44a4b988a7 100644
>>> --- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml
>>> +++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml
>>> @@ -27,6 +27,7 @@ allOf:
>>> enum:
>>> - xlnx,zynqmp-8.9a
>>> - xlnx,versal-8.9a
>>> + - xlnx,versal-net-5.1-emmc
>>
>> v5.1 is eMMC standard or Versal block version? If the first, it's not suitable for
>> compatibles.
>>
>> Also, what's the difference from xlnx,versal-8.9a?
> V5.1 is an eMMC standard and this compatible is defined based on sdhci arasan
> eMMC5.1 Host Controller(arasan,sdhci-5.1), where as in Versal, it’s a different
> controller and it is based on 4.51 Host Controller(arasan,sdhci-8.9a).

Mixing IP block versions and eMMC spec versions in one binding is a
great way to confuse.

> Versal Net Compatible is defined it this way to make it inline with the other
> existing SoC compatibles like "intel,keembay-sdhci-5.1-emmc".
> Please suggest if the compatible need to be renamed to "xlnx,versal-net-emmc"?

Is Versal Net uniquely identifying your SoC or IP block?

>
> Regards
> Sai Krishna
>
>>
>>> then:
>>> properties:
>>> clock-output-names:
>>> @@ -62,6 +63,11 @@ properties:
>>> description:
>>> For this device it is strongly suggested to include
>>> clock-output-names and '#clock-cells'.
>>> + - items:
>>> + - const: xlnx,versal-net-5.1-emmc # Versal Net eMMC PHY

You do not have items here, so move it to respective place at beginning,
just like others are defined.


Best regards,
Krzysztof

2023-03-28 07:34:24

by Michal Simek

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: mmc: arasan,sdci: Add Xilinx Versal Net compatible



On 3/28/23 09:14, Krzysztof Kozlowski wrote:
> On 27/03/2023 11:58, Potthuri, Sai Krishna wrote:
>> Hi Krzysztof,
>>
>>> -----Original Message-----
>>> From: Krzysztof Kozlowski <[email protected]>
>>> Sent: Friday, March 24, 2023 5:14 PM
>>> To: Potthuri, Sai Krishna <[email protected]>; Ulf Hansson
>>> <[email protected]>; Rob Herring <[email protected]>; Krzysztof
>>> Kozlowski <[email protected]>; Michal Simek
>>> <[email protected]>; Adrian Hunter <[email protected]>
>>> Cc: [email protected]; [email protected];
>>> [email protected]; [email protected]; git (AMD-
>>> Xilinx) <[email protected]>; [email protected]
>>> Subject: Re: [PATCH 1/2] dt-bindings: mmc: arasan,sdci: Add Xilinx Versal Net
>>> compatible
>>>
>>> On 24/03/2023 08:36, Sai Krishna Potthuri wrote:
>>>> Add Xilinx Versal Net compatible to support eMMC 5.1 PHY.
>>>>
>>>> Signed-off-by: Sai Krishna Potthuri <[email protected]>
>>>> ---
>>>> Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml | 6 ++++++
>>>> 1 file changed, 6 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml
>>>> b/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml
>>>> index 8296c34cfa00..cf44a4b988a7 100644
>>>> --- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml
>>>> +++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml
>>>> @@ -27,6 +27,7 @@ allOf:
>>>> enum:
>>>> - xlnx,zynqmp-8.9a
>>>> - xlnx,versal-8.9a
>>>> + - xlnx,versal-net-5.1-emmc
>>>
>>> v5.1 is eMMC standard or Versal block version? If the first, it's not suitable for
>>> compatibles.
>>>
>>> Also, what's the difference from xlnx,versal-8.9a?
>> V5.1 is an eMMC standard and this compatible is defined based on sdhci arasan
>> eMMC5.1 Host Controller(arasan,sdhci-5.1), where as in Versal, it’s a different
>> controller and it is based on 4.51 Host Controller(arasan,sdhci-8.9a).
>
> Mixing IP block versions and eMMC spec versions in one binding is a
> great way to confuse.

What do you suggest then?

>
>> Versal Net Compatible is defined it this way to make it inline with the other
>> existing SoC compatibles like "intel,keembay-sdhci-5.1-emmc".
>> Please suggest if the compatible need to be renamed to "xlnx,versal-net-emmc"?
>
> Is Versal Net uniquely identifying your SoC or IP block?

Yes. versal-net is unique identifier for specific silicon with fixed set if IPs.

Can you please refresh my mind if we can introduce specific compatible strings
for this SOC or should we used existing one if functionality is the same with
previous SOC family?
There could be currently unknown issues related to SOC wiring out of specific IP
version.

Thanks,
Michal

2023-03-28 07:39:16

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 1/2] dt-bindings: mmc: arasan,sdci: Add Xilinx Versal Net compatible

On 28/03/2023 09:31, Michal Simek wrote:
>
>
> On 3/28/23 09:14, Krzysztof Kozlowski wrote:
>> On 27/03/2023 11:58, Potthuri, Sai Krishna wrote:
>>> Hi Krzysztof,
>>>
>>>> -----Original Message-----
>>>> From: Krzysztof Kozlowski <[email protected]>
>>>> Sent: Friday, March 24, 2023 5:14 PM
>>>> To: Potthuri, Sai Krishna <[email protected]>; Ulf Hansson
>>>> <[email protected]>; Rob Herring <[email protected]>; Krzysztof
>>>> Kozlowski <[email protected]>; Michal Simek
>>>> <[email protected]>; Adrian Hunter <[email protected]>
>>>> Cc: [email protected]; [email protected];
>>>> [email protected]; [email protected]; git (AMD-
>>>> Xilinx) <[email protected]>; [email protected]
>>>> Subject: Re: [PATCH 1/2] dt-bindings: mmc: arasan,sdci: Add Xilinx Versal Net
>>>> compatible
>>>>
>>>> On 24/03/2023 08:36, Sai Krishna Potthuri wrote:
>>>>> Add Xilinx Versal Net compatible to support eMMC 5.1 PHY.
>>>>>
>>>>> Signed-off-by: Sai Krishna Potthuri <[email protected]>
>>>>> ---
>>>>> Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml | 6 ++++++
>>>>> 1 file changed, 6 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml
>>>>> b/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml
>>>>> index 8296c34cfa00..cf44a4b988a7 100644
>>>>> --- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml
>>>>> +++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml
>>>>> @@ -27,6 +27,7 @@ allOf:
>>>>> enum:
>>>>> - xlnx,zynqmp-8.9a
>>>>> - xlnx,versal-8.9a
>>>>> + - xlnx,versal-net-5.1-emmc
>>>>
>>>> v5.1 is eMMC standard or Versal block version? If the first, it's not suitable for
>>>> compatibles.
>>>>
>>>> Also, what's the difference from xlnx,versal-8.9a?
>>> V5.1 is an eMMC standard and this compatible is defined based on sdhci arasan
>>> eMMC5.1 Host Controller(arasan,sdhci-5.1), where as in Versal, it’s a different
>>> controller and it is based on 4.51 Host Controller(arasan,sdhci-8.9a).
>>
>> Mixing IP block versions and eMMC spec versions in one binding is a
>> great way to confuse.
>
> What do you suggest then?

Stick to IP block versions or code names. The eMMC spec version would
only make sense if you had such possibility:

xlnx,versal-net-emmc-5.0
xlnx,versal-net-emmc-5.1
xlnx,versal-net-emmc-x.y

So exactly one device with different blocks inside. This is very
uncommon, but there such SoC (SunPlus IIRC).

>
>>
>>> Versal Net Compatible is defined it this way to make it inline with the other
>>> existing SoC compatibles like "intel,keembay-sdhci-5.1-emmc".
>>> Please suggest if the compatible need to be renamed to "xlnx,versal-net-emmc"?
>>
>> Is Versal Net uniquely identifying your SoC or IP block?
>
> Yes. versal-net is unique identifier for specific silicon with fixed set if IPs.

Then I suggest xlnx,versal-net-emmc.

> Can you please refresh my mind if we can introduce specific compatible strings
> for this SOC or should we used existing one if functionality is the same with
> previous SOC family?

It's regular case and recommendation is always (for every SoC) the same:
https://elixir.bootlin.com/linux/v6.1-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L42

You should add new SoC specific compatible followed by existing one
(fallback).

> There could be currently unknown issues related to SOC wiring out of specific IP
> version.

Best regards,
Krzysztof