Re: [PATCH v2 1/5] PCI: mediatek: Add a structure to abstract the controller generations

[Honghui Zhang <honghui.zhang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>]

On Thu, 2017-07-27 at 10:58 +0800, honghui.zhang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org wrote:
> From: Ryder Lee <ryder.lee-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> 
> Introduce a structure "mtk_pcie_soc" to abstract the differences between
> controller generations, and the .startup() hook is used to encapsulate
> some SoC-dependent related setting. In doing so, the common code which
> will be reused by future chips.
> 
> In addition, we change the approaches to waiting Gen2 training by using
> readl_poll_timeout() calls.
> 
> Signed-off-by: Ryder Lee <ryder.lee-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Honghui Zhang <honghui.zhang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> ---
>  drivers/pci/host/pcie-mediatek.c | 81 +++++++++++++++++++++++-----------------
>  1 file changed, 47 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/pci/host/pcie-mediatek.c b/drivers/pci/host/pcie-mediatek.c
> index 5a9d858..7f952a4 100644
> --- a/drivers/pci/host/pcie-mediatek.c
> +++ b/drivers/pci/host/pcie-mediatek.c
> @@ -16,6 +16,7 @@
>  

Sorry about the disturb, I send the wrong patch for this one.
I will resend the right version for this last patch.

thanks.
>  #include <linux/clk.h>
>  #include <linux/delay.h>
> +#include <linux/iopoll.h>
>  #include <linux/kernel.h>
>  #include <linux/of_address.h>
>  #include <linux/of_pci.h>
> @@ -63,6 +64,18 @@
>  #define PCIE_FC_CREDIT_MASK	(GENMASK(31, 31) | GENMASK(28, 16))
>  #define PCIE_FC_CREDIT_VAL(x)	((x) << 16)
>  
> +struct mtk_pcie_port;
> +
> +/**
> + * struct mtk_pcie_soc - differentiate between host generations
> + * @ops: pointer to configuration access functions
> + * @startup: pointer to controller setting functions
> + */
> +struct mtk_pcie_soc {
> +	struct pci_ops *ops;
> +	int (*startup)(struct mtk_pcie_port *port);
> +};
> +
>  /**
>   * struct mtk_pcie_port - PCIe port information
>   * @base: IO mapped register base
> @@ -96,6 +109,7 @@ struct mtk_pcie_port {
>   * @busn: bus range
>   * @offset: IO / Memory offset
>   * @ports: pointer to PCIe port information
> + * @soc: pointer to SoC-dependent operations
>   */
>  struct mtk_pcie {
>  	struct device *dev;
> @@ -111,13 +125,9 @@ struct mtk_pcie {
>  		resource_size_t io;
>  	} offset;
>  	struct list_head ports;
> +	const struct mtk_pcie_soc *soc;
>  };
>  
> -static inline bool mtk_pcie_link_up(struct mtk_pcie_port *port)
> -{
> -	return !!(readl(port->base + PCIE_LINK_STATUS) & PCIE_PORT_LINKUP);
> -}
> -
>  static void mtk_pcie_subsys_powerdown(struct mtk_pcie *pcie)
>  {
>  	struct device *dev = pcie->dev;
> @@ -171,12 +181,30 @@ static struct pci_ops mtk_pcie_ops = {
>  	.write = pci_generic_config_write,
>  };
>  
> -static void mtk_pcie_configure_rc(struct mtk_pcie_port *port)
> +static int mtk_pcie_startup_ports(struct mtk_pcie_port *port)
>  {
>  	struct mtk_pcie *pcie = port->pcie;
>  	u32 func = PCI_FUNC(port->index << 3);
>  	u32 slot = PCI_SLOT(port->index << 3);
>  	u32 val;
> +	int err;
> +
> +	/* assert port PERST_N */
> +	val = readl(pcie->base + PCIE_SYS_CFG);
> +	val |= PCIE_PORT_PERST(port->index);
> +	writel(val, pcie->base + PCIE_SYS_CFG);
> +
> +	/* de-assert port PERST_N */
> +	val = readl(pcie->base + PCIE_SYS_CFG);
> +	val &= ~PCIE_PORT_PERST(port->index);
> +	writel(val, pcie->base + PCIE_SYS_CFG);
> +
> +	/* 100ms timeout value should be enough for Gen1/2 training */
> +	err = readl_poll_timeout(port->base + PCIE_LINK_STATUS, val,
> +				 !!(val & PCIE_PORT_LINKUP), 20,
> +				 100 * USEC_PER_MSEC);
> +	if (err)
> +		return -ETIMEDOUT;
>  
>  	/* enable interrupt */
>  	val = readl(pcie->base + PCIE_INT_ENABLE);
> @@ -209,30 +237,14 @@ static void mtk_pcie_configure_rc(struct mtk_pcie_port *port)
>  	writel(PCIE_CONF_ADDR(PCIE_FTS_NUM, func, slot, 0),
>  	       pcie->base + PCIE_CFG_ADDR);
>  	writel(val, pcie->base + PCIE_CFG_DATA);
> -}
>  
> -static void mtk_pcie_assert_ports(struct mtk_pcie_port *port)
> -{
> -	struct mtk_pcie *pcie = port->pcie;
> -	u32 val;
> -
> -	/* assert port PERST_N */
> -	val = readl(pcie->base + PCIE_SYS_CFG);
> -	val |= PCIE_PORT_PERST(port->index);
> -	writel(val, pcie->base + PCIE_SYS_CFG);
> -
> -	/* de-assert port PERST_N */
> -	val = readl(pcie->base + PCIE_SYS_CFG);
> -	val &= ~PCIE_PORT_PERST(port->index);
> -	writel(val, pcie->base + PCIE_SYS_CFG);
> -
> -	/* PCIe v2.0 need at least 100ms delay to train from Gen1 to Gen2 */
> -	msleep(100);
> +	return 0;
>  }
>  
>  static void mtk_pcie_enable_ports(struct mtk_pcie_port *port)
>  {
> -	struct device *dev = port->pcie->dev;
> +	struct mtk_pcie *pcie = port->pcie;
> +	struct device *dev = pcie->dev;
>  	int err;
>  
>  	err = clk_prepare_enable(port->sys_ck);
> @@ -250,13 +262,8 @@ static void mtk_pcie_enable_ports(struct mtk_pcie_port *port)
>  		goto err_phy_on;
>  	}
>  
> -	mtk_pcie_assert_ports(port);
> -
> -	/* if link up, then setup root port configuration space */
> -	if (mtk_pcie_link_up(port)) {
> -		mtk_pcie_configure_rc(port);
> +	if (!pcie->soc->startup(port))
>  		return;
> -	}
>  
>  	dev_info(dev, "Port%d link down\n", port->index);
>  
> @@ -480,7 +487,7 @@ static int mtk_pcie_register_host(struct pci_host_bridge *host)
>  
>  	host->busnr = pcie->busn.start;
>  	host->dev.parent = pcie->dev;
> -	host->ops = &mtk_pcie_ops;
> +	host->ops = pcie->soc->ops;
>  	host->map_irq = of_irq_parse_and_map_pci;
>  	host->swizzle_irq = pci_common_swizzle;
>  
> @@ -513,6 +520,7 @@ static int mtk_pcie_probe(struct platform_device *pdev)
>  	pcie = pci_host_bridge_priv(host);
>  
>  	pcie->dev = dev;
> +	pcie->soc = of_device_get_match_data(dev);
>  	platform_set_drvdata(pdev, pcie);
>  	INIT_LIST_HEAD(&pcie->ports);
>  
> @@ -537,9 +545,14 @@ static int mtk_pcie_probe(struct platform_device *pdev)
>  	return err;
>  }
>  
> +static struct mtk_pcie_soc mtk_pcie_soc_v1 = {
> +	.ops = &mtk_pcie_ops,
> +	.startup = mtk_pcie_startup_ports,
> +};
> +
>  static const struct of_device_id mtk_pcie_ids[] = {
> -	{ .compatible = "mediatek,mt7623-pcie"},
> -	{ .compatible = "mediatek,mt2701-pcie"},
> +	{ .compatible = "mediatek,mt2701-pcie", .data = &mtk_pcie_soc_v1 },
> +	{ .compatible = "mediatek,mt7623-pcie", .data = &mtk_pcie_soc_v1 },
>  	{},
>  };
>  


--
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