Re: [PATCH v4 1/2] ARM: shmobile: r8a7790: add internal PCI bridge nodes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 06/21/2014 01:10 AM, Arnd Bergmann wrote:

+       pci0: pci@ee090000 {
+               compatible = "renesas,pci-r8a7790";
+               clocks = <&mstp7_clks R8A7790_CLK_EHCI>;
+               reg = <0x0 0xee090000 0x0 0xc00>,
+                     <0x0 0xee080000 0x0 0x1100>;
+               interrupts = <0 108 IRQ_TYPE_LEVEL_HIGH>;
+               status = "disabled";
+
+               bus-range = <0 0>;
+               #address-cells = >;
+               #size-cells = <2>;
+               #interrupt-cells = <1>;
+               interrupt-map-mask = <0xff00 0 0 0x7>;
+               interrupt-map = <0x0000 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH
+                                0x0800 0 0 1 &gic 0 108 IRQ_TYPE_LEVEL_HIGH
+                                0x1000 0 0 2 &gic 0 108 IRQ_TYPE_LEVEL_HIGH>;
+       };

Hmm, this device node is not actually compliant to the PCI binding,
it needs a "ranges" property that can be used to look up the memory
and I/O space windows.

     The PCI driver doesn't support I/O space.

Well, whichever windows are registered by the driver should come
from the ranges property.

   I know. Too bad the driver's original author managed to misunderstand that...

It also needs a device_type property.

     Hm, are you sure about that? I thought only PCI devices should have it...

Yes, pretty sure it's needed in both the host controller and the
devices.

I don't understand the case of the PCI devices, honestly. Shouldn't the "device_type" prop reflect the device's functionality rather than the bus where it's located?

I realize that the driver doesn't currently use them, but you should
adhere to the binding anyway, so we can fix the driver at some point.

     Sigh, agreed about the need to fix the driver. Too bad you've spoken up
only now though. And you've ACKed the DT bindings without those properties
documented or even present in an example...

Yes, I realized that too late as well, sorry about it. For some reason
I only looked at the interrupt-map being done right and forgot to
check the ranges.

We definitely need to move the code handling the ranges into a common
location to avoid this mistake in the future.

It is already in the common location, AFAIK; what's lacking there is the code that parses "dma-ranges" as well but that should be pretty easy to add...

	Arnd

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux