* [PATCH v3 0/6] PCI: controllers: tidy code up
@ 2023-12-15 2:29 Yoshihiro Shimoda
2023-12-15 2:29 ` [PATCH v3 1/6] PCI: dwc: Drop host prefix from struct dw_pcie_host_ops Yoshihiro Shimoda
` (5 more replies)
0 siblings, 6 replies; 14+ messages in thread
From: Yoshihiro Shimoda @ 2023-12-15 2:29 UTC (permalink / raw)
To: lpieralisi, kw, robh, bhelgaas, jingoohan1, gustavo.pimentel, mani
Cc: linux-pci, linux-renesas-soc, Yoshihiro Shimoda
This patch series tidies the code of PCIe dwc controllers and some
controllers up.
Changes from v2:
https://lore.kernel.org/linux-pci/20231114055456.2231990-1-yoshihiro.shimoda.uh@renesas.com/
- Based on the latest pci.git / next branch.
- Add Suggestion-by and Reviewed-by tags.
- Move read/write accessors to the header file in the patch 4/6.
- Revise the commit description in the patch 5/6.
Changes from v1:
https://lore.kernel.org/linux-pci/20231113013300.2132152-1-yoshihiro.shimoda.uh@renesas.com/
- Based on the latest pci.git / next branch.
- Add a new patch to drop host prefix of members from dw_pcie_host_ops
in the patch 1/6.
- Add Reviewed-by tag in the patch 3/6.
- Drop unneeded local variable in the patch 4/6.
- Add new patches to resolve issues of clang warnings in the patch [56]/6.
Justin Stitt (1):
PCI: iproc: fix -Wvoid-pointer-to-enum-cast warning
Yoshihiro Shimoda (5):
PCI: dwc: Drop host prefix from struct dw_pcie_host_ops
PCI: dwc: Rename to .init in struct dw_pcie_ep_ops
PCI: dwc: Rename to .get_dbi_offset in struct dw_pcie_ep_ops
PCI: dwc: Add dw_pcie_ep_{read,write}_dbi[2] helpers
PCI: rcar-gen4: fix -Wvoid-pointer-to-enum-cast warning
drivers/pci/controller/dwc/pci-dra7xx.c | 4 +-
drivers/pci/controller/dwc/pci-exynos.c | 2 +-
drivers/pci/controller/dwc/pci-imx6.c | 6 +-
drivers/pci/controller/dwc/pci-keystone.c | 8 +-
.../pci/controller/dwc/pci-layerscape-ep.c | 7 +-
drivers/pci/controller/dwc/pci-layerscape.c | 2 +-
drivers/pci/controller/dwc/pci-meson.c | 2 +-
drivers/pci/controller/dwc/pcie-al.c | 2 +-
drivers/pci/controller/dwc/pcie-armada8k.c | 2 +-
drivers/pci/controller/dwc/pcie-artpec6.c | 4 +-
drivers/pci/controller/dwc/pcie-bt1.c | 4 +-
.../pci/controller/dwc/pcie-designware-ep.c | 188 ++++++------------
.../pci/controller/dwc/pcie-designware-host.c | 30 +--
.../pci/controller/dwc/pcie-designware-plat.c | 2 +-
drivers/pci/controller/dwc/pcie-designware.h | 105 +++++++++-
drivers/pci/controller/dwc/pcie-dw-rockchip.c | 2 +-
drivers/pci/controller/dwc/pcie-fu740.c | 2 +-
drivers/pci/controller/dwc/pcie-histb.c | 2 +-
drivers/pci/controller/dwc/pcie-intel-gw.c | 2 +-
drivers/pci/controller/dwc/pcie-keembay.c | 2 +-
drivers/pci/controller/dwc/pcie-kirin.c | 2 +-
drivers/pci/controller/dwc/pcie-qcom-ep.c | 2 +-
drivers/pci/controller/dwc/pcie-qcom.c | 6 +-
drivers/pci/controller/dwc/pcie-rcar-gen4.c | 12 +-
drivers/pci/controller/dwc/pcie-spear13xx.c | 2 +-
drivers/pci/controller/dwc/pcie-tegra194.c | 2 +-
drivers/pci/controller/dwc/pcie-uniphier-ep.c | 2 +-
drivers/pci/controller/dwc/pcie-uniphier.c | 2 +-
drivers/pci/controller/dwc/pcie-visconti.c | 2 +-
drivers/pci/controller/pcie-iproc-platform.c | 2 +-
30 files changed, 220 insertions(+), 192 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 1/6] PCI: dwc: Drop host prefix from struct dw_pcie_host_ops
2023-12-15 2:29 [PATCH v3 0/6] PCI: controllers: tidy code up Yoshihiro Shimoda
@ 2023-12-15 2:29 ` Yoshihiro Shimoda
2023-12-15 2:29 ` [PATCH v3 2/6] PCI: dwc: Rename to .init in struct dw_pcie_ep_ops Yoshihiro Shimoda
` (4 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Yoshihiro Shimoda @ 2023-12-15 2:29 UTC (permalink / raw)
To: lpieralisi, kw, robh, bhelgaas, jingoohan1, gustavo.pimentel, mani
Cc: linux-pci, linux-renesas-soc, Yoshihiro Shimoda, Serge Semin,
Jesper Nilsson, Nobuhiro Iwamatsu, Manivannan Sadhasivam,
Kunihiko Hayashi, Heiko Stuebner, Lei Chuanhua, Thomas Petazzoni,
Vignesh Raghavendra, Krzysztof Kozlowski, Alim Akhtar,
Richard Zhu, Lucas Stach, Shawn Guo, Sascha Hauer, Fabio Estevam,
Minghuan Lian, Mingkai Hu, Roy Zang, Yue Wang, Neil Armstrong,
Kevin Hilman, Jerome Brunet, Martin Blumenstingl,
Jonathan Chocron, Paul Walmsley, Greentime Hu, Xiaowei Song,
Binghui Wang, Andy Gross, Bjorn Andersson, Konrad Dybcio,
Marek Vasut, Pratyush Anand, Thierry Reding, Jonathan Hunter,
Masami Hiramatsu
Since the name of dw_pcie_host_ops indicates that it's for host
obviously, drop host prefix from the struct.
Suggested-by: Serge Semin <fancer.lancer@gmail.com>
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Reviewed-by: Serge Semin <fancer.lancer@gmail.com>
Acked-by: Jesper Nilsson <jesper.nilsson@axis.com>
Acked-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Acked-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
Acked-by: Heiko Stuebner <heiko@sntech.de>
Acked-by: Lei Chuanhua <lchuanhua@maxlinear.com>
Reviewed-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
Cc: Vignesh Raghavendra <vigneshr@ti.com>
Cc: Jingoo Han <jingoohan1@gmail.com>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc: Alim Akhtar <alim.akhtar@samsung.com>
Cc: Richard Zhu <hongxing.zhu@nxp.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Minghuan Lian <minghuan.Lian@nxp.com>
Cc: Mingkai Hu <mingkai.hu@nxp.com>
Cc: Roy Zang <roy.zang@nxp.com>
Cc: Yue Wang <yue.wang@Amlogic.com>
Cc: Neil Armstrong <neil.armstrong@linaro.org>
Cc: Kevin Hilman <khilman@baylibre.com>
Cc: Jerome Brunet <jbrunet@baylibre.com>
Cc: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Cc: Jonathan Chocron <jonnyc@amazon.com>
Cc: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
Cc: Paul Walmsley <paul.walmsley@sifive.com>
Cc: Greentime Hu <greentime.hu@sifive.com>
Cc: Xiaowei Song <songxiaowei@hisilicon.com>
Cc: Binghui Wang <wangbinghui@hisilicon.com>
Cc: Andy Gross <agross@kernel.org>
Cc: Bjorn Andersson <andersson@kernel.org>
Cc: Konrad Dybcio <konrad.dybcio@linaro.org>
Cc: Marek Vasut <marek.vasut+renesas@gmail.com>
Cc: Pratyush Anand <pratyush.anand@gmail.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Jonathan Hunter <jonathanh@nvidia.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
---
drivers/pci/controller/dwc/pci-dra7xx.c | 2 +-
drivers/pci/controller/dwc/pci-exynos.c | 2 +-
drivers/pci/controller/dwc/pci-imx6.c | 4 +--
drivers/pci/controller/dwc/pci-keystone.c | 6 ++--
drivers/pci/controller/dwc/pci-layerscape.c | 2 +-
drivers/pci/controller/dwc/pci-meson.c | 2 +-
drivers/pci/controller/dwc/pcie-al.c | 2 +-
drivers/pci/controller/dwc/pcie-armada8k.c | 2 +-
drivers/pci/controller/dwc/pcie-artpec6.c | 2 +-
drivers/pci/controller/dwc/pcie-bt1.c | 4 +--
.../pci/controller/dwc/pcie-designware-host.c | 30 +++++++++----------
drivers/pci/controller/dwc/pcie-designware.h | 8 ++---
drivers/pci/controller/dwc/pcie-dw-rockchip.c | 2 +-
drivers/pci/controller/dwc/pcie-fu740.c | 2 +-
drivers/pci/controller/dwc/pcie-histb.c | 2 +-
drivers/pci/controller/dwc/pcie-intel-gw.c | 2 +-
drivers/pci/controller/dwc/pcie-kirin.c | 2 +-
drivers/pci/controller/dwc/pcie-qcom.c | 6 ++--
drivers/pci/controller/dwc/pcie-rcar-gen4.c | 4 +--
drivers/pci/controller/dwc/pcie-spear13xx.c | 2 +-
drivers/pci/controller/dwc/pcie-tegra194.c | 2 +-
drivers/pci/controller/dwc/pcie-uniphier.c | 2 +-
drivers/pci/controller/dwc/pcie-visconti.c | 2 +-
23 files changed, 47 insertions(+), 47 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c b/drivers/pci/controller/dwc/pci-dra7xx.c
index b445ffe95e3f..6125a838f4b1 100644
--- a/drivers/pci/controller/dwc/pci-dra7xx.c
+++ b/drivers/pci/controller/dwc/pci-dra7xx.c
@@ -371,7 +371,7 @@ static int dra7xx_pcie_init_irq_domain(struct dw_pcie_rp *pp)
}
static const struct dw_pcie_host_ops dra7xx_pcie_host_ops = {
- .host_init = dra7xx_pcie_host_init,
+ .init = dra7xx_pcie_host_init,
};
static void dra7xx_pcie_ep_init(struct dw_pcie_ep *ep)
diff --git a/drivers/pci/controller/dwc/pci-exynos.c b/drivers/pci/controller/dwc/pci-exynos.c
index c6bede346932..673ae213203f 100644
--- a/drivers/pci/controller/dwc/pci-exynos.c
+++ b/drivers/pci/controller/dwc/pci-exynos.c
@@ -268,7 +268,7 @@ static int exynos_pcie_host_init(struct dw_pcie_rp *pp)
}
static const struct dw_pcie_host_ops exynos_pcie_host_ops = {
- .host_init = exynos_pcie_host_init,
+ .init = exynos_pcie_host_init,
};
static int exynos_add_pcie_port(struct exynos_pcie *ep,
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 74703362aeec..b02f6f14a411 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -1039,8 +1039,8 @@ static void imx6_pcie_host_exit(struct dw_pcie_rp *pp)
}
static const struct dw_pcie_host_ops imx6_pcie_host_ops = {
- .host_init = imx6_pcie_host_init,
- .host_deinit = imx6_pcie_host_exit,
+ .init = imx6_pcie_host_init,
+ .deinit = imx6_pcie_host_exit,
};
static const struct dw_pcie_ops dw_pcie_ops = {
diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
index 0def919f89fa..3711347ddc87 100644
--- a/drivers/pci/controller/dwc/pci-keystone.c
+++ b/drivers/pci/controller/dwc/pci-keystone.c
@@ -838,12 +838,12 @@ static int __init ks_pcie_host_init(struct dw_pcie_rp *pp)
}
static const struct dw_pcie_host_ops ks_pcie_host_ops = {
- .host_init = ks_pcie_host_init,
- .msi_host_init = ks_pcie_msi_host_init,
+ .init = ks_pcie_host_init,
+ .msi_init = ks_pcie_msi_host_init,
};
static const struct dw_pcie_host_ops ks_pcie_am654_host_ops = {
- .host_init = ks_pcie_host_init,
+ .init = ks_pcie_host_init,
};
static irqreturn_t ks_pcie_err_irq_handler(int irq, void *priv)
diff --git a/drivers/pci/controller/dwc/pci-layerscape.c b/drivers/pci/controller/dwc/pci-layerscape.c
index 37956e09c65b..0c3d7ef729cb 100644
--- a/drivers/pci/controller/dwc/pci-layerscape.c
+++ b/drivers/pci/controller/dwc/pci-layerscape.c
@@ -169,7 +169,7 @@ static int ls_pcie_host_init(struct dw_pcie_rp *pp)
}
static const struct dw_pcie_host_ops ls_pcie_host_ops = {
- .host_init = ls_pcie_host_init,
+ .init = ls_pcie_host_init,
.pme_turn_off = ls_pcie_send_turnoff_msg,
};
diff --git a/drivers/pci/controller/dwc/pci-meson.c b/drivers/pci/controller/dwc/pci-meson.c
index 407558f5d74a..6477c83262c2 100644
--- a/drivers/pci/controller/dwc/pci-meson.c
+++ b/drivers/pci/controller/dwc/pci-meson.c
@@ -389,7 +389,7 @@ static int meson_pcie_host_init(struct dw_pcie_rp *pp)
}
static const struct dw_pcie_host_ops meson_pcie_host_ops = {
- .host_init = meson_pcie_host_init,
+ .init = meson_pcie_host_init,
};
static const struct dw_pcie_ops dw_pcie_ops = {
diff --git a/drivers/pci/controller/dwc/pcie-al.c b/drivers/pci/controller/dwc/pcie-al.c
index b8cb77c9c4bd..6dfdda59f328 100644
--- a/drivers/pci/controller/dwc/pcie-al.c
+++ b/drivers/pci/controller/dwc/pcie-al.c
@@ -311,7 +311,7 @@ static int al_pcie_host_init(struct dw_pcie_rp *pp)
}
static const struct dw_pcie_host_ops al_pcie_host_ops = {
- .host_init = al_pcie_host_init,
+ .init = al_pcie_host_init,
};
static int al_pcie_probe(struct platform_device *pdev)
diff --git a/drivers/pci/controller/dwc/pcie-armada8k.c b/drivers/pci/controller/dwc/pcie-armada8k.c
index 5c999e15c357..b5c599ccaacf 100644
--- a/drivers/pci/controller/dwc/pcie-armada8k.c
+++ b/drivers/pci/controller/dwc/pcie-armada8k.c
@@ -225,7 +225,7 @@ static irqreturn_t armada8k_pcie_irq_handler(int irq, void *arg)
}
static const struct dw_pcie_host_ops armada8k_pcie_host_ops = {
- .host_init = armada8k_pcie_host_init,
+ .init = armada8k_pcie_host_init,
};
static int armada8k_add_pcie_port(struct armada8k_pcie *pcie,
diff --git a/drivers/pci/controller/dwc/pcie-artpec6.c b/drivers/pci/controller/dwc/pcie-artpec6.c
index 9b572a2b2c9a..2f32fcd8933c 100644
--- a/drivers/pci/controller/dwc/pcie-artpec6.c
+++ b/drivers/pci/controller/dwc/pcie-artpec6.c
@@ -333,7 +333,7 @@ static int artpec6_pcie_host_init(struct dw_pcie_rp *pp)
}
static const struct dw_pcie_host_ops artpec6_pcie_host_ops = {
- .host_init = artpec6_pcie_host_init,
+ .init = artpec6_pcie_host_init,
};
static void artpec6_pcie_ep_init(struct dw_pcie_ep *ep)
diff --git a/drivers/pci/controller/dwc/pcie-bt1.c b/drivers/pci/controller/dwc/pcie-bt1.c
index 17e696797ff5..76d0ddea8007 100644
--- a/drivers/pci/controller/dwc/pcie-bt1.c
+++ b/drivers/pci/controller/dwc/pcie-bt1.c
@@ -559,8 +559,8 @@ static void bt1_pcie_host_deinit(struct dw_pcie_rp *pp)
}
static const struct dw_pcie_host_ops bt1_pcie_host_ops = {
- .host_init = bt1_pcie_host_init,
- .host_deinit = bt1_pcie_host_deinit,
+ .init = bt1_pcie_host_init,
+ .deinit = bt1_pcie_host_deinit,
};
static struct bt1_pcie *bt1_pcie_create_data(struct platform_device *pdev)
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 7991f0e179b2..d5fc31f8345f 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -441,14 +441,14 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
bridge->ops = &dw_pcie_ops;
bridge->child_ops = &dw_child_pcie_ops;
- if (pp->ops->host_init) {
- ret = pp->ops->host_init(pp);
+ if (pp->ops->init) {
+ ret = pp->ops->init(pp);
if (ret)
return ret;
}
if (pci_msi_enabled()) {
- pp->has_msi_ctrl = !(pp->ops->msi_host_init ||
+ pp->has_msi_ctrl = !(pp->ops->msi_init ||
of_property_read_bool(np, "msi-parent") ||
of_property_read_bool(np, "msi-map"));
@@ -464,8 +464,8 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
goto err_deinit_host;
}
- if (pp->ops->msi_host_init) {
- ret = pp->ops->msi_host_init(pp);
+ if (pp->ops->msi_init) {
+ ret = pp->ops->msi_init(pp);
if (ret < 0)
goto err_deinit_host;
} else if (pp->has_msi_ctrl) {
@@ -502,8 +502,8 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
if (ret)
goto err_stop_link;
- if (pp->ops->host_post_init)
- pp->ops->host_post_init(pp);
+ if (pp->ops->post_init)
+ pp->ops->post_init(pp);
return 0;
@@ -518,8 +518,8 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
dw_pcie_free_msi(pp);
err_deinit_host:
- if (pp->ops->host_deinit)
- pp->ops->host_deinit(pp);
+ if (pp->ops->deinit)
+ pp->ops->deinit(pp);
return ret;
}
@@ -539,8 +539,8 @@ void dw_pcie_host_deinit(struct dw_pcie_rp *pp)
if (pp->has_msi_ctrl)
dw_pcie_free_msi(pp);
- if (pp->ops->host_deinit)
- pp->ops->host_deinit(pp);
+ if (pp->ops->deinit)
+ pp->ops->deinit(pp);
}
EXPORT_SYMBOL_GPL(dw_pcie_host_deinit);
@@ -842,8 +842,8 @@ int dw_pcie_suspend_noirq(struct dw_pcie *pci)
return ret;
}
- if (pci->pp.ops->host_deinit)
- pci->pp.ops->host_deinit(&pci->pp);
+ if (pci->pp.ops->deinit)
+ pci->pp.ops->deinit(&pci->pp);
pci->suspended = true;
@@ -860,8 +860,8 @@ int dw_pcie_resume_noirq(struct dw_pcie *pci)
pci->suspended = false;
- if (pci->pp.ops->host_init) {
- ret = pci->pp.ops->host_init(&pci->pp);
+ if (pci->pp.ops->init) {
+ ret = pci->pp.ops->init(&pci->pp);
if (ret) {
dev_err(pci->dev, "Host init failed: %d\n", ret);
return ret;
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 55ff76e3d384..5c4518ad1bec 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -300,10 +300,10 @@ enum dw_pcie_ltssm {
};
struct dw_pcie_host_ops {
- int (*host_init)(struct dw_pcie_rp *pp);
- void (*host_deinit)(struct dw_pcie_rp *pp);
- void (*host_post_init)(struct dw_pcie_rp *pp);
- int (*msi_host_init)(struct dw_pcie_rp *pp);
+ int (*init)(struct dw_pcie_rp *pp);
+ void (*deinit)(struct dw_pcie_rp *pp);
+ void (*post_init)(struct dw_pcie_rp *pp);
+ int (*msi_init)(struct dw_pcie_rp *pp);
void (*pme_turn_off)(struct dw_pcie_rp *pp);
};
diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index 2fe42c70097f..961dabcb1ec8 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -215,7 +215,7 @@ static int rockchip_pcie_host_init(struct dw_pcie_rp *pp)
}
static const struct dw_pcie_host_ops rockchip_pcie_host_ops = {
- .host_init = rockchip_pcie_host_init,
+ .init = rockchip_pcie_host_init,
};
static int rockchip_pcie_clk_init(struct rockchip_pcie *rockchip)
diff --git a/drivers/pci/controller/dwc/pcie-fu740.c b/drivers/pci/controller/dwc/pcie-fu740.c
index 1e9b44b8bba4..66367252032b 100644
--- a/drivers/pci/controller/dwc/pcie-fu740.c
+++ b/drivers/pci/controller/dwc/pcie-fu740.c
@@ -279,7 +279,7 @@ static int fu740_pcie_host_init(struct dw_pcie_rp *pp)
}
static const struct dw_pcie_host_ops fu740_pcie_host_ops = {
- .host_init = fu740_pcie_host_init,
+ .init = fu740_pcie_host_init,
};
static const struct dw_pcie_ops dw_pcie_ops = {
diff --git a/drivers/pci/controller/dwc/pcie-histb.c b/drivers/pci/controller/dwc/pcie-histb.c
index fd484cc7c481..7a11c618b9d9 100644
--- a/drivers/pci/controller/dwc/pcie-histb.c
+++ b/drivers/pci/controller/dwc/pcie-histb.c
@@ -198,7 +198,7 @@ static int histb_pcie_host_init(struct dw_pcie_rp *pp)
}
static const struct dw_pcie_host_ops histb_pcie_host_ops = {
- .host_init = histb_pcie_host_init,
+ .init = histb_pcie_host_init,
};
static void histb_pcie_host_disable(struct histb_pcie *hipcie)
diff --git a/drivers/pci/controller/dwc/pcie-intel-gw.c b/drivers/pci/controller/dwc/pcie-intel-gw.c
index c9c93524e01d..be52e9db44af 100644
--- a/drivers/pci/controller/dwc/pcie-intel-gw.c
+++ b/drivers/pci/controller/dwc/pcie-intel-gw.c
@@ -391,7 +391,7 @@ static const struct dw_pcie_ops intel_pcie_ops = {
};
static const struct dw_pcie_host_ops intel_pcie_dw_ops = {
- .host_init = intel_pcie_rc_init,
+ .init = intel_pcie_rc_init,
};
static int intel_pcie_probe(struct platform_device *pdev)
diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
index 2ee146767971..c50e183f88d6 100644
--- a/drivers/pci/controller/dwc/pcie-kirin.c
+++ b/drivers/pci/controller/dwc/pcie-kirin.c
@@ -672,7 +672,7 @@ static const struct dw_pcie_ops kirin_dw_pcie_ops = {
};
static const struct dw_pcie_host_ops kirin_pcie_host_ops = {
- .host_init = kirin_pcie_host_init,
+ .init = kirin_pcie_host_init,
};
static int kirin_pcie_power_off(struct kirin_pcie *kirin_pcie)
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 6902e97719d1..cd986f50ec4a 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -1244,9 +1244,9 @@ static void qcom_pcie_host_post_init(struct dw_pcie_rp *pp)
}
static const struct dw_pcie_host_ops qcom_pcie_dw_ops = {
- .host_init = qcom_pcie_host_init,
- .host_deinit = qcom_pcie_host_deinit,
- .host_post_init = qcom_pcie_host_post_init,
+ .init = qcom_pcie_host_init,
+ .deinit = qcom_pcie_host_deinit,
+ .post_init = qcom_pcie_host_post_init,
};
/* Qcom IP rev.: 2.1.0 Synopsys IP rev.: 4.01a */
diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
index 3bc45e513b3d..193ed88d3c2f 100644
--- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
@@ -307,8 +307,8 @@ static void rcar_gen4_pcie_host_deinit(struct dw_pcie_rp *pp)
}
static const struct dw_pcie_host_ops rcar_gen4_pcie_host_ops = {
- .host_init = rcar_gen4_pcie_host_init,
- .host_deinit = rcar_gen4_pcie_host_deinit,
+ .init = rcar_gen4_pcie_host_init,
+ .deinit = rcar_gen4_pcie_host_deinit,
};
static int rcar_gen4_add_dw_pcie_rp(struct rcar_gen4_pcie *rcar)
diff --git a/drivers/pci/controller/dwc/pcie-spear13xx.c b/drivers/pci/controller/dwc/pcie-spear13xx.c
index 99d47ae80331..201dced209f0 100644
--- a/drivers/pci/controller/dwc/pcie-spear13xx.c
+++ b/drivers/pci/controller/dwc/pcie-spear13xx.c
@@ -148,7 +148,7 @@ static int spear13xx_pcie_host_init(struct dw_pcie_rp *pp)
}
static const struct dw_pcie_host_ops spear13xx_pcie_host_ops = {
- .host_init = spear13xx_pcie_host_init,
+ .init = spear13xx_pcie_host_init,
};
static int spear13xx_add_pcie_port(struct spear13xx_pcie *spear13xx_pcie,
diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
index 0fe113598ebb..52e26ed61380 100644
--- a/drivers/pci/controller/dwc/pcie-tegra194.c
+++ b/drivers/pci/controller/dwc/pcie-tegra194.c
@@ -1060,7 +1060,7 @@ static const struct dw_pcie_ops tegra_dw_pcie_ops = {
};
static const struct dw_pcie_host_ops tegra_pcie_dw_host_ops = {
- .host_init = tegra_pcie_dw_host_init,
+ .init = tegra_pcie_dw_host_init,
};
static void tegra_pcie_disable_phy(struct tegra_pcie_dw *pcie)
diff --git a/drivers/pci/controller/dwc/pcie-uniphier.c b/drivers/pci/controller/dwc/pcie-uniphier.c
index 48c3eba817b4..354fb3bd0a19 100644
--- a/drivers/pci/controller/dwc/pcie-uniphier.c
+++ b/drivers/pci/controller/dwc/pcie-uniphier.c
@@ -311,7 +311,7 @@ static int uniphier_pcie_host_init(struct dw_pcie_rp *pp)
}
static const struct dw_pcie_host_ops uniphier_pcie_host_ops = {
- .host_init = uniphier_pcie_host_init,
+ .init = uniphier_pcie_host_init,
};
static int uniphier_pcie_host_enable(struct uniphier_pcie *pcie)
diff --git a/drivers/pci/controller/dwc/pcie-visconti.c b/drivers/pci/controller/dwc/pcie-visconti.c
index 71026fefa366..318c278e65c8 100644
--- a/drivers/pci/controller/dwc/pcie-visconti.c
+++ b/drivers/pci/controller/dwc/pcie-visconti.c
@@ -236,7 +236,7 @@ static int visconti_pcie_host_init(struct dw_pcie_rp *pp)
}
static const struct dw_pcie_host_ops visconti_pcie_host_ops = {
- .host_init = visconti_pcie_host_init,
+ .init = visconti_pcie_host_init,
};
static int visconti_get_resources(struct platform_device *pdev,
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 2/6] PCI: dwc: Rename to .init in struct dw_pcie_ep_ops
2023-12-15 2:29 [PATCH v3 0/6] PCI: controllers: tidy code up Yoshihiro Shimoda
2023-12-15 2:29 ` [PATCH v3 1/6] PCI: dwc: Drop host prefix from struct dw_pcie_host_ops Yoshihiro Shimoda
@ 2023-12-15 2:29 ` Yoshihiro Shimoda
2023-12-15 2:29 ` [PATCH v3 3/6] PCI: dwc: Rename to .get_dbi_offset " Yoshihiro Shimoda
` (3 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Yoshihiro Shimoda @ 2023-12-15 2:29 UTC (permalink / raw)
To: lpieralisi, kw, robh, bhelgaas, jingoohan1, gustavo.pimentel, mani
Cc: linux-pci, linux-renesas-soc, Yoshihiro Shimoda, Serge Semin,
Manivannan Sadhasivam, Srikanth Thokala, Jesper Nilsson,
Kunihiko Hayashi, Richard Zhu, Lucas Stach, Shawn Guo,
Sascha Hauer, Fabio Estevam, Minghuan Lian, Mingkai Hu, Roy Zang,
Masami Hiramatsu
Since the name of dw_pcie_ep_ops indicates that it's for ep obviously,
rename a member .ep_init to .init.
Suggested-by: Serge Semin <fancer.lancer@gmail.com>
Suggested-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Reviewed-by: Serge Semin <fancer.lancer@gmail.com>
Reviewed-by: Srikanth Thokala <srikanth.thokala@intel.com>
Acked-by: Jesper Nilsson <jesper.nilsson@axis.com>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Acked-by: Kunihiko Hayashi <hayashi.kunihiko@socionext.com>
---
Cc: Richard Zhu <hongxing.zhu@nxp.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Minghuan Lian <minghuan.Lian@nxp.com>
Cc: Mingkai Hu <mingkai.hu@nxp.com>
Cc: Roy Zang <roy.zang@nxp.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
---
drivers/pci/controller/dwc/pci-dra7xx.c | 2 +-
drivers/pci/controller/dwc/pci-imx6.c | 2 +-
drivers/pci/controller/dwc/pci-keystone.c | 2 +-
drivers/pci/controller/dwc/pci-layerscape-ep.c | 2 +-
drivers/pci/controller/dwc/pcie-artpec6.c | 2 +-
drivers/pci/controller/dwc/pcie-designware-ep.c | 4 ++--
drivers/pci/controller/dwc/pcie-designware-plat.c | 2 +-
drivers/pci/controller/dwc/pcie-designware.h | 2 +-
drivers/pci/controller/dwc/pcie-keembay.c | 2 +-
drivers/pci/controller/dwc/pcie-qcom-ep.c | 2 +-
drivers/pci/controller/dwc/pcie-rcar-gen4.c | 2 +-
drivers/pci/controller/dwc/pcie-uniphier-ep.c | 2 +-
12 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c b/drivers/pci/controller/dwc/pci-dra7xx.c
index 6125a838f4b1..1ac1be12a235 100644
--- a/drivers/pci/controller/dwc/pci-dra7xx.c
+++ b/drivers/pci/controller/dwc/pci-dra7xx.c
@@ -436,7 +436,7 @@ dra7xx_pcie_get_features(struct dw_pcie_ep *ep)
}
static const struct dw_pcie_ep_ops pcie_ep_ops = {
- .ep_init = dra7xx_pcie_ep_init,
+ .init = dra7xx_pcie_ep_init,
.raise_irq = dra7xx_pcie_raise_irq,
.get_features = dra7xx_pcie_get_features,
};
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index b02f6f14a411..644916a67a38 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -1093,7 +1093,7 @@ imx6_pcie_ep_get_features(struct dw_pcie_ep *ep)
}
static const struct dw_pcie_ep_ops pcie_ep_ops = {
- .ep_init = imx6_pcie_ep_init,
+ .init = imx6_pcie_ep_init,
.raise_irq = imx6_pcie_ep_raise_irq,
.get_features = imx6_pcie_ep_get_features,
};
diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
index 3711347ddc87..d0f50cceede9 100644
--- a/drivers/pci/controller/dwc/pci-keystone.c
+++ b/drivers/pci/controller/dwc/pci-keystone.c
@@ -944,7 +944,7 @@ ks_pcie_am654_get_features(struct dw_pcie_ep *ep)
}
static const struct dw_pcie_ep_ops ks_pcie_am654_ep_ops = {
- .ep_init = ks_pcie_am654_ep_init,
+ .init = ks_pcie_am654_ep_init,
.raise_irq = ks_pcie_am654_raise_irq,
.get_features = &ks_pcie_am654_get_features,
};
diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c b/drivers/pci/controller/dwc/pci-layerscape-ep.c
index 3d3c50ef4b6f..4e4b687ef508 100644
--- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
+++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
@@ -195,7 +195,7 @@ static unsigned int ls_pcie_ep_func_conf_select(struct dw_pcie_ep *ep,
}
static const struct dw_pcie_ep_ops ls_pcie_ep_ops = {
- .ep_init = ls_pcie_ep_init,
+ .init = ls_pcie_ep_init,
.raise_irq = ls_pcie_ep_raise_irq,
.get_features = ls_pcie_ep_get_features,
.func_conf_select = ls_pcie_ep_func_conf_select,
diff --git a/drivers/pci/controller/dwc/pcie-artpec6.c b/drivers/pci/controller/dwc/pcie-artpec6.c
index 2f32fcd8933c..f6afa96a97e4 100644
--- a/drivers/pci/controller/dwc/pcie-artpec6.c
+++ b/drivers/pci/controller/dwc/pcie-artpec6.c
@@ -370,7 +370,7 @@ static int artpec6_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
}
static const struct dw_pcie_ep_ops pcie_ep_ops = {
- .ep_init = artpec6_pcie_ep_init,
+ .init = artpec6_pcie_ep_init,
.raise_irq = artpec6_pcie_raise_irq,
};
diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index f6207989fc6a..ea99a97ce504 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -794,8 +794,8 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
list_add_tail(&ep_func->list, &ep->func_list);
}
- if (ep->ops->ep_init)
- ep->ops->ep_init(ep);
+ if (ep->ops->init)
+ ep->ops->init(ep);
ret = pci_epc_mem_init(epc, ep->phys_base, ep->addr_size,
ep->page_size);
diff --git a/drivers/pci/controller/dwc/pcie-designware-plat.c b/drivers/pci/controller/dwc/pcie-designware-plat.c
index b625841e98aa..97088b7663e0 100644
--- a/drivers/pci/controller/dwc/pcie-designware-plat.c
+++ b/drivers/pci/controller/dwc/pcie-designware-plat.c
@@ -74,7 +74,7 @@ dw_plat_pcie_get_features(struct dw_pcie_ep *ep)
}
static const struct dw_pcie_ep_ops pcie_ep_ops = {
- .ep_init = dw_plat_pcie_ep_init,
+ .init = dw_plat_pcie_ep_init,
.raise_irq = dw_plat_pcie_ep_raise_irq,
.get_features = dw_plat_pcie_get_features,
};
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 5c4518ad1bec..3bc03a93732f 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -332,7 +332,7 @@ struct dw_pcie_rp {
struct dw_pcie_ep_ops {
void (*pre_init)(struct dw_pcie_ep *ep);
- void (*ep_init)(struct dw_pcie_ep *ep);
+ void (*init)(struct dw_pcie_ep *ep);
void (*deinit)(struct dw_pcie_ep *ep);
int (*raise_irq)(struct dw_pcie_ep *ep, u8 func_no,
enum pci_epc_irq_type type, u16 interrupt_num);
diff --git a/drivers/pci/controller/dwc/pcie-keembay.c b/drivers/pci/controller/dwc/pcie-keembay.c
index 289bff99d762..3c38e047d5ed 100644
--- a/drivers/pci/controller/dwc/pcie-keembay.c
+++ b/drivers/pci/controller/dwc/pcie-keembay.c
@@ -325,7 +325,7 @@ keembay_pcie_get_features(struct dw_pcie_ep *ep)
}
static const struct dw_pcie_ep_ops keembay_pcie_ep_ops = {
- .ep_init = keembay_pcie_ep_init,
+ .init = keembay_pcie_ep_init,
.raise_irq = keembay_pcie_ep_raise_irq,
.get_features = keembay_pcie_get_features,
};
diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
index 9e58f055199a..2b6f7c144c61 100644
--- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
+++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
@@ -796,7 +796,7 @@ static void qcom_pcie_ep_init(struct dw_pcie_ep *ep)
}
static const struct dw_pcie_ep_ops pci_ep_ops = {
- .ep_init = qcom_pcie_ep_init,
+ .init = qcom_pcie_ep_init,
.raise_irq = qcom_pcie_ep_raise_irq,
.get_features = qcom_pcie_epc_get_features,
};
diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
index 193ed88d3c2f..1c017997fb3e 100644
--- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
@@ -408,7 +408,7 @@ static unsigned int rcar_gen4_pcie_ep_get_dbi2_offset(struct dw_pcie_ep *ep,
static const struct dw_pcie_ep_ops pcie_ep_ops = {
.pre_init = rcar_gen4_pcie_ep_pre_init,
- .ep_init = rcar_gen4_pcie_ep_init,
+ .init = rcar_gen4_pcie_ep_init,
.deinit = rcar_gen4_pcie_ep_deinit,
.raise_irq = rcar_gen4_pcie_ep_raise_irq,
.get_features = rcar_gen4_pcie_ep_get_features,
diff --git a/drivers/pci/controller/dwc/pcie-uniphier-ep.c b/drivers/pci/controller/dwc/pcie-uniphier-ep.c
index cba3c88fcf39..40bd468f7e11 100644
--- a/drivers/pci/controller/dwc/pcie-uniphier-ep.c
+++ b/drivers/pci/controller/dwc/pcie-uniphier-ep.c
@@ -284,7 +284,7 @@ uniphier_pcie_get_features(struct dw_pcie_ep *ep)
}
static const struct dw_pcie_ep_ops uniphier_pcie_ep_ops = {
- .ep_init = uniphier_pcie_ep_init,
+ .init = uniphier_pcie_ep_init,
.raise_irq = uniphier_pcie_ep_raise_irq,
.get_features = uniphier_pcie_get_features,
};
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 3/6] PCI: dwc: Rename to .get_dbi_offset in struct dw_pcie_ep_ops
2023-12-15 2:29 [PATCH v3 0/6] PCI: controllers: tidy code up Yoshihiro Shimoda
2023-12-15 2:29 ` [PATCH v3 1/6] PCI: dwc: Drop host prefix from struct dw_pcie_host_ops Yoshihiro Shimoda
2023-12-15 2:29 ` [PATCH v3 2/6] PCI: dwc: Rename to .init in struct dw_pcie_ep_ops Yoshihiro Shimoda
@ 2023-12-15 2:29 ` Yoshihiro Shimoda
2023-12-15 2:29 ` [PATCH v3 4/6] PCI: dwc: Add dw_pcie_ep_{read,write}_dbi[2] helpers Yoshihiro Shimoda
` (2 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Yoshihiro Shimoda @ 2023-12-15 2:29 UTC (permalink / raw)
To: lpieralisi, kw, robh, bhelgaas, jingoohan1, gustavo.pimentel, mani
Cc: linux-pci, linux-renesas-soc, Yoshihiro Shimoda,
Manivannan Sadhasivam, Serge Semin
Since meaning of .func_conf_select is difficult to understand,
rename it to .get_dbi_offset.
Suggested-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Reviewed-by: Serge Semin <fancer.lancer@gmail.com>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
.../pci/controller/dwc/pci-layerscape-ep.c | 5 +-
.../pci/controller/dwc/pcie-designware-ep.c | 108 +++++++++---------
drivers/pci/controller/dwc/pcie-designware.h | 2 +-
drivers/pci/controller/dwc/pcie-rcar-gen4.c | 4 +-
4 files changed, 59 insertions(+), 60 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c b/drivers/pci/controller/dwc/pci-layerscape-ep.c
index 4e4b687ef508..961ff1b719a1 100644
--- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
+++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
@@ -184,8 +184,7 @@ static int ls_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
}
}
-static unsigned int ls_pcie_ep_func_conf_select(struct dw_pcie_ep *ep,
- u8 func_no)
+static unsigned int ls_pcie_ep_get_dbi_offset(struct dw_pcie_ep *ep, u8 func_no)
{
struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
struct ls_pcie_ep *pcie = to_ls_pcie_ep(pci);
@@ -198,7 +197,7 @@ static const struct dw_pcie_ep_ops ls_pcie_ep_ops = {
.init = ls_pcie_ep_init,
.raise_irq = ls_pcie_ep_raise_irq,
.get_features = ls_pcie_ep_get_features,
- .func_conf_select = ls_pcie_ep_func_conf_select,
+ .get_dbi_offset = ls_pcie_ep_get_dbi_offset,
};
static const struct ls_pcie_ep_drvdata ls1_ep_drvdata = {
diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index ea99a97ce504..1100671db887 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -43,14 +43,14 @@ dw_pcie_ep_get_func_from_ep(struct dw_pcie_ep *ep, u8 func_no)
return NULL;
}
-static unsigned int dw_pcie_ep_func_select(struct dw_pcie_ep *ep, u8 func_no)
+static unsigned int dw_pcie_ep_get_dbi_offset(struct dw_pcie_ep *ep, u8 func_no)
{
- unsigned int func_offset = 0;
+ unsigned int dbi_offset = 0;
- if (ep->ops->func_conf_select)
- func_offset = ep->ops->func_conf_select(ep, func_no);
+ if (ep->ops->get_dbi_offset)
+ dbi_offset = ep->ops->get_dbi_offset(ep, func_no);
- return func_offset;
+ return dbi_offset;
}
static unsigned int dw_pcie_ep_get_dbi2_offset(struct dw_pcie_ep *ep, u8 func_no)
@@ -59,8 +59,8 @@ static unsigned int dw_pcie_ep_get_dbi2_offset(struct dw_pcie_ep *ep, u8 func_no
if (ep->ops->get_dbi2_offset)
dbi2_offset = ep->ops->get_dbi2_offset(ep, func_no);
- else if (ep->ops->func_conf_select) /* for backward compatibility */
- dbi2_offset = ep->ops->func_conf_select(ep, func_no);
+ else if (ep->ops->get_dbi_offset) /* for backward compatibility */
+ dbi2_offset = ep->ops->get_dbi_offset(ep, func_no);
return dbi2_offset;
}
@@ -68,14 +68,14 @@ static unsigned int dw_pcie_ep_get_dbi2_offset(struct dw_pcie_ep *ep, u8 func_no
static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, u8 func_no,
enum pci_barno bar, int flags)
{
- unsigned int func_offset, dbi2_offset;
+ unsigned int dbi_offset, dbi2_offset;
struct dw_pcie_ep *ep = &pci->ep;
u32 reg, reg_dbi2;
- func_offset = dw_pcie_ep_func_select(ep, func_no);
+ dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
dbi2_offset = dw_pcie_ep_get_dbi2_offset(ep, func_no);
- reg = func_offset + PCI_BASE_ADDRESS_0 + (4 * bar);
+ reg = dbi_offset + PCI_BASE_ADDRESS_0 + (4 * bar);
reg_dbi2 = dbi2_offset + PCI_BASE_ADDRESS_0 + (4 * bar);
dw_pcie_dbi_ro_wr_en(pci);
dw_pcie_writel_dbi2(pci, reg_dbi2, 0x0);
@@ -102,16 +102,16 @@ static u8 __dw_pcie_ep_find_next_cap(struct dw_pcie_ep *ep, u8 func_no,
u8 cap_ptr, u8 cap)
{
struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
- unsigned int func_offset = 0;
+ unsigned int dbi_offset = 0;
u8 cap_id, next_cap_ptr;
u16 reg;
if (!cap_ptr)
return 0;
- func_offset = dw_pcie_ep_func_select(ep, func_no);
+ dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
- reg = dw_pcie_readw_dbi(pci, func_offset + cap_ptr);
+ reg = dw_pcie_readw_dbi(pci, dbi_offset + cap_ptr);
cap_id = (reg & 0x00ff);
if (cap_id > PCI_CAP_ID_MAX)
@@ -127,13 +127,13 @@ static u8 __dw_pcie_ep_find_next_cap(struct dw_pcie_ep *ep, u8 func_no,
static u8 dw_pcie_ep_find_capability(struct dw_pcie_ep *ep, u8 func_no, u8 cap)
{
struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
- unsigned int func_offset = 0;
+ unsigned int dbi_offset = 0;
u8 next_cap_ptr;
u16 reg;
- func_offset = dw_pcie_ep_func_select(ep, func_no);
+ dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
- reg = dw_pcie_readw_dbi(pci, func_offset + PCI_CAPABILITY_LIST);
+ reg = dw_pcie_readw_dbi(pci, dbi_offset + PCI_CAPABILITY_LIST);
next_cap_ptr = (reg & 0x00ff);
return __dw_pcie_ep_find_next_cap(ep, func_no, next_cap_ptr, cap);
@@ -144,23 +144,23 @@ static int dw_pcie_ep_write_header(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
{
struct dw_pcie_ep *ep = epc_get_drvdata(epc);
struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
- unsigned int func_offset = 0;
+ unsigned int dbi_offset = 0;
- func_offset = dw_pcie_ep_func_select(ep, func_no);
+ dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
dw_pcie_dbi_ro_wr_en(pci);
- dw_pcie_writew_dbi(pci, func_offset + PCI_VENDOR_ID, hdr->vendorid);
- dw_pcie_writew_dbi(pci, func_offset + PCI_DEVICE_ID, hdr->deviceid);
- dw_pcie_writeb_dbi(pci, func_offset + PCI_REVISION_ID, hdr->revid);
- dw_pcie_writeb_dbi(pci, func_offset + PCI_CLASS_PROG, hdr->progif_code);
- dw_pcie_writew_dbi(pci, func_offset + PCI_CLASS_DEVICE,
+ dw_pcie_writew_dbi(pci, dbi_offset + PCI_VENDOR_ID, hdr->vendorid);
+ dw_pcie_writew_dbi(pci, dbi_offset + PCI_DEVICE_ID, hdr->deviceid);
+ dw_pcie_writeb_dbi(pci, dbi_offset + PCI_REVISION_ID, hdr->revid);
+ dw_pcie_writeb_dbi(pci, dbi_offset + PCI_CLASS_PROG, hdr->progif_code);
+ dw_pcie_writew_dbi(pci, dbi_offset + PCI_CLASS_DEVICE,
hdr->subclass_code | hdr->baseclass_code << 8);
- dw_pcie_writeb_dbi(pci, func_offset + PCI_CACHE_LINE_SIZE,
+ dw_pcie_writeb_dbi(pci, dbi_offset + PCI_CACHE_LINE_SIZE,
hdr->cache_line_size);
- dw_pcie_writew_dbi(pci, func_offset + PCI_SUBSYSTEM_VENDOR_ID,
+ dw_pcie_writew_dbi(pci, dbi_offset + PCI_SUBSYSTEM_VENDOR_ID,
hdr->subsys_vendor_id);
- dw_pcie_writew_dbi(pci, func_offset + PCI_SUBSYSTEM_ID, hdr->subsys_id);
- dw_pcie_writeb_dbi(pci, func_offset + PCI_INTERRUPT_PIN,
+ dw_pcie_writew_dbi(pci, dbi_offset + PCI_SUBSYSTEM_ID, hdr->subsys_id);
+ dw_pcie_writeb_dbi(pci, dbi_offset + PCI_INTERRUPT_PIN,
hdr->interrupt_pin);
dw_pcie_dbi_ro_wr_dis(pci);
@@ -243,17 +243,17 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
{
struct dw_pcie_ep *ep = epc_get_drvdata(epc);
struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
- unsigned int func_offset, dbi2_offset;
+ unsigned int dbi_offset, dbi2_offset;
enum pci_barno bar = epf_bar->barno;
size_t size = epf_bar->size;
int flags = epf_bar->flags;
u32 reg, reg_dbi2;
int ret, type;
- func_offset = dw_pcie_ep_func_select(ep, func_no);
+ dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
dbi2_offset = dw_pcie_ep_get_dbi2_offset(ep, func_no);
- reg = PCI_BASE_ADDRESS_0 + (4 * bar) + func_offset;
+ reg = PCI_BASE_ADDRESS_0 + (4 * bar) + dbi_offset;
reg_dbi2 = PCI_BASE_ADDRESS_0 + (4 * bar) + dbi2_offset;
if (!(flags & PCI_BASE_ADDRESS_SPACE))
@@ -337,16 +337,16 @@ static int dw_pcie_ep_get_msi(struct pci_epc *epc, u8 func_no, u8 vfunc_no)
struct dw_pcie_ep *ep = epc_get_drvdata(epc);
struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
u32 val, reg;
- unsigned int func_offset = 0;
+ unsigned int dbi_offset = 0;
struct dw_pcie_ep_func *ep_func;
ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
if (!ep_func || !ep_func->msi_cap)
return -EINVAL;
- func_offset = dw_pcie_ep_func_select(ep, func_no);
+ dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
- reg = ep_func->msi_cap + func_offset + PCI_MSI_FLAGS;
+ reg = ep_func->msi_cap + dbi_offset + PCI_MSI_FLAGS;
val = dw_pcie_readw_dbi(pci, reg);
if (!(val & PCI_MSI_FLAGS_ENABLE))
return -EINVAL;
@@ -362,16 +362,16 @@ static int dw_pcie_ep_set_msi(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
struct dw_pcie_ep *ep = epc_get_drvdata(epc);
struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
u32 val, reg;
- unsigned int func_offset = 0;
+ unsigned int dbi_offset = 0;
struct dw_pcie_ep_func *ep_func;
ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
if (!ep_func || !ep_func->msi_cap)
return -EINVAL;
- func_offset = dw_pcie_ep_func_select(ep, func_no);
+ dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
- reg = ep_func->msi_cap + func_offset + PCI_MSI_FLAGS;
+ reg = ep_func->msi_cap + dbi_offset + PCI_MSI_FLAGS;
val = dw_pcie_readw_dbi(pci, reg);
val &= ~PCI_MSI_FLAGS_QMASK;
val |= FIELD_PREP(PCI_MSI_FLAGS_QMASK, interrupts);
@@ -387,16 +387,16 @@ static int dw_pcie_ep_get_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no)
struct dw_pcie_ep *ep = epc_get_drvdata(epc);
struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
u32 val, reg;
- unsigned int func_offset = 0;
+ unsigned int dbi_offset = 0;
struct dw_pcie_ep_func *ep_func;
ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
if (!ep_func || !ep_func->msix_cap)
return -EINVAL;
- func_offset = dw_pcie_ep_func_select(ep, func_no);
+ dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
- reg = ep_func->msix_cap + func_offset + PCI_MSIX_FLAGS;
+ reg = ep_func->msix_cap + dbi_offset + PCI_MSIX_FLAGS;
val = dw_pcie_readw_dbi(pci, reg);
if (!(val & PCI_MSIX_FLAGS_ENABLE))
return -EINVAL;
@@ -412,7 +412,7 @@ static int dw_pcie_ep_set_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
struct dw_pcie_ep *ep = epc_get_drvdata(epc);
struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
u32 val, reg;
- unsigned int func_offset = 0;
+ unsigned int dbi_offset = 0;
struct dw_pcie_ep_func *ep_func;
ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
@@ -421,19 +421,19 @@ static int dw_pcie_ep_set_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
dw_pcie_dbi_ro_wr_en(pci);
- func_offset = dw_pcie_ep_func_select(ep, func_no);
+ dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
- reg = ep_func->msix_cap + func_offset + PCI_MSIX_FLAGS;
+ reg = ep_func->msix_cap + dbi_offset + PCI_MSIX_FLAGS;
val = dw_pcie_readw_dbi(pci, reg);
val &= ~PCI_MSIX_FLAGS_QSIZE;
val |= interrupts;
dw_pcie_writew_dbi(pci, reg, val);
- reg = ep_func->msix_cap + func_offset + PCI_MSIX_TABLE;
+ reg = ep_func->msix_cap + dbi_offset + PCI_MSIX_TABLE;
val = offset | bir;
dw_pcie_writel_dbi(pci, reg, val);
- reg = ep_func->msix_cap + func_offset + PCI_MSIX_PBA;
+ reg = ep_func->msix_cap + dbi_offset + PCI_MSIX_PBA;
val = (offset + (interrupts * PCI_MSIX_ENTRY_SIZE)) | bir;
dw_pcie_writel_dbi(pci, reg, val);
@@ -514,7 +514,7 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
struct dw_pcie_ep_func *ep_func;
struct pci_epc *epc = ep->epc;
unsigned int aligned_offset;
- unsigned int func_offset = 0;
+ unsigned int dbi_offset = 0;
u16 msg_ctrl, msg_data;
u32 msg_addr_lower, msg_addr_upper, reg;
u64 msg_addr;
@@ -525,22 +525,22 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
if (!ep_func || !ep_func->msi_cap)
return -EINVAL;
- func_offset = dw_pcie_ep_func_select(ep, func_no);
+ dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
/* Raise MSI per the PCI Local Bus Specification Revision 3.0, 6.8.1. */
- reg = ep_func->msi_cap + func_offset + PCI_MSI_FLAGS;
+ reg = ep_func->msi_cap + dbi_offset + PCI_MSI_FLAGS;
msg_ctrl = dw_pcie_readw_dbi(pci, reg);
has_upper = !!(msg_ctrl & PCI_MSI_FLAGS_64BIT);
- reg = ep_func->msi_cap + func_offset + PCI_MSI_ADDRESS_LO;
+ reg = ep_func->msi_cap + dbi_offset + PCI_MSI_ADDRESS_LO;
msg_addr_lower = dw_pcie_readl_dbi(pci, reg);
if (has_upper) {
- reg = ep_func->msi_cap + func_offset + PCI_MSI_ADDRESS_HI;
+ reg = ep_func->msi_cap + dbi_offset + PCI_MSI_ADDRESS_HI;
msg_addr_upper = dw_pcie_readl_dbi(pci, reg);
- reg = ep_func->msi_cap + func_offset + PCI_MSI_DATA_64;
+ reg = ep_func->msi_cap + dbi_offset + PCI_MSI_DATA_64;
msg_data = dw_pcie_readw_dbi(pci, reg);
} else {
msg_addr_upper = 0;
- reg = ep_func->msi_cap + func_offset + PCI_MSI_DATA_32;
+ reg = ep_func->msi_cap + dbi_offset + PCI_MSI_DATA_32;
msg_data = dw_pcie_readw_dbi(pci, reg);
}
aligned_offset = msg_addr_lower & (epc->mem->window.page_size - 1);
@@ -585,7 +585,7 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
struct dw_pcie_ep_func *ep_func;
struct pci_epf_msix_tbl *msix_tbl;
struct pci_epc *epc = ep->epc;
- unsigned int func_offset = 0;
+ unsigned int dbi_offset = 0;
u32 reg, msg_data, vec_ctrl;
unsigned int aligned_offset;
u32 tbl_offset;
@@ -597,9 +597,9 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
if (!ep_func || !ep_func->msix_cap)
return -EINVAL;
- func_offset = dw_pcie_ep_func_select(ep, func_no);
+ dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
- reg = ep_func->msix_cap + func_offset + PCI_MSIX_TABLE;
+ reg = ep_func->msix_cap + dbi_offset + PCI_MSIX_TABLE;
tbl_offset = dw_pcie_readl_dbi(pci, reg);
bir = FIELD_GET(PCI_MSIX_TABLE_BIR, tbl_offset);
tbl_offset &= PCI_MSIX_TABLE_OFFSET;
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 3bc03a93732f..5e36da166ffe 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -344,7 +344,7 @@ struct dw_pcie_ep_ops {
* return a 0, and implement code in callback function of platform
* driver.
*/
- unsigned int (*func_conf_select)(struct dw_pcie_ep *ep, u8 func_no);
+ unsigned int (*get_dbi_offset)(struct dw_pcie_ep *ep, u8 func_no);
unsigned int (*get_dbi2_offset)(struct dw_pcie_ep *ep, u8 func_no);
};
diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
index 1c017997fb3e..70492f562e48 100644
--- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
@@ -394,7 +394,7 @@ rcar_gen4_pcie_ep_get_features(struct dw_pcie_ep *ep)
return &rcar_gen4_pcie_epc_features;
}
-static unsigned int rcar_gen4_pcie_ep_func_conf_select(struct dw_pcie_ep *ep,
+static unsigned int rcar_gen4_pcie_ep_get_dbi_offset(struct dw_pcie_ep *ep,
u8 func_no)
{
return func_no * RCAR_GEN4_PCIE_EP_FUNC_DBI_OFFSET;
@@ -412,7 +412,7 @@ static const struct dw_pcie_ep_ops pcie_ep_ops = {
.deinit = rcar_gen4_pcie_ep_deinit,
.raise_irq = rcar_gen4_pcie_ep_raise_irq,
.get_features = rcar_gen4_pcie_ep_get_features,
- .func_conf_select = rcar_gen4_pcie_ep_func_conf_select,
+ .get_dbi_offset = rcar_gen4_pcie_ep_get_dbi_offset,
.get_dbi2_offset = rcar_gen4_pcie_ep_get_dbi2_offset,
};
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 4/6] PCI: dwc: Add dw_pcie_ep_{read,write}_dbi[2] helpers
2023-12-15 2:29 [PATCH v3 0/6] PCI: controllers: tidy code up Yoshihiro Shimoda
` (2 preceding siblings ...)
2023-12-15 2:29 ` [PATCH v3 3/6] PCI: dwc: Rename to .get_dbi_offset " Yoshihiro Shimoda
@ 2023-12-15 2:29 ` Yoshihiro Shimoda
2023-12-15 9:51 ` Serge Semin
2023-12-17 16:34 ` Manivannan Sadhasivam
2023-12-15 2:29 ` [PATCH v3 5/6] PCI: iproc: fix -Wvoid-pointer-to-enum-cast warning Yoshihiro Shimoda
2023-12-15 2:29 ` [PATCH v3 6/6] PCI: rcar-gen4: " Yoshihiro Shimoda
5 siblings, 2 replies; 14+ messages in thread
From: Yoshihiro Shimoda @ 2023-12-15 2:29 UTC (permalink / raw)
To: lpieralisi, kw, robh, bhelgaas, jingoohan1, gustavo.pimentel, mani
Cc: linux-pci, linux-renesas-soc, Yoshihiro Shimoda, Serge Semin
The current code calculated some dbi[2] registers' offset by calling
dw_pcie_ep_get_dbi[2]_offset() in each function. To improve code
readability, add dw_pcie_ep_{read,write}_dbi[2} and some data-width
related helpers.
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Reviewed-by: Serge Semin <fancer.lancer@gmail.com>
---
.../pci/controller/dwc/pcie-designware-ep.c | 184 ++++++------------
drivers/pci/controller/dwc/pcie-designware.h | 93 +++++++++
2 files changed, 153 insertions(+), 124 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index 1100671db887..3ab03c0c14c0 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -43,46 +43,19 @@ dw_pcie_ep_get_func_from_ep(struct dw_pcie_ep *ep, u8 func_no)
return NULL;
}
-static unsigned int dw_pcie_ep_get_dbi_offset(struct dw_pcie_ep *ep, u8 func_no)
-{
- unsigned int dbi_offset = 0;
-
- if (ep->ops->get_dbi_offset)
- dbi_offset = ep->ops->get_dbi_offset(ep, func_no);
-
- return dbi_offset;
-}
-
-static unsigned int dw_pcie_ep_get_dbi2_offset(struct dw_pcie_ep *ep, u8 func_no)
-{
- unsigned int dbi2_offset = 0;
-
- if (ep->ops->get_dbi2_offset)
- dbi2_offset = ep->ops->get_dbi2_offset(ep, func_no);
- else if (ep->ops->get_dbi_offset) /* for backward compatibility */
- dbi2_offset = ep->ops->get_dbi_offset(ep, func_no);
-
- return dbi2_offset;
-}
-
static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, u8 func_no,
enum pci_barno bar, int flags)
{
- unsigned int dbi_offset, dbi2_offset;
struct dw_pcie_ep *ep = &pci->ep;
- u32 reg, reg_dbi2;
-
- dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
- dbi2_offset = dw_pcie_ep_get_dbi2_offset(ep, func_no);
+ u32 reg;
- reg = dbi_offset + PCI_BASE_ADDRESS_0 + (4 * bar);
- reg_dbi2 = dbi2_offset + PCI_BASE_ADDRESS_0 + (4 * bar);
+ reg = PCI_BASE_ADDRESS_0 + (4 * bar);
dw_pcie_dbi_ro_wr_en(pci);
- dw_pcie_writel_dbi2(pci, reg_dbi2, 0x0);
- dw_pcie_writel_dbi(pci, reg, 0x0);
+ dw_pcie_ep_writel_dbi2(ep, func_no, reg, 0x0);
+ dw_pcie_ep_writel_dbi(ep, func_no, reg, 0x0);
if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) {
- dw_pcie_writel_dbi2(pci, reg_dbi2 + 4, 0x0);
- dw_pcie_writel_dbi(pci, reg + 4, 0x0);
+ dw_pcie_ep_writel_dbi2(ep, func_no, reg + 4, 0x0);
+ dw_pcie_ep_writel_dbi(ep, func_no, reg + 4, 0x0);
}
dw_pcie_dbi_ro_wr_dis(pci);
}
@@ -99,19 +72,15 @@ void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
EXPORT_SYMBOL_GPL(dw_pcie_ep_reset_bar);
static u8 __dw_pcie_ep_find_next_cap(struct dw_pcie_ep *ep, u8 func_no,
- u8 cap_ptr, u8 cap)
+ u8 cap_ptr, u8 cap)
{
- struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
- unsigned int dbi_offset = 0;
u8 cap_id, next_cap_ptr;
u16 reg;
if (!cap_ptr)
return 0;
- dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
-
- reg = dw_pcie_readw_dbi(pci, dbi_offset + cap_ptr);
+ reg = dw_pcie_ep_readw_dbi(ep, func_no, cap_ptr);
cap_id = (reg & 0x00ff);
if (cap_id > PCI_CAP_ID_MAX)
@@ -126,14 +95,10 @@ static u8 __dw_pcie_ep_find_next_cap(struct dw_pcie_ep *ep, u8 func_no,
static u8 dw_pcie_ep_find_capability(struct dw_pcie_ep *ep, u8 func_no, u8 cap)
{
- struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
- unsigned int dbi_offset = 0;
u8 next_cap_ptr;
u16 reg;
- dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
-
- reg = dw_pcie_readw_dbi(pci, dbi_offset + PCI_CAPABILITY_LIST);
+ reg = dw_pcie_ep_readw_dbi(ep, func_no, PCI_CAPABILITY_LIST);
next_cap_ptr = (reg & 0x00ff);
return __dw_pcie_ep_find_next_cap(ep, func_no, next_cap_ptr, cap);
@@ -144,24 +109,21 @@ static int dw_pcie_ep_write_header(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
{
struct dw_pcie_ep *ep = epc_get_drvdata(epc);
struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
- unsigned int dbi_offset = 0;
-
- dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
dw_pcie_dbi_ro_wr_en(pci);
- dw_pcie_writew_dbi(pci, dbi_offset + PCI_VENDOR_ID, hdr->vendorid);
- dw_pcie_writew_dbi(pci, dbi_offset + PCI_DEVICE_ID, hdr->deviceid);
- dw_pcie_writeb_dbi(pci, dbi_offset + PCI_REVISION_ID, hdr->revid);
- dw_pcie_writeb_dbi(pci, dbi_offset + PCI_CLASS_PROG, hdr->progif_code);
- dw_pcie_writew_dbi(pci, dbi_offset + PCI_CLASS_DEVICE,
- hdr->subclass_code | hdr->baseclass_code << 8);
- dw_pcie_writeb_dbi(pci, dbi_offset + PCI_CACHE_LINE_SIZE,
- hdr->cache_line_size);
- dw_pcie_writew_dbi(pci, dbi_offset + PCI_SUBSYSTEM_VENDOR_ID,
- hdr->subsys_vendor_id);
- dw_pcie_writew_dbi(pci, dbi_offset + PCI_SUBSYSTEM_ID, hdr->subsys_id);
- dw_pcie_writeb_dbi(pci, dbi_offset + PCI_INTERRUPT_PIN,
- hdr->interrupt_pin);
+ dw_pcie_ep_writew_dbi(ep, func_no, PCI_VENDOR_ID, hdr->vendorid);
+ dw_pcie_ep_writew_dbi(ep, func_no, PCI_DEVICE_ID, hdr->deviceid);
+ dw_pcie_ep_writeb_dbi(ep, func_no, PCI_REVISION_ID, hdr->revid);
+ dw_pcie_ep_writeb_dbi(ep, func_no, PCI_CLASS_PROG, hdr->progif_code);
+ dw_pcie_ep_writew_dbi(ep, func_no, PCI_CLASS_DEVICE,
+ hdr->subclass_code | hdr->baseclass_code << 8);
+ dw_pcie_ep_writeb_dbi(ep, func_no, PCI_CACHE_LINE_SIZE,
+ hdr->cache_line_size);
+ dw_pcie_ep_writew_dbi(ep, func_no, PCI_SUBSYSTEM_VENDOR_ID,
+ hdr->subsys_vendor_id);
+ dw_pcie_ep_writew_dbi(ep, func_no, PCI_SUBSYSTEM_ID, hdr->subsys_id);
+ dw_pcie_ep_writeb_dbi(ep, func_no, PCI_INTERRUPT_PIN,
+ hdr->interrupt_pin);
dw_pcie_dbi_ro_wr_dis(pci);
return 0;
@@ -243,18 +205,13 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
{
struct dw_pcie_ep *ep = epc_get_drvdata(epc);
struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
- unsigned int dbi_offset, dbi2_offset;
enum pci_barno bar = epf_bar->barno;
size_t size = epf_bar->size;
int flags = epf_bar->flags;
- u32 reg, reg_dbi2;
int ret, type;
+ u32 reg;
- dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
- dbi2_offset = dw_pcie_ep_get_dbi2_offset(ep, func_no);
-
- reg = PCI_BASE_ADDRESS_0 + (4 * bar) + dbi_offset;
- reg_dbi2 = PCI_BASE_ADDRESS_0 + (4 * bar) + dbi2_offset;
+ reg = PCI_BASE_ADDRESS_0 + (4 * bar);
if (!(flags & PCI_BASE_ADDRESS_SPACE))
type = PCIE_ATU_TYPE_MEM;
@@ -270,12 +227,12 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
dw_pcie_dbi_ro_wr_en(pci);
- dw_pcie_writel_dbi2(pci, reg_dbi2, lower_32_bits(size - 1));
- dw_pcie_writel_dbi(pci, reg, flags);
+ dw_pcie_ep_writel_dbi2(ep, func_no, reg, lower_32_bits(size - 1));
+ dw_pcie_ep_writel_dbi(ep, func_no, reg, flags);
if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) {
- dw_pcie_writel_dbi2(pci, reg_dbi2 + 4, upper_32_bits(size - 1));
- dw_pcie_writel_dbi(pci, reg + 4, 0);
+ dw_pcie_ep_writel_dbi2(ep, func_no, reg + 4, upper_32_bits(size - 1));
+ dw_pcie_ep_writel_dbi(ep, func_no, reg + 4, 0);
}
ep->epf_bar[bar] = epf_bar;
@@ -335,19 +292,15 @@ static int dw_pcie_ep_map_addr(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
static int dw_pcie_ep_get_msi(struct pci_epc *epc, u8 func_no, u8 vfunc_no)
{
struct dw_pcie_ep *ep = epc_get_drvdata(epc);
- struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
- u32 val, reg;
- unsigned int dbi_offset = 0;
struct dw_pcie_ep_func *ep_func;
+ u32 val, reg;
ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
if (!ep_func || !ep_func->msi_cap)
return -EINVAL;
- dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
-
- reg = ep_func->msi_cap + dbi_offset + PCI_MSI_FLAGS;
- val = dw_pcie_readw_dbi(pci, reg);
+ reg = ep_func->msi_cap + PCI_MSI_FLAGS;
+ val = dw_pcie_ep_readw_dbi(ep, func_no, reg);
if (!(val & PCI_MSI_FLAGS_ENABLE))
return -EINVAL;
@@ -361,22 +314,19 @@ static int dw_pcie_ep_set_msi(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
{
struct dw_pcie_ep *ep = epc_get_drvdata(epc);
struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
- u32 val, reg;
- unsigned int dbi_offset = 0;
struct dw_pcie_ep_func *ep_func;
+ u32 val, reg;
ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
if (!ep_func || !ep_func->msi_cap)
return -EINVAL;
- dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
-
- reg = ep_func->msi_cap + dbi_offset + PCI_MSI_FLAGS;
- val = dw_pcie_readw_dbi(pci, reg);
+ reg = ep_func->msi_cap + PCI_MSI_FLAGS;
+ val = dw_pcie_ep_readw_dbi(ep, func_no, reg);
val &= ~PCI_MSI_FLAGS_QMASK;
val |= FIELD_PREP(PCI_MSI_FLAGS_QMASK, interrupts);
dw_pcie_dbi_ro_wr_en(pci);
- dw_pcie_writew_dbi(pci, reg, val);
+ dw_pcie_ep_writew_dbi(ep, func_no, reg, val);
dw_pcie_dbi_ro_wr_dis(pci);
return 0;
@@ -385,19 +335,15 @@ static int dw_pcie_ep_set_msi(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
static int dw_pcie_ep_get_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no)
{
struct dw_pcie_ep *ep = epc_get_drvdata(epc);
- struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
- u32 val, reg;
- unsigned int dbi_offset = 0;
struct dw_pcie_ep_func *ep_func;
+ u32 val, reg;
ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
if (!ep_func || !ep_func->msix_cap)
return -EINVAL;
- dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
-
- reg = ep_func->msix_cap + dbi_offset + PCI_MSIX_FLAGS;
- val = dw_pcie_readw_dbi(pci, reg);
+ reg = ep_func->msix_cap + PCI_MSIX_FLAGS;
+ val = dw_pcie_ep_readw_dbi(ep, func_no, reg);
if (!(val & PCI_MSIX_FLAGS_ENABLE))
return -EINVAL;
@@ -411,9 +357,8 @@ static int dw_pcie_ep_set_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
{
struct dw_pcie_ep *ep = epc_get_drvdata(epc);
struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
- u32 val, reg;
- unsigned int dbi_offset = 0;
struct dw_pcie_ep_func *ep_func;
+ u32 val, reg;
ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
if (!ep_func || !ep_func->msix_cap)
@@ -421,21 +366,19 @@ static int dw_pcie_ep_set_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
dw_pcie_dbi_ro_wr_en(pci);
- dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
-
- reg = ep_func->msix_cap + dbi_offset + PCI_MSIX_FLAGS;
- val = dw_pcie_readw_dbi(pci, reg);
+ reg = ep_func->msix_cap + PCI_MSIX_FLAGS;
+ val = dw_pcie_ep_readw_dbi(ep, func_no, reg);
val &= ~PCI_MSIX_FLAGS_QSIZE;
val |= interrupts;
dw_pcie_writew_dbi(pci, reg, val);
- reg = ep_func->msix_cap + dbi_offset + PCI_MSIX_TABLE;
+ reg = ep_func->msix_cap + PCI_MSIX_TABLE;
val = offset | bir;
- dw_pcie_writel_dbi(pci, reg, val);
+ dw_pcie_ep_writel_dbi(ep, func_no, reg, val);
- reg = ep_func->msix_cap + dbi_offset + PCI_MSIX_PBA;
+ reg = ep_func->msix_cap + PCI_MSIX_PBA;
val = (offset + (interrupts * PCI_MSIX_ENTRY_SIZE)) | bir;
- dw_pcie_writel_dbi(pci, reg, val);
+ dw_pcie_ep_writel_dbi(ep, func_no, reg, val);
dw_pcie_dbi_ro_wr_dis(pci);
@@ -510,38 +453,34 @@ EXPORT_SYMBOL_GPL(dw_pcie_ep_raise_legacy_irq);
int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
u8 interrupt_num)
{
- struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+ u32 msg_addr_lower, msg_addr_upper, reg;
struct dw_pcie_ep_func *ep_func;
struct pci_epc *epc = ep->epc;
unsigned int aligned_offset;
- unsigned int dbi_offset = 0;
u16 msg_ctrl, msg_data;
- u32 msg_addr_lower, msg_addr_upper, reg;
- u64 msg_addr;
bool has_upper;
+ u64 msg_addr;
int ret;
ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
if (!ep_func || !ep_func->msi_cap)
return -EINVAL;
- dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
-
/* Raise MSI per the PCI Local Bus Specification Revision 3.0, 6.8.1. */
- reg = ep_func->msi_cap + dbi_offset + PCI_MSI_FLAGS;
- msg_ctrl = dw_pcie_readw_dbi(pci, reg);
+ reg = ep_func->msi_cap + PCI_MSI_FLAGS;
+ msg_ctrl = dw_pcie_ep_readw_dbi(ep, func_no, reg);
has_upper = !!(msg_ctrl & PCI_MSI_FLAGS_64BIT);
- reg = ep_func->msi_cap + dbi_offset + PCI_MSI_ADDRESS_LO;
- msg_addr_lower = dw_pcie_readl_dbi(pci, reg);
+ reg = ep_func->msi_cap + PCI_MSI_ADDRESS_LO;
+ msg_addr_lower = dw_pcie_ep_readl_dbi(ep, func_no, reg);
if (has_upper) {
- reg = ep_func->msi_cap + dbi_offset + PCI_MSI_ADDRESS_HI;
- msg_addr_upper = dw_pcie_readl_dbi(pci, reg);
- reg = ep_func->msi_cap + dbi_offset + PCI_MSI_DATA_64;
- msg_data = dw_pcie_readw_dbi(pci, reg);
+ reg = ep_func->msi_cap + PCI_MSI_ADDRESS_HI;
+ msg_addr_upper = dw_pcie_ep_readl_dbi(ep, func_no, reg);
+ reg = ep_func->msi_cap + PCI_MSI_DATA_64;
+ msg_data = dw_pcie_ep_readw_dbi(ep, func_no, reg);
} else {
msg_addr_upper = 0;
- reg = ep_func->msi_cap + dbi_offset + PCI_MSI_DATA_32;
- msg_data = dw_pcie_readw_dbi(pci, reg);
+ reg = ep_func->msi_cap + PCI_MSI_DATA_32;
+ msg_data = dw_pcie_ep_readw_dbi(ep, func_no, reg);
}
aligned_offset = msg_addr_lower & (epc->mem->window.page_size - 1);
msg_addr = ((u64)msg_addr_upper) << 32 |
@@ -582,10 +521,9 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
u16 interrupt_num)
{
struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
- struct dw_pcie_ep_func *ep_func;
struct pci_epf_msix_tbl *msix_tbl;
+ struct dw_pcie_ep_func *ep_func;
struct pci_epc *epc = ep->epc;
- unsigned int dbi_offset = 0;
u32 reg, msg_data, vec_ctrl;
unsigned int aligned_offset;
u32 tbl_offset;
@@ -597,10 +535,8 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
if (!ep_func || !ep_func->msix_cap)
return -EINVAL;
- dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
-
- reg = ep_func->msix_cap + dbi_offset + PCI_MSIX_TABLE;
- tbl_offset = dw_pcie_readl_dbi(pci, reg);
+ reg = ep_func->msix_cap + PCI_MSIX_TABLE;
+ tbl_offset = dw_pcie_ep_readl_dbi(ep, func_no, reg);
bir = FIELD_GET(PCI_MSIX_TABLE_BIR, tbl_offset);
tbl_offset &= PCI_MSIX_TABLE_OFFSET;
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 5e36da166ffe..b92e69041fe8 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -534,6 +534,99 @@ static inline enum dw_pcie_ltssm dw_pcie_get_ltssm(struct dw_pcie *pci)
return (enum dw_pcie_ltssm)FIELD_GET(PORT_LOGIC_LTSSM_STATE_MASK, val);
}
+static inline unsigned int dw_pcie_ep_get_dbi_offset(struct dw_pcie_ep *ep,
+ u8 func_no)
+{
+ unsigned int dbi_offset = 0;
+
+ if (ep->ops->get_dbi_offset)
+ dbi_offset = ep->ops->get_dbi_offset(ep, func_no);
+
+ return dbi_offset;
+}
+
+static inline unsigned int dw_pcie_ep_get_dbi2_offset(struct dw_pcie_ep *ep,
+ u8 func_no)
+{
+ unsigned int dbi2_offset = 0;
+
+ if (ep->ops->get_dbi2_offset)
+ dbi2_offset = ep->ops->get_dbi2_offset(ep, func_no);
+ else if (ep->ops->get_dbi_offset) /* for backward compatibility */
+ dbi2_offset = ep->ops->get_dbi_offset(ep, func_no);
+
+ return dbi2_offset;
+}
+
+static inline u32 dw_pcie_ep_read_dbi(struct dw_pcie_ep *ep, u8 func_no,
+ u32 reg, size_t size)
+{
+ unsigned int offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
+ struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+
+ return dw_pcie_read_dbi(pci, offset + reg, size);
+}
+
+static inline void dw_pcie_ep_write_dbi(struct dw_pcie_ep *ep, u8 func_no,
+ u32 reg, size_t size, u32 val)
+{
+ unsigned int offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
+ struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+
+ dw_pcie_write_dbi(pci, offset + reg, size, val);
+}
+
+static inline void dw_pcie_ep_write_dbi2(struct dw_pcie_ep *ep, u8 func_no,
+ u32 reg, size_t size, u32 val)
+{
+ unsigned int offset = dw_pcie_ep_get_dbi2_offset(ep, func_no);
+ struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+
+ dw_pcie_write_dbi2(pci, offset + reg, size, val);
+}
+
+static inline void dw_pcie_ep_writel_dbi(struct dw_pcie_ep *ep, u8 func_no,
+ u32 reg, u32 val)
+{
+ dw_pcie_ep_write_dbi(ep, func_no, reg, 0x4, val);
+}
+
+static inline u32 dw_pcie_ep_readl_dbi(struct dw_pcie_ep *ep, u8 func_no,
+ u32 reg)
+{
+ return dw_pcie_ep_read_dbi(ep, func_no, reg, 0x4);
+}
+
+static inline void dw_pcie_ep_writew_dbi(struct dw_pcie_ep *ep, u8 func_no,
+ u32 reg, u16 val)
+{
+ dw_pcie_ep_write_dbi(ep, func_no, reg, 0x2, val);
+}
+
+static inline u16 dw_pcie_ep_readw_dbi(struct dw_pcie_ep *ep, u8 func_no,
+ u32 reg)
+{
+ return dw_pcie_ep_read_dbi(ep, func_no, reg, 0x2);
+}
+
+static inline void dw_pcie_ep_writeb_dbi(struct dw_pcie_ep *ep, u8 func_no,
+ u32 reg, u8 val)
+{
+ dw_pcie_ep_write_dbi(ep, func_no, reg, 0x1, val);
+}
+
+static inline u8 dw_pcie_ep_readb_dbi(struct dw_pcie_ep *ep, u8 func_no,
+ u32 reg)
+{
+ return dw_pcie_ep_read_dbi(ep, func_no, reg, 0x1);
+}
+
+static inline void dw_pcie_ep_writel_dbi2(struct dw_pcie_ep *ep, u8 func_no,
+ u32 reg, u32 val)
+{
+ dw_pcie_ep_write_dbi2(ep, func_no, reg, 0x4, val);
+}
+
#ifdef CONFIG_PCIE_DW_HOST
irqreturn_t dw_handle_msi_irq(struct dw_pcie_rp *pp);
int dw_pcie_setup_rc(struct dw_pcie_rp *pp);
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 5/6] PCI: iproc: fix -Wvoid-pointer-to-enum-cast warning
2023-12-15 2:29 [PATCH v3 0/6] PCI: controllers: tidy code up Yoshihiro Shimoda
` (3 preceding siblings ...)
2023-12-15 2:29 ` [PATCH v3 4/6] PCI: dwc: Add dw_pcie_ep_{read,write}_dbi[2] helpers Yoshihiro Shimoda
@ 2023-12-15 2:29 ` Yoshihiro Shimoda
2023-12-15 2:29 ` [PATCH v3 6/6] PCI: rcar-gen4: " Yoshihiro Shimoda
5 siblings, 0 replies; 14+ messages in thread
From: Yoshihiro Shimoda @ 2023-12-15 2:29 UTC (permalink / raw)
To: lpieralisi, kw, robh, bhelgaas, jingoohan1, gustavo.pimentel, mani
Cc: linux-pci, linux-renesas-soc, Justin Stitt, Nathan Chancellor,
Yoshihiro Shimoda, Geert Uytterhoeven, Manivannan Sadhasivam
From: Justin Stitt <justinstitt@google.com>
When building with clang 18 I see the following warning:
| drivers/pci/controller/pcie-iproc-platform.c:55:15: warning: cast to smaller
| integer type 'enum iproc_pcie_type' from 'const void *' [-Wvoid-pointer-to-enum-cast]
| 55 | pcie->type = (enum iproc_pcie_type) of_device_get_match_data(dev);
To fix this issue, use uintptr_t instead.
Link: https://github.com/ClangBuiltLinux/linux/issues/1910
Reported-by: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Justin Stitt <justinstitt@google.com>
[shimoda: revise the commit description]
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
drivers/pci/controller/pcie-iproc-platform.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/controller/pcie-iproc-platform.c b/drivers/pci/controller/pcie-iproc-platform.c
index acdc583d2980..83cbc95f4384 100644
--- a/drivers/pci/controller/pcie-iproc-platform.c
+++ b/drivers/pci/controller/pcie-iproc-platform.c
@@ -52,7 +52,7 @@ static int iproc_pltfm_pcie_probe(struct platform_device *pdev)
pcie = pci_host_bridge_priv(bridge);
pcie->dev = dev;
- pcie->type = (enum iproc_pcie_type) of_device_get_match_data(dev);
+ pcie->type = (uintptr_t) of_device_get_match_data(dev);
ret = of_address_to_resource(np, 0, ®);
if (ret < 0) {
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 6/6] PCI: rcar-gen4: fix -Wvoid-pointer-to-enum-cast warning
2023-12-15 2:29 [PATCH v3 0/6] PCI: controllers: tidy code up Yoshihiro Shimoda
` (4 preceding siblings ...)
2023-12-15 2:29 ` [PATCH v3 5/6] PCI: iproc: fix -Wvoid-pointer-to-enum-cast warning Yoshihiro Shimoda
@ 2023-12-15 2:29 ` Yoshihiro Shimoda
5 siblings, 0 replies; 14+ messages in thread
From: Yoshihiro Shimoda @ 2023-12-15 2:29 UTC (permalink / raw)
To: lpieralisi, kw, robh, bhelgaas, jingoohan1, gustavo.pimentel, mani
Cc: linux-pci, linux-renesas-soc, Yoshihiro Shimoda,
Geert Uytterhoeven, Manivannan Sadhasivam
When building with clang 18 with adding -Wvoid-pointer-to-enum-cast,
the following error happens:
drivers/pci/controller/dwc/pcie-rcar-gen4.c:439:15: error: cast to smaller integer type 'enum dw_pcie_device_mode' from 'const void *' [-Werror,-Wvoid-pointer-to-enum-cast]
439 | rcar->mode = (enum dw_pcie_device_mode)of_device_get_match_data(&rcar->pdev->dev);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
To fix this issue, use uintptr_t instead.
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
drivers/pci/controller/dwc/pcie-rcar-gen4.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
index 70492f562e48..a1eb10e878f1 100644
--- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
@@ -436,7 +436,7 @@ static void rcar_gen4_remove_dw_pcie_ep(struct rcar_gen4_pcie *rcar)
/* Common */
static int rcar_gen4_add_dw_pcie(struct rcar_gen4_pcie *rcar)
{
- rcar->mode = (enum dw_pcie_device_mode)of_device_get_match_data(&rcar->pdev->dev);
+ rcar->mode = (uintptr_t)of_device_get_match_data(&rcar->pdev->dev);
switch (rcar->mode) {
case DW_PCIE_RC_TYPE:
--
2.34.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v3 4/6] PCI: dwc: Add dw_pcie_ep_{read,write}_dbi[2] helpers
2023-12-15 2:29 ` [PATCH v3 4/6] PCI: dwc: Add dw_pcie_ep_{read,write}_dbi[2] helpers Yoshihiro Shimoda
@ 2023-12-15 9:51 ` Serge Semin
2023-12-15 21:53 ` Krzysztof Wilczyński
2023-12-17 16:32 ` Manivannan Sadhasivam
2023-12-17 16:34 ` Manivannan Sadhasivam
1 sibling, 2 replies; 14+ messages in thread
From: Serge Semin @ 2023-12-15 9:51 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: lpieralisi, kw, robh, bhelgaas, jingoohan1, gustavo.pimentel,
mani, linux-pci, linux-renesas-soc
Hi Yoshihiro
On Fri, Dec 15, 2023 at 11:29:53AM +0900, Yoshihiro Shimoda wrote:
> The current code calculated some dbi[2] registers' offset by calling
> dw_pcie_ep_get_dbi[2]_offset() in each function. To improve code
> readability, add dw_pcie_ep_{read,write}_dbi[2} and some data-width
> related helpers.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Reviewed-by: Serge Semin <fancer.lancer@gmail.com>
> ---
> .../pci/controller/dwc/pcie-designware-ep.c | 184 ++++++------------
> drivers/pci/controller/dwc/pcie-designware.h | 93 +++++++++
> 2 files changed, 153 insertions(+), 124 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 1100671db887..3ab03c0c14c0 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -43,46 +43,19 @@ dw_pcie_ep_get_func_from_ep(struct dw_pcie_ep *ep, u8 func_no)
> return NULL;
> }
>
> -static unsigned int dw_pcie_ep_get_dbi_offset(struct dw_pcie_ep *ep, u8 func_no)
> -{
> - unsigned int dbi_offset = 0;
> -
> - if (ep->ops->get_dbi_offset)
> - dbi_offset = ep->ops->get_dbi_offset(ep, func_no);
> -
> - return dbi_offset;
> -}
> -
> -static unsigned int dw_pcie_ep_get_dbi2_offset(struct dw_pcie_ep *ep, u8 func_no)
> -{
> - unsigned int dbi2_offset = 0;
> -
> - if (ep->ops->get_dbi2_offset)
> - dbi2_offset = ep->ops->get_dbi2_offset(ep, func_no);
> - else if (ep->ops->get_dbi_offset) /* for backward compatibility */
> - dbi2_offset = ep->ops->get_dbi_offset(ep, func_no);
> -
> - return dbi2_offset;
> -}
> -
> static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, u8 func_no,
> enum pci_barno bar, int flags)
> {
> - unsigned int dbi_offset, dbi2_offset;
> struct dw_pcie_ep *ep = &pci->ep;
> - u32 reg, reg_dbi2;
> -
> - dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> - dbi2_offset = dw_pcie_ep_get_dbi2_offset(ep, func_no);
> + u32 reg;
>
> - reg = dbi_offset + PCI_BASE_ADDRESS_0 + (4 * bar);
> - reg_dbi2 = dbi2_offset + PCI_BASE_ADDRESS_0 + (4 * bar);
> + reg = PCI_BASE_ADDRESS_0 + (4 * bar);
> dw_pcie_dbi_ro_wr_en(pci);
> - dw_pcie_writel_dbi2(pci, reg_dbi2, 0x0);
> - dw_pcie_writel_dbi(pci, reg, 0x0);
> + dw_pcie_ep_writel_dbi2(ep, func_no, reg, 0x0);
> + dw_pcie_ep_writel_dbi(ep, func_no, reg, 0x0);
> if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) {
> - dw_pcie_writel_dbi2(pci, reg_dbi2 + 4, 0x0);
> - dw_pcie_writel_dbi(pci, reg + 4, 0x0);
> + dw_pcie_ep_writel_dbi2(ep, func_no, reg + 4, 0x0);
> + dw_pcie_ep_writel_dbi(ep, func_no, reg + 4, 0x0);
> }
> dw_pcie_dbi_ro_wr_dis(pci);
> }
> @@ -99,19 +72,15 @@ void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
> EXPORT_SYMBOL_GPL(dw_pcie_ep_reset_bar);
>
> static u8 __dw_pcie_ep_find_next_cap(struct dw_pcie_ep *ep, u8 func_no,
> - u8 cap_ptr, u8 cap)
> + u8 cap_ptr, u8 cap)
> {
> - struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> - unsigned int dbi_offset = 0;
> u8 cap_id, next_cap_ptr;
> u16 reg;
>
> if (!cap_ptr)
> return 0;
>
> - dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> -
> - reg = dw_pcie_readw_dbi(pci, dbi_offset + cap_ptr);
> + reg = dw_pcie_ep_readw_dbi(ep, func_no, cap_ptr);
> cap_id = (reg & 0x00ff);
>
> if (cap_id > PCI_CAP_ID_MAX)
> @@ -126,14 +95,10 @@ static u8 __dw_pcie_ep_find_next_cap(struct dw_pcie_ep *ep, u8 func_no,
>
> static u8 dw_pcie_ep_find_capability(struct dw_pcie_ep *ep, u8 func_no, u8 cap)
> {
> - struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> - unsigned int dbi_offset = 0;
> u8 next_cap_ptr;
> u16 reg;
>
> - dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> -
> - reg = dw_pcie_readw_dbi(pci, dbi_offset + PCI_CAPABILITY_LIST);
> + reg = dw_pcie_ep_readw_dbi(ep, func_no, PCI_CAPABILITY_LIST);
> next_cap_ptr = (reg & 0x00ff);
>
> return __dw_pcie_ep_find_next_cap(ep, func_no, next_cap_ptr, cap);
> @@ -144,24 +109,21 @@ static int dw_pcie_ep_write_header(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> {
> struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> - unsigned int dbi_offset = 0;
> -
> - dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
>
> dw_pcie_dbi_ro_wr_en(pci);
> - dw_pcie_writew_dbi(pci, dbi_offset + PCI_VENDOR_ID, hdr->vendorid);
> - dw_pcie_writew_dbi(pci, dbi_offset + PCI_DEVICE_ID, hdr->deviceid);
> - dw_pcie_writeb_dbi(pci, dbi_offset + PCI_REVISION_ID, hdr->revid);
> - dw_pcie_writeb_dbi(pci, dbi_offset + PCI_CLASS_PROG, hdr->progif_code);
> - dw_pcie_writew_dbi(pci, dbi_offset + PCI_CLASS_DEVICE,
> - hdr->subclass_code | hdr->baseclass_code << 8);
> - dw_pcie_writeb_dbi(pci, dbi_offset + PCI_CACHE_LINE_SIZE,
> - hdr->cache_line_size);
> - dw_pcie_writew_dbi(pci, dbi_offset + PCI_SUBSYSTEM_VENDOR_ID,
> - hdr->subsys_vendor_id);
> - dw_pcie_writew_dbi(pci, dbi_offset + PCI_SUBSYSTEM_ID, hdr->subsys_id);
> - dw_pcie_writeb_dbi(pci, dbi_offset + PCI_INTERRUPT_PIN,
> - hdr->interrupt_pin);
> + dw_pcie_ep_writew_dbi(ep, func_no, PCI_VENDOR_ID, hdr->vendorid);
> + dw_pcie_ep_writew_dbi(ep, func_no, PCI_DEVICE_ID, hdr->deviceid);
> + dw_pcie_ep_writeb_dbi(ep, func_no, PCI_REVISION_ID, hdr->revid);
> + dw_pcie_ep_writeb_dbi(ep, func_no, PCI_CLASS_PROG, hdr->progif_code);
> + dw_pcie_ep_writew_dbi(ep, func_no, PCI_CLASS_DEVICE,
> + hdr->subclass_code | hdr->baseclass_code << 8);
> + dw_pcie_ep_writeb_dbi(ep, func_no, PCI_CACHE_LINE_SIZE,
> + hdr->cache_line_size);
> + dw_pcie_ep_writew_dbi(ep, func_no, PCI_SUBSYSTEM_VENDOR_ID,
> + hdr->subsys_vendor_id);
> + dw_pcie_ep_writew_dbi(ep, func_no, PCI_SUBSYSTEM_ID, hdr->subsys_id);
> + dw_pcie_ep_writeb_dbi(ep, func_no, PCI_INTERRUPT_PIN,
> + hdr->interrupt_pin);
> dw_pcie_dbi_ro_wr_dis(pci);
>
> return 0;
> @@ -243,18 +205,13 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> {
> struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> - unsigned int dbi_offset, dbi2_offset;
> enum pci_barno bar = epf_bar->barno;
> size_t size = epf_bar->size;
> int flags = epf_bar->flags;
> - u32 reg, reg_dbi2;
> int ret, type;
> + u32 reg;
>
> - dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> - dbi2_offset = dw_pcie_ep_get_dbi2_offset(ep, func_no);
> -
> - reg = PCI_BASE_ADDRESS_0 + (4 * bar) + dbi_offset;
> - reg_dbi2 = PCI_BASE_ADDRESS_0 + (4 * bar) + dbi2_offset;
> + reg = PCI_BASE_ADDRESS_0 + (4 * bar);
>
> if (!(flags & PCI_BASE_ADDRESS_SPACE))
> type = PCIE_ATU_TYPE_MEM;
> @@ -270,12 +227,12 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>
> dw_pcie_dbi_ro_wr_en(pci);
>
> - dw_pcie_writel_dbi2(pci, reg_dbi2, lower_32_bits(size - 1));
> - dw_pcie_writel_dbi(pci, reg, flags);
> + dw_pcie_ep_writel_dbi2(ep, func_no, reg, lower_32_bits(size - 1));
> + dw_pcie_ep_writel_dbi(ep, func_no, reg, flags);
>
> if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) {
> - dw_pcie_writel_dbi2(pci, reg_dbi2 + 4, upper_32_bits(size - 1));
> - dw_pcie_writel_dbi(pci, reg + 4, 0);
> + dw_pcie_ep_writel_dbi2(ep, func_no, reg + 4, upper_32_bits(size - 1));
> + dw_pcie_ep_writel_dbi(ep, func_no, reg + 4, 0);
> }
>
> ep->epf_bar[bar] = epf_bar;
> @@ -335,19 +292,15 @@ static int dw_pcie_ep_map_addr(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> static int dw_pcie_ep_get_msi(struct pci_epc *epc, u8 func_no, u8 vfunc_no)
> {
> struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> - struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> - u32 val, reg;
> - unsigned int dbi_offset = 0;
> struct dw_pcie_ep_func *ep_func;
> + u32 val, reg;
>
> ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
> if (!ep_func || !ep_func->msi_cap)
> return -EINVAL;
>
> - dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> -
> - reg = ep_func->msi_cap + dbi_offset + PCI_MSI_FLAGS;
> - val = dw_pcie_readw_dbi(pci, reg);
> + reg = ep_func->msi_cap + PCI_MSI_FLAGS;
> + val = dw_pcie_ep_readw_dbi(ep, func_no, reg);
> if (!(val & PCI_MSI_FLAGS_ENABLE))
> return -EINVAL;
>
> @@ -361,22 +314,19 @@ static int dw_pcie_ep_set_msi(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> {
> struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> - u32 val, reg;
> - unsigned int dbi_offset = 0;
> struct dw_pcie_ep_func *ep_func;
> + u32 val, reg;
>
> ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
> if (!ep_func || !ep_func->msi_cap)
> return -EINVAL;
>
> - dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> -
> - reg = ep_func->msi_cap + dbi_offset + PCI_MSI_FLAGS;
> - val = dw_pcie_readw_dbi(pci, reg);
> + reg = ep_func->msi_cap + PCI_MSI_FLAGS;
> + val = dw_pcie_ep_readw_dbi(ep, func_no, reg);
> val &= ~PCI_MSI_FLAGS_QMASK;
> val |= FIELD_PREP(PCI_MSI_FLAGS_QMASK, interrupts);
> dw_pcie_dbi_ro_wr_en(pci);
> - dw_pcie_writew_dbi(pci, reg, val);
> + dw_pcie_ep_writew_dbi(ep, func_no, reg, val);
> dw_pcie_dbi_ro_wr_dis(pci);
>
> return 0;
> @@ -385,19 +335,15 @@ static int dw_pcie_ep_set_msi(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> static int dw_pcie_ep_get_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no)
> {
> struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> - struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> - u32 val, reg;
> - unsigned int dbi_offset = 0;
> struct dw_pcie_ep_func *ep_func;
> + u32 val, reg;
>
> ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
> if (!ep_func || !ep_func->msix_cap)
> return -EINVAL;
>
> - dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> -
> - reg = ep_func->msix_cap + dbi_offset + PCI_MSIX_FLAGS;
> - val = dw_pcie_readw_dbi(pci, reg);
> + reg = ep_func->msix_cap + PCI_MSIX_FLAGS;
> + val = dw_pcie_ep_readw_dbi(ep, func_no, reg);
> if (!(val & PCI_MSIX_FLAGS_ENABLE))
> return -EINVAL;
>
> @@ -411,9 +357,8 @@ static int dw_pcie_ep_set_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> {
> struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> - u32 val, reg;
> - unsigned int dbi_offset = 0;
> struct dw_pcie_ep_func *ep_func;
> + u32 val, reg;
>
> ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
> if (!ep_func || !ep_func->msix_cap)
> @@ -421,21 +366,19 @@ static int dw_pcie_ep_set_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>
> dw_pcie_dbi_ro_wr_en(pci);
>
> - dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> -
> - reg = ep_func->msix_cap + dbi_offset + PCI_MSIX_FLAGS;
> - val = dw_pcie_readw_dbi(pci, reg);
> + reg = ep_func->msix_cap + PCI_MSIX_FLAGS;
> + val = dw_pcie_ep_readw_dbi(ep, func_no, reg);
> val &= ~PCI_MSIX_FLAGS_QSIZE;
> val |= interrupts;
> dw_pcie_writew_dbi(pci, reg, val);
>
> - reg = ep_func->msix_cap + dbi_offset + PCI_MSIX_TABLE;
> + reg = ep_func->msix_cap + PCI_MSIX_TABLE;
> val = offset | bir;
> - dw_pcie_writel_dbi(pci, reg, val);
> + dw_pcie_ep_writel_dbi(ep, func_no, reg, val);
>
> - reg = ep_func->msix_cap + dbi_offset + PCI_MSIX_PBA;
> + reg = ep_func->msix_cap + PCI_MSIX_PBA;
> val = (offset + (interrupts * PCI_MSIX_ENTRY_SIZE)) | bir;
> - dw_pcie_writel_dbi(pci, reg, val);
> + dw_pcie_ep_writel_dbi(ep, func_no, reg, val);
>
> dw_pcie_dbi_ro_wr_dis(pci);
>
> @@ -510,38 +453,34 @@ EXPORT_SYMBOL_GPL(dw_pcie_ep_raise_legacy_irq);
> int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
> u8 interrupt_num)
> {
> - struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> + u32 msg_addr_lower, msg_addr_upper, reg;
> struct dw_pcie_ep_func *ep_func;
> struct pci_epc *epc = ep->epc;
> unsigned int aligned_offset;
> - unsigned int dbi_offset = 0;
> u16 msg_ctrl, msg_data;
> - u32 msg_addr_lower, msg_addr_upper, reg;
> - u64 msg_addr;
> bool has_upper;
> + u64 msg_addr;
> int ret;
>
> ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
> if (!ep_func || !ep_func->msi_cap)
> return -EINVAL;
>
> - dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> -
> /* Raise MSI per the PCI Local Bus Specification Revision 3.0, 6.8.1. */
> - reg = ep_func->msi_cap + dbi_offset + PCI_MSI_FLAGS;
> - msg_ctrl = dw_pcie_readw_dbi(pci, reg);
> + reg = ep_func->msi_cap + PCI_MSI_FLAGS;
> + msg_ctrl = dw_pcie_ep_readw_dbi(ep, func_no, reg);
> has_upper = !!(msg_ctrl & PCI_MSI_FLAGS_64BIT);
> - reg = ep_func->msi_cap + dbi_offset + PCI_MSI_ADDRESS_LO;
> - msg_addr_lower = dw_pcie_readl_dbi(pci, reg);
> + reg = ep_func->msi_cap + PCI_MSI_ADDRESS_LO;
> + msg_addr_lower = dw_pcie_ep_readl_dbi(ep, func_no, reg);
> if (has_upper) {
> - reg = ep_func->msi_cap + dbi_offset + PCI_MSI_ADDRESS_HI;
> - msg_addr_upper = dw_pcie_readl_dbi(pci, reg);
> - reg = ep_func->msi_cap + dbi_offset + PCI_MSI_DATA_64;
> - msg_data = dw_pcie_readw_dbi(pci, reg);
> + reg = ep_func->msi_cap + PCI_MSI_ADDRESS_HI;
> + msg_addr_upper = dw_pcie_ep_readl_dbi(ep, func_no, reg);
> + reg = ep_func->msi_cap + PCI_MSI_DATA_64;
> + msg_data = dw_pcie_ep_readw_dbi(ep, func_no, reg);
> } else {
> msg_addr_upper = 0;
> - reg = ep_func->msi_cap + dbi_offset + PCI_MSI_DATA_32;
> - msg_data = dw_pcie_readw_dbi(pci, reg);
> + reg = ep_func->msi_cap + PCI_MSI_DATA_32;
> + msg_data = dw_pcie_ep_readw_dbi(ep, func_no, reg);
> }
> aligned_offset = msg_addr_lower & (epc->mem->window.page_size - 1);
> msg_addr = ((u64)msg_addr_upper) << 32 |
> @@ -582,10 +521,9 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
> u16 interrupt_num)
> {
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> - struct dw_pcie_ep_func *ep_func;
> struct pci_epf_msix_tbl *msix_tbl;
> + struct dw_pcie_ep_func *ep_func;
> struct pci_epc *epc = ep->epc;
> - unsigned int dbi_offset = 0;
> u32 reg, msg_data, vec_ctrl;
> unsigned int aligned_offset;
> u32 tbl_offset;
> @@ -597,10 +535,8 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
> if (!ep_func || !ep_func->msix_cap)
> return -EINVAL;
>
> - dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> -
> - reg = ep_func->msix_cap + dbi_offset + PCI_MSIX_TABLE;
> - tbl_offset = dw_pcie_readl_dbi(pci, reg);
> + reg = ep_func->msix_cap + PCI_MSIX_TABLE;
> + tbl_offset = dw_pcie_ep_readl_dbi(ep, func_no, reg);
> bir = FIELD_GET(PCI_MSIX_TABLE_BIR, tbl_offset);
> tbl_offset &= PCI_MSIX_TABLE_OFFSET;
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 5e36da166ffe..b92e69041fe8 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -534,6 +534,99 @@ static inline enum dw_pcie_ltssm dw_pcie_get_ltssm(struct dw_pcie *pci)
> return (enum dw_pcie_ltssm)FIELD_GET(PORT_LOGIC_LTSSM_STATE_MASK, val);
> }
>
> +static inline unsigned int dw_pcie_ep_get_dbi_offset(struct dw_pcie_ep *ep,
> + u8 func_no)
> +{
> + unsigned int dbi_offset = 0;
> +
> + if (ep->ops->get_dbi_offset)
> + dbi_offset = ep->ops->get_dbi_offset(ep, func_no);
> +
> + return dbi_offset;
> +}
> +
> +static inline unsigned int dw_pcie_ep_get_dbi2_offset(struct dw_pcie_ep *ep,
> + u8 func_no)
> +{
> + unsigned int dbi2_offset = 0;
> +
> + if (ep->ops->get_dbi2_offset)
> + dbi2_offset = ep->ops->get_dbi2_offset(ep, func_no);
> + else if (ep->ops->get_dbi_offset) /* for backward compatibility */
> + dbi2_offset = ep->ops->get_dbi_offset(ep, func_no);
> +
> + return dbi2_offset;
> +}
> +
> +static inline u32 dw_pcie_ep_read_dbi(struct dw_pcie_ep *ep, u8 func_no,
> + u32 reg, size_t size)
> +{
> + unsigned int offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> +
> + return dw_pcie_read_dbi(pci, offset + reg, size);
> +}
> +
> +static inline void dw_pcie_ep_write_dbi(struct dw_pcie_ep *ep, u8 func_no,
> + u32 reg, size_t size, u32 val)
> +{
> + unsigned int offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> +
> + dw_pcie_write_dbi(pci, offset + reg, size, val);
> +}
> +
> +static inline void dw_pcie_ep_write_dbi2(struct dw_pcie_ep *ep, u8 func_no,
> + u32 reg, size_t size, u32 val)
> +{
> + unsigned int offset = dw_pcie_ep_get_dbi2_offset(ep, func_no);
> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> +
> + dw_pcie_write_dbi2(pci, offset + reg, size, val);
> +}
> +
> +static inline void dw_pcie_ep_writel_dbi(struct dw_pcie_ep *ep, u8 func_no,
> + u32 reg, u32 val)
> +{
> + dw_pcie_ep_write_dbi(ep, func_no, reg, 0x4, val);
> +}
> +
> +static inline u32 dw_pcie_ep_readl_dbi(struct dw_pcie_ep *ep, u8 func_no,
> + u32 reg)
> +{
> + return dw_pcie_ep_read_dbi(ep, func_no, reg, 0x4);
> +}
> +
> +static inline void dw_pcie_ep_writew_dbi(struct dw_pcie_ep *ep, u8 func_no,
> + u32 reg, u16 val)
> +{
> + dw_pcie_ep_write_dbi(ep, func_no, reg, 0x2, val);
> +}
> +
> +static inline u16 dw_pcie_ep_readw_dbi(struct dw_pcie_ep *ep, u8 func_no,
> + u32 reg)
> +{
> + return dw_pcie_ep_read_dbi(ep, func_no, reg, 0x2);
> +}
> +
> +static inline void dw_pcie_ep_writeb_dbi(struct dw_pcie_ep *ep, u8 func_no,
> + u32 reg, u8 val)
> +{
> + dw_pcie_ep_write_dbi(ep, func_no, reg, 0x1, val);
> +}
> +
> +static inline u8 dw_pcie_ep_readb_dbi(struct dw_pcie_ep *ep, u8 func_no,
> + u32 reg)
> +{
> + return dw_pcie_ep_read_dbi(ep, func_no, reg, 0x1);
> +}
> +
> +static inline void dw_pcie_ep_writel_dbi2(struct dw_pcie_ep *ep, u8 func_no,
> + u32 reg, u32 val)
> +{
> + dw_pcie_ep_write_dbi2(ep, func_no, reg, 0x4, val);
> +}
> +
A tiny nitpick. Since these are CSRs accessors it would be better for
readability to have them grouped with the rest of the IO-accessors
dw_pcie_writel_dbi()..dw_pcie_writel_dbi2(). Particularly have them
defined below the already available ones. So first normal
DBI-accessors would be placed and the EP-specific DBI-accessors
afterwords. Not sure whether it's that much required. So it's up to
Mani to decide. Perhaps the subsystem maintainers could fix it on
merge in? Bjorn, Krzysztof, Lorenzo?
-Serge(y)
> #ifdef CONFIG_PCIE_DW_HOST
> irqreturn_t dw_handle_msi_irq(struct dw_pcie_rp *pp);
> int dw_pcie_setup_rc(struct dw_pcie_rp *pp);
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 4/6] PCI: dwc: Add dw_pcie_ep_{read,write}_dbi[2] helpers
2023-12-15 9:51 ` Serge Semin
@ 2023-12-15 21:53 ` Krzysztof Wilczyński
2023-12-17 16:32 ` Manivannan Sadhasivam
1 sibling, 0 replies; 14+ messages in thread
From: Krzysztof Wilczyński @ 2023-12-15 21:53 UTC (permalink / raw)
To: Serge Semin
Cc: Yoshihiro Shimoda, lpieralisi, robh, bhelgaas, jingoohan1,
gustavo.pimentel, mani, linux-pci, linux-renesas-soc
Hello,
[...]
> > The current code calculated some dbi[2] registers' offset by calling
> > dw_pcie_ep_get_dbi[2]_offset() in each function. To improve code
> > readability, add dw_pcie_ep_{read,write}_dbi[2} and some data-width
> > related helpers.
[...]
> > +static inline unsigned int dw_pcie_ep_get_dbi_offset(struct dw_pcie_ep *ep,
> > + u8 func_no)
> > +{
> > + unsigned int dbi_offset = 0;
> > +
> > + if (ep->ops->get_dbi_offset)
> > + dbi_offset = ep->ops->get_dbi_offset(ep, func_no);
> > +
> > + return dbi_offset;
> > +}
> > +
> > +static inline unsigned int dw_pcie_ep_get_dbi2_offset(struct dw_pcie_ep *ep,
> > + u8 func_no)
> > +{
> > + unsigned int dbi2_offset = 0;
> > +
> > + if (ep->ops->get_dbi2_offset)
> > + dbi2_offset = ep->ops->get_dbi2_offset(ep, func_no);
> > + else if (ep->ops->get_dbi_offset) /* for backward compatibility */
> > + dbi2_offset = ep->ops->get_dbi_offset(ep, func_no);
> > +
> > + return dbi2_offset;
> > +}
> > +
> > +static inline u32 dw_pcie_ep_read_dbi(struct dw_pcie_ep *ep, u8 func_no,
> > + u32 reg, size_t size)
> > +{
> > + unsigned int offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > +
> > + return dw_pcie_read_dbi(pci, offset + reg, size);
> > +}
> > +
> > +static inline void dw_pcie_ep_write_dbi(struct dw_pcie_ep *ep, u8 func_no,
> > + u32 reg, size_t size, u32 val)
> > +{
> > + unsigned int offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > +
> > + dw_pcie_write_dbi(pci, offset + reg, size, val);
> > +}
> > +
> > +static inline void dw_pcie_ep_write_dbi2(struct dw_pcie_ep *ep, u8 func_no,
> > + u32 reg, size_t size, u32 val)
> > +{
> > + unsigned int offset = dw_pcie_ep_get_dbi2_offset(ep, func_no);
> > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > +
> > + dw_pcie_write_dbi2(pci, offset + reg, size, val);
> > +}
> > +
> > +static inline void dw_pcie_ep_writel_dbi(struct dw_pcie_ep *ep, u8 func_no,
> > + u32 reg, u32 val)
> > +{
> > + dw_pcie_ep_write_dbi(ep, func_no, reg, 0x4, val);
> > +}
> > +
> > +static inline u32 dw_pcie_ep_readl_dbi(struct dw_pcie_ep *ep, u8 func_no,
> > + u32 reg)
> > +{
> > + return dw_pcie_ep_read_dbi(ep, func_no, reg, 0x4);
> > +}
> > +
> > +static inline void dw_pcie_ep_writew_dbi(struct dw_pcie_ep *ep, u8 func_no,
> > + u32 reg, u16 val)
> > +{
> > + dw_pcie_ep_write_dbi(ep, func_no, reg, 0x2, val);
> > +}
> > +
> > +static inline u16 dw_pcie_ep_readw_dbi(struct dw_pcie_ep *ep, u8 func_no,
> > + u32 reg)
> > +{
> > + return dw_pcie_ep_read_dbi(ep, func_no, reg, 0x2);
> > +}
> > +
> > +static inline void dw_pcie_ep_writeb_dbi(struct dw_pcie_ep *ep, u8 func_no,
> > + u32 reg, u8 val)
> > +{
> > + dw_pcie_ep_write_dbi(ep, func_no, reg, 0x1, val);
> > +}
> > +
> > +static inline u8 dw_pcie_ep_readb_dbi(struct dw_pcie_ep *ep, u8 func_no,
> > + u32 reg)
> > +{
> > + return dw_pcie_ep_read_dbi(ep, func_no, reg, 0x1);
> > +}
> > +
> > +static inline void dw_pcie_ep_writel_dbi2(struct dw_pcie_ep *ep, u8 func_no,
> > + u32 reg, u32 val)
> > +{
> > + dw_pcie_ep_write_dbi2(ep, func_no, reg, 0x4, val);
> > +}
> > +
>
> A tiny nitpick. Since these are CSRs accessors it would be better for
> readability to have them grouped with the rest of the IO-accessors
> dw_pcie_writel_dbi()..dw_pcie_writel_dbi2(). Particularly have them
> defined below the already available ones. So first normal
> DBI-accessors would be placed and the EP-specific DBI-accessors
> afterwords. Not sure whether it's that much required. So it's up to
> Mani to decide. Perhaps the subsystem maintainers could fix it on
> merge in? Bjorn, Krzysztof, Lorenzo?
Yes, I can change order after I pull this series. Not to worry.
Krzysztof
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 4/6] PCI: dwc: Add dw_pcie_ep_{read,write}_dbi[2] helpers
2023-12-15 9:51 ` Serge Semin
2023-12-15 21:53 ` Krzysztof Wilczyński
@ 2023-12-17 16:32 ` Manivannan Sadhasivam
2023-12-19 0:21 ` Yoshihiro Shimoda
1 sibling, 1 reply; 14+ messages in thread
From: Manivannan Sadhasivam @ 2023-12-17 16:32 UTC (permalink / raw)
To: Serge Semin
Cc: Yoshihiro Shimoda, lpieralisi, kw, robh, bhelgaas, jingoohan1,
gustavo.pimentel, mani, linux-pci, linux-renesas-soc
On Fri, Dec 15, 2023 at 12:51:28PM +0300, Serge Semin wrote:
> Hi Yoshihiro
>
> On Fri, Dec 15, 2023 at 11:29:53AM +0900, Yoshihiro Shimoda wrote:
> > The current code calculated some dbi[2] registers' offset by calling
> > dw_pcie_ep_get_dbi[2]_offset() in each function. To improve code
> > readability, add dw_pcie_ep_{read,write}_dbi[2} and some data-width
> > related helpers.
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > Reviewed-by: Serge Semin <fancer.lancer@gmail.com>
> > ---
> > .../pci/controller/dwc/pcie-designware-ep.c | 184 ++++++------------
> > drivers/pci/controller/dwc/pcie-designware.h | 93 +++++++++
> > 2 files changed, 153 insertions(+), 124 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > index 1100671db887..3ab03c0c14c0 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > @@ -43,46 +43,19 @@ dw_pcie_ep_get_func_from_ep(struct dw_pcie_ep *ep, u8 func_no)
> > return NULL;
> > }
> >
> > -static unsigned int dw_pcie_ep_get_dbi_offset(struct dw_pcie_ep *ep, u8 func_no)
> > -{
> > - unsigned int dbi_offset = 0;
> > -
> > - if (ep->ops->get_dbi_offset)
> > - dbi_offset = ep->ops->get_dbi_offset(ep, func_no);
> > -
> > - return dbi_offset;
> > -}
> > -
> > -static unsigned int dw_pcie_ep_get_dbi2_offset(struct dw_pcie_ep *ep, u8 func_no)
> > -{
> > - unsigned int dbi2_offset = 0;
> > -
> > - if (ep->ops->get_dbi2_offset)
> > - dbi2_offset = ep->ops->get_dbi2_offset(ep, func_no);
> > - else if (ep->ops->get_dbi_offset) /* for backward compatibility */
> > - dbi2_offset = ep->ops->get_dbi_offset(ep, func_no);
> > -
> > - return dbi2_offset;
> > -}
> > -
> > static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, u8 func_no,
> > enum pci_barno bar, int flags)
> > {
> > - unsigned int dbi_offset, dbi2_offset;
> > struct dw_pcie_ep *ep = &pci->ep;
> > - u32 reg, reg_dbi2;
> > -
> > - dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> > - dbi2_offset = dw_pcie_ep_get_dbi2_offset(ep, func_no);
> > + u32 reg;
> >
> > - reg = dbi_offset + PCI_BASE_ADDRESS_0 + (4 * bar);
> > - reg_dbi2 = dbi2_offset + PCI_BASE_ADDRESS_0 + (4 * bar);
> > + reg = PCI_BASE_ADDRESS_0 + (4 * bar);
> > dw_pcie_dbi_ro_wr_en(pci);
> > - dw_pcie_writel_dbi2(pci, reg_dbi2, 0x0);
> > - dw_pcie_writel_dbi(pci, reg, 0x0);
> > + dw_pcie_ep_writel_dbi2(ep, func_no, reg, 0x0);
> > + dw_pcie_ep_writel_dbi(ep, func_no, reg, 0x0);
> > if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) {
> > - dw_pcie_writel_dbi2(pci, reg_dbi2 + 4, 0x0);
> > - dw_pcie_writel_dbi(pci, reg + 4, 0x0);
> > + dw_pcie_ep_writel_dbi2(ep, func_no, reg + 4, 0x0);
> > + dw_pcie_ep_writel_dbi(ep, func_no, reg + 4, 0x0);
> > }
> > dw_pcie_dbi_ro_wr_dis(pci);
> > }
> > @@ -99,19 +72,15 @@ void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
> > EXPORT_SYMBOL_GPL(dw_pcie_ep_reset_bar);
> >
> > static u8 __dw_pcie_ep_find_next_cap(struct dw_pcie_ep *ep, u8 func_no,
> > - u8 cap_ptr, u8 cap)
> > + u8 cap_ptr, u8 cap)
> > {
> > - struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > - unsigned int dbi_offset = 0;
> > u8 cap_id, next_cap_ptr;
> > u16 reg;
> >
> > if (!cap_ptr)
> > return 0;
> >
> > - dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> > -
> > - reg = dw_pcie_readw_dbi(pci, dbi_offset + cap_ptr);
> > + reg = dw_pcie_ep_readw_dbi(ep, func_no, cap_ptr);
> > cap_id = (reg & 0x00ff);
> >
> > if (cap_id > PCI_CAP_ID_MAX)
> > @@ -126,14 +95,10 @@ static u8 __dw_pcie_ep_find_next_cap(struct dw_pcie_ep *ep, u8 func_no,
> >
> > static u8 dw_pcie_ep_find_capability(struct dw_pcie_ep *ep, u8 func_no, u8 cap)
> > {
> > - struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > - unsigned int dbi_offset = 0;
> > u8 next_cap_ptr;
> > u16 reg;
> >
> > - dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> > -
> > - reg = dw_pcie_readw_dbi(pci, dbi_offset + PCI_CAPABILITY_LIST);
> > + reg = dw_pcie_ep_readw_dbi(ep, func_no, PCI_CAPABILITY_LIST);
> > next_cap_ptr = (reg & 0x00ff);
> >
> > return __dw_pcie_ep_find_next_cap(ep, func_no, next_cap_ptr, cap);
> > @@ -144,24 +109,21 @@ static int dw_pcie_ep_write_header(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> > {
> > struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> > struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > - unsigned int dbi_offset = 0;
> > -
> > - dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> >
> > dw_pcie_dbi_ro_wr_en(pci);
> > - dw_pcie_writew_dbi(pci, dbi_offset + PCI_VENDOR_ID, hdr->vendorid);
> > - dw_pcie_writew_dbi(pci, dbi_offset + PCI_DEVICE_ID, hdr->deviceid);
> > - dw_pcie_writeb_dbi(pci, dbi_offset + PCI_REVISION_ID, hdr->revid);
> > - dw_pcie_writeb_dbi(pci, dbi_offset + PCI_CLASS_PROG, hdr->progif_code);
> > - dw_pcie_writew_dbi(pci, dbi_offset + PCI_CLASS_DEVICE,
> > - hdr->subclass_code | hdr->baseclass_code << 8);
> > - dw_pcie_writeb_dbi(pci, dbi_offset + PCI_CACHE_LINE_SIZE,
> > - hdr->cache_line_size);
> > - dw_pcie_writew_dbi(pci, dbi_offset + PCI_SUBSYSTEM_VENDOR_ID,
> > - hdr->subsys_vendor_id);
> > - dw_pcie_writew_dbi(pci, dbi_offset + PCI_SUBSYSTEM_ID, hdr->subsys_id);
> > - dw_pcie_writeb_dbi(pci, dbi_offset + PCI_INTERRUPT_PIN,
> > - hdr->interrupt_pin);
> > + dw_pcie_ep_writew_dbi(ep, func_no, PCI_VENDOR_ID, hdr->vendorid);
> > + dw_pcie_ep_writew_dbi(ep, func_no, PCI_DEVICE_ID, hdr->deviceid);
> > + dw_pcie_ep_writeb_dbi(ep, func_no, PCI_REVISION_ID, hdr->revid);
> > + dw_pcie_ep_writeb_dbi(ep, func_no, PCI_CLASS_PROG, hdr->progif_code);
> > + dw_pcie_ep_writew_dbi(ep, func_no, PCI_CLASS_DEVICE,
> > + hdr->subclass_code | hdr->baseclass_code << 8);
> > + dw_pcie_ep_writeb_dbi(ep, func_no, PCI_CACHE_LINE_SIZE,
> > + hdr->cache_line_size);
> > + dw_pcie_ep_writew_dbi(ep, func_no, PCI_SUBSYSTEM_VENDOR_ID,
> > + hdr->subsys_vendor_id);
> > + dw_pcie_ep_writew_dbi(ep, func_no, PCI_SUBSYSTEM_ID, hdr->subsys_id);
> > + dw_pcie_ep_writeb_dbi(ep, func_no, PCI_INTERRUPT_PIN,
> > + hdr->interrupt_pin);
> > dw_pcie_dbi_ro_wr_dis(pci);
> >
> > return 0;
> > @@ -243,18 +205,13 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> > {
> > struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> > struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > - unsigned int dbi_offset, dbi2_offset;
> > enum pci_barno bar = epf_bar->barno;
> > size_t size = epf_bar->size;
> > int flags = epf_bar->flags;
> > - u32 reg, reg_dbi2;
> > int ret, type;
> > + u32 reg;
> >
> > - dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> > - dbi2_offset = dw_pcie_ep_get_dbi2_offset(ep, func_no);
> > -
> > - reg = PCI_BASE_ADDRESS_0 + (4 * bar) + dbi_offset;
> > - reg_dbi2 = PCI_BASE_ADDRESS_0 + (4 * bar) + dbi2_offset;
> > + reg = PCI_BASE_ADDRESS_0 + (4 * bar);
> >
> > if (!(flags & PCI_BASE_ADDRESS_SPACE))
> > type = PCIE_ATU_TYPE_MEM;
> > @@ -270,12 +227,12 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> >
> > dw_pcie_dbi_ro_wr_en(pci);
> >
> > - dw_pcie_writel_dbi2(pci, reg_dbi2, lower_32_bits(size - 1));
> > - dw_pcie_writel_dbi(pci, reg, flags);
> > + dw_pcie_ep_writel_dbi2(ep, func_no, reg, lower_32_bits(size - 1));
> > + dw_pcie_ep_writel_dbi(ep, func_no, reg, flags);
> >
> > if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) {
> > - dw_pcie_writel_dbi2(pci, reg_dbi2 + 4, upper_32_bits(size - 1));
> > - dw_pcie_writel_dbi(pci, reg + 4, 0);
> > + dw_pcie_ep_writel_dbi2(ep, func_no, reg + 4, upper_32_bits(size - 1));
> > + dw_pcie_ep_writel_dbi(ep, func_no, reg + 4, 0);
> > }
> >
> > ep->epf_bar[bar] = epf_bar;
> > @@ -335,19 +292,15 @@ static int dw_pcie_ep_map_addr(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> > static int dw_pcie_ep_get_msi(struct pci_epc *epc, u8 func_no, u8 vfunc_no)
> > {
> > struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> > - struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > - u32 val, reg;
> > - unsigned int dbi_offset = 0;
> > struct dw_pcie_ep_func *ep_func;
> > + u32 val, reg;
> >
> > ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
> > if (!ep_func || !ep_func->msi_cap)
> > return -EINVAL;
> >
> > - dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> > -
> > - reg = ep_func->msi_cap + dbi_offset + PCI_MSI_FLAGS;
> > - val = dw_pcie_readw_dbi(pci, reg);
> > + reg = ep_func->msi_cap + PCI_MSI_FLAGS;
> > + val = dw_pcie_ep_readw_dbi(ep, func_no, reg);
> > if (!(val & PCI_MSI_FLAGS_ENABLE))
> > return -EINVAL;
> >
> > @@ -361,22 +314,19 @@ static int dw_pcie_ep_set_msi(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> > {
> > struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> > struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > - u32 val, reg;
> > - unsigned int dbi_offset = 0;
> > struct dw_pcie_ep_func *ep_func;
> > + u32 val, reg;
> >
> > ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
> > if (!ep_func || !ep_func->msi_cap)
> > return -EINVAL;
> >
> > - dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> > -
> > - reg = ep_func->msi_cap + dbi_offset + PCI_MSI_FLAGS;
> > - val = dw_pcie_readw_dbi(pci, reg);
> > + reg = ep_func->msi_cap + PCI_MSI_FLAGS;
> > + val = dw_pcie_ep_readw_dbi(ep, func_no, reg);
> > val &= ~PCI_MSI_FLAGS_QMASK;
> > val |= FIELD_PREP(PCI_MSI_FLAGS_QMASK, interrupts);
> > dw_pcie_dbi_ro_wr_en(pci);
> > - dw_pcie_writew_dbi(pci, reg, val);
> > + dw_pcie_ep_writew_dbi(ep, func_no, reg, val);
> > dw_pcie_dbi_ro_wr_dis(pci);
> >
> > return 0;
> > @@ -385,19 +335,15 @@ static int dw_pcie_ep_set_msi(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> > static int dw_pcie_ep_get_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no)
> > {
> > struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> > - struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > - u32 val, reg;
> > - unsigned int dbi_offset = 0;
> > struct dw_pcie_ep_func *ep_func;
> > + u32 val, reg;
> >
> > ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
> > if (!ep_func || !ep_func->msix_cap)
> > return -EINVAL;
> >
> > - dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> > -
> > - reg = ep_func->msix_cap + dbi_offset + PCI_MSIX_FLAGS;
> > - val = dw_pcie_readw_dbi(pci, reg);
> > + reg = ep_func->msix_cap + PCI_MSIX_FLAGS;
> > + val = dw_pcie_ep_readw_dbi(ep, func_no, reg);
> > if (!(val & PCI_MSIX_FLAGS_ENABLE))
> > return -EINVAL;
> >
> > @@ -411,9 +357,8 @@ static int dw_pcie_ep_set_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> > {
> > struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> > struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > - u32 val, reg;
> > - unsigned int dbi_offset = 0;
> > struct dw_pcie_ep_func *ep_func;
> > + u32 val, reg;
> >
> > ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
> > if (!ep_func || !ep_func->msix_cap)
> > @@ -421,21 +366,19 @@ static int dw_pcie_ep_set_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> >
> > dw_pcie_dbi_ro_wr_en(pci);
> >
> > - dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> > -
> > - reg = ep_func->msix_cap + dbi_offset + PCI_MSIX_FLAGS;
> > - val = dw_pcie_readw_dbi(pci, reg);
> > + reg = ep_func->msix_cap + PCI_MSIX_FLAGS;
> > + val = dw_pcie_ep_readw_dbi(ep, func_no, reg);
> > val &= ~PCI_MSIX_FLAGS_QSIZE;
> > val |= interrupts;
> > dw_pcie_writew_dbi(pci, reg, val);
> >
> > - reg = ep_func->msix_cap + dbi_offset + PCI_MSIX_TABLE;
> > + reg = ep_func->msix_cap + PCI_MSIX_TABLE;
> > val = offset | bir;
> > - dw_pcie_writel_dbi(pci, reg, val);
> > + dw_pcie_ep_writel_dbi(ep, func_no, reg, val);
> >
> > - reg = ep_func->msix_cap + dbi_offset + PCI_MSIX_PBA;
> > + reg = ep_func->msix_cap + PCI_MSIX_PBA;
> > val = (offset + (interrupts * PCI_MSIX_ENTRY_SIZE)) | bir;
> > - dw_pcie_writel_dbi(pci, reg, val);
> > + dw_pcie_ep_writel_dbi(ep, func_no, reg, val);
> >
> > dw_pcie_dbi_ro_wr_dis(pci);
> >
> > @@ -510,38 +453,34 @@ EXPORT_SYMBOL_GPL(dw_pcie_ep_raise_legacy_irq);
> > int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
> > u8 interrupt_num)
> > {
> > - struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > + u32 msg_addr_lower, msg_addr_upper, reg;
> > struct dw_pcie_ep_func *ep_func;
> > struct pci_epc *epc = ep->epc;
> > unsigned int aligned_offset;
> > - unsigned int dbi_offset = 0;
> > u16 msg_ctrl, msg_data;
> > - u32 msg_addr_lower, msg_addr_upper, reg;
> > - u64 msg_addr;
> > bool has_upper;
> > + u64 msg_addr;
> > int ret;
> >
> > ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
> > if (!ep_func || !ep_func->msi_cap)
> > return -EINVAL;
> >
> > - dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> > -
> > /* Raise MSI per the PCI Local Bus Specification Revision 3.0, 6.8.1. */
> > - reg = ep_func->msi_cap + dbi_offset + PCI_MSI_FLAGS;
> > - msg_ctrl = dw_pcie_readw_dbi(pci, reg);
> > + reg = ep_func->msi_cap + PCI_MSI_FLAGS;
> > + msg_ctrl = dw_pcie_ep_readw_dbi(ep, func_no, reg);
> > has_upper = !!(msg_ctrl & PCI_MSI_FLAGS_64BIT);
> > - reg = ep_func->msi_cap + dbi_offset + PCI_MSI_ADDRESS_LO;
> > - msg_addr_lower = dw_pcie_readl_dbi(pci, reg);
> > + reg = ep_func->msi_cap + PCI_MSI_ADDRESS_LO;
> > + msg_addr_lower = dw_pcie_ep_readl_dbi(ep, func_no, reg);
> > if (has_upper) {
> > - reg = ep_func->msi_cap + dbi_offset + PCI_MSI_ADDRESS_HI;
> > - msg_addr_upper = dw_pcie_readl_dbi(pci, reg);
> > - reg = ep_func->msi_cap + dbi_offset + PCI_MSI_DATA_64;
> > - msg_data = dw_pcie_readw_dbi(pci, reg);
> > + reg = ep_func->msi_cap + PCI_MSI_ADDRESS_HI;
> > + msg_addr_upper = dw_pcie_ep_readl_dbi(ep, func_no, reg);
> > + reg = ep_func->msi_cap + PCI_MSI_DATA_64;
> > + msg_data = dw_pcie_ep_readw_dbi(ep, func_no, reg);
> > } else {
> > msg_addr_upper = 0;
> > - reg = ep_func->msi_cap + dbi_offset + PCI_MSI_DATA_32;
> > - msg_data = dw_pcie_readw_dbi(pci, reg);
> > + reg = ep_func->msi_cap + PCI_MSI_DATA_32;
> > + msg_data = dw_pcie_ep_readw_dbi(ep, func_no, reg);
> > }
> > aligned_offset = msg_addr_lower & (epc->mem->window.page_size - 1);
> > msg_addr = ((u64)msg_addr_upper) << 32 |
> > @@ -582,10 +521,9 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
> > u16 interrupt_num)
> > {
> > struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > - struct dw_pcie_ep_func *ep_func;
> > struct pci_epf_msix_tbl *msix_tbl;
> > + struct dw_pcie_ep_func *ep_func;
> > struct pci_epc *epc = ep->epc;
> > - unsigned int dbi_offset = 0;
> > u32 reg, msg_data, vec_ctrl;
> > unsigned int aligned_offset;
> > u32 tbl_offset;
> > @@ -597,10 +535,8 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
> > if (!ep_func || !ep_func->msix_cap)
> > return -EINVAL;
> >
> > - dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> > -
> > - reg = ep_func->msix_cap + dbi_offset + PCI_MSIX_TABLE;
> > - tbl_offset = dw_pcie_readl_dbi(pci, reg);
> > + reg = ep_func->msix_cap + PCI_MSIX_TABLE;
> > + tbl_offset = dw_pcie_ep_readl_dbi(ep, func_no, reg);
> > bir = FIELD_GET(PCI_MSIX_TABLE_BIR, tbl_offset);
> > tbl_offset &= PCI_MSIX_TABLE_OFFSET;
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> > index 5e36da166ffe..b92e69041fe8 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > @@ -534,6 +534,99 @@ static inline enum dw_pcie_ltssm dw_pcie_get_ltssm(struct dw_pcie *pci)
> > return (enum dw_pcie_ltssm)FIELD_GET(PORT_LOGIC_LTSSM_STATE_MASK, val);
> > }
> >
>
> > +static inline unsigned int dw_pcie_ep_get_dbi_offset(struct dw_pcie_ep *ep,
> > + u8 func_no)
> > +{
> > + unsigned int dbi_offset = 0;
> > +
> > + if (ep->ops->get_dbi_offset)
> > + dbi_offset = ep->ops->get_dbi_offset(ep, func_no);
> > +
> > + return dbi_offset;
> > +}
> > +
> > +static inline unsigned int dw_pcie_ep_get_dbi2_offset(struct dw_pcie_ep *ep,
> > + u8 func_no)
> > +{
> > + unsigned int dbi2_offset = 0;
> > +
> > + if (ep->ops->get_dbi2_offset)
> > + dbi2_offset = ep->ops->get_dbi2_offset(ep, func_no);
> > + else if (ep->ops->get_dbi_offset) /* for backward compatibility */
> > + dbi2_offset = ep->ops->get_dbi_offset(ep, func_no);
> > +
> > + return dbi2_offset;
> > +}
> > +
> > +static inline u32 dw_pcie_ep_read_dbi(struct dw_pcie_ep *ep, u8 func_no,
> > + u32 reg, size_t size)
> > +{
> > + unsigned int offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > +
> > + return dw_pcie_read_dbi(pci, offset + reg, size);
> > +}
> > +
> > +static inline void dw_pcie_ep_write_dbi(struct dw_pcie_ep *ep, u8 func_no,
> > + u32 reg, size_t size, u32 val)
> > +{
> > + unsigned int offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > +
> > + dw_pcie_write_dbi(pci, offset + reg, size, val);
> > +}
> > +
> > +static inline void dw_pcie_ep_write_dbi2(struct dw_pcie_ep *ep, u8 func_no,
> > + u32 reg, size_t size, u32 val)
> > +{
> > + unsigned int offset = dw_pcie_ep_get_dbi2_offset(ep, func_no);
> > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > +
> > + dw_pcie_write_dbi2(pci, offset + reg, size, val);
> > +}
> > +
> > +static inline void dw_pcie_ep_writel_dbi(struct dw_pcie_ep *ep, u8 func_no,
> > + u32 reg, u32 val)
> > +{
> > + dw_pcie_ep_write_dbi(ep, func_no, reg, 0x4, val);
> > +}
> > +
> > +static inline u32 dw_pcie_ep_readl_dbi(struct dw_pcie_ep *ep, u8 func_no,
> > + u32 reg)
> > +{
> > + return dw_pcie_ep_read_dbi(ep, func_no, reg, 0x4);
> > +}
> > +
> > +static inline void dw_pcie_ep_writew_dbi(struct dw_pcie_ep *ep, u8 func_no,
> > + u32 reg, u16 val)
> > +{
> > + dw_pcie_ep_write_dbi(ep, func_no, reg, 0x2, val);
> > +}
> > +
> > +static inline u16 dw_pcie_ep_readw_dbi(struct dw_pcie_ep *ep, u8 func_no,
> > + u32 reg)
> > +{
> > + return dw_pcie_ep_read_dbi(ep, func_no, reg, 0x2);
> > +}
> > +
> > +static inline void dw_pcie_ep_writeb_dbi(struct dw_pcie_ep *ep, u8 func_no,
> > + u32 reg, u8 val)
> > +{
> > + dw_pcie_ep_write_dbi(ep, func_no, reg, 0x1, val);
> > +}
> > +
> > +static inline u8 dw_pcie_ep_readb_dbi(struct dw_pcie_ep *ep, u8 func_no,
> > + u32 reg)
> > +{
> > + return dw_pcie_ep_read_dbi(ep, func_no, reg, 0x1);
> > +}
> > +
> > +static inline void dw_pcie_ep_writel_dbi2(struct dw_pcie_ep *ep, u8 func_no,
> > + u32 reg, u32 val)
> > +{
> > + dw_pcie_ep_write_dbi2(ep, func_no, reg, 0x4, val);
> > +}
> > +
>
> A tiny nitpick. Since these are CSRs accessors it would be better for
> readability to have them grouped with the rest of the IO-accessors
> dw_pcie_writel_dbi()..dw_pcie_writel_dbi2(). Particularly have them
> defined below the already available ones. So first normal
> DBI-accessors would be placed and the EP-specific DBI-accessors
> afterwords. Not sure whether it's that much required. So it's up to
> Mani to decide. Perhaps the subsystem maintainers could fix it on
> merge in? Bjorn, Krzysztof, Lorenzo?
>
+1
- Mani
> -Serge(y)
>
> > #ifdef CONFIG_PCIE_DW_HOST
> > irqreturn_t dw_handle_msi_irq(struct dw_pcie_rp *pp);
> > int dw_pcie_setup_rc(struct dw_pcie_rp *pp);
> > --
> > 2.34.1
> >
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 4/6] PCI: dwc: Add dw_pcie_ep_{read,write}_dbi[2] helpers
2023-12-15 2:29 ` [PATCH v3 4/6] PCI: dwc: Add dw_pcie_ep_{read,write}_dbi[2] helpers Yoshihiro Shimoda
2023-12-15 9:51 ` Serge Semin
@ 2023-12-17 16:34 ` Manivannan Sadhasivam
1 sibling, 0 replies; 14+ messages in thread
From: Manivannan Sadhasivam @ 2023-12-17 16:34 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: lpieralisi, kw, robh, bhelgaas, jingoohan1, gustavo.pimentel,
linux-pci, linux-renesas-soc, Serge Semin
On Fri, Dec 15, 2023 at 11:29:53AM +0900, Yoshihiro Shimoda wrote:
> The current code calculated some dbi[2] registers' offset by calling
> dw_pcie_ep_get_dbi[2]_offset() in each function. To improve code
> readability, add dw_pcie_ep_{read,write}_dbi[2} and some data-width
> related helpers.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
- Mani
> Reviewed-by: Serge Semin <fancer.lancer@gmail.com>
> ---
> .../pci/controller/dwc/pcie-designware-ep.c | 184 ++++++------------
> drivers/pci/controller/dwc/pcie-designware.h | 93 +++++++++
> 2 files changed, 153 insertions(+), 124 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 1100671db887..3ab03c0c14c0 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -43,46 +43,19 @@ dw_pcie_ep_get_func_from_ep(struct dw_pcie_ep *ep, u8 func_no)
> return NULL;
> }
>
> -static unsigned int dw_pcie_ep_get_dbi_offset(struct dw_pcie_ep *ep, u8 func_no)
> -{
> - unsigned int dbi_offset = 0;
> -
> - if (ep->ops->get_dbi_offset)
> - dbi_offset = ep->ops->get_dbi_offset(ep, func_no);
> -
> - return dbi_offset;
> -}
> -
> -static unsigned int dw_pcie_ep_get_dbi2_offset(struct dw_pcie_ep *ep, u8 func_no)
> -{
> - unsigned int dbi2_offset = 0;
> -
> - if (ep->ops->get_dbi2_offset)
> - dbi2_offset = ep->ops->get_dbi2_offset(ep, func_no);
> - else if (ep->ops->get_dbi_offset) /* for backward compatibility */
> - dbi2_offset = ep->ops->get_dbi_offset(ep, func_no);
> -
> - return dbi2_offset;
> -}
> -
> static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, u8 func_no,
> enum pci_barno bar, int flags)
> {
> - unsigned int dbi_offset, dbi2_offset;
> struct dw_pcie_ep *ep = &pci->ep;
> - u32 reg, reg_dbi2;
> -
> - dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> - dbi2_offset = dw_pcie_ep_get_dbi2_offset(ep, func_no);
> + u32 reg;
>
> - reg = dbi_offset + PCI_BASE_ADDRESS_0 + (4 * bar);
> - reg_dbi2 = dbi2_offset + PCI_BASE_ADDRESS_0 + (4 * bar);
> + reg = PCI_BASE_ADDRESS_0 + (4 * bar);
> dw_pcie_dbi_ro_wr_en(pci);
> - dw_pcie_writel_dbi2(pci, reg_dbi2, 0x0);
> - dw_pcie_writel_dbi(pci, reg, 0x0);
> + dw_pcie_ep_writel_dbi2(ep, func_no, reg, 0x0);
> + dw_pcie_ep_writel_dbi(ep, func_no, reg, 0x0);
> if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) {
> - dw_pcie_writel_dbi2(pci, reg_dbi2 + 4, 0x0);
> - dw_pcie_writel_dbi(pci, reg + 4, 0x0);
> + dw_pcie_ep_writel_dbi2(ep, func_no, reg + 4, 0x0);
> + dw_pcie_ep_writel_dbi(ep, func_no, reg + 4, 0x0);
> }
> dw_pcie_dbi_ro_wr_dis(pci);
> }
> @@ -99,19 +72,15 @@ void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
> EXPORT_SYMBOL_GPL(dw_pcie_ep_reset_bar);
>
> static u8 __dw_pcie_ep_find_next_cap(struct dw_pcie_ep *ep, u8 func_no,
> - u8 cap_ptr, u8 cap)
> + u8 cap_ptr, u8 cap)
> {
> - struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> - unsigned int dbi_offset = 0;
> u8 cap_id, next_cap_ptr;
> u16 reg;
>
> if (!cap_ptr)
> return 0;
>
> - dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> -
> - reg = dw_pcie_readw_dbi(pci, dbi_offset + cap_ptr);
> + reg = dw_pcie_ep_readw_dbi(ep, func_no, cap_ptr);
> cap_id = (reg & 0x00ff);
>
> if (cap_id > PCI_CAP_ID_MAX)
> @@ -126,14 +95,10 @@ static u8 __dw_pcie_ep_find_next_cap(struct dw_pcie_ep *ep, u8 func_no,
>
> static u8 dw_pcie_ep_find_capability(struct dw_pcie_ep *ep, u8 func_no, u8 cap)
> {
> - struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> - unsigned int dbi_offset = 0;
> u8 next_cap_ptr;
> u16 reg;
>
> - dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> -
> - reg = dw_pcie_readw_dbi(pci, dbi_offset + PCI_CAPABILITY_LIST);
> + reg = dw_pcie_ep_readw_dbi(ep, func_no, PCI_CAPABILITY_LIST);
> next_cap_ptr = (reg & 0x00ff);
>
> return __dw_pcie_ep_find_next_cap(ep, func_no, next_cap_ptr, cap);
> @@ -144,24 +109,21 @@ static int dw_pcie_ep_write_header(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> {
> struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> - unsigned int dbi_offset = 0;
> -
> - dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
>
> dw_pcie_dbi_ro_wr_en(pci);
> - dw_pcie_writew_dbi(pci, dbi_offset + PCI_VENDOR_ID, hdr->vendorid);
> - dw_pcie_writew_dbi(pci, dbi_offset + PCI_DEVICE_ID, hdr->deviceid);
> - dw_pcie_writeb_dbi(pci, dbi_offset + PCI_REVISION_ID, hdr->revid);
> - dw_pcie_writeb_dbi(pci, dbi_offset + PCI_CLASS_PROG, hdr->progif_code);
> - dw_pcie_writew_dbi(pci, dbi_offset + PCI_CLASS_DEVICE,
> - hdr->subclass_code | hdr->baseclass_code << 8);
> - dw_pcie_writeb_dbi(pci, dbi_offset + PCI_CACHE_LINE_SIZE,
> - hdr->cache_line_size);
> - dw_pcie_writew_dbi(pci, dbi_offset + PCI_SUBSYSTEM_VENDOR_ID,
> - hdr->subsys_vendor_id);
> - dw_pcie_writew_dbi(pci, dbi_offset + PCI_SUBSYSTEM_ID, hdr->subsys_id);
> - dw_pcie_writeb_dbi(pci, dbi_offset + PCI_INTERRUPT_PIN,
> - hdr->interrupt_pin);
> + dw_pcie_ep_writew_dbi(ep, func_no, PCI_VENDOR_ID, hdr->vendorid);
> + dw_pcie_ep_writew_dbi(ep, func_no, PCI_DEVICE_ID, hdr->deviceid);
> + dw_pcie_ep_writeb_dbi(ep, func_no, PCI_REVISION_ID, hdr->revid);
> + dw_pcie_ep_writeb_dbi(ep, func_no, PCI_CLASS_PROG, hdr->progif_code);
> + dw_pcie_ep_writew_dbi(ep, func_no, PCI_CLASS_DEVICE,
> + hdr->subclass_code | hdr->baseclass_code << 8);
> + dw_pcie_ep_writeb_dbi(ep, func_no, PCI_CACHE_LINE_SIZE,
> + hdr->cache_line_size);
> + dw_pcie_ep_writew_dbi(ep, func_no, PCI_SUBSYSTEM_VENDOR_ID,
> + hdr->subsys_vendor_id);
> + dw_pcie_ep_writew_dbi(ep, func_no, PCI_SUBSYSTEM_ID, hdr->subsys_id);
> + dw_pcie_ep_writeb_dbi(ep, func_no, PCI_INTERRUPT_PIN,
> + hdr->interrupt_pin);
> dw_pcie_dbi_ro_wr_dis(pci);
>
> return 0;
> @@ -243,18 +205,13 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> {
> struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> - unsigned int dbi_offset, dbi2_offset;
> enum pci_barno bar = epf_bar->barno;
> size_t size = epf_bar->size;
> int flags = epf_bar->flags;
> - u32 reg, reg_dbi2;
> int ret, type;
> + u32 reg;
>
> - dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> - dbi2_offset = dw_pcie_ep_get_dbi2_offset(ep, func_no);
> -
> - reg = PCI_BASE_ADDRESS_0 + (4 * bar) + dbi_offset;
> - reg_dbi2 = PCI_BASE_ADDRESS_0 + (4 * bar) + dbi2_offset;
> + reg = PCI_BASE_ADDRESS_0 + (4 * bar);
>
> if (!(flags & PCI_BASE_ADDRESS_SPACE))
> type = PCIE_ATU_TYPE_MEM;
> @@ -270,12 +227,12 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>
> dw_pcie_dbi_ro_wr_en(pci);
>
> - dw_pcie_writel_dbi2(pci, reg_dbi2, lower_32_bits(size - 1));
> - dw_pcie_writel_dbi(pci, reg, flags);
> + dw_pcie_ep_writel_dbi2(ep, func_no, reg, lower_32_bits(size - 1));
> + dw_pcie_ep_writel_dbi(ep, func_no, reg, flags);
>
> if (flags & PCI_BASE_ADDRESS_MEM_TYPE_64) {
> - dw_pcie_writel_dbi2(pci, reg_dbi2 + 4, upper_32_bits(size - 1));
> - dw_pcie_writel_dbi(pci, reg + 4, 0);
> + dw_pcie_ep_writel_dbi2(ep, func_no, reg + 4, upper_32_bits(size - 1));
> + dw_pcie_ep_writel_dbi(ep, func_no, reg + 4, 0);
> }
>
> ep->epf_bar[bar] = epf_bar;
> @@ -335,19 +292,15 @@ static int dw_pcie_ep_map_addr(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> static int dw_pcie_ep_get_msi(struct pci_epc *epc, u8 func_no, u8 vfunc_no)
> {
> struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> - struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> - u32 val, reg;
> - unsigned int dbi_offset = 0;
> struct dw_pcie_ep_func *ep_func;
> + u32 val, reg;
>
> ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
> if (!ep_func || !ep_func->msi_cap)
> return -EINVAL;
>
> - dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> -
> - reg = ep_func->msi_cap + dbi_offset + PCI_MSI_FLAGS;
> - val = dw_pcie_readw_dbi(pci, reg);
> + reg = ep_func->msi_cap + PCI_MSI_FLAGS;
> + val = dw_pcie_ep_readw_dbi(ep, func_no, reg);
> if (!(val & PCI_MSI_FLAGS_ENABLE))
> return -EINVAL;
>
> @@ -361,22 +314,19 @@ static int dw_pcie_ep_set_msi(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> {
> struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> - u32 val, reg;
> - unsigned int dbi_offset = 0;
> struct dw_pcie_ep_func *ep_func;
> + u32 val, reg;
>
> ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
> if (!ep_func || !ep_func->msi_cap)
> return -EINVAL;
>
> - dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> -
> - reg = ep_func->msi_cap + dbi_offset + PCI_MSI_FLAGS;
> - val = dw_pcie_readw_dbi(pci, reg);
> + reg = ep_func->msi_cap + PCI_MSI_FLAGS;
> + val = dw_pcie_ep_readw_dbi(ep, func_no, reg);
> val &= ~PCI_MSI_FLAGS_QMASK;
> val |= FIELD_PREP(PCI_MSI_FLAGS_QMASK, interrupts);
> dw_pcie_dbi_ro_wr_en(pci);
> - dw_pcie_writew_dbi(pci, reg, val);
> + dw_pcie_ep_writew_dbi(ep, func_no, reg, val);
> dw_pcie_dbi_ro_wr_dis(pci);
>
> return 0;
> @@ -385,19 +335,15 @@ static int dw_pcie_ep_set_msi(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> static int dw_pcie_ep_get_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no)
> {
> struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> - struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> - u32 val, reg;
> - unsigned int dbi_offset = 0;
> struct dw_pcie_ep_func *ep_func;
> + u32 val, reg;
>
> ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
> if (!ep_func || !ep_func->msix_cap)
> return -EINVAL;
>
> - dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> -
> - reg = ep_func->msix_cap + dbi_offset + PCI_MSIX_FLAGS;
> - val = dw_pcie_readw_dbi(pci, reg);
> + reg = ep_func->msix_cap + PCI_MSIX_FLAGS;
> + val = dw_pcie_ep_readw_dbi(ep, func_no, reg);
> if (!(val & PCI_MSIX_FLAGS_ENABLE))
> return -EINVAL;
>
> @@ -411,9 +357,8 @@ static int dw_pcie_ep_set_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> {
> struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> - u32 val, reg;
> - unsigned int dbi_offset = 0;
> struct dw_pcie_ep_func *ep_func;
> + u32 val, reg;
>
> ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
> if (!ep_func || !ep_func->msix_cap)
> @@ -421,21 +366,19 @@ static int dw_pcie_ep_set_msix(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>
> dw_pcie_dbi_ro_wr_en(pci);
>
> - dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> -
> - reg = ep_func->msix_cap + dbi_offset + PCI_MSIX_FLAGS;
> - val = dw_pcie_readw_dbi(pci, reg);
> + reg = ep_func->msix_cap + PCI_MSIX_FLAGS;
> + val = dw_pcie_ep_readw_dbi(ep, func_no, reg);
> val &= ~PCI_MSIX_FLAGS_QSIZE;
> val |= interrupts;
> dw_pcie_writew_dbi(pci, reg, val);
>
> - reg = ep_func->msix_cap + dbi_offset + PCI_MSIX_TABLE;
> + reg = ep_func->msix_cap + PCI_MSIX_TABLE;
> val = offset | bir;
> - dw_pcie_writel_dbi(pci, reg, val);
> + dw_pcie_ep_writel_dbi(ep, func_no, reg, val);
>
> - reg = ep_func->msix_cap + dbi_offset + PCI_MSIX_PBA;
> + reg = ep_func->msix_cap + PCI_MSIX_PBA;
> val = (offset + (interrupts * PCI_MSIX_ENTRY_SIZE)) | bir;
> - dw_pcie_writel_dbi(pci, reg, val);
> + dw_pcie_ep_writel_dbi(ep, func_no, reg, val);
>
> dw_pcie_dbi_ro_wr_dis(pci);
>
> @@ -510,38 +453,34 @@ EXPORT_SYMBOL_GPL(dw_pcie_ep_raise_legacy_irq);
> int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
> u8 interrupt_num)
> {
> - struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> + u32 msg_addr_lower, msg_addr_upper, reg;
> struct dw_pcie_ep_func *ep_func;
> struct pci_epc *epc = ep->epc;
> unsigned int aligned_offset;
> - unsigned int dbi_offset = 0;
> u16 msg_ctrl, msg_data;
> - u32 msg_addr_lower, msg_addr_upper, reg;
> - u64 msg_addr;
> bool has_upper;
> + u64 msg_addr;
> int ret;
>
> ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
> if (!ep_func || !ep_func->msi_cap)
> return -EINVAL;
>
> - dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> -
> /* Raise MSI per the PCI Local Bus Specification Revision 3.0, 6.8.1. */
> - reg = ep_func->msi_cap + dbi_offset + PCI_MSI_FLAGS;
> - msg_ctrl = dw_pcie_readw_dbi(pci, reg);
> + reg = ep_func->msi_cap + PCI_MSI_FLAGS;
> + msg_ctrl = dw_pcie_ep_readw_dbi(ep, func_no, reg);
> has_upper = !!(msg_ctrl & PCI_MSI_FLAGS_64BIT);
> - reg = ep_func->msi_cap + dbi_offset + PCI_MSI_ADDRESS_LO;
> - msg_addr_lower = dw_pcie_readl_dbi(pci, reg);
> + reg = ep_func->msi_cap + PCI_MSI_ADDRESS_LO;
> + msg_addr_lower = dw_pcie_ep_readl_dbi(ep, func_no, reg);
> if (has_upper) {
> - reg = ep_func->msi_cap + dbi_offset + PCI_MSI_ADDRESS_HI;
> - msg_addr_upper = dw_pcie_readl_dbi(pci, reg);
> - reg = ep_func->msi_cap + dbi_offset + PCI_MSI_DATA_64;
> - msg_data = dw_pcie_readw_dbi(pci, reg);
> + reg = ep_func->msi_cap + PCI_MSI_ADDRESS_HI;
> + msg_addr_upper = dw_pcie_ep_readl_dbi(ep, func_no, reg);
> + reg = ep_func->msi_cap + PCI_MSI_DATA_64;
> + msg_data = dw_pcie_ep_readw_dbi(ep, func_no, reg);
> } else {
> msg_addr_upper = 0;
> - reg = ep_func->msi_cap + dbi_offset + PCI_MSI_DATA_32;
> - msg_data = dw_pcie_readw_dbi(pci, reg);
> + reg = ep_func->msi_cap + PCI_MSI_DATA_32;
> + msg_data = dw_pcie_ep_readw_dbi(ep, func_no, reg);
> }
> aligned_offset = msg_addr_lower & (epc->mem->window.page_size - 1);
> msg_addr = ((u64)msg_addr_upper) << 32 |
> @@ -582,10 +521,9 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
> u16 interrupt_num)
> {
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> - struct dw_pcie_ep_func *ep_func;
> struct pci_epf_msix_tbl *msix_tbl;
> + struct dw_pcie_ep_func *ep_func;
> struct pci_epc *epc = ep->epc;
> - unsigned int dbi_offset = 0;
> u32 reg, msg_data, vec_ctrl;
> unsigned int aligned_offset;
> u32 tbl_offset;
> @@ -597,10 +535,8 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
> if (!ep_func || !ep_func->msix_cap)
> return -EINVAL;
>
> - dbi_offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> -
> - reg = ep_func->msix_cap + dbi_offset + PCI_MSIX_TABLE;
> - tbl_offset = dw_pcie_readl_dbi(pci, reg);
> + reg = ep_func->msix_cap + PCI_MSIX_TABLE;
> + tbl_offset = dw_pcie_ep_readl_dbi(ep, func_no, reg);
> bir = FIELD_GET(PCI_MSIX_TABLE_BIR, tbl_offset);
> tbl_offset &= PCI_MSIX_TABLE_OFFSET;
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 5e36da166ffe..b92e69041fe8 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -534,6 +534,99 @@ static inline enum dw_pcie_ltssm dw_pcie_get_ltssm(struct dw_pcie *pci)
> return (enum dw_pcie_ltssm)FIELD_GET(PORT_LOGIC_LTSSM_STATE_MASK, val);
> }
>
> +static inline unsigned int dw_pcie_ep_get_dbi_offset(struct dw_pcie_ep *ep,
> + u8 func_no)
> +{
> + unsigned int dbi_offset = 0;
> +
> + if (ep->ops->get_dbi_offset)
> + dbi_offset = ep->ops->get_dbi_offset(ep, func_no);
> +
> + return dbi_offset;
> +}
> +
> +static inline unsigned int dw_pcie_ep_get_dbi2_offset(struct dw_pcie_ep *ep,
> + u8 func_no)
> +{
> + unsigned int dbi2_offset = 0;
> +
> + if (ep->ops->get_dbi2_offset)
> + dbi2_offset = ep->ops->get_dbi2_offset(ep, func_no);
> + else if (ep->ops->get_dbi_offset) /* for backward compatibility */
> + dbi2_offset = ep->ops->get_dbi_offset(ep, func_no);
> +
> + return dbi2_offset;
> +}
> +
> +static inline u32 dw_pcie_ep_read_dbi(struct dw_pcie_ep *ep, u8 func_no,
> + u32 reg, size_t size)
> +{
> + unsigned int offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> +
> + return dw_pcie_read_dbi(pci, offset + reg, size);
> +}
> +
> +static inline void dw_pcie_ep_write_dbi(struct dw_pcie_ep *ep, u8 func_no,
> + u32 reg, size_t size, u32 val)
> +{
> + unsigned int offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> +
> + dw_pcie_write_dbi(pci, offset + reg, size, val);
> +}
> +
> +static inline void dw_pcie_ep_write_dbi2(struct dw_pcie_ep *ep, u8 func_no,
> + u32 reg, size_t size, u32 val)
> +{
> + unsigned int offset = dw_pcie_ep_get_dbi2_offset(ep, func_no);
> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> +
> + dw_pcie_write_dbi2(pci, offset + reg, size, val);
> +}
> +
> +static inline void dw_pcie_ep_writel_dbi(struct dw_pcie_ep *ep, u8 func_no,
> + u32 reg, u32 val)
> +{
> + dw_pcie_ep_write_dbi(ep, func_no, reg, 0x4, val);
> +}
> +
> +static inline u32 dw_pcie_ep_readl_dbi(struct dw_pcie_ep *ep, u8 func_no,
> + u32 reg)
> +{
> + return dw_pcie_ep_read_dbi(ep, func_no, reg, 0x4);
> +}
> +
> +static inline void dw_pcie_ep_writew_dbi(struct dw_pcie_ep *ep, u8 func_no,
> + u32 reg, u16 val)
> +{
> + dw_pcie_ep_write_dbi(ep, func_no, reg, 0x2, val);
> +}
> +
> +static inline u16 dw_pcie_ep_readw_dbi(struct dw_pcie_ep *ep, u8 func_no,
> + u32 reg)
> +{
> + return dw_pcie_ep_read_dbi(ep, func_no, reg, 0x2);
> +}
> +
> +static inline void dw_pcie_ep_writeb_dbi(struct dw_pcie_ep *ep, u8 func_no,
> + u32 reg, u8 val)
> +{
> + dw_pcie_ep_write_dbi(ep, func_no, reg, 0x1, val);
> +}
> +
> +static inline u8 dw_pcie_ep_readb_dbi(struct dw_pcie_ep *ep, u8 func_no,
> + u32 reg)
> +{
> + return dw_pcie_ep_read_dbi(ep, func_no, reg, 0x1);
> +}
> +
> +static inline void dw_pcie_ep_writel_dbi2(struct dw_pcie_ep *ep, u8 func_no,
> + u32 reg, u32 val)
> +{
> + dw_pcie_ep_write_dbi2(ep, func_no, reg, 0x4, val);
> +}
> +
> #ifdef CONFIG_PCIE_DW_HOST
> irqreturn_t dw_handle_msi_irq(struct dw_pcie_rp *pp);
> int dw_pcie_setup_rc(struct dw_pcie_rp *pp);
> --
> 2.34.1
>
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH v3 4/6] PCI: dwc: Add dw_pcie_ep_{read,write}_dbi[2] helpers
2023-12-17 16:32 ` Manivannan Sadhasivam
@ 2023-12-19 0:21 ` Yoshihiro Shimoda
2023-12-19 9:54 ` Serge Semin
0 siblings, 1 reply; 14+ messages in thread
From: Yoshihiro Shimoda @ 2023-12-19 0:21 UTC (permalink / raw)
To: Manivannan Sadhasivam, Serge Semin
Cc: lpieralisi, kw, robh, bhelgaas, jingoohan1, gustavo.pimentel,
linux-pci, linux-renesas-soc
Hello Serge, Manivannan,
> From: Manivannan Sadhasivam, Sent: Monday, December 18, 2023 1:33 AM
>
> On Fri, Dec 15, 2023 at 12:51:28PM +0300, Serge Semin wrote:
> > Hi Yoshihiro
> >
> > On Fri, Dec 15, 2023 at 11:29:53AM +0900, Yoshihiro Shimoda wrote:
> > > The current code calculated some dbi[2] registers' offset by calling
> > > dw_pcie_ep_get_dbi[2]_offset() in each function. To improve code
> > > readability, add dw_pcie_ep_{read,write}_dbi[2} and some data-width
> > > related helpers.
> > >
> > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > > Reviewed-by: Serge Semin <fancer.lancer@gmail.com>
> > > ---
> > > .../pci/controller/dwc/pcie-designware-ep.c | 184 ++++++------------
> > > drivers/pci/controller/dwc/pcie-designware.h | 93 +++++++++
> > > 2 files changed, 153 insertions(+), 124 deletions(-)
> > >
<snip>
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> > > index 5e36da166ffe..b92e69041fe8 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > > @@ -534,6 +534,99 @@ static inline enum dw_pcie_ltssm dw_pcie_get_ltssm(struct dw_pcie *pci)
> > > return (enum dw_pcie_ltssm)FIELD_GET(PORT_LOGIC_LTSSM_STATE_MASK, val);
> > > }
> > >
> >
> > > +static inline unsigned int dw_pcie_ep_get_dbi_offset(struct dw_pcie_ep *ep,
> > > + u8 func_no)
> > > +{
> > > + unsigned int dbi_offset = 0;
> > > +
> > > + if (ep->ops->get_dbi_offset)
> > > + dbi_offset = ep->ops->get_dbi_offset(ep, func_no);
> > > +
> > > + return dbi_offset;
> > > +}
> > > +
> > > +static inline unsigned int dw_pcie_ep_get_dbi2_offset(struct dw_pcie_ep *ep,
> > > + u8 func_no)
> > > +{
> > > + unsigned int dbi2_offset = 0;
> > > +
> > > + if (ep->ops->get_dbi2_offset)
> > > + dbi2_offset = ep->ops->get_dbi2_offset(ep, func_no);
> > > + else if (ep->ops->get_dbi_offset) /* for backward compatibility */
> > > + dbi2_offset = ep->ops->get_dbi_offset(ep, func_no);
> > > +
> > > + return dbi2_offset;
> > > +}
> > > +
> > > +static inline u32 dw_pcie_ep_read_dbi(struct dw_pcie_ep *ep, u8 func_no,
> > > + u32 reg, size_t size)
> > > +{
> > > + unsigned int offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> > > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > +
> > > + return dw_pcie_read_dbi(pci, offset + reg, size);
> > > +}
> > > +
> > > +static inline void dw_pcie_ep_write_dbi(struct dw_pcie_ep *ep, u8 func_no,
> > > + u32 reg, size_t size, u32 val)
> > > +{
> > > + unsigned int offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> > > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > +
> > > + dw_pcie_write_dbi(pci, offset + reg, size, val);
> > > +}
> > > +
> > > +static inline void dw_pcie_ep_write_dbi2(struct dw_pcie_ep *ep, u8 func_no,
> > > + u32 reg, size_t size, u32 val)
> > > +{
> > > + unsigned int offset = dw_pcie_ep_get_dbi2_offset(ep, func_no);
> > > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > +
> > > + dw_pcie_write_dbi2(pci, offset + reg, size, val);
> > > +}
> > > +
> > > +static inline void dw_pcie_ep_writel_dbi(struct dw_pcie_ep *ep, u8 func_no,
> > > + u32 reg, u32 val)
> > > +{
> > > + dw_pcie_ep_write_dbi(ep, func_no, reg, 0x4, val);
> > > +}
> > > +
> > > +static inline u32 dw_pcie_ep_readl_dbi(struct dw_pcie_ep *ep, u8 func_no,
> > > + u32 reg)
> > > +{
> > > + return dw_pcie_ep_read_dbi(ep, func_no, reg, 0x4);
> > > +}
> > > +
> > > +static inline void dw_pcie_ep_writew_dbi(struct dw_pcie_ep *ep, u8 func_no,
> > > + u32 reg, u16 val)
> > > +{
> > > + dw_pcie_ep_write_dbi(ep, func_no, reg, 0x2, val);
> > > +}
> > > +
> > > +static inline u16 dw_pcie_ep_readw_dbi(struct dw_pcie_ep *ep, u8 func_no,
> > > + u32 reg)
> > > +{
> > > + return dw_pcie_ep_read_dbi(ep, func_no, reg, 0x2);
> > > +}
> > > +
> > > +static inline void dw_pcie_ep_writeb_dbi(struct dw_pcie_ep *ep, u8 func_no,
> > > + u32 reg, u8 val)
> > > +{
> > > + dw_pcie_ep_write_dbi(ep, func_no, reg, 0x1, val);
> > > +}
> > > +
> > > +static inline u8 dw_pcie_ep_readb_dbi(struct dw_pcie_ep *ep, u8 func_no,
> > > + u32 reg)
> > > +{
> > > + return dw_pcie_ep_read_dbi(ep, func_no, reg, 0x1);
> > > +}
> > > +
> > > +static inline void dw_pcie_ep_writel_dbi2(struct dw_pcie_ep *ep, u8 func_no,
> > > + u32 reg, u32 val)
> > > +{
> > > + dw_pcie_ep_write_dbi2(ep, func_no, reg, 0x4, val);
> > > +}
> > > +
> >
> > A tiny nitpick. Since these are CSRs accessors it would be better for
> > readability to have them grouped with the rest of the IO-accessors
> > dw_pcie_writel_dbi()..dw_pcie_writel_dbi2(). Particularly have them
> > defined below the already available ones. So first normal
> > DBI-accessors would be placed and the EP-specific DBI-accessors
> > afterwords. Not sure whether it's that much required. So it's up to
> > Mani to decide. Perhaps the subsystem maintainers could fix it on
> > merge in? Bjorn, Krzysztof, Lorenzo?
> >
>
> +1
Thank you for your comment and a vote.
To be honest, I don't understand what grouping is better for readability...
Anyway, perhaps, I should modify the header file as v4 patches. On v3,
the IO-accessors are the following:
---
static inline void dw_pcie_writel_dbi(struct dw_pcie *pci, u32 reg, u32 val)
static inline u32 dw_pcie_readl_dbi(struct dw_pcie *pci, u32 reg)
static inline void dw_pcie_writew_dbi(struct dw_pcie *pci, u32 reg, u16 val)
static inline u16 dw_pcie_readw_dbi(struct dw_pcie *pci, u32 reg)
static inline void dw_pcie_writeb_dbi(struct dw_pcie *pci, u32 reg, u8 val)
static inline u8 dw_pcie_readb_dbi(struct dw_pcie *pci, u32 reg)
static inline void dw_pcie_writel_dbi2(struct dw_pcie *pci, u32 reg, u32 val)
static inline void dw_pcie_dbi_ro_wr_en(struct dw_pcie *pci) // not IO-accessors
static inline void dw_pcie_dbi_ro_wr_dis(struct dw_pcie *pci) // not IO-accessors
static inline int dw_pcie_start_link(struct dw_pcie *pci) // not IO-accessors
static inline void dw_pcie_stop_link(struct dw_pcie *pci) // not IO-accessors
static inline enum dw_pcie_ltssm dw_pcie_get_ltssm(struct dw_pcie *pci) // not IO-accessors
static inline unsigned int dw_pcie_ep_get_dbi_offset(struct dw_pcie_ep *ep, u8 func_no)
static inline unsigned int dw_pcie_ep_get_dbi2_offset(struct dw_pcie_ep *ep, u8 func_no)
static inline u32 dw_pcie_ep_read_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg, size_t size)
static inline void dw_pcie_ep_write_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg, size_t size, u32 val)
static inline void dw_pcie_ep_write_dbi2(struct dw_pcie_ep *ep, u8 func_no, u32 reg, size_t size, u32 val) // for dbi2
static inline void dw_pcie_ep_writel_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg, u32 val)
static inline u32 dw_pcie_ep_readl_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg)
static inline void dw_pcie_ep_writew_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg, u16 val)
static inline u16 dw_pcie_ep_readw_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg)
static inline void dw_pcie_ep_writeb_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg, u8 val)
static inline u8 dw_pcie_ep_readb_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg)
static inline void dw_pcie_ep_writel_dbi2(struct dw_pcie_ep *ep, u8 func_no, u32 reg, u32 val) // for dbi2
---
Perhaps the following order is better?
---
// normal DBI-accessors
static inline void dw_pcie_writel_dbi(struct dw_pcie *pci, u32 reg, u32 val)
static inline u32 dw_pcie_readl_dbi(struct dw_pcie *pci, u32 reg)
static inline void dw_pcie_writew_dbi(struct dw_pcie *pci, u32 reg, u16 val)
static inline u16 dw_pcie_readw_dbi(struct dw_pcie *pci, u32 reg)
static inline void dw_pcie_writeb_dbi(struct dw_pcie *pci, u32 reg, u8 val)
static inline u8 dw_pcie_readb_dbi(struct dw_pcie *pci, u32 reg)
static inline void dw_pcie_writel_dbi2(struct dw_pcie *pci, u32 reg, u32 val)
// EP-specific DBI-accessors for dbi
static inline unsigned int dw_pcie_ep_get_dbi_offset(struct dw_pcie_ep *ep, u8 func_no)
static inline u32 dw_pcie_ep_read_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg, size_t size)
static inline void dw_pcie_ep_write_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg, size_t size, u32 val)
static inline void dw_pcie_ep_writel_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg, u32 val)
static inline u32 dw_pcie_ep_readl_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg)
static inline void dw_pcie_ep_writew_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg, u16 val)
static inline u16 dw_pcie_ep_readw_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg)
static inline void dw_pcie_ep_writeb_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg, u8 val)
static inline u8 dw_pcie_ep_readb_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg)
// EP-specific DBI-accessors for dbi2
static inline unsigned int dw_pcie_ep_get_dbi2_offset(struct dw_pcie_ep *ep, u8 func_no)
static inline void dw_pcie_ep_write_dbi2(struct dw_pcie_ep *ep, u8 func_no, u32 reg, size_t size, u32 val)
static inline void dw_pcie_ep_writel_dbi2(struct dw_pcie_ep *ep, u8 func_no, u32 reg, u32 val)
// rest of inline functions
static inline void dw_pcie_dbi_ro_wr_en(struct dw_pcie *pci)
static inline void dw_pcie_dbi_ro_wr_dis(struct dw_pcie *pci)
static inline int dw_pcie_start_link(struct dw_pcie *pci)
static inline void dw_pcie_stop_link(struct dw_pcie *pci)
static inline enum dw_pcie_ltssm dw_pcie_get_ltssm(struct dw_pcie *pci)
---
Best regards,
Yoshihiro Shimoda
> - Mani
>
> > -Serge(y)
> >
> > > #ifdef CONFIG_PCIE_DW_HOST
> > > irqreturn_t dw_handle_msi_irq(struct dw_pcie_rp *pp);
> > > int dw_pcie_setup_rc(struct dw_pcie_rp *pp);
> > > --
> > > 2.34.1
> > >
> >
>
> --
> மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 4/6] PCI: dwc: Add dw_pcie_ep_{read,write}_dbi[2] helpers
2023-12-19 0:21 ` Yoshihiro Shimoda
@ 2023-12-19 9:54 ` Serge Semin
2023-12-20 5:10 ` Yoshihiro Shimoda
0 siblings, 1 reply; 14+ messages in thread
From: Serge Semin @ 2023-12-19 9:54 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: Manivannan Sadhasivam, lpieralisi, kw, robh, bhelgaas,
jingoohan1, gustavo.pimentel, linux-pci, linux-renesas-soc
Hi Yoshihiro
On Tue, Dec 19, 2023 at 12:21:12AM +0000, Yoshihiro Shimoda wrote:
> Hello Serge, Manivannan,
>
> > From: Manivannan Sadhasivam, Sent: Monday, December 18, 2023 1:33 AM
> >
> > On Fri, Dec 15, 2023 at 12:51:28PM +0300, Serge Semin wrote:
> > > Hi Yoshihiro
> > >
> > > On Fri, Dec 15, 2023 at 11:29:53AM +0900, Yoshihiro Shimoda wrote:
> > > > The current code calculated some dbi[2] registers' offset by calling
> > > > dw_pcie_ep_get_dbi[2]_offset() in each function. To improve code
> > > > readability, add dw_pcie_ep_{read,write}_dbi[2} and some data-width
> > > > related helpers.
> > > >
> > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > > > Reviewed-by: Serge Semin <fancer.lancer@gmail.com>
> > > > ---
> > > > .../pci/controller/dwc/pcie-designware-ep.c | 184 ++++++------------
> > > > drivers/pci/controller/dwc/pcie-designware.h | 93 +++++++++
> > > > 2 files changed, 153 insertions(+), 124 deletions(-)
> > > >
> <snip>
> > > > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> > > > index 5e36da166ffe..b92e69041fe8 100644
> > > > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > > > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > > > @@ -534,6 +534,99 @@ static inline enum dw_pcie_ltssm dw_pcie_get_ltssm(struct dw_pcie *pci)
> > > > return (enum dw_pcie_ltssm)FIELD_GET(PORT_LOGIC_LTSSM_STATE_MASK, val);
> > > > }
> > > >
> > >
> > > > +static inline unsigned int dw_pcie_ep_get_dbi_offset(struct dw_pcie_ep *ep,
> > > > + u8 func_no)
> > > > +{
> > > > + unsigned int dbi_offset = 0;
> > > > +
> > > > + if (ep->ops->get_dbi_offset)
> > > > + dbi_offset = ep->ops->get_dbi_offset(ep, func_no);
> > > > +
> > > > + return dbi_offset;
> > > > +}
> > > > +
> > > > +static inline unsigned int dw_pcie_ep_get_dbi2_offset(struct dw_pcie_ep *ep,
> > > > + u8 func_no)
> > > > +{
> > > > + unsigned int dbi2_offset = 0;
> > > > +
> > > > + if (ep->ops->get_dbi2_offset)
> > > > + dbi2_offset = ep->ops->get_dbi2_offset(ep, func_no);
> > > > + else if (ep->ops->get_dbi_offset) /* for backward compatibility */
> > > > + dbi2_offset = ep->ops->get_dbi_offset(ep, func_no);
> > > > +
> > > > + return dbi2_offset;
> > > > +}
> > > > +
> > > > +static inline u32 dw_pcie_ep_read_dbi(struct dw_pcie_ep *ep, u8 func_no,
> > > > + u32 reg, size_t size)
> > > > +{
> > > > + unsigned int offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> > > > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > > +
> > > > + return dw_pcie_read_dbi(pci, offset + reg, size);
> > > > +}
> > > > +
> > > > +static inline void dw_pcie_ep_write_dbi(struct dw_pcie_ep *ep, u8 func_no,
> > > > + u32 reg, size_t size, u32 val)
> > > > +{
> > > > + unsigned int offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> > > > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > > +
> > > > + dw_pcie_write_dbi(pci, offset + reg, size, val);
> > > > +}
> > > > +
> > > > +static inline void dw_pcie_ep_write_dbi2(struct dw_pcie_ep *ep, u8 func_no,
> > > > + u32 reg, size_t size, u32 val)
> > > > +{
> > > > + unsigned int offset = dw_pcie_ep_get_dbi2_offset(ep, func_no);
> > > > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > > +
> > > > + dw_pcie_write_dbi2(pci, offset + reg, size, val);
> > > > +}
> > > > +
> > > > +static inline void dw_pcie_ep_writel_dbi(struct dw_pcie_ep *ep, u8 func_no,
> > > > + u32 reg, u32 val)
> > > > +{
> > > > + dw_pcie_ep_write_dbi(ep, func_no, reg, 0x4, val);
> > > > +}
> > > > +
> > > > +static inline u32 dw_pcie_ep_readl_dbi(struct dw_pcie_ep *ep, u8 func_no,
> > > > + u32 reg)
> > > > +{
> > > > + return dw_pcie_ep_read_dbi(ep, func_no, reg, 0x4);
> > > > +}
> > > > +
> > > > +static inline void dw_pcie_ep_writew_dbi(struct dw_pcie_ep *ep, u8 func_no,
> > > > + u32 reg, u16 val)
> > > > +{
> > > > + dw_pcie_ep_write_dbi(ep, func_no, reg, 0x2, val);
> > > > +}
> > > > +
> > > > +static inline u16 dw_pcie_ep_readw_dbi(struct dw_pcie_ep *ep, u8 func_no,
> > > > + u32 reg)
> > > > +{
> > > > + return dw_pcie_ep_read_dbi(ep, func_no, reg, 0x2);
> > > > +}
> > > > +
> > > > +static inline void dw_pcie_ep_writeb_dbi(struct dw_pcie_ep *ep, u8 func_no,
> > > > + u32 reg, u8 val)
> > > > +{
> > > > + dw_pcie_ep_write_dbi(ep, func_no, reg, 0x1, val);
> > > > +}
> > > > +
> > > > +static inline u8 dw_pcie_ep_readb_dbi(struct dw_pcie_ep *ep, u8 func_no,
> > > > + u32 reg)
> > > > +{
> > > > + return dw_pcie_ep_read_dbi(ep, func_no, reg, 0x1);
> > > > +}
> > > > +
> > > > +static inline void dw_pcie_ep_writel_dbi2(struct dw_pcie_ep *ep, u8 func_no,
> > > > + u32 reg, u32 val)
> > > > +{
> > > > + dw_pcie_ep_write_dbi2(ep, func_no, reg, 0x4, val);
> > > > +}
> > > > +
> > >
> > > A tiny nitpick. Since these are CSRs accessors it would be better for
> > > readability to have them grouped with the rest of the IO-accessors
> > > dw_pcie_writel_dbi()..dw_pcie_writel_dbi2(). Particularly have them
> > > defined below the already available ones. So first normal
> > > DBI-accessors would be placed and the EP-specific DBI-accessors
> > > afterwords. Not sure whether it's that much required. So it's up to
> > > Mani to decide. Perhaps the subsystem maintainers could fix it on
> > > merge in? Bjorn, Krzysztof, Lorenzo?
> > >
> >
> > +1
>
> Thank you for your comment and a vote.
> To be honest, I don't understand what grouping is better for readability...
It's better to group the function definitions up by their
functionality so the code reader wouldn't need to jump over the
unrelated methods if one is looking for something particular, like
only EP-specific DBI accessors or EP-specific DBI2 accessors, etc.
> Anyway, perhaps, I should modify the header file as v4 patches. On v3,
> the IO-accessors are the following:
> ---
> static inline void dw_pcie_writel_dbi(struct dw_pcie *pci, u32 reg, u32 val)
> static inline u32 dw_pcie_readl_dbi(struct dw_pcie *pci, u32 reg)
> static inline void dw_pcie_writew_dbi(struct dw_pcie *pci, u32 reg, u16 val)
> static inline u16 dw_pcie_readw_dbi(struct dw_pcie *pci, u32 reg)
> static inline void dw_pcie_writeb_dbi(struct dw_pcie *pci, u32 reg, u8 val)
> static inline u8 dw_pcie_readb_dbi(struct dw_pcie *pci, u32 reg)
> static inline void dw_pcie_writel_dbi2(struct dw_pcie *pci, u32 reg, u32 val)
>
> static inline void dw_pcie_dbi_ro_wr_en(struct dw_pcie *pci) // not IO-accessors
> static inline void dw_pcie_dbi_ro_wr_dis(struct dw_pcie *pci) // not IO-accessors
> static inline int dw_pcie_start_link(struct dw_pcie *pci) // not IO-accessors
> static inline void dw_pcie_stop_link(struct dw_pcie *pci) // not IO-accessors
> static inline enum dw_pcie_ltssm dw_pcie_get_ltssm(struct dw_pcie *pci) // not IO-accessors
>
> static inline unsigned int dw_pcie_ep_get_dbi_offset(struct dw_pcie_ep *ep, u8 func_no)
> static inline unsigned int dw_pcie_ep_get_dbi2_offset(struct dw_pcie_ep *ep, u8 func_no)
> static inline u32 dw_pcie_ep_read_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg, size_t size)
> static inline void dw_pcie_ep_write_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg, size_t size, u32 val)
> static inline void dw_pcie_ep_write_dbi2(struct dw_pcie_ep *ep, u8 func_no, u32 reg, size_t size, u32 val) // for dbi2
> static inline void dw_pcie_ep_writel_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg, u32 val)
> static inline u32 dw_pcie_ep_readl_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg)
> static inline void dw_pcie_ep_writew_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg, u16 val)
> static inline u16 dw_pcie_ep_readw_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg)
> static inline void dw_pcie_ep_writeb_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg, u8 val)
> static inline u8 dw_pcie_ep_readb_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg)
> static inline void dw_pcie_ep_writel_dbi2(struct dw_pcie_ep *ep, u8 func_no, u32 reg, u32 val) // for dbi2
> ---
>
> Perhaps the following order is better?
> ---
> // normal DBI-accessors
> static inline void dw_pcie_writel_dbi(struct dw_pcie *pci, u32 reg, u32 val)
> static inline u32 dw_pcie_readl_dbi(struct dw_pcie *pci, u32 reg)
> static inline void dw_pcie_writew_dbi(struct dw_pcie *pci, u32 reg, u16 val)
> static inline u16 dw_pcie_readw_dbi(struct dw_pcie *pci, u32 reg)
> static inline void dw_pcie_writeb_dbi(struct dw_pcie *pci, u32 reg, u8 val)
> static inline u8 dw_pcie_readb_dbi(struct dw_pcie *pci, u32 reg)
> static inline void dw_pcie_writel_dbi2(struct dw_pcie *pci, u32 reg, u32 val)
>
> // EP-specific DBI-accessors for dbi
> static inline unsigned int dw_pcie_ep_get_dbi_offset(struct dw_pcie_ep *ep, u8 func_no)
> static inline u32 dw_pcie_ep_read_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg, size_t size)
> static inline void dw_pcie_ep_write_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg, size_t size, u32 val)
> static inline void dw_pcie_ep_writel_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg, u32 val)
> static inline u32 dw_pcie_ep_readl_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg)
> static inline void dw_pcie_ep_writew_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg, u16 val)
> static inline u16 dw_pcie_ep_readw_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg)
> static inline void dw_pcie_ep_writeb_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg, u8 val)
> static inline u8 dw_pcie_ep_readb_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg)
>
> // EP-specific DBI-accessors for dbi2
> static inline unsigned int dw_pcie_ep_get_dbi2_offset(struct dw_pcie_ep *ep, u8 func_no)
> static inline void dw_pcie_ep_write_dbi2(struct dw_pcie_ep *ep, u8 func_no, u32 reg, size_t size, u32 val)
> static inline void dw_pcie_ep_writel_dbi2(struct dw_pcie_ep *ep, u8 func_no, u32 reg, u32 val)
LGTM. Thanks.
-Serge(y)
>
> // rest of inline functions
> static inline void dw_pcie_dbi_ro_wr_en(struct dw_pcie *pci)
> static inline void dw_pcie_dbi_ro_wr_dis(struct dw_pcie *pci)
> static inline int dw_pcie_start_link(struct dw_pcie *pci)
> static inline void dw_pcie_stop_link(struct dw_pcie *pci)
> static inline enum dw_pcie_ltssm dw_pcie_get_ltssm(struct dw_pcie *pci)
> ---
>
> Best regards,
> Yoshihiro Shimoda
>
> > - Mani
> >
> > > -Serge(y)
> > >
> > > > #ifdef CONFIG_PCIE_DW_HOST
> > > > irqreturn_t dw_handle_msi_irq(struct dw_pcie_rp *pp);
> > > > int dw_pcie_setup_rc(struct dw_pcie_rp *pp);
> > > > --
> > > > 2.34.1
> > > >
> > >
> >
> > --
> > மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH v3 4/6] PCI: dwc: Add dw_pcie_ep_{read,write}_dbi[2] helpers
2023-12-19 9:54 ` Serge Semin
@ 2023-12-20 5:10 ` Yoshihiro Shimoda
0 siblings, 0 replies; 14+ messages in thread
From: Yoshihiro Shimoda @ 2023-12-20 5:10 UTC (permalink / raw)
To: Serge Semin
Cc: Manivannan Sadhasivam, lpieralisi, kw, robh, bhelgaas,
jingoohan1, gustavo.pimentel, linux-pci, linux-renesas-soc
Hi Serge,
> From: Serge Semin, Sent: Tuesday, December 19, 2023 6:55 PM
> On Tue, Dec 19, 2023 at 12:21:12AM +0000, Yoshihiro Shimoda wrote:
> > Hello Serge, Manivannan,
> >
> > > From: Manivannan Sadhasivam, Sent: Monday, December 18, 2023 1:33 AM
> > >
> > > On Fri, Dec 15, 2023 at 12:51:28PM +0300, Serge Semin wrote:
> > > > Hi Yoshihiro
> > > >
> > > > On Fri, Dec 15, 2023 at 11:29:53AM +0900, Yoshihiro Shimoda wrote:
> > > > > The current code calculated some dbi[2] registers' offset by calling
> > > > > dw_pcie_ep_get_dbi[2]_offset() in each function. To improve code
> > > > > readability, add dw_pcie_ep_{read,write}_dbi[2} and some data-width
> > > > > related helpers.
> > > > >
> > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > > > > Reviewed-by: Serge Semin <fancer.lancer@gmail.com>
> > > > > ---
> > > > > .../pci/controller/dwc/pcie-designware-ep.c | 184 ++++++------------
> > > > > drivers/pci/controller/dwc/pcie-designware.h | 93 +++++++++
> > > > > 2 files changed, 153 insertions(+), 124 deletions(-)
> > > > >
> > <snip>
> > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> > > > > index 5e36da166ffe..b92e69041fe8 100644
> > > > > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > > > > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > > > > @@ -534,6 +534,99 @@ static inline enum dw_pcie_ltssm dw_pcie_get_ltssm(struct dw_pcie *pci)
> > > > > return (enum dw_pcie_ltssm)FIELD_GET(PORT_LOGIC_LTSSM_STATE_MASK, val);
> > > > > }
> > > > >
> > > >
> > > > > +static inline unsigned int dw_pcie_ep_get_dbi_offset(struct dw_pcie_ep *ep,
> > > > > + u8 func_no)
> > > > > +{
> > > > > + unsigned int dbi_offset = 0;
> > > > > +
> > > > > + if (ep->ops->get_dbi_offset)
> > > > > + dbi_offset = ep->ops->get_dbi_offset(ep, func_no);
> > > > > +
> > > > > + return dbi_offset;
> > > > > +}
> > > > > +
> > > > > +static inline unsigned int dw_pcie_ep_get_dbi2_offset(struct dw_pcie_ep *ep,
> > > > > + u8 func_no)
> > > > > +{
> > > > > + unsigned int dbi2_offset = 0;
> > > > > +
> > > > > + if (ep->ops->get_dbi2_offset)
> > > > > + dbi2_offset = ep->ops->get_dbi2_offset(ep, func_no);
> > > > > + else if (ep->ops->get_dbi_offset) /* for backward compatibility */
> > > > > + dbi2_offset = ep->ops->get_dbi_offset(ep, func_no);
> > > > > +
> > > > > + return dbi2_offset;
> > > > > +}
> > > > > +
> > > > > +static inline u32 dw_pcie_ep_read_dbi(struct dw_pcie_ep *ep, u8 func_no,
> > > > > + u32 reg, size_t size)
> > > > > +{
> > > > > + unsigned int offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> > > > > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > > > +
> > > > > + return dw_pcie_read_dbi(pci, offset + reg, size);
> > > > > +}
> > > > > +
> > > > > +static inline void dw_pcie_ep_write_dbi(struct dw_pcie_ep *ep, u8 func_no,
> > > > > + u32 reg, size_t size, u32 val)
> > > > > +{
> > > > > + unsigned int offset = dw_pcie_ep_get_dbi_offset(ep, func_no);
> > > > > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > > > +
> > > > > + dw_pcie_write_dbi(pci, offset + reg, size, val);
> > > > > +}
> > > > > +
> > > > > +static inline void dw_pcie_ep_write_dbi2(struct dw_pcie_ep *ep, u8 func_no,
> > > > > + u32 reg, size_t size, u32 val)
> > > > > +{
> > > > > + unsigned int offset = dw_pcie_ep_get_dbi2_offset(ep, func_no);
> > > > > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > > > +
> > > > > + dw_pcie_write_dbi2(pci, offset + reg, size, val);
> > > > > +}
> > > > > +
> > > > > +static inline void dw_pcie_ep_writel_dbi(struct dw_pcie_ep *ep, u8 func_no,
> > > > > + u32 reg, u32 val)
> > > > > +{
> > > > > + dw_pcie_ep_write_dbi(ep, func_no, reg, 0x4, val);
> > > > > +}
> > > > > +
> > > > > +static inline u32 dw_pcie_ep_readl_dbi(struct dw_pcie_ep *ep, u8 func_no,
> > > > > + u32 reg)
> > > > > +{
> > > > > + return dw_pcie_ep_read_dbi(ep, func_no, reg, 0x4);
> > > > > +}
> > > > > +
> > > > > +static inline void dw_pcie_ep_writew_dbi(struct dw_pcie_ep *ep, u8 func_no,
> > > > > + u32 reg, u16 val)
> > > > > +{
> > > > > + dw_pcie_ep_write_dbi(ep, func_no, reg, 0x2, val);
> > > > > +}
> > > > > +
> > > > > +static inline u16 dw_pcie_ep_readw_dbi(struct dw_pcie_ep *ep, u8 func_no,
> > > > > + u32 reg)
> > > > > +{
> > > > > + return dw_pcie_ep_read_dbi(ep, func_no, reg, 0x2);
> > > > > +}
> > > > > +
> > > > > +static inline void dw_pcie_ep_writeb_dbi(struct dw_pcie_ep *ep, u8 func_no,
> > > > > + u32 reg, u8 val)
> > > > > +{
> > > > > + dw_pcie_ep_write_dbi(ep, func_no, reg, 0x1, val);
> > > > > +}
> > > > > +
> > > > > +static inline u8 dw_pcie_ep_readb_dbi(struct dw_pcie_ep *ep, u8 func_no,
> > > > > + u32 reg)
> > > > > +{
> > > > > + return dw_pcie_ep_read_dbi(ep, func_no, reg, 0x1);
> > > > > +}
> > > > > +
> > > > > +static inline void dw_pcie_ep_writel_dbi2(struct dw_pcie_ep *ep, u8 func_no,
> > > > > + u32 reg, u32 val)
> > > > > +{
> > > > > + dw_pcie_ep_write_dbi2(ep, func_no, reg, 0x4, val);
> > > > > +}
> > > > > +
> > > >
> > > > A tiny nitpick. Since these are CSRs accessors it would be better for
> > > > readability to have them grouped with the rest of the IO-accessors
> > > > dw_pcie_writel_dbi()..dw_pcie_writel_dbi2(). Particularly have them
> > > > defined below the already available ones. So first normal
> > > > DBI-accessors would be placed and the EP-specific DBI-accessors
> > > > afterwords. Not sure whether it's that much required. So it's up to
> > > > Mani to decide. Perhaps the subsystem maintainers could fix it on
> > > > merge in? Bjorn, Krzysztof, Lorenzo?
> > > >
> > >
> > > +1
> >
> > Thank you for your comment and a vote.
>
> > To be honest, I don't understand what grouping is better for readability...
>
> It's better to group the function definitions up by their
> functionality so the code reader wouldn't need to jump over the
> unrelated methods if one is looking for something particular, like
> only EP-specific DBI accessors or EP-specific DBI2 accessors, etc.
Thank you for the comment. I understood it.
> > Anyway, perhaps, I should modify the header file as v4 patches. On v3,
> > the IO-accessors are the following:
> > ---
> > static inline void dw_pcie_writel_dbi(struct dw_pcie *pci, u32 reg, u32 val)
> > static inline u32 dw_pcie_readl_dbi(struct dw_pcie *pci, u32 reg)
> > static inline void dw_pcie_writew_dbi(struct dw_pcie *pci, u32 reg, u16 val)
> > static inline u16 dw_pcie_readw_dbi(struct dw_pcie *pci, u32 reg)
> > static inline void dw_pcie_writeb_dbi(struct dw_pcie *pci, u32 reg, u8 val)
> > static inline u8 dw_pcie_readb_dbi(struct dw_pcie *pci, u32 reg)
> > static inline void dw_pcie_writel_dbi2(struct dw_pcie *pci, u32 reg, u32 val)
> >
> > static inline void dw_pcie_dbi_ro_wr_en(struct dw_pcie *pci) // not IO-accessors
> > static inline void dw_pcie_dbi_ro_wr_dis(struct dw_pcie *pci) // not IO-accessors
> > static inline int dw_pcie_start_link(struct dw_pcie *pci) // not IO-accessors
> > static inline void dw_pcie_stop_link(struct dw_pcie *pci) // not IO-accessors
> > static inline enum dw_pcie_ltssm dw_pcie_get_ltssm(struct dw_pcie *pci) // not IO-accessors
> >
> > static inline unsigned int dw_pcie_ep_get_dbi_offset(struct dw_pcie_ep *ep, u8 func_no)
> > static inline unsigned int dw_pcie_ep_get_dbi2_offset(struct dw_pcie_ep *ep, u8 func_no)
> > static inline u32 dw_pcie_ep_read_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg, size_t size)
> > static inline void dw_pcie_ep_write_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg, size_t size, u32 val)
> > static inline void dw_pcie_ep_write_dbi2(struct dw_pcie_ep *ep, u8 func_no, u32 reg, size_t size, u32 val) // for dbi2
> > static inline void dw_pcie_ep_writel_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg, u32 val)
> > static inline u32 dw_pcie_ep_readl_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg)
> > static inline void dw_pcie_ep_writew_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg, u16 val)
> > static inline u16 dw_pcie_ep_readw_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg)
> > static inline void dw_pcie_ep_writeb_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg, u8 val)
> > static inline u8 dw_pcie_ep_readb_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg)
> > static inline void dw_pcie_ep_writel_dbi2(struct dw_pcie_ep *ep, u8 func_no, u32 reg, u32 val) // for dbi2
> > ---
> >
>
> > Perhaps the following order is better?
> > ---
> > // normal DBI-accessors
> > static inline void dw_pcie_writel_dbi(struct dw_pcie *pci, u32 reg, u32 val)
> > static inline u32 dw_pcie_readl_dbi(struct dw_pcie *pci, u32 reg)
> > static inline void dw_pcie_writew_dbi(struct dw_pcie *pci, u32 reg, u16 val)
> > static inline u16 dw_pcie_readw_dbi(struct dw_pcie *pci, u32 reg)
> > static inline void dw_pcie_writeb_dbi(struct dw_pcie *pci, u32 reg, u8 val)
> > static inline u8 dw_pcie_readb_dbi(struct dw_pcie *pci, u32 reg)
> > static inline void dw_pcie_writel_dbi2(struct dw_pcie *pci, u32 reg, u32 val)
> >
> > // EP-specific DBI-accessors for dbi
> > static inline unsigned int dw_pcie_ep_get_dbi_offset(struct dw_pcie_ep *ep, u8 func_no)
> > static inline u32 dw_pcie_ep_read_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg, size_t size)
> > static inline void dw_pcie_ep_write_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg, size_t size, u32 val)
> > static inline void dw_pcie_ep_writel_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg, u32 val)
> > static inline u32 dw_pcie_ep_readl_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg)
> > static inline void dw_pcie_ep_writew_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg, u16 val)
> > static inline u16 dw_pcie_ep_readw_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg)
> > static inline void dw_pcie_ep_writeb_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg, u8 val)
> > static inline u8 dw_pcie_ep_readb_dbi(struct dw_pcie_ep *ep, u8 func_no, u32 reg)
> >
> > // EP-specific DBI-accessors for dbi2
> > static inline unsigned int dw_pcie_ep_get_dbi2_offset(struct dw_pcie_ep *ep, u8 func_no)
> > static inline void dw_pcie_ep_write_dbi2(struct dw_pcie_ep *ep, u8 func_no, u32 reg, size_t size, u32 val)
> > static inline void dw_pcie_ep_writel_dbi2(struct dw_pcie_ep *ep, u8 func_no, u32 reg, u32 val)
>
> LGTM. Thanks.
Thank you for your review! I'll fix this patch and submit v4 patch.
Best regards,
Yoshihiro Shimoda
> -Serge(y)
>
> >
> > // rest of inline functions
> > static inline void dw_pcie_dbi_ro_wr_en(struct dw_pcie *pci)
> > static inline void dw_pcie_dbi_ro_wr_dis(struct dw_pcie *pci)
> > static inline int dw_pcie_start_link(struct dw_pcie *pci)
> > static inline void dw_pcie_stop_link(struct dw_pcie *pci)
> > static inline enum dw_pcie_ltssm dw_pcie_get_ltssm(struct dw_pcie *pci)
> > ---
> >
> > Best regards,
> > Yoshihiro Shimoda
> >
> > > - Mani
> > >
> > > > -Serge(y)
> > > >
> > > > > #ifdef CONFIG_PCIE_DW_HOST
> > > > > irqreturn_t dw_handle_msi_irq(struct dw_pcie_rp *pp);
> > > > > int dw_pcie_setup_rc(struct dw_pcie_rp *pp);
> > > > > --
> > > > > 2.34.1
> > > > >
> > > >
> > >
> > > --
> > > மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2023-12-20 5:10 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-15 2:29 [PATCH v3 0/6] PCI: controllers: tidy code up Yoshihiro Shimoda
2023-12-15 2:29 ` [PATCH v3 1/6] PCI: dwc: Drop host prefix from struct dw_pcie_host_ops Yoshihiro Shimoda
2023-12-15 2:29 ` [PATCH v3 2/6] PCI: dwc: Rename to .init in struct dw_pcie_ep_ops Yoshihiro Shimoda
2023-12-15 2:29 ` [PATCH v3 3/6] PCI: dwc: Rename to .get_dbi_offset " Yoshihiro Shimoda
2023-12-15 2:29 ` [PATCH v3 4/6] PCI: dwc: Add dw_pcie_ep_{read,write}_dbi[2] helpers Yoshihiro Shimoda
2023-12-15 9:51 ` Serge Semin
2023-12-15 21:53 ` Krzysztof Wilczyński
2023-12-17 16:32 ` Manivannan Sadhasivam
2023-12-19 0:21 ` Yoshihiro Shimoda
2023-12-19 9:54 ` Serge Semin
2023-12-20 5:10 ` Yoshihiro Shimoda
2023-12-17 16:34 ` Manivannan Sadhasivam
2023-12-15 2:29 ` [PATCH v3 5/6] PCI: iproc: fix -Wvoid-pointer-to-enum-cast warning Yoshihiro Shimoda
2023-12-15 2:29 ` [PATCH v3 6/6] PCI: rcar-gen4: " Yoshihiro Shimoda
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).