On Mon, Jul 12, 2021 at 05:02:42PM +0200, Sebastian Reichel wrote: > General Electric Healthcare's PPD has a secondary processor from > NXP's Kinetis K20 series. That device has two SPI chip selects: > > The main interface's behaviour depends on the loaded firmware > and is currently unused. > > The secondary interface can be used to update the firmware using > EzPort protocol. This is implemented by this driver using the > kernel's firmware API. It's not done during probe time, since > the device has non-volatile memory and flashing lasts almost 3 > minutes. In thinking about this some more, why does it matter? Spin up a workqueue when probing and do the firmware loading then. That way you do not end up creating yet another custom user/kernel api just to do something as trivial as loading the firmware for a device. And I think the firmware loader even handles async loading, or at least it used to, maybe not anymore, it's a complex api, I recommend reading the docs... > > Signed-off-by: Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxx> > --- > .../ABI/testing/sysfs-driver-ge-achc | 14 + > drivers/misc/Kconfig | 11 + > drivers/misc/Makefile | 2 + > drivers/misc/gehc-achc.c | 136 +++++ > drivers/misc/nxp-ezport.c | 468 ++++++++++++++++++ > drivers/misc/nxp-ezport.h | 9 + Why is there two different modules here for the same piece of hardware? > drivers/spi/spidev.c | 1 - > 7 files changed, 640 insertions(+), 1 deletion(-) > create mode 100644 Documentation/ABI/testing/sysfs-driver-ge-achc > create mode 100644 drivers/misc/gehc-achc.c > create mode 100644 drivers/misc/nxp-ezport.c > create mode 100644 drivers/misc/nxp-ezport.h > > diff --git a/Documentation/ABI/testing/sysfs-driver-ge-achc b/Documentation/ABI/testing/sysfs-driver-ge-achc > new file mode 100644 > index 000000000000..f02812731082 > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-driver-ge-achc > @@ -0,0 +1,14 @@ > +What: /sys/bus/spi/<dev>/update_firmware > +Date: Jul 2021 > +Contact: sebastian.reichel@xxxxxxxxxxxxx > +Description: Write 1 to this file to update the ACHC microcontroller > + firmware via the EzPort interface. For this the kernel > + will load "achc.bin" via the firmware API (so usually > + from /lib/firmware). The write will block until the FW > + has either been flashed successfully or an error occured. > + > +What: /sys/bus/spi/<dev>/reset > +Date: Jul 2021 > +Contact: sebastian.reichel@xxxxxxxxxxxxx > +Description: Write 1 to this file to reset the microcontroller via the > + reset GPIO. The write will block until the reset completes. > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig > index f4fb5c52b863..1ebf7000671d 100644 > --- a/drivers/misc/Kconfig > +++ b/drivers/misc/Kconfig > @@ -208,6 +208,17 @@ config CS5535_CLOCK_EVENT_SRC > MFGPTs have a better resolution and max interval than the > generic PIT, and are suitable for use as high-res timers. > > +config GEHC_ACHC > + tristate "GEHC ACHC support" > + depends on SPI && SYSFS > + select FW_LOADER > + help > + Support for GE ACHC microcontroller, that is part of the GE > + PPD device. > + > + To compile this driver as a module, choose M here: the > + module will be called gehc-achc. > + > config HP_ILO > tristate "Channel interface driver for the HP iLO processor" > depends on PCI > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile > index e92a56d4442f..b50180796931 100644 > --- a/drivers/misc/Makefile > +++ b/drivers/misc/Makefile > @@ -24,6 +24,8 @@ obj-$(CONFIG_KGDB_TESTS) += kgdbts.o > obj-$(CONFIG_SGI_XP) += sgi-xp/ > obj-$(CONFIG_SGI_GRU) += sgi-gru/ > obj-$(CONFIG_CS5535_MFGPT) += cs5535-mfgpt.o > +obj-$(CONFIG_GEHC_ACHC) += gehc-achc-mod.o > +gehc-achc-mod-objs := gehc-achc.o nxp-ezport.o Yeah, you are creating 2 modules for one Kconfig option, why not put these together into 1? Makes the code smaller. > obj-$(CONFIG_HP_ILO) += hpilo.o > obj-$(CONFIG_APDS9802ALS) += apds9802als.o > obj-$(CONFIG_ISL29003) += isl29003.o > diff --git a/drivers/misc/gehc-achc.c b/drivers/misc/gehc-achc.c > new file mode 100644 > index 000000000000..893940e7ca4e > --- /dev/null > +++ b/drivers/misc/gehc-achc.c > @@ -0,0 +1,136 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * datasheet: https://www.nxp.com/docs/en/data-sheet/K20P144M120SF3.pdf > + * > + * Copyright (C) 2018-2021 Collabora > + * Copyright (C) 2018-2021 GE Healthcare > + */ > + > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/spi/spi.h> > +#include <linux/of.h> > +#include "nxp-ezport.h" > + > +#define ACHC_MAX_FREQ_HZ 300000 > + > +struct achc_data { > + struct spi_device *main; > + struct spi_device *ezport; > + struct gpio_desc *reset; > + > + struct mutex device_lock; /* avoid concurrent device access */ > +}; > + > +static ssize_t update_firmware_store(struct device *dev, struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct achc_data *achc = dev_get_drvdata(dev); > + int ret; > + > + if (count != 1 || buf[0] != '1') > + return -EINVAL; There a core kernel call to see if the data written to a sysfs file is "true/false" I would recommend, if you stick with this, to use that instead. thanks, greg k-h
