Messages in this thread | | | From | Tomeu Vizoso <> | Date | Fri, 12 Feb 2016 12:06:22 +0100 | Subject | Re: [PATCH v1 5/6] platform/chrome: Register USB PD charger device |
| |
On 10 February 2016 at 17:46, Lee Jones <lee.jones@linaro.org> wrote: > On Fri, 05 Feb 2016, Tomeu Vizoso wrote: > >> Check if a EC considers EC_CMD_USB_PD_PORTS a valid command and register >> a USB PD charger device if so. This check is needed for older versions >> of the ChromeOS EC firmware that don't support the EC_CMD_GET_FEATURES >> command. >> >> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com> >> --- >> >> drivers/mfd/cros_ec.c | 15 +++++++++++++++ >> drivers/platform/chrome/cros_ec_dev.c | 34 ++++++++++++++++++++++++++++++++++ >> include/linux/mfd/cros_ec.h | 2 ++ >> 3 files changed, 51 insertions(+) >> >> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c >> index fbe78b819fdd..20ca9794d2f3 100644 >> --- a/drivers/mfd/cros_ec.c >> +++ b/drivers/mfd/cros_ec.c >> @@ -202,5 +202,20 @@ EXPORT_SYMBOL(cros_ec_resume); >> >> #endif >> >> +static const struct mfd_cell cros_usb_pd_charger_devs[] = { >> + { >> + .name = "cros-usb-pd-charger", >> + .id = -1, >> + }, >> +}; >> + >> +int cros_ec_usb_pd_charger_register(struct device *dev) >> +{ >> + return mfd_add_devices(dev, 0, cros_usb_pd_charger_devs, >> + ARRAY_SIZE(cros_usb_pd_charger_devs), >> + NULL, 0, NULL); >> +} >> +EXPORT_SYMBOL(cros_ec_usb_pd_charger_register); > > I'm not keen on this idea at all. > > You're calling cros_ec_usb_pd_charger_register() from a device you > registered from this same source file. Seems very incestuous and > hacky. > > It's bad enough that we're calling into the platform driver from here > already, but seeing as we are, just extend the call to > cros_ec_query_all() to suit your purposes.
But cros_ec_query_all() is querying the EC that is directly accessible from the AP, while the EC that takes care of PD is the one registered from ec_pd_cell (it sits behind the other EC).
Maybe it's just that cros_ec_usb_pd_charger_register() belongs to cros_ec_dev.c?
Thanks,
Tomeu
| |