Re: [PATCH 2/5] extcon: Add FUSB302 USB TYPE-C controller support
From: Hans de Goede
Date: Tue May 16 2017 - 18:25:16 EST
Hi,
On 05/16/2017 02:07 PM, Heikki Krogerus wrote:
Hi Hans,
On Fri, May 12, 2017 at 11:22:46PM +0200, Hans de Goede wrote:
<snip>
To me making the combination of the 2 sources the problem
of the consumer seems just wrong, as you mentioned
above there should really be only one extcon for the
connector, likewise their should be only one definitive
source on what the input current limit is.
Why? The consumer driver, bq24290 in this case, does not need to
understand it has multiple sources. It will just react to events from
*a* source, and the psy framework takes care of everything else. The
psy framework will need some tuning, but that should not be a problem.
I'm preparing support for _PCL (power consumer list) ACPI method
handling for the psy framework, so I have some patches already. The
idea is that in the future USB ports could have virtual power source
object with _PCL.
This is in any case separate issue compared to the problem of telling
the BC1.2 detection to start in case of TYPEC_CC_RP_DEF.
TL;DR: We need some way to combine the current limit info
from TYPE-C and USB-2 BC detection into a single source and
to propagate that current to drivers which can actually set
the current-limit.
Note I'm happy to use something else then extcon for this,
but we do need some way to combine + propagate the
current-limit.
That must be handled using psy framework. Otherwise we'll just be
trying to re-invent the wheel. There is no problem for a consumer to
have multiple sources.
But we don't really have multiple sources here, we have only
one source, the USB-C cable hooked to an external power-supply.
Just because 2 different pieces of hardware may be involved in
determining the current limit does not mean that we should
model this as 2 different sources. Just as you rightfully
pointed out that me using 2 extcon devices for the single
Type-C connector is wrong, modelling it as 2 sources is
wrong too.
<snip>
Back to this topic. I can see at least the following problems:
1. Tell the BC1.2 detection to start from fusb302 driver
2. Deliver the power limits to the discrete charger ic or battery driver
3. Tell what ever regulates VBUS to start driving VBUS.
You are trying to solve everything using extcon, and that is wrong.
As indicated I'm not stuck on using extcon, I started using it
in my paches because it is used to set the current limit in some
other places already, but I'm fine with using something else.
The second problem definitely needs to be handled using psy framework.
The psy framework provides already everything needed for that.
So above you're talking about sources to the bq24190, which if
I understand you correctly suggest that you want the tcpm code to
register a power-supply device per Type-C port ? I'm not sure that
is a good idea, any registered power-supply devices will show up
under /sys/class/power_supply, currently on a typical system
there will be 2 entries under /sys/class/power_supply one for
the "Mains" or USB power input and one for the battery, adding
more entries there is likely to confuse userspace.
More in general having 2 power-supply devices for what is
in essence one power-input feels wrong.
We can still use the power-supply framework, but I would
envision this working like this:
The platform code which enumerates the type-c controller
sets a "input-current-power-supply-name" string device-property
on the tcpm's (parent)device. When this is set the tpcm code
will do a power_supply_get_by_name and set the input
current on the returned power_supply by setting the
POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT property.
For 3. (vbus) we could do something similar using a
"vbus-regulator-id" device-property and the regulator
framework, making the driver which controls Vbus register
itself as a regulator.
I can take a shot at implementing both suggestions, but
first lets try to get some general idea of how we want
to solve this.
Regards,
Hans