lkml.org 
[lkml]   [2022]   [Nov]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 1/3] dt-bindings: PHY: P2U: Add PCIe lane margining support
From

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
>

\
 
 \ /
  Last update: 2022-11-30 17:15    [W:0.040 / U:1.100 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site