diff mbox

[v4,10/18] ACPI / table: Print GIC information when MADT is parsed

Message ID 1410530416-30200-11-git-send-email-hanjun.guo@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Hanjun Guo Sept. 12, 2014, 2 p.m. UTC
When MADT is parsed, print GIC information to make the boot
log look pretty.

Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
Signed-off-by: Tomasz Nowicki <tomasz.nowicki@linaro.org>
---
 drivers/acpi/tables.c |   43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

Comments

Olof Johansson Sept. 15, 2014, 6:40 a.m. UTC | #1
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
Jon Masters Sept. 15, 2014, 2:11 p.m. UTC | #2
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.
Grant Likely Sept. 15, 2014, 5:52 p.m. UTC | #3
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.
Olof Johansson Sept. 15, 2014, 6:01 p.m. UTC | #4
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
Jon Masters Sept. 15, 2014, 9:55 p.m. UTC | #5
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 mbox

Patch

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);