From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752187AbcIIGqj (ORCPT ); Fri, 9 Sep 2016 02:46:39 -0400 Received: from mail-yw0-f182.google.com ([209.85.161.182]:33285 "EHLO mail-yw0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751222AbcIIGqf (ORCPT ); Fri, 9 Sep 2016 02:46:35 -0400 MIME-Version: 1.0 In-Reply-To: <87y4326l44.fsf@notabene.neil.brown.name> References: <8760q9a8m6.fsf@notabene.neil.brown.name> <878tv297a0.fsf@notabene.neil.brown.name> <87y4326l44.fsf@notabene.neil.brown.name> From: Baolin Wang Date: Fri, 9 Sep 2016 14:46:33 +0800 Message-ID: Subject: Re: [PATCH v16 0/4] Introduce usb charger framework to deal with the usb gadget power negotation To: NeilBrown Cc: NeilBrown , Felipe Balbi , Greg KH , Sebastian Reichel , Dmitry Eremin-Solenikov , David Woodhouse , robh@kernel.org, Jun Li , Marek Szyprowski , Ruslan Bilovol , Peter Chen , Alan Stern , r.baldyga@samsung.com, grygorii.strashko@ti.com, Yoshihiro Shimoda , Lee Jones , Mark Brown , Charles Keepax , patches@opensource.wolfsonmicro.com, Linux PM list , USB , device-mainlining@lists.linuxfoundation.org, LKML , "Bird, Timothy" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 9 September 2016 at 07:13, NeilBrown wrote: > On Thu, Sep 08 2016, Baolin Wang wrote: > >> On 8 September 2016 at 15:31, NeilBrown wrote: >>> On Thu, Sep 08 2016, Baolin Wang wrote: >>>> >>>> Now the usb charger will not get charger type from 'extcon' subsystem, >>>> we get the charger type from 'power_supply' and calllback >>>> 'get_charger_type' for users. >>> >>> I understand this. I think it is wrong because, in general, the >>> power_supply doesn't know what the charger_type is (it might know it is >>> USB, but most don't know which sort of USB). The PHY knows that, not >>> the power_supply. >> >> I don't think so. Now many platforms will detect the charger type by >> PMIC hardware, and we can get the charger type by PMIC hardware >> register. Then power supply driver can access the PMIC register to get >> the charger type. Here USB charger just considers if the accessing the >> PMIC register to get charger type is implemented in power supply, it >> is optional depending on what your platform designed. >> > > In practice, the USB PHY and the Power manager will often be in the same > IC (the PMIC) so the driver for one could look at the registers for the > other. > But there is no guarantee that the hardware works like that. It is > best to create a generally design. Yes, we hope to create one generally design, so we need to consider this situation: the power supply getting the charger type by accessing PMIC registers. The registers which save the charger type are not always belong to the USB PHY, may be just some registers on PMIC. Now in mainline kernel, there are 3 methods can get the charger type which need to integrate with USB charger framework: 1. power supply 2. extcon (need to add as you suggested) 3. others (by 'get_charger_type' callback of USB charger) > Conceptually, the PHY is separate from the power manager and a solution > which recognises that will be more universal. > > If the power manager can always just look at that phy registers to know > what sort of charger is connected, why does you framework need to work > with charger types at all? > >>>> >>>> Yes, but you must think about some special cases on some platforms. >>>> Users may need to change the current in some situations, thus we >>>> should export one API for users to change the current. (I think you >>>> misunderstand the current limit here, that is the current for power >>>> driver to draw). >>> >>> Can you be specific about these "special cases" please? >>> I cannot think of any. >> >> Suppose the USB configuration requests 100mA, then we should set the >> USB charger current is 100mA by __usb_charger_set_cur_limit_by_type() >> funtion, then notify this to power driver. > > ahh.... I had missed something there. It's a while since I looked > closely at these patches. > > Only.... this usage of usb_charger_set_cur_limit_by_type() is really > nonsensical. > > If the cable is detected as being DCP or CDP or ACA (or ACA/DOCK) then > the number negotiated with the USB configuration is not relevant and > should be ignored. There is a guaranteed minimum which is at least the > maximum that *can* be negotiated. Yes. If it is not relevant, we will no't set the current from USB configuration. Just when your charger type is SDP and the USB enumeration is done, we can get the USB configuration from host to set current. > > It is only when the cable appears to be a SDP (standard downstream > port) that the usb-config negotiation is relevant. That is because the > minimum guaranteed for SDP is only 100mA. > > NeilBrown -- Baolin.wang Best Regards