On Tue, Aug 27, 2013 at 5:45 PM, Kevin Hilman <khilman@xxxxxxxxxx> wrote: [...] > > Looking closer at this code as I'm trying to fully get my head around > all the IPC, I have some more comments. > > I think the split between pm33xx.c and the M3 driver is still confusing > here. For example, am33xx_ping_wkup_m3(), > am33xx_m3_state_machine_reset() and the guts of am33xx_pm_begin() all > belong inside the M3 driver, along with all the wakeup_src stuff, which > is info coming from the M3. > > IOW, the communication with M3 should be abstracted from pm33xx by the > M3 driver (or possibly an eventual remoteproc/rpmsg implementation) with > a well defined API. In this implementation, the interface is pretty > fuzzy and mixed between pm33xx.c and wkup_m3.c. > > The reason for the current split was to have the M3 driver just do the minimal that's needed to talk to get M3 and MPU talking. What made me do it this way was to attempt to address a previous comment on keeping options open for folks to use M3 for things other than PM stuff. The IPC stuff is how implementors of the firmware (anything other than the PM one that TI provides) want it to be. The top level idea was to have the users of the firmware (PM in this case) decide what functionality they want when talking to M3. They are also free to decide the register bitfield layout and other IPC details. This was also a feeble attempt to keep things extensible for AM437x where in addition to the broken mailbox usage there's now a control module bit to trigger the interrupt to M3 (what's worse? pick one that you hate more ;) The AM437x PM routines could then just register different callbacks that are triggered when the M3 interrupts the MPU. Hope this clears up some of the confusion. Regards, Vaibhav -- 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
