Everyone:
This is the series containing various small improvements that I made
while reading the code and researching commit history of pci-imx6.c
and pcie-designware*.c files. All changes are optional, so commits
that don't seem like an improvement can be easily dropped. Hopefully
each patch is self-explanatory.
I tested this series on i.MX6Q, i.MX7D and i.MX8MQ.
Feedback is welcome!
Thanks,
Andrey Smirnov
Andrey Smirnov (21):
PCI: imx6: Simplify imx7d_pcie_wait_for_phy_pll_lock()
PCI: imx6: Remove redundant debug tracing
PCI: imx6: Return -ETIMEOUT from imx6_pcie_wait_for_speed_change()
PCI: imx6: Remove duplicate macro definitions
PCI: imx6: Remove PCIE_PL_PFLR_* constants
PCI: imx6: Remove PCIE_PHY_RX_ASIC_OUT* constants
PCI: designware: Make use of IS_ALIGNED()
PCI: designware: Share code for dw_pcie_rd/wr_other_conf()
PCI: imx6: Drop imx6_pcie_link_up()
PCI: designware: imx6: Share PHY debug register definitions
PCI: designware: Make use of BIT() in constant definitions
PCI: imx6: Make use of BIT() in constant definitions
PCI: imx6: Simplify bit operations in PHY functions
PCI: imx6: Simplify pcie_phy_poll_ack()
PCI: imx6: Restrict PHY register data to 16-bit
PCI: imx6: Pass data to dw_pcie_writel_dbi() directly
PCI: imx6: Use common mask in imx6_pcie_reset_phy()
PCI: imx6: Simplify bit operations in imx6_setup_phy_mpll()
PCI: imx6: Remove magic numbers from imx6_pcie_establish_link()
PCI: designware: Make use of GENMASK/FIELD_PREP
PCI: designware: Remove superfluous shifting in definitions
drivers/pci/controller/dwc/pci-imx6.c | 151 +++++++-----------
.../pci/controller/dwc/pcie-designware-host.c | 61 +++----
drivers/pci/controller/dwc/pcie-designware.c | 18 +--
drivers/pci/controller/dwc/pcie-designware.h | 60 +++----
4 files changed, 116 insertions(+), 174 deletions(-)
--
2.19.1
Both pcie-designware.c and pci-imx6.c contain custom definitions for
PHY debug registers R0/R1 and on top of that there's already a
definition for R0 in pcie-designware.h. Move all of the definitions to
pcie-designware.h. No functional change intended.
Cc: Lorenzo Pieralisi <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Lucas Stach <[email protected]>
Cc: Leonard Crestez <[email protected]>
Cc: "A.s. Dong" <[email protected]>
Cc: Richard Zhu <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Andrey Smirnov <[email protected]>
---
drivers/pci/controller/dwc/pci-imx6.c | 6 ++----
drivers/pci/controller/dwc/pcie-designware.c | 12 +++---------
drivers/pci/controller/dwc/pcie-designware.h | 3 +++
3 files changed, 8 insertions(+), 13 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 74faca11eeae..c0b073cf20c0 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -82,8 +82,6 @@ struct imx6_pcie {
/* PCIe Port Logic registers (memory-mapped) */
#define PL_OFFSET 0x700
-#define PCIE_PHY_DEBUG_R0 (PL_OFFSET + 0x28)
-#define PCIE_PHY_DEBUG_R1 (PL_OFFSET + 0x2c)
#define PCIE_PHY_CTRL (PL_OFFSET + 0x114)
#define PCIE_PHY_CTRL_DATA_LOC 0
@@ -706,8 +704,8 @@ static int imx6_pcie_establish_link(struct imx6_pcie *imx6_pcie)
err_reset_phy:
dev_dbg(dev, "PHY DEBUG_R0=0x%08x DEBUG_R1=0x%08x\n",
- dw_pcie_readl_dbi(pci, PCIE_PHY_DEBUG_R0),
- dw_pcie_readl_dbi(pci, PCIE_PHY_DEBUG_R1));
+ dw_pcie_readl_dbi(pci, PCIE_PORT_DEBUG0),
+ dw_pcie_readl_dbi(pci, PCIE_PORT_DEBUG1));
imx6_pcie_reset_phy(imx6_pcie);
return ret;
}
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 67236379c61a..d123ac290b9e 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -14,12 +14,6 @@
#include "pcie-designware.h"
-/* PCIe Port Logic registers */
-#define PLR_OFFSET 0x700
-#define PCIE_PHY_DEBUG_R1 (PLR_OFFSET + 0x2c)
-#define PCIE_PHY_DEBUG_R1_LINK_UP (0x1 << 4)
-#define PCIE_PHY_DEBUG_R1_LINK_IN_TRAINING (0x1 << 29)
-
int dw_pcie_read(void __iomem *addr, int size, u32 *val)
{
if (!IS_ALIGNED((uintptr_t)addr, size)) {
@@ -334,9 +328,9 @@ int dw_pcie_link_up(struct dw_pcie *pci)
if (pci->ops->link_up)
return pci->ops->link_up(pci);
- val = readl(pci->dbi_base + PCIE_PHY_DEBUG_R1);
- return ((val & PCIE_PHY_DEBUG_R1_LINK_UP) &&
- (!(val & PCIE_PHY_DEBUG_R1_LINK_IN_TRAINING)));
+ val = readl(pci->dbi_base + PCIE_PORT_DEBUG1);
+ return ((val & PCIE_PORT_DEBUG1_LINK_UP) &&
+ (!(val & PCIE_PORT_DEBUG1_LINK_IN_TRAINING)));
}
void dw_pcie_setup(struct dw_pcie *pci)
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 9943d8c68335..58735fd01668 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -39,6 +39,9 @@
#define PCIE_PORT_DEBUG0 0x728
#define PORT_LOGIC_LTSSM_STATE_MASK 0x1f
#define PORT_LOGIC_LTSSM_STATE_L0 0x11
+#define PCIE_PORT_DEBUG1 0x72C
+#define PCIE_PORT_DEBUG1_LINK_UP (0x1 << 4)
+#define PCIE_PORT_DEBUG1_LINK_IN_TRAINING (0x1 << 29)
#define PCIE_LINK_WIDTH_SPEED_CONTROL 0x80C
#define PORT_LOGIC_SPEED_CHANGE (0x1 << 17)
--
2.19.1
Save a couple of lines of code by dropping assignement to 'var' and
passing constants as via function arguments directly. No functional
change intended.
Cc: Lorenzo Pieralisi <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Lucas Stach <[email protected]>
Cc: Leonard Crestez <[email protected]>
Cc: "A.s. Dong" <[email protected]>
Cc: Richard Zhu <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Andrey Smirnov <[email protected]>
---
drivers/pci/controller/dwc/pci-imx6.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 98e3730e75fa..bade9d608605 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -155,7 +155,6 @@ static int pcie_phy_wait_ack(struct imx6_pcie *imx6_pcie, int addr)
static int pcie_phy_read(struct imx6_pcie *imx6_pcie, int addr, u16 *data)
{
struct dw_pcie *pci = imx6_pcie->pci;
- u32 phy_ctl;
int ret;
ret = pcie_phy_wait_ack(imx6_pcie, addr);
@@ -163,8 +162,7 @@ static int pcie_phy_read(struct imx6_pcie *imx6_pcie, int addr, u16 *data)
return ret;
/* assert Read signal */
- phy_ctl = PCIE_PHY_CTRL_RD;
- dw_pcie_writel_dbi(pci, PCIE_PHY_CTRL, phy_ctl);
+ dw_pcie_writel_dbi(pci, PCIE_PHY_CTRL, PCIE_PHY_CTRL_RD);
ret = pcie_phy_poll_ack(imx6_pcie, 1);
if (ret)
@@ -202,8 +200,7 @@ static int pcie_phy_write(struct imx6_pcie *imx6_pcie, int addr, u16 data)
return ret;
/* deassert cap data */
- var = PCIE_PHY_CTRL_DATA(data);
- dw_pcie_writel_dbi(pci, PCIE_PHY_CTRL, var);
+ dw_pcie_writel_dbi(pci, PCIE_PHY_CTRL, PCIE_PHY_CTRL_DATA(data));
/* wait for ack de-assertion */
ret = pcie_phy_poll_ack(imx6_pcie, 0);
@@ -211,8 +208,7 @@ static int pcie_phy_write(struct imx6_pcie *imx6_pcie, int addr, u16 data)
return ret;
/* assert wr signal */
- var = PCIE_PHY_CTRL_WR;
- dw_pcie_writel_dbi(pci, PCIE_PHY_CTRL, var);
+ dw_pcie_writel_dbi(pci, PCIE_PHY_CTRL, PCIE_PHY_CTRL_WR);
/* wait for ack */
ret = pcie_phy_poll_ack(imx6_pcie, 1);
@@ -220,8 +216,7 @@ static int pcie_phy_write(struct imx6_pcie *imx6_pcie, int addr, u16 data)
return ret;
/* deassert wr signal */
- var = PCIE_PHY_CTRL_DATA(data);
- dw_pcie_writel_dbi(pci, PCIE_PHY_CTRL, var);
+ dw_pcie_writel_dbi(pci, PCIE_PHY_CTRL, PCIE_PHY_CTRL_DATA(data));
/* wait for ack de-assertion */
ret = pcie_phy_poll_ack(imx6_pcie, 0);
--
2.19.1
Explicitly define PCIE_RC_LCSR_LINK_SPEED and remove magic numbers
from imx6_pcie_establish_link(). No functional change intended.
Cc: Lorenzo Pieralisi <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Lucas Stach <[email protected]>
Cc: Leonard Crestez <[email protected]>
Cc: "A.s. Dong" <[email protected]>
Cc: Richard Zhu <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Andrey Smirnov <[email protected]>
---
drivers/pci/controller/dwc/pci-imx6.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 678f5fa85e12..2b312f287fd7 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -79,6 +79,7 @@ struct imx6_pcie {
#define PCIE_RC_LCR_MAX_LINK_SPEEDS_MASK 0xf
#define PCIE_RC_LCSR 0x80
+#define PCIE_RC_LCSR_LINK_SPEED GENMASK(19, 16)
/* PCIe Port Logic registers (memory-mapped) */
#define PL_OFFSET 0x700
@@ -688,8 +689,9 @@ static int imx6_pcie_establish_link(struct imx6_pcie *imx6_pcie)
dev_info(dev, "Link: Gen2 disabled\n");
}
- tmp = dw_pcie_readl_dbi(pci, PCIE_RC_LCSR);
- dev_info(dev, "Link up, Gen%i\n", (tmp >> 16) & 0xf);
+ tmp = FIELD_GET(PCIE_RC_LCSR_LINK_SPEED,
+ dw_pcie_readl_dbi(pci, PCIE_RC_LCSR));
+ dev_info(dev, "Link up, Gen%i\n", tmp);
return 0;
err_reset_phy:
--
2.19.1
Surrounding definitions no longer use explicit shift, so "<< 0" here
serve no purpose. Remove them. No functional change intended.
Cc: Lorenzo Pieralisi <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Lucas Stach <[email protected]>
Cc: Leonard Crestez <[email protected]>
Cc: "A.s. Dong" <[email protected]>
Cc: Richard Zhu <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Andrey Smirnov <[email protected]>
---
drivers/pci/controller/dwc/pcie-designware.h | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 0de653284fca..636689fd4ee7 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -63,14 +63,14 @@
#define PCIE_ATU_VIEWPORT 0x900
#define PCIE_ATU_REGION_INBOUND BIT(31)
#define PCIE_ATU_REGION_OUTBOUND 0
-#define PCIE_ATU_REGION_INDEX2 (0x2 << 0)
-#define PCIE_ATU_REGION_INDEX1 (0x1 << 0)
-#define PCIE_ATU_REGION_INDEX0 (0x0 << 0)
+#define PCIE_ATU_REGION_INDEX2 0x2
+#define PCIE_ATU_REGION_INDEX1 0x1
+#define PCIE_ATU_REGION_INDEX0 0x0
#define PCIE_ATU_CR1 0x904
-#define PCIE_ATU_TYPE_MEM (0x0 << 0)
-#define PCIE_ATU_TYPE_IO (0x2 << 0)
-#define PCIE_ATU_TYPE_CFG0 (0x4 << 0)
-#define PCIE_ATU_TYPE_CFG1 (0x5 << 0)
+#define PCIE_ATU_TYPE_MEM 0x0
+#define PCIE_ATU_TYPE_IO 0x2
+#define PCIE_ATU_TYPE_CFG0 0x4
+#define PCIE_ATU_TYPE_CFG1 0x5
#define PCIE_ATU_CR2 0x908
#define PCIE_ATU_ENABLE BIT(31)
#define PCIE_ATU_BAR_MODE_ENABLE BIT(30)
--
2.19.1
Make use of regmap_read_poll_timeout() to simplify
imx7d_pcie_wait_for_phy_pll_lock(). No functional change intended.
Cc: Lorenzo Pieralisi <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Lucas Stach <[email protected]>
Cc: Leonard Crestez <[email protected]>
Cc: "A.s. Dong" <[email protected]>
Cc: Richard Zhu <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Andrey Smirnov <[email protected]>
---
drivers/pci/controller/dwc/pci-imx6.c | 21 +++++++--------------
1 file changed, 7 insertions(+), 14 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 26087b3da590..c87ecc305dc7 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -69,9 +69,8 @@ struct imx6_pcie {
};
/* Parameters for the waiting for PCIe PHY PLL to lock on i.MX7 */
-#define PHY_PLL_LOCK_WAIT_MAX_RETRIES 2000
-#define PHY_PLL_LOCK_WAIT_USLEEP_MIN 50
#define PHY_PLL_LOCK_WAIT_USLEEP_MAX 200
+#define PHY_PLL_LOCK_WAIT_TIMEOUT (2000 * PHY_PLL_LOCK_WAIT_USLEEP_MAX)
/* PCIe Root Complex registers (memory-mapped) */
#define PCIE_RC_LCR 0x7c
@@ -418,20 +417,14 @@ static int imx6_pcie_enable_ref_clk(struct imx6_pcie *imx6_pcie)
static void imx7d_pcie_wait_for_phy_pll_lock(struct imx6_pcie *imx6_pcie)
{
u32 val;
- unsigned int retries;
struct device *dev = imx6_pcie->pci->dev;
- for (retries = 0; retries < PHY_PLL_LOCK_WAIT_MAX_RETRIES; retries++) {
- regmap_read(imx6_pcie->iomuxc_gpr, IOMUXC_GPR22, &val);
-
- if (val & IMX7D_GPR22_PCIE_PHY_PLL_LOCKED)
- return;
-
- usleep_range(PHY_PLL_LOCK_WAIT_USLEEP_MIN,
- PHY_PLL_LOCK_WAIT_USLEEP_MAX);
- }
-
- dev_err(dev, "PCIe PLL lock timeout\n");
+ if (regmap_read_poll_timeout(imx6_pcie->iomuxc_gpr,
+ IOMUXC_GPR22, val,
+ val & IMX7D_GPR22_PCIE_PHY_PLL_LOCKED,
+ PHY_PLL_LOCK_WAIT_USLEEP_MAX,
+ PHY_PLL_LOCK_WAIT_TIMEOUT))
+ dev_err(dev, "PCIe PLL lock timeout\n");
}
static void imx6_pcie_deassert_core_reset(struct imx6_pcie *imx6_pcie)
--
2.19.1
Default implementation of pcie_rd_other_conf() and
dw_pcie_wd_other_conf() share more than 80% of their code. Move shared
code into a dedicated subroutine and convert pcie_rd_other_conf() and
dw_pcie_wd_other_conf() to use it. No functional change intended.
Cc: Lorenzo Pieralisi <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Lucas Stach <[email protected]>
Cc: Leonard Crestez <[email protected]>
Cc: "A.s. Dong" <[email protected]>
Cc: Richard Zhu <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Andrey Smirnov <[email protected]>
---
.../pci/controller/dwc/pcie-designware-host.c | 61 +++++++------------
1 file changed, 23 insertions(+), 38 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 721d60a5d9e4..8f957cd6901b 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -512,8 +512,9 @@ int dw_pcie_host_init(struct pcie_port *pp)
return ret;
}
-static int dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus,
- u32 devfn, int where, int size, u32 *val)
+static int dw_pcie_access_other_conf(struct pcie_port *pp, struct pci_bus *bus,
+ u32 devfn, int where, int size, u32 *val,
+ bool write)
{
int ret, type;
u32 busdev, cfg_size;
@@ -521,9 +522,6 @@ static int dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus,
void __iomem *va_cfg_base;
struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
- if (pp->ops->rd_other_conf)
- return pp->ops->rd_other_conf(pp, bus, devfn, where, size, val);
-
busdev = PCIE_ATU_BUS(bus->number) | PCIE_ATU_DEV(PCI_SLOT(devfn)) |
PCIE_ATU_FUNC(PCI_FUNC(devfn));
@@ -542,7 +540,11 @@ static int dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus,
dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX1,
type, cpu_addr,
busdev, cfg_size);
- ret = dw_pcie_read(va_cfg_base + where, size, val);
+ if (write)
+ ret = dw_pcie_write(va_cfg_base + where, size, *val);
+ else
+ ret = dw_pcie_read(va_cfg_base + where, size, val);
+
if (pci->num_viewport <= 2)
dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX1,
PCIE_ATU_TYPE_IO, pp->io_base,
@@ -551,43 +553,26 @@ static int dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus,
return ret;
}
+static int dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus,
+ u32 devfn, int where, int size, u32 *val)
+{
+ if (pp->ops->rd_other_conf)
+ return pp->ops->rd_other_conf(pp, bus, devfn, where,
+ size, val);
+
+ return dw_pcie_access_other_conf(pp, bus, devfn, where, size, val,
+ false);
+}
+
static int dw_pcie_wr_other_conf(struct pcie_port *pp, struct pci_bus *bus,
u32 devfn, int where, int size, u32 val)
{
- int ret, type;
- u32 busdev, cfg_size;
- u64 cpu_addr;
- void __iomem *va_cfg_base;
- struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
-
if (pp->ops->wr_other_conf)
- return pp->ops->wr_other_conf(pp, bus, devfn, where, size, val);
-
- busdev = PCIE_ATU_BUS(bus->number) | PCIE_ATU_DEV(PCI_SLOT(devfn)) |
- PCIE_ATU_FUNC(PCI_FUNC(devfn));
+ return pp->ops->wr_other_conf(pp, bus, devfn, where,
+ size, val);
- if (bus->parent->number == pp->root_bus_nr) {
- type = PCIE_ATU_TYPE_CFG0;
- cpu_addr = pp->cfg0_base;
- cfg_size = pp->cfg0_size;
- va_cfg_base = pp->va_cfg0_base;
- } else {
- type = PCIE_ATU_TYPE_CFG1;
- cpu_addr = pp->cfg1_base;
- cfg_size = pp->cfg1_size;
- va_cfg_base = pp->va_cfg1_base;
- }
-
- dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX1,
- type, cpu_addr,
- busdev, cfg_size);
- ret = dw_pcie_write(va_cfg_base + where, size, val);
- if (pci->num_viewport <= 2)
- dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX1,
- PCIE_ATU_TYPE_IO, pp->io_base,
- pp->io_bus_addr, pp->io_size);
-
- return ret;
+ return dw_pcie_access_other_conf(pp, bus, devfn, where, size, &val,
+ true);
}
static int dw_pcie_valid_device(struct pcie_port *pp, struct pci_bus *bus,
--
2.19.1
Simplify bit operations in imx6_setup_phy_mpll() by using
GENMASK/FIELD_PREP. No functional change intended.
Cc: Lorenzo Pieralisi <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Lucas Stach <[email protected]>
Cc: Leonard Crestez <[email protected]>
Cc: "A.s. Dong" <[email protected]>
Cc: Richard Zhu <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Andrey Smirnov <[email protected]>
---
drivers/pci/controller/dwc/pci-imx6.c | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 38c8e8baa077..678f5fa85e12 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -96,12 +96,10 @@ struct imx6_pcie {
/* PHY registers (not memory-mapped) */
#define PCIE_PHY_ATEOVRD 0x10
#define PCIE_PHY_ATEOVRD_EN BIT(2)
-#define PCIE_PHY_ATEOVRD_REF_CLKDIV_SHIFT 0
-#define PCIE_PHY_ATEOVRD_REF_CLKDIV_MASK 0x1
+#define PCIE_PHY_ATEOVRD_REF_CLKDIV BIT(0)
#define PCIE_PHY_MPLL_OVRD_IN_LO 0x11
-#define PCIE_PHY_MPLL_MULTIPLIER_SHIFT 2
-#define PCIE_PHY_MPLL_MULTIPLIER_MASK 0x7f
+#define PCIE_PHY_MPLL_MULTIPLIER GENMASK(8, 2)
#define PCIE_PHY_MPLL_MULTIPLIER_OVRD BIT(9)
#define PHY_RX_OVRD_IN_LO 0x1005
@@ -565,16 +563,14 @@ static int imx6_setup_phy_mpll(struct imx6_pcie *imx6_pcie)
}
pcie_phy_read(imx6_pcie, PCIE_PHY_MPLL_OVRD_IN_LO, &val);
- val &= ~(PCIE_PHY_MPLL_MULTIPLIER_MASK <<
- PCIE_PHY_MPLL_MULTIPLIER_SHIFT);
- val |= mult << PCIE_PHY_MPLL_MULTIPLIER_SHIFT;
+ val &= ~PCIE_PHY_MPLL_MULTIPLIER;
+ val |= FIELD_PREP(PCIE_PHY_MPLL_MULTIPLIER, mult);
val |= PCIE_PHY_MPLL_MULTIPLIER_OVRD;
pcie_phy_write(imx6_pcie, PCIE_PHY_MPLL_OVRD_IN_LO, val);
pcie_phy_read(imx6_pcie, PCIE_PHY_ATEOVRD, &val);
- val &= ~(PCIE_PHY_ATEOVRD_REF_CLKDIV_MASK <<
- PCIE_PHY_ATEOVRD_REF_CLKDIV_SHIFT);
- val |= div << PCIE_PHY_ATEOVRD_REF_CLKDIV_SHIFT;
+ val &= ~PCIE_PHY_ATEOVRD_REF_CLKDIV;
+ val |= FIELD_PREP(PCIE_PHY_ATEOVRD_REF_CLKDIV, div);
val |= PCIE_PHY_ATEOVRD_EN;
pcie_phy_write(imx6_pcie, PCIE_PHY_ATEOVRD, val);
--
2.19.1
Convert various mult-bit fields to be defined using
GENMASK/FIELD_PREP. This way bit field boundaries are defined in a
single place only as well as defined in a way that makes it easier to
verify them against reference manual. No functional change intended.
Cc: Lorenzo Pieralisi <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Lucas Stach <[email protected]>
Cc: Leonard Crestez <[email protected]>
Cc: "A.s. Dong" <[email protected]>
Cc: Richard Zhu <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Andrey Smirnov <[email protected]>
---
drivers/pci/controller/dwc/pcie-designware.h | 29 +++++++++++---------
1 file changed, 16 insertions(+), 13 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 348e91b6daa2..0de653284fca 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -11,6 +11,7 @@
#ifndef _PCIE_DESIGNWARE_H
#define _PCIE_DESIGNWARE_H
+#include <linux/bitfield.h>
#include <linux/dma-mapping.h>
#include <linux/irq.h>
#include <linux/msi.h>
@@ -30,11 +31,12 @@
/* Synopsys-specific PCIe configuration registers */
#define PCIE_PORT_LINK_CONTROL 0x710
-#define PORT_LINK_MODE_MASK (0x3f << 16)
-#define PORT_LINK_MODE_1_LANES (0x1 << 16)
-#define PORT_LINK_MODE_2_LANES (0x3 << 16)
-#define PORT_LINK_MODE_4_LANES (0x7 << 16)
-#define PORT_LINK_MODE_8_LANES (0xf << 16)
+#define PORT_LINK_MODE_MASK GENMASK(21, 16)
+#define PORT_LINK_MODE(n) FIELD_PREP(PORT_LINK_MODE_MASK, n)
+#define PORT_LINK_MODE_1_LANES PORT_LINK_MODE(0x1)
+#define PORT_LINK_MODE_2_LANES PORT_LINK_MODE(0x3)
+#define PORT_LINK_MODE_4_LANES PORT_LINK_MODE(0x7)
+#define PORT_LINK_MODE_8_LANES PORT_LINK_MODE(0xf)
#define PCIE_PORT_DEBUG0 0x728
#define PORT_LOGIC_LTSSM_STATE_MASK 0x1f
@@ -45,11 +47,12 @@
#define PCIE_LINK_WIDTH_SPEED_CONTROL 0x80C
#define PORT_LOGIC_SPEED_CHANGE BIT(17)
-#define PORT_LOGIC_LINK_WIDTH_MASK (0x1f << 8)
-#define PORT_LOGIC_LINK_WIDTH_1_LANES (0x1 << 8)
-#define PORT_LOGIC_LINK_WIDTH_2_LANES (0x2 << 8)
-#define PORT_LOGIC_LINK_WIDTH_4_LANES (0x4 << 8)
-#define PORT_LOGIC_LINK_WIDTH_8_LANES (0x8 << 8)
+#define PORT_LOGIC_LINK_WIDTH_MASK GENMASK(12, 8)
+#define PORT_LOGIC_LINK_WIDTH(n) FIELD_PREP(PORT_LOGIC_LINK_WIDTH_MASK, n)
+#define PORT_LOGIC_LINK_WIDTH_1_LANES PORT_LOGIC_LINK_WIDTH(0x1)
+#define PORT_LOGIC_LINK_WIDTH_2_LANES PORT_LOGIC_LINK_WIDTH(0x2)
+#define PORT_LOGIC_LINK_WIDTH_4_LANES PORT_LOGIC_LINK_WIDTH(0x4)
+#define PORT_LOGIC_LINK_WIDTH_8_LANES PORT_LOGIC_LINK_WIDTH(0x8)
#define PCIE_MSI_ADDR_LO 0x820
#define PCIE_MSI_ADDR_HI 0x824
@@ -75,9 +78,9 @@
#define PCIE_ATU_UPPER_BASE 0x910
#define PCIE_ATU_LIMIT 0x914
#define PCIE_ATU_LOWER_TARGET 0x918
-#define PCIE_ATU_BUS(x) (((x) & 0xff) << 24)
-#define PCIE_ATU_DEV(x) (((x) & 0x1f) << 19)
-#define PCIE_ATU_FUNC(x) (((x) & 0x7) << 16)
+#define PCIE_ATU_BUS(x) FIELD_PREP(GENMASK(31, 24), x)
+#define PCIE_ATU_DEV(x) FIELD_PREP(GENMASK(23, 19), x)
+#define PCIE_ATU_FUNC(x) FIELD_PREP(GENMASK(18, 16), x)
#define PCIE_ATU_UPPER_TARGET 0x91C
#define PCIE_MISC_CONTROL_1_OFF 0x8BC
--
2.19.1
Simplify imx6_pcie_reset_phy() by using common mask. No functional
change intended.
Cc: Lorenzo Pieralisi <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Lucas Stach <[email protected]>
Cc: Leonard Crestez <[email protected]>
Cc: "A.s. Dong" <[email protected]>
Cc: Richard Zhu <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Andrey Smirnov <[email protected]>
---
drivers/pci/controller/dwc/pci-imx6.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index bade9d608605..38c8e8baa077 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -230,18 +230,18 @@ static int pcie_phy_write(struct imx6_pcie *imx6_pcie, int addr, u16 data)
static void imx6_pcie_reset_phy(struct imx6_pcie *imx6_pcie)
{
+ const u16 mask = PHY_RX_OVRD_IN_LO_RX_DATA_EN |
+ PHY_RX_OVRD_IN_LO_RX_PLL_EN;
u16 tmp;
pcie_phy_read(imx6_pcie, PHY_RX_OVRD_IN_LO, &tmp);
- tmp |= (PHY_RX_OVRD_IN_LO_RX_DATA_EN |
- PHY_RX_OVRD_IN_LO_RX_PLL_EN);
+ tmp |= mask;
pcie_phy_write(imx6_pcie, PHY_RX_OVRD_IN_LO, tmp);
usleep_range(2000, 3000);
pcie_phy_read(imx6_pcie, PHY_RX_OVRD_IN_LO, &tmp);
- tmp &= ~(PHY_RX_OVRD_IN_LO_RX_DATA_EN |
- PHY_RX_OVRD_IN_LO_RX_PLL_EN);
+ tmp &= ~mask;
pcie_phy_write(imx6_pcie, PHY_RX_OVRD_IN_LO, tmp);
}
--
2.19.1
PHY registers on i.MX6 are 16-bit wide, so we can get rid of explicit
masking if we restrict pcie_phy_read/pcie_phy_write to use 'u16'
instead of 'int'. No functional change intended.
Cc: Lorenzo Pieralisi <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Lucas Stach <[email protected]>
Cc: Leonard Crestez <[email protected]>
Cc: "A.s. Dong" <[email protected]>
Cc: Richard Zhu <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Andrey Smirnov <[email protected]>
---
drivers/pci/controller/dwc/pci-imx6.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index ddab1859a07e..98e3730e75fa 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -152,10 +152,10 @@ static int pcie_phy_wait_ack(struct imx6_pcie *imx6_pcie, int addr)
}
/* Read from the 16-bit PCIe PHY control registers (not memory-mapped) */
-static int pcie_phy_read(struct imx6_pcie *imx6_pcie, int addr, int *data)
+static int pcie_phy_read(struct imx6_pcie *imx6_pcie, int addr, u16 *data)
{
struct dw_pcie *pci = imx6_pcie->pci;
- u32 val, phy_ctl;
+ u32 phy_ctl;
int ret;
ret = pcie_phy_wait_ack(imx6_pcie, addr);
@@ -170,8 +170,7 @@ static int pcie_phy_read(struct imx6_pcie *imx6_pcie, int addr, int *data)
if (ret)
return ret;
- val = dw_pcie_readl_dbi(pci, PCIE_PHY_STAT);
- *data = val & 0xffff;
+ *data = dw_pcie_readl_dbi(pci, PCIE_PHY_STAT);
/* deassert Read signal */
dw_pcie_writel_dbi(pci, PCIE_PHY_CTRL, 0x00);
@@ -179,7 +178,7 @@ static int pcie_phy_read(struct imx6_pcie *imx6_pcie, int addr, int *data)
return pcie_phy_poll_ack(imx6_pcie, 0);
}
-static int pcie_phy_write(struct imx6_pcie *imx6_pcie, int addr, int data)
+static int pcie_phy_write(struct imx6_pcie *imx6_pcie, int addr, u16 data)
{
struct dw_pcie *pci = imx6_pcie->pci;
u32 var;
@@ -236,7 +235,7 @@ static int pcie_phy_write(struct imx6_pcie *imx6_pcie, int addr, int data)
static void imx6_pcie_reset_phy(struct imx6_pcie *imx6_pcie)
{
- u32 tmp;
+ u16 tmp;
pcie_phy_read(imx6_pcie, PHY_RX_OVRD_IN_LO, &tmp);
tmp |= (PHY_RX_OVRD_IN_LO_RX_DATA_EN |
@@ -547,7 +546,7 @@ static int imx6_setup_phy_mpll(struct imx6_pcie *imx6_pcie)
{
unsigned long phy_rate = clk_get_rate(imx6_pcie->pcie_phy);
int mult, div;
- u32 val;
+ u16 val;
switch (phy_rate) {
case 125000000:
--
2.19.1
Avoid using explicit left shifts and convert various definitions to
use BIT() instead. No functional change intended.
Cc: Lorenzo Pieralisi <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Lucas Stach <[email protected]>
Cc: Leonard Crestez <[email protected]>
Cc: "A.s. Dong" <[email protected]>
Cc: Richard Zhu <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Andrey Smirnov <[email protected]>
---
drivers/pci/controller/dwc/pci-imx6.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index c0b073cf20c0..2737526158fa 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -95,18 +95,18 @@ struct imx6_pcie {
/* PHY registers (not memory-mapped) */
#define PCIE_PHY_ATEOVRD 0x10
-#define PCIE_PHY_ATEOVRD_EN (0x1 << 2)
+#define PCIE_PHY_ATEOVRD_EN BIT(2)
#define PCIE_PHY_ATEOVRD_REF_CLKDIV_SHIFT 0
#define PCIE_PHY_ATEOVRD_REF_CLKDIV_MASK 0x1
#define PCIE_PHY_MPLL_OVRD_IN_LO 0x11
#define PCIE_PHY_MPLL_MULTIPLIER_SHIFT 2
#define PCIE_PHY_MPLL_MULTIPLIER_MASK 0x7f
-#define PCIE_PHY_MPLL_MULTIPLIER_OVRD (0x1 << 9)
+#define PCIE_PHY_MPLL_MULTIPLIER_OVRD BIT(9)
#define PHY_RX_OVRD_IN_LO 0x1005
-#define PHY_RX_OVRD_IN_LO_RX_DATA_EN (1 << 5)
-#define PHY_RX_OVRD_IN_LO_RX_PLL_EN (1 << 3)
+#define PHY_RX_OVRD_IN_LO_RX_DATA_EN BIT(5)
+#define PHY_RX_OVRD_IN_LO_RX_PLL_EN BIT(3)
static int pcie_phy_poll_ack(struct imx6_pcie *imx6_pcie, int exp_val)
{
--
2.19.1
Avoid using explicit left shifts and convert various definitions to
use BIT() instead. No functional change intended.
Cc: Lorenzo Pieralisi <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Lucas Stach <[email protected]>
Cc: Leonard Crestez <[email protected]>
Cc: "A.s. Dong" <[email protected]>
Cc: Richard Zhu <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Andrey Smirnov <[email protected]>
---
drivers/pci/controller/dwc/pcie-designware.c | 2 +-
drivers/pci/controller/dwc/pcie-designware.h | 18 +++++++++---------
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index d123ac290b9e..086e87a40316 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -300,7 +300,7 @@ void dw_pcie_disable_atu(struct dw_pcie *pci, int index,
}
dw_pcie_writel_dbi(pci, PCIE_ATU_VIEWPORT, region | index);
- dw_pcie_writel_dbi(pci, PCIE_ATU_CR2, ~PCIE_ATU_ENABLE);
+ dw_pcie_writel_dbi(pci, PCIE_ATU_CR2, (u32)~PCIE_ATU_ENABLE);
}
int dw_pcie_wait_for_link(struct dw_pcie *pci)
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 58735fd01668..348e91b6daa2 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -40,11 +40,11 @@
#define PORT_LOGIC_LTSSM_STATE_MASK 0x1f
#define PORT_LOGIC_LTSSM_STATE_L0 0x11
#define PCIE_PORT_DEBUG1 0x72C
-#define PCIE_PORT_DEBUG1_LINK_UP (0x1 << 4)
-#define PCIE_PORT_DEBUG1_LINK_IN_TRAINING (0x1 << 29)
+#define PCIE_PORT_DEBUG1_LINK_UP BIT(4)
+#define PCIE_PORT_DEBUG1_LINK_IN_TRAINING BIT(29)
#define PCIE_LINK_WIDTH_SPEED_CONTROL 0x80C
-#define PORT_LOGIC_SPEED_CHANGE (0x1 << 17)
+#define PORT_LOGIC_SPEED_CHANGE BIT(17)
#define PORT_LOGIC_LINK_WIDTH_MASK (0x1f << 8)
#define PORT_LOGIC_LINK_WIDTH_1_LANES (0x1 << 8)
#define PORT_LOGIC_LINK_WIDTH_2_LANES (0x2 << 8)
@@ -58,8 +58,8 @@
#define PCIE_MSI_INTR0_STATUS 0x830
#define PCIE_ATU_VIEWPORT 0x900
-#define PCIE_ATU_REGION_INBOUND (0x1 << 31)
-#define PCIE_ATU_REGION_OUTBOUND (0x0 << 31)
+#define PCIE_ATU_REGION_INBOUND BIT(31)
+#define PCIE_ATU_REGION_OUTBOUND 0
#define PCIE_ATU_REGION_INDEX2 (0x2 << 0)
#define PCIE_ATU_REGION_INDEX1 (0x1 << 0)
#define PCIE_ATU_REGION_INDEX0 (0x0 << 0)
@@ -69,8 +69,8 @@
#define PCIE_ATU_TYPE_CFG0 (0x4 << 0)
#define PCIE_ATU_TYPE_CFG1 (0x5 << 0)
#define PCIE_ATU_CR2 0x908
-#define PCIE_ATU_ENABLE (0x1 << 31)
-#define PCIE_ATU_BAR_MODE_ENABLE (0x1 << 30)
+#define PCIE_ATU_ENABLE BIT(31)
+#define PCIE_ATU_BAR_MODE_ENABLE BIT(30)
#define PCIE_ATU_LOWER_BASE 0x90C
#define PCIE_ATU_UPPER_BASE 0x910
#define PCIE_ATU_LIMIT 0x914
@@ -81,7 +81,7 @@
#define PCIE_ATU_UPPER_TARGET 0x91C
#define PCIE_MISC_CONTROL_1_OFF 0x8BC
-#define PCIE_DBI_RO_WR_EN (0x1 << 0)
+#define PCIE_DBI_RO_WR_EN BIT(0)
/*
* iATU Unroll-specific register definitions
@@ -108,7 +108,7 @@
((region) << 9)
#define PCIE_GET_ATU_INB_UNR_REG_OFFSET(region) \
- (((region) << 9) | (0x1 << 8))
+ (((region) << 9) | BIT(8))
#define MAX_MSI_IRQS 256
#define MAX_MSI_IRQS_PER_CTRL 32
--
2.19.1
Code using these constants was removed in commit a77c5422d758 ("PCI:
imx6: Remove broken Gen2 workaround"). No functional change intended.
Cc: Lorenzo Pieralisi <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Lucas Stach <[email protected]>
Cc: Leonard Crestez <[email protected]>
Cc: "A.s. Dong" <[email protected]>
Cc: Richard Zhu <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Andrey Smirnov <[email protected]>
---
drivers/pci/controller/dwc/pci-imx6.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 60803f1f16f8..937d630f8c9d 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -108,9 +108,6 @@ struct imx6_pcie {
#define PCIE_PHY_MPLL_MULTIPLIER_MASK 0x7f
#define PCIE_PHY_MPLL_MULTIPLIER_OVRD (0x1 << 9)
-#define PCIE_PHY_RX_ASIC_OUT 0x100D
-#define PCIE_PHY_RX_ASIC_OUT_VALID (1 << 0)
-
#define PHY_RX_OVRD_IN_LO 0x1005
#define PHY_RX_OVRD_IN_LO_RX_DATA_EN (1 << 5)
#define PHY_RX_OVRD_IN_LO_RX_PLL_EN (1 << 3)
--
2.19.1
Simplify the code by incorporating left shifts into constant
defnitions as well as using FIELD_PREP/GENMASK. No functional change
intended.
Cc: Lorenzo Pieralisi <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Lucas Stach <[email protected]>
Cc: Leonard Crestez <[email protected]>
Cc: "A.s. Dong" <[email protected]>
Cc: Richard Zhu <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Andrey Smirnov <[email protected]>
---
drivers/pci/controller/dwc/pci-imx6.c | 28 +++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 2737526158fa..40d348bb9a2b 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -84,11 +84,11 @@ struct imx6_pcie {
#define PL_OFFSET 0x700
#define PCIE_PHY_CTRL (PL_OFFSET + 0x114)
-#define PCIE_PHY_CTRL_DATA_LOC 0
-#define PCIE_PHY_CTRL_CAP_ADR_LOC 16
-#define PCIE_PHY_CTRL_CAP_DAT_LOC 17
-#define PCIE_PHY_CTRL_WR_LOC 18
-#define PCIE_PHY_CTRL_RD_LOC 19
+#define PCIE_PHY_CTRL_DATA(x) FIELD_PREP(GENMASK(15, 0), (x))
+#define PCIE_PHY_CTRL_CAP_ADR BIT(16)
+#define PCIE_PHY_CTRL_CAP_DAT BIT(17)
+#define PCIE_PHY_CTRL_WR BIT(18)
+#define PCIE_PHY_CTRL_RD BIT(19)
#define PCIE_PHY_STAT (PL_OFFSET + 0x110)
#define PCIE_PHY_STAT_ACK_LOC 16
@@ -135,17 +135,17 @@ static int pcie_phy_wait_ack(struct imx6_pcie *imx6_pcie, int addr)
u32 val;
int ret;
- val = addr << PCIE_PHY_CTRL_DATA_LOC;
+ val = PCIE_PHY_CTRL_DATA(addr);
dw_pcie_writel_dbi(pci, PCIE_PHY_CTRL, val);
- val |= (0x1 << PCIE_PHY_CTRL_CAP_ADR_LOC);
+ val |= PCIE_PHY_CTRL_CAP_ADR;
dw_pcie_writel_dbi(pci, PCIE_PHY_CTRL, val);
ret = pcie_phy_poll_ack(imx6_pcie, 1);
if (ret)
return ret;
- val = addr << PCIE_PHY_CTRL_DATA_LOC;
+ val = PCIE_PHY_CTRL_DATA(addr);
dw_pcie_writel_dbi(pci, PCIE_PHY_CTRL, val);
return pcie_phy_poll_ack(imx6_pcie, 0);
@@ -163,7 +163,7 @@ static int pcie_phy_read(struct imx6_pcie *imx6_pcie, int addr, int *data)
return ret;
/* assert Read signal */
- phy_ctl = 0x1 << PCIE_PHY_CTRL_RD_LOC;
+ phy_ctl = PCIE_PHY_CTRL_RD;
dw_pcie_writel_dbi(pci, PCIE_PHY_CTRL, phy_ctl);
ret = pcie_phy_poll_ack(imx6_pcie, 1);
@@ -191,11 +191,11 @@ static int pcie_phy_write(struct imx6_pcie *imx6_pcie, int addr, int data)
if (ret)
return ret;
- var = data << PCIE_PHY_CTRL_DATA_LOC;
+ var = PCIE_PHY_CTRL_DATA(data);
dw_pcie_writel_dbi(pci, PCIE_PHY_CTRL, var);
/* capture data */
- var |= (0x1 << PCIE_PHY_CTRL_CAP_DAT_LOC);
+ var |= PCIE_PHY_CTRL_CAP_DAT;
dw_pcie_writel_dbi(pci, PCIE_PHY_CTRL, var);
ret = pcie_phy_poll_ack(imx6_pcie, 1);
@@ -203,7 +203,7 @@ static int pcie_phy_write(struct imx6_pcie *imx6_pcie, int addr, int data)
return ret;
/* deassert cap data */
- var = data << PCIE_PHY_CTRL_DATA_LOC;
+ var = PCIE_PHY_CTRL_DATA(data);
dw_pcie_writel_dbi(pci, PCIE_PHY_CTRL, var);
/* wait for ack de-assertion */
@@ -212,7 +212,7 @@ static int pcie_phy_write(struct imx6_pcie *imx6_pcie, int addr, int data)
return ret;
/* assert wr signal */
- var = 0x1 << PCIE_PHY_CTRL_WR_LOC;
+ var = PCIE_PHY_CTRL_WR;
dw_pcie_writel_dbi(pci, PCIE_PHY_CTRL, var);
/* wait for ack */
@@ -221,7 +221,7 @@ static int pcie_phy_write(struct imx6_pcie *imx6_pcie, int addr, int data)
return ret;
/* deassert wr signal */
- var = data << PCIE_PHY_CTRL_DATA_LOC;
+ var = PCIE_PHY_CTRL_DATA(data);
dw_pcie_writel_dbi(pci, PCIE_PHY_CTRL, var);
/* wait for ack de-assertion */
--
2.19.1
Make the intent a bit more clear as well as get rid of explicit
arithmetic by using IS_ALIGNED() to determine if "addr" is aligned to
"size". No functional change intended.
Cc: Lorenzo Pieralisi <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Lucas Stach <[email protected]>
Cc: Leonard Crestez <[email protected]>
Cc: "A.s. Dong" <[email protected]>
Cc: Richard Zhu <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Andrey Smirnov <[email protected]>
---
drivers/pci/controller/dwc/pcie-designware.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 93ef8c31fb39..67236379c61a 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -22,7 +22,7 @@
int dw_pcie_read(void __iomem *addr, int size, u32 *val)
{
- if ((uintptr_t)addr & (size - 1)) {
+ if (!IS_ALIGNED((uintptr_t)addr, size)) {
*val = 0;
return PCIBIOS_BAD_REGISTER_NUMBER;
}
@@ -43,7 +43,7 @@ int dw_pcie_read(void __iomem *addr, int size, u32 *val)
int dw_pcie_write(void __iomem *addr, int size, u32 val)
{
- if ((uintptr_t)addr & (size - 1))
+ if (!IS_ALIGNED((uintptr_t)addr, size))
return PCIBIOS_BAD_REGISTER_NUMBER;
if (size == 4)
--
2.19.1
Both PCIE_LINK_WIDTH_SPEED_CONTROL and PORT_LOGIC_SPEED_CHANGE are
already defined in pcie-desingware.h, so drop duplicate definintion in
pci-imx6.c. No functional change intended.
Cc: Lorenzo Pieralisi <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Lucas Stach <[email protected]>
Cc: Leonard Crestez <[email protected]>
Cc: "A.s. Dong" <[email protected]>
Cc: Richard Zhu <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Andrey Smirnov <[email protected]>
---
drivers/pci/controller/dwc/pci-imx6.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index e990030564d8..2265f4b40e39 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -100,9 +100,6 @@ struct imx6_pcie {
#define PCIE_PHY_STAT (PL_OFFSET + 0x110)
#define PCIE_PHY_STAT_ACK_LOC 16
-#define PCIE_LINK_WIDTH_SPEED_CONTROL 0x80C
-#define PORT_LOGIC_SPEED_CHANGE (0x1 << 17)
-
/* PHY registers (not memory-mapped) */
#define PCIE_PHY_ATEOVRD 0x10
#define PCIE_PHY_ATEOVRD_EN (0x1 << 2)
--
2.19.1
Code using these constants was removed in commit a71280722eeb ("PCI:
imx6: Remove LTSSM disable workaround"). No functional change
intended.
Cc: Lorenzo Pieralisi <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Lucas Stach <[email protected]>
Cc: Leonard Crestez <[email protected]>
Cc: "A.s. Dong" <[email protected]>
Cc: Richard Zhu <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Andrey Smirnov <[email protected]>
---
drivers/pci/controller/dwc/pci-imx6.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 2265f4b40e39..60803f1f16f8 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -82,9 +82,6 @@ struct imx6_pcie {
/* PCIe Port Logic registers (memory-mapped) */
#define PL_OFFSET 0x700
-#define PCIE_PL_PFLR (PL_OFFSET + 0x08)
-#define PCIE_PL_PFLR_LINK_STATE_MASK (0x3f << 16)
-#define PCIE_PL_PFLR_FORCE_LINK (1 << 15)
#define PCIE_PHY_DEBUG_R0 (PL_OFFSET + 0x28)
#define PCIE_PHY_DEBUG_R1 (PL_OFFSET + 0x2c)
#define PCIE_PHY_DEBUG_R1_XMLH_LINK_IN_TRAINING (1 << 29)
--
2.19.1
Change error code from EINVAL to ETIMEDOUT in
imx6_pcie_wait_for_speed_change() since that error code seems more
appropriate.
Cc: Lorenzo Pieralisi <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Lucas Stach <[email protected]>
Cc: Leonard Crestez <[email protected]>
Cc: "A.s. Dong" <[email protected]>
Cc: Richard Zhu <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Andrey Smirnov <[email protected]>
---
drivers/pci/controller/dwc/pci-imx6.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 6ffadc29d21b..e990030564d8 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -624,7 +624,7 @@ static int imx6_pcie_wait_for_speed_change(struct imx6_pcie *imx6_pcie)
}
dev_err(dev, "Speed change timeout\n");
- return -EINVAL;
+ return -ETIMEDOUT;
}
static void imx6_pcie_ltssm_enable(struct device *dev)
--
2.19.1
All calls to imx6_pcie_wait_for_link() share the same error path and
the state of PHY debug registers will already be printed there.
Cc: Lorenzo Pieralisi <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Lucas Stach <[email protected]>
Cc: Leonard Crestez <[email protected]>
Cc: "A.s. Dong" <[email protected]>
Cc: Richard Zhu <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Andrey Smirnov <[email protected]>
---
drivers/pci/controller/dwc/pci-imx6.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index c87ecc305dc7..6ffadc29d21b 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -603,16 +603,9 @@ static int imx6_setup_phy_mpll(struct imx6_pcie *imx6_pcie)
static int imx6_pcie_wait_for_link(struct imx6_pcie *imx6_pcie)
{
struct dw_pcie *pci = imx6_pcie->pci;
- struct device *dev = pci->dev;
/* check if the link is up or not */
- if (!dw_pcie_wait_for_link(pci))
- return 0;
-
- dev_dbg(dev, "DEBUG_R0: 0x%08x, DEBUG_R1: 0x%08x\n",
- dw_pcie_readl_dbi(pci, PCIE_PHY_DEBUG_R0),
- dw_pcie_readl_dbi(pci, PCIE_PHY_DEBUG_R1));
- return -ETIMEDOUT;
+ return dw_pcie_wait_for_link(pci);
}
static int imx6_pcie_wait_for_speed_change(struct imx6_pcie *imx6_pcie)
--
2.19.1
Simplify pcie_phy_poll_ack() by incorporating shifting into constant
definition and convert the code to use 'bool'. No functional change
intended.
Cc: Lorenzo Pieralisi <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Lucas Stach <[email protected]>
Cc: Leonard Crestez <[email protected]>
Cc: "A.s. Dong" <[email protected]>
Cc: Richard Zhu <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Andrey Smirnov <[email protected]>
---
drivers/pci/controller/dwc/pci-imx6.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 40d348bb9a2b..ddab1859a07e 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -91,7 +91,7 @@ struct imx6_pcie {
#define PCIE_PHY_CTRL_RD BIT(19)
#define PCIE_PHY_STAT (PL_OFFSET + 0x110)
-#define PCIE_PHY_STAT_ACK_LOC 16
+#define PCIE_PHY_STAT_ACK BIT(16)
/* PHY registers (not memory-mapped) */
#define PCIE_PHY_ATEOVRD 0x10
@@ -108,16 +108,16 @@ struct imx6_pcie {
#define PHY_RX_OVRD_IN_LO_RX_DATA_EN BIT(5)
#define PHY_RX_OVRD_IN_LO_RX_PLL_EN BIT(3)
-static int pcie_phy_poll_ack(struct imx6_pcie *imx6_pcie, int exp_val)
+static int pcie_phy_poll_ack(struct imx6_pcie *imx6_pcie, bool exp_val)
{
struct dw_pcie *pci = imx6_pcie->pci;
- u32 val;
+ bool val;
u32 max_iterations = 10;
u32 wait_counter = 0;
do {
- val = dw_pcie_readl_dbi(pci, PCIE_PHY_STAT);
- val = (val >> PCIE_PHY_STAT_ACK_LOC) & 0x1;
+ val = dw_pcie_readl_dbi(pci, PCIE_PHY_STAT) &
+ PCIE_PHY_STAT_ACK;
wait_counter++;
if (val == exp_val)
--
2.19.1
Until commit 4d107d3b5a68 ("PCI: imx6: Move link up check into
imx6_pcie_wait_for_link()") the driver relied on both LINK_UP and
LINK_IN_TRAINING to determine if PCIE link was up.
I can't seem to find out why, but code using LINK_IN_TRAINING seem to
disappear after commit 886bc5ceb5cc ("PCI: designware: Add generic
dw_pcie_wait_for_link()") and from then on only LINK_UP seems to be
tested (dw_pcie_wait_for_link() -> dw_pcie_link_up() ->
imx6_pcie_link_up()).
At the same time the default behavior of dw_pcie_link_up() went from
using just LINK_UP in commit dac29e6c5460 ("PCI: designware: Add
default link up check if sub-driver doesn't override") to using both
LINK_IN_TRAINING and LINK_UP in commit 01c076732e82 ("PCI: designware:
Check LTSSM training bit before deciding link is up").
Given all that it seems that i.MX6 doesn't really need a special
.link_up() callback and imx6_pcie_link_up() can be dropped.
Cc: Lorenzo Pieralisi <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Fabio Estevam <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Lucas Stach <[email protected]>
Cc: Leonard Crestez <[email protected]>
Cc: "A.s. Dong" <[email protected]>
Cc: Richard Zhu <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Andrey Smirnov <[email protected]>
---
drivers/pci/controller/dwc/pci-imx6.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 937d630f8c9d..74faca11eeae 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -84,8 +84,6 @@ struct imx6_pcie {
#define PL_OFFSET 0x700
#define PCIE_PHY_DEBUG_R0 (PL_OFFSET + 0x28)
#define PCIE_PHY_DEBUG_R1 (PL_OFFSET + 0x2c)
-#define PCIE_PHY_DEBUG_R1_XMLH_LINK_IN_TRAINING (1 << 29)
-#define PCIE_PHY_DEBUG_R1_XMLH_LINK_UP (1 << 4)
#define PCIE_PHY_CTRL (PL_OFFSET + 0x114)
#define PCIE_PHY_CTRL_DATA_LOC 0
@@ -732,12 +730,6 @@ static int imx6_pcie_host_init(struct pcie_port *pp)
return 0;
}
-static int imx6_pcie_link_up(struct dw_pcie *pci)
-{
- return dw_pcie_readl_dbi(pci, PCIE_PHY_DEBUG_R1) &
- PCIE_PHY_DEBUG_R1_XMLH_LINK_UP;
-}
-
static const struct dw_pcie_host_ops imx6_pcie_host_ops = {
.host_init = imx6_pcie_host_init,
};
@@ -769,9 +761,7 @@ static int imx6_add_pcie_port(struct imx6_pcie *imx6_pcie,
return 0;
}
-static const struct dw_pcie_ops dw_pcie_ops = {
- .link_up = imx6_pcie_link_up,
-};
+static const struct dw_pcie_ops dw_pcie_ops;
#ifdef CONFIG_PM_SLEEP
static void imx6_pcie_ltssm_disable(struct device *dev)
--
2.19.1
On Thu, 2018-12-20 at 23:27 -0800, Andrey Smirnov wrote:
> Until commit 4d107d3b5a68 ("PCI: imx6: Move link up check into
> imx6_pcie_wait_for_link()") the driver relied on both LINK_UP and
> LINK_IN_TRAINING to determine if PCIE link was up.
I've already got a patch in that fixed this issue. Queued for 4.19 and
4.14 stable.
The problem was created by 886bc5ceb5cc in one branch and by
4d107d3b5a68 on another branch, that when merged in 562df5c8521e, the
merge kept some pieces from each commit and dropped other pieces, which
resulted in this check getting dropped.
On Fri, Dec 21, 2018 at 10:55 AM Trent Piepho <[email protected]> wrote:
>
> On Thu, 2018-12-20 at 23:27 -0800, Andrey Smirnov wrote:
> > Until commit 4d107d3b5a68 ("PCI: imx6: Move link up check into
> > imx6_pcie_wait_for_link()") the driver relied on both LINK_UP and
> > LINK_IN_TRAINING to determine if PCIE link was up.
>
> I've already got a patch in that fixed this issue. Queued for 4.19 and
> 4.14 stable.
>
> The problem was created by 886bc5ceb5cc in one branch and by
> 4d107d3b5a68 on another branch, that when merged in 562df5c8521e, the
> merge kept some pieces from each commit and dropped other pieces, which
> resulted in this check getting dropped.
OK, cool, this patch can be dropped then.
Thanks,
Andrey Smirnov
Hi,
On 21/12/2018 07:27, Andrey Smirnov wrote:
> Convert various mult-bit fields to be defined using
> GENMASK/FIELD_PREP. This way bit field boundaries are defined in a
> single place only as well as defined in a way that makes it easier to
> verify them against reference manual. No functional change intended.
>
> Cc: Lorenzo Pieralisi <[email protected]>
> Cc: Bjorn Helgaas <[email protected]>
> Cc: Fabio Estevam <[email protected]>
> Cc: Chris Healy <[email protected]>
> Cc: Lucas Stach <[email protected]>
> Cc: Leonard Crestez <[email protected]>
> Cc: "A.s. Dong" <[email protected]>
> Cc: Richard Zhu <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Andrey Smirnov <[email protected]>
> ---
> drivers/pci/controller/dwc/pcie-designware.h | 29 +++++++++++---------
> 1 file changed, 16 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 348e91b6daa2..0de653284fca 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -11,6 +11,7 @@
> #ifndef _PCIE_DESIGNWARE_H
> #define _PCIE_DESIGNWARE_H
>
> +#include <linux/bitfield.h>
> #include <linux/dma-mapping.h>
> #include <linux/irq.h>
> #include <linux/msi.h>
> @@ -30,11 +31,12 @@
>
> /* Synopsys-specific PCIe configuration registers */
> #define PCIE_PORT_LINK_CONTROL 0x710
> -#define PORT_LINK_MODE_MASK (0x3f << 16)
> -#define PORT_LINK_MODE_1_LANES (0x1 << 16)
> -#define PORT_LINK_MODE_2_LANES (0x3 << 16)
> -#define PORT_LINK_MODE_4_LANES (0x7 << 16)
> -#define PORT_LINK_MODE_8_LANES (0xf << 16)
> +#define PORT_LINK_MODE_MASK GENMASK(21, 16)
> +#define PORT_LINK_MODE(n) FIELD_PREP(PORT_LINK_MODE_MASK, n)
> +#define PORT_LINK_MODE_1_LANES PORT_LINK_MODE(0x1)
> +#define PORT_LINK_MODE_2_LANES PORT_LINK_MODE(0x3)
> +#define PORT_LINK_MODE_4_LANES PORT_LINK_MODE(0x7)
> +#define PORT_LINK_MODE_8_LANES PORT_LINK_MODE(0xf)
>
> #define PCIE_PORT_DEBUG0 0x728
> #define PORT_LOGIC_LTSSM_STATE_MASK 0x1f
> @@ -45,11 +47,12 @@
>
> #define PCIE_LINK_WIDTH_SPEED_CONTROL 0x80C
> #define PORT_LOGIC_SPEED_CHANGE BIT(17)
> -#define PORT_LOGIC_LINK_WIDTH_MASK (0x1f << 8)
> -#define PORT_LOGIC_LINK_WIDTH_1_LANES (0x1 << 8)
> -#define PORT_LOGIC_LINK_WIDTH_2_LANES (0x2 << 8)
> -#define PORT_LOGIC_LINK_WIDTH_4_LANES (0x4 << 8)
> -#define PORT_LOGIC_LINK_WIDTH_8_LANES (0x8 << 8)
> +#define PORT_LOGIC_LINK_WIDTH_MASK GENMASK(12, 8)
> +#define PORT_LOGIC_LINK_WIDTH(n) FIELD_PREP(PORT_LOGIC_LINK_WIDTH_MASK, n)
> +#define PORT_LOGIC_LINK_WIDTH_1_LANES PORT_LOGIC_LINK_WIDTH(0x1)
> +#define PORT_LOGIC_LINK_WIDTH_2_LANES PORT_LOGIC_LINK_WIDTH(0x2)
> +#define PORT_LOGIC_LINK_WIDTH_4_LANES PORT_LOGIC_LINK_WIDTH(0x4)
> +#define PORT_LOGIC_LINK_WIDTH_8_LANES PORT_LOGIC_LINK_WIDTH(0x8)
>
> #define PCIE_MSI_ADDR_LO 0x820
> #define PCIE_MSI_ADDR_HI 0x824
> @@ -75,9 +78,9 @@
> #define PCIE_ATU_UPPER_BASE 0x910
> #define PCIE_ATU_LIMIT 0x914
> #define PCIE_ATU_LOWER_TARGET 0x918
> -#define PCIE_ATU_BUS(x) (((x) & 0xff) << 24)
> -#define PCIE_ATU_DEV(x) (((x) & 0x1f) << 19)
> -#define PCIE_ATU_FUNC(x) (((x) & 0x7) << 16)
> +#define PCIE_ATU_BUS(x) FIELD_PREP(GENMASK(31, 24), x)
> +#define PCIE_ATU_DEV(x) FIELD_PREP(GENMASK(23, 19), x)
> +#define PCIE_ATU_FUNC(x) FIELD_PREP(GENMASK(18, 16), x)
> #define PCIE_ATU_UPPER_TARGET 0x91C
>
> #define PCIE_MISC_CONTROL_1_OFF 0x8BC
>
I wasn't aware of the existence of FIELD_PREP(), seems to be quite handy :)
Acked-by: Gustavo Pimentel <[email protected]>
Thanks.
Hi,
On 21/12/2018 07:27, Andrey Smirnov wrote:
> Avoid using explicit left shifts and convert various definitions to
> use BIT() instead. No functional change intended.
>
> Cc: Lorenzo Pieralisi <[email protected]>
> Cc: Bjorn Helgaas <[email protected]>
> Cc: Fabio Estevam <[email protected]>
> Cc: Chris Healy <[email protected]>
> Cc: Lucas Stach <[email protected]>
> Cc: Leonard Crestez <[email protected]>
> Cc: "A.s. Dong" <[email protected]>
> Cc: Richard Zhu <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Andrey Smirnov <[email protected]>
> ---
> drivers/pci/controller/dwc/pcie-designware.c | 2 +-
> drivers/pci/controller/dwc/pcie-designware.h | 18 +++++++++---------
> 2 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index d123ac290b9e..086e87a40316 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -300,7 +300,7 @@ void dw_pcie_disable_atu(struct dw_pcie *pci, int index,
> }
>
> dw_pcie_writel_dbi(pci, PCIE_ATU_VIEWPORT, region | index);
> - dw_pcie_writel_dbi(pci, PCIE_ATU_CR2, ~PCIE_ATU_ENABLE);
> + dw_pcie_writel_dbi(pci, PCIE_ATU_CR2, (u32)~PCIE_ATU_ENABLE);
This is unrelated with the patch description purpose.
> }
>
> int dw_pcie_wait_for_link(struct dw_pcie *pci)
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 58735fd01668..348e91b6daa2 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -40,11 +40,11 @@
> #define PORT_LOGIC_LTSSM_STATE_MASK 0x1f
> #define PORT_LOGIC_LTSSM_STATE_L0 0x11
> #define PCIE_PORT_DEBUG1 0x72C
> -#define PCIE_PORT_DEBUG1_LINK_UP (0x1 << 4)
> -#define PCIE_PORT_DEBUG1_LINK_IN_TRAINING (0x1 << 29)
> +#define PCIE_PORT_DEBUG1_LINK_UP BIT(4)
> +#define PCIE_PORT_DEBUG1_LINK_IN_TRAINING BIT(29)
>
> #define PCIE_LINK_WIDTH_SPEED_CONTROL 0x80C
> -#define PORT_LOGIC_SPEED_CHANGE (0x1 << 17)
> +#define PORT_LOGIC_SPEED_CHANGE BIT(17)
> #define PORT_LOGIC_LINK_WIDTH_MASK (0x1f << 8)
> #define PORT_LOGIC_LINK_WIDTH_1_LANES (0x1 << 8)
> #define PORT_LOGIC_LINK_WIDTH_2_LANES (0x2 << 8)
> @@ -58,8 +58,8 @@
> #define PCIE_MSI_INTR0_STATUS 0x830
>
> #define PCIE_ATU_VIEWPORT 0x900
> -#define PCIE_ATU_REGION_INBOUND (0x1 << 31)
> -#define PCIE_ATU_REGION_OUTBOUND (0x0 << 31)
> +#define PCIE_ATU_REGION_INBOUND BIT(31)
> +#define PCIE_ATU_REGION_OUTBOUND 0
> #define PCIE_ATU_REGION_INDEX2 (0x2 << 0)
> #define PCIE_ATU_REGION_INDEX1 (0x1 << 0)
> #define PCIE_ATU_REGION_INDEX0 (0x0 << 0)
> @@ -69,8 +69,8 @@
> #define PCIE_ATU_TYPE_CFG0 (0x4 << 0)
> #define PCIE_ATU_TYPE_CFG1 (0x5 << 0)
> #define PCIE_ATU_CR2 0x908
> -#define PCIE_ATU_ENABLE (0x1 << 31)
> -#define PCIE_ATU_BAR_MODE_ENABLE (0x1 << 30)
> +#define PCIE_ATU_ENABLE BIT(31)
> +#define PCIE_ATU_BAR_MODE_ENABLE BIT(30)
> #define PCIE_ATU_LOWER_BASE 0x90C
> #define PCIE_ATU_UPPER_BASE 0x910
> #define PCIE_ATU_LIMIT 0x914
> @@ -81,7 +81,7 @@
> #define PCIE_ATU_UPPER_TARGET 0x91C
>
> #define PCIE_MISC_CONTROL_1_OFF 0x8BC
> -#define PCIE_DBI_RO_WR_EN (0x1 << 0)
> +#define PCIE_DBI_RO_WR_EN BIT(0)
>
> /*
> * iATU Unroll-specific register definitions
> @@ -108,7 +108,7 @@
> ((region) << 9)
>
> #define PCIE_GET_ATU_INB_UNR_REG_OFFSET(region) \
> - (((region) << 9) | (0x1 << 8))
> + (((region) << 9) | BIT(8))
>
> #define MAX_MSI_IRQS 256
> #define MAX_MSI_IRQS_PER_CTRL 32
>
Regards,
Gustavo
Hi,
On 21/12/2018 07:27, Andrey Smirnov wrote:
> Surrounding definitions no longer use explicit shift, so "<< 0" here
> serve no purpose. Remove them. No functional change intended.
>
> Cc: Lorenzo Pieralisi <[email protected]>
> Cc: Bjorn Helgaas <[email protected]>
> Cc: Fabio Estevam <[email protected]>
> Cc: Chris Healy <[email protected]>
> Cc: Lucas Stach <[email protected]>
> Cc: Leonard Crestez <[email protected]>
> Cc: "A.s. Dong" <[email protected]>
> Cc: Richard Zhu <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Andrey Smirnov <[email protected]>
> ---
> drivers/pci/controller/dwc/pcie-designware.h | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 0de653284fca..636689fd4ee7 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -63,14 +63,14 @@
> #define PCIE_ATU_VIEWPORT 0x900
> #define PCIE_ATU_REGION_INBOUND BIT(31)
> #define PCIE_ATU_REGION_OUTBOUND 0
> -#define PCIE_ATU_REGION_INDEX2 (0x2 << 0)
> -#define PCIE_ATU_REGION_INDEX1 (0x1 << 0)
> -#define PCIE_ATU_REGION_INDEX0 (0x0 << 0)
> +#define PCIE_ATU_REGION_INDEX2 0x2
> +#define PCIE_ATU_REGION_INDEX1 0x1
> +#define PCIE_ATU_REGION_INDEX0 0x0
> #define PCIE_ATU_CR1 0x904
> -#define PCIE_ATU_TYPE_MEM (0x0 << 0)
> -#define PCIE_ATU_TYPE_IO (0x2 << 0)
> -#define PCIE_ATU_TYPE_CFG0 (0x4 << 0)
> -#define PCIE_ATU_TYPE_CFG1 (0x5 << 0)
> +#define PCIE_ATU_TYPE_MEM 0x0
> +#define PCIE_ATU_TYPE_IO 0x2
> +#define PCIE_ATU_TYPE_CFG0 0x4
> +#define PCIE_ATU_TYPE_CFG1 0x5
> #define PCIE_ATU_CR2 0x908
> #define PCIE_ATU_ENABLE BIT(31)
> #define PCIE_ATU_BAR_MODE_ENABLE BIT(30)
>
Agree.
Acked-off-by: Gustavo Pimentel <[email protected]>
Thanks.
Hi,
On 21/12/2018 07:26, Andrey Smirnov wrote:
> Everyone:
>
> This is the series containing various small improvements that I made
> while reading the code and researching commit history of pci-imx6.c
> and pcie-designware*.c files. All changes are optional, so commits
> that don't seem like an improvement can be easily dropped. Hopefully
> each patch is self-explanatory.
>
> I tested this series on i.MX6Q, i.MX7D and i.MX8MQ.
>
> Feedback is welcome!
>
> Thanks,
> Andrey Smirnov
>
> Andrey Smirnov (21):
> PCI: imx6: Simplify imx7d_pcie_wait_for_phy_pll_lock()
> PCI: imx6: Remove redundant debug tracing
> PCI: imx6: Return -ETIMEOUT from imx6_pcie_wait_for_speed_change()
> PCI: imx6: Remove duplicate macro definitions
> PCI: imx6: Remove PCIE_PL_PFLR_* constants
> PCI: imx6: Remove PCIE_PHY_RX_ASIC_OUT* constants
> PCI: designware: Make use of IS_ALIGNED()
> PCI: designware: Share code for dw_pcie_rd/wr_other_conf()
> PCI: imx6: Drop imx6_pcie_link_up()
> PCI: designware: imx6: Share PHY debug register definitions
> PCI: designware: Make use of BIT() in constant definitions
> PCI: imx6: Make use of BIT() in constant definitions
> PCI: imx6: Simplify bit operations in PHY functions
> PCI: imx6: Simplify pcie_phy_poll_ack()
> PCI: imx6: Restrict PHY register data to 16-bit
> PCI: imx6: Pass data to dw_pcie_writel_dbi() directly
> PCI: imx6: Use common mask in imx6_pcie_reset_phy()
> PCI: imx6: Simplify bit operations in imx6_setup_phy_mpll()
> PCI: imx6: Remove magic numbers from imx6_pcie_establish_link()
> PCI: designware: Make use of GENMASK/FIELD_PREP
> PCI: designware: Remove superfluous shifting in definitions
s/DesignWare/dwc in all patches, please.
Regards,
Gustavo
>
> drivers/pci/controller/dwc/pci-imx6.c | 151 +++++++-----------
> .../pci/controller/dwc/pcie-designware-host.c | 61 +++----
> drivers/pci/controller/dwc/pcie-designware.c | 18 +--
> drivers/pci/controller/dwc/pcie-designware.h | 60 +++----
> 4 files changed, 116 insertions(+), 174 deletions(-)
>
Hi,
On 21/12/2018 07:27, Andrey Smirnov wrote:
> Make the intent a bit more clear as well as get rid of explicit
> arithmetic by using IS_ALIGNED() to determine if "addr" is aligned to
> "size". No functional change intended.
>
> Cc: Lorenzo Pieralisi <[email protected]>
> Cc: Bjorn Helgaas <[email protected]>
> Cc: Fabio Estevam <[email protected]>
> Cc: Chris Healy <[email protected]>
> Cc: Lucas Stach <[email protected]>
> Cc: Leonard Crestez <[email protected]>
> Cc: "A.s. Dong" <[email protected]>
> Cc: Richard Zhu <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Andrey Smirnov <[email protected]>
> ---
> drivers/pci/controller/dwc/pcie-designware.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 93ef8c31fb39..67236379c61a 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -22,7 +22,7 @@
>
> int dw_pcie_read(void __iomem *addr, int size, u32 *val)
> {
> - if ((uintptr_t)addr & (size - 1)) {
> + if (!IS_ALIGNED((uintptr_t)addr, size)) {
> *val = 0;
> return PCIBIOS_BAD_REGISTER_NUMBER;
> }
> @@ -43,7 +43,7 @@ int dw_pcie_read(void __iomem *addr, int size, u32 *val)
>
> int dw_pcie_write(void __iomem *addr, int size, u32 val)
> {
> - if ((uintptr_t)addr & (size - 1))
> + if (!IS_ALIGNED((uintptr_t)addr, size))
> return PCIBIOS_BAD_REGISTER_NUMBER;
>
> if (size == 4)
>
Sounds good.
Acked-by: Gustavo Pimentel <[email protected]>
Hi,
On 21/12/2018 07:27, Andrey Smirnov wrote:
> Default implementation of pcie_rd_other_conf() and
> dw_pcie_wd_other_conf() share more than 80% of their code. Move shared
> code into a dedicated subroutine and convert pcie_rd_other_conf() and
> dw_pcie_wd_other_conf() to use it. No functional change intended.
>
> Cc: Lorenzo Pieralisi <[email protected]>
> Cc: Bjorn Helgaas <[email protected]>
> Cc: Fabio Estevam <[email protected]>
> Cc: Chris Healy <[email protected]>
> Cc: Lucas Stach <[email protected]>
> Cc: Leonard Crestez <[email protected]>
> Cc: "A.s. Dong" <[email protected]>
> Cc: Richard Zhu <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Andrey Smirnov <[email protected]>
> ---
> .../pci/controller/dwc/pcie-designware-host.c | 61 +++++++------------
> 1 file changed, 23 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 721d60a5d9e4..8f957cd6901b 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -512,8 +512,9 @@ int dw_pcie_host_init(struct pcie_port *pp)
> return ret;
> }
>
> -static int dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus,
> - u32 devfn, int where, int size, u32 *val)
> +static int dw_pcie_access_other_conf(struct pcie_port *pp, struct pci_bus *bus,
> + u32 devfn, int where, int size, u32 *val,
> + bool write)
> {
> int ret, type;
> u32 busdev, cfg_size;
> @@ -521,9 +522,6 @@ static int dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus,
> void __iomem *va_cfg_base;
> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>
> - if (pp->ops->rd_other_conf)
> - return pp->ops->rd_other_conf(pp, bus, devfn, where, size, val);
> -
> busdev = PCIE_ATU_BUS(bus->number) | PCIE_ATU_DEV(PCI_SLOT(devfn)) |
> PCIE_ATU_FUNC(PCI_FUNC(devfn));
>
> @@ -542,7 +540,11 @@ static int dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus,
> dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX1,
> type, cpu_addr,
> busdev, cfg_size);
> - ret = dw_pcie_read(va_cfg_base + where, size, val);
> + if (write)
> + ret = dw_pcie_write(va_cfg_base + where, size, *val);
> + else
> + ret = dw_pcie_read(va_cfg_base + where, size, val);
> +
> if (pci->num_viewport <= 2)
> dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX1,
> PCIE_ATU_TYPE_IO, pp->io_base,
> @@ -551,43 +553,26 @@ static int dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus,
> return ret;
> }
>
> +static int dw_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus,
> + u32 devfn, int where, int size, u32 *val)
> +{
> + if (pp->ops->rd_other_conf)
> + return pp->ops->rd_other_conf(pp, bus, devfn, where,
> + size, val);
> +
> + return dw_pcie_access_other_conf(pp, bus, devfn, where, size, val,
> + false);
> +}
> +
> static int dw_pcie_wr_other_conf(struct pcie_port *pp, struct pci_bus *bus,
> u32 devfn, int where, int size, u32 val)
> {
> - int ret, type;
> - u32 busdev, cfg_size;
> - u64 cpu_addr;
> - void __iomem *va_cfg_base;
> - struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> -
> if (pp->ops->wr_other_conf)
> - return pp->ops->wr_other_conf(pp, bus, devfn, where, size, val);
> -
> - busdev = PCIE_ATU_BUS(bus->number) | PCIE_ATU_DEV(PCI_SLOT(devfn)) |
> - PCIE_ATU_FUNC(PCI_FUNC(devfn));
> + return pp->ops->wr_other_conf(pp, bus, devfn, where,
> + size, val);
>
> - if (bus->parent->number == pp->root_bus_nr) {
> - type = PCIE_ATU_TYPE_CFG0;
> - cpu_addr = pp->cfg0_base;
> - cfg_size = pp->cfg0_size;
> - va_cfg_base = pp->va_cfg0_base;
> - } else {
> - type = PCIE_ATU_TYPE_CFG1;
> - cpu_addr = pp->cfg1_base;
> - cfg_size = pp->cfg1_size;
> - va_cfg_base = pp->va_cfg1_base;
> - }
> -
> - dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX1,
> - type, cpu_addr,
> - busdev, cfg_size);
> - ret = dw_pcie_write(va_cfg_base + where, size, val);
> - if (pci->num_viewport <= 2)
> - dw_pcie_prog_outbound_atu(pci, PCIE_ATU_REGION_INDEX1,
> - PCIE_ATU_TYPE_IO, pp->io_base,
> - pp->io_bus_addr, pp->io_size);
> -
> - return ret;
> + return dw_pcie_access_other_conf(pp, bus, devfn, where, size, &val,
> + true);
> }
>
> static int dw_pcie_valid_device(struct pcie_port *pp, struct pci_bus *bus,
>
Nice!
Acked-by: Gustavo Pimentel <[email protected]>
Regards,
Gustavo
On Wed, Dec 26, 2018 at 7:19 AM Gustavo Pimentel
<[email protected]> wrote:
>
> Hi,
>
> On 21/12/2018 07:27, Andrey Smirnov wrote:
> > Avoid using explicit left shifts and convert various definitions to
> > use BIT() instead. No functional change intended.
> >
> > Cc: Lorenzo Pieralisi <[email protected]>
> > Cc: Bjorn Helgaas <[email protected]>
> > Cc: Fabio Estevam <[email protected]>
> > Cc: Chris Healy <[email protected]>
> > Cc: Lucas Stach <[email protected]>
> > Cc: Leonard Crestez <[email protected]>
> > Cc: "A.s. Dong" <[email protected]>
> > Cc: Richard Zhu <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Signed-off-by: Andrey Smirnov <[email protected]>
> > ---
> > drivers/pci/controller/dwc/pcie-designware.c | 2 +-
> > drivers/pci/controller/dwc/pcie-designware.h | 18 +++++++++---------
> > 2 files changed, 10 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > index d123ac290b9e..086e87a40316 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > @@ -300,7 +300,7 @@ void dw_pcie_disable_atu(struct dw_pcie *pci, int index,
> > }
> >
> > dw_pcie_writel_dbi(pci, PCIE_ATU_VIEWPORT, region | index);
> > - dw_pcie_writel_dbi(pci, PCIE_ATU_CR2, ~PCIE_ATU_ENABLE);
> > + dw_pcie_writel_dbi(pci, PCIE_ATU_CR2, (u32)~PCIE_ATU_ENABLE);
>
> This is unrelated with the patch description purpose.
>
This is a direct result of converting PCIE_ATU_ENABLE to BIT(31).
BIT(31) expands to (1UL << 31) so, without that cast I get
drivers/pci/controller/dwc/pcie-designware.c: In function
‘dw_pcie_disable_atu’:
drivers/pci/controller/dwc/pcie-designware.c:303:40: warning: large
integer implicitly truncated to unsigned type [-Woverflow]
dw_pcie_writel_dbi(pci, PCIE_ATU_CR2, ~PCIE_ATU_ENABLE);
on AArch64. I am guessing that original definition of (1 << 31) avoids
this problem by being an "int" instead of "unsigned long".
Thanks,
Andrey Smirnov
Hi,
On 02/01/2019 18:28, Andrey Smirnov wrote:
> On Wed, Dec 26, 2018 at 7:19 AM Gustavo Pimentel
> <[email protected]> wrote:
>>
>> Hi,
>>
>> On 21/12/2018 07:27, Andrey Smirnov wrote:
>>> Avoid using explicit left shifts and convert various definitions to
>>> use BIT() instead. No functional change intended.
>>>
>>> Cc: Lorenzo Pieralisi <[email protected]>
>>> Cc: Bjorn Helgaas <[email protected]>
>>> Cc: Fabio Estevam <[email protected]>
>>> Cc: Chris Healy <[email protected]>
>>> Cc: Lucas Stach <[email protected]>
>>> Cc: Leonard Crestez <[email protected]>
>>> Cc: "A.s. Dong" <[email protected]>
>>> Cc: Richard Zhu <[email protected]>
>>> Cc: [email protected]
>>> Cc: [email protected]
>>> Cc: [email protected]
>>> Cc: [email protected]
>>> Signed-off-by: Andrey Smirnov <[email protected]>
>>> ---
>>> drivers/pci/controller/dwc/pcie-designware.c | 2 +-
>>> drivers/pci/controller/dwc/pcie-designware.h | 18 +++++++++---------
>>> 2 files changed, 10 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
>>> index d123ac290b9e..086e87a40316 100644
>>> --- a/drivers/pci/controller/dwc/pcie-designware.c
>>> +++ b/drivers/pci/controller/dwc/pcie-designware.c
>>> @@ -300,7 +300,7 @@ void dw_pcie_disable_atu(struct dw_pcie *pci, int index,
>>> }
>>>
>>> dw_pcie_writel_dbi(pci, PCIE_ATU_VIEWPORT, region | index);
>>> - dw_pcie_writel_dbi(pci, PCIE_ATU_CR2, ~PCIE_ATU_ENABLE);
>>> + dw_pcie_writel_dbi(pci, PCIE_ATU_CR2, (u32)~PCIE_ATU_ENABLE);
>>
>> This is unrelated with the patch description purpose.
>>
>
> This is a direct result of converting PCIE_ATU_ENABLE to BIT(31).
> BIT(31) expands to (1UL << 31) so, without that cast I get
>
> drivers/pci/controller/dwc/pcie-designware.c: In function
> ‘dw_pcie_disable_atu’:
> drivers/pci/controller/dwc/pcie-designware.c:303:40: warning: large
> integer implicitly truncated to unsigned type [-Woverflow]
> dw_pcie_writel_dbi(pci, PCIE_ATU_CR2, ~PCIE_ATU_ENABLE);
>
> on AArch64. I am guessing that original definition of (1 << 31) avoids
> this problem by being an "int" instead of "unsigned long".
Ok, understood.
Acked-by: Gustavo Pimentel <[email protected]>
>
> Thanks,
> Andrey Smirnov
>
On Fri, Jan 4, 2019 at 10:38 AM Joe Perches <[email protected]> wrote:
>
> On Wed, 2019-01-02 at 09:33 +0000, Gustavo Pimentel wrote:
> > On 21/12/2018 07:27, Andrey Smirnov wrote:
> > > Make the intent a bit more clear as well as get rid of explicit
> > > arithmetic by using IS_ALIGNED() to determine if "addr" is aligned to
> > > "size". No functional change intended.
> []
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> []
> > > @@ -22,7 +22,7 @@
> > >
> > > int dw_pcie_read(void __iomem *addr, int size, u32 *val)
> > > {
> > > - if ((uintptr_t)addr & (size - 1)) {
> > > + if (!IS_ALIGNED((uintptr_t)addr, size)) {
>
> The (uintptr_t) cast could probably be removed as well.
>
Unfortunately no. IS_ALIGNED(x, a) is going to expand (((x) &
((typeof(x))(a) - 1)) == 0), so we'll end up with (((addr) & ((void
*)(size) - 1)) which has two problems:
1. It tries to use & operator on two void * pointers
2. On 64-bit platforms (e.g. AArch64) it will try to cast an "int" to
a pointer, resulting in "warning: cast to pointer from integer of
different size"
When working on it, I initially wrote the code without the cast, but
was quickly reprimanded by GCC and had to add it back in.
Thanks,
Andrey Smirnov
On Wed, 2019-01-02 at 09:33 +0000, Gustavo Pimentel wrote:
> On 21/12/2018 07:27, Andrey Smirnov wrote:
> > Make the intent a bit more clear as well as get rid of explicit
> > arithmetic by using IS_ALIGNED() to determine if "addr" is aligned to
> > "size". No functional change intended.
[]
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
[]
> > @@ -22,7 +22,7 @@
> >
> > int dw_pcie_read(void __iomem *addr, int size, u32 *val)
> > {
> > - if ((uintptr_t)addr & (size - 1)) {
> > + if (!IS_ALIGNED((uintptr_t)addr, size)) {
The (uintptr_t) cast could probably be removed as well.
> > @@ -43,7 +43,7 @@ int dw_pcie_read(void __iomem *addr, int size, u32 *val)
> >
> > int dw_pcie_write(void __iomem *addr, int size, u32 val)
> > {
> > - if ((uintptr_t)addr & (size - 1))
> > + if (!IS_ALIGNED((uintptr_t)addr, size))
> > return PCIBIOS_BAD_REGISTER_NUMBER;
here too