* [PATCH v2 0/8] PCI: hisi: Cleanups
@ 2016-10-12 13:42 Bjorn Helgaas
2016-10-12 13:42 ` [PATCH v2 1/8] PCI: hisi: Add local struct device pointers Bjorn Helgaas
` (9 more replies)
0 siblings, 10 replies; 12+ messages in thread
From: Bjorn Helgaas @ 2016-10-12 13:42 UTC (permalink / raw)
To: Zhou Wang, Gabriele Paoloni; +Cc: linux-pci
- Add local "dev" pointers to reduce repetition of things like
"&pdev->dev".
- Remove platform drvdata because it appears unused (we called
platform_set_drvdata() but not platform_get_drvdata()).
- Name private struct pointer consistently within driver.
- Remove redundant struct members.
- Use generic DesignWare accessors when possible.
- Pass device-specific struct to internal functions for consistency.
- Include register block base in PCIE_SYS_STATE4 address to simplify
users.
Nothing here should change the behavior of the driver.
Changes from v1:
I dropped the following patches because they were a lot of churn for
questionable benefit:
PCI: hisi: Rename APB accessors
PCI: hisi: Rename config accessors
PCI: hisi: Name private struct pointer "hisi" consistently
(Instead of renaming *all* the pointers, I only renamed enough to
make them consistent within this file.)
PCI: hisi: Swap order of hisi_apb_writel() reg/val arguments
---
Bjorn Helgaas (8):
PCI: hisi: Add local struct device pointers
PCI: hisi: Remove unused platform data
PCI: hisi: Name private struct pointer "hisi_pcie" consistently
PCI: hisi: Remove redundant struct hisi_pcie.reg_base
PCI: hisi: Use generic DesignWare accessors
PCI: hisi: Include register block base in PCIE_SYS_STATE4 address
PCI: hisi: Pass device-specific struct to internal functions
PCI: hisi: Reorder struct hisi_pcie
drivers/pci/host/pcie-hisi.c | 86 +++++++++++++++++-------------------------
1 file changed, 35 insertions(+), 51 deletions(-)
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 1/8] PCI: hisi: Add local struct device pointers
2016-10-12 13:42 [PATCH v2 0/8] PCI: hisi: Cleanups Bjorn Helgaas
@ 2016-10-12 13:42 ` Bjorn Helgaas
2016-10-12 13:42 ` [PATCH v2 2/8] PCI: hisi: Remove unused platform data Bjorn Helgaas
` (8 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Bjorn Helgaas @ 2016-10-12 13:42 UTC (permalink / raw)
To: Zhou Wang, Gabriele Paoloni; +Cc: linux-pci
Use a local "struct device *dev" for brevity and consistency with other
drivers. No functional change intended.
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/pci/host/pcie-hisi.c | 26 ++++++++++++++------------
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/drivers/pci/host/pcie-hisi.c b/drivers/pci/host/pcie-hisi.c
index 7ee9dfc..1c08028 100644
--- a/drivers/pci/host/pcie-hisi.c
+++ b/drivers/pci/host/pcie-hisi.c
@@ -143,16 +143,17 @@ static struct pcie_host_ops hisi_pcie_host_ops = {
static int hisi_add_pcie_port(struct pcie_port *pp,
struct platform_device *pdev)
{
+ struct device *dev = pp->dev;
int ret;
u32 port_id;
struct hisi_pcie *hisi_pcie = to_hisi_pcie(pp);
- if (of_property_read_u32(pdev->dev.of_node, "port-id", &port_id)) {
- dev_err(&pdev->dev, "failed to read port-id\n");
+ if (of_property_read_u32(dev->of_node, "port-id", &port_id)) {
+ dev_err(dev, "failed to read port-id\n");
return -EINVAL;
}
if (port_id > 3) {
- dev_err(&pdev->dev, "Invalid port-id: %d\n", port_id);
+ dev_err(dev, "Invalid port-id: %d\n", port_id);
return -EINVAL;
}
hisi_pcie->port_id = port_id;
@@ -161,7 +162,7 @@ static int hisi_add_pcie_port(struct pcie_port *pp,
ret = dw_pcie_host_init(pp);
if (ret) {
- dev_err(&pdev->dev, "failed to initialize host\n");
+ dev_err(dev, "failed to initialize host\n");
return ret;
}
@@ -170,6 +171,7 @@ static int hisi_add_pcie_port(struct pcie_port *pp,
static int hisi_pcie_probe(struct platform_device *pdev)
{
+ struct device *dev = &pdev->dev;
struct hisi_pcie *hisi_pcie;
struct pcie_port *pp;
const struct of_device_id *match;
@@ -177,28 +179,28 @@ static int hisi_pcie_probe(struct platform_device *pdev)
struct device_driver *driver;
int ret;
- hisi_pcie = devm_kzalloc(&pdev->dev, sizeof(*hisi_pcie), GFP_KERNEL);
+ hisi_pcie = devm_kzalloc(dev, sizeof(*hisi_pcie), GFP_KERNEL);
if (!hisi_pcie)
return -ENOMEM;
pp = &hisi_pcie->pp;
- pp->dev = &pdev->dev;
- driver = (pdev->dev).driver;
+ pp->dev = dev;
+ driver = dev->driver;
- match = of_match_device(driver->of_match_table, &pdev->dev);
+ match = of_match_device(driver->of_match_table, dev);
hisi_pcie->soc_ops = (struct pcie_soc_ops *) match->data;
hisi_pcie->subctrl =
syscon_regmap_lookup_by_compatible("hisilicon,pcie-sas-subctrl");
if (IS_ERR(hisi_pcie->subctrl)) {
- dev_err(pp->dev, "cannot get subctrl base\n");
+ dev_err(dev, "cannot get subctrl base\n");
return PTR_ERR(hisi_pcie->subctrl);
}
reg = platform_get_resource_byname(pdev, IORESOURCE_MEM, "rc_dbi");
- hisi_pcie->reg_base = devm_ioremap_resource(&pdev->dev, reg);
+ hisi_pcie->reg_base = devm_ioremap_resource(dev, reg);
if (IS_ERR(hisi_pcie->reg_base)) {
- dev_err(pp->dev, "cannot get rc_dbi base\n");
+ dev_err(dev, "cannot get rc_dbi base\n");
return PTR_ERR(hisi_pcie->reg_base);
}
@@ -210,7 +212,7 @@ static int hisi_pcie_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, hisi_pcie);
- dev_warn(pp->dev, "only 32-bit config accesses supported; smaller writes may corrupt adjacent RW1C fields\n");
+ dev_warn(dev, "only 32-bit config accesses supported; smaller writes may corrupt adjacent RW1C fields\n");
return 0;
}
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 2/8] PCI: hisi: Remove unused platform data
2016-10-12 13:42 [PATCH v2 0/8] PCI: hisi: Cleanups Bjorn Helgaas
2016-10-12 13:42 ` [PATCH v2 1/8] PCI: hisi: Add local struct device pointers Bjorn Helgaas
@ 2016-10-12 13:42 ` Bjorn Helgaas
2016-10-12 13:42 ` [PATCH v2 3/8] PCI: hisi: Name private struct pointer "hisi_pcie" consistently Bjorn Helgaas
` (7 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Bjorn Helgaas @ 2016-10-12 13:42 UTC (permalink / raw)
To: Zhou Wang, Gabriele Paoloni; +Cc: linux-pci
The hisi driver never uses the platform drvdata pointer, so don't bother
setting it. No functional change intended.
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/pci/host/pcie-hisi.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/pci/host/pcie-hisi.c b/drivers/pci/host/pcie-hisi.c
index 1c08028..d4a5812 100644
--- a/drivers/pci/host/pcie-hisi.c
+++ b/drivers/pci/host/pcie-hisi.c
@@ -210,8 +210,6 @@ static int hisi_pcie_probe(struct platform_device *pdev)
if (ret)
return ret;
- platform_set_drvdata(pdev, hisi_pcie);
-
dev_warn(dev, "only 32-bit config accesses supported; smaller writes may corrupt adjacent RW1C fields\n");
return 0;
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 3/8] PCI: hisi: Name private struct pointer "hisi_pcie" consistently
2016-10-12 13:42 [PATCH v2 0/8] PCI: hisi: Cleanups Bjorn Helgaas
2016-10-12 13:42 ` [PATCH v2 1/8] PCI: hisi: Add local struct device pointers Bjorn Helgaas
2016-10-12 13:42 ` [PATCH v2 2/8] PCI: hisi: Remove unused platform data Bjorn Helgaas
@ 2016-10-12 13:42 ` Bjorn Helgaas
2016-10-12 13:42 ` [PATCH v2 4/8] PCI: hisi: Remove redundant struct hisi_pcie.reg_base Bjorn Helgaas
` (6 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Bjorn Helgaas @ 2016-10-12 13:42 UTC (permalink / raw)
To: Zhou Wang, Gabriele Paoloni; +Cc: linux-pci
Most struct hisi_pcie pointers are already called "hisi_pcie". Change
the rest of them to match. No functional change intended.
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/pci/host/pcie-hisi.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/pci/host/pcie-hisi.c b/drivers/pci/host/pcie-hisi.c
index d4a5812..28c95b8 100644
--- a/drivers/pci/host/pcie-hisi.c
+++ b/drivers/pci/host/pcie-hisi.c
@@ -33,7 +33,7 @@
struct hisi_pcie;
struct pcie_soc_ops {
- int (*hisi_pcie_link_up)(struct hisi_pcie *pcie);
+ int (*hisi_pcie_link_up)(struct hisi_pcie *hisi_pcie);
};
struct hisi_pcie {
@@ -44,15 +44,15 @@ struct hisi_pcie {
struct pcie_soc_ops *soc_ops;
};
-static inline void hisi_pcie_apb_writel(struct hisi_pcie *pcie,
+static inline void hisi_pcie_apb_writel(struct hisi_pcie *hisi_pcie,
u32 val, u32 reg)
{
- writel(val, pcie->reg_base + reg);
+ writel(val, hisi_pcie->reg_base + reg);
}
-static inline u32 hisi_pcie_apb_readl(struct hisi_pcie *pcie, u32 reg)
+static inline u32 hisi_pcie_apb_readl(struct hisi_pcie *hisi_pcie, u32 reg)
{
- return readl(pcie->reg_base + reg);
+ return readl(hisi_pcie->reg_base + reg);
}
/* HipXX PCIe host only supports 32-bit config access */
@@ -61,12 +61,12 @@ static int hisi_pcie_cfg_read(struct pcie_port *pp, int where, int size,
{
u32 reg;
u32 reg_val;
- struct hisi_pcie *pcie = to_hisi_pcie(pp);
+ struct hisi_pcie *hisi_pcie = to_hisi_pcie(pp);
void *walker = ®_val;
walker += (where & 0x3);
reg = where & ~0x3;
- reg_val = hisi_pcie_apb_readl(pcie, reg);
+ reg_val = hisi_pcie_apb_readl(hisi_pcie, reg);
if (size == 1)
*val = *(u8 __force *) walker;
@@ -86,21 +86,21 @@ static int hisi_pcie_cfg_write(struct pcie_port *pp, int where, int size,
{
u32 reg_val;
u32 reg;
- struct hisi_pcie *pcie = to_hisi_pcie(pp);
+ struct hisi_pcie *hisi_pcie = to_hisi_pcie(pp);
void *walker = ®_val;
walker += (where & 0x3);
reg = where & ~0x3;
if (size == 4)
- hisi_pcie_apb_writel(pcie, val, reg);
+ hisi_pcie_apb_writel(hisi_pcie, val, reg);
else if (size == 2) {
- reg_val = hisi_pcie_apb_readl(pcie, reg);
+ reg_val = hisi_pcie_apb_readl(hisi_pcie, reg);
*(u16 __force *) walker = val;
- hisi_pcie_apb_writel(pcie, reg_val, reg);
+ hisi_pcie_apb_writel(hisi_pcie, reg_val, reg);
} else if (size == 1) {
- reg_val = hisi_pcie_apb_readl(pcie, reg);
+ reg_val = hisi_pcie_apb_readl(hisi_pcie, reg);
*(u8 __force *) walker = val;
- hisi_pcie_apb_writel(pcie, reg_val, reg);
+ hisi_pcie_apb_writel(hisi_pcie, reg_val, reg);
} else
return PCIBIOS_BAD_REGISTER_NUMBER;
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 4/8] PCI: hisi: Remove redundant struct hisi_pcie.reg_base
2016-10-12 13:42 [PATCH v2 0/8] PCI: hisi: Cleanups Bjorn Helgaas
` (2 preceding siblings ...)
2016-10-12 13:42 ` [PATCH v2 3/8] PCI: hisi: Name private struct pointer "hisi_pcie" consistently Bjorn Helgaas
@ 2016-10-12 13:42 ` Bjorn Helgaas
2016-10-12 13:43 ` [PATCH v2 5/8] PCI: hisi: Use generic DesignWare accessors Bjorn Helgaas
` (5 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Bjorn Helgaas @ 2016-10-12 13:42 UTC (permalink / raw)
To: Zhou Wang, Gabriele Paoloni; +Cc: linux-pci
Remove the struct hisi_pcie.reg_base member, which is a duplicate of the
generic pp.dbi_base member. No functional change intended.
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/pci/host/pcie-hisi.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/pci/host/pcie-hisi.c b/drivers/pci/host/pcie-hisi.c
index 28c95b8..f2bb206 100644
--- a/drivers/pci/host/pcie-hisi.c
+++ b/drivers/pci/host/pcie-hisi.c
@@ -38,7 +38,6 @@ struct pcie_soc_ops {
struct hisi_pcie {
struct regmap *subctrl;
- void __iomem *reg_base;
u32 port_id;
struct pcie_port pp;
struct pcie_soc_ops *soc_ops;
@@ -47,12 +46,12 @@ struct hisi_pcie {
static inline void hisi_pcie_apb_writel(struct hisi_pcie *hisi_pcie,
u32 val, u32 reg)
{
- writel(val, hisi_pcie->reg_base + reg);
+ writel(val, hisi_pcie->pp.dbi_base + reg);
}
static inline u32 hisi_pcie_apb_readl(struct hisi_pcie *hisi_pcie, u32 reg)
{
- return readl(hisi_pcie->reg_base + reg);
+ return readl(hisi_pcie->pp.dbi_base + reg);
}
/* HipXX PCIe host only supports 32-bit config access */
@@ -198,14 +197,12 @@ static int hisi_pcie_probe(struct platform_device *pdev)
}
reg = platform_get_resource_byname(pdev, IORESOURCE_MEM, "rc_dbi");
- hisi_pcie->reg_base = devm_ioremap_resource(dev, reg);
- if (IS_ERR(hisi_pcie->reg_base)) {
+ pp->dbi_base = devm_ioremap_resource(dev, reg);
+ if (IS_ERR(pp->dbi_base)) {
dev_err(dev, "cannot get rc_dbi base\n");
- return PTR_ERR(hisi_pcie->reg_base);
+ return PTR_ERR(pp->dbi_base);
}
- hisi_pcie->pp.dbi_base = hisi_pcie->reg_base;
-
ret = hisi_add_pcie_port(pp, pdev);
if (ret)
return ret;
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 5/8] PCI: hisi: Use generic DesignWare accessors
2016-10-12 13:42 [PATCH v2 0/8] PCI: hisi: Cleanups Bjorn Helgaas
` (3 preceding siblings ...)
2016-10-12 13:42 ` [PATCH v2 4/8] PCI: hisi: Remove redundant struct hisi_pcie.reg_base Bjorn Helgaas
@ 2016-10-12 13:43 ` Bjorn Helgaas
2016-10-12 13:43 ` [PATCH v2 6/8] PCI: hisi: Include register block base in PCIE_SYS_STATE4 address Bjorn Helgaas
` (4 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Bjorn Helgaas @ 2016-10-12 13:43 UTC (permalink / raw)
To: Zhou Wang, Gabriele Paoloni; +Cc: linux-pci
The dw_pcie_readl_rc() and dw_pcie_writel_rc() interfaces already add in
pp->dbi_base, so use those instead of doing it ourselves in the hisi
driver. No functional change intended.
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/pci/host/pcie-hisi.c | 29 ++++++++---------------------
1 file changed, 8 insertions(+), 21 deletions(-)
diff --git a/drivers/pci/host/pcie-hisi.c b/drivers/pci/host/pcie-hisi.c
index f2bb206..d0e081b 100644
--- a/drivers/pci/host/pcie-hisi.c
+++ b/drivers/pci/host/pcie-hisi.c
@@ -43,29 +43,17 @@ struct hisi_pcie {
struct pcie_soc_ops *soc_ops;
};
-static inline void hisi_pcie_apb_writel(struct hisi_pcie *hisi_pcie,
- u32 val, u32 reg)
-{
- writel(val, hisi_pcie->pp.dbi_base + reg);
-}
-
-static inline u32 hisi_pcie_apb_readl(struct hisi_pcie *hisi_pcie, u32 reg)
-{
- return readl(hisi_pcie->pp.dbi_base + reg);
-}
-
/* HipXX PCIe host only supports 32-bit config access */
static int hisi_pcie_cfg_read(struct pcie_port *pp, int where, int size,
u32 *val)
{
u32 reg;
u32 reg_val;
- struct hisi_pcie *hisi_pcie = to_hisi_pcie(pp);
void *walker = ®_val;
walker += (where & 0x3);
reg = where & ~0x3;
- reg_val = hisi_pcie_apb_readl(hisi_pcie, reg);
+ reg_val = dw_pcie_readl_rc(pp, reg);
if (size == 1)
*val = *(u8 __force *) walker;
@@ -85,21 +73,20 @@ static int hisi_pcie_cfg_write(struct pcie_port *pp, int where, int size,
{
u32 reg_val;
u32 reg;
- struct hisi_pcie *hisi_pcie = to_hisi_pcie(pp);
void *walker = ®_val;
walker += (where & 0x3);
reg = where & ~0x3;
if (size == 4)
- hisi_pcie_apb_writel(hisi_pcie, val, reg);
+ dw_pcie_writel_rc(pp, reg, val);
else if (size == 2) {
- reg_val = hisi_pcie_apb_readl(hisi_pcie, reg);
+ reg_val = dw_pcie_readl_rc(pp, reg);
*(u16 __force *) walker = val;
- hisi_pcie_apb_writel(hisi_pcie, reg_val, reg);
+ dw_pcie_writel_rc(pp, reg, reg_val);
} else if (size == 1) {
- reg_val = hisi_pcie_apb_readl(hisi_pcie, reg);
+ reg_val = dw_pcie_readl_rc(pp, reg);
*(u8 __force *) walker = val;
- hisi_pcie_apb_writel(hisi_pcie, reg_val, reg);
+ dw_pcie_writel_rc(pp, reg, reg_val);
} else
return PCIBIOS_BAD_REGISTER_NUMBER;
@@ -118,10 +105,10 @@ static int hisi_pcie_link_up_hip05(struct hisi_pcie *hisi_pcie)
static int hisi_pcie_link_up_hip06(struct hisi_pcie *hisi_pcie)
{
+ struct pcie_port *pp = &hisi_pcie->pp;
u32 val;
- val = hisi_pcie_apb_readl(hisi_pcie, PCIE_HIP06_CTRL_OFF +
- PCIE_SYS_STATE4);
+ val = dw_pcie_readl_rc(pp, PCIE_HIP06_CTRL_OFF + PCIE_SYS_STATE4);
return ((val & PCIE_LTSSM_STATE_MASK) == PCIE_LTSSM_LINKUP_STATE);
}
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 6/8] PCI: hisi: Include register block base in PCIE_SYS_STATE4 address
2016-10-12 13:42 [PATCH v2 0/8] PCI: hisi: Cleanups Bjorn Helgaas
` (4 preceding siblings ...)
2016-10-12 13:43 ` [PATCH v2 5/8] PCI: hisi: Use generic DesignWare accessors Bjorn Helgaas
@ 2016-10-12 13:43 ` Bjorn Helgaas
2016-10-12 13:43 ` [PATCH v2 7/8] PCI: hisi: Pass device-specific struct to internal functions Bjorn Helgaas
` (3 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Bjorn Helgaas @ 2016-10-12 13:43 UTC (permalink / raw)
To: Zhou Wang, Gabriele Paoloni; +Cc: linux-pci
Include the PCIE_HIP06_CTRL_OFF block base in the PCIE_SYS_STATE4 register
address so reads of PCIE_SYS_STATE4 don't have to mention both. No
functional change intended.
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/pci/host/pcie-hisi.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/pci/host/pcie-hisi.c b/drivers/pci/host/pcie-hisi.c
index d0e081b..b508aae 100644
--- a/drivers/pci/host/pcie-hisi.c
+++ b/drivers/pci/host/pcie-hisi.c
@@ -22,11 +22,11 @@
#include "pcie-designware.h"
-#define PCIE_LTSSM_LINKUP_STATE 0x11
-#define PCIE_LTSSM_STATE_MASK 0x3F
-#define PCIE_SUBCTRL_SYS_STATE4_REG 0x6818
-#define PCIE_SYS_STATE4 0x31c
-#define PCIE_HIP06_CTRL_OFF 0x1000
+#define PCIE_SUBCTRL_SYS_STATE4_REG 0x6818
+#define PCIE_HIP06_CTRL_OFF 0x1000
+#define PCIE_SYS_STATE4 (PCIE_HIP06_CTRL_OFF + 0x31c)
+#define PCIE_LTSSM_LINKUP_STATE 0x11
+#define PCIE_LTSSM_STATE_MASK 0x3F
#define to_hisi_pcie(x) container_of(x, struct hisi_pcie, pp)
@@ -108,7 +108,7 @@ static int hisi_pcie_link_up_hip06(struct hisi_pcie *hisi_pcie)
struct pcie_port *pp = &hisi_pcie->pp;
u32 val;
- val = dw_pcie_readl_rc(pp, PCIE_HIP06_CTRL_OFF + PCIE_SYS_STATE4);
+ val = dw_pcie_readl_rc(pp, PCIE_SYS_STATE4);
return ((val & PCIE_LTSSM_STATE_MASK) == PCIE_LTSSM_LINKUP_STATE);
}
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 7/8] PCI: hisi: Pass device-specific struct to internal functions
2016-10-12 13:42 [PATCH v2 0/8] PCI: hisi: Cleanups Bjorn Helgaas
` (5 preceding siblings ...)
2016-10-12 13:43 ` [PATCH v2 6/8] PCI: hisi: Include register block base in PCIE_SYS_STATE4 address Bjorn Helgaas
@ 2016-10-12 13:43 ` Bjorn Helgaas
2016-10-12 13:43 ` [PATCH v2 8/8] PCI: hisi: Reorder struct hisi_pcie Bjorn Helgaas
` (2 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Bjorn Helgaas @ 2016-10-12 13:43 UTC (permalink / raw)
To: Zhou Wang, Gabriele Paoloni; +Cc: linux-pci
Only interfaces used from outside the driver, e.g., those called by the
DesignWare core, need to accept pointers to the generic struct pcie_port.
Internal interfaces can accept pointers to the device-specific struct,
which makes them more straightforward. No functional change intended.
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/pci/host/pcie-hisi.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/host/pcie-hisi.c b/drivers/pci/host/pcie-hisi.c
index b508aae..bfff666 100644
--- a/drivers/pci/host/pcie-hisi.c
+++ b/drivers/pci/host/pcie-hisi.c
@@ -126,13 +126,13 @@ static struct pcie_host_ops hisi_pcie_host_ops = {
.link_up = hisi_pcie_link_up,
};
-static int hisi_add_pcie_port(struct pcie_port *pp,
- struct platform_device *pdev)
+static int hisi_add_pcie_port(struct hisi_pcie *hisi_pcie,
+ struct platform_device *pdev)
{
+ struct pcie_port *pp = &hisi_pcie->pp;
struct device *dev = pp->dev;
int ret;
u32 port_id;
- struct hisi_pcie *hisi_pcie = to_hisi_pcie(pp);
if (of_property_read_u32(dev->of_node, "port-id", &port_id)) {
dev_err(dev, "failed to read port-id\n");
@@ -190,7 +190,7 @@ static int hisi_pcie_probe(struct platform_device *pdev)
return PTR_ERR(pp->dbi_base);
}
- ret = hisi_add_pcie_port(pp, pdev);
+ ret = hisi_add_pcie_port(hisi_pcie, pdev);
if (ret)
return ret;
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 8/8] PCI: hisi: Reorder struct hisi_pcie
2016-10-12 13:42 [PATCH v2 0/8] PCI: hisi: Cleanups Bjorn Helgaas
` (6 preceding siblings ...)
2016-10-12 13:43 ` [PATCH v2 7/8] PCI: hisi: Pass device-specific struct to internal functions Bjorn Helgaas
@ 2016-10-12 13:43 ` Bjorn Helgaas
2016-10-12 16:05 ` [PATCH v2 0/8] PCI: hisi: Cleanups Bjorn Helgaas
2016-10-14 2:17 ` Gabriele Paoloni
9 siblings, 0 replies; 12+ messages in thread
From: Bjorn Helgaas @ 2016-10-12 13:43 UTC (permalink / raw)
To: Zhou Wang, Gabriele Paoloni; +Cc: linux-pci
Reorder struct hisi_pcie to put generic fields first. No functional change
intended.
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/pci/host/pcie-hisi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/host/pcie-hisi.c b/drivers/pci/host/pcie-hisi.c
index bfff666..56154c2 100644
--- a/drivers/pci/host/pcie-hisi.c
+++ b/drivers/pci/host/pcie-hisi.c
@@ -37,9 +37,9 @@ struct pcie_soc_ops {
};
struct hisi_pcie {
+ struct pcie_port pp; /* pp.dbi_base is DT rc_dbi */
struct regmap *subctrl;
u32 port_id;
- struct pcie_port pp;
struct pcie_soc_ops *soc_ops;
};
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 0/8] PCI: hisi: Cleanups
2016-10-12 13:42 [PATCH v2 0/8] PCI: hisi: Cleanups Bjorn Helgaas
` (7 preceding siblings ...)
2016-10-12 13:43 ` [PATCH v2 8/8] PCI: hisi: Reorder struct hisi_pcie Bjorn Helgaas
@ 2016-10-12 16:05 ` Bjorn Helgaas
2016-10-14 2:19 ` Gabriele Paoloni
2016-10-14 2:17 ` Gabriele Paoloni
9 siblings, 1 reply; 12+ messages in thread
From: Bjorn Helgaas @ 2016-10-12 16:05 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: Zhou Wang, Gabriele Paoloni, linux-pci
On Wed, Oct 12, 2016 at 08:42:27AM -0500, Bjorn Helgaas wrote:
> - Add local "dev" pointers to reduce repetition of things like
> "&pdev->dev".
>
> - Remove platform drvdata because it appears unused (we called
> platform_set_drvdata() but not platform_get_drvdata()).
>
> - Name private struct pointer consistently within driver.
>
> - Remove redundant struct members.
>
> - Use generic DesignWare accessors when possible.
>
> - Pass device-specific struct to internal functions for consistency.
>
> - Include register block base in PCIE_SYS_STATE4 address to simplify
> users.
>
> Nothing here should change the behavior of the driver.
>
> Changes from v1:
> I dropped the following patches because they were a lot of churn for
> questionable benefit:
> PCI: hisi: Rename APB accessors
> PCI: hisi: Rename config accessors
> PCI: hisi: Name private struct pointer "hisi" consistently
> (Instead of renaming *all* the pointers, I only renamed enough to
> make them consistent within this file.)
> PCI: hisi: Swap order of hisi_apb_writel() reg/val arguments
>
> ---
>
> Bjorn Helgaas (8):
> PCI: hisi: Add local struct device pointers
> PCI: hisi: Remove unused platform data
> PCI: hisi: Name private struct pointer "hisi_pcie" consistently
> PCI: hisi: Remove redundant struct hisi_pcie.reg_base
> PCI: hisi: Use generic DesignWare accessors
> PCI: hisi: Include register block base in PCIE_SYS_STATE4 address
> PCI: hisi: Pass device-specific struct to internal functions
> PCI: hisi: Reorder struct hisi_pcie
>
>
> drivers/pci/host/pcie-hisi.c | 86 +++++++++++++++++-------------------------
> 1 file changed, 35 insertions(+), 51 deletions(-)
I applied these to pci/host-hisi for v4.9. I hope to ask Linus to
pull them tomorrow, so if you see any issues, let me know soon.
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH v2 0/8] PCI: hisi: Cleanups
2016-10-12 13:42 [PATCH v2 0/8] PCI: hisi: Cleanups Bjorn Helgaas
` (8 preceding siblings ...)
2016-10-12 16:05 ` [PATCH v2 0/8] PCI: hisi: Cleanups Bjorn Helgaas
@ 2016-10-14 2:17 ` Gabriele Paoloni
9 siblings, 0 replies; 12+ messages in thread
From: Gabriele Paoloni @ 2016-10-14 2:17 UTC (permalink / raw)
To: Bjorn Helgaas, Wangzhou (B); +Cc: linux-pci
SGkgQmpvcm4NCg0KPiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBsaW51eC1w
Y2ktb3duZXJAdmdlci5rZXJuZWwub3JnIFttYWlsdG86bGludXgtcGNpLQ0KPiBvd25lckB2Z2Vy
Lmtlcm5lbC5vcmddIE9uIEJlaGFsZiBPZiBCam9ybiBIZWxnYWFzDQo+IFNlbnQ6IDEyIE9jdG9i
ZXIgMjAxNiAyMTo0Mg0KPiBUbzogV2FuZ3pob3UgKEIpOyBHYWJyaWVsZSBQYW9sb25pDQo+IENj
OiBsaW51eC1wY2lAdmdlci5rZXJuZWwub3JnDQo+IFN1YmplY3Q6IFtQQVRDSCB2MiAwLzhdIFBD
STogaGlzaTogQ2xlYW51cHMNCj4gDQo+ICAgLSBBZGQgbG9jYWwgImRldiIgcG9pbnRlcnMgdG8g
cmVkdWNlIHJlcGV0aXRpb24gb2YgdGhpbmdzIGxpa2UNCj4gICAgICImcGRldi0+ZGV2Ii4NCj4g
DQo+ICAgLSBSZW1vdmUgcGxhdGZvcm0gZHJ2ZGF0YSBiZWNhdXNlIGl0IGFwcGVhcnMgdW51c2Vk
ICh3ZSBjYWxsZWQNCj4gICAgIHBsYXRmb3JtX3NldF9kcnZkYXRhKCkgYnV0IG5vdCBwbGF0Zm9y
bV9nZXRfZHJ2ZGF0YSgpKS4NCj4gDQo+ICAgLSBOYW1lIHByaXZhdGUgc3RydWN0IHBvaW50ZXIg
Y29uc2lzdGVudGx5IHdpdGhpbiBkcml2ZXIuDQo+IA0KPiAgIC0gUmVtb3ZlIHJlZHVuZGFudCBz
dHJ1Y3QgbWVtYmVycy4NCj4gDQo+ICAgLSBVc2UgZ2VuZXJpYyBEZXNpZ25XYXJlIGFjY2Vzc29y
cyB3aGVuIHBvc3NpYmxlLg0KPiANCj4gICAtIFBhc3MgZGV2aWNlLXNwZWNpZmljIHN0cnVjdCB0
byBpbnRlcm5hbCBmdW5jdGlvbnMgZm9yIGNvbnNpc3RlbmN5Lg0KPiANCj4gICAtIEluY2x1ZGUg
cmVnaXN0ZXIgYmxvY2sgYmFzZSBpbiBQQ0lFX1NZU19TVEFURTQgYWRkcmVzcyB0byBzaW1wbGlm
eQ0KPiAgICAgdXNlcnMuDQo+IA0KPiBOb3RoaW5nIGhlcmUgc2hvdWxkIGNoYW5nZSB0aGUgYmVo
YXZpb3Igb2YgdGhlIGRyaXZlci4NCj4gDQo+IENoYW5nZXMgZnJvbSB2MToNCj4gICBJIGRyb3Bw
ZWQgdGhlIGZvbGxvd2luZyBwYXRjaGVzIGJlY2F1c2UgdGhleSB3ZXJlIGEgbG90IG9mIGNodXJu
IGZvcg0KPiAgIHF1ZXN0aW9uYWJsZSBiZW5lZml0Og0KPiAgICAgUENJOiBoaXNpOiBSZW5hbWUg
QVBCIGFjY2Vzc29ycw0KPiAgICAgUENJOiBoaXNpOiBSZW5hbWUgY29uZmlnIGFjY2Vzc29ycw0K
PiAgICAgUENJOiBoaXNpOiBOYW1lIHByaXZhdGUgc3RydWN0IHBvaW50ZXIgImhpc2kiIGNvbnNp
c3RlbnRseQ0KPiAgICAgICAoSW5zdGVhZCBvZiByZW5hbWluZyAqYWxsKiB0aGUgcG9pbnRlcnMs
IEkgb25seSByZW5hbWVkIGVub3VnaCB0bw0KPiAgICAgICBtYWtlIHRoZW0gY29uc2lzdGVudCB3
aXRoaW4gdGhpcyBmaWxlLikNCj4gICAgIFBDSTogaGlzaTogU3dhcCBvcmRlciBvZiBoaXNpX2Fw
Yl93cml0ZWwoKSByZWcvdmFsIGFyZ3VtZW50cw0KPiANCj4gLS0tDQo+IA0KPiBCam9ybiBIZWxn
YWFzICg4KToNCj4gICAgICAgUENJOiBoaXNpOiBBZGQgbG9jYWwgc3RydWN0IGRldmljZSBwb2lu
dGVycw0KPiAgICAgICBQQ0k6IGhpc2k6IFJlbW92ZSB1bnVzZWQgcGxhdGZvcm0gZGF0YQ0KPiAg
ICAgICBQQ0k6IGhpc2k6IE5hbWUgcHJpdmF0ZSBzdHJ1Y3QgcG9pbnRlciAiaGlzaV9wY2llIiBj
b25zaXN0ZW50bHkNCj4gICAgICAgUENJOiBoaXNpOiBSZW1vdmUgcmVkdW5kYW50IHN0cnVjdCBo
aXNpX3BjaWUucmVnX2Jhc2UNCj4gICAgICAgUENJOiBoaXNpOiBVc2UgZ2VuZXJpYyBEZXNpZ25X
YXJlIGFjY2Vzc29ycw0KPiAgICAgICBQQ0k6IGhpc2k6IEluY2x1ZGUgcmVnaXN0ZXIgYmxvY2sg
YmFzZSBpbiBQQ0lFX1NZU19TVEFURTQgYWRkcmVzcw0KPiAgICAgICBQQ0k6IGhpc2k6IFBhc3Mg
ZGV2aWNlLXNwZWNpZmljIHN0cnVjdCB0byBpbnRlcm5hbCBmdW5jdGlvbnMNCj4gICAgICAgUENJ
OiBoaXNpOiBSZW9yZGVyIHN0cnVjdCBoaXNpX3BjaWUNCj4gDQo+IA0KPiAgZHJpdmVycy9wY2kv
aG9zdC9wY2llLWhpc2kuYyB8ICAgODYgKysrKysrKysrKysrKysrKystLS0tLS0tLS0tLS0tLS0t
LQ0KPiAtLS0tLS0tLQ0KPiAgMSBmaWxlIGNoYW5nZWQsIDM1IGluc2VydGlvbnMoKyksIDUxIGRl
bGV0aW9ucygtKQ0KPiAtLQ0KPiBUbyB1bnN1YnNjcmliZSBmcm9tIHRoaXMgbGlzdDogc2VuZCB0
aGUgbGluZSAidW5zdWJzY3JpYmUgbGludXgtcGNpIiBpbg0KPiB0aGUgYm9keSBvZiBhIG1lc3Nh
Z2UgdG8gbWFqb3Jkb21vQHZnZXIua2VybmVsLm9yZw0KPiBNb3JlIG1ham9yZG9tbyBpbmZvIGF0
ICBodHRwOi8vdmdlci5rZXJuZWwub3JnL21ham9yZG9tby1pbmZvLmh0bWwNCg0KRm9yIHRoZSB3
aG9sZSBwYXRjaHNldA0KDQpBY2tlZC1ieTogR2FicmllbGUgUGFvbG9uaSA8Z2FicmllbGUucGFv
bG9uaUBodWF3ZWkuY29tPg0K
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH v2 0/8] PCI: hisi: Cleanups
2016-10-12 16:05 ` [PATCH v2 0/8] PCI: hisi: Cleanups Bjorn Helgaas
@ 2016-10-14 2:19 ` Gabriele Paoloni
0 siblings, 0 replies; 12+ messages in thread
From: Gabriele Paoloni @ 2016-10-14 2:19 UTC (permalink / raw)
To: Bjorn Helgaas, Bjorn Helgaas; +Cc: Wangzhou (B), linux-pci
> -----Original Message-----
> From: Bjorn Helgaas [mailto:helgaas@kernel.org]
> Sent: 13 October 2016 00:06
> To: Bjorn Helgaas
> Cc: Wangzhou (B); Gabriele Paoloni; linux-pci@vger.kernel.org
> Subject: Re: [PATCH v2 0/8] PCI: hisi: Cleanups
>=20
> On Wed, Oct 12, 2016 at 08:42:27AM -0500, Bjorn Helgaas wrote:
> > - Add local "dev" pointers to reduce repetition of things like
> > "&pdev->dev".
> >
> > - Remove platform drvdata because it appears unused (we called
> > platform_set_drvdata() but not platform_get_drvdata()).
> >
> > - Name private struct pointer consistently within driver.
> >
> > - Remove redundant struct members.
> >
> > - Use generic DesignWare accessors when possible.
> >
> > - Pass device-specific struct to internal functions for consistency.
> >
> > - Include register block base in PCIE_SYS_STATE4 address to
> simplify
> > users.
> >
> > Nothing here should change the behavior of the driver.
> >
> > Changes from v1:
> > I dropped the following patches because they were a lot of churn
> for
> > questionable benefit:
> > PCI: hisi: Rename APB accessors
> > PCI: hisi: Rename config accessors
> > PCI: hisi: Name private struct pointer "hisi" consistently
> > (Instead of renaming *all* the pointers, I only renamed enough
> to
> > make them consistent within this file.)
> > PCI: hisi: Swap order of hisi_apb_writel() reg/val arguments
> >
> > ---
> >
> > Bjorn Helgaas (8):
> > PCI: hisi: Add local struct device pointers
> > PCI: hisi: Remove unused platform data
> > PCI: hisi: Name private struct pointer "hisi_pcie" consistently
> > PCI: hisi: Remove redundant struct hisi_pcie.reg_base
> > PCI: hisi: Use generic DesignWare accessors
> > PCI: hisi: Include register block base in PCIE_SYS_STATE4
> address
> > PCI: hisi: Pass device-specific struct to internal functions
> > PCI: hisi: Reorder struct hisi_pcie
> >
> >
> > drivers/pci/host/pcie-hisi.c | 86 +++++++++++++++++---------------
> ----------
> > 1 file changed, 35 insertions(+), 51 deletions(-)
>=20
> I applied these to pci/host-hisi for v4.9. I hope to ask Linus to
> pull them tomorrow, so if you see any issues, let me know soon.
Hi Bjorn, I just acked the whole patchset
Many Thanks for this
Gab
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2016-10-14 2:19 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-12 13:42 [PATCH v2 0/8] PCI: hisi: Cleanups Bjorn Helgaas
2016-10-12 13:42 ` [PATCH v2 1/8] PCI: hisi: Add local struct device pointers Bjorn Helgaas
2016-10-12 13:42 ` [PATCH v2 2/8] PCI: hisi: Remove unused platform data Bjorn Helgaas
2016-10-12 13:42 ` [PATCH v2 3/8] PCI: hisi: Name private struct pointer "hisi_pcie" consistently Bjorn Helgaas
2016-10-12 13:42 ` [PATCH v2 4/8] PCI: hisi: Remove redundant struct hisi_pcie.reg_base Bjorn Helgaas
2016-10-12 13:43 ` [PATCH v2 5/8] PCI: hisi: Use generic DesignWare accessors Bjorn Helgaas
2016-10-12 13:43 ` [PATCH v2 6/8] PCI: hisi: Include register block base in PCIE_SYS_STATE4 address Bjorn Helgaas
2016-10-12 13:43 ` [PATCH v2 7/8] PCI: hisi: Pass device-specific struct to internal functions Bjorn Helgaas
2016-10-12 13:43 ` [PATCH v2 8/8] PCI: hisi: Reorder struct hisi_pcie Bjorn Helgaas
2016-10-12 16:05 ` [PATCH v2 0/8] PCI: hisi: Cleanups Bjorn Helgaas
2016-10-14 2:19 ` Gabriele Paoloni
2016-10-14 2:17 ` Gabriele Paoloni
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.