linux-kernel - Re: [PATCH v5 5/5] gicv2m: acpi: Introducing GICv2m ACPI support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <56686C9F.6060804@amd.com>
Date:	Wed, 9 Dec 2015 12:02:07 -0600
From:	Suravee Suthikulanit <suravee.suthikulpanit@....com>
To:	Marc Zyngier <marc.zyngier@....com>
CC:	<tglx@...utronix.de>, <jason@...edaemon.net>, <rjw@...ysocki.net>,
	Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
	Will Deacon <will.deacon@....com>,
	Catalin Marinas <Catalin.Marinas@....com>,
	<hanjun.guo@...aro.org>, <tomasz.nowicki@...aro.org>,
	<graeme.gregory@...aro.org>, <dhdang@....com>,
	<linux-arm-kernel@...ts.infradead.org>,
	<linux-kernel@...r.kernel.org>, <linux-acpi@...r.kernel.org>
Subject: Re: [PATCH v5 5/5] gicv2m: acpi: Introducing GICv2m ACPI support

Hi Marc,

On 12/9/2015 4:38 AM, Marc Zyngier wrote:
> On Tue, 8 Dec 2015 17:48:06 -0800
> Suravee Suthikulpanit <Suravee.Suthikulpanit@....com> wrote:
>
>> This patch introduces gicv2m_acpi_init(), which uses information
>> in MADT GIC MSI frames structure to initialize GICv2m driver.
>>
>> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@....com>
>> Signed-off-by: Hanjun Guo <hanjun.guo@...aro.org>
>> ---
>>   drivers/irqchip/irq-gic-v2m.c   | 95 +++++++++++++++++++++++++++++++++++++++++
>>   drivers/irqchip/irq-gic.c       |  3 ++
>>   include/linux/irqchip/arm-gic.h |  4 ++
>>   3 files changed, 102 insertions(+)
>>
>> diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c
>> [...]
>> @@ -359,6 +368,8 @@ static int __init gicv2m_init_one(struct fwnode_handle *fwnode,
>>
>>   	if (to_of_node(fwnode))
>>   		name = to_of_node(fwnode)->name;
>> +	else
>> +		name = irq_domain_get_irqchip_fwnode_name(fwnode);
>
> Don't bother with that, the name associated with the domain is
> absolutely meaningless. You are already printing the frame address,
> which is enough to identify it, should someone need to debug this.
>
> Drop the name from the previous patch as well, and that will make one
> less difference to care about. Patch #3 can die as well.
>

Ok. I'll just leave them blank (i.e. const char *name ="")

>> [...]
>> diff --git a/include/linux/irqchip/arm-gic.h b/include/linux/irqchip/arm-gic.h
>> index bae69e5..30b2ccb 100644
>> --- a/include/linux/irqchip/arm-gic.h
>> +++ b/include/linux/irqchip/arm-gic.h
>> @@ -108,6 +108,10 @@ void gic_init(unsigned int nr, int start,
>>
>>   int gicv2m_of_init(struct device_node *node, struct irq_domain *parent);
>>
>> +#ifdef CONFIG_ACPI
>> +int gicv2m_acpi_init(struct irq_domain *parent);
>> +#endif
>> +
>
> How about having a single:
>
> int gicv2m_init(struct fwnode_handle *parent_handle,
> 		struct irq_domain *parent_domain);
>
> which in turn calls either gicv2m_of_init or gicv2m_acpi_init? Saves
> some #ifdef, and avoids another entry point.

Sounds good. I'll take care of this.

>>   void gic_send_sgi(unsigned int cpu_id, unsigned int irq);
>>   int gic_get_cpu_id(unsigned int cpu);
>>   void gic_migrate_target(unsigned int new_cpu_id);
>
> Otherwise, this looks good.
>
> Thanks,
>
> 	M.
>

Thanks. Sending out v6 in a little bit.

Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/