[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b81c69e0-13a2-6a25-21c7-c0d8804f9c3b@nvidia.com>
Date: Wed, 30 Nov 2022 20:41:14 +0530
From: Manikanta Maddireddy <mmaddireddy@...dia.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
vkoul@...nel.org, kishon@...nel.org,
krzysztof.kozlowski+dt@...aro.org, thierry.reding@...il.com,
jonathanh@...dia.com, vidyas@...dia.com
Cc: linux-phy@...ts.infradead.org, devicetree@...r.kernel.org,
linux-tegra@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-pci@...r.kernel.org, bhelgaas@...gle.com,
lpieralisi@...nel.org, nkristam@...dia.com
Subject: Re: [PATCH 1/3] dt-bindings: PHY: P2U: Add PCIe lane margining
support
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@...dia.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
>
Powered by blists - more mailing lists