linux-kernel - Re: [PATCH 5/5] extcon: intel-cht-wc: Add support for monitoring external USB Type-C controller

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170424142550.GB14268@kuha.fi.intel.com>
Date:   Mon, 24 Apr 2017 17:25:50 +0300
From:   Heikki Krogerus <heikki.krogerus@...ux.intel.com>
To:     Hans de Goede <hdegoede@...hat.com>
Cc:     MyungJoo Ham <myungjoo.ham@...sung.com>,
        Chanwoo Choi <cw00.choi@...sung.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 5/5] extcon: intel-cht-wc: Add support for monitoring
 external USB Type-C controller

Hi Hans,

On Fri, Apr 21, 2017 at 03:01:19PM +0200, Hans de Goede wrote:
> On some boards the Whiskey Cove PMIC is combined with an external USB
> Type-C controller, in this case extcon consumers should use the Type-C
> extcon state, except when the USB Type-C controller detects a current
> limit of 500 mA which may indicate USB-C to USB-A cable at which point
> the extcon consumer should use the Whiskey Cove's BC-1.2 detection's
> state.

Doesn't this mean you have several extcon_dev registered for a
single port? I'm not completely sure how extcon framework is meant to
be used, but shouldn't a single extcon_dev represent all the types of
connectors a port is meant to support?

> Since the Type-C controller info is incomplete and needs to be
> supplemented with the BC1.2 detection result in some cases, this
> commit makes the intel-cht-wc extcon driver monitor the extcon device
> registered by the Type-C controller and report that as our extcon state
> except when BC-1.2 detection should be used. This allows extcon
> consumers on these boards to keep monitoring only the intel-cht-wc extcon
> and then get complete extcon info from that, which combines the Type-C
> and BC-1.2 info.

I don't really understand why does this need to be done in such a
complex manner? Both components should just report the result and be
done with it, and there is no need for any coupling between the two.
If there is a consumer for the power, the driver for it can decide on
its own the limits and maximum power from the two sources.

> Signed-off-by: Hans de Goede <hdegoede@...hat.com>
> ---
>  drivers/extcon/extcon-intel-cht-wc.c | 96 ++++++++++++++++++++++++++++++------

I did not realize we had this driver. It really should register a psy.
We need to be able to advertise the results of the detection to the
consumers in order to support for example boards that have a discrete
charger ic or battery attached to the SoC instead of PMIC, and without
being forced to use board/platform specific quirks like this in the
driver.

This driver should just report the result of the detection forward and
let the psy framework take care of notifying the other components,
consumers and whatnot, if there are any.

Thanks,

-- 
heikki