| Message ID | 1410530416-30200-11-git-send-email-hanjun.guo@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Sep 12, 2014 at 10:00:08PM +0800, Hanjun Guo wrote: > When MADT is parsed, print GIC information to make the boot > log look pretty. If you do this for prettiness, having an example output in the commit message would be a nice touch. The question is why it's needed though? Do we really need all this verbose information in dmesg when it could be found out in other ways instead? x86 is notoriously verbose in some of these aspects, I'm not sure it's something we need to mimic here? -Olof
Hi Olof, How about having it verbose in the initial implementation (in case there are any fun times bringing up early systems) and then revisiting this? x86 is very verbose but it can be helpful. Jon.
On Sun, 14 Sep 2014 23:40:10 -0700, Olof Johansson <olof@lixom.net> wrote: > On Fri, Sep 12, 2014 at 10:00:08PM +0800, Hanjun Guo wrote: > > When MADT is parsed, print GIC information to make the boot > > log look pretty. > > If you do this for prettiness, having an example output in the commit > message would be a nice touch. Agreed. > The question is why it's needed though? Do we really need all this verbose > information in dmesg when it could be found out in other ways instead? > > x86 is notoriously verbose in some of these aspects, I'm not sure it's > something we need to mimic here? For consistency, it does make sense to have the common x86 and ARM64 code outputing information in the same way, so I think the patch still makes sense. All of them could be moved to KERN_DEBUG as a separate patch if we want to cull early boot output. I would split that off and discuss it separately. g.
On Mon, Sep 15, 2014 at 10:52 AM, Grant Likely <grant.likely@linaro.org> wrote: > On Sun, 14 Sep 2014 23:40:10 -0700, Olof Johansson <olof@lixom.net> wrote: >> On Fri, Sep 12, 2014 at 10:00:08PM +0800, Hanjun Guo wrote: >> > When MADT is parsed, print GIC information to make the boot >> > log look pretty. >> >> If you do this for prettiness, having an example output in the commit >> message would be a nice touch. > > Agreed. > >> The question is why it's needed though? Do we really need all this verbose >> information in dmesg when it could be found out in other ways instead? >> >> x86 is notoriously verbose in some of these aspects, I'm not sure it's >> something we need to mimic here? > > For consistency, it does make sense to have the common x86 and ARM64 > code outputing information in the same way, so I think the patch still > makes sense. All of them could be moved to KERN_DEBUG as a separate > patch if we want to cull early boot output. I would split that off and > discuss it separately. Yeah, pr_debug() makes sense here -- in particular if you're going to have a potentially slow serial console to print all this to at info level. I disagree on order though, there's no reason to hold off fixing this and doing it as a separate patch. -Olof
On 09/15/2014 02:01 PM, Olof Johansson wrote: > On Mon, Sep 15, 2014 at 10:52 AM, Grant Likely <grant.likely@linaro.org> wrote: >> On Sun, 14 Sep 2014 23:40:10 -0700, Olof Johansson <olof@lixom.net> wrote: >>> On Fri, Sep 12, 2014 at 10:00:08PM +0800, Hanjun Guo wrote: >>>> When MADT is parsed, print GIC information to make the boot >>>> log look pretty. >>> >>> If you do this for prettiness, having an example output in the commit >>> message would be a nice touch. >> >> Agreed. Here's example output from a hardware platform (just as an example in this thread, mostly for archiving purposes): [ 0.000000] ACPI: GICC (acpi_id[0x0000] address[00000000e112f000] MPDIR[0x0] enabled) [ 0.000000] ACPI: GICC (acpi_id[0x0001] address[00000000e112f000] MPDIR[0x1] enabled) [ 0.000000] ACPI: GICC (acpi_id[0x0100] address[00000000e112f000] MPDIR[0x100] enabled) [ 0.000000] ACPI: GICC (acpi_id[0x0101] address[00000000e112f000] MPDIR[0x101] enabled) [ 0.000000] ACPI: GICC (acpi_id[0x0200] address[00000000e112f000] MPDIR[0x200] enabled) [ 0.000000] ACPI: GICC (acpi_id[0x0201] address[00000000e112f000] MPDIR[0x201] enabled) Jon.
diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c index b18e45e..47f36d4 100644 --- a/drivers/acpi/tables.c +++ b/drivers/acpi/tables.c @@ -183,6 +183,49 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header) } break; + case ACPI_MADT_TYPE_GENERIC_INTERRUPT: + { + struct acpi_madt_generic_interrupt *p = + (struct acpi_madt_generic_interrupt *)header; + pr_info("GICC (acpi_id[0x%04x] address[%p] MPDIR[0x%llx] %s)\n", + p->uid, (void *)(unsigned long)p->base_address, + p->arm_mpidr, + (p->flags & ACPI_MADT_ENABLED) ? "enabled" : "disabled"); + + } + break; + + case ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR: + { + struct acpi_madt_generic_distributor *p = + (struct acpi_madt_generic_distributor *)header; + pr_info("GIC Distributor (gic_id[0x%04x] address[%p] gsi_base[%d])\n", + p->gic_id, + (void *)(unsigned long)p->base_address, + p->global_irq_base); + } + break; + + case ACPI_MADT_TYPE_GENERIC_MSI_FRAME: + { + struct acpi_madt_generic_msi_frame *p = + (struct acpi_madt_generic_msi_frame *)header; + pr_info("GIC MSI Frame (msi_fame_id[%d] address[%p])\n", + p->msi_frame_id, + (void *)(unsigned long)p->base_address); + } + break; + + case ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR: + { + struct acpi_madt_generic_redistributor *p = + (struct acpi_madt_generic_redistributor *)header; + pr_info("GIC Redistributor (address[%p] region_size[0x%x])\n", + (void *)(unsigned long)p->base_address, + p->length); + } + break; + default: pr_warn("Found unsupported MADT entry (type = 0x%x)\n", header->type);