From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756890AbbDPHAw (ORCPT ); Thu, 16 Apr 2015 03:00:52 -0400 Received: from mailout4.samsung.com ([203.254.224.34]:19247 "EHLO mailout4.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753061AbbDPHAo (ORCPT ); Thu, 16 Apr 2015 03:00:44 -0400 X-AuditID: cbfee68e-f79c56d000006efb-4e-552f5e186a1c Message-id: <552F5E17.5020902@samsung.com> Date: Thu, 16 Apr 2015 16:00:39 +0900 From: Chanwoo Choi User-Agent: Mozilla/5.0 (X11; Linux i686; rv:17.0) Gecko/20130106 Thunderbird/17.0.2 MIME-version: 1.0 To: Peter Chen , "Balbi, Felipe" , "ABRAHAM, KISHON VIJAY" Cc: Roger Quadros , Robert Baldyga , myungjoo.ham@samsung.com, linux-usb@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, m.szyprowski@samsung.com Subject: Re: [PATCH v3 2/4] extcon: usb-gpio: add support for VBUS detection References: <55278DD3.3020409@samsung.com> <55279550.1060609@samsung.com> <552CE579.6060702@ti.com> <552CE5C0.3010207@ti.com> <552CEC97.1050205@samsung.com> <552CEE42.1050907@ti.com> <552CFA1E.5070400@samsung.com> <20150415032734.GA29095@shlinux2> <552E1862.6090709@ti.com> <552E2EBF.5090906@samsung.com> <20150416015922.GB22269@shlinux2> In-reply-to: <20150416015922.GB22269@shlinux2> Content-type: text/plain; charset=ISO-8859-1 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrDIsWRmVeSWpSXmKPExsWyRsSkWFciTj/U4O1OEYuD9+st5h85x2px 4WkPm8XlXXPYLBYta2W2WHvkLrvF7cYVbBbHZv9lsnhweCe7Rc8jLQcuj3+H+5k8+rasYvQ4 fmM7k8fnTXIBLFFcNimpOZllqUX6dglcGSu+zmQqWOFdseFXI1sD4w/bLkZODgkBE4lfk/qZ IWwxiQv31rOB2EICSxkl3k7PhKm503INKM4FFF/EKPFzWjMThPOAUWLJ6kvsIFW8AloSt39M YQKxWQRUJa486AKz2YDi+1/cAJsqKhAmsXL6FRaIekGJH5PvgdkiAiUSK5c/ANvALHCDUaLv AchQDg5hAR+JhTdYIZbdYZKYseQM2DJOAT2Jm80bwIYyC+hI7G+dBmXLS2xe85YZpEFC4Bq7 xOTu9awQFwlIfJt8iAVkqISArMSmA1AvS0ocXHGDZQKj2CwkN81CMnYWkrELGJlXMYqmFiQX FCelFxnpFSfmFpfmpesl5+duYgRG4ul/z/p2MN48YH2IUYCDUYmHd+EC3VAh1sSy4srcQ4ym QFdMZJYSTc4HxnteSbyhsZmRhamJqbGRuaWZkjhvgtTPYCGB9MSS1OzU1ILUovii0pzU4kOM TBycUg2MvupcDH+0vq/QLQj5YbLthMfd6PaMrcvuBl8+mL/Gq5Y3s5jldkPr/BUOycHrK+32 hk09P+kEd96MD+wrpBI9TjWlXwjd1j1Fc+L17W8nN08zi9oWYHnlWh778eXFIs6nNJTe9j7Y mVR04nSZcY2B+ura8GbP9vKcx8E/mX6tk7PWnCaWpH1RiaU4I9FQi7moOBEASf3Wnr8CAAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrOIsWRmVeSWpSXmKPExsVy+t9jAV2JOP1Qg46ZXBYH79dbzD9yjtXi wtMeNovLu+awWSxa1spssfbIXXaL240r2CyOzf7LZPHg8E52i55HWg5cHv8O9zN59G1Zxehx /MZ2Jo/Pm+QCWKIaGG0yUhNTUosUUvOS81My89JtlbyD453jTc0MDHUNLS3MlRTyEnNTbZVc fAJ03TJzgO5RUihLzCkFCgUkFhcr6dthmhAa4qZrAdMYoesbEgTXY2SABhLWMGas+DqTqWCF d8WGX41sDYw/bLsYOTkkBEwk7rRcY4OwxSQu3FsPZHNxCAksYpT4Oa2ZCcJ5wCixZPUldpAq XgEtids/pjCB2CwCqhJXHnSB2WxA8f0vboBNEhUIk1g5/QoLRL2gxI/J98BsEYESiZXLH4Bt YBa4wSjR9wBkKAeHsICPxMIbrBDL7jBJzFhyBmwZp4CexM3mDWBDmQV0JPa3ToOy5SU2r3nL PIFRYBaSHbOQlM1CUraAkXkVo2hqQXJBcVJ6rqFecWJucWleul5yfu4mRnCkP5PawbiyweIQ owAHoxIP74IFuqFCrIllxZW5hxglOJiVRHiXWuiHCvGmJFZWpRblxxeV5qQWH2I0BQbBRGYp 0eR8YBLKK4k3NDYxM7I0Mje0MDI2VxLnnaMrFyokkJ5YkpqdmlqQWgTTx8TBKdXAaGssoj7d cdnpQ4yrr4etZThrzdIut2TBxbBXMw5sLv/07caXq7tesy5I/blEombd9KBFfE9vt0u5Pl1y o0zn/634r+fvOoXrTXiqcfO5ppR1+ZRJuQ25shVHZ3RNbNRU3/P81ZQPr1RZfdnqjL3setee a9opcacmbAfn7dsGOzjSWf/WTFwgtFmJpTgj0VCLuag4EQAoK8zgCgMAAA== DLP-Filter: Pass X-MTR: 20000000000000000@CPGS X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Peter, On 04/16/2015 10:59 AM, Peter Chen wrote: > On Wed, Apr 15, 2015 at 06:26:23PM +0900, Chanwoo Choi wrote: >> Hi Roger and Peter, >> >> On 04/15/2015 04:50 PM, Roger Quadros wrote: >>> On 15/04/15 06:27, Peter Chen wrote: >>>> On Tue, Apr 14, 2015 at 08:29:34PM +0900, Chanwoo Choi wrote: >>>>> On 04/14/2015 07:38 PM, Roger Quadros wrote: >>>>>> On 14/04/15 13:31, Chanwoo Choi wrote: >>>>>>> On 04/14/2015 07:02 PM, Roger Quadros wrote: >>>>>>>> Fixed Kishon's id. >>>>>>>> >>>>>>>> On 14/04/15 13:01, Roger Quadros wrote: >>>>>>>>> On 10/04/15 12:18, Chanwoo Choi wrote: >>>>>>>>>> On 04/10/2015 05:46 PM, Robert Baldyga wrote: >>>>>>>>>>> On 04/10/2015 10:10 AM, Chanwoo Choi wrote: >>>>>>>>>>>> On 04/10/2015 04:45 PM, Robert Baldyga wrote: >>>>>>>>>>>>> On 04/10/2015 09:17 AM, Chanwoo Choi wrote: >>>>>>>>>>>>>> Hi Robert, >>>>>>>>>>>>>> >>>>>>>>>>>>>> On 04/09/2015 06:24 PM, Robert Baldyga wrote: >>>>>>>>>>>>>>> Hi Chanwoo, >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> On 04/09/2015 11:07 AM, Chanwoo Choi wrote: >>>>>>>>>>>>>>>> Hi Robert, >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> On 04/09/2015 04:57 PM, Robert Baldyga wrote: >>>>>>>>>>>>>>>>> Hi Chanwoo, >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> On 04/09/2015 04:12 AM, Chanwoo Choi wrote: >>>>>>>>>>>>>>>>>> Hi Robert, >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> [snip] >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> But, I have one question about case[3] >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>>> If id is low and vbus is high, this patch will update the state of both USB and USB-HOST cable as attached state. >>>>>>>>>>>>>>>>>> Is it possible that two different cables (both USB and USB-HOST) are connected to one port simultaneously? >>>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>>> It's because state of single USB cable connection cannot be completely >>>>>>>>>>>>>>>>> described using single extcon cable. USB cable state has two bits (VBUS >>>>>>>>>>>>>>>>> and ID), so we need to use two cables for single cable connection. We >>>>>>>>>>>>>>>>> use following convention: >>>>>>>>>>>>>>>>> cable "USB" = VBUS >>>>>>>>>>>>>>>>> cable "USB-HOST" = !ID. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> I think that extcon provider driver have to update the only one cable state >>>>>>>>>>>>>>>> of either USB or USB-HOST because USB and USB-HOST feature can not be used >>>>>>>>>>>>>>>> at the same time through one h/w port. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>>> If extcon-usb-gpio.c update two connected event of both USB and USB-HOST cable >>>>>>>>>>>>>>>> at the same time, the extcon consumer driver can not decide what handle either USB or USB-HOST. >>>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> It can. USB OTG allows for that. Moreover device can be host even if >>>>>>>>>>>>>>> ID=1 (so detected cable type is USB device), or peripheral when ID=0 (so >>>>>>>>>>>>>>> detected cable type is USB host). Devices would need to have complete >>>>>>>>>>>>>>> information about USB cable connection, because OTG state machine needs >>>>>>>>>>>>>> >>>>>>>>>>>>>> As I knew, USB OTG port don't send the attached cable of both USB and USB-HOST >>>>>>>>>>>>>> at the same time. The case3 in your patch update two cable state about one h/w port. >>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> It's because simple "USB" or "USB-HOST" means nothing for USB OTG >>>>>>>>>>>>> machine. It needs to know exact VBUS and ID states, which cannot be >>>>>>>>>>>>> concluded basing on cable type only. That's why I have used "USB-HOST" >>>>>>>>>>>>> name together with "USB" to pass additional information about USB cable >>>>>>>>>>>>> connection. >>>>>>>>>>>> >>>>>>>>>>>> I think this method is not proper to support this case. >>>>>>>>>>>> It may cause the confusion about other case using USB/USB-HOST cable state >>>>>>>>>>>> except of you commented case. >>>>>>>>>>> >>>>>>>>>>> That's why I finally proposed to use "USB-ID" and "USB-VBUS" in parallel >>>>>>>>>>> with old names. It seems to be simpler solution than adding new >>>>>>>>>>> mechanism notifying about VBUS and ID states changes. >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> As I commented on previous reply, I don't agree to use 'USB-ID' and 'USB-VBUS'. >>>>>>>>>> If we add new strange 'USB-ID' and 'USB-VBUS' name, we would add non-general cable >>>>>>>>>> name continuoulsy. >>>>>>>>>> >>>>>>>>>> I think that extcon core provide the helper API to get the value of VBUS. >>>>>>>>>> But I need to consider it. >>>>>>>>> >>>>>>>>> Now it is starting to look like existing extcon states are not suitable for USB/PHY drivers to deliver >>>>>>>>> VBUS and ID information reliably. >>>>>>>>> This is because based on your comments the "USB" and "USB-HOST" states look like some fuzzy states >>>>>>>>> and have no direct correspondence with "VBUS" and "ID". The fact that they can't become >>>>>>>>> attached simultaneously makes me conclude that "USB" and "USB-HOST" cable states are really >>>>>>>>> capturing only the ID pin state. >>>>>>>>> >>>>>>>>> I can suggest the following options >>>>>>>>> a) let "USB" and "USB-HOST" only indicate ID pin status. Add a new cable state for "VBUS" notification. >>>>>>>>> Maybe call it "USB-POWER" or something. >>>>>>> >>>>>>> We must discuss it before using the new cable name >>>>>>> such as "USB-POWER", "USB-ID" and "USB-VBUS". >>>>>> >>>>>> I didn't say to add "USB-ID" or "USB-VBUS". solution (a) was to have the following >>>>> >>>>> Right. Robert suggested the "USB-ID" and "USB-VBUS" cable name on previous mail in mail thread. >>>> >>>> From USB/USB-PHY driver point, it needs to know id and vbus value >>>> for their internal logic, so as extcon users, the cable name >>>> is better to reflect meaning of id and vbus, like "USB-ID" and "USB-VBUS", >>>> if the power is from vbus pin at USB cable, I don't think we need another >>>> cable name "USB-POWER" even the USB/USB-PHY driver don't need it. >>> >>> I agree as well that this is the *best* option for USB case. Just because Chanwoo was >>> objecting these names I suggested "USB-POWER". >>> >>> Chanwoo, can we simply get rid of "USB" and "USB-HOST" cables and move to >>> "USB-ID" and "USB-VBUS"? >> >> I'm wondering about changing the previous cable name from 'USB'/'USB-HOST' >> to 'USB-ID/USB-VBUS' because extcon framework update the state of cable by >> using uevent and the user-space process would catch the changed state by >> using cable name ('USB'/'USB-HOST'). >> >> The user-space process may not consider the both id and vbus of USB. >> If 'USB-ID'/'USB-VBUS' cable name is used instead of 'USB'/'USB-HOST', >> It may cause the confusion about what is meaning of cable name >> on user-space process. > >>>From the user point, maybe the name of 'USB-OTG' is more suitable > due to below reasons: > - The users usually call this Micro-AB cable as 'USB-OTG' cable > - When this Micro-AB cable is inserted, the current port may will work as > host role, but if OTG HNP is supported, this port may be switched to device > role on the fly, eg, use case like Apple Carplay. OK. I agree that using the 'USB-OTG' cable name instead of 'USB-HOST'. - 'USB' for usb device - 'USB-HOST' -> 'USB-OTG' for usb host > >> >> So, >> I prefer to use existing 'USB' and 'USB-HOST' cable name. >> and then want to add additional method to get the vbus state. >> >> I think two following method to get the vbus state. >> 1) Add the extcon_{get|set}_vbus_state() >> - extcon_{get|set}_vbus_state() >> - the list of of return value >> #define EXTCON_USB_VBUS_OFF 0 >> #define EXTCON_USB_VBUS_ON 1 >> >> When USB/USB-HOST is attached and receive the notification onextcon consumer driver >> ,extcon consumer driver would get the vbus state by extcon_get_vbus_state(). >> >> 2) Add the notifier chain for vbus state update >> - extcon_{register|unregister}_vbus_notifier() >> - the list of notifier event >> #define EXTCON_USB_VBUS_OFF 0 >> #define EXTCON_USB_VBUS_ON 1 >> > > Ok, from USB point, external id/vbus value can't decide > which role the controller will be, the controller driver > will decide role according to many things, eg, user configurations, > id/vbus value, OTG HNP, etc. > > So, from USB controller/phy driver, it doesn't care which cable is > inserted, it cares about id/vbus value. Eg, it can get id/vbus value > and it will be notified when the id/vbus value has changed. OK, I change the notifier name and add notifier events as following: - extcon_{register|unregister}_usb_notifier(struct extcon_dev *edev, struct notifier_block *nb); - list of notifier events #define EXTCON_USB_ID_L_VBUS_L 0 /* ID low and VBUS low */ #define EXTCON_USB_ID_L_VBUS_H 1 /* ID low and VBUS high */ #define EXTCON_USB_ID_H_VBUS_L 2 /* ID high and VBUS low */ #define EXTCON_USB_ID_H_VBUS_H 3 /* ID high and VBUS high */ I think that we need the opinion of Felipe and Kishon about this notifier chain. > >> >> 3) add the new cable 'USB-POWER' by Roger suggestion . >> - When 'USB-POWER' cable is attached, extcon will update the cable state >> 'USB-POWER' means only the vbus state. But, 'USB-POWER' is not h/w cable. >> The user-space process would handle this uevent of 'USB-POWER' >> such as h/w cable's uevent. I think it is not clear on the user-space process aspect. > > Would you explain the user for 'USB-POWER', and what it stands for from > user point? IMO, I think '*-POWER' keyword is not standard cable name on the user-space. As I commend on upper reply, I agree USB/USB-OTG cable name. [snip] Thanks, Chanwoo Choi