pci and pcie device-tree binding - range No cells

pci and pcie device-tree binding - range No cells

[Michal Simek]

Hi Grant and others,

I have a question regarding number of cells in ranges property
for pci and pcie nodes.

Linux pci/pcie powerpc DTSes contain 7 cells (xpedite5370.dts, sequoia.dts, etc)
but also 6 cells format too (mpc832x_mds.dts)

Here is shown 6 cells ranges format and describe
http://devicetree.org/Device_Tree_Usage#PCI_Host_Bridge

And also in documentation in the linux
Documentation/devicetree/bindings/pci/83xx-512x-pci.txt

Both format uses:
#size-cells = <2>;
#address-cells = <3>;

What is valid format?

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

Re: pci and pcie device-tree binding - range No cells

[Rob Herring]

On 12/10/2012 06:20 AM, Michal Simek wrote:
> Hi Grant and others,
> 
> I have a question regarding number of cells in ranges property
> for pci and pcie nodes.
> 
> Linux pci/pcie powerpc DTSes contain 7 cells (xpedite5370.dts,
> sequoia.dts, etc)
> but also 6 cells format too (mpc832x_mds.dts)
> 
> Here is shown 6 cells ranges format and describe
> http://devicetree.org/Device_Tree_Usage#PCI_Host_Bridge
> 
> And also in documentation in the linux
> Documentation/devicetree/bindings/pci/83xx-512x-pci.txt
> 
> Both format uses:
> #size-cells = <2>;
> #address-cells = <3>;
> 
> What is valid format?

Both. 7 cells are valid when the host (parent) bus is 64-bit and 6 cells
are valid when the host bus is 32-bit. The ranges property is <<child
address> <parent address> <size>>. The parent address #address-cells is
taken from the parent node.

Rob

Re: pci and pcie device-tree binding - range No cells

[Michal Simek]

On 12/10/2012 03:26 PM, Rob Herring wrote:
> On 12/10/2012 06:20 AM, Michal Simek wrote:
>> Hi Grant and others,
>>
>> I have a question regarding number of cells in ranges property
>> for pci and pcie nodes.
>>
>> Linux pci/pcie powerpc DTSes contain 7 cells (xpedite5370.dts,
>> sequoia.dts, etc)
>> but also 6 cells format too (mpc832x_mds.dts)
>>
>> Here is shown 6 cells ranges format and describe
>> http://devicetree.org/Device_Tree_Usage#PCI_Host_Bridge
>>
>> And also in documentation in the linux
>> Documentation/devicetree/bindings/pci/83xx-512x-pci.txt
>>
>> Both format uses:
>> #size-cells = <2>;
>> #address-cells = <3>;
>>
>> What is valid format?
>
> Both. 7 cells are valid when the host (parent) bus is 64-bit and 6 cells
> are valid when the host bus is 32-bit. The ranges property is <<child
> address> <parent address> <size>>. The parent address #address-cells is
> taken from the parent node.

Ok. Got it.

Here is what we use on zynq and microblaze - both 32bit which should be fine.

	ps7_axi_interconnect_0: axi@0 {
		#address-cells = <1>;
		#size-cells = <1>;
		axi_pcie_0: axi-pcie@50000000 {
			#address-cells = <3>;
			#size-cells = <2>;
			compatible = "xlnx,axi-pcie-1.05.a";
			ranges = < 0x02000000 0 0x60000000 0x60000000 0 0x10000000 >;
			...
		}
	}

What I am wondering is pci_process_bridge_OF_ranges() at arch/powerpc/kernel/pci-common.c
where there are used some hardcoded values which should be probably loaded from device-tree.

For example:
683         int np = pna + 5;
...
702                 pci_addr = of_read_number(ranges + 1, 2);
703                 cpu_addr = of_translate_address(dev, ranges + 3);
704                 size = of_read_number(ranges + pna + 3, 2);


Unfortunately we have copied it to microblaze.

Thanks,
Michal






-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

Re: pci and pcie device-tree binding - range No cells

[Rob Herring]

On 12/10/2012 09:05 AM, Michal Simek wrote:
> On 12/10/2012 03:26 PM, Rob Herring wrote:
>> On 12/10/2012 06:20 AM, Michal Simek wrote:
>>> Hi Grant and others,
>>>
>>> I have a question regarding number of cells in ranges property
>>> for pci and pcie nodes.
>>>
>>> Linux pci/pcie powerpc DTSes contain 7 cells (xpedite5370.dts,
>>> sequoia.dts, etc)
>>> but also 6 cells format too (mpc832x_mds.dts)
>>>
>>> Here is shown 6 cells ranges format and describe
>>> http://devicetree.org/Device_Tree_Usage#PCI_Host_Bridge
>>>
>>> And also in documentation in the linux
>>> Documentation/devicetree/bindings/pci/83xx-512x-pci.txt
>>>
>>> Both format uses:
>>> #size-cells = <2>;
>>> #address-cells = <3>;
>>>
>>> What is valid format?
>>
>> Both. 7 cells are valid when the host (parent) bus is 64-bit and 6 cells
>> are valid when the host bus is 32-bit. The ranges property is <<child
>> address> <parent address> <size>>. The parent address #address-cells is
>> taken from the parent node.
> 
> Ok. Got it.
> 
> Here is what we use on zynq and microblaze - both 32bit which should be
> fine.
> 
>     ps7_axi_interconnect_0: axi@0 {
>         #address-cells = <1>;
>         #size-cells = <1>;
>         axi_pcie_0: axi-pcie@50000000 {
>             #address-cells = <3>;
>             #size-cells = <2>;
>             compatible = "xlnx,axi-pcie-1.05.a";
>             ranges = < 0x02000000 0 0x60000000 0x60000000 0 0x10000000 >;
>             ...
>         }
>     }
> 
> What I am wondering is pci_process_bridge_OF_ranges() at
> arch/powerpc/kernel/pci-common.c
> where there are used some hardcoded values which should be probably
> loaded from device-tree.
> 
> For example:
> 683         int np = pna + 5;
> ...
> 702                 pci_addr = of_read_number(ranges + 1, 2);
> 703                 cpu_addr = of_translate_address(dev, ranges + 3);
> 704                 size = of_read_number(ranges + pna + 3, 2);

These would always be correct whether you have 6 or 7 cells. pna is the
parent bus address cells size. The pci address is fixed at 3 cells.

> 
> 
> Unfortunately we have copied it to microblaze.

I look at the PCI DT code in powerpc and see a whole bunch of code that
seems like it should be common. The different per arch pci structs
complicates that. No one has really gotten to looking at PCI DT on ARM
yet except you and Thierry for Tegra. We definitely don't want to create
a 3rd copy. Starting the process of moving it to something like
drivers/pci/pci-of.c would be great.

Rob

> 
> Thanks,
> Michal
> 
> 
> 
> 
> 
> 

Re: pci and pcie device-tree binding - range No cells

[Michal Simek]

On 12/10/2012 04:21 PM, Rob Herring wrote:
> On 12/10/2012 09:05 AM, Michal Simek wrote:
>> On 12/10/2012 03:26 PM, Rob Herring wrote:
>>> On 12/10/2012 06:20 AM, Michal Simek wrote:
>>>> Hi Grant and others,
>>>>
>>>> I have a question regarding number of cells in ranges property
>>>> for pci and pcie nodes.
>>>>
>>>> Linux pci/pcie powerpc DTSes contain 7 cells (xpedite5370.dts,
>>>> sequoia.dts, etc)
>>>> but also 6 cells format too (mpc832x_mds.dts)
>>>>
>>>> Here is shown 6 cells ranges format and describe
>>>> http://devicetree.org/Device_Tree_Usage#PCI_Host_Bridge
>>>>
>>>> And also in documentation in the linux
>>>> Documentation/devicetree/bindings/pci/83xx-512x-pci.txt
>>>>
>>>> Both format uses:
>>>> #size-cells = <2>;
>>>> #address-cells = <3>;
>>>>
>>>> What is valid format?
>>>
>>> Both. 7 cells are valid when the host (parent) bus is 64-bit and 6 cells
>>> are valid when the host bus is 32-bit. The ranges property is <<child
>>> address> <parent address> <size>>. The parent address #address-cells is
>>> taken from the parent node.
>>
>> Ok. Got it.
>>
>> Here is what we use on zynq and microblaze - both 32bit which should be
>> fine.
>>
>>      ps7_axi_interconnect_0: axi@0 {
>>          #address-cells = <1>;
>>          #size-cells = <1>;
>>          axi_pcie_0: axi-pcie@50000000 {
>>              #address-cells = <3>;
>>              #size-cells = <2>;
>>              compatible = "xlnx,axi-pcie-1.05.a";
>>              ranges = < 0x02000000 0 0x60000000 0x60000000 0 0x10000000 >;
>>              ...
>>          }
>>      }
>>
>> What I am wondering is pci_process_bridge_OF_ranges() at
>> arch/powerpc/kernel/pci-common.c
>> where there are used some hardcoded values which should be probably
>> loaded from device-tree.
>>
>> For example:
>> 683         int np = pna + 5;
>> ...
>> 702                 pci_addr = of_read_number(ranges + 1, 2);
>> 703                 cpu_addr = of_translate_address(dev, ranges + 3);
>> 704                 size = of_read_number(ranges + pna + 3, 2);
>
> These would always be correct whether you have 6 or 7 cells. pna is the
> parent bus address cells size. The pci address is fixed at 3 cells.

Sorry for my pci ignorance (have never got hw for mb/zynq)
I just want to get better overview how we should we our drivers to be compatible.

Does it mean that pci is supposed be always 64 bit wide?
And there is no option to have just 32bit values.

>> Unfortunately we have copied it to microblaze.
>
> I look at the PCI DT code in powerpc and see a whole bunch of code that
> seems like it should be common. The different per arch pci structs
> complicates that. No one has really gotten to looking at PCI DT on ARM
> yet except you and Thierry for Tegra. We definitely don't want to create
> a 3rd copy. Starting the process of moving it to something like
> drivers/pci/pci-of.c would be great.

We have done pcie working on zynq and the same pcie host is working on Microblaze too.
There are just small differences. That's why I have sent another email
("Sharing PCIE driver between Microblaze and Arm zynq") to find out proper location.

PCI: Microblaze shares almost the same code with powerpc that's why I am definitely open
to move this code out of architecture.

Thanks,
Michal



-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

RE: pci and pcie device-tree binding - range No cells

[David Laight]

> Does it mean that pci is supposed be always 64 bit wide?
> And there is no option to have just 32bit values.

I certainly believe that all PCIe (not PCI) transfers are
nominally multiples of 64bit data.
There are (effectively) 8 byte-lane enables to allow partial word
transfers (I'm not sure whether disjoint subwords are valid).

Some PCIe slave->32bit bus bridges always do two slave cycles.
Even though, for a 32bit word transfer, one of them ends up having
no byte enables asserted.

	David

Re: pci and pcie device-tree binding - range No cells

[Michal Simek]

On 12/10/2012 04:52 PM, David Laight wrote:
>> Does it mean that pci is supposed be always 64 bit wide?
>> And there is no option to have just 32bit values.
>
> I certainly believe that all PCIe (not PCI) transfers are
> nominally multiples of 64bit data.

And PCI? That powerpc/pci-common code was designed for PCI
and microblaze also used it for PCI.

CC: Thomas: I think it will be interesting to see this discussion
because you are using size-cell/address-cells equal 1.
http://www.spinics.net/lists/arm-kernel/msg211839.html

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

Re: pci and pcie device-tree binding - range No cells

[Thomas Petazzoni]

Dear Michal Simek,

On Mon, 10 Dec 2012 17:05:13 +0100, Michal Simek wrote:

> CC: Thomas: I think it will be interesting to see this discussion
> because you are using size-cell/address-cells equal 1.
> http://www.spinics.net/lists/arm-kernel/msg211839.html

Thanks for Cc'ing me.

The thing is that on Marvell SoCs, we don't need to describe statically
in the Device Tree the translation between CPU addresses and PCI device
addresses, because those translations are set up dynamically at run
time through address decoding windows.

Marvell SoCs have up to 20 configurable address windows, which allow
you, at run time, to say "I would like the range from physical address
0xYYYYYYYY to 0xZZZZZZZZ to correspond to the PCIe device in port 1,
lane 2, or to the NAND, or to this or that device". Therefore, in the
PCIe driver I proposed for the Armada 370/XP SoCs [1], there is no need
to encode all those ranges statically in the DT.

The only "ranges" property I'm using is to allow the DT sub-nodes
describing each PCIe port/lane to access the CPU registers that allow
to see if the PCIe link is up or down, access the PCI configuration
space and so on. So all ranges in my "ranges" property correspond to
normal CPU registers, like the one you would put in the "reg" property
for any device. The fact that those devices are PCIe is really
orthogonal here.

Of course, I have no idea if I'm doing a correct usage of the DT, but I
certainly don't need those 6 values ranges with bits to say if it's I/O
space, memory space, bus number, device number and so on. The physical
addresses at which I'm setting up my address decoding windows are
decided dynamically at runtime, depending on the number of PCIe devices
that are found in the different PCIe slots (for now, those windows have
a statically defined size, but I'd ideally would like to size them to
match exactly the size of the PCIe device memory, in order to avoid
wasting physical address space, but I haven't found how to get the size
needed for each PCIe device during the ARM pcibios initialization
sequence).

For those willing to have a look at the PCIe patch set for Armada
370/XP, see:
http://lists.infradead.org/pipermail/linux-arm-kernel/2012-December/136455.html.
The core of the PCIe driver and its DT binding documentation is at
http://lists.infradead.org/pipermail/linux-arm-kernel/2012-December/136711.html.

Thanks a lot for your comments,

Thomas

[1] that I hope to extend to cover previous Marvell SoCs as well, they
    work basically the same way
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: pci and pcie device-tree binding - range No cells

[Rob Herring]

On 12/10/2012 11:15 AM, Thomas Petazzoni wrote:
> Dear Michal Simek,
> 
> On Mon, 10 Dec 2012 17:05:13 +0100, Michal Simek wrote:
> 
>> CC: Thomas: I think it will be interesting to see this discussion
>> because you are using size-cell/address-cells equal 1.
>> http://www.spinics.net/lists/arm-kernel/msg211839.html
> 
> Thanks for Cc'ing me.
> 
> The thing is that on Marvell SoCs, we don't need to describe statically
> in the Device Tree the translation between CPU addresses and PCI device
> addresses, because those translations are set up dynamically at run
> time through address decoding windows.
> 
> Marvell SoCs have up to 20 configurable address windows, which allow
> you, at run time, to say "I would like the range from physical address
> 0xYYYYYYYY to 0xZZZZZZZZ to correspond to the PCIe device in port 1,
> lane 2, or to the NAND, or to this or that device". Therefore, in the
> PCIe driver I proposed for the Armada 370/XP SoCs [1], there is no need
> to encode all those ranges statically in the DT.

That's not a unique feature. I'm not sure if any powerpc systems do that
though.

> The only "ranges" property I'm using is to allow the DT sub-nodes
> describing each PCIe port/lane to access the CPU registers that allow
> to see if the PCIe link is up or down, access the PCI configuration
> space and so on. So all ranges in my "ranges" property correspond to
> normal CPU registers, like the one you would put in the "reg" property
> for any device. The fact that those devices are PCIe is really
> orthogonal here.

That doesn't really sound right.

> Of course, I have no idea if I'm doing a correct usage of the DT, but I
> certainly don't need those 6 values ranges with bits to say if it's I/O
> space, memory space, bus number, device number and so on. The physical
> addresses at which I'm setting up my address decoding windows are
> decided dynamically at runtime, depending on the number of PCIe devices
> that are found in the different PCIe slots (for now, those windows have
> a statically defined size, but I'd ideally would like to size them to
> match exactly the size of the PCIe device memory, in order to avoid
> wasting physical address space, but I haven't found how to get the size
> needed for each PCIe device during the ARM pcibios initialization
> sequence).

I don't think deviating from the normal binding is the right approach.
Perhaps the host driver should fill in the ranges property with the
addresses it uses. Then any child devices will get the right address
translation.

Also, while the h/w may support practically any config, there are
practical constraints of what Linux will use like there's no reason to
support more than 64K i/o space. PCI memory addresses generally start at
0x100000. You probably don't need more than 1 memory window per root
complex (although prefetchable memory may also be needed).

You could let the DT settings drive the address window configuration.

Rob

> For those willing to have a look at the PCIe patch set for Armada
> 370/XP, see:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2012-December/136455.html.
> The core of the PCIe driver and its DT binding documentation is at
> http://lists.infradead.org/pipermail/linux-arm-kernel/2012-December/136711.html.
> 
> Thanks a lot for your comments,
> 
> Thomas
> 
> [1] that I hope to extend to cover previous Marvell SoCs as well, they
>     work basically the same way
> 

Re: pci and pcie device-tree binding - range No cells

[Thomas Petazzoni]

Dear Rob Herring,

On Mon, 10 Dec 2012 17:24:44 -0600, Rob Herring wrote:

> > Marvell SoCs have up to 20 configurable address windows, which allow
> > you, at run time, to say "I would like the range from physical
> > address 0xYYYYYYYY to 0xZZZZZZZZ to correspond to the PCIe device
> > in port 1, lane 2, or to the NAND, or to this or that device".
> > Therefore, in the PCIe driver I proposed for the Armada 370/XP SoCs
> > [1], there is no need to encode all those ranges statically in the
> > DT.
> 
> That's not a unique feature. I'm not sure if any powerpc systems do
> that though.

Yes, probably not an unique feature.

> > The only "ranges" property I'm using is to allow the DT sub-nodes
> > describing each PCIe port/lane to access the CPU registers that
> > allow to see if the PCIe link is up or down, access the PCI
> > configuration space and so on. So all ranges in my "ranges"
> > property correspond to normal CPU registers, like the one you would
> > put in the "reg" property for any device. The fact that those
> > devices are PCIe is really orthogonal here.
> 
> That doesn't really sound right.

Very likely, but I still don't get what is "the right way".

> I don't think deviating from the normal binding is the right approach.
> Perhaps the host driver should fill in the ranges property with the
> addresses it uses. Then any child devices will get the right address
> translation.

I don't really understand what you mean here. If you look at the host
driver code (arch/arm/mach-mvebu/pcie.c), for each PCIe interface
is simply does:

 * Create an address decoding window for the memory BAR
 * Create an address decoding window for the I/O BAR
 * Associate the memory BAR window address and the I/O bar window
   address with the PCIe interface

And that's it. See
https://github.com/MISL-EBU-System-SW/mainline-public/blob/marvell-pcie-v1/arch/arm/mach-mvebu/pcie.c#L107.

So this driver is both "deciding" of the physical addresses for each
PCIe interface, and "associating" them with the PCIe interfaces. How is
it useful to feed some addresses back into the Device Tree?

> Also, while the h/w may support practically any config, there are
> practical constraints of what Linux will use like there's no reason to
> support more than 64K i/o space. PCI memory addresses generally start
> at 0x100000. You probably don't need more than 1 memory window per
> root complex (although prefetchable memory may also be needed).

I allocate one 64K I/O window and one memory window per PCIe interface
whose link is up (i.e a PCIe device is connected).

> You could let the DT settings drive the address window configuration.

No, because I don't want to have absolute addresses for the windows: I
have 10 PCIe interfaces, but often, only a few of them are used. So I
don't want in the Device Tree to over-allocate hundreds of MB of
physical address space if it's not useful.

PCIe is dynamic, address window configuration is dynamic. And we should
hardcode all this configuration statically in the DT? Doesn't seem like
the right solution.

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: pci and pcie device-tree binding - range No cells

[Grant Likely]

On Wed, Dec 12, 2012 at 4:16 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Rob Herring,
>
> On Mon, 10 Dec 2012 17:24:44 -0600, Rob Herring wrote:
>
>> > Marvell SoCs have up to 20 configurable address windows, which allow
>> > you, at run time, to say "I would like the range from physical
>> > address 0xYYYYYYYY to 0xZZZZZZZZ to correspond to the PCIe device
>> > in port 1, lane 2, or to the NAND, or to this or that device".
>> > Therefore, in the PCIe driver I proposed for the Armada 370/XP SoCs
>> > [1], there is no need to encode all those ranges statically in the
>> > DT.
>>
>> That's not a unique feature. I'm not sure if any powerpc systems do
>> that though.
>
> Yes, probably not an unique feature.
>
>> > The only "ranges" property I'm using is to allow the DT sub-nodes
>> > describing each PCIe port/lane to access the CPU registers that
>> > allow to see if the PCIe link is up or down, access the PCI
>> > configuration space and so on. So all ranges in my "ranges"
>> > property correspond to normal CPU registers, like the one you would
>> > put in the "reg" property for any device. The fact that those
>> > devices are PCIe is really orthogonal here.
>>
>> That doesn't really sound right.
>
> Very likely, but I still don't get what is "the right way".

Hi Thomas,

I just went and looked at your binding. Here's the snippet I found interesting:

pcie-controller {
+ compatible = "marvell,armada-370-xp-pcie";
+ status = "disabled";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0       0xd0040000 0x2000 /* port0x1_port0 */
+  0x2000  0xd0042000 0x2000 /* port2x1_port0 */
+  0x4000  0xd0044000 0x2000 /* port0x1_port1 */
+  0x8000  0xd0048000 0x2000 /* port0x1_port2 */
+  0xC000  0xd004C000 0x2000 /* port0x1_port3 */
+  0x10000 0xd0080000 0x2000 /* port1x1_port0 */
+  0x12000 0xd0082000 0x2000 /* port3x1_port0 */
+  0x14000 0xd0084000 0x2000 /* port1x1_port1 */
+  0x18000 0xd0088000 0x2000 /* port1x1_port2 */
+  0x1C000 0xd008C000 0x2000 /* port1x1_port3 */>;
+
+ pcie0.0 at 0xd0040000 {
+ reg = <0x0 0x2000>;
+ interrupts = <58>;
+ clocks = <&gateclk 5>;
+ marvell,pcie-port = <0>;
+ marvell,pcie-lane = <0>;
+ status = "disabled";
+ };
+
+ pcie0.1 at 0xd0044000 {
+ reg = <0x4000 0x2000>;
+ interrupts = <59>;
+ clocks = <&gateclk 5>;
+ marvell,pcie-port = <0>;
+ marvell,pcie-lane = <1>;
+ status = "disabled";
+ };
[... rest trimmed for berevity]

You're right, if you're doing dynamic allocation of windows, then you
really don't need to have a ranges property. However, the way the PCI
node is set up definitely looks incorrect.

PCI already has a very specific binding for pci host controller nodes.
First, #address-cells=<3>; #size-cells=<2>; and device_type="pcie"
must be there. You don't want to break this. You can find details on
the pci and pci-express binding here:
http://www.openfirmware.org/1275/bindings/pci/pci2_1.pdf
http://www.openfirmware.org/1275/bindings/pci/pci-express.txt

For the child nodes, PCI is a discoverable bus, so normally I wouldn't
expect to see child nodes at all when using a dtb. The only time nodes
should be populated is when a device has non-discoverable
charactersitics. In your example above you do have some additional
data, but I don't know enough about pci-express to say how best to
encode them or whether they are needed at all. Ben might have some
thoughts on this.

When the PCI child nodes are present, it is important to stick with
the established PCI addressing scheme which uses 3 cells for
addressing. The first entry in the reg property must represent the
configuration space so that DT nodes can be matched up with discovered
devices. There is no requirement to include mappings for the memory
and io regions if the host controller can assign them dynamically.

I don't think you should need a ranges property at all for what you're
doing. Access to config space is generally managed by the PCI host
controller drivers and subsystem, and PCI device drivers don't
typically use of_ calls directly.

g.

Re: pci and pcie device-tree binding - range No cells

[Rob Herring]

On 12/12/2012 10:16 AM, Thomas Petazzoni wrote:
> Dear Rob Herring,
> 
> On Mon, 10 Dec 2012 17:24:44 -0600, Rob Herring wrote:
> 
>>> Marvell SoCs have up to 20 configurable address windows, which allow
>>> you, at run time, to say "I would like the range from physical
>>> address 0xYYYYYYYY to 0xZZZZZZZZ to correspond to the PCIe device
>>> in port 1, lane 2, or to the NAND, or to this or that device".
>>> Therefore, in the PCIe driver I proposed for the Armada 370/XP SoCs
>>> [1], there is no need to encode all those ranges statically in the
>>> DT.
>>
>> That's not a unique feature. I'm not sure if any powerpc systems do
>> that though.
> 
> Yes, probably not an unique feature.
> 
>>> The only "ranges" property I'm using is to allow the DT sub-nodes
>>> describing each PCIe port/lane to access the CPU registers that
>>> allow to see if the PCIe link is up or down, access the PCI
>>> configuration space and so on. So all ranges in my "ranges"
>>> property correspond to normal CPU registers, like the one you would
>>> put in the "reg" property for any device. The fact that those
>>> devices are PCIe is really orthogonal here.
>>
>> That doesn't really sound right.
> 
> Very likely, but I still don't get what is "the right way".
> 
>> I don't think deviating from the normal binding is the right approach.
>> Perhaps the host driver should fill in the ranges property with the
>> addresses it uses. Then any child devices will get the right address
>> translation.
> 
> I don't really understand what you mean here. If you look at the host
> driver code (arch/arm/mach-mvebu/pcie.c), for each PCIe interface
> is simply does:
> 
>  * Create an address decoding window for the memory BAR
>  * Create an address decoding window for the I/O BAR
>  * Associate the memory BAR window address and the I/O bar window
>    address with the PCIe interface
> 
> And that's it. See
> https://github.com/MISL-EBU-System-SW/mainline-public/blob/marvell-pcie-v1/arch/arm/mach-mvebu/pcie.c#L107.
> 
> So this driver is both "deciding" of the physical addresses for each
> PCIe interface, and "associating" them with the PCIe interfaces. How is
> it useful to feed some addresses back into the Device Tree?

I'm not completely sure for PCI, but the ranges is necessary to
translate addresses of child nodes.

If you don't need ranges then you could omit it. If you need ranges,
then you should follow the PCI binding whether it is put in the DTS or
you dynamically fill it in. This could be filled in by the bootloader as
well if you have PCI devices you need to boot from.

>> Also, while the h/w may support practically any config, there are
>> practical constraints of what Linux will use like there's no reason to
>> support more than 64K i/o space. PCI memory addresses generally start
>> at 0x100000. You probably don't need more than 1 memory window per
>> root complex (although prefetchable memory may also be needed).
> 
> I allocate one 64K I/O window and one memory window per PCIe interface
> whose link is up (i.e a PCIe device is connected).
> 
>> You could let the DT settings drive the address window configuration.
> 
> No, because I don't want to have absolute addresses for the windows: I
> have 10 PCIe interfaces, but often, only a few of them are used. So I
> don't want in the Device Tree to over-allocate hundreds of MB of
> physical address space if it's not useful.

How many you have is probably board dependent and not probe-able, right?
So you would at least know the subset of root complexes that you are
using. I know you want to find the size of all the cards up front and
size windows based on that, but I don't think that is going to be possible.

> 
> PCIe is dynamic, address window configuration is dynamic. And we should
> hardcode all this configuration statically in the DT? Doesn't seem like
> the right solution.

I'm just throwing out ideas. There are many cases of flexibility in h/w
designs which are never used. H/w is often designed in a vacuum without
s/w input. Not saying that is the case here, but you do have to consider
that.

Rob

> 
> Best regards,
> 
> Thomas
> 

Re: pci and pcie device-tree binding - range No cells

[Rob Herring]

On 12/10/2012 09:37 AM, Michal Simek wrote:
> On 12/10/2012 04:21 PM, Rob Herring wrote:
>> On 12/10/2012 09:05 AM, Michal Simek wrote:
>>> On 12/10/2012 03:26 PM, Rob Herring wrote:
>>>> On 12/10/2012 06:20 AM, Michal Simek wrote:
>>>>> Hi Grant and others,
>>>>>
>>>>> I have a question regarding number of cells in ranges property
>>>>> for pci and pcie nodes.
>>>>>
>>>>> Linux pci/pcie powerpc DTSes contain 7 cells (xpedite5370.dts,
>>>>> sequoia.dts, etc)
>>>>> but also 6 cells format too (mpc832x_mds.dts)
>>>>>
>>>>> Here is shown 6 cells ranges format and describe
>>>>> http://devicetree.org/Device_Tree_Usage#PCI_Host_Bridge
>>>>>
>>>>> And also in documentation in the linux
>>>>> Documentation/devicetree/bindings/pci/83xx-512x-pci.txt
>>>>>
>>>>> Both format uses:
>>>>> #size-cells = <2>;
>>>>> #address-cells = <3>;
>>>>>
>>>>> What is valid format?
>>>>
>>>> Both. 7 cells are valid when the host (parent) bus is 64-bit and 6
>>>> cells
>>>> are valid when the host bus is 32-bit. The ranges property is <<child
>>>> address> <parent address> <size>>. The parent address #address-cells is
>>>> taken from the parent node.
>>>
>>> Ok. Got it.
>>>
>>> Here is what we use on zynq and microblaze - both 32bit which should be
>>> fine.
>>>
>>>      ps7_axi_interconnect_0: axi@0 {
>>>          #address-cells = <1>;
>>>          #size-cells = <1>;
>>>          axi_pcie_0: axi-pcie@50000000 {
>>>              #address-cells = <3>;
>>>              #size-cells = <2>;
>>>              compatible = "xlnx,axi-pcie-1.05.a";
>>>              ranges = < 0x02000000 0 0x60000000 0x60000000 0
>>> 0x10000000 >;
>>>              ...
>>>          }
>>>      }
>>>
>>> What I am wondering is pci_process_bridge_OF_ranges() at
>>> arch/powerpc/kernel/pci-common.c
>>> where there are used some hardcoded values which should be probably
>>> loaded from device-tree.
>>>
>>> For example:
>>> 683         int np = pna + 5;
>>> ...
>>> 702                 pci_addr = of_read_number(ranges + 1, 2);
>>> 703                 cpu_addr = of_translate_address(dev, ranges + 3);
>>> 704                 size = of_read_number(ranges + pna + 3, 2);
>>
>> These would always be correct whether you have 6 or 7 cells. pna is the
>> parent bus address cells size. The pci address is fixed at 3 cells.
> 
> Sorry for my pci ignorance (have never got hw for mb/zynq)
> I just want to get better overview how we should we our drivers to be
> compatible.
> 
> Does it mean that pci is supposed be always 64 bit wide?
> And there is no option to have just 32bit values.

That's what the DT documentation says.

BTW, you can use 2 cells even if you only have a 32-bit bus.

> 
>>> Unfortunately we have copied it to microblaze.
>>
>> I look at the PCI DT code in powerpc and see a whole bunch of code that
>> seems like it should be common. The different per arch pci structs
>> complicates that. No one has really gotten to looking at PCI DT on ARM
>> yet except you and Thierry for Tegra. We definitely don't want to create
>> a 3rd copy. Starting the process of moving it to something like
>> drivers/pci/pci-of.c would be great.
> 
> We have done pcie working on zynq and the same pcie host is working on
> Microblaze too.
> There are just small differences. That's why I have sent another email
> ("Sharing PCIE driver between Microblaze and Arm zynq") to find out
> proper location.

Yes, I've followed the thread. There are lots of areas for consolidation
with PCIe hosts both across architectures and within ARM. There are
multiple people using DW PCIe IP although not upstream yet that I'm
aware of.

Rob

> 
> PCI: Microblaze shares almost the same code with powerpc that's why I am
> definitely open
> to move this code out of architecture.
> 
> Thanks,
> Michal
> 
> 
> 

Re: pci and pcie device-tree binding - range No cells

[Michal Simek]

On 12/10/2012 05:02 PM, Rob Herring wrote:
> On 12/10/2012 09:37 AM, Michal Simek wrote:
>> On 12/10/2012 04:21 PM, Rob Herring wrote:
>>> On 12/10/2012 09:05 AM, Michal Simek wrote:
>>>> On 12/10/2012 03:26 PM, Rob Herring wrote:
>>>>> On 12/10/2012 06:20 AM, Michal Simek wrote:
>>>>>> Hi Grant and others,
>>>>>>
>>>>>> I have a question regarding number of cells in ranges property
>>>>>> for pci and pcie nodes.
>>>>>>
>>>>>> Linux pci/pcie powerpc DTSes contain 7 cells (xpedite5370.dts,
>>>>>> sequoia.dts, etc)
>>>>>> but also 6 cells format too (mpc832x_mds.dts)
>>>>>>
>>>>>> Here is shown 6 cells ranges format and describe
>>>>>> http://devicetree.org/Device_Tree_Usage#PCI_Host_Bridge
>>>>>>
>>>>>> And also in documentation in the linux
>>>>>> Documentation/devicetree/bindings/pci/83xx-512x-pci.txt
>>>>>>
>>>>>> Both format uses:
>>>>>> #size-cells = <2>;
>>>>>> #address-cells = <3>;
>>>>>>
>>>>>> What is valid format?
>>>>>
>>>>> Both. 7 cells are valid when the host (parent) bus is 64-bit and 6
>>>>> cells
>>>>> are valid when the host bus is 32-bit. The ranges property is <<child
>>>>> address> <parent address> <size>>. The parent address #address-cells is
>>>>> taken from the parent node.
>>>>
>>>> Ok. Got it.
>>>>
>>>> Here is what we use on zynq and microblaze - both 32bit which should be
>>>> fine.
>>>>
>>>>       ps7_axi_interconnect_0: axi@0 {
>>>>           #address-cells = <1>;
>>>>           #size-cells = <1>;
>>>>           axi_pcie_0: axi-pcie@50000000 {
>>>>               #address-cells = <3>;
>>>>               #size-cells = <2>;
>>>>               compatible = "xlnx,axi-pcie-1.05.a";
>>>>               ranges = < 0x02000000 0 0x60000000 0x60000000 0
>>>> 0x10000000 >;
>>>>               ...
>>>>           }
>>>>       }
>>>>
>>>> What I am wondering is pci_process_bridge_OF_ranges() at
>>>> arch/powerpc/kernel/pci-common.c
>>>> where there are used some hardcoded values which should be probably
>>>> loaded from device-tree.
>>>>
>>>> For example:
>>>> 683         int np = pna + 5;
>>>> ...
>>>> 702                 pci_addr = of_read_number(ranges + 1, 2);
>>>> 703                 cpu_addr = of_translate_address(dev, ranges + 3);
>>>> 704                 size = of_read_number(ranges + pna + 3, 2);
>>>
>>> These would always be correct whether you have 6 or 7 cells. pna is the
>>> parent bus address cells size. The pci address is fixed at 3 cells.
>>
>> Sorry for my pci ignorance (have never got hw for mb/zynq)
>> I just want to get better overview how we should we our drivers to be
>> compatible.
>>
>> Does it mean that pci is supposed be always 64 bit wide?
>> And there is no option to have just 32bit values.
>
> That's what the DT documentation says.
>
> BTW, you can use 2 cells even if you only have a 32-bit bus.

ok - no problem with that. Just want to be sure about it.

>>>> Unfortunately we have copied it to microblaze.
>>>
>>> I look at the PCI DT code in powerpc and see a whole bunch of code that
>>> seems like it should be common. The different per arch pci structs
>>> complicates that. No one has really gotten to looking at PCI DT on ARM
>>> yet except you and Thierry for Tegra. We definitely don't want to create
>>> a 3rd copy. Starting the process of moving it to something like
>>> drivers/pci/pci-of.c would be great.
>>
>> We have done pcie working on zynq and the same pcie host is working on
>> Microblaze too.
>> There are just small differences. That's why I have sent another email
>> ("Sharing PCIE driver between Microblaze and Arm zynq") to find out
>> proper location.
>
> Yes, I've followed the thread. There are lots of areas for consolidation
> with PCIe hosts both across architectures and within ARM. There are
> multiple people using DW PCIe IP although not upstream yet that I'm
> aware of.

Your suggestions are really appreciate.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

Re: pci and pcie device-tree binding - range No cells

[Grant Likely]

On Mon, 10 Dec 2012 16:37:26 +0100, Michal Simek <monstr@monstr.eu> wrote:
> On 12/10/2012 04:21 PM, Rob Herring wrote:
> > On 12/10/2012 09:05 AM, Michal Simek wrote:
> >> On 12/10/2012 03:26 PM, Rob Herring wrote:
> >>> On 12/10/2012 06:20 AM, Michal Simek wrote:
> >>>> Hi Grant and others,
> >>>>
> >>>> I have a question regarding number of cells in ranges property
> >>>> for pci and pcie nodes.
> >>>>
> >>>> Linux pci/pcie powerpc DTSes contain 7 cells (xpedite5370.dts,
> >>>> sequoia.dts, etc)
> >>>> but also 6 cells format too (mpc832x_mds.dts)
> >>>>
> >>>> Here is shown 6 cells ranges format and describe
> >>>> http://devicetree.org/Device_Tree_Usage#PCI_Host_Bridge
> >>>>
> >>>> And also in documentation in the linux
> >>>> Documentation/devicetree/bindings/pci/83xx-512x-pci.txt
> >>>>
> >>>> Both format uses:
> >>>> #size-cells = <2>;
> >>>> #address-cells = <3>;
> >>>>
> >>>> What is valid format?
> >>>
> >>> Both. 7 cells are valid when the host (parent) bus is 64-bit and 6 cells
> >>> are valid when the host bus is 32-bit. The ranges property is <<child
> >>> address> <parent address> <size>>. The parent address #address-cells is
> >>> taken from the parent node.
> >>
> >> Ok. Got it.
> >>
> >> Here is what we use on zynq and microblaze - both 32bit which should be
> >> fine.
> >>
> >>      ps7_axi_interconnect_0: axi@0 {
> >>          #address-cells = <1>;
> >>          #size-cells = <1>;
> >>          axi_pcie_0: axi-pcie@50000000 {
> >>              #address-cells = <3>;
> >>              #size-cells = <2>;
> >>              compatible = "xlnx,axi-pcie-1.05.a";
> >>              ranges = < 0x02000000 0 0x60000000 0x60000000 0 0x10000000 >;
> >>              ...
> >>          }
> >>      }
> >>
> >> What I am wondering is pci_process_bridge_OF_ranges() at
> >> arch/powerpc/kernel/pci-common.c
> >> where there are used some hardcoded values which should be probably
> >> loaded from device-tree.
> >>
> >> For example:
> >> 683         int np = pna + 5;
> >> ...
> >> 702                 pci_addr = of_read_number(ranges + 1, 2);
> >> 703                 cpu_addr = of_translate_address(dev, ranges + 3);
> >> 704                 size = of_read_number(ranges + pna + 3, 2);
> >
> > These would always be correct whether you have 6 or 7 cells. pna is the
> > parent bus address cells size. The pci address is fixed at 3 cells.
> 
> Sorry for my pci ignorance (have never got hw for mb/zynq)
> I just want to get better overview how we should we our drivers to be compatible.
> 
> Does it mean that pci is supposed be always 64 bit wide?
> And there is no option to have just 32bit values.

Yes, PCIe addressing is always 64 bits wide. Even on 32bit PCI systems
we use 64 bit PCI addressing in the device tree.

g.

Re: pci and pcie device-tree binding - range No cells

[Benjamin Herrenschmidt]

On Mon, 2012-12-10 at 21:43 +0000, Grant Likely wrote:
> > Sorry for my pci ignorance (have never got hw for mb/zynq)
> > I just want to get better overview how we should we our drivers to
> be compatible.
> > 
> > Does it mean that pci is supposed be always 64 bit wide?
> > And there is no option to have just 32bit values.
> 
> Yes, PCIe addressing is always 64 bits wide. Even on 32bit PCI systems
> we use 64 bit PCI addressing in the device tree.

Right. The size & format of an address cell for PCI is specified in the
OF PCI bindings and we follow that binding. It's always 3 cells.

Cheers,
Ben.

Re: pci and pcie device-tree binding - range No cells

[Mitch Bradley]

On 12/10/2012 12:38 PM, Benjamin Herrenschmidt wrote:
> On Mon, 2012-12-10 at 21:43 +0000, Grant Likely wrote:
>>> Sorry for my pci ignorance (have never got hw for mb/zynq)
>>> I just want to get better overview how we should we our drivers to
>> be compatible.
>>>
>>> Does it mean that pci is supposed be always 64 bit wide?
>>> And there is no option to have just 32bit values.
>>
>> Yes, PCIe addressing is always 64 bits wide. Even on 32bit PCI systems
>> we use 64 bit PCI addressing in the device tree.
> 
> Right. The size & format of an address cell for PCI is specified in the
> OF PCI bindings and we follow that binding. It's always 3 cells.

.. and the reason why it must be 3 cells, even if the host PCI bus only
supports 32-bit addressing, is because a plug-in PCI card has no way of
knowing what the host supports.


> 
> Cheers,
> Ben.
> 
> 
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss
> 

Re: pci and pcie device-tree binding - range No cells

[Grant Likely]

On Mon, 10 Dec 2012 09:21:51 -0600, Rob Herring <robherring2@gmail.com> wrote:
> On 12/10/2012 09:05 AM, Michal Simek wrote:
> > On 12/10/2012 03:26 PM, Rob Herring wrote:
> >> On 12/10/2012 06:20 AM, Michal Simek wrote:
> >>> Hi Grant and others,
> >>>
> >>> I have a question regarding number of cells in ranges property
> >>> for pci and pcie nodes.
> >>>
> >>> Linux pci/pcie powerpc DTSes contain 7 cells (xpedite5370.dts,
> >>> sequoia.dts, etc)
> >>> but also 6 cells format too (mpc832x_mds.dts)
> >>>
> >>> Here is shown 6 cells ranges format and describe
> >>> http://devicetree.org/Device_Tree_Usage#PCI_Host_Bridge
> >>>
> >>> And also in documentation in the linux
> >>> Documentation/devicetree/bindings/pci/83xx-512x-pci.txt
> >>>
> >>> Both format uses:
> >>> #size-cells = <2>;
> >>> #address-cells = <3>;
> >>>
> >>> What is valid format?
> >>
> >> Both. 7 cells are valid when the host (parent) bus is 64-bit and 6 cells
> >> are valid when the host bus is 32-bit. The ranges property is <<child
> >> address> <parent address> <size>>. The parent address #address-cells is
> >> taken from the parent node.
> > 
> > Ok. Got it.
> > 
> > Here is what we use on zynq and microblaze - both 32bit which should be
> > fine.
> > 
> >     ps7_axi_interconnect_0: axi@0 {
> >         #address-cells = <1>;
> >         #size-cells = <1>;
> >         axi_pcie_0: axi-pcie@50000000 {
> >             #address-cells = <3>;
> >             #size-cells = <2>;
> >             compatible = "xlnx,axi-pcie-1.05.a";
> >             ranges = < 0x02000000 0 0x60000000 0x60000000 0 0x10000000 >;
> >             ...
> >         }
> >     }
> > 
> > What I am wondering is pci_process_bridge_OF_ranges() at
> > arch/powerpc/kernel/pci-common.c
> > where there are used some hardcoded values which should be probably
> > loaded from device-tree.
> > 
> > For example:
> > 683         int np = pna + 5;
> > ...
> > 702                 pci_addr = of_read_number(ranges + 1, 2);
> > 703                 cpu_addr = of_translate_address(dev, ranges + 3);
> > 704                 size = of_read_number(ranges + pna + 3, 2);
> 
> These would always be correct whether you have 6 or 7 cells. pna is the
> parent bus address cells size. The pci address is fixed at 3 cells.
> 
> > 
> > 
> > Unfortunately we have copied it to microblaze.
> 
> I look at the PCI DT code in powerpc and see a whole bunch of code that
> seems like it should be common. The different per arch pci structs
> complicates that. No one has really gotten to looking at PCI DT on ARM
> yet except you and Thierry for Tegra. We definitely don't want to create
> a 3rd copy. Starting the process of moving it to something like
> drivers/pci/pci-of.c would be great.

A lot of it should be common. The microblaze code is a copy of the
powerpc version. I'll strongly nack any attempt to add a third!  :-)

drivers/pci/pci-of.c would be good. I'd also accept drivers/of/pci.c
which might actually be a good idea in the short term so that it gets
appropriate supervision while being generalized before being moved into
the pci directory.

g.

Re: pci and pcie device-tree binding - range No cells

[Michal Simek]

On 12/10/2012 10:41 PM, Grant Likely wrote:
> On Mon, 10 Dec 2012 09:21:51 -0600, Rob Herring <robherring2@gmail.com> wrote:
>> On 12/10/2012 09:05 AM, Michal Simek wrote:
>>> On 12/10/2012 03:26 PM, Rob Herring wrote:
>>>> On 12/10/2012 06:20 AM, Michal Simek wrote:
>>>>> Hi Grant and others,
>>>>>
>>>>> I have a question regarding number of cells in ranges property
>>>>> for pci and pcie nodes.
>>>>>
>>>>> Linux pci/pcie powerpc DTSes contain 7 cells (xpedite5370.dts,
>>>>> sequoia.dts, etc)
>>>>> but also 6 cells format too (mpc832x_mds.dts)
>>>>>
>>>>> Here is shown 6 cells ranges format and describe
>>>>> http://devicetree.org/Device_Tree_Usage#PCI_Host_Bridge
>>>>>
>>>>> And also in documentation in the linux
>>>>> Documentation/devicetree/bindings/pci/83xx-512x-pci.txt
>>>>>
>>>>> Both format uses:
>>>>> #size-cells = <2>;
>>>>> #address-cells = <3>;
>>>>>
>>>>> What is valid format?
>>>>
>>>> Both. 7 cells are valid when the host (parent) bus is 64-bit and 6 cells
>>>> are valid when the host bus is 32-bit. The ranges property is <<child
>>>> address> <parent address> <size>>. The parent address #address-cells is
>>>> taken from the parent node.
>>>
>>> Ok. Got it.
>>>
>>> Here is what we use on zynq and microblaze - both 32bit which should be
>>> fine.
>>>
>>>      ps7_axi_interconnect_0: axi@0 {
>>>          #address-cells = <1>;
>>>          #size-cells = <1>;
>>>          axi_pcie_0: axi-pcie@50000000 {
>>>              #address-cells = <3>;
>>>              #size-cells = <2>;
>>>              compatible = "xlnx,axi-pcie-1.05.a";
>>>              ranges = < 0x02000000 0 0x60000000 0x60000000 0 0x10000000 >;
>>>              ...
>>>          }
>>>      }
>>>
>>> What I am wondering is pci_process_bridge_OF_ranges() at
>>> arch/powerpc/kernel/pci-common.c
>>> where there are used some hardcoded values which should be probably
>>> loaded from device-tree.
>>>
>>> For example:
>>> 683         int np = pna + 5;
>>> ...
>>> 702                 pci_addr = of_read_number(ranges + 1, 2);
>>> 703                 cpu_addr = of_translate_address(dev, ranges + 3);
>>> 704                 size = of_read_number(ranges + pna + 3, 2);
>>
>> These would always be correct whether you have 6 or 7 cells. pna is the
>> parent bus address cells size. The pci address is fixed at 3 cells.
>>
>>>
>>>
>>> Unfortunately we have copied it to microblaze.
>>
>> I look at the PCI DT code in powerpc and see a whole bunch of code that
>> seems like it should be common. The different per arch pci structs
>> complicates that. No one has really gotten to looking at PCI DT on ARM
>> yet except you and Thierry for Tegra. We definitely don't want to create
>> a 3rd copy. Starting the process of moving it to something like
>> drivers/pci/pci-of.c would be great.
>
> A lot of it should be common. The microblaze code is a copy of the
> powerpc version. I'll strongly nack any attempt to add a third!  :-)

Yes it. There are some things which we had fixed because that powerpc
port is big endian only and we support PCIe on little endian too.
But changes are really cosmetic.


> drivers/pci/pci-of.c would be good. I'd also accept drivers/of/pci.c
> which might actually be a good idea in the short term so that it gets
> appropriate supervision while being generalized before being moved into
> the pci directory.

Ben: Are you willing to move that ppc code to this location?
It is probably not good idea that I should do it when I even don't have
hardware available for testing (Asking someone else).

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

Re: pci and pcie device-tree binding - range No cells

[Grant Likely]

On Wed, Dec 12, 2012 at 10:37 AM, Michal Simek <monstr@monstr.eu> wrote:
> On 12/10/2012 10:41 PM, Grant Likely wrote:
>> drivers/pci/pci-of.c would be good. I'd also accept drivers/of/pci.c
>> which might actually be a good idea in the short term so that it gets
>> appropriate supervision while being generalized before being moved into
>> the pci directory.
>
> Ben: Are you willing to move that ppc code to this location?
> It is probably not good idea that I should do it when I even don't have
> hardware available for testing (Asking someone else).

You're a clever guy, you are more than capable of crafting the patch,
even if you can't test on hardware. :-)

I refactored most of the OF support code without having access to most
of the affected hardware. Once I got the changes out there for review
I also asked for spot testing before getting it into linux-next for
even more testing.

g.

Re: pci and pcie device-tree binding - range No cells

[Andrew Murray]

On Wed, Dec 12, 2012 at 10:49 AM, Grant Likely wrote:
> On Wed, Dec 12, 2012 at 10:37 AM, Michal Simek <monstr@monstr.eu<mailto:m=
onstr@monstr.eu>> wrote:
> > On 12/10/2012 10:41 PM, Grant Likely wrote:
> >> drivers/pci/pci-of.c would be good. I'd also accept drivers/of/pci.c
> >> which might actually be a good idea in the short term so that it gets
> >> appropriate supervision while being generalized before being moved int=
o
> >> the pci directory.
> >
> > Ben: Are you willing to move that ppc code to this location?
> > It is probably not good idea that I should do it when I even don't have
> > hardware available for testing (Asking someone else).
>=20
> You're a clever guy, you are more than capable of crafting the patch,
> even if you can't test on hardware. :-)
>=20
> I refactored most of the OF support code without having access to most
> of the affected hardware. Once I got the changes out there for review
> I also asked for spot testing before getting it into linux-next for
> even more testing.

I've been working on a relatively architecture agnostic PCI host bridge dri=
ver
and also wanted to avoid duplicating more generic DT parsing code for PCI
bindings.

I've ended up with a patch which provides an iterator for returning resourc=
es
based on the the typical 'ranges' binding. This has ended up living in
drivers/of/address.c. I originally started out in drivers/of/pci.c and
drivers/pci/pci-of.c but found there were good (and static) implementations=
 in
drivers/of/address.c which can be reused (e.g. of_bus_pci_get_flags,
bus->count_cells).

I'm not just ready to post it - but can do before early next week if you ca=
n
wait.

Andrew Murray

Re: pci and pcie device-tree binding - range No cells

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2190 bytes --]

On Wed, Dec 12, 2012 at 12:19:12PM +0000, Andrew Murray wrote:
> On Wed, Dec 12, 2012 at 10:49 AM, Grant Likely wrote:
> > On Wed, Dec 12, 2012 at 10:37 AM, Michal Simek <monstr@monstr.eu<mailto:monstr@monstr.eu>> wrote:
> > > On 12/10/2012 10:41 PM, Grant Likely wrote:
> > >> drivers/pci/pci-of.c would be good. I'd also accept drivers/of/pci.c
> > >> which might actually be a good idea in the short term so that it gets
> > >> appropriate supervision while being generalized before being moved into
> > >> the pci directory.
> > >
> > > Ben: Are you willing to move that ppc code to this location?
> > > It is probably not good idea that I should do it when I even don't have
> > > hardware available for testing (Asking someone else).
> > 
> > You're a clever guy, you are more than capable of crafting the patch,
> > even if you can't test on hardware. :-)
> > 
> > I refactored most of the OF support code without having access to most
> > of the affected hardware. Once I got the changes out there for review
> > I also asked for spot testing before getting it into linux-next for
> > even more testing.
> 
> I've been working on a relatively architecture agnostic PCI host bridge driver
> and also wanted to avoid duplicating more generic DT parsing code for PCI
> bindings.
> 
> I've ended up with a patch which provides an iterator for returning resources
> based on the the typical 'ranges' binding. This has ended up living in
> drivers/of/address.c. I originally started out in drivers/of/pci.c and
> drivers/pci/pci-of.c but found there were good (and static) implementations in
> drivers/of/address.c which can be reused (e.g. of_bus_pci_get_flags,
> bus->count_cells).
> 
> I'm not just ready to post it - but can do before early next week if you can
> wait.

I already posted a similar patch[0] as part of a larger series to bring
DT support to Tegra PCIe back in July. I suppose what you have must be
something pretty close to that. Most of the stuff that had me occupied
since then should be done soon and I was planning on resurrecting the
series one of these days.

Thierry

[0]: https://patchwork.kernel.org/patch/1244451/

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: pci and pcie device-tree binding - range No cells

[Andrew Murray]

On Wed, Dec 12, 2012 at 01:34:24PM +0000, Thierry Reding wrote:
> On Wed, Dec 12, 2012 at 12:19:12PM +0000, Andrew Murray wrote:
> > I've been working on a relatively architecture agnostic PCI host bridge=
 driver
> > and also wanted to avoid duplicating more generic DT parsing code for P=
CI
> > bindings.
> >=20
> > I've ended up with a patch which provides an iterator for returning res=
ources
> > based on the the typical 'ranges' binding. This has ended up living in
> > drivers/of/address.c. I originally started out in drivers/of/pci.c and
> > drivers/pci/pci-of.c but found there were good (and static) implementat=
ions in
> > drivers/of/address.c which can be reused (e.g. of_bus_pci_get_flags,
> > bus->count_cells).
> >=20
> > I'm not just ready to post it - but can do before early next week if yo=
u can
> > wait.
>=20
> I already posted a similar patch[0] as part of a larger series to bring
> DT support to Tegra PCIe back in July. I suppose what you have must be
> something pretty close to that. Most of the stuff that had me occupied
> since then should be done soon and I was planning on resurrecting the
> series one of these days.

Thanks for the reference. I've submitted my patch, it's along the lines of =
your
existing patch.

I'm happy to take the best bits from both, drop mine, etc.

Andrew Murray

Re: pci and pcie device-tree binding - range No cells

[Michal Simek]

On 12/12/2012 11:49 AM, Grant Likely wrote:
> On Wed, Dec 12, 2012 at 10:37 AM, Michal Simek <monstr@monstr.eu> wrote:
>> On 12/10/2012 10:41 PM, Grant Likely wrote:
>>> drivers/pci/pci-of.c would be good. I'd also accept drivers/of/pci.c
>>> which might actually be a good idea in the short term so that it gets
>>> appropriate supervision while being generalized before being moved into
>>> the pci directory.
>>
>> Ben: Are you willing to move that ppc code to this location?
>> It is probably not good idea that I should do it when I even don't have
>> hardware available for testing (Asking someone else).
>
> You're a clever guy, you are more than capable of crafting the patch,
> even if you can't test on hardware. :-)
>
> I refactored most of the OF support code without having access to most
> of the affected hardware. Once I got the changes out there for review
> I also asked for spot testing before getting it into linux-next for
> even more testing.

Fair enough. :-)

Good time to start to look for how to work with board farm.

Thanks,
Michal


-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian