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.
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
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.
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...).
Cheers,
Ben.
The reason being that *every* OS instance has to know about *every*
other OSes interrupt sources, which is a little gross. You can see this
happening already in "arch/powerpc/boot/dts/p2020rdb_camp_core0.dts" and
"arch/powerpc/boot/dts/p2020rdb_camp_core1.dts":
// p2020rdb_camp_core0.dts
mpic: pic@40000 {
...
// Sources used by the OS on core 1
protected-sources = <
42 76 77 78 79 /* serial1 , dma2 */
29 30 34 26 /* enet0, pci1 */
0xe0 0xe1 0xe2 0xe3 /* msi */
0xe4 0xe5 0xe6 0xe7
>;
};
// p2020rdb_camp_core1.dts
mpic: pic@40000 {
...
// Sources used by the OS on core 0
protected-sources = <
17 18 43 42 59 47 /*ecm, mem, i2c, serial0, spi,gpio */
16 20 21 22 23 28 /* L2, dma1, USB */
03 35 36 40 31 32 33 /* mdio, enet1, enet2 */
72 45 58 25 /* sdhci, crypto , pci */
>;
};
It is going to be a real pain to keep all of the lists up to date.
Especially considering we already have sufficient information in the
device tree to do this work. I do understand the concern of
finding/testing the older systems. However, is the testing of those
systems enough to keep out the proposed change and potentially lower
maintenance in the future? Is the legacy system argument the only
reason to keep this change out or are there other technical deficiencies?
Also, in the proposed MPIC modifications there is a check for protected
sources (it is treated as an alias for "pic-no-reset"; see PATCH 3 in
the set) that should provide functionality equivalent to what systems
using "protected-sources" already have. That check only looks for the
presence of "protected-sources" and does not process the cells. Another
option would be to leave in the protected sources implementation (but
undocumented in the binding) and have the full "pic-no-reset" behavior
there as well (and documented in the binding).
If this has no chance of acceptance (?), then I will just re-submit the
binding and implementation with "protected-sources" and the limited form
of "pic-no-reset".
On 02/07/2011 03:45 PM, Benjamin Herrenschmidt wrote:
quoted
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
-----Original Message-----
From: devicetree-discuss-
bounces+stuart.yoder=3Dfreescale.com@lists.ozlabs.org [mailto:devicetree-
discuss-bounces+stuart.yoder=3Dfreescale.com@lists.ozlabs.org] On Behalf =
Of
Benjamin Herrenschmidt
Sent: Monday, February 07, 2011 3:46 PM
To: Meador Inge
Cc: Hollis Blanchard; devicetree-discuss@lists.ozlabs.org; linuxppc-
dev@lists.ozlabs.org
Subject: Re: [PATCH v3 1/4] powerpc: Removing support for 'protected-
sources'
=20
=20
quoted
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.
=20
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 no=
t
opposing adding a new, better, mechanism for newer platforms.
=20
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.
=20
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 u=
se
a positive variant of "protected-sources", call it "allowed-sources". May=
be
even make it a series of ranges. Then have the MPIC only access these.
=20
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...).
What is the use case for "allowed-sources"? For AMP the only device
nodes in your device tree should be your AMP partition's devices,
thus any interrupt specifiers in your dev tree are "allowed".
The MPIC is a shared device and thus the need for no-reset.
So, all newer platforms should need is "no-reset".
Stuart