diff mbox series

[v3,14/26] PCI: keystone: Add support for PCIe RC in AM654x Platforms

Message ID 20190325093947.32633-15-kishon@ti.com (mailing list archive)
State New, archived
Headers show
Series Add support for PCIe RC and EP mode in TI's AM654 SoC | expand

Commit Message

Kishon Vijay Abraham I March 25, 2019, 9:39 a.m. UTC
Add PCIe RC support for AM654x Platforms in pci-keystone.c

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
 drivers/pci/controller/dwc/Kconfig        |   2 +-
 drivers/pci/controller/dwc/pci-keystone.c | 161 ++++++++++++++++++++--
 2 files changed, 148 insertions(+), 15 deletions(-)

Comments

Bjorn Helgaas April 13, 2019, 3:26 p.m. UTC | #1
On Mon, Mar 25, 2019 at 03:09:35PM +0530, Kishon Vijay Abraham I wrote:
> Add PCIe RC support for AM654x Platforms in pci-keystone.c

> +static int ks_pcie_am654_msi_host_init(struct pcie_port *pp)
> +{
> +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +	struct device *dev = pci->dev;
> +
> +	dev_vdbg(dev, "dummy function so that DW core doesn't configure MSI\n");

Drive-by nit: this is the first occurrence of dev_vdbg() in
drivers/pci, and while dev_vdbg() might be useful somewhere, I'm not
sure this is the place.

It took me 10 minutes of archaeology to figure out what it is and how
to use it.  Apparently it does nothing at all unless you define
VERBOSE_DEBUG, which can be set via Kconfig in a few cases:

  drivers/dma/Makefile:subdir-ccflags-$(CONFIG_DMADEVICES_VDEBUG) += -DVERBOSE_DEBUG
  drivers/usb/dwc2/Makefile:ccflags-$(CONFIG_USB_DWC2_VERBOSE)    += -DVERBOSE_DEBUG
  drivers/usb/gadget/Makefile:subdir-ccflags-$(CONFIG_USB_GADGET_VERBOSE) += -DVERBOSE_DEBUG

but is more commonly set by adding "#define VERBOSE_DEBUG" manually
in a source file.  Neither one applies here, so I don't think this
actually does anything.

Anyway, I suspect this particular message would make a lot of sense as
a *comment*, but maybe not as a mysterious dev_vdbg() that never
actually does anything.

> +	return 0;
> +}
Lorenzo Pieralisi April 16, 2019, 2:15 p.m. UTC | #2
On Sat, Apr 13, 2019 at 10:26:33AM -0500, Bjorn Helgaas wrote:
> On Mon, Mar 25, 2019 at 03:09:35PM +0530, Kishon Vijay Abraham I wrote:
> > Add PCIe RC support for AM654x Platforms in pci-keystone.c
> 
> > +static int ks_pcie_am654_msi_host_init(struct pcie_port *pp)
> > +{
> > +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > +	struct device *dev = pci->dev;
> > +
> > +	dev_vdbg(dev, "dummy function so that DW core doesn't configure MSI\n");
> 
> Drive-by nit: this is the first occurrence of dev_vdbg() in
> drivers/pci, and while dev_vdbg() might be useful somewhere, I'm not
> sure this is the place.
> 
> It took me 10 minutes of archaeology to figure out what it is and how
> to use it.  Apparently it does nothing at all unless you define
> VERBOSE_DEBUG, which can be set via Kconfig in a few cases:
> 
>   drivers/dma/Makefile:subdir-ccflags-$(CONFIG_DMADEVICES_VDEBUG) += -DVERBOSE_DEBUG
>   drivers/usb/dwc2/Makefile:ccflags-$(CONFIG_USB_DWC2_VERBOSE)    += -DVERBOSE_DEBUG
>   drivers/usb/gadget/Makefile:subdir-ccflags-$(CONFIG_USB_GADGET_VERBOSE) += -DVERBOSE_DEBUG
> 
> but is more commonly set by adding "#define VERBOSE_DEBUG" manually
> in a source file.  Neither one applies here, so I don't think this
> actually does anything.
> 
> Anyway, I suspect this particular message would make a lot of sense as
> a *comment*, but maybe not as a mysterious dev_vdbg() that never
> actually does anything.

I agree. Kishon ? I would do what Bjorn suggests here, please let me
know.

Lorenzo
Kishon Vijay Abraham I April 17, 2019, 1:06 p.m. UTC | #3
Hi Lorenzo,

On 16/04/19 7:45 PM, Lorenzo Pieralisi wrote:
> On Sat, Apr 13, 2019 at 10:26:33AM -0500, Bjorn Helgaas wrote:
>> On Mon, Mar 25, 2019 at 03:09:35PM +0530, Kishon Vijay Abraham I wrote:
>>> Add PCIe RC support for AM654x Platforms in pci-keystone.c
>>
>>> +static int ks_pcie_am654_msi_host_init(struct pcie_port *pp)
>>> +{
>>> +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>>> +	struct device *dev = pci->dev;
>>> +
>>> +	dev_vdbg(dev, "dummy function so that DW core doesn't configure MSI\n");
>>
>> Drive-by nit: this is the first occurrence of dev_vdbg() in
>> drivers/pci, and while dev_vdbg() might be useful somewhere, I'm not
>> sure this is the place.
>>
>> It took me 10 minutes of archaeology to figure out what it is and how
>> to use it.  Apparently it does nothing at all unless you define
>> VERBOSE_DEBUG, which can be set via Kconfig in a few cases:
>>
>>   drivers/dma/Makefile:subdir-ccflags-$(CONFIG_DMADEVICES_VDEBUG) += -DVERBOSE_DEBUG
>>   drivers/usb/dwc2/Makefile:ccflags-$(CONFIG_USB_DWC2_VERBOSE)    += -DVERBOSE_DEBUG
>>   drivers/usb/gadget/Makefile:subdir-ccflags-$(CONFIG_USB_GADGET_VERBOSE) += -DVERBOSE_DEBUG
>>
>> but is more commonly set by adding "#define VERBOSE_DEBUG" manually
>> in a source file.  Neither one applies here, so I don't think this
>> actually does anything.
>>
>> Anyway, I suspect this particular message would make a lot of sense as
>> a *comment*, but maybe not as a mysterious dev_vdbg() that never
>> actually does anything.
> 
> I agree. Kishon ? I would do what Bjorn suggests here, please let me
> know.

Sure.

Thanks
Kishon
diff mbox series

Patch

diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index 6ea74b1c0d94..d1d00833e0b3 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -104,7 +104,7 @@  config PCIE_SPEAR13XX
 
 config PCI_KEYSTONE
 	bool "TI Keystone PCIe controller"
-	depends on ARCH_KEYSTONE || (ARM && COMPILE_TEST)
+	depends on ARCH_KEYSTONE || ARCH_K3 || ((ARM || ARM64) && COMPILE_TEST)
 	depends on PCI_MSI_IRQ_DOMAIN
 	select PCIE_DW_HOST
 	help
diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
index 93296d434f40..a6a482bd648f 100644
--- a/drivers/pci/controller/dwc/pci-keystone.c
+++ b/drivers/pci/controller/dwc/pci-keystone.c
@@ -11,6 +11,7 @@ 
 
 #include <linux/clk.h>
 #include <linux/delay.h>
+#include <linux/gpio/consumer.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/irqchip/chained_irq.h>
@@ -18,6 +19,7 @@ 
 #include <linux/mfd/syscon.h>
 #include <linux/msi.h>
 #include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/of_irq.h>
 #include <linux/of_pci.h>
 #include <linux/phy/phy.h>
@@ -65,6 +67,7 @@ 
 #define ERR_IRQ_STATUS			0x1c4
 #define ERR_IRQ_ENABLE_SET		0x1c8
 #define ERR_AER				BIT(5)	/* ECRC error */
+#define AM6_ERR_AER			BIT(4)	/* AM6 ECRC error */
 #define ERR_AXI				BIT(4)	/* AXI tag lookup fatal error */
 #define ERR_CORR			BIT(3)	/* Correctable error */
 #define ERR_NONFATAL			BIT(2)	/* Non-fatal error */
@@ -88,8 +91,15 @@ 
 
 #define KS_PCIE_SYSCLOCKOUTEN		BIT(0)
 
+#define AM654_PCIE_DEV_TYPE_MASK	0x3
+
 #define to_keystone_pcie(x)		dev_get_drvdata((x)->dev)
 
+struct ks_pcie_of_data {
+	const struct dw_pcie_host_ops *host_ops;
+	unsigned int version;
+};
+
 struct keystone_pcie {
 	struct dw_pcie		*pci;
 	/* PCI Device ID */
@@ -109,6 +119,7 @@  struct keystone_pcie {
 	/* Application register space */
 	void __iomem		*va_app_base;	/* DT 1st resource */
 	struct resource		app;
+	bool			is_am6;
 };
 
 static u32 ks_pcie_app_readl(struct keystone_pcie *ks_pcie, u32 offset)
@@ -250,6 +261,16 @@  static void ks_pcie_handle_legacy_irq(struct keystone_pcie *ks_pcie,
 	ks_pcie_app_writel(ks_pcie, IRQ_EOI, offset);
 }
 
+static int ks_pcie_am654_msi_host_init(struct pcie_port *pp)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	struct device *dev = pci->dev;
+
+	dev_vdbg(dev, "dummy function so that DW core doesn't configure MSI\n");
+
+	return 0;
+}
+
 static void ks_pcie_enable_error_irq(struct keystone_pcie *ks_pcie)
 {
 	ks_pcie_app_writel(ks_pcie, ERR_IRQ_ENABLE_SET, ERR_IRQ_ALL);
@@ -276,10 +297,10 @@  static irqreturn_t ks_pcie_handle_error_irq(struct keystone_pcie *ks_pcie)
 	if (reg & ERR_CORR)
 		dev_dbg(dev, "Correctable Error\n");
 
-	if (reg & ERR_AXI)
+	if (!ks_pcie->is_am6 && (reg & ERR_AXI))
 		dev_err(dev, "AXI tag lookup fatal Error\n");
 
-	if (reg & ERR_AER)
+	if (reg & ERR_AER || (ks_pcie->is_am6 && (reg & AM6_ERR_AER)))
 		dev_err(dev, "ECRC Error\n");
 
 	ks_pcie_app_writel(ks_pcie, ERR_IRQ_STATUS, reg);
@@ -377,6 +398,9 @@  static void ks_pcie_setup_rc_app_regs(struct keystone_pcie *ks_pcie)
 	dw_pcie_writel_dbi(pci, PCI_BASE_ADDRESS_1, 0);
 	ks_pcie_clear_dbi_mode(ks_pcie);
 
+	if (ks_pcie->is_am6)
+		return;
+
 	val = ilog2(OB_WIN_SIZE);
 	ks_pcie_app_writel(ks_pcie, OB_SIZE, val);
 
@@ -619,6 +643,8 @@  static int ks_pcie_config_msi_irq(struct keystone_pcie *ks_pcie)
 
 	intc_np = of_get_child_by_name(np, "msi-interrupt-controller");
 	if (!intc_np) {
+		if (ks_pcie->is_am6)
+			return 0;
 		dev_warn(dev, "msi-interrupt-controller node is absent\n");
 		return -EINVAL;
 	}
@@ -668,6 +694,12 @@  static int ks_pcie_config_legacy_irq(struct keystone_pcie *ks_pcie)
 
 	intc_np = of_get_child_by_name(np, "legacy-interrupt-controller");
 	if (!intc_np) {
+		/*
+		 * Since legacy interrupts are modeled as edge-interrupts in
+		 * AM6, keep it disabled for now.
+		 */
+		if (ks_pcie->is_am6)
+			return 0;
 		dev_warn(dev, "legacy-interrupt-controller node is absent\n");
 		return -EINVAL;
 	}
@@ -749,8 +781,10 @@  static int __init ks_pcie_init_id(struct keystone_pcie *ks_pcie)
 	if (ret)
 		return ret;
 
+	dw_pcie_dbi_ro_wr_en(pci);
 	dw_pcie_writew_dbi(pci, PCI_VENDOR_ID, id & PCIE_VENDORID_MASK);
 	dw_pcie_writew_dbi(pci, PCI_DEVICE_ID, id >> PCIE_DEVICEID_SHIFT);
+	dw_pcie_dbi_ro_wr_dis(pci);
 
 	return 0;
 }
@@ -803,6 +837,11 @@  static const struct dw_pcie_host_ops ks_pcie_host_ops = {
 	.scan_bus = ks_pcie_v3_65_scan_bus,
 };
 
+static const struct dw_pcie_host_ops ks_pcie_am654_host_ops = {
+	.host_init = ks_pcie_host_init,
+	.msi_host_init = ks_pcie_am654_msi_host_init,
+};
+
 static irqreturn_t ks_pcie_err_irq_handler(int irq, void *priv)
 {
 	struct keystone_pcie *ks_pcie = priv;
@@ -826,7 +865,6 @@  static int __init ks_pcie_add_pcie_port(struct keystone_pcie *ks_pcie,
 
 	pp->va_cfg1_base = pp->va_cfg0_base;
 
-	pp->ops = &ks_pcie_host_ops;
 	ret = dw_pcie_host_init(pp);
 	if (ret) {
 		dev_err(dev, "failed to initialize host\n");
@@ -836,14 +874,6 @@  static int __init ks_pcie_add_pcie_port(struct keystone_pcie *ks_pcie,
 	return 0;
 }
 
-static const struct of_device_id ks_pcie_of_match[] = {
-	{
-		.type = "pci",
-		.compatible = "ti,keystone-pcie",
-	},
-	{ },
-};
-
 static const struct dw_pcie_ops ks_pcie_dw_pcie_ops = {
 	.start_link = ks_pcie_start_link,
 	.stop_link = ks_pcie_stop_link,
@@ -913,14 +943,67 @@  static int ks_pcie_set_mode(struct device *dev)
 	return 0;
 }
 
+static int ks_pcie_am654_set_mode(struct device *dev)
+{
+	struct device_node *np = dev->of_node;
+	struct regmap *syscon;
+	u32 val;
+	u32 mask;
+	int ret = 0;
+
+	syscon = syscon_regmap_lookup_by_phandle(np, "ti,syscon-pcie-mode");
+	if (IS_ERR(syscon))
+		return 0;
+
+	mask = AM654_PCIE_DEV_TYPE_MASK;
+	val = RC;
+
+	ret = regmap_update_bits(syscon, 0, mask, val);
+	if (ret) {
+		dev_err(dev, "failed to set pcie mode\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static const struct ks_pcie_of_data ks_pcie_rc_of_data = {
+	.host_ops = &ks_pcie_host_ops,
+	.version = 0x365A,
+};
+
+static const struct ks_pcie_of_data ks_pcie_am654_rc_of_data = {
+	.host_ops = &ks_pcie_am654_host_ops,
+	.version = 0x490A,
+};
+
+static const struct of_device_id ks_pcie_of_match[] = {
+	{
+		.type = "pci",
+		.data = &ks_pcie_rc_of_data,
+		.compatible = "ti,keystone-pcie",
+	},
+	{
+		.data = &ks_pcie_am654_rc_of_data,
+		.compatible = "ti,am654-pcie-rc",
+	},
+	{ },
+};
+
 static int __init ks_pcie_probe(struct platform_device *pdev)
 {
+	const struct dw_pcie_host_ops *host_ops;
 	struct device *dev = &pdev->dev;
 	struct device_node *np = dev->of_node;
+	const struct ks_pcie_of_data *data;
+	const struct of_device_id *match;
 	struct dw_pcie *pci;
 	struct keystone_pcie *ks_pcie;
 	struct device_link **link;
+	struct gpio_desc *gpiod;
+	void __iomem *atu_base;
 	struct resource *res;
+	unsigned int version;
 	void __iomem *base;
 	u32 num_viewport;
 	struct phy **phy;
@@ -930,6 +1013,14 @@  static int __init ks_pcie_probe(struct platform_device *pdev)
 	int irq;
 	int i;
 
+	match = of_match_device(of_match_ptr(ks_pcie_of_match), dev);
+	data = (struct ks_pcie_of_data *)match->data;
+	if (!data)
+		return -EINVAL;
+
+	version = data->version;
+	host_ops = data->host_ops;
+
 	ks_pcie = devm_kzalloc(dev, sizeof(*ks_pcie), GFP_KERNEL);
 	if (!ks_pcie)
 		return -ENOMEM;
@@ -950,9 +1041,13 @@  static int __init ks_pcie_probe(struct platform_device *pdev)
 	if (IS_ERR(base))
 		return PTR_ERR(base);
 
+	if (of_device_is_compatible(np, "ti,am654-pcie-rc"))
+		ks_pcie->is_am6 = true;
+
 	pci->dbi_base = base;
 	pci->dev = dev;
 	pci->ops = &ks_pcie_dw_pcie_ops;
+	pci->version = version;
 
 	ret = of_property_read_u32(np, "num-viewport", &num_viewport);
 	if (ret < 0) {
@@ -1011,6 +1106,15 @@  static int __init ks_pcie_probe(struct platform_device *pdev)
 	ks_pcie->num_viewport = num_viewport;
 	ks_pcie->phy = phy;
 
+	gpiod = devm_gpiod_get_optional(dev, "reset",
+					GPIOD_OUT_LOW);
+	if (IS_ERR(gpiod)) {
+		ret = PTR_ERR(gpiod);
+		if (ret != -EPROBE_DEFER)
+			dev_err(dev, "Failed to get reset GPIO\n");
+		goto err_link;
+	}
+
 	ret = ks_pcie_enable_phy(ks_pcie);
 	if (ret) {
 		dev_err(dev, "failed to enable phy\n");
@@ -1025,10 +1129,39 @@  static int __init ks_pcie_probe(struct platform_device *pdev)
 		goto err_get_sync;
 	}
 
-	ret = ks_pcie_set_mode(dev);
-	if (ret < 0)
-		goto err_get_sync;
+	if (pci->version >= 0x480A) {
+		res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "atu");
+		atu_base = devm_ioremap_resource(dev, res);
+		if (IS_ERR(atu_base)) {
+			ret = PTR_ERR(atu_base);
+			goto err_get_sync;
+		}
+
+		pci->atu_base = atu_base;
+
+		ret = ks_pcie_am654_set_mode(dev);
+		if (ret < 0)
+			goto err_get_sync;
+	} else {
+		ret = ks_pcie_set_mode(dev);
+		if (ret < 0)
+			goto err_get_sync;
+	}
+
+	/*
+	 * "Power Sequencing and Reset Signal Timings" table in
+	 * PCI EXPRESS CARD ELECTROMECHANICAL SPECIFICATION, REV. 2.0
+	 * indicates PERST# should be deasserted after minimum of 100us
+	 * once REFCLK is stable. The REFCLK to the connector in RC
+	 * mode is selected while enabling the PHY. So deassert PERST#
+	 * after 100 us.
+	 */
+	if (gpiod) {
+		usleep_range(100, 200);
+		gpiod_set_value_cansleep(gpiod, 1);
+	}
 
+	pci->pp.ops = host_ops;
 	ret = ks_pcie_add_pcie_port(ks_pcie, pdev);
 	if (ret < 0)
 		goto err_get_sync;