linux-kernel - Re: [PATCH v9 2/4] gadget: Support for the usb charger framework

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 6 Apr 2016 18:46:36 +0800
From:	Baolin Wang <baolin.wang@...aro.org>
To:	Peter Chen <hzpeterchen@...il.com>
Cc:	Felipe Balbi <balbi@...nel.org>,
	Greg KH <gregkh@...uxfoundation.org>,
	Sebastian Reichel <sre@...nel.org>,
	Dmitry Eremin-Solenikov <dbaryshkov@...il.com>,
	David Woodhouse <dwmw2@...radead.org>,
	Peter Chen <peter.chen@...escale.com>,
	Alan Stern <stern@...land.harvard.edu>, r.baldyga@...sung.com,
	Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>,
	Lee Jones <lee.jones@...aro.org>,
	Mark Brown <broonie@...nel.org>,
	Charles Keepax <ckeepax@...nsource.wolfsonmicro.com>,
	patches@...nsource.wolfsonmicro.com,
	Linux PM list <linux-pm@...r.kernel.org>,
	USB <linux-usb@...r.kernel.org>,
	device-mainlining@...ts.linuxfoundation.org,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v9 2/4] gadget: Support for the usb charger framework

On 6 April 2016 at 15:19, Peter Chen <hzpeterchen@...il.com> wrote:
> On Fri, Apr 01, 2016 at 03:21:50PM +0800, Baolin Wang wrote:
>>
>> @@ -563,6 +564,8 @@ struct usb_gadget_ops {
>>       struct usb_ep *(*match_ep)(struct usb_gadget *,
>>                       struct usb_endpoint_descriptor *,
>>                       struct usb_ss_ep_comp_descriptor *);
>> +     /* get the charger type */
>> +     enum usb_charger_type (*get_charger_type)(struct usb_gadget *);
>>  };
>
> 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.

>
>>
>>  /**
>> @@ -635,6 +638,8 @@ struct usb_gadget {
>>       unsigned                        out_epnum;
>>       unsigned                        in_epnum;
>>       struct usb_otg_caps             *otg_caps;
>> +     /* negotiate the power with the usb charger */
>> +     struct usb_charger              *charger;
>>
>>       unsigned                        sg_supported:1;
>>       unsigned                        is_otg:1;
>> @@ -839,10 +844,20 @@ static inline int usb_gadget_vbus_connect(struct usb_gadget *gadget)
>>   * reporting how much power the device may consume.  For example, this
>>   * could affect how quickly batteries are recharged.
>>   *
>> + * It will also notify the USB charger how much power the device may
>> + * consume if there is a USB charger linking with the gadget.
>> + *
>>   * Returns zero on success, else negative errno.
>>   */
>>  static inline int usb_gadget_vbus_draw(struct usb_gadget *gadget, unsigned mA)
>>  {
>> +     enum usb_charger_type type;
>> +
>> +     if (gadget->charger) {
>> +             type = usb_charger_get_type(gadget->charger);
>> +             usb_charger_set_cur_limit_by_type(gadget->charger, type, mA);
>> +     }
>> +
>
> You may do something redundant.
>
> - Charger detection only occurs at connection period, at other periods,
> we only change the current limit, and notify charger IC. That is to
> say, we may only need to save charger type and current limit at
> usb_charger structure, we don't need to distinguish all chargers
> type from time to time.

That's right. I just want to get the charger type as one parameter to
set current. The function is implemented as below:

enum usb_charger_type usb_charger_get_type(struct usb_charger *uchger)
{
        enum usb_charger_type type;

        mutex_lock(&uchger->lock);
        type = uchger->type;
        mutex_unlock(&uchger->lock);

        return type;
}

>
> - The purpose of usb_gadget_vbus_draw design is notify charger IC too,
> so you can do set current limit and notify charger IC together at this
> API together, it has already covered all situations. We don't need to
> notify charger IC again when the gadget status has changed again.

It did not notify charger IC again. You are right,
usb_gadget_vbus_draw design will notify charger IC, so we want to
record the current in usb charger framework at the same time.

>
>>       if (!gadget->ops->vbus_draw)
>>               return -EOPNOTSUPP;
>>       return gadget->ops->vbus_draw(gadget, mA);
>> --
>> 1.7.9.5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
>> the body of a message to majordomo@...r.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> --
>
> Best Regards,
> Peter Chen



-- 
Baolin.wang
Best Regards