On 08/06/2017 07:36 AM, Hans de Goede wrote:
Hi,

On 06-08-17 16:22, Guenter Roeck wrote:
On 08/06/2017 05:35 AM, Hans de Goede wrote:
The fusb302 port-controller relies on an external device doing USB2
charger-type detection.

The Intel Whiskey Cove PMIC with which the fusb302 is combined on some
X86/ACPI platforms already has a charger-type detection driver which
uses extcon to communicate the detected charger-type.

This commit uses the tcpm_get_usb2_current_limit_extcon helper to enable
USB2 charger detection on these systems. Note that the "fcs,extcon-name"
property name is only for kernel internal use by X86/ACPI platform code
and as such is NOT documented in the devicetree bindings.

Signed-off-by: Hans de Goede <hdego...@redhat.com>
---
  drivers/staging/typec/fusb302/fusb302.c | 16 ++++++++++++++++
  1 file changed, 16 insertions(+)

diff --git a/drivers/staging/typec/fusb302/fusb302.c 
b/drivers/staging/typec/fusb302/fusb302.c
index be454b5ff6c7..1d8c9b66df2f 100644
--- a/drivers/staging/typec/fusb302/fusb302.c
+++ b/drivers/staging/typec/fusb302/fusb302.c
@@ -1201,6 +1201,8 @@ static void init_tcpc_dev(struct tcpc_dev 
*fusb302_tcpc_dev)
  {
      fusb302_tcpc_dev->init = tcpm_init;
      fusb302_tcpc_dev->get_vbus = tcpm_get_vbus;
+    fusb302_tcpc_dev->get_usb2_current_limit =
+        tcpm_get_usb2_current_limit_extcon;
      fusb302_tcpc_dev->set_cc = tcpm_set_cc;
      fusb302_tcpc_dev->get_cc = tcpm_get_cc;
      fusb302_tcpc_dev->set_polarity = tcpm_set_polarity;
@@ -1685,6 +1687,7 @@ static int fusb302_probe(struct i2c_client *client,
      struct fusb302_chip *chip;
      struct i2c_adapter *adapter;
      struct device *dev = &client->dev;
+    const char *name;
      int ret = 0;
      u32 val;
@@ -1717,6 +1720,19 @@ static int fusb302_probe(struct i2c_client *client,
      if (device_property_read_u32(dev, "fcs,operating-snk-mw", &val) == 0)
          chip->tcpc_config.operating_snk_mw = val;
+    /*
+     * Devicetree platforms should get extcon via phandle (not yet
+     * supported). On ACPI platforms, we get the name from a device prop.
+     * This device prop is for kernel internal use only and is expected
+     * to be set by the platform code which also registers the i2c client
+     * for the fusb302.
+     */
+    if (device_property_read_string(dev, "fcs,extcon-name", &name) == 0) {

Those new devicetree properties need to be documented and approved by dt 
maintainers.

As indicated by the comment, this property should not be used in the devicetree
case, notice this is a device-property and not an of property and since it is
not intended to be used with devicetree at all (in devicetree a phandle rather
then a name would be used), it has no place under Documentation/devicetree at 
all.


Ok. I thought device properties were supposed to unify dt and non-dt situations,
but apparently not. Oh well :-(.

Also there is no current binding documentation for the "fcs,fusb302" compatible
and the weird "fcs,int_n" gpio which really is an irq which it already uses.


Yes, one of those TODO items for moving the code out of staging. "fcs," should
possibly be "fusb302,", and int_n _is_ weird.



+        chip->tcpc_dev.usb2_extcon = extcon_get_extcon_dev(name);
+        if (!chip->tcpc_dev.usb2_extcon)

extcon_get_extcon_dev() can also return an ERR_PTR.

As before, I am not really happy typing this into tcpm via tcpc_dev.
Until we have a better solution, I would prefer for that code to stay with the 
fusb302
code.

Ok, I tried to make this all re-usable since I think other port-controller 
drivers
will need something similar, but I can kill the entire tcpm-helpers.c file in v2
and then put everything in fusb302.c. I take it that that is what you want ?

Yes, please.

Also, please copy Yueyao Zhu for the fusb302 changes in v2.

Thanks,
Guenter

Reply via email to