| Message ID | 20220720060112epcms2p30a05414992cf814e5886af2b70c0f58f@epcms2p3 |
|---|---|
| State | New |
| Headers | show |
| Series | [v4,1/5] dt-bindings: pci: Add ARTPEC-8 PCIe controller | expand |
On 20/07/2022 08:01, Wangseok Lee wrote: > Add support Axis, ARTPEC-8 SoC. ARTPEC-8 is the SoC platform of Axis > Communications. This is based on arm64 and support GEN4 & 2lane. This > PCIe controller is based on DesignWare Hardware core and uses DesignWare > core functions to implement the driver. "pcie-artpec6. c" supports artpec6 > and artpec7 H/W. artpec8 can not be expanded because H/W configuration is > completely different from artpec6/7. PHY and sub controller are different. > > Signed-off-by: Wangseok Lee <wangseok.lee@samsung.com> > Signed-off-by: Jaeho Cho <jaeho79.cho@samsung.com> > --- > v3->v4 : > -Remove unnecessary enum type > -Fix indentation > Thanks for the changes. This starts to look good, however I am not going to ack it. This is also not a strong NAK, as I would respect Bjorn and other maintainers decision. I don't like the approach of creating only Artpec-8 specific driver. Samsung heavily reuses its block in all Exynos devices. Now it re-uses them for other designs as well. Therefore, even if merging with existing Exynos PCIe driver is not feasible (we had such discussions), I expect this to cover all Samsung Foundry PCIe devices. From all current designs up to future licensed blocks, including some new Samsung Exynos SoC. Or at least be ready for it. However it seems you are interested only in achieving one goal here - satisfy Axis. I believe it is not the "upstream approach". Next month you come up with same driver for different customer and you keep insisting "it's different!". To get my ack I want to see something generic for Samsung Exynos SoC and other licensed or designed blocks, instead of something made for only one of your customers. Best regards, Krzysztof
On Wed, Jul 20, 2022 at 03:01:12PM +0900, Wangseok Lee wrote: > Add support Axis, ARTPEC-8 SoC. ARTPEC-8 is the SoC platform of Axis > Communications. This is based on arm64 and support GEN4 & 2lane. This > PCIe controller is based on DesignWare Hardware core and uses DesignWare > core functions to implement the driver. "pcie-artpec6. c" supports artpec6 > and artpec7 H/W. artpec8 can not be expanded because H/W configuration is > completely different from artpec6/7. PHY and sub controller are different. > > Signed-off-by: Wangseok Lee <wangseok.lee@samsung.com> > Signed-off-by: Jaeho Cho <jaeho79.cho@samsung.com> Since you are the one sending the patch, your sign-off should be last: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=v5.18#n363 > +config PCIE_ARTPEC8_HOST > + bool "Axis ARTPEC-8 PCIe controller Host Mode" > + depends on ARCH_ARTPEC || COMPILE_TEST > + depends on PCI_MSI_IRQ_DOMAIN > + depends on PCI_ENDPOINT > + select PCI_EPF_TEST > + select PCIE_DW_HOST > + select PCIE_ARTPEC8 > + help > + Say 'Y' here to enable support for the PCIe controller in the > + ARTPEC-8 SoC to work in host mode. Add a blank line between paragraphs. Mentioned previously: https://lore.kernel.org/r/20220603160314.GA76545@bhelgaas > + This PCIe controller is based on DesignWare hardware core and > + uses DesignWare core functions to implement the driver. I don't think Kconfig users need to know how the driver is implemented. Not really even sure they need to know that it's based on DesignWare hardware. Users need to be able to connect the hardware in front of them with the drivers for it, which means they start with the name on the box. > +config PCIE_ARTPEC8_EP > + bool "Axis ARTPEC-8 PCIe controller Endpoint Mode" > + depends on ARCH_ARTPEC || COMPILE_TEST > + depends on PCI_ENDPOINT > + depends on PCI_ENDPOINT_CONFIGFS > + select PCI_EPF_TEST > + select PCIE_DW_EP > + select PCIE_ARTPEC8 > + help > + Say 'Y' here to enable support for the PCIe controller in the > + ARTPEC-8 SoC to work in endpoint mode. Same. > + This PCIe controller is based on DesignWare hardware core and > + uses DesignWare core functions to implement the driver. > +#define PCIE_GEN3_EQUALIZATION_DISABLE (0x1 << 16) Why the mix of "(0x1 << 16)" and "BIT(...)" below? Pick one. > +#define PCIE_GEN3_EQ_PHASE_2_3 (0x1 << 9) > +#define PCIE_GEN3_RXEQ_PH01_EN (0x1 << 12) > +#define PCIE_GEN3_RXEQ_RGRDLESS_RXTS (0x1 << 13) > + > +#define FAST_LINK_MODE (7) > + > +/* PCIe ELBI registers */ > +#define PCIE_IRQ0_STS 0x000 > +#define PCIE_IRQ1_STS 0x004 > +#define PCIE_IRQ2_STS 0x008 > +#define IRQ2_STS_IRQ_MSI_ST BIT(20) > +static const int artpec8_pcie_dbi_addr_con[] = { > + FSYS_PCIE_DBI_ADDR_CON Since this is an array, I assume you envision adding more entries eventually. That means you should have a comma at the end of this entry, as you do in artpec8_pcie_clks[] below, so the FSYS_PCIE_DBI_ADDR_CON line will not need to change when you add entries in the future. > +}; > +static u32 artpec8_pcie_read_dbi(struct dw_pcie *pci, void __iomem *base, > + u32 reg, size_t size) > +{ > + struct artpec8_pcie *artpec8_ctrl = to_artpec8_pcie(pci); Typical names for pointers like this are "dra7xx", "rockchip", "artpec6_pcie", and even "pcie". "artpec6_pcie" seems unnecessarily wordy. "pcie" seems possibly confusing since "pci" is the common pointer to struct dw_pcie. I think "artpec8" would be perfect. > + u32 val; > + bool is_atu = false; > + > + if (base == pci->atu_base) { > + is_atu = true; > + base = pci->dbi_base; > + regmap_write(artpec8_ctrl->sysreg, > + artpec8_pcie_dbi_addr_con[artpec8_ctrl->link_id], This is not safe because nothing guarantees that artpec8_ctrl->link_id == 0. Any other index other than zero would be out of bounds. That suggests that making artpec8_pcie_dbi_addr_con[] an array is over-engineered. If you need more than one value in the future, you could make it an array at that time. For now it just makes the code hard to read. > + FSYS_PCIE_DBI_ADDR_OVR_ATU); > + } > +static int artpec8_pcie_get_subsys_resources(struct platform_device *pdev, > + struct artpec8_pcie *artpec8_ctrl) > +{ > + struct device *dev = &pdev->dev; > + > + /* External Local Bus interface(ELBI) Register */ Add a space before things like "(ELBI)". Also below for "(DBI)". > + artpec8_ctrl->elbi_base = > + devm_platform_ioremap_resource_byname(pdev, "elbi"); > + if (IS_ERR(artpec8_ctrl->elbi_base)) { > + dev_err(dev, "failed to map elbi_base\n"); > + return PTR_ERR(artpec8_ctrl->elbi_base); > + } > + > + artpec8_ctrl->sysreg = > + syscon_regmap_lookup_by_phandle(dev->of_node, > + "samsung,fsys-sysreg"); > + if (IS_ERR(artpec8_ctrl->sysreg)) { > + dev_err(dev, "fsys sysreg regmap lookup failed.\n"); Omit periods on error messages (as you did for the elbi_base message above). This also applies to several messages below. > + return PTR_ERR(artpec8_ctrl->sysreg); > + } > + > + artpec8_ctrl->pmu_syscon = > + syscon_regmap_lookup_by_phandle(dev->of_node, > + "samsung,syscon-phandle"); > + if (IS_ERR(artpec8_ctrl->pmu_syscon)) { > + dev_err(dev, "pmu syscon regmap lookup failed.\n"); > + return PTR_ERR(artpec8_ctrl->pmu_syscon); > + } > +static int artpec8_pcie_get_rc_mem_resources(struct platform_device *pdev, > + struct artpec8_pcie *artpec8_ctrl) > +{ > + struct dw_pcie *pci = artpec8_ctrl->pci; > + > + /* Data Bus Interface(DBI) Register */ > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "addr_space"); > + if (!res) > + return -EINVAL; Add a blank line here. > + ep->phys_base = res->start; > + ep->addr_size = resource_size(res); > + ret = devm_clk_bulk_get(dev, ARRAY_SIZE(artpec8_pcie_clks), > + artpec8_pcie_clks); > + if (ret) > + return ret; > + > + return 0; This is the same as: return devm_clk_bulk_get(dev, ...); > + ret = clk_bulk_prepare_enable(ARRAY_SIZE(artpec8_pcie_clks), > + artpec8_pcie_clks); > + if (ret) > + return ret; > + > + return 0; Again, same as: return clk_bulk_prepare_enable(...); > +static u8 artpec8_pcie_iatu_unroll_enabled(struct dw_pcie *pci) > +{ > + u32 val; > + > + val = dw_pcie_readl_dbi(pci, PCIE_ATU_VIEWPORT); > + pci->atu_base = pci->dbi_base + DEFAULT_DBI_ATU_OFFSET; This line is out of place. This function is essentially a boolean and should not have side effects like updating pci->atu_base. > + > + if (val == 0xffffffff) > + return 1; > + > + return 0; This function as a whole is a copy of dw_pcie_iatu_unroll_enabled(), which is static in pcie-designware.c. I assume you'll resolve this eventually (maybe not in this initial patch) so we don't have two copies. > +static void artpec8_pcie_ep_init(struct dw_pcie_ep *ep) > +{ > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep); > + enum pci_barno bar; Add a blank line here. > + /* > + * Currently PCIe EP core is not setting iatu_unroll_enabled > + * so let's handle it here. We need to find proper place to > + * initialize this so that it can be used for other EP > + * controllers as well. > + */ > + pci->iatu_unroll_enabled = artpec8_pcie_iatu_unroll_enabled(pci); > +static int __init artpec8_add_pcie_ep(struct artpec8_pcie *artpec8_ctrl, > + struct platform_device *pdev) Why is this __init when the caller is not __init? > + ret = dw_pcie_ep_init(ep); > + if (ret) > + return ret; > + > + return 0; Again: return dw_pcie_ep_init(ep); > +static int __init artpec8_add_pcie_port(struct artpec8_pcie *artpec8_ctrl, > + struct platform_device *pdev) Why is this __init? > + pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL); > + if (!pci) > + return -ENOMEM; > + > + pdata = of_device_get_match_data(dev); > + > + if (!pdata) > + return -ENODEV; > + > + mode = (enum dw_pcie_device_mode)pdata->mode; > + > + artpec8_ctrl->pci = pci; > + artpec8_ctrl->pdata = pdata; > + artpec8_ctrl->mode = mode; > + > + pci->dev = dev; > + pci->ops = pdata->dwc_ops; > + pci->dbi_base2 = NULL; > + pci->dbi_base = NULL; These are guaranteed NULL already by the kzalloc(). > +static int __exit artpec8_pcie_remove(struct platform_device *pdev) Why do we need __exit and __exit_p below? exynos, keystone, and kirin use them, but I'm not sure they need them either. I don't know all the details, but most of our drivers don't use them, so I assume this one doesn't need them either. > +static struct platform_driver artpec8_pcie_driver = { > + .probe = artpec8_pcie_probe, > + .remove = __exit_p(artpec8_pcie_remove), __exit_p(), see above. > + .driver = { > + .name = "artpec8-pcie", > + .of_match_table = artpec8_pcie_of_match, > + }, > +};
On Thu, Jul 21, 2022 at 11:04:00AM +0200, Krzysztof Kozlowski wrote: > On 20/07/2022 08:01, Wangseok Lee wrote: > > Add support Axis, ARTPEC-8 SoC. ARTPEC-8 is the SoC platform of Axis > > Communications. This is based on arm64 and support GEN4 & 2lane. This > > PCIe controller is based on DesignWare Hardware core and uses DesignWare > > core functions to implement the driver. "pcie-artpec6. c" supports artpec6 > > and artpec7 H/W. artpec8 can not be expanded because H/W configuration is > > completely different from artpec6/7. PHY and sub controller are different. > > > > Signed-off-by: Wangseok Lee <wangseok.lee@samsung.com> > > Signed-off-by: Jaeho Cho <jaeho79.cho@samsung.com> > > --- > > v3->v4 : > > -Remove unnecessary enum type > > -Fix indentation > > > > Thanks for the changes. This starts to look good, however I am not going > to ack it. This is also not a strong NAK, as I would respect Bjorn and > other maintainers decision. > > I don't like the approach of creating only Artpec-8 specific driver. > Samsung heavily reuses its block in all Exynos devices. Now it re-uses > them for other designs as well. Therefore, even if merging with existing > Exynos PCIe driver is not feasible (we had such discussions), I expect > this to cover all Samsung Foundry PCIe devices. From all current designs > up to future licensed blocks, including some new Samsung Exynos SoC. Or > at least be ready for it. I would certainly prefer fewer drivers but I don't know enough about the underlying IP and the places it's integrated to to know what's practical. The only way I could figure that out would be by manually comparing the drivers for similarity. I assume/expect all driver authors are doing that. Bjorn
On 21/07/2022 22:58, Bjorn Helgaas wrote: > On Thu, Jul 21, 2022 at 11:04:00AM +0200, Krzysztof Kozlowski wrote: >> On 20/07/2022 08:01, Wangseok Lee wrote: >>> Add support Axis, ARTPEC-8 SoC. ARTPEC-8 is the SoC platform of Axis >>> Communications. This is based on arm64 and support GEN4 & 2lane. This >>> PCIe controller is based on DesignWare Hardware core and uses DesignWare >>> core functions to implement the driver. "pcie-artpec6. c" supports artpec6 >>> and artpec7 H/W. artpec8 can not be expanded because H/W configuration is >>> completely different from artpec6/7. PHY and sub controller are different. >>> >>> Signed-off-by: Wangseok Lee <wangseok.lee@samsung.com> >>> Signed-off-by: Jaeho Cho <jaeho79.cho@samsung.com> >>> --- >>> v3->v4 : >>> -Remove unnecessary enum type >>> -Fix indentation >>> >> >> Thanks for the changes. This starts to look good, however I am not going >> to ack it. This is also not a strong NAK, as I would respect Bjorn and >> other maintainers decision. >> >> I don't like the approach of creating only Artpec-8 specific driver. >> Samsung heavily reuses its block in all Exynos devices. Now it re-uses >> them for other designs as well. Therefore, even if merging with existing >> Exynos PCIe driver is not feasible (we had such discussions), I expect >> this to cover all Samsung Foundry PCIe devices. From all current designs >> up to future licensed blocks, including some new Samsung Exynos SoC. Or >> at least be ready for it. > > I would certainly prefer fewer drivers but I don't know enough about > the underlying IP and the places it's integrated to to know what's > practical. The only way I could figure that out would be by manually > comparing the drivers for similarity. I assume/expect all driver > authors are doing that. Merging with existing Exynos PCIe driver (and phy) might be indeed tricky, as existing one does not support that much as here. However I really expect that all current designs from Samsung - Exynos SoC, Artpec and for other customers - have very similar PCIe thus this should be a generic, new generation Samsung PCIe driver. If designed that way, also the naming should be back Samsung specific, no Axis/Artpec. Best regards, Krzysztof
On Thu, Jul 21, 2022 at 2:58 PM Bjorn Helgaas <helgaas@kernel.org> wrote: > > On Thu, Jul 21, 2022 at 11:04:00AM +0200, Krzysztof Kozlowski wrote: > > On 20/07/2022 08:01, Wangseok Lee wrote: > > > Add support Axis, ARTPEC-8 SoC. ARTPEC-8 is the SoC platform of Axis > > > Communications. This is based on arm64 and support GEN4 & 2lane. This > > > PCIe controller is based on DesignWare Hardware core and uses DesignWare > > > core functions to implement the driver. "pcie-artpec6. c" supports artpec6 > > > and artpec7 H/W. artpec8 can not be expanded because H/W configuration is > > > completely different from artpec6/7. PHY and sub controller are different. > > > > > > Signed-off-by: Wangseok Lee <wangseok.lee@samsung.com> > > > Signed-off-by: Jaeho Cho <jaeho79.cho@samsung.com> > > > --- > > > v3->v4 : > > > -Remove unnecessary enum type > > > -Fix indentation > > > > > > > Thanks for the changes. This starts to look good, however I am not going > > to ack it. This is also not a strong NAK, as I would respect Bjorn and > > other maintainers decision. > > > > I don't like the approach of creating only Artpec-8 specific driver. > > Samsung heavily reuses its block in all Exynos devices. Now it re-uses > > them for other designs as well. Therefore, even if merging with existing > > Exynos PCIe driver is not feasible (we had such discussions), I expect > > this to cover all Samsung Foundry PCIe devices. From all current designs > > up to future licensed blocks, including some new Samsung Exynos SoC. Or > > at least be ready for it. > > I would certainly prefer fewer drivers but I don't know enough about > the underlying IP and the places it's integrated to to know what's > practical. The only way I could figure that out would be by manually > comparing the drivers for similarity. I assume/expect all driver > authors are doing that. Sadly, I would not assume that. :( Just looking at the ELBI registers between Exynos and this one, the registers look completely different with nothing shared between the 2. Maybe sharing is possible with some new, yet to be upstreamed Samsung PCIe IP, but not the existing one. Same vendor is not always a good reason to share a driver. I think the way to further shrink the vendor DWC drivers is getting the DWC core and PCI core to do more in terms of clocks, resets, and phys management, handling of PERST, and link state management. We should also get rid of vendor drivers doing their own abstraction layers (ops structs) (looking at you QCom). Rob
. On Wed, Jul 20, 2022 at 12:01 AM Wangseok Lee <wangseok.lee@samsung.com> wrote: > > Add support Axis, ARTPEC-8 SoC. ARTPEC-8 is the SoC platform of Axis > Communications. This is based on arm64 and support GEN4 & 2lane. This > PCIe controller is based on DesignWare Hardware core and uses DesignWare > core functions to implement the driver. "pcie-artpec6. c" supports artpec6 > and artpec7 H/W. artpec8 can not be expanded because H/W configuration is > completely different from artpec6/7. PHY and sub controller are different. > > Signed-off-by: Wangseok Lee <wangseok.lee@samsung.com> > Signed-off-by: Jaeho Cho <jaeho79.cho@samsung.com> > --- > v3->v4 : > -Remove unnecessary enum type > -Fix indentation > > v2->v3 : > -Add 'COMPILE_TEST' and improvement help on kconfig > -Reorder obj on makefile > -Use clk_bulk_api > -Remove unnecessary comment > -Redefine the ELBI register to distinguish between offset and bit > definition > -Improvement order local variable of function > -Remove unnecessary local return variable > > v1->v2 : > Improvement review comment of Krzysztof on driver code. > -Debug messages for probe or other functions. > -Inconsistent coding style (different indentation in structure members) > -Inconsistent code (artpec8_pcie_get_subsystem_resources() gets device > from pdev and from pci so you have two same pointers; or artpec8_pcie_ > get_ep_mem_resources() stores dev as local variable but uses instead > pdev->dev) > -Not using devm_platform_ioremap_resource() > -Printing messages in interrupt handlers > -Several local/static structures or array are not const > --- > drivers/pci/controller/dwc/Kconfig | 31 ++ > drivers/pci/controller/dwc/Makefile | 1 + > drivers/pci/controller/dwc/pcie-artpec8.c | 788 ++++++++++++++++++++++++++++++ > 3 files changed, 820 insertions(+) > create mode 100644 drivers/pci/controller/dwc/pcie-artpec8.c > > diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig > index 62ce3ab..2b16637 100644 > --- a/drivers/pci/controller/dwc/Kconfig > +++ b/drivers/pci/controller/dwc/Kconfig > @@ -222,6 +222,37 @@ config PCIE_ARTPEC6_EP > Enables support for the PCIe controller in the ARTPEC-6 SoC to work in > endpoint mode. This uses the DesignWare core. > > +config PCIE_ARTPEC8 > + bool "Axis ARTPEC-8 PCIe controller" > + > +config PCIE_ARTPEC8_HOST > + bool "Axis ARTPEC-8 PCIe controller Host Mode" > + depends on ARCH_ARTPEC || COMPILE_TEST > + depends on PCI_MSI_IRQ_DOMAIN > + depends on PCI_ENDPOINT > + select PCI_EPF_TEST > + select PCIE_DW_HOST > + select PCIE_ARTPEC8 > + help > + Say 'Y' here to enable support for the PCIe controller in the > + ARTPEC-8 SoC to work in host mode. > + This PCIe controller is based on DesignWare hardware core and > + uses DesignWare core functions to implement the driver. > + > +config PCIE_ARTPEC8_EP > + bool "Axis ARTPEC-8 PCIe controller Endpoint Mode" > + depends on ARCH_ARTPEC || COMPILE_TEST > + depends on PCI_ENDPOINT > + depends on PCI_ENDPOINT_CONFIGFS > + select PCI_EPF_TEST > + select PCIE_DW_EP > + select PCIE_ARTPEC8 > + help > + Say 'Y' here to enable support for the PCIe controller in the > + ARTPEC-8 SoC to work in endpoint mode. > + This PCIe controller is based on DesignWare hardware core and > + uses DesignWare core functions to implement the driver. > + > config PCIE_ROCKCHIP_DW_HOST > bool "Rockchip DesignWare PCIe controller" > select PCIE_DW > diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile > index 8ba7b67..95f5877 100644 > --- a/drivers/pci/controller/dwc/Makefile > +++ b/drivers/pci/controller/dwc/Makefile > @@ -15,6 +15,7 @@ obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o > obj-$(CONFIG_PCIE_QCOM_EP) += pcie-qcom-ep.o > obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o > obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o > +obj-$(CONFIG_PCIE_ARTPEC8) += pcie-artpec8.o > obj-$(CONFIG_PCIE_ROCKCHIP_DW_HOST) += pcie-dw-rockchip.o > obj-$(CONFIG_PCIE_INTEL_GW) += pcie-intel-gw.o > obj-$(CONFIG_PCIE_KEEMBAY) += pcie-keembay.o > diff --git a/drivers/pci/controller/dwc/pcie-artpec8.c b/drivers/pci/controller/dwc/pcie-artpec8.c > new file mode 100644 > index 0000000..11eddf0 > --- /dev/null > +++ b/drivers/pci/controller/dwc/pcie-artpec8.c > @@ -0,0 +1,788 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * PCIe controller driver for Axis ARTPEC-8 SoC > + * > + * Copyright (C) 2019 Samsung Electronics Co., Ltd. > + * http://www.samsung.com > + * > + * Author: Jaeho Cho <jaeho79.cho@samsung.com> > + * This file is based on driver/pci/controller/dwc/pci-exynos.c > + */ > + > +#include <linux/clk.h> > +#include <linux/module.h> > +#include <linux/mfd/syscon.h> > +#include <linux/of_device.h> > +#include <linux/regmap.h> > +#include <linux/resource.h> > +#include <linux/types.h> > +#include <linux/phy/phy.h> > + > +#include "pcie-designware.h" > + > +#define to_artpec8_pcie(x) dev_get_drvdata((x)->dev) > + > +/* Gen3 Control Register */ > +#define PCIE_GEN3_RELATED_OFF 0x890 This is a DWC DBI register, so it belongs in the DWC code. IIRC, it already is. > +#define PCIE_GEN3_EQUALIZATION_DISABLE (0x1 << 16) > +#define PCIE_GEN3_EQ_PHASE_2_3 (0x1 << 9) > +#define PCIE_GEN3_RXEQ_PH01_EN (0x1 << 12) > +#define PCIE_GEN3_RXEQ_RGRDLESS_RXTS (0x1 << 13) > + > +#define FAST_LINK_MODE (7) > + > +/* PCIe ELBI registers */ > +#define PCIE_IRQ0_STS 0x000 > +#define PCIE_IRQ1_STS 0x004 > +#define PCIE_IRQ2_STS 0x008 > +#define IRQ2_STS_IRQ_MSI_ST BIT(20) > +#define PCIE_IRQ5_STS 0x00C > +#define PCIE_IRQ0_EN 0x010 > +#define PCIE_IRQ1_EN 0x014 > +#define PCIE_IRQ2_EN 0x018 > +#define IRQ2_EN_IRQ_MSI BIT(20) > +#define PCIE_IRQ5_EN 0x01C > +#define PCIE_APP_LTSSM_ENABLE 0x054 > +#define APP_LTSSM_ENABLE_EN_BIT BIT(0) > +#define PCIE_ELBI_CXPL_DEBUG_00_31 0x2C8 > +#define PCIE_ELBI_CXPL_DEBUG_32_63 0x2CC > +#define PCIE_ARTPEC8_DEVICE_TYPE 0x080 > +#define DEVICE_TYPE_EP 0x0 > +#define DEVICE_TYPE_LEG_EP BIT(0) > +#define DEVICE_TYPE_RC BIT(2) > +#define LTSSM_STATE_MASK 0x3F > +#define LTSSM_STATE_L0 0x11 > + > +/* FSYS glue logic system registers */ > +#define FSYS_PCIE_CON 0x424 > +#define PCIE_PERSTN BIT(5) > +#define FSYS_PCIE_DBI_ADDR_CON 0x428 > +#define FSYS_PCIE_DBI_ADDR_OVR_CDM 0x00 > +#define FSYS_PCIE_DBI_ADDR_OVR_SHADOW 0x12 > +#define FSYS_PCIE_DBI_ADDR_OVR_ATU 0x36 > + > +/* PMU SYSCON Offsets */ > +#define PMU_SYSCON_PCIE_ISOLATION 0x3200 > + > +/* BUS P/S SYSCON Offsets */ > +#define BUS_SYSCON_BUS_PATH_ENABLE 0x0 > + > +#define PCIE_CLEAR_ISOLATION 0 > +#define PCIE_SET_ISOLATION 1 > + > +#define PCIE_REG_BIT_LOW 0 > +#define PCIE_REG_BIT_HIGH 1 Kind of pointless defines. > + > +struct artpec8_pcie { > + struct dw_pcie *pci; Make this a struct so you have 1 memory alloc. > + const struct artpec8_pcie_pdata *pdata; > + void __iomem *elbi_base; > + struct regmap *sysreg; > + struct regmap *pmu_syscon; > + struct regmap *bus_s_syscon; > + struct regmap *bus_p_syscon; > + enum dw_pcie_device_mode mode; > + int link_id; > + struct phy *phy; > +}; > + > +struct artpec8_pcie_res_ops { > + int (*get_mem_resources)(struct platform_device *pdev, > + struct artpec8_pcie *artpec8_ctrl); > + int (*get_clk_resources)(struct platform_device *pdev); > + int (*init_clk_resources)(void); > + void (*deinit_clk_resources)(void); > +}; > + > +struct artpec8_pcie_pdata { > + const struct dw_pcie_ops *dwc_ops; > + const struct dw_pcie_host_ops *host_ops; > + const struct artpec8_pcie_res_ops *res_ops; > + enum dw_pcie_device_mode mode; > +}; > + > +static const int artpec8_pcie_dbi_addr_con[] = { > + FSYS_PCIE_DBI_ADDR_CON > +}; > + > +static struct clk_bulk_data artpec8_pcie_clks[] = { > + { .id = "pipe" }, > + { .id = "dbi" }, > + { .id = "mstr" }, > + { .id = "slv" }, > +}; > + > +static u32 artpec8_pcie_read_dbi(struct dw_pcie *pci, void __iomem *base, > + u32 reg, size_t size) > +{ > + struct artpec8_pcie *artpec8_ctrl = to_artpec8_pcie(pci); > + u32 val; > + bool is_atu = false; > + > + if (base == pci->atu_base) { > + is_atu = true; > + base = pci->dbi_base; > + regmap_write(artpec8_ctrl->sysreg, > + artpec8_pcie_dbi_addr_con[artpec8_ctrl->link_id], > + FSYS_PCIE_DBI_ADDR_OVR_ATU); > + } > + > + dw_pcie_read(base + reg, size, &val); > + > + if (is_atu) > + regmap_write(artpec8_ctrl->sysreg, > + artpec8_pcie_dbi_addr_con[artpec8_ctrl->link_id], > + FSYS_PCIE_DBI_ADDR_OVR_CDM); Well, this is "interesting"... I certainly hope this is not how new Samsung platforms do it. > + > + return val; > +} > + > +static void artpec8_pcie_write_dbi(struct dw_pcie *pci, void __iomem *base, > + u32 reg, size_t size, u32 val) > +{ > + struct artpec8_pcie *artpec8_ctrl = to_artpec8_pcie(pci); > + bool is_atu = false; > + > + if (base == pci->atu_base) { > + is_atu = true; > + base = pci->dbi_base; > + regmap_write(artpec8_ctrl->sysreg, > + artpec8_pcie_dbi_addr_con[artpec8_ctrl->link_id], > + FSYS_PCIE_DBI_ADDR_OVR_ATU); > + } > + > + dw_pcie_write(base + reg, size, val); > + > + if (is_atu) > + regmap_write(artpec8_ctrl->sysreg, > + artpec8_pcie_dbi_addr_con[artpec8_ctrl->link_id], > + FSYS_PCIE_DBI_ADDR_OVR_CDM); > +} > + > +static void artpec8_pcie_write_dbi2(struct dw_pcie *pci, void __iomem *base, > + u32 reg, size_t size, u32 val) > +{ > + struct artpec8_pcie *artpec8_ctrl = to_artpec8_pcie(pci); > + > + regmap_write(artpec8_ctrl->sysreg, > + artpec8_pcie_dbi_addr_con[artpec8_ctrl->link_id], > + FSYS_PCIE_DBI_ADDR_OVR_SHADOW); > + > + dw_pcie_write(base + reg, size, val); > + > + regmap_write(artpec8_ctrl->sysreg, > + artpec8_pcie_dbi_addr_con[artpec8_ctrl->link_id], > + FSYS_PCIE_DBI_ADDR_OVR_CDM); > +} > + > +static int artpec8_pcie_get_subsys_resources(struct platform_device *pdev, > + struct artpec8_pcie *artpec8_ctrl) > +{ > + struct device *dev = &pdev->dev; > + > + /* External Local Bus interface(ELBI) Register */ > + artpec8_ctrl->elbi_base = > + devm_platform_ioremap_resource_byname(pdev, "elbi"); > + if (IS_ERR(artpec8_ctrl->elbi_base)) { > + dev_err(dev, "failed to map elbi_base\n"); > + return PTR_ERR(artpec8_ctrl->elbi_base); > + } > + > + artpec8_ctrl->sysreg = > + syscon_regmap_lookup_by_phandle(dev->of_node, > + "samsung,fsys-sysreg"); > + if (IS_ERR(artpec8_ctrl->sysreg)) { > + dev_err(dev, "fsys sysreg regmap lookup failed.\n"); > + return PTR_ERR(artpec8_ctrl->sysreg); > + } > + > + artpec8_ctrl->pmu_syscon = > + syscon_regmap_lookup_by_phandle(dev->of_node, > + "samsung,syscon-phandle"); > + if (IS_ERR(artpec8_ctrl->pmu_syscon)) { > + dev_err(dev, "pmu syscon regmap lookup failed.\n"); > + return PTR_ERR(artpec8_ctrl->pmu_syscon); > + } > + > + artpec8_ctrl->bus_s_syscon = > + syscon_regmap_lookup_by_phandle(dev->of_node, > + "samsung,syscon-bus-s-fsys"); > + if (IS_ERR(artpec8_ctrl->bus_s_syscon)) { > + dev_err(dev, "bus_s_syscon regmap lookup failed.\n"); > + return PTR_ERR(artpec8_ctrl->bus_s_syscon); > + } > + > + artpec8_ctrl->bus_p_syscon = > + syscon_regmap_lookup_by_phandle(dev->of_node, > + "samsung,syscon-bus-p-fsys"); > + if (IS_ERR(artpec8_ctrl->bus_p_syscon)) { > + dev_err(dev, "bus_p_syscon regmap lookup failed.\n"); > + return PTR_ERR(artpec8_ctrl->bus_p_syscon); > + } > + > + return 0; > +} > + > +static int artpec8_pcie_get_rc_mem_resources(struct platform_device *pdev, > + struct artpec8_pcie *artpec8_ctrl) > +{ > + struct dw_pcie *pci = artpec8_ctrl->pci; > + > + /* Data Bus Interface(DBI) Register */ > + pci->dbi_base = devm_platform_ioremap_resource_byname(pdev, "dbi"); > + if (IS_ERR(pci->dbi_base)) > + return PTR_ERR(pci->dbi_base); > + > + return 0; > +} > + > +static int artpec8_pcie_get_ep_mem_resources(struct platform_device *pdev, > + struct artpec8_pcie *artpec8_ctrl) > +{ > + struct dw_pcie *pci = artpec8_ctrl->pci; > + struct device *dev = &pdev->dev; > + struct dw_pcie_ep *ep = &pci->ep; > + struct resource *res; > + > + pci->dbi_base = devm_platform_ioremap_resource_byname(pdev, "dbi"); > + if (IS_ERR(pci->dbi_base)) { > + dev_err(dev, "failed to map ep_dbics\n"); > + return -ENOMEM; > + } > + > + pci->dbi_base2 = devm_platform_ioremap_resource_byname(pdev, "dbi2"); > + if (IS_ERR(pci->dbi_base2)) { > + dev_err(dev, "failed to map ep_dbics2\n"); > + return -ENOMEM; > + } > + > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "addr_space"); The DWC core will get all these > + if (!res) > + return -EINVAL; > + ep->phys_base = res->start; > + ep->addr_size = resource_size(res); > + > + return 0; > +} > + > +static int artpec8_pcie_get_clk_resources(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + int ret; > + > + ret = devm_clk_bulk_get(dev, ARRAY_SIZE(artpec8_pcie_clks), > + artpec8_pcie_clks); > + if (ret) > + return ret; > + > + return 0; > +} > + > +static int artpec8_pcie_init_clk_resources(void) > +{ > + int ret; > + > + ret = clk_bulk_prepare_enable(ARRAY_SIZE(artpec8_pcie_clks), > + artpec8_pcie_clks); > + if (ret) > + return ret; > + > + return 0; > +} > + > +static void artpec8_pcie_deinit_clk_resources(void) > +{ > + clk_bulk_disable_unprepare(ARRAY_SIZE(artpec8_pcie_clks), > + artpec8_pcie_clks); > +} > + > +static const struct artpec8_pcie_res_ops artpec8_pcie_rc_res_ops = { > + .get_mem_resources = artpec8_pcie_get_rc_mem_resources, > + .get_clk_resources = artpec8_pcie_get_clk_resources, > + .init_clk_resources = artpec8_pcie_init_clk_resources, > + .deinit_clk_resources = artpec8_pcie_deinit_clk_resources, > +}; > + > +static const struct artpec8_pcie_res_ops artpec8_pcie_ep_res_ops = { > + .get_mem_resources = artpec8_pcie_get_ep_mem_resources, > + .get_clk_resources = artpec8_pcie_get_clk_resources, > + .init_clk_resources = artpec8_pcie_init_clk_resources, > + .deinit_clk_resources = artpec8_pcie_deinit_clk_resources, Get rid of artpec8_pcie_res_ops. There's only one difference in functions. > +}; > + > +static int artpec8_pcie_config_phy_power_isolation(struct dw_pcie *pci, u8 val) Doesn't this belong in the PHY driver? Sounds like it should be a power-domain rather than directly touching some registers. > +{ > + struct artpec8_pcie *artpec8_ctrl = to_artpec8_pcie(pci); > + > + return regmap_write(artpec8_ctrl->pmu_syscon, PMU_SYSCON_PCIE_ISOLATION, > + val); > +} > + > +static int artpec8_pcie_config_bus_enable(struct dw_pcie *pci, u8 val) > +{ > + struct artpec8_pcie *artpec8_ctrl = to_artpec8_pcie(pci); > + int ret; > + > + ret = regmap_write(artpec8_ctrl->bus_p_syscon, > + BUS_SYSCON_BUS_PATH_ENABLE, val); > + if (ret) > + return ret; > + > + return regmap_write(artpec8_ctrl->bus_s_syscon, > + BUS_SYSCON_BUS_PATH_ENABLE, val); > +} > + > +static int artpec8_pcie_config_isolation(struct dw_pcie *pci, u8 val) Again, sounds like a power domain. > +{ > + int ret; > + /* reg_val[0] : for phy power isolation */ > + /* reg_val[1] : for bus enable */ > + u8 reg_val[2]; > + > + switch (val) { > + case PCIE_CLEAR_ISOLATION: > + reg_val[0] = PCIE_REG_BIT_LOW; > + reg_val[1] = PCIE_REG_BIT_HIGH; > + break; > + case PCIE_SET_ISOLATION: > + reg_val[0] = PCIE_REG_BIT_HIGH; > + reg_val[1] = PCIE_REG_BIT_LOW; > + break; > + default: > + return -EINVAL; > + } > + > + ret = artpec8_pcie_config_phy_power_isolation(pci, reg_val[0]); > + if (ret) > + return ret; > + > + return artpec8_pcie_config_bus_enable(pci, reg_val[1]); > +} > + > +static int artpec8_pcie_config_perstn(struct dw_pcie *pci, u8 val) > +{ > + struct artpec8_pcie *artpec8_ctrl = to_artpec8_pcie(pci); > + unsigned int bits; > + > + if (val == PCIE_REG_BIT_HIGH) > + bits = PCIE_PERSTN; > + else > + bits = 0; > + > + return regmap_update_bits(artpec8_ctrl->sysreg, FSYS_PCIE_CON, > + PCIE_PERSTN, bits); > +} > + > +static void artpec8_pcie_stop_link(struct dw_pcie *pci) > +{ > + struct artpec8_pcie *artpec8_ctrl = to_artpec8_pcie(pci); > + u32 val; > + > + val = readl(artpec8_ctrl->elbi_base + PCIE_APP_LTSSM_ENABLE); > + > + val &= ~APP_LTSSM_ENABLE_EN_BIT; > + writel(val, artpec8_ctrl->elbi_base + PCIE_APP_LTSSM_ENABLE); > +} > + > +static int artpec8_pcie_start_link(struct dw_pcie *pci) > +{ > + struct artpec8_pcie *artpec8_ctrl = to_artpec8_pcie(pci); > + u32 val; > + > + dw_pcie_dbi_ro_wr_en(pci); > + > + /* Equalization disable */ > + val = artpec8_pcie_read_dbi(pci, pci->dbi_base, PCIE_GEN3_RELATED_OFF, > + 4); > + artpec8_pcie_write_dbi(pci, pci->dbi_base, PCIE_GEN3_RELATED_OFF, 4, > + val | PCIE_GEN3_EQUALIZATION_DISABLE); Can this be in host_init instead? You shouldn't need dw_pcie_dbi_ro_wr_en() in that case. > + > + dw_pcie_dbi_ro_wr_dis(pci); > + > + /* assert LTSSM enable */ > + val = readl(artpec8_ctrl->elbi_base + PCIE_APP_LTSSM_ENABLE); > + > + val |= APP_LTSSM_ENABLE_EN_BIT; > + writel(val, artpec8_ctrl->elbi_base + PCIE_APP_LTSSM_ENABLE); > + > + return 0; > +} > + > +static irqreturn_t artpec8_pcie_msi_irq_handler(int irq, void *arg) > +{ > + struct artpec8_pcie *artpec8_ctrl = arg; > + struct dw_pcie *pci = artpec8_ctrl->pci; > + struct pcie_port *pp = &pci->pp; > + u32 val; > + > + val = readl(artpec8_ctrl->elbi_base + PCIE_IRQ2_STS); > + > + if ((val & IRQ2_STS_IRQ_MSI_ST) == IRQ2_STS_IRQ_MSI_ST) { > + val &= IRQ2_STS_IRQ_MSI_ST; > + writel(val, artpec8_ctrl->elbi_base + PCIE_IRQ2_STS); > + dw_handle_msi_irq(pp); > + } > + > + return IRQ_HANDLED; > +} > + > +static void artpec8_pcie_msi_init(struct artpec8_pcie *artpec8_ctrl) > +{ > + u32 val; > + > + /* enable MSI interrupt */ > + val = readl(artpec8_ctrl->elbi_base + PCIE_IRQ2_EN); > + val |= IRQ2_EN_IRQ_MSI; > + writel(val, artpec8_ctrl->elbi_base + PCIE_IRQ2_EN); > +} > + > +static void artpec8_pcie_enable_interrupts(struct artpec8_pcie *artpec8_ctrl) > +{ > + if (IS_ENABLED(CONFIG_PCI_MSI)) > + artpec8_pcie_msi_init(artpec8_ctrl); > +} > + > +static int artpec8_pcie_rd_own_conf(struct pci_bus *bus, unsigned int devfn, > + int where, int size, u32 *val) > +{ > + struct dw_pcie *pci = to_dw_pcie_from_pp(bus->sysdata); > + > + if (PCI_SLOT(devfn)) { > + PCI_SET_ERROR_RESPONSE(val); > + return PCIBIOS_DEVICE_NOT_FOUND; > + } Why do you need custom read/write functions? The DWC core handles this check. > + > + *val = dw_pcie_read_dbi(pci, where, size); > + return PCIBIOS_SUCCESSFUL; > +} > + > +static int artpec8_pcie_wr_own_conf(struct pci_bus *bus, unsigned int devfn, > + int where, int size, u32 val) > +{ > + struct dw_pcie *pci = to_dw_pcie_from_pp(bus->sysdata); > + > + if (PCI_SLOT(devfn)) > + return PCIBIOS_DEVICE_NOT_FOUND; > + > + dw_pcie_write_dbi(pci, where, size, val); > + return PCIBIOS_SUCCESSFUL; > +} > + > +static struct pci_ops artpec8_pci_ops = { > + .read = artpec8_pcie_rd_own_conf, > + .write = artpec8_pcie_wr_own_conf, > +}; > + > +static int artpec8_pcie_link_up(struct dw_pcie *pci) > +{ > + struct artpec8_pcie *artpec8_ctrl = to_artpec8_pcie(pci); > + u32 val; > + > + val = readl(artpec8_ctrl->elbi_base + PCIE_ELBI_CXPL_DEBUG_00_31); > + > + return (val & LTSSM_STATE_MASK) == LTSSM_STATE_L0; > +} > + > +static int artpec8_pcie_host_init(struct pcie_port *pp) > +{ > + struct dw_pcie *pci = to_dw_pcie_from_pp(pp); > + struct artpec8_pcie *artpec8_ctrl = to_artpec8_pcie(pci); > + > + pp->bridge->ops = &artpec8_pci_ops; > + > + dw_pcie_writel_dbi(pci, PCIE_GEN3_RELATED_OFF, (PCIE_GEN3_EQ_PHASE_2_3 | > + PCIE_GEN3_RXEQ_PH01_EN | > + PCIE_GEN3_RXEQ_RGRDLESS_RXTS)); > + > + artpec8_pcie_enable_interrupts(artpec8_ctrl); > + > + return 0; > +} > + > +static const struct dw_pcie_host_ops artpec8_pcie_host_ops = { > + .host_init = artpec8_pcie_host_init, > +}; > + > +static u8 artpec8_pcie_iatu_unroll_enabled(struct dw_pcie *pci) > +{ > + u32 val; > + > + val = dw_pcie_readl_dbi(pci, PCIE_ATU_VIEWPORT); > + pci->atu_base = pci->dbi_base + DEFAULT_DBI_ATU_OFFSET; > + > + if (val == 0xffffffff) > + return 1; > + > + return 0; > +} > + > +static void artpec8_pcie_ep_init(struct dw_pcie_ep *ep) > +{ > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep); > + enum pci_barno bar; > + /* > + * Currently PCIe EP core is not setting iatu_unroll_enabled > + * so let's handle it here. We need to find proper place to > + * initialize this so that it can be used for other EP > + * controllers as well. I think there's patches to address this. In any case, don't work-around issues with other code here. Fix the DWC core to do what you need. > + */ > + pci->iatu_unroll_enabled = artpec8_pcie_iatu_unroll_enabled(pci); Why do you need to detect this? You don't know which type of iatu access your h/w supports? > + > + for (bar = BAR_0; bar <= BAR_5; bar++) > + dw_pcie_ep_reset_bar(pci, bar); Also seen a patch for this. > +} > + > +static int artpec8_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no, > + enum pci_epc_irq_type type, u16 interrupt_num) > +{ > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep); > + > + switch (type) { > + case PCI_EPC_IRQ_LEGACY: > + return -EINVAL; > + case PCI_EPC_IRQ_MSI: > + return dw_pcie_ep_raise_msi_irq(ep, func_no, interrupt_num); > + default: > + dev_err(pci->dev, "UNKNOWN IRQ type\n"); > + } > + > + return 0; > +} > + > +static const struct pci_epc_features artpec8_pcie_epc_features = { > + .linkup_notifier = false, > + .msi_capable = true, > + .msix_capable = false, > +}; > + > +static const struct pci_epc_features* > +artpec8_pcie_ep_get_features(struct dw_pcie_ep *ep) > +{ > + return &artpec8_pcie_epc_features; > +} > + > +static const struct dw_pcie_ep_ops artpec8_dw_pcie_ep_ops = { > + .ep_init = artpec8_pcie_ep_init, > + .raise_irq = artpec8_pcie_raise_irq, > + .get_features = artpec8_pcie_ep_get_features, > +}; > + > +static int __init artpec8_add_pcie_ep(struct artpec8_pcie *artpec8_ctrl, > + struct platform_device *pdev) > +{ > + struct dw_pcie *pci = artpec8_ctrl->pci; > + struct dw_pcie_ep *ep = &pci->ep; > + int ret; > + > + ep->ops = &artpec8_dw_pcie_ep_ops; > + > + dw_pcie_writel_dbi(pci, PCIE_GEN3_RELATED_OFF, (PCIE_GEN3_EQ_PHASE_2_3 | > + PCIE_GEN3_RXEQ_PH01_EN | > + PCIE_GEN3_RXEQ_RGRDLESS_RXTS)); > + > + ret = dw_pcie_ep_init(ep); > + if (ret) > + return ret; > + > + return 0; > +} > + > +static int __init artpec8_add_pcie_port(struct artpec8_pcie *artpec8_ctrl, > + struct platform_device *pdev) > +{ > + struct dw_pcie *pci = artpec8_ctrl->pci; > + struct pcie_port *pp = &pci->pp; > + struct device *dev = &pdev->dev; > + int irq; > + int irq_flags; > + int ret; > + > + if (IS_ENABLED(CONFIG_PCI_MSI)) { > + irq = platform_get_irq_byname(pdev, "intr"); > + if (!irq) > + return -ENODEV; > + > + irq_flags = IRQF_SHARED | IRQF_NO_THREAD; > + > + ret = devm_request_irq(dev, irq, artpec8_pcie_msi_irq_handler, > + irq_flags, "artpec8-pcie", artpec8_ctrl); > + if (ret) > + return ret; > + } > + > + /* Prevent core from messing with the IRQ, since it's muxed */ > + pp->msi_irq = -ENODEV; > + > + return dw_pcie_host_init(pp); > +} > + > +static const struct dw_pcie_ops artpec8_dw_pcie_ops = { > + .read_dbi = artpec8_pcie_read_dbi, > + .write_dbi = artpec8_pcie_write_dbi, > + .write_dbi2 = artpec8_pcie_write_dbi2, > + .start_link = artpec8_pcie_start_link, > + .stop_link = artpec8_pcie_stop_link, > + .link_up = artpec8_pcie_link_up, > +}; > + > +static int artpec8_pcie_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct artpec8_pcie *artpec8_ctrl; > + struct dw_pcie *pci; > + const struct artpec8_pcie_pdata *pdata; > + enum dw_pcie_device_mode mode; > + struct pcie_port *pp; > + struct device_node *np = dev->of_node; > + int ret; > + > + artpec8_ctrl = devm_kzalloc(dev, sizeof(*artpec8_ctrl), GFP_KERNEL); > + if (!artpec8_ctrl) > + return -ENOMEM; > + > + pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL); > + if (!pci) > + return -ENOMEM; > + > + pdata = of_device_get_match_data(dev); > + > + if (!pdata) > + return -ENODEV; > + > + mode = (enum dw_pcie_device_mode)pdata->mode; > + > + artpec8_ctrl->pci = pci; > + artpec8_ctrl->pdata = pdata; > + artpec8_ctrl->mode = mode; > + > + pci->dev = dev; > + pci->ops = pdata->dwc_ops; > + pci->dbi_base2 = NULL; > + pci->dbi_base = NULL; > + pp = &pci->pp; > + pp->ops = artpec8_ctrl->pdata->host_ops; > + > + if (mode == DW_PCIE_RC_TYPE) > + artpec8_ctrl->link_id = of_alias_get_id(np, "pcierc"); Nope. We don't use aliases for PCI devices. Based on how you use it, this should be an arg cell for the sysreg phandle. > + else > + artpec8_ctrl->link_id = of_alias_get_id(np, "pcieep"); > + > + ret = artpec8_pcie_get_subsys_resources(pdev, artpec8_ctrl); > + if (ret) > + return ret; > + > + if (pdata->res_ops && pdata->res_ops->get_mem_resources) { > + ret = pdata->res_ops->get_mem_resources(pdev, artpec8_ctrl); > + if (ret) > + return ret; > + } > + > + if (pdata->res_ops && pdata->res_ops->get_clk_resources) { > + ret = pdata->res_ops->get_clk_resources(pdev); > + if (ret) > + return ret; > + > + ret = pdata->res_ops->init_clk_resources(); > + if (ret) > + return ret; > + } > + > + platform_set_drvdata(pdev, artpec8_ctrl); > + > + ret = artpec8_pcie_config_isolation(pci, PCIE_CLEAR_ISOLATION); > + if (ret) > + return ret; > + > + ret = artpec8_pcie_config_perstn(pci, PCIE_REG_BIT_HIGH); > + if (ret) > + return ret; > + > + artpec8_ctrl->phy = devm_of_phy_get(dev, np, NULL); > + if (IS_ERR(artpec8_ctrl->phy)) > + return PTR_ERR(artpec8_ctrl->phy); > + > + phy_init(artpec8_ctrl->phy); > + phy_reset(artpec8_ctrl->phy); > + > + switch (mode) { > + case DW_PCIE_RC_TYPE: > + writel(DEVICE_TYPE_RC, artpec8_ctrl->elbi_base + > + PCIE_ARTPEC8_DEVICE_TYPE); > + ret = artpec8_add_pcie_port(artpec8_ctrl, pdev); > + if (ret < 0) > + goto fail_probe; > + break; > + case DW_PCIE_EP_TYPE: > + writel(DEVICE_TYPE_EP, artpec8_ctrl->elbi_base + > + PCIE_ARTPEC8_DEVICE_TYPE); > + > + ret = artpec8_add_pcie_ep(artpec8_ctrl, pdev); > + if (ret < 0) > + goto fail_probe; > + break; > + default: > + ret = -EINVAL; > + goto fail_probe; > + } > + > + return 0; > + > +fail_probe: > + phy_exit(artpec8_ctrl->phy); > + if (pdata->res_ops && pdata->res_ops->deinit_clk_resources) > + pdata->res_ops->deinit_clk_resources(); > + > + return ret; > +} > + > +static int __exit artpec8_pcie_remove(struct platform_device *pdev) > +{ > + struct artpec8_pcie *artpec8_ctrl = platform_get_drvdata(pdev); > + const struct artpec8_pcie_pdata *pdata = artpec8_ctrl->pdata; > + > + if (pdata->res_ops && pdata->res_ops->deinit_clk_resources) > + pdata->res_ops->deinit_clk_resources(); > + > + return 0; > +} > + > +static const struct artpec8_pcie_pdata artpec8_pcie_rc_pdata = { > + .dwc_ops = &artpec8_dw_pcie_ops, > + .host_ops = &artpec8_pcie_host_ops, > + .res_ops = &artpec8_pcie_rc_res_ops, > + .mode = DW_PCIE_RC_TYPE, > +}; > + > +static const struct artpec8_pcie_pdata artpec8_pcie_ep_pdata = { > + .dwc_ops = &artpec8_dw_pcie_ops, > + .host_ops = &artpec8_pcie_host_ops, > + .res_ops = &artpec8_pcie_ep_res_ops, > + .mode = DW_PCIE_EP_TYPE, > +}; > + > +static const struct of_device_id artpec8_pcie_of_match[] = { > + { > + .compatible = "axis,artpec8-pcie", > + .data = &artpec8_pcie_rc_pdata, > + }, > + { > + .compatible = "axis,artpec8-pcie-ep", > + .data = &artpec8_pcie_ep_pdata, > + }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, artpec8_pcie_of_match); > + > +static struct platform_driver artpec8_pcie_driver = { > + .probe = artpec8_pcie_probe, > + .remove = __exit_p(artpec8_pcie_remove), > + .driver = { > + .name = "artpec8-pcie", > + .of_match_table = artpec8_pcie_of_match, > + }, > +}; > + > +module_platform_driver(artpec8_pcie_driver); > + > +MODULE_LICENSE("GPL"); > +MODULE_AUTHOR("Jaeho Cho <jaeho79.cho@samsung.com>"); > -- > 2.9.5
diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig index 62ce3ab..2b16637 100644 --- a/drivers/pci/controller/dwc/Kconfig +++ b/drivers/pci/controller/dwc/Kconfig @@ -222,6 +222,37 @@ config PCIE_ARTPEC6_EP Enables support for the PCIe controller in the ARTPEC-6 SoC to work in endpoint mode. This uses the DesignWare core. +config PCIE_ARTPEC8 + bool "Axis ARTPEC-8 PCIe controller" + +config PCIE_ARTPEC8_HOST + bool "Axis ARTPEC-8 PCIe controller Host Mode" + depends on ARCH_ARTPEC || COMPILE_TEST + depends on PCI_MSI_IRQ_DOMAIN + depends on PCI_ENDPOINT + select PCI_EPF_TEST + select PCIE_DW_HOST + select PCIE_ARTPEC8 + help + Say 'Y' here to enable support for the PCIe controller in the + ARTPEC-8 SoC to work in host mode. + This PCIe controller is based on DesignWare hardware core and + uses DesignWare core functions to implement the driver. + +config PCIE_ARTPEC8_EP + bool "Axis ARTPEC-8 PCIe controller Endpoint Mode" + depends on ARCH_ARTPEC || COMPILE_TEST + depends on PCI_ENDPOINT + depends on PCI_ENDPOINT_CONFIGFS + select PCI_EPF_TEST + select PCIE_DW_EP + select PCIE_ARTPEC8 + help + Say 'Y' here to enable support for the PCIe controller in the + ARTPEC-8 SoC to work in endpoint mode. + This PCIe controller is based on DesignWare hardware core and + uses DesignWare core functions to implement the driver. + config PCIE_ROCKCHIP_DW_HOST bool "Rockchip DesignWare PCIe controller" select PCIE_DW diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile index 8ba7b67..95f5877 100644 --- a/drivers/pci/controller/dwc/Makefile +++ b/drivers/pci/controller/dwc/Makefile @@ -15,6 +15,7 @@ obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o obj-$(CONFIG_PCIE_QCOM_EP) += pcie-qcom-ep.o obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o +obj-$(CONFIG_PCIE_ARTPEC8) += pcie-artpec8.o obj-$(CONFIG_PCIE_ROCKCHIP_DW_HOST) += pcie-dw-rockchip.o obj-$(CONFIG_PCIE_INTEL_GW) += pcie-intel-gw.o obj-$(CONFIG_PCIE_KEEMBAY) += pcie-keembay.o diff --git a/drivers/pci/controller/dwc/pcie-artpec8.c b/drivers/pci/controller/dwc/pcie-artpec8.c new file mode 100644 index 0000000..11eddf0 --- /dev/null +++ b/drivers/pci/controller/dwc/pcie-artpec8.c @@ -0,0 +1,788 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * PCIe controller driver for Axis ARTPEC-8 SoC + * + * Copyright (C) 2019 Samsung Electronics Co., Ltd. + * http://www.samsung.com + * + * Author: Jaeho Cho <jaeho79.cho@samsung.com> + * This file is based on driver/pci/controller/dwc/pci-exynos.c + */ + +#include <linux/clk.h> +#include <linux/module.h> +#include <linux/mfd/syscon.h> +#include <linux/of_device.h> +#include <linux/regmap.h> +#include <linux/resource.h> +#include <linux/types.h> +#include <linux/phy/phy.h> + +#include "pcie-designware.h" + +#define to_artpec8_pcie(x) dev_get_drvdata((x)->dev) + +/* Gen3 Control Register */ +#define PCIE_GEN3_RELATED_OFF 0x890 +#define PCIE_GEN3_EQUALIZATION_DISABLE (0x1 << 16) +#define PCIE_GEN3_EQ_PHASE_2_3 (0x1 << 9) +#define PCIE_GEN3_RXEQ_PH01_EN (0x1 << 12) +#define PCIE_GEN3_RXEQ_RGRDLESS_RXTS (0x1 << 13) + +#define FAST_LINK_MODE (7) + +/* PCIe ELBI registers */ +#define PCIE_IRQ0_STS 0x000 +#define PCIE_IRQ1_STS 0x004 +#define PCIE_IRQ2_STS 0x008 +#define IRQ2_STS_IRQ_MSI_ST BIT(20) +#define PCIE_IRQ5_STS 0x00C +#define PCIE_IRQ0_EN 0x010 +#define PCIE_IRQ1_EN 0x014 +#define PCIE_IRQ2_EN 0x018 +#define IRQ2_EN_IRQ_MSI BIT(20) +#define PCIE_IRQ5_EN 0x01C +#define PCIE_APP_LTSSM_ENABLE 0x054 +#define APP_LTSSM_ENABLE_EN_BIT BIT(0) +#define PCIE_ELBI_CXPL_DEBUG_00_31 0x2C8 +#define PCIE_ELBI_CXPL_DEBUG_32_63 0x2CC +#define PCIE_ARTPEC8_DEVICE_TYPE 0x080 +#define DEVICE_TYPE_EP 0x0 +#define DEVICE_TYPE_LEG_EP BIT(0) +#define DEVICE_TYPE_RC BIT(2) +#define LTSSM_STATE_MASK 0x3F +#define LTSSM_STATE_L0 0x11 + +/* FSYS glue logic system registers */ +#define FSYS_PCIE_CON 0x424 +#define PCIE_PERSTN BIT(5) +#define FSYS_PCIE_DBI_ADDR_CON 0x428 +#define FSYS_PCIE_DBI_ADDR_OVR_CDM 0x00 +#define FSYS_PCIE_DBI_ADDR_OVR_SHADOW 0x12 +#define FSYS_PCIE_DBI_ADDR_OVR_ATU 0x36 + +/* PMU SYSCON Offsets */ +#define PMU_SYSCON_PCIE_ISOLATION 0x3200 + +/* BUS P/S SYSCON Offsets */ +#define BUS_SYSCON_BUS_PATH_ENABLE 0x0 + +#define PCIE_CLEAR_ISOLATION 0 +#define PCIE_SET_ISOLATION 1 + +#define PCIE_REG_BIT_LOW 0 +#define PCIE_REG_BIT_HIGH 1 + +struct artpec8_pcie { + struct dw_pcie *pci; + const struct artpec8_pcie_pdata *pdata; + void __iomem *elbi_base; + struct regmap *sysreg; + struct regmap *pmu_syscon; + struct regmap *bus_s_syscon; + struct regmap *bus_p_syscon; + enum dw_pcie_device_mode mode; + int link_id; + struct phy *phy; +}; + +struct artpec8_pcie_res_ops { + int (*get_mem_resources)(struct platform_device *pdev, + struct artpec8_pcie *artpec8_ctrl); + int (*get_clk_resources)(struct platform_device *pdev); + int (*init_clk_resources)(void); + void (*deinit_clk_resources)(void); +}; + +struct artpec8_pcie_pdata { + const struct dw_pcie_ops *dwc_ops; + const struct dw_pcie_host_ops *host_ops; + const struct artpec8_pcie_res_ops *res_ops; + enum dw_pcie_device_mode mode; +}; + +static const int artpec8_pcie_dbi_addr_con[] = { + FSYS_PCIE_DBI_ADDR_CON +}; + +static struct clk_bulk_data artpec8_pcie_clks[] = { + { .id = "pipe" }, + { .id = "dbi" }, + { .id = "mstr" }, + { .id = "slv" }, +}; + +static u32 artpec8_pcie_read_dbi(struct dw_pcie *pci, void __iomem *base, + u32 reg, size_t size) +{ + struct artpec8_pcie *artpec8_ctrl = to_artpec8_pcie(pci); + u32 val; + bool is_atu = false; + + if (base == pci->atu_base) { + is_atu = true; + base = pci->dbi_base; + regmap_write(artpec8_ctrl->sysreg, + artpec8_pcie_dbi_addr_con[artpec8_ctrl->link_id], + FSYS_PCIE_DBI_ADDR_OVR_ATU); + } + + dw_pcie_read(base + reg, size, &val); + + if (is_atu) + regmap_write(artpec8_ctrl->sysreg, + artpec8_pcie_dbi_addr_con[artpec8_ctrl->link_id], + FSYS_PCIE_DBI_ADDR_OVR_CDM); + + return val; +} + +static void artpec8_pcie_write_dbi(struct dw_pcie *pci, void __iomem *base, + u32 reg, size_t size, u32 val) +{ + struct artpec8_pcie *artpec8_ctrl = to_artpec8_pcie(pci); + bool is_atu = false; + + if (base == pci->atu_base) { + is_atu = true; + base = pci->dbi_base; + regmap_write(artpec8_ctrl->sysreg, + artpec8_pcie_dbi_addr_con[artpec8_ctrl->link_id], + FSYS_PCIE_DBI_ADDR_OVR_ATU); + } + + dw_pcie_write(base + reg, size, val); + + if (is_atu) + regmap_write(artpec8_ctrl->sysreg, + artpec8_pcie_dbi_addr_con[artpec8_ctrl->link_id], + FSYS_PCIE_DBI_ADDR_OVR_CDM); +} + +static void artpec8_pcie_write_dbi2(struct dw_pcie *pci, void __iomem *base, + u32 reg, size_t size, u32 val) +{ + struct artpec8_pcie *artpec8_ctrl = to_artpec8_pcie(pci); + + regmap_write(artpec8_ctrl->sysreg, + artpec8_pcie_dbi_addr_con[artpec8_ctrl->link_id], + FSYS_PCIE_DBI_ADDR_OVR_SHADOW); + + dw_pcie_write(base + reg, size, val); + + regmap_write(artpec8_ctrl->sysreg, + artpec8_pcie_dbi_addr_con[artpec8_ctrl->link_id], + FSYS_PCIE_DBI_ADDR_OVR_CDM); +} + +static int artpec8_pcie_get_subsys_resources(struct platform_device *pdev, + struct artpec8_pcie *artpec8_ctrl) +{ + struct device *dev = &pdev->dev; + + /* External Local Bus interface(ELBI) Register */ + artpec8_ctrl->elbi_base = + devm_platform_ioremap_resource_byname(pdev, "elbi"); + if (IS_ERR(artpec8_ctrl->elbi_base)) { + dev_err(dev, "failed to map elbi_base\n"); + return PTR_ERR(artpec8_ctrl->elbi_base); + } + + artpec8_ctrl->sysreg = + syscon_regmap_lookup_by_phandle(dev->of_node, + "samsung,fsys-sysreg"); + if (IS_ERR(artpec8_ctrl->sysreg)) { + dev_err(dev, "fsys sysreg regmap lookup failed.\n"); + return PTR_ERR(artpec8_ctrl->sysreg); + } + + artpec8_ctrl->pmu_syscon = + syscon_regmap_lookup_by_phandle(dev->of_node, + "samsung,syscon-phandle"); + if (IS_ERR(artpec8_ctrl->pmu_syscon)) { + dev_err(dev, "pmu syscon regmap lookup failed.\n"); + return PTR_ERR(artpec8_ctrl->pmu_syscon); + } + + artpec8_ctrl->bus_s_syscon = + syscon_regmap_lookup_by_phandle(dev->of_node, + "samsung,syscon-bus-s-fsys"); + if (IS_ERR(artpec8_ctrl->bus_s_syscon)) { + dev_err(dev, "bus_s_syscon regmap lookup failed.\n"); + return PTR_ERR(artpec8_ctrl->bus_s_syscon); + } + + artpec8_ctrl->bus_p_syscon = + syscon_regmap_lookup_by_phandle(dev->of_node, + "samsung,syscon-bus-p-fsys"); + if (IS_ERR(artpec8_ctrl->bus_p_syscon)) { + dev_err(dev, "bus_p_syscon regmap lookup failed.\n"); + return PTR_ERR(artpec8_ctrl->bus_p_syscon); + } + + return 0; +} + +static int artpec8_pcie_get_rc_mem_resources(struct platform_device *pdev, + struct artpec8_pcie *artpec8_ctrl) +{ + struct dw_pcie *pci = artpec8_ctrl->pci; + + /* Data Bus Interface(DBI) Register */ + pci->dbi_base = devm_platform_ioremap_resource_byname(pdev, "dbi"); + if (IS_ERR(pci->dbi_base)) + return PTR_ERR(pci->dbi_base); + + return 0; +} + +static int artpec8_pcie_get_ep_mem_resources(struct platform_device *pdev, + struct artpec8_pcie *artpec8_ctrl) +{ + struct dw_pcie *pci = artpec8_ctrl->pci; + struct device *dev = &pdev->dev; + struct dw_pcie_ep *ep = &pci->ep; + struct resource *res; + + pci->dbi_base = devm_platform_ioremap_resource_byname(pdev, "dbi"); + if (IS_ERR(pci->dbi_base)) { + dev_err(dev, "failed to map ep_dbics\n"); + return -ENOMEM; + } + + pci->dbi_base2 = devm_platform_ioremap_resource_byname(pdev, "dbi2"); + if (IS_ERR(pci->dbi_base2)) { + dev_err(dev, "failed to map ep_dbics2\n"); + return -ENOMEM; + } + + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "addr_space"); + if (!res) + return -EINVAL; + ep->phys_base = res->start; + ep->addr_size = resource_size(res); + + return 0; +} + +static int artpec8_pcie_get_clk_resources(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + int ret; + + ret = devm_clk_bulk_get(dev, ARRAY_SIZE(artpec8_pcie_clks), + artpec8_pcie_clks); + if (ret) + return ret; + + return 0; +} + +static int artpec8_pcie_init_clk_resources(void) +{ + int ret; + + ret = clk_bulk_prepare_enable(ARRAY_SIZE(artpec8_pcie_clks), + artpec8_pcie_clks); + if (ret) + return ret; + + return 0; +} + +static void artpec8_pcie_deinit_clk_resources(void) +{ + clk_bulk_disable_unprepare(ARRAY_SIZE(artpec8_pcie_clks), + artpec8_pcie_clks); +} + +static const struct artpec8_pcie_res_ops artpec8_pcie_rc_res_ops = { + .get_mem_resources = artpec8_pcie_get_rc_mem_resources, + .get_clk_resources = artpec8_pcie_get_clk_resources, + .init_clk_resources = artpec8_pcie_init_clk_resources, + .deinit_clk_resources = artpec8_pcie_deinit_clk_resources, +}; + +static const struct artpec8_pcie_res_ops artpec8_pcie_ep_res_ops = { + .get_mem_resources = artpec8_pcie_get_ep_mem_resources, + .get_clk_resources = artpec8_pcie_get_clk_resources, + .init_clk_resources = artpec8_pcie_init_clk_resources, + .deinit_clk_resources = artpec8_pcie_deinit_clk_resources, +}; + +static int artpec8_pcie_config_phy_power_isolation(struct dw_pcie *pci, u8 val) +{ + struct artpec8_pcie *artpec8_ctrl = to_artpec8_pcie(pci); + + return regmap_write(artpec8_ctrl->pmu_syscon, PMU_SYSCON_PCIE_ISOLATION, + val); +} + +static int artpec8_pcie_config_bus_enable(struct dw_pcie *pci, u8 val) +{ + struct artpec8_pcie *artpec8_ctrl = to_artpec8_pcie(pci); + int ret; + + ret = regmap_write(artpec8_ctrl->bus_p_syscon, + BUS_SYSCON_BUS_PATH_ENABLE, val); + if (ret) + return ret; + + return regmap_write(artpec8_ctrl->bus_s_syscon, + BUS_SYSCON_BUS_PATH_ENABLE, val); +} + +static int artpec8_pcie_config_isolation(struct dw_pcie *pci, u8 val) +{ + int ret; + /* reg_val[0] : for phy power isolation */ + /* reg_val[1] : for bus enable */ + u8 reg_val[2]; + + switch (val) { + case PCIE_CLEAR_ISOLATION: + reg_val[0] = PCIE_REG_BIT_LOW; + reg_val[1] = PCIE_REG_BIT_HIGH; + break; + case PCIE_SET_ISOLATION: + reg_val[0] = PCIE_REG_BIT_HIGH; + reg_val[1] = PCIE_REG_BIT_LOW; + break; + default: + return -EINVAL; + } + + ret = artpec8_pcie_config_phy_power_isolation(pci, reg_val[0]); + if (ret) + return ret; + + return artpec8_pcie_config_bus_enable(pci, reg_val[1]); +} + +static int artpec8_pcie_config_perstn(struct dw_pcie *pci, u8 val) +{ + struct artpec8_pcie *artpec8_ctrl = to_artpec8_pcie(pci); + unsigned int bits; + + if (val == PCIE_REG_BIT_HIGH) + bits = PCIE_PERSTN; + else + bits = 0; + + return regmap_update_bits(artpec8_ctrl->sysreg, FSYS_PCIE_CON, + PCIE_PERSTN, bits); +} + +static void artpec8_pcie_stop_link(struct dw_pcie *pci) +{ + struct artpec8_pcie *artpec8_ctrl = to_artpec8_pcie(pci); + u32 val; + + val = readl(artpec8_ctrl->elbi_base + PCIE_APP_LTSSM_ENABLE); + + val &= ~APP_LTSSM_ENABLE_EN_BIT; + writel(val, artpec8_ctrl->elbi_base + PCIE_APP_LTSSM_ENABLE); +} + +static int artpec8_pcie_start_link(struct dw_pcie *pci) +{ + struct artpec8_pcie *artpec8_ctrl = to_artpec8_pcie(pci); + u32 val; + + dw_pcie_dbi_ro_wr_en(pci); + + /* Equalization disable */ + val = artpec8_pcie_read_dbi(pci, pci->dbi_base, PCIE_GEN3_RELATED_OFF, + 4); + artpec8_pcie_write_dbi(pci, pci->dbi_base, PCIE_GEN3_RELATED_OFF, 4, + val | PCIE_GEN3_EQUALIZATION_DISABLE); + + dw_pcie_dbi_ro_wr_dis(pci); + + /* assert LTSSM enable */ + val = readl(artpec8_ctrl->elbi_base + PCIE_APP_LTSSM_ENABLE); + + val |= APP_LTSSM_ENABLE_EN_BIT; + writel(val, artpec8_ctrl->elbi_base + PCIE_APP_LTSSM_ENABLE); + + return 0; +} + +static irqreturn_t artpec8_pcie_msi_irq_handler(int irq, void *arg) +{ + struct artpec8_pcie *artpec8_ctrl = arg; + struct dw_pcie *pci = artpec8_ctrl->pci; + struct pcie_port *pp = &pci->pp; + u32 val; + + val = readl(artpec8_ctrl->elbi_base + PCIE_IRQ2_STS); + + if ((val & IRQ2_STS_IRQ_MSI_ST) == IRQ2_STS_IRQ_MSI_ST) { + val &= IRQ2_STS_IRQ_MSI_ST; + writel(val, artpec8_ctrl->elbi_base + PCIE_IRQ2_STS); + dw_handle_msi_irq(pp); + } + + return IRQ_HANDLED; +} + +static void artpec8_pcie_msi_init(struct artpec8_pcie *artpec8_ctrl) +{ + u32 val; + + /* enable MSI interrupt */ + val = readl(artpec8_ctrl->elbi_base + PCIE_IRQ2_EN); + val |= IRQ2_EN_IRQ_MSI; + writel(val, artpec8_ctrl->elbi_base + PCIE_IRQ2_EN); +} + +static void artpec8_pcie_enable_interrupts(struct artpec8_pcie *artpec8_ctrl) +{ + if (IS_ENABLED(CONFIG_PCI_MSI)) + artpec8_pcie_msi_init(artpec8_ctrl); +} + +static int artpec8_pcie_rd_own_conf(struct pci_bus *bus, unsigned int devfn, + int where, int size, u32 *val) +{ + struct dw_pcie *pci = to_dw_pcie_from_pp(bus->sysdata); + + if (PCI_SLOT(devfn)) { + PCI_SET_ERROR_RESPONSE(val); + return PCIBIOS_DEVICE_NOT_FOUND; + } + + *val = dw_pcie_read_dbi(pci, where, size); + return PCIBIOS_SUCCESSFUL; +} + +static int artpec8_pcie_wr_own_conf(struct pci_bus *bus, unsigned int devfn, + int where, int size, u32 val) +{ + struct dw_pcie *pci = to_dw_pcie_from_pp(bus->sysdata); + + if (PCI_SLOT(devfn)) + return PCIBIOS_DEVICE_NOT_FOUND; + + dw_pcie_write_dbi(pci, where, size, val); + return PCIBIOS_SUCCESSFUL; +} + +static struct pci_ops artpec8_pci_ops = { + .read = artpec8_pcie_rd_own_conf, + .write = artpec8_pcie_wr_own_conf, +}; + +static int artpec8_pcie_link_up(struct dw_pcie *pci) +{ + struct artpec8_pcie *artpec8_ctrl = to_artpec8_pcie(pci); + u32 val; + + val = readl(artpec8_ctrl->elbi_base + PCIE_ELBI_CXPL_DEBUG_00_31); + + return (val & LTSSM_STATE_MASK) == LTSSM_STATE_L0; +} + +static int artpec8_pcie_host_init(struct pcie_port *pp) +{ + struct dw_pcie *pci = to_dw_pcie_from_pp(pp); + struct artpec8_pcie *artpec8_ctrl = to_artpec8_pcie(pci); + + pp->bridge->ops = &artpec8_pci_ops; + + dw_pcie_writel_dbi(pci, PCIE_GEN3_RELATED_OFF, (PCIE_GEN3_EQ_PHASE_2_3 | + PCIE_GEN3_RXEQ_PH01_EN | + PCIE_GEN3_RXEQ_RGRDLESS_RXTS)); + + artpec8_pcie_enable_interrupts(artpec8_ctrl); + + return 0; +} + +static const struct dw_pcie_host_ops artpec8_pcie_host_ops = { + .host_init = artpec8_pcie_host_init, +}; + +static u8 artpec8_pcie_iatu_unroll_enabled(struct dw_pcie *pci) +{ + u32 val; + + val = dw_pcie_readl_dbi(pci, PCIE_ATU_VIEWPORT); + pci->atu_base = pci->dbi_base + DEFAULT_DBI_ATU_OFFSET; + + if (val == 0xffffffff) + return 1; + + return 0; +} + +static void artpec8_pcie_ep_init(struct dw_pcie_ep *ep) +{ + struct dw_pcie *pci = to_dw_pcie_from_ep(ep); + enum pci_barno bar; + /* + * Currently PCIe EP core is not setting iatu_unroll_enabled + * so let's handle it here. We need to find proper place to + * initialize this so that it can be used for other EP + * controllers as well. + */ + pci->iatu_unroll_enabled = artpec8_pcie_iatu_unroll_enabled(pci); + + for (bar = BAR_0; bar <= BAR_5; bar++) + dw_pcie_ep_reset_bar(pci, bar); +} + +static int artpec8_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no, + enum pci_epc_irq_type type, u16 interrupt_num) +{ + struct dw_pcie *pci = to_dw_pcie_from_ep(ep); + + switch (type) { + case PCI_EPC_IRQ_LEGACY: + return -EINVAL; + case PCI_EPC_IRQ_MSI: + return dw_pcie_ep_raise_msi_irq(ep, func_no, interrupt_num); + default: + dev_err(pci->dev, "UNKNOWN IRQ type\n"); + } + + return 0; +} + +static const struct pci_epc_features artpec8_pcie_epc_features = { + .linkup_notifier = false, + .msi_capable = true, + .msix_capable = false, +}; + +static const struct pci_epc_features* +artpec8_pcie_ep_get_features(struct dw_pcie_ep *ep) +{ + return &artpec8_pcie_epc_features; +} + +static const struct dw_pcie_ep_ops artpec8_dw_pcie_ep_ops = { + .ep_init = artpec8_pcie_ep_init, + .raise_irq = artpec8_pcie_raise_irq, + .get_features = artpec8_pcie_ep_get_features, +}; + +static int __init artpec8_add_pcie_ep(struct artpec8_pcie *artpec8_ctrl, + struct platform_device *pdev) +{ + struct dw_pcie *pci = artpec8_ctrl->pci; + struct dw_pcie_ep *ep = &pci->ep; + int ret; + + ep->ops = &artpec8_dw_pcie_ep_ops; + + dw_pcie_writel_dbi(pci, PCIE_GEN3_RELATED_OFF, (PCIE_GEN3_EQ_PHASE_2_3 | + PCIE_GEN3_RXEQ_PH01_EN | + PCIE_GEN3_RXEQ_RGRDLESS_RXTS)); + + ret = dw_pcie_ep_init(ep); + if (ret) + return ret; + + return 0; +} + +static int __init artpec8_add_pcie_port(struct artpec8_pcie *artpec8_ctrl, + struct platform_device *pdev) +{ + struct dw_pcie *pci = artpec8_ctrl->pci; + struct pcie_port *pp = &pci->pp; + struct device *dev = &pdev->dev; + int irq; + int irq_flags; + int ret; + + if (IS_ENABLED(CONFIG_PCI_MSI)) { + irq = platform_get_irq_byname(pdev, "intr"); + if (!irq) + return -ENODEV; + + irq_flags = IRQF_SHARED | IRQF_NO_THREAD; + + ret = devm_request_irq(dev, irq, artpec8_pcie_msi_irq_handler, + irq_flags, "artpec8-pcie", artpec8_ctrl); + if (ret) + return ret; + } + + /* Prevent core from messing with the IRQ, since it's muxed */ + pp->msi_irq = -ENODEV; + + return dw_pcie_host_init(pp); +} + +static const struct dw_pcie_ops artpec8_dw_pcie_ops = { + .read_dbi = artpec8_pcie_read_dbi, + .write_dbi = artpec8_pcie_write_dbi, + .write_dbi2 = artpec8_pcie_write_dbi2, + .start_link = artpec8_pcie_start_link, + .stop_link = artpec8_pcie_stop_link, + .link_up = artpec8_pcie_link_up, +}; + +static int artpec8_pcie_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct artpec8_pcie *artpec8_ctrl; + struct dw_pcie *pci; + const struct artpec8_pcie_pdata *pdata; + enum dw_pcie_device_mode mode; + struct pcie_port *pp; + struct device_node *np = dev->of_node; + int ret; + + artpec8_ctrl = devm_kzalloc(dev, sizeof(*artpec8_ctrl), GFP_KERNEL); + if (!artpec8_ctrl) + return -ENOMEM; + + pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL); + if (!pci) + return -ENOMEM; + + pdata = of_device_get_match_data(dev); + + if (!pdata) + return -ENODEV; + + mode = (enum dw_pcie_device_mode)pdata->mode; + + artpec8_ctrl->pci = pci; + artpec8_ctrl->pdata = pdata; + artpec8_ctrl->mode = mode; + + pci->dev = dev; + pci->ops = pdata->dwc_ops; + pci->dbi_base2 = NULL; + pci->dbi_base = NULL; + pp = &pci->pp; + pp->ops = artpec8_ctrl->pdata->host_ops; + + if (mode == DW_PCIE_RC_TYPE) + artpec8_ctrl->link_id = of_alias_get_id(np, "pcierc"); + else + artpec8_ctrl->link_id = of_alias_get_id(np, "pcieep"); + + ret = artpec8_pcie_get_subsys_resources(pdev, artpec8_ctrl); + if (ret) + return ret; + + if (pdata->res_ops && pdata->res_ops->get_mem_resources) { + ret = pdata->res_ops->get_mem_resources(pdev, artpec8_ctrl); + if (ret) + return ret; + } + + if (pdata->res_ops && pdata->res_ops->get_clk_resources) { + ret = pdata->res_ops->get_clk_resources(pdev); + if (ret) + return ret; + + ret = pdata->res_ops->init_clk_resources(); + if (ret) + return ret; + } + + platform_set_drvdata(pdev, artpec8_ctrl); + + ret = artpec8_pcie_config_isolation(pci, PCIE_CLEAR_ISOLATION); + if (ret) + return ret; + + ret = artpec8_pcie_config_perstn(pci, PCIE_REG_BIT_HIGH); + if (ret) + return ret; + + artpec8_ctrl->phy = devm_of_phy_get(dev, np, NULL); + if (IS_ERR(artpec8_ctrl->phy)) + return PTR_ERR(artpec8_ctrl->phy); + + phy_init(artpec8_ctrl->phy); + phy_reset(artpec8_ctrl->phy); + + switch (mode) { + case DW_PCIE_RC_TYPE: + writel(DEVICE_TYPE_RC, artpec8_ctrl->elbi_base + + PCIE_ARTPEC8_DEVICE_TYPE); + ret = artpec8_add_pcie_port(artpec8_ctrl, pdev); + if (ret < 0) + goto fail_probe; + break; + case DW_PCIE_EP_TYPE: + writel(DEVICE_TYPE_EP, artpec8_ctrl->elbi_base + + PCIE_ARTPEC8_DEVICE_TYPE); + + ret = artpec8_add_pcie_ep(artpec8_ctrl, pdev); + if (ret < 0) + goto fail_probe; + break; + default: + ret = -EINVAL; + goto fail_probe; + } + + return 0; + +fail_probe: + phy_exit(artpec8_ctrl->phy); + if (pdata->res_ops && pdata->res_ops->deinit_clk_resources) + pdata->res_ops->deinit_clk_resources(); + + return ret; +} + +static int __exit artpec8_pcie_remove(struct platform_device *pdev) +{ + struct artpec8_pcie *artpec8_ctrl = platform_get_drvdata(pdev); + const struct artpec8_pcie_pdata *pdata = artpec8_ctrl->pdata; + + if (pdata->res_ops && pdata->res_ops->deinit_clk_resources) + pdata->res_ops->deinit_clk_resources(); + + return 0; +} + +static const struct artpec8_pcie_pdata artpec8_pcie_rc_pdata = { + .dwc_ops = &artpec8_dw_pcie_ops, + .host_ops = &artpec8_pcie_host_ops, + .res_ops = &artpec8_pcie_rc_res_ops, + .mode = DW_PCIE_RC_TYPE, +}; + +static const struct artpec8_pcie_pdata artpec8_pcie_ep_pdata = { + .dwc_ops = &artpec8_dw_pcie_ops, + .host_ops = &artpec8_pcie_host_ops, + .res_ops = &artpec8_pcie_ep_res_ops, + .mode = DW_PCIE_EP_TYPE, +}; + +static const struct of_device_id artpec8_pcie_of_match[] = { + { + .compatible = "axis,artpec8-pcie", + .data = &artpec8_pcie_rc_pdata, + }, + { + .compatible = "axis,artpec8-pcie-ep", + .data = &artpec8_pcie_ep_pdata, + }, + {}, +}; +MODULE_DEVICE_TABLE(of, artpec8_pcie_of_match); + +static struct platform_driver artpec8_pcie_driver = { + .probe = artpec8_pcie_probe, + .remove = __exit_p(artpec8_pcie_remove), + .driver = { + .name = "artpec8-pcie", + .of_match_table = artpec8_pcie_of_match, + }, +}; + +module_platform_driver(artpec8_pcie_driver); + +MODULE_LICENSE("GPL"); +MODULE_AUTHOR("Jaeho Cho <jaeho79.cho@samsung.com>");