| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 16 Jan 2020 14:09:04 -0800
From: Prashant Malani <pmalani@...omium.org>
To: Benson Leung <bleung@...gle.com>
Cc: Enric Balletbo i Serra <enric.balletbo@...labora.com>,
Guenter Roeck <groeck@...omium.org>,
Benson Leung <bleung@...omium.org>,
Lee Jones <lee.jones@...aro.org>, sre@...nel.org,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
linux-pm@...r.kernel.org, Jon Flatley <jflat@...omium.org>
Subject: Re: [PATCH v6 3/3] power: supply: cros-ec-usbpd-charger: Fix host events
On Thu, Jan 16, 2020 at 11:46 AM Benson Leung <bleung@...gle.com> wrote:
>
> On Tue, Jan 14, 2020 at 03:22:22PM -0800, Prashant Malani wrote:
> > From: Jon Flatley <jflat@...omium.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.
> >
> > Signed-off-by: Jon Flatley <jflat@...omium.org>
> > Signed-off-by: Prashant Malani <pmalani@...omium.org>
>
> Only a minor nit. Otherwise,
> Reviewed-by: Benson Leung <bleung@...omium.org>
>
>
> > ---
> >
> > Changes in v6(pmalani@...omium.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..7f7e051262170 100644
> > --- a/drivers/power/supply/cros_usbpd-charger.c
> > +++ b/drivers/power/supply/cros_usbpd-charger.c
> > @@ -8,6 +8,7 @@
> > #include <linux/mfd/cros_ec.h>
> > #include <linux/module.h>
> > #include <linux/platform_data/cros_ec_commands.h>
> > +#include <linux/platform_data/cros_usbpd_notify.h>
>
> Really minor nit. Alphabetize this #include. This insertion should
> be one line below.
Done. Thanks.
>
> > #include <linux/platform_data/cros_ec_proto.h>
> > #include <linux/platform_device.h>
> > #include <linux/power_supply.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
> >
>
> --
> Benson Leung
> Staff Software Engineer
> Chrome OS Kernel
> Google Inc.
> bleung@...gle.com
> Chromium OS Project
> bleung@...omium.org
Powered by blists - more mailing lists