Hi Bjorn, >>>> Thanks for the patch. >> >> On 06/27/2017 01:43 AM, Bjorn Andersson wrote: >>> While it's very common to use RPMSG for communicating with firmware >>> running on these remoteprocs there is no functional dependency on RPMSG. >> >> This is not entirely accurate though. RPMSG is the IPC transport on >> these remoteprocs, you seem to suggest that there are alternatives for >> these remoteprocs. Without RPMSG, you can boot, but you will not be able >> to talk to the remoteprocs, so I would call it a functional dependency. >> > > IMHO this functional dependency is not from the remoteproc driver but > your system (and firmware) design. It should be possible to write > firmware that exposes any other type of virtio device. You may want to add this clarification to your commit description then. > >>> As such RPMSG should be selected by the system integrator and not >>> automatically by the remoteproc drivers. >>> >>> This does solve problems reported with circular Kconfig dependencies for >>> Davinci and Keystone remoteproc drivers. >> >> The Keystone one issue shows up on linux-next (and not on 4.12-rcX) due >> to the differing options on RESET_CONTROLLER on VIDEO_QCOM_VENUS >> (through QCOM_SCOM). > > That's probably why I didn't see this when build testing before pushing > this. > >> This can also be resolved by changing the depends on RESET_CONTROLLER >> to a select RESET_CONTROLLER or dropping the line. >> > > Except that this would violate the general rule of "don't use 'select' > for user-selectable options". Yeah well, there seems to all sorts of usage w.r.t RESET_CONTROLLER and VIRTIO. And if the ARCHs enable the ARCH_HAS_RESET_CONTROLLER, the RESET_CONTROLLER dependencies are not even needed. A quick grep on 4.12-rc7 yielded 20 occurrences that uses depends on and 21 for selects RESET_CONTROLLER. Similar numbers on VIRTIO are 9 and 9 (with a split between different VIRTIO drivers even). > >> The davinci one is tricky though, as I did change it from using a select >> to a depends on dependency, and obviously ppc64_defconfig is something >> that I would not check. Posted a cleanup series that removes the VIRTUALIZATION dependency from REMOTEPROC and RPMSG_VIRTIO options, the latter fixes the ppc64_defconfig issue with davinci remoteproc. >> > > While I hate the process of figuring out and enable all the dependencies > to be able to enable a particular config, this change does reduce the > risk of running into more of these cyclic dependencies. > >> This patch definitely resolves both issues, but it is not obvious that >> someone would also have to enable RPMSG_VIRTIO to have these remoteprocs >> useful when looking at either of the menuconfig help. >> > > This is a common issue we have with config options and while I hate to > add another item to the list of things that you can miss in your system > configuration I would prefer that my Kconfig is maintainable. > > This also means that most remoteproc drivers should "depends on MAILBOX" > and not select either the framework or the specific drivers. But let's > try to ignore that until after the merge window. Yeah ok, as long as you follow a consistent rule across all remoteproc/rpmsg drivers, that's fine. In fact, other than RPMSG_VIRTIO, the two drivers I added for this merge window do use/switches to a depends on convention. regards Suman
