[IP REVIEW PATCH 0/3] Add support for Lane Margining at Receiver

[IP REVIEW PATCH 0/3] Add support for Lane Margining at Receiver

[Manikanta Maddireddy]

Tegra234 supports Lane Margining at Receiver feature. However, this
requires programming of per lane PIPE2UPHY hardware instance to relay
the lane margin control and margin status information between PCIe
controller and UPHY modules. This series adds this support in PIPE2UPHY
driver.

Manikanta Maddireddy (3):
  phy: tegra: p2u: Add lane margin support
  dts: soc: t234: Add uphy lane number and intr in p2u nodes
  dt-bindings: PHY: P2U: Add PCIe lane margining support

 .../bindings/phy/phy-tegra194-p2u.yaml        |  50 ++++
 arch/arm64/boot/dts/nvidia/tegra234.dtsi      | 120 ++++++++
 drivers/phy/tegra/phy-tegra194-p2u.c          | 274 ++++++++++++++++++
 3 files changed, 444 insertions(+)

-- 
2.25.1

[PATCH 1/3] dt-bindings: PHY: P2U: Add PCIe lane margining support

[Manikanta Maddireddy]

Tegra234 supports PCIe lane margining. P2U HW acts as a relay to exchange
margin control data and margin status between PCIe controller and UPHY.

Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
---
 .../bindings/phy/phy-tegra194-p2u.yaml        | 50 +++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/phy-tegra194-p2u.yaml b/Documentation/devicetree/bindings/phy/phy-tegra194-p2u.yaml
index 4dc5205d893b..0ba3f6a0b474 100644
--- a/Documentation/devicetree/bindings/phy/phy-tegra194-p2u.yaml
+++ b/Documentation/devicetree/bindings/phy/phy-tegra194-p2u.yaml
@@ -40,6 +40,51 @@ properties:
   '#phy-cells':
     const: 0
 
+  interrupts:
+    items:
+      description: P2U interrupt for Gen4 lane margining functionality.
+
+  interrupt-names:
+    items:
+      - const: intr
+
+  nvidia,bpmp:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description: Must contain a pair of phandles to BPMP controller node followed by P2U ID.
+    items:
+      - items:
+          - description: phandle to BPMP controller node
+          - description: P2U instance ID
+            maximum: 24
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - nvidia,tegra194-p2u
+    then:
+      required:
+        - reg
+        - reg-names
+        - '#phy-cells'
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - nvidia,tegra234-p2u
+    then:
+      required:
+        - reg
+        - reg-names
+        - '#phy-cells'
+        - interrupts
+        - interrupt-names
+        - nvidia,bpmp
+
 additionalProperties: false
 
 examples:
@@ -49,5 +94,10 @@ examples:
         reg = <0x03e10000 0x10000>;
         reg-names = "ctl";
 
+        interrupts = <0 337 0x04>;
+        interrupt-names = "intr";
+
+        nvidia,bpmp = <&bpmp 1>;
+
         #phy-cells = <0>;
     };
-- 
2.25.1

Re: [PATCH 1/3] dt-bindings: PHY: P2U: Add PCIe lane margining support

[Krzysztof Kozlowski]

On 24/11/2022 09:35, Manikanta Maddireddy wrote:
> Tegra234 supports PCIe lane margining. P2U HW acts as a relay to exchange

typo: merging?

> margin control data and margin status between PCIe controller and UPHY.

Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC.  It might happen, that command when run on an older
kernel, gives you outdated entries.  Therefore please be sure you base
your patches on recent Linux kernel.


> 
> Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
> ---
>  .../bindings/phy/phy-tegra194-p2u.yaml        | 50 +++++++++++++++++++
>  1 file changed, 50 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/phy/phy-tegra194-p2u.yaml b/Documentation/devicetree/bindings/phy/phy-tegra194-p2u.yaml
> index 4dc5205d893b..0ba3f6a0b474 100644
> --- a/Documentation/devicetree/bindings/phy/phy-tegra194-p2u.yaml
> +++ b/Documentation/devicetree/bindings/phy/phy-tegra194-p2u.yaml
> @@ -40,6 +40,51 @@ properties:
>    '#phy-cells':
>      const: 0
>  
> +  interrupts:
> +    items:
> +      description: P2U interrupt for Gen4 lane margining functionality.

typo: merging?

> +
> +  interrupt-names:
> +    items:
> +      - const: intr

Drop entire property, not really useful.

> +
> +  nvidia,bpmp:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    description: Must contain a pair of phandles to BPMP controller node followed by P2U ID.
> +    items:
> +      - items:
> +          - description: phandle to BPMP controller node
> +          - description: P2U instance ID
> +            maximum: 24
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - nvidia,tegra194-p2u
> +    then:
> +      required:
> +        - reg
> +        - reg-names
> +        - '#phy-cells'

That's not how it should be done. You have only two variants here, so
add a "required:" block with above and only one if:then: clause for
interrupts and nvidia,bpmp.

Requiring reg/reg-names/phy-cells should be in separate patch with its
own reasoning.


Best regards,
Krzysztof

Re: [PATCH 1/3] dt-bindings: PHY: P2U: Add PCIe lane margining support

[Manikanta Maddireddy]

Thank you for quick review. I will wait for other reviewers to review 
patch 2 & 3.
I will address all review comments and sendnew revision.

On 11/24/2022 2:15 PM, Krzysztof Kozlowski wrote:
> External email: Use caution opening links or attachments
>
>
> On 24/11/2022 09:35, Manikanta Maddireddy wrote:
>> Tegra234 supports PCIe lane margining. P2U HW acts as a relay to exchange
> typo: merging?
It is not typo, it is PCIe feature lane margining.
https://pcisig.com/pushing-limits-understanding-lane-margining-pcie%C2%AE
>
>> margin control data and margin status between PCIe controller and UPHY.
> Please use scripts/get_maintainers.pl to get a list of necessary people
> and lists to CC.  It might happen, that command when run on an older
> kernel, gives you outdated entries.  Therefore please be sure you base
> your patches on recent Linux kernel.
I verified these patches on 6.0.0-rc6 kernel and executed get_maintainers.pl
script on it. Did I miss anyone here?
>
>
>> Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
>> ---
>>   .../bindings/phy/phy-tegra194-p2u.yaml        | 50 +++++++++++++++++++
>>   1 file changed, 50 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/phy-tegra194-p2u.yaml b/Documentation/devicetree/bindings/phy/phy-tegra194-p2u.yaml
>> index 4dc5205d893b..0ba3f6a0b474 100644
>> --- a/Documentation/devicetree/bindings/phy/phy-tegra194-p2u.yaml
>> +++ b/Documentation/devicetree/bindings/phy/phy-tegra194-p2u.yaml
>> @@ -40,6 +40,51 @@ properties:
>>     '#phy-cells':
>>       const: 0
>>
>> +  interrupts:
>> +    items:
>> +      description: P2U interrupt for Gen4 lane margining functionality.
> typo: merging?
It is not typo, it is PCIe feature lane margining.
https://pcisig.com/pushing-limits-understanding-lane-margining-pcie%C2%AE
>
>> +
>> +  interrupt-names:
>> +    items:
>> +      - const: intr
> Drop entire property, not really useful.
In driver, I am using platform_get_irq_byname(), I will change it to 
platform_get_irq()
and drop this property.
>
>> +
>> +  nvidia,bpmp:
>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>> +    description: Must contain a pair of phandles to BPMP controller node followed by P2U ID.
>> +    items:
>> +      - items:
>> +          - description: phandle to BPMP controller node
>> +          - description: P2U instance ID
>> +            maximum: 24
>> +
>> +allOf:
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - nvidia,tegra194-p2u
>> +    then:
>> +      required:
>> +        - reg
>> +        - reg-names
>> +        - '#phy-cells'
> That's not how it should be done. You have only two variants here, so
> add a "required:" block with above and only one if:then: clause for
> interrupts and nvidia,bpmp.
>
> Requiring reg/reg-names/phy-cells should be in separate patch with its
> own reasoning.
Ok, I will create two separate patches and add if:then clause only for 
tegra234.
As per understanding final change will look like below, right?


required:
   - reg
   - reg-names
   - '#phy-cells'

allOf:
   - if:
       properties:
         compatible:
           contains:
             enum:
               - nvidia,tegra234-p2u
     then:
       required:
         - interrupts
         - nvidia,bpmp

>
>
> Best regards,
> Krzysztof
>

Re: [PATCH 1/3] dt-bindings: PHY: P2U: Add PCIe lane margining support

[Krzysztof Kozlowski]

On 30/11/2022 16:11, Manikanta Maddireddy wrote:
> Thank you for quick review. I will wait for other reviewers to review 
> patch 2 & 3.
> I will address all review comments and sendnew revision.
> 
> On 11/24/2022 2:15 PM, Krzysztof Kozlowski wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> On 24/11/2022 09:35, Manikanta Maddireddy wrote:
>>> Tegra234 supports PCIe lane margining. P2U HW acts as a relay to exchange
>> typo: merging?
> It is not typo, it is PCIe feature lane margining.
> https://pcisig.com/pushing-limits-understanding-lane-margining-pcie%C2%AE
>>
>>> margin control data and margin status between PCIe controller and UPHY.
>> Please use scripts/get_maintainers.pl to get a list of necessary people
>> and lists to CC.  It might happen, that command when run on an older
>> kernel, gives you outdated entries.  Therefore please be sure you base
>> your patches on recent Linux kernel.
> I verified these patches on 6.0.0-rc6 kernel and executed get_maintainers.pl
> script on it. Did I miss anyone here?

Yes. At least Rob, maybe more. You need to CC all
maintainers/reviewers/supporters and all mailing lists.

It's not my task to verify each of these addresses to check whether you
really missed someone or not. I spotted at least one missing address so
just run get_maintainers.pl and use all entries from there.


>>
>>
>>> Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
>>> ---
>>>   .../bindings/phy/phy-tegra194-p2u.yaml        | 50 +++++++++++++++++++
>>>   1 file changed, 50 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/phy/phy-tegra194-p2u.yaml b/Documentation/devicetree/bindings/phy/phy-tegra194-p2u.yaml
>>> index 4dc5205d893b..0ba3f6a0b474 100644
>>> --- a/Documentation/devicetree/bindings/phy/phy-tegra194-p2u.yaml
>>> +++ b/Documentation/devicetree/bindings/phy/phy-tegra194-p2u.yaml
>>> @@ -40,6 +40,51 @@ properties:
>>>     '#phy-cells':
>>>       const: 0
>>>
>>> +  interrupts:
>>> +    items:
>>> +      description: P2U interrupt for Gen4 lane margining functionality.
>> typo: merging?
> It is not typo, it is PCIe feature lane margining.
> https://pcisig.com/pushing-limits-understanding-lane-margining-pcie%C2%AE
>>
>>> +
>>> +  interrupt-names:
>>> +    items:
>>> +      - const: intr
>> Drop entire property, not really useful.
> In driver, I am using platform_get_irq_byname(), I will change it to 
> platform_get_irq()
> and drop this property.
>>
>>> +
>>> +  nvidia,bpmp:
>>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>>> +    description: Must contain a pair of phandles to BPMP controller node followed by P2U ID.
>>> +    items:
>>> +      - items:
>>> +          - description: phandle to BPMP controller node
>>> +          - description: P2U instance ID
>>> +            maximum: 24
>>> +
>>> +allOf:
>>> +  - if:
>>> +      properties:
>>> +        compatible:
>>> +          contains:
>>> +            enum:
>>> +              - nvidia,tegra194-p2u
>>> +    then:
>>> +      required:
>>> +        - reg
>>> +        - reg-names
>>> +        - '#phy-cells'
>> That's not how it should be done. You have only two variants here, so
>> add a "required:" block with above and only one if:then: clause for
>> interrupts and nvidia,bpmp.
>>
>> Requiring reg/reg-names/phy-cells should be in separate patch with its
>> own reasoning.
> Ok, I will create two separate patches and add if:then clause only for 
> tegra234.
> As per understanding final change will look like below, right?
> 
> 
> required:
>    - reg
>    - reg-names
>    - '#phy-cells'
> 
> allOf:
>    - if:
>        properties:
>          compatible:
>            contains:
>              enum:
>                - nvidia,tegra234-p2u
>      then:
>        required:
>          - interrupts
>          - nvidia,bpmp

yes

Best regards,
Krzysztof

Re: [PATCH 1/3] dt-bindings: PHY: P2U: Add PCIe lane margining support

[Manikanta Maddireddy]

On 11/30/2022 8:45 PM, Krzysztof Kozlowski wrote:
> External email: Use caution opening links or attachments
>
>
> On 30/11/2022 16:11, Manikanta Maddireddy wrote:
>> Thank you for quick review. I will wait for other reviewers to review
>> patch 2 & 3.
>> I will address all review comments and sendnew revision.
>>
>> On 11/24/2022 2:15 PM, Krzysztof Kozlowski wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> On 24/11/2022 09:35, Manikanta Maddireddy wrote:
>>>> Tegra234 supports PCIe lane margining. P2U HW acts as a relay to exchange
>>> typo: merging?
>> It is not typo, it is PCIe feature lane margining.
>> https://pcisig.com/pushing-limits-understanding-lane-margining-pcie%C2%AE
>>>> margin control data and margin status between PCIe controller and UPHY.
>>> Please use scripts/get_maintainers.pl to get a list of necessary people
>>> and lists to CC.  It might happen, that command when run on an older
>>> kernel, gives you outdated entries.  Therefore please be sure you base
>>> your patches on recent Linux kernel.
>> I verified these patches on 6.0.0-rc6 kernel and executed get_maintainers.pl
>> script on it. Did I miss anyone here?
> Yes. At least Rob, maybe more. You need to CC all
> maintainers/reviewers/supporters and all mailing lists.
>
> It's not my task to verify each of these addresses to check whether you
> really missed someone or not. I spotted at least one missing address so
> just run get_maintainers.pl and use all entries from there.
I rechecked my "git send-email" command, I did add Rob's email. In my 
Linux file path, I found a file with name "--to=robh+dt@kernel.org".
I am not sure what mistake I made during "git send-email". I will do 
dry-run before sending v2.

I rechecked mailing list from get_maintainers.pl, I added everyone from 
the list expect Tegra developers who contributed to the file 
tegra234.dtsi, but not for PCIe.
>
>>>
>>>> Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
>>>> ---
>>>>    .../bindings/phy/phy-tegra194-p2u.yaml        | 50 +++++++++++++++++++
>>>>    1 file changed, 50 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/phy/phy-tegra194-p2u.yaml b/Documentation/devicetree/bindings/phy/phy-tegra194-p2u.yaml
>>>> index 4dc5205d893b..0ba3f6a0b474 100644
>>>> --- a/Documentation/devicetree/bindings/phy/phy-tegra194-p2u.yaml
>>>> +++ b/Documentation/devicetree/bindings/phy/phy-tegra194-p2u.yaml
>>>> @@ -40,6 +40,51 @@ properties:
>>>>      '#phy-cells':
>>>>        const: 0
>>>>
>>>> +  interrupts:
>>>> +    items:
>>>> +      description: P2U interrupt for Gen4 lane margining functionality.
>>> typo: merging?
>> It is not typo, it is PCIe feature lane margining.
>> https://pcisig.com/pushing-limits-understanding-lane-margining-pcie%C2%AE
>>>> +
>>>> +  interrupt-names:
>>>> +    items:
>>>> +      - const: intr
>>> Drop entire property, not really useful.
>> In driver, I am using platform_get_irq_byname(), I will change it to
>> platform_get_irq()
>> and drop this property.
>>>> +
>>>> +  nvidia,bpmp:
>>>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>>>> +    description: Must contain a pair of phandles to BPMP controller node followed by P2U ID.
>>>> +    items:
>>>> +      - items:
>>>> +          - description: phandle to BPMP controller node
>>>> +          - description: P2U instance ID
>>>> +            maximum: 24
>>>> +
>>>> +allOf:
>>>> +  - if:
>>>> +      properties:
>>>> +        compatible:
>>>> +          contains:
>>>> +            enum:
>>>> +              - nvidia,tegra194-p2u
>>>> +    then:
>>>> +      required:
>>>> +        - reg
>>>> +        - reg-names
>>>> +        - '#phy-cells'
>>> That's not how it should be done. You have only two variants here, so
>>> add a "required:" block with above and only one if:then: clause for
>>> interrupts and nvidia,bpmp.
>>>
>>> Requiring reg/reg-names/phy-cells should be in separate patch with its
>>> own reasoning.
>> Ok, I will create two separate patches and add if:then clause only for
>> tegra234.
>> As per understanding final change will look like below, right?
>>
>>
>> required:
>>     - reg
>>     - reg-names
>>     - '#phy-cells'
>>
>> allOf:
>>     - if:
>>         properties:
>>           compatible:
>>             contains:
>>               enum:
>>                 - nvidia,tegra234-p2u
>>       then:
>>         required:
>>           - interrupts
>>           - nvidia,bpmp
> yes
>
> Best regards,
> Krzysztof
>

[PATCH 2/3] arm64: tegra: Add uphy lane number and intr in p2u nodes

[Manikanta Maddireddy]

UPHY lane number is required to exchange lane margin data between P2U
and UPHY. Add uphy lane number in p2u device tree nodes.

Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
---
 arch/arm64/boot/dts/nvidia/tegra234.dtsi | 120 +++++++++++++++++++++++
 1 file changed, 120 insertions(+)

diff --git a/arch/arm64/boot/dts/nvidia/tegra234.dtsi b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
index eaf05ee9acd1..ec8a28a9d380 100644
--- a/arch/arm64/boot/dts/nvidia/tegra234.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
@@ -1109,6 +1109,11 @@ p2u_hsio_0: phy@3e00000 {
 			reg = <0x03e00000 0x10000>;
 			reg-names = "ctl";
 
+			interrupts = <0 336 0x04>;
+			interrupt-names = "intr";
+
+			nvidia,bpmp = <&bpmp 0>;
+
 			#phy-cells = <0>;
 		};
 
@@ -1117,6 +1122,11 @@ p2u_hsio_1: phy@3e10000 {
 			reg = <0x03e10000 0x10000>;
 			reg-names = "ctl";
 
+			interrupts = <0 337 0x04>;
+			interrupt-names = "intr";
+
+			nvidia,bpmp = <&bpmp 1>;
+
 			#phy-cells = <0>;
 		};
 
@@ -1125,6 +1135,11 @@ p2u_hsio_2: phy@3e20000 {
 			reg = <0x03e20000 0x10000>;
 			reg-names = "ctl";
 
+			interrupts = <0 338 0x04>;
+			interrupt-names = "intr";
+
+			nvidia,bpmp = <&bpmp 2>;
+
 			#phy-cells = <0>;
 		};
 
@@ -1133,6 +1148,11 @@ p2u_hsio_3: phy@3e30000 {
 			reg = <0x03e30000 0x10000>;
 			reg-names = "ctl";
 
+			interrupts = <0 339 0x04>;
+			interrupt-names = "intr";
+
+			nvidia,bpmp = <&bpmp 3>;
+
 			#phy-cells = <0>;
 		};
 
@@ -1141,6 +1161,11 @@ p2u_hsio_4: phy@3e40000 {
 			reg = <0x03e40000 0x10000>;
 			reg-names = "ctl";
 
+			interrupts = <0 340 0x04>;
+			interrupt-names = "intr";
+
+			nvidia,bpmp = <&bpmp 4>;
+
 			#phy-cells = <0>;
 		};
 
@@ -1149,6 +1174,11 @@ p2u_hsio_5: phy@3e50000 {
 			reg = <0x03e50000 0x10000>;
 			reg-names = "ctl";
 
+			interrupts = <0 341 0x04>;
+			interrupt-names = "intr";
+
+			nvidia,bpmp = <&bpmp 5>;
+
 			#phy-cells = <0>;
 		};
 
@@ -1157,6 +1187,11 @@ p2u_hsio_6: phy@3e60000 {
 			reg = <0x03e60000 0x10000>;
 			reg-names = "ctl";
 
+			interrupts = <0 342 0x04>;
+			interrupt-names = "intr";
+
+			nvidia,bpmp = <&bpmp 6>;
+
 			#phy-cells = <0>;
 		};
 
@@ -1165,6 +1200,11 @@ p2u_hsio_7: phy@3e70000 {
 			reg = <0x03e70000 0x10000>;
 			reg-names = "ctl";
 
+			interrupts = <0 343 0x04>;
+			interrupt-names = "intr";
+
+			nvidia,bpmp = <&bpmp 7>;
+
 			#phy-cells = <0>;
 		};
 
@@ -1173,6 +1213,11 @@ p2u_nvhs_0: phy@3e90000 {
 			reg = <0x03e90000 0x10000>;
 			reg-names = "ctl";
 
+			interrupts = <0 344 0x04>;
+			interrupt-names = "intr";
+
+			nvidia,bpmp = <&bpmp 8>;
+
 			#phy-cells = <0>;
 		};
 
@@ -1181,6 +1226,11 @@ p2u_nvhs_1: phy@3ea0000 {
 			reg = <0x03ea0000 0x10000>;
 			reg-names = "ctl";
 
+			interrupts = <0 345 0x04>;
+			interrupt-names = "intr";
+
+			nvidia,bpmp = <&bpmp 9>;
+
 			#phy-cells = <0>;
 		};
 
@@ -1189,6 +1239,11 @@ p2u_nvhs_2: phy@3eb0000 {
 			reg = <0x03eb0000 0x10000>;
 			reg-names = "ctl";
 
+			interrupts = <0 346 0x04>;
+			interrupt-names = "intr";
+
+			nvidia,bpmp = <&bpmp 10>;
+
 			#phy-cells = <0>;
 		};
 
@@ -1197,6 +1252,11 @@ p2u_nvhs_3: phy@3ec0000 {
 			reg = <0x03ec0000 0x10000>;
 			reg-names = "ctl";
 
+			interrupts = <0 347 0x04>;
+			interrupt-names = "intr";
+
+			nvidia,bpmp = <&bpmp 11>;
+
 			#phy-cells = <0>;
 		};
 
@@ -1205,6 +1265,11 @@ p2u_nvhs_4: phy@3ed0000 {
 			reg = <0x03ed0000 0x10000>;
 			reg-names = "ctl";
 
+			interrupts = <0 348 0x04>;
+			interrupt-names = "intr";
+
+			nvidia,bpmp = <&bpmp 12>;
+
 			#phy-cells = <0>;
 		};
 
@@ -1213,6 +1278,11 @@ p2u_nvhs_5: phy@3ee0000 {
 			reg = <0x03ee0000 0x10000>;
 			reg-names = "ctl";
 
+			interrupts = <0 349 0x04>;
+			interrupt-names = "intr";
+
+			nvidia,bpmp = <&bpmp 13>;
+
 			#phy-cells = <0>;
 		};
 
@@ -1221,6 +1291,11 @@ p2u_nvhs_6: phy@3ef0000 {
 			reg = <0x03ef0000 0x10000>;
 			reg-names = "ctl";
 
+			interrupts = <0 350 0x04>;
+			interrupt-names = "intr";
+
+			nvidia,bpmp = <&bpmp 14>;
+
 			#phy-cells = <0>;
 		};
 
@@ -1229,6 +1304,11 @@ p2u_nvhs_7: phy@3f00000 {
 			reg = <0x03f00000 0x10000>;
 			reg-names = "ctl";
 
+			interrupts = <0 351 0x04>;
+			interrupt-names = "intr";
+
+			nvidia,bpmp = <&bpmp 15>;
+
 			#phy-cells = <0>;
 		};
 
@@ -1237,6 +1317,11 @@ p2u_gbe_0: phy@3f20000 {
 			reg = <0x03f20000 0x10000>;
 			reg-names = "ctl";
 
+			interrupts = <0 203 0x04>;
+			interrupt-names = "intr";
+
+			nvidia,bpmp = <&bpmp 16>;
+
 			#phy-cells = <0>;
 		};
 
@@ -1245,6 +1330,11 @@ p2u_gbe_1: phy@3f30000 {
 			reg = <0x03f30000 0x10000>;
 			reg-names = "ctl";
 
+			interrupts = <0 220 0x04>;
+			interrupt-names = "intr";
+
+			nvidia,bpmp = <&bpmp 17>;
+
 			#phy-cells = <0>;
 		};
 
@@ -1253,6 +1343,11 @@ p2u_gbe_2: phy@3f40000 {
 			reg = <0x03f40000 0x10000>;
 			reg-names = "ctl";
 
+			interrupts = <0 221 0x04>;
+			interrupt-names = "intr";
+
+			nvidia,bpmp = <&bpmp 18>;
+
 			#phy-cells = <0>;
 		};
 
@@ -1261,6 +1356,11 @@ p2u_gbe_3: phy@3f50000 {
 			reg = <0x03f50000 0x10000>;
 			reg-names = "ctl";
 
+			interrupts = <0 222 0x04>;
+			interrupt-names = "intr";
+
+			nvidia,bpmp = <&bpmp 19>;
+
 			#phy-cells = <0>;
 		};
 
@@ -1269,6 +1369,11 @@ p2u_gbe_4: phy@3f60000 {
 			reg = <0x03f60000 0x10000>;
 			reg-names = "ctl";
 
+			interrupts = <0 108 0x04>;
+			interrupt-names = "intr";
+
+			nvidia,bpmp = <&bpmp 20>;
+
 			#phy-cells = <0>;
 		};
 
@@ -1277,6 +1382,11 @@ p2u_gbe_5: phy@3f70000 {
 			reg = <0x03f70000 0x10000>;
 			reg-names = "ctl";
 
+			interrupts = <0 109 0x04>;
+			interrupt-names = "intr";
+
+			nvidia,bpmp = <&bpmp 21>;
+
 			#phy-cells = <0>;
 		};
 
@@ -1285,6 +1395,11 @@ p2u_gbe_6: phy@3f80000 {
 			reg = <0x03f80000 0x10000>;
 			reg-names = "ctl";
 
+			interrupts = <0 110 0x04>;
+			interrupt-names = "intr";
+
+			nvidia,bpmp = <&bpmp 22>;
+
 			#phy-cells = <0>;
 		};
 
@@ -1293,6 +1408,11 @@ p2u_gbe_7: phy@3f90000 {
 			reg = <0x03f90000 0x10000>;
 			reg-names = "ctl";
 
+			interrupts = <0 111 0x04>;
+			interrupt-names = "intr";
+
+			nvidia,bpmp = <&bpmp 23>;
+
 			#phy-cells = <0>;
 		};
 
-- 
2.25.1

Re: [PATCH 2/3] arm64: tegra: Add uphy lane number and intr in p2u nodes

[Jon Hunter]

On 24/11/2022 08:35, Manikanta Maddireddy wrote:
> UPHY lane number is required to exchange lane margin data between P2U
> and UPHY. Add uphy lane number in p2u device tree nodes.
> 
> Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
> ---
>   arch/arm64/boot/dts/nvidia/tegra234.dtsi | 120 +++++++++++++++++++++++
>   1 file changed, 120 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/nvidia/tegra234.dtsi b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> index eaf05ee9acd1..ec8a28a9d380 100644
> --- a/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> +++ b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
> @@ -1109,6 +1109,11 @@ p2u_hsio_0: phy@3e00000 {
>   			reg = <0x03e00000 0x10000>;
>   			reg-names = "ctl";
>   
> +			interrupts = <0 336 0x04>;


Please use definitions and don't hard-code the values apart from the 
interrupt number.

Jon

-- 
nvpublic

Re: [PATCH 2/3] arm64: tegra: Add uphy lane number and intr in p2u nodes

[Manikanta Maddireddy]

On 12/1/2022 3:14 PM, Jon Hunter wrote:
>
> On 24/11/2022 08:35, Manikanta Maddireddy wrote:
>> UPHY lane number is required to exchange lane margin data between P2U
>> and UPHY. Add uphy lane number in p2u device tree nodes.
>>
>> Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
>> ---
>>   arch/arm64/boot/dts/nvidia/tegra234.dtsi | 120 +++++++++++++++++++++++
>>   1 file changed, 120 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/nvidia/tegra234.dtsi 
>> b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
>> index eaf05ee9acd1..ec8a28a9d380 100644
>> --- a/arch/arm64/boot/dts/nvidia/tegra234.dtsi
>> +++ b/arch/arm64/boot/dts/nvidia/tegra234.dtsi
>> @@ -1109,6 +1109,11 @@ p2u_hsio_0: phy@3e00000 {
>>               reg = <0x03e00000 0x10000>;
>>               reg-names = "ctl";
>>   +            interrupts = <0 336 0x04>;
>
>
> Please use definitions and don't hard-code the values apart from the 
> interrupt number.
Ack, I will address in V2.

Thanks,
Manikanta
>
> Jon
>

[PATCH 3/3] phy: tegra: Add lane margin support

[Manikanta Maddireddy]

Per PCIe r6.0.1, section 4.2.18, Lane Margining at Receiver is mandatory
for all Ports supporting a data rate of 16.0 GT/s or higher. Tegra234
supports Gen4 and Receiver Lane Margining with per lane PIPE2UPHY instance
acting as a relay between PCIe controller and Universal PHY (UPHY).

P2U driver enables MARGIN_SW_READY and MARGIN_READY bits to start snooping
on changes in lane margin control register in PCIe configuration space.
P2U HW generates MARGIN_START or MARGIN_CHANGE interrupt after it copied
margin control data to P2U_RX_MARGIN_CTRL register. On MARGIN_START or
MARGIN_CHANGE interrupt, P2U driver copies margin control data to UPHY
via mailbox communication. UPHY HW performs margining operation and
P2U driver copies margin status from UPHY to P2U_RX_MARGIN_STATUS
register. P2U driver repeats this until PCIe controller issues margin
stop command.

Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
---
 drivers/phy/tegra/phy-tegra194-p2u.c | 272 +++++++++++++++++++++++++++
 1 file changed, 272 insertions(+)

diff --git a/drivers/phy/tegra/phy-tegra194-p2u.c b/drivers/phy/tegra/phy-tegra194-p2u.c
index 633e6b747275..a86b4af70ab9 100644
--- a/drivers/phy/tegra/phy-tegra194-p2u.c
+++ b/drivers/phy/tegra/phy-tegra194-p2u.c
@@ -7,12 +7,15 @@
  * Author: Vidya Sagar <vidyas@nvidia.com>
  */
 
+#include <linux/delay.h>
 #include <linux/err.h>
 #include <linux/io.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_platform.h>
 #include <linux/phy/phy.h>
+#include <soc/tegra/bpmp.h>
+#include <soc/tegra/bpmp-abi.h>
 
 #define P2U_CONTROL_CMN			0x74
 #define P2U_CONTROL_CMN_ENABLE_L2_EXIT_RATE_CHANGE		BIT(13)
@@ -31,14 +34,57 @@
 #define P2U_DIR_SEARCH_CTRL				0xd4
 #define P2U_DIR_SEARCH_CTRL_GEN4_FINE_GRAIN_SEARCH_TWICE	BIT(18)
 
+#define P2U_RX_MARGIN_SW_INT_EN		0xe0
+#define P2U_RX_MARGIN_SW_INT_EN_READINESS		BIT(0)
+#define P2U_RX_MARGIN_SW_INT_EN_MARGIN_START		BIT(1)
+#define P2U_RX_MARGIN_SW_INT_EN_MARGIN_CHANGE		BIT(2)
+#define P2U_RX_MARGIN_SW_INT_EN_MARGIN_STOP		BIT(3)
+
+#define P2U_RX_MARGIN_SW_INT		0xe4
+#define P2U_RX_MARGIN_SW_INT_MASK			0xf
+#define P2U_RX_MARGIN_SW_INT_READINESS			BIT(0)
+#define P2U_RX_MARGIN_SW_INT_MARGIN_START		BIT(1)
+#define P2U_RX_MARGIN_SW_INT_MARGIN_CHANGE		BIT(2)
+#define P2U_RX_MARGIN_SW_INT_MARGIN_STOP		BIT(3)
+
+#define P2U_RX_MARGIN_SW_STATUS		0xe8
+#define P2U_RX_MARGIN_SW_STATUS_MARGIN_SW_READY		BIT(0)
+#define P2U_RX_MARGIN_SW_STATUS_MARGIN_READY		BIT(1)
+#define P2U_RX_MARGIN_SW_STATUS_PHY_MARGIN_STATUS	BIT(2)
+#define P2U_RX_MARGIN_SW_STATUS_PHY_MARGIN_ERROR_STATUS	BIT(3)
+
+#define P2U_RX_MARGIN_CTRL		0xec
+
+#define P2U_RX_MARGIN_STATUS		0xf0
+#define P2U_RX_MARGIN_STATUS_ERRORS_MASK		0xffff
+
+enum margin_state {
+	RX_MARGIN_START_CHANGE = 1,
+	RX_MARGIN_STOP,
+	RX_MARGIN_GET_MARGIN,
+};
+
 struct tegra_p2u_of_data {
 	bool one_dir_search;
+	bool lane_margin;
 };
 
 struct tegra_p2u {
 	void __iomem *base;
 	bool skip_sz_protection_en; /* Needed to support two retimers */
 	struct tegra_p2u_of_data *of_data;
+	struct device *dev;
+	struct tegra_bpmp *bpmp;
+	u32 id;
+	atomic_t margin_state;
+};
+
+struct margin_ctrl {
+	u32 en:1;
+	u32 clr:1;
+	u32 x:7;
+	u32 y:6;
+	u32 n_blks:8;
 };
 
 static inline void p2u_writel(struct tegra_p2u *phy, const u32 value,
@@ -83,6 +129,14 @@ static int tegra_p2u_power_on(struct phy *x)
 		p2u_writel(phy, val, P2U_DIR_SEARCH_CTRL);
 	}
 
+	if (phy->of_data->lane_margin) {
+		p2u_writel(phy, P2U_RX_MARGIN_SW_INT_EN_READINESS |
+			   P2U_RX_MARGIN_SW_INT_EN_MARGIN_START |
+			   P2U_RX_MARGIN_SW_INT_EN_MARGIN_CHANGE |
+			   P2U_RX_MARGIN_SW_INT_EN_MARGIN_STOP,
+			   P2U_RX_MARGIN_SW_INT_EN);
+	}
+
 	return 0;
 }
 
@@ -104,17 +158,195 @@ static const struct phy_ops ops = {
 	.owner = THIS_MODULE,
 };
 
+static int tegra_p2u_set_margin_control(struct tegra_p2u *phy, u32 ctrl_data)
+{
+	struct tegra_bpmp_message msg;
+	struct mrq_uphy_response resp;
+	struct mrq_uphy_request req;
+	struct margin_ctrl ctrl;
+	int err;
+
+	memcpy(&ctrl, &ctrl_data, sizeof(ctrl_data));
+
+	memset(&req, 0, sizeof(req));
+	memset(&resp, 0, sizeof(resp));
+
+	req.lane = phy->id;
+	req.cmd = CMD_UPHY_PCIE_LANE_MARGIN_CONTROL;
+	req.uphy_set_margin_control.en = ctrl.en;
+	req.uphy_set_margin_control.clr = ctrl.clr;
+	req.uphy_set_margin_control.x = ctrl.x;
+	req.uphy_set_margin_control.y = ctrl.y;
+	req.uphy_set_margin_control.nblks = ctrl.n_blks;
+
+	memset(&msg, 0, sizeof(msg));
+	msg.mrq = MRQ_UPHY;
+	msg.tx.data = &req;
+	msg.tx.size = sizeof(req);
+	msg.rx.data = &resp;
+	msg.rx.size = sizeof(resp);
+
+	err = tegra_bpmp_transfer(phy->bpmp, &msg);
+	if (err)
+		return err;
+	if (msg.rx.ret)
+		return -EINVAL;
+
+	return 0;
+}
+
+static int tegra_p2u_get_margin_status(struct tegra_p2u *phy, u32 *val)
+{
+	struct tegra_bpmp_message msg;
+	struct mrq_uphy_response resp;
+	struct mrq_uphy_request req;
+	int rc;
+
+	req.lane = phy->id;
+	req.cmd = CMD_UPHY_PCIE_LANE_MARGIN_STATUS;
+
+	memset(&msg, 0, sizeof(msg));
+	msg.mrq = MRQ_UPHY;
+	msg.tx.data = &req;
+	msg.tx.size = sizeof(req);
+	msg.rx.data = &resp;
+	msg.rx.size = sizeof(resp);
+
+	rc = tegra_bpmp_transfer(phy->bpmp, &msg);
+	if (rc)
+		return rc;
+	if (msg.rx.ret)
+		return -EINVAL;
+
+	*val = resp.uphy_get_margin_status.status;
+
+	return 0;
+}
+
+static irqreturn_t tegra_p2u_irq_thread(int irq, void *arg)
+{
+	struct tegra_p2u *phy = arg;
+	struct device *dev = phy->dev;
+	u32 val;
+	int state, ret;
+
+	do {
+		state = atomic_read(&phy->margin_state);
+		switch (state) {
+		case RX_MARGIN_START_CHANGE:
+		case RX_MARGIN_STOP:
+			/* Read margin control data and copy it to UPHY. */
+			val = p2u_readl(phy, P2U_RX_MARGIN_CTRL);
+			ret = tegra_p2u_set_margin_control(phy, val);
+			if (ret) {
+				dev_err(dev, "failed to set margin control: %d\n", ret);
+				break;
+			}
+
+			p2u_writel(phy, P2U_RX_MARGIN_SW_STATUS_MARGIN_SW_READY |
+				   P2U_RX_MARGIN_SW_STATUS_MARGIN_READY |
+				   P2U_RX_MARGIN_SW_STATUS_PHY_MARGIN_STATUS |
+				   P2U_RX_MARGIN_SW_STATUS_PHY_MARGIN_ERROR_STATUS,
+				   P2U_RX_MARGIN_SW_STATUS);
+
+			usleep_range(10, 11);
+
+			if (state == RX_MARGIN_STOP) {
+				/* Return from the loop if PCIe ctrl issues margin stop cmd. */
+				p2u_writel(phy, P2U_RX_MARGIN_SW_STATUS_MARGIN_SW_READY |
+					   P2U_RX_MARGIN_SW_STATUS_MARGIN_READY |
+					   P2U_RX_MARGIN_SW_STATUS_PHY_MARGIN_ERROR_STATUS,
+					   P2U_RX_MARGIN_SW_STATUS);
+
+				return IRQ_HANDLED;
+			}
+
+			atomic_set(&phy->margin_state, RX_MARGIN_GET_MARGIN);
+			break;
+
+		case RX_MARGIN_GET_MARGIN:
+			/*
+			 * Read margin status from UPHY and program it in P2U_RX_MARGIN_STATUS
+			 * register. This data will reflect in PCIe controller's margining lane
+			 * status register.
+			 */
+			ret = tegra_p2u_get_margin_status(phy, &val);
+			if (ret) {
+				dev_err(dev, "failed to get margin status: %d\n", ret);
+				break;
+			}
+			p2u_writel(phy, val & P2U_RX_MARGIN_STATUS_ERRORS_MASK,
+				   P2U_RX_MARGIN_STATUS);
+
+			p2u_writel(phy, P2U_RX_MARGIN_SW_STATUS_MARGIN_SW_READY |
+				   P2U_RX_MARGIN_SW_STATUS_MARGIN_READY |
+				   P2U_RX_MARGIN_SW_STATUS_PHY_MARGIN_ERROR_STATUS,
+				   P2U_RX_MARGIN_SW_STATUS);
+
+			msleep(20);
+			break;
+
+		default:
+			dev_err(dev, "Invalid margin state: %d\n", state);
+			return IRQ_HANDLED;
+		};
+	} while (1);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t tegra_p2u_irq_handler(int irq, void *arg)
+{
+	struct tegra_p2u *phy = (struct tegra_p2u *)arg;
+	u32 val = 0;
+	irqreturn_t ret = IRQ_HANDLED;
+
+	val = p2u_readl(phy, P2U_RX_MARGIN_SW_INT);
+	p2u_writel(phy, val, P2U_RX_MARGIN_SW_INT);
+
+	/*
+	 * When PCIe link trains to Gen4, P2U HW generate READINESS interrupt. Set MARGIN_SW_READY
+	 * and MARGIN_READY bits to enable P2U HW sample lane margin control data from PCIe
+	 * controller's configuration space.
+	 */
+	if (val & P2U_RX_MARGIN_SW_INT_READINESS)
+		p2u_writel(phy, P2U_RX_MARGIN_SW_STATUS_MARGIN_SW_READY |
+			   P2U_RX_MARGIN_SW_STATUS_MARGIN_READY,
+			   P2U_RX_MARGIN_SW_STATUS);
+
+	/*
+	 * P2U HW generates MARGIN_START or MARGIN_CHANGE interrupt after it copied margin control
+	 * data to P2U_RX_MARGIN_CTRL register.
+	 */
+	if ((val & P2U_RX_MARGIN_SW_INT_MARGIN_START) ||
+	    (val & P2U_RX_MARGIN_SW_INT_MARGIN_CHANGE)) {
+		atomic_set(&phy->margin_state, RX_MARGIN_START_CHANGE);
+		ret = IRQ_WAKE_THREAD;
+	}
+
+	/* P2U HW generates MARGIN_STOP interrupt when PCIe controller issues margin stop cmd. */
+	if (val & P2U_RX_MARGIN_SW_INT_MARGIN_STOP)
+		atomic_set(&phy->margin_state, RX_MARGIN_STOP);
+
+	return ret;
+}
+
 static int tegra_p2u_probe(struct platform_device *pdev)
 {
 	struct phy_provider *phy_provider;
 	struct device *dev = &pdev->dev;
 	struct phy *generic_phy;
 	struct tegra_p2u *phy;
+	int ret;
+	u32 irq;
 
 	phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
 	if (!phy)
 		return -ENOMEM;
 
+	phy->dev = dev;
+	platform_set_drvdata(pdev, phy);
+
 	phy->of_data =
 		(struct tegra_p2u_of_data *)of_device_get_match_data(dev);
 	if (!phy->of_data)
@@ -140,15 +372,54 @@ static int tegra_p2u_probe(struct platform_device *pdev)
 	if (IS_ERR(phy_provider))
 		return PTR_ERR(phy_provider);
 
+	if (phy->of_data->lane_margin) {
+		irq = platform_get_irq_byname(pdev, "intr");
+		if (irq < 0) {
+			dev_err(dev, "failed to get intr interrupt\n");
+			return irq;
+		}
+
+		ret = devm_request_threaded_irq(dev, irq, tegra_p2u_irq_handler,
+						tegra_p2u_irq_thread, 0,
+						"tegra-p2u-intr", phy);
+		if (ret) {
+			dev_err(dev, "failed to request intr irq\n");
+			return ret;
+		}
+
+		ret = of_property_read_u32_index(dev->of_node, "nvidia,bpmp",
+						 1, &phy->id);
+		if (ret) {
+			dev_err(dev, "failed to read P2U id: %d\n", ret);
+			return ret;
+		}
+
+		phy->bpmp = tegra_bpmp_get(dev);
+		if (IS_ERR(phy->bpmp))
+			return PTR_ERR(phy->bpmp);
+	}
+
+	return 0;
+}
+
+static int tegra_p2u_remove(struct platform_device *pdev)
+{
+	struct tegra_p2u *phy = platform_get_drvdata(pdev);
+
+	if (phy->of_data->lane_margin)
+		tegra_bpmp_put(phy->bpmp);
+
 	return 0;
 }
 
 static const struct tegra_p2u_of_data tegra194_p2u_of_data = {
 	.one_dir_search = false,
+	.lane_margin = false,
 };
 
 static const struct tegra_p2u_of_data tegra234_p2u_of_data = {
 	.one_dir_search = true,
+	.lane_margin = true,
 };
 
 static const struct of_device_id tegra_p2u_id_table[] = {
@@ -166,6 +437,7 @@ MODULE_DEVICE_TABLE(of, tegra_p2u_id_table);
 
 static struct platform_driver tegra_p2u_driver = {
 	.probe = tegra_p2u_probe,
+	.remove = tegra_p2u_remove,
 	.driver = {
 		.name = "tegra194-p2u",
 		.of_match_table = tegra_p2u_id_table,
-- 
2.25.1