Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1332674

Re: [PATCH v1 5/6] platform/chrome: Register USB PD charger device

From Tomeu Vizoso <tomeu.vizoso@collabora.com>
Newsgroups linux.kernel
Subject Re: [PATCH v1 5/6] platform/chrome: Register USB PD charger device
Date 2016-02-12 12:10 +0100
Message-ID <r1jLP-6aQ-13@gated-at.bofh.it> (permalink)
References <qYOMa-P6-5@gated-at.bofh.it> <qYOMc-P6-49@gated-at.bofh.it> <r0G7L-4Gb-9@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


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

Back to linux.kernel | Previous | NextPrevious in thread | Find similar | Unroll thread


Thread

Re: [PATCH v1 5/6] platform/chrome: Register USB PD charger device Lee Jones <lee.jones@linaro.org> - 2016-02-10 17:50 +0100
  Re: [PATCH v1 5/6] platform/chrome: Register USB PD charger device Tomeu Vizoso <tomeu.vizoso@collabora.com> - 2016-02-12 12:10 +0100

csiph-web