Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932881AbcKCB2L (ORCPT ); Wed, 2 Nov 2016 21:28:11 -0400 Received: from mx2.suse.de ([195.135.220.15]:38189 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753859AbcKCB0C (ORCPT ); Wed, 2 Nov 2016 21:26:02 -0400 From: NeilBrown To: Baolin Wang Date: Thu, 03 Nov 2016 12:25:42 +1100 Cc: Felipe Balbi , Greg KH , Sebastian Reichel , Dmitry Eremin-Solenikov , David Woodhouse , robh@kernel.org, Jun Li , 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 Subject: Re: [PATCH v18 0/4] Introduce usb charger framework to deal with the usb gadget power negotation In-Reply-To: References: <87k2cttptn.fsf@notabene.neil.brown.name> <87a8dls7yn.fsf@notabene.neil.brown.name> User-Agent: Notmuch/0.22.1 (http://notmuchmail.org) Emacs/24.5.1 (x86_64-suse-linux-gnu) Message-ID: <871sytqrqh.fsf@notabene.neil.brown.name> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha256; protocol="application/pgp-signature" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5300 Lines: 118 --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable On Tue, Nov 01 2016, Baolin Wang wrote: >> So I won't be responding on this topic any further until I see a genuine >> attempt to understand and resolve the inconsistencies with >> usb_register_notifier(). > > Any better solution? I'm not sure exactly what you are asking, so I'll assume you are asking the question I want to answer :-) 1/ Liase with the extcon developers to resolve the inconsistencies with USB connector types. e.g. current there is both "EXTCON_USB" and "EXTCON_CHG_USB_SDP" which both seem to suggest a standard downstream port. There is no documentation describing how these relate, and no consistent practice to copy. I suspect the intention is that EXTCON_USB and EXTCON_USB_HOST indicated that data capabilities of the cable, while EXTCON_CHG_USB* indicate the power capabilities of the cable.=20 So EXTCON_CHG_USB_SDP should always appear together with EXTCON_USB while EXTCON_CHG_USB_DCP would not, and EXTCON_CHG_USB_ACA would normally appear with EXTCON_USB_HOST (I think). Some drivers follow this model, particularly extcon-max14577.c but it is not consistent. This policy should be well documented and possibly existing drivers should be updated to follow it. At the same time it would make sense to resolve EXTCON_CHG_USB_SLOW and EXTCON_CHG_USB_FAST. These names don't mean much. They were recently removed from drivers/power/axp288_charger.c which is good, but are still used in drivers/extcon/extcon-max* Possibly they should be changed to names from the standard, or possibly they should be renamed to identify the current they are expected to provide. e.g. EXTCON_CHG_USB_500MA and EXTCON_CHG_USB_1A 2/ Change all usb phys to register an extcon and to send appropriate notifications. Many already do, but I don't think it is universal. It is probable that the extcon should be registered using common code instead of each phy driver having its own extcon_get_edev_by_phandle() or whatever. If the usb phy driver needs to look at battery charger registers to know what sort of cable was connected (which I believe is the case for the chips you are interested in), then it should do that. 3/ Currently some USB controllers discover that a cable was connected by listening on an extcon, and some by registering for a usb_notifier (described below) ... though there seem to only be 2 left which do that. Now that all USB phys send connection information via extcon (see 2), the USB controllers should be changed to all find out about the cable using extcon. 4/ struct usb_phy contains: /* for notification of usb_phy_events */ struct atomic_notifier_head notifier; This is used inconsistently. Sometimes the argument passed is NULL, sometimes it is a pointer to 'vbus_draw' - the current limited negotiated via USB, sometimes it is a pointer the the gadget though as far as I can tell, that last one is never used. This should be changed to be consistent. This notifier is no longer needed to tell the USB controller that a cable was connected (extcon now does that, see 3) so it is only used to communicate the 'vbus_draw' information. So it should be changed to *only* send a notification when vbus_draw is known, and it should carry that information. This should probably be done in common code, and removed from individual drivers. 5/ Now that all cable connection notifications are sent over extcon and all vbus_draw notifications are sent over the usb_phy notifier, write some support code that a power supply client can use to be told what power is available. e.g. a battery charger driver would call: register_power_client(.....) or similar, providing a phandle (or similar) for the usb phy and a function to call back when the available current changes (or maybe a work_struct containing the function pointer) register_power_client() would then register with extcon and separately with the usb_phy notifier. When the different events arrive it calculates what ranges of currents are expected and calls the call-back function with those details. 6/ Any battery charger that needs to know the available current can now call register_power_client() and get the information delivered. NeilBrown --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIcBAEBCAAGBQJYGpIWAAoJEDnsnt1WYoG5xiIP/3sO7ZK4dKiXY9NuCcvBIdkp di/9S6p3/msqzoa5pFGSmGq4cSW5PBUFviu96BLXopJD+l18YTqySh3iDbyogj3W xdhi0TJBw4c+sxdNB+/hGqTSMevnuLsNqLnbyjIjAm+N2YM02X0dFS8fYER3Ojp/ k764dZW3n5cn3Ipju8QI+6qwfcRSBgr8Cl5/l5WnrIxLyBaW5CMuhTe+k5awdPDL /btBa+d7T0mbQHTEKxAZx2NA4ksKE2jXyaAB4NKYGVPGCbTBLD1zBI6tWwrtOhjD +ceUME4fBolUayxYZFO1Pn/CAwCGU+AOOsH9dY3UCMTY/T01S6fL69wItu3F3jpl cXAKRG1miCh/WWnR78VyNo/4q4XofdXEQqAlQVfe/gU1nj3zg+eItZjvGhxi0I6m jmfD7pHp67RoxJUAnsMxf3qhFKHT9zx5+NKkMAqbdir3NBI9GCpxDlC2y5GGtnWJ J7HvKTCDJX8oHDR66C7wJ+PGftsaUZaA0Iz+NcBhT9WZSXsencK5+qwLGINChIqy oEdERN2NHI1+wboeQLQGqwl9jIdHNh36/pVkOVyxd/8Jw7LyNwkNr4dZr8XgU4EW +n3rasBCf6VrH3QBjtn3aWPjq2J+3IdDsV9SGjMLm3ISisa0h8sZ6HJ29N5mI/iG TDX49ZqHITVtFsmKvLb7 =ja/Q -----END PGP SIGNATURE----- --=-=-=--