Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <c755ee45-fb6d-1f7c-2a3b-bcaa9d292c1b@nvidia.com>
Date:   Wed, 30 Nov 2022 21:43:45 +0530
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.5.0
Subject: Re: [PATCH 1/3] dt-bindings: PHY: P2U: Add PCIe lane margining
 support
Content-Language: en-US
To:     Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
        vkoul@kernel.org, kishon@kernel.org,
        krzysztof.kozlowski+dt@linaro.org, thierry.reding@gmail.com,
        jonathanh@nvidia.com, vidyas@nvidia.com
Cc:     linux-phy@lists.infradead.org, devicetree@vger.kernel.org,
        linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-pci@vger.kernel.org, bhelgaas@google.com,
        lpieralisi@kernel.org, nkristam@nvidia.com
References: <20221124083510.3008139-1-mmaddireddy@nvidia.com>
 <20221124083510.3008139-2-mmaddireddy@nvidia.com>
 <0b203c11-851c-fd6c-faeb-e283b158f1c8@linaro.org>
 <b81c69e0-13a2-6a25-21c7-c0d8804f9c3b@nvidia.com>
 <7ce2fd0f-621d-dfbb-2ec0-53ccbb3308ae@linaro.org>
From:   Manikanta Maddireddy <mmaddireddy@nvidia.com>
In-Reply-To: <7ce2fd0f-621d-dfbb-2ec0-53ccbb3308ae@linaro.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?VDlEZmhLNWtZZTNCVDhteDAxakxHbVNxNEJiRzJmWlptUDFkTWo1eEhFQk1h?=
 =?utf-8?B?M3ZPdlhNMFpxY0hjMnVnN2hMRXI3azhMOUVHSVV2MXh0N0FNb2lsR0dJWXJi?=
 =?utf-8?B?TlpNY0JTYk1Jbm5MZGh2eU0zRkZBSnAya3BJUEpGcVNzZEx2WiszNEVZWC9J?=
 =?utf-8?B?eGx2R1hxRkJkRlJzZnFNL3lkMU1zTi85bjZBMWpQVjhVUmwxdXJqaHVaOS9H?=
 =?utf-8?B?bXlobGhsa0pQUmdHRk9MQlpxdStwZ3lyNnMyeUQ0enpXZkx5OTBvMHBkQVJW?=
 =?utf-8?B?NFV5TnE1ekhHUzdJc1RkK3REYlBlakJaSGRTODk2Sm9oWXZwSEVMRzBKK1Fw?=
 =?utf-8?B?WkYwZ0V3aFdXeGJDalllWkJhajBjbEgxZnhLWkVKODlMNXo2YWIxK0k5MDFt?=
 =?utf-8?B?U0h1aGN2UitRUkdlbVJWcHVjSGRKV1pqT0REM01FenBvQ1ZYTEtTTjFUTzRl?=
 =?utf-8?B?eWZtU3pVSHlSVDRJaEdTR3ViMlpRNVA5ODhMcStrQ1dZbUozU2d5eTQxbGFj?=
 =?utf-8?B?MHM5L2JuQi9TUnhVaHd5eUE0VDN4d1ZMU1dpWGJINXN0SlhQbjN2V24vSXpU?=
 =?utf-8?B?VFdLQ2pMblJmV0VYWWsyZlJtekFBbm51ZExlMGJWZVJvTFhaRFBVazZtcUo5?=
 =?utf-8?B?d1ZwZGwxeHdmbW1kNVoyRWdGeXNzUnRTTVg4SDU3QmJyNk5UTWhDc0hvYU1N?=
 =?utf-8?B?VHpGQTZBcko0Q25TRkVYSGxmZE5sMmNxV1lQbmttaDhkd3dlTHJ2b0xPbW9K?=
 =?utf-8?B?My9UM2lLYmdmZFlqcUNqeTJNRURmd1lKM3hPLzE0WW1jMDVKamcvaDd0eVA0?=
 =?utf-8?B?YlhLZnhnZVd5ODBsMmRYZHdteTZQLy9CM2EvN1Nrek9ZQ1JnQXQ5MkQ3YXJ2?=
 =?utf-8?B?SDRTczVhc0Y3eGVtc2RQazFJZWtEV3Vkb3ZEVHhUZURqVmJiUFJpT0JhcmpJ?=
 =?utf-8?B?QnZ3R01OaW13T1VFN1d3NjRjeXFMTU91RWYyRlNCNXdZaG5TL2c2RlI2VFJB?=
 =?utf-8?B?bHNWRHhuMkVDckRDRlNrcTJ6SjA3ME03eVdUaTl4QWFkRkNHendqckhGSFVy?=
 =?utf-8?B?dVJETjRYd0xLMGVXMldWL3IrQy85ZVdTeTNwUm5HQXNQcjdjcUV0WktXSTVG?=
 =?utf-8?B?UGdueERrOG1kZktIZnRQb2RlU1pHY2hzZURjNnlFNUhWVFB1RitjS2haNmpN?=
 =?utf-8?B?MmUvMjlmSHhFU2kzZnd2WlZ1T0RHV09KZVNpdFlYaDJjcGFTUFprSGhIYUlM?=
 =?utf-8?B?MTB0RUY3SVlwSUZsSzU4Q2N2T3pHTmlOc2k4U2cvUlFkWVhlVVQ3dUdjS3d6?=
 =?utf-8?B?aXNWR2luSFRSdFRvSWxUNE9ObEhEdjI2THdDSjRuRTcxeVRFRXNtZEFLdjE1?=
 =?utf-8?B?TkNxaUt0Z3VjdDhxWlVYaXpyNTVDdkRZOVVWL01RVVF2bXlqOFJiQTFsbnV4?=
 =?utf-8?B?LzVnOTQrZHI2TnVTYzVjSWVHUmFHejUyeVVrZ0txaXJkTFZFMzJpQzg4bkN4?=
 =?utf-8?B?NXQrU0E3NFU3K0NkVXhnVFRvUkkrUGdYMHIyQ1dlaVh3MWJVd2VYRCtRRnNJ?=
 =?utf-8?B?SzhsNFQ0WFFJbWpabDl5OU9lV0p0QnliNktKMVBiNjdaVVNTd2d5eWpCYUVV?=
 =?utf-8?B?Mi9teitiRGpzTzREN2NCalQvdUhoUXM0ejF3K3M2K1BXWmdjY0hDNHpzVlhJ?=
 =?utf-8?B?aXNBZ0k1dnVOT3ZKZTA1NjNZWFhMZ0N6S1RxanRBSFk0S0w0bUlxVnBLNXNt?=
 =?utf-8?B?NFFMZDcrK1Q2aWdrdzdIK01VeU50ZTN1WGwweFRnak5zcjhqU3JQMzNoK21v?=
 =?utf-8?B?aGtYTkJnZStsVS9OMjBCZ3J2WmNjdEZwN2ZSKzI0eE82cGlMVFRMR3JqZUpu?=
 =?utf-8?B?VUk1WDlhTmR4QUVXY1RNamN6UFNQc1k4aGRYb0dZSjQ1THVrdTNnQXhOV2lW?=
 =?utf-8?B?Lytvb3NDOFZySXk4WHk4MTBvamxwOWxOK0ErdUNIQ1dZWnB5M0xsOGwxUG9i?=
 =?utf-8?B?YUpIMnhoNXVXN09KVitVNDU4U200NWVPZmRNTVVhU1YzN1hXQi85Yi9ldWtQ?=
 =?utf-8?B?SG1Ma3R3MERKeVVScXJDZXhJR1dZYnlzT2xxRmdYSzZtZllCcW9jclNGajc1?=
 =?utf-8?B?bWdQWEVOaTdOaEhTUlBoKzF4RTZuS3U5K1F2ZVYwZ0kyY3ljSm44VjNsRURz?=
 =?utf-8?B?Qnc9PQ==?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: d7fd7861-c305-4382-652e-08dad2ede3f9
X-MS-Exchange-CrossTenant-AuthSource: DM6PR12MB3356.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Nov 2022 16:14:00.1607
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: BfY3/Ujxh7DY3ZRDpAk8pBD6/IaJwTzXgsVAuiRMC0hqifEpHM3LXirPvU1gHErSfTmKubRt08jLd1vT607/Tg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR12MB7583
X-Spam-Status: No, score=-1.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,
        NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,
        SPF_NONE autolearn=no autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


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
>