All of lore.kernel.org
 help / color / mirror / Atom feed
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Meador Inge <meador_inge@mentor.com>
Cc: Hollis Blanchard <hollis_blanchard@mentor.com>,
	devicetree-discuss@lists.ozlabs.org,
	linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH v4 2/2] powerpc: make MPIC honor the "pic-no-reset" device tree property
Date: Fri, 11 Mar 2011 09:13:48 +1100	[thread overview]
Message-ID: <1299795228.22236.508.camel@pasglop> (raw)
In-Reply-To: <4D790901.9000701@mentor.com>

On Thu, 2011-03-10 at 11:23 -0600, Meador Inge wrote:
> AFAIK, we can't rely on 'set_affinity' always being called.  I don't 
> think it is called at all when !defined(CONFIG_SMP) and if it was,
> then that would be an error:
> 
>         /* include/linux/irq.h */
> 
>         #else /* CONFIG_SMP */
> 
>         static inline int irq_set_affinity(unsigned int irq,
>                 const struct cpumask *m)
>         {
>                 return -EINVAL;
>         }

You are right. We do need to set a sane default then.

> > partially initialized in set_type, I'd say just modify set_type to
> > initialize the source as well, and problem solved, no ?
> 
> The priority has to be initialized as well.  They could both be done
> in 
> 'mpic_set_irq_type', but that seems like a weird place since it has 
> nothing to do with actually setting the type.
> 
> Since we already have 'mpic_irq_set_priority' and 'mpic_set_vector', 
> perhaps a better option is to add 'mpic_set_destination' and put the 
> following in 'mpic_host_map' (using the cpuid helper function
> suggested 
> above):
> 
>         /* Lazy source init when MPIC_NO_RESET */
>         if (!mpic_is_ipi(mpic, hw) && (mpic->flags & MPIC_NO_RESET)) {
>                 mpic_set_vector(virq, hw);
>                 mpic_set_destination(virq, mpic_cpuid(mpic));
>                 mpic_irq_set_priority(virq, 8);
>         }
> 
> It is more overhead, but it reads well.  Thoughts?

No objection.

Cheers,
Ben.

WARNING: multiple messages have this Message-ID (diff)
From: Benjamin Herrenschmidt <benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
To: Meador Inge <meador_inge-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
Cc: Hollis Blanchard
	<hollis_blanchard-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
Subject: Re: [PATCH v4 2/2] powerpc: make MPIC honor the "pic-no-reset" device tree property
Date: Fri, 11 Mar 2011 09:13:48 +1100	[thread overview]
Message-ID: <1299795228.22236.508.camel@pasglop> (raw)
In-Reply-To: <4D790901.9000701-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>

On Thu, 2011-03-10 at 11:23 -0600, Meador Inge wrote:
> AFAIK, we can't rely on 'set_affinity' always being called.  I don't 
> think it is called at all when !defined(CONFIG_SMP) and if it was,
> then that would be an error:
> 
>         /* include/linux/irq.h */
> 
>         #else /* CONFIG_SMP */
> 
>         static inline int irq_set_affinity(unsigned int irq,
>                 const struct cpumask *m)
>         {
>                 return -EINVAL;
>         }

You are right. We do need to set a sane default then.

> > partially initialized in set_type, I'd say just modify set_type to
> > initialize the source as well, and problem solved, no ?
> 
> The priority has to be initialized as well.  They could both be done
> in 
> 'mpic_set_irq_type', but that seems like a weird place since it has 
> nothing to do with actually setting the type.
> 
> Since we already have 'mpic_irq_set_priority' and 'mpic_set_vector', 
> perhaps a better option is to add 'mpic_set_destination' and put the 
> following in 'mpic_host_map' (using the cpuid helper function
> suggested 
> above):
> 
>         /* Lazy source init when MPIC_NO_RESET */
>         if (!mpic_is_ipi(mpic, hw) && (mpic->flags & MPIC_NO_RESET)) {
>                 mpic_set_vector(virq, hw);
>                 mpic_set_destination(virq, mpic_cpuid(mpic));
>                 mpic_irq_set_priority(virq, 8);
>         }
> 
> It is more overhead, but it reads well.  Thoughts?

No objection.

Cheers,
Ben.

  parent reply	other threads:[~2011-03-10 22:13 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-02-25 21:59 [PATCH v4 0/2] powerpc: Open PIC binding and "pic-no-reset" Meador Inge
2011-02-25 21:59 ` Meador Inge
2011-02-25 21:59 ` [PATCH v4 1/2] powerpc: document the Open PIC device tree binding Meador Inge
2011-02-28  7:44   ` Grant Likely
2011-02-28  7:44     ` Grant Likely
2011-02-25 21:59 ` [PATCH v4 2/2] powerpc: make MPIC honor the "pic-no-reset" device tree property Meador Inge
2011-02-25 21:59   ` Meador Inge
2011-03-02  3:22   ` Benjamin Herrenschmidt
2011-03-02  3:22     ` Benjamin Herrenschmidt
2011-03-10 17:23     ` Meador Inge
2011-03-10 17:23       ` Meador Inge
2011-03-10 22:11       ` Benjamin Herrenschmidt
2011-03-10 22:11         ` Benjamin Herrenschmidt
2011-03-10 22:13       ` Benjamin Herrenschmidt [this message]
2011-03-10 22:13         ` Benjamin Herrenschmidt

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=1299795228.22236.508.camel@pasglop \
    --to=benh@kernel.crashing.org \
    --cc=devicetree-discuss@lists.ozlabs.org \
    --cc=hollis_blanchard@mentor.com \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=meador_inge@mentor.com \
    /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.