[PATCH v2 1/2] PCI: keystone: remove handle of PCI mode configuration

[PATCH v2 1/2] PCI: keystone: remove handle of PCI mode configuration

[Murali Karicheri]

Keystone PCI hardware supports both RC and EP modes and devcfg
register has bits to boot strap the device to either of these modes.
It seems proper to add this functionality to the boot loader rather
than in the driver as device will be operating in either mode, not
both any time. Currently the driver supports only RC mode and hence
register configuration in the driver is not needed and driver can
assume this is a RC hardware.

Also update the DT documentation accordingly.

Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
---
 - Added this separate patch to remove pci mode handling
 .../devicetree/bindings/pci/pci-keystone.txt       |    4 +---
 drivers/pci/host/pci-keystone.c                    |   21 ++------------------
 2 files changed, 3 insertions(+), 22 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/pci-keystone.txt b/Documentation/devicetree/bindings/pci/pci-keystone.txt
index ceb3e24..bedacf0 100644
--- a/Documentation/devicetree/bindings/pci/pci-keystone.txt
+++ b/Documentation/devicetree/bindings/pci/pci-keystone.txt
@@ -13,9 +13,7 @@ Required Properties:-
 
 compatibility: "ti,keystone-pcie"
 reg:	index 1 is the base address and length of DW application registers.
-	index 2 is the base address and length of PCI mode configuration
-	register.
-	index 3 is the base address and length of PCI device ID register.
+	index 2 is the base address and length of PCI device ID register.
 
 pcie_msi_intc : Interrupt controller device node for MSI IRQ chip
 	interrupt-cells: should be set to 1
diff --git a/drivers/pci/host/pci-keystone.c b/drivers/pci/host/pci-keystone.c
index ff8ed25..f1119eb 100644
--- a/drivers/pci/host/pci-keystone.c
+++ b/drivers/pci/host/pci-keystone.c
@@ -35,10 +35,6 @@
 #define MAX_MSI_HOST_IRQS		8
 #define MAX_LEGACY_HOST_IRQS		4
 
-/* RC mode settings masks */
-#define PCIE_RC_MODE		BIT(2)
-#define PCIE_MODE_MASK		(BIT(1) | BIT(2))
-
 /* DEV_STAT_CTRL */
 #define PCIE_CAP_BASE		0x70
 
@@ -355,7 +351,6 @@ static int __init ks_pcie_probe(struct platform_device *pdev)
 	void __iomem *reg_p;
 	struct phy *phy;
 	int ret = 0;
-	u32 val;
 
 	ks_pcie = devm_kzalloc(&pdev->dev, sizeof(*ks_pcie),
 				GFP_KERNEL);
@@ -365,18 +360,6 @@ static int __init ks_pcie_probe(struct platform_device *pdev)
 	}
 	pp = &ks_pcie->pp;
 
-	/* index 2 is the devcfg register for RC mode settings */
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
-	reg_p = devm_ioremap_resource(dev, res);
-	if (IS_ERR(reg_p))
-		return PTR_ERR(reg_p);
-
-	/* enable RC mode in devcfg */
-	val = readl(reg_p);
-	val &= ~PCIE_MODE_MASK;
-	val |= PCIE_RC_MODE;
-	writel(val, reg_p);
-
 	/* initialize SerDes Phy if present */
 	phy = devm_phy_get(dev, "pcie-phy");
 	if (!IS_ERR_OR_NULL(phy)) {
@@ -385,8 +368,8 @@ static int __init ks_pcie_probe(struct platform_device *pdev)
 			return ret;
 	}
 
-	/* index 3 is to read PCI DEVICE_ID */
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 3);
+	/* index 2 is to read PCI DEVICE_ID */
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
 	reg_p = devm_ioremap_resource(dev, res);
 	if (IS_ERR(reg_p))
 		return PTR_ERR(reg_p);
-- 
1.7.9.5

[PATCH v2 1/2] PCI: keystone: remove handle of PCI mode configuration

[Murali Karicheri]

Keystone PCI hardware supports both RC and EP modes and devcfg
register has bits to boot strap the device to either of these modes.
It seems proper to add this functionality to the boot loader rather
than in the driver as device will be operating in either mode, not
both any time. Currently the driver supports only RC mode and hence
register configuration in the driver is not needed and driver can
assume this is a RC hardware.

Also update the DT documentation accordingly.

Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
---
 - Added this separate patch to remove pci mode handling
 .../devicetree/bindings/pci/pci-keystone.txt       |    4 +---
 drivers/pci/host/pci-keystone.c                    |   21 ++------------------
 2 files changed, 3 insertions(+), 22 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/pci-keystone.txt b/Documentation/devicetree/bindings/pci/pci-keystone.txt
index ceb3e24..bedacf0 100644
--- a/Documentation/devicetree/bindings/pci/pci-keystone.txt
+++ b/Documentation/devicetree/bindings/pci/pci-keystone.txt
@@ -13,9 +13,7 @@ Required Properties:-
 
 compatibility: "ti,keystone-pcie"
 reg:	index 1 is the base address and length of DW application registers.
-	index 2 is the base address and length of PCI mode configuration
-	register.
-	index 3 is the base address and length of PCI device ID register.
+	index 2 is the base address and length of PCI device ID register.
 
 pcie_msi_intc : Interrupt controller device node for MSI IRQ chip
 	interrupt-cells: should be set to 1
diff --git a/drivers/pci/host/pci-keystone.c b/drivers/pci/host/pci-keystone.c
index ff8ed25..f1119eb 100644
--- a/drivers/pci/host/pci-keystone.c
+++ b/drivers/pci/host/pci-keystone.c
@@ -35,10 +35,6 @@
 #define MAX_MSI_HOST_IRQS		8
 #define MAX_LEGACY_HOST_IRQS		4
 
-/* RC mode settings masks */
-#define PCIE_RC_MODE		BIT(2)
-#define PCIE_MODE_MASK		(BIT(1) | BIT(2))
-
 /* DEV_STAT_CTRL */
 #define PCIE_CAP_BASE		0x70
 
@@ -355,7 +351,6 @@ static int __init ks_pcie_probe(struct platform_device *pdev)
 	void __iomem *reg_p;
 	struct phy *phy;
 	int ret = 0;
-	u32 val;
 
 	ks_pcie = devm_kzalloc(&pdev->dev, sizeof(*ks_pcie),
 				GFP_KERNEL);
@@ -365,18 +360,6 @@ static int __init ks_pcie_probe(struct platform_device *pdev)
 	}
 	pp = &ks_pcie->pp;
 
-	/* index 2 is the devcfg register for RC mode settings */
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
-	reg_p = devm_ioremap_resource(dev, res);
-	if (IS_ERR(reg_p))
-		return PTR_ERR(reg_p);
-
-	/* enable RC mode in devcfg */
-	val = readl(reg_p);
-	val &= ~PCIE_MODE_MASK;
-	val |= PCIE_RC_MODE;
-	writel(val, reg_p);
-
 	/* initialize SerDes Phy if present */
 	phy = devm_phy_get(dev, "pcie-phy");
 	if (!IS_ERR_OR_NULL(phy)) {
@@ -385,8 +368,8 @@ static int __init ks_pcie_probe(struct platform_device *pdev)
 			return ret;
 	}
 
-	/* index 3 is to read PCI DEVICE_ID */
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 3);
+	/* index 2 is to read PCI DEVICE_ID */
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
 	reg_p = devm_ioremap_resource(dev, res);
 	if (IS_ERR(reg_p))
 		return PTR_ERR(reg_p);
-- 
1.7.9.5

[PATCH v2 1/2] PCI: keystone: remove handle of PCI mode configuration

[Murali Karicheri]

Keystone PCI hardware supports both RC and EP modes and devcfg
register has bits to boot strap the device to either of these modes.
It seems proper to add this functionality to the boot loader rather
than in the driver as device will be operating in either mode, not
both any time. Currently the driver supports only RC mode and hence
register configuration in the driver is not needed and driver can
assume this is a RC hardware.

Also update the DT documentation accordingly.

Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
---
 - Added this separate patch to remove pci mode handling
 .../devicetree/bindings/pci/pci-keystone.txt       |    4 +---
 drivers/pci/host/pci-keystone.c                    |   21 ++------------------
 2 files changed, 3 insertions(+), 22 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/pci-keystone.txt b/Documentation/devicetree/bindings/pci/pci-keystone.txt
index ceb3e24..bedacf0 100644
--- a/Documentation/devicetree/bindings/pci/pci-keystone.txt
+++ b/Documentation/devicetree/bindings/pci/pci-keystone.txt
@@ -13,9 +13,7 @@ Required Properties:-
 
 compatibility: "ti,keystone-pcie"
 reg:	index 1 is the base address and length of DW application registers.
-	index 2 is the base address and length of PCI mode configuration
-	register.
-	index 3 is the base address and length of PCI device ID register.
+	index 2 is the base address and length of PCI device ID register.
 
 pcie_msi_intc : Interrupt controller device node for MSI IRQ chip
 	interrupt-cells: should be set to 1
diff --git a/drivers/pci/host/pci-keystone.c b/drivers/pci/host/pci-keystone.c
index ff8ed25..f1119eb 100644
--- a/drivers/pci/host/pci-keystone.c
+++ b/drivers/pci/host/pci-keystone.c
@@ -35,10 +35,6 @@
 #define MAX_MSI_HOST_IRQS		8
 #define MAX_LEGACY_HOST_IRQS		4
 
-/* RC mode settings masks */
-#define PCIE_RC_MODE		BIT(2)
-#define PCIE_MODE_MASK		(BIT(1) | BIT(2))
-
 /* DEV_STAT_CTRL */
 #define PCIE_CAP_BASE		0x70
 
@@ -355,7 +351,6 @@ static int __init ks_pcie_probe(struct platform_device *pdev)
 	void __iomem *reg_p;
 	struct phy *phy;
 	int ret = 0;
-	u32 val;
 
 	ks_pcie = devm_kzalloc(&pdev->dev, sizeof(*ks_pcie),
 				GFP_KERNEL);
@@ -365,18 +360,6 @@ static int __init ks_pcie_probe(struct platform_device *pdev)
 	}
 	pp = &ks_pcie->pp;
 
-	/* index 2 is the devcfg register for RC mode settings */
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
-	reg_p = devm_ioremap_resource(dev, res);
-	if (IS_ERR(reg_p))
-		return PTR_ERR(reg_p);
-
-	/* enable RC mode in devcfg */
-	val = readl(reg_p);
-	val &= ~PCIE_MODE_MASK;
-	val |= PCIE_RC_MODE;
-	writel(val, reg_p);
-
 	/* initialize SerDes Phy if present */
 	phy = devm_phy_get(dev, "pcie-phy");
 	if (!IS_ERR_OR_NULL(phy)) {
@@ -385,8 +368,8 @@ static int __init ks_pcie_probe(struct platform_device *pdev)
 			return ret;
 	}
 
-	/* index 3 is to read PCI DEVICE_ID */
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 3);
+	/* index 2 is to read PCI DEVICE_ID */
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
 	reg_p = devm_ioremap_resource(dev, res);
 	if (IS_ERR(reg_p))
 		return PTR_ERR(reg_p);
-- 
1.7.9.5

[PATCH v2 2/2] PCI: keystone: update to support multiple pci ports

[Murali Karicheri]

K2E SoC has two PCI ports. The SATA controller is connected to second
PCI port (port 1). This patch enhances the driver to support multiple
ports.

Update the DT Documentation for the new attribute, ti,pcie-port and
remove the note for bootargs as this is no longer needed.

Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
---
 - updated based on comment against v1
 .../devicetree/bindings/pci/pci-keystone.txt       |    8 +++++---
 drivers/pci/host/pci-keystone.c                    |   15 ++++++++++-----
 drivers/pci/host/pci-keystone.h                    |    4 ++--
 3 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/pci-keystone.txt b/Documentation/devicetree/bindings/pci/pci-keystone.txt
index bedacf0..c8f5773 100644
--- a/Documentation/devicetree/bindings/pci/pci-keystone.txt
+++ b/Documentation/devicetree/bindings/pci/pci-keystone.txt
@@ -56,11 +56,13 @@ Optional properties:-
 	phy-names: name of the Generic Keystine SerDes phy for PCI
 	  - If boot loader already does PCI link establishment, then phys and
 	    phy-names shouldn't be present.
+	ti,pcie-port: PCI port number. This is used to configure the PCI port
+	    number. For example K2E SoC supports 2 PCI ports and PCI bindings
+	    for the second port adds ti,pcie-port = <1> to identify second port
+	    and driver uses this to configure the PCI mode register for second
+	    port. If not present, port number 0 is assumed.
 
 Designware DT Properties not applicable for Keystone PCI
 
 1. pcie_bus clock-names not used.  Instead, a phandle to phys is used.
 
-Note for PCI driver usage
-=========================
-Driver requires pci=pcie_bus_perf in the bootargs for proper functioning.
diff --git a/drivers/pci/host/pci-keystone.c b/drivers/pci/host/pci-keystone.c
index f1119eb..893820d 100644
--- a/drivers/pci/host/pci-keystone.c
+++ b/drivers/pci/host/pci-keystone.c
@@ -253,8 +253,8 @@ static int keystone_pcie_fault(unsigned long addr, unsigned int fsr,
 
 static void __init ks_pcie_host_init(struct pcie_port *pp)
 {
-	u32 vendor_device_id, val;
 	struct keystone_pcie *ks_pcie = to_keystone_pcie(pp);
+	u32 val;
 
 	ks_pcie_establish_link(ks_pcie);
 	ks_dw_pcie_setup_rc_app_regs(ks_pcie);
@@ -263,8 +263,7 @@ static void __init ks_pcie_host_init(struct pcie_port *pp)
 			pp->dbi_base + PCI_IO_BASE);
 
 	/* update the Vendor ID */
-	vendor_device_id = readl(ks_pcie->va_reg_pciid);
-	writew((vendor_device_id >> 16), pp->dbi_base + PCI_DEVICE_ID);
+	writew(ks_pcie->device_id, pp->dbi_base + PCI_DEVICE_ID);
 
 	/* update the DEV_STAT_CTRL to publish right mrrs */
 	val = readl(pp->dbi_base + PCIE_CAP_BASE + PCI_EXP_DEVCTL);
@@ -345,12 +344,13 @@ static int __exit ks_pcie_remove(struct platform_device *pdev)
 static int __init ks_pcie_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
 	struct keystone_pcie *ks_pcie;
+	int ret = 0, port_id = 0;
 	struct pcie_port *pp;
 	struct resource *res;
 	void __iomem *reg_p;
 	struct phy *phy;
-	int ret = 0;
 
 	ks_pcie = devm_kzalloc(&pdev->dev, sizeof(*ks_pcie),
 				GFP_KERNEL);
@@ -360,6 +360,9 @@ static int __init ks_pcie_probe(struct platform_device *pdev)
 	}
 	pp = &ks_pcie->pp;
 
+	/* read the PCI port id */
+	of_property_read_u32(np, "ti,pcie-port", &port_id);
+
 	/* initialize SerDes Phy if present */
 	phy = devm_phy_get(dev, "pcie-phy");
 	if (!IS_ERR_OR_NULL(phy)) {
@@ -373,7 +376,9 @@ static int __init ks_pcie_probe(struct platform_device *pdev)
 	reg_p = devm_ioremap_resource(dev, res);
 	if (IS_ERR(reg_p))
 		return PTR_ERR(reg_p);
-	ks_pcie->va_reg_pciid = reg_p;
+	ks_pcie->device_id = readl(reg_p) >> 16;
+	devm_iounmap(dev, reg_p);
+	devm_release_mem_region(dev, res->start, resource_size(res));
 
 	pp->dev = dev;
 	platform_set_drvdata(pdev, ks_pcie);
diff --git a/drivers/pci/host/pci-keystone.h b/drivers/pci/host/pci-keystone.h
index 729ea7d..80cfa8e 100644
--- a/drivers/pci/host/pci-keystone.h
+++ b/drivers/pci/host/pci-keystone.h
@@ -19,8 +19,8 @@
 struct keystone_pcie {
 	struct	clk		*clk;
 	struct	pcie_port	pp;
-	void __iomem		*va_reg_pciid;
-
+	/* PCI Device ID */
+	u32			device_id;
 	int			num_legacy_host_irqs;
 	int			legacy_host_irqs[MAX_LEGACY_HOST_IRQS];
 	struct			device_node *legacy_intc_np;
-- 
1.7.9.5

[PATCH v2 2/2] PCI: keystone: update to support multiple pci ports

[Murali Karicheri]

K2E SoC has two PCI ports. The SATA controller is connected to second
PCI port (port 1). This patch enhances the driver to support multiple
ports.

Update the DT Documentation for the new attribute, ti,pcie-port and
remove the note for bootargs as this is no longer needed.

Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
---
 - updated based on comment against v1
 .../devicetree/bindings/pci/pci-keystone.txt       |    8 +++++---
 drivers/pci/host/pci-keystone.c                    |   15 ++++++++++-----
 drivers/pci/host/pci-keystone.h                    |    4 ++--
 3 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/pci-keystone.txt b/Documentation/devicetree/bindings/pci/pci-keystone.txt
index bedacf0..c8f5773 100644
--- a/Documentation/devicetree/bindings/pci/pci-keystone.txt
+++ b/Documentation/devicetree/bindings/pci/pci-keystone.txt
@@ -56,11 +56,13 @@ Optional properties:-
 	phy-names: name of the Generic Keystine SerDes phy for PCI
 	  - If boot loader already does PCI link establishment, then phys and
 	    phy-names shouldn't be present.
+	ti,pcie-port: PCI port number. This is used to configure the PCI port
+	    number. For example K2E SoC supports 2 PCI ports and PCI bindings
+	    for the second port adds ti,pcie-port = <1> to identify second port
+	    and driver uses this to configure the PCI mode register for second
+	    port. If not present, port number 0 is assumed.
 
 Designware DT Properties not applicable for Keystone PCI
 
 1. pcie_bus clock-names not used.  Instead, a phandle to phys is used.
 
-Note for PCI driver usage
-=========================
-Driver requires pci=pcie_bus_perf in the bootargs for proper functioning.
diff --git a/drivers/pci/host/pci-keystone.c b/drivers/pci/host/pci-keystone.c
index f1119eb..893820d 100644
--- a/drivers/pci/host/pci-keystone.c
+++ b/drivers/pci/host/pci-keystone.c
@@ -253,8 +253,8 @@ static int keystone_pcie_fault(unsigned long addr, unsigned int fsr,
 
 static void __init ks_pcie_host_init(struct pcie_port *pp)
 {
-	u32 vendor_device_id, val;
 	struct keystone_pcie *ks_pcie = to_keystone_pcie(pp);
+	u32 val;
 
 	ks_pcie_establish_link(ks_pcie);
 	ks_dw_pcie_setup_rc_app_regs(ks_pcie);
@@ -263,8 +263,7 @@ static void __init ks_pcie_host_init(struct pcie_port *pp)
 			pp->dbi_base + PCI_IO_BASE);
 
 	/* update the Vendor ID */
-	vendor_device_id = readl(ks_pcie->va_reg_pciid);
-	writew((vendor_device_id >> 16), pp->dbi_base + PCI_DEVICE_ID);
+	writew(ks_pcie->device_id, pp->dbi_base + PCI_DEVICE_ID);
 
 	/* update the DEV_STAT_CTRL to publish right mrrs */
 	val = readl(pp->dbi_base + PCIE_CAP_BASE + PCI_EXP_DEVCTL);
@@ -345,12 +344,13 @@ static int __exit ks_pcie_remove(struct platform_device *pdev)
 static int __init ks_pcie_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
 	struct keystone_pcie *ks_pcie;
+	int ret = 0, port_id = 0;
 	struct pcie_port *pp;
 	struct resource *res;
 	void __iomem *reg_p;
 	struct phy *phy;
-	int ret = 0;
 
 	ks_pcie = devm_kzalloc(&pdev->dev, sizeof(*ks_pcie),
 				GFP_KERNEL);
@@ -360,6 +360,9 @@ static int __init ks_pcie_probe(struct platform_device *pdev)
 	}
 	pp = &ks_pcie->pp;
 
+	/* read the PCI port id */
+	of_property_read_u32(np, "ti,pcie-port", &port_id);
+
 	/* initialize SerDes Phy if present */
 	phy = devm_phy_get(dev, "pcie-phy");
 	if (!IS_ERR_OR_NULL(phy)) {
@@ -373,7 +376,9 @@ static int __init ks_pcie_probe(struct platform_device *pdev)
 	reg_p = devm_ioremap_resource(dev, res);
 	if (IS_ERR(reg_p))
 		return PTR_ERR(reg_p);
-	ks_pcie->va_reg_pciid = reg_p;
+	ks_pcie->device_id = readl(reg_p) >> 16;
+	devm_iounmap(dev, reg_p);
+	devm_release_mem_region(dev, res->start, resource_size(res));
 
 	pp->dev = dev;
 	platform_set_drvdata(pdev, ks_pcie);
diff --git a/drivers/pci/host/pci-keystone.h b/drivers/pci/host/pci-keystone.h
index 729ea7d..80cfa8e 100644
--- a/drivers/pci/host/pci-keystone.h
+++ b/drivers/pci/host/pci-keystone.h
@@ -19,8 +19,8 @@
 struct keystone_pcie {
 	struct	clk		*clk;
 	struct	pcie_port	pp;
-	void __iomem		*va_reg_pciid;
-
+	/* PCI Device ID */
+	u32			device_id;
 	int			num_legacy_host_irqs;
 	int			legacy_host_irqs[MAX_LEGACY_HOST_IRQS];
 	struct			device_node *legacy_intc_np;
-- 
1.7.9.5

[PATCH v2 2/2] PCI: keystone: update to support multiple pci ports

[Murali Karicheri]

K2E SoC has two PCI ports. The SATA controller is connected to second
PCI port (port 1). This patch enhances the driver to support multiple
ports.

Update the DT Documentation for the new attribute, ti,pcie-port and
remove the note for bootargs as this is no longer needed.

Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
---
 - updated based on comment against v1
 .../devicetree/bindings/pci/pci-keystone.txt       |    8 +++++---
 drivers/pci/host/pci-keystone.c                    |   15 ++++++++++-----
 drivers/pci/host/pci-keystone.h                    |    4 ++--
 3 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/pci-keystone.txt b/Documentation/devicetree/bindings/pci/pci-keystone.txt
index bedacf0..c8f5773 100644
--- a/Documentation/devicetree/bindings/pci/pci-keystone.txt
+++ b/Documentation/devicetree/bindings/pci/pci-keystone.txt
@@ -56,11 +56,13 @@ Optional properties:-
 	phy-names: name of the Generic Keystine SerDes phy for PCI
 	  - If boot loader already does PCI link establishment, then phys and
 	    phy-names shouldn't be present.
+	ti,pcie-port: PCI port number. This is used to configure the PCI port
+	    number. For example K2E SoC supports 2 PCI ports and PCI bindings
+	    for the second port adds ti,pcie-port = <1> to identify second port
+	    and driver uses this to configure the PCI mode register for second
+	    port. If not present, port number 0 is assumed.
 
 Designware DT Properties not applicable for Keystone PCI
 
 1. pcie_bus clock-names not used.  Instead, a phandle to phys is used.
 
-Note for PCI driver usage
-=========================
-Driver requires pci=pcie_bus_perf in the bootargs for proper functioning.
diff --git a/drivers/pci/host/pci-keystone.c b/drivers/pci/host/pci-keystone.c
index f1119eb..893820d 100644
--- a/drivers/pci/host/pci-keystone.c
+++ b/drivers/pci/host/pci-keystone.c
@@ -253,8 +253,8 @@ static int keystone_pcie_fault(unsigned long addr, unsigned int fsr,
 
 static void __init ks_pcie_host_init(struct pcie_port *pp)
 {
-	u32 vendor_device_id, val;
 	struct keystone_pcie *ks_pcie = to_keystone_pcie(pp);
+	u32 val;
 
 	ks_pcie_establish_link(ks_pcie);
 	ks_dw_pcie_setup_rc_app_regs(ks_pcie);
@@ -263,8 +263,7 @@ static void __init ks_pcie_host_init(struct pcie_port *pp)
 			pp->dbi_base + PCI_IO_BASE);
 
 	/* update the Vendor ID */
-	vendor_device_id = readl(ks_pcie->va_reg_pciid);
-	writew((vendor_device_id >> 16), pp->dbi_base + PCI_DEVICE_ID);
+	writew(ks_pcie->device_id, pp->dbi_base + PCI_DEVICE_ID);
 
 	/* update the DEV_STAT_CTRL to publish right mrrs */
 	val = readl(pp->dbi_base + PCIE_CAP_BASE + PCI_EXP_DEVCTL);
@@ -345,12 +344,13 @@ static int __exit ks_pcie_remove(struct platform_device *pdev)
 static int __init ks_pcie_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
 	struct keystone_pcie *ks_pcie;
+	int ret = 0, port_id = 0;
 	struct pcie_port *pp;
 	struct resource *res;
 	void __iomem *reg_p;
 	struct phy *phy;
-	int ret = 0;
 
 	ks_pcie = devm_kzalloc(&pdev->dev, sizeof(*ks_pcie),
 				GFP_KERNEL);
@@ -360,6 +360,9 @@ static int __init ks_pcie_probe(struct platform_device *pdev)
 	}
 	pp = &ks_pcie->pp;
 
+	/* read the PCI port id */
+	of_property_read_u32(np, "ti,pcie-port", &port_id);
+
 	/* initialize SerDes Phy if present */
 	phy = devm_phy_get(dev, "pcie-phy");
 	if (!IS_ERR_OR_NULL(phy)) {
@@ -373,7 +376,9 @@ static int __init ks_pcie_probe(struct platform_device *pdev)
 	reg_p = devm_ioremap_resource(dev, res);
 	if (IS_ERR(reg_p))
 		return PTR_ERR(reg_p);
-	ks_pcie->va_reg_pciid = reg_p;
+	ks_pcie->device_id = readl(reg_p) >> 16;
+	devm_iounmap(dev, reg_p);
+	devm_release_mem_region(dev, res->start, resource_size(res));
 
 	pp->dev = dev;
 	platform_set_drvdata(pdev, ks_pcie);
diff --git a/drivers/pci/host/pci-keystone.h b/drivers/pci/host/pci-keystone.h
index 729ea7d..80cfa8e 100644
--- a/drivers/pci/host/pci-keystone.h
+++ b/drivers/pci/host/pci-keystone.h
@@ -19,8 +19,8 @@
 struct keystone_pcie {
 	struct	clk		*clk;
 	struct	pcie_port	pp;
-	void __iomem		*va_reg_pciid;
-
+	/* PCI Device ID */
+	u32			device_id;
 	int			num_legacy_host_irqs;
 	int			legacy_host_irqs[MAX_LEGACY_HOST_IRQS];
 	struct			device_node *legacy_intc_np;
-- 
1.7.9.5

Re: [PATCH v2 1/2] PCI: keystone: remove handle of PCI mode configuration

[Arnd Bergmann]

On Tuesday 09 September 2014 16:17:14 Murali Karicheri wrote:
> Keystone PCI hardware supports both RC and EP modes and devcfg
> register has bits to boot strap the device to either of these modes.
> It seems proper to add this functionality to the boot loader rather
> than in the driver as device will be operating in either mode, not
> both any time. Currently the driver supports only RC mode and hence
> register configuration in the driver is not needed and driver can
> assume this is a RC hardware.
> 
> Also update the DT documentation accordingly.
> 
> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
> 

Thanks for doing this,

Acked-by: Arnd Bergmann <arnd@arndb.de>

[PATCH v2 1/2] PCI: keystone: remove handle of PCI mode configuration

[Arnd Bergmann]

On Tuesday 09 September 2014 16:17:14 Murali Karicheri wrote:
> Keystone PCI hardware supports both RC and EP modes and devcfg
> register has bits to boot strap the device to either of these modes.
> It seems proper to add this functionality to the boot loader rather
> than in the driver as device will be operating in either mode, not
> both any time. Currently the driver supports only RC mode and hence
> register configuration in the driver is not needed and driver can
> assume this is a RC hardware.
> 
> Also update the DT documentation accordingly.
> 
> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
> 

Thanks for doing this,

Acked-by: Arnd Bergmann <arnd@arndb.de>

Re: [PATCH v2 2/2] PCI: keystone: update to support multiple pci ports

[Arnd Bergmann]

On Tuesday 09 September 2014 16:17:15 Murali Karicheri wrote:
> K2E SoC has two PCI ports. The SATA controller is connected to second
> PCI port (port 1). This patch enhances the driver to support multiple
> ports.
> 
> Update the DT Documentation for the new attribute, ti,pcie-port and
> remove the note for bootargs as this is no longer needed.
> 
> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
> ---
>  - updated based on comment against v1
>  .../devicetree/bindings/pci/pci-keystone.txt       |    8 +++++---
>  drivers/pci/host/pci-keystone.c                    |   15 ++++++++++-----
>  drivers/pci/host/pci-keystone.h                    |    4 ++--
>  3 files changed, 17 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/pci-keystone.txt b/Documentation/devicetree/bindings/pci/pci-keystone.txt
> index bedacf0..c8f5773 100644
> --- a/Documentation/devicetree/bindings/pci/pci-keystone.txt
> +++ b/Documentation/devicetree/bindings/pci/pci-keystone.txt
> @@ -56,11 +56,13 @@ Optional properties:-
>  	phy-names: name of the Generic Keystine SerDes phy for PCI
>  	  - If boot loader already does PCI link establishment, then phys and
>  	    phy-names shouldn't be present.
> +	ti,pcie-port: PCI port number. This is used to configure the PCI port
> +	    number. For example K2E SoC supports 2 PCI ports and PCI bindings
> +	    for the second port adds ti,pcie-port = <1> to identify second port
> +	    and driver uses this to configure the PCI mode register for second
> +	    port. If not present, port number 0 is assumed.

Is this still needed? I thought with patch 1 removed, we no longer had
a reason to know the port number.

> -=========================
> -Driver requires pci=pcie_bus_perf in the bootargs for proper functioning.
> diff --git a/drivers/pci/host/pci-keystone.c b/drivers/pci/host/pci-keystone.c
> index f1119eb..893820d 100644
> --- a/drivers/pci/host/pci-keystone.c
> +++ b/drivers/pci/host/pci-keystone.c
> @@ -253,8 +253,8 @@ static int keystone_pcie_fault(unsigned long addr, unsigned int fsr,
>  
>  static void __init ks_pcie_host_init(struct pcie_port *pp)
>  {
> -	u32 vendor_device_id, val;
>  	struct keystone_pcie *ks_pcie = to_keystone_pcie(pp);
> +	u32 val;
>  
>  	ks_pcie_establish_link(ks_pcie);
>  	ks_dw_pcie_setup_rc_app_regs(ks_pcie);
> @@ -263,8 +263,7 @@ static void __init ks_pcie_host_init(struct pcie_port *pp)
>  			pp->dbi_base + PCI_IO_BASE);
>  
>  	/* update the Vendor ID */
> -	vendor_device_id = readl(ks_pcie->va_reg_pciid);
> -	writew((vendor_device_id >> 16), pp->dbi_base + PCI_DEVICE_ID);
> +	writew(ks_pcie->device_id, pp->dbi_base + PCI_DEVICE_ID);
>  
>  	/* update the DEV_STAT_CTRL to publish right mrrs */
>  	val = readl(pp->dbi_base + PCIE_CAP_BASE + PCI_EXP_DEVCTL);

This change must have slipped in accidentally, at least it's not
described in the changelog. Should this be another patch?
The change seems useful.

	Arnd

[PATCH v2 2/2] PCI: keystone: update to support multiple pci ports

[Arnd Bergmann]

On Tuesday 09 September 2014 16:17:15 Murali Karicheri wrote:
> K2E SoC has two PCI ports. The SATA controller is connected to second
> PCI port (port 1). This patch enhances the driver to support multiple
> ports.
> 
> Update the DT Documentation for the new attribute, ti,pcie-port and
> remove the note for bootargs as this is no longer needed.
> 
> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
> ---
>  - updated based on comment against v1
>  .../devicetree/bindings/pci/pci-keystone.txt       |    8 +++++---
>  drivers/pci/host/pci-keystone.c                    |   15 ++++++++++-----
>  drivers/pci/host/pci-keystone.h                    |    4 ++--
>  3 files changed, 17 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/pci-keystone.txt b/Documentation/devicetree/bindings/pci/pci-keystone.txt
> index bedacf0..c8f5773 100644
> --- a/Documentation/devicetree/bindings/pci/pci-keystone.txt
> +++ b/Documentation/devicetree/bindings/pci/pci-keystone.txt
> @@ -56,11 +56,13 @@ Optional properties:-
>  	phy-names: name of the Generic Keystine SerDes phy for PCI
>  	  - If boot loader already does PCI link establishment, then phys and
>  	    phy-names shouldn't be present.
> +	ti,pcie-port: PCI port number. This is used to configure the PCI port
> +	    number. For example K2E SoC supports 2 PCI ports and PCI bindings
> +	    for the second port adds ti,pcie-port = <1> to identify second port
> +	    and driver uses this to configure the PCI mode register for second
> +	    port. If not present, port number 0 is assumed.

Is this still needed? I thought with patch 1 removed, we no longer had
a reason to know the port number.

> -=========================
> -Driver requires pci=pcie_bus_perf in the bootargs for proper functioning.
> diff --git a/drivers/pci/host/pci-keystone.c b/drivers/pci/host/pci-keystone.c
> index f1119eb..893820d 100644
> --- a/drivers/pci/host/pci-keystone.c
> +++ b/drivers/pci/host/pci-keystone.c
> @@ -253,8 +253,8 @@ static int keystone_pcie_fault(unsigned long addr, unsigned int fsr,
>  
>  static void __init ks_pcie_host_init(struct pcie_port *pp)
>  {
> -	u32 vendor_device_id, val;
>  	struct keystone_pcie *ks_pcie = to_keystone_pcie(pp);
> +	u32 val;
>  
>  	ks_pcie_establish_link(ks_pcie);
>  	ks_dw_pcie_setup_rc_app_regs(ks_pcie);
> @@ -263,8 +263,7 @@ static void __init ks_pcie_host_init(struct pcie_port *pp)
>  			pp->dbi_base + PCI_IO_BASE);
>  
>  	/* update the Vendor ID */
> -	vendor_device_id = readl(ks_pcie->va_reg_pciid);
> -	writew((vendor_device_id >> 16), pp->dbi_base + PCI_DEVICE_ID);
> +	writew(ks_pcie->device_id, pp->dbi_base + PCI_DEVICE_ID);
>  
>  	/* update the DEV_STAT_CTRL to publish right mrrs */
>  	val = readl(pp->dbi_base + PCIE_CAP_BASE + PCI_EXP_DEVCTL);

This change must have slipped in accidentally, at least it's not
described in the changelog. Should this be another patch?
The change seems useful.

	Arnd

Re: [PATCH v2 2/2] PCI: keystone: update to support multiple pci ports

[Murali Karicheri]

On 09/09/2014 04:28 PM, Arnd Bergmann wrote:
> On Tuesday 09 September 2014 16:17:15 Murali Karicheri wrote:
>> K2E SoC has two PCI ports. The SATA controller is connected to second
>> PCI port (port 1). This patch enhances the driver to support multiple
>> ports.
>>
>> Update the DT Documentation for the new attribute, ti,pcie-port and
>> remove the note for bootargs as this is no longer needed.
>>
>> Signed-off-by: Murali Karicheri<m-karicheri2@ti.com>
>> ---
>>   - updated based on comment against v1
>>   .../devicetree/bindings/pci/pci-keystone.txt       |    8 +++++---
>>   drivers/pci/host/pci-keystone.c                    |   15 ++++++++++-----
>>   drivers/pci/host/pci-keystone.h                    |    4 ++--
>>   3 files changed, 17 insertions(+), 10 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/pci-keystone.txt b/Documentation/devicetree/bindings/pci/pci-keystone.txt
>> index bedacf0..c8f5773 100644
>> --- a/Documentation/devicetree/bindings/pci/pci-keystone.txt
>> +++ b/Documentation/devicetree/bindings/pci/pci-keystone.txt
>> @@ -56,11 +56,13 @@ Optional properties:-
>>   	phy-names: name of the Generic Keystine SerDes phy for PCI
>>   	  - If boot loader already does PCI link establishment, then phys and
>>   	    phy-names shouldn't be present.
>> +	ti,pcie-port: PCI port number. This is used to configure the PCI port
>> +	    number. For example K2E SoC supports 2 PCI ports and PCI bindings
>> +	    for the second port adds ti,pcie-port =<1>  to identify second port
>> +	    and driver uses this to configure the PCI mode register for second
>> +	    port. If not present, port number 0 is assumed.
>
> Is this still needed? I thought with patch 1 removed, we no longer had
> a reason to know the port number.
>
You are right. I will remove it and re-send.

>> -=========================
>> -Driver requires pci=pcie_bus_perf in the bootargs for proper functioning.
>> diff --git a/drivers/pci/host/pci-keystone.c b/drivers/pci/host/pci-keystone.c
>> index f1119eb..893820d 100644
>> --- a/drivers/pci/host/pci-keystone.c
>> +++ b/drivers/pci/host/pci-keystone.c
>> @@ -253,8 +253,8 @@ static int keystone_pcie_fault(unsigned long addr, unsigned int fsr,
>>
>>   static void __init ks_pcie_host_init(struct pcie_port *pp)
>>   {
>> -	u32 vendor_device_id, val;
>>   	struct keystone_pcie *ks_pcie = to_keystone_pcie(pp);
>> +	u32 val;
>>
>>   	ks_pcie_establish_link(ks_pcie);
>>   	ks_dw_pcie_setup_rc_app_regs(ks_pcie);
>> @@ -263,8 +263,7 @@ static void __init ks_pcie_host_init(struct pcie_port *pp)
>>   			pp->dbi_base + PCI_IO_BASE);
>>
>>   	/* update the Vendor ID */
>> -	vendor_device_id = readl(ks_pcie->va_reg_pciid);
>> -	writew((vendor_device_id>>  16), pp->dbi_base + PCI_DEVICE_ID);
>> +	writew(ks_pcie->device_id, pp->dbi_base + PCI_DEVICE_ID);
>>
>>   	/* update the DEV_STAT_CTRL to publish right mrrs */
>>   	val = readl(pp->dbi_base + PCIE_CAP_BASE + PCI_EXP_DEVCTL);
>
> This change must have slipped in accidentally, at least it's not
> described in the changelog. Should this be another patch?
> The change seems useful.
Are you referring to mrrs or update to device id?  device id is in a SoC 
register at index2 and is read and updated by the driver here. MRRS 
update was originally in the code.

Murali
>
> 	Arnd

Re: [PATCH v2 2/2] PCI: keystone: update to support multiple pci ports

[Murali Karicheri]

On 09/09/2014 04:28 PM, Arnd Bergmann wrote:
> On Tuesday 09 September 2014 16:17:15 Murali Karicheri wrote:
>> K2E SoC has two PCI ports. The SATA controller is connected to second
>> PCI port (port 1). This patch enhances the driver to support multiple
>> ports.
>>
>> Update the DT Documentation for the new attribute, ti,pcie-port and
>> remove the note for bootargs as this is no longer needed.
>>
>> Signed-off-by: Murali Karicheri<m-karicheri2-l0cyMroinI0@public.gmane.org>
>> ---
>>   - updated based on comment against v1
>>   .../devicetree/bindings/pci/pci-keystone.txt       |    8 +++++---
>>   drivers/pci/host/pci-keystone.c                    |   15 ++++++++++-----
>>   drivers/pci/host/pci-keystone.h                    |    4 ++--
>>   3 files changed, 17 insertions(+), 10 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/pci-keystone.txt b/Documentation/devicetree/bindings/pci/pci-keystone.txt
>> index bedacf0..c8f5773 100644
>> --- a/Documentation/devicetree/bindings/pci/pci-keystone.txt
>> +++ b/Documentation/devicetree/bindings/pci/pci-keystone.txt
>> @@ -56,11 +56,13 @@ Optional properties:-
>>   	phy-names: name of the Generic Keystine SerDes phy for PCI
>>   	  - If boot loader already does PCI link establishment, then phys and
>>   	    phy-names shouldn't be present.
>> +	ti,pcie-port: PCI port number. This is used to configure the PCI port
>> +	    number. For example K2E SoC supports 2 PCI ports and PCI bindings
>> +	    for the second port adds ti,pcie-port =<1>  to identify second port
>> +	    and driver uses this to configure the PCI mode register for second
>> +	    port. If not present, port number 0 is assumed.
>
> Is this still needed? I thought with patch 1 removed, we no longer had
> a reason to know the port number.
>
You are right. I will remove it and re-send.

>> -=========================
>> -Driver requires pci=pcie_bus_perf in the bootargs for proper functioning.
>> diff --git a/drivers/pci/host/pci-keystone.c b/drivers/pci/host/pci-keystone.c
>> index f1119eb..893820d 100644
>> --- a/drivers/pci/host/pci-keystone.c
>> +++ b/drivers/pci/host/pci-keystone.c
>> @@ -253,8 +253,8 @@ static int keystone_pcie_fault(unsigned long addr, unsigned int fsr,
>>
>>   static void __init ks_pcie_host_init(struct pcie_port *pp)
>>   {
>> -	u32 vendor_device_id, val;
>>   	struct keystone_pcie *ks_pcie = to_keystone_pcie(pp);
>> +	u32 val;
>>
>>   	ks_pcie_establish_link(ks_pcie);
>>   	ks_dw_pcie_setup_rc_app_regs(ks_pcie);
>> @@ -263,8 +263,7 @@ static void __init ks_pcie_host_init(struct pcie_port *pp)
>>   			pp->dbi_base + PCI_IO_BASE);
>>
>>   	/* update the Vendor ID */
>> -	vendor_device_id = readl(ks_pcie->va_reg_pciid);
>> -	writew((vendor_device_id>>  16), pp->dbi_base + PCI_DEVICE_ID);
>> +	writew(ks_pcie->device_id, pp->dbi_base + PCI_DEVICE_ID);
>>
>>   	/* update the DEV_STAT_CTRL to publish right mrrs */
>>   	val = readl(pp->dbi_base + PCIE_CAP_BASE + PCI_EXP_DEVCTL);
>
> This change must have slipped in accidentally, at least it's not
> described in the changelog. Should this be another patch?
> The change seems useful.
Are you referring to mrrs or update to device id?  device id is in a SoC 
register at index2 and is read and updated by the driver here. MRRS 
update was originally in the code.

Murali
>
> 	Arnd

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/2] PCI: keystone: update to support multiple pci ports

[Murali Karicheri]

On 09/09/2014 04:28 PM, Arnd Bergmann wrote:
> On Tuesday 09 September 2014 16:17:15 Murali Karicheri wrote:
>> K2E SoC has two PCI ports. The SATA controller is connected to second
>> PCI port (port 1). This patch enhances the driver to support multiple
>> ports.
>>
>> Update the DT Documentation for the new attribute, ti,pcie-port and
>> remove the note for bootargs as this is no longer needed.
>>
>> Signed-off-by: Murali Karicheri<m-karicheri2@ti.com>
>> ---
>>   - updated based on comment against v1
>>   .../devicetree/bindings/pci/pci-keystone.txt       |    8 +++++---
>>   drivers/pci/host/pci-keystone.c                    |   15 ++++++++++-----
>>   drivers/pci/host/pci-keystone.h                    |    4 ++--
>>   3 files changed, 17 insertions(+), 10 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/pci/pci-keystone.txt b/Documentation/devicetree/bindings/pci/pci-keystone.txt
>> index bedacf0..c8f5773 100644
>> --- a/Documentation/devicetree/bindings/pci/pci-keystone.txt
>> +++ b/Documentation/devicetree/bindings/pci/pci-keystone.txt
>> @@ -56,11 +56,13 @@ Optional properties:-
>>   	phy-names: name of the Generic Keystine SerDes phy for PCI
>>   	  - If boot loader already does PCI link establishment, then phys and
>>   	    phy-names shouldn't be present.
>> +	ti,pcie-port: PCI port number. This is used to configure the PCI port
>> +	    number. For example K2E SoC supports 2 PCI ports and PCI bindings
>> +	    for the second port adds ti,pcie-port =<1>  to identify second port
>> +	    and driver uses this to configure the PCI mode register for second
>> +	    port. If not present, port number 0 is assumed.
>
> Is this still needed? I thought with patch 1 removed, we no longer had
> a reason to know the port number.
>
You are right. I will remove it and re-send.

>> -=========================
>> -Driver requires pci=pcie_bus_perf in the bootargs for proper functioning.
>> diff --git a/drivers/pci/host/pci-keystone.c b/drivers/pci/host/pci-keystone.c
>> index f1119eb..893820d 100644
>> --- a/drivers/pci/host/pci-keystone.c
>> +++ b/drivers/pci/host/pci-keystone.c
>> @@ -253,8 +253,8 @@ static int keystone_pcie_fault(unsigned long addr, unsigned int fsr,
>>
>>   static void __init ks_pcie_host_init(struct pcie_port *pp)
>>   {
>> -	u32 vendor_device_id, val;
>>   	struct keystone_pcie *ks_pcie = to_keystone_pcie(pp);
>> +	u32 val;
>>
>>   	ks_pcie_establish_link(ks_pcie);
>>   	ks_dw_pcie_setup_rc_app_regs(ks_pcie);
>> @@ -263,8 +263,7 @@ static void __init ks_pcie_host_init(struct pcie_port *pp)
>>   			pp->dbi_base + PCI_IO_BASE);
>>
>>   	/* update the Vendor ID */
>> -	vendor_device_id = readl(ks_pcie->va_reg_pciid);
>> -	writew((vendor_device_id>>  16), pp->dbi_base + PCI_DEVICE_ID);
>> +	writew(ks_pcie->device_id, pp->dbi_base + PCI_DEVICE_ID);
>>
>>   	/* update the DEV_STAT_CTRL to publish right mrrs */
>>   	val = readl(pp->dbi_base + PCIE_CAP_BASE + PCI_EXP_DEVCTL);
>
> This change must have slipped in accidentally, at least it's not
> described in the changelog. Should this be another patch?
> The change seems useful.
Are you referring to mrrs or update to device id?  device id is in a SoC 
register at index2 and is read and updated by the driver here. MRRS 
update was originally in the code.

Murali
>
> 	Arnd

Re: [PATCH v2 2/2] PCI: keystone: update to support multiple pci ports

[Arnd Bergmann]

On Tuesday 09 September 2014 16:42:46 Murali Karicheri wrote:
> >>
> >>      /* update the Vendor ID */
> >> -    vendor_device_id = readl(ks_pcie->va_reg_pciid);
> >> -    writew((vendor_device_id>>  16), pp->dbi_base + PCI_DEVICE_ID);
> >> +    writew(ks_pcie->device_id, pp->dbi_base + PCI_DEVICE_ID);
> >>
> >>      /* update the DEV_STAT_CTRL to publish right mrrs */
> >>      val = readl(pp->dbi_base + PCIE_CAP_BASE + PCI_EXP_DEVCTL);
> >
> > This change must have slipped in accidentally, at least it's not
> > described in the changelog. Should this be another patch?
> > The change seems useful.
> Are you referring to mrrs or update to device id?  device id is in a SoC 
> register at index2 and is read and updated by the driver here. MRRS 
> update was originally in the code.
> 

I meant the device id change. Maybe you accidentally did 'git commit
--amend' during a rebase and that replaced the real changelog with
the one of the patch in front of it and merged the two patches?

That happened to me a few times and would explain the strange mix
of two changes.

	Arnd

[PATCH v2 2/2] PCI: keystone: update to support multiple pci ports

[Arnd Bergmann]

On Tuesday 09 September 2014 16:42:46 Murali Karicheri wrote:
> >>
> >>      /* update the Vendor ID */
> >> -    vendor_device_id = readl(ks_pcie->va_reg_pciid);
> >> -    writew((vendor_device_id>>  16), pp->dbi_base + PCI_DEVICE_ID);
> >> +    writew(ks_pcie->device_id, pp->dbi_base + PCI_DEVICE_ID);
> >>
> >>      /* update the DEV_STAT_CTRL to publish right mrrs */
> >>      val = readl(pp->dbi_base + PCIE_CAP_BASE + PCI_EXP_DEVCTL);
> >
> > This change must have slipped in accidentally, at least it's not
> > described in the changelog. Should this be another patch?
> > The change seems useful.
> Are you referring to mrrs or update to device id?  device id is in a SoC 
> register at index2 and is read and updated by the driver here. MRRS 
> update was originally in the code.
> 

I meant the device id change. Maybe you accidentally did 'git commit
--amend' during a rebase and that replaced the real changelog with
the one of the patch in front of it and merged the two patches?

That happened to me a few times and would explain the strange mix
of two changes.

	Arnd

Re: [PATCH v2 2/2] PCI: keystone: update to support multiple pci ports

[Murali Karicheri]

On 09/09/2014 05:09 PM, Arnd Bergmann wrote:
> On Tuesday 09 September 2014 16:42:46 Murali Karicheri wrote:
>>>>
>>>>       /* update the Vendor ID */
>>>> -    vendor_device_id = readl(ks_pcie->va_reg_pciid);
>>>> -    writew((vendor_device_id>>   16), pp->dbi_base + PCI_DEVICE_ID);
>>>> +    writew(ks_pcie->device_id, pp->dbi_base + PCI_DEVICE_ID);
>>>>
>>>>       /* update the DEV_STAT_CTRL to publish right mrrs */
>>>>       val = readl(pp->dbi_base + PCIE_CAP_BASE + PCI_EXP_DEVCTL);
>>>
>>> This change must have slipped in accidentally, at least it's not
>>> described in the changelog. Should this be another patch?
>>> The change seems useful.
>> Are you referring to mrrs or update to device id?  device id is in a SoC
>> register at index2 and is read and updated by the driver here. MRRS
>> update was originally in the code.
>>
>
> I meant the device id change. Maybe you accidentally did 'git commit
> --amend' during a rebase and that replaced the real changelog with
> the one of the patch in front of it and merged the two patches?
>
Actually this is an inteded. The vendor ID is in a register indicated by
reg offset and as per the device spec, it needs to be read and updated 
by the software. Now since multiple instances of PCI device needs to be 
read the same register, the reading happens in the probe() and same is 
unmapped after that.

+	ks_pcie->device_id = readl(reg_p) >> 16;
+	devm_iounmap(dev, reg_p);
+	devm_release_mem_region(dev, res->start, resource_size(res));

Afetr that in ks_pcie_host_init(), it update the device_id in the RC's 
config space.

Thanks

Murali
> That happened to me a few times and would explain the strange mix
> of two changes.
>
> 	Arnd

Re: [PATCH v2 2/2] PCI: keystone: update to support multiple pci ports

[Murali Karicheri]

On 09/09/2014 05:09 PM, Arnd Bergmann wrote:
> On Tuesday 09 September 2014 16:42:46 Murali Karicheri wrote:
>>>>
>>>>       /* update the Vendor ID */
>>>> -    vendor_device_id = readl(ks_pcie->va_reg_pciid);
>>>> -    writew((vendor_device_id>>   16), pp->dbi_base + PCI_DEVICE_ID);
>>>> +    writew(ks_pcie->device_id, pp->dbi_base + PCI_DEVICE_ID);
>>>>
>>>>       /* update the DEV_STAT_CTRL to publish right mrrs */
>>>>       val = readl(pp->dbi_base + PCIE_CAP_BASE + PCI_EXP_DEVCTL);
>>>
>>> This change must have slipped in accidentally, at least it's not
>>> described in the changelog. Should this be another patch?
>>> The change seems useful.
>> Are you referring to mrrs or update to device id?  device id is in a SoC
>> register at index2 and is read and updated by the driver here. MRRS
>> update was originally in the code.
>>
>
> I meant the device id change. Maybe you accidentally did 'git commit
> --amend' during a rebase and that replaced the real changelog with
> the one of the patch in front of it and merged the two patches?
>
Actually this is an inteded. The vendor ID is in a register indicated by
reg offset and as per the device spec, it needs to be read and updated 
by the software. Now since multiple instances of PCI device needs to be 
read the same register, the reading happens in the probe() and same is 
unmapped after that.

+	ks_pcie->device_id = readl(reg_p) >> 16;
+	devm_iounmap(dev, reg_p);
+	devm_release_mem_region(dev, res->start, resource_size(res));

Afetr that in ks_pcie_host_init(), it update the device_id in the RC's 
config space.

Thanks

Murali
> That happened to me a few times and would explain the strange mix
> of two changes.
>
> 	Arnd

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/2] PCI: keystone: update to support multiple pci ports

[Murali Karicheri]

On 09/09/2014 05:09 PM, Arnd Bergmann wrote:
> On Tuesday 09 September 2014 16:42:46 Murali Karicheri wrote:
>>>>
>>>>       /* update the Vendor ID */
>>>> -    vendor_device_id = readl(ks_pcie->va_reg_pciid);
>>>> -    writew((vendor_device_id>>   16), pp->dbi_base + PCI_DEVICE_ID);
>>>> +    writew(ks_pcie->device_id, pp->dbi_base + PCI_DEVICE_ID);
>>>>
>>>>       /* update the DEV_STAT_CTRL to publish right mrrs */
>>>>       val = readl(pp->dbi_base + PCIE_CAP_BASE + PCI_EXP_DEVCTL);
>>>
>>> This change must have slipped in accidentally, at least it's not
>>> described in the changelog. Should this be another patch?
>>> The change seems useful.
>> Are you referring to mrrs or update to device id?  device id is in a SoC
>> register at index2 and is read and updated by the driver here. MRRS
>> update was originally in the code.
>>
>
> I meant the device id change. Maybe you accidentally did 'git commit
> --amend' during a rebase and that replaced the real changelog with
> the one of the patch in front of it and merged the two patches?
>
Actually this is an inteded. The vendor ID is in a register indicated by
reg offset and as per the device spec, it needs to be read and updated 
by the software. Now since multiple instances of PCI device needs to be 
read the same register, the reading happens in the probe() and same is 
unmapped after that.

+	ks_pcie->device_id = readl(reg_p) >> 16;
+	devm_iounmap(dev, reg_p);
+	devm_release_mem_region(dev, res->start, resource_size(res));

Afetr that in ks_pcie_host_init(), it update the device_id in the RC's 
config space.

Thanks

Murali
> That happened to me a few times and would explain the strange mix
> of two changes.
>
> 	Arnd

Re: [PATCH v2 2/2] PCI: keystone: update to support multiple pci ports

[Murali Karicheri]

On 09/09/2014 05:49 PM, Murali Karicheri wrote:
> On 09/09/2014 05:09 PM, Arnd Bergmann wrote:
>> On Tuesday 09 September 2014 16:42:46 Murali Karicheri wrote:
>>>>>
>>>>> /* update the Vendor ID */
>>>>> - vendor_device_id = readl(ks_pcie->va_reg_pciid);
>>>>> - writew((vendor_device_id>> 16), pp->dbi_base + PCI_DEVICE_ID);
>>>>> + writew(ks_pcie->device_id, pp->dbi_base + PCI_DEVICE_ID);
>>>>>
>>>>> /* update the DEV_STAT_CTRL to publish right mrrs */
>>>>> val = readl(pp->dbi_base + PCIE_CAP_BASE + PCI_EXP_DEVCTL);
>>>>
>>>> This change must have slipped in accidentally, at least it's not
>>>> described in the changelog. Should this be another patch?
>>>> The change seems useful.
>>> Are you referring to mrrs or update to device id? device id is in a SoC
>>> register at index2 and is read and updated by the driver here. MRRS
>>> update was originally in the code.
>>>
>>
>> I meant the device id change. Maybe you accidentally did 'git commit
>> --amend' during a rebase and that replaced the real changelog with
>> the one of the patch in front of it and merged the two patches?
>>
> Actually this is an inteded. The vendor ID is in a register indicated by
> reg offset and as per the device spec, it needs to be read and updated
> by the software. Now since multiple instances of PCI device needs to be
> read the same register, the reading happens in the probe() and same is
> unmapped after that.
>
> + ks_pcie->device_id = readl(reg_p) >> 16;
> + devm_iounmap(dev, reg_p);
> + devm_release_mem_region(dev, res->start, resource_size(res));
>
> Afetr that in ks_pcie_host_init(), it update the device_id in the RC's
> config space.
>
BTW, I will update the commit log with more description to indicate the 
above and re-send it if this is fine.

> Thanks
>
> Murali
>> That happened to me a few times and would explain the strange mix
>> of two changes.
>>
>> Arnd
>

Re: [PATCH v2 2/2] PCI: keystone: update to support multiple pci ports

[Murali Karicheri]

On 09/09/2014 05:49 PM, Murali Karicheri wrote:
> On 09/09/2014 05:09 PM, Arnd Bergmann wrote:
>> On Tuesday 09 September 2014 16:42:46 Murali Karicheri wrote:
>>>>>
>>>>> /* update the Vendor ID */
>>>>> - vendor_device_id = readl(ks_pcie->va_reg_pciid);
>>>>> - writew((vendor_device_id>> 16), pp->dbi_base + PCI_DEVICE_ID);
>>>>> + writew(ks_pcie->device_id, pp->dbi_base + PCI_DEVICE_ID);
>>>>>
>>>>> /* update the DEV_STAT_CTRL to publish right mrrs */
>>>>> val = readl(pp->dbi_base + PCIE_CAP_BASE + PCI_EXP_DEVCTL);
>>>>
>>>> This change must have slipped in accidentally, at least it's not
>>>> described in the changelog. Should this be another patch?
>>>> The change seems useful.
>>> Are you referring to mrrs or update to device id? device id is in a SoC
>>> register at index2 and is read and updated by the driver here. MRRS
>>> update was originally in the code.
>>>
>>
>> I meant the device id change. Maybe you accidentally did 'git commit
>> --amend' during a rebase and that replaced the real changelog with
>> the one of the patch in front of it and merged the two patches?
>>
> Actually this is an inteded. The vendor ID is in a register indicated by
> reg offset and as per the device spec, it needs to be read and updated
> by the software. Now since multiple instances of PCI device needs to be
> read the same register, the reading happens in the probe() and same is
> unmapped after that.
>
> + ks_pcie->device_id = readl(reg_p) >> 16;
> + devm_iounmap(dev, reg_p);
> + devm_release_mem_region(dev, res->start, resource_size(res));
>
> Afetr that in ks_pcie_host_init(), it update the device_id in the RC's
> config space.
>
BTW, I will update the commit log with more description to indicate the 
above and re-send it if this is fine.

> Thanks
>
> Murali
>> That happened to me a few times and would explain the strange mix
>> of two changes.
>>
>> Arnd
>

[PATCH v2 2/2] PCI: keystone: update to support multiple pci ports

[Murali Karicheri]

On 09/09/2014 05:49 PM, Murali Karicheri wrote:
> On 09/09/2014 05:09 PM, Arnd Bergmann wrote:
>> On Tuesday 09 September 2014 16:42:46 Murali Karicheri wrote:
>>>>>
>>>>> /* update the Vendor ID */
>>>>> - vendor_device_id = readl(ks_pcie->va_reg_pciid);
>>>>> - writew((vendor_device_id>> 16), pp->dbi_base + PCI_DEVICE_ID);
>>>>> + writew(ks_pcie->device_id, pp->dbi_base + PCI_DEVICE_ID);
>>>>>
>>>>> /* update the DEV_STAT_CTRL to publish right mrrs */
>>>>> val = readl(pp->dbi_base + PCIE_CAP_BASE + PCI_EXP_DEVCTL);
>>>>
>>>> This change must have slipped in accidentally, at least it's not
>>>> described in the changelog. Should this be another patch?
>>>> The change seems useful.
>>> Are you referring to mrrs or update to device id? device id is in a SoC
>>> register at index2 and is read and updated by the driver here. MRRS
>>> update was originally in the code.
>>>
>>
>> I meant the device id change. Maybe you accidentally did 'git commit
>> --amend' during a rebase and that replaced the real changelog with
>> the one of the patch in front of it and merged the two patches?
>>
> Actually this is an inteded. The vendor ID is in a register indicated by
> reg offset and as per the device spec, it needs to be read and updated
> by the software. Now since multiple instances of PCI device needs to be
> read the same register, the reading happens in the probe() and same is
> unmapped after that.
>
> + ks_pcie->device_id = readl(reg_p) >> 16;
> + devm_iounmap(dev, reg_p);
> + devm_release_mem_region(dev, res->start, resource_size(res));
>
> Afetr that in ks_pcie_host_init(), it update the device_id in the RC's
> config space.
>
BTW, I will update the commit log with more description to indicate the 
above and re-send it if this is fine.

> Thanks
>
> Murali
>> That happened to me a few times and would explain the strange mix
>> of two changes.
>>
>> Arnd
>

Re: [PATCH v2 2/2] PCI: keystone: update to support multiple pci ports

[Arnd Bergmann]

On Tuesday 09 September 2014 17:49:19 Murali Karicheri wrote:
> Actually this is an inteded. The vendor ID is in a register indicated by
> reg offset and as per the device spec, it needs to be read and updated 
> by the software. Now since multiple instances of PCI device needs to be 
> read the same register, the reading happens in the probe() and same is 
> unmapped after that.
> 
> +       ks_pcie->device_id = readl(reg_p) >> 16;
> +       devm_iounmap(dev, reg_p);
> +       devm_release_mem_region(dev, res->start, resource_size(res));
> 
> Afetr that in ks_pcie_host_init(), it update the device_id in the RC's 
> config space.

I'm not sure I understand the purpose of this. Do you mean you read
the vendor/device ID of whichever device happens to get probed first
and then program the same ID into all other devices as well?

What if the order changes between two boots? Why does the vendor/device
ID of the host bridge even matter at all?

	Arnd

Re: [PATCH v2 2/2] PCI: keystone: update to support multiple pci ports

[Arnd Bergmann]

On Tuesday 09 September 2014 17:49:19 Murali Karicheri wrote:
> Actually this is an inteded. The vendor ID is in a register indicated by
> reg offset and as per the device spec, it needs to be read and updated 
> by the software. Now since multiple instances of PCI device needs to be 
> read the same register, the reading happens in the probe() and same is 
> unmapped after that.
> 
> +       ks_pcie->device_id = readl(reg_p) >> 16;
> +       devm_iounmap(dev, reg_p);
> +       devm_release_mem_region(dev, res->start, resource_size(res));
> 
> Afetr that in ks_pcie_host_init(), it update the device_id in the RC's 
> config space.

I'm not sure I understand the purpose of this. Do you mean you read
the vendor/device ID of whichever device happens to get probed first
and then program the same ID into all other devices as well?

What if the order changes between two boots? Why does the vendor/device
ID of the host bridge even matter at all?

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/2] PCI: keystone: update to support multiple pci ports

[Arnd Bergmann]

On Tuesday 09 September 2014 17:49:19 Murali Karicheri wrote:
> Actually this is an inteded. The vendor ID is in a register indicated by
> reg offset and as per the device spec, it needs to be read and updated 
> by the software. Now since multiple instances of PCI device needs to be 
> read the same register, the reading happens in the probe() and same is 
> unmapped after that.
> 
> +       ks_pcie->device_id = readl(reg_p) >> 16;
> +       devm_iounmap(dev, reg_p);
> +       devm_release_mem_region(dev, res->start, resource_size(res));
> 
> Afetr that in ks_pcie_host_init(), it update the device_id in the RC's 
> config space.

I'm not sure I understand the purpose of this. Do you mean you read
the vendor/device ID of whichever device happens to get probed first
and then program the same ID into all other devices as well?

What if the order changes between two boots? Why does the vendor/device
ID of the host bridge even matter at all?

	Arnd

Re: [PATCH v2 2/2] PCI: keystone: update to support multiple pci ports

[Murali Karicheri]

On 09/09/2014 05:52 PM, Arnd Bergmann wrote:
> On Tuesday 09 September 2014 17:49:19 Murali Karicheri wrote:
>> Actually this is an inteded. The vendor ID is in a register indicated by
>> reg offset and as per the device spec, it needs to be read and updated
>> by the software. Now since multiple instances of PCI device needs to be
>> read the same register, the reading happens in the probe() and same is
>> unmapped after that.
>>
>> +       ks_pcie->device_id = readl(reg_p)>>  16;
>> +       devm_iounmap(dev, reg_p);
>> +       devm_release_mem_region(dev, res->start, resource_size(res));
>>
>> Afetr that in ks_pcie_host_init(), it update the device_id in the RC's
>> config space.
>
> I'm not sure I understand the purpose of this. Do you mean you read
> the vendor/device ID of whichever device happens to get probed first
> and then program the same ID into all other devices as well?

My mistake. It is the device ID, not vendor ID. The PCI driver supports 
PCI h/w on K2HK, K2E and K2L SoCs for which PCI device IDs are assigned as

+#define PCIE_RC_K2HK		0xb008
+#define PCIE_RC_K2E		0xb009
+#define PCIE_RC_K2L		0xb00a
+

and the same driver code runs on all these h/w. The device ID is not 
filled in by default by the h/w, in the config space of the RC at offset 
1000h "VENDOR_DEVICE_ID Vendor and Device Identification Register". 
Same is available in a seperate SoC register whose offset is specified 
by index 2. This is read by driver and updated in the config space. The 
Vendor ID is however set by default.

There is a mrrs PCI quirk required for Keystone PCI which depends on 
this vendor ID to be filled correctly as it match vendor id/ device id 
of the bridge device to apply the quirk.

Hope this clarify your question.

Murali

>
> What if the order changes between two boots? Why does the vendor/device
> ID of the host bridge even matter at all?
>
> 	Arnd

Re: [PATCH v2 2/2] PCI: keystone: update to support multiple pci ports

[Murali Karicheri]

On 09/09/2014 05:52 PM, Arnd Bergmann wrote:
> On Tuesday 09 September 2014 17:49:19 Murali Karicheri wrote:
>> Actually this is an inteded. The vendor ID is in a register indicated by
>> reg offset and as per the device spec, it needs to be read and updated
>> by the software. Now since multiple instances of PCI device needs to be
>> read the same register, the reading happens in the probe() and same is
>> unmapped after that.
>>
>> +       ks_pcie->device_id = readl(reg_p)>>  16;
>> +       devm_iounmap(dev, reg_p);
>> +       devm_release_mem_region(dev, res->start, resource_size(res));
>>
>> Afetr that in ks_pcie_host_init(), it update the device_id in the RC's
>> config space.
>
> I'm not sure I understand the purpose of this. Do you mean you read
> the vendor/device ID of whichever device happens to get probed first
> and then program the same ID into all other devices as well?

My mistake. It is the device ID, not vendor ID. The PCI driver supports 
PCI h/w on K2HK, K2E and K2L SoCs for which PCI device IDs are assigned as

+#define PCIE_RC_K2HK		0xb008
+#define PCIE_RC_K2E		0xb009
+#define PCIE_RC_K2L		0xb00a
+

and the same driver code runs on all these h/w. The device ID is not 
filled in by default by the h/w, in the config space of the RC at offset 
1000h "VENDOR_DEVICE_ID Vendor and Device Identification Register". 
Same is available in a seperate SoC register whose offset is specified 
by index 2. This is read by driver and updated in the config space. The 
Vendor ID is however set by default.

There is a mrrs PCI quirk required for Keystone PCI which depends on 
this vendor ID to be filled correctly as it match vendor id/ device id 
of the bridge device to apply the quirk.

Hope this clarify your question.

Murali

>
> What if the order changes between two boots? Why does the vendor/device
> ID of the host bridge even matter at all?
>
> 	Arnd

[PATCH v2 2/2] PCI: keystone: update to support multiple pci ports

[Murali Karicheri]

On 09/09/2014 05:52 PM, Arnd Bergmann wrote:
> On Tuesday 09 September 2014 17:49:19 Murali Karicheri wrote:
>> Actually this is an inteded. The vendor ID is in a register indicated by
>> reg offset and as per the device spec, it needs to be read and updated
>> by the software. Now since multiple instances of PCI device needs to be
>> read the same register, the reading happens in the probe() and same is
>> unmapped after that.
>>
>> +       ks_pcie->device_id = readl(reg_p)>>  16;
>> +       devm_iounmap(dev, reg_p);
>> +       devm_release_mem_region(dev, res->start, resource_size(res));
>>
>> Afetr that in ks_pcie_host_init(), it update the device_id in the RC's
>> config space.
>
> I'm not sure I understand the purpose of this. Do you mean you read
> the vendor/device ID of whichever device happens to get probed first
> and then program the same ID into all other devices as well?

My mistake. It is the device ID, not vendor ID. The PCI driver supports 
PCI h/w on K2HK, K2E and K2L SoCs for which PCI device IDs are assigned as

+#define PCIE_RC_K2HK		0xb008
+#define PCIE_RC_K2E		0xb009
+#define PCIE_RC_K2L		0xb00a
+

and the same driver code runs on all these h/w. The device ID is not 
filled in by default by the h/w, in the config space of the RC at offset 
1000h "VENDOR_DEVICE_ID Vendor and Device Identification Register". 
Same is available in a seperate SoC register whose offset is specified 
by index 2. This is read by driver and updated in the config space. The 
Vendor ID is however set by default.

There is a mrrs PCI quirk required for Keystone PCI which depends on 
this vendor ID to be filled correctly as it match vendor id/ device id 
of the bridge device to apply the quirk.

Hope this clarify your question.

Murali

>
> What if the order changes between two boots? Why does the vendor/device
> ID of the host bridge even matter at all?
>
> 	Arnd

Re: [PATCH v2 2/2] PCI: keystone: update to support multiple pci ports

[Arnd Bergmann]

On Tuesday 09 September 2014 18:50:13 Murali Karicheri wrote:
> My mistake. It is the device ID, not vendor ID. The PCI driver supports 
> PCI h/w on K2HK, K2E and K2L SoCs for which PCI device IDs are assigned as
> 
> +#define PCIE_RC_K2HK           0xb008
> +#define PCIE_RC_K2E            0xb009
> +#define PCIE_RC_K2L            0xb00a
> +
> 
> and the same driver code runs on all these h/w. The device ID is not 
> filled in by default by the h/w, in the config space of the RC at offset 
> 1000h "VENDOR_DEVICE_ID Vendor and Device Identification Register". 
> Same is available in a seperate SoC register whose offset is specified 
> by index 2. This is read by driver and updated in the config space. The 
> Vendor ID is however set by default.
> 
> There is a mrrs PCI quirk required for Keystone PCI which depends on 
> this vendor ID to be filled correctly as it match vendor id/ device id 
> of the bridge device to apply the quirk.
> 
> Hope this clarify your question.

Ok, I understand now. Yes, this makes sense, though I wonder if it would
have been easier to handle the quirk in a different way based on the
driver rather than the PCI ID. It's probably not worth revisiting though,
unless Bjorn wants to see it done differently now.

	Arnd

[PATCH v2 2/2] PCI: keystone: update to support multiple pci ports

[Arnd Bergmann]

On Tuesday 09 September 2014 18:50:13 Murali Karicheri wrote:
> My mistake. It is the device ID, not vendor ID. The PCI driver supports 
> PCI h/w on K2HK, K2E and K2L SoCs for which PCI device IDs are assigned as
> 
> +#define PCIE_RC_K2HK           0xb008
> +#define PCIE_RC_K2E            0xb009
> +#define PCIE_RC_K2L            0xb00a
> +
> 
> and the same driver code runs on all these h/w. The device ID is not 
> filled in by default by the h/w, in the config space of the RC at offset 
> 1000h "VENDOR_DEVICE_ID Vendor and Device Identification Register". 
> Same is available in a seperate SoC register whose offset is specified 
> by index 2. This is read by driver and updated in the config space. The 
> Vendor ID is however set by default.
> 
> There is a mrrs PCI quirk required for Keystone PCI which depends on 
> this vendor ID to be filled correctly as it match vendor id/ device id 
> of the bridge device to apply the quirk.
> 
> Hope this clarify your question.

Ok, I understand now. Yes, this makes sense, though I wonder if it would
have been easier to handle the quirk in a different way based on the
driver rather than the PCI ID. It's probably not worth revisiting though,
unless Bjorn wants to see it done differently now.

	Arnd

Re: [PATCH v2 2/2] PCI: keystone: update to support multiple pci ports

[Murali Karicheri]

On 09/10/2014 04:22 AM, Arnd Bergmann wrote:
> On Tuesday 09 September 2014 18:50:13 Murali Karicheri wrote:
>> My mistake. It is the device ID, not vendor ID. The PCI driver supports
>> PCI h/w on K2HK, K2E and K2L SoCs for which PCI device IDs are assigned as
>>
>> +#define PCIE_RC_K2HK           0xb008
>> +#define PCIE_RC_K2E            0xb009
>> +#define PCIE_RC_K2L            0xb00a
>> +
>>
>> and the same driver code runs on all these h/w. The device ID is not
>> filled in by default by the h/w, in the config space of the RC at offset
>> 1000h "VENDOR_DEVICE_ID Vendor and Device Identification Register".
>> Same is available in a seperate SoC register whose offset is specified
>> by index 2. This is read by driver and updated in the config space. The
>> Vendor ID is however set by default.
>>
>> There is a mrrs PCI quirk required for Keystone PCI which depends on
>> this vendor ID to be filled correctly as it match vendor id/ device id
>> of the bridge device to apply the quirk.
>>
>> Hope this clarify your question.
>
> Ok, I understand now. Yes, this makes sense, though I wonder if it would
> have been easier to handle the quirk in a different way based on the
> driver rather than the PCI ID. It's probably not worth revisiting though,
> unless Bjorn wants to see it done differently now.

Bjorn has reviewed the quirk patch and wanted to check the device ID and 
vendor ID to avoid applying the quirck when bridge is non TI and same 
device ID.

So support for two PCI port requires only the PCI device id related 
change. So I will modify the subject and commit log to reflect the same 
and re-send the patch.

Thanks again Arnd for your comments.

Regards,

Murali
>
> 	Arnd
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] PCI: keystone: update to support multiple pci ports

[Murali Karicheri]

On 09/10/2014 04:22 AM, Arnd Bergmann wrote:
> On Tuesday 09 September 2014 18:50:13 Murali Karicheri wrote:
>> My mistake. It is the device ID, not vendor ID. The PCI driver supports
>> PCI h/w on K2HK, K2E and K2L SoCs for which PCI device IDs are assigned as
>>
>> +#define PCIE_RC_K2HK           0xb008
>> +#define PCIE_RC_K2E            0xb009
>> +#define PCIE_RC_K2L            0xb00a
>> +
>>
>> and the same driver code runs on all these h/w. The device ID is not
>> filled in by default by the h/w, in the config space of the RC at offset
>> 1000h "VENDOR_DEVICE_ID Vendor and Device Identification Register".
>> Same is available in a seperate SoC register whose offset is specified
>> by index 2. This is read by driver and updated in the config space. The
>> Vendor ID is however set by default.
>>
>> There is a mrrs PCI quirk required for Keystone PCI which depends on
>> this vendor ID to be filled correctly as it match vendor id/ device id
>> of the bridge device to apply the quirk.
>>
>> Hope this clarify your question.
>
> Ok, I understand now. Yes, this makes sense, though I wonder if it would
> have been easier to handle the quirk in a different way based on the
> driver rather than the PCI ID. It's probably not worth revisiting though,
> unless Bjorn wants to see it done differently now.

Bjorn has reviewed the quirk patch and wanted to check the device ID and 
vendor ID to avoid applying the quirck when bridge is non TI and same 
device ID.

So support for two PCI port requires only the PCI device id related 
change. So I will modify the subject and commit log to reflect the same 
and re-send the patch.

Thanks again Arnd for your comments.

Regards,

Murali
>
> 	Arnd
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 2/2] PCI: keystone: update to support multiple pci ports

[Murali Karicheri]

On 09/10/2014 04:22 AM, Arnd Bergmann wrote:
> On Tuesday 09 September 2014 18:50:13 Murali Karicheri wrote:
>> My mistake. It is the device ID, not vendor ID. The PCI driver supports
>> PCI h/w on K2HK, K2E and K2L SoCs for which PCI device IDs are assigned as
>>
>> +#define PCIE_RC_K2HK           0xb008
>> +#define PCIE_RC_K2E            0xb009
>> +#define PCIE_RC_K2L            0xb00a
>> +
>>
>> and the same driver code runs on all these h/w. The device ID is not
>> filled in by default by the h/w, in the config space of the RC at offset
>> 1000h "VENDOR_DEVICE_ID Vendor and Device Identification Register".
>> Same is available in a seperate SoC register whose offset is specified
>> by index 2. This is read by driver and updated in the config space. The
>> Vendor ID is however set by default.
>>
>> There is a mrrs PCI quirk required for Keystone PCI which depends on
>> this vendor ID to be filled correctly as it match vendor id/ device id
>> of the bridge device to apply the quirk.
>>
>> Hope this clarify your question.
>
> Ok, I understand now. Yes, this makes sense, though I wonder if it would
> have been easier to handle the quirk in a different way based on the
> driver rather than the PCI ID. It's probably not worth revisiting though,
> unless Bjorn wants to see it done differently now.

Bjorn has reviewed the quirk patch and wanted to check the device ID and 
vendor ID to avoid applying the quirck when bridge is non TI and same 
device ID.

So support for two PCI port requires only the PCI device id related 
change. So I will modify the subject and commit log to reflect the same 
and re-send the patch.

Thanks again Arnd for your comments.

Regards,

Murali
>
> 	Arnd
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html