RE: [PATCH v12] [PATCH] PCI: Xilinx-NWL-PCIe: Adding support for Xilinx NWL PCIe Host Controller

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Thanks Bjorn.
Sorry for troubling you, I have built it it did not break anything, it is working fine.

Thanks 
Bharat

> On Sun, Mar 06, 2016 at 10:02:14PM +0530, Bharat Kumar Gogada wrote:
> > Adding PCIe Root Port driver for Xilinx PCIe NWL bridge IP.
> >
> > Reviewed-by: Marc Zyngier <marc.zyngier@xxxxxxx>
> > Acked-by: Rob Herring <robh@xxxxxxxxxx>
> > Signed-off-by: Bharat Kumar Gogada <bharatku@xxxxxxxxxx>
> > Signed-off-by: Ravi Kiran Gummaluri <rgummal@xxxxxxxxxx>
> > ---
> > Changes for v12:
> > -> Removed nwl_setup_sspl function, it will be added after more
> > testing.
> > -> Broke nwl_pcie_link_up into nwl_pcie_link_up, nwl_phy_link_up
> > functions.
> > -> Using generic functions for configuration read and write.
> > -> Removed unneccessary comments.
> > -> Removed unneccessary new lines.
> > -> Added #define for number of legacy interrupts.
> > -> Changed MSI interrupt handlers registering sequence.
> > ---
> >  .../devicetree/bindings/pci/xilinx-nwl-pcie.txt    |  68 ++
> >  drivers/pci/host/Kconfig                           |  10 +
> >  drivers/pci/host/Makefile                          |   1 +
> >  drivers/pci/host/pcie-xilinx-nwl.c                 | 912 +++++++++++++++++++++
> >  4 files changed, 991 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/pci/xilinx-nwl-pcie.txt
> >  create mode 100644 drivers/pci/host/pcie-xilinx-nwl.c
> 
> I applied this to pci/host-xilinx-nwl for v4.6.
> 
> I cleaned up a bunch of stuff: whitespace, spelling, unused definitions, etc.,
> and also some minor code cleanups.  I can't build it myself, so please check
> and make sure I didn't break anything.
> 
> You can browse or check out the branch from here:
> https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/host-
> xilinx-nwl
> 
> Here's the diff showing what I changed relative to the patch you
> posted:
> 
> diff --git a/Documentation/devicetree/bindings/pci/xilinx-nwl-pcie.txt
> b/Documentation/devicetree/bindings/pci/xilinx-nwl-pcie.txt
> index 90e8f32..337fc97 100644
> --- a/Documentation/devicetree/bindings/pci/xilinx-nwl-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/xilinx-nwl-pcie.txt
> @@ -7,7 +7,7 @@ Required properties:
>  - #interrupt-cells: specifies the number of cells needed to encode an
>  	interrupt source. The value must be 1.
>  - reg: Should contain Bridge, PCIe Controller registers location,
> -	configuration sapce, and length
> +	configuration space, and length
>  - reg-names: Must include the following entries:
>  	"breg": bridge registers
>  	"pcireg": PCIe controller registers
> @@ -15,8 +15,8 @@ Required properties:
>  - device_type: must be "pci"
>  - interrupts: Should contain NWL PCIe interrupt
>  - interrupt-names: Must include the following entries:
> -	"msi1, msi0": interrupt asserted when msi is received
> -	"intx": interrupt that is asserted when an legacy interrupt is received
> +	"msi1, msi0": interrupt asserted when MSI is received
> +	"intx": interrupt asserted when a legacy interrupt is received
>  	"misc": interrupt asserted when miscellaneous is received
>  - interrupt-map-mask and interrupt-map: standard PCI properties to define
> the
>  	mapping of the PCI interface to interrupt numbers.
> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig index
> 466a601..c39989a 100644
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -21,7 +21,7 @@ config PCIE_XILINX_NWL
>  	depends on ARCH_ZYNQMP
>  	select PCI_MSI_IRQ_DOMAIN if PCI_MSI
>  	help
> -	 Say 'Y' here if you want kernel to support for Xilinx
> +	 Say 'Y' here if you want kernel support for Xilinx
>  	 NWL PCIe controller. The controller can act as Root Port
>  	 or End Point. The current option selection will only
>  	 support root port enabling.
> diff --git a/drivers/pci/host/pcie-xilinx-nwl.c b/drivers/pci/host/pcie-xilinx-
> nwl.c
> index e5774dc..7a2dc2e 100644
> --- a/drivers/pci/host/pcie-xilinx-nwl.c
> +++ b/drivers/pci/host/pcie-xilinx-nwl.c
> @@ -27,26 +27,15 @@
> 
>  /* Bridge core config registers */
>  #define BRCFG_PCIE_RX0			0x00000000
> -#define BRCFG_PCIE_RX1			0x00000004
> -#define BRCFG_AXI_MASTER		0x00000008
> -#define BRCFG_PCIE_TX			0x0000000C
>  #define BRCFG_INTERRUPT			0x00000010
> -#define BRCFG_RAM_DISABLE0		0x00000014
> -#define BRCFG_RAM_DISABLE1		0x00000018
> -#define BRCFG_PCIE_RELAXED_ORDER	0x0000001C
>  #define BRCFG_PCIE_RX_MSG_FILTER	0x00000020
> 
> -/* Attribute registers */
> -#define NWL_ATTRIB_100			0x00000190
> -
>  /* Egress - Bridge translation registers */
>  #define E_BREG_CAPABILITIES		0x00000200
> -#define E_BREG_STATUS			0x00000204
>  #define E_BREG_CONTROL			0x00000208
>  #define E_BREG_BASE_LO			0x00000210
>  #define E_BREG_BASE_HI			0x00000214
>  #define E_ECAM_CAPABILITIES		0x00000220
> -#define E_ECAM_STATUS			0x00000224
>  #define E_ECAM_CONTROL			0x00000228
>  #define E_ECAM_BASE_LO			0x00000230
>  #define E_ECAM_BASE_HI			0x00000234
> @@ -68,11 +57,6 @@
>  #define MSGF_MSI_STATUS_HI		0x00000444
>  #define MSGF_MSI_MASK_LO		0x00000448
>  #define MSGF_MSI_MASK_HI		0x0000044C
> -#define MSGF_RX_FIFO_POP		0x00000484
> -#define MSGF_RX_FIFO_TYPE		0x00000488
> -#define MSGF_RX_FIFO_ADDRLO		0x00000490
> -#define MSGF_RX_FIFO_ADDRHI		0x00000494
> -#define MSGF_RX_FIFO_DATA		0x00000498
> 
>  /* Msg filter mask bits */
>  #define CFG_ENABLE_PM_MSG_FWD		BIT(1)
> @@ -116,10 +100,6 @@
>  					MSGF_MISC_SR_PCIE_CORE | \
>  					MSGF_MISC_SR_PCIE_CORE_ERR)
> 
> -/* Message rx fifo type mask bits */
> -#define MSGF_RX_FIFO_TYPE_MSI		(1)
> -#define MSGF_RX_FIFO_TYPE_TYPE		GENMASK(1, 0)
> -
>  /* Legacy interrupt status mask bits */
>  #define MSGF_LEG_SR_INTA		BIT(0)
>  #define MSGF_LEG_SR_INTB		BIT(1)
> @@ -144,30 +124,27 @@
> 
>  /* E_ECAM status mask bits */
>  #define E_ECAM_PRESENT			BIT(0)
> -#define E_ECAM_SR_WR_PEND		BIT(16)
> -#define E_ECAM_SR_RD_PEND		BIT(0)
> -#define E_ECAM_SR_MASKALL		(E_ECAM_SR_WR_PEND |
> E_ECAM_SR_RD_PEND)
>  #define E_ECAM_CR_ENABLE		BIT(0)
>  #define E_ECAM_SIZE_LOC			GENMASK(20, 16)
>  #define E_ECAM_SIZE_SHIFT		16
>  #define ECAM_BUS_LOC_SHIFT		20
>  #define ECAM_DEV_LOC_SHIFT		12
>  #define NWL_ECAM_VALUE_DEFAULT		12
> -#define NWL_ECAM_SIZE_MIN		4096
> 
> -#define ATTR_UPSTREAM_FACING		BIT(6)
>  #define CFG_DMA_REG_BAR			GENMASK(2, 0)
> 
>  #define INT_PCI_MSI_NR			(2 * 32)
> -
>  #define INTX_NUM			4
> +
>  /* Readin the PS_LINKUP */
>  #define PS_LINKUP_OFFSET		0x00000238
>  #define PCIE_PHY_LINKUP_BIT		BIT(0)
>  #define PHY_RDY_LINKUP_BIT		BIT(1)
> -#define PCIE_USER_LINKUP		0
> -#define PHY_RDY_LINKUP			1
> -#define LINKUP_ITER_CHECK		5
> +
> +/* Parameters for the waiting for link up routine */
> +#define LINK_WAIT_MAX_RETRIES          10
> +#define LINK_WAIT_USLEEP_MIN           90000
> +#define LINK_WAIT_USLEEP_MAX           100000
> 
>  struct nwl_msi {			/* MSI information */
>  	struct irq_domain *msi_domain;
> @@ -194,7 +171,6 @@ struct nwl_pcie {
>  	u32 ecam_value;
>  	u8 last_busno;
>  	u8 root_busno;
> -	u8 link_up;
>  	struct nwl_msi msi;
>  	struct irq_domain *legacy_irq_domain;
>  };
> @@ -223,11 +199,26 @@ static bool nwl_phy_link_up(struct nwl_pcie *pcie)
>  	return false;
>  }
> 
> +static int nwl_wait_for_link(struct nwl_pcie *pcie) {
> +	int retries;
> +
> +	/* check if the link is up or not */
> +	for (retries = 0; retries < LINK_WAIT_MAX_RETRIES; retries++) {
> +		if (nwl_phy_link_up(pcie))
> +			return 0;
> +		usleep_range(LINK_WAIT_USLEEP_MIN,
> LINK_WAIT_USLEEP_MAX);
> +	}
> +
> +	dev_err(pcie->dev, "PHY link never came up\n");
> +	return -ETIMEDOUT;
> +}
> +
>  static bool nwl_pcie_valid_device(struct pci_bus *bus, unsigned int devfn)  {
>  	struct nwl_pcie *pcie = bus->sysdata;
> 
> -	/* Check link,before accessing downstream ports */
> +	/* Check link before accessing downstream ports */
>  	if (bus->number != pcie->root_busno) {
>  		if (!nwl_pcie_link_up(pcie))
>  			return false;
> @@ -250,9 +241,8 @@ static bool nwl_pcie_valid_device(struct pci_bus
> *bus, unsigned int devfn)
>   * Return: Base address of the configuration space needed to be
>   *	   accessed.
>   */
> -static void __iomem *nwl_pcie_map_bus(struct pci_bus *bus,
> -						 unsigned int devfn,
> -						 int where)
> +static void __iomem *nwl_pcie_map_bus(struct pci_bus *bus, unsigned int
> devfn,
> +				      int where)
>  {
>  	struct nwl_pcie *pcie = bus->sysdata;
>  	int relbus;
> @@ -323,8 +313,7 @@ static void nwl_pcie_leg_handler(struct irq_desc
> *desc)
> 
>  	while ((status = nwl_bridge_readl(pcie, MSGF_LEG_STATUS) &
>  				MSGF_LEG_SR_MASKALL) != 0) {
> -		for_each_set_bit(bit, &status, 4) {
> -
> +		for_each_set_bit(bit, &status, INTX_NUM) {
>  			virq = irq_find_mapping(pcie->legacy_irq_domain,
>  						bit + 1);
>  			if (virq)
> @@ -357,29 +346,25 @@ static void nwl_pcie_handle_msi_irq(struct
> nwl_pcie *pcie, u32 status_reg)  static void
> nwl_pcie_msi_handler_high(struct irq_desc *desc)  {
>  	struct irq_chip *chip = irq_desc_get_chip(desc);
> -	struct nwl_pcie *pcie;
> +	struct nwl_pcie *pcie = irq_desc_get_handler_data(desc);
> 
>  	chained_irq_enter(chip, desc);
> -	pcie = irq_desc_get_handler_data(desc);
>  	nwl_pcie_handle_msi_irq(pcie, MSGF_MSI_STATUS_HI);
> -
>  	chained_irq_exit(chip, desc);
>  }
> 
>  static void nwl_pcie_msi_handler_low(struct irq_desc *desc)  {
>  	struct irq_chip *chip = irq_desc_get_chip(desc);
> -	struct nwl_pcie *pcie;
> +	struct nwl_pcie *pcie = irq_desc_get_handler_data(desc);
> 
>  	chained_irq_enter(chip, desc);
> -	pcie = irq_desc_get_handler_data(desc);
>  	nwl_pcie_handle_msi_irq(pcie, MSGF_MSI_STATUS_LO);
> -
>  	chained_irq_exit(chip, desc);
>  }
> 
>  static int nwl_legacy_map(struct irq_domain *domain, unsigned int irq,
> -				irq_hw_number_t hwirq)
> +			  irq_hw_number_t hwirq)
>  {
>  	irq_set_chip_and_handler(irq, &dummy_irq_chip,
> handle_simple_irq);
>  	irq_set_chip_data(irq, domain->host_data); @@ -441,16 +426,13
> @@ static int nwl_irq_domain_alloc(struct irq_domain *domain, unsigned int
> virq,
>  	mutex_lock(&msi->lock);
>  	bit = bitmap_find_next_zero_area(msi->bitmap, INT_PCI_MSI_NR,
> 0,
>  					 nr_irqs, 0);
> -	if (bit < INT_PCI_MSI_NR)
> -		bitmap_set(msi->bitmap, bit, nr_irqs);
> -	else
> -		bit = -ENOSPC;
> -
> -	if (bit < 0) {
> +	if (bit >= INT_PCI_MSI_NR) {
>  		mutex_unlock(&msi->lock);
> -		return bit;
> +		return -ENOSPC;
>  	}
> 
> +	bitmap_set(msi->bitmap, bit, nr_irqs);
> +
>  	for (i = 0; i < nr_irqs; i++) {
>  		irq_domain_set_info(domain, virq + i, bit + i, &nwl_irq_chip,
>  				domain->host_data, handle_simple_irq, @@
> -518,14 +500,14 @@ static int nwl_pcie_init_msi_irq_domain(struct nwl_pcie
> *pcie)
>  	struct nwl_msi *msi = &pcie->msi;
> 
>  	msi->dev_domain = irq_domain_add_linear(NULL,
> INT_PCI_MSI_NR,
> -					&dev_msi_domain_ops, pcie);
> +						&dev_msi_domain_ops,
> pcie);
>  	if (!msi->dev_domain) {
>  		dev_err(pcie->dev, "failed to create dev IRQ domain\n");
>  		return -ENOMEM;
>  	}
>  	msi->msi_domain = pci_msi_create_irq_domain(fwnode,
> -
> 	&nwl_msi_domain_info,
> -							msi->dev_domain);
> +						    &nwl_msi_domain_info,
> +						    msi->dev_domain);
>  	if (!msi->msi_domain) {
>  		dev_err(pcie->dev, "failed to create msi IRQ domain\n");
>  		irq_domain_remove(msi->dev_domain);
> @@ -557,7 +539,6 @@ static int nwl_pcie_init_irq_domain(struct nwl_pcie
> *pcie)
>  	}
> 
>  	nwl_pcie_init_msi_irq_domain(pcie);
> -
>  	return 0;
>  }
> 
> @@ -582,9 +563,10 @@ static int nwl_pcie_enable_msi(struct nwl_pcie
> *pcie, struct pci_bus *bus)
>  		ret = -EINVAL;
>  		goto err;
>  	}
> -	/* Register msi handler */
> +
>  	irq_set_chained_handler_and_data(msi->irq_msi1,
>  					 nwl_pcie_msi_handler_high, pcie);
> +
>  	/* Get msi_0 IRQ number */
>  	msi->irq_msi0 = platform_get_irq_byname(pdev, "msi0");
>  	if (msi->irq_msi0 < 0) {
> @@ -593,9 +575,9 @@ static int nwl_pcie_enable_msi(struct nwl_pcie *pcie,
> struct pci_bus *bus)
>  		goto err;
>  	}
> 
> -	/* Register msi handler */
>  	irq_set_chained_handler_and_data(msi->irq_msi0,
>  					 nwl_pcie_msi_handler_low, pcie);
> +
>  	/* Check for msii_present bit */
>  	ret = nwl_bridge_readl(pcie, I_MSII_CAPABILITIES) & MSII_PRESENT;
>  	if (!ret) {
> @@ -618,7 +600,7 @@ static int nwl_pcie_enable_msi(struct nwl_pcie *pcie,
> struct pci_bus *bus)
>  	nwl_bridge_writel(pcie, upper_32_bits(base), I_MSII_BASE_HI);
> 
>  	/*
> -	 * For high range msi interrupts: disable, clear any pending,
> +	 * For high range MSI interrupts: disable, clear any pending,
>  	 * and enable
>  	 */
>  	nwl_bridge_writel(pcie, (u32)~MSGF_MSI_SR_HI_MASK,
> MSGF_MSI_MASK_HI); @@ -629,7 +611,7 @@ static int
> nwl_pcie_enable_msi(struct nwl_pcie *pcie, struct pci_bus *bus)
>  	nwl_bridge_writel(pcie, MSGF_MSI_SR_HI_MASK,
> MSGF_MSI_MASK_HI);
> 
>  	/*
> -	 * For low range msi interrupts: disable, clear any pending,
> +	 * For low range MSI interrupts: disable, clear any pending,
>  	 * and enable
>  	 */
>  	nwl_bridge_writel(pcie, (u32)~MSGF_MSI_SR_LO_MASK,
> MSGF_MSI_MASK_LO); @@ -651,15 +633,13 @@ static int
> nwl_pcie_bridge_init(struct nwl_pcie *pcie)
>  	struct platform_device *pdev = to_platform_device(pcie->dev);
>  	u32 breg_val, ecam_val, first_busno = 0;
>  	int err;
> -	int check_link_up = 0;
> -	bool link_up;
> 
> -	/* Check for BREG present bit */
>  	breg_val = nwl_bridge_readl(pcie, E_BREG_CAPABILITIES) &
> BREG_PRESENT;
>  	if (!breg_val) {
>  		dev_err(pcie->dev, "BREG is not present\n");
>  		return breg_val;
>  	}
> +
>  	/* Write bridge_off to breg base */
>  	nwl_bridge_writel(pcie, lower_32_bits(pcie->phys_breg_base),
>  			  E_BREG_BASE_LO);
> @@ -680,15 +660,10 @@ static int nwl_pcie_bridge_init(struct nwl_pcie
> *pcie)
>  	/* Enable msg filtering details */
>  	nwl_bridge_writel(pcie, CFG_ENABLE_MSG_FILTER_MASK,
>  			  BRCFG_PCIE_RX_MSG_FILTER);
> -	do {
> -		link_up = nwl_phy_link_up(pcie);
> -		if (!link_up) {
> -			check_link_up++;
> -			if (check_link_up > LINKUP_ITER_CHECK)
> -				return -ENODEV;
> -			msleep(1000);
> -		}
> -	} while (!link_up);
> +
> +	err = nwl_wait_for_link(pcie);
> +	if (err)
> +		return err;
> 
>  	ecam_val = nwl_bridge_readl(pcie, E_ECAM_CAPABILITIES) &
> E_ECAM_PRESENT;
>  	if (!ecam_val) {
> @@ -718,8 +693,7 @@ static int nwl_pcie_bridge_init(struct nwl_pcie *pcie)
>  	ecam_val |= (pcie->last_busno << E_ECAM_SIZE_SHIFT);
>  	writel(ecam_val, (pcie->ecam_base + PCI_PRIMARY_BUS));
> 
> -	pcie->link_up = nwl_pcie_link_up(pcie);
> -	if (pcie->link_up)
> +	if (nwl_pcie_link_up(pcie))
>  		dev_info(pcie->dev, "Link is UP\n");
>  	else
>  		dev_info(pcie->dev, "Link is DOWN\n"); @@ -731,7 +705,7
> @@ static int nwl_pcie_bridge_init(struct nwl_pcie *pcie)
>  			pcie->irq_misc);
>  		return -EINVAL;
>  	}
> -	/* Register misc handler */
> +
>  	err = devm_request_irq(pcie->dev, pcie->irq_misc,
>  			       nwl_pcie_misc_handler, IRQF_SHARED,
>  			       "nwl_pcie:misc", pcie);
> @@ -770,7 +744,7 @@ static int nwl_pcie_bridge_init(struct nwl_pcie *pcie)
> }
> 
>  static int nwl_pcie_parse_dt(struct nwl_pcie *pcie,
> -					struct platform_device *pdev)
> +			     struct platform_device *pdev)
>  {
>  	struct device_node *node = pcie->dev->of_node;
>  	struct resource *res;
> @@ -809,7 +783,6 @@ static int nwl_pcie_parse_dt(struct nwl_pcie *pcie,
>  		return -EINVAL;
>  	}
> 
> -	/* Register intx handler */
>  	irq_set_chained_handler_and_data(pcie->irq_intx,
>  					 nwl_pcie_leg_handler, pcie);
> 
> @@ -835,16 +808,15 @@ static int nwl_pcie_probe(struct platform_device
> *pdev)
>  	if (!pcie)
>  		return -ENOMEM;
> 
> -	pcie->ecam_value = NWL_ECAM_VALUE_DEFAULT;
> -
>  	pcie->dev = &pdev->dev;
> +	pcie->ecam_value = NWL_ECAM_VALUE_DEFAULT;
> 
>  	err = nwl_pcie_parse_dt(pcie, pdev);
>  	if (err) {
>  		dev_err(pcie->dev, "Parsing DT failed\n");
>  		return err;
>  	}
> -	/* Bridge initialization */
> +
>  	err = nwl_pcie_bridge_init(pcie);
>  	if (err) {
>  		dev_err(pcie->dev, "HW Initalization failed\n"); @@ -868,7
> +840,6 @@ static int nwl_pcie_probe(struct platform_device *pdev)
>  	if (!bus)
>  		return -ENOMEM;
> 
> -	/* Enable MSI */
>  	if (IS_ENABLED(CONFIG_PCI_MSI)) {
>  		err = nwl_pcie_enable_msi(pcie, bus);
>  		if (err < 0) {
> @@ -883,7 +854,6 @@ static int nwl_pcie_probe(struct platform_device
> *pdev)
>  		pcie_bus_configure_settings(child);
>  	pci_bus_add_devices(bus);
>  	platform_set_drvdata(pdev, pcie);
> -
>  	return 0;
>  }
> 
> @@ -893,7 +863,6 @@ static int nwl_pcie_remove(struct platform_device
> *pdev)
> 
>  	nwl_pcie_free_irq_domain(pcie);
>  	platform_set_drvdata(pdev, NULL);
> -
>  	return 0;
>  }
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux