From: Simon Horman <horms@verge.net.au>
To: Phil Edworthy <phil.edworthy@renesas.com>
Cc: linux-pci@vger.kernel.org, linux-sh@vger.kernel.org,
Bjorn Helgaas <bhelgaas@google.com>,
Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Subject: Re: [PATCH 2/2] PCI: host: pcie-rcar: Style and formatting changes.
Date: Sat, 28 Jun 2014 00:14:48 +0000 [thread overview]
Message-ID: <20140628001448.GE27128@verge.net.au> (raw)
In-Reply-To: <1403888155-9709-3-git-send-email-phil.edworthy@renesas.com>
On Fri, Jun 27, 2014 at 05:55:55PM +0100, Phil Edworthy wrote:
> This patch just makes symbol and function name changes to avoid
> potential conflicts, along with minor formatting changes.
> The only other change is to replace an arg passed into a func
> with a local variable.
>
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> ---
> drivers/pci/host/pcie-rcar.c | 131 ++++++++++++++++++++++---------------------
> 1 file changed, 67 insertions(+), 64 deletions(-)
>
> diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c
> index 680a8cc..efdea07 100644
> --- a/drivers/pci/host/pcie-rcar.c
> +++ b/drivers/pci/host/pcie-rcar.c
> @@ -105,7 +105,7 @@
> #define PCIE_CONF_DEV(d) (((d) & 0x1f) << 19)
> #define PCIE_CONF_FUNC(f) (((f) & 0x7) << 16)
>
> -#define PCI_MAX_RESOURCES 4
> +#define RCAR_PCI_MAX_RESOURCES 4
> #define MAX_NR_INBOUND_MAPS 6
>
> struct rcar_msi {
> @@ -127,7 +127,7 @@ static inline struct rcar_msi *to_rcar_msi(struct msi_chip *chip)
> struct rcar_pcie {
> struct device *dev;
> void __iomem *base;
> - struct resource res[PCI_MAX_RESOURCES];
> + struct resource res[RCAR_PCI_MAX_RESOURCES];
> struct resource busn;
> int root_bus_nr;
> struct clk *clk;
> @@ -140,36 +140,37 @@ static inline struct rcar_pcie *sys_to_pcie(struct pci_sys_data *sys)
> return sys->private_data;
> }
>
> -static void pci_write_reg(struct rcar_pcie *pcie, unsigned long val,
> - unsigned long reg)
> +static void rcar_pci_write_reg(struct rcar_pcie *pcie, unsigned long val,
> + unsigned long reg)
> {
> writel(val, pcie->base + reg);
> }
>
> -static unsigned long pci_read_reg(struct rcar_pcie *pcie, unsigned long reg)
> +static unsigned long rcar_pci_read_reg(struct rcar_pcie *pcie,
> + unsigned long reg)
> {
> return readl(pcie->base + reg);
> }
>
> enum {
> - PCI_ACCESS_READ,
> - PCI_ACCESS_WRITE,
> + RCAR_PCI_ACCESS_READ,
> + RCAR_PCI_ACCESS_WRITE,
> };
>
> static void rcar_rmw32(struct rcar_pcie *pcie, int where, u32 mask, u32 data)
> {
> int shift = 8 * (where & 3);
> - u32 val = pci_read_reg(pcie, where & ~3);
> + u32 val = rcar_pci_read_reg(pcie, where & ~3);
>
> val &= ~(mask << shift);
> val |= data << shift;
> - pci_write_reg(pcie, val, where & ~3);
> + rcar_pci_write_reg(pcie, val, where & ~3);
> }
>
> static u32 rcar_read_conf(struct rcar_pcie *pcie, int where)
> {
> int shift = 8 * (where & 3);
> - u32 val = pci_read_reg(pcie, where & ~3);
> + u32 val = rcar_pci_read_reg(pcie, where & ~3);
>
> return val >> shift;
> }
> @@ -205,14 +206,14 @@ static int rcar_pcie_config_access(struct rcar_pcie *pcie,
> if (dev != 0)
> return PCIBIOS_DEVICE_NOT_FOUND;
>
> - if (access_type = PCI_ACCESS_READ) {
> - *data = pci_read_reg(pcie, PCICONF(index));
> + if (access_type = RCAR_PCI_ACCESS_READ) {
> + *data = rcar_pci_read_reg(pcie, PCICONF(index));
> } else {
> /* Keep an eye out for changes to the root bus number */
> if (pci_is_root_bus(bus) && (reg = PCI_PRIMARY_BUS))
> pcie->root_bus_nr = *data & 0xff;
>
> - pci_write_reg(pcie, *data, PCICONF(index));
> + rcar_pci_write_reg(pcie, *data, PCICONF(index));
> }
>
> return PCIBIOS_SUCCESSFUL;
> @@ -222,20 +223,20 @@ static int rcar_pcie_config_access(struct rcar_pcie *pcie,
> return PCIBIOS_DEVICE_NOT_FOUND;
>
> /* Clear errors */
> - pci_write_reg(pcie, pci_read_reg(pcie, PCIEERRFR), PCIEERRFR);
> + rcar_pci_write_reg(pcie, rcar_pci_read_reg(pcie, PCIEERRFR), PCIEERRFR);
>
> /* Set the PIO address */
> - pci_write_reg(pcie, PCIE_CONF_BUS(bus->number) | PCIE_CONF_DEV(dev) |
> - PCIE_CONF_FUNC(func) | reg, PCIECAR);
> + rcar_pci_write_reg(pcie, PCIE_CONF_BUS(bus->number) |
> + PCIE_CONF_DEV(dev) | PCIE_CONF_FUNC(func) | reg, PCIECAR);
>
> /* Enable the configuration access */
> if (bus->parent->number = pcie->root_bus_nr)
> - pci_write_reg(pcie, CONFIG_SEND_ENABLE | TYPE0, PCIECCTLR);
> + rcar_pci_write_reg(pcie, CONFIG_SEND_ENABLE | TYPE0, PCIECCTLR);
> else
> - pci_write_reg(pcie, CONFIG_SEND_ENABLE | TYPE1, PCIECCTLR);
> + rcar_pci_write_reg(pcie, CONFIG_SEND_ENABLE | TYPE1, PCIECCTLR);
>
> /* Check for errors */
> - if (pci_read_reg(pcie, PCIEERRFR) & UNSUPPORTED_REQUEST)
> + if (rcar_pci_read_reg(pcie, PCIEERRFR) & UNSUPPORTED_REQUEST)
> return PCIBIOS_DEVICE_NOT_FOUND;
>
> /* Check for master and target aborts */
> @@ -243,13 +244,13 @@ static int rcar_pcie_config_access(struct rcar_pcie *pcie,
> (PCI_STATUS_REC_MASTER_ABORT | PCI_STATUS_REC_TARGET_ABORT))
> return PCIBIOS_DEVICE_NOT_FOUND;
>
> - if (access_type = PCI_ACCESS_READ)
> - *data = pci_read_reg(pcie, PCIECDR);
> + if (access_type = RCAR_PCI_ACCESS_READ)
> + *data = rcar_pci_read_reg(pcie, PCIECDR);
> else
> - pci_write_reg(pcie, *data, PCIECDR);
> + rcar_pci_write_reg(pcie, *data, PCIECDR);
>
> /* Disable the configuration access */
> - pci_write_reg(pcie, 0, PCIECCTLR);
> + rcar_pci_write_reg(pcie, 0, PCIECCTLR);
>
> return PCIBIOS_SUCCESSFUL;
> }
> @@ -260,7 +261,7 @@ static int rcar_pcie_read_conf(struct pci_bus *bus, unsigned int devfn,
> struct rcar_pcie *pcie = sys_to_pcie(bus->sysdata);
> int ret;
>
> - ret = rcar_pcie_config_access(pcie, PCI_ACCESS_READ,
> + ret = rcar_pcie_config_access(pcie, RCAR_PCI_ACCESS_READ,
> bus, devfn, where, val);
> if (ret != PCIBIOS_SUCCESSFUL) {
> *val = 0xffffffff;
> @@ -287,7 +288,7 @@ static int rcar_pcie_write_conf(struct pci_bus *bus, unsigned int devfn,
> int shift, ret;
> u32 data;
>
> - ret = rcar_pcie_config_access(pcie, PCI_ACCESS_READ,
> + ret = rcar_pcie_config_access(pcie, RCAR_PCI_ACCESS_READ,
> bus, devfn, where, &data);
> if (ret != PCIBIOS_SUCCESSFUL)
> return ret;
> @@ -307,7 +308,7 @@ static int rcar_pcie_write_conf(struct pci_bus *bus, unsigned int devfn,
> } else
> data = val;
>
> - ret = rcar_pcie_config_access(pcie, PCI_ACCESS_WRITE,
> + ret = rcar_pcie_config_access(pcie, RCAR_PCI_ACCESS_WRITE,
> bus, devfn, where, &data);
>
> return ret;
> @@ -318,14 +319,15 @@ static struct pci_ops rcar_pcie_ops = {
> .write = rcar_pcie_write_conf,
> };
>
> -static void rcar_pcie_setup_window(int win, struct resource *res,
> - struct rcar_pcie *pcie)
> +static void rcar_pcie_setup_window(int win, struct rcar_pcie *pcie)
> {
> + struct resource *res = &pcie->res[win];
> +
The removal of the res parameter is not in keeping with the changelog.
Could you break it out into a separate patch?
> /* Setup PCIe address space mappings for each resource */
> resource_size_t size;
> u32 mask;
>
> - pci_write_reg(pcie, 0x00000000, PCIEPTCTLR(win));
> + rcar_pci_write_reg(pcie, 0x00000000, PCIEPTCTLR(win));
>
> /*
> * The PAMR mask is calculated in units of 128Bytes, which
> @@ -333,17 +335,17 @@ static void rcar_pcie_setup_window(int win, struct resource *res,
> */
> size = resource_size(res);
> mask = (roundup_pow_of_two(size) / SZ_128) - 1;
> - pci_write_reg(pcie, mask << 7, PCIEPAMR(win));
> + rcar_pci_write_reg(pcie, mask << 7, PCIEPAMR(win));
>
> - pci_write_reg(pcie, upper_32_bits(res->start), PCIEPARH(win));
> - pci_write_reg(pcie, lower_32_bits(res->start), PCIEPARL(win));
> + rcar_pci_write_reg(pcie, upper_32_bits(res->start), PCIEPARH(win));
> + rcar_pci_write_reg(pcie, lower_32_bits(res->start), PCIEPARL(win));
>
> /* First resource is for IO */
> mask = PAR_ENABLE;
> if (res->flags & IORESOURCE_IO)
> mask |= IO_SPACE;
>
> - pci_write_reg(pcie, mask, PCIEPTCTLR(win));
> + rcar_pci_write_reg(pcie, mask, PCIEPTCTLR(win));
> }
>
> static int rcar_pcie_setup(int nr, struct pci_sys_data *sys)
> @@ -355,13 +357,13 @@ static int rcar_pcie_setup(int nr, struct pci_sys_data *sys)
> pcie->root_bus_nr = -1;
>
> /* Setup PCI resources */
> - for (i = 0; i < PCI_MAX_RESOURCES; i++) {
> + for (i = 0; i < RCAR_PCI_MAX_RESOURCES; i++) {
>
> res = &pcie->res[i];
> if (!res->flags)
> continue;
>
> - rcar_pcie_setup_window(i, res, pcie);
> + rcar_pcie_setup_window(i, pcie);
>
> if (res->flags & IORESOURCE_IO)
> pci_ioremap_io(nr * SZ_64K, res->start);
> @@ -407,7 +409,7 @@ static int phy_wait_for_ack(struct rcar_pcie *pcie)
> unsigned int timeout = 100;
>
> while (timeout--) {
> - if (pci_read_reg(pcie, H1_PCIEPHYADRR) & PHY_ACK)
> + if (rcar_pci_read_reg(pcie, H1_PCIEPHYADRR) & PHY_ACK)
> return 0;
>
> udelay(100);
> @@ -430,15 +432,15 @@ static void phy_write_reg(struct rcar_pcie *pcie,
> ((addr & 0xff) << ADR_POS);
>
> /* Set write data */
> - pci_write_reg(pcie, data, H1_PCIEPHYDOUTR);
> - pci_write_reg(pcie, phyaddr, H1_PCIEPHYADRR);
> + rcar_pci_write_reg(pcie, data, H1_PCIEPHYDOUTR);
> + rcar_pci_write_reg(pcie, phyaddr, H1_PCIEPHYADRR);
>
> /* Ignore errors as they will be dealt with if the data link is down */
> phy_wait_for_ack(pcie);
>
> /* Clear command */
> - pci_write_reg(pcie, 0, H1_PCIEPHYDOUTR);
> - pci_write_reg(pcie, 0, H1_PCIEPHYADRR);
> + rcar_pci_write_reg(pcie, 0, H1_PCIEPHYDOUTR);
> + rcar_pci_write_reg(pcie, 0, H1_PCIEPHYADRR);
>
> /* Ignore errors as they will be dealt with if the data link is down */
> phy_wait_for_ack(pcie);
> @@ -449,7 +451,7 @@ static int rcar_pcie_wait_for_dl(struct rcar_pcie *pcie)
> unsigned int timeout = 10;
>
> while (timeout--) {
> - if ((pci_read_reg(pcie, PCIETSTR) & DATA_LINK_ACTIVE))
> + if ((rcar_pci_read_reg(pcie, PCIETSTR) & DATA_LINK_ACTIVE))
> return 0;
>
> msleep(5);
> @@ -463,17 +465,17 @@ static int rcar_pcie_hw_init(struct rcar_pcie *pcie)
> int err;
>
> /* Begin initialization */
> - pci_write_reg(pcie, 0, PCIETCTLR);
> + rcar_pci_write_reg(pcie, 0, PCIETCTLR);
>
> /* Set mode */
> - pci_write_reg(pcie, 1, PCIEMSR);
> + rcar_pci_write_reg(pcie, 1, PCIEMSR);
>
> /*
> * Initial header for port config space is type 1, set the device
> * class to match. Hardware takes care of propagating the IDSETR
> * settings, so there is no need to bother with a quirk.
> */
> - pci_write_reg(pcie, PCI_CLASS_BRIDGE_PCI << 16, IDSETR1);
> + rcar_pci_write_reg(pcie, PCI_CLASS_BRIDGE_PCI << 16, IDSETR1);
>
> /*
> * Setup Secondary Bus Number & Subordinate Bus Number, even though
> @@ -497,17 +499,17 @@ static int rcar_pcie_hw_init(struct rcar_pcie *pcie)
> rcar_rmw32(pcie, REXPCAP(PCI_EXP_SLTCAP), PCI_EXP_SLTCAP_PSN, 0);
>
> /* Set the completion timer timeout to the maximum 50ms. */
> - rcar_rmw32(pcie, TLCTLR+1, 0x3f, 50);
> + rcar_rmw32(pcie, TLCTLR + 1, 0x3f, 50);
>
> /* Terminate list of capabilities (Next Capability Offset=0) */
> rcar_rmw32(pcie, RVCCAP(0), 0xfff00000, 0);
>
> /* Enable MSI */
> if (IS_ENABLED(CONFIG_PCI_MSI))
> - pci_write_reg(pcie, 0x101f0000, PCIEMSITXR);
> + rcar_pci_write_reg(pcie, 0x101f0000, PCIEMSITXR);
>
> /* Finish initialization - establish a PCI Express link */
> - pci_write_reg(pcie, CFINIT, PCIETCTLR);
> + rcar_pci_write_reg(pcie, CFINIT, PCIETCTLR);
>
> /* This will timeout if we don't have a link. */
> err = rcar_pcie_wait_for_dl(pcie);
> @@ -545,7 +547,7 @@ static int rcar_pcie_hw_init_h1(struct rcar_pcie *pcie)
> phy_write_reg(pcie, 0, 0x66, 0x1, 0x00008000);
>
> while (timeout--) {
> - if (pci_read_reg(pcie, H1_PCIEPHYSR))
> + if (rcar_pci_read_reg(pcie, H1_PCIEPHYSR))
> return rcar_pcie_hw_init(pcie);
>
> msleep(5);
> @@ -584,7 +586,7 @@ static irqreturn_t rcar_pcie_msi_irq(int irq, void *data)
> struct rcar_msi *msi = &pcie->msi;
> unsigned long reg;
>
> - reg = pci_read_reg(pcie, PCIEMSIFR);
> + reg = rcar_pci_read_reg(pcie, PCIEMSIFR);
>
> /* MSI & INTx share an interrupt - we only handle MSI here */
> if (!reg)
> @@ -595,7 +597,7 @@ static irqreturn_t rcar_pcie_msi_irq(int irq, void *data)
> unsigned int irq;
>
> /* clear the interrupt */
> - pci_write_reg(pcie, 1 << index, PCIEMSIFR);
> + rcar_pci_write_reg(pcie, 1 << index, PCIEMSIFR);
>
> irq = irq_find_mapping(msi->domain, index);
> if (irq) {
> @@ -609,7 +611,7 @@ static irqreturn_t rcar_pcie_msi_irq(int irq, void *data)
> }
>
> /* see if there's any more pending in this vector */
> - reg = pci_read_reg(pcie, PCIEMSIFR);
> + reg = rcar_pci_read_reg(pcie, PCIEMSIFR);
> }
>
> return IRQ_HANDLED;
> @@ -636,8 +638,8 @@ static int rcar_msi_setup_irq(struct msi_chip *chip, struct pci_dev *pdev,
>
> irq_set_msi_desc(irq, desc);
>
> - msg.address_lo = pci_read_reg(pcie, PCIEMSIALR) & ~MSIFE;
> - msg.address_hi = pci_read_reg(pcie, PCIEMSIAUR);
> + msg.address_lo = rcar_pci_read_reg(pcie, PCIEMSIALR) & ~MSIFE;
> + msg.address_hi = rcar_pci_read_reg(pcie, PCIEMSIAUR);
> msg.data = hwirq;
>
> write_msi_msg(irq, &msg);
> @@ -714,11 +716,11 @@ static int rcar_pcie_enable_msi(struct rcar_pcie *pcie)
> msi->pages = __get_free_pages(GFP_KERNEL, 0);
> base = virt_to_phys((void *)msi->pages);
>
> - pci_write_reg(pcie, base | MSIFE, PCIEMSIALR);
> - pci_write_reg(pcie, 0, PCIEMSIAUR);
> + rcar_pci_write_reg(pcie, base | MSIFE, PCIEMSIALR);
> + rcar_pci_write_reg(pcie, 0, PCIEMSIAUR);
>
> /* enable all MSI interrupts */
> - pci_write_reg(pcie, 0xffffffff, PCIEMSIIER);
> + rcar_pci_write_reg(pcie, 0xffffffff, PCIEMSIIER);
>
> return 0;
>
> @@ -811,6 +813,7 @@ static int rcar_pcie_inbound_ranges(struct rcar_pcie *pcie,
> if (cpu_addr > 0) {
> unsigned long nr_zeros = __ffs64(cpu_addr);
> u64 alignment = 1ULL << nr_zeros;
> +
> size = min(range->size, alignment);
> } else {
> size = range->size;
> @@ -826,13 +829,13 @@ static int rcar_pcie_inbound_ranges(struct rcar_pcie *pcie,
> * Set up 64-bit inbound regions as the range parser doesn't
> * distinguish between 32 and 64-bit types.
> */
> - pci_write_reg(pcie, lower_32_bits(pci_addr), PCIEPRAR(idx));
> - pci_write_reg(pcie, lower_32_bits(cpu_addr), PCIELAR(idx));
> - pci_write_reg(pcie, lower_32_bits(mask) | flags, PCIELAMR(idx));
> + rcar_pci_write_reg(pcie, lower_32_bits(pci_addr), PCIEPRAR(idx));
> + rcar_pci_write_reg(pcie, lower_32_bits(cpu_addr), PCIELAR(idx));
> + rcar_pci_write_reg(pcie, lower_32_bits(mask) | flags, PCIELAMR(idx));
>
> - pci_write_reg(pcie, upper_32_bits(pci_addr), PCIEPRAR(idx+1));
> - pci_write_reg(pcie, upper_32_bits(cpu_addr), PCIELAR(idx+1));
> - pci_write_reg(pcie, 0, PCIELAMR(idx+1));
> + rcar_pci_write_reg(pcie, upper_32_bits(pci_addr), PCIEPRAR(idx+1));
> + rcar_pci_write_reg(pcie, upper_32_bits(cpu_addr), PCIELAR(idx+1));
> + rcar_pci_write_reg(pcie, 0, PCIELAMR(idx + 1));
>
> pci_addr += size;
> cpu_addr += size;
> @@ -937,7 +940,7 @@ static int rcar_pcie_probe(struct platform_device *pdev)
> of_pci_range_to_resource(&range, pdev->dev.of_node,
> &pcie->res[win++]);
>
> - if (win > PCI_MAX_RESOURCES)
> + if (win > RCAR_PCI_MAX_RESOURCES)
> break;
> }
>
> @@ -967,7 +970,7 @@ static int rcar_pcie_probe(struct platform_device *pdev)
> return 0;
> }
>
> - data = pci_read_reg(pcie, MACSR);
> + data = rcar_pci_read_reg(pcie, MACSR);
> dev_info(&pdev->dev, "PCIe x%d: link up\n", (data >> 20) & 0x3f);
>
> rcar_pcie_enable(pcie);
> --
> 2.0.0
>
next prev parent reply other threads:[~2014-06-28 0:14 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-06-27 16:55 [PATCH 0/2] PCI: host: pcie-rcar: minor clean ups Phil Edworthy
2014-06-27 16:55 ` [PATCH 1/2] PCI: host: pcie-rcar: Use correct initial HW settings Phil Edworthy
2014-06-27 16:55 ` [PATCH 2/2] PCI: host: pcie-rcar: Style and formatting changes Phil Edworthy
2014-06-28 0:14 ` Simon Horman [this message]
2014-06-28 0:15 ` Simon Horman
2014-06-28 0:22 ` Sergei Shtylyov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140628001448.GE27128@verge.net.au \
--to=horms@verge.net.au \
--cc=bhelgaas@google.com \
--cc=linux-pci@vger.kernel.org \
--cc=linux-sh@vger.kernel.org \
--cc=phil.edworthy@renesas.com \
--cc=sergei.shtylyov@cogentembedded.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox