Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752206AbcDFOAI (ORCPT ); Wed, 6 Apr 2016 10:00:08 -0400 Received: from mga03.intel.com ([134.134.136.65]:18818 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750890AbcDFOAG (ORCPT ); Wed, 6 Apr 2016 10:00:06 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,447,1455004800"; d="asc'?scan'208";a="681767396" From: Felipe Balbi To: Jun Li , Baolin Wang , Peter Chen Cc: Greg KH , Sebastian Reichel , Dmitry Eremin-Solenikov , David Woodhouse , Peter Chen , Alan Stern , Yoshihiro Shimoda , Lee Jones , Mark Brown , Charles Keepax , "patches\@opensource.wolfsonmicro.com" , Linux PM list , USB , "device-mainlining\@lists.linuxfoundation.org" , LKML Subject: RE: [PATCH v9 2/4] gadget: Support for the usb charger framework In-Reply-To: References: <13c2f4fb71958bf9a5527acbed8b8b60dc569656.1459494744.git.baolin.wang@linaro.org> <20160406071956.GA21101@shlinux2.ap.freescale.net> <871t6j2ai2.fsf@intel.com> <87oa9m28x0.fsf@intel.com> User-Agent: Notmuch/0.21+96~g9bbc54b (http://notmuchmail.org) Emacs/25.0.90.3 (x86_64-pc-linux-gnu) Date: Wed, 06 Apr 2016 16:58:03 +0300 Message-ID: <87egai261g.fsf@intel.com> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha1; protocol="application/pgp-signature" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3894 Lines: 105 --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Hi, Jun Li writes: >> >> >> > Since we already have get_charger_type callback at usb_charger >> >> >> > structure, why we still need this API at usb_gadget_ops? >> >> >> >> >> >> In case some users want to get charger type at gadget level. >> >> >> >> >> > Why gadget needs to know charger type? I also don't catch the >> >> > intent of >> >> >> >> because some gadgets need to call usb_gadget_vbus_draw(), although >> >> for that they need power in mA rather. >> > >> > In below change of usb_gadget_vbus_draw(), already can get charger >> > type via usb_charger_get_type(). >> > >> > static inline int usb_gadget_vbus_draw(struct usb_gadget *gadget, >> > unsigned mA) { >> > + enum usb_charger_type type; >> > + >> > + if (gadget->charger) { >> > + type =3D usb_charger_get_type(gadget->charger); >> > + usb_charger_set_cur_limit_by_type(gadget->charger, typ= e, >> mA); >> > + } >> > + >> > if (!gadget->ops->vbus_draw) >> > return -EOPNOTSUPP; >> > return gadget->ops->vbus_draw(gadget, mA); >> > >> > Could you detail in what situation gadget->ops-> get_charger_type() is >> used? >>=20 >> isn't it right there in the code ? Isn't usb_gadget_vbus_draw() calling >> it ? What did I miss here ? > > Well, that's true, so my real meaning is why gadget need get charger type > via another new api gadget->ops->get_charger_type(). because of semantics. usb_gadget_vbus_draw() is *only* supposed to connect a load across vbus and gnd so some battery can be charged. Also, we need to abstract away this ->get_charger_type() operation because it might be different for each UDC. $subject has a fragility, however: It's assuming that we should always call ->get_charger_type() before ->vbus_draw(), but that's a good default, I'd say. >> >> > This api, as my understanding, gadget only need report gadget state >> >> changes. >> >> > All information required for usb charger is charger type and gadget >> >> state. >> >> >> >> you're making an assumption about how the HW is laid out which might >> >> not be true. >> >> >> > >> > What other information you refer to here? Or what I am not aware of? >>=20 >> what I'm trying to say is that you're assuming gadgets don't need to know >> anything other than charger type and gadget state (suspended, resume, >> enumerated, default state, addressed, etc), but that might not be true f= or >> all UDCs. You can't make that assumption that charger type and gadget >> state is enough. The real question is what do we need *now*, but still >> keep in mind that what we need *now* might be valid 2 years from now, so >> API needs to be a little flexible. > > Get your point, flexible, I just thought create an api without any user > for existing case/spec, wouldn't it be better to let the real user add it > later when it's needed. that sure is a fair point. =2D-=20 balbi --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJXBRXsAAoJEIaOsuA1yqREYj4P/RFFMxZ+RNMNa3TfoVsj7dtZ WlYosaTbneTJ0LL/qkqx2OlfEfHyt24PN6xMXnNxx/DD8ste6zc+HxyKc8+Y6VQu YzEt7wCOhtte8vvJB82co+JUraO21S4aLa9I/6Gtq5odBBiPyDosnmYlusT0geEy T8qmwjWT5tLsG9gqMcyLbrhhu4hnTPpaeLb2bbPRgEXzHWi6vPbIqyCpAHyLfQ0j 7ZANyA5eychiq24OPSmZmt9sEyriFxKRacr3fbt2yUDyL+K0ItR5ore8+kLNs2P+ JM9EY4C0bc3pyttjIY1AjVnz98Lz+Dy2PpVnJvmQEdL0fjiLynWbtiq/GyHutUb2 ay87fr/+3NxV5QTrl+6wBj486xFSo51yegfj3bw3lP6grtSuwHWsffD8v3wjHuI9 i7HccRYS8Yu0WHjDm363yhXEQ1J/BS3MBR5l3vL+dW8j/XU3dX8VkPeXSsBZOrzN BMxF7nQ/lpoK0zJI9bf1XG3GEk0t1qeJBQsSVJMxicdTpjkyqzNO9pBgg2n+WDWH FqE4u6fdhNsF3pfO3MYQvUw7IitfpiBpQvDk9q/FR4NB7e5/iUaQB8kJqqrZ0pc4 7Ca94b+3WBFHkE4EJCkro1D/5/Q0Hqj/k0cOp3wHDvWbi0v6jUOY9nbY8WoKhs9B Q1raQNevLrrIOa+DaF3R =lbXJ -----END PGP SIGNATURE----- --=-=-=--