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