Messages in this thread | | | Date | Wed, 30 Nov 2022 20:41:14 +0530 | Subject | Re: [PATCH 1/3] dt-bindings: PHY: P2U: Add PCIe lane margining support | From | 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 >
| |