of_serial and device trees

of_serial and device trees

[Simon Kagstrom]

Hi!

I'm working on a custom Freescale 8544-based board and having some
trouble with getting the device tree right. In particular, we have a
setup which looks like this:

* We have an FPGA which contains a few serial ports, an interrupt
  controller and some other devices

* The serial ports are "normal" 16550s and interrupt the FPGA interrupt
  controller

* The interrupt controller is in turn cascaded to the 8544 OpenPIC
  (mpic).

My problem is that I can't figure out how to express this correctly in
the device tree for the board. My device tree looks like this ("isf" is
the name of the FPGA):

	isf@c0020000 {
		#address-cells = <1>;
		#size-cells = <1>;
		reg = <c0020000 00020000>;

                isf_pic: isf_pic@4000 {
                        device_type = "isf-pic";
                        compatible = "isf-pic";
                        interrupt-controller;
                        clock-frequency = <0>;
                        #address-cells = <0>;
                        #interrupt-cells = <2>;
                        reg = <4000 100>;
                        built-in;
                        interrupts = <4 1>;
                        interrupt-parent = <&mpic>;
                };

                serial2: isf-serial@4200 {
                        device_type = "serial";
                        compatible = "ns16550";
                        reg = <4200 100>;
                        clock-frequency = <13ab6680>; /* 330Mhz */
                        interrupts = <0 1>;
                        interrupt-parent = <&isf_pic>;
                };
                <... and a few more serial ports which look the same >

Following the example set in mpc85xx_ds.c, I lookup the interrupt
controller manually in the platform setup code (init_IRQ). Basically

        np = of_find_compatible_node(NULL, "isf-pic", "isf-pic");
        if (np == NULL) {
                printk(KERN_ERR "Could not find ISF PIC\n");
                return;
        }

and thereafter calling a custom setup function for the FPGA interrupt
controller. That appears to work fine, but I'm wondering how to get the
serial ports detected properly by of_serial.

I suppose I can do something similar in code to what is done in
arch/powerpc/kernel/legacy_serial.c:find_legacy_serial_ports, but I
thought I could do without that with the device trees. Is it so, or do
I have to add code to walk through and setup the rest of the serial
ports?

Thanks in advance,
// Simon

Re: of_serial and device trees

[Scott Wood]

On Tue, Mar 24, 2009 at 09:49:59AM +0100, Simon Kagstrom wrote:
> My problem is that I can't figure out how to express this correctly in
> the device tree for the board. My device tree looks like this ("isf" is
> the name of the FPGA):
> 
> 	isf@c0020000 {
> 		#address-cells = <1>;
> 		#size-cells = <1>;
> 		reg = <c0020000 00020000>;
> 

Change this line to: ranges = <0 c0020000 00020000>;
This causes reg resources in child nodes to be properly translated.  What
you have above indicates that all 0x20000 bytes are for the driver of *this*
node.

Add compatible = "simple-bus".  This lets children of this node be probed
by of_platform drivers (make sure you list simple-bus when calling
of_platform_bus_probe), and in a few other places (like legacy_serial.c).

>                 isf_pic: isf_pic@4000 {
>                         device_type = "isf-pic";
>                         compatible = "isf-pic";
>                         interrupt-controller;
>                         clock-frequency = <0>;
>                         #address-cells = <0>;
>                         #interrupt-cells = <2>;
>                         reg = <4000 100>;
>                         built-in;
>                         interrupts = <4 1>;
>                         interrupt-parent = <&mpic>;

Ged rid of built-in, clock-frequency, and device_type.

Compatible should be of the form "vendor,device" -- and does "isf"
uniquely identify the specific FPGA logic, or are there other versions
out there (or likely to exist in the future)?  Note that there are some
bad examples in existing device trees that have yet to be fixed.

Node name should be interrupt-controller, not isf_pic.

>                         clock-frequency = <13ab6680>; /* 330Mhz */

Note that dts-v1 syntax has C-like constants, with decimal by default. 
I'm guessing you're working with a relatively old kernel?

> and thereafter calling a custom setup function for the FPGA interrupt
> controller. That appears to work fine, but I'm wondering how to get the
> serial ports detected properly by of_serial.

The simple-bus compatible, and ranges property, should do it.  I'm
guessing that your FPGA PIC driver isn't getting its register address
from the device tree, given that it works without the ranges property?

-Scott

Re: of_serial and device trees

[Simon Kagstrom]

Hi Scott!

Thanks a lot for all your help with the device tree! As you suspected,
I'm writing for a fairly old kernel (2.6.21, WindRiver). I know, I know.

On Tue, 24 Mar 2009 10:55:45 -0500
Scott Wood <scottwood@freescale.com> wrote:

> Add compatible = "simple-bus".  This lets children of this node be probed
> by of_platform drivers (make sure you list simple-bus when calling
> of_platform_bus_probe), and in a few other places (like legacy_serial.c).

Right, thanks. I've added this of_platform_bus_probe as an initcall,
and it now tries to configure the serial ports as expected (and with
some other issues).

> Compatible should be of the form "vendor,device" -- and does "isf"
> uniquely identify the specific FPGA logic, or are there other versions
> out there (or likely to exist in the future)?  Note that there are some
> bad examples in existing device trees that have yet to be fixed.

There are no other versions yet, but I suppose there will be (it's
implemented in a FPGA after all!). So what is the general handling of
versions, should it be something like

  compatible = "ericsson,isf-pic", "ericsson,isf-pic-v2"

etc if we'd make new revisions of the device?

> I'm guessing that your FPGA PIC driver isn't getting its register address
> from the device tree, given that it works without the ranges property?

It is, but I didn't check it for correctness yet, so I suppose I might
have mapped the wrong thing. The code looks like this:

        struct resource phys_addr;

        if (of_address_to_resource(np, 0, &phys_addr) != 0) {
                 printk(KERN_ERR": Could not get ISF PIC memory resource\n");
                 return NULL;
         }

	spin_lock_init(&isf->lock);
	isf->ioaddr = ioremap(phys_addr.start,
	                phys_addr.end - phys_addr.start);

So I should use the reg property and platform_get_resource() instead?

// Simon

Re: of_serial and device trees

[Scott Wood]

Simon Kagstrom wrote:
> There are no other versions yet, but I suppose there will be (it's
> implemented in a FPGA after all!). So what is the general handling of
> versions, should it be something like
> 
>   compatible = "ericsson,isf-pic", "ericsson,isf-pic-v2"
> 
> etc if we'd make new revisions of the device?

Looks good.

>> I'm guessing that your FPGA PIC driver isn't getting its register address
>> from the device tree, given that it works without the ranges property?
> 
> It is, but I didn't check it for correctness yet, so I suppose I might
> have mapped the wrong thing. The code looks like this:
> 
>         struct resource phys_addr;
> 
>         if (of_address_to_resource(np, 0, &phys_addr) != 0) {
>                  printk(KERN_ERR": Could not get ISF PIC memory resource\n");
>                  return NULL;
>          }
> 
> 	spin_lock_init(&isf->lock);
> 	isf->ioaddr = ioremap(phys_addr.start,
> 	                phys_addr.end - phys_addr.start);

That looks good -- I'd have expected of_address_to_resource to fail, 
though, when the ranges property was missing.  The kernel's device tree 
parsing code can sometimes be overly tolerant of broken device trees, 
which is probably what happened.

-Scott

Re: of_serial and device trees

[Michael Ellerman]

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

On Wed, 2009-03-25 at 11:39 -0500, Scott Wood wrote:
> Simon Kagstrom wrote:
> > There are no other versions yet, but I suppose there will be (it's
> > implemented in a FPGA after all!). So what is the general handling of
> > versions, should it be something like
> > 
> >   compatible = "ericsson,isf-pic", "ericsson,isf-pic-v2"
> > 
> > etc if we'd make new revisions of the device?
> 
> Looks good.
> 
> >> I'm guessing that your FPGA PIC driver isn't getting its register address
> >> from the device tree, given that it works without the ranges property?
> > 
> > It is, but I didn't check it for correctness yet, so I suppose I might
> > have mapped the wrong thing. The code looks like this:
> > 
> >         struct resource phys_addr;
> > 
> >         if (of_address_to_resource(np, 0, &phys_addr) != 0) {
> >                  printk(KERN_ERR": Could not get ISF PIC memory resource\n");
> >                  return NULL;
> >          }
> > 
> > 	spin_lock_init(&isf->lock);
> > 	isf->ioaddr = ioremap(phys_addr.start,
> > 	                phys_addr.end - phys_addr.start);
> 
> That looks good -- I'd have expected of_address_to_resource to fail, 
> though, when the ranges property was missing.  The kernel's device tree 
> parsing code can sometimes be overly tolerant of broken device trees, 
> which is probably what happened.

It assumes a missing ranges == empty ranges, which means 1:1. See the
comment in of_translate_one(). And thank Apple ;)

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]