Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

On 31 March 2016 at 16:15, Felipe Balbi  wrote:
>
> Hi Baolin,
>
> Baolin Wang  writes:
 >> Make sense. In our company's solution, charger detection can be done
 >> by hardware from PMIC at first, then it will not affect the DP/DM
 >> line when gadget starts to enumeration.
 >
 > I see, charger type detection is done automatically by PMIC when VBUS
 > is detected in your case, you just assume the process is complete
 > before SW do gadget connect. To make the framework common, you may do
 one time charger type check when vbus is on, and save it to avoid repeat
 charger type check.

 OK. I'll add one judgement to check if the charger type is set in
 'usb_charger_detect_type()' function.
>>>
>>> Just adding a judgement isn't enough here, your framework should make sure
>>> usb_charger_detect_type() is called before gadget connect, with that, the
>>> existing caller place just gets the charger type from the saved value.
>>> The real charger type detection done by usb_charger_detect_type() can
>>> be called only when vbus is on.
>>> e.g. maybe in usb_udc_vbus_handler() before usb_udc_connect_control().
>>
>> Yeah, Like Felipe suggested, I think we need to introduce one
>> 'charger_detect()' method to do the SW charger type detection at the
>> right gadget state. Thanks for your comments.
>
> Just to be clear, we add ->charger_detect() when we know of a platform
> which needs to manually detect the charger type. Until then, we ignore
> that situation. It might be a good idea, however, do document this in
> comments on your structure definition stating that if we need to detect
> charger type, a new method should be added ;-)

Make sense. Thanks.

>
> cheers
>
> --
> balbi



-- 
Baolin.wang
Best Regards

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

On 31 March 2016 at 16:15, Felipe Balbi  wrote:
>
> Hi Baolin,
>
> Baolin Wang  writes:
 >> Make sense. In our company's solution, charger detection can be done
 >> by hardware from PMIC at first, then it will not affect the DP/DM
 >> line when gadget starts to enumeration.
 >
 > I see, charger type detection is done automatically by PMIC when VBUS
 > is detected in your case, you just assume the process is complete
 > before SW do gadget connect. To make the framework common, you may do
 one time charger type check when vbus is on, and save it to avoid repeat
 charger type check.

 OK. I'll add one judgement to check if the charger type is set in
 'usb_charger_detect_type()' function.
>>>
>>> Just adding a judgement isn't enough here, your framework should make sure
>>> usb_charger_detect_type() is called before gadget connect, with that, the
>>> existing caller place just gets the charger type from the saved value.
>>> The real charger type detection done by usb_charger_detect_type() can
>>> be called only when vbus is on.
>>> e.g. maybe in usb_udc_vbus_handler() before usb_udc_connect_control().
>>
>> Yeah, Like Felipe suggested, I think we need to introduce one
>> 'charger_detect()' method to do the SW charger type detection at the
>> right gadget state. Thanks for your comments.
>
> Just to be clear, we add ->charger_detect() when we know of a platform
> which needs to manually detect the charger type. Until then, we ignore
> that situation. It might be a good idea, however, do document this in
> comments on your structure definition stating that if we need to detect
> charger type, a new method should be added ;-)

Make sense. Thanks.

>
> cheers
>
> --
> balbi



-- 
Baolin.wang
Best Regards

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Felipe Balbi]

Hi Baolin,

Baolin Wang  writes:
>>> >> Make sense. In our company's solution, charger detection can be done
>>> >> by hardware from PMIC at first, then it will not affect the DP/DM
>>> >> line when gadget starts to enumeration.
>>> >
>>> > I see, charger type detection is done automatically by PMIC when VBUS
>>> > is detected in your case, you just assume the process is complete
>>> > before SW do gadget connect. To make the framework common, you may do
>>> one time charger type check when vbus is on, and save it to avoid repeat
>>> charger type check.
>>>
>>> OK. I'll add one judgement to check if the charger type is set in
>>> 'usb_charger_detect_type()' function.
>>
>> Just adding a judgement isn't enough here, your framework should make sure
>> usb_charger_detect_type() is called before gadget connect, with that, the
>> existing caller place just gets the charger type from the saved value.
>> The real charger type detection done by usb_charger_detect_type() can
>> be called only when vbus is on.
>> e.g. maybe in usb_udc_vbus_handler() before usb_udc_connect_control().
>
> Yeah, Like Felipe suggested, I think we need to introduce one
> 'charger_detect()' method to do the SW charger type detection at the
> right gadget state. Thanks for your comments.

Just to be clear, we add ->charger_detect() when we know of a platform
which needs to manually detect the charger type. Until then, we ignore
that situation. It might be a good idea, however, do document this in
comments on your structure definition stating that if we need to detect
charger type, a new method should be added ;-)

cheers

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Felipe Balbi]

Hi Baolin,

Baolin Wang  writes:
>>> >> Make sense. In our company's solution, charger detection can be done
>>> >> by hardware from PMIC at first, then it will not affect the DP/DM
>>> >> line when gadget starts to enumeration.
>>> >
>>> > I see, charger type detection is done automatically by PMIC when VBUS
>>> > is detected in your case, you just assume the process is complete
>>> > before SW do gadget connect. To make the framework common, you may do
>>> one time charger type check when vbus is on, and save it to avoid repeat
>>> charger type check.
>>>
>>> OK. I'll add one judgement to check if the charger type is set in
>>> 'usb_charger_detect_type()' function.
>>
>> Just adding a judgement isn't enough here, your framework should make sure
>> usb_charger_detect_type() is called before gadget connect, with that, the
>> existing caller place just gets the charger type from the saved value.
>> The real charger type detection done by usb_charger_detect_type() can
>> be called only when vbus is on.
>> e.g. maybe in usb_udc_vbus_handler() before usb_udc_connect_control().
>
> Yeah, Like Felipe suggested, I think we need to introduce one
> 'charger_detect()' method to do the SW charger type detection at the
> right gadget state. Thanks for your comments.

Just to be clear, we add ->charger_detect() when we know of a platform
which needs to manually detect the charger type. Until then, we ignore
that situation. It might be a good idea, however, do document this in
comments on your structure definition stating that if we need to detect
charger type, a new method should be added ;-)

cheers

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

On 31 March 2016 at 14:12, Jun Li <jun...@nxp.com> wrote:
> Hi
>
>> -Original Message-
>> From: Baolin Wang [mailto:baolin.w...@linaro.org]
>> Sent: Thursday, March 31, 2016 1:23 PM
>> To: Jun Li <jun...@nxp.com>
>> Cc: Peter Chen <hzpeterc...@gmail.com>; Felipe Balbi <ba...@kernel.org>;
>> Greg KH <gre...@linuxfoundation.org>; Sebastian Reichel <s...@kernel.org>;
>> Dmitry Eremin-Solenikov <dbarysh...@gmail.com>; David Woodhouse
>> <dw...@infradead.org>; Peter Chen <peter.c...@freescale.com>; Alan Stern
>> <st...@rowland.harvard.edu>; r.bald...@samsung.com; Yoshihiro Shimoda
>> <yoshihiro.shimoda...@renesas.com>; Lee Jones <lee.jo...@linaro.org>; Mark
>> Brown <broo...@kernel.org>; Charles Keepax
>> <ckee...@opensource.wolfsonmicro.com>; patc...@opensource.wolfsonmicro.com;
>> Linux PM list <linux...@vger.kernel.org>; USB <linux-...@vger.kernel.org>;
>> device-mainlin...@lists.linuxfoundation.org; LKML > ker...@vger.kernel.org>
>> Subject: Re: [PATCH v8 0/4] Introduce usb charger framework to deal with
>> the usb gadget power negotation
>>
>> On 30 March 2016 at 18:58, Jun Li <jun...@nxp.com> wrote:
>> >> >> >> > Seems you don't want to guarantee charger type detection is
>> >> >> >> > done before gadget connection(pullup DP), right?
>> >> >> >> > I see you call usb_charger_detect_type() in each gadget usb
>> >> >> >> > state
>> >> >> >> changes.
>> >> >> >>
>> >> >> >> I am not sure I get your point correctly, please correct me if
>> >> >> >> I misunderstand you.
>> >> >> >> We need to check the charger type at every event comes from the
>> >> >> >> usb gadget state changes or the extcon device state changes,
>> >> >> >> which means a new charger plugin or pullup.
>> >> >> >>
>> >> >> >
>> >> >> > According to usb charger spec, my understanding is you can't do
>> >> >> > real charger detection procedure *after* gadget
>> >> >> > _connection_(pullup DP), also I don't
>> >> >>
>> >> >> Why can not? Charger detection is usually from PMIC.
>> >> >
>> >> > Charger detection process will impact DP/DM line state, see usb
>> >> > charger spec v1.2 for detail detection process, section 4.6.3 says:
>> >> >
>> >> > "A PD is allowed to *disconnect* and repeat the charger detection
>> >> > process multiple times while attached. The PD is required to wait
>> >> > for a time of at least TCP_VDM_EN max between disconnecting and
>> >> > restarting the charger detection process."
>> >> >
>> >> > As Peter mentioned, the charger detection should happen between
>> >> > VBUS detection and gadget pull up DP for first plug in case. So
>> >> > when gadget connect (pullup DP), you should already know the
>> charger type.
>> >>
>> >> Make sense. In our company's solution, charger detection can be done
>> >> by hardware from PMIC at first, then it will not affect the DP/DM
>> >> line when gadget starts to enumeration.
>> >
>> > I see, charger type detection is done automatically by PMIC when VBUS
>> > is detected in your case, you just assume the process is complete
>> > before SW do gadget connect. To make the framework common, you may do
>> one time charger type check when vbus is on, and save it to avoid repeat
>> charger type check.
>>
>> OK. I'll add one judgement to check if the charger type is set in
>> 'usb_charger_detect_type()' function.
>
> Just adding a judgement isn't enough here, your framework should make sure
> usb_charger_detect_type() is called before gadget connect, with that, the
> existing caller place just gets the charger type from the saved value.
> The real charger type detection done by usb_charger_detect_type() can
> be called only when vbus is on.
> e.g. maybe in usb_udc_vbus_handler() before usb_udc_connect_control().

Yeah, Like Felipe suggested, I think we need to introduce one
'charger_detect()' method to do the SW charger type detection at the
right gadget state. Thanks for your comments.

>>
>> --
>> Baolin.wang
>> Best Regards



-- 
Baolin.wang
Best Regards

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

On 31 March 2016 at 14:12, Jun Li  wrote:
> Hi
>
>> -Original Message-
>> From: Baolin Wang [mailto:baolin.w...@linaro.org]
>> Sent: Thursday, March 31, 2016 1:23 PM
>> To: Jun Li 
>> Cc: Peter Chen ; Felipe Balbi ;
>> Greg KH ; Sebastian Reichel ;
>> Dmitry Eremin-Solenikov ; David Woodhouse
>> ; Peter Chen ; Alan Stern
>> ; r.bald...@samsung.com; Yoshihiro Shimoda
>> ; Lee Jones ; Mark
>> Brown ; Charles Keepax
>> ; patc...@opensource.wolfsonmicro.com;
>> Linux PM list ; USB ;
>> device-mainlin...@lists.linuxfoundation.org; LKML > ker...@vger.kernel.org>
>> Subject: Re: [PATCH v8 0/4] Introduce usb charger framework to deal with
>> the usb gadget power negotation
>>
>> On 30 March 2016 at 18:58, Jun Li  wrote:
>> >> >> >> > Seems you don't want to guarantee charger type detection is
>> >> >> >> > done before gadget connection(pullup DP), right?
>> >> >> >> > I see you call usb_charger_detect_type() in each gadget usb
>> >> >> >> > state
>> >> >> >> changes.
>> >> >> >>
>> >> >> >> I am not sure I get your point correctly, please correct me if
>> >> >> >> I misunderstand you.
>> >> >> >> We need to check the charger type at every event comes from the
>> >> >> >> usb gadget state changes or the extcon device state changes,
>> >> >> >> which means a new charger plugin or pullup.
>> >> >> >>
>> >> >> >
>> >> >> > According to usb charger spec, my understanding is you can't do
>> >> >> > real charger detection procedure *after* gadget
>> >> >> > _connection_(pullup DP), also I don't
>> >> >>
>> >> >> Why can not? Charger detection is usually from PMIC.
>> >> >
>> >> > Charger detection process will impact DP/DM line state, see usb
>> >> > charger spec v1.2 for detail detection process, section 4.6.3 says:
>> >> >
>> >> > "A PD is allowed to *disconnect* and repeat the charger detection
>> >> > process multiple times while attached. The PD is required to wait
>> >> > for a time of at least TCP_VDM_EN max between disconnecting and
>> >> > restarting the charger detection process."
>> >> >
>> >> > As Peter mentioned, the charger detection should happen between
>> >> > VBUS detection and gadget pull up DP for first plug in case. So
>> >> > when gadget connect (pullup DP), you should already know the
>> charger type.
>> >>
>> >> Make sense. In our company's solution, charger detection can be done
>> >> by hardware from PMIC at first, then it will not affect the DP/DM
>> >> line when gadget starts to enumeration.
>> >
>> > I see, charger type detection is done automatically by PMIC when VBUS
>> > is detected in your case, you just assume the process is complete
>> > before SW do gadget connect. To make the framework common, you may do
>> one time charger type check when vbus is on, and save it to avoid repeat
>> charger type check.
>>
>> OK. I'll add one judgement to check if the charger type is set in
>> 'usb_charger_detect_type()' function.
>
> Just adding a judgement isn't enough here, your framework should make sure
> usb_charger_detect_type() is called before gadget connect, with that, the
> existing caller place just gets the charger type from the saved value.
> The real charger type detection done by usb_charger_detect_type() can
> be called only when vbus is on.
> e.g. maybe in usb_udc_vbus_handler() before usb_udc_connect_control().

Yeah, Like Felipe suggested, I think we need to introduce one
'charger_detect()' method to do the SW charger type detection at the
right gadget state. Thanks for your comments.

>>
>> --
>> Baolin.wang
>> Best Regards



-- 
Baolin.wang
Best Regards

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

On 31 March 2016 at 14:18, Felipe Balbi  wrote:
>
> Hi,
>
> Baolin Wang  writes:
>> [ text/plain ]
>> On 30 March 2016 at 19:24, Felipe Balbi  wrote:
> >> >> >
> >> >> > Seems you don't want to guarantee charger type detection is done
> >> >> > before gadget connection(pullup DP), right?
> >> >> > I see you call usb_charger_detect_type() in each gadget usb
> >> >> > state
> >> >> changes.
> >> >>
> >> >> I am not sure I get your point correctly, please correct me if I
> >> >> misunderstand you.
> >> >> We need to check the charger type at every event comes from the
> >> >> usb gadget state changes or the extcon device state changes, which
> >> >> means a new charger plugin or pullup.
> >> >>
> >> >
> >> > According to usb charger spec, my understanding is you can't do
> >> > real charger detection procedure *after* gadget _connection_(pullup
> >> > DP), also I don't
> >>
> >> Why can not? Charger detection is usually from PMIC.
> >
> > Charger detection process will impact DP/DM line state, see usb
> > charger spec v1.2 for detail detection process, section 4.6.3 says:
> >
> > "A PD is allowed to *disconnect* and repeat the charger detection
> > process multiple times while attached. The PD is required to wait for
> > a time of at least TCP_VDM_EN max between disconnecting and restarting
> > the charger detection process."
> >
> > As Peter mentioned, the charger detection should happen between VBUS
> > detection and gadget pull up DP for first plug in case. So when
> > gadget connect (pullup DP), you should already know the charger type.
>
> Make sense. In our company's solution, charger detection can be done by
> hardware from PMIC at first, then it will not affect the DP/DM line when
> gadget starts to enumeration.

 I see, charger type detection is done automatically by PMIC when VBUS
 is detected in your case, you just assume the process is complete
>>>
>>> assuming this finishes before gadget starts is a bad idea. It would've
>>> been much more robust to delay usb_gadget_connect() until we KNOW
>>> charger detection has completed.
>>
>> It is hardware action to detect the charger type quickly. It actually
>> *gets* the charger type and does not means *detect* charger type in
>> 'usb_charger_detect_type()' function. Maybe I need to change the
>> function name as 'usb_charger_get_type()'.
>
> yes.
>
>> If some udc drivers want to detect charger type in
>> 'gadget->ops->get_charger_type()' callback, they should avoid
>> impacting DP/DM line state at the right gadget state. Thanks.
>
> they shouldn't detect is get_type(), the semantics doesn't work. If, at
> some point, we have to do SW detection of the charger, then a new
> ->charger_detect() method will have to be added.

Make sense.

>
> --
> balbi



-- 
Baolin.wang
Best Regards

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

On 31 March 2016 at 14:18, Felipe Balbi  wrote:
>
> Hi,
>
> Baolin Wang  writes:
>> [ text/plain ]
>> On 30 March 2016 at 19:24, Felipe Balbi  wrote:
> >> >> >
> >> >> > Seems you don't want to guarantee charger type detection is done
> >> >> > before gadget connection(pullup DP), right?
> >> >> > I see you call usb_charger_detect_type() in each gadget usb
> >> >> > state
> >> >> changes.
> >> >>
> >> >> I am not sure I get your point correctly, please correct me if I
> >> >> misunderstand you.
> >> >> We need to check the charger type at every event comes from the
> >> >> usb gadget state changes or the extcon device state changes, which
> >> >> means a new charger plugin or pullup.
> >> >>
> >> >
> >> > According to usb charger spec, my understanding is you can't do
> >> > real charger detection procedure *after* gadget _connection_(pullup
> >> > DP), also I don't
> >>
> >> Why can not? Charger detection is usually from PMIC.
> >
> > Charger detection process will impact DP/DM line state, see usb
> > charger spec v1.2 for detail detection process, section 4.6.3 says:
> >
> > "A PD is allowed to *disconnect* and repeat the charger detection
> > process multiple times while attached. The PD is required to wait for
> > a time of at least TCP_VDM_EN max between disconnecting and restarting
> > the charger detection process."
> >
> > As Peter mentioned, the charger detection should happen between VBUS
> > detection and gadget pull up DP for first plug in case. So when
> > gadget connect (pullup DP), you should already know the charger type.
>
> Make sense. In our company's solution, charger detection can be done by
> hardware from PMIC at first, then it will not affect the DP/DM line when
> gadget starts to enumeration.

 I see, charger type detection is done automatically by PMIC when VBUS
 is detected in your case, you just assume the process is complete
>>>
>>> assuming this finishes before gadget starts is a bad idea. It would've
>>> been much more robust to delay usb_gadget_connect() until we KNOW
>>> charger detection has completed.
>>
>> It is hardware action to detect the charger type quickly. It actually
>> *gets* the charger type and does not means *detect* charger type in
>> 'usb_charger_detect_type()' function. Maybe I need to change the
>> function name as 'usb_charger_get_type()'.
>
> yes.
>
>> If some udc drivers want to detect charger type in
>> 'gadget->ops->get_charger_type()' callback, they should avoid
>> impacting DP/DM line state at the right gadget state. Thanks.
>
> they shouldn't detect is get_type(), the semantics doesn't work. If, at
> some point, we have to do SW detection of the charger, then a new
> ->charger_detect() method will have to be added.

Make sense.

>
> --
> balbi



-- 
Baolin.wang
Best Regards

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Felipe Balbi]

Hi,

Baolin Wang  writes:
> [ text/plain ]
> On 30 March 2016 at 19:24, Felipe Balbi  wrote:
 >> >> >
 >> >> > Seems you don't want to guarantee charger type detection is done
 >> >> > before gadget connection(pullup DP), right?
 >> >> > I see you call usb_charger_detect_type() in each gadget usb
 >> >> > state
 >> >> changes.
 >> >>
 >> >> I am not sure I get your point correctly, please correct me if I
 >> >> misunderstand you.
 >> >> We need to check the charger type at every event comes from the
 >> >> usb gadget state changes or the extcon device state changes, which
 >> >> means a new charger plugin or pullup.
 >> >>
 >> >
 >> > According to usb charger spec, my understanding is you can't do
 >> > real charger detection procedure *after* gadget _connection_(pullup
 >> > DP), also I don't
 >>
 >> Why can not? Charger detection is usually from PMIC.
 >
 > Charger detection process will impact DP/DM line state, see usb
 > charger spec v1.2 for detail detection process, section 4.6.3 says:
 >
 > "A PD is allowed to *disconnect* and repeat the charger detection
 > process multiple times while attached. The PD is required to wait for
 > a time of at least TCP_VDM_EN max between disconnecting and restarting
 > the charger detection process."
 >
 > As Peter mentioned, the charger detection should happen between VBUS
 > detection and gadget pull up DP for first plug in case. So when
 > gadget connect (pullup DP), you should already know the charger type.

 Make sense. In our company's solution, charger detection can be done by
 hardware from PMIC at first, then it will not affect the DP/DM line when
 gadget starts to enumeration.
>>>
>>> I see, charger type detection is done automatically by PMIC when VBUS
>>> is detected in your case, you just assume the process is complete
>>
>> assuming this finishes before gadget starts is a bad idea. It would've
>> been much more robust to delay usb_gadget_connect() until we KNOW
>> charger detection has completed.
>
> It is hardware action to detect the charger type quickly. It actually
> *gets* the charger type and does not means *detect* charger type in
> 'usb_charger_detect_type()' function. Maybe I need to change the
> function name as 'usb_charger_get_type()'.

yes.

> If some udc drivers want to detect charger type in
> 'gadget->ops->get_charger_type()' callback, they should avoid
> impacting DP/DM line state at the right gadget state. Thanks.

they shouldn't detect is get_type(), the semantics doesn't work. If, at
some point, we have to do SW detection of the charger, then a new
->charger_detect() method will have to be added.

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Felipe Balbi]

Hi,

Baolin Wang  writes:
> [ text/plain ]
> On 30 March 2016 at 19:24, Felipe Balbi  wrote:
 >> >> >
 >> >> > Seems you don't want to guarantee charger type detection is done
 >> >> > before gadget connection(pullup DP), right?
 >> >> > I see you call usb_charger_detect_type() in each gadget usb
 >> >> > state
 >> >> changes.
 >> >>
 >> >> I am not sure I get your point correctly, please correct me if I
 >> >> misunderstand you.
 >> >> We need to check the charger type at every event comes from the
 >> >> usb gadget state changes or the extcon device state changes, which
 >> >> means a new charger plugin or pullup.
 >> >>
 >> >
 >> > According to usb charger spec, my understanding is you can't do
 >> > real charger detection procedure *after* gadget _connection_(pullup
 >> > DP), also I don't
 >>
 >> Why can not? Charger detection is usually from PMIC.
 >
 > Charger detection process will impact DP/DM line state, see usb
 > charger spec v1.2 for detail detection process, section 4.6.3 says:
 >
 > "A PD is allowed to *disconnect* and repeat the charger detection
 > process multiple times while attached. The PD is required to wait for
 > a time of at least TCP_VDM_EN max between disconnecting and restarting
 > the charger detection process."
 >
 > As Peter mentioned, the charger detection should happen between VBUS
 > detection and gadget pull up DP for first plug in case. So when
 > gadget connect (pullup DP), you should already know the charger type.

 Make sense. In our company's solution, charger detection can be done by
 hardware from PMIC at first, then it will not affect the DP/DM line when
 gadget starts to enumeration.
>>>
>>> I see, charger type detection is done automatically by PMIC when VBUS
>>> is detected in your case, you just assume the process is complete
>>
>> assuming this finishes before gadget starts is a bad idea. It would've
>> been much more robust to delay usb_gadget_connect() until we KNOW
>> charger detection has completed.
>
> It is hardware action to detect the charger type quickly. It actually
> *gets* the charger type and does not means *detect* charger type in
> 'usb_charger_detect_type()' function. Maybe I need to change the
> function name as 'usb_charger_get_type()'.

yes.

> If some udc drivers want to detect charger type in
> 'gadget->ops->get_charger_type()' callback, they should avoid
> impacting DP/DM line state at the right gadget state. Thanks.

they shouldn't detect is get_type(), the semantics doesn't work. If, at
some point, we have to do SW detection of the charger, then a new
->charger_detect() method will have to be added.

-- 
balbi


signature.asc
Description: PGP signature

RE: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Jun Li]

Hi

> -Original Message-
> From: Baolin Wang [mailto:baolin.w...@linaro.org]
> Sent: Thursday, March 31, 2016 1:23 PM
> To: Jun Li <jun...@nxp.com>
> Cc: Peter Chen <hzpeterc...@gmail.com>; Felipe Balbi <ba...@kernel.org>;
> Greg KH <gre...@linuxfoundation.org>; Sebastian Reichel <s...@kernel.org>;
> Dmitry Eremin-Solenikov <dbarysh...@gmail.com>; David Woodhouse
> <dw...@infradead.org>; Peter Chen <peter.c...@freescale.com>; Alan Stern
> <st...@rowland.harvard.edu>; r.bald...@samsung.com; Yoshihiro Shimoda
> <yoshihiro.shimoda...@renesas.com>; Lee Jones <lee.jo...@linaro.org>; Mark
> Brown <broo...@kernel.org>; Charles Keepax
> <ckee...@opensource.wolfsonmicro.com>; patc...@opensource.wolfsonmicro.com;
> Linux PM list <linux...@vger.kernel.org>; USB <linux-...@vger.kernel.org>;
> device-mainlin...@lists.linuxfoundation.org; LKML  ker...@vger.kernel.org>
> Subject: Re: [PATCH v8 0/4] Introduce usb charger framework to deal with
> the usb gadget power negotation
> 
> On 30 March 2016 at 18:58, Jun Li <jun...@nxp.com> wrote:
> >> >> >> > Seems you don't want to guarantee charger type detection is
> >> >> >> > done before gadget connection(pullup DP), right?
> >> >> >> > I see you call usb_charger_detect_type() in each gadget usb
> >> >> >> > state
> >> >> >> changes.
> >> >> >>
> >> >> >> I am not sure I get your point correctly, please correct me if
> >> >> >> I misunderstand you.
> >> >> >> We need to check the charger type at every event comes from the
> >> >> >> usb gadget state changes or the extcon device state changes,
> >> >> >> which means a new charger plugin or pullup.
> >> >> >>
> >> >> >
> >> >> > According to usb charger spec, my understanding is you can't do
> >> >> > real charger detection procedure *after* gadget
> >> >> > _connection_(pullup DP), also I don't
> >> >>
> >> >> Why can not? Charger detection is usually from PMIC.
> >> >
> >> > Charger detection process will impact DP/DM line state, see usb
> >> > charger spec v1.2 for detail detection process, section 4.6.3 says:
> >> >
> >> > "A PD is allowed to *disconnect* and repeat the charger detection
> >> > process multiple times while attached. The PD is required to wait
> >> > for a time of at least TCP_VDM_EN max between disconnecting and
> >> > restarting the charger detection process."
> >> >
> >> > As Peter mentioned, the charger detection should happen between
> >> > VBUS detection and gadget pull up DP for first plug in case. So
> >> > when gadget connect (pullup DP), you should already know the
> charger type.
> >>
> >> Make sense. In our company's solution, charger detection can be done
> >> by hardware from PMIC at first, then it will not affect the DP/DM
> >> line when gadget starts to enumeration.
> >
> > I see, charger type detection is done automatically by PMIC when VBUS
> > is detected in your case, you just assume the process is complete
> > before SW do gadget connect. To make the framework common, you may do
> one time charger type check when vbus is on, and save it to avoid repeat
> charger type check.
> 
> OK. I'll add one judgement to check if the charger type is set in
> 'usb_charger_detect_type()' function.

Just adding a judgement isn't enough here, your framework should make sure
usb_charger_detect_type() is called before gadget connect, with that, the
existing caller place just gets the charger type from the saved value.
The real charger type detection done by usb_charger_detect_type() can
be called only when vbus is on.
e.g. maybe in usb_udc_vbus_handler() before usb_udc_connect_control().

> 
> --
> Baolin.wang
> Best Regards

RE: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Jun Li]

Hi

> -Original Message-
> From: Baolin Wang [mailto:baolin.w...@linaro.org]
> Sent: Thursday, March 31, 2016 1:23 PM
> To: Jun Li 
> Cc: Peter Chen ; Felipe Balbi ;
> Greg KH ; Sebastian Reichel ;
> Dmitry Eremin-Solenikov ; David Woodhouse
> ; Peter Chen ; Alan Stern
> ; r.bald...@samsung.com; Yoshihiro Shimoda
> ; Lee Jones ; Mark
> Brown ; Charles Keepax
> ; patc...@opensource.wolfsonmicro.com;
> Linux PM list ; USB ;
> device-mainlin...@lists.linuxfoundation.org; LKML  ker...@vger.kernel.org>
> Subject: Re: [PATCH v8 0/4] Introduce usb charger framework to deal with
> the usb gadget power negotation
> 
> On 30 March 2016 at 18:58, Jun Li  wrote:
> >> >> >> > Seems you don't want to guarantee charger type detection is
> >> >> >> > done before gadget connection(pullup DP), right?
> >> >> >> > I see you call usb_charger_detect_type() in each gadget usb
> >> >> >> > state
> >> >> >> changes.
> >> >> >>
> >> >> >> I am not sure I get your point correctly, please correct me if
> >> >> >> I misunderstand you.
> >> >> >> We need to check the charger type at every event comes from the
> >> >> >> usb gadget state changes or the extcon device state changes,
> >> >> >> which means a new charger plugin or pullup.
> >> >> >>
> >> >> >
> >> >> > According to usb charger spec, my understanding is you can't do
> >> >> > real charger detection procedure *after* gadget
> >> >> > _connection_(pullup DP), also I don't
> >> >>
> >> >> Why can not? Charger detection is usually from PMIC.
> >> >
> >> > Charger detection process will impact DP/DM line state, see usb
> >> > charger spec v1.2 for detail detection process, section 4.6.3 says:
> >> >
> >> > "A PD is allowed to *disconnect* and repeat the charger detection
> >> > process multiple times while attached. The PD is required to wait
> >> > for a time of at least TCP_VDM_EN max between disconnecting and
> >> > restarting the charger detection process."
> >> >
> >> > As Peter mentioned, the charger detection should happen between
> >> > VBUS detection and gadget pull up DP for first plug in case. So
> >> > when gadget connect (pullup DP), you should already know the
> charger type.
> >>
> >> Make sense. In our company's solution, charger detection can be done
> >> by hardware from PMIC at first, then it will not affect the DP/DM
> >> line when gadget starts to enumeration.
> >
> > I see, charger type detection is done automatically by PMIC when VBUS
> > is detected in your case, you just assume the process is complete
> > before SW do gadget connect. To make the framework common, you may do
> one time charger type check when vbus is on, and save it to avoid repeat
> charger type check.
> 
> OK. I'll add one judgement to check if the charger type is set in
> 'usb_charger_detect_type()' function.

Just adding a judgement isn't enough here, your framework should make sure
usb_charger_detect_type() is called before gadget connect, with that, the
existing caller place just gets the charger type from the saved value.
The real charger type detection done by usb_charger_detect_type() can
be called only when vbus is on.
e.g. maybe in usb_udc_vbus_handler() before usb_udc_connect_control().

> 
> --
> Baolin.wang
> Best Regards

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

On 30 March 2016 at 19:24, Felipe Balbi  wrote:
>>> >> >> >
>>> >> >> > Seems you don't want to guarantee charger type detection is done
>>> >> >> > before gadget connection(pullup DP), right?
>>> >> >> > I see you call usb_charger_detect_type() in each gadget usb
>>> >> >> > state
>>> >> >> changes.
>>> >> >>
>>> >> >> I am not sure I get your point correctly, please correct me if I
>>> >> >> misunderstand you.
>>> >> >> We need to check the charger type at every event comes from the
>>> >> >> usb gadget state changes or the extcon device state changes, which
>>> >> >> means a new charger plugin or pullup.
>>> >> >>
>>> >> >
>>> >> > According to usb charger spec, my understanding is you can't do
>>> >> > real charger detection procedure *after* gadget _connection_(pullup
>>> >> > DP), also I don't
>>> >>
>>> >> Why can not? Charger detection is usually from PMIC.
>>> >
>>> > Charger detection process will impact DP/DM line state, see usb
>>> > charger spec v1.2 for detail detection process, section 4.6.3 says:
>>> >
>>> > "A PD is allowed to *disconnect* and repeat the charger detection
>>> > process multiple times while attached. The PD is required to wait for
>>> > a time of at least TCP_VDM_EN max between disconnecting and restarting
>>> > the charger detection process."
>>> >
>>> > As Peter mentioned, the charger detection should happen between VBUS
>>> > detection and gadget pull up DP for first plug in case. So when
>>> > gadget connect (pullup DP), you should already know the charger type.
>>>
>>> Make sense. In our company's solution, charger detection can be done by
>>> hardware from PMIC at first, then it will not affect the DP/DM line when
>>> gadget starts to enumeration.
>>
>> I see, charger type detection is done automatically by PMIC when VBUS
>> is detected in your case, you just assume the process is complete
>
> assuming this finishes before gadget starts is a bad idea. It would've
> been much more robust to delay usb_gadget_connect() until we KNOW
> charger detection has completed.

It is hardware action to detect the charger type quickly. It actually
*gets* the charger type and does not means *detect* charger type in
'usb_charger_detect_type()' function. Maybe I need to change the
function name as 'usb_charger_get_type()'.

If some udc drivers want to detect charger type in
'gadget->ops->get_charger_type()' callback, they should avoid
impacting DP/DM line state at the right gadget state. Thanks.

>
> --
> balbi



-- 
Baolin.wang
Best Regards

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

On 30 March 2016 at 19:24, Felipe Balbi  wrote:
>>> >> >> >
>>> >> >> > Seems you don't want to guarantee charger type detection is done
>>> >> >> > before gadget connection(pullup DP), right?
>>> >> >> > I see you call usb_charger_detect_type() in each gadget usb
>>> >> >> > state
>>> >> >> changes.
>>> >> >>
>>> >> >> I am not sure I get your point correctly, please correct me if I
>>> >> >> misunderstand you.
>>> >> >> We need to check the charger type at every event comes from the
>>> >> >> usb gadget state changes or the extcon device state changes, which
>>> >> >> means a new charger plugin or pullup.
>>> >> >>
>>> >> >
>>> >> > According to usb charger spec, my understanding is you can't do
>>> >> > real charger detection procedure *after* gadget _connection_(pullup
>>> >> > DP), also I don't
>>> >>
>>> >> Why can not? Charger detection is usually from PMIC.
>>> >
>>> > Charger detection process will impact DP/DM line state, see usb
>>> > charger spec v1.2 for detail detection process, section 4.6.3 says:
>>> >
>>> > "A PD is allowed to *disconnect* and repeat the charger detection
>>> > process multiple times while attached. The PD is required to wait for
>>> > a time of at least TCP_VDM_EN max between disconnecting and restarting
>>> > the charger detection process."
>>> >
>>> > As Peter mentioned, the charger detection should happen between VBUS
>>> > detection and gadget pull up DP for first plug in case. So when
>>> > gadget connect (pullup DP), you should already know the charger type.
>>>
>>> Make sense. In our company's solution, charger detection can be done by
>>> hardware from PMIC at first, then it will not affect the DP/DM line when
>>> gadget starts to enumeration.
>>
>> I see, charger type detection is done automatically by PMIC when VBUS
>> is detected in your case, you just assume the process is complete
>
> assuming this finishes before gadget starts is a bad idea. It would've
> been much more robust to delay usb_gadget_connect() until we KNOW
> charger detection has completed.

It is hardware action to detect the charger type quickly. It actually
*gets* the charger type and does not means *detect* charger type in
'usb_charger_detect_type()' function. Maybe I need to change the
function name as 'usb_charger_get_type()'.

If some udc drivers want to detect charger type in
'gadget->ops->get_charger_type()' callback, they should avoid
impacting DP/DM line state at the right gadget state. Thanks.

>
> --
> balbi



-- 
Baolin.wang
Best Regards

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

On 30 March 2016 at 18:58, Jun Li  wrote:
>> >> >> > Seems you don't want to guarantee charger type detection is done
>> >> >> > before gadget connection(pullup DP), right?
>> >> >> > I see you call usb_charger_detect_type() in each gadget usb
>> >> >> > state
>> >> >> changes.
>> >> >>
>> >> >> I am not sure I get your point correctly, please correct me if I
>> >> >> misunderstand you.
>> >> >> We need to check the charger type at every event comes from the
>> >> >> usb gadget state changes or the extcon device state changes, which
>> >> >> means a new charger plugin or pullup.
>> >> >>
>> >> >
>> >> > According to usb charger spec, my understanding is you can't do
>> >> > real charger detection procedure *after* gadget _connection_(pullup
>> >> > DP), also I don't
>> >>
>> >> Why can not? Charger detection is usually from PMIC.
>> >
>> > Charger detection process will impact DP/DM line state, see usb
>> > charger spec v1.2 for detail detection process, section 4.6.3 says:
>> >
>> > "A PD is allowed to *disconnect* and repeat the charger detection
>> > process multiple times while attached. The PD is required to wait for
>> > a time of at least TCP_VDM_EN max between disconnecting and restarting
>> > the charger detection process."
>> >
>> > As Peter mentioned, the charger detection should happen between VBUS
>> > detection and gadget pull up DP for first plug in case. So when
>> > gadget connect (pullup DP), you should already know the charger type.
>>
>> Make sense. In our company's solution, charger detection can be done by
>> hardware from PMIC at first, then it will not affect the DP/DM line when
>> gadget starts to enumeration.
>
> I see, charger type detection is done automatically by PMIC when VBUS is
> detected in your case, you just assume the process is complete before SW
> do gadget connect. To make the framework common, you may do one time charger 
> type check when vbus is on, and save it to avoid repeat charger type check.

OK. I'll add one judgement to check if the charger type is set in
'usb_charger_detect_type()' function.

-- 
Baolin.wang
Best Regards

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

On 30 March 2016 at 18:58, Jun Li  wrote:
>> >> >> > Seems you don't want to guarantee charger type detection is done
>> >> >> > before gadget connection(pullup DP), right?
>> >> >> > I see you call usb_charger_detect_type() in each gadget usb
>> >> >> > state
>> >> >> changes.
>> >> >>
>> >> >> I am not sure I get your point correctly, please correct me if I
>> >> >> misunderstand you.
>> >> >> We need to check the charger type at every event comes from the
>> >> >> usb gadget state changes or the extcon device state changes, which
>> >> >> means a new charger plugin or pullup.
>> >> >>
>> >> >
>> >> > According to usb charger spec, my understanding is you can't do
>> >> > real charger detection procedure *after* gadget _connection_(pullup
>> >> > DP), also I don't
>> >>
>> >> Why can not? Charger detection is usually from PMIC.
>> >
>> > Charger detection process will impact DP/DM line state, see usb
>> > charger spec v1.2 for detail detection process, section 4.6.3 says:
>> >
>> > "A PD is allowed to *disconnect* and repeat the charger detection
>> > process multiple times while attached. The PD is required to wait for
>> > a time of at least TCP_VDM_EN max between disconnecting and restarting
>> > the charger detection process."
>> >
>> > As Peter mentioned, the charger detection should happen between VBUS
>> > detection and gadget pull up DP for first plug in case. So when
>> > gadget connect (pullup DP), you should already know the charger type.
>>
>> Make sense. In our company's solution, charger detection can be done by
>> hardware from PMIC at first, then it will not affect the DP/DM line when
>> gadget starts to enumeration.
>
> I see, charger type detection is done automatically by PMIC when VBUS is
> detected in your case, you just assume the process is complete before SW
> do gadget connect. To make the framework common, you may do one time charger 
> type check when vbus is on, and save it to avoid repeat charger type check.

OK. I'll add one judgement to check if the charger type is set in
'usb_charger_detect_type()' function.

-- 
Baolin.wang
Best Regards

RE: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Felipe Balbi]

Hi,

Jun Li <jun...@nxp.com> writes:
>> -Original Message-
>> From: Baolin Wang [mailto:baolin.w...@linaro.org]
>> Sent: Wednesday, March 30, 2016 5:31 PM
>> To: Jun Li <jun...@nxp.com>
>> Cc: Peter Chen <hzpeterc...@gmail.com>; Felipe Balbi <ba...@kernel.org>;
>> Greg KH <gre...@linuxfoundation.org>; Sebastian Reichel <s...@kernel.org>;
>> Dmitry Eremin-Solenikov <dbarysh...@gmail.com>; David Woodhouse
>> <dw...@infradead.org>; Peter Chen <peter.c...@freescale.com>; Alan Stern
>> <st...@rowland.harvard.edu>; r.bald...@samsung.com; Yoshihiro Shimoda
>> <yoshihiro.shimoda...@renesas.com>; Lee Jones <lee.jo...@linaro.org>; Mark
>> Brown <broo...@kernel.org>; Charles Keepax
>> <ckee...@opensource.wolfsonmicro.com>; patc...@opensource.wolfsonmicro.com;
>> Linux PM list <linux...@vger.kernel.org>; USB <linux-...@vger.kernel.org>;
>> device-mainlin...@lists.linuxfoundation.org; LKML > ker...@vger.kernel.org>
>> Subject: Re: [PATCH v8 0/4] Introduce usb charger framework to deal with
>> the usb gadget power negotation
>> 
>> On 30 March 2016 at 16:07, Jun Li <jun...@nxp.com> wrote:
>> > Hi
>> 
>> >> On 30 March 2016 at 10:54, Jun Li <jun...@nxp.com> wrote:
>> >> >> >> It is not for udc driver but for power users who want to
>> >> >> >> negotiate with USB subsystem.
>> >> >> >>
>> >> >> >
>> >> >> > Seems you don't want to guarantee charger type detection is done
>> >> >> > before gadget connection(pullup DP), right?
>> >> >> > I see you call usb_charger_detect_type() in each gadget usb
>> >> >> > state
>> >> >> changes.
>> >> >>
>> >> >> I am not sure I get your point correctly, please correct me if I
>> >> >> misunderstand you.
>> >> >> We need to check the charger type at every event comes from the
>> >> >> usb gadget state changes or the extcon device state changes, which
>> >> >> means a new charger plugin or pullup.
>> >> >>
>> >> >
>> >> > According to usb charger spec, my understanding is you can't do
>> >> > real charger detection procedure *after* gadget _connection_(pullup
>> >> > DP), also I don't
>> >>
>> >> Why can not? Charger detection is usually from PMIC.
>> >
>> > Charger detection process will impact DP/DM line state, see usb
>> > charger spec v1.2 for detail detection process, section 4.6.3 says:
>> >
>> > "A PD is allowed to *disconnect* and repeat the charger detection
>> > process multiple times while attached. The PD is required to wait for
>> > a time of at least TCP_VDM_EN max between disconnecting and restarting
>> > the charger detection process."
>> >
>> > As Peter mentioned, the charger detection should happen between VBUS
>> > detection and gadget pull up DP for first plug in case. So when
>> > gadget connect (pullup DP), you should already know the charger type.
>> 
>> Make sense. In our company's solution, charger detection can be done by
>> hardware from PMIC at first, then it will not affect the DP/DM line when
>> gadget starts to enumeration. 
>
> I see, charger type detection is done automatically by PMIC when VBUS
> is detected in your case, you just assume the process is complete

assuming this finishes before gadget starts is a bad idea. It would've
been much more robust to delay usb_gadget_connect() until we KNOW
charger detection has completed.

-- 
balbi


signature.asc
Description: PGP signature

RE: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Felipe Balbi]

Hi,

Jun Li  writes:
>> -Original Message-
>> From: Baolin Wang [mailto:baolin.w...@linaro.org]
>> Sent: Wednesday, March 30, 2016 5:31 PM
>> To: Jun Li 
>> Cc: Peter Chen ; Felipe Balbi ;
>> Greg KH ; Sebastian Reichel ;
>> Dmitry Eremin-Solenikov ; David Woodhouse
>> ; Peter Chen ; Alan Stern
>> ; r.bald...@samsung.com; Yoshihiro Shimoda
>> ; Lee Jones ; Mark
>> Brown ; Charles Keepax
>> ; patc...@opensource.wolfsonmicro.com;
>> Linux PM list ; USB ;
>> device-mainlin...@lists.linuxfoundation.org; LKML > ker...@vger.kernel.org>
>> Subject: Re: [PATCH v8 0/4] Introduce usb charger framework to deal with
>> the usb gadget power negotation
>> 
>> On 30 March 2016 at 16:07, Jun Li  wrote:
>> > Hi
>> 
>> >> On 30 March 2016 at 10:54, Jun Li  wrote:
>> >> >> >> It is not for udc driver but for power users who want to
>> >> >> >> negotiate with USB subsystem.
>> >> >> >>
>> >> >> >
>> >> >> > Seems you don't want to guarantee charger type detection is done
>> >> >> > before gadget connection(pullup DP), right?
>> >> >> > I see you call usb_charger_detect_type() in each gadget usb
>> >> >> > state
>> >> >> changes.
>> >> >>
>> >> >> I am not sure I get your point correctly, please correct me if I
>> >> >> misunderstand you.
>> >> >> We need to check the charger type at every event comes from the
>> >> >> usb gadget state changes or the extcon device state changes, which
>> >> >> means a new charger plugin or pullup.
>> >> >>
>> >> >
>> >> > According to usb charger spec, my understanding is you can't do
>> >> > real charger detection procedure *after* gadget _connection_(pullup
>> >> > DP), also I don't
>> >>
>> >> Why can not? Charger detection is usually from PMIC.
>> >
>> > Charger detection process will impact DP/DM line state, see usb
>> > charger spec v1.2 for detail detection process, section 4.6.3 says:
>> >
>> > "A PD is allowed to *disconnect* and repeat the charger detection
>> > process multiple times while attached. The PD is required to wait for
>> > a time of at least TCP_VDM_EN max between disconnecting and restarting
>> > the charger detection process."
>> >
>> > As Peter mentioned, the charger detection should happen between VBUS
>> > detection and gadget pull up DP for first plug in case. So when
>> > gadget connect (pullup DP), you should already know the charger type.
>> 
>> Make sense. In our company's solution, charger detection can be done by
>> hardware from PMIC at first, then it will not affect the DP/DM line when
>> gadget starts to enumeration. 
>
> I see, charger type detection is done automatically by PMIC when VBUS
> is detected in your case, you just assume the process is complete

assuming this finishes before gadget starts is a bad idea. It would've
been much more robust to delay usb_gadget_connect() until we KNOW
charger detection has completed.

-- 
balbi


signature.asc
Description: PGP signature

RE: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Jun Li]

> -Original Message-
> From: Baolin Wang [mailto:baolin.w...@linaro.org]
> Sent: Wednesday, March 30, 2016 5:31 PM
> To: Jun Li <jun...@nxp.com>
> Cc: Peter Chen <hzpeterc...@gmail.com>; Felipe Balbi <ba...@kernel.org>;
> Greg KH <gre...@linuxfoundation.org>; Sebastian Reichel <s...@kernel.org>;
> Dmitry Eremin-Solenikov <dbarysh...@gmail.com>; David Woodhouse
> <dw...@infradead.org>; Peter Chen <peter.c...@freescale.com>; Alan Stern
> <st...@rowland.harvard.edu>; r.bald...@samsung.com; Yoshihiro Shimoda
> <yoshihiro.shimoda...@renesas.com>; Lee Jones <lee.jo...@linaro.org>; Mark
> Brown <broo...@kernel.org>; Charles Keepax
> <ckee...@opensource.wolfsonmicro.com>; patc...@opensource.wolfsonmicro.com;
> Linux PM list <linux...@vger.kernel.org>; USB <linux-...@vger.kernel.org>;
> device-mainlin...@lists.linuxfoundation.org; LKML  ker...@vger.kernel.org>
> Subject: Re: [PATCH v8 0/4] Introduce usb charger framework to deal with
> the usb gadget power negotation
> 
> On 30 March 2016 at 16:07, Jun Li <jun...@nxp.com> wrote:
> > Hi
> 
> >> On 30 March 2016 at 10:54, Jun Li <jun...@nxp.com> wrote:
> >> >> >> It is not for udc driver but for power users who want to
> >> >> >> negotiate with USB subsystem.
> >> >> >>
> >> >> >
> >> >> > Seems you don't want to guarantee charger type detection is done
> >> >> > before gadget connection(pullup DP), right?
> >> >> > I see you call usb_charger_detect_type() in each gadget usb
> >> >> > state
> >> >> changes.
> >> >>
> >> >> I am not sure I get your point correctly, please correct me if I
> >> >> misunderstand you.
> >> >> We need to check the charger type at every event comes from the
> >> >> usb gadget state changes or the extcon device state changes, which
> >> >> means a new charger plugin or pullup.
> >> >>
> >> >
> >> > According to usb charger spec, my understanding is you can't do
> >> > real charger detection procedure *after* gadget _connection_(pullup
> >> > DP), also I don't
> >>
> >> Why can not? Charger detection is usually from PMIC.
> >
> > Charger detection process will impact DP/DM line state, see usb
> > charger spec v1.2 for detail detection process, section 4.6.3 says:
> >
> > "A PD is allowed to *disconnect* and repeat the charger detection
> > process multiple times while attached. The PD is required to wait for
> > a time of at least TCP_VDM_EN max between disconnecting and restarting
> > the charger detection process."
> >
> > As Peter mentioned, the charger detection should happen between VBUS
> > detection and gadget pull up DP for first plug in case. So when
> > gadget connect (pullup DP), you should already know the charger type.
> 
> Make sense. In our company's solution, charger detection can be done by
> hardware from PMIC at first, then it will not affect the DP/DM line when
> gadget starts to enumeration. 

I see, charger type detection is done automatically by PMIC when VBUS is
detected in your case, you just assume the process is complete before SW
do gadget connect. To make the framework common, you may do one time charger 
type check when vbus is on, and save it to avoid repeat charger type check.

> In the 'usb_charger_detect_type()', it
> usually get the charger type from type registers has been done by hardware
> from PMIC, which can not affect the DP/DM line.
> 
> >
> > Li Jun
> >
> >>
> >> > think it's necessary to check charger type at every event from usb
> >> gadget.
> >>
> >> My meaning is not every event from usb gadget. When the usb gadget
> >> state changes or the extcon device (maybe GPIO detection) state
> >> changes, which means charger plugin or pullup, we need to check the
> >> charger type to set current.
> >
> > From your below code, you call usb_charger_notify_others() in every
> > state change.
> 
> I think it does not matter. In case the usb charger missed some gadget
> state changes. Or I replace it with 'USB_STATE_CONFIGURED' state.
> Thanks.
> 
> >
> > if (uchger->old_gadget_state != state) {
> > uchger->old_gadget_state = state;
> >
> > if (state >= USB_STATE_ATTACHED)
> > uchger_state = USB_CHARGER_PRESENT;
> > else if (state == USB_STATE_NOTATTACHED)
> > uchger_state = USB_CHARGER_REMOVE;
> > else
> > /* this else will never happen */
> > uchger_state = USB_CHARGER_DEFAULT;
> >
> > usb_charger_notify_others(uchger, uchger_state); }
> >
> >>
> >> > Something in gadget driver you can utilize is only vbus detection,
> >> > and report diff current by diff usb state if it's a SDP.
> >>
> >> --
> >> Baolin.wang
> >> Best Regards
> 
> 
> 
> --
> Baolin.wang
> Best Regards

RE: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Jun Li]

> -Original Message-
> From: Baolin Wang [mailto:baolin.w...@linaro.org]
> Sent: Wednesday, March 30, 2016 5:31 PM
> To: Jun Li 
> Cc: Peter Chen ; Felipe Balbi ;
> Greg KH ; Sebastian Reichel ;
> Dmitry Eremin-Solenikov ; David Woodhouse
> ; Peter Chen ; Alan Stern
> ; r.bald...@samsung.com; Yoshihiro Shimoda
> ; Lee Jones ; Mark
> Brown ; Charles Keepax
> ; patc...@opensource.wolfsonmicro.com;
> Linux PM list ; USB ;
> device-mainlin...@lists.linuxfoundation.org; LKML  ker...@vger.kernel.org>
> Subject: Re: [PATCH v8 0/4] Introduce usb charger framework to deal with
> the usb gadget power negotation
> 
> On 30 March 2016 at 16:07, Jun Li  wrote:
> > Hi
> 
> >> On 30 March 2016 at 10:54, Jun Li  wrote:
> >> >> >> It is not for udc driver but for power users who want to
> >> >> >> negotiate with USB subsystem.
> >> >> >>
> >> >> >
> >> >> > Seems you don't want to guarantee charger type detection is done
> >> >> > before gadget connection(pullup DP), right?
> >> >> > I see you call usb_charger_detect_type() in each gadget usb
> >> >> > state
> >> >> changes.
> >> >>
> >> >> I am not sure I get your point correctly, please correct me if I
> >> >> misunderstand you.
> >> >> We need to check the charger type at every event comes from the
> >> >> usb gadget state changes or the extcon device state changes, which
> >> >> means a new charger plugin or pullup.
> >> >>
> >> >
> >> > According to usb charger spec, my understanding is you can't do
> >> > real charger detection procedure *after* gadget _connection_(pullup
> >> > DP), also I don't
> >>
> >> Why can not? Charger detection is usually from PMIC.
> >
> > Charger detection process will impact DP/DM line state, see usb
> > charger spec v1.2 for detail detection process, section 4.6.3 says:
> >
> > "A PD is allowed to *disconnect* and repeat the charger detection
> > process multiple times while attached. The PD is required to wait for
> > a time of at least TCP_VDM_EN max between disconnecting and restarting
> > the charger detection process."
> >
> > As Peter mentioned, the charger detection should happen between VBUS
> > detection and gadget pull up DP for first plug in case. So when
> > gadget connect (pullup DP), you should already know the charger type.
> 
> Make sense. In our company's solution, charger detection can be done by
> hardware from PMIC at first, then it will not affect the DP/DM line when
> gadget starts to enumeration. 

I see, charger type detection is done automatically by PMIC when VBUS is
detected in your case, you just assume the process is complete before SW
do gadget connect. To make the framework common, you may do one time charger 
type check when vbus is on, and save it to avoid repeat charger type check.

> In the 'usb_charger_detect_type()', it
> usually get the charger type from type registers has been done by hardware
> from PMIC, which can not affect the DP/DM line.
> 
> >
> > Li Jun
> >
> >>
> >> > think it's necessary to check charger type at every event from usb
> >> gadget.
> >>
> >> My meaning is not every event from usb gadget. When the usb gadget
> >> state changes or the extcon device (maybe GPIO detection) state
> >> changes, which means charger plugin or pullup, we need to check the
> >> charger type to set current.
> >
> > From your below code, you call usb_charger_notify_others() in every
> > state change.
> 
> I think it does not matter. In case the usb charger missed some gadget
> state changes. Or I replace it with 'USB_STATE_CONFIGURED' state.
> Thanks.
> 
> >
> > if (uchger->old_gadget_state != state) {
> > uchger->old_gadget_state = state;
> >
> > if (state >= USB_STATE_ATTACHED)
> > uchger_state = USB_CHARGER_PRESENT;
> > else if (state == USB_STATE_NOTATTACHED)
> > uchger_state = USB_CHARGER_REMOVE;
> > else
> > /* this else will never happen */
> > uchger_state = USB_CHARGER_DEFAULT;
> >
> > usb_charger_notify_others(uchger, uchger_state); }
> >
> >>
> >> > Something in gadget driver you can utilize is only vbus detection,
> >> > and report diff current by diff usb state if it's a SDP.
> >>
> >> --
> >> Baolin.wang
> >> Best Regards
> 
> 
> 
> --
> Baolin.wang
> Best Regards

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

On 30 March 2016 at 17:19, Peter Chen  wrote:
> On Wed, Mar 30, 2016 at 04:40:31PM +0800, Baolin Wang wrote:
>> >> > - Third, since composite driver covers 500mA (and more for CDP) after 
>> >> > set
>> >> > configuration and 2mA after suspend, and vbus handler covers connect
>> >> > and disconnect. I can't see any reasons we need to notify gadget state
>> >> > for power driver, do we really need to have usb_charger_plug_by_gadget?
>> >>
>> >> In some solutions, gadget does not negotiate with the current. They
>> >> just send out one signal to power driver to set the current when the
>> >> gadget state is changed (plugin or not). So we need to check the
>> >> charger state by the gadget state to notify the charger IC to set
>> >> current.
>> >>
>> >
>> > Would you give some examples?
>>
>> OK. I explain it in detail. Now charger detection can be from gadget
>> itself or PMIC, and we focus on gadget detection. Charger IC (charger
>> driver) is separate with gadget.
>> When the usb cable is plugin, we need to report the plugin event to
>> charger driver to set current after setting configuration for gadget.
>> The usb charger is responsible for reporting plugin event to charger
>> driver. But how usb charger get the plugin event? It can get the
>> plugin event from gadget state (if the gadget state is more than
>> 'USB_STATE_ATTACHED', it means one cable plugin). So we need notify
>> gadget state to usb charger.
>>
>
> Ok, I see, it only changes current limit at function usb_gadget_vbus_draw
> in your patch 2/4. Then, we need to make sure 
> usb_charger_set_cur_limit_by_type
> is called before calling usb_gadget_set_state(gadget, USB_STATE_CONFIGURED).

That's right.

>
> It seems you have not implemented usb_charger_plug_by_gadget in your patch 
> set.

It is implemented in patch 3/4.

>
> --
> Best Regards,
> Peter Chen



-- 
Baolin.wang
Best Regards

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

On 30 March 2016 at 17:19, Peter Chen  wrote:
> On Wed, Mar 30, 2016 at 04:40:31PM +0800, Baolin Wang wrote:
>> >> > - Third, since composite driver covers 500mA (and more for CDP) after 
>> >> > set
>> >> > configuration and 2mA after suspend, and vbus handler covers connect
>> >> > and disconnect. I can't see any reasons we need to notify gadget state
>> >> > for power driver, do we really need to have usb_charger_plug_by_gadget?
>> >>
>> >> In some solutions, gadget does not negotiate with the current. They
>> >> just send out one signal to power driver to set the current when the
>> >> gadget state is changed (plugin or not). So we need to check the
>> >> charger state by the gadget state to notify the charger IC to set
>> >> current.
>> >>
>> >
>> > Would you give some examples?
>>
>> OK. I explain it in detail. Now charger detection can be from gadget
>> itself or PMIC, and we focus on gadget detection. Charger IC (charger
>> driver) is separate with gadget.
>> When the usb cable is plugin, we need to report the plugin event to
>> charger driver to set current after setting configuration for gadget.
>> The usb charger is responsible for reporting plugin event to charger
>> driver. But how usb charger get the plugin event? It can get the
>> plugin event from gadget state (if the gadget state is more than
>> 'USB_STATE_ATTACHED', it means one cable plugin). So we need notify
>> gadget state to usb charger.
>>
>
> Ok, I see, it only changes current limit at function usb_gadget_vbus_draw
> in your patch 2/4. Then, we need to make sure 
> usb_charger_set_cur_limit_by_type
> is called before calling usb_gadget_set_state(gadget, USB_STATE_CONFIGURED).

That's right.

>
> It seems you have not implemented usb_charger_plug_by_gadget in your patch 
> set.

It is implemented in patch 3/4.

>
> --
> Best Regards,
> Peter Chen



-- 
Baolin.wang
Best Regards

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

On 30 March 2016 at 16:07, Jun Li  wrote:
> Hi

>> On 30 March 2016 at 10:54, Jun Li  wrote:
>> >> >> It is not for udc driver but for power users who want to negotiate
>> >> >> with USB subsystem.
>> >> >>
>> >> >
>> >> > Seems you don't want to guarantee charger type detection is done
>> >> > before gadget connection(pullup DP), right?
>> >> > I see you call usb_charger_detect_type() in each gadget usb state
>> >> changes.
>> >>
>> >> I am not sure I get your point correctly, please correct me if I
>> >> misunderstand you.
>> >> We need to check the charger type at every event comes from the usb
>> >> gadget state changes or the extcon device state changes, which means
>> >> a new charger plugin or pullup.
>> >>
>> >
>> > According to usb charger spec, my understanding is you can't do real
>> > charger detection procedure *after* gadget _connection_(pullup DP),
>> > also I don't
>>
>> Why can not? Charger detection is usually from PMIC.
>
> Charger detection process will impact DP/DM line state, see usb charger
> spec v1.2 for detail detection process, section 4.6.3 says:
>
> "A PD is allowed to *disconnect* and repeat the charger detection process
> multiple times while attached. The PD is required to wait for a time of at
> least TCP_VDM_EN max between disconnecting and restarting the charger
> detection process."
>
> As Peter mentioned, the charger detection should happen between VBUS
> detection and gadget pull up DP for first plug in case. So when
> gadget connect (pullup DP), you should already know the charger type.

Make sense. In our company's solution, charger detection can be done
by hardware from PMIC at first, then it will not affect the DP/DM line
when gadget starts to enumeration. In the 'usb_charger_detect_type()',
it usually get the charger type from type registers has been done by
hardware from PMIC, which can not affect the DP/DM line.

>
> Li Jun
>
>>
>> > think it's necessary to check charger type at every event from usb
>> gadget.
>>
>> My meaning is not every event from usb gadget. When the usb gadget state
>> changes or the extcon device (maybe GPIO detection) state changes, which
>> means charger plugin or pullup, we need to check the charger type to set
>> current.
>
> From your below code, you call usb_charger_notify_others() in
> every state change.

I think it does not matter. In case the usb charger missed some gadget
state changes. Or I replace it with 'USB_STATE_CONFIGURED' state.
Thanks.

>
> if (uchger->old_gadget_state != state) {
> uchger->old_gadget_state = state;
>
> if (state >= USB_STATE_ATTACHED)
> uchger_state = USB_CHARGER_PRESENT;
> else if (state == USB_STATE_NOTATTACHED)
> uchger_state = USB_CHARGER_REMOVE;
> else
> /* this else will never happen */
> uchger_state = USB_CHARGER_DEFAULT;
>
> usb_charger_notify_others(uchger, uchger_state);
> }
>
>>
>> > Something in gadget driver you can utilize is only vbus detection, and
>> > report diff current by diff usb state if it's a SDP.
>>
>> --
>> Baolin.wang
>> Best Regards



-- 
Baolin.wang
Best Regards

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

On 30 March 2016 at 16:07, Jun Li  wrote:
> Hi

>> On 30 March 2016 at 10:54, Jun Li  wrote:
>> >> >> It is not for udc driver but for power users who want to negotiate
>> >> >> with USB subsystem.
>> >> >>
>> >> >
>> >> > Seems you don't want to guarantee charger type detection is done
>> >> > before gadget connection(pullup DP), right?
>> >> > I see you call usb_charger_detect_type() in each gadget usb state
>> >> changes.
>> >>
>> >> I am not sure I get your point correctly, please correct me if I
>> >> misunderstand you.
>> >> We need to check the charger type at every event comes from the usb
>> >> gadget state changes or the extcon device state changes, which means
>> >> a new charger plugin or pullup.
>> >>
>> >
>> > According to usb charger spec, my understanding is you can't do real
>> > charger detection procedure *after* gadget _connection_(pullup DP),
>> > also I don't
>>
>> Why can not? Charger detection is usually from PMIC.
>
> Charger detection process will impact DP/DM line state, see usb charger
> spec v1.2 for detail detection process, section 4.6.3 says:
>
> "A PD is allowed to *disconnect* and repeat the charger detection process
> multiple times while attached. The PD is required to wait for a time of at
> least TCP_VDM_EN max between disconnecting and restarting the charger
> detection process."
>
> As Peter mentioned, the charger detection should happen between VBUS
> detection and gadget pull up DP for first plug in case. So when
> gadget connect (pullup DP), you should already know the charger type.

Make sense. In our company's solution, charger detection can be done
by hardware from PMIC at first, then it will not affect the DP/DM line
when gadget starts to enumeration. In the 'usb_charger_detect_type()',
it usually get the charger type from type registers has been done by
hardware from PMIC, which can not affect the DP/DM line.

>
> Li Jun
>
>>
>> > think it's necessary to check charger type at every event from usb
>> gadget.
>>
>> My meaning is not every event from usb gadget. When the usb gadget state
>> changes or the extcon device (maybe GPIO detection) state changes, which
>> means charger plugin or pullup, we need to check the charger type to set
>> current.
>
> From your below code, you call usb_charger_notify_others() in
> every state change.

I think it does not matter. In case the usb charger missed some gadget
state changes. Or I replace it with 'USB_STATE_CONFIGURED' state.
Thanks.

>
> if (uchger->old_gadget_state != state) {
> uchger->old_gadget_state = state;
>
> if (state >= USB_STATE_ATTACHED)
> uchger_state = USB_CHARGER_PRESENT;
> else if (state == USB_STATE_NOTATTACHED)
> uchger_state = USB_CHARGER_REMOVE;
> else
> /* this else will never happen */
> uchger_state = USB_CHARGER_DEFAULT;
>
> usb_charger_notify_others(uchger, uchger_state);
> }
>
>>
>> > Something in gadget driver you can utilize is only vbus detection, and
>> > report diff current by diff usb state if it's a SDP.
>>
>> --
>> Baolin.wang
>> Best Regards



-- 
Baolin.wang
Best Regards

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Peter Chen]

On Wed, Mar 30, 2016 at 04:40:31PM +0800, Baolin Wang wrote:
> >> > - Third, since composite driver covers 500mA (and more for CDP) after set
> >> > configuration and 2mA after suspend, and vbus handler covers connect
> >> > and disconnect. I can't see any reasons we need to notify gadget state
> >> > for power driver, do we really need to have usb_charger_plug_by_gadget?
> >>
> >> In some solutions, gadget does not negotiate with the current. They
> >> just send out one signal to power driver to set the current when the
> >> gadget state is changed (plugin or not). So we need to check the
> >> charger state by the gadget state to notify the charger IC to set
> >> current.
> >>
> >
> > Would you give some examples?
> 
> OK. I explain it in detail. Now charger detection can be from gadget
> itself or PMIC, and we focus on gadget detection. Charger IC (charger
> driver) is separate with gadget.
> When the usb cable is plugin, we need to report the plugin event to
> charger driver to set current after setting configuration for gadget.
> The usb charger is responsible for reporting plugin event to charger
> driver. But how usb charger get the plugin event? It can get the
> plugin event from gadget state (if the gadget state is more than
> 'USB_STATE_ATTACHED', it means one cable plugin). So we need notify
> gadget state to usb charger.
> 

Ok, I see, it only changes current limit at function usb_gadget_vbus_draw
in your patch 2/4. Then, we need to make sure usb_charger_set_cur_limit_by_type
is called before calling usb_gadget_set_state(gadget, USB_STATE_CONFIGURED).

It seems you have not implemented usb_charger_plug_by_gadget in your patch set.

-- 
Best Regards,
Peter Chen

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Peter Chen]

On Wed, Mar 30, 2016 at 04:40:31PM +0800, Baolin Wang wrote:
> >> > - Third, since composite driver covers 500mA (and more for CDP) after set
> >> > configuration and 2mA after suspend, and vbus handler covers connect
> >> > and disconnect. I can't see any reasons we need to notify gadget state
> >> > for power driver, do we really need to have usb_charger_plug_by_gadget?
> >>
> >> In some solutions, gadget does not negotiate with the current. They
> >> just send out one signal to power driver to set the current when the
> >> gadget state is changed (plugin or not). So we need to check the
> >> charger state by the gadget state to notify the charger IC to set
> >> current.
> >>
> >
> > Would you give some examples?
> 
> OK. I explain it in detail. Now charger detection can be from gadget
> itself or PMIC, and we focus on gadget detection. Charger IC (charger
> driver) is separate with gadget.
> When the usb cable is plugin, we need to report the plugin event to
> charger driver to set current after setting configuration for gadget.
> The usb charger is responsible for reporting plugin event to charger
> driver. But how usb charger get the plugin event? It can get the
> plugin event from gadget state (if the gadget state is more than
> 'USB_STATE_ATTACHED', it means one cable plugin). So we need notify
> gadget state to usb charger.
> 

Ok, I see, it only changes current limit at function usb_gadget_vbus_draw
in your patch 2/4. Then, we need to make sure usb_charger_set_cur_limit_by_type
is called before calling usb_gadget_set_state(gadget, USB_STATE_CONFIGURED).

It seems you have not implemented usb_charger_plug_by_gadget in your patch set.

-- 
Best Regards,
Peter Chen

RE: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Jun Li]

Hi

> -Original Message-
> From: Baolin Wang [mailto:baolin.w...@linaro.org]
> Sent: Wednesday, March 30, 2016 2:15 PM
> To: Jun Li <jun...@nxp.com>
> Cc: Peter Chen <hzpeterc...@gmail.com>; Felipe Balbi <ba...@kernel.org>;
> Greg KH <gre...@linuxfoundation.org>; Sebastian Reichel <s...@kernel.org>;
> Dmitry Eremin-Solenikov <dbarysh...@gmail.com>; David Woodhouse
> <dw...@infradead.org>; Peter Chen <peter.c...@freescale.com>; Alan Stern
> <st...@rowland.harvard.edu>; r.bald...@samsung.com; Yoshihiro Shimoda
> <yoshihiro.shimoda...@renesas.com>; Lee Jones <lee.jo...@linaro.org>; Mark
> Brown <broo...@kernel.org>; Charles Keepax
> <ckee...@opensource.wolfsonmicro.com>; patc...@opensource.wolfsonmicro.com;
> Linux PM list <linux...@vger.kernel.org>; USB <linux-...@vger.kernel.org>;
> device-mainlin...@lists.linuxfoundation.org; LKML  ker...@vger.kernel.org>
> Subject: Re: [PATCH v8 0/4] Introduce usb charger framework to deal with
> the usb gadget power negotation
> 
> On 30 March 2016 at 10:54, Jun Li <jun...@nxp.com> wrote:
> >> >> It is not for udc driver but for power users who want to negotiate
> >> >> with USB subsystem.
> >> >>
> >> >
> >> > Seems you don't want to guarantee charger type detection is done
> >> > before gadget connection(pullup DP), right?
> >> > I see you call usb_charger_detect_type() in each gadget usb state
> >> changes.
> >>
> >> I am not sure I get your point correctly, please correct me if I
> >> misunderstand you.
> >> We need to check the charger type at every event comes from the usb
> >> gadget state changes or the extcon device state changes, which means
> >> a new charger plugin or pullup.
> >>
> >
> > According to usb charger spec, my understanding is you can't do real
> > charger detection procedure *after* gadget _connection_(pullup DP),
> > also I don't
> 
> Why can not? Charger detection is usually from PMIC.

Charger detection process will impact DP/DM line state, see usb charger
spec v1.2 for detail detection process, section 4.6.3 says:

"A PD is allowed to *disconnect* and repeat the charger detection process
multiple times while attached. The PD is required to wait for a time of at
least TCP_VDM_EN max between disconnecting and restarting the charger
detection process."

As Peter mentioned, the charger detection should happen between VBUS
detection and gadget pull up DP for first plug in case. So when
gadget connect (pullup DP), you should already know the charger type.

Li Jun
 
> 
> > think it's necessary to check charger type at every event from usb
> gadget.
> 
> My meaning is not every event from usb gadget. When the usb gadget state
> changes or the extcon device (maybe GPIO detection) state changes, which
> means charger plugin or pullup, we need to check the charger type to set
> current.

From your below code, you call usb_charger_notify_others() in
every state change.

if (uchger->old_gadget_state != state) {
uchger->old_gadget_state = state;

if (state >= USB_STATE_ATTACHED)
uchger_state = USB_CHARGER_PRESENT;
else if (state == USB_STATE_NOTATTACHED)
uchger_state = USB_CHARGER_REMOVE;
else
/* this else will never happen */
uchger_state = USB_CHARGER_DEFAULT;

usb_charger_notify_others(uchger, uchger_state);
}

> 
> > Something in gadget driver you can utilize is only vbus detection, and
> > report diff current by diff usb state if it's a SDP.
> 
> --
> Baolin.wang
> Best Regards

RE: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Jun Li]

Hi

> -Original Message-
> From: Baolin Wang [mailto:baolin.w...@linaro.org]
> Sent: Wednesday, March 30, 2016 2:15 PM
> To: Jun Li 
> Cc: Peter Chen ; Felipe Balbi ;
> Greg KH ; Sebastian Reichel ;
> Dmitry Eremin-Solenikov ; David Woodhouse
> ; Peter Chen ; Alan Stern
> ; r.bald...@samsung.com; Yoshihiro Shimoda
> ; Lee Jones ; Mark
> Brown ; Charles Keepax
> ; patc...@opensource.wolfsonmicro.com;
> Linux PM list ; USB ;
> device-mainlin...@lists.linuxfoundation.org; LKML  ker...@vger.kernel.org>
> Subject: Re: [PATCH v8 0/4] Introduce usb charger framework to deal with
> the usb gadget power negotation
> 
> On 30 March 2016 at 10:54, Jun Li  wrote:
> >> >> It is not for udc driver but for power users who want to negotiate
> >> >> with USB subsystem.
> >> >>
> >> >
> >> > Seems you don't want to guarantee charger type detection is done
> >> > before gadget connection(pullup DP), right?
> >> > I see you call usb_charger_detect_type() in each gadget usb state
> >> changes.
> >>
> >> I am not sure I get your point correctly, please correct me if I
> >> misunderstand you.
> >> We need to check the charger type at every event comes from the usb
> >> gadget state changes or the extcon device state changes, which means
> >> a new charger plugin or pullup.
> >>
> >
> > According to usb charger spec, my understanding is you can't do real
> > charger detection procedure *after* gadget _connection_(pullup DP),
> > also I don't
> 
> Why can not? Charger detection is usually from PMIC.

Charger detection process will impact DP/DM line state, see usb charger
spec v1.2 for detail detection process, section 4.6.3 says:

"A PD is allowed to *disconnect* and repeat the charger detection process
multiple times while attached. The PD is required to wait for a time of at
least TCP_VDM_EN max between disconnecting and restarting the charger
detection process."

As Peter mentioned, the charger detection should happen between VBUS
detection and gadget pull up DP for first plug in case. So when
gadget connect (pullup DP), you should already know the charger type.

Li Jun
 
> 
> > think it's necessary to check charger type at every event from usb
> gadget.
> 
> My meaning is not every event from usb gadget. When the usb gadget state
> changes or the extcon device (maybe GPIO detection) state changes, which
> means charger plugin or pullup, we need to check the charger type to set
> current.

From your below code, you call usb_charger_notify_others() in
every state change.

if (uchger->old_gadget_state != state) {
uchger->old_gadget_state = state;

if (state >= USB_STATE_ATTACHED)
uchger_state = USB_CHARGER_PRESENT;
else if (state == USB_STATE_NOTATTACHED)
uchger_state = USB_CHARGER_REMOVE;
else
/* this else will never happen */
uchger_state = USB_CHARGER_DEFAULT;

usb_charger_notify_others(uchger, uchger_state);
}

> 
> > Something in gadget driver you can utilize is only vbus detection, and
> > report diff current by diff usb state if it's a SDP.
> 
> --
> Baolin.wang
> Best Regards

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

On 30 March 2016 at 15:42, Peter Chen  wrote:
> On Wed, Mar 30, 2016 at 03:07:49PM +0800, Baolin Wang wrote:
>> On 30 March 2016 at 10:05, Peter Chen  wrote:
>> > On Tue, Mar 29, 2016 at 10:23:14AM -0700, Mark Brown wrote:
>> >> On Mon, Mar 28, 2016 at 03:13:51PM +0800, Peter Chen wrote:
>> >> > On Mon, Mar 28, 2016 at 02:51:40PM +0800, Baolin Wang wrote:
>> >>
>> >> > > > I am afraid I still not find the user (udc driver) for this 
>> >> > > > framework, I would
>> >> > > > like to see how udc driver block the enumeration until the charger 
>> >> > > > detection
>> >> > > > has finished, or am I missing something?
>> >>
>> >> > > It is not for udc driver but for power users who want to negotiate
>> >> > > with USB subsystem.
>> >>
>> >> > Then, where is the code the test user to decide what kinds of USB 
>> >> > charger
>> >> > (SDP, CDP, DCP) is connecting now?
>> >>
>> >> Even without detection of CDP and DCP we have configurability within SDP
>> >> - there's the 2.5mA suspended limit, the 100mA default limit and the
>> >> higher 500mA limit which can be negotiated.
>> >
>> > Well, things may be a little complicated.
>>
>> I try to address your comments, maybe Mark need to do some complements. 
>> Thanks.
>>
>> >
>> > - First, how to design get_charger_type for each udc driver?
>> > Since the charger detection process affects dp/dm signal, it can't be done
>> > during the enumeration or after that. So, the detection process can be only
>> > done after vbus has detected and before udc pull up dp.
>> >
>> > Then, when the get_charger_type do real charger detection? My suggestion 
>> > is,
>> > if the charger type is unknown, we do real one, else, we just return the
>> > stored type value.
>>
>> I think that is why we let udc driver to implement the
>> 'gadget->ops->get_charger_type()' callback, which can be controlled by
>> udc driver.
>>
>> >
>> > - Second, When to notify charger IC to charger:
>> > For SDP and CDP, it needs to notify charger IC after set configuration
>> > has finished.
>> > For DCP (and ACA, I am not sure), can notify charger IC after charger
>> > detection has finished or later.
>> >
>> > So, when we get charger is present, we need to notify charger IC at once
>> > for DCP (and ACA); But for SDP and CDP, we need to let the gadget
>> > composite core to notify charger IC after set configuration has finished,
>> > like the patch 2/4 does.
>>
>> But mostly charger IC is separate with gadget, so the charger IC
>> doesn't need to worry about the gadget status.
>>
>
> The reason why we need to combine charger IC with USB gadget (charger)
> driver is the charger IC should not charge as much as it wants, it
> needs to consider USB charger style and USB connection
> (un-configuration vs configuration) situation.

Yes, I agree with the charger IC should not charge as much as it
wants, but we don't want to combine two separated things. Like you
said, udc driver can implement 'get_charger_type()' callback to check
the charger type at the right time, then set the current after
checking the charger type (now the gadget has been set configuration).

>
>> >
>> > - Third, since composite driver covers 500mA (and more for CDP) after set
>> > configuration and 2mA after suspend, and vbus handler covers connect
>> > and disconnect. I can't see any reasons we need to notify gadget state
>> > for power driver, do we really need to have usb_charger_plug_by_gadget?
>>
>> In some solutions, gadget does not negotiate with the current. They
>> just send out one signal to power driver to set the current when the
>> gadget state is changed (plugin or not). So we need to check the
>> charger state by the gadget state to notify the charger IC to set
>> current.
>>
>
> Would you give some examples?

OK. I explain it in detail. Now charger detection can be from gadget
itself or PMIC, and we focus on gadget detection. Charger IC (charger
driver) is separate with gadget.
When the usb cable is plugin, we need to report the plugin event to
charger driver to set current after setting configuration for gadget.
The usb charger is responsible for reporting plugin event to charger
driver. But how usb charger get the plugin event? It can get the
plugin event from gadget state (if the gadget state is more than
'USB_STATE_ATTACHED', it means one cable plugin). So we need notify
gadget state to usb charger.

>
> --
> Best Regards,
> Peter Chen



-- 
Baolin.wang
Best Regards

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

On 30 March 2016 at 15:42, Peter Chen  wrote:
> On Wed, Mar 30, 2016 at 03:07:49PM +0800, Baolin Wang wrote:
>> On 30 March 2016 at 10:05, Peter Chen  wrote:
>> > On Tue, Mar 29, 2016 at 10:23:14AM -0700, Mark Brown wrote:
>> >> On Mon, Mar 28, 2016 at 03:13:51PM +0800, Peter Chen wrote:
>> >> > On Mon, Mar 28, 2016 at 02:51:40PM +0800, Baolin Wang wrote:
>> >>
>> >> > > > I am afraid I still not find the user (udc driver) for this 
>> >> > > > framework, I would
>> >> > > > like to see how udc driver block the enumeration until the charger 
>> >> > > > detection
>> >> > > > has finished, or am I missing something?
>> >>
>> >> > > It is not for udc driver but for power users who want to negotiate
>> >> > > with USB subsystem.
>> >>
>> >> > Then, where is the code the test user to decide what kinds of USB 
>> >> > charger
>> >> > (SDP, CDP, DCP) is connecting now?
>> >>
>> >> Even without detection of CDP and DCP we have configurability within SDP
>> >> - there's the 2.5mA suspended limit, the 100mA default limit and the
>> >> higher 500mA limit which can be negotiated.
>> >
>> > Well, things may be a little complicated.
>>
>> I try to address your comments, maybe Mark need to do some complements. 
>> Thanks.
>>
>> >
>> > - First, how to design get_charger_type for each udc driver?
>> > Since the charger detection process affects dp/dm signal, it can't be done
>> > during the enumeration or after that. So, the detection process can be only
>> > done after vbus has detected and before udc pull up dp.
>> >
>> > Then, when the get_charger_type do real charger detection? My suggestion 
>> > is,
>> > if the charger type is unknown, we do real one, else, we just return the
>> > stored type value.
>>
>> I think that is why we let udc driver to implement the
>> 'gadget->ops->get_charger_type()' callback, which can be controlled by
>> udc driver.
>>
>> >
>> > - Second, When to notify charger IC to charger:
>> > For SDP and CDP, it needs to notify charger IC after set configuration
>> > has finished.
>> > For DCP (and ACA, I am not sure), can notify charger IC after charger
>> > detection has finished or later.
>> >
>> > So, when we get charger is present, we need to notify charger IC at once
>> > for DCP (and ACA); But for SDP and CDP, we need to let the gadget
>> > composite core to notify charger IC after set configuration has finished,
>> > like the patch 2/4 does.
>>
>> But mostly charger IC is separate with gadget, so the charger IC
>> doesn't need to worry about the gadget status.
>>
>
> The reason why we need to combine charger IC with USB gadget (charger)
> driver is the charger IC should not charge as much as it wants, it
> needs to consider USB charger style and USB connection
> (un-configuration vs configuration) situation.

Yes, I agree with the charger IC should not charge as much as it
wants, but we don't want to combine two separated things. Like you
said, udc driver can implement 'get_charger_type()' callback to check
the charger type at the right time, then set the current after
checking the charger type (now the gadget has been set configuration).

>
>> >
>> > - Third, since composite driver covers 500mA (and more for CDP) after set
>> > configuration and 2mA after suspend, and vbus handler covers connect
>> > and disconnect. I can't see any reasons we need to notify gadget state
>> > for power driver, do we really need to have usb_charger_plug_by_gadget?
>>
>> In some solutions, gadget does not negotiate with the current. They
>> just send out one signal to power driver to set the current when the
>> gadget state is changed (plugin or not). So we need to check the
>> charger state by the gadget state to notify the charger IC to set
>> current.
>>
>
> Would you give some examples?

OK. I explain it in detail. Now charger detection can be from gadget
itself or PMIC, and we focus on gadget detection. Charger IC (charger
driver) is separate with gadget.
When the usb cable is plugin, we need to report the plugin event to
charger driver to set current after setting configuration for gadget.
The usb charger is responsible for reporting plugin event to charger
driver. But how usb charger get the plugin event? It can get the
plugin event from gadget state (if the gadget state is more than
'USB_STATE_ATTACHED', it means one cable plugin). So we need notify
gadget state to usb charger.

>
> --
> Best Regards,
> Peter Chen



-- 
Baolin.wang
Best Regards

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Peter Chen]

On Wed, Mar 30, 2016 at 03:07:49PM +0800, Baolin Wang wrote:
> On 30 March 2016 at 10:05, Peter Chen  wrote:
> > On Tue, Mar 29, 2016 at 10:23:14AM -0700, Mark Brown wrote:
> >> On Mon, Mar 28, 2016 at 03:13:51PM +0800, Peter Chen wrote:
> >> > On Mon, Mar 28, 2016 at 02:51:40PM +0800, Baolin Wang wrote:
> >>
> >> > > > I am afraid I still not find the user (udc driver) for this 
> >> > > > framework, I would
> >> > > > like to see how udc driver block the enumeration until the charger 
> >> > > > detection
> >> > > > has finished, or am I missing something?
> >>
> >> > > It is not for udc driver but for power users who want to negotiate
> >> > > with USB subsystem.
> >>
> >> > Then, where is the code the test user to decide what kinds of USB charger
> >> > (SDP, CDP, DCP) is connecting now?
> >>
> >> Even without detection of CDP and DCP we have configurability within SDP
> >> - there's the 2.5mA suspended limit, the 100mA default limit and the
> >> higher 500mA limit which can be negotiated.
> >
> > Well, things may be a little complicated.
> 
> I try to address your comments, maybe Mark need to do some complements. 
> Thanks.
> 
> >
> > - First, how to design get_charger_type for each udc driver?
> > Since the charger detection process affects dp/dm signal, it can't be done
> > during the enumeration or after that. So, the detection process can be only
> > done after vbus has detected and before udc pull up dp.
> >
> > Then, when the get_charger_type do real charger detection? My suggestion is,
> > if the charger type is unknown, we do real one, else, we just return the
> > stored type value.
> 
> I think that is why we let udc driver to implement the
> 'gadget->ops->get_charger_type()' callback, which can be controlled by
> udc driver.
> 
> >
> > - Second, When to notify charger IC to charger:
> > For SDP and CDP, it needs to notify charger IC after set configuration
> > has finished.
> > For DCP (and ACA, I am not sure), can notify charger IC after charger
> > detection has finished or later.
> >
> > So, when we get charger is present, we need to notify charger IC at once
> > for DCP (and ACA); But for SDP and CDP, we need to let the gadget
> > composite core to notify charger IC after set configuration has finished,
> > like the patch 2/4 does.
> 
> But mostly charger IC is separate with gadget, so the charger IC
> doesn't need to worry about the gadget status.
> 

The reason why we need to combine charger IC with USB gadget (charger)
driver is the charger IC should not charge as much as it wants, it
needs to consider USB charger style and USB connection
(un-configuration vs configuration) situation.

> >
> > - Third, since composite driver covers 500mA (and more for CDP) after set
> > configuration and 2mA after suspend, and vbus handler covers connect
> > and disconnect. I can't see any reasons we need to notify gadget state
> > for power driver, do we really need to have usb_charger_plug_by_gadget?
> 
> In some solutions, gadget does not negotiate with the current. They
> just send out one signal to power driver to set the current when the
> gadget state is changed (plugin or not). So we need to check the
> charger state by the gadget state to notify the charger IC to set
> current.
> 

Would you give some examples?

-- 
Best Regards,
Peter Chen

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Peter Chen]

On Wed, Mar 30, 2016 at 03:07:49PM +0800, Baolin Wang wrote:
> On 30 March 2016 at 10:05, Peter Chen  wrote:
> > On Tue, Mar 29, 2016 at 10:23:14AM -0700, Mark Brown wrote:
> >> On Mon, Mar 28, 2016 at 03:13:51PM +0800, Peter Chen wrote:
> >> > On Mon, Mar 28, 2016 at 02:51:40PM +0800, Baolin Wang wrote:
> >>
> >> > > > I am afraid I still not find the user (udc driver) for this 
> >> > > > framework, I would
> >> > > > like to see how udc driver block the enumeration until the charger 
> >> > > > detection
> >> > > > has finished, or am I missing something?
> >>
> >> > > It is not for udc driver but for power users who want to negotiate
> >> > > with USB subsystem.
> >>
> >> > Then, where is the code the test user to decide what kinds of USB charger
> >> > (SDP, CDP, DCP) is connecting now?
> >>
> >> Even without detection of CDP and DCP we have configurability within SDP
> >> - there's the 2.5mA suspended limit, the 100mA default limit and the
> >> higher 500mA limit which can be negotiated.
> >
> > Well, things may be a little complicated.
> 
> I try to address your comments, maybe Mark need to do some complements. 
> Thanks.
> 
> >
> > - First, how to design get_charger_type for each udc driver?
> > Since the charger detection process affects dp/dm signal, it can't be done
> > during the enumeration or after that. So, the detection process can be only
> > done after vbus has detected and before udc pull up dp.
> >
> > Then, when the get_charger_type do real charger detection? My suggestion is,
> > if the charger type is unknown, we do real one, else, we just return the
> > stored type value.
> 
> I think that is why we let udc driver to implement the
> 'gadget->ops->get_charger_type()' callback, which can be controlled by
> udc driver.
> 
> >
> > - Second, When to notify charger IC to charger:
> > For SDP and CDP, it needs to notify charger IC after set configuration
> > has finished.
> > For DCP (and ACA, I am not sure), can notify charger IC after charger
> > detection has finished or later.
> >
> > So, when we get charger is present, we need to notify charger IC at once
> > for DCP (and ACA); But for SDP and CDP, we need to let the gadget
> > composite core to notify charger IC after set configuration has finished,
> > like the patch 2/4 does.
> 
> But mostly charger IC is separate with gadget, so the charger IC
> doesn't need to worry about the gadget status.
> 

The reason why we need to combine charger IC with USB gadget (charger)
driver is the charger IC should not charge as much as it wants, it
needs to consider USB charger style and USB connection
(un-configuration vs configuration) situation.

> >
> > - Third, since composite driver covers 500mA (and more for CDP) after set
> > configuration and 2mA after suspend, and vbus handler covers connect
> > and disconnect. I can't see any reasons we need to notify gadget state
> > for power driver, do we really need to have usb_charger_plug_by_gadget?
> 
> In some solutions, gadget does not negotiate with the current. They
> just send out one signal to power driver to set the current when the
> gadget state is changed (plugin or not). So we need to check the
> charger state by the gadget state to notify the charger IC to set
> current.
> 

Would you give some examples?

-- 
Best Regards,
Peter Chen

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

On 30 March 2016 at 10:05, Peter Chen  wrote:
> On Tue, Mar 29, 2016 at 10:23:14AM -0700, Mark Brown wrote:
>> On Mon, Mar 28, 2016 at 03:13:51PM +0800, Peter Chen wrote:
>> > On Mon, Mar 28, 2016 at 02:51:40PM +0800, Baolin Wang wrote:
>>
>> > > > I am afraid I still not find the user (udc driver) for this framework, 
>> > > > I would
>> > > > like to see how udc driver block the enumeration until the charger 
>> > > > detection
>> > > > has finished, or am I missing something?
>>
>> > > It is not for udc driver but for power users who want to negotiate
>> > > with USB subsystem.
>>
>> > Then, where is the code the test user to decide what kinds of USB charger
>> > (SDP, CDP, DCP) is connecting now?
>>
>> Even without detection of CDP and DCP we have configurability within SDP
>> - there's the 2.5mA suspended limit, the 100mA default limit and the
>> higher 500mA limit which can be negotiated.
>
> Well, things may be a little complicated.

I try to address your comments, maybe Mark need to do some complements. Thanks.

>
> - First, how to design get_charger_type for each udc driver?
> Since the charger detection process affects dp/dm signal, it can't be done
> during the enumeration or after that. So, the detection process can be only
> done after vbus has detected and before udc pull up dp.
>
> Then, when the get_charger_type do real charger detection? My suggestion is,
> if the charger type is unknown, we do real one, else, we just return the
> stored type value.

I think that is why we let udc driver to implement the
'gadget->ops->get_charger_type()' callback, which can be controlled by
udc driver.

>
> - Second, When to notify charger IC to charger:
> For SDP and CDP, it needs to notify charger IC after set configuration
> has finished.
> For DCP (and ACA, I am not sure), can notify charger IC after charger
> detection has finished or later.
>
> So, when we get charger is present, we need to notify charger IC at once
> for DCP (and ACA); But for SDP and CDP, we need to let the gadget
> composite core to notify charger IC after set configuration has finished,
> like the patch 2/4 does.

But mostly charger IC is separate with gadget, so the charger IC
doesn't need to worry about the gadget status.

>
> - Third, since composite driver covers 500mA (and more for CDP) after set
> configuration and 2mA after suspend, and vbus handler covers connect
> and disconnect. I can't see any reasons we need to notify gadget state
> for power driver, do we really need to have usb_charger_plug_by_gadget?

In some solutions, gadget does not negotiate with the current. They
just send out one signal to power driver to set the current when the
gadget state is changed (plugin or not). So we need to check the
charger state by the gadget state to notify the charger IC to set
current.

>
> --
> Best Regards,
> eter Chen



-- 
Baolin.wang
Best Regards

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

On 30 March 2016 at 10:05, Peter Chen  wrote:
> On Tue, Mar 29, 2016 at 10:23:14AM -0700, Mark Brown wrote:
>> On Mon, Mar 28, 2016 at 03:13:51PM +0800, Peter Chen wrote:
>> > On Mon, Mar 28, 2016 at 02:51:40PM +0800, Baolin Wang wrote:
>>
>> > > > I am afraid I still not find the user (udc driver) for this framework, 
>> > > > I would
>> > > > like to see how udc driver block the enumeration until the charger 
>> > > > detection
>> > > > has finished, or am I missing something?
>>
>> > > It is not for udc driver but for power users who want to negotiate
>> > > with USB subsystem.
>>
>> > Then, where is the code the test user to decide what kinds of USB charger
>> > (SDP, CDP, DCP) is connecting now?
>>
>> Even without detection of CDP and DCP we have configurability within SDP
>> - there's the 2.5mA suspended limit, the 100mA default limit and the
>> higher 500mA limit which can be negotiated.
>
> Well, things may be a little complicated.

I try to address your comments, maybe Mark need to do some complements. Thanks.

>
> - First, how to design get_charger_type for each udc driver?
> Since the charger detection process affects dp/dm signal, it can't be done
> during the enumeration or after that. So, the detection process can be only
> done after vbus has detected and before udc pull up dp.
>
> Then, when the get_charger_type do real charger detection? My suggestion is,
> if the charger type is unknown, we do real one, else, we just return the
> stored type value.

I think that is why we let udc driver to implement the
'gadget->ops->get_charger_type()' callback, which can be controlled by
udc driver.

>
> - Second, When to notify charger IC to charger:
> For SDP and CDP, it needs to notify charger IC after set configuration
> has finished.
> For DCP (and ACA, I am not sure), can notify charger IC after charger
> detection has finished or later.
>
> So, when we get charger is present, we need to notify charger IC at once
> for DCP (and ACA); But for SDP and CDP, we need to let the gadget
> composite core to notify charger IC after set configuration has finished,
> like the patch 2/4 does.

But mostly charger IC is separate with gadget, so the charger IC
doesn't need to worry about the gadget status.

>
> - Third, since composite driver covers 500mA (and more for CDP) after set
> configuration and 2mA after suspend, and vbus handler covers connect
> and disconnect. I can't see any reasons we need to notify gadget state
> for power driver, do we really need to have usb_charger_plug_by_gadget?

In some solutions, gadget does not negotiate with the current. They
just send out one signal to power driver to set the current when the
gadget state is changed (plugin or not). So we need to check the
charger state by the gadget state to notify the charger IC to set
current.

>
> --
> Best Regards,
> eter Chen



-- 
Baolin.wang
Best Regards

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

On 30 March 2016 at 10:54, Jun Li  wrote:
>> >> It is not for udc driver but for power users who want to negotiate
>> >> with USB subsystem.
>> >>
>> >
>> > Seems you don't want to guarantee charger type detection is done
>> > before gadget connection(pullup DP), right?
>> > I see you call usb_charger_detect_type() in each gadget usb state
>> changes.
>>
>> I am not sure I get your point correctly, please correct me if I
>> misunderstand you.
>> We need to check the charger type at every event comes from the usb gadget
>> state changes or the extcon device state changes, which means a new
>> charger plugin or pullup.
>>
>
> According to usb charger spec, my understanding is you can't do real charger
> detection procedure *after* gadget _connection_(pullup DP), also I don't

Why can not? Charger detection is usually from PMIC.

> think it's necessary to check charger type at every event from usb gadget.

My meaning is not every event from usb gadget. When the usb gadget
state changes or the extcon device (maybe GPIO detection) state
changes, which means charger plugin or pullup, we need to check the
charger type to set current.

> Something in gadget driver you can utilize is only vbus detection, and
> report diff current by diff usb state if it's a SDP.

-- 
Baolin.wang
Best Regards

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

On 30 March 2016 at 10:54, Jun Li  wrote:
>> >> It is not for udc driver but for power users who want to negotiate
>> >> with USB subsystem.
>> >>
>> >
>> > Seems you don't want to guarantee charger type detection is done
>> > before gadget connection(pullup DP), right?
>> > I see you call usb_charger_detect_type() in each gadget usb state
>> changes.
>>
>> I am not sure I get your point correctly, please correct me if I
>> misunderstand you.
>> We need to check the charger type at every event comes from the usb gadget
>> state changes or the extcon device state changes, which means a new
>> charger plugin or pullup.
>>
>
> According to usb charger spec, my understanding is you can't do real charger
> detection procedure *after* gadget _connection_(pullup DP), also I don't

Why can not? Charger detection is usually from PMIC.

> think it's necessary to check charger type at every event from usb gadget.

My meaning is not every event from usb gadget. When the usb gadget
state changes or the extcon device (maybe GPIO detection) state
changes, which means charger plugin or pullup, we need to check the
charger type to set current.

> Something in gadget driver you can utilize is only vbus detection, and
> report diff current by diff usb state if it's a SDP.

-- 
Baolin.wang
Best Regards

RE: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Jun Li]

> -Original Message-
> From: Baolin Wang [mailto:baolin.w...@linaro.org]
> Sent: Tuesday, March 29, 2016 5:49 PM
> To: Jun Li <jun...@nxp.com>
> Cc: Peter Chen <hzpeterc...@gmail.com>; Felipe Balbi <ba...@kernel.org>;
> Greg KH <gre...@linuxfoundation.org>; Sebastian Reichel <s...@kernel.org>;
> Dmitry Eremin-Solenikov <dbarysh...@gmail.com>; David Woodhouse
> <dw...@infradead.org>; Peter Chen <peter.c...@freescale.com>; Alan Stern
> <st...@rowland.harvard.edu>; r.bald...@samsung.com; Yoshihiro Shimoda
> <yoshihiro.shimoda...@renesas.com>; Lee Jones <lee.jo...@linaro.org>; Mark
> Brown <broo...@kernel.org>; Charles Keepax
> <ckee...@opensource.wolfsonmicro.com>; patc...@opensource.wolfsonmicro.com;
> Linux PM list <linux...@vger.kernel.org>; USB <linux-...@vger.kernel.org>;
> device-mainlin...@lists.linuxfoundation.org; LKML  ker...@vger.kernel.org>
> Subject: Re: [PATCH v8 0/4] Introduce usb charger framework to deal with
> the usb gadget power negotation
> 
> On 29 March 2016 at 16:45, Jun Li <jun...@nxp.com> wrote:
> >
> >
> >> -Original Message-
> >> From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-
> >> ow...@vger.kernel.org] On Behalf Of Baolin Wang
> >> Sent: Monday, March 28, 2016 2:52 PM
> >> To: Peter Chen <hzpeterc...@gmail.com>
> >> Cc: Felipe Balbi <ba...@kernel.org>; Greg KH
> >> <gre...@linuxfoundation.org>; Sebastian Reichel <s...@kernel.org>;
> >> Dmitry Eremin-Solenikov <dbarysh...@gmail.com>; David Woodhouse
> >> <dw...@infradead.org>; Peter Chen <peter.c...@freescale.com>; Alan
> >> Stern <st...@rowland.harvard.edu>; r.bald...@samsung.com; Yoshihiro
> >> Shimoda <yoshihiro.shimoda...@renesas.com>; Lee Jones
> >> <lee.jo...@linaro.org>; Mark Brown <broo...@kernel.org>; Charles
> >> Keepax <ckee...@opensource.wolfsonmicro.com>;
> >> patc...@opensource.wolfsonmicro.com;
> >> Linux PM list <linux...@vger.kernel.org>; USB
> >> <linux-...@vger.kernel.org>;
> >> device-mainlin...@lists.linuxfoundation.org; LKML  >> ker...@vger.kernel.org>
> >> Subject: Re: [PATCH v8 0/4] Introduce usb charger framework to deal
> >> with the usb gadget power negotation
> >>
> >> On 25 March 2016 at 15:09, Peter Chen <hzpeterc...@gmail.com> wrote:
> >> > On Thu, Mar 24, 2016 at 08:35:53PM +0800, Baolin Wang wrote:
> >> >> Currently the Linux kernel does not provide any standard
> >> >> integration of this feature that integrates the USB subsystem with
> >> >> the system power regulation provided by PMICs meaning that either
> >> >> vendors must add this in their kernels or USB gadget devices based
> >> >> on Linux (such as mobile phones) may not behave as they should.
> >> >> Thus provide a
> >> standard framework for doing this in kernel.
> >> >>
> >> >> Now introduce one user with wm831x_power to support and test the
> >> >> usb charger, which is pending testing. Moreover there may be other
> >> >> potential users will use it in future.
> >> >>
> >> >
> >> > I am afraid I still not find the user (udc driver) for this
> >> > framework, I would like to see how udc driver block the enumeration
> >> > until the charger detection has finished, or am I missing something?
> >>
> >> It is not for udc driver but for power users who want to negotiate
> >> with USB subsystem.
> >>
> >
> > Seems you don't want to guarantee charger type detection is done
> > before gadget connection(pullup DP), right?
> > I see you call usb_charger_detect_type() in each gadget usb state
> changes.
> 
> I am not sure I get your point correctly, please correct me if I
> misunderstand you.
> We need to check the charger type at every event comes from the usb gadget
> state changes or the extcon device state changes, which means a new
> charger plugin or pullup.
> 

According to usb charger spec, my understanding is you can't do real charger
detection procedure *after* gadget _connection_(pullup DP), also I don't
think it's necessary to check charger type at every event from usb gadget.
Something in gadget driver you can utilize is only vbus detection, and
report diff current by diff usb state if it's a SDP.

> >
> > Li Jun
> >> >
> >> > --
> >> > Best Regards,
> >> > Peter Chen
> >>
> >>
> >>
> >> --
> >> Baolin.wang
> >> Best Regards
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-usb"
> >> in the body of a message to majord...@vger.kernel.org More majordomo
> >> info at http://vger.kernel.org/majordomo-info.html
> 
> 
> 
> --
> Baolin.wang
> Best Regards

RE: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Jun Li]

> -Original Message-
> From: Baolin Wang [mailto:baolin.w...@linaro.org]
> Sent: Tuesday, March 29, 2016 5:49 PM
> To: Jun Li 
> Cc: Peter Chen ; Felipe Balbi ;
> Greg KH ; Sebastian Reichel ;
> Dmitry Eremin-Solenikov ; David Woodhouse
> ; Peter Chen ; Alan Stern
> ; r.bald...@samsung.com; Yoshihiro Shimoda
> ; Lee Jones ; Mark
> Brown ; Charles Keepax
> ; patc...@opensource.wolfsonmicro.com;
> Linux PM list ; USB ;
> device-mainlin...@lists.linuxfoundation.org; LKML  ker...@vger.kernel.org>
> Subject: Re: [PATCH v8 0/4] Introduce usb charger framework to deal with
> the usb gadget power negotation
> 
> On 29 March 2016 at 16:45, Jun Li  wrote:
> >
> >
> >> -Original Message-
> >> From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-
> >> ow...@vger.kernel.org] On Behalf Of Baolin Wang
> >> Sent: Monday, March 28, 2016 2:52 PM
> >> To: Peter Chen 
> >> Cc: Felipe Balbi ; Greg KH
> >> ; Sebastian Reichel ;
> >> Dmitry Eremin-Solenikov ; David Woodhouse
> >> ; Peter Chen ; Alan
> >> Stern ; r.bald...@samsung.com; Yoshihiro
> >> Shimoda ; Lee Jones
> >> ; Mark Brown ; Charles
> >> Keepax ;
> >> patc...@opensource.wolfsonmicro.com;
> >> Linux PM list ; USB
> >> ;
> >> device-mainlin...@lists.linuxfoundation.org; LKML  >> ker...@vger.kernel.org>
> >> Subject: Re: [PATCH v8 0/4] Introduce usb charger framework to deal
> >> with the usb gadget power negotation
> >>
> >> On 25 March 2016 at 15:09, Peter Chen  wrote:
> >> > On Thu, Mar 24, 2016 at 08:35:53PM +0800, Baolin Wang wrote:
> >> >> Currently the Linux kernel does not provide any standard
> >> >> integration of this feature that integrates the USB subsystem with
> >> >> the system power regulation provided by PMICs meaning that either
> >> >> vendors must add this in their kernels or USB gadget devices based
> >> >> on Linux (such as mobile phones) may not behave as they should.
> >> >> Thus provide a
> >> standard framework for doing this in kernel.
> >> >>
> >> >> Now introduce one user with wm831x_power to support and test the
> >> >> usb charger, which is pending testing. Moreover there may be other
> >> >> potential users will use it in future.
> >> >>
> >> >
> >> > I am afraid I still not find the user (udc driver) for this
> >> > framework, I would like to see how udc driver block the enumeration
> >> > until the charger detection has finished, or am I missing something?
> >>
> >> It is not for udc driver but for power users who want to negotiate
> >> with USB subsystem.
> >>
> >
> > Seems you don't want to guarantee charger type detection is done
> > before gadget connection(pullup DP), right?
> > I see you call usb_charger_detect_type() in each gadget usb state
> changes.
> 
> I am not sure I get your point correctly, please correct me if I
> misunderstand you.
> We need to check the charger type at every event comes from the usb gadget
> state changes or the extcon device state changes, which means a new
> charger plugin or pullup.
> 

According to usb charger spec, my understanding is you can't do real charger
detection procedure *after* gadget _connection_(pullup DP), also I don't
think it's necessary to check charger type at every event from usb gadget.
Something in gadget driver you can utilize is only vbus detection, and
report diff current by diff usb state if it's a SDP.

> >
> > Li Jun
> >> >
> >> > --
> >> > Best Regards,
> >> > Peter Chen
> >>
> >>
> >>
> >> --
> >> Baolin.wang
> >> Best Regards
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-usb"
> >> in the body of a message to majord...@vger.kernel.org More majordomo
> >> info at http://vger.kernel.org/majordomo-info.html
> 
> 
> 
> --
> Baolin.wang
> Best Regards

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Peter Chen]

On Tue, Mar 29, 2016 at 10:23:14AM -0700, Mark Brown wrote:
> On Mon, Mar 28, 2016 at 03:13:51PM +0800, Peter Chen wrote:
> > On Mon, Mar 28, 2016 at 02:51:40PM +0800, Baolin Wang wrote:
> 
> > > > I am afraid I still not find the user (udc driver) for this framework, 
> > > > I would
> > > > like to see how udc driver block the enumeration until the charger 
> > > > detection
> > > > has finished, or am I missing something?
> 
> > > It is not for udc driver but for power users who want to negotiate
> > > with USB subsystem.
> 
> > Then, where is the code the test user to decide what kinds of USB charger
> > (SDP, CDP, DCP) is connecting now?
> 
> Even without detection of CDP and DCP we have configurability within SDP
> - there's the 2.5mA suspended limit, the 100mA default limit and the
> higher 500mA limit which can be negotiated.

Well, things may be a little complicated.

- First, how to design get_charger_type for each udc driver?
Since the charger detection process affects dp/dm signal, it can't be done
during the enumeration or after that. So, the detection process can be only
done after vbus has detected and before udc pull up dp.

Then, when the get_charger_type do real charger detection? My suggestion is,
if the charger type is unknown, we do real one, else, we just return the
stored type value.

- Second, When to notify charger IC to charger:
For SDP and CDP, it needs to notify charger IC after set configuration
has finished.
For DCP (and ACA, I am not sure), can notify charger IC after charger
detection has finished or later.

So, when we get charger is present, we need to notify charger IC at once
for DCP (and ACA); But for SDP and CDP, we need to let the gadget
composite core to notify charger IC after set configuration has finished,
like the patch 2/4 does.

- Third, since composite driver covers 500mA (and more for CDP) after set
configuration and 2mA after suspend, and vbus handler covers connect
and disconnect. I can't see any reasons we need to notify gadget state
for power driver, do we really need to have usb_charger_plug_by_gadget?

-- 
Best Regards,
eter Chen

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Peter Chen]

On Tue, Mar 29, 2016 at 10:23:14AM -0700, Mark Brown wrote:
> On Mon, Mar 28, 2016 at 03:13:51PM +0800, Peter Chen wrote:
> > On Mon, Mar 28, 2016 at 02:51:40PM +0800, Baolin Wang wrote:
> 
> > > > I am afraid I still not find the user (udc driver) for this framework, 
> > > > I would
> > > > like to see how udc driver block the enumeration until the charger 
> > > > detection
> > > > has finished, or am I missing something?
> 
> > > It is not for udc driver but for power users who want to negotiate
> > > with USB subsystem.
> 
> > Then, where is the code the test user to decide what kinds of USB charger
> > (SDP, CDP, DCP) is connecting now?
> 
> Even without detection of CDP and DCP we have configurability within SDP
> - there's the 2.5mA suspended limit, the 100mA default limit and the
> higher 500mA limit which can be negotiated.

Well, things may be a little complicated.

- First, how to design get_charger_type for each udc driver?
Since the charger detection process affects dp/dm signal, it can't be done
during the enumeration or after that. So, the detection process can be only
done after vbus has detected and before udc pull up dp.

Then, when the get_charger_type do real charger detection? My suggestion is,
if the charger type is unknown, we do real one, else, we just return the
stored type value.

- Second, When to notify charger IC to charger:
For SDP and CDP, it needs to notify charger IC after set configuration
has finished.
For DCP (and ACA, I am not sure), can notify charger IC after charger
detection has finished or later.

So, when we get charger is present, we need to notify charger IC at once
for DCP (and ACA); But for SDP and CDP, we need to let the gadget
composite core to notify charger IC after set configuration has finished,
like the patch 2/4 does.

- Third, since composite driver covers 500mA (and more for CDP) after set
configuration and 2mA after suspend, and vbus handler covers connect
and disconnect. I can't see any reasons we need to notify gadget state
for power driver, do we really need to have usb_charger_plug_by_gadget?

-- 
Best Regards,
eter Chen

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Mark Brown]

On Mon, Mar 28, 2016 at 03:13:51PM +0800, Peter Chen wrote:
> On Mon, Mar 28, 2016 at 02:51:40PM +0800, Baolin Wang wrote:

> > > I am afraid I still not find the user (udc driver) for this framework, I 
> > > would
> > > like to see how udc driver block the enumeration until the charger 
> > > detection
> > > has finished, or am I missing something?

> > It is not for udc driver but for power users who want to negotiate
> > with USB subsystem.

> Then, where is the code the test user to decide what kinds of USB charger
> (SDP, CDP, DCP) is connecting now?

Even without detection of CDP and DCP we have configurability within SDP
- there's the 2.5mA suspended limit, the 100mA default limit and the
higher 500mA limit which can be negotiated.


signature.asc
Description: PGP signature

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Mark Brown]

On Mon, Mar 28, 2016 at 03:13:51PM +0800, Peter Chen wrote:
> On Mon, Mar 28, 2016 at 02:51:40PM +0800, Baolin Wang wrote:

> > > I am afraid I still not find the user (udc driver) for this framework, I 
> > > would
> > > like to see how udc driver block the enumeration until the charger 
> > > detection
> > > has finished, or am I missing something?

> > It is not for udc driver but for power users who want to negotiate
> > with USB subsystem.

> Then, where is the code the test user to decide what kinds of USB charger
> (SDP, CDP, DCP) is connecting now?

Even without detection of CDP and DCP we have configurability within SDP
- there's the 2.5mA suspended limit, the 100mA default limit and the
higher 500mA limit which can be negotiated.


signature.asc
Description: PGP signature

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Mark Brown]

On Tue, Mar 29, 2016 at 10:05:23AM +0800, Baolin Wang wrote:

> Yes, The user 'wm831x_power' did not implement any callbacks in
> 'usb_charger_detect_type()' function, but in
> 'usb_charger_detect_type()' function it just supplies different
> callbacks to get the charger type with simple logic. Anyway we can try
> to implement one callback to get the charger type and test the API.

The wm831x doesn't have any charger detection features, what it does
have is hardware for implementing a current limit on the power drawn
from USB.  What it's looking for from this framework is for something to
tell it how much current it should draw from USB so it can program the
hardware appropriately to impose that limit.


signature.asc
Description: PGP signature

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Mark Brown]

On Tue, Mar 29, 2016 at 10:05:23AM +0800, Baolin Wang wrote:

> Yes, The user 'wm831x_power' did not implement any callbacks in
> 'usb_charger_detect_type()' function, but in
> 'usb_charger_detect_type()' function it just supplies different
> callbacks to get the charger type with simple logic. Anyway we can try
> to implement one callback to get the charger type and test the API.

The wm831x doesn't have any charger detection features, what it does
have is hardware for implementing a current limit on the power drawn
from USB.  What it's looking for from this framework is for something to
tell it how much current it should draw from USB so it can program the
hardware appropriately to impose that limit.


signature.asc
Description: PGP signature

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

On 29 March 2016 at 16:45, Jun Li <jun...@nxp.com> wrote:
>
>
>> -Original Message-
>> From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-
>> ow...@vger.kernel.org] On Behalf Of Baolin Wang
>> Sent: Monday, March 28, 2016 2:52 PM
>> To: Peter Chen <hzpeterc...@gmail.com>
>> Cc: Felipe Balbi <ba...@kernel.org>; Greg KH <gre...@linuxfoundation.org>;
>> Sebastian Reichel <s...@kernel.org>; Dmitry Eremin-Solenikov
>> <dbarysh...@gmail.com>; David Woodhouse <dw...@infradead.org>; Peter Chen
>> <peter.c...@freescale.com>; Alan Stern <st...@rowland.harvard.edu>;
>> r.bald...@samsung.com; Yoshihiro Shimoda
>> <yoshihiro.shimoda...@renesas.com>; Lee Jones <lee.jo...@linaro.org>; Mark
>> Brown <broo...@kernel.org>; Charles Keepax
>> <ckee...@opensource.wolfsonmicro.com>; patc...@opensource.wolfsonmicro.com;
>> Linux PM list <linux...@vger.kernel.org>; USB <linux-...@vger.kernel.org>;
>> device-mainlin...@lists.linuxfoundation.org; LKML > ker...@vger.kernel.org>
>> Subject: Re: [PATCH v8 0/4] Introduce usb charger framework to deal with
>> the usb gadget power negotation
>>
>> On 25 March 2016 at 15:09, Peter Chen <hzpeterc...@gmail.com> wrote:
>> > On Thu, Mar 24, 2016 at 08:35:53PM +0800, Baolin Wang wrote:
>> >> Currently the Linux kernel does not provide any standard integration
>> >> of this feature that integrates the USB subsystem with the system
>> >> power regulation provided by PMICs meaning that either vendors must
>> >> add this in their kernels or USB gadget devices based on Linux (such
>> >> as mobile phones) may not behave as they should. Thus provide a
>> standard framework for doing this in kernel.
>> >>
>> >> Now introduce one user with wm831x_power to support and test the usb
>> >> charger, which is pending testing. Moreover there may be other
>> >> potential users will use it in future.
>> >>
>> >
>> > I am afraid I still not find the user (udc driver) for this framework,
>> > I would like to see how udc driver block the enumeration until the
>> > charger detection has finished, or am I missing something?
>>
>> It is not for udc driver but for power users who want to negotiate with
>> USB subsystem.
>>
>
> Seems you don't want to guarantee charger type detection is done before
> gadget connection(pullup DP), right?
> I see you call usb_charger_detect_type() in each gadget usb state changes.

I am not sure I get your point correctly, please correct me if I
misunderstand you.
We need to check the charger type at every event comes from the usb
gadget state changes or the extcon device state changes, which means a
new charger plugin or pullup.

>
> Li Jun
>> >
>> > --
>> > Best Regards,
>> > Peter Chen
>>
>>
>>
>> --
>> Baolin.wang
>> Best Regards
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
>> the body of a message to majord...@vger.kernel.org More majordomo info at
>> http://vger.kernel.org/majordomo-info.html



-- 
Baolin.wang
Best Regards

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

On 29 March 2016 at 16:45, Jun Li  wrote:
>
>
>> -Original Message-
>> From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-
>> ow...@vger.kernel.org] On Behalf Of Baolin Wang
>> Sent: Monday, March 28, 2016 2:52 PM
>> To: Peter Chen 
>> Cc: Felipe Balbi ; Greg KH ;
>> Sebastian Reichel ; Dmitry Eremin-Solenikov
>> ; David Woodhouse ; Peter Chen
>> ; Alan Stern ;
>> r.bald...@samsung.com; Yoshihiro Shimoda
>> ; Lee Jones ; Mark
>> Brown ; Charles Keepax
>> ; patc...@opensource.wolfsonmicro.com;
>> Linux PM list ; USB ;
>> device-mainlin...@lists.linuxfoundation.org; LKML > ker...@vger.kernel.org>
>> Subject: Re: [PATCH v8 0/4] Introduce usb charger framework to deal with
>> the usb gadget power negotation
>>
>> On 25 March 2016 at 15:09, Peter Chen  wrote:
>> > On Thu, Mar 24, 2016 at 08:35:53PM +0800, Baolin Wang wrote:
>> >> Currently the Linux kernel does not provide any standard integration
>> >> of this feature that integrates the USB subsystem with the system
>> >> power regulation provided by PMICs meaning that either vendors must
>> >> add this in their kernels or USB gadget devices based on Linux (such
>> >> as mobile phones) may not behave as they should. Thus provide a
>> standard framework for doing this in kernel.
>> >>
>> >> Now introduce one user with wm831x_power to support and test the usb
>> >> charger, which is pending testing. Moreover there may be other
>> >> potential users will use it in future.
>> >>
>> >
>> > I am afraid I still not find the user (udc driver) for this framework,
>> > I would like to see how udc driver block the enumeration until the
>> > charger detection has finished, or am I missing something?
>>
>> It is not for udc driver but for power users who want to negotiate with
>> USB subsystem.
>>
>
> Seems you don't want to guarantee charger type detection is done before
> gadget connection(pullup DP), right?
> I see you call usb_charger_detect_type() in each gadget usb state changes.

I am not sure I get your point correctly, please correct me if I
misunderstand you.
We need to check the charger type at every event comes from the usb
gadget state changes or the extcon device state changes, which means a
new charger plugin or pullup.

>
> Li Jun
>> >
>> > --
>> > Best Regards,
>> > Peter Chen
>>
>>
>>
>> --
>> Baolin.wang
>> Best Regards
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
>> the body of a message to majord...@vger.kernel.org More majordomo info at
>> http://vger.kernel.org/majordomo-info.html



-- 
Baolin.wang
Best Regards

RE: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Jun Li]

> -Original Message-
> From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-
> ow...@vger.kernel.org] On Behalf Of Baolin Wang
> Sent: Monday, March 28, 2016 2:52 PM
> To: Peter Chen <hzpeterc...@gmail.com>
> Cc: Felipe Balbi <ba...@kernel.org>; Greg KH <gre...@linuxfoundation.org>;
> Sebastian Reichel <s...@kernel.org>; Dmitry Eremin-Solenikov
> <dbarysh...@gmail.com>; David Woodhouse <dw...@infradead.org>; Peter Chen
> <peter.c...@freescale.com>; Alan Stern <st...@rowland.harvard.edu>;
> r.bald...@samsung.com; Yoshihiro Shimoda
> <yoshihiro.shimoda...@renesas.com>; Lee Jones <lee.jo...@linaro.org>; Mark
> Brown <broo...@kernel.org>; Charles Keepax
> <ckee...@opensource.wolfsonmicro.com>; patc...@opensource.wolfsonmicro.com;
> Linux PM list <linux...@vger.kernel.org>; USB <linux-...@vger.kernel.org>;
> device-mainlin...@lists.linuxfoundation.org; LKML  ker...@vger.kernel.org>
> Subject: Re: [PATCH v8 0/4] Introduce usb charger framework to deal with
> the usb gadget power negotation
> 
> On 25 March 2016 at 15:09, Peter Chen <hzpeterc...@gmail.com> wrote:
> > On Thu, Mar 24, 2016 at 08:35:53PM +0800, Baolin Wang wrote:
> >> Currently the Linux kernel does not provide any standard integration
> >> of this feature that integrates the USB subsystem with the system
> >> power regulation provided by PMICs meaning that either vendors must
> >> add this in their kernels or USB gadget devices based on Linux (such
> >> as mobile phones) may not behave as they should. Thus provide a
> standard framework for doing this in kernel.
> >>
> >> Now introduce one user with wm831x_power to support and test the usb
> >> charger, which is pending testing. Moreover there may be other
> >> potential users will use it in future.
> >>
> >
> > I am afraid I still not find the user (udc driver) for this framework,
> > I would like to see how udc driver block the enumeration until the
> > charger detection has finished, or am I missing something?
> 
> It is not for udc driver but for power users who want to negotiate with
> USB subsystem.
> 

Seems you don't want to guarantee charger type detection is done before
gadget connection(pullup DP), right?
I see you call usb_charger_detect_type() in each gadget usb state changes.

Li Jun  
> >
> > --
> > Best Regards,
> > Peter Chen
> 
> 
> 
> --
> Baolin.wang
> Best Regards
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majord...@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html

RE: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Jun Li]

> -Original Message-
> From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-
> ow...@vger.kernel.org] On Behalf Of Baolin Wang
> Sent: Monday, March 28, 2016 2:52 PM
> To: Peter Chen 
> Cc: Felipe Balbi ; Greg KH ;
> Sebastian Reichel ; Dmitry Eremin-Solenikov
> ; David Woodhouse ; Peter Chen
> ; Alan Stern ;
> r.bald...@samsung.com; Yoshihiro Shimoda
> ; Lee Jones ; Mark
> Brown ; Charles Keepax
> ; patc...@opensource.wolfsonmicro.com;
> Linux PM list ; USB ;
> device-mainlin...@lists.linuxfoundation.org; LKML  ker...@vger.kernel.org>
> Subject: Re: [PATCH v8 0/4] Introduce usb charger framework to deal with
> the usb gadget power negotation
> 
> On 25 March 2016 at 15:09, Peter Chen  wrote:
> > On Thu, Mar 24, 2016 at 08:35:53PM +0800, Baolin Wang wrote:
> >> Currently the Linux kernel does not provide any standard integration
> >> of this feature that integrates the USB subsystem with the system
> >> power regulation provided by PMICs meaning that either vendors must
> >> add this in their kernels or USB gadget devices based on Linux (such
> >> as mobile phones) may not behave as they should. Thus provide a
> standard framework for doing this in kernel.
> >>
> >> Now introduce one user with wm831x_power to support and test the usb
> >> charger, which is pending testing. Moreover there may be other
> >> potential users will use it in future.
> >>
> >
> > I am afraid I still not find the user (udc driver) for this framework,
> > I would like to see how udc driver block the enumeration until the
> > charger detection has finished, or am I missing something?
> 
> It is not for udc driver but for power users who want to negotiate with
> USB subsystem.
> 

Seems you don't want to guarantee charger type detection is done before
gadget connection(pullup DP), right?
I see you call usb_charger_detect_type() in each gadget usb state changes.

Li Jun  
> >
> > --
> > Best Regards,
> > Peter Chen
> 
> 
> 
> --
> Baolin.wang
> Best Regards
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majord...@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

On 29 March 2016 at 08:32, Peter Chen  wrote:
>
>>
>> On 28 March 2016 at 15:13, Peter Chen  wrote:
>> > On Mon, Mar 28, 2016 at 02:51:40PM +0800, Baolin Wang wrote:
>> >> On 25 March 2016 at 15:09, Peter Chen  wrote:
>> >> > On Thu, Mar 24, 2016 at 08:35:53PM +0800, Baolin Wang wrote:
>> >> >> Currently the Linux kernel does not provide any standard
>> >> >> integration of this feature that integrates the USB subsystem with
>> >> >> the system power regulation provided by PMICs meaning that either
>> >> >> vendors must add this in their kernels or USB gadget devices based
>> >> >> on Linux (such as mobile phones) may not behave as they should. Thus
>> provide a standard framework for doing this in kernel.
>> >> >>
>> >> >> Now introduce one user with wm831x_power to support and test the
>> >> >> usb charger, which is pending testing. Moreover there may be other
>> >> >> potential users will use it in future.
>> >> >>
>> >> >
>> >> > I am afraid I still not find the user (udc driver) for this
>> >> > framework, I would like to see how udc driver block the enumeration
>> >> > until the charger detection has finished, or am I missing something?
>> >>
>> >> It is not for udc driver but for power users who want to negotiate
>> >> with USB subsystem.
>> >>
>> >
>> > Then, where is the code the test user to decide what kinds of USB
>> > charger (SDP, CDP, DCP) is connecting now?
>>
>> Users can choose to implement multiple ways to get the charger type In
>> 'usb_charger_detect_type()' function.
>>
>> >
>
> Then, how you test your code? When we introduce an API, at least, there is one
> user for it.

At first I've tested the API on my board to make sure it can work
well, another hand we introduce one user 'wm831x_power' to support and
test the usb charger.

Yes, The user 'wm831x_power' did not implement any callbacks in
'usb_charger_detect_type()' function, but in
'usb_charger_detect_type()' function it just supplies different
callbacks to get the charger type with simple logic. Anyway we can try
to implement one callback to get the charger type and test the API.
Thanks.

>
> Peter



-- 
Baolin.wang
Best Regards

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

On 29 March 2016 at 08:32, Peter Chen  wrote:
>
>>
>> On 28 March 2016 at 15:13, Peter Chen  wrote:
>> > On Mon, Mar 28, 2016 at 02:51:40PM +0800, Baolin Wang wrote:
>> >> On 25 March 2016 at 15:09, Peter Chen  wrote:
>> >> > On Thu, Mar 24, 2016 at 08:35:53PM +0800, Baolin Wang wrote:
>> >> >> Currently the Linux kernel does not provide any standard
>> >> >> integration of this feature that integrates the USB subsystem with
>> >> >> the system power regulation provided by PMICs meaning that either
>> >> >> vendors must add this in their kernels or USB gadget devices based
>> >> >> on Linux (such as mobile phones) may not behave as they should. Thus
>> provide a standard framework for doing this in kernel.
>> >> >>
>> >> >> Now introduce one user with wm831x_power to support and test the
>> >> >> usb charger, which is pending testing. Moreover there may be other
>> >> >> potential users will use it in future.
>> >> >>
>> >> >
>> >> > I am afraid I still not find the user (udc driver) for this
>> >> > framework, I would like to see how udc driver block the enumeration
>> >> > until the charger detection has finished, or am I missing something?
>> >>
>> >> It is not for udc driver but for power users who want to negotiate
>> >> with USB subsystem.
>> >>
>> >
>> > Then, where is the code the test user to decide what kinds of USB
>> > charger (SDP, CDP, DCP) is connecting now?
>>
>> Users can choose to implement multiple ways to get the charger type In
>> 'usb_charger_detect_type()' function.
>>
>> >
>
> Then, how you test your code? When we introduce an API, at least, there is one
> user for it.

At first I've tested the API on my board to make sure it can work
well, another hand we introduce one user 'wm831x_power' to support and
test the usb charger.

Yes, The user 'wm831x_power' did not implement any callbacks in
'usb_charger_detect_type()' function, but in
'usb_charger_detect_type()' function it just supplies different
callbacks to get the charger type with simple logic. Anyway we can try
to implement one callback to get the charger type and test the API.
Thanks.

>
> Peter



-- 
Baolin.wang
Best Regards

RE: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Peter Chen]

 
> 
> On 28 March 2016 at 15:13, Peter Chen  wrote:
> > On Mon, Mar 28, 2016 at 02:51:40PM +0800, Baolin Wang wrote:
> >> On 25 March 2016 at 15:09, Peter Chen  wrote:
> >> > On Thu, Mar 24, 2016 at 08:35:53PM +0800, Baolin Wang wrote:
> >> >> Currently the Linux kernel does not provide any standard
> >> >> integration of this feature that integrates the USB subsystem with
> >> >> the system power regulation provided by PMICs meaning that either
> >> >> vendors must add this in their kernels or USB gadget devices based
> >> >> on Linux (such as mobile phones) may not behave as they should. Thus
> provide a standard framework for doing this in kernel.
> >> >>
> >> >> Now introduce one user with wm831x_power to support and test the
> >> >> usb charger, which is pending testing. Moreover there may be other
> >> >> potential users will use it in future.
> >> >>
> >> >
> >> > I am afraid I still not find the user (udc driver) for this
> >> > framework, I would like to see how udc driver block the enumeration
> >> > until the charger detection has finished, or am I missing something?
> >>
> >> It is not for udc driver but for power users who want to negotiate
> >> with USB subsystem.
> >>
> >
> > Then, where is the code the test user to decide what kinds of USB
> > charger (SDP, CDP, DCP) is connecting now?
> 
> Users can choose to implement multiple ways to get the charger type In
> 'usb_charger_detect_type()' function.
> 
> >
 
Then, how you test your code? When we introduce an API, at least, there is one
user for it.

Peter

RE: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Peter Chen]

 
> 
> On 28 March 2016 at 15:13, Peter Chen  wrote:
> > On Mon, Mar 28, 2016 at 02:51:40PM +0800, Baolin Wang wrote:
> >> On 25 March 2016 at 15:09, Peter Chen  wrote:
> >> > On Thu, Mar 24, 2016 at 08:35:53PM +0800, Baolin Wang wrote:
> >> >> Currently the Linux kernel does not provide any standard
> >> >> integration of this feature that integrates the USB subsystem with
> >> >> the system power regulation provided by PMICs meaning that either
> >> >> vendors must add this in their kernels or USB gadget devices based
> >> >> on Linux (such as mobile phones) may not behave as they should. Thus
> provide a standard framework for doing this in kernel.
> >> >>
> >> >> Now introduce one user with wm831x_power to support and test the
> >> >> usb charger, which is pending testing. Moreover there may be other
> >> >> potential users will use it in future.
> >> >>
> >> >
> >> > I am afraid I still not find the user (udc driver) for this
> >> > framework, I would like to see how udc driver block the enumeration
> >> > until the charger detection has finished, or am I missing something?
> >>
> >> It is not for udc driver but for power users who want to negotiate
> >> with USB subsystem.
> >>
> >
> > Then, where is the code the test user to decide what kinds of USB
> > charger (SDP, CDP, DCP) is connecting now?
> 
> Users can choose to implement multiple ways to get the charger type In
> 'usb_charger_detect_type()' function.
> 
> >
 
Then, how you test your code? When we introduce an API, at least, there is one
user for it.

Peter

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

On 28 March 2016 at 15:13, Peter Chen  wrote:
> On Mon, Mar 28, 2016 at 02:51:40PM +0800, Baolin Wang wrote:
>> On 25 March 2016 at 15:09, Peter Chen  wrote:
>> > On Thu, Mar 24, 2016 at 08:35:53PM +0800, Baolin Wang wrote:
>> >> Currently the Linux kernel does not provide any standard integration of 
>> >> this
>> >> feature that integrates the USB subsystem with the system power regulation
>> >> provided by PMICs meaning that either vendors must add this in their 
>> >> kernels
>> >> or USB gadget devices based on Linux (such as mobile phones) may not 
>> >> behave
>> >> as they should. Thus provide a standard framework for doing this in 
>> >> kernel.
>> >>
>> >> Now introduce one user with wm831x_power to support and test the usb 
>> >> charger,
>> >> which is pending testing. Moreover there may be other potential users 
>> >> will use
>> >> it in future.
>> >>
>> >
>> > I am afraid I still not find the user (udc driver) for this framework, I 
>> > would
>> > like to see how udc driver block the enumeration until the charger 
>> > detection
>> > has finished, or am I missing something?
>>
>> It is not for udc driver but for power users who want to negotiate
>> with USB subsystem.
>>
>
> Then, where is the code the test user to decide what kinds of USB charger
> (SDP, CDP, DCP) is connecting now?

Users can choose to implement multiple ways to get the charger type In
'usb_charger_detect_type()' function.

>
> --
> Best Regards,
> Peter Chen



-- 
Baolin.wang
Best Regards

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

On 28 March 2016 at 15:13, Peter Chen  wrote:
> On Mon, Mar 28, 2016 at 02:51:40PM +0800, Baolin Wang wrote:
>> On 25 March 2016 at 15:09, Peter Chen  wrote:
>> > On Thu, Mar 24, 2016 at 08:35:53PM +0800, Baolin Wang wrote:
>> >> Currently the Linux kernel does not provide any standard integration of 
>> >> this
>> >> feature that integrates the USB subsystem with the system power regulation
>> >> provided by PMICs meaning that either vendors must add this in their 
>> >> kernels
>> >> or USB gadget devices based on Linux (such as mobile phones) may not 
>> >> behave
>> >> as they should. Thus provide a standard framework for doing this in 
>> >> kernel.
>> >>
>> >> Now introduce one user with wm831x_power to support and test the usb 
>> >> charger,
>> >> which is pending testing. Moreover there may be other potential users 
>> >> will use
>> >> it in future.
>> >>
>> >
>> > I am afraid I still not find the user (udc driver) for this framework, I 
>> > would
>> > like to see how udc driver block the enumeration until the charger 
>> > detection
>> > has finished, or am I missing something?
>>
>> It is not for udc driver but for power users who want to negotiate
>> with USB subsystem.
>>
>
> Then, where is the code the test user to decide what kinds of USB charger
> (SDP, CDP, DCP) is connecting now?

Users can choose to implement multiple ways to get the charger type In
'usb_charger_detect_type()' function.

>
> --
> Best Regards,
> Peter Chen



-- 
Baolin.wang
Best Regards

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Peter Chen]

On Mon, Mar 28, 2016 at 02:51:40PM +0800, Baolin Wang wrote:
> On 25 March 2016 at 15:09, Peter Chen  wrote:
> > On Thu, Mar 24, 2016 at 08:35:53PM +0800, Baolin Wang wrote:
> >> Currently the Linux kernel does not provide any standard integration of 
> >> this
> >> feature that integrates the USB subsystem with the system power regulation
> >> provided by PMICs meaning that either vendors must add this in their 
> >> kernels
> >> or USB gadget devices based on Linux (such as mobile phones) may not behave
> >> as they should. Thus provide a standard framework for doing this in kernel.
> >>
> >> Now introduce one user with wm831x_power to support and test the usb 
> >> charger,
> >> which is pending testing. Moreover there may be other potential users will 
> >> use
> >> it in future.
> >>
> >
> > I am afraid I still not find the user (udc driver) for this framework, I 
> > would
> > like to see how udc driver block the enumeration until the charger detection
> > has finished, or am I missing something?
> 
> It is not for udc driver but for power users who want to negotiate
> with USB subsystem.
> 

Then, where is the code the test user to decide what kinds of USB charger
(SDP, CDP, DCP) is connecting now?

-- 
Best Regards,
Peter Chen

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Peter Chen]

On Mon, Mar 28, 2016 at 02:51:40PM +0800, Baolin Wang wrote:
> On 25 March 2016 at 15:09, Peter Chen  wrote:
> > On Thu, Mar 24, 2016 at 08:35:53PM +0800, Baolin Wang wrote:
> >> Currently the Linux kernel does not provide any standard integration of 
> >> this
> >> feature that integrates the USB subsystem with the system power regulation
> >> provided by PMICs meaning that either vendors must add this in their 
> >> kernels
> >> or USB gadget devices based on Linux (such as mobile phones) may not behave
> >> as they should. Thus provide a standard framework for doing this in kernel.
> >>
> >> Now introduce one user with wm831x_power to support and test the usb 
> >> charger,
> >> which is pending testing. Moreover there may be other potential users will 
> >> use
> >> it in future.
> >>
> >
> > I am afraid I still not find the user (udc driver) for this framework, I 
> > would
> > like to see how udc driver block the enumeration until the charger detection
> > has finished, or am I missing something?
> 
> It is not for udc driver but for power users who want to negotiate
> with USB subsystem.
> 

Then, where is the code the test user to decide what kinds of USB charger
(SDP, CDP, DCP) is connecting now?

-- 
Best Regards,
Peter Chen

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

On 25 March 2016 at 15:09, Peter Chen  wrote:
> On Thu, Mar 24, 2016 at 08:35:53PM +0800, Baolin Wang wrote:
>> Currently the Linux kernel does not provide any standard integration of this
>> feature that integrates the USB subsystem with the system power regulation
>> provided by PMICs meaning that either vendors must add this in their kernels
>> or USB gadget devices based on Linux (such as mobile phones) may not behave
>> as they should. Thus provide a standard framework for doing this in kernel.
>>
>> Now introduce one user with wm831x_power to support and test the usb charger,
>> which is pending testing. Moreover there may be other potential users will 
>> use
>> it in future.
>>
>
> I am afraid I still not find the user (udc driver) for this framework, I would
> like to see how udc driver block the enumeration until the charger detection
> has finished, or am I missing something?

It is not for udc driver but for power users who want to negotiate
with USB subsystem.

>
> --
> Best Regards,
> Peter Chen



-- 
Baolin.wang
Best Regards

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Baolin Wang]

On 25 March 2016 at 15:09, Peter Chen  wrote:
> On Thu, Mar 24, 2016 at 08:35:53PM +0800, Baolin Wang wrote:
>> Currently the Linux kernel does not provide any standard integration of this
>> feature that integrates the USB subsystem with the system power regulation
>> provided by PMICs meaning that either vendors must add this in their kernels
>> or USB gadget devices based on Linux (such as mobile phones) may not behave
>> as they should. Thus provide a standard framework for doing this in kernel.
>>
>> Now introduce one user with wm831x_power to support and test the usb charger,
>> which is pending testing. Moreover there may be other potential users will 
>> use
>> it in future.
>>
>
> I am afraid I still not find the user (udc driver) for this framework, I would
> like to see how udc driver block the enumeration until the charger detection
> has finished, or am I missing something?

It is not for udc driver but for power users who want to negotiate
with USB subsystem.

>
> --
> Best Regards,
> Peter Chen



-- 
Baolin.wang
Best Regards

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Peter Chen]

On Thu, Mar 24, 2016 at 08:35:53PM +0800, Baolin Wang wrote:
> Currently the Linux kernel does not provide any standard integration of this
> feature that integrates the USB subsystem with the system power regulation
> provided by PMICs meaning that either vendors must add this in their kernels
> or USB gadget devices based on Linux (such as mobile phones) may not behave
> as they should. Thus provide a standard framework for doing this in kernel.
> 
> Now introduce one user with wm831x_power to support and test the usb charger,
> which is pending testing. Moreover there may be other potential users will use
> it in future.
> 

I am afraid I still not find the user (udc driver) for this framework, I would
like to see how udc driver block the enumeration until the charger detection
has finished, or am I missing something?

-- 
Best Regards,
Peter Chen

Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Peter Chen]

On Thu, Mar 24, 2016 at 08:35:53PM +0800, Baolin Wang wrote:
> Currently the Linux kernel does not provide any standard integration of this
> feature that integrates the USB subsystem with the system power regulation
> provided by PMICs meaning that either vendors must add this in their kernels
> or USB gadget devices based on Linux (such as mobile phones) may not behave
> as they should. Thus provide a standard framework for doing this in kernel.
> 
> Now introduce one user with wm831x_power to support and test the usb charger,
> which is pending testing. Moreover there may be other potential users will use
> it in future.
> 

I am afraid I still not find the user (udc driver) for this framework, I would
like to see how udc driver block the enumeration until the charger detection
has finished, or am I missing something?

-- 
Best Regards,
Peter Chen