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

[Gabriele Paoloni]

> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@xxxxxxxx]
> Sent: 08 February 2016 16:33
> To: Gabriele Paoloni
> Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; Guohanjun (Hanjun Guo);
> Wangzhou (B); liudongdong (C); Linuxarm; qiujiang; bhelgaas@xxxxxxxxxx;
> Lorenzo.Pieralisi@xxxxxxx; tn@xxxxxxxxxxxx; linux-pci@xxxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx; xuwei (O); linux-acpi@xxxxxxxxxxxxxxx;
> jcm@xxxxxxxxxx; zhangjukuo; Liguozhu (Kenneth)
> Subject: Re: [RFC PATCH v2 1/3] PCI: hisi: re-architect Hip05/Hip06
> controllers driver to preapare for ACPI
> 
> On Monday 08 February 2016 16:06:54 Gabriele Paoloni wrote:
> > >
> > > On Monday 08 February 2016 12:41:02 Gabriele Paoloni 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;
> > > > +   else if (size == 4)
> > > > +           *val = reg_val;
> > > > +   else
> > > > +           return PCIBIOS_BAD_REGISTER_NUMBER;
> > > > +
> > > > +   return PCIBIOS_SUCCESSFUL;
> > > > +}
> > >
> > > Isn't this the same hack that Qualcomm are using?
> >
> > As far as I can see Qualcomm defines its own config access
> > mechanism only for RC config read and also it seems they're
> > having problems with reporting the device class...
> >
> > https://github.com/torvalds/linux/blob/master/drivers/pci/host/pcie-
> qcom.c#L474
> >
> > Our problem is that our HW can only perform 32b rd/wr accesses
> > So we can't use readw/readb/writew/writeb...
> >
> >
> 
> Sorry, my mistake, I meant Cavium not Qualcomm.
> See https://lkml.org/lkml/2016/2/5/689 for the patches.

Well, looking at it Cavium seems quite different, 

On read they need to trigger the retrieval of the
config space info writing to the lower 32b of a 64b register,
then they need to read data back on the upper 64b of the
same register and adjust the content to remove the garbage...

We just use 32b accesses and adjust grab the appropriate
bytes depending on the read/write sizes...

Thanks 

Gab

> 
> 	Arnd