lkml.org 
[lkml]   [2022]   [Jan]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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

\
 
 \ /
  Last update: 2022-01-27 10:40    [W:0.113 / U:0.164 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site