On Wed, Jun 25, 2025 at 10:22:28PM +0530, Shradha Todi wrote:
> Add host and endpoint controller driver support for FSD SoC.
> +++ b/drivers/pci/controller/dwc/pci-exynos.c
> @@ -20,6 +20,8 @@
> #include <linux/regulator/consumer.h>
> #include <linux/mod_devicetable.h>
> #include <linux/module.h>
> +#include <linux/regmap.h>
> +#include <linux/mfd/syscon.h>
The trend is to sort these alphabetically. The last couple additions
didn't observe this, but maybe these new ones could go a little
farther up and make it more sorted rather than less?
> +#define FSD_PCIE_CXPL_DEBUG_00_31 0x2C8
Existing #defines use lower-case hex; please follow suit.
> +/* to store different SoC variants of Samsung */
> +enum samsung_pcie_variants {
> + FSD,
> + EXYNOS_5433,
> +};
> struct samsung_pcie_pdata {
> struct pci_ops *pci_ops;
> const struct dw_pcie_ops *dwc_ops;
> const struct dw_pcie_host_ops *host_ops;
> + const struct dw_pcie_ep_ops *ep_ops;
> const struct samsung_res_ops *res_ops;
> + unsigned int soc_variant;
> + enum dw_pcie_device_mode device_mode;
> };
> +static u32 fsd_pcie_read_dbi(struct dw_pcie *pci, void __iomem *base,
> + u32 reg, size_t size)
> +{
> + void __iomem *addr;
> + u32 val;
> +
> + addr = fsd_atu_setting(pci, base);
> +
> + dw_pcie_read(addr + reg, size, &val);
> +
> + return val;
Remove blank lines to match style of fsd_pcie_write_dbi2().
> +}
> +
> +static void fsd_pcie_write_dbi(struct dw_pcie *pci, void __iomem *base,
> + u32 reg, size_t size, u32 val)
> +{
> + void __iomem *addr;
> +
> + addr = fsd_atu_setting(pci, base);
> +
> + dw_pcie_write(addr + reg, size, val);
Ditto.
> +}
> +
> +static void fsd_pcie_write_dbi2(struct dw_pcie *pci, void __iomem *base,
> + u32 reg, size_t size, u32 val)
> +{
> + struct exynos_pcie *ep = to_exynos_pcie(pci);
> +
> + fsd_atu_setting(pci, base);
> + dw_pcie_write(pci->dbi_base + reg, size, val);
> + regmap_write(ep->sysreg, ep->sysreg_offset, ADDR_TYPE_DBI);
> +}
> +static int fsd_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
> + unsigned int type, u16 interrupt_num)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> +
> + switch (type) {
> + case PCI_IRQ_INTX:
> + return dw_pcie_ep_raise_intx_irq(ep, func_no);
> + case PCI_IRQ_MSIX:
> + dev_err(pci->dev, "EP does not support MSIX\n");
s/MSIX/MSI-X/ to match spec usage.
> @@ -373,13 +617,43 @@ static int exynos_pcie_probe(struct platform_device *pdev)
> return ret;
>
> platform_set_drvdata(pdev, ep);
> - ret = samsung_irq_init(ep, pdev);
> - if (ret)
> - goto fail_regulator;
> - ep->pci.pp.ops = pdata->host_ops;
> - ret = dw_pcie_host_init(&ep->pci.pp);
> - if (ret < 0)
> +
> + if (pdata->res_ops->set_device_mode)
> + pdata->res_ops->set_device_mode(ep);
> +
> + switch (ep->pdata->device_mode) {
> + case DW_PCIE_RC_TYPE:
> + ret = samsung_irq_init(ep, pdev);
> + if (ret)
> + goto fail_regulator;
> +
> + ep->pci.pp.ops = pdata->host_ops;
> +
> + ret = dw_pcie_host_init(&ep->pci.pp);
> + if (ret < 0)
> + goto fail_phy_init;
> +
> + break;
> + case DW_PCIE_EP_TYPE:
> + phy_init(ep->phy);
> +
> + ep->pci.ep.ops = pdata->ep_ops;
> +
> + ret = dw_pcie_ep_init(&ep->pci.ep);
> + if (ret < 0)
> + goto fail_phy_init;
> +
> + ret = dw_pcie_ep_init_registers(&ep->pci.ep);
> + if (ret)
> + goto fail_phy_init;
> +
> + pci_epc_init_notify(ep->pci.ep.epc);
> +
> + break;
> + default:
> + dev_err(dev, "invalid device type\n");
> goto fail_phy_init;
> + }
This would be a little nicer if you added soc_variant and device_mode
and the code that sets and tests them for exynos_5433 first in a
separate patch. Then it would be more obvious that the new FSD parts
don't affect exynos_5433 since this patch would only be *adding*
FSD-specific things.
> static const struct samsung_pcie_pdata exynos_5433_pcie_rc_pdata = {
> .dwc_ops = &exynos_dw_pcie_ops,
> .pci_ops = &exynos_pci_ops,
> .host_ops = &exynos_pcie_host_ops,
> .res_ops = &exynos_res_ops_data,
> + .soc_variant = EXYNOS_5433,
> + .device_mode = DW_PCIE_RC_TYPE,
> };
> static const struct of_device_id exynos_pcie_of_match[] = {
> @@ -449,6 +756,14 @@ static const struct of_device_id exynos_pcie_of_match[] = {
> .compatible = "samsung,exynos5433-pcie",
> .data = (void *) &exynos_5433_pcie_rc_pdata,
> },
