All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v8 1/4] platform: chrome: Add cros-usbpd-notify driver
@ 2020-01-24 23:18 Prashant Malani
  2020-01-24 23:18 ` [PATCH v8 2/4] mfd: cros_ec: Add cros-usbpd-notify subdevice Prashant Malani
                   ` (3 more replies)
  0 siblings, 4 replies; 17+ messages in thread
From: Prashant Malani @ 2020-01-24 23:18 UTC (permalink / raw)
  To: enric.balletbo, groeck, bleung, lee.jones, sre
  Cc: linux-kernel, linux-pm, Jon Flatley, Prashant Malani, Gwendal Grignou

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 platforms that don't have the ACPI device defined, the driver gets
instantiated for ECs which support the EC_FEATURE_USB_PD feature bit,
and 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>
Reviewed-by: Benson Leung <bleung@chromium.org>
Signed-off-by: Jon Flatley <jflat@chromium.org>
Signed-off-by: Prashant Malani <pmalani@chromium.org>
---

Changes in v8(pmalani@chromium.org):
- Fix style nits.
- Remove unrequired header.
- Remove #ifdef CONFIG_OF dependency for platform driver registration.
- Add module compile text to Kconfig help section.

Changes in v7(pmalani@chromium.org):
- Removed use of module_platform_driver() and module_acpi_driver() since
  that was causing redefinition compilation errors on arm64 defconfig.
  Instead, explicitly defined the init and exit routines and
  register/unregister the platform and ACPI drivers there.
- Alphabetize #include header.

Changes in v6(pmalani@chromium.org):
- Fix build error from typo in cros_usbpd_notify_acpi_device_ids
  variable name.

Changes in v5(pmalani@chromium.org):
- Split the driver into platform and ACPI variants, each enclosed by
  CONFIG_OF and CONFIG_ACPI #ifdefs respectively.
- Updated the copyright year to 2020.
- Reworded the commit message and Kconfig description to incorporate
  the modified driver structure.

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               |  13 ++
 drivers/platform/chrome/Makefile              |   1 +
 drivers/platform/chrome/cros_usbpd_notify.c   | 170 ++++++++++++++++++
 .../linux/platform_data/cros_usbpd_notify.h   |  17 ++
 4 files changed, 201 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..e45e0fe057586 100644
--- a/drivers/platform/chrome/Kconfig
+++ b/drivers/platform/chrome/Kconfig
@@ -226,6 +226,19 @@ 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 platforms which don't have this device it
+	  will get initialized on ECs which support the feature
+	  EC_FEATURE_USB_PD.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called cros_usbpd_notify.
+
 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..6ead5c62b3c5f
--- /dev/null
+++ b/drivers/platform/chrome/cros_usbpd_notify.c
@@ -0,0 +1,170 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2020 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_proto.h>
+#include <linux/platform_data/cros_usbpd_notify.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_notify_acpi_device_ids);
+
+static struct acpi_driver cros_usbpd_notify_acpi_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,
+	},
+};
+
+#endif /* 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_plat_driver = {
+	.driver = {
+		.name = DRV_NAME,
+	},
+	.probe = cros_usbpd_notify_probe_plat,
+	.remove = cros_usbpd_notify_remove_plat,
+};
+
+static int __init cros_usbpd_notify_init(void)
+{
+	int ret;
+
+	ret = platform_driver_register(&cros_usbpd_notify_plat_driver);
+	if (ret < 0)
+		return ret;
+
+#ifdef CONFIG_ACPI
+	acpi_bus_register_driver(&cros_usbpd_notify_acpi_driver);
+#endif
+	return 0;
+}
+
+static void __exit cros_usbpd_notify_exit(void)
+{
+#ifdef CONFIG_ACPI
+	acpi_bus_unregister_driver(&cros_usbpd_notify_acpi_driver);
+#endif
+	platform_driver_unregister(&cros_usbpd_notify_plat_driver);
+}
+
+module_init(cros_usbpd_notify_init);
+module_exit(cros_usbpd_notify_exit);
+
+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..4f2791722b6d3
--- /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 2020 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.25.0.341.g760bfbb309-goog


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v8 2/4] mfd: cros_ec: Add cros-usbpd-notify subdevice
  2020-01-24 23:18 [PATCH v8 1/4] platform: chrome: Add cros-usbpd-notify driver Prashant Malani
@ 2020-01-24 23:18 ` Prashant Malani
  2020-01-24 23:18 ` [PATCH v8 3/4] mfd: cros_ec: Check DT node for usbpd-notify add Prashant Malani
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 17+ messages in thread
From: Prashant Malani @ 2020-01-24 23:18 UTC (permalink / raw)
  To: enric.balletbo, groeck, bleung, lee.jones, sre
  Cc: linux-kernel, linux-pm, Prashant Malani

Add the cros-usbpd-notify driver as a subdevice on platforms that
support the EC_FEATURE_USB_PD EC feature flag and don't have the
ACPI PD notification device defined.

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

Reviewed-by: Benson Leung <bleung@chromium.org>
Signed-off-by: Prashant Malani <pmalani@chromium.org>
---

Changes in v8:
- No changes. Patch v6 was applied, but maintaining it in this series,
  since Patch 3/4 adds a change to the if condition.

Changes in v7:
- No changes.

Changes in v6:
- No changes.

Changes in v5:
- Updated the IS_ENABLED() check to check for CONFIG_OF instead of
  !CONFIG_ACPI according to upstream comments.

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 | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
index c4b977a5dd966..d0c28a4c10ad0 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,23 @@ 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 platforms that don't have the PD notifier ACPI
+	 * device entry defined.
+	 */
+	if (IS_ENABLED(CONFIG_OF)) {
+		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.25.0.341.g760bfbb309-goog


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v8 3/4] mfd: cros_ec: Check DT node for usbpd-notify add
  2020-01-24 23:18 [PATCH v8 1/4] platform: chrome: Add cros-usbpd-notify driver Prashant Malani
  2020-01-24 23:18 ` [PATCH v8 2/4] mfd: cros_ec: Add cros-usbpd-notify subdevice Prashant Malani
@ 2020-01-24 23:18 ` Prashant Malani
  2020-01-27 14:50   ` Enric Balletbo i Serra
  2020-02-24 10:27   ` Lee Jones
  2020-01-24 23:18 ` [PATCH v8 4/4] power: supply: cros-ec-usbpd-charger: Fix host events Prashant Malani
  2020-01-27 14:58 ` [PATCH v8 1/4] platform: chrome: Add cros-usbpd-notify driver Enric Balletbo i Serra
  3 siblings, 2 replies; 17+ messages in thread
From: Prashant Malani @ 2020-01-24 23:18 UTC (permalink / raw)
  To: enric.balletbo, groeck, bleung, lee.jones, sre
  Cc: linux-kernel, linux-pm, Prashant Malani

Add a check to ensure there is indeed an EC device tree entry before
adding the cros-usbpd-notify device. This covers configs where both
CONFIG_ACPI and CONFIG_OF are defined, but the EC device is defined
using device tree and not in ACPI.

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

Changes in v8:
- Patch first introduced in v8 of the series.

 drivers/mfd/cros_ec_dev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
index d0c28a4c10ad0..411e80fc9a066 100644
--- a/drivers/mfd/cros_ec_dev.c
+++ b/drivers/mfd/cros_ec_dev.c
@@ -212,7 +212,7 @@ static int ec_device_probe(struct platform_device *pdev)
 	 * explicitly added on platforms that don't have the PD notifier ACPI
 	 * device entry defined.
 	 */
-	if (IS_ENABLED(CONFIG_OF)) {
+	if (IS_ENABLED(CONFIG_OF) && ec->ec_dev->dev->of_node) {
 		if (cros_ec_check_features(ec, EC_FEATURE_USB_PD)) {
 			retval = mfd_add_hotplug_devices(ec->dev,
 					cros_usbpd_notify_cells,
-- 
2.25.0.341.g760bfbb309-goog


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v8 4/4] power: supply: cros-ec-usbpd-charger: Fix host events
  2020-01-24 23:18 [PATCH v8 1/4] platform: chrome: Add cros-usbpd-notify driver Prashant Malani
  2020-01-24 23:18 ` [PATCH v8 2/4] mfd: cros_ec: Add cros-usbpd-notify subdevice Prashant Malani
  2020-01-24 23:18 ` [PATCH v8 3/4] mfd: cros_ec: Check DT node for usbpd-notify add Prashant Malani
@ 2020-01-24 23:18 ` Prashant Malani
  2020-01-27 14:58 ` [PATCH v8 1/4] platform: chrome: Add cros-usbpd-notify driver Enric Balletbo i Serra
  3 siblings, 0 replies; 17+ messages in thread
From: Prashant Malani @ 2020-01-24 23:18 UTC (permalink / raw)
  To: enric.balletbo, groeck, bleung, lee.jones, sre
  Cc: linux-kernel, linux-pm, Jon Flatley, Prashant Malani

From: Jon Flatley <jflat@chromium.org>

There's a bug on ACPI platforms where host events from the ECPD ACPI
device never make their way to the cros-ec-usbpd-charger driver. This
makes it so the only time the charger driver updates its state is when
user space accesses its sysfs attributes.

Now that these events have been unified into a single notifier chain on
both ACPI and non-ACPI platforms, update the charger driver to use this
new notifier.

Reviewed-by: Benson Leung <bleung@chromium.org>
Co-Developed-by: Prashant Malani <pmalani@chromium.org>
Signed-off-by: Jon Flatley <jflat@chromium.org>
Signed-off-by: Prashant Malani <pmalani@chromium.org>
---

Changes in v8(pmalani@chromium.org):
- No changes.

Changes in v7(pmalani@chromium.org):
- Alphabetize #include header.

Changes in v6(pmalani@chromium.org):
- Patch first introduced into the series in v6.

 drivers/power/supply/Kconfig              |  2 +-
 drivers/power/supply/cros_usbpd-charger.c | 50 ++++++++---------------
 2 files changed, 19 insertions(+), 33 deletions(-)

diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index 27164a1d3c7c4..ba74ddd793c3d 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -659,7 +659,7 @@ config CHARGER_RT9455
 
 config CHARGER_CROS_USBPD
 	tristate "ChromeOS EC based USBPD charger"
-	depends on CROS_EC
+	depends on CROS_USBPD_NOTIFY
 	default n
 	help
 	  Say Y here to enable ChromeOS EC based USBPD charger
diff --git a/drivers/power/supply/cros_usbpd-charger.c b/drivers/power/supply/cros_usbpd-charger.c
index 6cc7c3910e098..7eea080048f43 100644
--- a/drivers/power/supply/cros_usbpd-charger.c
+++ b/drivers/power/supply/cros_usbpd-charger.c
@@ -9,6 +9,7 @@
 #include <linux/module.h>
 #include <linux/platform_data/cros_ec_commands.h>
 #include <linux/platform_data/cros_ec_proto.h>
+#include <linux/platform_data/cros_usbpd_notify.h>
 #include <linux/platform_device.h>
 #include <linux/power_supply.h>
 #include <linux/slab.h>
@@ -524,32 +525,21 @@ static int cros_usbpd_charger_property_is_writeable(struct power_supply *psy,
 }
 
 static int cros_usbpd_charger_ec_event(struct notifier_block *nb,
-				       unsigned long queued_during_suspend,
+				       unsigned long host_event,
 				       void *_notify)
 {
-	struct cros_ec_device *ec_device;
-	struct charger_data *charger;
-	u32 host_event;
+	struct charger_data *charger = container_of(nb, struct charger_data,
+						    notifier);
 
-	charger = container_of(nb, struct charger_data, notifier);
-	ec_device = charger->ec_device;
-
-	host_event = cros_ec_get_host_event(ec_device);
-	if (host_event & EC_HOST_EVENT_MASK(EC_HOST_EVENT_PD_MCU)) {
-		cros_usbpd_charger_power_changed(charger->ports[0]->psy);
-		return NOTIFY_OK;
-	} else {
-		return NOTIFY_DONE;
-	}
+	cros_usbpd_charger_power_changed(charger->ports[0]->psy);
+	return NOTIFY_OK;
 }
 
 static void cros_usbpd_charger_unregister_notifier(void *data)
 {
 	struct charger_data *charger = data;
-	struct cros_ec_device *ec_device = charger->ec_device;
 
-	blocking_notifier_chain_unregister(&ec_device->event_notifier,
-					   &charger->notifier);
+	cros_usbpd_unregister_notify(&charger->notifier);
 }
 
 static int cros_usbpd_charger_probe(struct platform_device *pd)
@@ -683,21 +673,17 @@ static int cros_usbpd_charger_probe(struct platform_device *pd)
 		goto fail;
 	}
 
-	if (ec_device->mkbp_event_supported) {
-		/* Get PD events from the EC */
-		charger->notifier.notifier_call = cros_usbpd_charger_ec_event;
-		ret = blocking_notifier_chain_register(
-						&ec_device->event_notifier,
-						&charger->notifier);
-		if (ret < 0) {
-			dev_warn(dev, "failed to register notifier\n");
-		} else {
-			ret = devm_add_action_or_reset(dev,
-					cros_usbpd_charger_unregister_notifier,
-					charger);
-			if (ret < 0)
-				goto fail;
-		}
+	/* Get PD events from the EC */
+	charger->notifier.notifier_call = cros_usbpd_charger_ec_event;
+	ret = cros_usbpd_register_notify(&charger->notifier);
+	if (ret < 0) {
+		dev_warn(dev, "failed to register notifier\n");
+	} else {
+		ret = devm_add_action_or_reset(dev,
+				cros_usbpd_charger_unregister_notifier,
+				charger);
+		if (ret < 0)
+			goto fail;
 	}
 
 	return 0;
-- 
2.25.0.341.g760bfbb309-goog


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v8 3/4] mfd: cros_ec: Check DT node for usbpd-notify add
  2020-01-24 23:18 ` [PATCH v8 3/4] mfd: cros_ec: Check DT node for usbpd-notify add Prashant Malani
@ 2020-01-27 14:50   ` Enric Balletbo i Serra
  2020-02-10 10:11     ` Enric Balletbo i Serra
  2020-02-24 10:27   ` Lee Jones
  1 sibling, 1 reply; 17+ messages in thread
From: Enric Balletbo i Serra @ 2020-01-27 14:50 UTC (permalink / raw)
  To: Prashant Malani, groeck, bleung, lee.jones, sre; +Cc: linux-kernel, linux-pm

Hi Prashant,

On 25/1/20 0:18, Prashant Malani wrote:
> Add a check to ensure there is indeed an EC device tree entry before
> adding the cros-usbpd-notify device. This covers configs where both
> CONFIG_ACPI and CONFIG_OF are defined, but the EC device is defined
> using device tree and not in ACPI.
> 
> Signed-off-by: Prashant Malani <pmalani@chromium.org>

With this change, an playing with different CONFIG_ACPI + CONFIG_OF combinations
I don't see anymore the problem where the driver is registered twice on
CONFIG_ACPI side. So,

Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

Maybe it requires a fixes tag if Lee already picked the other patch?

Fixes: 4602dce0361e ("mfd: cros_ec: Add cros-usbpd-notify subdevice")

> ---
> 
> Changes in v8:
> - Patch first introduced in v8 of the series.
> 
>  drivers/mfd/cros_ec_dev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
> index d0c28a4c10ad0..411e80fc9a066 100644
> --- a/drivers/mfd/cros_ec_dev.c
> +++ b/drivers/mfd/cros_ec_dev.c
> @@ -212,7 +212,7 @@ static int ec_device_probe(struct platform_device *pdev)
>  	 * explicitly added on platforms that don't have the PD notifier ACPI
>  	 * device entry defined.
>  	 */
> -	if (IS_ENABLED(CONFIG_OF)) {
> +	if (IS_ENABLED(CONFIG_OF) && ec->ec_dev->dev->of_node) {
>  		if (cros_ec_check_features(ec, EC_FEATURE_USB_PD)) {
>  			retval = mfd_add_hotplug_devices(ec->dev,
>  					cros_usbpd_notify_cells,
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v8 1/4] platform: chrome: Add cros-usbpd-notify driver
  2020-01-24 23:18 [PATCH v8 1/4] platform: chrome: Add cros-usbpd-notify driver Prashant Malani
                   ` (2 preceding siblings ...)
  2020-01-24 23:18 ` [PATCH v8 4/4] power: supply: cros-ec-usbpd-charger: Fix host events Prashant Malani
@ 2020-01-27 14:58 ` Enric Balletbo i Serra
  2020-01-27 17:19   ` Prashant Malani
  2020-01-27 18:44   ` Benson Leung
  3 siblings, 2 replies; 17+ messages in thread
From: Enric Balletbo i Serra @ 2020-01-27 14:58 UTC (permalink / raw)
  To: Prashant Malani, groeck, bleung, lee.jones, sre
  Cc: linux-kernel, linux-pm, Jon Flatley, Gwendal Grignou

Hi Prashant,

On 25/1/20 0:18, 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 platforms that don't have the ACPI device defined, the driver gets
> instantiated for ECs which support the EC_FEATURE_USB_PD feature bit,
> and 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>
> Reviewed-by: Benson Leung <bleung@chromium.org>
> Signed-off-by: Jon Flatley <jflat@chromium.org>
> Signed-off-by: Prashant Malani <pmalani@chromium.org>
> ---
> 

Just a nit below but for my own reference:

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

I am not sure if we're on time to get this merged on this merge window, though.

Thanks,
 Enric

> Changes in v8(pmalani@chromium.org):
> - Fix style nits.
> - Remove unrequired header.
> - Remove #ifdef CONFIG_OF dependency for platform driver registration.
> - Add module compile text to Kconfig help section.
> 
> Changes in v7(pmalani@chromium.org):
> - Removed use of module_platform_driver() and module_acpi_driver() since
>   that was causing redefinition compilation errors on arm64 defconfig.
>   Instead, explicitly defined the init and exit routines and
>   register/unregister the platform and ACPI drivers there.
> - Alphabetize #include header.
> 
> Changes in v6(pmalani@chromium.org):
> - Fix build error from typo in cros_usbpd_notify_acpi_device_ids
>   variable name.
> 
> Changes in v5(pmalani@chromium.org):
> - Split the driver into platform and ACPI variants, each enclosed by
>   CONFIG_OF and CONFIG_ACPI #ifdefs respectively.
> - Updated the copyright year to 2020.
> - Reworded the commit message and Kconfig description to incorporate
>   the modified driver structure.
> 
> 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               |  13 ++
>  drivers/platform/chrome/Makefile              |   1 +
>  drivers/platform/chrome/cros_usbpd_notify.c   | 170 ++++++++++++++++++
>  .../linux/platform_data/cros_usbpd_notify.h   |  17 ++
>  4 files changed, 201 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..e45e0fe057586 100644
> --- a/drivers/platform/chrome/Kconfig
> +++ b/drivers/platform/chrome/Kconfig
> @@ -226,6 +226,19 @@ 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

Like other cros-ec subdevices I am wondering if we should depend on
MFD_CROS_EC_DEV instead of CROS_EC (as doesn't really makes sense to only
depends on CROS_EC)

Also, like other cros-ec subdevices, we might be interested on have a:

       default MFD_CROS_EC_DEV

So it is selected when the cros-ec dev is configured by default.

Thanks,

 Enric

> +	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 platforms which don't have this device it
> +	  will get initialized on ECs which support the feature
> +	  EC_FEATURE_USB_PD.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called cros_usbpd_notify.
> +
>  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..6ead5c62b3c5f
> --- /dev/null
> +++ b/drivers/platform/chrome/cros_usbpd_notify.c
> @@ -0,0 +1,170 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright 2020 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_proto.h>
> +#include <linux/platform_data/cros_usbpd_notify.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_notify_acpi_device_ids);
> +
> +static struct acpi_driver cros_usbpd_notify_acpi_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,
> +	},
> +};
> +
> +#endif /* 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_plat_driver = {
> +	.driver = {
> +		.name = DRV_NAME,
> +	},
> +	.probe = cros_usbpd_notify_probe_plat,
> +	.remove = cros_usbpd_notify_remove_plat,
> +};
> +
> +static int __init cros_usbpd_notify_init(void)
> +{
> +	int ret;
> +
> +	ret = platform_driver_register(&cros_usbpd_notify_plat_driver);
> +	if (ret < 0)
> +		return ret;
> +
> +#ifdef CONFIG_ACPI
> +	acpi_bus_register_driver(&cros_usbpd_notify_acpi_driver);
> +#endif
> +	return 0;
> +}
> +
> +static void __exit cros_usbpd_notify_exit(void)
> +{
> +#ifdef CONFIG_ACPI
> +	acpi_bus_unregister_driver(&cros_usbpd_notify_acpi_driver);
> +#endif
> +	platform_driver_unregister(&cros_usbpd_notify_plat_driver);
> +}
> +
> +module_init(cros_usbpd_notify_init);
> +module_exit(cros_usbpd_notify_exit);
> +
> +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..4f2791722b6d3
> --- /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 2020 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 */
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v8 1/4] platform: chrome: Add cros-usbpd-notify driver
  2020-01-27 14:58 ` [PATCH v8 1/4] platform: chrome: Add cros-usbpd-notify driver Enric Balletbo i Serra
@ 2020-01-27 17:19   ` Prashant Malani
  2020-01-27 18:44   ` Benson Leung
  1 sibling, 0 replies; 17+ messages in thread
From: Prashant Malani @ 2020-01-27 17:19 UTC (permalink / raw)
  To: Enric Balletbo i Serra
  Cc: Guenter Roeck, Benson Leung, Lee Jones, sre,
	Linux Kernel Mailing List, linux-pm, Jon Flatley,
	Gwendal Grignou

Hi Enric,

On Mon, Jan 27, 2020 at 6:58 AM Enric Balletbo i Serra
<enric.balletbo@collabora.com> wrote:
>
> Hi Prashant,
>
> On 25/1/20 0:18, 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 platforms that don't have the ACPI device defined, the driver gets
> > instantiated for ECs which support the EC_FEATURE_USB_PD feature bit,
> > and 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>
> > Reviewed-by: Benson Leung <bleung@chromium.org>
> > Signed-off-by: Jon Flatley <jflat@chromium.org>
> > Signed-off-by: Prashant Malani <pmalani@chromium.org>
> > ---
> >
>
> Just a nit below but for my own reference:
>
> Acked-for-chrome-platform: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>
> I am not sure if we're on time to get this merged on this merge window, though.
>
> Thanks,
>  Enric
>
> > Changes in v8(pmalani@chromium.org):
> > - Fix style nits.
> > - Remove unrequired header.
> > - Remove #ifdef CONFIG_OF dependency for platform driver registration.
> > - Add module compile text to Kconfig help section.
> >
> > Changes in v7(pmalani@chromium.org):
> > - Removed use of module_platform_driver() and module_acpi_driver() since
> >   that was causing redefinition compilation errors on arm64 defconfig.
> >   Instead, explicitly defined the init and exit routines and
> >   register/unregister the platform and ACPI drivers there.
> > - Alphabetize #include header.
> >
> > Changes in v6(pmalani@chromium.org):
> > - Fix build error from typo in cros_usbpd_notify_acpi_device_ids
> >   variable name.
> >
> > Changes in v5(pmalani@chromium.org):
> > - Split the driver into platform and ACPI variants, each enclosed by
> >   CONFIG_OF and CONFIG_ACPI #ifdefs respectively.
> > - Updated the copyright year to 2020.
> > - Reworded the commit message and Kconfig description to incorporate
> >   the modified driver structure.
> >
> > 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               |  13 ++
> >  drivers/platform/chrome/Makefile              |   1 +
> >  drivers/platform/chrome/cros_usbpd_notify.c   | 170 ++++++++++++++++++
> >  .../linux/platform_data/cros_usbpd_notify.h   |  17 ++
> >  4 files changed, 201 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..e45e0fe057586 100644
> > --- a/drivers/platform/chrome/Kconfig
> > +++ b/drivers/platform/chrome/Kconfig
> > @@ -226,6 +226,19 @@ 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
>
> Like other cros-ec subdevices I am wondering if we should depend on
> MFD_CROS_EC_DEV instead of CROS_EC (as doesn't really makes sense to only
> depends on CROS_EC)
>
> Also, like other cros-ec subdevices, we might be interested on have a:
>
>        default MFD_CROS_EC_DEV
>
> So it is selected when the cros-ec dev is configured by default.
Sounds good. Shall I push a new version of the patchset, or can you
make the modification while applying the current patch version?

Thanks,
>
> Thanks,
>
>  Enric
>
> > +     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 platforms which don't have this device it
> > +       will get initialized on ECs which support the feature
> > +       EC_FEATURE_USB_PD.
> > +
> > +       To compile this driver as a module, choose M here: the
> > +       module will be called cros_usbpd_notify.
> > +
> >  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..6ead5c62b3c5f
> > --- /dev/null
> > +++ b/drivers/platform/chrome/cros_usbpd_notify.c
> > @@ -0,0 +1,170 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright 2020 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_proto.h>
> > +#include <linux/platform_data/cros_usbpd_notify.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_notify_acpi_device_ids);
> > +
> > +static struct acpi_driver cros_usbpd_notify_acpi_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,
> > +     },
> > +};
> > +
> > +#endif /* 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_plat_driver = {
> > +     .driver = {
> > +             .name = DRV_NAME,
> > +     },
> > +     .probe = cros_usbpd_notify_probe_plat,
> > +     .remove = cros_usbpd_notify_remove_plat,
> > +};
> > +
> > +static int __init cros_usbpd_notify_init(void)
> > +{
> > +     int ret;
> > +
> > +     ret = platform_driver_register(&cros_usbpd_notify_plat_driver);
> > +     if (ret < 0)
> > +             return ret;
> > +
> > +#ifdef CONFIG_ACPI
> > +     acpi_bus_register_driver(&cros_usbpd_notify_acpi_driver);
> > +#endif
> > +     return 0;
> > +}
> > +
> > +static void __exit cros_usbpd_notify_exit(void)
> > +{
> > +#ifdef CONFIG_ACPI
> > +     acpi_bus_unregister_driver(&cros_usbpd_notify_acpi_driver);
> > +#endif
> > +     platform_driver_unregister(&cros_usbpd_notify_plat_driver);
> > +}
> > +
> > +module_init(cros_usbpd_notify_init);
> > +module_exit(cros_usbpd_notify_exit);
> > +
> > +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..4f2791722b6d3
> > --- /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 2020 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 */
> >

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v8 1/4] platform: chrome: Add cros-usbpd-notify driver
  2020-01-27 14:58 ` [PATCH v8 1/4] platform: chrome: Add cros-usbpd-notify driver Enric Balletbo i Serra
  2020-01-27 17:19   ` Prashant Malani
@ 2020-01-27 18:44   ` Benson Leung
  2020-01-29  1:11     ` Prashant Malani
  1 sibling, 1 reply; 17+ messages in thread
From: Benson Leung @ 2020-01-27 18:44 UTC (permalink / raw)
  To: Enric Balletbo i Serra
  Cc: Prashant Malani, groeck, bleung, lee.jones, sre, linux-kernel,
	linux-pm, Jon Flatley, Gwendal Grignou

[-- Attachment #1: Type: text/plain, Size: 12889 bytes --]

On Mon, Jan 27, 2020 at 03:58:42PM +0100, Enric Balletbo i Serra wrote:
> Hi Prashant,
> 
> On 25/1/20 0:18, 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 platforms that don't have the ACPI device defined, the driver gets
> > instantiated for ECs which support the EC_FEATURE_USB_PD feature bit,
> > and 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>
> > Reviewed-by: Benson Leung <bleung@chromium.org>
> > Signed-off-by: Jon Flatley <jflat@chromium.org>
> > Signed-off-by: Prashant Malani <pmalani@chromium.org>
> > ---
> > 
> 
> Just a nit below but for my own reference:
> 
> Acked-for-chrome-platform: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> 
> I am not sure if we're on time to get this merged on this merge window, though.

I'm OK with creating a branch for this series and merging it into
chrome-platform-5.7 once Linus releases v5.6-rc1 late next week.

> 
> Thanks,
>  Enric
> 
> > Changes in v8(pmalani@chromium.org):
> > - Fix style nits.
> > - Remove unrequired header.
> > - Remove #ifdef CONFIG_OF dependency for platform driver registration.
> > - Add module compile text to Kconfig help section.
> > 
> > Changes in v7(pmalani@chromium.org):
> > - Removed use of module_platform_driver() and module_acpi_driver() since
> >   that was causing redefinition compilation errors on arm64 defconfig.
> >   Instead, explicitly defined the init and exit routines and
> >   register/unregister the platform and ACPI drivers there.
> > - Alphabetize #include header.
> > 
> > Changes in v6(pmalani@chromium.org):
> > - Fix build error from typo in cros_usbpd_notify_acpi_device_ids
> >   variable name.
> > 
> > Changes in v5(pmalani@chromium.org):
> > - Split the driver into platform and ACPI variants, each enclosed by
> >   CONFIG_OF and CONFIG_ACPI #ifdefs respectively.
> > - Updated the copyright year to 2020.
> > - Reworded the commit message and Kconfig description to incorporate
> >   the modified driver structure.
> > 
> > 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               |  13 ++
> >  drivers/platform/chrome/Makefile              |   1 +
> >  drivers/platform/chrome/cros_usbpd_notify.c   | 170 ++++++++++++++++++
> >  .../linux/platform_data/cros_usbpd_notify.h   |  17 ++
> >  4 files changed, 201 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..e45e0fe057586 100644
> > --- a/drivers/platform/chrome/Kconfig
> > +++ b/drivers/platform/chrome/Kconfig
> > @@ -226,6 +226,19 @@ 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
> 
> Like other cros-ec subdevices I am wondering if we should depend on
> MFD_CROS_EC_DEV instead of CROS_EC (as doesn't really makes sense to only
> depends on CROS_EC)
> 
> Also, like other cros-ec subdevices, we might be interested on have a:
> 
>        default MFD_CROS_EC_DEV
> 
> So it is selected when the cros-ec dev is configured by default.
> 
> Thanks,
> 
>  Enric
> 
> > +	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 platforms which don't have this device it
> > +	  will get initialized on ECs which support the feature
> > +	  EC_FEATURE_USB_PD.
> > +
> > +	  To compile this driver as a module, choose M here: the
> > +	  module will be called cros_usbpd_notify.
> > +
> >  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..6ead5c62b3c5f
> > --- /dev/null
> > +++ b/drivers/platform/chrome/cros_usbpd_notify.c
> > @@ -0,0 +1,170 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright 2020 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_proto.h>
> > +#include <linux/platform_data/cros_usbpd_notify.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_notify_acpi_device_ids);
> > +
> > +static struct acpi_driver cros_usbpd_notify_acpi_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,
> > +	},
> > +};
> > +
> > +#endif /* 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_plat_driver = {
> > +	.driver = {
> > +		.name = DRV_NAME,
> > +	},
> > +	.probe = cros_usbpd_notify_probe_plat,
> > +	.remove = cros_usbpd_notify_remove_plat,
> > +};
> > +
> > +static int __init cros_usbpd_notify_init(void)
> > +{
> > +	int ret;
> > +
> > +	ret = platform_driver_register(&cros_usbpd_notify_plat_driver);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +#ifdef CONFIG_ACPI
> > +	acpi_bus_register_driver(&cros_usbpd_notify_acpi_driver);
> > +#endif
> > +	return 0;
> > +}
> > +
> > +static void __exit cros_usbpd_notify_exit(void)
> > +{
> > +#ifdef CONFIG_ACPI
> > +	acpi_bus_unregister_driver(&cros_usbpd_notify_acpi_driver);
> > +#endif
> > +	platform_driver_unregister(&cros_usbpd_notify_plat_driver);
> > +}
> > +
> > +module_init(cros_usbpd_notify_init);
> > +module_exit(cros_usbpd_notify_exit);
> > +
> > +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..4f2791722b6d3
> > --- /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 2020 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 */
> > 

-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v8 1/4] platform: chrome: Add cros-usbpd-notify driver
  2020-01-27 18:44   ` Benson Leung
@ 2020-01-29  1:11     ` Prashant Malani
  2020-01-29  8:37       ` Enric Balletbo i Serra
  0 siblings, 1 reply; 17+ messages in thread
From: Prashant Malani @ 2020-01-29  1:11 UTC (permalink / raw)
  To: Benson Leung
  Cc: Enric Balletbo i Serra, Guenter Roeck, Benson Leung, Lee Jones,
	sre, Linux Kernel Mailing List, linux-pm, Jon Flatley,
	Gwendal Grignou

On Mon, Jan 27, 2020 at 10:44 AM Benson Leung <bleung@google.com> wrote:
>
> On Mon, Jan 27, 2020 at 03:58:42PM +0100, Enric Balletbo i Serra wrote:
> > Hi Prashant,
> >
> > On 25/1/20 0:18, 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 platforms that don't have the ACPI device defined, the driver gets
> > > instantiated for ECs which support the EC_FEATURE_USB_PD feature bit,
> > > and 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>
> > > Reviewed-by: Benson Leung <bleung@chromium.org>
> > > Signed-off-by: Jon Flatley <jflat@chromium.org>
> > > Signed-off-by: Prashant Malani <pmalani@chromium.org>
> > > ---
> > >
> >
> > Just a nit below but for my own reference:
> >
> > Acked-for-chrome-platform: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> >
> > I am not sure if we're on time to get this merged on this merge window, though.
>
> I'm OK with creating a branch for this series and merging it into
> chrome-platform-5.7 once Linus releases v5.6-rc1 late next week.
Thanks; I'm guessing one of the maintainers will perform the creation
of chrome-platform-5.7 and merge this patch into that branch.
Also, kindly pick https://lkml.org/lkml/2020/1/24/2068 , i.e patch 4/4
of this series (I think an earlier version of this patch, i.e
https://lkml.org/lkml/2020/1/17/628 was marked "Reviewed-by: Sebastian
Reichel <sebastian.reichel@collabora.com>"

Thanks!
>
> >
> > Thanks,
> >  Enric
> >
> > > Changes in v8(pmalani@chromium.org):
> > > - Fix style nits.
> > > - Remove unrequired header.
> > > - Remove #ifdef CONFIG_OF dependency for platform driver registration.
> > > - Add module compile text to Kconfig help section.
> > >
> > > Changes in v7(pmalani@chromium.org):
> > > - Removed use of module_platform_driver() and module_acpi_driver() since
> > >   that was causing redefinition compilation errors on arm64 defconfig.
> > >   Instead, explicitly defined the init and exit routines and
> > >   register/unregister the platform and ACPI drivers there.
> > > - Alphabetize #include header.
> > >
> > > Changes in v6(pmalani@chromium.org):
> > > - Fix build error from typo in cros_usbpd_notify_acpi_device_ids
> > >   variable name.
> > >
> > > Changes in v5(pmalani@chromium.org):
> > > - Split the driver into platform and ACPI variants, each enclosed by
> > >   CONFIG_OF and CONFIG_ACPI #ifdefs respectively.
> > > - Updated the copyright year to 2020.
> > > - Reworded the commit message and Kconfig description to incorporate
> > >   the modified driver structure.
> > >
> > > 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               |  13 ++
> > >  drivers/platform/chrome/Makefile              |   1 +
> > >  drivers/platform/chrome/cros_usbpd_notify.c   | 170 ++++++++++++++++++
> > >  .../linux/platform_data/cros_usbpd_notify.h   |  17 ++
> > >  4 files changed, 201 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..e45e0fe057586 100644
> > > --- a/drivers/platform/chrome/Kconfig
> > > +++ b/drivers/platform/chrome/Kconfig
> > > @@ -226,6 +226,19 @@ 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
> >
> > Like other cros-ec subdevices I am wondering if we should depend on
> > MFD_CROS_EC_DEV instead of CROS_EC (as doesn't really makes sense to only
> > depends on CROS_EC)
> >
> > Also, like other cros-ec subdevices, we might be interested on have a:
> >
> >        default MFD_CROS_EC_DEV
> >
> > So it is selected when the cros-ec dev is configured by default.
> >
> > Thanks,
> >
> >  Enric
> >
> > > +   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 platforms which don't have this device it
> > > +     will get initialized on ECs which support the feature
> > > +     EC_FEATURE_USB_PD.
> > > +
> > > +     To compile this driver as a module, choose M here: the
> > > +     module will be called cros_usbpd_notify.
> > > +
> > >  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..6ead5c62b3c5f
> > > --- /dev/null
> > > +++ b/drivers/platform/chrome/cros_usbpd_notify.c
> > > @@ -0,0 +1,170 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/*
> > > + * Copyright 2020 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_proto.h>
> > > +#include <linux/platform_data/cros_usbpd_notify.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_notify_acpi_device_ids);
> > > +
> > > +static struct acpi_driver cros_usbpd_notify_acpi_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,
> > > +   },
> > > +};
> > > +
> > > +#endif /* 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_plat_driver = {
> > > +   .driver = {
> > > +           .name = DRV_NAME,
> > > +   },
> > > +   .probe = cros_usbpd_notify_probe_plat,
> > > +   .remove = cros_usbpd_notify_remove_plat,
> > > +};
> > > +
> > > +static int __init cros_usbpd_notify_init(void)
> > > +{
> > > +   int ret;
> > > +
> > > +   ret = platform_driver_register(&cros_usbpd_notify_plat_driver);
> > > +   if (ret < 0)
> > > +           return ret;
> > > +
> > > +#ifdef CONFIG_ACPI
> > > +   acpi_bus_register_driver(&cros_usbpd_notify_acpi_driver);
> > > +#endif
> > > +   return 0;
> > > +}
> > > +
> > > +static void __exit cros_usbpd_notify_exit(void)
> > > +{
> > > +#ifdef CONFIG_ACPI
> > > +   acpi_bus_unregister_driver(&cros_usbpd_notify_acpi_driver);
> > > +#endif
> > > +   platform_driver_unregister(&cros_usbpd_notify_plat_driver);
> > > +}
> > > +
> > > +module_init(cros_usbpd_notify_init);
> > > +module_exit(cros_usbpd_notify_exit);
> > > +
> > > +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..4f2791722b6d3
> > > --- /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 2020 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 */
> > >
>
> --
> Benson Leung
> Staff Software Engineer
> Chrome OS Kernel
> Google Inc.
> bleung@google.com
> Chromium OS Project
> bleung@chromium.org

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v8 1/4] platform: chrome: Add cros-usbpd-notify driver
  2020-01-29  1:11     ` Prashant Malani
@ 2020-01-29  8:37       ` Enric Balletbo i Serra
  2020-02-03 23:42         ` Benson Leung
  0 siblings, 1 reply; 17+ messages in thread
From: Enric Balletbo i Serra @ 2020-01-29  8:37 UTC (permalink / raw)
  To: Prashant Malani, Benson Leung
  Cc: Guenter Roeck, Benson Leung, Lee Jones, sre,
	Linux Kernel Mailing List, linux-pm, Jon Flatley,
	Gwendal Grignou



On 29/1/20 2:11, Prashant Malani wrote:
> On Mon, Jan 27, 2020 at 10:44 AM Benson Leung <bleung@google.com> wrote:
>>
>> On Mon, Jan 27, 2020 at 03:58:42PM +0100, Enric Balletbo i Serra wrote:
>>> Hi Prashant,
>>>
>>> On 25/1/20 0:18, 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 platforms that don't have the ACPI device defined, the driver gets
>>>> instantiated for ECs which support the EC_FEATURE_USB_PD feature bit,
>>>> and 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>
>>>> Reviewed-by: Benson Leung <bleung@chromium.org>
>>>> Signed-off-by: Jon Flatley <jflat@chromium.org>
>>>> Signed-off-by: Prashant Malani <pmalani@chromium.org>
>>>> ---
>>>>
>>>
>>> Just a nit below but for my own reference:
>>>
>>> Acked-for-chrome-platform: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>>>
>>> I am not sure if we're on time to get this merged on this merge window, though.
>>
>> I'm OK with creating a branch for this series and merging it into
>> chrome-platform-5.7 once Linus releases v5.6-rc1 late next week.
> Thanks; I'm guessing one of the maintainers will perform the creation
> of chrome-platform-5.7 and merge this patch into that branch.
> Also, kindly pick https://lkml.org/lkml/2020/1/24/2068 , i.e patch 4/4
> of this series (I think an earlier version of this patch, i.e
> https://lkml.org/lkml/2020/1/17/628 was marked "Reviewed-by: Sebastian
> Reichel <sebastian.reichel@collabora.com>"
> 

That patch should go through Sebastian's tree, we will create an immutable
branch for him when rc1 is released.

Thanks,
 Enric

> Thanks!
>>
>>>
>>> Thanks,
>>>  Enric
>>>
>>>> Changes in v8(pmalani@chromium.org):
>>>> - Fix style nits.
>>>> - Remove unrequired header.
>>>> - Remove #ifdef CONFIG_OF dependency for platform driver registration.
>>>> - Add module compile text to Kconfig help section.
>>>>
>>>> Changes in v7(pmalani@chromium.org):
>>>> - Removed use of module_platform_driver() and module_acpi_driver() since
>>>>   that was causing redefinition compilation errors on arm64 defconfig.
>>>>   Instead, explicitly defined the init and exit routines and
>>>>   register/unregister the platform and ACPI drivers there.
>>>> - Alphabetize #include header.
>>>>
>>>> Changes in v6(pmalani@chromium.org):
>>>> - Fix build error from typo in cros_usbpd_notify_acpi_device_ids
>>>>   variable name.
>>>>
>>>> Changes in v5(pmalani@chromium.org):
>>>> - Split the driver into platform and ACPI variants, each enclosed by
>>>>   CONFIG_OF and CONFIG_ACPI #ifdefs respectively.
>>>> - Updated the copyright year to 2020.
>>>> - Reworded the commit message and Kconfig description to incorporate
>>>>   the modified driver structure.
>>>>
>>>> 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               |  13 ++
>>>>  drivers/platform/chrome/Makefile              |   1 +
>>>>  drivers/platform/chrome/cros_usbpd_notify.c   | 170 ++++++++++++++++++
>>>>  .../linux/platform_data/cros_usbpd_notify.h   |  17 ++
>>>>  4 files changed, 201 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..e45e0fe057586 100644
>>>> --- a/drivers/platform/chrome/Kconfig
>>>> +++ b/drivers/platform/chrome/Kconfig
>>>> @@ -226,6 +226,19 @@ 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
>>>
>>> Like other cros-ec subdevices I am wondering if we should depend on
>>> MFD_CROS_EC_DEV instead of CROS_EC (as doesn't really makes sense to only
>>> depends on CROS_EC)
>>>
>>> Also, like other cros-ec subdevices, we might be interested on have a:
>>>
>>>        default MFD_CROS_EC_DEV
>>>
>>> So it is selected when the cros-ec dev is configured by default.
>>>
>>> Thanks,
>>>
>>>  Enric
>>>
>>>> +   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 platforms which don't have this device it
>>>> +     will get initialized on ECs which support the feature
>>>> +     EC_FEATURE_USB_PD.
>>>> +
>>>> +     To compile this driver as a module, choose M here: the
>>>> +     module will be called cros_usbpd_notify.
>>>> +
>>>>  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..6ead5c62b3c5f
>>>> --- /dev/null
>>>> +++ b/drivers/platform/chrome/cros_usbpd_notify.c
>>>> @@ -0,0 +1,170 @@
>>>> +// SPDX-License-Identifier: GPL-2.0-only
>>>> +/*
>>>> + * Copyright 2020 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_proto.h>
>>>> +#include <linux/platform_data/cros_usbpd_notify.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_notify_acpi_device_ids);
>>>> +
>>>> +static struct acpi_driver cros_usbpd_notify_acpi_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,
>>>> +   },
>>>> +};
>>>> +
>>>> +#endif /* 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_plat_driver = {
>>>> +   .driver = {
>>>> +           .name = DRV_NAME,
>>>> +   },
>>>> +   .probe = cros_usbpd_notify_probe_plat,
>>>> +   .remove = cros_usbpd_notify_remove_plat,
>>>> +};
>>>> +
>>>> +static int __init cros_usbpd_notify_init(void)
>>>> +{
>>>> +   int ret;
>>>> +
>>>> +   ret = platform_driver_register(&cros_usbpd_notify_plat_driver);
>>>> +   if (ret < 0)
>>>> +           return ret;
>>>> +
>>>> +#ifdef CONFIG_ACPI
>>>> +   acpi_bus_register_driver(&cros_usbpd_notify_acpi_driver);
>>>> +#endif
>>>> +   return 0;
>>>> +}
>>>> +
>>>> +static void __exit cros_usbpd_notify_exit(void)
>>>> +{
>>>> +#ifdef CONFIG_ACPI
>>>> +   acpi_bus_unregister_driver(&cros_usbpd_notify_acpi_driver);
>>>> +#endif
>>>> +   platform_driver_unregister(&cros_usbpd_notify_plat_driver);
>>>> +}
>>>> +
>>>> +module_init(cros_usbpd_notify_init);
>>>> +module_exit(cros_usbpd_notify_exit);
>>>> +
>>>> +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..4f2791722b6d3
>>>> --- /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 2020 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 */
>>>>
>>
>> --
>> Benson Leung
>> Staff Software Engineer
>> Chrome OS Kernel
>> Google Inc.
>> bleung@google.com
>> Chromium OS Project
>> bleung@chromium.org

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v8 1/4] platform: chrome: Add cros-usbpd-notify driver
  2020-01-29  8:37       ` Enric Balletbo i Serra
@ 2020-02-03 23:42         ` Benson Leung
  2020-02-04  0:40           ` Prashant Malani
  0 siblings, 1 reply; 17+ messages in thread
From: Benson Leung @ 2020-02-03 23:42 UTC (permalink / raw)
  To: Enric Balletbo i Serra
  Cc: Prashant Malani, Guenter Roeck, Lee Jones, Sebastian Reichel,
	Linux Kernel Mailing List, linux-pm, Jon Flatley,
	Gwendal Grignou

Hi Enric, Hi Prashant,

On Wed, Jan 29, 2020 at 12:37 AM Enric Balletbo i Serra
<enric.balletbo@collabora.com> wrote:
> >> I'm OK with creating a branch for this series and merging it into
> >> chrome-platform-5.7 once Linus releases v5.6-rc1 late next week.
> > Thanks; I'm guessing one of the maintainers will perform the creation
> > of chrome-platform-5.7 and merge this patch into that branch.
> > Also, kindly pick https://lkml.org/lkml/2020/1/24/2068 , i.e patch 4/4
> > of this series (I think an earlier version of this patch, i.e
> > https://lkml.org/lkml/2020/1/17/628 was marked "Reviewed-by: Sebastian
> > Reichel <sebastian.reichel@collabora.com>"
> >
>
> That patch should go through Sebastian's tree, we will create an immutable
> branch for him when rc1 is released.
>

Before rc1 is released, I've gone ahead and created a staging branch
on chrome-platform collaboration repo here:
https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git/log/?h=staging-cros-usbpd-notify

Here are the two commits I have right now:
e8573ca91ae4 power: supply: cros-ec-usbpd-charger: Fix host events
a49c1263a22b platform: chrome: Add cros-usbpd-notify driver

Enric, I went ahead and modified the Kconfig on the first patch to
depend on MFD_CROS_EC_DEV and default it as well.
Prashant, let me know how these look to you. We can convert the branch
to an immutable next week.

-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v8 1/4] platform: chrome: Add cros-usbpd-notify driver
  2020-02-03 23:42         ` Benson Leung
@ 2020-02-04  0:40           ` Prashant Malani
  2020-02-10  1:24             ` Benson Leung
  0 siblings, 1 reply; 17+ messages in thread
From: Prashant Malani @ 2020-02-04  0:40 UTC (permalink / raw)
  To: Benson Leung
  Cc: Enric Balletbo i Serra, Guenter Roeck, Lee Jones,
	Sebastian Reichel, Linux Kernel Mailing List,
	open list:POWER SUPPLY CLASS/SUBSYSTEM and DRIVERS, Jon Flatley,
	Gwendal Grignou

On Mon, Feb 3, 2020 at 3:42 PM Benson Leung <bleung@google.com> wrote:
>
> Hi Enric, Hi Prashant,
>
> On Wed, Jan 29, 2020 at 12:37 AM Enric Balletbo i Serra
> <enric.balletbo@collabora.com> wrote:
> > >> I'm OK with creating a branch for this series and merging it into
> > >> chrome-platform-5.7 once Linus releases v5.6-rc1 late next week.
> > > Thanks; I'm guessing one of the maintainers will perform the creation
> > > of chrome-platform-5.7 and merge this patch into that branch.
> > > Also, kindly pick https://lkml.org/lkml/2020/1/24/2068 , i.e patch 4/4
> > > of this series (I think an earlier version of this patch, i.e
> > > https://lkml.org/lkml/2020/1/17/628 was marked "Reviewed-by: Sebastian
> > > Reichel <sebastian.reichel@collabora.com>"
> > >
> >
> > That patch should go through Sebastian's tree, we will create an immutable
> > branch for him when rc1 is released.
> >
>
> Before rc1 is released, I've gone ahead and created a staging branch
> on chrome-platform collaboration repo here:
> https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git/log/?h=staging-cros-usbpd-notify
>
> Here are the two commits I have right now:
> e8573ca91ae4 power: supply: cros-ec-usbpd-charger: Fix host events
> a49c1263a22b platform: chrome: Add cros-usbpd-notify driver
>
> Enric, I went ahead and modified the Kconfig on the first patch to
> depend on MFD_CROS_EC_DEV and default it as well.
> Prashant, let me know how these look to you. We can convert the branch
> to an immutable next week.
Thanks Benson, looks good to me.

>
> --
> Benson Leung
> Staff Software Engineer
> Chrome OS Kernel
> Google Inc.
> bleung@google.com
> Chromium OS Project
> bleung@chromium.org

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v8 1/4] platform: chrome: Add cros-usbpd-notify driver
  2020-02-04  0:40           ` Prashant Malani
@ 2020-02-10  1:24             ` Benson Leung
  0 siblings, 0 replies; 17+ messages in thread
From: Benson Leung @ 2020-02-10  1:24 UTC (permalink / raw)
  To: Prashant Malani
  Cc: Enric Balletbo i Serra, Guenter Roeck, Lee Jones,
	Sebastian Reichel, Linux Kernel Mailing List,
	open list:POWER SUPPLY CLASS/SUBSYSTEM and DRIVERS, Jon Flatley,
	Gwendal Grignou

Hey Prashant,

On Mon, Feb 3, 2020 at 4:40 PM Prashant Malani <pmalani@chromium.org> wrote:
>
> On Mon, Feb 3, 2020 at 3:42 PM Benson Leung <bleung@google.com> wrote:
> >
> > Hi Enric, Hi Prashant,
> >
> > On Wed, Jan 29, 2020 at 12:37 AM Enric Balletbo i Serra
> > <enric.balletbo@collabora.com> wrote:
> > > >> I'm OK with creating a branch for this series and merging it into
> > > >> chrome-platform-5.7 once Linus releases v5.6-rc1 late next week.
> > > > Thanks; I'm guessing one of the maintainers will perform the creation
> > > > of chrome-platform-5.7 and merge this patch into that branch.
> > > > Also, kindly pick https://lkml.org/lkml/2020/1/24/2068 , i.e patch 4/4
> > > > of this series (I think an earlier version of this patch, i.e
> > > > https://lkml.org/lkml/2020/1/17/628 was marked "Reviewed-by: Sebastian
> > > > Reichel <sebastian.reichel@collabora.com>"
> > > >
> > >
> > > That patch should go through Sebastian's tree, we will create an immutable
> > > branch for him when rc1 is released.
> > >
> >
> > Before rc1 is released, I've gone ahead and created a staging branch
> > on chrome-platform collaboration repo here:
> > https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git/log/?h=staging-cros-usbpd-notify
> >
> > Here are the two commits I have right now:
> > e8573ca91ae4 power: supply: cros-ec-usbpd-charger: Fix host events
> > a49c1263a22b platform: chrome: Add cros-usbpd-notify driver
> >
> > Enric, I went ahead and modified the Kconfig on the first patch to
> > depend on MFD_CROS_EC_DEV and default it as well.
> > Prashant, let me know how these look to you. We can convert the branch
> > to an immutable next week.
> Thanks Benson, looks good to me.

I've rebased the branch and marked it immutable.

https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git/log/?h=ib-chrome-platform-power-supply-cros-usbpd-notify

I can send a pull request for Sebastian's sake too.

>
> >
> > --
> > Benson Leung
> > Staff Software Engineer
> > Chrome OS Kernel
> > Google Inc.
> > bleung@google.com
> > Chromium OS Project
> > bleung@chromium.org



-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v8 3/4] mfd: cros_ec: Check DT node for usbpd-notify add
  2020-01-27 14:50   ` Enric Balletbo i Serra
@ 2020-02-10 10:11     ` Enric Balletbo i Serra
       [not found]       ` <CACeCKadtoAA0z88dYy3O-tQE=KLpR5Rx=ZXkkEKyhnAsKyqzjw@mail.gmail.com>
  0 siblings, 1 reply; 17+ messages in thread
From: Enric Balletbo i Serra @ 2020-02-10 10:11 UTC (permalink / raw)
  To: Prashant Malani, groeck, bleung, lee.jones, sre; +Cc: linux-kernel, linux-pm

Hi Prashant,

On 27/1/20 15:50, Enric Balletbo i Serra wrote:
> Hi Prashant,
> 
> On 25/1/20 0:18, Prashant Malani wrote:
>> Add a check to ensure there is indeed an EC device tree entry before
>> adding the cros-usbpd-notify device. This covers configs where both
>> CONFIG_ACPI and CONFIG_OF are defined, but the EC device is defined
>> using device tree and not in ACPI.
>>
>> Signed-off-by: Prashant Malani <pmalani@chromium.org>
> 
> With this change, an playing with different CONFIG_ACPI + CONFIG_OF combinations
> I don't see anymore the problem where the driver is registered twice on
> CONFIG_ACPI side. So,
> 
> Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> 
> Maybe it requires a fixes tag if Lee already picked the other patch?
> 
> Fixes: 4602dce0361e ("mfd: cros_ec: Add cros-usbpd-notify subdevice")
> 

Now that v7 from mfd side was merged and v8 from platform side was merged, could
you resend this specific patch alone collecting all the fixes and tested tags. I
guess will be more clear for mfd people.

Thanks,
 Enric

>> ---
>>
>> Changes in v8:
>> - Patch first introduced in v8 of the series.
>>
>>  drivers/mfd/cros_ec_dev.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
>> index d0c28a4c10ad0..411e80fc9a066 100644
>> --- a/drivers/mfd/cros_ec_dev.c
>> +++ b/drivers/mfd/cros_ec_dev.c
>> @@ -212,7 +212,7 @@ static int ec_device_probe(struct platform_device *pdev)
>>  	 * explicitly added on platforms that don't have the PD notifier ACPI
>>  	 * device entry defined.
>>  	 */
>> -	if (IS_ENABLED(CONFIG_OF)) {
>> +	if (IS_ENABLED(CONFIG_OF) && ec->ec_dev->dev->of_node) {
>>  		if (cros_ec_check_features(ec, EC_FEATURE_USB_PD)) {
>>  			retval = mfd_add_hotplug_devices(ec->dev,
>>  					cros_usbpd_notify_cells,
>>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v8 3/4] mfd: cros_ec: Check DT node for usbpd-notify add
       [not found]       ` <CACeCKadtoAA0z88dYy3O-tQE=KLpR5Rx=ZXkkEKyhnAsKyqzjw@mail.gmail.com>
@ 2020-02-10 16:38         ` Enric Balletbo i Serra
  2020-02-10 18:59           ` Prashant Malani
  0 siblings, 1 reply; 17+ messages in thread
From: Enric Balletbo i Serra @ 2020-02-10 16:38 UTC (permalink / raw)
  To: Prashant Malani
  Cc: Guenter Roeck, Benson Leung, lee.jones, sre, linux-kernel, linux-pm

Hi Prashant,

On 10/2/20 17:32, Prashant Malani wrote:
> Hi Enric,
> 
> On Mon, Feb 10, 2020, 02:11 Enric Balletbo i Serra <enric.balletbo@collabora.com
> <mailto:enric.balletbo@collabora.com>> wrote:
> 
>     Hi Prashant,
> 
>     On 27/1/20 15:50, Enric Balletbo i Serra wrote:
>     > Hi Prashant,
>     >
>     > On 25/1/20 0:18, Prashant Malani wrote:
>     >> Add a check to ensure there is indeed an EC device tree entry before
>     >> adding the cros-usbpd-notify device. This covers configs where both
>     >> CONFIG_ACPI and CONFIG_OF are defined, but the EC device is defined
>     >> using device tree and not in ACPI.
>     >>
>     >> Signed-off-by: Prashant Malani <pmalani@chromium.org
>     <mailto:pmalani@chromium.org>>
>     >
>     > With this change, an playing with different CONFIG_ACPI + CONFIG_OF
>     combinations
>     > I don't see anymore the problem where the driver is registered twice on
>     > CONFIG_ACPI side. So,
>     >
>     > Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com
>     <mailto:enric.balletbo@collabora.com>>
>     >
>     > Maybe it requires a fixes tag if Lee already picked the other patch?
>     >
>     > Fixes: 4602dce0361e ("mfd: cros_ec: Add cros-usbpd-notify subdevice")
>     >
> 
>     Now that v7 from mfd side was merged and v8 from platform side was merged, could
>     you resend this specific patch alone collecting all the fixes and tested tags. I
>     guess will be more clear for mfd people.
> 
> 
> Sounds good. Should I maintain the same versioning and series info i.e v9 3/4?
> Or just v9?
> 

I'd do "[PATCH RESEND] mfd: cros_ec: Check DT node for usbpd-notify add" and
then after the "---" explain that you are resending this alone because the other
patches are already applied, and reference this patch series.

> Thanks,
> 
>     Thanks,
>      Enric
> 
>     >> ---
>     >>
>     >> Changes in v8:
>     >> - Patch first introduced in v8 of the series.
>     >>
>     >>  drivers/mfd/cros_ec_dev.c | 2 +-
>     >>  1 file changed, 1 insertion(+), 1 deletion(-)
>     >>
>     >> diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
>     >> index d0c28a4c10ad0..411e80fc9a066 100644
>     >> --- a/drivers/mfd/cros_ec_dev.c
>     >> +++ b/drivers/mfd/cros_ec_dev.c
>     >> @@ -212,7 +212,7 @@ static int ec_device_probe(struct platform_device *pdev)
>     >>       * explicitly added on platforms that don't have the PD notifier ACPI
>     >>       * device entry defined.
>     >>       */
>     >> -    if (IS_ENABLED(CONFIG_OF)) {
>     >> +    if (IS_ENABLED(CONFIG_OF) && ec->ec_dev->dev->of_node) {
>     >>              if (cros_ec_check_features(ec, EC_FEATURE_USB_PD)) {
>     >>                      retval = mfd_add_hotplug_devices(ec->dev,
>     >>                                      cros_usbpd_notify_cells,
>     >>
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v8 3/4] mfd: cros_ec: Check DT node for usbpd-notify add
  2020-02-10 16:38         ` Enric Balletbo i Serra
@ 2020-02-10 18:59           ` Prashant Malani
  0 siblings, 0 replies; 17+ messages in thread
From: Prashant Malani @ 2020-02-10 18:59 UTC (permalink / raw)
  To: Enric Balletbo i Serra
  Cc: Guenter Roeck, Benson Leung, Lee Jones, Sebastian Reichel,
	Linux Kernel Mailing List,
	open list:POWER SUPPLY CLASS/SUBSYSTEM and DRIVERS

Hi Enric,

On Mon, Feb 10, 2020 at 8:38 AM Enric Balletbo i Serra
<enric.balletbo@collabora.com> wrote:
>
> Hi Prashant,
>
> On 10/2/20 17:32, Prashant Malani wrote:
> > Hi Enric,
> >
> > On Mon, Feb 10, 2020, 02:11 Enric Balletbo i Serra <enric.balletbo@collabora.com
> > <mailto:enric.balletbo@collabora.com>> wrote:
> >
> >     Hi Prashant,
> >
> >     On 27/1/20 15:50, Enric Balletbo i Serra wrote:
> >     > Hi Prashant,
> >     >
> >     > On 25/1/20 0:18, Prashant Malani wrote:
> >     >> Add a check to ensure there is indeed an EC device tree entry before
> >     >> adding the cros-usbpd-notify device. This covers configs where both
> >     >> CONFIG_ACPI and CONFIG_OF are defined, but the EC device is defined
> >     >> using device tree and not in ACPI.
> >     >>
> >     >> Signed-off-by: Prashant Malani <pmalani@chromium.org
> >     <mailto:pmalani@chromium.org>>
> >     >
> >     > With this change, an playing with different CONFIG_ACPI + CONFIG_OF
> >     combinations
> >     > I don't see anymore the problem where the driver is registered twice on
> >     > CONFIG_ACPI side. So,
> >     >
> >     > Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com
> >     <mailto:enric.balletbo@collabora.com>>
> >     >
> >     > Maybe it requires a fixes tag if Lee already picked the other patch?
> >     >
> >     > Fixes: 4602dce0361e ("mfd: cros_ec: Add cros-usbpd-notify subdevice")
> >     >
> >
> >     Now that v7 from mfd side was merged and v8 from platform side was merged, could
> >     you resend this specific patch alone collecting all the fixes and tested tags. I
> >     guess will be more clear for mfd people.
> >
> >
> > Sounds good. Should I maintain the same versioning and series info i.e v9 3/4?
> > Or just v9?
> >
>
> I'd do "[PATCH RESEND] mfd: cros_ec: Check DT node for usbpd-notify add" and
> then after the "---" explain that you are resending this alone because the other
> patches are already applied, and reference this patch series.

Got it. Will re-send across the patch now.

Thanks!

>
> > Thanks,
> >
> >     Thanks,
> >      Enric
> >
> >     >> ---
> >     >>
> >     >> Changes in v8:
> >     >> - Patch first introduced in v8 of the series.
> >     >>
> >     >>  drivers/mfd/cros_ec_dev.c | 2 +-
> >     >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >     >>
> >     >> diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
> >     >> index d0c28a4c10ad0..411e80fc9a066 100644
> >     >> --- a/drivers/mfd/cros_ec_dev.c
> >     >> +++ b/drivers/mfd/cros_ec_dev.c
> >     >> @@ -212,7 +212,7 @@ static int ec_device_probe(struct platform_device *pdev)
> >     >>       * explicitly added on platforms that don't have the PD notifier ACPI
> >     >>       * device entry defined.
> >     >>       */
> >     >> -    if (IS_ENABLED(CONFIG_OF)) {
> >     >> +    if (IS_ENABLED(CONFIG_OF) && ec->ec_dev->dev->of_node) {
> >     >>              if (cros_ec_check_features(ec, EC_FEATURE_USB_PD)) {
> >     >>                      retval = mfd_add_hotplug_devices(ec->dev,
> >     >>                                      cros_usbpd_notify_cells,
> >     >>
> >

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v8 3/4] mfd: cros_ec: Check DT node for usbpd-notify add
  2020-01-24 23:18 ` [PATCH v8 3/4] mfd: cros_ec: Check DT node for usbpd-notify add Prashant Malani
  2020-01-27 14:50   ` Enric Balletbo i Serra
@ 2020-02-24 10:27   ` Lee Jones
  1 sibling, 0 replies; 17+ messages in thread
From: Lee Jones @ 2020-02-24 10:27 UTC (permalink / raw)
  To: Prashant Malani
  Cc: enric.balletbo, groeck, bleung, sre, linux-kernel, linux-pm

On Fri, 24 Jan 2020, Prashant Malani wrote:

> Add a check to ensure there is indeed an EC device tree entry before
> adding the cros-usbpd-notify device. This covers configs where both
> CONFIG_ACPI and CONFIG_OF are defined, but the EC device is defined
> using device tree and not in ACPI.

Don't this 'fix' a patch in *this* set?

If so, please squash and resend.

> Signed-off-by: Prashant Malani <pmalani@chromium.org>
> ---
> 
> Changes in v8:
> - Patch first introduced in v8 of the series.
> 
>  drivers/mfd/cros_ec_dev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c
> index d0c28a4c10ad0..411e80fc9a066 100644
> --- a/drivers/mfd/cros_ec_dev.c
> +++ b/drivers/mfd/cros_ec_dev.c
> @@ -212,7 +212,7 @@ static int ec_device_probe(struct platform_device *pdev)
>  	 * explicitly added on platforms that don't have the PD notifier ACPI
>  	 * device entry defined.
>  	 */
> -	if (IS_ENABLED(CONFIG_OF)) {
> +	if (IS_ENABLED(CONFIG_OF) && ec->ec_dev->dev->of_node) {
>  		if (cros_ec_check_features(ec, EC_FEATURE_USB_PD)) {
>  			retval = mfd_add_hotplug_devices(ec->dev,
>  					cros_usbpd_notify_cells,

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2020-02-24 10:27 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-24 23:18 [PATCH v8 1/4] platform: chrome: Add cros-usbpd-notify driver Prashant Malani
2020-01-24 23:18 ` [PATCH v8 2/4] mfd: cros_ec: Add cros-usbpd-notify subdevice Prashant Malani
2020-01-24 23:18 ` [PATCH v8 3/4] mfd: cros_ec: Check DT node for usbpd-notify add Prashant Malani
2020-01-27 14:50   ` Enric Balletbo i Serra
2020-02-10 10:11     ` Enric Balletbo i Serra
     [not found]       ` <CACeCKadtoAA0z88dYy3O-tQE=KLpR5Rx=ZXkkEKyhnAsKyqzjw@mail.gmail.com>
2020-02-10 16:38         ` Enric Balletbo i Serra
2020-02-10 18:59           ` Prashant Malani
2020-02-24 10:27   ` Lee Jones
2020-01-24 23:18 ` [PATCH v8 4/4] power: supply: cros-ec-usbpd-charger: Fix host events Prashant Malani
2020-01-27 14:58 ` [PATCH v8 1/4] platform: chrome: Add cros-usbpd-notify driver Enric Balletbo i Serra
2020-01-27 17:19   ` Prashant Malani
2020-01-27 18:44   ` Benson Leung
2020-01-29  1:11     ` Prashant Malani
2020-01-29  8:37       ` Enric Balletbo i Serra
2020-02-03 23:42         ` Benson Leung
2020-02-04  0:40           ` Prashant Malani
2020-02-10  1:24             ` Benson Leung

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.