Hi Iwamatsu-san, Bjorn, On 20 March 2015 22:45, Bjorn Helgaas wrote: > On Wed, Mar 11, 2015 at 01:23:50PM +0900, Nobuhiro Iwamatsu wrote: > > The reg variable used when setting PCIECAR register need to be masked by > 0xFC > > by restriction of the corresponding register. > > This adds PCIE_CONF_REGNO are macros for masking changes that > PCIE_CONF_REGNO > > is used when setting PCIECAR register. > > > > Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@xxxxxxxxxxx> > > --- > > drivers/pci/host/pcie-rcar.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/pci/host/pcie-rcar.c b/drivers/pci/host/pcie-rcar.c > > index c57bd0a..1f493ef 100644 > > --- a/drivers/pci/host/pcie-rcar.c > > +++ b/drivers/pci/host/pcie-rcar.c > > @@ -104,6 +104,7 @@ > > #define PCIE_CONF_BUS(b) (((b) & 0xff) << 24) > > #define PCIE_CONF_DEV(d) (((d) & 0x1f) << 19) > > #define PCIE_CONF_FUNC(f) (((f) & 0x7) << 16) > > +#define PCIE_CONF_REGNO(r) (r & 0xfc) > > > > #define RCAR_PCI_MAX_RESOURCES 4 > > #define MAX_NR_INBOUND_MAPS 6 > > @@ -227,7 +228,8 @@ static int rcar_pcie_config_access(struct rcar_pcie *pcie, > > > > /* Set the PIO address */ > > rcar_pci_write_reg(pcie, PCIE_CONF_BUS(bus->number) | > > - PCIE_CONF_DEV(dev) | PCIE_CONF_FUNC(func) | reg, PCIECAR); > > + PCIE_CONF_DEV(dev) | PCIE_CONF_FUNC(func) | > > + PCIE_CONF_REGNO(reg), PCIECAR); > > What about the other rcar_pci_read_reg()/rcar_pci_write_reg() calls (the > ones for "if (pci_is_root_bus())"? Do those need to be limited to the > first 256 bytes also? > > It seems a little strange to mask "reg = where & ~3" at the top (making it > 4-byte aligned), then mask "r & 0xfc" here (limiting to first 256 bytes and > also making it 4-byte aligned). > > Seems like those two operations should either be combined or completely > split. I would prefer that we return an error if the register is outside the supported range instead of simply masking it off. Also, shouldn't the valid range include the Extended register numbers? > > /* Enable the configuration access */ > > if (bus->parent->number == pcie->root_bus_nr) > > -- > > 2.1.3 > > Thanks Phil -- 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
