On 10 February 2016 at 17:46, Lee Jones <[email protected]> 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 <[email protected]>
>> ---
>>
>>  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

Reply via email to