From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751506AbdHFOgi (ORCPT ); Sun, 6 Aug 2017 10:36:38 -0400 Received: from mx1.redhat.com ([209.132.183.28]:49608 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751282AbdHFOgg (ORCPT ); Sun, 6 Aug 2017 10:36:36 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 357AD6146E Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=hdegoede@redhat.com Subject: Re: [PATCH 08/18] staging: typec: fusb302: Add support for USB2 charger detection through extcon To: Guenter Roeck , Darren Hart , Andy Shevchenko , Wolfram Sang , Sebastian Reichel , Greg Kroah-Hartman , Heikki Krogerus Cc: platform-driver-x86@vger.kernel.org, linux-kernel@vger.kernel.org, linux-i2c@vger.kernel.org, Liam Breck , Tony Lindgren , linux-pm@vger.kernel.org, devel@driverdev.osuosl.org References: <20170806123555.5124-1-hdegoede@redhat.com> <20170806123555.5124-9-hdegoede@redhat.com> <9de0a2f3-7316-5a7d-d715-29e83a278844@roeck-us.net> From: Hans de Goede Message-ID: <60616f13-207e-f998-7151-ad670c876166@redhat.com> Date: Sun, 6 Aug 2017 16:36:32 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <9de0a2f3-7316-5a7d-d715-29e83a278844@roeck-us.net> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Sun, 06 Aug 2017 14:36:36 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 >> --- >> 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. 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. > >> + 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 ? Regards, Hans