From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753007AbdCMJIY (ORCPT ); Mon, 13 Mar 2017 05:08:24 -0400 Received: from mail-oi0-f48.google.com ([209.85.218.48]:33654 "EHLO mail-oi0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751918AbdCMInh (ORCPT ); Mon, 13 Mar 2017 04:43:37 -0400 MIME-Version: 1.0 In-Reply-To: References: <87zih3m73h.fsf@notabene.neil.brown.name> From: Baolin Wang Date: Mon, 13 Mar 2017 16:43:35 +0800 Message-ID: Subject: Re: [PATCH v19 0/4] Introduce usb charger framework to deal with the usb gadget power negotation To: Jun Li Cc: NeilBrown , Felipe Balbi , Greg KH , Sebastian Reichel , Dmitry Eremin-Solenikov , David Woodhouse , "robh@kernel.org" , Marek Szyprowski , Ruslan Bilovol , Peter Chen , Alan Stern , "grygorii.strashko@ti.com" , Yoshihiro Shimoda , Lee Jones , Mark Brown , John Stultz , Charles Keepax , "patches@opensource.wolfsonmicro.com" , Linux PM list , USB , "device-mainlining@lists.linuxfoundation.org" , LKML Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id v2D98f9N021864 On 13 March 2017 at 09:09, Jun Li wrote: > Hi, > >> -----Original Message----- >> From: Baolin Wang [mailto:baolin.wang@linaro.org] >> Sent: Friday, March 10, 2017 6:52 PM >> To: Jun Li >> Cc: NeilBrown ; Felipe Balbi ; Greg KH >> ; Sebastian Reichel ; Dmitry >> Eremin-Solenikov ; David Woodhouse >> ; robh@kernel.org; Marek Szyprowski >> ; Ruslan Bilovol ; >> Peter Chen ; Alan Stern >> ; grygorii.strashko@ti.com; Yoshihiro Shimoda >> ; Lee Jones ; >> Mark Brown ; John Stultz ; >> Charles Keepax ; >> patches@opensource.wolfsonmicro.com; Linux PM list > pm@vger.kernel.org>; USB ; device- >> mainlining@lists.linuxfoundation.org; LKML >> Subject: Re: [PATCH v19 0/4] Introduce usb charger framework to deal with >> the usb gadget power negotation >> >> On 10 March 2017 at 16:27, Jun Li wrote: >> > Hi >> > >> >> -----Original Message----- >> >> From: Baolin Wang [mailto:baolin.wang@linaro.org] >> >> Sent: Friday, March 10, 2017 3:15 PM >> >> To: Jun Li >> >> Cc: NeilBrown ; Felipe Balbi ; Greg >> >> KH ; Sebastian Reichel ; >> >> Dmitry Eremin-Solenikov ; David Woodhouse >> >> ; robh@kernel.org; Marek Szyprowski >> >> ; Ruslan Bilovol >> >> ; Peter Chen ; >> >> Alan Stern ; grygorii.strashko@ti.com; >> >> Yoshihiro Shimoda ; Lee Jones >> >> ; Mark Brown ; John Stultz >> >> ; Charles Keepax >> >> ; >> >> patches@opensource.wolfsonmicro.com; Linux PM list > >> pm@vger.kernel.org>; USB ; device- >> >> mainlining@lists.linuxfoundation.org; LKML >> >> >> >> Subject: Re: [PATCH v19 0/4] Introduce usb charger framework to deal >> >> with the usb gadget power negotation >> >> >> >> On 10 March 2017 at 14:30, Jun Li wrote: >> >> >> >> > >> >> >> >> > Will generic phy need add extcon as well? >> >> >> >> >> >> >> >> Yes, will add a 'struct extcon_dev*' in 'struct usb_phy', which >> >> >> >> will be common code. >> >> >> >> >> >> >> > >> >> >> > I mean the common code need add 'struct extcon_dev' into both >> >> >> > 'struct phy' and 'struct usb_phy', right? as some/new usb phy >> >> >> > use that generic phy >> >> >> driver. >> >> >> >> >> >> Ah, you remind me. Seems we need also add one 'struct extcon_dev' >> >> >> into 'struct phy'. >> >> >> >> >> >> >> > >> >> >> >> >> > >> >> >> >> >> >> Secondly, I also agreed with Peter's comments: Not only >> >> >> >> >> >> USB PHY to register an extcon, but also for the drivers >> >> >> >> >> >> which can detect USB charger type, it may be USB >> >> >> >> >> >> controller driver, USB type-c driver, pmic driver, and >> >> >> >> >> >> these drivers may not have an extcon device since the >> >> >> >> >> >> internal part can finish >> >> the vbus detect. >> >> >> >> >> > >> >> >> >> >> > Whichever part can detect vbus, the driver for that part >> >> >> >> >> > must be able to find the extcon and trigger a notification. >> >> >> >> >> > Maybe one part can detect VBUS, another can measure the >> >> >> >> >> > resistance on ID and a third can work through the state >> >> >> >> >> > machine to determine if D+ and D- are shorted together. >> >> >> >> >> > Somehow these three need to work together to determine >> >> >> >> >> > what is >> >> >> >> >> plugged >> >> >> >> >> > in to the external connection port. Somewhere there much >> >> >> >> >> > an >> >> >> 'extcon' >> >> >> >> >> > device which represents that port and which the three >> >> >> >> >> > devices can find and can interact with. >> >> >> >> >> > I think it makes sense for the usb_phy to be the connection >> point. >> >> >> >> >> > Each of the devices can get to the phy, and the phy can >> >> >> >> >> > get to the >> >> >> >> extcon. >> >> >> >> >> > It doesn't matter very much if the usb phy driver creates >> >> >> >> >> > the extcon, or if something else creates the extcon and >> >> >> >> >> > the phy driver performs a lookup to find it (e.g. based on >> devicetree info). >> >> >> >> >> > >> >> >> >> >> > The point is that there is obviously an external physical >> >> >> >> >> > connection, and so there should be an 'extcon' device that >> >> >> represents it. >> >> >> >> >> >> >> >> >> >> Peter & Jun, is it OK for you every phy has one extcon >> >> >> >> >> device to receive VBUS notification, especially for >> >> >> >> >> detecting the charger type by >> >> >> >> software? >> >> >> >> >> >> >> >> >> > >> >> >> >> > My understanding is phy/usb_phy as the connection point, will >> >> >> >> > send the notification to PMIC driver which actually control >> >> >> >> > the charge current, also this will be done in your common >> >> >> >> > framework, >> >> right? >> >> >> >> >> >> >> >> Not in USB charger framework. If we are all agree every usb_phy >> >> >> >> can register one extcon device, can get correct charger type >> >> >> >> and send out correct vbus_draw information, then we don't need >> >> >> >> USB charger framework as Neil suggested. So this will be okay >> >> >> >> for your case (especially for detecting the charger type by >> software) ? >> >> >> > >> >> >> > In my case, charger detection is done by controller driver and I >> >> >> > need do charger type detection internally, and only pass the >> >> >> > current draw info via phy which will send out, this seems ok for >> >> >> > me, but I think it will be good if you or someone can show us an >> >> >> > example user based on the >> >> >> design Neil suggested. >> >> >> > Will you work out that common code if this USB charger framework >> >> >> > is not >> >> >> needed? >> >> >> >> >> >> I will add a 'struct extcon_dev*' in 'struct usb_phy' and struct >> >> >> phy“. Others are already ready if everyone has no complain about >> >> >> current design, except >> >> > >> >> > Only adding extcon_dev into usb_phy/phy and all others are ready? >> >> > My understanding you will also do: >> >> > - We need find a central place to send the notification(phy common >> part). >> >> >> >> That will include these implementation when adding extcon_dev. >> >> >> > >> > OK, thanks. >> > >> >> > - If the extcon_dev is directly added in usb_phy/phy, PMIC needs >> >> > some >> >> API to findup it. >> >> >> >> PMIC can find extcon device by phandle. >> > >> > extcon_dev(not a reference pointer) is directly added in usb_phy/phy, >> > not via devicetree, how PMIC find it by phandle? >> >> From my understanding, here we should add one pointer (extcon_dev *), >> since usb phy is not one external connect device. > > Agreed. > >> >> > >> >> >> >> > >> >> >> my one concern. (I am afraid if it is enough to send out vbus draw >> >> >> information from USB phy driver, for example you will miss super >> >> >> speed (900mA), which need get the speed information from gadget >> >> >> driver.) >> >> >> >> >> > >> >> > Can we handle this in USB(so has super speed information) and just >> >> > send out 900mA directly? >> >> >> >> From Neil's suggestion, we only have one place to send out current >> >> information from usb phy, so I have this concern and doubt if we >> >> still need the USB charger framework. >> > >> > So if put it in phy/usb_phy, this is a problem, that only one place >> > should have the infor of both speed and usb state, how about put it at >> > usb_gadget, then, e.g. send the notification in >> usb_gadget_vbus_connect()? >> >> That is same what USB charger did, from this point, we need USB charger to >> send out vbus draw information according to speed and usb state. But I >> should listen to other guys suggestion. Peter and Felipe, what do you think? > > So now the only to do work is to find a common place to send the notification out > (based on gadget speed and sate). Yes. -- Baolin.wang Best Regards From mboxrd@z Thu Jan 1 00:00:00 1970 From: Baolin Wang Subject: Re: [PATCH v19 0/4] Introduce usb charger framework to deal with the usb gadget power negotation Date: Mon, 13 Mar 2017 16:43:35 +0800 Message-ID: References: <87zih3m73h.fsf@notabene.neil.brown.name> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-oi0-f43.google.com ([209.85.218.43]:35523 "EHLO mail-oi0-f43.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751827AbdCMInr (ORCPT ); Mon, 13 Mar 2017 04:43:47 -0400 Received: by mail-oi0-f43.google.com with SMTP id 62so74696021oih.2 for ; Mon, 13 Mar 2017 01:43:46 -0700 (PDT) In-Reply-To: Sender: linux-pm-owner@vger.kernel.org List-Id: linux-pm@vger.kernel.org To: Jun Li Cc: NeilBrown , Felipe Balbi , Greg KH , Sebastian Reichel , Dmitry Eremin-Solenikov , David Woodhouse , "robh@kernel.org" , Marek Szyprowski , Ruslan Bilovol , Peter Chen , Alan Stern , "grygorii.strashko@ti.com" , Yoshihiro Shimoda , Lee Jones , Mark Brown , John Stultz , Charles Keepax , "patches@opensource.wolfsonmicro.com" On 13 March 2017 at 09:09, Jun Li wrote: > Hi, > >> -----Original Message----- >> From: Baolin Wang [mailto:baolin.wang@linaro.org] >> Sent: Friday, March 10, 2017 6:52 PM >> To: Jun Li >> Cc: NeilBrown ; Felipe Balbi ; Greg KH >> ; Sebastian Reichel ; Dmitry >> Eremin-Solenikov ; David Woodhouse >> ; robh@kernel.org; Marek Szyprowski >> ; Ruslan Bilovol ; >> Peter Chen ; Alan Stern >> ; grygorii.strashko@ti.com; Yoshihiro Shimoda >> ; Lee Jones ; >> Mark Brown ; John Stultz ; >> Charles Keepax ; >> patches@opensource.wolfsonmicro.com; Linux PM list > pm@vger.kernel.org>; USB ; device- >> mainlining@lists.linuxfoundation.org; LKML >> Subject: Re: [PATCH v19 0/4] Introduce usb charger framework to deal wit= h >> the usb gadget power negotation >> >> On 10 March 2017 at 16:27, Jun Li wrote: >> > Hi >> > >> >> -----Original Message----- >> >> From: Baolin Wang [mailto:baolin.wang@linaro.org] >> >> Sent: Friday, March 10, 2017 3:15 PM >> >> To: Jun Li >> >> Cc: NeilBrown ; Felipe Balbi ; Greg >> >> KH ; Sebastian Reichel ; >> >> Dmitry Eremin-Solenikov ; David Woodhouse >> >> ; robh@kernel.org; Marek Szyprowski >> >> ; Ruslan Bilovol >> >> ; Peter Chen ; >> >> Alan Stern ; grygorii.strashko@ti.com; >> >> Yoshihiro Shimoda ; Lee Jones >> >> ; Mark Brown ; John Stultz >> >> ; Charles Keepax >> >> ; >> >> patches@opensource.wolfsonmicro.com; Linux PM list > >> pm@vger.kernel.org>; USB ; device- >> >> mainlining@lists.linuxfoundation.org; LKML >> >> >> >> Subject: Re: [PATCH v19 0/4] Introduce usb charger framework to deal >> >> with the usb gadget power negotation >> >> >> >> On 10 March 2017 at 14:30, Jun Li wrote: >> >> >> >> > >> >> >> >> > Will generic phy need add extcon as well? >> >> >> >> >> >> >> >> Yes, will add a 'struct extcon_dev*' in 'struct usb_phy', which >> >> >> >> will be common code. >> >> >> >> >> >> >> > >> >> >> > I mean the common code need add 'struct extcon_dev' into both >> >> >> > 'struct phy' and 'struct usb_phy', right? as some/new usb phy >> >> >> > use that generic phy >> >> >> driver. >> >> >> >> >> >> Ah, you remind me. Seems we need also add one 'struct extcon_dev' >> >> >> into 'struct phy'. >> >> >> >> >> >> >> > >> >> >> >> >> > >> >> >> >> >> >> Secondly, I also agreed with Peter's comments: Not only >> >> >> >> >> >> USB PHY to register an extcon, but also for the drivers >> >> >> >> >> >> which can detect USB charger type, it may be USB >> >> >> >> >> >> controller driver, USB type-c driver, pmic driver, and >> >> >> >> >> >> these drivers may not have an extcon device since the >> >> >> >> >> >> internal part can finish >> >> the vbus detect. >> >> >> >> >> > >> >> >> >> >> > Whichever part can detect vbus, the driver for that part >> >> >> >> >> > must be able to find the extcon and trigger a notification= . >> >> >> >> >> > Maybe one part can detect VBUS, another can measure the >> >> >> >> >> > resistance on ID and a third can work through the state >> >> >> >> >> > machine to determine if D+ and D- are shorted together. >> >> >> >> >> > Somehow these three need to work together to determine >> >> >> >> >> > what is >> >> >> >> >> plugged >> >> >> >> >> > in to the external connection port. Somewhere there much >> >> >> >> >> > an >> >> >> 'extcon' >> >> >> >> >> > device which represents that port and which the three >> >> >> >> >> > devices can find and can interact with. >> >> >> >> >> > I think it makes sense for the usb_phy to be the connectio= n >> point. >> >> >> >> >> > Each of the devices can get to the phy, and the phy can >> >> >> >> >> > get to the >> >> >> >> extcon. >> >> >> >> >> > It doesn't matter very much if the usb phy driver creates >> >> >> >> >> > the extcon, or if something else creates the extcon and >> >> >> >> >> > the phy driver performs a lookup to find it (e.g. based on >> devicetree info). >> >> >> >> >> > >> >> >> >> >> > The point is that there is obviously an external physical >> >> >> >> >> > connection, and so there should be an 'extcon' device that >> >> >> represents it. >> >> >> >> >> >> >> >> >> >> Peter & Jun, is it OK for you every phy has one extcon >> >> >> >> >> device to receive VBUS notification, especially for >> >> >> >> >> detecting the charger type by >> >> >> >> software? >> >> >> >> >> >> >> >> >> > >> >> >> >> > My understanding is phy/usb_phy as the connection point, will >> >> >> >> > send the notification to PMIC driver which actually control >> >> >> >> > the charge current, also this will be done in your common >> >> >> >> > framework, >> >> right? >> >> >> >> >> >> >> >> Not in USB charger framework. If we are all agree every usb_phy >> >> >> >> can register one extcon device, can get correct charger type >> >> >> >> and send out correct vbus_draw information, then we don't need >> >> >> >> USB charger framework as Neil suggested. So this will be okay >> >> >> >> for your case (especially for detecting the charger type by >> software) ? >> >> >> > >> >> >> > In my case, charger detection is done by controller driver and I >> >> >> > need do charger type detection internally, and only pass the >> >> >> > current draw info via phy which will send out, this seems ok for >> >> >> > me, but I think it will be good if you or someone can show us an >> >> >> > example user based on the >> >> >> design Neil suggested. >> >> >> > Will you work out that common code if this USB charger framework >> >> >> > is not >> >> >> needed? >> >> >> >> >> >> I will add a 'struct extcon_dev*' in 'struct usb_phy' and struct >> >> >> phy=E2=80=9C. Others are already ready if everyone has no complain= about >> >> >> current design, except >> >> > >> >> > Only adding extcon_dev into usb_phy/phy and all others are ready? >> >> > My understanding you will also do: >> >> > - We need find a central place to send the notification(phy common >> part). >> >> >> >> That will include these implementation when adding extcon_dev. >> >> >> > >> > OK, thanks. >> > >> >> > - If the extcon_dev is directly added in usb_phy/phy, PMIC needs >> >> > some >> >> API to findup it. >> >> >> >> PMIC can find extcon device by phandle. >> > >> > extcon_dev(not a reference pointer) is directly added in usb_phy/phy, >> > not via devicetree, how PMIC find it by phandle? >> >> From my understanding, here we should add one pointer (extcon_dev *), >> since usb phy is not one external connect device. > > Agreed. > >> >> > >> >> >> >> > >> >> >> my one concern. (I am afraid if it is enough to send out vbus draw >> >> >> information from USB phy driver, for example you will miss super >> >> >> speed (900mA), which need get the speed information from gadget >> >> >> driver.) >> >> >> >> >> > >> >> > Can we handle this in USB(so has super speed information) and just >> >> > send out 900mA directly? >> >> >> >> From Neil's suggestion, we only have one place to send out current >> >> information from usb phy, so I have this concern and doubt if we >> >> still need the USB charger framework. >> > >> > So if put it in phy/usb_phy, this is a problem, that only one place >> > should have the infor of both speed and usb state, how about put it at >> > usb_gadget, then, e.g. send the notification in >> usb_gadget_vbus_connect()? >> >> That is same what USB charger did, from this point, we need USB charger = to >> send out vbus draw information according to speed and usb state. But I >> should listen to other guys suggestion. Peter and Felipe, what do you th= ink? > > So now the only to do work is to find a common place to send the notifica= tion out > (based on gadget speed and sate). Yes. --=20 Baolin.wang Best Regards