linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: marc.zyngier@arm.com (Marc Zyngier)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] Re: Linkstation Mini and __machine_arch_type problem, not booting since 3.8
Date: Fri, 19 Jun 2015 10:13:07 +0100	[thread overview]
Message-ID: <5583DD23.90505@arm.com> (raw)
In-Reply-To: <1434677895.4767.11.camel@dolka.fr>

On 19/06/15 02:38, Benjamin Cama wrote:
> Hi Marc,
> 
> Le jeudi 18 juin 2015 ? 08:52 +0100, Marc Zyngier a ?crit :
>> On 18/06/15 03:12, Benjamin Cama wrote:
>>> And I did it. And I stumbled upon commit
>>> a71b092a9c68685a270ebdde7b5986ba8787e575 ?ARM: Convert handle_IRQ to
>>> use __handle_domain_irq? (author Cc'ed). The new function
>>> __handle_domain_irq (in kernel/irq/irqdesc.c, which comes just two
>>> commits before, with 76ba59f8366f2d9282cb5bda9de75b4b68cbe55f) is
>>> subtly different from the one it replaces, handle_IRQ, as it checks if
>>> the irq is not 0 as well as checking for an upper bound. Removing the
>>> check for 0 makes my machine work again! But honestly, I do not know if
>>> a zero irq is legit, so I hope some more knowledgeable people will tell
>>> me if this is OK.
>>>
>>> -- >8 --
>>> Subject: [PATCH] Make __handle_domain_irq accept IRQ 0
>>>
>>> The same as handle_IRQ did before.
>>>
>>> Signed-off-by: Benjamin Cama <benoar@dolka.fr>
>>> ---
>>>  kernel/irq/irqdesc.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
>>> index a1782f8..bfbeeb6 100644
>>> --- a/kernel/irq/irqdesc.c
>>> +++ b/kernel/irq/irqdesc.c
>>> @@ -365,7 +365,7 @@ int __handle_domain_irq(struct irq_domain 
>>> *domain, unsigned int hwirq,
>>>          * Some hardware gives randomly wrong interrupts.  Rather
>>>          * than crashing, do something sensible.
>>>          */
>>> -       if (unlikely(!irq || irq >= nr_irqs)) {
>>> +       if (unlikely(irq >= nr_irqs)) {
>>>                 ack_bad_irq(irq);
>>>                 ret = -EINVAL;
>>>         } else {
>>>
>>
>> Unfortunately, this is the wrong thing to do. IRQ0 is invalid, has been
>> for a very long time, and actually represents the lack of interrupt.
> 
> OK, sorry for the mistake, I didn't know. Shouldn't the IRQ
> initialization routine check this and warn the user that it may cause
> problems? ?Silently? making IRQ0 forbidden doesn't help for the
> platforms that are not fixed yet.

Well, this is hardly a new rule. It has been like this for quite a long
time: http://yarchive.net/comp/linux/zero.html

As for the checking and warning, this is on a very hot path, for an
error case that really shouldn't occur.

<rant>
I've also come to the conclusion that people often choose to ignore
warnings. It boots, so who cares? Funnily enough, they react when their
toy doesn't work any more.
</rant>

> 
>> The way you can address this is by making sure your favourite platform
>> does not use IRQ0 at all, which is done by not assuming that Linux IRQ
>> number (which is always completely virtual) is the same as the number
>> designating the actual HW interrupt line.
>>
>> For example, have a look at 18f3aec (ARM: 8230/1: sa1100: shift IRQs by
>> one) for an example of such a (very simple) conversion. You'll need to
>> tweak irq.c too.
>>
>> Other commits for sa1100 will hopefully convince you to switch to irq
>> domains altogether. This will greatly facilitate a possible further
>> transition to DT if you wish to do so.
> 
> I read quite a bit about all this virtual/hardware IRQ stuff, and the
> irq domains, interesting. I see that orion5x seems to be one of the
> last platforms not using it (appart from DT-converted boards); is it a
> bad sign about its durability??

No, we have platforms that will most probably never be converted to DT,
and they are very far from rotting in the tree.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

  reply	other threads:[~2015-06-19  9:13 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-15 13:51 Linkstation Mini and __machine_arch_type problem, not booting since 3.8 Benjamin Cama
2015-06-16  9:20 ` Benjamin Cama
2015-06-18  2:12   ` [PATCH] " Benjamin Cama
2015-06-18  7:52     ` Marc Zyngier
2015-06-18  8:14       ` Arnd Bergmann
2015-06-18 13:23         ` Andrew Lunn
2015-06-19  1:38       ` Benjamin Cama
2015-06-19  9:13         ` Marc Zyngier [this message]
2015-06-19 12:16           ` Benjamin Cama
2015-06-19 13:01             ` Jason Cooper
2015-06-19 13:13             ` Russell King - ARM Linux
2015-06-19 13:46               ` Benjamin Cama
2015-06-19 15:25                 ` Jason Cooper
2015-06-19 15:48                   ` Russell King - ARM Linux
2015-06-19 17:13                     ` Jason Cooper
2015-06-21 17:37                       ` Benjamin Cama
2015-06-22 12:08                         ` Jason Cooper
2015-06-22 17:49                           ` Benjamin Cama
2015-06-22 17:58                             ` Russell King - ARM Linux
2015-06-22 18:04                             ` Jason Cooper
     [not found]                               ` <1436710916.5657.169.camel@dolka.fr>
2015-07-14 14:25                                 ` [PATCH] ARM: orion5x: fix legacy orion5x IRQ numbers Benjamin Cama
2015-07-14 20:50                                   ` Arnd Bergmann
2015-08-14 15:46                                     ` Gregory CLEMENT
2015-06-19 15:44                 ` [PATCH] Re: Linkstation Mini and __machine_arch_type problem, not booting since 3.8 Russell King - ARM Linux
2015-06-20  1:01                   ` Benjamin Cama
2015-06-18  8:12 ` Gregory CLEMENT
2015-06-19  1:50   ` Benjamin Cama
2015-06-19  9:33     ` Gregory CLEMENT
2015-06-19 11:41       ` Jason Cooper
2015-06-20  0:28         ` Benjamin Cama
2015-06-20 14:36           ` Andrew Lunn
2015-06-21 17:36             ` Benjamin Cama
2015-06-21 20:07               ` Andrew Lunn
     [not found]                 ` <1434995000.5657.42.camel@dolka.fr>
2015-06-22 18:23                   ` SERIAL_OF_PLATFORM default setting for DT headless systems Jason Cooper
2015-06-22 19:42                     ` Benjamin Cama
2015-06-22 12:00               ` Linkstation Mini and __machine_arch_type problem, not booting since 3.8 Jason Cooper
2015-06-22 17:44                 ` Benjamin Cama
2015-06-19 22:38     ` Andrew Lunn

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=5583DD23.90505@arm.com \
    --to=marc.zyngier@arm.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).