linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Meador Inge <meador_inge@mentor.com>
To: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Hollis Blanchard <hollis_blanchard@mentor.com>,
	devicetree-discuss@lists.ozlabs.org,
	linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH v3 1/4] powerpc: Removing support for 'protected-sources'
Date: Mon, 07 Feb 2011 18:32:39 -0600	[thread overview]
Message-ID: <4D508F27.8060307@mentor.com> (raw)
In-Reply-To: <1297115136.14982.82.camel@pasglop>

On 02/07/2011 03:45 PM, Benjamin Herrenschmidt wrote:
>
>> In my previous reply I said that "it is not so much as a need as it is a
>> potential simplification."  After further reflection, I don't think that
>> is completely true.  As we get into AMP systems with higher core counts,
>> then implementing this functionality using the existing
>> "protected-sources" implementation versus the new "pic-no-reset" work is
>> going to be harder to maintain.
>
> I'm not arguing that your approach isn't more suitable for AMP systems,
> I just want to leave the existing protected-sources mechanism alone. I'm
> not opposing adding a new, better, mechanism for newer platforms.

Is the mechanism mentioned earlier of having "protected-sources" as a 
synonym for "pic-no-reset" not suitable?  Or would you like the current 
protected sources implementation left completely intact?

> However, I'd name it differently. "pic-no-reset" doesn't carry enough
> meaning in that case. What we want to point out here is that the PIC
> has been pre-initialized.
>
> Another option, which may be cleaner, is to stick to "no-reset" (no need
> for pic- prefix) and make it do just that (prevent the reset), and then

It originally was "no-reset", but that was considered too broad. [1] :)

> use a positive variant of "protected-sources", call it
> "allowed-sources". Maybe even make it a series of ranges. Then have the
> MPIC only access these.

That would work, but I still don't like having to mention this 
information twice in the device tree.  All the sources encoded in the 
various "interrupts" properties _are_ the allowed sources, right?

> I think this is more robust as it would also prevent "accidental" use of
> the wrong sources (bad device-tree, drivers that let you muck around
> with irq numbers, etc...).

That would be nice.  All though, it may not be as helpful as it sounds. 
  There is as much of a risk that someone will botch the 
"allowed-sources" property as there is they will botch the "interrupts" 
property.  We could perhaps still preform these checks without the extra 
property: if a source is not mentioned in an interrupts property, then 
it is not allowed.

> Cheers,
> Ben.
>

[1] http://lists.ozlabs.org/pipermail/linuxppc-dev/2011-February/088244.html

-- 
Meador Inge     | meador_inge AT mentor.com
Mentor Embedded | http://www.mentor.com/embedded-software

  reply	other threads:[~2011-02-08  0:32 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-02-04 23:25 [PATCH v3 0/4] powerpc: Open PIC binding and "pic-no-reset" Meador Inge
2011-02-04 23:25 ` [PATCH v3 1/4] powerpc: Removing support for 'protected-sources' Meador Inge
2011-02-06 23:35   ` Benjamin Herrenschmidt
2011-02-07  1:32     ` Meador Inge
2011-02-07  1:37       ` Benjamin Herrenschmidt
2011-02-07 18:02     ` Meador Inge
2011-02-07 21:45       ` Benjamin Herrenschmidt
2011-02-08  0:32         ` Meador Inge [this message]
2011-02-08 15:13         ` Yoder Stuart-B08248
2011-02-04 23:25 ` [PATCH v3 2/4] powerpc: document the Open PIC device tree binding Meador Inge
2011-02-04 23:25 ` [PATCH v3 3/4] powerpc: make MPIC honor the "pic-no-reset" device tree property Meador Inge
2011-02-04 23:25 ` [PATCH v3 4/4] powerpc: Replacing "protected-sources" with "pic-no-reset" in DTS files Meador Inge
     [not found] ` <AANLkTinda9TX+Ng=kL-HHLOdqRnUZ6uitQKyZcRUHVco@mail.gmail.com>
2011-02-11  2:01   ` [PATCH v3 0/4] powerpc: Open PIC binding and "pic-no-reset" Meador Inge
2011-02-11  3:26     ` Meador Inge
2011-02-11 14:58       ` Yoder Stuart-B08248
2011-02-11 17:35         ` Meador Inge
2011-02-11 18:41         ` Scott Wood
2011-02-11 18:59           ` Grant Likely

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=4D508F27.8060307@mentor.com \
    --to=meador_inge@mentor.com \
    --cc=benh@kernel.crashing.org \
    --cc=devicetree-discuss@lists.ozlabs.org \
    --cc=hollis_blanchard@mentor.com \
    --cc=linuxppc-dev@lists.ozlabs.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).