| Message ID | 20201211144236.3825-2-nadeem@cadence.com |
|---|---|
| State | New |
| Headers | show |
| Series | PCI: cadence: Retrain Link to work around Gen2 | expand |
On Fri, Dec 11, 2020 at 9:03 AM Nadeem Athani <nadeem@cadence.com> 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@cadence.com> > --- > 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. Rob
Hi Rob / Kishon, > -----Original Message----- > From: Rob Herring <robh@kernel.org> > Sent: Friday, December 11, 2020 10:32 PM > To: Athani Nadeem Ladkhan <nadeem@cadence.com> > Cc: Tom Joseph <tjoseph@cadence.com>; Lorenzo Pieralisi > <lorenzo.pieralisi@arm.com>; Bjorn Helgaas <bhelgaas@google.com>; PCI > <linux-pci@vger.kernel.org>; linux-kernel@vger.kernel.org; Kishon Vijay > Abraham I <kishon@ti.com>; devicetree@vger.kernel.org; Milind Parab > <mparab@cadence.com>; Swapnil Kashinath Jakhade > <sjakhade@cadence.com>; Parshuram Raju Thombare > <pthombar@cadence.com> > 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@cadence.com> > 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@cadence.com> > > --- > > 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? Nadeem > > Rob
Hi Nadeem, On 12/12/20 12:37 pm, Athani Nadeem Ladkhan wrote: > Hi Rob / Kishon, > >> -----Original Message----- >> From: Rob Herring <robh@kernel.org> >> Sent: Friday, December 11, 2020 10:32 PM >> To: Athani Nadeem Ladkhan <nadeem@cadence.com> >> Cc: Tom Joseph <tjoseph@cadence.com>; Lorenzo Pieralisi >> <lorenzo.pieralisi@arm.com>; Bjorn Helgaas <bhelgaas@google.com>; PCI >> <linux-pci@vger.kernel.org>; linux-kernel@vger.kernel.org; Kishon Vijay >> Abraham I <kishon@ti.com>; devicetree@vger.kernel.org; Milind Parab >> <mparab@cadence.com>; Swapnil Kashinath Jakhade >> <sjakhade@cadence.com>; Parshuram Raju Thombare >> <pthombar@cadence.com> >> 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@cadence.com> >> 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@cadence.com> >>> --- >>> 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. Thank You, Kishon
On Sun, Dec 13, 2020 at 10:21 PM Kishon Vijay Abraham I <kishon@ti.com> wrote: > > Hi Nadeem, > > On 12/12/20 12:37 pm, Athani Nadeem Ladkhan wrote: > > Hi Rob / Kishon, > > > >> -----Original Message----- > >> From: Rob Herring <robh@kernel.org> > >> Sent: Friday, December 11, 2020 10:32 PM > >> To: Athani Nadeem Ladkhan <nadeem@cadence.com> > >> Cc: Tom Joseph <tjoseph@cadence.com>; Lorenzo Pieralisi > >> <lorenzo.pieralisi@arm.com>; Bjorn Helgaas <bhelgaas@google.com>; PCI > >> <linux-pci@vger.kernel.org>; linux-kernel@vger.kernel.org; Kishon Vijay > >> Abraham I <kishon@ti.com>; devicetree@vger.kernel.org; Milind Parab > >> <mparab@cadence.com>; Swapnil Kashinath Jakhade > >> <sjakhade@cadence.com>; Parshuram Raju Thombare > >> <pthombar@cadence.com> > >> 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@cadence.com> > >> 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@cadence.com> > >>> --- > >>> 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. Rob
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@ti.com> wrote: >> >> Hi Nadeem, >> >> On 12/12/20 12:37 pm, Athani Nadeem Ladkhan wrote: >>> Hi Rob / Kishon, >>> >>>> -----Original Message----- >>>> From: Rob Herring <robh@kernel.org> >>>> Sent: Friday, December 11, 2020 10:32 PM >>>> To: Athani Nadeem Ladkhan <nadeem@cadence.com> >>>> Cc: Tom Joseph <tjoseph@cadence.com>; Lorenzo Pieralisi >>>> <lorenzo.pieralisi@arm.com>; Bjorn Helgaas <bhelgaas@google.com>; PCI >>>> <linux-pci@vger.kernel.org>; linux-kernel@vger.kernel.org; Kishon Vijay >>>> Abraham I <kishon@ti.com>; devicetree@vger.kernel.org; Milind Parab >>>> <mparab@cadence.com>; Swapnil Kashinath Jakhade >>>> <sjakhade@cadence.com>; Parshuram Raju Thombare >>>> <pthombar@cadence.com> >>>> 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@cadence.com> >>>> 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@cadence.com> >>>>> --- >>>>> 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@ti.com Thank You, Kishon
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 reg: maxItems: 2
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@cadence.com> --- Documentation/devicetree/bindings/pci/cdns,cdns-pcie-host.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)