current-speed property in serial devices causes kernel panic

current-speed property in serial devices causes kernel panic

[Timur Tabi]

I see this code in function of_platform_serial_setup():

static int __devinit of_platform_serial_setup(struct of_device *ofdev,
                                        int type, struct uart_port *port)
{
        struct resource resource;
        struct device_node *np = ofdev->node;
        const unsigned int *clk, *spd;
        int ret;
                     
        memset(port, 0, sizeof *port);
        spd = of_get_property(np, "current-speed", NULL);
...
        port->custom_divisor = *clk / (16 * (*spd));
        
        return 0;
}

There is no check in this code to make sure spd is not null.  And sure enough, in most DTS files, current-speed does not exist.  So whenever this function is called on a node like this, the kernel panics.

I'm adding support for a new 86xx platform, and I'm also creating a driver for a new SOC device.  In my platform driver, I have this code:

static struct of_device_id mpc86xx_ids[] = {
        { .type = "soc", },
        {}
};
   
static int __init mpc86xx_declare_of_platform_devices(void)  
{
        printk(KERN_ALERT "%s\n", __FUNCTION__);

        if (!machine_is(mpc86xx_hpcn))
                return 0;

        of_platform_bus_probe(NULL, mpc86xx_ids, NULL);

        return 0;
}
device_initcall(mpc86xx_declare_of_platform_devices);

The kernel panic occurs only if I call of_platform_bus_probe().

If you look at the code for the 836x platform, you'll see that it also has serial SOC devices and it also calls of_platform_bus_probe(), but it doesn't experience kernel panics.

Is the call to of_platform_bus_probe() effectively trying to probe the serial devices twice?  I just don't understand why this code isn't working.

Re: current-speed property in serial devices causes kernel panic

[Kumar Gala]

On Jun 29, 2007, at 11:26 PM, Timur Tabi wrote:

> I see this code in function of_platform_serial_setup():
>
> static int __devinit of_platform_serial_setup(struct of_device *ofdev,
>                                         int type, struct uart_port  
> *port)
> {
>         struct resource resource;
>         struct device_node *np = ofdev->node;
>         const unsigned int *clk, *spd;
>         int ret;
>
>         memset(port, 0, sizeof *port);
>         spd = of_get_property(np, "current-speed", NULL);
> ...
>         port->custom_divisor = *clk / (16 * (*spd));
>
>         return 0;
> }
>
> There is no check in this code to make sure spd is not null.  And  
> sure enough, in most DTS files, current-speed does not exist.  So  
> whenever this function is called on a node like this, the kernel  
> panics.

I'm wondering why we set port->custom_divisor instead of setting port- 
 >speed. (plus the null check).

- k

Re: current-speed property in serial devices causes kernel panic

[Timur Tabi]

Timur Tabi wrote:
> The kernel panic occurs only if I call of_platform_bus_probe().

FYI, thanks to Kumar, I found the problem: I had CONFIG_SERIAL_OF_PLATFORM enabled.  This 
causes of_serial.c to be compiled and loaded.  On my platform, that wasn't doing anything 
because initially I wasn't calling of_platform_bus_probe().  When I added that call to my 
platform code, of_serial.c sprang to life and tried to probe the serial nodes in my device 
tree.  Apparently, of_serial.c is not compatible with fsl_soc.c.

-- 
Timur Tabi
Linux Kernel Developer @ Freescale