Nishanth Menon <nm@xxxxxx> writes: > Kevin Hilman had written, on 11/19/2010 03:20 PM, the following: > >>> Request for testing this series for comparison between master and this >>> series requested for additional platforms where available. >> >> I haven't yet been through the entire series, but some general comments >> to share before the weekend: > thanks for comments so far. I will wait for the complete series to be > reviewed before reposting a v2. > >> >> The secure mode code is growing in size and complexity, so I think it >> should be removed from pm34xx.c and moved into it's own file >> (secure3xxx.c ?) This will help keep pm34xx.c lean, and it can call >> into secure code as needed. > > IMHO - we should do that set of cleanups as part of Jean's patch > series where we transition to sdram where possible - that will allow > us to have C code replacement for sleep34xx.S and optimize better in > conjunction with sram_idle function and helpers. No, I'd like to see the secure code in your patch all in a separate file. Jean's cleanups are independent of better organization and structure of your code. >> Even better (and already suggested in some comments on patch 8), now >> that there are board-configurable knobs, this should be set up as a very >> simple platform driver/device so boards can pass configuration in a >> standard way instead of having new APIs for use by board code to set >> configuration settings. > > in this specific context, having a platform data device is more of an > overkill - 90% of the HS platforms (just a guess based on the limited > devices I know of and is not a hard statistics) use the TI defaults - > we do have exceptional customers who do their own PPA - and having a > device-driver model IMHO is an overkill for that flexibility. The board-level configuration is only one (minor) reason to have a separate driver for the secure stuff. Discussions on the other patches suggested other reasons for this as well, including some reasons from you as well. :) Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
