Messages in this thread | | | Date | Thu, 27 Jan 2022 10:40:18 +0100 | From | Greg KH <> | Subject | Re: [PATCH 1/2] drivers/misc: add a driver for HPS |
| |
On Thu, Jan 27, 2022 at 07:35:44PM +1100, Sami Kyöstilä wrote: > This patch introduces a driver for the ChromeOS snooping protection > sensor (aka. HPS). The driver supports a sensor connected to the I2C bus > and identified as "GOOG0020" in the ACPI tables. > > When loaded, the driver exports the sensor to userspace through a > character device. This initial version of the device only supports power > management, i.e., communicating with the sensor must be done through I2C > from userspace. > > Power management is implemented by enabling the respective power GPIO > while at least one userspace process holds an open fd on the character > device. By default, the device is powered down if there are no active > clients. > > Note that the driver makes no effort to preserve the state of the sensor > between power down and power up events. Userspace is responsible for > reinitializing any needed state once power has been restored. > > The device firmware, I2C protocol and other documentation is available > at https://chromium.googlesource.com/chromiumos/platform/hps-firmware.
How about a userspace tool that interacts with this new ioctl interface as well so that we can understand how the driver is supposed to work?
> > Signed-off-by: Sami Kyöstilä <skyostil@chromium.org> > --- > > MAINTAINERS | 6 ++ > drivers/misc/Kconfig | 10 ++ > drivers/misc/Makefile | 1 + > drivers/misc/hps-i2c.c | 223 +++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 240 insertions(+) > create mode 100644 drivers/misc/hps-i2c.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index ea3e6c914384..9dea4b8c2ab5 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -8798,6 +8798,12 @@ S: Maintained > W: http://artax.karlin.mff.cuni.cz/~mikulas/vyplody/hpfs/index-e.cgi > F: fs/hpfs/ > > +HPS (ChromeOS snooping protection sensor) DRIVER > +M: Sami Kyöstilä <skyostil@chromium.org> > +R: Evan Benn <evanbenn@chromium.org> > +S: Maintained > +F: drivers/misc/hps-i2c.c > + > HSI SUBSYSTEM > M: Sebastian Reichel <sre@kernel.org> > S: Maintained > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig > index 0f5a49fc7c9e..b48b7803f537 100644 > --- a/drivers/misc/Kconfig > +++ b/drivers/misc/Kconfig > @@ -244,6 +244,16 @@ config HP_ILO > To compile this driver as a module, choose M here: the > module will be called hpilo. > > +config HPS_I2C > + tristate "ChromeOS HPS device support" > + depends on HID && I2C && PM > + help > + Say Y here if you want to enable support for the ChromeOS > + anti-snooping sensor (HPS), attached via I2C. The driver supports a > + sensor connected to the I2C bus and exposes it as a character device. > + To save power, the sensor is automatically powered down when no > + clients are accessing it. > + > config QCOM_COINCELL > tristate "Qualcomm coincell charger support" > depends on MFD_SPMI_PMIC || COMPILE_TEST > diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile > index a086197af544..162a7d530dab 100644 > --- a/drivers/misc/Makefile > +++ b/drivers/misc/Makefile > @@ -26,6 +26,7 @@ obj-$(CONFIG_SGI_GRU) += sgi-gru/ > obj-$(CONFIG_CS5535_MFGPT) += cs5535-mfgpt.o > obj-$(CONFIG_GEHC_ACHC) += gehc-achc.o > obj-$(CONFIG_HP_ILO) += hpilo.o > +obj-$(CONFIG_HPS_I2C) += hps-i2c.o > obj-$(CONFIG_APDS9802ALS) += apds9802als.o > obj-$(CONFIG_ISL29003) += isl29003.o > obj-$(CONFIG_ISL29020) += isl29020.o > diff --git a/drivers/misc/hps-i2c.c b/drivers/misc/hps-i2c.c > new file mode 100644 > index 000000000000..fe9f073b0352 > --- /dev/null > +++ b/drivers/misc/hps-i2c.c > @@ -0,0 +1,223 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Driver for the ChromeOS anti-snooping sensor (HPS), attached via I2C. > + * > + * The driver exposes HPS as a character device, although currently no read or > + * write operations are supported. Instead, the driver only controls the power > + * state of the sensor, keeping it on only while userspace holds an open file > + * descriptor to the HPS device. > + * > + * Copyright 2022 Google LLC. > + */ > + > +#include <linux/acpi.h> > +#include <linux/cdev.h> > +#include <linux/fs.h> > +#include <linux/gpio/consumer.h> > +#include <linux/i2c.h> > +#include <linux/module.h> > +#include <linux/pm_runtime.h> > + > +#define HPS_ACPI_ID "GOOG0020" > +#define HPS_MAX_DEVICES 1 > + > +struct hps_drvdata { > + struct i2c_client *client; > + > + struct cdev cdev; > + struct class *cdev_class;
As you only have 1 device, please just use the miscdev interface, not the chardev interface. Makes your code much smaller and easier to review and maintain over time and does not "burn" a whole major number for this tiny driver.
thanks,
greg k-h
| |