All of lore.kernel.org
 help / color / mirror / Atom feed
From: hdegoede@redhat.com (Hans de Goede)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/3] irqchip: sun4i: Don't mask + unmask for the non oneshot case
Date: Fri, 14 Mar 2014 19:42:00 +0100	[thread overview]
Message-ID: <53234D78.5050901@redhat.com> (raw)
In-Reply-To: <alpine.DEB.2.02.1403141228290.18573@ionos.tec.linutronix.de>

Hi,

On 03/14/2014 12:35 PM, Thomas Gleixner wrote:
> On Thu, 13 Mar 2014, Hans de Goede wrote:
> 
>> Since sun4i and sun5i are single core SOCs there is no need to mask non
>> oneshot IRQs, to achieve this we use handle_fasteoi_irq with a dummy eoi.
> 
> This is slightly wrong :)
> 
> Even on a SMP system there is no need to mask the interrupt when the
> controller works like that sunxi one. If the controller is not broken
> beyond repair then it does not deliver the same interrupt to a
> different cpu. Such a thing would always deliver it to all cores and
> those would race to grab the spinlock and mask it. I've seen such
> horror, but don't ask how that performs.
> 
> The reason why you can spare the mask/unmask dance is that the
> controller does not require any action and clears the interrupt when
> the level goes back to inactive. That happens when the device handler
> acks it at the device level.
> 
> Now there might be the case when the device reactivates the interrupt
> before the RETI. But that does not matter as we run the primary
> interrupt handlers with interrupts disabled.

Ok, I'm going to wait a bit to see if Maxime has anything to say
to this second series, and then I'll do a v2 with the commit msg
fixed.

Regards,

Hans


> 
> Thanks,
> 
> 	tglx
>  
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  drivers/irqchip/irq-sun4i.c | 11 ++++++++++-
>>  1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/irqchip/irq-sun4i.c b/drivers/irqchip/irq-sun4i.c
>> index a0ed1ea..0a71990 100644
>> --- a/drivers/irqchip/irq-sun4i.c
>> +++ b/drivers/irqchip/irq-sun4i.c
>> @@ -74,8 +74,17 @@ static void sun4i_irq_unmask(struct irq_data *irqd)
>>  	       sun4i_irq_base + SUN4I_IRQ_ENABLE_REG(reg));
>>  }
>>  
>> +/*
>> + * Since sun4i and sun5i are single core SOCs there is no need to mask non
>> + * oneshot IRQs, to achieve this we use handle_fasteoi_irq with a dummy eoi.
>> + */
>> +static void sun4i_irq_dummy_eoi(struct irq_data *irqd)
>> +{
>> +}
>> +
>>  static struct irq_chip sun4i_irq_chip = {
>>  	.name		= "sun4i_irq",
>> +	.irq_eoi	= sun4i_irq_dummy_eoi,
>>  	.irq_mask	= sun4i_irq_mask,
>>  	.irq_unmask	= sun4i_irq_unmask,
>>  };
>> @@ -97,7 +106,7 @@ static int sun4i_irq_map(struct irq_domain *d, unsigned int virq,
>>  					 handle_fasteoi_irq);
>>  	else
>>  		irq_set_chip_and_handler(virq, &sun4i_irq_chip,
>> -					 handle_level_irq);
>> +					 handle_fasteoi_irq);
>>  
>>  	set_irq_flags(virq, IRQF_VALID | IRQF_PROBE);
>>  
>> -- 
>> 1.9.0
>>
>>

WARNING: multiple messages have this Message-ID (diff)
From: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
To: Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
Cc: Maxime Ripard
	<maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	devicetree <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
Subject: Re: [PATCH 1/3] irqchip: sun4i: Don't mask + unmask for the non oneshot case
Date: Fri, 14 Mar 2014 19:42:00 +0100	[thread overview]
Message-ID: <53234D78.5050901@redhat.com> (raw)
In-Reply-To: <alpine.DEB.2.02.1403141228290.18573-3cz04HxQygjZikZi3RtOZ1XZhhPuCNm+@public.gmane.org>

Hi,

On 03/14/2014 12:35 PM, Thomas Gleixner wrote:
> On Thu, 13 Mar 2014, Hans de Goede wrote:
> 
>> Since sun4i and sun5i are single core SOCs there is no need to mask non
>> oneshot IRQs, to achieve this we use handle_fasteoi_irq with a dummy eoi.
> 
> This is slightly wrong :)
> 
> Even on a SMP system there is no need to mask the interrupt when the
> controller works like that sunxi one. If the controller is not broken
> beyond repair then it does not deliver the same interrupt to a
> different cpu. Such a thing would always deliver it to all cores and
> those would race to grab the spinlock and mask it. I've seen such
> horror, but don't ask how that performs.
> 
> The reason why you can spare the mask/unmask dance is that the
> controller does not require any action and clears the interrupt when
> the level goes back to inactive. That happens when the device handler
> acks it at the device level.
> 
> Now there might be the case when the device reactivates the interrupt
> before the RETI. But that does not matter as we run the primary
> interrupt handlers with interrupts disabled.

Ok, I'm going to wait a bit to see if Maxime has anything to say
to this second series, and then I'll do a v2 with the commit msg
fixed.

Regards,

Hans


> 
> Thanks,
> 
> 	tglx
>  
>> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> ---
>>  drivers/irqchip/irq-sun4i.c | 11 ++++++++++-
>>  1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/irqchip/irq-sun4i.c b/drivers/irqchip/irq-sun4i.c
>> index a0ed1ea..0a71990 100644
>> --- a/drivers/irqchip/irq-sun4i.c
>> +++ b/drivers/irqchip/irq-sun4i.c
>> @@ -74,8 +74,17 @@ static void sun4i_irq_unmask(struct irq_data *irqd)
>>  	       sun4i_irq_base + SUN4I_IRQ_ENABLE_REG(reg));
>>  }
>>  
>> +/*
>> + * Since sun4i and sun5i are single core SOCs there is no need to mask non
>> + * oneshot IRQs, to achieve this we use handle_fasteoi_irq with a dummy eoi.
>> + */
>> +static void sun4i_irq_dummy_eoi(struct irq_data *irqd)
>> +{
>> +}
>> +
>>  static struct irq_chip sun4i_irq_chip = {
>>  	.name		= "sun4i_irq",
>> +	.irq_eoi	= sun4i_irq_dummy_eoi,
>>  	.irq_mask	= sun4i_irq_mask,
>>  	.irq_unmask	= sun4i_irq_unmask,
>>  };
>> @@ -97,7 +106,7 @@ static int sun4i_irq_map(struct irq_domain *d, unsigned int virq,
>>  					 handle_fasteoi_irq);
>>  	else
>>  		irq_set_chip_and_handler(virq, &sun4i_irq_chip,
>> -					 handle_level_irq);
>> +					 handle_fasteoi_irq);
>>  
>>  	set_irq_flags(virq, IRQF_VALID | IRQF_PROBE);
>>  
>> -- 
>> 1.9.0
>>
>>

WARNING: multiple messages have this Message-ID (diff)
From: Hans de Goede <hdegoede@redhat.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Maxime Ripard <maxime.ripard@free-electrons.com>,
	linux-arm-kernel@lists.infradead.org,
	devicetree <devicetree@vger.kernel.org>,
	linux-kernel@vger.kernel.org, linux-sunxi@googlegroups.com
Subject: Re: [PATCH 1/3] irqchip: sun4i: Don't mask + unmask for the non oneshot case
Date: Fri, 14 Mar 2014 19:42:00 +0100	[thread overview]
Message-ID: <53234D78.5050901@redhat.com> (raw)
In-Reply-To: <alpine.DEB.2.02.1403141228290.18573@ionos.tec.linutronix.de>

Hi,

On 03/14/2014 12:35 PM, Thomas Gleixner wrote:
> On Thu, 13 Mar 2014, Hans de Goede wrote:
> 
>> Since sun4i and sun5i are single core SOCs there is no need to mask non
>> oneshot IRQs, to achieve this we use handle_fasteoi_irq with a dummy eoi.
> 
> This is slightly wrong :)
> 
> Even on a SMP system there is no need to mask the interrupt when the
> controller works like that sunxi one. If the controller is not broken
> beyond repair then it does not deliver the same interrupt to a
> different cpu. Such a thing would always deliver it to all cores and
> those would race to grab the spinlock and mask it. I've seen such
> horror, but don't ask how that performs.
> 
> The reason why you can spare the mask/unmask dance is that the
> controller does not require any action and clears the interrupt when
> the level goes back to inactive. That happens when the device handler
> acks it at the device level.
> 
> Now there might be the case when the device reactivates the interrupt
> before the RETI. But that does not matter as we run the primary
> interrupt handlers with interrupts disabled.

Ok, I'm going to wait a bit to see if Maxime has anything to say
to this second series, and then I'll do a v2 with the commit msg
fixed.

Regards,

Hans


> 
> Thanks,
> 
> 	tglx
>  
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  drivers/irqchip/irq-sun4i.c | 11 ++++++++++-
>>  1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/irqchip/irq-sun4i.c b/drivers/irqchip/irq-sun4i.c
>> index a0ed1ea..0a71990 100644
>> --- a/drivers/irqchip/irq-sun4i.c
>> +++ b/drivers/irqchip/irq-sun4i.c
>> @@ -74,8 +74,17 @@ static void sun4i_irq_unmask(struct irq_data *irqd)
>>  	       sun4i_irq_base + SUN4I_IRQ_ENABLE_REG(reg));
>>  }
>>  
>> +/*
>> + * Since sun4i and sun5i are single core SOCs there is no need to mask non
>> + * oneshot IRQs, to achieve this we use handle_fasteoi_irq with a dummy eoi.
>> + */
>> +static void sun4i_irq_dummy_eoi(struct irq_data *irqd)
>> +{
>> +}
>> +
>>  static struct irq_chip sun4i_irq_chip = {
>>  	.name		= "sun4i_irq",
>> +	.irq_eoi	= sun4i_irq_dummy_eoi,
>>  	.irq_mask	= sun4i_irq_mask,
>>  	.irq_unmask	= sun4i_irq_unmask,
>>  };
>> @@ -97,7 +106,7 @@ static int sun4i_irq_map(struct irq_domain *d, unsigned int virq,
>>  					 handle_fasteoi_irq);
>>  	else
>>  		irq_set_chip_and_handler(virq, &sun4i_irq_chip,
>> -					 handle_level_irq);
>> +					 handle_fasteoi_irq);
>>  
>>  	set_irq_flags(virq, IRQF_VALID | IRQF_PROBE);
>>  
>> -- 
>> 1.9.0
>>
>>

  reply	other threads:[~2014-03-14 18:42 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-03-13 19:49 [PATCH 0/3] irqchip: sun4i: various cleanups Hans de Goede
2014-03-13 19:49 ` Hans de Goede
2014-03-13 19:49 ` Hans de Goede
2014-03-13 19:50 ` [PATCH 1/3] irqchip: sun4i: Don't mask + unmask for the non oneshot case Hans de Goede
2014-03-13 19:50   ` Hans de Goede
2014-03-13 19:50   ` Hans de Goede
2014-03-14 11:35   ` Thomas Gleixner
2014-03-14 11:35     ` Thomas Gleixner
2014-03-14 11:35     ` Thomas Gleixner
2014-03-14 18:42     ` Hans de Goede [this message]
2014-03-14 18:42       ` Hans de Goede
2014-03-14 18:42       ` Hans de Goede
2014-03-13 19:50 ` [PATCH 2/3] irqchip: sun4i: Simplify irq mapping Hans de Goede
2014-03-13 19:50   ` Hans de Goede
2014-03-13 19:50   ` Hans de Goede
2014-03-17 11:10   ` Maxime Ripard
2014-03-17 11:10     ` Maxime Ripard
2014-03-17 11:10     ` Maxime Ripard
2014-03-13 19:50 ` [PATCH 3/3] irqchip: sun4i: simplify sun4i_irq_ack Hans de Goede
2014-03-13 19:50   ` Hans de Goede
2014-03-13 19:50   ` Hans de Goede

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=53234D78.5050901@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.