Messages in this thread |  | | Subject | Re: [PATCH v4 2/2] mfd: cros_ec: Add cros-usbpd-notify subdevice | From | Enric Balletbo i Serra <> | Date | Tue, 7 Jan 2020 17:51:09 +0100 |
| |
Hi Prashant,
On 3/1/20 0:52, Prashant Malani wrote: > Hi Enric, > > > On Mon, Dec 23, 2019 at 12:29 PM Prashant Malani <pmalani@chromium.org> wrote: >> >> Hi Enric, >> >> On Sun, Dec 22, 2019 at 11:25 PM Enric Balletbo i Serra >> <enric.balletbo@collabora.com> wrote: >>> >>> Hi Prashant, >>> >>> On 20/12/19 20:38, Prashant Malani wrote: >>>> Add the cros-usbpd-notify driver as a subdevice on non-ACPI platforms >>>> that support the EC_FEATURE_USB_PD EC feature flag. >>>> >>>> This driver allows other cros-ec devices to receive PD event >>>> notifications from the Chrome OS Embedded Controller (EC) via a >>>> notification chain. >>>> >>>> Signed-off-by: Prashant Malani <pmalani@chromium.org> >>>> --- >>>> >>>> Changes in v4: >>>> - Removed #ifndef usage; instead, moved cros-usbpd-notify to a separate >>>> mfd_cell and used an IS_ENABLED() check. >>>> - Changed commit title and description slightly to reflect change in >>>> code. >>>> >>>> drivers/mfd/cros_ec_dev.c | 21 +++++++++++++++++++++ >>>> 1 file changed, 21 insertions(+) >>>> >>>> diff --git a/drivers/mfd/cros_ec_dev.c b/drivers/mfd/cros_ec_dev.c >>>> index c4b977a5dd966..da198abe2b0a6 100644 >>>> --- a/drivers/mfd/cros_ec_dev.c >>>> +++ b/drivers/mfd/cros_ec_dev.c >>>> @@ -5,6 +5,7 @@ >>>> * Copyright (C) 2014 Google, Inc. >>>> */ >>>> >>>> +#include <linux/kconfig.h> >>>> #include <linux/mfd/core.h> >>>> #include <linux/mfd/cros_ec.h> >>>> #include <linux/module.h> >>>> @@ -87,6 +88,10 @@ static const struct mfd_cell cros_usbpd_charger_cells[] = { >>>> { .name = "cros-usbpd-logger", }, >>>> }; >>>> >>>> +static const struct mfd_cell cros_usbpd_notify_cells[] = { >>>> + { .name = "cros-usbpd-notify", }, >>>> +}; >>>> + >>>> static const struct cros_feature_to_cells cros_subdevices[] = { >>>> { >>>> .id = EC_FEATURE_CEC, >>>> @@ -202,6 +207,22 @@ static int ec_device_probe(struct platform_device *pdev) >>>> } >>>> } >>>> >>>> + /* >>>> + * The PD notifier driver cell is separate since it only needs to be >>>> + * explicitly added on non-ACPI platforms. >>> >>> >>> Sorry to not catch this before, but a worry arose. Is non-ACPI platforms or >>> non-X86 platforms or on OF platforms? >>> >>> ARM64 for example has the CONFIG_ACPI symbol set to yes, with the below >>> condition condition will not work on Kevin for example and IIUC this is not what >>> we want, I think we want IS_ENABLED(CONFIG_OF)? >> Thanks for noting this. I will check with a kevin, and with the >> internal build flags to verify whether there are ARM64 which have the >> GOOG0003 PD notification device. >> I'll update this thread with my findings. > AFAICT from the Chrome OS kernel build step .config output, kevin > doesn't have CONFIG_ACPI enabled (it is marked as "# CONFIG_ACPI is > not set"), and it doesn't look like there are Chrome OS ARM devices > that use ACPI (I believe it's only used on Chrome OS x86-based > devices). So perhaps it is not a concern? >
The problem is, although it is not used in your configs, it can be selected, and fwiw some defconfigs in mainline have it enabled, i.e the arm64 defconfig.
I think you're testing the patch on x86 but I suspect we want also the notifier on some arm64 platforms (like kevin) right? In such case I won't get the notifier because CONFIG_OF and CONFIG_ACPI is enabled on my defconfig.
My guess is that the logic should be if IS_ENABLED(CONFIG_OF) call cros_ec_check_features, otherwise ACPI will do the magic instead of (!IS_ENABLED(CONFIG_ACPI))
Best, Enric
>> >> Best, >> >>> >>> Thanks, >>> Enric >>> >>>> + */ >>>> + if (!IS_ENABLED(CONFIG_ACPI)) { >>>> + if (cros_ec_check_features(ec, EC_FEATURE_USB_PD)) { >>>> + retval = mfd_add_hotplug_devices(ec->dev, >>>> + cros_usbpd_notify_cells, >>>> + ARRAY_SIZE(cros_usbpd_notify_cells)); >>>> + if (retval) >>>> + dev_err(ec->dev, >>>> + "failed to add PD notify devices: %d\n", >>>> + retval); >>>> + } >>>> + } >>>> + >>>> /* >>>> * The following subdevices cannot be detected by sending the >>>> * EC_FEATURE_GET_CMD to the Embedded Controller device. >>>>
|  |