Thanks for reviewing and the comments. I will surely include them in next version. -Tanmay On Fri, Mar 7, 2014 at 12:37 AM, Jingoo Han <jg1.han@xxxxxxxxxxx> wrote: > On Thursday, March 06, 2014 3:06 PM, Tanmay Inamdar wrote: >> > > Hi Tanmay Inamdar, > > I added some minor comments. :-) > >> This patch adds the AppliedMicro X-Gene SOC PCIe controller driver. >> X-Gene PCIe controller supports maxmum upto 8 lanes and GEN3 speed. > > Would you fix the followings? > > s/maxmum/maximum > s/upto/up to > >> X-Gene SOC supports maximum 5 PCIe ports. >> >> Signed-off-by: Tanmay Inamdar <tinamdar@xxxxxxx> >> --- >> drivers/pci/host/Kconfig | 10 + >> drivers/pci/host/Makefile | 1 + >> drivers/pci/host/pci-xgene.c | 739 ++++++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 750 insertions(+) >> create mode 100644 drivers/pci/host/pci-xgene.c > > [.....] > >> --- /dev/null >> +++ b/drivers/pci/host/pci-xgene.c > > [.....] > >> + >> +static int xgene_pcie_parse_map_dma_ranges(struct xgene_pcie_port *port) >> +{ >> + struct device_node *np = port->node; >> + struct of_pci_range range; >> + struct of_pci_range_parser parser; >> + struct device *dev = port->dev; >> + u8 ib_reg_mask = 0; >> + >> + if (pci_dma_range_parser_init(&parser, np)) { >> + dev_err(dev, "missing dma-ranges property\n"); >> + return -EINVAL; >> + } >> + >> + /* Get the dma-ranges from DT */ >> + for_each_of_pci_range(&parser, &range) { >> + u64 end = range.cpu_addr + range.size - 1; >> + dev_dbg(port->dev, "0x%08x 0x%016llx..0x%016llx -> 0x%016llx\n", >> + range.flags, range.cpu_addr, end, range.pci_addr); >> + xgene_pcie_setup_ib_reg(port, &range, &ib_reg_mask); >> + } >> + return 0; >> +} >> + >> +static int __init xgene_pcie_probe_bridge(struct platform_device *pdev) > > Please, remove '__init' marker in order to fix section mismatch > warning. > >> +{ >> + struct device_node *np = of_node_get(pdev->dev.of_node); >> + struct xgene_pcie_port *port; >> + struct pci_host_bridge *bridge; >> + resource_size_t lastbus; >> + u32 lanes = 0, speed = 0; >> + u64 cfg_addr = 0; >> + int ret; >> + >> + port = devm_kzalloc(&pdev->dev, sizeof(*port), GFP_KERNEL); >> + if (!port) >> + return -ENOMEM; >> + port->node = np; >> + port->dev = &pdev->dev; >> + >> + ret = xgene_pcie_map_reg(port, pdev, &cfg_addr); >> + if (ret) >> + return ret; >> + >> + ret = xgene_pcie_init_port(port); >> + if (ret) >> + return ret; >> + xgene_pcie_program_core(port->csr_base); >> + xgene_pcie_setup_root_complex(port); >> + >> + bridge = of_create_pci_host_bridge(&pdev->dev, &xgene_pcie_ops, port); >> + if (IS_ERR_OR_NULL(bridge)) >> + return PTR_ERR(bridge); >> + >> + ret = xgene_pcie_map_ranges(port, bridge, cfg_addr); >> + if (ret) >> + return ret; >> + >> + ret = xgene_pcie_parse_map_dma_ranges(port); >> + if (ret) >> + return ret; >> + >> + xgene_pcie_poll_linkup(port, &lanes, &speed); >> + if (!port->link_up) >> + dev_info(port->dev, "(rc) link down\n"); >> + else >> + dev_info(port->dev, "(rc) x%d gen-%d link up\n", >> + lanes, speed + 1); >> + platform_set_drvdata(pdev, port); >> + lastbus = pci_rescan_bus(bridge->bus); >> + pci_bus_update_busn_res_end(bridge->bus, lastbus); >> + return 0; >> +} >> + >> +static const struct of_device_id xgene_pcie_match_table[] __initconst = { > > Please, remove '__initconst' marker in order to fix section mismatch > warning. > >> + {.compatible = "apm,xgene-pcie",}, >> + {}, >> +}; >> + >> +static struct platform_driver xgene_pcie_driver = { >> + .driver = { >> + .name = "xgene-pcie", >> + .owner = THIS_MODULE, >> + .of_match_table = of_match_ptr(xgene_pcie_match_table), >> + }, > > Please fix this indent style, as below: > > + }, > > See you later. :-) > > Best regards, > Jingoo Han > >> + .probe = xgene_pcie_probe_bridge, >> +}; >> +module_platform_driver(xgene_pcie_driver); >> + >> +MODULE_AUTHOR("Tanmay Inamdar <tinamdar@xxxxxxx>"); >> +MODULE_DESCRIPTION("APM X-Gene PCIe driver"); >> +MODULE_LICENSE("GPL v2"); >> -- >> 1.7.9.5 > -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html