On Tuesday, October 08, 2013 3:07 PM, Pratyush Anand wrote:
> On Tue, Oct 08, 2013 at 08:07:24AM +0800, Bjorn Helgaas wrote:
> > On Mon, Oct 7, 2013 at 6:04 PM, Bjorn Helgaas <bhelgaas@xxxxxxxxxx> wrote:
> > > The following variables and functions are used only in pcie-designware.c,
> > > so make them static:
> > ...
> > > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> > > index c10e9ac..900e875 100644
> > > --- a/drivers/pci/host/pcie-designware.c
> > > +++ b/drivers/pci/host/pcie-designware.c
> > > @@ -64,7 +64,7 @@
> > >
> > > static struct hw_pci dw_pci;
> > >
> > > -unsigned long global_io_offset;
> > > +static unsigned long global_io_offset;
> >
> > While you're looking at this, I think "cfg_read()" and "cfg_write()"
> > are too generic to be global symbols. I don't know if it would make
> > sense to rename them "dw_cfg_read()", pass around pointers in a
> > structure or what.
>
> In fact cfg_read, cfg_write & sys_to_pcie should go to common library,
> as you will find similar function in other files too.
>
(+CC each PCIe host controller maintainer)
Yes, you're right. I think so, too. :-)
cfg_read, cfg_write & sys_to_pcie can go to common library.
I looked at pcie-designware.c, pci-tegra.c, & pci-mvebu.c.
These drivers can use common cfg_read(), cfg_write(), and sys_to_pcie().
For example, common cfg_read() can be used as below:
If there is no objection, I will send the patch.
However, I cannot decide which file is a good place for the common
cfg_read(), cfg_write(), and sys_to_pcie().
int cfg_read(void __iomem *addr, int where, int size, u32 *val)
{
*val = readl(addr);
if (size == 1)
*val = (*val >> (8 * (where & 3))) & 0xff;
else if (size == 2)
*val = (*val >> (8 * (where & 3))) & 0xffff;
else if (size != 4)
return PCIBIOS_BAD_REGISTER_NUMBER;
return PCIBIOS_SUCCESSFUL;
}
./drivers/pci/host/pci-mvebu.c
@@ -243,14 +243,7 @@ static int mvebu_pcie_hw_rd_conf(struct mvebu_pcie_port *port,
writel(PCIE_CONF_ADDR(bus->number, devfn, where),
port->base + PCIE_CONF_ADDR_OFF);
- *val = readl(port->base + PCIE_CONF_DATA_OFF);
-
- if (size == 1)
- *val = (*val >> (8 * (where & 3))) & 0xff;
- else if (size == 2)
- *val = (*val >> (8 * (where & 3))) & 0xffff;
-
- return PCIBIOS_SUCCESSFUL;
+ return cfg_read(port->base + PCIE_CONF_DATA_OFF, where, size, val);
}
./drivers/pci/host/pci-tegra.c
@@ -462,14 +462,7 @@ static int tegra_pcie_read_conf(struct pci_bus *bus, unsigned int devfn,
return PCIBIOS_DEVICE_NOT_FOUND;
}
- *value = readl(addr);
-
- if (size == 1)
- *value = (*value >> (8 * (where & 3))) & 0xff;
- else if (size == 2)
- *value = (*value >> (8 * (where & 3))) & 0xffff;
-
- return PCIBIOS_SUCCESSFUL;
+ return cfg_read(addr, where, size, val);
}
Best regards,
Jingoo Han
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
