On Tue, 21 Apr 2015 00:59:35 +0000 "Zha, Qipeng" <qipeng.zha@xxxxxxxxx> wrote: > Hi Alan > > Fei, owner of intel_scu_ipc.c, suggested separate these two, since they are different hw, > Not prefer to manage them in one driver. They don't look that different to me except that it lacks some of the cleanups as well as the naming to make it more Linxulike (intel_scu_ipc_iowrite16 etc) that were done to the scu_ipc code. What we really want to avoid is code elsewhere that ends up doing if (broxton) intel_pmc_ipc_foo(x) else intel_scu_ipc_foo(x); I guess to judge that really means seeing who uses these routines. At the moment the user you've posted takes the existing not very pretty API and wraps it up again in another one with no obvious reason for all the layering, but it does at least then provide the upper layer as a sort of abstraction. For the code itself - there are quite a few things that could be const - you keep the register bases in both the platform resources of the subdevices and directly, yet don't seem to use them directly ? Alan -- To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html