Re: [PATCH v3 1/4] powerpc: Removing support for 'protected-sources'

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".