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@xxxxxxxxxx> >>> --- >>> .../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
