diff mbox

[07/27] irqchip: Declare cortex-a7's irqchip to initialize gic from dt

Message ID 1397122124-15690-8-git-send-email-cw00.choi@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Chanwoo Choi April 10, 2014, 9:28 a.m. UTC
This patch declare coretex-a7's irqchip to initialze gic from dt
with "arm,cortex-a7-gic" data.

Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/irqchip/irq-gic.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Marc Zyngier April 10, 2014, 10:04 a.m. UTC | #1
On Thu, Apr 10 2014 at 10:28:24 am BST, Chanwoo Choi <cw00.choi@samsung.com> wrote:
> This patch declare coretex-a7's irqchip to initialze gic from dt
> with "arm,cortex-a7-gic" data.
>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/irqchip/irq-gic.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index 4300b66..8e906e4 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -1069,6 +1069,7 @@ gic_of_init(struct device_node *node, struct device_node *parent)
>  }
>  IRQCHIP_DECLARE(cortex_a15_gic, "arm,cortex-a15-gic", gic_of_init);
>  IRQCHIP_DECLARE(cortex_a9_gic, "arm,cortex-a9-gic", gic_of_init);
> +IRQCHIP_DECLARE(cortex_a7_gic, "arm,cortex-a7-gic", gic_of_init);
>  IRQCHIP_DECLARE(msm_8660_qgic, "qcom,msm-8660-qgic", gic_of_init);
>  IRQCHIP_DECLARE(msm_qgic2, "qcom,msm-qgic2", gic_of_init);

Frankly, this patch adds no value. Are we going to add
"arm,cortex-a12-gic", "arm,cortex-a17-gic", "arm,cortex-a53-gic",
"arm,cortex-a57-gic"? And that's just to mention the ARM Ltd cores...

Instead, how about defining a generic "arm,gic" property, and mandate
that new DT files are using that? We can always use a more precise
compatible for quirks.

Mark, what do you think? I think this has been discussed in the past
already.

	M.
armdev April 10, 2014, 10:09 a.m. UTC | #2
On 10-Apr-2014, at 3:34 pm, Marc Zyngier <marc.zyngier@arm.com> wrote:

> On Thu, Apr 10 2014 at 10:28:24 am BST, Chanwoo Choi <cw00.choi@samsung.com> wrote:
>> This patch declare coretex-a7's irqchip to initialze gic from dt
>> with "arm,cortex-a7-gic" data.
>> 
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
>> drivers/irqchip/irq-gic.c | 1 +
>> 1 file changed, 1 insertion(+)
>> 
>> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
>> index 4300b66..8e906e4 100644
>> --- a/drivers/irqchip/irq-gic.c
>> +++ b/drivers/irqchip/irq-gic.c
>> @@ -1069,6 +1069,7 @@ gic_of_init(struct device_node *node, struct device_node *parent)
>> }
>> IRQCHIP_DECLARE(cortex_a15_gic, "arm,cortex-a15-gic", gic_of_init);
>> IRQCHIP_DECLARE(cortex_a9_gic, "arm,cortex-a9-gic", gic_of_init);
>> +IRQCHIP_DECLARE(cortex_a7_gic, "arm,cortex-a7-gic", gic_of_init);
>> IRQCHIP_DECLARE(msm_8660_qgic, "qcom,msm-8660-qgic", gic_of_init);
>> IRQCHIP_DECLARE(msm_qgic2, "qcom,msm-qgic2", gic_of_init);
> 
> Frankly, this patch adds no value. Are we going to add
> "arm,cortex-a12-gic", "arm,cortex-a17-gic", "arm,cortex-a53-gic",
> "arm,cortex-a57-gic"? And that's just to mention the ARM Ltd cores...
> 
> Instead, how about defining a generic "arm,gic" property, and mandate
> that new DT files are using that? We can always use a more precise
> compatible for quirks.
> 

How about keeping it simple and tied to arm gic versions
arm,gicv1, arm,gicv2, arm,gicv2ve

> Mark, what do you think? I think this has been discussed in the past
> already.
> 
> 	M.
> -- 
> Jazz is not dead. It just smells funny.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Marc Zyngier April 10, 2014, 10:21 a.m. UTC | #3
On Thu, Apr 10 2014 at 11:09:02 am BST, armdev <armdev.ftm@gmail.com> wrote:
> On 10-Apr-2014, at 3:34 pm, Marc Zyngier <marc.zyngier@arm.com> wrote:
>
>> On Thu, Apr 10 2014 at 10:28:24 am BST, Chanwoo Choi <cw00.choi@samsung.com> wrote:
>>> This patch declare coretex-a7's irqchip to initialze gic from dt
>>> with "arm,cortex-a7-gic" data.
>>> 
>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>>> ---
>>> drivers/irqchip/irq-gic.c | 1 +
>>> 1 file changed, 1 insertion(+)
>>> 
>>> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
>>> index 4300b66..8e906e4 100644
>>> --- a/drivers/irqchip/irq-gic.c
>>> +++ b/drivers/irqchip/irq-gic.c
>>> @@ -1069,6 +1069,7 @@ gic_of_init(struct device_node *node, struct device_node *parent)
>>> }
>>> IRQCHIP_DECLARE(cortex_a15_gic, "arm,cortex-a15-gic", gic_of_init);
>>> IRQCHIP_DECLARE(cortex_a9_gic, "arm,cortex-a9-gic", gic_of_init);
>>> +IRQCHIP_DECLARE(cortex_a7_gic, "arm,cortex-a7-gic", gic_of_init);
>>> IRQCHIP_DECLARE(msm_8660_qgic, "qcom,msm-8660-qgic", gic_of_init);
>>> IRQCHIP_DECLARE(msm_qgic2, "qcom,msm-qgic2", gic_of_init);
>> 
>> Frankly, this patch adds no value. Are we going to add
>> "arm,cortex-a12-gic", "arm,cortex-a17-gic", "arm,cortex-a53-gic",
>> "arm,cortex-a57-gic"? And that's just to mention the ARM Ltd cores...
>> 
>> Instead, how about defining a generic "arm,gic" property, and mandate
>> that new DT files are using that? We can always use a more precise
>> compatible for quirks.
>> 
>
> How about keeping it simple and tied to arm gic versions
> arm,gicv1, arm,gicv2, arm,gicv2ve

That's a variation on the same theme. As for GICv2, we don't need to
distinguish between having the Virtualization Extentions, the binding
already allows you to tell one from the other.

	M.
armdev April 10, 2014, 10:30 a.m. UTC | #4
On 10-Apr-2014, at 3:51 pm, Marc Zyngier <marc.zyngier@arm.com> wrote:

> On Thu, Apr 10 2014 at 11:09:02 am BST, armdev <armdev.ftm@gmail.com> wrote:
>> On 10-Apr-2014, at 3:34 pm, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> 
>>> On Thu, Apr 10 2014 at 10:28:24 am BST, Chanwoo Choi <cw00.choi@samsung.com> wrote:
>>>> This patch declare coretex-a7's irqchip to initialze gic from dt
>>>> with "arm,cortex-a7-gic" data.
>>>> 
>>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>>>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>>>> ---
>>>> drivers/irqchip/irq-gic.c | 1 +
>>>> 1 file changed, 1 insertion(+)
>>>> 
>>>> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
>>>> index 4300b66..8e906e4 100644
>>>> --- a/drivers/irqchip/irq-gic.c
>>>> +++ b/drivers/irqchip/irq-gic.c
>>>> @@ -1069,6 +1069,7 @@ gic_of_init(struct device_node *node, struct device_node *parent)
>>>> }
>>>> IRQCHIP_DECLARE(cortex_a15_gic, "arm,cortex-a15-gic", gic_of_init);
>>>> IRQCHIP_DECLARE(cortex_a9_gic, "arm,cortex-a9-gic", gic_of_init);
>>>> +IRQCHIP_DECLARE(cortex_a7_gic, "arm,cortex-a7-gic", gic_of_init);
>>>> IRQCHIP_DECLARE(msm_8660_qgic, "qcom,msm-8660-qgic", gic_of_init);
>>>> IRQCHIP_DECLARE(msm_qgic2, "qcom,msm-qgic2", gic_of_init);
>>> 
>>> Frankly, this patch adds no value. Are we going to add
>>> "arm,cortex-a12-gic", "arm,cortex-a17-gic", "arm,cortex-a53-gic",
>>> "arm,cortex-a57-gic"? And that's just to mention the ARM Ltd cores...
>>> 
>>> Instead, how about defining a generic "arm,gic" property, and mandate
>>> that new DT files are using that? We can always use a more precise
>>> compatible for quirks.
>>> 
>> 
>> How about keeping it simple and tied to arm gic versions
>> arm,gicv1, arm,gicv2, arm,gicv2ve
> 
> That's a variation on the same theme. As for GICv2, we don't need to
> distinguish between having the Virtualization Extentions, the binding
> already allows you to tell one from the other.
> 
So if there be just 2 types of gic, it would be simple.
gicv1 - 2 address sets (gicc and gicd)
gicv2 - 4 sets (gicc gicd gicv gich) and 1 maintenance interrupt. Right?

> 	M.
> -- 
> Jazz is not dead. It just smells funny.
Chanho Park April 10, 2014, 10:37 a.m. UTC | #5
Hi,

> -----Original Message-----
> From: linux-arm-kernel [mailto:linux-arm-kernel-
> bounces@lists.infradead.org] On Behalf Of Marc Zyngier
> Sent: Thursday, April 10, 2014 7:05 PM
> To: Chanwoo Choi
> Cc: mark.rutland@arm.com; linux-samsung-soc@vger.kernel.org;
> t.figa@samsung.com; hyunhee.kim@samsung.com; sw0312.kim@samsung.com;
> linux-kernel@vger.kernel.org; yj44.cho@samsung.com; inki.dae@samsung.com;
> kyungmin.park@samsung.com; kgene.kim@samsung.com; Thomas Gleixner;
> linux-arm-kernel@lists.infradead.org
> Subject: Re: [PATCH 07/27] irqchip: Declare cortex-a7's irqchip to
> initialize gic from dt
> 
> On Thu, Apr 10 2014 at 10:28:24 am BST, Chanwoo Choi
> <cw00.choi@samsung.com> wrote:
> > This patch declare coretex-a7's irqchip to initialze gic from dt
> > with "arm,cortex-a7-gic" data.
> >
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > ---
> >  drivers/irqchip/irq-gic.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> > index 4300b66..8e906e4 100644
> > --- a/drivers/irqchip/irq-gic.c
> > +++ b/drivers/irqchip/irq-gic.c
> > @@ -1069,6 +1069,7 @@ gic_of_init(struct device_node *node, struct
> device_node *parent)
> >  }
> >  IRQCHIP_DECLARE(cortex_a15_gic, "arm,cortex-a15-gic", gic_of_init);
> >  IRQCHIP_DECLARE(cortex_a9_gic, "arm,cortex-a9-gic", gic_of_init);
> > +IRQCHIP_DECLARE(cortex_a7_gic, "arm,cortex-a7-gic", gic_of_init);
> >  IRQCHIP_DECLARE(msm_8660_qgic, "qcom,msm-8660-qgic", gic_of_init);
> >  IRQCHIP_DECLARE(msm_qgic2, "qcom,msm-qgic2", gic_of_init);
> 
> Frankly, this patch adds no value. Are we going to add
> "arm,cortex-a12-gic", "arm,cortex-a17-gic", "arm,cortex-a53-gic",
> "arm,cortex-a57-gic"? And that's just to mention the ARM Ltd cores...
> 
> Instead, how about defining a generic "arm,gic" property, and mandate
> that new DT files are using that? We can always use a more precise
> compatible for quirks.

I prefer it would be arm,gicv2 instead arm-gic.
In case of GICv3 of arm64, it can be arm,gicv3.

Best Regards,
Chanho Park
Marc Zyngier April 10, 2014, 10:41 a.m. UTC | #6
On Thu, Apr 10 2014 at 11:30:41 am BST, armdev <armdev.ftm@gmail.com> wrote:
> On 10-Apr-2014, at 3:51 pm, Marc Zyngier <marc.zyngier@arm.com> wrote:
>
>> On Thu, Apr 10 2014 at 11:09:02 am BST, armdev <armdev.ftm@gmail.com> wrote:
>>> On 10-Apr-2014, at 3:34 pm, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>> 
>>>> On Thu, Apr 10 2014 at 10:28:24 am BST, Chanwoo Choi <cw00.choi@samsung.com> wrote:
>>>>> This patch declare coretex-a7's irqchip to initialze gic from dt
>>>>> with "arm,cortex-a7-gic" data.
>>>>> 
>>>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>>>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>>>>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>>>>> ---
>>>>> drivers/irqchip/irq-gic.c | 1 +
>>>>> 1 file changed, 1 insertion(+)
>>>>> 
>>>>> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
>>>>> index 4300b66..8e906e4 100644
>>>>> --- a/drivers/irqchip/irq-gic.c
>>>>> +++ b/drivers/irqchip/irq-gic.c
>>>>> @@ -1069,6 +1069,7 @@ gic_of_init(struct device_node *node, struct device_node *parent)
>>>>> }
>>>>> IRQCHIP_DECLARE(cortex_a15_gic, "arm,cortex-a15-gic", gic_of_init);
>>>>> IRQCHIP_DECLARE(cortex_a9_gic, "arm,cortex-a9-gic", gic_of_init);
>>>>> +IRQCHIP_DECLARE(cortex_a7_gic, "arm,cortex-a7-gic", gic_of_init);
>>>>> IRQCHIP_DECLARE(msm_8660_qgic, "qcom,msm-8660-qgic", gic_of_init);
>>>>> IRQCHIP_DECLARE(msm_qgic2, "qcom,msm-qgic2", gic_of_init);
>>>> 
>>>> Frankly, this patch adds no value. Are we going to add
>>>> "arm,cortex-a12-gic", "arm,cortex-a17-gic", "arm,cortex-a53-gic",
>>>> "arm,cortex-a57-gic"? And that's just to mention the ARM Ltd cores...
>>>> 
>>>> Instead, how about defining a generic "arm,gic" property, and mandate
>>>> that new DT files are using that? We can always use a more precise
>>>> compatible for quirks.
>>>> 
>>> 
>>> How about keeping it simple and tied to arm gic versions
>>> arm,gicv1, arm,gicv2, arm,gicv2ve
>> 
>> That's a variation on the same theme. As for GICv2, we don't need to
>> distinguish between having the Virtualization Extentions, the binding
>> already allows you to tell one from the other.
>> 
> So if there be just 2 types of gic, it would be simple.

Not exactly. We just happen to support two revisions of the GIC
architecture with the same binding. GICv3 has an entierely separate
binding.

> gicv1 - 2 address sets (gicc and gicd)

Yes.

> gicv2 - 4 sets (gicc gicd gicv gich) and 1 maintenance interrupt. Right?

No.

The presence of the GICV, GICH and maintenance interrupt are indicative
of the support for the Virtualization Extentions. GICv2 itself can
perfectly be built without it.

	M.
armdev April 10, 2014, 10:42 a.m. UTC | #7
On 10-Apr-2014, at 4:11 pm, Marc Zyngier <marc.zyngier@arm.com> wrote:

> On Thu, Apr 10 2014 at 11:30:41 am BST, armdev <armdev.ftm@gmail.com> wrote:
>> On 10-Apr-2014, at 3:51 pm, Marc Zyngier <marc.zyngier@arm.com> wrote:
>> 
>>> On Thu, Apr 10 2014 at 11:09:02 am BST, armdev <armdev.ftm@gmail.com> wrote:
>>>> On 10-Apr-2014, at 3:34 pm, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>>> 
>>>>> On Thu, Apr 10 2014 at 10:28:24 am BST, Chanwoo Choi <cw00.choi@samsung.com> wrote:
>>>>>> This patch declare coretex-a7's irqchip to initialze gic from dt
>>>>>> with "arm,cortex-a7-gic" data.
>>>>>> 
>>>>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>>>>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>>>>>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>>>>>> ---
>>>>>> drivers/irqchip/irq-gic.c | 1 +
>>>>>> 1 file changed, 1 insertion(+)
>>>>>> 
>>>>>> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
>>>>>> index 4300b66..8e906e4 100644
>>>>>> --- a/drivers/irqchip/irq-gic.c
>>>>>> +++ b/drivers/irqchip/irq-gic.c
>>>>>> @@ -1069,6 +1069,7 @@ gic_of_init(struct device_node *node, struct device_node *parent)
>>>>>> }
>>>>>> IRQCHIP_DECLARE(cortex_a15_gic, "arm,cortex-a15-gic", gic_of_init);
>>>>>> IRQCHIP_DECLARE(cortex_a9_gic, "arm,cortex-a9-gic", gic_of_init);
>>>>>> +IRQCHIP_DECLARE(cortex_a7_gic, "arm,cortex-a7-gic", gic_of_init);
>>>>>> IRQCHIP_DECLARE(msm_8660_qgic, "qcom,msm-8660-qgic", gic_of_init);
>>>>>> IRQCHIP_DECLARE(msm_qgic2, "qcom,msm-qgic2", gic_of_init);
>>>>> 
>>>>> Frankly, this patch adds no value. Are we going to add
>>>>> "arm,cortex-a12-gic", "arm,cortex-a17-gic", "arm,cortex-a53-gic",
>>>>> "arm,cortex-a57-gic"? And that's just to mention the ARM Ltd cores...
>>>>> 
>>>>> Instead, how about defining a generic "arm,gic" property, and mandate
>>>>> that new DT files are using that? We can always use a more precise
>>>>> compatible for quirks.
>>>>> 
>>>> 
>>>> How about keeping it simple and tied to arm gic versions
>>>> arm,gicv1, arm,gicv2, arm,gicv2ve
>>> 
>>> That's a variation on the same theme. As for GICv2, we don't need to
>>> distinguish between having the Virtualization Extentions, the binding
>>> already allows you to tell one from the other.
>>> 
>> So if there be just 2 types of gic, it would be simple.
> 
> Not exactly. We just happen to support two revisions of the GIC
> architecture with the same binding. GICv3 has an entierely separate
> binding.
> 
>> gicv1 - 2 address sets (gicc and gicd)
> 
> Yes.
> 
>> gicv2 - 4 sets (gicc gicd gicv gich) and 1 maintenance interrupt. Right?
> 
> No.
> 
> The presence of the GICV, GICH and maintenance interrupt are indicative
> of the support for the Virtualization Extentions. GICv2 itself can
> perfectly be built without it.

then does gicv2-ve makes sense ?
> 
> 	M.
> -- 
> Jazz is not dead. It just smells funny.
Marc Zyngier April 10, 2014, 10:46 a.m. UTC | #8
On Thu, Apr 10 2014 at 11:37:12 am BST, Chanho Park <chanho61.park@samsung.com> wrote:
> Hi,
>
>> -----Original Message-----
>> From: linux-arm-kernel [mailto:linux-arm-kernel-
>> bounces@lists.infradead.org] On Behalf Of Marc Zyngier
>> Sent: Thursday, April 10, 2014 7:05 PM
>> To: Chanwoo Choi
>> Cc: mark.rutland@arm.com; linux-samsung-soc@vger.kernel.org;
>> t.figa@samsung.com; hyunhee.kim@samsung.com; sw0312.kim@samsung.com;
>> linux-kernel@vger.kernel.org; yj44.cho@samsung.com; inki.dae@samsung.com;
>> kyungmin.park@samsung.com; kgene.kim@samsung.com; Thomas Gleixner;
>> linux-arm-kernel@lists.infradead.org
>> Subject: Re: [PATCH 07/27] irqchip: Declare cortex-a7's irqchip to
>> initialize gic from dt
>> 
>> On Thu, Apr 10 2014 at 10:28:24 am BST, Chanwoo Choi
>> <cw00.choi@samsung.com> wrote:
>> > This patch declare coretex-a7's irqchip to initialze gic from dt
>> > with "arm,cortex-a7-gic" data.
>> >
>> > Cc: Thomas Gleixner <tglx@linutronix.de>
>> > Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> > ---
>> >  drivers/irqchip/irq-gic.c | 1 +
>> >  1 file changed, 1 insertion(+)
>> >
>> > diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
>> > index 4300b66..8e906e4 100644
>> > --- a/drivers/irqchip/irq-gic.c
>> > +++ b/drivers/irqchip/irq-gic.c
>> > @@ -1069,6 +1069,7 @@ gic_of_init(struct device_node *node, struct
>> device_node *parent)
>> >  }
>> >  IRQCHIP_DECLARE(cortex_a15_gic, "arm,cortex-a15-gic", gic_of_init);
>> >  IRQCHIP_DECLARE(cortex_a9_gic, "arm,cortex-a9-gic", gic_of_init);
>> > +IRQCHIP_DECLARE(cortex_a7_gic, "arm,cortex-a7-gic", gic_of_init);
>> >  IRQCHIP_DECLARE(msm_8660_qgic, "qcom,msm-8660-qgic", gic_of_init);
>> >  IRQCHIP_DECLARE(msm_qgic2, "qcom,msm-qgic2", gic_of_init);
>> 
>> Frankly, this patch adds no value. Are we going to add
>> "arm,cortex-a12-gic", "arm,cortex-a17-gic", "arm,cortex-a53-gic",
>> "arm,cortex-a57-gic"? And that's just to mention the ARM Ltd cores...
>> 
>> Instead, how about defining a generic "arm,gic" property, and mandate
>> that new DT files are using that? We can always use a more precise
>> compatible for quirks.
>
> I prefer it would be arm,gicv2 instead arm-gic.

If you prefer, fine by me. Consider spelling it "arm,gic-v2", which
seems to be the current convention for version numbers.

> In case of GICv3 of arm64, it can be arm,gicv3.

GICv3 and arm64 are independent of each other, and the binding has
already been specified as "arm,gic-v3".

	M.
Marc Zyngier April 10, 2014, 10:50 a.m. UTC | #9
On Thu, Apr 10 2014 at 11:42:56 am BST, armdev <armdev.ftm@gmail.com> wrote:
> On 10-Apr-2014, at 4:11 pm, Marc Zyngier <marc.zyngier@arm.com> wrote:
>
>> On Thu, Apr 10 2014 at 11:30:41 am BST, armdev <armdev.ftm@gmail.com> wrote:
>>> On 10-Apr-2014, at 3:51 pm, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>> 
>>>> On Thu, Apr 10 2014 at 11:09:02 am BST, armdev <armdev.ftm@gmail.com> wrote:
>>>>> On 10-Apr-2014, at 3:34 pm, Marc Zyngier <marc.zyngier@arm.com> wrote:
>>>>> 
>>>>>> On Thu, Apr 10 2014 at 10:28:24 am BST, Chanwoo Choi <cw00.choi@samsung.com> wrote:
>>>>>>> This patch declare coretex-a7's irqchip to initialze gic from dt
>>>>>>> with "arm,cortex-a7-gic" data.
>>>>>>> 
>>>>>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>>>>>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>>>>>>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>>>>>>> ---
>>>>>>> drivers/irqchip/irq-gic.c | 1 +
>>>>>>> 1 file changed, 1 insertion(+)
>>>>>>> 
>>>>>>> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
>>>>>>> index 4300b66..8e906e4 100644
>>>>>>> --- a/drivers/irqchip/irq-gic.c
>>>>>>> +++ b/drivers/irqchip/irq-gic.c
>>>>>>> @@ -1069,6 +1069,7 @@ gic_of_init(struct device_node *node, struct device_node *parent)
>>>>>>> }
>>>>>>> IRQCHIP_DECLARE(cortex_a15_gic, "arm,cortex-a15-gic", gic_of_init);
>>>>>>> IRQCHIP_DECLARE(cortex_a9_gic, "arm,cortex-a9-gic", gic_of_init);
>>>>>>> +IRQCHIP_DECLARE(cortex_a7_gic, "arm,cortex-a7-gic", gic_of_init);
>>>>>>> IRQCHIP_DECLARE(msm_8660_qgic, "qcom,msm-8660-qgic", gic_of_init);
>>>>>>> IRQCHIP_DECLARE(msm_qgic2, "qcom,msm-qgic2", gic_of_init);
>>>>>> 
>>>>>> Frankly, this patch adds no value. Are we going to add
>>>>>> "arm,cortex-a12-gic", "arm,cortex-a17-gic", "arm,cortex-a53-gic",
>>>>>> "arm,cortex-a57-gic"? And that's just to mention the ARM Ltd cores...
>>>>>> 
>>>>>> Instead, how about defining a generic "arm,gic" property, and mandate
>>>>>> that new DT files are using that? We can always use a more precise
>>>>>> compatible for quirks.
>>>>>> 
>>>>> 
>>>>> How about keeping it simple and tied to arm gic versions
>>>>> arm,gicv1, arm,gicv2, arm,gicv2ve
>>>> 
>>>> That's a variation on the same theme. As for GICv2, we don't need to
>>>> distinguish between having the Virtualization Extentions, the binding
>>>> already allows you to tell one from the other.
>>>> 
>>> So if there be just 2 types of gic, it would be simple.
>> 
>> Not exactly. We just happen to support two revisions of the GIC
>> architecture with the same binding. GICv3 has an entierely separate
>> binding.
>> 
>>> gicv1 - 2 address sets (gicc and gicd)
>> 
>> Yes.
>> 
>>> gicv2 - 4 sets (gicc gicd gicv gich) and 1 maintenance interrupt. Right?
>> 
>> No.
>> 
>> The presence of the GICV, GICH and maintenance interrupt are indicative
>> of the support for the Virtualization Extentions. GICv2 itself can
>> perfectly be built without it.
>
> then does gicv2-ve makes sense ?

Read what I just wrote. You find the GICV region, you have the VE
extensions. You don't find them, they are not present. No need for an
overloaded compatible string, they both conform to the same
*Architecture Spec*.

	M.
Mark Rutland April 10, 2014, 1:02 p.m. UTC | #10
On Thu, Apr 10, 2014 at 11:04:59AM +0100, Marc Zyngier wrote:
> On Thu, Apr 10 2014 at 10:28:24 am BST, Chanwoo Choi <cw00.choi@samsung.com> wrote:
> > This patch declare coretex-a7's irqchip to initialze gic from dt
> > with "arm,cortex-a7-gic" data.
> >
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > ---
> >  drivers/irqchip/irq-gic.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> > index 4300b66..8e906e4 100644
> > --- a/drivers/irqchip/irq-gic.c
> > +++ b/drivers/irqchip/irq-gic.c
> > @@ -1069,6 +1069,7 @@ gic_of_init(struct device_node *node, struct device_node *parent)
> >  }
> >  IRQCHIP_DECLARE(cortex_a15_gic, "arm,cortex-a15-gic", gic_of_init);
> >  IRQCHIP_DECLARE(cortex_a9_gic, "arm,cortex-a9-gic", gic_of_init);
> > +IRQCHIP_DECLARE(cortex_a7_gic, "arm,cortex-a7-gic", gic_of_init);
> >  IRQCHIP_DECLARE(msm_8660_qgic, "qcom,msm-8660-qgic", gic_of_init);
> >  IRQCHIP_DECLARE(msm_qgic2, "qcom,msm-qgic2", gic_of_init);
> 
> Frankly, this patch adds no value. Are we going to add
> "arm,cortex-a12-gic", "arm,cortex-a17-gic", "arm,cortex-a53-gic",
> "arm,cortex-a57-gic"? And that's just to mention the ARM Ltd cores...
> 
> Instead, how about defining a generic "arm,gic" property, and mandate
> that new DT files are using that? We can always use a more precise
> compatible for quirks.

Nit: s/property/compatible/

As mentioned elsewhere, "arm,gic-v2" would seem to fit the bill (and we
can have "arm,gic" or "arm,gic-v1" for v1).

> Mark, what do you think? I think this has been discussed in the past
> already.

It's been repeatedly brought up and agreed on, it's just that no-one's
implemented it. I agree that having an "arm,gic-v2" binding that people
can place in their compatible list is a sensible thing to do, and I'd be
happy to Ack patches implementing it.

Cheers,
Mark.
diff mbox

Patch

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 4300b66..8e906e4 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -1069,6 +1069,7 @@  gic_of_init(struct device_node *node, struct device_node *parent)
 }
 IRQCHIP_DECLARE(cortex_a15_gic, "arm,cortex-a15-gic", gic_of_init);
 IRQCHIP_DECLARE(cortex_a9_gic, "arm,cortex-a9-gic", gic_of_init);
+IRQCHIP_DECLARE(cortex_a7_gic, "arm,cortex-a7-gic", gic_of_init);
 IRQCHIP_DECLARE(msm_8660_qgic, "qcom,msm-8660-qgic", gic_of_init);
 IRQCHIP_DECLARE(msm_qgic2, "qcom,msm-qgic2", gic_of_init);