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