[PATCH v4 1/2] platform: chrome: Add cros-usbpd-notify driver

[PATCH v4 1/2] platform: chrome: Add cros-usbpd-notify driver

[Prashant Malani]

From: Jon Flatley <jflat@chromium.org>

ChromiumOS uses ACPI device with HID "GOOG0003" for power delivery
related events. The existing cros-usbpd-charger driver relies on these
events without ever actually receiving them on ACPI platforms. This is
because in the ChromeOS kernel trees, the GOOG0003 device is owned by an
ACPI driver that offers firmware updates to USB-C chargers.

Introduce a new platform driver under cros-ec, the ChromeOS embedded
controller, that handles these PD events and dispatches them
appropriately over a notifier chain to all drivers that use them.

On non-ACPI platforms, the driver gets instantiated for ECs which
support the EC_FEATURE_USB_PD feature bit, and on such platforms, the
notification events will get delivered using the MKBP event handling
mechanism.

Co-Developed-by: Prashant Malani <pmalani@chromium.org>
Reviewed-by: Gwendal Grignou <gwendal@chromium.org>
Signed-off-by: Jon Flatley <jflat@chromium.org>
Signed-off-by: Prashant Malani <pmalani@chromium.org>
---

Changes in v4(pmalani@chromium.org):
- No code changes, but added new version so that versioning is
  consistent with the next patch in the series.

Changes in v3 (pmalani@chromium.org):
- Renamed driver and files from "cros_ec_pd_notify" to
  "cros_usbpd_notify" to be more consistent with other naming.
- Moved the change to include cros-usbpd-notify in the charger MFD into
  a separate follow-on patch.

Changes in v2 (pmalani@chromium.org):
- Removed dependency on DT entry; instead, we will instantiate the
  driver on detecting EC_FEATURE_USB_PD for non-ACPI platforms.
- Modified the cros-ec-pd-notify device to be an mfd_cell under
  usbpdcharger for non-ACPI platforms. Altered the platform_probe() call
  to derive the cros EC structs appropriately.
- Replaced "usbpd_notify" with "pd_notify" in functions and structures.
- Addressed comments from upstream maintainer.

 drivers/platform/chrome/Kconfig               |   9 ++
 drivers/platform/chrome/Makefile              |   1 +
 drivers/platform/chrome/cros_usbpd_notify.c   | 151 ++++++++++++++++++
 .../linux/platform_data/cros_usbpd_notify.h   |  17 ++
 4 files changed, 178 insertions(+)
 create mode 100644 drivers/platform/chrome/cros_usbpd_notify.c
 create mode 100644 include/linux/platform_data/cros_usbpd_notify.h

diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
index 5f57282a28da0..3a8a98f2fb4d1 100644
--- a/drivers/platform/chrome/Kconfig
+++ b/drivers/platform/chrome/Kconfig
@@ -226,6 +226,15 @@ config CROS_USBPD_LOGGER
 	  To compile this driver as a module, choose M here: the
 	  module will be called cros_usbpd_logger.
 
+config CROS_USBPD_NOTIFY
+	tristate "ChromeOS Type-C power delivery event notifier"
+	depends on CROS_EC
+	help
+	  If you say Y here, you get support for Type-C PD event notifications
+	  from the ChromeOS EC. On ACPI platorms this driver will bind to the
+	  GOOG0003 ACPI device, and on non-ACPI platforms this driver will get
+	  initialized on ECs which support the feature EC_FEATURE_USB_PD.
+
 source "drivers/platform/chrome/wilco_ec/Kconfig"
 
 endif # CHROMEOS_PLATFORMS
diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
index aacd5920d8a18..f6465f8ef0b5e 100644
--- a/drivers/platform/chrome/Makefile
+++ b/drivers/platform/chrome/Makefile
@@ -22,5 +22,6 @@ obj-$(CONFIG_CROS_EC_DEBUGFS)		+= cros_ec_debugfs.o
 obj-$(CONFIG_CROS_EC_SENSORHUB)		+= cros_ec_sensorhub.o
 obj-$(CONFIG_CROS_EC_SYSFS)		+= cros_ec_sysfs.o
 obj-$(CONFIG_CROS_USBPD_LOGGER)		+= cros_usbpd_logger.o
+obj-$(CONFIG_CROS_USBPD_NOTIFY)		+= cros_usbpd_notify.o
 
 obj-$(CONFIG_WILCO_EC)			+= wilco_ec/
diff --git a/drivers/platform/chrome/cros_usbpd_notify.c b/drivers/platform/chrome/cros_usbpd_notify.c
new file mode 100644
index 0000000000000..05a7db834d2e0
--- /dev/null
+++ b/drivers/platform/chrome/cros_usbpd_notify.c
@@ -0,0 +1,151 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2019 Google LLC
+ *
+ * This driver serves as the receiver of cros_ec PD host events.
+ */
+
+#include <linux/acpi.h>
+#include <linux/module.h>
+#include <linux/mfd/cros_ec.h>
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_usbpd_notify.h>
+#include <linux/platform_data/cros_ec_proto.h>
+#include <linux/platform_device.h>
+
+#define DRV_NAME "cros-usbpd-notify"
+#define ACPI_DRV_NAME "GOOG0003"
+
+static BLOCKING_NOTIFIER_HEAD(cros_usbpd_notifier_list);
+
+/**
+ * cros_usbpd_register_notify - Register a notifier callback for PD events.
+ * @nb: Notifier block pointer to register
+ *
+ * On ACPI platforms this corresponds to host events on the ECPD
+ * "GOOG0003" ACPI device. On non-ACPI platforms this will filter mkbp events
+ * for USB PD events.
+ *
+ * Return: 0 on success or negative error code.
+ */
+int cros_usbpd_register_notify(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_register(
+			&cros_usbpd_notifier_list, nb);
+}
+EXPORT_SYMBOL_GPL(cros_usbpd_register_notify);
+
+
+/**
+ * cros_usbpd_unregister_notify - Unregister notifier callback for PD events.
+ * @nb: Notifier block pointer to unregister
+ *
+ * Unregister a notifier callback that was previously registered with
+ * cros_usbpd_register_notify().
+ */
+void cros_usbpd_unregister_notify(struct notifier_block *nb)
+{
+	blocking_notifier_chain_unregister(&cros_usbpd_notifier_list, nb);
+}
+EXPORT_SYMBOL_GPL(cros_usbpd_unregister_notify);
+
+#ifdef CONFIG_ACPI
+
+static int cros_usbpd_notify_add_acpi(struct acpi_device *adev)
+{
+	return 0;
+}
+
+static void cros_usbpd_notify_acpi(struct acpi_device *adev, u32 event)
+{
+	blocking_notifier_call_chain(&cros_usbpd_notifier_list, event, NULL);
+}
+
+static const struct acpi_device_id cros_usbpd_notify_acpi_device_ids[] = {
+	{ ACPI_DRV_NAME, 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, cros_usbpd_acpi_device_ids);
+
+static struct acpi_driver cros_usbpd_notify_driver = {
+	.name = DRV_NAME,
+	.class = DRV_NAME,
+	.ids = cros_usbpd_notify_acpi_device_ids,
+	.ops = {
+		.add = cros_usbpd_notify_add_acpi,
+		.notify = cros_usbpd_notify_acpi,
+	},
+};
+module_acpi_driver(cros_usbpd_notify_driver);
+
+#else /* CONFIG_ACPI */
+
+static int cros_usbpd_notify_plat(struct notifier_block *nb,
+		unsigned long queued_during_suspend, void *data)
+{
+	struct cros_ec_device *ec_dev = (struct cros_ec_device *)data;
+	u32 host_event = cros_ec_get_host_event(ec_dev);
+
+	if (!host_event)
+		return NOTIFY_BAD;
+
+	if (host_event & EC_HOST_EVENT_MASK(EC_HOST_EVENT_PD_MCU)) {
+		blocking_notifier_call_chain(&cros_usbpd_notifier_list,
+				host_event, NULL);
+		return NOTIFY_OK;
+	}
+	return NOTIFY_DONE;
+}
+
+static int cros_usbpd_notify_probe_plat(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct cros_ec_dev *ecdev = dev_get_drvdata(dev->parent);
+	struct notifier_block *nb;
+	int ret;
+
+	nb = devm_kzalloc(dev, sizeof(*nb), GFP_KERNEL);
+	if (!nb)
+		return -ENOMEM;
+
+	nb->notifier_call = cros_usbpd_notify_plat;
+	dev_set_drvdata(dev, nb);
+
+	ret = blocking_notifier_chain_register(&ecdev->ec_dev->event_notifier,
+						nb);
+	if (ret < 0) {
+		dev_err(dev, "Failed to register notifier\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int cros_usbpd_notify_remove_plat(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct cros_ec_dev *ecdev = dev_get_drvdata(dev->parent);
+	struct notifier_block *nb =
+		(struct notifier_block *)dev_get_drvdata(dev);
+
+	blocking_notifier_chain_unregister(&ecdev->ec_dev->event_notifier,
+			nb);
+
+	return 0;
+}
+
+static struct platform_driver cros_usbpd_notify_driver = {
+	.driver = {
+		.name = DRV_NAME,
+	},
+	.probe = cros_usbpd_notify_probe_plat,
+	.remove = cros_usbpd_notify_remove_plat,
+};
+module_platform_driver(cros_usbpd_notify_driver);
+
+#endif /* CONFIG_ACPI */
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("ChromeOS power delivery notifier device");
+MODULE_AUTHOR("Jon Flatley <jflat@chromium.org>");
+MODULE_ALIAS("platform:" DRV_NAME);
diff --git a/include/linux/platform_data/cros_usbpd_notify.h b/include/linux/platform_data/cros_usbpd_notify.h
new file mode 100644
index 0000000000000..cd7c7bebf18da
--- /dev/null
+++ b/include/linux/platform_data/cros_usbpd_notify.h
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * ChromeOS EC Power Delivery Notifier Driver
+ *
+ * Copyright 2019 Google LLC
+ */
+
+#ifndef __LINUX_PLATFORM_DATA_CROS_USBPD_NOTIFY_H
+#define __LINUX_PLATFORM_DATA_CROS_USBPD_NOTIFY_H
+
+#include <linux/notifier.h>
+
+int cros_usbpd_register_notify(struct notifier_block *nb);
+
+void cros_usbpd_unregister_notify(struct notifier_block *nb);
+
+#endif  /* __LINUX_PLATFORM_DATA_CROS_USBPD_NOTIFY_H */
-- 
2.24.1.735.g03f4e72817-goog

[PATCH v4 2/2] mfd: cros_ec: Add cros-usbpd-notify subdevice

[Prashant Malani]

Add the cros-usbpd-notify driver as a subdevice on non-ACPI platforms
that support the EC_FEATURE_USB_PD EC feature flag.

This driver allows other cros-ec devices to receive PD event
notifications from the Chrome OS Embedded Controller (EC) via a
notification chain.

Signed-off-by: Prashant Malani <pmalani@chromium.org>
---

Changes in v4:
- Removed #ifndef usage; instead, moved cros-usbpd-notify to a separate
  mfd_cell and used an IS_ENABLED() check.
- Changed commit title and description slightly to reflect change in
  code.

 drivers/mfd/cros_ec_dev.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
index c4b977a5dd966..da198abe2b0a6 100644
--- a/drivers/mfd/cros_ec_dev.c
+++ b/drivers/mfd/cros_ec_dev.c
@@ -5,6 +5,7 @@
  * Copyright (C) 2014 Google, Inc.
  */
 
+#include <linux/kconfig.h>
 #include <linux/mfd/core.h>
 #include <linux/mfd/cros_ec.h>
 #include <linux/module.h>
@@ -87,6 +88,10 @@ static const struct mfd_cell cros_usbpd_charger_cells[] = {
 	{ .name = "cros-usbpd-logger", },
 };
 
+static const struct mfd_cell cros_usbpd_notify_cells[] = {
+	{ .name = "cros-usbpd-notify", },
+};
+
 static const struct cros_feature_to_cells cros_subdevices[] = {
 	{
 		.id		= EC_FEATURE_CEC,
@@ -202,6 +207,22 @@ static int ec_device_probe(struct platform_device *pdev)
 		}
 	}
 
+	/*
+	 * The PD notifier driver cell is separate since it only needs to be
+	 * explicitly added on non-ACPI platforms.
+	 */
+	if (!IS_ENABLED(CONFIG_ACPI)) {
+		if (cros_ec_check_features(ec, EC_FEATURE_USB_PD)) {
+			retval = mfd_add_hotplug_devices(ec->dev,
+					cros_usbpd_notify_cells,
+					ARRAY_SIZE(cros_usbpd_notify_cells));
+			if (retval)
+				dev_err(ec->dev,
+					"failed to add PD notify devices: %d\n",
+					retval);
+		}
+	}
+
 	/*
 	 * The following subdevices cannot be detected by sending the
 	 * EC_FEATURE_GET_CMD to the Embedded Controller device.
-- 
2.24.1.735.g03f4e72817-goog

Re: [PATCH v4 1/2] platform: chrome: Add cros-usbpd-notify driver

[Enric Balletbo i Serra]

Hi Prashant,

On 20/12/19 20:38, Prashant Malani wrote:
> From: Jon Flatley <jflat@chromium.org>
> 
> ChromiumOS uses ACPI device with HID "GOOG0003" for power delivery
> related events. The existing cros-usbpd-charger driver relies on these
> events without ever actually receiving them on ACPI platforms. This is
> because in the ChromeOS kernel trees, the GOOG0003 device is owned by an
> ACPI driver that offers firmware updates to USB-C chargers.
> 
> Introduce a new platform driver under cros-ec, the ChromeOS embedded
> controller, that handles these PD events and dispatches them
> appropriately over a notifier chain to all drivers that use them.
> 
> On non-ACPI platforms, the driver gets instantiated for ECs which
> support the EC_FEATURE_USB_PD feature bit, and on such platforms, the
> notification events will get delivered using the MKBP event handling
> mechanism.
> 
> Co-Developed-by: Prashant Malani <pmalani@chromium.org>
> Reviewed-by: Gwendal Grignou <gwendal@chromium.org>
> Signed-off-by: Jon Flatley <jflat@chromium.org>
> Signed-off-by: Prashant Malani <pmalani@chromium.org>

For my own reference:

Acked-for-chrome-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

> ---
> 
> Changes in v4(pmalani@chromium.org):
> - No code changes, but added new version so that versioning is
>   consistent with the next patch in the series.
> 
> Changes in v3 (pmalani@chromium.org):
> - Renamed driver and files from "cros_ec_pd_notify" to
>   "cros_usbpd_notify" to be more consistent with other naming.
> - Moved the change to include cros-usbpd-notify in the charger MFD into
>   a separate follow-on patch.
> 
> Changes in v2 (pmalani@chromium.org):
> - Removed dependency on DT entry; instead, we will instantiate the
>   driver on detecting EC_FEATURE_USB_PD for non-ACPI platforms.
> - Modified the cros-ec-pd-notify device to be an mfd_cell under
>   usbpdcharger for non-ACPI platforms. Altered the platform_probe() call
>   to derive the cros EC structs appropriately.
> - Replaced "usbpd_notify" with "pd_notify" in functions and structures.
> - Addressed comments from upstream maintainer.
> 
>  drivers/platform/chrome/Kconfig               |   9 ++
>  drivers/platform/chrome/Makefile              |   1 +
>  drivers/platform/chrome/cros_usbpd_notify.c   | 151 ++++++++++++++++++
>  .../linux/platform_data/cros_usbpd_notify.h   |  17 ++
>  4 files changed, 178 insertions(+)
>  create mode 100644 drivers/platform/chrome/cros_usbpd_notify.c
>  create mode 100644 include/linux/platform_data/cros_usbpd_notify.h
> 
> diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
> index 5f57282a28da0..3a8a98f2fb4d1 100644
> --- a/drivers/platform/chrome/Kconfig
> +++ b/drivers/platform/chrome/Kconfig
> @@ -226,6 +226,15 @@ config CROS_USBPD_LOGGER
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called cros_usbpd_logger.
>  
> +config CROS_USBPD_NOTIFY
> +	tristate "ChromeOS Type-C power delivery event notifier"
> +	depends on CROS_EC
> +	help
> +	  If you say Y here, you get support for Type-C PD event notifications
> +	  from the ChromeOS EC. On ACPI platorms this driver will bind to the
> +	  GOOG0003 ACPI device, and on non-ACPI platforms this driver will get
> +	  initialized on ECs which support the feature EC_FEATURE_USB_PD.
> +
>  source "drivers/platform/chrome/wilco_ec/Kconfig"
>  
>  endif # CHROMEOS_PLATFORMS
> diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
> index aacd5920d8a18..f6465f8ef0b5e 100644
> --- a/drivers/platform/chrome/Makefile
> +++ b/drivers/platform/chrome/Makefile
> @@ -22,5 +22,6 @@ obj-$(CONFIG_CROS_EC_DEBUGFS)		+= cros_ec_debugfs.o
>  obj-$(CONFIG_CROS_EC_SENSORHUB)		+= cros_ec_sensorhub.o
>  obj-$(CONFIG_CROS_EC_SYSFS)		+= cros_ec_sysfs.o
>  obj-$(CONFIG_CROS_USBPD_LOGGER)		+= cros_usbpd_logger.o
> +obj-$(CONFIG_CROS_USBPD_NOTIFY)		+= cros_usbpd_notify.o
>  
>  obj-$(CONFIG_WILCO_EC)			+= wilco_ec/
> diff --git a/drivers/platform/chrome/cros_usbpd_notify.c b/drivers/platform/chrome/cros_usbpd_notify.c
> new file mode 100644
> index 0000000000000..05a7db834d2e0
> --- /dev/null
> +++ b/drivers/platform/chrome/cros_usbpd_notify.c
> @@ -0,0 +1,151 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright 2019 Google LLC
> + *
> + * This driver serves as the receiver of cros_ec PD host events.
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/module.h>
> +#include <linux/mfd/cros_ec.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_usbpd_notify.h>
> +#include <linux/platform_data/cros_ec_proto.h>
> +#include <linux/platform_device.h>
> +
> +#define DRV_NAME "cros-usbpd-notify"
> +#define ACPI_DRV_NAME "GOOG0003"
> +
> +static BLOCKING_NOTIFIER_HEAD(cros_usbpd_notifier_list);
> +
> +/**
> + * cros_usbpd_register_notify - Register a notifier callback for PD events.
> + * @nb: Notifier block pointer to register
> + *
> + * On ACPI platforms this corresponds to host events on the ECPD
> + * "GOOG0003" ACPI device. On non-ACPI platforms this will filter mkbp events
> + * for USB PD events.
> + *
> + * Return: 0 on success or negative error code.
> + */
> +int cros_usbpd_register_notify(struct notifier_block *nb)
> +{
> +	return blocking_notifier_chain_register(
> +			&cros_usbpd_notifier_list, nb);
> +}
> +EXPORT_SYMBOL_GPL(cros_usbpd_register_notify);
> +
> +
> +/**
> + * cros_usbpd_unregister_notify - Unregister notifier callback for PD events.
> + * @nb: Notifier block pointer to unregister
> + *
> + * Unregister a notifier callback that was previously registered with
> + * cros_usbpd_register_notify().
> + */
> +void cros_usbpd_unregister_notify(struct notifier_block *nb)
> +{
> +	blocking_notifier_chain_unregister(&cros_usbpd_notifier_list, nb);
> +}
> +EXPORT_SYMBOL_GPL(cros_usbpd_unregister_notify);
> +
> +#ifdef CONFIG_ACPI
> +
> +static int cros_usbpd_notify_add_acpi(struct acpi_device *adev)
> +{
> +	return 0;
> +}
> +
> +static void cros_usbpd_notify_acpi(struct acpi_device *adev, u32 event)
> +{
> +	blocking_notifier_call_chain(&cros_usbpd_notifier_list, event, NULL);
> +}
> +
> +static const struct acpi_device_id cros_usbpd_notify_acpi_device_ids[] = {
> +	{ ACPI_DRV_NAME, 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(acpi, cros_usbpd_acpi_device_ids);
> +
> +static struct acpi_driver cros_usbpd_notify_driver = {
> +	.name = DRV_NAME,
> +	.class = DRV_NAME,
> +	.ids = cros_usbpd_notify_acpi_device_ids,
> +	.ops = {
> +		.add = cros_usbpd_notify_add_acpi,
> +		.notify = cros_usbpd_notify_acpi,
> +	},
> +};
> +module_acpi_driver(cros_usbpd_notify_driver);
> +
> +#else /* CONFIG_ACPI */
> +
> +static int cros_usbpd_notify_plat(struct notifier_block *nb,
> +		unsigned long queued_during_suspend, void *data)
> +{
> +	struct cros_ec_device *ec_dev = (struct cros_ec_device *)data;
> +	u32 host_event = cros_ec_get_host_event(ec_dev);
> +
> +	if (!host_event)
> +		return NOTIFY_BAD;
> +
> +	if (host_event & EC_HOST_EVENT_MASK(EC_HOST_EVENT_PD_MCU)) {
> +		blocking_notifier_call_chain(&cros_usbpd_notifier_list,
> +				host_event, NULL);
> +		return NOTIFY_OK;
> +	}
> +	return NOTIFY_DONE;
> +}
> +
> +static int cros_usbpd_notify_probe_plat(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct cros_ec_dev *ecdev = dev_get_drvdata(dev->parent);
> +	struct notifier_block *nb;
> +	int ret;
> +
> +	nb = devm_kzalloc(dev, sizeof(*nb), GFP_KERNEL);
> +	if (!nb)
> +		return -ENOMEM;
> +
> +	nb->notifier_call = cros_usbpd_notify_plat;
> +	dev_set_drvdata(dev, nb);
> +
> +	ret = blocking_notifier_chain_register(&ecdev->ec_dev->event_notifier,
> +						nb);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to register notifier\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int cros_usbpd_notify_remove_plat(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct cros_ec_dev *ecdev = dev_get_drvdata(dev->parent);
> +	struct notifier_block *nb =
> +		(struct notifier_block *)dev_get_drvdata(dev);
> +
> +	blocking_notifier_chain_unregister(&ecdev->ec_dev->event_notifier,
> +			nb);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver cros_usbpd_notify_driver = {
> +	.driver = {
> +		.name = DRV_NAME,
> +	},
> +	.probe = cros_usbpd_notify_probe_plat,
> +	.remove = cros_usbpd_notify_remove_plat,
> +};
> +module_platform_driver(cros_usbpd_notify_driver);
> +
> +#endif /* CONFIG_ACPI */
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("ChromeOS power delivery notifier device");
> +MODULE_AUTHOR("Jon Flatley <jflat@chromium.org>");
> +MODULE_ALIAS("platform:" DRV_NAME);
> diff --git a/include/linux/platform_data/cros_usbpd_notify.h b/include/linux/platform_data/cros_usbpd_notify.h
> new file mode 100644
> index 0000000000000..cd7c7bebf18da
> --- /dev/null
> +++ b/include/linux/platform_data/cros_usbpd_notify.h
> @@ -0,0 +1,17 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * ChromeOS EC Power Delivery Notifier Driver
> + *
> + * Copyright 2019 Google LLC
> + */
> +
> +#ifndef __LINUX_PLATFORM_DATA_CROS_USBPD_NOTIFY_H
> +#define __LINUX_PLATFORM_DATA_CROS_USBPD_NOTIFY_H
> +
> +#include <linux/notifier.h>
> +
> +int cros_usbpd_register_notify(struct notifier_block *nb);
> +
> +void cros_usbpd_unregister_notify(struct notifier_block *nb);
> +
> +#endif  /* __LINUX_PLATFORM_DATA_CROS_USBPD_NOTIFY_H */
> 

Re: [PATCH v4 2/2] mfd: cros_ec: Add cros-usbpd-notify subdevice

[Enric Balletbo i Serra]

Hi Prashant,

On 20/12/19 20:38, Prashant Malani wrote:
> Add the cros-usbpd-notify driver as a subdevice on non-ACPI platforms
> that support the EC_FEATURE_USB_PD EC feature flag.
> 
> This driver allows other cros-ec devices to receive PD event
> notifications from the Chrome OS Embedded Controller (EC) via a
> notification chain.
> 
> Signed-off-by: Prashant Malani <pmalani@chromium.org>
> ---
> 
> Changes in v4:
> - Removed #ifndef usage; instead, moved cros-usbpd-notify to a separate
>   mfd_cell and used an IS_ENABLED() check.
> - Changed commit title and description slightly to reflect change in
>   code.
> 
>  drivers/mfd/cros_ec_dev.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
> index c4b977a5dd966..da198abe2b0a6 100644
> --- a/drivers/mfd/cros_ec_dev.c
> +++ b/drivers/mfd/cros_ec_dev.c
> @@ -5,6 +5,7 @@
>   * Copyright (C) 2014 Google, Inc.
>   */
>  
> +#include <linux/kconfig.h>
>  #include <linux/mfd/core.h>
>  #include <linux/mfd/cros_ec.h>
>  #include <linux/module.h>
> @@ -87,6 +88,10 @@ static const struct mfd_cell cros_usbpd_charger_cells[] = {
>  	{ .name = "cros-usbpd-logger", },
>  };
>  
> +static const struct mfd_cell cros_usbpd_notify_cells[] = {
> +	{ .name = "cros-usbpd-notify", },
> +};
> +
>  static const struct cros_feature_to_cells cros_subdevices[] = {
>  	{
>  		.id		= EC_FEATURE_CEC,
> @@ -202,6 +207,22 @@ static int ec_device_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> +	/*
> +	 * The PD notifier driver cell is separate since it only needs to be
> +	 * explicitly added on non-ACPI platforms.


Sorry to not catch this before, but a worry arose. Is non-ACPI platforms or
non-X86 platforms or on OF platforms?

ARM64 for example has the CONFIG_ACPI symbol set to yes, with the below
condition condition will not work on Kevin for example and IIUC this is not what
we want, I think we want IS_ENABLED(CONFIG_OF)?

Thanks,
 Enric

> +	 */
> +	if (!IS_ENABLED(CONFIG_ACPI)) {
> +		if (cros_ec_check_features(ec, EC_FEATURE_USB_PD)) {
> +			retval = mfd_add_hotplug_devices(ec->dev,
> +					cros_usbpd_notify_cells,
> +					ARRAY_SIZE(cros_usbpd_notify_cells));
> +			if (retval)
> +				dev_err(ec->dev,
> +					"failed to add PD notify devices: %d\n",
> +					retval);
> +		}
> +	}
> +
>  	/*
>  	 * The following subdevices cannot be detected by sending the
>  	 * EC_FEATURE_GET_CMD to the Embedded Controller device.
> 

Re: [PATCH v4 1/2] platform: chrome: Add cros-usbpd-notify driver

[Enric Balletbo i Serra]

Hi Prashant,

On 23/12/19 8:18, Enric Balletbo i Serra wrote:
> Hi Prashant,
> 
> On 20/12/19 20:38, Prashant Malani wrote:
>> From: Jon Flatley <jflat@chromium.org>
>>
>> ChromiumOS uses ACPI device with HID "GOOG0003" for power delivery
>> related events. The existing cros-usbpd-charger driver relies on these
>> events without ever actually receiving them on ACPI platforms. This is
>> because in the ChromeOS kernel trees, the GOOG0003 device is owned by an
>> ACPI driver that offers firmware updates to USB-C chargers.
>>
>> Introduce a new platform driver under cros-ec, the ChromeOS embedded
>> controller, that handles these PD events and dispatches them
>> appropriately over a notifier chain to all drivers that use them.
>>
>> On non-ACPI platforms, the driver gets instantiated for ECs which
>> support the EC_FEATURE_USB_PD feature bit, and on such platforms, the
>> notification events will get delivered using the MKBP event handling
>> mechanism.
>>
>> Co-Developed-by: Prashant Malani <pmalani@chromium.org>
>> Reviewed-by: Gwendal Grignou <gwendal@chromium.org>
>> Signed-off-by: Jon Flatley <jflat@chromium.org>
>> Signed-off-by: Prashant Malani <pmalani@chromium.org>
> 
> For my own reference:
> 
> Acked-for-chrome-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> 

Sorry, too much rush acking this patch, here applies the same comment as patch 2

>> ---
>>
>> Changes in v4(pmalani@chromium.org):
>> - No code changes, but added new version so that versioning is
>>   consistent with the next patch in the series.
>>
>> Changes in v3 (pmalani@chromium.org):
>> - Renamed driver and files from "cros_ec_pd_notify" to
>>   "cros_usbpd_notify" to be more consistent with other naming.
>> - Moved the change to include cros-usbpd-notify in the charger MFD into
>>   a separate follow-on patch.
>>
>> Changes in v2 (pmalani@chromium.org):
>> - Removed dependency on DT entry; instead, we will instantiate the
>>   driver on detecting EC_FEATURE_USB_PD for non-ACPI platforms.
>> - Modified the cros-ec-pd-notify device to be an mfd_cell under
>>   usbpdcharger for non-ACPI platforms. Altered the platform_probe() call
>>   to derive the cros EC structs appropriately.
>> - Replaced "usbpd_notify" with "pd_notify" in functions and structures.
>> - Addressed comments from upstream maintainer.
>>
>>  drivers/platform/chrome/Kconfig               |   9 ++
>>  drivers/platform/chrome/Makefile              |   1 +
>>  drivers/platform/chrome/cros_usbpd_notify.c   | 151 ++++++++++++++++++
>>  .../linux/platform_data/cros_usbpd_notify.h   |  17 ++
>>  4 files changed, 178 insertions(+)
>>  create mode 100644 drivers/platform/chrome/cros_usbpd_notify.c
>>  create mode 100644 include/linux/platform_data/cros_usbpd_notify.h
>>
>> diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
>> index 5f57282a28da0..3a8a98f2fb4d1 100644
>> --- a/drivers/platform/chrome/Kconfig
>> +++ b/drivers/platform/chrome/Kconfig
>> @@ -226,6 +226,15 @@ config CROS_USBPD_LOGGER
>>  	  To compile this driver as a module, choose M here: the
>>  	  module will be called cros_usbpd_logger.
>>  
>> +config CROS_USBPD_NOTIFY
>> +	tristate "ChromeOS Type-C power delivery event notifier"
>> +	depends on CROS_EC
>> +	help
>> +	  If you say Y here, you get support for Type-C PD event notifications
>> +	  from the ChromeOS EC. On ACPI platorms this driver will bind to the
>> +	  GOOG0003 ACPI device, and on non-ACPI platforms this driver will get
>> +	  initialized on ECs which support the feature EC_FEATURE_USB_PD.
>> +
>>  source "drivers/platform/chrome/wilco_ec/Kconfig"
>>  
>>  endif # CHROMEOS_PLATFORMS
>> diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
>> index aacd5920d8a18..f6465f8ef0b5e 100644
>> --- a/drivers/platform/chrome/Makefile
>> +++ b/drivers/platform/chrome/Makefile
>> @@ -22,5 +22,6 @@ obj-$(CONFIG_CROS_EC_DEBUGFS)		+= cros_ec_debugfs.o
>>  obj-$(CONFIG_CROS_EC_SENSORHUB)		+= cros_ec_sensorhub.o
>>  obj-$(CONFIG_CROS_EC_SYSFS)		+= cros_ec_sysfs.o
>>  obj-$(CONFIG_CROS_USBPD_LOGGER)		+= cros_usbpd_logger.o
>> +obj-$(CONFIG_CROS_USBPD_NOTIFY)		+= cros_usbpd_notify.o
>>  
>>  obj-$(CONFIG_WILCO_EC)			+= wilco_ec/
>> diff --git a/drivers/platform/chrome/cros_usbpd_notify.c b/drivers/platform/chrome/cros_usbpd_notify.c
>> new file mode 100644
>> index 0000000000000..05a7db834d2e0
>> --- /dev/null
>> +++ b/drivers/platform/chrome/cros_usbpd_notify.c
>> @@ -0,0 +1,151 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright 2019 Google LLC
>> + *
>> + * This driver serves as the receiver of cros_ec PD host events.
>> + */
>> +
>> +#include <linux/acpi.h>
>> +#include <linux/module.h>
>> +#include <linux/mfd/cros_ec.h>
>> +#include <linux/platform_data/cros_ec_commands.h>
>> +#include <linux/platform_data/cros_usbpd_notify.h>
>> +#include <linux/platform_data/cros_ec_proto.h>
>> +#include <linux/platform_device.h>
>> +
>> +#define DRV_NAME "cros-usbpd-notify"
>> +#define ACPI_DRV_NAME "GOOG0003"
>> +
>> +static BLOCKING_NOTIFIER_HEAD(cros_usbpd_notifier_list);
>> +
>> +/**
>> + * cros_usbpd_register_notify - Register a notifier callback for PD events.
>> + * @nb: Notifier block pointer to register
>> + *
>> + * On ACPI platforms this corresponds to host events on the ECPD
>> + * "GOOG0003" ACPI device. On non-ACPI platforms this will filter mkbp events
>> + * for USB PD events.
>> + *
>> + * Return: 0 on success or negative error code.
>> + */
>> +int cros_usbpd_register_notify(struct notifier_block *nb)
>> +{
>> +	return blocking_notifier_chain_register(
>> +			&cros_usbpd_notifier_list, nb);
>> +}
>> +EXPORT_SYMBOL_GPL(cros_usbpd_register_notify);
>> +
>> +
>> +/**
>> + * cros_usbpd_unregister_notify - Unregister notifier callback for PD events.
>> + * @nb: Notifier block pointer to unregister
>> + *
>> + * Unregister a notifier callback that was previously registered with
>> + * cros_usbpd_register_notify().
>> + */
>> +void cros_usbpd_unregister_notify(struct notifier_block *nb)
>> +{
>> +	blocking_notifier_chain_unregister(&cros_usbpd_notifier_list, nb);
>> +}
>> +EXPORT_SYMBOL_GPL(cros_usbpd_unregister_notify);
>> +
>> +#ifdef CONFIG_ACPI
>> +

For arm64 will follow this path, and this is not what we want, right?

>> +static int cros_usbpd_notify_add_acpi(struct acpi_device *adev)
>> +{
>> +	return 0;
>> +}
>> +
>> +static void cros_usbpd_notify_acpi(struct acpi_device *adev, u32 event)
>> +{
>> +	blocking_notifier_call_chain(&cros_usbpd_notifier_list, event, NULL);
>> +}
>> +
>> +static const struct acpi_device_id cros_usbpd_notify_acpi_device_ids[] = {
>> +	{ ACPI_DRV_NAME, 0 },
>> +	{ }
>> +};
>> +MODULE_DEVICE_TABLE(acpi, cros_usbpd_acpi_device_ids);
>> +
>> +static struct acpi_driver cros_usbpd_notify_driver = {
>> +	.name = DRV_NAME,
>> +	.class = DRV_NAME,
>> +	.ids = cros_usbpd_notify_acpi_device_ids,
>> +	.ops = {
>> +		.add = cros_usbpd_notify_add_acpi,
>> +		.notify = cros_usbpd_notify_acpi,
>> +	},
>> +};
>> +module_acpi_driver(cros_usbpd_notify_driver);
>> +
>> +#else /* CONFIG_ACPI */
>> +
>> +static int cros_usbpd_notify_plat(struct notifier_block *nb,
>> +		unsigned long queued_during_suspend, void *data)
>> +{
>> +	struct cros_ec_device *ec_dev = (struct cros_ec_device *)data;
>> +	u32 host_event = cros_ec_get_host_event(ec_dev);
>> +
>> +	if (!host_event)
>> +		return NOTIFY_BAD;
>> +
>> +	if (host_event & EC_HOST_EVENT_MASK(EC_HOST_EVENT_PD_MCU)) {
>> +		blocking_notifier_call_chain(&cros_usbpd_notifier_list,
>> +				host_event, NULL);
>> +		return NOTIFY_OK;
>> +	}
>> +	return NOTIFY_DONE;
>> +}
>> +
>> +static int cros_usbpd_notify_probe_plat(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct cros_ec_dev *ecdev = dev_get_drvdata(dev->parent);
>> +	struct notifier_block *nb;
>> +	int ret;
>> +
>> +	nb = devm_kzalloc(dev, sizeof(*nb), GFP_KERNEL);
>> +	if (!nb)
>> +		return -ENOMEM;
>> +
>> +	nb->notifier_call = cros_usbpd_notify_plat;
>> +	dev_set_drvdata(dev, nb);
>> +
>> +	ret = blocking_notifier_chain_register(&ecdev->ec_dev->event_notifier,
>> +						nb);
>> +	if (ret < 0) {
>> +		dev_err(dev, "Failed to register notifier\n");
>> +		return ret;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int cros_usbpd_notify_remove_plat(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct cros_ec_dev *ecdev = dev_get_drvdata(dev->parent);
>> +	struct notifier_block *nb =
>> +		(struct notifier_block *)dev_get_drvdata(dev);
>> +
>> +	blocking_notifier_chain_unregister(&ecdev->ec_dev->event_notifier,
>> +			nb);
>> +
>> +	return 0;
>> +}
>> +
>> +static struct platform_driver cros_usbpd_notify_driver = {
>> +	.driver = {
>> +		.name = DRV_NAME,
>> +	},
>> +	.probe = cros_usbpd_notify_probe_plat,
>> +	.remove = cros_usbpd_notify_remove_plat,
>> +};
>> +module_platform_driver(cros_usbpd_notify_driver);
>> +
>> +#endif /* CONFIG_ACPI */
>> +
>> +MODULE_LICENSE("GPL");
>> +MODULE_DESCRIPTION("ChromeOS power delivery notifier device");
>> +MODULE_AUTHOR("Jon Flatley <jflat@chromium.org>");
>> +MODULE_ALIAS("platform:" DRV_NAME);
>> diff --git a/include/linux/platform_data/cros_usbpd_notify.h b/include/linux/platform_data/cros_usbpd_notify.h
>> new file mode 100644
>> index 0000000000000..cd7c7bebf18da
>> --- /dev/null
>> +++ b/include/linux/platform_data/cros_usbpd_notify.h
>> @@ -0,0 +1,17 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * ChromeOS EC Power Delivery Notifier Driver
>> + *
>> + * Copyright 2019 Google LLC
>> + */
>> +
>> +#ifndef __LINUX_PLATFORM_DATA_CROS_USBPD_NOTIFY_H
>> +#define __LINUX_PLATFORM_DATA_CROS_USBPD_NOTIFY_H
>> +
>> +#include <linux/notifier.h>
>> +
>> +int cros_usbpd_register_notify(struct notifier_block *nb);
>> +
>> +void cros_usbpd_unregister_notify(struct notifier_block *nb);
>> +
>> +#endif  /* __LINUX_PLATFORM_DATA_CROS_USBPD_NOTIFY_H */
>>

Re: [PATCH v4 2/2] mfd: cros_ec: Add cros-usbpd-notify subdevice

[Prashant Malani]

Hi Enric,

On Sun, Dec 22, 2019 at 11:25 PM Enric Balletbo i Serra
<enric.balletbo@collabora.com> wrote:
>
> Hi Prashant,
>
> On 20/12/19 20:38, Prashant Malani wrote:
> > Add the cros-usbpd-notify driver as a subdevice on non-ACPI platforms
> > that support the EC_FEATURE_USB_PD EC feature flag.
> >
> > This driver allows other cros-ec devices to receive PD event
> > notifications from the Chrome OS Embedded Controller (EC) via a
> > notification chain.
> >
> > Signed-off-by: Prashant Malani <pmalani@chromium.org>
> > ---
> >
> > Changes in v4:
> > - Removed #ifndef usage; instead, moved cros-usbpd-notify to a separate
> >   mfd_cell and used an IS_ENABLED() check.
> > - Changed commit title and description slightly to reflect change in
> >   code.
> >
> >  drivers/mfd/cros_ec_dev.c | 21 +++++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> >
> > diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
> > index c4b977a5dd966..da198abe2b0a6 100644
> > --- a/drivers/mfd/cros_ec_dev.c
> > +++ b/drivers/mfd/cros_ec_dev.c
> > @@ -5,6 +5,7 @@
> >   * Copyright (C) 2014 Google, Inc.
> >   */
> >
> > +#include <linux/kconfig.h>
> >  #include <linux/mfd/core.h>
> >  #include <linux/mfd/cros_ec.h>
> >  #include <linux/module.h>
> > @@ -87,6 +88,10 @@ static const struct mfd_cell cros_usbpd_charger_cells[] = {
> >       { .name = "cros-usbpd-logger", },
> >  };
> >
> > +static const struct mfd_cell cros_usbpd_notify_cells[] = {
> > +     { .name = "cros-usbpd-notify", },
> > +};
> > +
> >  static const struct cros_feature_to_cells cros_subdevices[] = {
> >       {
> >               .id             = EC_FEATURE_CEC,
> > @@ -202,6 +207,22 @@ static int ec_device_probe(struct platform_device *pdev)
> >               }
> >       }
> >
> > +     /*
> > +      * The PD notifier driver cell is separate since it only needs to be
> > +      * explicitly added on non-ACPI platforms.
>
>
> Sorry to not catch this before, but a worry arose. Is non-ACPI platforms or
> non-X86 platforms or on OF platforms?
>
> ARM64 for example has the CONFIG_ACPI symbol set to yes, with the below
> condition condition will not work on Kevin for example and IIUC this is not what
> we want, I think we want IS_ENABLED(CONFIG_OF)?
Thanks for noting this. I will check with a kevin, and with the
internal build flags to verify whether there are ARM64 which have the
GOOG0003 PD notification device.
I'll update this thread with my findings.

Best,

>
> Thanks,
>  Enric
>
> > +      */
> > +     if (!IS_ENABLED(CONFIG_ACPI)) {
> > +             if (cros_ec_check_features(ec, EC_FEATURE_USB_PD)) {
> > +                     retval = mfd_add_hotplug_devices(ec->dev,
> > +                                     cros_usbpd_notify_cells,
> > +                                     ARRAY_SIZE(cros_usbpd_notify_cells));
> > +                     if (retval)
> > +                             dev_err(ec->dev,
> > +                                     "failed to add PD notify devices: %d\n",
> > +                                     retval);
> > +             }
> > +     }
> > +
> >       /*
> >        * The following subdevices cannot be detected by sending the
> >        * EC_FEATURE_GET_CMD to the Embedded Controller device.
> >

Re: [PATCH v4 2/2] mfd: cros_ec: Add cros-usbpd-notify subdevice

[Prashant Malani]

Hi Enric,


On Mon, Dec 23, 2019 at 12:29 PM Prashant Malani <pmalani@chromium.org> wrote:
>
> Hi Enric,
>
> On Sun, Dec 22, 2019 at 11:25 PM Enric Balletbo i Serra
> <enric.balletbo@collabora.com> wrote:
> >
> > Hi Prashant,
> >
> > On 20/12/19 20:38, Prashant Malani wrote:
> > > Add the cros-usbpd-notify driver as a subdevice on non-ACPI platforms
> > > that support the EC_FEATURE_USB_PD EC feature flag.
> > >
> > > This driver allows other cros-ec devices to receive PD event
> > > notifications from the Chrome OS Embedded Controller (EC) via a
> > > notification chain.
> > >
> > > Signed-off-by: Prashant Malani <pmalani@chromium.org>
> > > ---
> > >
> > > Changes in v4:
> > > - Removed #ifndef usage; instead, moved cros-usbpd-notify to a separate
> > >   mfd_cell and used an IS_ENABLED() check.
> > > - Changed commit title and description slightly to reflect change in
> > >   code.
> > >
> > >  drivers/mfd/cros_ec_dev.c | 21 +++++++++++++++++++++
> > >  1 file changed, 21 insertions(+)
> > >
> > > diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
> > > index c4b977a5dd966..da198abe2b0a6 100644
> > > --- a/drivers/mfd/cros_ec_dev.c
> > > +++ b/drivers/mfd/cros_ec_dev.c
> > > @@ -5,6 +5,7 @@
> > >   * Copyright (C) 2014 Google, Inc.
> > >   */
> > >
> > > +#include <linux/kconfig.h>
> > >  #include <linux/mfd/core.h>
> > >  #include <linux/mfd/cros_ec.h>
> > >  #include <linux/module.h>
> > > @@ -87,6 +88,10 @@ static const struct mfd_cell cros_usbpd_charger_cells[] = {
> > >       { .name = "cros-usbpd-logger", },
> > >  };
> > >
> > > +static const struct mfd_cell cros_usbpd_notify_cells[] = {
> > > +     { .name = "cros-usbpd-notify", },
> > > +};
> > > +
> > >  static const struct cros_feature_to_cells cros_subdevices[] = {
> > >       {
> > >               .id             = EC_FEATURE_CEC,
> > > @@ -202,6 +207,22 @@ static int ec_device_probe(struct platform_device *pdev)
> > >               }
> > >       }
> > >
> > > +     /*
> > > +      * The PD notifier driver cell is separate since it only needs to be
> > > +      * explicitly added on non-ACPI platforms.
> >
> >
> > Sorry to not catch this before, but a worry arose. Is non-ACPI platforms or
> > non-X86 platforms or on OF platforms?
> >
> > ARM64 for example has the CONFIG_ACPI symbol set to yes, with the below
> > condition condition will not work on Kevin for example and IIUC this is not what
> > we want, I think we want IS_ENABLED(CONFIG_OF)?
> Thanks for noting this. I will check with a kevin, and with the
> internal build flags to verify whether there are ARM64 which have the
> GOOG0003 PD notification device.
> I'll update this thread with my findings.
AFAICT from the Chrome OS kernel build step .config output, kevin
doesn't have CONFIG_ACPI enabled (it is marked as "# CONFIG_ACPI is
not set"), and it doesn't look like there are Chrome OS ARM devices
that use ACPI (I believe it's only used on Chrome OS x86-based
devices). So perhaps it is not a concern?

>
> Best,
>
> >
> > Thanks,
> >  Enric
> >
> > > +      */
> > > +     if (!IS_ENABLED(CONFIG_ACPI)) {
> > > +             if (cros_ec_check_features(ec, EC_FEATURE_USB_PD)) {
> > > +                     retval = mfd_add_hotplug_devices(ec->dev,
> > > +                                     cros_usbpd_notify_cells,
> > > +                                     ARRAY_SIZE(cros_usbpd_notify_cells));
> > > +                     if (retval)
> > > +                             dev_err(ec->dev,
> > > +                                     "failed to add PD notify devices: %d\n",
> > > +                                     retval);
> > > +             }
> > > +     }
> > > +
> > >       /*
> > >        * The following subdevices cannot be detected by sending the
> > >        * EC_FEATURE_GET_CMD to the Embedded Controller device.
> > >

Re: [PATCH v4 2/2] mfd: cros_ec: Add cros-usbpd-notify subdevice

[Enric Balletbo i Serra]

Hi Prashant,

On 3/1/20 0:52, Prashant Malani wrote:
> Hi Enric,
> 
> 
> On Mon, Dec 23, 2019 at 12:29 PM Prashant Malani <pmalani@chromium.org> wrote:
>>
>> Hi Enric,
>>
>> On Sun, Dec 22, 2019 at 11:25 PM Enric Balletbo i Serra
>> <enric.balletbo@collabora.com> wrote:
>>>
>>> Hi Prashant,
>>>
>>> On 20/12/19 20:38, Prashant Malani wrote:
>>>> Add the cros-usbpd-notify driver as a subdevice on non-ACPI platforms
>>>> that support the EC_FEATURE_USB_PD EC feature flag.
>>>>
>>>> This driver allows other cros-ec devices to receive PD event
>>>> notifications from the Chrome OS Embedded Controller (EC) via a
>>>> notification chain.
>>>>
>>>> Signed-off-by: Prashant Malani <pmalani@chromium.org>
>>>> ---
>>>>
>>>> Changes in v4:
>>>> - Removed #ifndef usage; instead, moved cros-usbpd-notify to a separate
>>>>   mfd_cell and used an IS_ENABLED() check.
>>>> - Changed commit title and description slightly to reflect change in
>>>>   code.
>>>>
>>>>  drivers/mfd/cros_ec_dev.c | 21 +++++++++++++++++++++
>>>>  1 file changed, 21 insertions(+)
>>>>
>>>> diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
>>>> index c4b977a5dd966..da198abe2b0a6 100644
>>>> --- a/drivers/mfd/cros_ec_dev.c
>>>> +++ b/drivers/mfd/cros_ec_dev.c
>>>> @@ -5,6 +5,7 @@
>>>>   * Copyright (C) 2014 Google, Inc.
>>>>   */
>>>>
>>>> +#include <linux/kconfig.h>
>>>>  #include <linux/mfd/core.h>
>>>>  #include <linux/mfd/cros_ec.h>
>>>>  #include <linux/module.h>
>>>> @@ -87,6 +88,10 @@ static const struct mfd_cell cros_usbpd_charger_cells[] = {
>>>>       { .name = "cros-usbpd-logger", },
>>>>  };
>>>>
>>>> +static const struct mfd_cell cros_usbpd_notify_cells[] = {
>>>> +     { .name = "cros-usbpd-notify", },
>>>> +};
>>>> +
>>>>  static const struct cros_feature_to_cells cros_subdevices[] = {
>>>>       {
>>>>               .id             = EC_FEATURE_CEC,
>>>> @@ -202,6 +207,22 @@ static int ec_device_probe(struct platform_device *pdev)
>>>>               }
>>>>       }
>>>>
>>>> +     /*
>>>> +      * The PD notifier driver cell is separate since it only needs to be
>>>> +      * explicitly added on non-ACPI platforms.
>>>
>>>
>>> Sorry to not catch this before, but a worry arose. Is non-ACPI platforms or
>>> non-X86 platforms or on OF platforms?
>>>
>>> ARM64 for example has the CONFIG_ACPI symbol set to yes, with the below
>>> condition condition will not work on Kevin for example and IIUC this is not what
>>> we want, I think we want IS_ENABLED(CONFIG_OF)?
>> Thanks for noting this. I will check with a kevin, and with the
>> internal build flags to verify whether there are ARM64 which have the
>> GOOG0003 PD notification device.
>> I'll update this thread with my findings.
> AFAICT from the Chrome OS kernel build step .config output, kevin
> doesn't have CONFIG_ACPI enabled (it is marked as "# CONFIG_ACPI is
> not set"), and it doesn't look like there are Chrome OS ARM devices
> that use ACPI (I believe it's only used on Chrome OS x86-based
> devices). So perhaps it is not a concern?
> 

The problem is, although it is not used in your configs, it can be selected, and
fwiw some defconfigs in mainline have it enabled, i.e the arm64 defconfig.

I think you're testing the patch on x86 but I suspect we want also the notifier
on some arm64 platforms (like kevin) right? In such case I won't get the
notifier because CONFIG_OF and CONFIG_ACPI is enabled on my defconfig.

My guess is that the logic should be if IS_ENABLED(CONFIG_OF) call
cros_ec_check_features, otherwise ACPI will do the magic instead of
(!IS_ENABLED(CONFIG_ACPI))

Best,
 Enric

>>
>> Best,
>>
>>>
>>> Thanks,
>>>  Enric
>>>
>>>> +      */
>>>> +     if (!IS_ENABLED(CONFIG_ACPI)) {
>>>> +             if (cros_ec_check_features(ec, EC_FEATURE_USB_PD)) {
>>>> +                     retval = mfd_add_hotplug_devices(ec->dev,
>>>> +                                     cros_usbpd_notify_cells,
>>>> +                                     ARRAY_SIZE(cros_usbpd_notify_cells));
>>>> +                     if (retval)
>>>> +                             dev_err(ec->dev,
>>>> +                                     "failed to add PD notify devices: %d\n",
>>>> +                                     retval);
>>>> +             }
>>>> +     }
>>>> +
>>>>       /*
>>>>        * The following subdevices cannot be detected by sending the
>>>>        * EC_FEATURE_GET_CMD to the Embedded Controller device.
>>>>

Re: [PATCH v4 2/2] mfd: cros_ec: Add cros-usbpd-notify subdevice

[Prashant Malani]

Hi Enric,

On Tue, Jan 7, 2020 at 8:51 AM Enric Balletbo i Serra
<enric.balletbo@collabora.com> wrote:
>
> Hi Prashant,
>
> On 3/1/20 0:52, Prashant Malani wrote:
> > Hi Enric,
> >
> >>> Sorry to not catch this before, but a worry arose. Is non-ACPI platforms or
> >>> non-X86 platforms or on OF platforms?
> >>>
> >>> ARM64 for example has the CONFIG_ACPI symbol set to yes, with the below
> >>> condition condition will not work on Kevin for example and IIUC this is not what
> >>> we want, I think we want IS_ENABLED(CONFIG_OF)?
> >> Thanks for noting this. I will check with a kevin, and with the
> >> internal build flags to verify whether there are ARM64 which have the
> >> GOOG0003 PD notification device.
> >> I'll update this thread with my findings.
> > AFAICT from the Chrome OS kernel build step .config output, kevin
> > doesn't have CONFIG_ACPI enabled (it is marked as "# CONFIG_ACPI is
> > not set"), and it doesn't look like there are Chrome OS ARM devices
> > that use ACPI (I believe it's only used on Chrome OS x86-based
> > devices). So perhaps it is not a concern?
> >
>
> The problem is, although it is not used in your configs, it can be selected, and
> fwiw some defconfigs in mainline have it enabled, i.e the arm64 defconfig.
>
> I think you're testing the patch on x86 but I suspect we want also the notifier
> on some arm64 platforms (like kevin) right? In such case I won't get the
> notifier because CONFIG_OF and CONFIG_ACPI is enabled on my defconfig.
I tested the patchset on an arm64 platform and confirmed that
CONFIG_ACPI is enabled, but I suppose that is not a sufficient test
given that, as you mentioned, upstream configs like arm64 defconfig
have it enabled.
>
> My guess is that the logic should be if IS_ENABLED(CONFIG_OF) call
> cros_ec_check_features, otherwise ACPI will do the magic instead of
> (!IS_ENABLED(CONFIG_ACPI))
Noted. Thanks! I have a pending query on Patch 1/2. Once that is
resolved, I will update both patches.
>
> Best,
>  Enric
>
> >>
> >> Best,
> >>
> >>>
> >>> Thanks,
> >>>  Enric
> >>>
> >>>> +      */
> >>>> +     if (!IS_ENABLED(CONFIG_ACPI)) {
> >>>> +             if (cros_ec_check_features(ec, EC_FEATURE_USB_PD)) {
> >>>> +                     retval = mfd_add_hotplug_devices(ec->dev,
> >>>> +                                     cros_usbpd_notify_cells,
> >>>> +                                     ARRAY_SIZE(cros_usbpd_notify_cells));
> >>>> +                     if (retval)
> >>>> +                             dev_err(ec->dev,
> >>>> +                                     "failed to add PD notify devices: %d\n",
> >>>> +                                     retval);
> >>>> +             }
> >>>> +     }
> >>>> +
> >>>>       /*
> >>>>        * The following subdevices cannot be detected by sending the
> >>>>        * EC_FEATURE_GET_CMD to the Embedded Controller device.
> >>>>

Re: [PATCH v4 1/2] platform: chrome: Add cros-usbpd-notify driver

[Prashant Malani]

Hi Enric,

On Sun, Dec 22, 2019 at 11:40 PM Enric Balletbo i Serra
<enric.balletbo@collabora.com> wrote:
>
> Hi Prashant,
>
> On 23/12/19 8:18, Enric Balletbo i Serra wrote:
> > Hi Prashant,
> >
> > On 20/12/19 20:38, Prashant Malani wrote:
> >> From: Jon Flatley <jflat@chromium.org>
> >>
> >> ChromiumOS uses ACPI device with HID "GOOG0003" for power delivery
> >> related events. The existing cros-usbpd-charger driver relies on these
> >> events without ever actually receiving them on ACPI platforms. This is
> >> because in the ChromeOS kernel trees, the GOOG0003 device is owned by an
> >> ACPI driver that offers firmware updates to USB-C chargers.
> >>
> >> Introduce a new platform driver under cros-ec, the ChromeOS embedded
> >> controller, that handles these PD events and dispatches them
> >> appropriately over a notifier chain to all drivers that use them.
> >>
> >> On non-ACPI platforms, the driver gets instantiated for ECs which
> >> support the EC_FEATURE_USB_PD feature bit, and on such platforms, the
> >> notification events will get delivered using the MKBP event handling
> >> mechanism.
> >>
> >> Co-Developed-by: Prashant Malani <pmalani@chromium.org>
> >> Reviewed-by: Gwendal Grignou <gwendal@chromium.org>
> >> Signed-off-by: Jon Flatley <jflat@chromium.org>
> >> Signed-off-by: Prashant Malani <pmalani@chromium.org>
> >
> > For my own reference:
> >
> > Acked-for-chrome-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> >
>
> Sorry, too much rush acking this patch, here applies the same comment as patch 2
>
> >> ---
> >>
> >> Changes in v4(pmalani@chromium.org):
> >> - No code changes, but added new version so that versioning is
> >>   consistent with the next patch in the series.
> >>
> >> Changes in v3 (pmalani@chromium.org):
> >> - Renamed driver and files from "cros_ec_pd_notify" to
> >>   "cros_usbpd_notify" to be more consistent with other naming.
> >> - Moved the change to include cros-usbpd-notify in the charger MFD into
> >>   a separate follow-on patch.
> >>
> >> Changes in v2 (pmalani@chromium.org):
> >> - Removed dependency on DT entry; instead, we will instantiate the
> >>   driver on detecting EC_FEATURE_USB_PD for non-ACPI platforms.
> >> - Modified the cros-ec-pd-notify device to be an mfd_cell under
> >>   usbpdcharger for non-ACPI platforms. Altered the platform_probe() call
> >>   to derive the cros EC structs appropriately.
> >> - Replaced "usbpd_notify" with "pd_notify" in functions and structures.
> >> - Addressed comments from upstream maintainer.
> >>
> >>  drivers/platform/chrome/Kconfig               |   9 ++
> >>  drivers/platform/chrome/Makefile              |   1 +
> >>  drivers/platform/chrome/cros_usbpd_notify.c   | 151 ++++++++++++++++++
> >>  .../linux/platform_data/cros_usbpd_notify.h   |  17 ++
> >>  4 files changed, 178 insertions(+)
> >>  create mode 100644 drivers/platform/chrome/cros_usbpd_notify.c
> >>  create mode 100644 include/linux/platform_data/cros_usbpd_notify.h
> >>
> >> diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
> >> index 5f57282a28da0..3a8a98f2fb4d1 100644
> >> --- a/drivers/platform/chrome/Kconfig
> >> +++ b/drivers/platform/chrome/Kconfig
> >> @@ -226,6 +226,15 @@ config CROS_USBPD_LOGGER
> >>        To compile this driver as a module, choose M here: the
> >>        module will be called cros_usbpd_logger.
> >>
> >> +config CROS_USBPD_NOTIFY
> >> +    tristate "ChromeOS Type-C power delivery event notifier"
> >> +    depends on CROS_EC
> >> +    help
> >> +      If you say Y here, you get support for Type-C PD event notifications
> >> +      from the ChromeOS EC. On ACPI platorms this driver will bind to the
> >> +      GOOG0003 ACPI device, and on non-ACPI platforms this driver will get
> >> +      initialized on ECs which support the feature EC_FEATURE_USB_PD.
> >> +
> >>  source "drivers/platform/chrome/wilco_ec/Kconfig"
> >>
> >>  endif # CHROMEOS_PLATFORMS
> >> diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
> >> index aacd5920d8a18..f6465f8ef0b5e 100644
> >> --- a/drivers/platform/chrome/Makefile
> >> +++ b/drivers/platform/chrome/Makefile
> >> @@ -22,5 +22,6 @@ obj-$(CONFIG_CROS_EC_DEBUGFS)              += cros_ec_debugfs.o
> >>  obj-$(CONFIG_CROS_EC_SENSORHUB)             += cros_ec_sensorhub.o
> >>  obj-$(CONFIG_CROS_EC_SYSFS)         += cros_ec_sysfs.o
> >>  obj-$(CONFIG_CROS_USBPD_LOGGER)             += cros_usbpd_logger.o
> >> +obj-$(CONFIG_CROS_USBPD_NOTIFY)             += cros_usbpd_notify.o
> >>
> >>  obj-$(CONFIG_WILCO_EC)                      += wilco_ec/
> >> diff --git a/drivers/platform/chrome/cros_usbpd_notify.c b/drivers/platform/chrome/cros_usbpd_notify.c
> >> new file mode 100644
> >> index 0000000000000..05a7db834d2e0
> >> --- /dev/null
> >> +++ b/drivers/platform/chrome/cros_usbpd_notify.c
> >> @@ -0,0 +1,151 @@
> >> +// SPDX-License-Identifier: GPL-2.0-only
> >> +/*
> >> + * Copyright 2019 Google LLC
> >> + *
> >> + * This driver serves as the receiver of cros_ec PD host events.
> >> + */
> >> +
> >> +#include <linux/acpi.h>
> >> +#include <linux/module.h>
> >> +#include <linux/mfd/cros_ec.h>
> >> +#include <linux/platform_data/cros_ec_commands.h>
> >> +#include <linux/platform_data/cros_usbpd_notify.h>
> >> +#include <linux/platform_data/cros_ec_proto.h>
> >> +#include <linux/platform_device.h>
> >> +
> >> +#define DRV_NAME "cros-usbpd-notify"
> >> +#define ACPI_DRV_NAME "GOOG0003"
> >> +
> >> +static BLOCKING_NOTIFIER_HEAD(cros_usbpd_notifier_list);
> >> +
> >> +/**
> >> + * cros_usbpd_register_notify - Register a notifier callback for PD events.
> >> + * @nb: Notifier block pointer to register
> >> + *
> >> + * On ACPI platforms this corresponds to host events on the ECPD
> >> + * "GOOG0003" ACPI device. On non-ACPI platforms this will filter mkbp events
> >> + * for USB PD events.
> >> + *
> >> + * Return: 0 on success or negative error code.
> >> + */
> >> +int cros_usbpd_register_notify(struct notifier_block *nb)
> >> +{
> >> +    return blocking_notifier_chain_register(
> >> +                    &cros_usbpd_notifier_list, nb);
> >> +}
> >> +EXPORT_SYMBOL_GPL(cros_usbpd_register_notify);
> >> +
> >> +
> >> +/**
> >> + * cros_usbpd_unregister_notify - Unregister notifier callback for PD events.
> >> + * @nb: Notifier block pointer to unregister
> >> + *
> >> + * Unregister a notifier callback that was previously registered with
> >> + * cros_usbpd_register_notify().
> >> + */
> >> +void cros_usbpd_unregister_notify(struct notifier_block *nb)
> >> +{
> >> +    blocking_notifier_chain_unregister(&cros_usbpd_notifier_list, nb);
> >> +}
> >> +EXPORT_SYMBOL_GPL(cros_usbpd_unregister_notify);
> >> +
> >> +#ifdef CONFIG_ACPI
> >> +
>
> For arm64 will follow this path, and this is not what we want, right?

(Following on from your latest email in Patch 2/2 in this series): I
agree it would (follow this path) if arm64 defconfig enables ACPI in
the mainline (side note: Chrome OS's kevin build leaves this config
disabled).
To ensure I make the right update to the patch, is the suggestion for
this patch that we use #ifndef CONFIG_OF instead of #ifdef CONFIG_ACPI
?
I think that may also be problematic since !CONFIG_OF doesn't
necessarily imply CONFIG_ACPI.

Perhaps we should just make them two separate ifdef blocks, i.e #ifdef
CONFIG_OF, and then #ifdef CONFIG_ACPI? Would be great to hear your
recommendation here.

Thanks again!

Re: [PATCH v4 1/2] platform: chrome: Add cros-usbpd-notify driver

[Enric Balletbo i Serra]

Hi Prashant,

On 7/1/20 19:07, Prashant Malani wrote:
> Hi Enric,
> 
> On Sun, Dec 22, 2019 at 11:40 PM Enric Balletbo i Serra
> <enric.balletbo@collabora.com> wrote:
>>
>> Hi Prashant,
>>
>> On 23/12/19 8:18, Enric Balletbo i Serra wrote:
>>> Hi Prashant,
>>>
>>> On 20/12/19 20:38, Prashant Malani wrote:
>>>> From: Jon Flatley <jflat@chromium.org>
>>>>
>>>> ChromiumOS uses ACPI device with HID "GOOG0003" for power delivery
>>>> related events. The existing cros-usbpd-charger driver relies on these
>>>> events without ever actually receiving them on ACPI platforms. This is
>>>> because in the ChromeOS kernel trees, the GOOG0003 device is owned by an
>>>> ACPI driver that offers firmware updates to USB-C chargers.
>>>>
>>>> Introduce a new platform driver under cros-ec, the ChromeOS embedded
>>>> controller, that handles these PD events and dispatches them
>>>> appropriately over a notifier chain to all drivers that use them.
>>>>
>>>> On non-ACPI platforms, the driver gets instantiated for ECs which
>>>> support the EC_FEATURE_USB_PD feature bit, and on such platforms, the
>>>> notification events will get delivered using the MKBP event handling
>>>> mechanism.
>>>>
>>>> Co-Developed-by: Prashant Malani <pmalani@chromium.org>
>>>> Reviewed-by: Gwendal Grignou <gwendal@chromium.org>
>>>> Signed-off-by: Jon Flatley <jflat@chromium.org>
>>>> Signed-off-by: Prashant Malani <pmalani@chromium.org>
>>>
>>> For my own reference:
>>>
>>> Acked-for-chrome-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>>>
>>
>> Sorry, too much rush acking this patch, here applies the same comment as patch 2
>>
>>>> ---
>>>>
>>>> Changes in v4(pmalani@chromium.org):
>>>> - No code changes, but added new version so that versioning is
>>>>   consistent with the next patch in the series.
>>>>
>>>> Changes in v3 (pmalani@chromium.org):
>>>> - Renamed driver and files from "cros_ec_pd_notify" to
>>>>   "cros_usbpd_notify" to be more consistent with other naming.
>>>> - Moved the change to include cros-usbpd-notify in the charger MFD into
>>>>   a separate follow-on patch.
>>>>
>>>> Changes in v2 (pmalani@chromium.org):
>>>> - Removed dependency on DT entry; instead, we will instantiate the
>>>>   driver on detecting EC_FEATURE_USB_PD for non-ACPI platforms.
>>>> - Modified the cros-ec-pd-notify device to be an mfd_cell under
>>>>   usbpdcharger for non-ACPI platforms. Altered the platform_probe() call
>>>>   to derive the cros EC structs appropriately.
>>>> - Replaced "usbpd_notify" with "pd_notify" in functions and structures.
>>>> - Addressed comments from upstream maintainer.
>>>>
>>>>  drivers/platform/chrome/Kconfig               |   9 ++
>>>>  drivers/platform/chrome/Makefile              |   1 +
>>>>  drivers/platform/chrome/cros_usbpd_notify.c   | 151 ++++++++++++++++++
>>>>  .../linux/platform_data/cros_usbpd_notify.h   |  17 ++
>>>>  4 files changed, 178 insertions(+)
>>>>  create mode 100644 drivers/platform/chrome/cros_usbpd_notify.c
>>>>  create mode 100644 include/linux/platform_data/cros_usbpd_notify.h
>>>>
>>>> diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
>>>> index 5f57282a28da0..3a8a98f2fb4d1 100644
>>>> --- a/drivers/platform/chrome/Kconfig
>>>> +++ b/drivers/platform/chrome/Kconfig
>>>> @@ -226,6 +226,15 @@ config CROS_USBPD_LOGGER
>>>>        To compile this driver as a module, choose M here: the
>>>>        module will be called cros_usbpd_logger.
>>>>
>>>> +config CROS_USBPD_NOTIFY
>>>> +    tristate "ChromeOS Type-C power delivery event notifier"
>>>> +    depends on CROS_EC
>>>> +    help
>>>> +      If you say Y here, you get support for Type-C PD event notifications
>>>> +      from the ChromeOS EC. On ACPI platorms this driver will bind to the
>>>> +      GOOG0003 ACPI device, and on non-ACPI platforms this driver will get
>>>> +      initialized on ECs which support the feature EC_FEATURE_USB_PD.
>>>> +
>>>>  source "drivers/platform/chrome/wilco_ec/Kconfig"
>>>>
>>>>  endif # CHROMEOS_PLATFORMS
>>>> diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
>>>> index aacd5920d8a18..f6465f8ef0b5e 100644
>>>> --- a/drivers/platform/chrome/Makefile
>>>> +++ b/drivers/platform/chrome/Makefile
>>>> @@ -22,5 +22,6 @@ obj-$(CONFIG_CROS_EC_DEBUGFS)              += cros_ec_debugfs.o
>>>>  obj-$(CONFIG_CROS_EC_SENSORHUB)             += cros_ec_sensorhub.o
>>>>  obj-$(CONFIG_CROS_EC_SYSFS)         += cros_ec_sysfs.o
>>>>  obj-$(CONFIG_CROS_USBPD_LOGGER)             += cros_usbpd_logger.o
>>>> +obj-$(CONFIG_CROS_USBPD_NOTIFY)             += cros_usbpd_notify.o
>>>>
>>>>  obj-$(CONFIG_WILCO_EC)                      += wilco_ec/
>>>> diff --git a/drivers/platform/chrome/cros_usbpd_notify.c b/drivers/platform/chrome/cros_usbpd_notify.c
>>>> new file mode 100644
>>>> index 0000000000000..05a7db834d2e0
>>>> --- /dev/null
>>>> +++ b/drivers/platform/chrome/cros_usbpd_notify.c
>>>> @@ -0,0 +1,151 @@
>>>> +// SPDX-License-Identifier: GPL-2.0-only
>>>> +/*
>>>> + * Copyright 2019 Google LLC
>>>> + *
>>>> + * This driver serves as the receiver of cros_ec PD host events.
>>>> + */
>>>> +
>>>> +#include <linux/acpi.h>
>>>> +#include <linux/module.h>
>>>> +#include <linux/mfd/cros_ec.h>
>>>> +#include <linux/platform_data/cros_ec_commands.h>
>>>> +#include <linux/platform_data/cros_usbpd_notify.h>
>>>> +#include <linux/platform_data/cros_ec_proto.h>
>>>> +#include <linux/platform_device.h>
>>>> +
>>>> +#define DRV_NAME "cros-usbpd-notify"
>>>> +#define ACPI_DRV_NAME "GOOG0003"
>>>> +
>>>> +static BLOCKING_NOTIFIER_HEAD(cros_usbpd_notifier_list);
>>>> +
>>>> +/**
>>>> + * cros_usbpd_register_notify - Register a notifier callback for PD events.
>>>> + * @nb: Notifier block pointer to register
>>>> + *
>>>> + * On ACPI platforms this corresponds to host events on the ECPD
>>>> + * "GOOG0003" ACPI device. On non-ACPI platforms this will filter mkbp events
>>>> + * for USB PD events.
>>>> + *
>>>> + * Return: 0 on success or negative error code.
>>>> + */
>>>> +int cros_usbpd_register_notify(struct notifier_block *nb)
>>>> +{
>>>> +    return blocking_notifier_chain_register(
>>>> +                    &cros_usbpd_notifier_list, nb);
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(cros_usbpd_register_notify);
>>>> +
>>>> +
>>>> +/**
>>>> + * cros_usbpd_unregister_notify - Unregister notifier callback for PD events.
>>>> + * @nb: Notifier block pointer to unregister
>>>> + *
>>>> + * Unregister a notifier callback that was previously registered with
>>>> + * cros_usbpd_register_notify().
>>>> + */
>>>> +void cros_usbpd_unregister_notify(struct notifier_block *nb)
>>>> +{
>>>> +    blocking_notifier_chain_unregister(&cros_usbpd_notifier_list, nb);
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(cros_usbpd_unregister_notify);
>>>> +
>>>> +#ifdef CONFIG_ACPI
>>>> +
>>
>> For arm64 will follow this path, and this is not what we want, right?
> 
> (Following on from your latest email in Patch 2/2 in this series): I
> agree it would (follow this path) if arm64 defconfig enables ACPI in
> the mainline (side note: Chrome OS's kevin build leaves this config
> disabled).
> To ensure I make the right update to the patch, is the suggestion for
> this patch that we use #ifndef CONFIG_OF instead of #ifdef CONFIG_ACPI
> ?
> I think that may also be problematic since !CONFIG_OF doesn't
> necessarily imply CONFIG_ACPI.
> 
> Perhaps we should just make them two separate ifdef blocks, i.e #ifdef
> CONFIG_OF, and then #ifdef CONFIG_ACPI? Would be great to hear your
> recommendation here.
> 

If understand correctly what you want is on some devices (usually the ones that
use ACPI to instantiate the devices) use the notify ACPI callback, otherwise, on
devices using OF to instantiate the devices, create a notifier. The driver
should work on both cases. The problem is that CONFIG_OF and CONFIG_ACPI are not
 exclusive and select one does not imply not select the other. So yes I'm fine
on have both ifdef blocks, but in the cros_ec_dev (patch 2/2) you should
register the notifier only if IS_ENABLED(CONFIG_OF)

Cheers,
 Enric

> Thanks again!
> 

Re: [PATCH v4 1/2] platform: chrome: Add cros-usbpd-notify driver

[Prashant Malani]

Hi Enric,

> > Perhaps we should just make them two separate ifdef blocks, i.e #ifdef
> > CONFIG_OF, and then #ifdef CONFIG_ACPI? Would be great to hear your
> > recommendation here.
> >
>
> If understand correctly what you want is on some devices (usually the ones that
> use ACPI to instantiate the devices) use the notify ACPI callback, otherwise, on
> devices using OF to instantiate the devices, create a notifier. The driver
> should work on both cases. The problem is that CONFIG_OF and CONFIG_ACPI are not
>  exclusive and select one does not imply not select the other. So yes I'm fine
> on have both ifdef blocks, but in the cros_ec_dev (patch 2/2) you should
> register the notifier only if IS_ENABLED(CONFIG_OF)
>
Thanks. Will upload the updated patches soon.

Best regards,
> Cheers,
>  Enric
>
> > Thanks again!
> >