On Wed, Sep 10, 2014 at 12:10 AM, Kevin Hilman <khilman@xxxxxxxxxx> wrote: > What I think you need to do (and what I've recommended at least once in > earlier reviews) put all the (non-rproc) wkup_m3 IPC into into one > driver and create a well-described, well-documented API that the > platform PM code will use. > > IMO, the current "split" is very difficult to read/understand, which > means it will even more difficult to maintain. I strongly agree. A remoteproc driver should generally only register its hardware-specific implementation of the rproc_ops via the remoteproc framework. It is not expected to expose public API of its own - that's why we have the generic remoteproc layer for. It makes perfect sense not to use rpmsg for communications if it's not lightweight enough, but exposing a new set of IPC API should take place in a separate well-documented driver. In addition, the suggested remoteproc driver seems to act both as a low-level hardware-specific driver that plugs into remoteproc (by registering rproc_ops via the remoteproc framework), and also as a high-level driver that utilizes the remoteproc public API (by calling rproc_boot). This alone calls for scrutinizing the design as this is not very intuitive. At least for the remoteproc part: if you could provide a simple and straight-forward remoteproc driver that just implements the rproc_ops, this could be merged very quickly. The rest of the code most probably belongs in a different entity, just like Kevin suggested. Thanks, Ohad. -- 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
