* [PATCH 0/3] PCI: dwc: Improve code readability
@ 2023-11-13 1:32 Yoshihiro Shimoda
2023-11-13 1:32 ` [PATCH 1/3] PCI: dwc: Rename to .init in struct dw_pcie_ep_ops Yoshihiro Shimoda
` (3 more replies)
0 siblings, 4 replies; 16+ messages in thread
From: Yoshihiro Shimoda @ 2023-11-13 1:32 UTC (permalink / raw)
To: lpieralisi, kw, robh, bhelgaas, jingoohan1, gustavo.pimentel,
mani, minghuan.Lian, mingkai.hu, roy.zang
Cc: marek.vasut+renesas, linux-pci, linuxppc-dev, linux-renesas-soc,
Yoshihiro Shimoda
This patch series is based on the latest pci.git / next branch.
The patch 1/3 is related to the "note" in the commit [1]
The patches [23]/3 are related to the "Note" which in the commit [2].
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=9baa8a18e31b7167885c11c38841ce92bbe20f4f
[2]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=7873b49b41b92edb3395bff9a528eaf89da5e41c
Yoshihiro Shimoda (3):
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
drivers/pci/controller/dwc/pci-dra7xx.c | 2 +-
drivers/pci/controller/dwc/pci-imx6.c | 2 +-
drivers/pci/controller/dwc/pci-keystone.c | 2 +-
.../pci/controller/dwc/pci-layerscape-ep.c | 7 +-
drivers/pci/controller/dwc/pcie-artpec6.c | 2 +-
.../pci/controller/dwc/pcie-designware-ep.c | 248 ++++++++++--------
.../pci/controller/dwc/pcie-designware-plat.c | 2 +-
drivers/pci/controller/dwc/pcie-designware.h | 4 +-
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 | 6 +-
drivers/pci/controller/dwc/pcie-uniphier-ep.c | 2 +-
12 files changed, 154 insertions(+), 127 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH 1/3] PCI: dwc: Rename to .init in struct dw_pcie_ep_ops 2023-11-13 1:32 [PATCH 0/3] PCI: dwc: Improve code readability Yoshihiro Shimoda @ 2023-11-13 1:32 ` Yoshihiro Shimoda 2023-11-13 10:14 ` Serge Semin 2023-11-13 1:32 ` [PATCH 2/3] PCI: dwc: Rename to .get_dbi_offset " Yoshihiro Shimoda ` (2 subsequent siblings) 3 siblings, 1 reply; 16+ messages in thread From: Yoshihiro Shimoda @ 2023-11-13 1:32 UTC (permalink / raw) To: lpieralisi, kw, robh, bhelgaas, jingoohan1, gustavo.pimentel, mani, minghuan.Lian, mingkai.hu, roy.zang Cc: marek.vasut+renesas, linux-pci, linuxppc-dev, linux-renesas-soc, Yoshihiro Shimoda Since the name of dw_pcie_ep_ops indicates that it's for ep obviously, rename a member .ep_init to .init. Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> --- 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 b445ffe95e3f..f9182cd6fe67 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 74703362aeec..737d4d90fef2 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 0def919f89fa..655c7307eb88 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 9b572a2b2c9a..27ff425c0698 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 55ff76e3d384..cad0e4c24e11 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 3bc45e513b3d..2b7e0f213fb2 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] 16+ messages in thread
* Re: [PATCH 1/3] PCI: dwc: Rename to .init in struct dw_pcie_ep_ops 2023-11-13 1:32 ` [PATCH 1/3] PCI: dwc: Rename to .init in struct dw_pcie_ep_ops Yoshihiro Shimoda @ 2023-11-13 10:14 ` Serge Semin 2023-11-13 11:49 ` Yoshihiro Shimoda 0 siblings, 1 reply; 16+ messages in thread From: Serge Semin @ 2023-11-13 10:14 UTC (permalink / raw) To: Yoshihiro Shimoda Cc: lpieralisi, kw, robh, bhelgaas, jingoohan1, gustavo.pimentel, mani, minghuan.Lian, mingkai.hu, roy.zang, marek.vasut+renesas, linux-pci, linuxppc-dev, linux-renesas-soc Hi Yoshihiro. On Mon, Nov 13, 2023 at 10:32:58AM +0900, Yoshihiro Shimoda wrote: > Since the name of dw_pcie_ep_ops indicates that it's for ep obviously, > rename a member .ep_init to .init. Thanks for the series. This change particularly looks good. But since you are fixing the redundant prefixes anyway, could you also fix the dw_pcie_host_ops structure too (drop host_ prefixes from the .host_init() and .host_deinit() fields)? The change was discussed a while ago here https://lore.kernel.org/linux-pci/20230802104049.GB57374@thinkpad/ in the context of your long-going patchset. It's better to be done in the framework of a separate patch released within this series. -Serge(y) > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > --- > 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 b445ffe95e3f..f9182cd6fe67 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 74703362aeec..737d4d90fef2 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 0def919f89fa..655c7307eb88 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 9b572a2b2c9a..27ff425c0698 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 55ff76e3d384..cad0e4c24e11 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 3bc45e513b3d..2b7e0f213fb2 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 [flat|nested] 16+ messages in thread
* RE: [PATCH 1/3] PCI: dwc: Rename to .init in struct dw_pcie_ep_ops 2023-11-13 10:14 ` Serge Semin @ 2023-11-13 11:49 ` Yoshihiro Shimoda 0 siblings, 0 replies; 16+ messages in thread From: Yoshihiro Shimoda @ 2023-11-13 11:49 UTC (permalink / raw) To: Serge Semin Cc: lpieralisi, kw, robh, bhelgaas, jingoohan1, gustavo.pimentel, mani, minghuan.Lian, mingkai.hu, roy.zang, marek.vasut+renesas, linux-pci, linuxppc-dev, linux-renesas-soc Hi Serge, > From: Serge Semin, Sent: Monday, November 13, 2023 7:15 PM > > Hi Yoshihiro. > > On Mon, Nov 13, 2023 at 10:32:58AM +0900, Yoshihiro Shimoda wrote: > > Since the name of dw_pcie_ep_ops indicates that it's for ep obviously, > > rename a member .ep_init to .init. > > Thanks for the series. This change particularly looks good. But since > you are fixing the redundant prefixes anyway, could you also fix the > dw_pcie_host_ops structure too (drop host_ prefixes from the > .host_init() and .host_deinit() fields)? The change was discussed a > while ago here > https://lore.kernel.org/linux-pci/20230802104049.GB57374@thinkpad/ > > It's better to be done in the framework of a separate patch released > within this series. Thank you for reminding me about the discussion. I'll add such a patch in v2. Best regards, Yoshihiro Shimoda > -Serge(y) > > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > --- > > 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 b445ffe95e3f..f9182cd6fe67 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 74703362aeec..737d4d90fef2 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 0def919f89fa..655c7307eb88 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 9b572a2b2c9a..27ff425c0698 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 55ff76e3d384..cad0e4c24e11 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 3bc45e513b3d..2b7e0f213fb2 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 [flat|nested] 16+ messages in thread
* [PATCH 2/3] PCI: dwc: Rename to .get_dbi_offset in struct dw_pcie_ep_ops 2023-11-13 1:32 [PATCH 0/3] PCI: dwc: Improve code readability Yoshihiro Shimoda 2023-11-13 1:32 ` [PATCH 1/3] PCI: dwc: Rename to .init in struct dw_pcie_ep_ops Yoshihiro Shimoda @ 2023-11-13 1:32 ` Yoshihiro Shimoda 2023-11-13 10:34 ` Serge Semin 2023-11-13 1:33 ` [PATCH 3/3] PCI: dwc: Add dw_pcie_ep_{read,write}_dbi[2] helpers Yoshihiro Shimoda 2023-11-13 10:09 ` [PATCH 0/3] PCI: dwc: Improve code readability Krzysztof Wilczyński 3 siblings, 1 reply; 16+ messages in thread From: Yoshihiro Shimoda @ 2023-11-13 1:32 UTC (permalink / raw) To: lpieralisi, kw, robh, bhelgaas, jingoohan1, gustavo.pimentel, mani, minghuan.Lian, mingkai.hu, roy.zang Cc: marek.vasut+renesas, linux-pci, linuxppc-dev, linux-renesas-soc, Yoshihiro Shimoda Since meaning of .func_conf_select is difficult to understand, rename it to .get_dbi_offset. Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> --- .../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 cad0e4c24e11..485ce52b2416 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 2b7e0f213fb2..8ef03d249001 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] 16+ messages in thread
* Re: [PATCH 2/3] PCI: dwc: Rename to .get_dbi_offset in struct dw_pcie_ep_ops 2023-11-13 1:32 ` [PATCH 2/3] PCI: dwc: Rename to .get_dbi_offset " Yoshihiro Shimoda @ 2023-11-13 10:34 ` Serge Semin 0 siblings, 0 replies; 16+ messages in thread From: Serge Semin @ 2023-11-13 10:34 UTC (permalink / raw) To: Yoshihiro Shimoda Cc: lpieralisi, kw, robh, bhelgaas, jingoohan1, gustavo.pimentel, mani, minghuan.Lian, mingkai.hu, roy.zang, marek.vasut+renesas, linux-pci, linuxppc-dev, linux-renesas-soc On Mon, Nov 13, 2023 at 10:32:59AM +0900, Yoshihiro Shimoda wrote: > Since meaning of .func_conf_select is difficult to understand, > rename it to .get_dbi_offset. This change looks good. Thanks. Reviewed-by: Serge Semin <fancer.lancer@gmail.com> There are redundant initializers will have been left after this patch is applied, but it will be naturally fixed in the next patch. -Serge(y) > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > --- > .../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 cad0e4c24e11..485ce52b2416 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 2b7e0f213fb2..8ef03d249001 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 [flat|nested] 16+ messages in thread
* [PATCH 3/3] PCI: dwc: Add dw_pcie_ep_{read,write}_dbi[2] helpers 2023-11-13 1:32 [PATCH 0/3] PCI: dwc: Improve code readability Yoshihiro Shimoda 2023-11-13 1:32 ` [PATCH 1/3] PCI: dwc: Rename to .init in struct dw_pcie_ep_ops Yoshihiro Shimoda 2023-11-13 1:32 ` [PATCH 2/3] PCI: dwc: Rename to .get_dbi_offset " Yoshihiro Shimoda @ 2023-11-13 1:33 ` Yoshihiro Shimoda 2023-11-13 12:41 ` Serge Semin 2023-11-13 10:09 ` [PATCH 0/3] PCI: dwc: Improve code readability Krzysztof Wilczyński 3 siblings, 1 reply; 16+ messages in thread From: Yoshihiro Shimoda @ 2023-11-13 1:33 UTC (permalink / raw) To: lpieralisi, kw, robh, bhelgaas, jingoohan1, gustavo.pimentel, mani, minghuan.Lian, mingkai.hu, roy.zang Cc: marek.vasut+renesas, linux-pci, linuxppc-dev, linux-renesas-soc, Yoshihiro Shimoda 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> --- .../pci/controller/dwc/pcie-designware-ep.c | 230 ++++++++++-------- 1 file changed, 129 insertions(+), 101 deletions(-) diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c index 1100671db887..dcbed49c9613 100644 --- a/drivers/pci/controller/dwc/pcie-designware-ep.c +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c @@ -65,24 +65,89 @@ static unsigned int dw_pcie_ep_get_dbi2_offset(struct dw_pcie_ep *ep, u8 func_no return dbi2_offset; } +static 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 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 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); +} + 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); - - 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); + reg_dbi2 = 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_dbi2, 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_dbi2 + 4, 0x0); + dw_pcie_ep_writel_dbi(ep, func_no, reg + 4, 0x0); } dw_pcie_dbi_ro_wr_dis(pci); } @@ -99,19 +164,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 +187,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 +201,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 +297,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 +319,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 +384,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 +406,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 +427,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 +449,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 +458,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 +545,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 +613,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 +627,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; -- 2.34.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] PCI: dwc: Add dw_pcie_ep_{read,write}_dbi[2] helpers 2023-11-13 1:33 ` [PATCH 3/3] PCI: dwc: Add dw_pcie_ep_{read,write}_dbi[2] helpers Yoshihiro Shimoda @ 2023-11-13 12:41 ` Serge Semin 2023-11-14 1:29 ` Yoshihiro Shimoda 0 siblings, 1 reply; 16+ messages in thread From: Serge Semin @ 2023-11-13 12:41 UTC (permalink / raw) To: Yoshihiro Shimoda Cc: lpieralisi, kw, robh, bhelgaas, jingoohan1, gustavo.pimentel, mani, minghuan.Lian, mingkai.hu, roy.zang, marek.vasut+renesas, linux-pci, linuxppc-dev, linux-renesas-soc On Mon, Nov 13, 2023 at 10:33:00AM +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. Thanks for submitting this cleanup patch. That's exactly what I meant here https://lore.kernel.org/linux-pci/j4g4ijnxd7qyacszlwyi3tdztkw2nmnjwyhdqf2l2yj3h2mvje@iqsrqiodqbhq/ and Mani later here https://lore.kernel.org/linux-pci/20230728023444.GA4433@thinkpad/ Please note a few nitpicks below. > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > --- > .../pci/controller/dwc/pcie-designware-ep.c | 230 ++++++++++-------- > 1 file changed, 129 insertions(+), 101 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c > index 1100671db887..dcbed49c9613 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c > @@ -65,24 +65,89 @@ static unsigned int dw_pcie_ep_get_dbi2_offset(struct dw_pcie_ep *ep, u8 func_no > return dbi2_offset; > } > > +static 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 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 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); > +} > + I am not sure whether the methods above are supposed to be defined here instead of being moved to the "pcie-designware.h" header file together with dw_pcie_ep_get_dbi2_offset() and dw_pcie_ep_get_dbi_offset(). The later place seems more suitable seeing the accessors are generic, look similar to the dw_pcie_{write,read}_dbi{,2}() functions and might be useful in the platform drivers. On the other hand no LLDDs would have used it currently. So I'll leave this as a food for thoughts for the driver and subsystem maintainers. > 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); > - > - 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); > + reg_dbi2 = PCI_BASE_ADDRESS_0 + (4 * bar); Semantics of the both variables is identical, could you please drop "reg_dbi2" and just use the "reg" variable instead here? You must have just missed it because a similar change is done in the rest of the places in this patch. > 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_dbi2, 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_dbi2 + 4, 0x0); > + dw_pcie_ep_writel_dbi(ep, func_no, reg + 4, 0x0); > } > dw_pcie_dbi_ro_wr_dis(pci); > } > @@ -99,19 +164,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 +187,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 +201,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 +297,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 +319,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 +384,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; Special kudos for preserving and adding the reversed xmas tree order here and below. =) -Serge(y) > > 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 +406,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 +427,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 +449,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 +458,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 +545,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 +613,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 +627,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; > > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH 3/3] PCI: dwc: Add dw_pcie_ep_{read,write}_dbi[2] helpers 2023-11-13 12:41 ` Serge Semin @ 2023-11-14 1:29 ` Yoshihiro Shimoda 0 siblings, 0 replies; 16+ messages in thread From: Yoshihiro Shimoda @ 2023-11-14 1:29 UTC (permalink / raw) To: Serge Semin Cc: lpieralisi, kw, robh, bhelgaas, jingoohan1, gustavo.pimentel, mani, minghuan.Lian, mingkai.hu, roy.zang, marek.vasut+renesas, linux-pci, linuxppc-dev, linux-renesas-soc Hello Serge, > From: Serge Semin, Sent: Monday, November 13, 2023 9:41 PM > > On Mon, Nov 13, 2023 at 10:33:00AM +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. > > Thanks for submitting this cleanup patch. That's exactly what I meant > here <snip URL> > and Mani later here <snip URL> > > Please note a few nitpicks below. Thank you for your review! > > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > --- > > .../pci/controller/dwc/pcie-designware-ep.c | 230 ++++++++++-------- > > 1 file changed, 129 insertions(+), 101 deletions(-) > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c > > index 1100671db887..dcbed49c9613 100644 > > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c > > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c > > @@ -65,24 +65,89 @@ static unsigned int dw_pcie_ep_get_dbi2_offset(struct dw_pcie_ep *ep, u8 func_no > > return dbi2_offset; > > } > > > > +static 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 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 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); > > +} > > + > > I am not sure whether the methods above are supposed to be defined > here instead of being moved to the "pcie-designware.h" header file > together with dw_pcie_ep_get_dbi2_offset() and > dw_pcie_ep_get_dbi_offset(). The later place seems more suitable > seeing the accessors are generic, look similar to the > dw_pcie_{write,read}_dbi{,2}() functions and might be useful in the > platform drivers. On the other hand no LLDDs would have used it > currently. So I'll leave this as a food for thoughts for the driver > and subsystem maintainers. Perhaps, when a device driver needs to use these functions actually, we can move these functions to pcie-designware.h, I think. > > 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); > > - > > - 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); > > + reg_dbi2 = PCI_BASE_ADDRESS_0 + (4 * bar); > > Semantics of the both variables is identical, could you please drop > "reg_dbi2" and just use the "reg" variable instead here? You must have > just missed it because a similar change is done in the rest of the > places in this patch. I think so. I'll drop reg_dbi2 on v2. > > 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_dbi2, 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_dbi2 + 4, 0x0); > > + dw_pcie_ep_writel_dbi(ep, func_no, reg + 4, 0x0); > > } > > dw_pcie_dbi_ro_wr_dis(pci); > > } > > @@ -99,19 +164,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 +187,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 +201,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 +297,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 +319,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 +384,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; > > Special kudos for preserving and adding the reversed xmas tree order > here and below. =) Yes :) Best regards, Yoshihiro Shimoda > -Serge(y) > > > > > 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 +406,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 +427,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 +449,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 +458,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 +545,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 +613,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 +627,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; > > > > -- > > 2.34.1 > > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/3] PCI: dwc: Improve code readability 2023-11-13 1:32 [PATCH 0/3] PCI: dwc: Improve code readability Yoshihiro Shimoda ` (2 preceding siblings ...) 2023-11-13 1:33 ` [PATCH 3/3] PCI: dwc: Add dw_pcie_ep_{read,write}_dbi[2] helpers Yoshihiro Shimoda @ 2023-11-13 10:09 ` Krzysztof Wilczyński 2023-11-13 11:07 ` Geert Uytterhoeven 3 siblings, 1 reply; 16+ messages in thread From: Krzysztof Wilczyński @ 2023-11-13 10:09 UTC (permalink / raw) To: Yoshihiro Shimoda Cc: lpieralisi, robh, bhelgaas, jingoohan1, gustavo.pimentel, mani, minghuan.Lian, mingkai.hu, roy.zang, marek.vasut+renesas, linux-pci, linuxppc-dev, linux-renesas-soc Hi Yoshihiro! > This patch series is based on the latest pci.git / next branch. [...] Thank you for following up to tidy things up! Much appreciated. Now, while you are looking at things, can you also take care about the following: drivers/pci/controller/dwc/pcie-rcar-gen4.c:439:15: warning: cast to smaller integer type 'enum dw_pcie_device_mode' from 'const void *' [-Wvoid-pointer-to-enum-cast] This requires adding structs for each data member of the of_device_id type. Some examples from other drivers: - https://elixir.bootlin.com/linux/v6.6.1/source/drivers/pci/controller/dwc/pcie-tegra194.c#L2440 - https://elixir.bootlin.com/linux/v6.6.1/source/drivers/pci/controller/dwc/pci-keystone.c#L1074 Thank you! :) Krzysztof ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/3] PCI: dwc: Improve code readability 2023-11-13 10:09 ` [PATCH 0/3] PCI: dwc: Improve code readability Krzysztof Wilczyński @ 2023-11-13 11:07 ` Geert Uytterhoeven 2023-11-13 11:47 ` Yoshihiro Shimoda 0 siblings, 1 reply; 16+ messages in thread From: Geert Uytterhoeven @ 2023-11-13 11:07 UTC (permalink / raw) To: Krzysztof Wilczyński Cc: Yoshihiro Shimoda, lpieralisi, robh, bhelgaas, jingoohan1, gustavo.pimentel, mani, minghuan.Lian, mingkai.hu, roy.zang, marek.vasut+renesas, linux-pci, linuxppc-dev, linux-renesas-soc Hi Krzysztof, On Mon, Nov 13, 2023 at 11:09 AM Krzysztof Wilczyński <kw@linux.com> wrote: > > This patch series is based on the latest pci.git / next branch. > [...] > > Thank you for following up to tidy things up! Much appreciated. > > Now, while you are looking at things, can you also take care about the following: > > drivers/pci/controller/dwc/pcie-rcar-gen4.c:439:15: warning: cast to smaller integer type 'enum dw_pcie_device_mode' from 'const void *' [-Wvoid-pointer-to-enum-cast] > > This requires adding structs for each data member of the of_device_id type. That sounds like overkill to me. An intermediate cast to uintptr_t should fix the issue as well. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH 0/3] PCI: dwc: Improve code readability 2023-11-13 11:07 ` Geert Uytterhoeven @ 2023-11-13 11:47 ` Yoshihiro Shimoda 2023-11-13 12:22 ` Krzysztof Wilczyński 0 siblings, 1 reply; 16+ messages in thread From: Yoshihiro Shimoda @ 2023-11-13 11:47 UTC (permalink / raw) To: Geert Uytterhoeven, Krzysztof Wilczyński Cc: lpieralisi, robh, bhelgaas, jingoohan1, gustavo.pimentel, mani, minghuan.Lian, mingkai.hu, roy.zang, marek.vasut+renesas, linux-pci, linuxppc-dev, linux-renesas-soc Hi Krzysztof-san, Geert-san, > From: Geert Uytterhoeven, Sent: Monday, November 13, 2023 8:07 PM > > Hi Krzysztof, > > On Mon, Nov 13, 2023 at 11:09 AM Krzysztof Wilczyński <kw@linux.com> wrote: > > > This patch series is based on the latest pci.git / next branch. > > [...] > > > > Thank you for following up to tidy things up! Much appreciated. > > > > Now, while you are looking at things, can you also take care about the following: > > > > drivers/pci/controller/dwc/pcie-rcar-gen4.c:439:15: warning: cast to smaller integer type 'enum dw_pcie_device_mode' > from 'const void *' [-Wvoid-pointer-to-enum-cast] Thank you for the report! > > This requires adding structs for each data member of the of_device_id type. > > That sounds like overkill to me. > An intermediate cast to uintptr_t should fix the issue as well. I confirmed that the uintptr_t fixed the issue. I also think that adding a new struct with the mode is overkill. So, I would like to fix the issue by using the cast of uintptr_t. Best regards, Yoshihiro Shimoda > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/3] PCI: dwc: Improve code readability 2023-11-13 11:47 ` Yoshihiro Shimoda @ 2023-11-13 12:22 ` Krzysztof Wilczyński 2023-11-13 12:57 ` Geert Uytterhoeven 2023-11-14 0:20 ` Yoshihiro Shimoda 0 siblings, 2 replies; 16+ messages in thread From: Krzysztof Wilczyński @ 2023-11-13 12:22 UTC (permalink / raw) To: Yoshihiro Shimoda Cc: Geert Uytterhoeven, lpieralisi, robh, bhelgaas, jingoohan1, gustavo.pimentel, mani, minghuan.Lian, mingkai.hu, roy.zang, marek.vasut+renesas, linux-pci, linuxppc-dev, linux-renesas-soc Hello, [...] > > > Now, while you are looking at things, can you also take care about the following: > > > > > > drivers/pci/controller/dwc/pcie-rcar-gen4.c:439:15: warning: cast to smaller integer type 'enum dw_pcie_device_mode' > > from 'const void *' [-Wvoid-pointer-to-enum-cast] > > Thank you for the report! > > > > This requires adding structs for each data member of the of_device_id type. > > > > That sounds like overkill to me. > > An intermediate cast to uintptr_t should fix the issue as well. > > I confirmed that the uintptr_t fixed the issue. We declined a similar fix in the past[1] ... > I also think that adding a new struct with the mode is overkill. ... with the hopes that a driver could drop the switch statements in place of using the other pattern. Also, to be consistent with other drivers that do this already. > So, I would like to fix the issue by using the cast of uintptr_t. Sure. I appreciate that this would be more work. When you send your patch, can you include an update to the iproc driver (and credit the original author from [1])? I would appreciate it. 1. https://lore.kernel.org/linux-pci/20230814230008.GA196797@bhelgaas/ Krzysztof ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/3] PCI: dwc: Improve code readability 2023-11-13 12:22 ` Krzysztof Wilczyński @ 2023-11-13 12:57 ` Geert Uytterhoeven 2023-11-13 13:18 ` Krzysztof Wilczyński 2023-11-14 0:20 ` Yoshihiro Shimoda 1 sibling, 1 reply; 16+ messages in thread From: Geert Uytterhoeven @ 2023-11-13 12:57 UTC (permalink / raw) To: Krzysztof Wilczyński Cc: Yoshihiro Shimoda, lpieralisi, robh, bhelgaas, jingoohan1, gustavo.pimentel, mani, minghuan.Lian, mingkai.hu, roy.zang, marek.vasut+renesas, linux-pci, linuxppc-dev, linux-renesas-soc Hi Krzysztof, On Mon, Nov 13, 2023 at 1:22 PM Krzysztof Wilczyński <kw@linux.com> wrote: > > > > Now, while you are looking at things, can you also take care about the following: > > > > > > > > drivers/pci/controller/dwc/pcie-rcar-gen4.c:439:15: warning: cast to smaller integer type 'enum dw_pcie_device_mode' > > > from 'const void *' [-Wvoid-pointer-to-enum-cast] > > > > Thank you for the report! > > > > > > This requires adding structs for each data member of the of_device_id type. > > > > > > That sounds like overkill to me. > > > An intermediate cast to uintptr_t should fix the issue as well. > > > > I confirmed that the uintptr_t fixed the issue. > > We declined a similar fix in the past[1] ... > > > I also think that adding a new struct with the mode is overkill. > > ... with the hopes that a driver could drop the switch statements in place > of using the other pattern. Also, to be consistent with other drivers that > do this already. Note that the issue of casting is something we cannot fix easily: some *_device_id structs use "kernel_ulong_t" for the "data" member, others use "void *". git grep -W "_device_id" -- include/linux/mod_devicetable.h | grep data In addition, several drivers use multiple types of device IDs, so you cannot settle on one type to avoid casts. Also, putting enum values in instances of that struct, as suggested, increases kernel size, for IMHO no additional gain. If there is more data to put in the struct, I agree it makes sense to use a struct. > > So, I would like to fix the issue by using the cast of uintptr_t. > > Sure. I appreciate that this would be more work. When you send your > patch, can you include an update to the iproc driver (and credit the > original author from [1])? I would appreciate it. > > 1. https://lore.kernel.org/linux-pci/20230814230008.GA196797@bhelgaas/ Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/3] PCI: dwc: Improve code readability 2023-11-13 12:57 ` Geert Uytterhoeven @ 2023-11-13 13:18 ` Krzysztof Wilczyński 0 siblings, 0 replies; 16+ messages in thread From: Krzysztof Wilczyński @ 2023-11-13 13:18 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Yoshihiro Shimoda, lpieralisi, robh, bhelgaas, jingoohan1, gustavo.pimentel, mani, minghuan.Lian, mingkai.hu, roy.zang, marek.vasut+renesas, linux-pci, linuxppc-dev, linux-renesas-soc Hello, [...] > > > I confirmed that the uintptr_t fixed the issue. > > > > We declined a similar fix in the past[1] ... > > > > > I also think that adding a new struct with the mode is overkill. > > > > ... with the hopes that a driver could drop the switch statements in place > > of using the other pattern. Also, to be consistent with other drivers that > > do this already. > > Note that the issue of casting is something we cannot fix easily: > some *_device_id structs use "kernel_ulong_t" for the "data" member, > others use "void *". > > git grep -W "_device_id" -- include/linux/mod_devicetable.h | grep data > > In addition, several drivers use multiple types of device IDs, so you > cannot settle on one type to avoid casts. > > Also, putting enum values in instances of that struct, as suggested, > increases kernel size, for IMHO no additional gain. All good points! Thank you for taking the time to get back to me. Appreciated. :) > If there is more data to put in the struct, I agree it makes sense to use > a struct. Yeah. Perhaps if there is such a need in the future, indeed. Krzysztof ^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH 0/3] PCI: dwc: Improve code readability 2023-11-13 12:22 ` Krzysztof Wilczyński 2023-11-13 12:57 ` Geert Uytterhoeven @ 2023-11-14 0:20 ` Yoshihiro Shimoda 1 sibling, 0 replies; 16+ messages in thread From: Yoshihiro Shimoda @ 2023-11-14 0:20 UTC (permalink / raw) To: Krzysztof Wilczyński Cc: Geert Uytterhoeven, lpieralisi, robh, bhelgaas, jingoohan1, gustavo.pimentel, mani, minghuan.Lian, mingkai.hu, roy.zang, marek.vasut+renesas, linux-pci, linuxppc-dev, linux-renesas-soc Hello, > From: Krzysztof Wilczyński, Sent: Monday, November 13, 2023 9:22 PM > > [...] > > > > Now, while you are looking at things, can you also take care about the following: > > > > > > > > drivers/pci/controller/dwc/pcie-rcar-gen4.c:439:15: warning: cast to smaller integer type 'enum > dw_pcie_device_mode' > > > from 'const void *' [-Wvoid-pointer-to-enum-cast] > > > > Thank you for the report! > > > > > > This requires adding structs for each data member of the of_device_id type. > > > > > > That sounds like overkill to me. > > > An intermediate cast to uintptr_t should fix the issue as well. > > > > I confirmed that the uintptr_t fixed the issue. > > We declined a similar fix in the past[1] ... > > > I also think that adding a new struct with the mode is overkill. > > ... with the hopes that a driver could drop the switch statements in place > of using the other pattern. Also, to be consistent with other drivers that > do this already. > > > So, I would like to fix the issue by using the cast of uintptr_t. > > Sure. I appreciate that this would be more work. When you send your > patch, can you include an update to the iproc driver (and credit the > original author from [1])? I would appreciate it. > > 1. https://lore.kernel.org/linux-pci/20230814230008.GA196797@bhelgaas/ I got it. I'll include the following patch on v2. https://lore.kernel.org/linux-pci/20230814-void-drivers-pci-controller-pcie-iproc-platform-v1-1-81a121607851@google.com/ Best regards, Yoshihiro Shimoda > > Krzysztof ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2023-11-14 1:29 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-13 1:32 [PATCH 0/3] PCI: dwc: Improve code readability Yoshihiro Shimoda
2023-11-13 1:32 ` [PATCH 1/3] PCI: dwc: Rename to .init in struct dw_pcie_ep_ops Yoshihiro Shimoda
2023-11-13 10:14 ` Serge Semin
2023-11-13 11:49 ` Yoshihiro Shimoda
2023-11-13 1:32 ` [PATCH 2/3] PCI: dwc: Rename to .get_dbi_offset " Yoshihiro Shimoda
2023-11-13 10:34 ` Serge Semin
2023-11-13 1:33 ` [PATCH 3/3] PCI: dwc: Add dw_pcie_ep_{read,write}_dbi[2] helpers Yoshihiro Shimoda
2023-11-13 12:41 ` Serge Semin
2023-11-14 1:29 ` Yoshihiro Shimoda
2023-11-13 10:09 ` [PATCH 0/3] PCI: dwc: Improve code readability Krzysztof Wilczyński
2023-11-13 11:07 ` Geert Uytterhoeven
2023-11-13 11:47 ` Yoshihiro Shimoda
2023-11-13 12:22 ` Krzysztof Wilczyński
2023-11-13 12:57 ` Geert Uytterhoeven
2023-11-13 13:18 ` Krzysztof Wilczyński
2023-11-14 0:20 ` 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).