Re: [PATCH v3 5/6] PCI: mediatek: Add new generation controller support

2017-08-08 Thread Bjorn Helgaas 
On Fri, Aug 04, 2017 at 08:06:41PM +0800, honghui.zh...@mediatek.com wrote:
> From: Ryder Lee 
> 
> MediaTek's PCIe host controller has two generation HWs, the new
> generation HW has two root ports, it shares most probing flow with the
> legacy controller. But the read/write config space logical is different
> from the legacy controller. The per-port register must be touched for
> read/write config space, And the per-port register base are in separate
> address space.

Can you also include the new controller IDs ("MT7622/MT2712"?) in the
subject line of this patch?

> Add support for new Gen2 controller which can be found on MT7622/MT2712.

Re: [PATCH v3 5/6] PCI: mediatek: Add new generation controller support

2017-08-08 Thread Bjorn Helgaas 
On Fri, Aug 04, 2017 at 08:06:41PM +0800, honghui.zh...@mediatek.com wrote:
> From: Ryder Lee 
> 
> MediaTek's PCIe host controller has two generation HWs, the new
> generation HW has two root ports, it shares most probing flow with the
> legacy controller. But the read/write config space logical is different
> from the legacy controller. The per-port register must be touched for
> read/write config space, And the per-port register base are in separate
> address space.

Can you also include the new controller IDs ("MT7622/MT2712"?) in the
subject line of this patch?

> Add support for new Gen2 controller which can be found on MT7622/MT2712.

Re: [PATCH v3 5/6] PCI: mediatek: Add new generation controller support

2017-08-07 Thread Honghui Zhang 
On Sun, 2017-08-06 at 10:06 +0800, Ryder Lee wrote:
> Hi Honghui,
> 
> If you plan to send next version, then I would suggest some minor
> changes.
> 
> On Fri, 2017-08-04 at 20:06 +0800, honghui.zh...@mediatek.com wrote:
> > +#define PCIE_CRSTB BIT(3)
> > +#define PCIE_PERSTBBIT(8)
> > +#define PCI_LINKDOWN_RST_ENGENMASK(15, 13)
> 
> PCIE_LINKDOWN_RST_EN

Thanks, I will change it in the next version.

> 
> > +#define PCIE_LINK_STATUS_V20x804
> > +#define PCIE_PORT_LINKUP_V2BIT(10)
> > +
> > +
> > +static int mtk_pcie_hw_rd_cfg(struct mtk_pcie_port *port, u32 bus, u32 
> > devfn,
> > + int where, int size, u32 *val)
> > +{
> > +   int reg, shift = 8 * (where & 3);
> 
> int reg => u32

sharp eyes, thanks.

> 
> > +   /* Write PCIe Configuration Transaction Header for cfgrd */
> > +   writel(CFG_HEADER_DW0(CFG_WRRD_TYPE_0, CFG_RD_FMT),
> > +  port->base + PCIE_CFG_HEADER0);
> > +   writel(CFG_HEADER_DW1(where, size), port->base + PCIE_CFG_HEADER1);
> > +   writel(CFG_HEADER_DW2(where, PCI_FUNC(devfn), PCI_SLOT(devfn), bus),
> > +  port->base + PCIE_CFG_HEADER2);
> > +
> > +   /* Trigger h/w to transmit Cfgrd TLP */
> > +   reg = readl(port->base + PCIE_APP_TLP_REQ);
> > +   writel(reg | APP_CFG_REQ, port->base + PCIE_APP_TLP_REQ);
> > +
> > +   /* Check completion status */
> > +   if (mtk_pcie_check_cfg_cpld(port))
> > +   return PCIBIOS_SET_FAILED;
> > +
> > +   /* Read cpld payload of Cfgrd */
> > +   *val = readl(port->base + PCIE_CFG_RDATA);
> > +
> > +   switch (size) {
> > +   case 4:
> > +   break;
> > +   case 3:
> > +   *val = (*val >> shift) & 0xff;
> > +   break;
> > +   case 2:
> > +   *val = (*val >> shift) & 0x;
> > +   break;
> > +   case 1:
> > +   *val = (*val >> shift) & 0xff;
> > +   break;
> > +   default:
> > +   return PCIBIOS_BAD_REGISTER_NUMBER;
> > +   }
> 
> Do we really need case 3? I guess case 1, 2 or 4 should be enough.
> 

I will change this as the following snippet:
if (size == 1)
*val = (*val >> (8 * (where & 3))) & 0xff;
else if (size == 2)
*val = (*val >> (8 * (where & 3))) & 0x;

thanks.

> > +   return PCIBIOS_SUCCESSFUL;
> > +}
> > +
> > +static int mtk_pcie_hw_wr_cfg(struct mtk_pcie_port *port, u32 bus, u32 
> > devfn,
> > + int where, int size, u32 val)
> > +{
> > +   /* Write PCIe Configuration Transaction Header for Cfgwr */
> > +   writel(CFG_HEADER_DW0(CFG_WRRD_TYPE_0, CFG_WR_FMT),
> > +  port->base + PCIE_CFG_HEADER0);
> > +   writel(CFG_HEADER_DW1(where, size), port->base + PCIE_CFG_HEADER1);
> > +   writel(CFG_HEADER_DW2(where, PCI_FUNC(devfn), PCI_SLOT(devfn), bus),
> > +  port->base + PCIE_CFG_HEADER2);
> > +
> > +   /* Write cfgwr data */
> > +   val = val << 8 * (where & 3);
> > +   writel(val, port->base + PCIE_CFG_WDATA);
> > +
> > +   /* Trigger h/w to transmit Cfgwr TLP */
> > +   val = readl(port->base + PCIE_APP_TLP_REQ);
> > +   val |= APP_CFG_REQ;
> > +   writel(val, port->base + PCIE_APP_TLP_REQ);
> > +
> > +   /* Check completion status */
> > +   return mtk_pcie_check_cfg_cpld(port);
> > +}
> > +
> > +static int mtk_pcie_config_read(struct pci_bus *bus, unsigned int devfn,
> > +   int where, int size, u32 *val)
> > +{
> > +   struct mtk_pcie_port *port = NULL, *iter, *tmp;
> > +   struct mtk_pcie *pcie = bus->sysdata;
> > +   u32 bn = bus->number;
> > +   int ret;
> > +
> > +   list_for_each_entry_safe(iter, tmp, >ports, list)
> > +   if (iter->index == PCI_SLOT(devfn)) {
> > +   port = iter;
> > +   break;
> > +   }
> > +
> > +   if (!port) {
> > +   *val = ~0;
> > +   return PCIBIOS_DEVICE_NOT_FOUND;
> > +   }
> 
> list_for_each_entry(), since you don't really remove or free something.
> 
> I know you need to find port->base to write/read configuration space. I
> think it's better to move this part to another function. You can take a
> look at pci-mvebu.c mvebu_pcie_find_portmvebu().


Sure, I will add the mtk_pcie_find_port interface, then the code is a
bit more readable.

thanks.

>  
> > +   ret = mtk_pcie_hw_rd_cfg(port, bn, devfn, where, size, val);
> > +   if (ret)
> > +   *val = ~0;
> > +
> > +   return ret;
> > +}
> > +
> > +static int mtk_pcie_config_write(struct pci_bus *bus, unsigned int devfn,
> > +int where, int size, u32 val)
> > +{
> > +   struct mtk_pcie_port *port = NULL, *iter, *tmp;
> > +   struct mtk_pcie *pcie = bus->sysdata;
> > +   u32 bn = bus->number;
> > +
> > +   list_for_each_entry_safe(iter, tmp, >ports, list)
> > +   if (iter->index == PCI_SLOT(devfn)) {
> > +   port = iter;
> > +   break;
> > +   }
> > +
> > +   if (!port)
> > +   return PCIBIOS_DEVICE_NOT_FOUND;
> 
> Ditto.
> 
> >

Re: [PATCH v3 5/6] PCI: mediatek: Add new generation controller support

2017-08-07 Thread Honghui Zhang 
On Sun, 2017-08-06 at 10:06 +0800, Ryder Lee wrote:
> Hi Honghui,
> 
> If you plan to send next version, then I would suggest some minor
> changes.
> 
> On Fri, 2017-08-04 at 20:06 +0800, honghui.zh...@mediatek.com wrote:
> > +#define PCIE_CRSTB BIT(3)
> > +#define PCIE_PERSTBBIT(8)
> > +#define PCI_LINKDOWN_RST_ENGENMASK(15, 13)
> 
> PCIE_LINKDOWN_RST_EN

Thanks, I will change it in the next version.

> 
> > +#define PCIE_LINK_STATUS_V20x804
> > +#define PCIE_PORT_LINKUP_V2BIT(10)
> > +
> > +
> > +static int mtk_pcie_hw_rd_cfg(struct mtk_pcie_port *port, u32 bus, u32 
> > devfn,
> > + int where, int size, u32 *val)
> > +{
> > +   int reg, shift = 8 * (where & 3);
> 
> int reg => u32

sharp eyes, thanks.

> 
> > +   /* Write PCIe Configuration Transaction Header for cfgrd */
> > +   writel(CFG_HEADER_DW0(CFG_WRRD_TYPE_0, CFG_RD_FMT),
> > +  port->base + PCIE_CFG_HEADER0);
> > +   writel(CFG_HEADER_DW1(where, size), port->base + PCIE_CFG_HEADER1);
> > +   writel(CFG_HEADER_DW2(where, PCI_FUNC(devfn), PCI_SLOT(devfn), bus),
> > +  port->base + PCIE_CFG_HEADER2);
> > +
> > +   /* Trigger h/w to transmit Cfgrd TLP */
> > +   reg = readl(port->base + PCIE_APP_TLP_REQ);
> > +   writel(reg | APP_CFG_REQ, port->base + PCIE_APP_TLP_REQ);
> > +
> > +   /* Check completion status */
> > +   if (mtk_pcie_check_cfg_cpld(port))
> > +   return PCIBIOS_SET_FAILED;
> > +
> > +   /* Read cpld payload of Cfgrd */
> > +   *val = readl(port->base + PCIE_CFG_RDATA);
> > +
> > +   switch (size) {
> > +   case 4:
> > +   break;
> > +   case 3:
> > +   *val = (*val >> shift) & 0xff;
> > +   break;
> > +   case 2:
> > +   *val = (*val >> shift) & 0x;
> > +   break;
> > +   case 1:
> > +   *val = (*val >> shift) & 0xff;
> > +   break;
> > +   default:
> > +   return PCIBIOS_BAD_REGISTER_NUMBER;
> > +   }
> 
> Do we really need case 3? I guess case 1, 2 or 4 should be enough.
> 

I will change this as the following snippet:
if (size == 1)
*val = (*val >> (8 * (where & 3))) & 0xff;
else if (size == 2)
*val = (*val >> (8 * (where & 3))) & 0x;

thanks.

> > +   return PCIBIOS_SUCCESSFUL;
> > +}
> > +
> > +static int mtk_pcie_hw_wr_cfg(struct mtk_pcie_port *port, u32 bus, u32 
> > devfn,
> > + int where, int size, u32 val)
> > +{
> > +   /* Write PCIe Configuration Transaction Header for Cfgwr */
> > +   writel(CFG_HEADER_DW0(CFG_WRRD_TYPE_0, CFG_WR_FMT),
> > +  port->base + PCIE_CFG_HEADER0);
> > +   writel(CFG_HEADER_DW1(where, size), port->base + PCIE_CFG_HEADER1);
> > +   writel(CFG_HEADER_DW2(where, PCI_FUNC(devfn), PCI_SLOT(devfn), bus),
> > +  port->base + PCIE_CFG_HEADER2);
> > +
> > +   /* Write cfgwr data */
> > +   val = val << 8 * (where & 3);
> > +   writel(val, port->base + PCIE_CFG_WDATA);
> > +
> > +   /* Trigger h/w to transmit Cfgwr TLP */
> > +   val = readl(port->base + PCIE_APP_TLP_REQ);
> > +   val |= APP_CFG_REQ;
> > +   writel(val, port->base + PCIE_APP_TLP_REQ);
> > +
> > +   /* Check completion status */
> > +   return mtk_pcie_check_cfg_cpld(port);
> > +}
> > +
> > +static int mtk_pcie_config_read(struct pci_bus *bus, unsigned int devfn,
> > +   int where, int size, u32 *val)
> > +{
> > +   struct mtk_pcie_port *port = NULL, *iter, *tmp;
> > +   struct mtk_pcie *pcie = bus->sysdata;
> > +   u32 bn = bus->number;
> > +   int ret;
> > +
> > +   list_for_each_entry_safe(iter, tmp, >ports, list)
> > +   if (iter->index == PCI_SLOT(devfn)) {
> > +   port = iter;
> > +   break;
> > +   }
> > +
> > +   if (!port) {
> > +   *val = ~0;
> > +   return PCIBIOS_DEVICE_NOT_FOUND;
> > +   }
> 
> list_for_each_entry(), since you don't really remove or free something.
> 
> I know you need to find port->base to write/read configuration space. I
> think it's better to move this part to another function. You can take a
> look at pci-mvebu.c mvebu_pcie_find_portmvebu().


Sure, I will add the mtk_pcie_find_port interface, then the code is a
bit more readable.

thanks.

>  
> > +   ret = mtk_pcie_hw_rd_cfg(port, bn, devfn, where, size, val);
> > +   if (ret)
> > +   *val = ~0;
> > +
> > +   return ret;
> > +}
> > +
> > +static int mtk_pcie_config_write(struct pci_bus *bus, unsigned int devfn,
> > +int where, int size, u32 val)
> > +{
> > +   struct mtk_pcie_port *port = NULL, *iter, *tmp;
> > +   struct mtk_pcie *pcie = bus->sysdata;
> > +   u32 bn = bus->number;
> > +
> > +   list_for_each_entry_safe(iter, tmp, >ports, list)
> > +   if (iter->index == PCI_SLOT(devfn)) {
> > +   port = iter;
> > +   break;
> > +   }
> > +
> > +   if (!port)
> > +   return PCIBIOS_DEVICE_NOT_FOUND;
> 
> Ditto.
> 
> >

Re: [PATCH v3 5/6] PCI: mediatek: Add new generation controller support

2017-08-05 Thread Ryder Lee 
Hi Honghui,

If you plan to send next version, then I would suggest some minor
changes.

On Fri, 2017-08-04 at 20:06 +0800, honghui.zh...@mediatek.com wrote:
> +#define PCIE_CRSTB   BIT(3)
> +#define PCIE_PERSTB  BIT(8)
> +#define PCI_LINKDOWN_RST_EN  GENMASK(15, 13)

PCIE_LINKDOWN_RST_EN

> +#define PCIE_LINK_STATUS_V2  0x804
> +#define PCIE_PORT_LINKUP_V2  BIT(10)
> +
> +
> +static int mtk_pcie_hw_rd_cfg(struct mtk_pcie_port *port, u32 bus, u32 devfn,
> +   int where, int size, u32 *val)
> +{
> + int reg, shift = 8 * (where & 3);

int reg => u32

> + /* Write PCIe Configuration Transaction Header for cfgrd */
> + writel(CFG_HEADER_DW0(CFG_WRRD_TYPE_0, CFG_RD_FMT),
> +port->base + PCIE_CFG_HEADER0);
> + writel(CFG_HEADER_DW1(where, size), port->base + PCIE_CFG_HEADER1);
> + writel(CFG_HEADER_DW2(where, PCI_FUNC(devfn), PCI_SLOT(devfn), bus),
> +port->base + PCIE_CFG_HEADER2);
> +
> + /* Trigger h/w to transmit Cfgrd TLP */
> + reg = readl(port->base + PCIE_APP_TLP_REQ);
> + writel(reg | APP_CFG_REQ, port->base + PCIE_APP_TLP_REQ);
> +
> + /* Check completion status */
> + if (mtk_pcie_check_cfg_cpld(port))
> + return PCIBIOS_SET_FAILED;
> +
> + /* Read cpld payload of Cfgrd */
> + *val = readl(port->base + PCIE_CFG_RDATA);
> +
> + switch (size) {
> + case 4:
> + break;
> + case 3:
> + *val = (*val >> shift) & 0xff;
> + break;
> + case 2:
> + *val = (*val >> shift) & 0x;
> + break;
> + case 1:
> + *val = (*val >> shift) & 0xff;
> + break;
> + default:
> + return PCIBIOS_BAD_REGISTER_NUMBER;
> + }

Do we really need case 3? I guess case 1, 2 or 4 should be enough.

> + return PCIBIOS_SUCCESSFUL;
> +}
> +
> +static int mtk_pcie_hw_wr_cfg(struct mtk_pcie_port *port, u32 bus, u32 devfn,
> +   int where, int size, u32 val)
> +{
> + /* Write PCIe Configuration Transaction Header for Cfgwr */
> + writel(CFG_HEADER_DW0(CFG_WRRD_TYPE_0, CFG_WR_FMT),
> +port->base + PCIE_CFG_HEADER0);
> + writel(CFG_HEADER_DW1(where, size), port->base + PCIE_CFG_HEADER1);
> + writel(CFG_HEADER_DW2(where, PCI_FUNC(devfn), PCI_SLOT(devfn), bus),
> +port->base + PCIE_CFG_HEADER2);
> +
> + /* Write cfgwr data */
> + val = val << 8 * (where & 3);
> + writel(val, port->base + PCIE_CFG_WDATA);
> +
> + /* Trigger h/w to transmit Cfgwr TLP */
> + val = readl(port->base + PCIE_APP_TLP_REQ);
> + val |= APP_CFG_REQ;
> + writel(val, port->base + PCIE_APP_TLP_REQ);
> +
> + /* Check completion status */
> + return mtk_pcie_check_cfg_cpld(port);
> +}
> +
> +static int mtk_pcie_config_read(struct pci_bus *bus, unsigned int devfn,
> + int where, int size, u32 *val)
> +{
> + struct mtk_pcie_port *port = NULL, *iter, *tmp;
> + struct mtk_pcie *pcie = bus->sysdata;
> + u32 bn = bus->number;
> + int ret;
> +
> + list_for_each_entry_safe(iter, tmp, >ports, list)
> + if (iter->index == PCI_SLOT(devfn)) {
> + port = iter;
> + break;
> + }
> +
> + if (!port) {
> + *val = ~0;
> + return PCIBIOS_DEVICE_NOT_FOUND;
> + }

list_for_each_entry(), since you don't really remove or free something.

I know you need to find port->base to write/read configuration space. I
think it's better to move this part to another function. You can take a
look at pci-mvebu.c mvebu_pcie_find_portmvebu().
 
> + ret = mtk_pcie_hw_rd_cfg(port, bn, devfn, where, size, val);
> + if (ret)
> + *val = ~0;
> +
> + return ret;
> +}
> +
> +static int mtk_pcie_config_write(struct pci_bus *bus, unsigned int devfn,
> +  int where, int size, u32 val)
> +{
> + struct mtk_pcie_port *port = NULL, *iter, *tmp;
> + struct mtk_pcie *pcie = bus->sysdata;
> + u32 bn = bus->number;
> +
> + list_for_each_entry_safe(iter, tmp, >ports, list)
> + if (iter->index == PCI_SLOT(devfn)) {
> + port = iter;
> + break;
> + }
> +
> + if (!port)
> + return PCIBIOS_DEVICE_NOT_FOUND;

Ditto.

> + return mtk_pcie_hw_wr_cfg(port, bn, devfn, where, size, val);
> +}
> +
> +static struct pci_ops mtk_pcie_ops_v2 = {
> + .read  = mtk_pcie_config_read,
> + .write = mtk_pcie_config_write,
> +};
> +
> +static int mtk_pcie_startup_ports_v2(struct mtk_pcie_port *port)
> +{
> + struct mtk_pcie *pcie = port->pcie;
> + struct resource *mem = >mem;
> + u32 val;
> + size_t size;
> + int err;
> +
> + /* mt7622 platforms need to enable LTSSM and ASPM from PCIe subsys */
> + if (pcie->base) {
> + val = readl(pcie->base + PCIE_SYS_CFG_V2);
> +

Re: [PATCH v3 5/6] PCI: mediatek: Add new generation controller support

2017-08-05 Thread Ryder Lee 
Hi Honghui,

If you plan to send next version, then I would suggest some minor
changes.

On Fri, 2017-08-04 at 20:06 +0800, honghui.zh...@mediatek.com wrote:
> +#define PCIE_CRSTB   BIT(3)
> +#define PCIE_PERSTB  BIT(8)
> +#define PCI_LINKDOWN_RST_EN  GENMASK(15, 13)

PCIE_LINKDOWN_RST_EN

> +#define PCIE_LINK_STATUS_V2  0x804
> +#define PCIE_PORT_LINKUP_V2  BIT(10)
> +
> +
> +static int mtk_pcie_hw_rd_cfg(struct mtk_pcie_port *port, u32 bus, u32 devfn,
> +   int where, int size, u32 *val)
> +{
> + int reg, shift = 8 * (where & 3);

int reg => u32

> + /* Write PCIe Configuration Transaction Header for cfgrd */
> + writel(CFG_HEADER_DW0(CFG_WRRD_TYPE_0, CFG_RD_FMT),
> +port->base + PCIE_CFG_HEADER0);
> + writel(CFG_HEADER_DW1(where, size), port->base + PCIE_CFG_HEADER1);
> + writel(CFG_HEADER_DW2(where, PCI_FUNC(devfn), PCI_SLOT(devfn), bus),
> +port->base + PCIE_CFG_HEADER2);
> +
> + /* Trigger h/w to transmit Cfgrd TLP */
> + reg = readl(port->base + PCIE_APP_TLP_REQ);
> + writel(reg | APP_CFG_REQ, port->base + PCIE_APP_TLP_REQ);
> +
> + /* Check completion status */
> + if (mtk_pcie_check_cfg_cpld(port))
> + return PCIBIOS_SET_FAILED;
> +
> + /* Read cpld payload of Cfgrd */
> + *val = readl(port->base + PCIE_CFG_RDATA);
> +
> + switch (size) {
> + case 4:
> + break;
> + case 3:
> + *val = (*val >> shift) & 0xff;
> + break;
> + case 2:
> + *val = (*val >> shift) & 0x;
> + break;
> + case 1:
> + *val = (*val >> shift) & 0xff;
> + break;
> + default:
> + return PCIBIOS_BAD_REGISTER_NUMBER;
> + }

Do we really need case 3? I guess case 1, 2 or 4 should be enough.

> + return PCIBIOS_SUCCESSFUL;
> +}
> +
> +static int mtk_pcie_hw_wr_cfg(struct mtk_pcie_port *port, u32 bus, u32 devfn,
> +   int where, int size, u32 val)
> +{
> + /* Write PCIe Configuration Transaction Header for Cfgwr */
> + writel(CFG_HEADER_DW0(CFG_WRRD_TYPE_0, CFG_WR_FMT),
> +port->base + PCIE_CFG_HEADER0);
> + writel(CFG_HEADER_DW1(where, size), port->base + PCIE_CFG_HEADER1);
> + writel(CFG_HEADER_DW2(where, PCI_FUNC(devfn), PCI_SLOT(devfn), bus),
> +port->base + PCIE_CFG_HEADER2);
> +
> + /* Write cfgwr data */
> + val = val << 8 * (where & 3);
> + writel(val, port->base + PCIE_CFG_WDATA);
> +
> + /* Trigger h/w to transmit Cfgwr TLP */
> + val = readl(port->base + PCIE_APP_TLP_REQ);
> + val |= APP_CFG_REQ;
> + writel(val, port->base + PCIE_APP_TLP_REQ);
> +
> + /* Check completion status */
> + return mtk_pcie_check_cfg_cpld(port);
> +}
> +
> +static int mtk_pcie_config_read(struct pci_bus *bus, unsigned int devfn,
> + int where, int size, u32 *val)
> +{
> + struct mtk_pcie_port *port = NULL, *iter, *tmp;
> + struct mtk_pcie *pcie = bus->sysdata;
> + u32 bn = bus->number;
> + int ret;
> +
> + list_for_each_entry_safe(iter, tmp, >ports, list)
> + if (iter->index == PCI_SLOT(devfn)) {
> + port = iter;
> + break;
> + }
> +
> + if (!port) {
> + *val = ~0;
> + return PCIBIOS_DEVICE_NOT_FOUND;
> + }

list_for_each_entry(), since you don't really remove or free something.

I know you need to find port->base to write/read configuration space. I
think it's better to move this part to another function. You can take a
look at pci-mvebu.c mvebu_pcie_find_portmvebu().
 
> + ret = mtk_pcie_hw_rd_cfg(port, bn, devfn, where, size, val);
> + if (ret)
> + *val = ~0;
> +
> + return ret;
> +}
> +
> +static int mtk_pcie_config_write(struct pci_bus *bus, unsigned int devfn,
> +  int where, int size, u32 val)
> +{
> + struct mtk_pcie_port *port = NULL, *iter, *tmp;
> + struct mtk_pcie *pcie = bus->sysdata;
> + u32 bn = bus->number;
> +
> + list_for_each_entry_safe(iter, tmp, >ports, list)
> + if (iter->index == PCI_SLOT(devfn)) {
> + port = iter;
> + break;
> + }
> +
> + if (!port)
> + return PCIBIOS_DEVICE_NOT_FOUND;

Ditto.

> + return mtk_pcie_hw_wr_cfg(port, bn, devfn, where, size, val);
> +}
> +
> +static struct pci_ops mtk_pcie_ops_v2 = {
> + .read  = mtk_pcie_config_read,
> + .write = mtk_pcie_config_write,
> +};
> +
> +static int mtk_pcie_startup_ports_v2(struct mtk_pcie_port *port)
> +{
> + struct mtk_pcie *pcie = port->pcie;
> + struct resource *mem = >mem;
> + u32 val;
> + size_t size;
> + int err;
> +
> + /* mt7622 platforms need to enable LTSSM and ASPM from PCIe subsys */
> + if (pcie->base) {
> + val = readl(pcie->base + PCIE_SYS_CFG_V2);
> +

[PATCH v3 5/6] PCI: mediatek: Add new generation controller support

2017-08-04 Thread honghui.zhang 
From: Ryder Lee 

MediaTek's PCIe host controller has two generation HWs, the new
generation HW has two root ports, it shares most probing flow with the
legacy controller. But the read/write config space logical is different
from the legacy controller. The per-port register must be touched for
read/write config space, And the per-port register base are in separate
address space.

Add support for new Gen2 controller which can be found on MT7622/MT2712.

Signed-off-by: Ryder Lee 
Signed-off-by: Honghui Zhang 
---
 drivers/pci/host/Kconfig |   5 +-
 drivers/pci/host/pcie-mediatek.c | 473 ++-
 2 files changed, 471 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index 89d61c2..5b1ae9f 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -182,14 +182,13 @@ config PCIE_ROCKCHIP
 
 config PCIE_MEDIATEK
bool "MediaTek PCIe controller"
-   depends on ARM && (ARCH_MEDIATEK || COMPILE_TEST)
+   depends on (ARM || ARM64) && (ARCH_MEDIATEK || COMPILE_TEST)
depends on OF
depends on PCI
select PCIEPORTBUS
help
  Say Y here if you want to enable PCIe controller support on
- MT7623 series SoCs.  There is one single root complex with 3 root
- ports available.  Each port supports Gen2 lane x1.
+ MediaTek SoCs.
 
 config PCIE_TANGO_SMP8759
bool "Tango SMP8759 PCIe controller (DANGEROUS)"
diff --git a/drivers/pci/host/pcie-mediatek.c b/drivers/pci/host/pcie-mediatek.c
index 63d300f..91eb53b 100644
--- a/drivers/pci/host/pcie-mediatek.c
+++ b/drivers/pci/host/pcie-mediatek.c
@@ -3,6 +3,7 @@
  *
  * Copyright (c) 2017 MediaTek Inc.
  * Author: Ryder Lee 
+ *Honghui Zhang 
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
@@ -17,6 +18,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -64,16 +67,77 @@
 #define PCIE_FC_CREDIT_MASK(GENMASK(31, 31) | GENMASK(28, 16))
 #define PCIE_FC_CREDIT_VAL(x)  ((x) << 16)
 
+/* PCIe V2 share registers */
+#define PCIE_SYS_CFG_V20x0
+#define PCIE_CSR_LTSSM_EN(x)   BIT(0 + (x) * 8)
+#define PCIE_CSR_ASPM_L1_EN(x) BIT(1 + (x) * 8)
+
+/* PCIe V2 per-port registers */
+#define PCIE_INT_MASK  0x420
+#define INTX_MASK  GENMASK(19, 16)
+#define INTX_SHIFT 16
+#define INTX_NUM   4
+#define PCIE_INT_STATUS0x424
+
+#define PCIE_AHB_TRANS_BASE0_L 0x438
+#define PCIE_AHB_TRANS_BASE0_H 0x43c
+#define AHB2PCIE_BASEL(x)  ((x) & GENMASK(31, 0))
+#define AHB2PCIE_BASEH(x)  (((u64)(x) >> 32) & GENMASK(31, 0))
+#define AHB2PCIE_SIZE(x)   ((x) & GENMASK(4, 0))
+#define PCIE_AXI_WINDOW0   0x448
+#define WIN_ENABLE BIT(7)
+
+/* PCIe V2 Configuration Transaction Header */
+#define PCIE_CFG_HEADER0   0x460
+#define PCIE_CFG_HEADER1   0x464
+#define PCIE_CFG_HEADER2   0x468
+#define PCIE_CFG_WDATA 0x470
+#define PCIE_APP_TLP_REQ   0x488
+#define PCIE_CFG_RDATA 0x48c
+#define APP_CFG_REQBIT(0)
+#define APP_CPL_STATUS GENMASK(7, 5)
+
+#define CFG_WRRD_TYPE_04
+#define CFG_WR_FMT 2
+#define CFG_RD_FMT 0
+
+#define CFG_DW0_LENGTH(length) ((length) & GENMASK(9, 0))
+#define CFG_DW0_TYPE(type) (((type) << 24) & GENMASK(28, 24))
+#define CFG_DW0_FMT(fmt)   (((fmt) << 29) & GENMASK(31, 29))
+#define CFG_DW2_REGN(regn) ((regn) & GENMASK(11, 2))
+#define CFG_DW2_FUN(fun)   (((fun) << 16) & GENMASK(18, 16))
+#define CFG_DW2_DEV(dev)   (((dev) << 19) & GENMASK(23, 19))
+#define CFG_DW2_BUS(bus)   (((bus) << 24) & GENMASK(31, 24))
+#define CFG_HEADER_DW0(type, fmt) \
+   (CFG_DW0_LENGTH(1) | CFG_DW0_TYPE(type) | CFG_DW0_FMT(fmt))
+#define CFG_HEADER_DW1(where, size) \
+   (GENMASK(((size) - 1), 0) << ((where) & 0x3))
+#define CFG_HEADER_DW2(regn, fun, dev, bus) \
+   (CFG_DW2_REGN(regn) | CFG_DW2_FUN(fun) | \
+   CFG_DW2_DEV(dev) | CFG_DW2_BUS(bus))
+
+#define PCIE_RST_CTRL  0x510
+#define PCIE_PHY_RSTB  BIT(0)
+#define PCIE_PIPE_SRSTBBIT(1)
+#define PCIE_MAC_SRSTB BIT(2)
+#define PCIE_CRSTB BIT(3)
+#define PCIE_PERSTBBIT(8)
+#define PCI_LINKDOWN_RST_ENGENMASK(15, 13)
+#define PCIE_LINK_STATUS_V20x804
+#define PCIE_PORT_LINKUP_V2BIT(10)
+
 struct mtk_pcie_port;
 
 /**
  * struct mtk_pcie_soc - differentiate between host generations
  * @ops: pointer to configuration access functions
  * @startup: pointer to controller setting functions
+ * @setup_irq: pointer to initialize IRQ functions
  */
 struct mtk_pcie_soc {
struct pci_ops *ops;
int

[PATCH v3 5/6] PCI: mediatek: Add new generation controller support

2017-08-04 Thread honghui.zhang 
From: Ryder Lee 

MediaTek's PCIe host controller has two generation HWs, the new
generation HW has two root ports, it shares most probing flow with the
legacy controller. But the read/write config space logical is different
from the legacy controller. The per-port register must be touched for
read/write config space, And the per-port register base are in separate
address space.

Add support for new Gen2 controller which can be found on MT7622/MT2712.

Signed-off-by: Ryder Lee 
Signed-off-by: Honghui Zhang 
---
 drivers/pci/host/Kconfig |   5 +-
 drivers/pci/host/pcie-mediatek.c | 473 ++-
 2 files changed, 471 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index 89d61c2..5b1ae9f 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -182,14 +182,13 @@ config PCIE_ROCKCHIP
 
 config PCIE_MEDIATEK
bool "MediaTek PCIe controller"
-   depends on ARM && (ARCH_MEDIATEK || COMPILE_TEST)
+   depends on (ARM || ARM64) && (ARCH_MEDIATEK || COMPILE_TEST)
depends on OF
depends on PCI
select PCIEPORTBUS
help
  Say Y here if you want to enable PCIe controller support on
- MT7623 series SoCs.  There is one single root complex with 3 root
- ports available.  Each port supports Gen2 lane x1.
+ MediaTek SoCs.
 
 config PCIE_TANGO_SMP8759
bool "Tango SMP8759 PCIe controller (DANGEROUS)"
diff --git a/drivers/pci/host/pcie-mediatek.c b/drivers/pci/host/pcie-mediatek.c
index 63d300f..91eb53b 100644
--- a/drivers/pci/host/pcie-mediatek.c
+++ b/drivers/pci/host/pcie-mediatek.c
@@ -3,6 +3,7 @@
  *
  * Copyright (c) 2017 MediaTek Inc.
  * Author: Ryder Lee 
+ *Honghui Zhang 
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
@@ -17,6 +18,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include 
 #include 
 #include 
@@ -64,16 +67,77 @@
 #define PCIE_FC_CREDIT_MASK(GENMASK(31, 31) | GENMASK(28, 16))
 #define PCIE_FC_CREDIT_VAL(x)  ((x) << 16)
 
+/* PCIe V2 share registers */
+#define PCIE_SYS_CFG_V20x0
+#define PCIE_CSR_LTSSM_EN(x)   BIT(0 + (x) * 8)
+#define PCIE_CSR_ASPM_L1_EN(x) BIT(1 + (x) * 8)
+
+/* PCIe V2 per-port registers */
+#define PCIE_INT_MASK  0x420
+#define INTX_MASK  GENMASK(19, 16)
+#define INTX_SHIFT 16
+#define INTX_NUM   4
+#define PCIE_INT_STATUS0x424
+
+#define PCIE_AHB_TRANS_BASE0_L 0x438
+#define PCIE_AHB_TRANS_BASE0_H 0x43c
+#define AHB2PCIE_BASEL(x)  ((x) & GENMASK(31, 0))
+#define AHB2PCIE_BASEH(x)  (((u64)(x) >> 32) & GENMASK(31, 0))
+#define AHB2PCIE_SIZE(x)   ((x) & GENMASK(4, 0))
+#define PCIE_AXI_WINDOW0   0x448
+#define WIN_ENABLE BIT(7)
+
+/* PCIe V2 Configuration Transaction Header */
+#define PCIE_CFG_HEADER0   0x460
+#define PCIE_CFG_HEADER1   0x464
+#define PCIE_CFG_HEADER2   0x468
+#define PCIE_CFG_WDATA 0x470
+#define PCIE_APP_TLP_REQ   0x488
+#define PCIE_CFG_RDATA 0x48c
+#define APP_CFG_REQBIT(0)
+#define APP_CPL_STATUS GENMASK(7, 5)
+
+#define CFG_WRRD_TYPE_04
+#define CFG_WR_FMT 2
+#define CFG_RD_FMT 0
+
+#define CFG_DW0_LENGTH(length) ((length) & GENMASK(9, 0))
+#define CFG_DW0_TYPE(type) (((type) << 24) & GENMASK(28, 24))
+#define CFG_DW0_FMT(fmt)   (((fmt) << 29) & GENMASK(31, 29))
+#define CFG_DW2_REGN(regn) ((regn) & GENMASK(11, 2))
+#define CFG_DW2_FUN(fun)   (((fun) << 16) & GENMASK(18, 16))
+#define CFG_DW2_DEV(dev)   (((dev) << 19) & GENMASK(23, 19))
+#define CFG_DW2_BUS(bus)   (((bus) << 24) & GENMASK(31, 24))
+#define CFG_HEADER_DW0(type, fmt) \
+   (CFG_DW0_LENGTH(1) | CFG_DW0_TYPE(type) | CFG_DW0_FMT(fmt))
+#define CFG_HEADER_DW1(where, size) \
+   (GENMASK(((size) - 1), 0) << ((where) & 0x3))
+#define CFG_HEADER_DW2(regn, fun, dev, bus) \
+   (CFG_DW2_REGN(regn) | CFG_DW2_FUN(fun) | \
+   CFG_DW2_DEV(dev) | CFG_DW2_BUS(bus))
+
+#define PCIE_RST_CTRL  0x510
+#define PCIE_PHY_RSTB  BIT(0)
+#define PCIE_PIPE_SRSTBBIT(1)
+#define PCIE_MAC_SRSTB BIT(2)
+#define PCIE_CRSTB BIT(3)
+#define PCIE_PERSTBBIT(8)
+#define PCI_LINKDOWN_RST_ENGENMASK(15, 13)
+#define PCIE_LINK_STATUS_V20x804
+#define PCIE_PORT_LINKUP_V2BIT(10)
+
 struct mtk_pcie_port;
 
 /**
  * struct mtk_pcie_soc - differentiate between host generations
  * @ops: pointer to configuration access functions
  * @startup: pointer to controller setting functions
+ * @setup_irq: pointer to initialize IRQ functions
  */
 struct mtk_pcie_soc {
struct pci_ops *ops;
int (*startup)(struct mtk_pcie_port *port);
+   int (*setup_irq)(struct mtk_pcie_port *port, struct device_node *node);
 };