| Message ID | 20200117002820.56872-3-pmalani@chromium.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | [v7,1/3] platform: chrome: Add cros-usbpd-notify driver | expand |
Hi, On Thu, Jan 16, 2020 at 04:28:24PM -0800, Prashant Malani wrote: > 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> > --- Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com> I currently have one cros_usbpd-charger patch queued in -next. This patch looks like it should not create conflicts, but it's probably better to merge an immutable branch. -- Sebastian > > 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 >
Hi Sebastian, On 17/1/20 2:12, Sebastian Reichel wrote: > Hi, > > On Thu, Jan 16, 2020 at 04:28:24PM -0800, Prashant Malani wrote: >> 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> >> --- > > Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com> > > I currently have one cros_usbpd-charger patch queued in -next. > This patch looks like it should not create conflicts, but it's > probably better to merge an immutable branch. > I still have some concerns on the platform/chrome side and I have some problems with this driver testing it on Samsung Chromebook Plus (kevin). So it is not ready yet to merge, I'll create an im when ready. Thanks, Enric > -- Sebastian > >> >> 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 >>
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;