On Sun, Jun 17, 2018 at 11:37 PM Lee Jones <lee.jo...@linaro.org> wrote: > > On Mon, 04 Jun 2018, Enrico Granata wrote: > > > From: Enrico Granata <egran...@chromium.org> > > > > If one builds the Chrome EC support as modules, there is no explicit > > dependency chain amongst the several modules (LPC, CHARDEV, CORE, ...) > > > > This makes it possible - for instance - to rmmod cros_ec_core, even though > > cros_ec_dev actively uses it for data transport to the EC chip. > > > > This commit makes two changes in an attempt to address this: > > a) moves cros_ec_proto.c as part of the CORE module; this removes the > > possibility of unloading cros_ec_core while cros_ec_dev is using it; > > b) enables cros_ec_core to explicitly register a runtime dependency on > > the kernel module that is using its cmd and pkt transfer functions > > > > Series-to: LKML <linux-kernel@vger.kernel.org> > > Series-cc: gwen...@google.com > > Signed-off-by: Enrico Granata <egran...@chromium.org> > > --- > > drivers/mfd/Kconfig | 1 - > > drivers/mfd/Makefile | 2 +- > > drivers/mfd/cros_ec.c | 18 ++++++++++++++++-- > > drivers/mfd/cros_ec_i2c.c | 1 + > > drivers/{platform/chrome => mfd}/cros_ec_proto.c | 0 > > drivers/mfd/cros_ec_spi.c | 1 + > > drivers/platform/chrome/Kconfig | 5 ----- > > drivers/platform/chrome/Makefile | 1 - > > drivers/platform/chrome/cros_ec_lpc.c | 1 + > > include/linux/mfd/cros_ec.h | 2 ++ > > 10 files changed, 22 insertions(+), 10 deletions(-) > > rename drivers/{platform/chrome => mfd}/cros_ec_proto.c (100%) > > > > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > > index b860eb5aa194..a8daa2072ae6 100644 > > --- a/drivers/mfd/Kconfig > > +++ b/drivers/mfd/Kconfig > > @@ -194,7 +194,6 @@ config MFD_CROS_EC > > tristate "ChromeOS Embedded Controller" > > select MFD_CORE > > select CHROME_PLATFORMS > > - select CROS_EC_PROTO > > depends on X86 || ARM || ARM64 || COMPILE_TEST > > help > > If you say Y here you get support for the ChromeOS Embedded > > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile > > index d9d2cf0d32ef..20537bd27695 100644 > > --- a/drivers/mfd/Makefile > > +++ b/drivers/mfd/Makefile > > @@ -12,7 +12,7 @@ obj-$(CONFIG_MFD_SM501) += sm501.o > > obj-$(CONFIG_MFD_ASIC3) += asic3.o tmio_core.o > > obj-$(CONFIG_MFD_BCM590XX) += bcm590xx.o > > obj-$(CONFIG_MFD_BD9571MWV) += bd9571mwv.o > > -cros_ec_core-objs := cros_ec.o > > +cros_ec_core-objs := cros_ec.o cros_ec_proto.o > > cros_ec_core-$(CONFIG_ACPI) += cros_ec_acpi_gpe.o > > obj-$(CONFIG_MFD_CROS_EC) += cros_ec_core.o > > obj-$(CONFIG_MFD_CROS_EC_I2C) += cros_ec_i2c.o > > diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c > > index d61024141e2b..9660dc0fc079 100644 > > --- a/drivers/mfd/cros_ec.c > > +++ b/drivers/mfd/cros_ec.c > > @@ -168,8 +168,19 @@ int cros_ec_register(struct cros_ec_device *ec_dev) > > */ > > err = cros_ec_sleep_event(ec_dev, 0); > > if (err < 0) > > - dev_dbg(ec_dev->dev, "Error %d clearing sleep event to ec", > > - err); > > + dev_dbg(dev, "Error %d clearing sleep event to ec", err); > > This change is unrelated to the patch. > > > + if (ec_dev->xfer_fcn_owner != NULL) { > > if (ec_dev->xfer_fcn_owner) ? > > > + err = try_module_get(ec_dev->xfer_fcn_owner); > > + if (err < 0) { > > This logic is wrong. try_module_get() returns a bool.
I do not think this is needed at all: the transport drivers (i2c, api) are using cros_ec_register() provided by cros_ec core, which should bump up refcount on cros_ec core module preventing it from unloading. And unloading transport driver should unregister EC instance, stopping IO. If cros_ec_dev is not prepared to handle "dead" devices - it is issue for cros_ec_dev to solve. By the way, it is not solvable my disallowing module unloading: one can always tear down a device by manually unbinding driver via sysfs. Thanks. -- Dmitry