Re: [RFC PATCH V2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI

[Dongdong Liu]

å 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 = &reg_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.

> > +       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;
}

/* HipXX PCIe host only supports 32-bit config access */
int hisi_pcie_common_cfg_write(void __iomem *reg_base, int where, int  size,
				u32 val)
{
	void __iomem *addr;
	u32 mask, tmp;

	addr = reg_base + (where & ~0x3);
	if (size == 4) {
		writel(val, addr);
		return PCIBIOS_SUCCESSFUL;
	} else {
		mask = ~(((1 << (size * 8)) - 1) << ((where & 0x3) * 8));
	}

	tmp = readl(addr) & mask;
	tmp |= val << ((where & 0x3) * 8);
	writel(tmp, addr);

	return PCIBIOS_SUCCESSFUL;
}


Thanks
Dongdong

> 	Arnd
>
> .