2019-09-26 10:55:41

by Kefeng Wang

[permalink] [raw]
Subject: [PATCH] PCI: mobiveil: Fix csr_read/write build issue

The riscv has csr_read/write macro, see arch/riscv/include/asm/csr.h,
the same function naming will cause build error, rename them to
__csr_read/write to fix it.

drivers/pci/controller/pcie-mobiveil.c:238:69: error: macro "csr_read" passed 3 arguments, but takes just 1
static u32 csr_read(struct mobiveil_pcie *pcie, u32 off, size_t size)

drivers/pci/controller/pcie-mobiveil.c:253:80: error: macro "csr_write" passed 4 arguments, but takes just 2
static void csr_write(struct mobiveil_pcie *pcie, u32 val, u32 off, size_t size)

Cc: Hou Zhiqiang <[email protected]>
Cc: Lorenzo Pieralisi <[email protected]>
Cc: Minghuan Lian <[email protected]>
Cc: Subrahmanya Lingappa <[email protected]>
Cc: Karthikeyan Mitran <[email protected]>
Cc: Lorenzo Pieralisi <[email protected]>
Fixes: bcbe0d9a8d93 ("PCI: mobiveil: Unify register accessors")
Signed-off-by: Kefeng Wang <[email protected]>
---
drivers/pci/controller/pcie-mobiveil.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/controller/pcie-mobiveil.c b/drivers/pci/controller/pcie-mobiveil.c
index a45a6447b01d..7ba1138c3667 100644
--- a/drivers/pci/controller/pcie-mobiveil.c
+++ b/drivers/pci/controller/pcie-mobiveil.c
@@ -235,7 +235,7 @@ static int mobiveil_pcie_write(void __iomem *addr, int size, u32 val)
return PCIBIOS_SUCCESSFUL;
}

-static u32 csr_read(struct mobiveil_pcie *pcie, u32 off, size_t size)
+static u32 __csr_read(struct mobiveil_pcie *pcie, u32 off, size_t size)
{
void *addr;
u32 val;
@@ -250,7 +250,7 @@ static u32 csr_read(struct mobiveil_pcie *pcie, u32 off, size_t size)
return val;
}

-static void csr_write(struct mobiveil_pcie *pcie, u32 val, u32 off, size_t size)
+static void __csr_write(struct mobiveil_pcie *pcie, u32 val, u32 off, size_t size)
{
void *addr;
int ret;
@@ -264,12 +264,12 @@ static void csr_write(struct mobiveil_pcie *pcie, u32 val, u32 off, size_t size)

static u32 csr_readl(struct mobiveil_pcie *pcie, u32 off)
{
- return csr_read(pcie, off, 0x4);
+ return __csr_read(pcie, off, 0x4);
}

static void csr_writel(struct mobiveil_pcie *pcie, u32 val, u32 off)
{
- csr_write(pcie, val, off, 0x4);
+ __csr_write(pcie, val, off, 0x4);
}

static bool mobiveil_pcie_link_up(struct mobiveil_pcie *pcie)
--
2.20.1


2019-09-26 12:43:00

by Andrew Murray

[permalink] [raw]
Subject: Re: [PATCH] PCI: mobiveil: Fix csr_read/write build issue

On Wed, Sep 25, 2019 at 10:21:21PM +0800, Kefeng Wang wrote:
> The riscv has csr_read/write macro, see arch/riscv/include/asm/csr.h,
> the same function naming will cause build error, rename them to
> __csr_read/write to fix it.
>
> drivers/pci/controller/pcie-mobiveil.c:238:69: error: macro "csr_read" passed 3 arguments, but takes just 1
> static u32 csr_read(struct mobiveil_pcie *pcie, u32 off, size_t size)
>
> drivers/pci/controller/pcie-mobiveil.c:253:80: error: macro "csr_write" passed 4 arguments, but takes just 2
> static void csr_write(struct mobiveil_pcie *pcie, u32 val, u32 off, size_t size)
>
> Cc: Hou Zhiqiang <[email protected]>
> Cc: Lorenzo Pieralisi <[email protected]>
> Cc: Minghuan Lian <[email protected]>
> Cc: Subrahmanya Lingappa <[email protected]>
> Cc: Karthikeyan Mitran <[email protected]>
> Cc: Lorenzo Pieralisi <[email protected]>
> Fixes: bcbe0d9a8d93 ("PCI: mobiveil: Unify register accessors")
> Signed-off-by: Kefeng Wang <[email protected]>
> ---
> drivers/pci/controller/pcie-mobiveil.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/controller/pcie-mobiveil.c b/drivers/pci/controller/pcie-mobiveil.c
> index a45a6447b01d..7ba1138c3667 100644
> --- a/drivers/pci/controller/pcie-mobiveil.c
> +++ b/drivers/pci/controller/pcie-mobiveil.c
> @@ -235,7 +235,7 @@ static int mobiveil_pcie_write(void __iomem *addr, int size, u32 val)
> return PCIBIOS_SUCCESSFUL;
> }
>
> -static u32 csr_read(struct mobiveil_pcie *pcie, u32 off, size_t size)
> +static u32 __csr_read(struct mobiveil_pcie *pcie, u32 off, size_t size)
> {
> void *addr;
> u32 val;
> @@ -250,7 +250,7 @@ static u32 csr_read(struct mobiveil_pcie *pcie, u32 off, size_t size)
> return val;
> }
>
> -static void csr_write(struct mobiveil_pcie *pcie, u32 val, u32 off, size_t size)
> +static void __csr_write(struct mobiveil_pcie *pcie, u32 val, u32 off, size_t size)
> {
> void *addr;
> int ret;
> @@ -264,12 +264,12 @@ static void csr_write(struct mobiveil_pcie *pcie, u32 val, u32 off, size_t size)
>
> static u32 csr_readl(struct mobiveil_pcie *pcie, u32 off)
> {
> - return csr_read(pcie, off, 0x4);
> + return __csr_read(pcie, off, 0x4);
> }
>
> static void csr_writel(struct mobiveil_pcie *pcie, u32 val, u32 off)
> {
> - csr_write(pcie, val, off, 0x4);
> + __csr_write(pcie, val, off, 0x4);
> }

Reviewed-by: Andrew Murray <[email protected]>

Though I'd be just as happy if the csr_[read,write][l,] functions were renamed
to mobiveil_csr_[read,write][l,].

>
> static bool mobiveil_pcie_link_up(struct mobiveil_pcie *pcie)
> --
> 2.20.1
>

2019-09-27 23:16:52

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH] PCI: mobiveil: Fix csr_read/write build issue

On Thu, Sep 26, 2019 at 10:29:34AM +0100, Andrew Murray wrote:
> Though I'd be just as happy if the csr_[read,write][l,] functions were
> renamed to mobiveil_csr_[read,write][l,].

Please do that instead, using such generic names as csr_* in a driver
is a bad idea, with or without a __ prefix.

2019-09-29 01:19:27

by Kefeng Wang

[permalink] [raw]
Subject: [PATCH v2] PCI: mobiveil: Fix csr_read/write build issue

The riscv has csr_read/write macro, see arch/riscv/include/asm/csr.h,
the same function naming will cause build error, using such generic names
in a driver is bad, rename csr_[read,write][l,] to mobiveil_csr_read/write
to fix it.

drivers/pci/controller/pcie-mobiveil.c:238:69: error: macro "csr_read" passed 3 arguments, but takes just 1
static u32 csr_read(struct mobiveil_pcie *pcie, u32 off, size_t size)

drivers/pci/controller/pcie-mobiveil.c:253:80: error: macro "csr_write" passed 4 arguments, but takes just 2
static void csr_write(struct mobiveil_pcie *pcie, u32 val, u32 off, size_t size)

Cc: Hou Zhiqiang <[email protected]>
Cc: Lorenzo Pieralisi <[email protected]>
Cc: Minghuan Lian <[email protected]>
Cc: Subrahmanya Lingappa <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Andrew Murray <[email protected]>
Fixes: bcbe0d9a8d93 ("PCI: mobiveil: Unify register accessors")
Signed-off-by: Kefeng Wang <[email protected]>
---

v2:
- using mobiveil prefix suggested by Andrew and Christoph

drivers/pci/controller/pcie-mobiveil.c | 115 +++++++++++++------------
1 file changed, 58 insertions(+), 57 deletions(-)

diff --git a/drivers/pci/controller/pcie-mobiveil.c b/drivers/pci/controller/pcie-mobiveil.c
index a45a6447b01d..5e6144b0fb95 100644
--- a/drivers/pci/controller/pcie-mobiveil.c
+++ b/drivers/pci/controller/pcie-mobiveil.c
@@ -235,7 +235,7 @@ static int mobiveil_pcie_write(void __iomem *addr, int size, u32 val)
return PCIBIOS_SUCCESSFUL;
}

-static u32 csr_read(struct mobiveil_pcie *pcie, u32 off, size_t size)
+static u32 mobiveil_csr_read(struct mobiveil_pcie *pcie, u32 off, size_t size)
{
void *addr;
u32 val;
@@ -250,7 +250,8 @@ static u32 csr_read(struct mobiveil_pcie *pcie, u32 off, size_t size)
return val;
}

-static void csr_write(struct mobiveil_pcie *pcie, u32 val, u32 off, size_t size)
+static void mobiveil_csr_write(struct mobiveil_pcie *pcie, u32 val, u32 off,
+ size_t size)
{
void *addr;
int ret;
@@ -262,19 +263,19 @@ static void csr_write(struct mobiveil_pcie *pcie, u32 val, u32 off, size_t size)
dev_err(&pcie->pdev->dev, "write CSR address failed\n");
}

-static u32 csr_readl(struct mobiveil_pcie *pcie, u32 off)
+static u32 mobiveil_csr_readl(struct mobiveil_pcie *pcie, u32 off)
{
- return csr_read(pcie, off, 0x4);
+ return mobiveil_csr_read(pcie, off, 0x4);
}

-static void csr_writel(struct mobiveil_pcie *pcie, u32 val, u32 off)
+static void mobiveil_csr_writel(struct mobiveil_pcie *pcie, u32 val, u32 off)
{
- csr_write(pcie, val, off, 0x4);
+ mobiveil_csr_write(pcie, val, off, 0x4);
}

static bool mobiveil_pcie_link_up(struct mobiveil_pcie *pcie)
{
- return (csr_readl(pcie, LTSSM_STATUS) &
+ return (mobiveil_csr_readl(pcie, LTSSM_STATUS) &
LTSSM_STATUS_L0_MASK) == LTSSM_STATUS_L0;
}

@@ -323,7 +324,7 @@ static void __iomem *mobiveil_pcie_map_bus(struct pci_bus *bus,
PCI_SLOT(devfn) << PAB_DEVICE_SHIFT |
PCI_FUNC(devfn) << PAB_FUNCTION_SHIFT;

- csr_writel(pcie, value, PAB_AXI_AMAP_PEX_WIN_L(WIN_NUM_0));
+ mobiveil_csr_writel(pcie, value, PAB_AXI_AMAP_PEX_WIN_L(WIN_NUM_0));

return pcie->config_axi_slave_base + where;
}
@@ -353,13 +354,13 @@ static void mobiveil_pcie_isr(struct irq_desc *desc)
chained_irq_enter(chip, desc);

/* read INTx status */
- val = csr_readl(pcie, PAB_INTP_AMBA_MISC_STAT);
- mask = csr_readl(pcie, PAB_INTP_AMBA_MISC_ENB);
+ val = mobiveil_csr_readl(pcie, PAB_INTP_AMBA_MISC_STAT);
+ mask = mobiveil_csr_readl(pcie, PAB_INTP_AMBA_MISC_ENB);
intr_status = val & mask;

/* Handle INTx */
if (intr_status & PAB_INTP_INTX_MASK) {
- shifted_status = csr_readl(pcie, PAB_INTP_AMBA_MISC_STAT);
+ shifted_status = mobiveil_csr_readl(pcie, PAB_INTP_AMBA_MISC_STAT);
shifted_status &= PAB_INTP_INTX_MASK;
shifted_status >>= PAB_INTX_START;
do {
@@ -373,12 +374,12 @@ static void mobiveil_pcie_isr(struct irq_desc *desc)
bit);

/* clear interrupt handled */
- csr_writel(pcie, 1 << (PAB_INTX_START + bit),
- PAB_INTP_AMBA_MISC_STAT);
+ mobiveil_csr_writel(pcie, 1 << (PAB_INTX_START + bit),
+ PAB_INTP_AMBA_MISC_STAT);
}

- shifted_status = csr_readl(pcie,
- PAB_INTP_AMBA_MISC_STAT);
+ shifted_status = mobiveil_csr_readl(pcie,
+ PAB_INTP_AMBA_MISC_STAT);
shifted_status &= PAB_INTP_INTX_MASK;
shifted_status >>= PAB_INTX_START;
} while (shifted_status != 0);
@@ -413,7 +414,7 @@ static void mobiveil_pcie_isr(struct irq_desc *desc)
}

/* Clear the interrupt status */
- csr_writel(pcie, intr_status, PAB_INTP_AMBA_MISC_STAT);
+ mobiveil_csr_writel(pcie, intr_status, PAB_INTP_AMBA_MISC_STAT);
chained_irq_exit(chip, desc);
}

@@ -474,24 +475,24 @@ static void program_ib_windows(struct mobiveil_pcie *pcie, int win_num,
return;
}

- value = csr_readl(pcie, PAB_PEX_AMAP_CTRL(win_num));
+ value = mobiveil_csr_readl(pcie, PAB_PEX_AMAP_CTRL(win_num));
value &= ~(AMAP_CTRL_TYPE_MASK << AMAP_CTRL_TYPE_SHIFT | WIN_SIZE_MASK);
value |= type << AMAP_CTRL_TYPE_SHIFT | 1 << AMAP_CTRL_EN_SHIFT |
(lower_32_bits(size64) & WIN_SIZE_MASK);
- csr_writel(pcie, value, PAB_PEX_AMAP_CTRL(win_num));
+ mobiveil_csr_writel(pcie, value, PAB_PEX_AMAP_CTRL(win_num));

- csr_writel(pcie, upper_32_bits(size64),
- PAB_EXT_PEX_AMAP_SIZEN(win_num));
+ mobiveil_csr_writel(pcie, upper_32_bits(size64),
+ PAB_EXT_PEX_AMAP_SIZEN(win_num));

- csr_writel(pcie, lower_32_bits(cpu_addr),
- PAB_PEX_AMAP_AXI_WIN(win_num));
- csr_writel(pcie, upper_32_bits(cpu_addr),
- PAB_EXT_PEX_AMAP_AXI_WIN(win_num));
+ mobiveil_csr_writel(pcie, lower_32_bits(cpu_addr),
+ PAB_PEX_AMAP_AXI_WIN(win_num));
+ mobiveil_csr_writel(pcie, upper_32_bits(cpu_addr),
+ PAB_EXT_PEX_AMAP_AXI_WIN(win_num));

- csr_writel(pcie, lower_32_bits(pci_addr),
- PAB_PEX_AMAP_PEX_WIN_L(win_num));
- csr_writel(pcie, upper_32_bits(pci_addr),
- PAB_PEX_AMAP_PEX_WIN_H(win_num));
+ mobiveil_csr_writel(pcie, lower_32_bits(pci_addr),
+ PAB_PEX_AMAP_PEX_WIN_L(win_num));
+ mobiveil_csr_writel(pcie, upper_32_bits(pci_addr),
+ PAB_PEX_AMAP_PEX_WIN_H(win_num));

pcie->ib_wins_configured++;
}
@@ -515,27 +516,27 @@ static void program_ob_windows(struct mobiveil_pcie *pcie, int win_num,
* program Enable Bit to 1, Type Bit to (00) base 2, AXI Window Size Bit
* to 4 KB in PAB_AXI_AMAP_CTRL register
*/
- value = csr_readl(pcie, PAB_AXI_AMAP_CTRL(win_num));
+ value = mobiveil_csr_readl(pcie, PAB_AXI_AMAP_CTRL(win_num));
value &= ~(WIN_TYPE_MASK << WIN_TYPE_SHIFT | WIN_SIZE_MASK);
value |= 1 << WIN_ENABLE_SHIFT | type << WIN_TYPE_SHIFT |
(lower_32_bits(size64) & WIN_SIZE_MASK);
- csr_writel(pcie, value, PAB_AXI_AMAP_CTRL(win_num));
+ mobiveil_csr_writel(pcie, value, PAB_AXI_AMAP_CTRL(win_num));

- csr_writel(pcie, upper_32_bits(size64), PAB_EXT_AXI_AMAP_SIZE(win_num));
+ mobiveil_csr_writel(pcie, upper_32_bits(size64), PAB_EXT_AXI_AMAP_SIZE(win_num));

/*
* program AXI window base with appropriate value in
* PAB_AXI_AMAP_AXI_WIN0 register
*/
- csr_writel(pcie, lower_32_bits(cpu_addr) & (~AXI_WINDOW_ALIGN_MASK),
- PAB_AXI_AMAP_AXI_WIN(win_num));
- csr_writel(pcie, upper_32_bits(cpu_addr),
- PAB_EXT_AXI_AMAP_AXI_WIN(win_num));
+ mobiveil_csr_writel(pcie, lower_32_bits(cpu_addr) & (~AXI_WINDOW_ALIGN_MASK),
+ PAB_AXI_AMAP_AXI_WIN(win_num));
+ mobiveil_csr_writel(pcie, upper_32_bits(cpu_addr),
+ PAB_EXT_AXI_AMAP_AXI_WIN(win_num));

- csr_writel(pcie, lower_32_bits(pci_addr),
- PAB_AXI_AMAP_PEX_WIN_L(win_num));
- csr_writel(pcie, upper_32_bits(pci_addr),
- PAB_AXI_AMAP_PEX_WIN_H(win_num));
+ mobiveil_csr_writel(pcie, lower_32_bits(pci_addr),
+ PAB_AXI_AMAP_PEX_WIN_L(win_num));
+ mobiveil_csr_writel(pcie, upper_32_bits(pci_addr),
+ PAB_AXI_AMAP_PEX_WIN_H(win_num));

pcie->ob_wins_configured++;
}
@@ -579,42 +580,42 @@ static int mobiveil_host_init(struct mobiveil_pcie *pcie)
struct resource_entry *win;

/* setup bus numbers */
- value = csr_readl(pcie, PCI_PRIMARY_BUS);
+ value = mobiveil_csr_readl(pcie, PCI_PRIMARY_BUS);
value &= 0xff000000;
value |= 0x00ff0100;
- csr_writel(pcie, value, PCI_PRIMARY_BUS);
+ mobiveil_csr_writel(pcie, value, PCI_PRIMARY_BUS);

/*
* program Bus Master Enable Bit in Command Register in PAB Config
* Space
*/
- value = csr_readl(pcie, PCI_COMMAND);
+ value = mobiveil_csr_readl(pcie, PCI_COMMAND);
value |= PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER;
- csr_writel(pcie, value, PCI_COMMAND);
+ mobiveil_csr_writel(pcie, value, PCI_COMMAND);

/*
* program PIO Enable Bit to 1 (and PEX PIO Enable to 1) in PAB_CTRL
* register
*/
- pab_ctrl = csr_readl(pcie, PAB_CTRL);
+ pab_ctrl = mobiveil_csr_readl(pcie, PAB_CTRL);
pab_ctrl |= (1 << AMBA_PIO_ENABLE_SHIFT) | (1 << PEX_PIO_ENABLE_SHIFT);
- csr_writel(pcie, pab_ctrl, PAB_CTRL);
+ mobiveil_csr_writel(pcie, pab_ctrl, PAB_CTRL);

- csr_writel(pcie, (PAB_INTP_INTX_MASK | PAB_INTP_MSI_MASK),
- PAB_INTP_AMBA_MISC_ENB);
+ mobiveil_csr_writel(pcie, (PAB_INTP_INTX_MASK | PAB_INTP_MSI_MASK),
+ PAB_INTP_AMBA_MISC_ENB);

/*
* program PIO Enable Bit to 1 and Config Window Enable Bit to 1 in
* PAB_AXI_PIO_CTRL Register
*/
- value = csr_readl(pcie, PAB_AXI_PIO_CTRL);
+ value = mobiveil_csr_readl(pcie, PAB_AXI_PIO_CTRL);
value |= APIO_EN_MASK;
- csr_writel(pcie, value, PAB_AXI_PIO_CTRL);
+ mobiveil_csr_writel(pcie, value, PAB_AXI_PIO_CTRL);

/* Enable PCIe PIO master */
- value = csr_readl(pcie, PAB_PEX_PIO_CTRL);
+ value = mobiveil_csr_readl(pcie, PAB_PEX_PIO_CTRL);
value |= 1 << PIO_ENABLE_SHIFT;
- csr_writel(pcie, value, PAB_PEX_PIO_CTRL);
+ mobiveil_csr_writel(pcie, value, PAB_PEX_PIO_CTRL);

/*
* we'll program one outbound window for config reads and
@@ -647,10 +648,10 @@ static int mobiveil_host_init(struct mobiveil_pcie *pcie)
}

/* fixup for PCIe class register */
- value = csr_readl(pcie, PAB_INTP_AXI_PIO_CLASS);
+ value = mobiveil_csr_readl(pcie, PAB_INTP_AXI_PIO_CLASS);
value &= 0xff;
value |= (PCI_CLASS_BRIDGE_PCI << 16);
- csr_writel(pcie, value, PAB_INTP_AXI_PIO_CLASS);
+ mobiveil_csr_writel(pcie, value, PAB_INTP_AXI_PIO_CLASS);

/* setup MSI hardware registers */
mobiveil_pcie_enable_msi(pcie);
@@ -668,9 +669,9 @@ static void mobiveil_mask_intx_irq(struct irq_data *data)
pcie = irq_desc_get_chip_data(desc);
mask = 1 << ((data->hwirq + PAB_INTX_START) - 1);
raw_spin_lock_irqsave(&pcie->intx_mask_lock, flags);
- shifted_val = csr_readl(pcie, PAB_INTP_AMBA_MISC_ENB);
+ shifted_val = mobiveil_csr_readl(pcie, PAB_INTP_AMBA_MISC_ENB);
shifted_val &= ~mask;
- csr_writel(pcie, shifted_val, PAB_INTP_AMBA_MISC_ENB);
+ mobiveil_csr_writel(pcie, shifted_val, PAB_INTP_AMBA_MISC_ENB);
raw_spin_unlock_irqrestore(&pcie->intx_mask_lock, flags);
}

@@ -684,9 +685,9 @@ static void mobiveil_unmask_intx_irq(struct irq_data *data)
pcie = irq_desc_get_chip_data(desc);
mask = 1 << ((data->hwirq + PAB_INTX_START) - 1);
raw_spin_lock_irqsave(&pcie->intx_mask_lock, flags);
- shifted_val = csr_readl(pcie, PAB_INTP_AMBA_MISC_ENB);
+ shifted_val = mobiveil_csr_readl(pcie, PAB_INTP_AMBA_MISC_ENB);
shifted_val |= mask;
- csr_writel(pcie, shifted_val, PAB_INTP_AMBA_MISC_ENB);
+ mobiveil_csr_writel(pcie, shifted_val, PAB_INTP_AMBA_MISC_ENB);
raw_spin_unlock_irqrestore(&pcie->intx_mask_lock, flags);
}

--
2.20.1

2019-09-30 08:32:34

by Andrew Murray

[permalink] [raw]
Subject: Re: [PATCH v2] PCI: mobiveil: Fix csr_read/write build issue

On Sun, Sep 29, 2019 at 09:35:05AM +0800, Kefeng Wang wrote:
> The riscv has csr_read/write macro, see arch/riscv/include/asm/csr.h,
> the same function naming will cause build error, using such generic names
> in a driver is bad, rename csr_[read,write][l,] to mobiveil_csr_read/write
> to fix it.
>
> drivers/pci/controller/pcie-mobiveil.c:238:69: error: macro "csr_read" passed 3 arguments, but takes just 1
> static u32 csr_read(struct mobiveil_pcie *pcie, u32 off, size_t size)
>
> drivers/pci/controller/pcie-mobiveil.c:253:80: error: macro "csr_write" passed 4 arguments, but takes just 2
> static void csr_write(struct mobiveil_pcie *pcie, u32 val, u32 off, size_t size)
>
> Cc: Hou Zhiqiang <[email protected]>
> Cc: Lorenzo Pieralisi <[email protected]>
> Cc: Minghuan Lian <[email protected]>
> Cc: Subrahmanya Lingappa <[email protected]>
> Cc: Christoph Hellwig <[email protected]>
> Cc: Andrew Murray <[email protected]>
> Fixes: bcbe0d9a8d93 ("PCI: mobiveil: Unify register accessors")
> Signed-off-by: Kefeng Wang <[email protected]>
> ---
>
> v2:
> - using mobiveil prefix suggested by Andrew and Christoph
>
> drivers/pci/controller/pcie-mobiveil.c | 115 +++++++++++++------------
> 1 file changed, 58 insertions(+), 57 deletions(-)
>
> diff --git a/drivers/pci/controller/pcie-mobiveil.c b/drivers/pci/controller/pcie-mobiveil.c
> index a45a6447b01d..5e6144b0fb95 100644
> --- a/drivers/pci/controller/pcie-mobiveil.c
> +++ b/drivers/pci/controller/pcie-mobiveil.c
> @@ -235,7 +235,7 @@ static int mobiveil_pcie_write(void __iomem *addr, int size, u32 val)
> return PCIBIOS_SUCCESSFUL;
> }
>
> -static u32 csr_read(struct mobiveil_pcie *pcie, u32 off, size_t size)
> +static u32 mobiveil_csr_read(struct mobiveil_pcie *pcie, u32 off, size_t size)
> {
> void *addr;
> u32 val;
> @@ -250,7 +250,8 @@ static u32 csr_read(struct mobiveil_pcie *pcie, u32 off, size_t size)
> return val;
> }
>
> -static void csr_write(struct mobiveil_pcie *pcie, u32 val, u32 off, size_t size)
> +static void mobiveil_csr_write(struct mobiveil_pcie *pcie, u32 val, u32 off,
> + size_t size)
> {
> void *addr;
> int ret;
> @@ -262,19 +263,19 @@ static void csr_write(struct mobiveil_pcie *pcie, u32 val, u32 off, size_t size)
> dev_err(&pcie->pdev->dev, "write CSR address failed\n");
> }
>
> -static u32 csr_readl(struct mobiveil_pcie *pcie, u32 off)
> +static u32 mobiveil_csr_readl(struct mobiveil_pcie *pcie, u32 off)
> {
> - return csr_read(pcie, off, 0x4);
> + return mobiveil_csr_read(pcie, off, 0x4);
> }
>
> -static void csr_writel(struct mobiveil_pcie *pcie, u32 val, u32 off)
> +static void mobiveil_csr_writel(struct mobiveil_pcie *pcie, u32 val, u32 off)
> {
> - csr_write(pcie, val, off, 0x4);
> + mobiveil_csr_write(pcie, val, off, 0x4);
> }
>
> static bool mobiveil_pcie_link_up(struct mobiveil_pcie *pcie)
> {
> - return (csr_readl(pcie, LTSSM_STATUS) &
> + return (mobiveil_csr_readl(pcie, LTSSM_STATUS) &
> LTSSM_STATUS_L0_MASK) == LTSSM_STATUS_L0;
> }
>
> @@ -323,7 +324,7 @@ static void __iomem *mobiveil_pcie_map_bus(struct pci_bus *bus,
> PCI_SLOT(devfn) << PAB_DEVICE_SHIFT |
> PCI_FUNC(devfn) << PAB_FUNCTION_SHIFT;
>
> - csr_writel(pcie, value, PAB_AXI_AMAP_PEX_WIN_L(WIN_NUM_0));
> + mobiveil_csr_writel(pcie, value, PAB_AXI_AMAP_PEX_WIN_L(WIN_NUM_0));
>
> return pcie->config_axi_slave_base + where;
> }
> @@ -353,13 +354,13 @@ static void mobiveil_pcie_isr(struct irq_desc *desc)
> chained_irq_enter(chip, desc);
>
> /* read INTx status */
> - val = csr_readl(pcie, PAB_INTP_AMBA_MISC_STAT);
> - mask = csr_readl(pcie, PAB_INTP_AMBA_MISC_ENB);
> + val = mobiveil_csr_readl(pcie, PAB_INTP_AMBA_MISC_STAT);
> + mask = mobiveil_csr_readl(pcie, PAB_INTP_AMBA_MISC_ENB);
> intr_status = val & mask;
>
> /* Handle INTx */
> if (intr_status & PAB_INTP_INTX_MASK) {
> - shifted_status = csr_readl(pcie, PAB_INTP_AMBA_MISC_STAT);
> + shifted_status = mobiveil_csr_readl(pcie, PAB_INTP_AMBA_MISC_STAT);
> shifted_status &= PAB_INTP_INTX_MASK;
> shifted_status >>= PAB_INTX_START;
> do {
> @@ -373,12 +374,12 @@ static void mobiveil_pcie_isr(struct irq_desc *desc)
> bit);
>
> /* clear interrupt handled */
> - csr_writel(pcie, 1 << (PAB_INTX_START + bit),
> - PAB_INTP_AMBA_MISC_STAT);
> + mobiveil_csr_writel(pcie, 1 << (PAB_INTX_START + bit),
> + PAB_INTP_AMBA_MISC_STAT);
> }
>
> - shifted_status = csr_readl(pcie,
> - PAB_INTP_AMBA_MISC_STAT);
> + shifted_status = mobiveil_csr_readl(pcie,
> + PAB_INTP_AMBA_MISC_STAT);
> shifted_status &= PAB_INTP_INTX_MASK;
> shifted_status >>= PAB_INTX_START;
> } while (shifted_status != 0);
> @@ -413,7 +414,7 @@ static void mobiveil_pcie_isr(struct irq_desc *desc)
> }
>
> /* Clear the interrupt status */
> - csr_writel(pcie, intr_status, PAB_INTP_AMBA_MISC_STAT);
> + mobiveil_csr_writel(pcie, intr_status, PAB_INTP_AMBA_MISC_STAT);
> chained_irq_exit(chip, desc);
> }
>
> @@ -474,24 +475,24 @@ static void program_ib_windows(struct mobiveil_pcie *pcie, int win_num,
> return;
> }
>
> - value = csr_readl(pcie, PAB_PEX_AMAP_CTRL(win_num));
> + value = mobiveil_csr_readl(pcie, PAB_PEX_AMAP_CTRL(win_num));
> value &= ~(AMAP_CTRL_TYPE_MASK << AMAP_CTRL_TYPE_SHIFT | WIN_SIZE_MASK);
> value |= type << AMAP_CTRL_TYPE_SHIFT | 1 << AMAP_CTRL_EN_SHIFT |
> (lower_32_bits(size64) & WIN_SIZE_MASK);
> - csr_writel(pcie, value, PAB_PEX_AMAP_CTRL(win_num));
> + mobiveil_csr_writel(pcie, value, PAB_PEX_AMAP_CTRL(win_num));
>
> - csr_writel(pcie, upper_32_bits(size64),
> - PAB_EXT_PEX_AMAP_SIZEN(win_num));
> + mobiveil_csr_writel(pcie, upper_32_bits(size64),
> + PAB_EXT_PEX_AMAP_SIZEN(win_num));
>
> - csr_writel(pcie, lower_32_bits(cpu_addr),
> - PAB_PEX_AMAP_AXI_WIN(win_num));
> - csr_writel(pcie, upper_32_bits(cpu_addr),
> - PAB_EXT_PEX_AMAP_AXI_WIN(win_num));
> + mobiveil_csr_writel(pcie, lower_32_bits(cpu_addr),
> + PAB_PEX_AMAP_AXI_WIN(win_num));
> + mobiveil_csr_writel(pcie, upper_32_bits(cpu_addr),
> + PAB_EXT_PEX_AMAP_AXI_WIN(win_num));
>
> - csr_writel(pcie, lower_32_bits(pci_addr),
> - PAB_PEX_AMAP_PEX_WIN_L(win_num));
> - csr_writel(pcie, upper_32_bits(pci_addr),
> - PAB_PEX_AMAP_PEX_WIN_H(win_num));
> + mobiveil_csr_writel(pcie, lower_32_bits(pci_addr),
> + PAB_PEX_AMAP_PEX_WIN_L(win_num));
> + mobiveil_csr_writel(pcie, upper_32_bits(pci_addr),
> + PAB_PEX_AMAP_PEX_WIN_H(win_num));
>
> pcie->ib_wins_configured++;
> }
> @@ -515,27 +516,27 @@ static void program_ob_windows(struct mobiveil_pcie *pcie, int win_num,
> * program Enable Bit to 1, Type Bit to (00) base 2, AXI Window Size Bit
> * to 4 KB in PAB_AXI_AMAP_CTRL register
> */
> - value = csr_readl(pcie, PAB_AXI_AMAP_CTRL(win_num));
> + value = mobiveil_csr_readl(pcie, PAB_AXI_AMAP_CTRL(win_num));
> value &= ~(WIN_TYPE_MASK << WIN_TYPE_SHIFT | WIN_SIZE_MASK);
> value |= 1 << WIN_ENABLE_SHIFT | type << WIN_TYPE_SHIFT |
> (lower_32_bits(size64) & WIN_SIZE_MASK);
> - csr_writel(pcie, value, PAB_AXI_AMAP_CTRL(win_num));
> + mobiveil_csr_writel(pcie, value, PAB_AXI_AMAP_CTRL(win_num));
>
> - csr_writel(pcie, upper_32_bits(size64), PAB_EXT_AXI_AMAP_SIZE(win_num));
> + mobiveil_csr_writel(pcie, upper_32_bits(size64), PAB_EXT_AXI_AMAP_SIZE(win_num));

Thanks for the respin, this looks much better.

However some of the line lengths, such as this one, are now too long. Can
you move them to multiple lines where needed?

Thanks,

Andrew Murray

>
> /*
> * program AXI window base with appropriate value in
> * PAB_AXI_AMAP_AXI_WIN0 register
> */
> - csr_writel(pcie, lower_32_bits(cpu_addr) & (~AXI_WINDOW_ALIGN_MASK),
> - PAB_AXI_AMAP_AXI_WIN(win_num));
> - csr_writel(pcie, upper_32_bits(cpu_addr),
> - PAB_EXT_AXI_AMAP_AXI_WIN(win_num));
> + mobiveil_csr_writel(pcie, lower_32_bits(cpu_addr) & (~AXI_WINDOW_ALIGN_MASK),
> + PAB_AXI_AMAP_AXI_WIN(win_num));
> + mobiveil_csr_writel(pcie, upper_32_bits(cpu_addr),
> + PAB_EXT_AXI_AMAP_AXI_WIN(win_num));
>
> - csr_writel(pcie, lower_32_bits(pci_addr),
> - PAB_AXI_AMAP_PEX_WIN_L(win_num));
> - csr_writel(pcie, upper_32_bits(pci_addr),
> - PAB_AXI_AMAP_PEX_WIN_H(win_num));
> + mobiveil_csr_writel(pcie, lower_32_bits(pci_addr),
> + PAB_AXI_AMAP_PEX_WIN_L(win_num));
> + mobiveil_csr_writel(pcie, upper_32_bits(pci_addr),
> + PAB_AXI_AMAP_PEX_WIN_H(win_num));
>
> pcie->ob_wins_configured++;
> }
> @@ -579,42 +580,42 @@ static int mobiveil_host_init(struct mobiveil_pcie *pcie)
> struct resource_entry *win;
>
> /* setup bus numbers */
> - value = csr_readl(pcie, PCI_PRIMARY_BUS);
> + value = mobiveil_csr_readl(pcie, PCI_PRIMARY_BUS);
> value &= 0xff000000;
> value |= 0x00ff0100;
> - csr_writel(pcie, value, PCI_PRIMARY_BUS);
> + mobiveil_csr_writel(pcie, value, PCI_PRIMARY_BUS);
>
> /*
> * program Bus Master Enable Bit in Command Register in PAB Config
> * Space
> */
> - value = csr_readl(pcie, PCI_COMMAND);
> + value = mobiveil_csr_readl(pcie, PCI_COMMAND);
> value |= PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER;
> - csr_writel(pcie, value, PCI_COMMAND);
> + mobiveil_csr_writel(pcie, value, PCI_COMMAND);
>
> /*
> * program PIO Enable Bit to 1 (and PEX PIO Enable to 1) in PAB_CTRL
> * register
> */
> - pab_ctrl = csr_readl(pcie, PAB_CTRL);
> + pab_ctrl = mobiveil_csr_readl(pcie, PAB_CTRL);
> pab_ctrl |= (1 << AMBA_PIO_ENABLE_SHIFT) | (1 << PEX_PIO_ENABLE_SHIFT);
> - csr_writel(pcie, pab_ctrl, PAB_CTRL);
> + mobiveil_csr_writel(pcie, pab_ctrl, PAB_CTRL);
>
> - csr_writel(pcie, (PAB_INTP_INTX_MASK | PAB_INTP_MSI_MASK),
> - PAB_INTP_AMBA_MISC_ENB);
> + mobiveil_csr_writel(pcie, (PAB_INTP_INTX_MASK | PAB_INTP_MSI_MASK),
> + PAB_INTP_AMBA_MISC_ENB);
>
> /*
> * program PIO Enable Bit to 1 and Config Window Enable Bit to 1 in
> * PAB_AXI_PIO_CTRL Register
> */
> - value = csr_readl(pcie, PAB_AXI_PIO_CTRL);
> + value = mobiveil_csr_readl(pcie, PAB_AXI_PIO_CTRL);
> value |= APIO_EN_MASK;
> - csr_writel(pcie, value, PAB_AXI_PIO_CTRL);
> + mobiveil_csr_writel(pcie, value, PAB_AXI_PIO_CTRL);
>
> /* Enable PCIe PIO master */
> - value = csr_readl(pcie, PAB_PEX_PIO_CTRL);
> + value = mobiveil_csr_readl(pcie, PAB_PEX_PIO_CTRL);
> value |= 1 << PIO_ENABLE_SHIFT;
> - csr_writel(pcie, value, PAB_PEX_PIO_CTRL);
> + mobiveil_csr_writel(pcie, value, PAB_PEX_PIO_CTRL);
>
> /*
> * we'll program one outbound window for config reads and
> @@ -647,10 +648,10 @@ static int mobiveil_host_init(struct mobiveil_pcie *pcie)
> }
>
> /* fixup for PCIe class register */
> - value = csr_readl(pcie, PAB_INTP_AXI_PIO_CLASS);
> + value = mobiveil_csr_readl(pcie, PAB_INTP_AXI_PIO_CLASS);
> value &= 0xff;
> value |= (PCI_CLASS_BRIDGE_PCI << 16);
> - csr_writel(pcie, value, PAB_INTP_AXI_PIO_CLASS);
> + mobiveil_csr_writel(pcie, value, PAB_INTP_AXI_PIO_CLASS);
>
> /* setup MSI hardware registers */
> mobiveil_pcie_enable_msi(pcie);
> @@ -668,9 +669,9 @@ static void mobiveil_mask_intx_irq(struct irq_data *data)
> pcie = irq_desc_get_chip_data(desc);
> mask = 1 << ((data->hwirq + PAB_INTX_START) - 1);
> raw_spin_lock_irqsave(&pcie->intx_mask_lock, flags);
> - shifted_val = csr_readl(pcie, PAB_INTP_AMBA_MISC_ENB);
> + shifted_val = mobiveil_csr_readl(pcie, PAB_INTP_AMBA_MISC_ENB);
> shifted_val &= ~mask;
> - csr_writel(pcie, shifted_val, PAB_INTP_AMBA_MISC_ENB);
> + mobiveil_csr_writel(pcie, shifted_val, PAB_INTP_AMBA_MISC_ENB);
> raw_spin_unlock_irqrestore(&pcie->intx_mask_lock, flags);
> }
>
> @@ -684,9 +685,9 @@ static void mobiveil_unmask_intx_irq(struct irq_data *data)
> pcie = irq_desc_get_chip_data(desc);
> mask = 1 << ((data->hwirq + PAB_INTX_START) - 1);
> raw_spin_lock_irqsave(&pcie->intx_mask_lock, flags);
> - shifted_val = csr_readl(pcie, PAB_INTP_AMBA_MISC_ENB);
> + shifted_val = mobiveil_csr_readl(pcie, PAB_INTP_AMBA_MISC_ENB);
> shifted_val |= mask;
> - csr_writel(pcie, shifted_val, PAB_INTP_AMBA_MISC_ENB);
> + mobiveil_csr_writel(pcie, shifted_val, PAB_INTP_AMBA_MISC_ENB);
> raw_spin_unlock_irqrestore(&pcie->intx_mask_lock, flags);
> }
>
> --
> 2.20.1
>

2019-10-04 08:08:43

by Kefeng Wang

[permalink] [raw]
Subject: [PATCH v3] PCI: mobiveil: Fix csr_read/write build issue

The riscv has csr_read/write macro, see arch/riscv/include/asm/csr.h,
the same function naming will cause build error, using such generic names
in a driver is bad, rename csr_[read,write][l,] to mobiveil_csr_read/write
to fix it.

drivers/pci/controller/pcie-mobiveil.c:238:69: error: macro "csr_read" passed 3 arguments, but takes just 1
static u32 csr_read(struct mobiveil_pcie *pcie, u32 off, size_t size)

drivers/pci/controller/pcie-mobiveil.c:253:80: error: macro "csr_write" passed 4 arguments, but takes just 2
static void csr_write(struct mobiveil_pcie *pcie, u32 val, u32 off, size_t size)

Cc: Hou Zhiqiang <[email protected]>
Cc: Lorenzo Pieralisi <[email protected]>
Cc: Minghuan Lian <[email protected]>
Cc: Subrahmanya Lingappa <[email protected]>
Cc: Andrew Murray <[email protected]>
Fixes: bcbe0d9a8d93 ("PCI: mobiveil: Unify register accessors")
Signed-off-by: Kefeng Wang <[email protected]>
---
v3:
- reduce line lengths suggested by Andrew

v2:
- using mobiveil prefix suggested by Andrew and Christoph

drivers/pci/controller/pcie-mobiveil.c | 119 +++++++++++++------------
1 file changed, 62 insertions(+), 57 deletions(-)

diff --git a/drivers/pci/controller/pcie-mobiveil.c b/drivers/pci/controller/pcie-mobiveil.c
index a45a6447b01d..32f37d08d5bc 100644
--- a/drivers/pci/controller/pcie-mobiveil.c
+++ b/drivers/pci/controller/pcie-mobiveil.c
@@ -235,7 +235,7 @@ static int mobiveil_pcie_write(void __iomem *addr, int size, u32 val)
return PCIBIOS_SUCCESSFUL;
}

-static u32 csr_read(struct mobiveil_pcie *pcie, u32 off, size_t size)
+static u32 mobiveil_csr_read(struct mobiveil_pcie *pcie, u32 off, size_t size)
{
void *addr;
u32 val;
@@ -250,7 +250,8 @@ static u32 csr_read(struct mobiveil_pcie *pcie, u32 off, size_t size)
return val;
}

-static void csr_write(struct mobiveil_pcie *pcie, u32 val, u32 off, size_t size)
+static void mobiveil_csr_write(struct mobiveil_pcie *pcie, u32 val, u32 off,
+ size_t size)
{
void *addr;
int ret;
@@ -262,19 +263,19 @@ static void csr_write(struct mobiveil_pcie *pcie, u32 val, u32 off, size_t size)
dev_err(&pcie->pdev->dev, "write CSR address failed\n");
}

-static u32 csr_readl(struct mobiveil_pcie *pcie, u32 off)
+static u32 mobiveil_csr_readl(struct mobiveil_pcie *pcie, u32 off)
{
- return csr_read(pcie, off, 0x4);
+ return mobiveil_csr_read(pcie, off, 0x4);
}

-static void csr_writel(struct mobiveil_pcie *pcie, u32 val, u32 off)
+static void mobiveil_csr_writel(struct mobiveil_pcie *pcie, u32 val, u32 off)
{
- csr_write(pcie, val, off, 0x4);
+ mobiveil_csr_write(pcie, val, off, 0x4);
}

static bool mobiveil_pcie_link_up(struct mobiveil_pcie *pcie)
{
- return (csr_readl(pcie, LTSSM_STATUS) &
+ return (mobiveil_csr_readl(pcie, LTSSM_STATUS) &
LTSSM_STATUS_L0_MASK) == LTSSM_STATUS_L0;
}

@@ -323,7 +324,7 @@ static void __iomem *mobiveil_pcie_map_bus(struct pci_bus *bus,
PCI_SLOT(devfn) << PAB_DEVICE_SHIFT |
PCI_FUNC(devfn) << PAB_FUNCTION_SHIFT;

- csr_writel(pcie, value, PAB_AXI_AMAP_PEX_WIN_L(WIN_NUM_0));
+ mobiveil_csr_writel(pcie, value, PAB_AXI_AMAP_PEX_WIN_L(WIN_NUM_0));

return pcie->config_axi_slave_base + where;
}
@@ -353,13 +354,14 @@ static void mobiveil_pcie_isr(struct irq_desc *desc)
chained_irq_enter(chip, desc);

/* read INTx status */
- val = csr_readl(pcie, PAB_INTP_AMBA_MISC_STAT);
- mask = csr_readl(pcie, PAB_INTP_AMBA_MISC_ENB);
+ val = mobiveil_csr_readl(pcie, PAB_INTP_AMBA_MISC_STAT);
+ mask = mobiveil_csr_readl(pcie, PAB_INTP_AMBA_MISC_ENB);
intr_status = val & mask;

/* Handle INTx */
if (intr_status & PAB_INTP_INTX_MASK) {
- shifted_status = csr_readl(pcie, PAB_INTP_AMBA_MISC_STAT);
+ shifted_status = mobiveil_csr_readl(pcie,
+ PAB_INTP_AMBA_MISC_STAT);
shifted_status &= PAB_INTP_INTX_MASK;
shifted_status >>= PAB_INTX_START;
do {
@@ -373,12 +375,13 @@ static void mobiveil_pcie_isr(struct irq_desc *desc)
bit);

/* clear interrupt handled */
- csr_writel(pcie, 1 << (PAB_INTX_START + bit),
- PAB_INTP_AMBA_MISC_STAT);
+ mobiveil_csr_writel(pcie,
+ 1 << (PAB_INTX_START + bit),
+ PAB_INTP_AMBA_MISC_STAT);
}

- shifted_status = csr_readl(pcie,
- PAB_INTP_AMBA_MISC_STAT);
+ shifted_status = mobiveil_csr_readl(pcie,
+ PAB_INTP_AMBA_MISC_STAT);
shifted_status &= PAB_INTP_INTX_MASK;
shifted_status >>= PAB_INTX_START;
} while (shifted_status != 0);
@@ -413,7 +416,7 @@ static void mobiveil_pcie_isr(struct irq_desc *desc)
}

/* Clear the interrupt status */
- csr_writel(pcie, intr_status, PAB_INTP_AMBA_MISC_STAT);
+ mobiveil_csr_writel(pcie, intr_status, PAB_INTP_AMBA_MISC_STAT);
chained_irq_exit(chip, desc);
}

@@ -474,24 +477,24 @@ static void program_ib_windows(struct mobiveil_pcie *pcie, int win_num,
return;
}

- value = csr_readl(pcie, PAB_PEX_AMAP_CTRL(win_num));
+ value = mobiveil_csr_readl(pcie, PAB_PEX_AMAP_CTRL(win_num));
value &= ~(AMAP_CTRL_TYPE_MASK << AMAP_CTRL_TYPE_SHIFT | WIN_SIZE_MASK);
value |= type << AMAP_CTRL_TYPE_SHIFT | 1 << AMAP_CTRL_EN_SHIFT |
(lower_32_bits(size64) & WIN_SIZE_MASK);
- csr_writel(pcie, value, PAB_PEX_AMAP_CTRL(win_num));
+ mobiveil_csr_writel(pcie, value, PAB_PEX_AMAP_CTRL(win_num));

- csr_writel(pcie, upper_32_bits(size64),
- PAB_EXT_PEX_AMAP_SIZEN(win_num));
+ mobiveil_csr_writel(pcie, upper_32_bits(size64),
+ PAB_EXT_PEX_AMAP_SIZEN(win_num));

- csr_writel(pcie, lower_32_bits(cpu_addr),
- PAB_PEX_AMAP_AXI_WIN(win_num));
- csr_writel(pcie, upper_32_bits(cpu_addr),
- PAB_EXT_PEX_AMAP_AXI_WIN(win_num));
+ mobiveil_csr_writel(pcie, lower_32_bits(cpu_addr),
+ PAB_PEX_AMAP_AXI_WIN(win_num));
+ mobiveil_csr_writel(pcie, upper_32_bits(cpu_addr),
+ PAB_EXT_PEX_AMAP_AXI_WIN(win_num));

- csr_writel(pcie, lower_32_bits(pci_addr),
- PAB_PEX_AMAP_PEX_WIN_L(win_num));
- csr_writel(pcie, upper_32_bits(pci_addr),
- PAB_PEX_AMAP_PEX_WIN_H(win_num));
+ mobiveil_csr_writel(pcie, lower_32_bits(pci_addr),
+ PAB_PEX_AMAP_PEX_WIN_L(win_num));
+ mobiveil_csr_writel(pcie, upper_32_bits(pci_addr),
+ PAB_PEX_AMAP_PEX_WIN_H(win_num));

pcie->ib_wins_configured++;
}
@@ -515,27 +518,29 @@ static void program_ob_windows(struct mobiveil_pcie *pcie, int win_num,
* program Enable Bit to 1, Type Bit to (00) base 2, AXI Window Size Bit
* to 4 KB in PAB_AXI_AMAP_CTRL register
*/
- value = csr_readl(pcie, PAB_AXI_AMAP_CTRL(win_num));
+ value = mobiveil_csr_readl(pcie, PAB_AXI_AMAP_CTRL(win_num));
value &= ~(WIN_TYPE_MASK << WIN_TYPE_SHIFT | WIN_SIZE_MASK);
value |= 1 << WIN_ENABLE_SHIFT | type << WIN_TYPE_SHIFT |
(lower_32_bits(size64) & WIN_SIZE_MASK);
- csr_writel(pcie, value, PAB_AXI_AMAP_CTRL(win_num));
+ mobiveil_csr_writel(pcie, value, PAB_AXI_AMAP_CTRL(win_num));

- csr_writel(pcie, upper_32_bits(size64), PAB_EXT_AXI_AMAP_SIZE(win_num));
+ mobiveil_csr_writel(pcie, upper_32_bits(size64),
+ PAB_EXT_AXI_AMAP_SIZE(win_num));

/*
* program AXI window base with appropriate value in
* PAB_AXI_AMAP_AXI_WIN0 register
*/
- csr_writel(pcie, lower_32_bits(cpu_addr) & (~AXI_WINDOW_ALIGN_MASK),
- PAB_AXI_AMAP_AXI_WIN(win_num));
- csr_writel(pcie, upper_32_bits(cpu_addr),
- PAB_EXT_AXI_AMAP_AXI_WIN(win_num));
+ mobiveil_csr_writel(pcie,
+ lower_32_bits(cpu_addr) & (~AXI_WINDOW_ALIGN_MASK),
+ PAB_AXI_AMAP_AXI_WIN(win_num));
+ mobiveil_csr_writel(pcie, upper_32_bits(cpu_addr),
+ PAB_EXT_AXI_AMAP_AXI_WIN(win_num));

- csr_writel(pcie, lower_32_bits(pci_addr),
- PAB_AXI_AMAP_PEX_WIN_L(win_num));
- csr_writel(pcie, upper_32_bits(pci_addr),
- PAB_AXI_AMAP_PEX_WIN_H(win_num));
+ mobiveil_csr_writel(pcie, lower_32_bits(pci_addr),
+ PAB_AXI_AMAP_PEX_WIN_L(win_num));
+ mobiveil_csr_writel(pcie, upper_32_bits(pci_addr),
+ PAB_AXI_AMAP_PEX_WIN_H(win_num));

pcie->ob_wins_configured++;
}
@@ -579,42 +584,42 @@ static int mobiveil_host_init(struct mobiveil_pcie *pcie)
struct resource_entry *win;

/* setup bus numbers */
- value = csr_readl(pcie, PCI_PRIMARY_BUS);
+ value = mobiveil_csr_readl(pcie, PCI_PRIMARY_BUS);
value &= 0xff000000;
value |= 0x00ff0100;
- csr_writel(pcie, value, PCI_PRIMARY_BUS);
+ mobiveil_csr_writel(pcie, value, PCI_PRIMARY_BUS);

/*
* program Bus Master Enable Bit in Command Register in PAB Config
* Space
*/
- value = csr_readl(pcie, PCI_COMMAND);
+ value = mobiveil_csr_readl(pcie, PCI_COMMAND);
value |= PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER;
- csr_writel(pcie, value, PCI_COMMAND);
+ mobiveil_csr_writel(pcie, value, PCI_COMMAND);

/*
* program PIO Enable Bit to 1 (and PEX PIO Enable to 1) in PAB_CTRL
* register
*/
- pab_ctrl = csr_readl(pcie, PAB_CTRL);
+ pab_ctrl = mobiveil_csr_readl(pcie, PAB_CTRL);
pab_ctrl |= (1 << AMBA_PIO_ENABLE_SHIFT) | (1 << PEX_PIO_ENABLE_SHIFT);
- csr_writel(pcie, pab_ctrl, PAB_CTRL);
+ mobiveil_csr_writel(pcie, pab_ctrl, PAB_CTRL);

- csr_writel(pcie, (PAB_INTP_INTX_MASK | PAB_INTP_MSI_MASK),
- PAB_INTP_AMBA_MISC_ENB);
+ mobiveil_csr_writel(pcie, (PAB_INTP_INTX_MASK | PAB_INTP_MSI_MASK),
+ PAB_INTP_AMBA_MISC_ENB);

/*
* program PIO Enable Bit to 1 and Config Window Enable Bit to 1 in
* PAB_AXI_PIO_CTRL Register
*/
- value = csr_readl(pcie, PAB_AXI_PIO_CTRL);
+ value = mobiveil_csr_readl(pcie, PAB_AXI_PIO_CTRL);
value |= APIO_EN_MASK;
- csr_writel(pcie, value, PAB_AXI_PIO_CTRL);
+ mobiveil_csr_writel(pcie, value, PAB_AXI_PIO_CTRL);

/* Enable PCIe PIO master */
- value = csr_readl(pcie, PAB_PEX_PIO_CTRL);
+ value = mobiveil_csr_readl(pcie, PAB_PEX_PIO_CTRL);
value |= 1 << PIO_ENABLE_SHIFT;
- csr_writel(pcie, value, PAB_PEX_PIO_CTRL);
+ mobiveil_csr_writel(pcie, value, PAB_PEX_PIO_CTRL);

/*
* we'll program one outbound window for config reads and
@@ -647,10 +652,10 @@ static int mobiveil_host_init(struct mobiveil_pcie *pcie)
}

/* fixup for PCIe class register */
- value = csr_readl(pcie, PAB_INTP_AXI_PIO_CLASS);
+ value = mobiveil_csr_readl(pcie, PAB_INTP_AXI_PIO_CLASS);
value &= 0xff;
value |= (PCI_CLASS_BRIDGE_PCI << 16);
- csr_writel(pcie, value, PAB_INTP_AXI_PIO_CLASS);
+ mobiveil_csr_writel(pcie, value, PAB_INTP_AXI_PIO_CLASS);

/* setup MSI hardware registers */
mobiveil_pcie_enable_msi(pcie);
@@ -668,9 +673,9 @@ static void mobiveil_mask_intx_irq(struct irq_data *data)
pcie = irq_desc_get_chip_data(desc);
mask = 1 << ((data->hwirq + PAB_INTX_START) - 1);
raw_spin_lock_irqsave(&pcie->intx_mask_lock, flags);
- shifted_val = csr_readl(pcie, PAB_INTP_AMBA_MISC_ENB);
+ shifted_val = mobiveil_csr_readl(pcie, PAB_INTP_AMBA_MISC_ENB);
shifted_val &= ~mask;
- csr_writel(pcie, shifted_val, PAB_INTP_AMBA_MISC_ENB);
+ mobiveil_csr_writel(pcie, shifted_val, PAB_INTP_AMBA_MISC_ENB);
raw_spin_unlock_irqrestore(&pcie->intx_mask_lock, flags);
}

@@ -684,9 +689,9 @@ static void mobiveil_unmask_intx_irq(struct irq_data *data)
pcie = irq_desc_get_chip_data(desc);
mask = 1 << ((data->hwirq + PAB_INTX_START) - 1);
raw_spin_lock_irqsave(&pcie->intx_mask_lock, flags);
- shifted_val = csr_readl(pcie, PAB_INTP_AMBA_MISC_ENB);
+ shifted_val = mobiveil_csr_readl(pcie, PAB_INTP_AMBA_MISC_ENB);
shifted_val |= mask;
- csr_writel(pcie, shifted_val, PAB_INTP_AMBA_MISC_ENB);
+ mobiveil_csr_writel(pcie, shifted_val, PAB_INTP_AMBA_MISC_ENB);
raw_spin_unlock_irqrestore(&pcie->intx_mask_lock, flags);
}

--
2.20.1

2019-10-04 08:28:29

by Andrew Murray

[permalink] [raw]
Subject: Re: [PATCH v3] PCI: mobiveil: Fix csr_read/write build issue

On Fri, Oct 04, 2019 at 12:19:25PM +0800, Kefeng Wang wrote:
> The riscv has csr_read/write macro, see arch/riscv/include/asm/csr.h,
> the same function naming will cause build error, using such generic names
> in a driver is bad, rename csr_[read,write][l,] to mobiveil_csr_read/write
> to fix it.
>
> drivers/pci/controller/pcie-mobiveil.c:238:69: error: macro "csr_read" passed 3 arguments, but takes just 1
> static u32 csr_read(struct mobiveil_pcie *pcie, u32 off, size_t size)
>
> drivers/pci/controller/pcie-mobiveil.c:253:80: error: macro "csr_write" passed 4 arguments, but takes just 2
> static void csr_write(struct mobiveil_pcie *pcie, u32 val, u32 off, size_t size)
>
> Cc: Hou Zhiqiang <[email protected]>
> Cc: Lorenzo Pieralisi <[email protected]>
> Cc: Minghuan Lian <[email protected]>
> Cc: Subrahmanya Lingappa <[email protected]>
> Cc: Andrew Murray <[email protected]>
> Fixes: bcbe0d9a8d93 ("PCI: mobiveil: Unify register accessors")
> Signed-off-by: Kefeng Wang <[email protected]>
> ---

Thanks for the respin:

Reviewed-by: Andrew Murray <[email protected]>

> v3:
> - reduce line lengths suggested by Andrew
>
> v2:
> - using mobiveil prefix suggested by Andrew and Christoph
>
> drivers/pci/controller/pcie-mobiveil.c | 119 +++++++++++++------------
> 1 file changed, 62 insertions(+), 57 deletions(-)
>
> diff --git a/drivers/pci/controller/pcie-mobiveil.c b/drivers/pci/controller/pcie-mobiveil.c
> index a45a6447b01d..32f37d08d5bc 100644
> --- a/drivers/pci/controller/pcie-mobiveil.c
> +++ b/drivers/pci/controller/pcie-mobiveil.c
> @@ -235,7 +235,7 @@ static int mobiveil_pcie_write(void __iomem *addr, int size, u32 val)
> return PCIBIOS_SUCCESSFUL;
> }
>
> -static u32 csr_read(struct mobiveil_pcie *pcie, u32 off, size_t size)
> +static u32 mobiveil_csr_read(struct mobiveil_pcie *pcie, u32 off, size_t size)
> {
> void *addr;
> u32 val;
> @@ -250,7 +250,8 @@ static u32 csr_read(struct mobiveil_pcie *pcie, u32 off, size_t size)
> return val;
> }
>
> -static void csr_write(struct mobiveil_pcie *pcie, u32 val, u32 off, size_t size)
> +static void mobiveil_csr_write(struct mobiveil_pcie *pcie, u32 val, u32 off,
> + size_t size)
> {
> void *addr;
> int ret;
> @@ -262,19 +263,19 @@ static void csr_write(struct mobiveil_pcie *pcie, u32 val, u32 off, size_t size)
> dev_err(&pcie->pdev->dev, "write CSR address failed\n");
> }
>
> -static u32 csr_readl(struct mobiveil_pcie *pcie, u32 off)
> +static u32 mobiveil_csr_readl(struct mobiveil_pcie *pcie, u32 off)
> {
> - return csr_read(pcie, off, 0x4);
> + return mobiveil_csr_read(pcie, off, 0x4);
> }
>
> -static void csr_writel(struct mobiveil_pcie *pcie, u32 val, u32 off)
> +static void mobiveil_csr_writel(struct mobiveil_pcie *pcie, u32 val, u32 off)
> {
> - csr_write(pcie, val, off, 0x4);
> + mobiveil_csr_write(pcie, val, off, 0x4);
> }
>
> static bool mobiveil_pcie_link_up(struct mobiveil_pcie *pcie)
> {
> - return (csr_readl(pcie, LTSSM_STATUS) &
> + return (mobiveil_csr_readl(pcie, LTSSM_STATUS) &
> LTSSM_STATUS_L0_MASK) == LTSSM_STATUS_L0;
> }
>
> @@ -323,7 +324,7 @@ static void __iomem *mobiveil_pcie_map_bus(struct pci_bus *bus,
> PCI_SLOT(devfn) << PAB_DEVICE_SHIFT |
> PCI_FUNC(devfn) << PAB_FUNCTION_SHIFT;
>
> - csr_writel(pcie, value, PAB_AXI_AMAP_PEX_WIN_L(WIN_NUM_0));
> + mobiveil_csr_writel(pcie, value, PAB_AXI_AMAP_PEX_WIN_L(WIN_NUM_0));
>
> return pcie->config_axi_slave_base + where;
> }
> @@ -353,13 +354,14 @@ static void mobiveil_pcie_isr(struct irq_desc *desc)
> chained_irq_enter(chip, desc);
>
> /* read INTx status */
> - val = csr_readl(pcie, PAB_INTP_AMBA_MISC_STAT);
> - mask = csr_readl(pcie, PAB_INTP_AMBA_MISC_ENB);
> + val = mobiveil_csr_readl(pcie, PAB_INTP_AMBA_MISC_STAT);
> + mask = mobiveil_csr_readl(pcie, PAB_INTP_AMBA_MISC_ENB);
> intr_status = val & mask;
>
> /* Handle INTx */
> if (intr_status & PAB_INTP_INTX_MASK) {
> - shifted_status = csr_readl(pcie, PAB_INTP_AMBA_MISC_STAT);
> + shifted_status = mobiveil_csr_readl(pcie,
> + PAB_INTP_AMBA_MISC_STAT);
> shifted_status &= PAB_INTP_INTX_MASK;
> shifted_status >>= PAB_INTX_START;
> do {
> @@ -373,12 +375,13 @@ static void mobiveil_pcie_isr(struct irq_desc *desc)
> bit);
>
> /* clear interrupt handled */
> - csr_writel(pcie, 1 << (PAB_INTX_START + bit),
> - PAB_INTP_AMBA_MISC_STAT);
> + mobiveil_csr_writel(pcie,
> + 1 << (PAB_INTX_START + bit),
> + PAB_INTP_AMBA_MISC_STAT);
> }
>
> - shifted_status = csr_readl(pcie,
> - PAB_INTP_AMBA_MISC_STAT);
> + shifted_status = mobiveil_csr_readl(pcie,
> + PAB_INTP_AMBA_MISC_STAT);
> shifted_status &= PAB_INTP_INTX_MASK;
> shifted_status >>= PAB_INTX_START;
> } while (shifted_status != 0);
> @@ -413,7 +416,7 @@ static void mobiveil_pcie_isr(struct irq_desc *desc)
> }
>
> /* Clear the interrupt status */
> - csr_writel(pcie, intr_status, PAB_INTP_AMBA_MISC_STAT);
> + mobiveil_csr_writel(pcie, intr_status, PAB_INTP_AMBA_MISC_STAT);
> chained_irq_exit(chip, desc);
> }
>
> @@ -474,24 +477,24 @@ static void program_ib_windows(struct mobiveil_pcie *pcie, int win_num,
> return;
> }
>
> - value = csr_readl(pcie, PAB_PEX_AMAP_CTRL(win_num));
> + value = mobiveil_csr_readl(pcie, PAB_PEX_AMAP_CTRL(win_num));
> value &= ~(AMAP_CTRL_TYPE_MASK << AMAP_CTRL_TYPE_SHIFT | WIN_SIZE_MASK);
> value |= type << AMAP_CTRL_TYPE_SHIFT | 1 << AMAP_CTRL_EN_SHIFT |
> (lower_32_bits(size64) & WIN_SIZE_MASK);
> - csr_writel(pcie, value, PAB_PEX_AMAP_CTRL(win_num));
> + mobiveil_csr_writel(pcie, value, PAB_PEX_AMAP_CTRL(win_num));
>
> - csr_writel(pcie, upper_32_bits(size64),
> - PAB_EXT_PEX_AMAP_SIZEN(win_num));
> + mobiveil_csr_writel(pcie, upper_32_bits(size64),
> + PAB_EXT_PEX_AMAP_SIZEN(win_num));
>
> - csr_writel(pcie, lower_32_bits(cpu_addr),
> - PAB_PEX_AMAP_AXI_WIN(win_num));
> - csr_writel(pcie, upper_32_bits(cpu_addr),
> - PAB_EXT_PEX_AMAP_AXI_WIN(win_num));
> + mobiveil_csr_writel(pcie, lower_32_bits(cpu_addr),
> + PAB_PEX_AMAP_AXI_WIN(win_num));
> + mobiveil_csr_writel(pcie, upper_32_bits(cpu_addr),
> + PAB_EXT_PEX_AMAP_AXI_WIN(win_num));
>
> - csr_writel(pcie, lower_32_bits(pci_addr),
> - PAB_PEX_AMAP_PEX_WIN_L(win_num));
> - csr_writel(pcie, upper_32_bits(pci_addr),
> - PAB_PEX_AMAP_PEX_WIN_H(win_num));
> + mobiveil_csr_writel(pcie, lower_32_bits(pci_addr),
> + PAB_PEX_AMAP_PEX_WIN_L(win_num));
> + mobiveil_csr_writel(pcie, upper_32_bits(pci_addr),
> + PAB_PEX_AMAP_PEX_WIN_H(win_num));
>
> pcie->ib_wins_configured++;
> }
> @@ -515,27 +518,29 @@ static void program_ob_windows(struct mobiveil_pcie *pcie, int win_num,
> * program Enable Bit to 1, Type Bit to (00) base 2, AXI Window Size Bit
> * to 4 KB in PAB_AXI_AMAP_CTRL register
> */
> - value = csr_readl(pcie, PAB_AXI_AMAP_CTRL(win_num));
> + value = mobiveil_csr_readl(pcie, PAB_AXI_AMAP_CTRL(win_num));
> value &= ~(WIN_TYPE_MASK << WIN_TYPE_SHIFT | WIN_SIZE_MASK);
> value |= 1 << WIN_ENABLE_SHIFT | type << WIN_TYPE_SHIFT |
> (lower_32_bits(size64) & WIN_SIZE_MASK);
> - csr_writel(pcie, value, PAB_AXI_AMAP_CTRL(win_num));
> + mobiveil_csr_writel(pcie, value, PAB_AXI_AMAP_CTRL(win_num));
>
> - csr_writel(pcie, upper_32_bits(size64), PAB_EXT_AXI_AMAP_SIZE(win_num));
> + mobiveil_csr_writel(pcie, upper_32_bits(size64),
> + PAB_EXT_AXI_AMAP_SIZE(win_num));
>
> /*
> * program AXI window base with appropriate value in
> * PAB_AXI_AMAP_AXI_WIN0 register
> */
> - csr_writel(pcie, lower_32_bits(cpu_addr) & (~AXI_WINDOW_ALIGN_MASK),
> - PAB_AXI_AMAP_AXI_WIN(win_num));
> - csr_writel(pcie, upper_32_bits(cpu_addr),
> - PAB_EXT_AXI_AMAP_AXI_WIN(win_num));
> + mobiveil_csr_writel(pcie,
> + lower_32_bits(cpu_addr) & (~AXI_WINDOW_ALIGN_MASK),
> + PAB_AXI_AMAP_AXI_WIN(win_num));
> + mobiveil_csr_writel(pcie, upper_32_bits(cpu_addr),
> + PAB_EXT_AXI_AMAP_AXI_WIN(win_num));
>
> - csr_writel(pcie, lower_32_bits(pci_addr),
> - PAB_AXI_AMAP_PEX_WIN_L(win_num));
> - csr_writel(pcie, upper_32_bits(pci_addr),
> - PAB_AXI_AMAP_PEX_WIN_H(win_num));
> + mobiveil_csr_writel(pcie, lower_32_bits(pci_addr),
> + PAB_AXI_AMAP_PEX_WIN_L(win_num));
> + mobiveil_csr_writel(pcie, upper_32_bits(pci_addr),
> + PAB_AXI_AMAP_PEX_WIN_H(win_num));
>
> pcie->ob_wins_configured++;
> }
> @@ -579,42 +584,42 @@ static int mobiveil_host_init(struct mobiveil_pcie *pcie)
> struct resource_entry *win;
>
> /* setup bus numbers */
> - value = csr_readl(pcie, PCI_PRIMARY_BUS);
> + value = mobiveil_csr_readl(pcie, PCI_PRIMARY_BUS);
> value &= 0xff000000;
> value |= 0x00ff0100;
> - csr_writel(pcie, value, PCI_PRIMARY_BUS);
> + mobiveil_csr_writel(pcie, value, PCI_PRIMARY_BUS);
>
> /*
> * program Bus Master Enable Bit in Command Register in PAB Config
> * Space
> */
> - value = csr_readl(pcie, PCI_COMMAND);
> + value = mobiveil_csr_readl(pcie, PCI_COMMAND);
> value |= PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER;
> - csr_writel(pcie, value, PCI_COMMAND);
> + mobiveil_csr_writel(pcie, value, PCI_COMMAND);
>
> /*
> * program PIO Enable Bit to 1 (and PEX PIO Enable to 1) in PAB_CTRL
> * register
> */
> - pab_ctrl = csr_readl(pcie, PAB_CTRL);
> + pab_ctrl = mobiveil_csr_readl(pcie, PAB_CTRL);
> pab_ctrl |= (1 << AMBA_PIO_ENABLE_SHIFT) | (1 << PEX_PIO_ENABLE_SHIFT);
> - csr_writel(pcie, pab_ctrl, PAB_CTRL);
> + mobiveil_csr_writel(pcie, pab_ctrl, PAB_CTRL);
>
> - csr_writel(pcie, (PAB_INTP_INTX_MASK | PAB_INTP_MSI_MASK),
> - PAB_INTP_AMBA_MISC_ENB);
> + mobiveil_csr_writel(pcie, (PAB_INTP_INTX_MASK | PAB_INTP_MSI_MASK),
> + PAB_INTP_AMBA_MISC_ENB);
>
> /*
> * program PIO Enable Bit to 1 and Config Window Enable Bit to 1 in
> * PAB_AXI_PIO_CTRL Register
> */
> - value = csr_readl(pcie, PAB_AXI_PIO_CTRL);
> + value = mobiveil_csr_readl(pcie, PAB_AXI_PIO_CTRL);
> value |= APIO_EN_MASK;
> - csr_writel(pcie, value, PAB_AXI_PIO_CTRL);
> + mobiveil_csr_writel(pcie, value, PAB_AXI_PIO_CTRL);
>
> /* Enable PCIe PIO master */
> - value = csr_readl(pcie, PAB_PEX_PIO_CTRL);
> + value = mobiveil_csr_readl(pcie, PAB_PEX_PIO_CTRL);
> value |= 1 << PIO_ENABLE_SHIFT;
> - csr_writel(pcie, value, PAB_PEX_PIO_CTRL);
> + mobiveil_csr_writel(pcie, value, PAB_PEX_PIO_CTRL);
>
> /*
> * we'll program one outbound window for config reads and
> @@ -647,10 +652,10 @@ static int mobiveil_host_init(struct mobiveil_pcie *pcie)
> }
>
> /* fixup for PCIe class register */
> - value = csr_readl(pcie, PAB_INTP_AXI_PIO_CLASS);
> + value = mobiveil_csr_readl(pcie, PAB_INTP_AXI_PIO_CLASS);
> value &= 0xff;
> value |= (PCI_CLASS_BRIDGE_PCI << 16);
> - csr_writel(pcie, value, PAB_INTP_AXI_PIO_CLASS);
> + mobiveil_csr_writel(pcie, value, PAB_INTP_AXI_PIO_CLASS);
>
> /* setup MSI hardware registers */
> mobiveil_pcie_enable_msi(pcie);
> @@ -668,9 +673,9 @@ static void mobiveil_mask_intx_irq(struct irq_data *data)
> pcie = irq_desc_get_chip_data(desc);
> mask = 1 << ((data->hwirq + PAB_INTX_START) - 1);
> raw_spin_lock_irqsave(&pcie->intx_mask_lock, flags);
> - shifted_val = csr_readl(pcie, PAB_INTP_AMBA_MISC_ENB);
> + shifted_val = mobiveil_csr_readl(pcie, PAB_INTP_AMBA_MISC_ENB);
> shifted_val &= ~mask;
> - csr_writel(pcie, shifted_val, PAB_INTP_AMBA_MISC_ENB);
> + mobiveil_csr_writel(pcie, shifted_val, PAB_INTP_AMBA_MISC_ENB);
> raw_spin_unlock_irqrestore(&pcie->intx_mask_lock, flags);
> }
>
> @@ -684,9 +689,9 @@ static void mobiveil_unmask_intx_irq(struct irq_data *data)
> pcie = irq_desc_get_chip_data(desc);
> mask = 1 << ((data->hwirq + PAB_INTX_START) - 1);
> raw_spin_lock_irqsave(&pcie->intx_mask_lock, flags);
> - shifted_val = csr_readl(pcie, PAB_INTP_AMBA_MISC_ENB);
> + shifted_val = mobiveil_csr_readl(pcie, PAB_INTP_AMBA_MISC_ENB);
> shifted_val |= mask;
> - csr_writel(pcie, shifted_val, PAB_INTP_AMBA_MISC_ENB);
> + mobiveil_csr_writel(pcie, shifted_val, PAB_INTP_AMBA_MISC_ENB);
> raw_spin_unlock_irqrestore(&pcie->intx_mask_lock, flags);
> }
>
> --
> 2.20.1
>

2019-10-15 15:08:08

by Lorenzo Pieralisi

[permalink] [raw]
Subject: Re: [PATCH v3] PCI: mobiveil: Fix csr_read/write build issue

On Fri, Oct 04, 2019 at 12:19:25PM +0800, Kefeng Wang wrote:
> The riscv has csr_read/write macro, see arch/riscv/include/asm/csr.h,
> the same function naming will cause build error, using such generic names
> in a driver is bad, rename csr_[read,write][l,] to mobiveil_csr_read/write
> to fix it.
>
> drivers/pci/controller/pcie-mobiveil.c:238:69: error: macro "csr_read" passed 3 arguments, but takes just 1
> static u32 csr_read(struct mobiveil_pcie *pcie, u32 off, size_t size)
>
> drivers/pci/controller/pcie-mobiveil.c:253:80: error: macro "csr_write" passed 4 arguments, but takes just 2
> static void csr_write(struct mobiveil_pcie *pcie, u32 val, u32 off, size_t size)
>
> Cc: Hou Zhiqiang <[email protected]>
> Cc: Lorenzo Pieralisi <[email protected]>
> Cc: Minghuan Lian <[email protected]>
> Cc: Subrahmanya Lingappa <[email protected]>
> Cc: Andrew Murray <[email protected]>
> Fixes: bcbe0d9a8d93 ("PCI: mobiveil: Unify register accessors")
> Signed-off-by: Kefeng Wang <[email protected]>
> ---
> v3:
> - reduce line lengths suggested by Andrew
>
> v2:
> - using mobiveil prefix suggested by Andrew and Christoph
>
> drivers/pci/controller/pcie-mobiveil.c | 119 +++++++++++++------------
> 1 file changed, 62 insertions(+), 57 deletions(-)

Applied to pci/mobiveil, thanks.

Lorenzo

> diff --git a/drivers/pci/controller/pcie-mobiveil.c b/drivers/pci/controller/pcie-mobiveil.c
> index a45a6447b01d..32f37d08d5bc 100644
> --- a/drivers/pci/controller/pcie-mobiveil.c
> +++ b/drivers/pci/controller/pcie-mobiveil.c
> @@ -235,7 +235,7 @@ static int mobiveil_pcie_write(void __iomem *addr, int size, u32 val)
> return PCIBIOS_SUCCESSFUL;
> }
>
> -static u32 csr_read(struct mobiveil_pcie *pcie, u32 off, size_t size)
> +static u32 mobiveil_csr_read(struct mobiveil_pcie *pcie, u32 off, size_t size)
> {
> void *addr;
> u32 val;
> @@ -250,7 +250,8 @@ static u32 csr_read(struct mobiveil_pcie *pcie, u32 off, size_t size)
> return val;
> }
>
> -static void csr_write(struct mobiveil_pcie *pcie, u32 val, u32 off, size_t size)
> +static void mobiveil_csr_write(struct mobiveil_pcie *pcie, u32 val, u32 off,
> + size_t size)
> {
> void *addr;
> int ret;
> @@ -262,19 +263,19 @@ static void csr_write(struct mobiveil_pcie *pcie, u32 val, u32 off, size_t size)
> dev_err(&pcie->pdev->dev, "write CSR address failed\n");
> }
>
> -static u32 csr_readl(struct mobiveil_pcie *pcie, u32 off)
> +static u32 mobiveil_csr_readl(struct mobiveil_pcie *pcie, u32 off)
> {
> - return csr_read(pcie, off, 0x4);
> + return mobiveil_csr_read(pcie, off, 0x4);
> }
>
> -static void csr_writel(struct mobiveil_pcie *pcie, u32 val, u32 off)
> +static void mobiveil_csr_writel(struct mobiveil_pcie *pcie, u32 val, u32 off)
> {
> - csr_write(pcie, val, off, 0x4);
> + mobiveil_csr_write(pcie, val, off, 0x4);
> }
>
> static bool mobiveil_pcie_link_up(struct mobiveil_pcie *pcie)
> {
> - return (csr_readl(pcie, LTSSM_STATUS) &
> + return (mobiveil_csr_readl(pcie, LTSSM_STATUS) &
> LTSSM_STATUS_L0_MASK) == LTSSM_STATUS_L0;
> }
>
> @@ -323,7 +324,7 @@ static void __iomem *mobiveil_pcie_map_bus(struct pci_bus *bus,
> PCI_SLOT(devfn) << PAB_DEVICE_SHIFT |
> PCI_FUNC(devfn) << PAB_FUNCTION_SHIFT;
>
> - csr_writel(pcie, value, PAB_AXI_AMAP_PEX_WIN_L(WIN_NUM_0));
> + mobiveil_csr_writel(pcie, value, PAB_AXI_AMAP_PEX_WIN_L(WIN_NUM_0));
>
> return pcie->config_axi_slave_base + where;
> }
> @@ -353,13 +354,14 @@ static void mobiveil_pcie_isr(struct irq_desc *desc)
> chained_irq_enter(chip, desc);
>
> /* read INTx status */
> - val = csr_readl(pcie, PAB_INTP_AMBA_MISC_STAT);
> - mask = csr_readl(pcie, PAB_INTP_AMBA_MISC_ENB);
> + val = mobiveil_csr_readl(pcie, PAB_INTP_AMBA_MISC_STAT);
> + mask = mobiveil_csr_readl(pcie, PAB_INTP_AMBA_MISC_ENB);
> intr_status = val & mask;
>
> /* Handle INTx */
> if (intr_status & PAB_INTP_INTX_MASK) {
> - shifted_status = csr_readl(pcie, PAB_INTP_AMBA_MISC_STAT);
> + shifted_status = mobiveil_csr_readl(pcie,
> + PAB_INTP_AMBA_MISC_STAT);
> shifted_status &= PAB_INTP_INTX_MASK;
> shifted_status >>= PAB_INTX_START;
> do {
> @@ -373,12 +375,13 @@ static void mobiveil_pcie_isr(struct irq_desc *desc)
> bit);
>
> /* clear interrupt handled */
> - csr_writel(pcie, 1 << (PAB_INTX_START + bit),
> - PAB_INTP_AMBA_MISC_STAT);
> + mobiveil_csr_writel(pcie,
> + 1 << (PAB_INTX_START + bit),
> + PAB_INTP_AMBA_MISC_STAT);
> }
>
> - shifted_status = csr_readl(pcie,
> - PAB_INTP_AMBA_MISC_STAT);
> + shifted_status = mobiveil_csr_readl(pcie,
> + PAB_INTP_AMBA_MISC_STAT);
> shifted_status &= PAB_INTP_INTX_MASK;
> shifted_status >>= PAB_INTX_START;
> } while (shifted_status != 0);
> @@ -413,7 +416,7 @@ static void mobiveil_pcie_isr(struct irq_desc *desc)
> }
>
> /* Clear the interrupt status */
> - csr_writel(pcie, intr_status, PAB_INTP_AMBA_MISC_STAT);
> + mobiveil_csr_writel(pcie, intr_status, PAB_INTP_AMBA_MISC_STAT);
> chained_irq_exit(chip, desc);
> }
>
> @@ -474,24 +477,24 @@ static void program_ib_windows(struct mobiveil_pcie *pcie, int win_num,
> return;
> }
>
> - value = csr_readl(pcie, PAB_PEX_AMAP_CTRL(win_num));
> + value = mobiveil_csr_readl(pcie, PAB_PEX_AMAP_CTRL(win_num));
> value &= ~(AMAP_CTRL_TYPE_MASK << AMAP_CTRL_TYPE_SHIFT | WIN_SIZE_MASK);
> value |= type << AMAP_CTRL_TYPE_SHIFT | 1 << AMAP_CTRL_EN_SHIFT |
> (lower_32_bits(size64) & WIN_SIZE_MASK);
> - csr_writel(pcie, value, PAB_PEX_AMAP_CTRL(win_num));
> + mobiveil_csr_writel(pcie, value, PAB_PEX_AMAP_CTRL(win_num));
>
> - csr_writel(pcie, upper_32_bits(size64),
> - PAB_EXT_PEX_AMAP_SIZEN(win_num));
> + mobiveil_csr_writel(pcie, upper_32_bits(size64),
> + PAB_EXT_PEX_AMAP_SIZEN(win_num));
>
> - csr_writel(pcie, lower_32_bits(cpu_addr),
> - PAB_PEX_AMAP_AXI_WIN(win_num));
> - csr_writel(pcie, upper_32_bits(cpu_addr),
> - PAB_EXT_PEX_AMAP_AXI_WIN(win_num));
> + mobiveil_csr_writel(pcie, lower_32_bits(cpu_addr),
> + PAB_PEX_AMAP_AXI_WIN(win_num));
> + mobiveil_csr_writel(pcie, upper_32_bits(cpu_addr),
> + PAB_EXT_PEX_AMAP_AXI_WIN(win_num));
>
> - csr_writel(pcie, lower_32_bits(pci_addr),
> - PAB_PEX_AMAP_PEX_WIN_L(win_num));
> - csr_writel(pcie, upper_32_bits(pci_addr),
> - PAB_PEX_AMAP_PEX_WIN_H(win_num));
> + mobiveil_csr_writel(pcie, lower_32_bits(pci_addr),
> + PAB_PEX_AMAP_PEX_WIN_L(win_num));
> + mobiveil_csr_writel(pcie, upper_32_bits(pci_addr),
> + PAB_PEX_AMAP_PEX_WIN_H(win_num));
>
> pcie->ib_wins_configured++;
> }
> @@ -515,27 +518,29 @@ static void program_ob_windows(struct mobiveil_pcie *pcie, int win_num,
> * program Enable Bit to 1, Type Bit to (00) base 2, AXI Window Size Bit
> * to 4 KB in PAB_AXI_AMAP_CTRL register
> */
> - value = csr_readl(pcie, PAB_AXI_AMAP_CTRL(win_num));
> + value = mobiveil_csr_readl(pcie, PAB_AXI_AMAP_CTRL(win_num));
> value &= ~(WIN_TYPE_MASK << WIN_TYPE_SHIFT | WIN_SIZE_MASK);
> value |= 1 << WIN_ENABLE_SHIFT | type << WIN_TYPE_SHIFT |
> (lower_32_bits(size64) & WIN_SIZE_MASK);
> - csr_writel(pcie, value, PAB_AXI_AMAP_CTRL(win_num));
> + mobiveil_csr_writel(pcie, value, PAB_AXI_AMAP_CTRL(win_num));
>
> - csr_writel(pcie, upper_32_bits(size64), PAB_EXT_AXI_AMAP_SIZE(win_num));
> + mobiveil_csr_writel(pcie, upper_32_bits(size64),
> + PAB_EXT_AXI_AMAP_SIZE(win_num));
>
> /*
> * program AXI window base with appropriate value in
> * PAB_AXI_AMAP_AXI_WIN0 register
> */
> - csr_writel(pcie, lower_32_bits(cpu_addr) & (~AXI_WINDOW_ALIGN_MASK),
> - PAB_AXI_AMAP_AXI_WIN(win_num));
> - csr_writel(pcie, upper_32_bits(cpu_addr),
> - PAB_EXT_AXI_AMAP_AXI_WIN(win_num));
> + mobiveil_csr_writel(pcie,
> + lower_32_bits(cpu_addr) & (~AXI_WINDOW_ALIGN_MASK),
> + PAB_AXI_AMAP_AXI_WIN(win_num));
> + mobiveil_csr_writel(pcie, upper_32_bits(cpu_addr),
> + PAB_EXT_AXI_AMAP_AXI_WIN(win_num));
>
> - csr_writel(pcie, lower_32_bits(pci_addr),
> - PAB_AXI_AMAP_PEX_WIN_L(win_num));
> - csr_writel(pcie, upper_32_bits(pci_addr),
> - PAB_AXI_AMAP_PEX_WIN_H(win_num));
> + mobiveil_csr_writel(pcie, lower_32_bits(pci_addr),
> + PAB_AXI_AMAP_PEX_WIN_L(win_num));
> + mobiveil_csr_writel(pcie, upper_32_bits(pci_addr),
> + PAB_AXI_AMAP_PEX_WIN_H(win_num));
>
> pcie->ob_wins_configured++;
> }
> @@ -579,42 +584,42 @@ static int mobiveil_host_init(struct mobiveil_pcie *pcie)
> struct resource_entry *win;
>
> /* setup bus numbers */
> - value = csr_readl(pcie, PCI_PRIMARY_BUS);
> + value = mobiveil_csr_readl(pcie, PCI_PRIMARY_BUS);
> value &= 0xff000000;
> value |= 0x00ff0100;
> - csr_writel(pcie, value, PCI_PRIMARY_BUS);
> + mobiveil_csr_writel(pcie, value, PCI_PRIMARY_BUS);
>
> /*
> * program Bus Master Enable Bit in Command Register in PAB Config
> * Space
> */
> - value = csr_readl(pcie, PCI_COMMAND);
> + value = mobiveil_csr_readl(pcie, PCI_COMMAND);
> value |= PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER;
> - csr_writel(pcie, value, PCI_COMMAND);
> + mobiveil_csr_writel(pcie, value, PCI_COMMAND);
>
> /*
> * program PIO Enable Bit to 1 (and PEX PIO Enable to 1) in PAB_CTRL
> * register
> */
> - pab_ctrl = csr_readl(pcie, PAB_CTRL);
> + pab_ctrl = mobiveil_csr_readl(pcie, PAB_CTRL);
> pab_ctrl |= (1 << AMBA_PIO_ENABLE_SHIFT) | (1 << PEX_PIO_ENABLE_SHIFT);
> - csr_writel(pcie, pab_ctrl, PAB_CTRL);
> + mobiveil_csr_writel(pcie, pab_ctrl, PAB_CTRL);
>
> - csr_writel(pcie, (PAB_INTP_INTX_MASK | PAB_INTP_MSI_MASK),
> - PAB_INTP_AMBA_MISC_ENB);
> + mobiveil_csr_writel(pcie, (PAB_INTP_INTX_MASK | PAB_INTP_MSI_MASK),
> + PAB_INTP_AMBA_MISC_ENB);
>
> /*
> * program PIO Enable Bit to 1 and Config Window Enable Bit to 1 in
> * PAB_AXI_PIO_CTRL Register
> */
> - value = csr_readl(pcie, PAB_AXI_PIO_CTRL);
> + value = mobiveil_csr_readl(pcie, PAB_AXI_PIO_CTRL);
> value |= APIO_EN_MASK;
> - csr_writel(pcie, value, PAB_AXI_PIO_CTRL);
> + mobiveil_csr_writel(pcie, value, PAB_AXI_PIO_CTRL);
>
> /* Enable PCIe PIO master */
> - value = csr_readl(pcie, PAB_PEX_PIO_CTRL);
> + value = mobiveil_csr_readl(pcie, PAB_PEX_PIO_CTRL);
> value |= 1 << PIO_ENABLE_SHIFT;
> - csr_writel(pcie, value, PAB_PEX_PIO_CTRL);
> + mobiveil_csr_writel(pcie, value, PAB_PEX_PIO_CTRL);
>
> /*
> * we'll program one outbound window for config reads and
> @@ -647,10 +652,10 @@ static int mobiveil_host_init(struct mobiveil_pcie *pcie)
> }
>
> /* fixup for PCIe class register */
> - value = csr_readl(pcie, PAB_INTP_AXI_PIO_CLASS);
> + value = mobiveil_csr_readl(pcie, PAB_INTP_AXI_PIO_CLASS);
> value &= 0xff;
> value |= (PCI_CLASS_BRIDGE_PCI << 16);
> - csr_writel(pcie, value, PAB_INTP_AXI_PIO_CLASS);
> + mobiveil_csr_writel(pcie, value, PAB_INTP_AXI_PIO_CLASS);
>
> /* setup MSI hardware registers */
> mobiveil_pcie_enable_msi(pcie);
> @@ -668,9 +673,9 @@ static void mobiveil_mask_intx_irq(struct irq_data *data)
> pcie = irq_desc_get_chip_data(desc);
> mask = 1 << ((data->hwirq + PAB_INTX_START) - 1);
> raw_spin_lock_irqsave(&pcie->intx_mask_lock, flags);
> - shifted_val = csr_readl(pcie, PAB_INTP_AMBA_MISC_ENB);
> + shifted_val = mobiveil_csr_readl(pcie, PAB_INTP_AMBA_MISC_ENB);
> shifted_val &= ~mask;
> - csr_writel(pcie, shifted_val, PAB_INTP_AMBA_MISC_ENB);
> + mobiveil_csr_writel(pcie, shifted_val, PAB_INTP_AMBA_MISC_ENB);
> raw_spin_unlock_irqrestore(&pcie->intx_mask_lock, flags);
> }
>
> @@ -684,9 +689,9 @@ static void mobiveil_unmask_intx_irq(struct irq_data *data)
> pcie = irq_desc_get_chip_data(desc);
> mask = 1 << ((data->hwirq + PAB_INTX_START) - 1);
> raw_spin_lock_irqsave(&pcie->intx_mask_lock, flags);
> - shifted_val = csr_readl(pcie, PAB_INTP_AMBA_MISC_ENB);
> + shifted_val = mobiveil_csr_readl(pcie, PAB_INTP_AMBA_MISC_ENB);
> shifted_val |= mask;
> - csr_writel(pcie, shifted_val, PAB_INTP_AMBA_MISC_ENB);
> + mobiveil_csr_writel(pcie, shifted_val, PAB_INTP_AMBA_MISC_ENB);
> raw_spin_unlock_irqrestore(&pcie->intx_mask_lock, flags);
> }
>
> --
> 2.20.1
>