linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/2] PCI: cadence: Retrain Link to work around Gen2
@ 2020-12-11 14:42 Nadeem Athani
  2020-12-11 14:42 ` [PATCH v4 1/2] dt-bindings: pci: Retrain Link to work around Gen2 training defect Nadeem Athani
  2020-12-11 14:42 ` [PATCH v4 2/2] PCI: cadence: " Nadeem Athani
  0 siblings, 2 replies; 8+ messages in thread
From: Nadeem Athani @ 2020-12-11 14:42 UTC (permalink / raw)
  To: tjoseph, lorenzo.pieralisi, robh, bhelgaas, linux-pci,
	linux-kernel, kishon, devicetree
  Cc: nadeem, mparab, sjakhade, pthombar

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. In future IP 
revisions this will not be applicable.

Version history:
Changes in v4:
- Added a quirk flag based on a new compatible string.
- Change of api for link up: cdns_pcie_host_wait_for_link().
Changes in v3:
- To set retrain link bit,checking device capability & link status.
- 32bit read in place of 8bit.
- Minor correction in patch comment.
- Change in variable & macro name.
Changes in v2:
- 16bit read in place of 8bit.

Nadeem Athani (2):
  dt-bindings: pci: Retrain Link to work around Gen2 training defect.
  PCI: cadence: Retrain Link to work around Gen2 training defect.

 .../bindings/pci/cdns,cdns-pcie-host.yaml          |  4 +-
 drivers/pci/controller/cadence/pcie-cadence-host.c | 67 ++++++++++++++++------
 drivers/pci/controller/cadence/pcie-cadence-plat.c | 13 +++++
 drivers/pci/controller/cadence/pcie-cadence.h      | 11 +++-
 4 files changed, 76 insertions(+), 19 deletions(-)

-- 
2.15.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v4 1/2] dt-bindings: pci: Retrain Link to work around Gen2 training defect.
  2020-12-11 14:42 [PATCH v4 0/2] PCI: cadence: Retrain Link to work around Gen2 Nadeem Athani
@ 2020-12-11 14:42 ` Nadeem Athani
  2020-12-11 17:02   ` Rob Herring
  2020-12-11 14:42 ` [PATCH v4 2/2] PCI: cadence: " Nadeem Athani
  1 sibling, 1 reply; 8+ messages in thread
From: Nadeem Athani @ 2020-12-11 14:42 UTC (permalink / raw)
  To: tjoseph, lorenzo.pieralisi, robh, bhelgaas, linux-pci,
	linux-kernel, kishon, devicetree
  Cc: nadeem, mparab, sjakhade, pthombar

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
 
   reg:
     maxItems: 2
-- 
2.15.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v4 2/2] PCI: cadence: Retrain Link to work around Gen2 training defect.
  2020-12-11 14:42 [PATCH v4 0/2] PCI: cadence: Retrain Link to work around Gen2 Nadeem Athani
  2020-12-11 14:42 ` [PATCH v4 1/2] dt-bindings: pci: Retrain Link to work around Gen2 training defect Nadeem Athani
@ 2020-12-11 14:42 ` Nadeem Athani
  1 sibling, 0 replies; 8+ messages in thread
From: Nadeem Athani @ 2020-12-11 14:42 UTC (permalink / raw)
  To: tjoseph, lorenzo.pieralisi, robh, bhelgaas, linux-pci,
	linux-kernel, kishon, devicetree
  Cc: nadeem, mparab, sjakhade, pthombar

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.

Signed-off-by: Nadeem Athani <nadeem@cadence.com>
---
 drivers/pci/controller/cadence/pcie-cadence-host.c | 67 ++++++++++++++++------
 drivers/pci/controller/cadence/pcie-cadence-plat.c | 13 +++++
 drivers/pci/controller/cadence/pcie-cadence.h      | 11 +++-
 3 files changed, 73 insertions(+), 18 deletions(-)

diff --git a/drivers/pci/controller/cadence/pcie-cadence-host.c b/drivers/pci/controller/cadence/pcie-cadence-host.c
index 811c1cb2e8de..36dccf7241fe 100644
--- a/drivers/pci/controller/cadence/pcie-cadence-host.c
+++ b/drivers/pci/controller/cadence/pcie-cadence-host.c
@@ -77,6 +77,53 @@ static struct pci_ops cdns_pcie_host_ops = {
 	.write		= pci_generic_config_write,
 };
 
+static int cdns_pcie_host_wait_for_link(struct cdns_pcie *pcie)
+{
+	struct device *dev = pcie->dev;
+	int retries;
+
+	/* Check if the link is up or not */
+	for (retries = 0; retries < LINK_WAIT_MAX_RETRIES; retries++) {
+		if (cdns_pcie_link_up(pcie)) {
+			dev_info(dev, "Link up\n");
+			return 0;
+		}
+		usleep_range(LINK_WAIT_USLEEP_MIN, LINK_WAIT_USLEEP_MAX);
+	}
+
+	return -ETIMEDOUT;
+}
+
+static void cdns_pcie_retrain(struct cdns_pcie *pcie)
+{
+	u32 lnk_cap_sls, pcie_cap_off = CDNS_PCIE_RP_CAP_OFFSET;
+	u16 lnk_stat, lnk_ctl;
+
+	if (cdns_pcie_host_wait_for_link(pcie))
+		return;
+
+	/*
+	 * Set retrain bit if current speed is 2.5 GB/s,
+	 * but the PCIe root port support is > 2.5 GB/s.
+	 */
+
+	lnk_cap_sls = cdns_pcie_readl(pcie, (CDNS_PCIE_RP_BASE + pcie_cap_off +
+					     PCI_EXP_LNKCAP));
+	if ((lnk_cap_sls & PCI_EXP_LNKCAP_SLS) <= PCI_EXP_LNKCAP_SLS_2_5GB)
+		return;
+
+	lnk_stat = cdns_pcie_rp_readw(pcie, pcie_cap_off + PCI_EXP_LNKSTA);
+	if ((lnk_stat & PCI_EXP_LNKSTA_CLS) == PCI_EXP_LNKSTA_CLS_2_5GB) {
+		lnk_ctl = cdns_pcie_rp_readw(pcie,
+					     pcie_cap_off + PCI_EXP_LNKCTL);
+		lnk_ctl |= PCI_EXP_LNKCTL_RL;
+		cdns_pcie_rp_writew(pcie, pcie_cap_off + PCI_EXP_LNKCTL,
+				    lnk_ctl);
+
+		if (cdns_pcie_host_wait_for_link(pcie))
+			return;
+	}
+}
 
 static int cdns_pcie_host_init_root_port(struct cdns_pcie_rc *rc)
 {
@@ -115,6 +162,9 @@ static int cdns_pcie_host_init_root_port(struct cdns_pcie_rc *rc)
 	cdns_pcie_rp_writeb(pcie, PCI_CLASS_PROG, 0);
 	cdns_pcie_rp_writew(pcie, PCI_CLASS_DEVICE, PCI_CLASS_BRIDGE_PCI);
 
+	if (rc->quirk_retrain_flag)
+		cdns_pcie_retrain(pcie);
+
 	return 0;
 }
 
@@ -398,23 +448,6 @@ static int cdns_pcie_host_init(struct device *dev,
 	return cdns_pcie_host_init_address_translation(rc);
 }
 
-static int cdns_pcie_host_wait_for_link(struct cdns_pcie *pcie)
-{
-	struct device *dev = pcie->dev;
-	int retries;
-
-	/* Check if the link is up or not */
-	for (retries = 0; retries < LINK_WAIT_MAX_RETRIES; retries++) {
-		if (cdns_pcie_link_up(pcie)) {
-			dev_info(dev, "Link up\n");
-			return 0;
-		}
-		usleep_range(LINK_WAIT_USLEEP_MIN, LINK_WAIT_USLEEP_MAX);
-	}
-
-	return -ETIMEDOUT;
-}
-
 int cdns_pcie_host_setup(struct cdns_pcie_rc *rc)
 {
 	struct device *dev = rc->pcie.dev;
diff --git a/drivers/pci/controller/cadence/pcie-cadence-plat.c b/drivers/pci/controller/cadence/pcie-cadence-plat.c
index 5fee0f89ab59..97b4b4f98fa4 100644
--- a/drivers/pci/controller/cadence/pcie-cadence-plat.c
+++ b/drivers/pci/controller/cadence/pcie-cadence-plat.c
@@ -28,6 +28,7 @@ struct cdns_plat_pcie {
 
 struct cdns_plat_pcie_of_data {
 	bool is_rc;
+	bool quirk_retrain_flag;
 };
 
 static const struct of_device_id cdns_plat_pcie_of_match[];
@@ -78,6 +79,7 @@ static int cdns_plat_pcie_probe(struct platform_device *pdev)
 		rc = pci_host_bridge_priv(bridge);
 		rc->pcie.dev = dev;
 		rc->pcie.ops = &cdns_plat_ops;
+		rc->quirk_retrain_flag = data->quirk_retrain_flag;
 		cdns_plat_pcie->pcie = &rc->pcie;
 		cdns_plat_pcie->is_rc = is_rc;
 
@@ -156,6 +158,13 @@ static void cdns_plat_pcie_shutdown(struct platform_device *pdev)
 
 static const struct cdns_plat_pcie_of_data cdns_plat_pcie_host_of_data = {
 	.is_rc = true,
+	.quirk_retrain_flag = false,
+};
+
+static const struct cdns_plat_pcie_of_data
+		    cdns_plat_pcie_host_quirk_retrain_of_data = {
+	.is_rc = true,
+	.quirk_retrain_flag = true,
 };
 
 static const struct cdns_plat_pcie_of_data cdns_plat_pcie_ep_of_data = {
@@ -167,6 +176,10 @@ static const struct of_device_id cdns_plat_pcie_of_match[] = {
 		.compatible = "cdns,cdns-pcie-host",
 		.data = &cdns_plat_pcie_host_of_data,
 	},
+	{
+		.compatible = "cdns,cdns-pcie-host-quirk-retrain",
+		.data = &cdns_plat_pcie_host_quirk_retrain_of_data,
+	},
 	{
 		.compatible = "cdns,cdns-pcie-ep",
 		.data = &cdns_plat_pcie_ep_of_data,
diff --git a/drivers/pci/controller/cadence/pcie-cadence.h b/drivers/pci/controller/cadence/pcie-cadence.h
index 30eba6cafe2c..0f29128a5d0a 100644
--- a/drivers/pci/controller/cadence/pcie-cadence.h
+++ b/drivers/pci/controller/cadence/pcie-cadence.h
@@ -119,7 +119,7 @@
  * Root Port Registers (PCI configuration space for the root port function)
  */
 #define CDNS_PCIE_RP_BASE	0x00200000
-
+#define CDNS_PCIE_RP_CAP_OFFSET 0xc0
 
 /*
  * Address Translation Registers
@@ -291,6 +291,7 @@ struct cdns_pcie {
  * @device_id: PCI device ID
  * @avail_ib_bar: Satus of RP_BAR0, RP_BAR1 and	RP_NO_BAR if it's free or
  *                available
+ * @quirk_retrain_flag: Retrain link as quirk for PCIe Gen2
  */
 struct cdns_pcie_rc {
 	struct cdns_pcie	pcie;
@@ -299,6 +300,7 @@ struct cdns_pcie_rc {
 	u32			vendor_id;
 	u32			device_id;
 	bool			avail_ib_bar[CDNS_PCIE_RP_MAX_IB];
+	bool			quirk_retrain_flag;
 };
 
 /**
@@ -414,6 +416,13 @@ static inline void cdns_pcie_rp_writew(struct cdns_pcie *pcie,
 	cdns_pcie_write_sz(addr, 0x2, value);
 }
 
+static inline u16 cdns_pcie_rp_readw(struct cdns_pcie *pcie, u32 reg)
+{
+	void __iomem *addr = pcie->reg_base + CDNS_PCIE_RP_BASE + reg;
+
+	return cdns_pcie_read_sz(addr, 0x2);
+}
+
 /* Endpoint Function register access */
 static inline void cdns_pcie_ep_fn_writeb(struct cdns_pcie *pcie, u8 fn,
 					  u32 reg, u8 value)
-- 
2.15.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 1/2] dt-bindings: pci: Retrain Link to work around Gen2 training defect.
  2020-12-11 14:42 ` [PATCH v4 1/2] dt-bindings: pci: Retrain Link to work around Gen2 training defect Nadeem Athani
@ 2020-12-11 17:02   ` Rob Herring
  2020-12-12  7:07     ` Athani Nadeem Ladkhan
  0 siblings, 1 reply; 8+ messages in thread
From: Rob Herring @ 2020-12-11 17:02 UTC (permalink / raw)
  To: Nadeem Athani
  Cc: Tom Joseph, Lorenzo Pieralisi, Bjorn Helgaas, PCI, linux-kernel,
	Kishon Vijay Abraham I, devicetree, Milind Parab,
	Swapnil Kashinath Jakhade, pthombar

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH v4 1/2] dt-bindings: pci: Retrain Link to work around Gen2 training defect.
  2020-12-11 17:02   ` Rob Herring
@ 2020-12-12  7:07     ` Athani Nadeem Ladkhan
  2020-12-14  4:20       ` Kishon Vijay Abraham I
  0 siblings, 1 reply; 8+ messages in thread
From: Athani Nadeem Ladkhan @ 2020-12-12  7:07 UTC (permalink / raw)
  To: Rob Herring, Kishon Vijay Abraham I
  Cc: Tom Joseph, Lorenzo Pieralisi, Bjorn Helgaas, PCI, linux-kernel,
	devicetree, Milind Parab, Swapnil Kashinath Jakhade,
	Parshuram Raju Thombare

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 1/2] dt-bindings: pci: Retrain Link to work around Gen2 training defect.
  2020-12-12  7:07     ` Athani Nadeem Ladkhan
@ 2020-12-14  4:20       ` Kishon Vijay Abraham I
  2020-12-14 15:05         ` Rob Herring
  0 siblings, 1 reply; 8+ messages in thread
From: Kishon Vijay Abraham I @ 2020-12-14  4:20 UTC (permalink / raw)
  To: Athani Nadeem Ladkhan, Rob Herring
  Cc: Tom Joseph, Lorenzo Pieralisi, Bjorn Helgaas, PCI, linux-kernel,
	devicetree, Milind Parab, Swapnil Kashinath Jakhade,
	Parshuram Raju Thombare

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 1/2] dt-bindings: pci: Retrain Link to work around Gen2 training defect.
  2020-12-14  4:20       ` Kishon Vijay Abraham I
@ 2020-12-14 15:05         ` Rob Herring
  2020-12-15  7:08           ` Kishon Vijay Abraham I
  0 siblings, 1 reply; 8+ messages in thread
From: Rob Herring @ 2020-12-14 15:05 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Athani Nadeem Ladkhan, Tom Joseph, Lorenzo Pieralisi,
	Bjorn Helgaas, PCI, linux-kernel, devicetree, Milind Parab,
	Swapnil Kashinath Jakhade, Parshuram Raju Thombare

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4 1/2] dt-bindings: pci: Retrain Link to work around Gen2 training defect.
  2020-12-14 15:05         ` Rob Herring
@ 2020-12-15  7:08           ` Kishon Vijay Abraham I
  0 siblings, 0 replies; 8+ messages in thread
From: Kishon Vijay Abraham I @ 2020-12-15  7:08 UTC (permalink / raw)
  To: Rob Herring
  Cc: Athani Nadeem Ladkhan, Tom Joseph, Lorenzo Pieralisi,
	Bjorn Helgaas, PCI, linux-kernel, devicetree, Milind Parab,
	Swapnil Kashinath Jakhade, Parshuram Raju Thombare

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-12-15  7:10 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-11 14:42 [PATCH v4 0/2] PCI: cadence: Retrain Link to work around Gen2 Nadeem Athani
2020-12-11 14:42 ` [PATCH v4 1/2] dt-bindings: pci: Retrain Link to work around Gen2 training defect Nadeem Athani
2020-12-11 17:02   ` Rob Herring
2020-12-12  7:07     ` Athani Nadeem Ladkhan
2020-12-14  4:20       ` Kishon Vijay Abraham I
2020-12-14 15:05         ` Rob Herring
2020-12-15  7:08           ` Kishon Vijay Abraham I
2020-12-11 14:42 ` [PATCH v4 2/2] PCI: cadence: " Nadeem Athani

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).