2021-02-03 00:41:32

by Stephen Boyd

[permalink] [raw]
Subject: [PATCHv4 0/3] iio: Add a ChromeOS EC MKBP proximity driver

This is a different approach to [1] where I tried to add this proximity
sensor logic to the input subsystem. Instead, we'll take the approach of
making a small IIO proximity driver that parses the EC switch bitmap to
find out if the front proximity sensor is detecting something or not.
This allows us to treat proximity sensors as IIO devices all the time in
userspace instead of handling this switch on the EC via the input
subsystem and then other proximity sensors via IIO.

I propose this is all merged through IIO subsystem. Please ack
the first patch so it can be merged that way.

Changes from v3:
* Added SPI and cros-ec wrapper nodes to yaml example
* Ignore notifier registration return code that is always zero

Changes from v2:
* Check iio clock and use IIO time if not boottime

Changes from v1:
* Driver moved location
* Put mkbp everywhere
* Fixed up DT binding to not fail and make sure is a child of cros-ec
* Simplified logic for sending a message
* Dropped CONFIG_OF usage
* Sorted includes

[1] https://lore.kernel.org/r/[email protected]

Cc: Dmitry Torokhov <[email protected]>
Cc: Benson Leung <[email protected]>
Cc: Guenter Roeck <[email protected]>
Cc: Douglas Anderson <[email protected]>
Cc: Gwendal Grignou <[email protected]>
Cc: <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Enric Balletbo i Serra <[email protected]>

Stephen Boyd (3):
platform/chrome: cros_ec: Add SW_FRONT_PROXIMITY MKBP define
dt-bindings: iio: Add cros ec proximity yaml doc
iio: proximity: Add a ChromeOS EC MKBP proximity driver

.../google,cros-ec-mkbp-proximity.yaml | 46 ++++
.../bindings/mfd/google,cros-ec.yaml | 3 +
drivers/iio/proximity/Kconfig | 11 +
drivers/iio/proximity/Makefile | 1 +
.../iio/proximity/cros_ec_mkbp_proximity.c | 242 ++++++++++++++++++
.../linux/platform_data/cros_ec_commands.h | 1 +
6 files changed, 304 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/proximity/google,cros-ec-mkbp-proximity.yaml
create mode 100644 drivers/iio/proximity/cros_ec_mkbp_proximity.c


base-commit: 19c329f6808995b142b3966301f217c831e7cf31
--
https://chromeos.dev


2021-02-03 00:41:45

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH v4 3/3] iio: proximity: Add a ChromeOS EC MKBP proximity driver

Add support for a ChromeOS EC proximity driver that exposes a "front"
proximity sensor via the IIO subsystem. The EC decides when front
proximity is near and sets an MKBP switch 'EC_MKBP_FRONT_PROXIMITY' to
notify the kernel of proximity. Similarly, when proximity detects
something far away it sets the switch bit to 0. For now this driver
exposes a single sensor, but it could be expanded in the future via more
MKBP bits if desired.

Cc: Dmitry Torokhov <[email protected]>
Cc: Benson Leung <[email protected]>
Cc: Guenter Roeck <[email protected]>
Cc: Douglas Anderson <[email protected]>
Cc: Gwendal Grignou <[email protected]>
Reviewed-by: Enric Balletbo i Serra <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
drivers/iio/proximity/Kconfig | 11 +
drivers/iio/proximity/Makefile | 1 +
.../iio/proximity/cros_ec_mkbp_proximity.c | 242 ++++++++++++++++++
3 files changed, 254 insertions(+)
create mode 100644 drivers/iio/proximity/cros_ec_mkbp_proximity.c

diff --git a/drivers/iio/proximity/Kconfig b/drivers/iio/proximity/Kconfig
index 12672a0e89ed..7c7203ca3ac6 100644
--- a/drivers/iio/proximity/Kconfig
+++ b/drivers/iio/proximity/Kconfig
@@ -21,6 +21,17 @@ endmenu

menu "Proximity and distance sensors"

+config CROS_EC_MKBP_PROXIMITY
+ tristate "ChromeOS EC MKBP Proximity sensor"
+ depends on CROS_EC
+ help
+ Say Y here to enable the proximity sensor implemented via the ChromeOS EC MKBP
+ switches protocol. You must enable one bus option (CROS_EC_I2C or CROS_EC_SPI)
+ to use this.
+
+ To compile this driver as a module, choose M here: the
+ module will be called cros_ec_mkbp_proximity.
+
config ISL29501
tristate "Intersil ISL29501 Time Of Flight sensor"
depends on I2C
diff --git a/drivers/iio/proximity/Makefile b/drivers/iio/proximity/Makefile
index 9c1aca1a8b79..cbdac09433eb 100644
--- a/drivers/iio/proximity/Makefile
+++ b/drivers/iio/proximity/Makefile
@@ -5,6 +5,7 @@

# When adding new entries keep the list in alphabetical order
obj-$(CONFIG_AS3935) += as3935.o
+obj-$(CONFIG_CROS_EC_MKBP_PROXIMITY) += cros_ec_mkbp_proximity.o
obj-$(CONFIG_ISL29501) += isl29501.o
obj-$(CONFIG_LIDAR_LITE_V2) += pulsedlight-lidar-lite-v2.o
obj-$(CONFIG_MB1232) += mb1232.o
diff --git a/drivers/iio/proximity/cros_ec_mkbp_proximity.c b/drivers/iio/proximity/cros_ec_mkbp_proximity.c
new file mode 100644
index 000000000000..23bed4562b34
--- /dev/null
+++ b/drivers/iio/proximity/cros_ec_mkbp_proximity.c
@@ -0,0 +1,242 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for cros-ec proximity sensor exposed through MKBP switch
+ *
+ * Copyright 2021 Google LLC.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/notifier.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_proto.h>
+
+#include <linux/iio/events.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+
+#include <asm/unaligned.h>
+
+struct cros_ec_mkbp_proximity_data {
+ struct cros_ec_device *ec;
+ struct iio_dev *indio_dev;
+ struct mutex lock;
+ struct notifier_block notifier;
+ bool enabled;
+};
+
+static const struct iio_event_spec cros_ec_mkbp_proximity_events[] = {
+ {
+ .type = IIO_EV_TYPE_THRESH,
+ .dir = IIO_EV_DIR_EITHER,
+ .mask_separate = BIT(IIO_EV_INFO_ENABLE),
+ },
+};
+
+static const struct iio_chan_spec cros_ec_mkbp_proximity_chan_spec[] = {
+ {
+ .type = IIO_PROXIMITY,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+ .event_spec = cros_ec_mkbp_proximity_events,
+ .num_event_specs = ARRAY_SIZE(cros_ec_mkbp_proximity_events),
+ },
+};
+
+static int cros_ec_mkbp_proximity_parse_state(const void *data)
+{
+ u32 switches = get_unaligned_le32(data);
+
+ return !!(switches & BIT(EC_MKBP_FRONT_PROXIMITY));
+}
+
+static int cros_ec_mkbp_proximity_query(struct cros_ec_device *ec_dev,
+ int *state)
+{
+ struct {
+ struct cros_ec_command msg;
+ union {
+ struct ec_params_mkbp_info params;
+ u32 switches;
+ };
+ } __packed buf = { };
+ struct ec_params_mkbp_info *params = &buf.params;
+ struct cros_ec_command *msg = &buf.msg;
+ u32 *switches = &buf.switches;
+ size_t insize = sizeof(*switches);
+ int ret;
+
+ msg->command = EC_CMD_MKBP_INFO;
+ msg->version = 1;
+ msg->outsize = sizeof(*params);
+ msg->insize = insize;
+
+ params->info_type = EC_MKBP_INFO_CURRENT;
+ params->event_type = EC_MKBP_EVENT_SWITCH;
+
+ ret = cros_ec_cmd_xfer_status(ec_dev, msg);
+ if (ret < 0)
+ return ret;
+
+ if (ret != insize) {
+ dev_warn(ec_dev->dev, "wrong result size: %d != %zu\n", ret,
+ insize);
+ return -EPROTO;
+ }
+
+ *state = cros_ec_mkbp_proximity_parse_state(switches);
+ return IIO_VAL_INT;
+}
+
+static int cros_ec_mkbp_proximity_notify(struct notifier_block *nb,
+ unsigned long queued_during_suspend,
+ void *_ec)
+{
+ struct cros_ec_mkbp_proximity_data *data;
+ struct cros_ec_device *ec = _ec;
+ u8 event_type = ec->event_data.event_type & EC_MKBP_EVENT_TYPE_MASK;
+ void *switches = &ec->event_data.data.switches;
+ struct iio_dev *indio_dev;
+ s64 timestamp;
+ int state, dir;
+ u64 ev;
+
+ if (event_type == EC_MKBP_EVENT_SWITCH) {
+ data = container_of(nb, struct cros_ec_mkbp_proximity_data,
+ notifier);
+ indio_dev = data->indio_dev;
+
+ mutex_lock(&data->lock);
+ if (data->enabled) {
+ timestamp = ktime_to_ns(ec->last_event_time);
+ if (iio_device_get_clock(indio_dev) != CLOCK_BOOTTIME)
+ timestamp = iio_get_time_ns(indio_dev);
+ state = cros_ec_mkbp_proximity_parse_state(switches);
+ dir = state ? IIO_EV_DIR_FALLING : IIO_EV_DIR_RISING;
+
+ ev = IIO_UNMOD_EVENT_CODE(IIO_PROXIMITY, 0,
+ IIO_EV_TYPE_THRESH, dir);
+ iio_push_event(indio_dev, ev, timestamp);
+ }
+ mutex_unlock(&data->lock);
+ }
+
+ return NOTIFY_OK;
+}
+
+static int cros_ec_mkbp_proximity_read_raw(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan, int *val,
+ int *val2, long mask)
+{
+ struct cros_ec_mkbp_proximity_data *data = iio_priv(indio_dev);
+ struct cros_ec_device *ec = data->ec;
+
+ if (chan->type != IIO_PROXIMITY)
+ return -EINVAL;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ return cros_ec_mkbp_proximity_query(ec, val);
+ }
+
+ return -EINVAL;
+}
+
+static int cros_ec_mkbp_proximity_read_event_config(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ enum iio_event_type type,
+ enum iio_event_direction dir)
+{
+ struct cros_ec_mkbp_proximity_data *data = iio_priv(indio_dev);
+
+ return data->enabled;
+}
+
+static int cros_ec_mkbp_proximity_write_event_config(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ enum iio_event_type type,
+ enum iio_event_direction dir, int state)
+{
+ struct cros_ec_mkbp_proximity_data *data = iio_priv(indio_dev);
+
+ mutex_lock(&data->lock);
+ data->enabled = state;
+ mutex_unlock(&data->lock);
+
+ return 0;
+}
+
+static const struct iio_info cros_ec_mkbp_proximity_info = {
+ .read_raw = cros_ec_mkbp_proximity_read_raw,
+ .read_event_config = cros_ec_mkbp_proximity_read_event_config,
+ .write_event_config = cros_ec_mkbp_proximity_write_event_config,
+};
+
+static int cros_ec_mkbp_proximity_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct cros_ec_device *ec = dev_get_drvdata(dev->parent);
+ struct iio_dev *indio_dev;
+ struct cros_ec_mkbp_proximity_data *data;
+ int ret;
+
+ indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ data = iio_priv(indio_dev);
+ data->ec = ec;
+ data->indio_dev = indio_dev;
+ mutex_init(&data->lock);
+ platform_set_drvdata(pdev, data);
+
+ indio_dev->name = dev->driver->name;
+ indio_dev->info = &cros_ec_mkbp_proximity_info;
+ indio_dev->modes = INDIO_DIRECT_MODE;
+ indio_dev->channels = cros_ec_mkbp_proximity_chan_spec;
+ indio_dev->num_channels = ARRAY_SIZE(cros_ec_mkbp_proximity_chan_spec);
+
+ ret = devm_iio_device_register(dev, indio_dev);
+ if (ret)
+ return ret;
+
+ data->notifier.notifier_call = cros_ec_mkbp_proximity_notify;
+ blocking_notifier_chain_register(&ec->event_notifier, &data->notifier);
+
+ return 0;
+}
+
+static int cros_ec_mkbp_proximity_remove(struct platform_device *pdev)
+{
+ struct cros_ec_mkbp_proximity_data *data = platform_get_drvdata(pdev);
+ struct cros_ec_device *ec = data->ec;
+
+ blocking_notifier_chain_unregister(&ec->event_notifier,
+ &data->notifier);
+
+ return 0;
+}
+
+static const struct of_device_id cros_ec_mkbp_proximity_of_match[] = {
+ { .compatible = "google,cros-ec-mkbp-proximity" },
+ {}
+};
+MODULE_DEVICE_TABLE(of, cros_ec_mkbp_proximity_of_match);
+
+static struct platform_driver cros_ec_mkbp_proximity_driver = {
+ .driver = {
+ .name = "cros-ec-mkbp-proximity",
+ .of_match_table = of_match_ptr(cros_ec_mkbp_proximity_of_match),
+ },
+ .probe = cros_ec_mkbp_proximity_probe,
+ .remove = cros_ec_mkbp_proximity_remove,
+};
+module_platform_driver(cros_ec_mkbp_proximity_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("ChromeOS EC MKBP proximity sensor driver");
--
https://chromeos.dev

2021-02-03 00:43:23

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH v4 1/3] platform/chrome: cros_ec: Add SW_FRONT_PROXIMITY MKBP define

Some cros ECs support a front proximity MKBP event via
'EC_MKBP_FRONT_PROXIMITY'. Add this define so it can be used in a
future patch.

Cc: Dmitry Torokhov <[email protected]>
Cc: Benson Leung <[email protected]>
Cc: Guenter Roeck <[email protected]>
Cc: Douglas Anderson <[email protected]>
Cc: Gwendal Grignou <[email protected]>
Acked-by: Enric Balletbo i Serra <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
include/linux/platform_data/cros_ec_commands.h | 1 +
1 file changed, 1 insertion(+)

diff --git a/include/linux/platform_data/cros_ec_commands.h b/include/linux/platform_data/cros_ec_commands.h
index 86376779ab31..776e0b2be0e9 100644
--- a/include/linux/platform_data/cros_ec_commands.h
+++ b/include/linux/platform_data/cros_ec_commands.h
@@ -3457,6 +3457,7 @@ struct ec_response_get_next_event_v1 {
#define EC_MKBP_LID_OPEN 0
#define EC_MKBP_TABLET_MODE 1
#define EC_MKBP_BASE_ATTACHED 2
+#define EC_MKBP_FRONT_PROXIMITY 3

/* Run keyboard factory test scanning */
#define EC_CMD_KEYBOARD_FACTORY_TEST 0x0068
--
https://chromeos.dev

2021-02-03 00:43:43

by Stephen Boyd

[permalink] [raw]
Subject: [PATCH v4 2/3] dt-bindings: iio: Add cros ec proximity yaml doc

Some cros ECs support a front proximity MKBP event via
'EC_MKBP_FRONT_PROXIMITY'. Add a DT binding to document this feature via
a node that is a child of the main cros_ec device node. Devices that
have this ability will describe this in firmware.

Cc: Dmitry Torokhov <[email protected]>
Cc: Benson Leung <[email protected]>
Cc: Guenter Roeck <[email protected]>
Cc: Douglas Anderson <[email protected]>
Cc: Gwendal Grignou <[email protected]>
Cc: <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Enric Balletbo i Serra <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
.../google,cros-ec-mkbp-proximity.yaml | 46 +++++++++++++++++++
.../bindings/mfd/google,cros-ec.yaml | 3 ++
2 files changed, 49 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/proximity/google,cros-ec-mkbp-proximity.yaml

diff --git a/Documentation/devicetree/bindings/iio/proximity/google,cros-ec-mkbp-proximity.yaml b/Documentation/devicetree/bindings/iio/proximity/google,cros-ec-mkbp-proximity.yaml
new file mode 100644
index 000000000000..d82b929af445
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/proximity/google,cros-ec-mkbp-proximity.yaml
@@ -0,0 +1,46 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+
+$id: http://devicetree.org/schemas/iio/proximity/google,cros-ec-mkbp-proximity.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ChromeOS EC MKBP Proximity Sensor
+
+maintainers:
+ - Stephen Boyd <[email protected]>
+ - Benson Leung <[email protected]>
+ - Enric Balletbo i Serra <[email protected]>
+
+description: |
+ Google's ChromeOS EC sometimes has the ability to detect user proximity.
+ This is implemented on the EC as near/far logic and exposed to the OS
+ via an MKBP switch bit.
+
+properties:
+ compatible:
+ const: google,cros-ec-mkbp-proximity
+
+ label:
+ description: Name for proximity sensor
+
+required:
+ - compatible
+
+unevaluatedProperties: false
+additionalProperties: false
+
+examples:
+ - |
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ ec@0 {
+ compatible = "google,cros-ec-spi";
+ reg = <0>;
+ proximity {
+ compatible = "google,cros-ec-mkbp-proximity";
+ label = "proximity-wifi-lte";
+ };
+ };
+ };
diff --git a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
index 76bf16ee27ec..479a9f15de32 100644
--- a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
+++ b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
@@ -94,6 +94,9 @@ properties:
keyboard-controller:
$ref: "/schemas/input/google,cros-ec-keyb.yaml#"

+ proximity:
+ $ref: "/schemas/iio/proximity/google,cros-ec-mkbp-proximity.yaml#"
+
codecs:
type: object
additionalProperties: false
--
https://chromeos.dev

2021-02-03 08:33:54

by Enric Balletbo Serra

[permalink] [raw]
Subject: Re: [PATCH v4 2/3] dt-bindings: iio: Add cros ec proximity yaml doc

Hi Stephen,

Missatge de Stephen Boyd <[email protected]> del dia dt., 2 de febr.
2021 a les 19:53:
>
> Some cros ECs support a front proximity MKBP event via
> 'EC_MKBP_FRONT_PROXIMITY'. Add a DT binding to document this feature via
> a node that is a child of the main cros_ec device node. Devices that
> have this ability will describe this in firmware.
>
> Cc: Dmitry Torokhov <[email protected]>
> Cc: Benson Leung <[email protected]>
> Cc: Guenter Roeck <[email protected]>
> Cc: Douglas Anderson <[email protected]>
> Cc: Gwendal Grignou <[email protected]>
> Cc: <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Enric Balletbo i Serra <[email protected]>
> Signed-off-by: Stephen Boyd <[email protected]>

Thanks for adding a full example, IIRC this is preferred by Rob and we
are also trying to apply this rule on all the cros-ec related
bindings, so the dt_bindings_check really checks a full example. From
my side looks good to me.

Reviewed-by: Enric Balletbo i Serra <[email protected]>

> ---
> .../google,cros-ec-mkbp-proximity.yaml | 46 +++++++++++++++++++
> .../bindings/mfd/google,cros-ec.yaml | 3 ++
> 2 files changed, 49 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/proximity/google,cros-ec-mkbp-proximity.yaml
>
> diff --git a/Documentation/devicetree/bindings/iio/proximity/google,cros-ec-mkbp-proximity.yaml b/Documentation/devicetree/bindings/iio/proximity/google,cros-ec-mkbp-proximity.yaml
> new file mode 100644
> index 000000000000..d82b929af445
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/proximity/google,cros-ec-mkbp-proximity.yaml
> @@ -0,0 +1,46 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +
> +$id: http://devicetree.org/schemas/iio/proximity/google,cros-ec-mkbp-proximity.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ChromeOS EC MKBP Proximity Sensor
> +
> +maintainers:
> + - Stephen Boyd <[email protected]>
> + - Benson Leung <[email protected]>
> + - Enric Balletbo i Serra <[email protected]>
> +
> +description: |
> + Google's ChromeOS EC sometimes has the ability to detect user proximity.
> + This is implemented on the EC as near/far logic and exposed to the OS
> + via an MKBP switch bit.
> +
> +properties:
> + compatible:
> + const: google,cros-ec-mkbp-proximity
> +
> + label:
> + description: Name for proximity sensor
> +
> +required:
> + - compatible
> +
> +unevaluatedProperties: false
> +additionalProperties: false
> +
> +examples:
> + - |
> + spi {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + ec@0 {
> + compatible = "google,cros-ec-spi";
> + reg = <0>;
> + proximity {
> + compatible = "google,cros-ec-mkbp-proximity";
> + label = "proximity-wifi-lte";
> + };
> + };
> + };
> diff --git a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> index 76bf16ee27ec..479a9f15de32 100644
> --- a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> +++ b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> @@ -94,6 +94,9 @@ properties:
> keyboard-controller:
> $ref: "/schemas/input/google,cros-ec-keyb.yaml#"
>
> + proximity:
> + $ref: "/schemas/iio/proximity/google,cros-ec-mkbp-proximity.yaml#"
> +
> codecs:
> type: object
> additionalProperties: false
> --
> https://chromeos.dev
>

2021-02-06 16:49:08

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] iio: proximity: Add a ChromeOS EC MKBP proximity driver

On Tue, 2 Feb 2021 10:44:34 -0800
Stephen Boyd <[email protected]> wrote:

> Add support for a ChromeOS EC proximity driver that exposes a "front"
> proximity sensor via the IIO subsystem. The EC decides when front
> proximity is near and sets an MKBP switch 'EC_MKBP_FRONT_PROXIMITY' to
> notify the kernel of proximity. Similarly, when proximity detects
> something far away it sets the switch bit to 0. For now this driver
> exposes a single sensor, but it could be expanded in the future via more
> MKBP bits if desired.
>
> Cc: Dmitry Torokhov <[email protected]>
> Cc: Benson Leung <[email protected]>
> Cc: Guenter Roeck <[email protected]>
> Cc: Douglas Anderson <[email protected]>
> Cc: Gwendal Grignou <[email protected]>
> Reviewed-by: Enric Balletbo i Serra <[email protected]>
> Signed-off-by: Stephen Boyd <[email protected]>

Just one thing noticed on a final read through. I'm happy to fix up
whilst applying. Otherwise all I'm waiting for now is to give
Rob time to look at the device tree binding if he wants to do so.

Jonathan

> diff --git a/drivers/iio/proximity/cros_ec_mkbp_proximity.c b/drivers/iio/proximity/cros_ec_mkbp_proximity.c
> new file mode 100644
> index 000000000000..23bed4562b34
> --- /dev/null
> +++ b/drivers/iio/proximity/cros_ec_mkbp_proximity.c
> @@ -0,0 +1,242 @@
...

> +static struct platform_driver cros_ec_mkbp_proximity_driver = {
> + .driver = {
> + .name = "cros-ec-mkbp-proximity",
> + .of_match_table = of_match_ptr(cros_ec_mkbp_proximity_of_match),
I'm going to assume we know no one is going to use this with
ACPI via PRP0001 given presumably the firmware on these devices
is tightly controlled.

However, we should should still drop the of_match_ptr
as it will lead to an unused warning for cros_ec_mkbp_proximity_of_match
if anyone builds this without CONFIG_OF + it sets a general bad
precedence that I'd rather wasn't around for people to copy.
Note that in general we are slowly ripping these out of IIO but
probably lots still there.

If this is all that is needed in this version I'll just do it
whilst applying unless anyone shouts.

Jonathan

> + },
> + .probe = cros_ec_mkbp_proximity_probe,
> + .remove = cros_ec_mkbp_proximity_remove,
> +};
> +module_platform_driver(cros_ec_mkbp_proximity_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("ChromeOS EC MKBP proximity sensor driver");

2021-02-07 03:23:39

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] iio: proximity: Add a ChromeOS EC MKBP proximity driver

Quoting Jonathan Cameron (2021-02-06 08:17:11)
> On Tue, 2 Feb 2021 10:44:34 -0800
> Stephen Boyd <[email protected]> wrote:
>
> > +static struct platform_driver cros_ec_mkbp_proximity_driver = {
> > + .driver = {
> > + .name = "cros-ec-mkbp-proximity",
> > + .of_match_table = of_match_ptr(cros_ec_mkbp_proximity_of_match),
> I'm going to assume we know no one is going to use this with
> ACPI via PRP0001 given presumably the firmware on these devices
> is tightly controlled.

Correct.

>
> However, we should should still drop the of_match_ptr
> as it will lead to an unused warning for cros_ec_mkbp_proximity_of_match
> if anyone builds this without CONFIG_OF + it sets a general bad
> precedence that I'd rather wasn't around for people to copy.
> Note that in general we are slowly ripping these out of IIO but
> probably lots still there.
>
> If this is all that is needed in this version I'll just do it
> whilst applying unless anyone shouts.
>

Agreed. Thanks for fixing that last little bit.

2021-02-10 00:46:26

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v4 2/3] dt-bindings: iio: Add cros ec proximity yaml doc

On Tue, Feb 02, 2021 at 10:44:33AM -0800, Stephen Boyd wrote:
> Some cros ECs support a front proximity MKBP event via
> 'EC_MKBP_FRONT_PROXIMITY'. Add a DT binding to document this feature via
> a node that is a child of the main cros_ec device node. Devices that
> have this ability will describe this in firmware.
>
> Cc: Dmitry Torokhov <[email protected]>
> Cc: Benson Leung <[email protected]>
> Cc: Guenter Roeck <[email protected]>
> Cc: Douglas Anderson <[email protected]>
> Cc: Gwendal Grignou <[email protected]>
> Cc: <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Enric Balletbo i Serra <[email protected]>
> Signed-off-by: Stephen Boyd <[email protected]>
> ---
> .../google,cros-ec-mkbp-proximity.yaml | 46 +++++++++++++++++++
> .../bindings/mfd/google,cros-ec.yaml | 3 ++
> 2 files changed, 49 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/proximity/google,cros-ec-mkbp-proximity.yaml
>
> diff --git a/Documentation/devicetree/bindings/iio/proximity/google,cros-ec-mkbp-proximity.yaml b/Documentation/devicetree/bindings/iio/proximity/google,cros-ec-mkbp-proximity.yaml
> new file mode 100644
> index 000000000000..d82b929af445
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/proximity/google,cros-ec-mkbp-proximity.yaml
> @@ -0,0 +1,46 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +
> +$id: http://devicetree.org/schemas/iio/proximity/google,cros-ec-mkbp-proximity.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ChromeOS EC MKBP Proximity Sensor
> +
> +maintainers:
> + - Stephen Boyd <[email protected]>
> + - Benson Leung <[email protected]>
> + - Enric Balletbo i Serra <[email protected]>
> +
> +description: |
> + Google's ChromeOS EC sometimes has the ability to detect user proximity.
> + This is implemented on the EC as near/far logic and exposed to the OS
> + via an MKBP switch bit.
> +
> +properties:
> + compatible:
> + const: google,cros-ec-mkbp-proximity
> +
> + label:
> + description: Name for proximity sensor
> +
> +required:
> + - compatible
> +
> +unevaluatedProperties: false
> +additionalProperties: false

Only need one. In this case 'additionalProperties'.

> +
> +examples:
> + - |
> + spi {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + ec@0 {
> + compatible = "google,cros-ec-spi";
> + reg = <0>;
> + proximity {
> + compatible = "google,cros-ec-mkbp-proximity";
> + label = "proximity-wifi-lte";
> + };

The complete examples I prefer is 1 example for the whole MFD in the MFD
schema and no example here.

> + };
> + };
> diff --git a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> index 76bf16ee27ec..479a9f15de32 100644
> --- a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> +++ b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> @@ -94,6 +94,9 @@ properties:
> keyboard-controller:
> $ref: "/schemas/input/google,cros-ec-keyb.yaml#"
>
> + proximity:
> + $ref: "/schemas/iio/proximity/google,cros-ec-mkbp-proximity.yaml#"
> +
> codecs:
> type: object
> additionalProperties: false
> --
> https://chromeos.dev
>

2021-02-10 02:44:05

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v4 2/3] dt-bindings: iio: Add cros ec proximity yaml doc

Quoting Rob Herring (2021-02-09 13:13:47)
> On Tue, Feb 02, 2021 at 10:44:33AM -0800, Stephen Boyd wrote:
> > + description: Name for proximity sensor
> > +
> > +required:
> > + - compatible
> > +
> > +unevaluatedProperties: false
> > +additionalProperties: false
>
> Only need one. In this case 'additionalProperties'.

Got it.

>
> > +
> > +examples:
> > + - |
> > + spi {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + ec@0 {
> > + compatible = "google,cros-ec-spi";
> > + reg = <0>;
> > + proximity {
> > + compatible = "google,cros-ec-mkbp-proximity";
> > + label = "proximity-wifi-lte";
> > + };
>
> The complete examples I prefer is 1 example for the whole MFD in the MFD
> schema and no example here.

Alright. I can add it to the mfd binding instead.

>
> > + };
> > + };
> > diff --git a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> > index 76bf16ee27ec..479a9f15de32 100644
> > --- a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> > +++ b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> > @@ -94,6 +94,9 @@ properties:
> > keyboard-controller:
> > $ref: "/schemas/input/google,cros-ec-keyb.yaml#"

2021-02-10 02:45:47

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] iio: proximity: Add a ChromeOS EC MKBP proximity driver

Quoting Stephen Boyd (2021-02-06 19:21:39)
> Quoting Jonathan Cameron (2021-02-06 08:17:11)
> > On Tue, 2 Feb 2021 10:44:34 -0800
> > Stephen Boyd <[email protected]> wrote:
> >
> > > +static struct platform_driver cros_ec_mkbp_proximity_driver = {
> > > + .driver = {
> > > + .name = "cros-ec-mkbp-proximity",
> > > + .of_match_table = of_match_ptr(cros_ec_mkbp_proximity_of_match),
> > I'm going to assume we know no one is going to use this with
> > ACPI via PRP0001 given presumably the firmware on these devices
> > is tightly controlled.
>
> Correct.
>
> >
> > However, we should should still drop the of_match_ptr
> > as it will lead to an unused warning for cros_ec_mkbp_proximity_of_match
> > if anyone builds this without CONFIG_OF + it sets a general bad
> > precedence that I'd rather wasn't around for people to copy.
> > Note that in general we are slowly ripping these out of IIO but
> > probably lots still there.
> >
> > If this is all that is needed in this version I'll just do it
> > whilst applying unless anyone shouts.
> >
>
> Agreed. Thanks for fixing that last little bit.

Seems Rob wanted a small tweak to the binding so I'll resend this now
and drop the of_match_ptr() usage.