Re: [PATCH] serial: apbuart: Let boards without apbuart boot

[Miguel Ojeda]

On Sun, May 16, 2010 at 12:20 PM, Anton Vorontsov
<cbouatmailru@xxxxxxxxx> wrote:
> On Sun, May 16, 2010 at 10:41:50AM +0200, Miguel Ojeda wrote:
>> On Fri, May 14, 2010 at 4:19 PM, Anton Vorontsov <avorontsov@xxxxxxxxxx> wrote:
>> > This patch fixes the following oops:
>> >
>>
>> I sent a patch that fixes that oops too. It is in mm right now. Please check it.
>
> Hm, and my patch is in Greg's tree already...

Well, that's weird... Andrew sent an email to Greg 4 days ago with the
patch after Kristoffer ack'ed it. Maybe Greg missed it.

>
> Greg, if it didn't make it anywhere further your tree, please
> drop it in favour of Miguel's patch, Miguel was the first who
> sent a fix for this problem.
>

Thanks, but Kristoffer should review your patch too, as they are not
exactly the same: My patch changes does an additional check at
grlib_apbuart_init() (which also calls grlib_apbuart_configure()),
sets the global variable grlib_apbuart_port_nr to 0 and prints a
message to the user; keeping the void return type of
grlib_apbuart_configure().

See it here: https://patchwork.kernel.org/patch/98713/

> Thanks,
>
>> >  Unable to handle kernel paging request for data at address 0x00000000
>> >  Faulting instruction address: 0xc016dea8
>> >  Oops: Kernel access of bad area, sig: 11 [#1]
>> >  P1020 RDB
>> >  last sysfs file:
>> >  NIP: c016dea8 LR: c016dea0 CTR: c0010948
>> >  REGS: c033fea0 TRAP: 0300   Not tainted  (2.6.34-rc7-00108-g83b5177-dirty)
>> >  [...]
>> >  NIP [c016dea8] grlib_apbuart_configure+0xd0/0x3bc
>> >  LR [c016dea0] grlib_apbuart_configure+0xc8/0x3bc
>> >  Call Trace:
>> >  [c033ff50] [c016dea0] grlib_apbuart_configure+0xc8/0x3bc (unreliable)
>> >  [c033ffa0] [c0316144] apbuart_console_init+0x10/0x34
>> >  [c033ffb0] [c0314a10] console_init+0x40/0x5c
>> >
>> > There's no apbuart on P1020 boards, and what's worse, there's no
>> > clock-frequency property for it. The driver didn't handle this
>> > case, which resulted in the oops above. Once we fix this, the
>> > next oops pops up:
>> >
>> >  Unable to handle kernel paging request for data at address 0x00000030
>> >  Faulting instruction address: 0xc0166ecc
>> >  Oops: Kernel access of bad area, sig: 11 [#1]
>> >  P1020 RDB
>> >  [...]
>> >  NIP [c0166ecc] uart_set_options+0xd0/0x164
>> >  LR [c0166e38] uart_set_options+0x3c/0x164
>> >  Call Trace:
>> >  [c033fef0] [c0330000] 0xc0330000 (unreliable)
>> >  [c033ff50] [c03160dc] apbuart_console_setup+0xa8/0x100
>> >  [c033ff70] [c003d668] register_console+0x338/0x3ec
>> >  [c033ffa0] [c0316154] apbuart_console_init+0x20/0x34
>> >  [c033ffb0] [c0314a10] console_init+0x40/0x5c
>> >  [c033ffc0] [c0300968] start_kernel+0x12c/0x234
>> >  [c033fff0] [c0000398] skpinv+0x2b0/0x2ec
>> >
>> > And that one is because the driver tries to register the console
>> > even if there were no matches in grlib_apbuart_configure().
>> >
>> > While at it, also annotate grlib_apbuart_configure() with __init,
>> > that fixes several section mismatches:
>> >
>> >  Section mismatch in reference from the function
>> >  grlib_apbuart_configure() to the variable .init.data:apbuart_match
>> >  The function grlib_apbuart_configure() references the variable
>> >  __initdata apbuart_match.
>> >
>> > Signed-off-by: Anton Vorontsov <avorontsov@xxxxxxxxxx>
>> > ---
>> >  drivers/serial/apbuart.c |   19 ++++++++++++++-----
>> >  1 files changed, 14 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/drivers/serial/apbuart.c b/drivers/serial/apbuart.c
>> > index fe91319..c908734 100644
>> > --- a/drivers/serial/apbuart.c
>> > +++ b/drivers/serial/apbuart.c
>> > @@ -520,11 +520,16 @@ static struct console grlib_apbuart_console = {
>> >  };
>> >
>> >
>> > -static void grlib_apbuart_configure(void);
>> > +static int __init grlib_apbuart_configure(void);
>> >
>> >  static int __init apbuart_console_init(void)
>> >  {
>> > -       grlib_apbuart_configure();
>> > +       int ret;
>> > +
>> > +       ret = grlib_apbuart_configure();
>> > +       if (ret)
>> > +               return ret;
>> > +
>> >        register_console(&grlib_apbuart_console);
>> >        return 0;
>> >  }
>> > @@ -593,7 +598,7 @@ static struct of_platform_driver grlib_apbuart_of_driver = {
>> >  };
>> >
>> >
>> > -static void grlib_apbuart_configure(void)
>> > +static int __init grlib_apbuart_configure(void)
>> >  {
>> >        static int enum_done;
>> >        struct device_node *np, *rp;
>> > @@ -606,12 +611,14 @@ static void grlib_apbuart_configure(void)
>> >        struct amba_prom_registers *regs;
>> >
>> >        if (enum_done)
>> > -               return;
>> > +               return -ENODEV;
>> >
>> >        /* Get bus frequency */
>> >        rp = of_find_node_by_path("/");
>> >        rp = of_get_next_child(rp, NULL);
>> >        prop = of_get_property(rp, "clock-frequency", NULL);
>> > +       if (!prop)
>> > +               return -ENODEV;
>> >        freq_khz = *prop;
>> >
>> >        line = 0;
>> > @@ -629,7 +636,7 @@ static void grlib_apbuart_configure(void)
>> >                        d = *device;
>> >
>> >                if (!irqs || !regs)
>> > -                       return;
>> > +                       return -ENODEV;
>> >
>> >                grlib_apbuart_nodes[line] = np;
>> >
>> > @@ -658,6 +665,8 @@ static void grlib_apbuart_configure(void)
>> >        enum_done = 1;
>> >
>> >        grlib_apbuart_driver.nr = grlib_apbuart_port_nr = line;
>> > +
>> > +       return line ? 0 : -ENODEV;
>> >  }
>> >
>> >  static int __init grlib_apbuart_init(void)
>> > --
>> > 1.7.0.5
>
> --
> Anton Vorontsov
> email: cbouatmailru@xxxxxxxxx
> irc://irc.freenode.net/bd2
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/