lkml.org 
[lkml]   [2018]   [Dec]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v6 2/2] PCI: amlogic: Add the Amlogic Meson PCIe controller driver
    On Thu, Nov 22, 2018 at 04:53:54PM +0800, Hanjie Lin wrote:

    [...]

    > +static int meson_pcie_rd_own_conf(struct pcie_port *pp, int where, int size,
    > + u32 *val)
    > +{
    > + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
    > +
    > + /*
    > + * there is a bug of MESON AXG pcie controller that software can not
    > + * programe PCI_CLASS_DEVICE register, so we must return a fake right
    > + * value to ensure driver could probe successfully.
    > + */
    > + if (where == PCI_CLASS_REVISION) {
    > + *val = readl(pci->dbi_base + PCI_CLASS_REVISION);
    > + /* keep revision id */
    > + *val &= PCI_CLASS_REVISION_MASK;
    > + *val |= PCI_CLASS_BRIDGE_PCI << 16;
    > + return PCIBIOS_SUCCESSFUL;
    > + }

    As I said before, this looks broken. If this code (or other drivers with
    the same broken assumptions, eg dwc/pcie-qcom.c) carries out a, say,
    byte sized config access of eg PCI_CLASS_DEVICE you will get junk out of
    it according to your comment above.

    I would like to pick Bjorn's brain on this to see what we can really do
    to fix this (and other) drivers.

    Thanks,
    Lorenzo

    > + return dw_pcie_read(pci->dbi_base + where, size, val);
    > +}
    > +
    > +static int meson_pcie_wr_own_conf(struct pcie_port *pp, int where,
    > + int size, u32 val)
    > +{
    > + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
    > +
    > + return dw_pcie_write(pci->dbi_base + where, size, val);
    > +}
    > +
    > +static int meson_pcie_link_up(struct dw_pcie *pci)
    > +{
    > + struct meson_pcie *mp = to_meson_pcie(pci);
    > + struct device *dev = pci->dev;
    > + u32 smlh_up = 0;
    > + u32 ltssm_up = 0;
    > + u32 rdlh_up = 0;
    > + u32 speed_okay = 0;
    > + u32 cnt = 0;
    > + u32 state12, state17;
    > +
    > + while (smlh_up == 0 || rdlh_up == 0 || ltssm_up == 0 ||
    > + speed_okay == 0) {
    > + udelay(20);
    > +
    > + state12 = meson_cfg_readl(mp, PCIE_CFG_STATUS12);
    > + state17 = meson_cfg_readl(mp, PCIE_CFG_STATUS17);
    > + smlh_up = IS_SMLH_LINK_UP(state12);
    > + rdlh_up = IS_RDLH_LINK_UP(state12);
    > + ltssm_up = IS_LTSSM_UP(state12);
    > +
    > + if (PM_CURRENT_STATE(state17) < PCIE_GEN3)
    > + speed_okay = 1;
    > +
    > + if (smlh_up)
    > + dev_dbg(dev, "smlh_link_up is on\n");
    > + if (rdlh_up)
    > + dev_dbg(dev, "rdlh_link_up is on\n");
    > + if (ltssm_up)
    > + dev_dbg(dev, "ltssm_up is on\n");
    > + if (speed_okay)
    > + dev_dbg(dev, "speed_okay\n");
    > +
    > + cnt++;
    > +
    > + if (cnt >= WAIT_LINKUP_TIMEOUT) {
    > + dev_err(dev, "Error: Wait linkup timeout.\n");
    > + return 0;
    > + }
    > + }
    > +
    > + return 1;
    > +}
    > +
    > +static int meson_pcie_host_init(struct pcie_port *pp)
    > +{
    > + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
    > + struct meson_pcie *mp = to_meson_pcie(pci);
    > + int ret;
    > +
    > + ret = meson_pcie_establish_link(mp);
    > + if (ret)
    > + return ret;
    > +
    > + meson_pcie_enable_interrupts(mp);
    > +
    > + return 0;
    > +}
    > +
    > +static const struct dw_pcie_host_ops meson_pcie_host_ops = {
    > + .rd_own_conf = meson_pcie_rd_own_conf,
    > + .wr_own_conf = meson_pcie_wr_own_conf,
    > + .host_init = meson_pcie_host_init,
    > +};
    > +
    > +static int meson_add_pcie_port(struct meson_pcie *mp,
    > + struct platform_device *pdev)
    > +{
    > + struct dw_pcie *pci = &mp->pci;
    > + struct pcie_port *pp = &pci->pp;
    > + struct device *dev = &pdev->dev;
    > + int ret;
    > +
    > + if (IS_ENABLED(CONFIG_PCI_MSI)) {
    > + pp->msi_irq = platform_get_irq(pdev, 0);
    > + if (pp->msi_irq < 0) {
    > + dev_err(dev, "failed to get msi irq\n");
    > + return pp->msi_irq;
    > + }
    > + }
    > +
    > + pp->ops = &meson_pcie_host_ops;
    > + pci->dbi_base = mp->mem_res.elbi_base;
    > +
    > + ret = dw_pcie_host_init(pp);
    > + if (ret) {
    > + dev_err(dev, "failed to initialize host\n");
    > + return ret;
    > + }
    > +
    > + return 0;
    > +}
    > +
    > +static const struct dw_pcie_ops dw_pcie_ops = {
    > + .link_up = meson_pcie_link_up,
    > +};
    > +
    > +static int meson_pcie_probe(struct platform_device *pdev)
    > +{
    > + struct device *dev = &pdev->dev;
    > + struct dw_pcie *pci;
    > + struct meson_pcie *mp;
    > + int ret;
    > +
    > + mp = devm_kzalloc(dev, sizeof(*mp), GFP_KERNEL);
    > + if (!mp)
    > + return -ENOMEM;
    > +
    > + pci = &mp->pci;
    > + pci->dev = dev;
    > + pci->ops = &dw_pcie_ops;
    > +
    > + mp->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
    > + if (IS_ERR(mp->reset_gpio)) {
    > + dev_err(dev, "Get reset gpio failed\n");
    > + return PTR_ERR(mp->reset_gpio);
    > + }
    > +
    > + ret = meson_pcie_get_resets(mp);
    > + if (ret) {
    > + dev_err(dev, "Get reset resource failed, %d\n", ret);
    > + return ret;
    > + }
    > +
    > + ret = meson_pcie_get_mems(pdev, mp);
    > + if (ret) {
    > + dev_err(dev, "Get memory resource failed, %d\n", ret);
    > + return ret;
    > + }
    > +
    > + meson_pcie_power_on(mp);
    > + meson_pcie_reset(mp);
    > +
    > + ret = meson_pcie_probe_clocks(mp);
    > + if (ret) {
    > + dev_err(dev, "Init clock resources failed, %d\n", ret);
    > + return ret;
    > + }
    > +
    > + platform_set_drvdata(pdev, mp);
    > +
    > + ret = meson_add_pcie_port(mp, pdev);
    > + if (ret < 0) {
    > + dev_err(dev, "Add PCIE port failed, %d\n", ret);
    > + return ret;
    > + }
    > +
    > + return 0;
    > +}
    > +
    > +static const struct of_device_id meson_pcie_of_match[] = {
    > + {
    > + .compatible = "amlogic,axg-pcie",
    > + },
    > + {},
    > +};
    > +
    > +static struct platform_driver meson_pcie_driver = {
    > + .probe = meson_pcie_probe,
    > + .driver = {
    > + .name = "meson-pcie",
    > + .of_match_table = meson_pcie_of_match,
    > + },
    > +};
    > +
    > +builtin_platform_driver(meson_pcie_driver);
    > --
    > 2.7.4
    >

    \
     
     \ /
      Last update: 2018-12-03 17:43    [W:5.769 / U:0.548 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site