RE: [PATCH v10 4/6] PCI: hisi: Add PCIe host support for HiSilicon SoC Hip05

[Gabriele Paoloni]

> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@xxxxxxxx]
> Sent: Tuesday, October 13, 2015 12:12 PM
> To: Bjorn Helgaas
> Cc: Wangzhou (B); Bjorn Helgaas; jingoohan1@xxxxxxxxx;
> pratyush.anand@xxxxxxxxx; linux@xxxxxxxxxxxxxxxx;
> thomas.petazzoni@xxxxxxxxxxxxxxxxxx; Gabriele Paoloni;
> lorenzo.pieralisi@xxxxxxx; james.morse@xxxxxxx; Liviu.Dudau@xxxxxxx;
> jason@xxxxxxxxxxxxxx; robh@xxxxxxxxxx; gabriel.fernandez@xxxxxxxxxx;
> Minghuan.Lian@xxxxxxxxxxxxx; linux-pci@xxxxxxxxxxxxxxx; linux-arm-
> kernel@xxxxxxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; zhangjukuo; qiuzhenfa; liudongdong (C);
> qiujiang; xuwei (O); Liguozhu (Kenneth); Wangkefeng (Kevin); Rob
> Herring
> Subject: Re: [PATCH v10 4/6] PCI: hisi: Add PCIe host support for
> HiSilicon SoC Hip05
> 
> On Monday 12 October 2015 16:35:45 Bjorn Helgaas wrote:
> >
> > > +{
> > > +     u64 addr;
> > > +     struct device_node *msi_node;
> > > +     struct resource res;
> > > +     struct device_node *np = pp->dev->of_node;
> > > +     struct hisi_pcie *pcie = to_hisi_pcie(pp);
> > > +
> > > +     msi_node = of_parse_phandle(np, "msi-parent", 0);
> > > +     if (!msi_node) {
> > > +             dev_err(pp->dev, "failed to find msi-parent\n");
> > > +             return -EINVAL;
> > > +     }
> > > +     of_address_to_resource(msi_node, 0, &res);
> >
> > Does this use the "msi-parent" node in the same way as other drivers
> > do?  I'm sure there must be other places where we extract struct
> > resource information from an "msi-parent" node, but I don't see them.
> >
> > I'm trying to verify that this isn't some kind of incompatible
> > extension of the "msi-parent" property.  I cc'd Arnd and Rob (DT
> > experts).
> 
> This is not ok, what this does is that it relies on a particular
> implementation of the MSI controller and directly accesses its
> registers.

Hi Arnd, thanks for reviewing.

What we do is to retrieve the msi-parent physical address and we store it
in our internal PCIe register locations...

So we do not operate directly on the msi controller registers...

So I wonder if the current implementation is Ok to retrieve the
msi-parent address....

Thanks

Gab


> 
> Instead, it should reference only the msi irq domain and let the
> driver for the MSI controller access the registers. Otherwise this
> code has to be rewritten once the same PCI host code appears in
> a machine that has a real GICv2m or GICv3.
> 
> 	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/