On Thursday, September 1, 2016 10:05:29 AM CEST Dongdong Liu wrote:
>
> 在 2016/8/31 19:45, Arnd Bergmann 写道:
> > On Wednesday, August 31, 2016 7:48:12 PM CEST Dongdong Liu wrote:
> >> +
> >> +/* HipXX PCIe host only supports 32-bit config access */
> >> +int hisi_pcie_common_cfg_read(void __iomem *reg_base, int where, int size,
> >> + u32 *val)
> >> +{
> >> + u32 reg;
> >> + u32 reg_val;
> >> + void *walker = ®_val;
> >> +
> >> + walker += (where & 0x3);
> >> + reg = where & ~0x3;
> >> + reg_val = readl(reg_base + reg);
> >> +
> >> + if (size == 1)
> >> + *val = *(u8 __force *) walker;
> >> + else if (size == 2)
> >> + *val = *(u16 __force *) walker;
> >
> > What is the __force for?
>
> Hi Arnd, thanks for replying.
>
> __force is used to, well, force a conversion, like casting from or to a bitwise type, else the Sparse checker will throw a warning.
I know what it's for in general, but in this case there is no __bitwise
or __iomem or any other annotation on either side of the assignment.
> >
> >> + else if (size == 4)
> >> + *val = reg_val;
> >> + else
> >> + return PCIBIOS_BAD_REGISTER_NUMBER;
> >> +
> >> + return PCIBIOS_SUCCESSFUL;
> >
> > It looks like you are reimplementing pci_generic_config_read32/pci_generic_config_write32
> > read here, better use them directly.
> >
>
> For our host bridge, access RC and EP config space are not the same way.
> Our host bridge is non ECAM only for the RC bus config space;
> for any other bus underneath the root bus we support ECAM access.
>
> hisi_pcie_common_cfg_read is used to read RC config space, only supports 32-bit config access.
> hisi_pcie_common_cfg_read/hisi_pcie_common_cfg_write may change as below will be better.
>
> /* HipXX PCIe host only supports 32-bit config access */
> int hisi_pcie_common_cfg_read(void __iomem *reg_base, int where, int size,
> u32 *val)
> {
> void __iomem *addr;
>
> addr = reg_base + (where & ~0x3);
> *val = readl(addr);
>
> if (size <= 2)
> *val = (*val >> (8 * (where & 3))) & ((1 << (size * 8)) - 1);
>
> return PCIBIOS_SUCCESSFUL;
> }
My point was: why not call pci_generic_config_read32() when accessing
the RC config space instead of duplicating the code from it?
Arnd
--
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
