Re: [PATCH v4 1/2] dt-bindings: pci: Retrain Link to work around Gen2 training defect.

[Kishon Vijay Abraham I]

Hi,

On 14/12/20 8:35 pm, Rob Herring wrote:
> On Sun, Dec 13, 2020 at 10:21 PM Kishon Vijay Abraham I <kishon@xxxxxx> wrote:
>>
>> Hi Nadeem,
>>
>> On 12/12/20 12:37 pm, Athani Nadeem Ladkhan wrote:
>>> Hi Rob / Kishon,
>>>
>>>> -----Original Message-----
>>>> From: Rob Herring <robh@xxxxxxxxxx>
>>>> Sent: Friday, December 11, 2020 10:32 PM
>>>> To: Athani Nadeem Ladkhan <nadeem@xxxxxxxxxxx>
>>>> Cc: Tom Joseph <tjoseph@xxxxxxxxxxx>; Lorenzo Pieralisi
>>>> <lorenzo.pieralisi@xxxxxxx>; Bjorn Helgaas <bhelgaas@xxxxxxxxxx>; PCI
>>>> <linux-pci@xxxxxxxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx; Kishon Vijay
>>>> Abraham I <kishon@xxxxxx>; devicetree@xxxxxxxxxxxxxxx; Milind Parab
>>>> <mparab@xxxxxxxxxxx>; Swapnil Kashinath Jakhade
>>>> <sjakhade@xxxxxxxxxxx>; Parshuram Raju Thombare
>>>> <pthombar@xxxxxxxxxxx>
>>>> Subject: Re: [PATCH v4 1/2] dt-bindings: pci: Retrain Link to work around
>>>> Gen2 training defect.
>>>>
>>>> EXTERNAL MAIL
>>>>
>>>>
>>>> On Fri, Dec 11, 2020 at 9:03 AM Nadeem Athani <nadeem@xxxxxxxxxxx>
>>>> wrote:
>>>>>
>>>>> Cadence controller will not initiate autonomous speed change if
>>>>> strapped as Gen2. The Retrain Link bit is set as quirk to enable this speed
>>>> change.
>>>>> Adding a quirk flag based on a new compatible string.
>>>>>
>>>>> Signed-off-by: Nadeem Athani <nadeem@xxxxxxxxxxx>
>>>>> ---
>>>>>  Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml | 4
>>>>> +++-
>>>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git
>>>>> a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml
>>>>> b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml
>>>>> index 293b8ec318bc..204d78f9efe3 100644
>>>>> --- a/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml
>>>>> +++ b/Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml
>>>>> @@ -15,7 +15,9 @@ allOf:
>>>>>
>>>>>  properties:
>>>>>    compatible:
>>>>> -    const: cdns,cdns-pcie-host
>>>>> +    enum:
>>>>> +        - cdns,cdns-pcie-host
>>>>> +        - cdns,cdns-pcie-host-quirk-retrain
>>>>
>>>> So, we'll just keep adding quirk strings on to the compatible? I don't think so.
>>>> Compatible strings should map to a specific implementation/platform and
>>>> quirks can then be implied from them. This is the only way we can implement
>>>> quirks in the OS without firmware
>>>> (DT) changes.
>>> Ok, I will change the compatible string to " ti,j721e-pcie-host" in place of  " cdns,cdns-pcie-host-quirk-retrain" .
>>> @Kishon Vijay Abraham I: Is this fine? Or will you suggest an appropriate name?
>>
>> IMHO it should be something like "cdns,cdns-pcie-host-vX", since the
>> quirk itself is not specific to TI platform rather Cadence IP version.
> 
> That's fine if Cadence has a need for it, but for TI platforms use the
> TI compatible string. ECOs on version X IP without changing X is not
> uncommon.

Okay. I re-worked the patch to be applicable only to TI's J721E SoC
http://lore.kernel.org/r/20201215070009.27937-1-kishon@xxxxxx

Thank You,
Kishon