lkml.org 
[lkml]   [2008]   [Sep]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [patch 3/3] x86: io-apic - code style cleaning for setup_IO_APIC_irqs
    [Ingo Molnar - Sat, Sep 06, 2008 at 08:49:14PM +0200]
    |
    | * Maciej W. Rozycki <macro@linux-mips.org> wrote:
    |
    | > On Sat, 6 Sep 2008, Cyrill Gorcunov wrote:
    | >
    | > > Ingo, how about the following approach? We don't introduce new
    | > > functions but rather srink the code by new printout form.
    | >
    | > Honestly, this one should probably use sprintf() or suchlike to avoid the
    | > mess of printk() calls building a line of output from pieces. It's quite
    | > easy to calculate here what the maximum size of the buffer required could
    | > be and automatic arrays can have variable size, so no need for the hassle
    | > of heap management. Calls to printk() without a trailing newline should
    | > be avoided where possible as it messes up logging priority if a message
    | > pops up from an interrupt inbetween.
    |
    | hm, is it worth the trouble? This is during very early init.
    |
    | Ingo
    |

    Ingo, Maciej,

    here is an another attempt to satisfy the requirements :)
    Please review and comment if it looks better or worse now.

    - Cyrill -

    ---
    From: Cyrill Gorcunov <gorcunov@gmail.com>
    Subject: [PATCH] x86: io-apic - cleanup of setup_IO_APIC_irqs v3

    Use local buffer to accumulate all not connected pin info
    on each io-apic and printout it if needed.

    Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
    ---


    Index: linux-2.6.git/arch/x86/kernel/io_apic.c
    ===================================================================
    --- linux-2.6.git.orig/arch/x86/kernel/io_apic.c 2008-09-07 10:10:15.000000000 +0400
    +++ linux-2.6.git/arch/x86/kernel/io_apic.c 2008-09-07 13:50:46.000000000 +0400
    @@ -1514,46 +1514,69 @@ static void setup_IO_APIC_irq(int apic,
    ioapic_write_entry(apic, pin, entry);
    }

    +/* check and setup io-apic irq */
    +static int __init setup_ioapic_irq(int apic, int pin)
    +{
    + int idx, irq;
    +
    + idx = find_irq_entry(apic, pin, mp_INT);
    + if (idx == -1)
    + return -1;
    +
    + irq = pin_2_irq(idx, apic, pin);
    +
    +#ifdef CONFIG_X86_32
    + if (multi_timer_check(apic, irq))
    + return 0;
    +#endif
    +
    + add_pin_to_irq(irq, apic, pin);
    + setup_IO_APIC_irq(apic, pin, irq,
    + irq_trigger(idx), irq_polarity(idx));
    + return 0;
    +}
    +
    static void __init setup_IO_APIC_irqs(void)
    {
    - int apic, pin, idx, irq;
    - int notcon = 0;
    + int apic, pin;
    + char buf[64], *p;
    + int len;

    apic_printk(APIC_VERBOSE, KERN_DEBUG "init IO_APIC IRQs\n");

    + p = buf, buf[0] = 0, len = 0;
    +
    for (apic = 0; apic < nr_ioapics; apic++) {
    for (pin = 0; pin < nr_ioapic_registers[apic]; pin++) {

    - idx = find_irq_entry(apic, pin, mp_INT);
    - if (idx == -1) {
    - apic_printk(APIC_VERBOSE,
    - KERN_DEBUG " %d-%d",
    - mp_ioapics[apic].mp_apicid, pin);
    - if (!notcon)
    - notcon = 1;
    + if (!setup_ioapic_irq(apic, pin))
    continue;
    - }

    - irq = pin_2_irq(idx, apic, pin);
    -#ifdef CONFIG_X86_32
    - if (multi_timer_check(apic, irq))
    + /*
    + * pin is not connected
    + * - we try to save this info in the buffer
    + * and print it out later
    + * - we don't expect too much not connected
    + * pins (as MP specification says the maximum
    + * value is 2 not connected pins) but if we
    + * have buggy BIOS or just get too many of them -
    + * truncate output with "..."
    + */
    + if (!p)
    continue;
    -#endif
    - add_pin_to_irq(irq, apic, pin);
    -
    - setup_IO_APIC_irq(apic, pin, irq,
    - irq_trigger(idx), irq_polarity(idx));
    + len += snprintf(p + len, sizeof(buf) - len, " %d-%d",
    + mp_ioapics[apic].mp_apicid, pin);
    + if (len >= sizeof(buf)) {
    + memcpy(&buf[sizeof(buf) - 4], "...", 3);
    + p = NULL;
    + }
    }
    - if (notcon) {
    - apic_printk(APIC_VERBOSE,
    - KERN_DEBUG " (apicid-pin) not connected\n");
    - notcon = 0;
    + if (buf[0]) {
    + apic_printk(APIC_VERBOSE, KERN_DEBUG
    + " (apicid-pin) not connected:%s\n", buf);
    + p = buf, buf[0] = 0, len = 0;
    }
    }
    -
    - if (notcon)
    - apic_printk(APIC_VERBOSE,
    - KERN_DEBUG " (apicid-pin) not connected\n");
    }

    /*

    \
     
     \ /
      Last update: 2008-09-07 12:03    [W:2.791 / U:0.192 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site