On Tuesday 13 October 2015 14:49:07 Gabriele Paoloni wrote:

quoted

On Monday 12 October 2015 16:35:45 Bjorn Helgaas wrote:

quoted

quoted

+{
+     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...

Ah, thanks for the clarification, I missed that part.

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....

Not sure. Why do you do this? Normally the PCI host does not know
or care about the address of the MSI controller, because the messages
are just passed up to the parent bus as DMA transfers.

I don't think what you do here is safe because the 'reg' property
of the MSI controller might point to the address that is used for
the message directly.

	Arnd