Introduce usb charger framework to deal with the usb gadget power negotation

drivers/usb/gadget/charger.c new file +5 -0

+/*
+ * Sysfs attributes:
+ *
+ * These sysfs attributes are used for showing and setting different type
+ * (SDP/DCP/CDP/ACA) chargers' current limitation.

Hi!

Quoted 9 lines

drivers/usb/gadget/charger.c new file +9 -0

+/*
+ * Sysfs attributes:
+ *
+ * These sysfs attributes are used for showing and setting different type
+ * (SDP/DCP/CDP/ACA) chargers' current limitation.
+ */
+static ssize_t sdp_limit_show(struct device *dev,
+                           struct device_attribute *attr,
+                           char *buf)

drivers/usb/gadget/charger.c line 60

Quoted 6 lines

drivers/usb/gadget/charger.c new file +6 -0

+{
+     struct usb_charger *uchger = dev_to_uchger(dev);
+
+     return sprintf(buf, "%d\n", uchger->cur_limit.sdp_cur_limit);
+}
+

Sysfs attributes... can we get appropriate documentation for them?

Thanks,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

drivers/power/wm831x_power.c +3 -0

@@ -13,6 +13,7 @@
 #include <linux/platform_device.h>
 #include <linux/power_supply.h>
 #include <linux/slab.h>
+#include <linux/usb/usb_charger.h>
 
 #include <linux/mfd/wm831x/core.h>
 #include <linux/mfd/wm831x/auxadc.h>

@@ -31,6 +32,8 @@ struct wm831x_power {
        char usb_name[20];
        char battery_name[20];
        bool have_battery;
+       struct usb_charger *usb_charger;
+       struct notifier_block usb_notify;
 };
 
 static int wm831x_power_check_online(struct wm831x *wm831x, int supply,

drivers/power/wm831x_power.c +7 -0

+       0,
+       2,
+       100,
+       500,
+       900,
+       1500,
+       1800,

Quoted 23 lines · Baolin Wang

drivers/power/wm831x_power.c +3 -0

@@ -13,6 +13,7 @@
 #include <linux/platform_device.h>
 #include <linux/power_supply.h>
 #include <linux/slab.h>
+#include <linux/usb/usb_charger.h>
 
 #include <linux/mfd/wm831x/core.h>
 #include <linux/mfd/wm831x/auxadc.h>

@@ -31,6 +32,8 @@ struct wm831x_power {
        char usb_name[20];
        char battery_name[20];
        bool have_battery;
+       struct usb_charger *usb_charger;
+       struct notifier_block usb_notify;
 };
 
 static int wm831x_power_check_online(struct wm831x *wm831x, int supply,

@@ -125,6 +128,43 @@ static enum power_supply_property wm831x_usb_props[] = {
        POWER_SUPPLY_PROP_VOLTAGE_NOW,
 };
 

drivers/power/wm831x_power.c static enum power_supply_property wm831x_usb_props[] = { · line 132

Quoted 2 lines

drivers/power/wm831x_power.c +2 -0

+/* In milliamps */
+static unsigned int wm831x_usb_limits[] = {

const

Quoted 12 lines

drivers/power/wm831x_power.c +11 -0

+       0,
+       2,
+       100,
+       500,
+       900,
+       1500,
+       1800,
+       550,
+};
+
+static int wm831x_usb_limit_change(struct notifier_block *nb,

drivers/power/wm831x_power.c static enum power_supply_property wm831x_usb_props[] = { · line 149

Quoted 6 lines

drivers/power/wm831x_power.c +6 -0

+                                  unsigned long limit, void *data)
+{
+       struct wm831x_power *wm831x_power = container_of(nb,
+                                                        struct wm831x_power,
+                                                        usb_notify);
+       int i, best;

unsigned int

drivers/power/wm831x_power.c static enum power_supply_property wm831x_usb_props[] = { · line 160

Quoted 12 lines

drivers/power/wm831x_power.c +11 -0

+
+       /* Find the highest supported limit */
+       best = 0;
+       for (i = 0; i < ARRAY_SIZE(wm831x_usb_limits); i++) {
+               if (limit >= wm831x_usb_limits[i] &&
+                   wm831x_usb_limits[best] < wm831x_usb_limits[i])
+                       best = i;
+       }
+
+       dev_dbg(wm831x_power->wm831x->dev,
+               "Limiting USB current to %dmA", wm831x_usb_limits[best]);

%u

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

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.

Quoted 6 lines · Baolin Wang

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.

Quoted 4 lines

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

Quoted 12 lines · Peter Chen

Quoted 10 lines · Baolin Wang

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.

Quoted 3 lines

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.

Quoted 17 lines · Baolin Wang

Quoted 15 lines · Peter Chen

Quoted 10 lines · Baolin Wang

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?

Quoted 3 lines

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

Quoted 29 lines · Peter Chen

Quoted 8 lines · Baolin Wang

Quoted 7 lines · Peter Chen

Quoted 6 lines · Baolin Wang

Quoted 5 lines

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.

Quoted 16 lines

Quoted 6 lines

Quoted 6 lines

Quoted 5 lines

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.

Quoted 7 lines

Quoted 3 lines

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?

Quoted 4 lines

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

Quoted 1 line

Then, how you test your code? When we introduce an API, at least, there is one user for it.

Peter

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.

Quoted 6 lines · Baolin Wang

Quoted 3 lines

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?

Quoted 8 lines · Peter Chen

Quoted 6 lines · Baolin Wang

Quoted 3 lines

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.

Quoted 2 lines

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.

Quoted 10 lines · Mark Brown

Quoted 8 lines · Peter Chen

Quoted 6 lines · Baolin Wang

Quoted 3 lines

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.

Quoted 6 lines

Quoted 2 lines

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

Quoted 11 lines · Peter Chen

Quoted 10 lines · Mark Brown

Quoted 8 lines · Peter Chen

Quoted 6 lines · Baolin Wang

Quoted 3 lines

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.

Quoted 8 lines

Quoted 6 lines

Quoted 2 lines

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.

Quoted 9 lines

- 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.

Quoted 7 lines

- 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.

Quoted 4 lines

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.

Quoted 5 lines

- 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.

Quoted 49 lines · Baolin Wang

Quoted 11 lines · Peter Chen

Quoted 10 lines · Mark Brown

Quoted 8 lines · Peter Chen

Quoted 6 lines · Baolin Wang

Quoted 3 lines

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.

Quoted 8 lines

Quoted 6 lines

Quoted 2 lines

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.

Quoted 9 lines

- 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.

Quoted 11 lines

- 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.

Quoted 3 lines

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.

Quoted 7 lines

Quoted 6 lines

- 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?

Quoted 6 lines

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

Quoted 5 lines

Quoted 5 lines

Quoted 4 lines

- 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?

Quoted 8 lines

Quoted 6 lines

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.

Quoted 14 lines · Baolin Wang

Quoted 5 lines

Quoted 5 lines

Quoted 4 lines

- 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?

Quoted 8 lines

Quoted 6 lines

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?

Quoted 11 lines

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

Quoted 39 lines

-----Original Message-----
From: [email protected] [mailto:linux-usb- [email protected]] On Behalf Of Baolin Wang
Sent: Monday, March 28, 2016 2:52 PM
To: Peter Chen <[email protected]>
Cc: Felipe Balbi <[email protected]>; Greg KH <[email protected]>; Sebastian Reichel <[email protected]>; Dmitry Eremin-Solenikov <[email protected]>; David Woodhouse <[email protected]>; Peter Chen <[email protected]>; Alan Stern <[email protected]>; [email protected]; Yoshihiro Shimoda <[email protected]>; Lee Jones <[email protected]>; Mark Brown <[email protected]>; Charles Keepax <[email protected]>; [email protected]; Linux PM list <[email protected]>; USB <[email protected]>; [email protected]; LKML <linux- [email protected]>
Subject: Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

Quoted 6 lines · Peter Chen

Quoted 5 lines · Baolin Wang

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.

Quoted 9 lines

Quoted 5 lines

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?

Quoted 3 lines

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

Quoted 14 lines

Quoted 4 lines

--
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 [email protected] More majordomo info at
http://vger.kernel.org/majordomo-info.html

-----Original Message-----
From: Baolin Wang [mailto:[email protected]]
Sent: Tuesday, March 29, 2016 5:49 PM
To: Jun Li <[email protected]>
Cc: Peter Chen <[email protected]>; Felipe Balbi <[email protected]>; Greg KH <[email protected]>; Sebastian Reichel <[email protected]>; Dmitry Eremin-Solenikov <[email protected]>; David Woodhouse <[email protected]>; Peter Chen <[email protected]>; Alan Stern <[email protected]>; [email protected]; Yoshihiro Shimoda <[email protected]>; Lee Jones <[email protected]>; Mark Brown <[email protected]>; Charles Keepax <[email protected]>; [email protected]; Linux PM list <[email protected]>; USB <[email protected]>; [email protected]; LKML <linux- [email protected]>
Subject: Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

Quoted 42 lines · Jun Li

Quoted 42 lines

-----Original Message-----
From: [email protected] [mailto:linux-usb- [email protected]] On Behalf Of Baolin Wang
Sent: Monday, March 28, 2016 2:52 PM
To: Peter Chen <[email protected]>
Cc: Felipe Balbi <[email protected]>; Greg KH <[email protected]>; Sebastian Reichel <[email protected]>; Dmitry Eremin-Solenikov <[email protected]>; David Woodhouse <[email protected]>; Peter Chen <[email protected]>; Alan Stern <[email protected]>; [email protected]; Yoshihiro Shimoda <[email protected]>; Lee Jones <[email protected]>; Mark Brown <[email protected]>; Charles Keepax <[email protected]>; [email protected];
Linux PM list <[email protected]>; USB <[email protected]>; [email protected]; LKML <linux- [email protected]>
Subject: Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

Quoted 7 lines · Peter Chen

Quoted 6 lines · Baolin Wang

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.

Quoted 9 lines

Quoted 5 lines

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?

Quoted 7 lines

Quoted 3 lines

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.

Quoted 16 lines

Li Jun

Quoted 14 lines

Quoted 4 lines

--
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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Baolin.wang
Best Regards

Quoted 7 lines

Quoted 7 lines

Quoted 3 lines

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

Quoted 8 lines

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.

-----Original Message-----
From: Baolin Wang [mailto:[email protected]]
Sent: Wednesday, March 30, 2016 2:15 PM
To: Jun Li <[email protected]>
Cc: Peter Chen <[email protected]>; Felipe Balbi <[email protected]>; Greg KH <[email protected]>; Sebastian Reichel <[email protected]>; Dmitry Eremin-Solenikov <[email protected]>; David Woodhouse <[email protected]>; Peter Chen <[email protected]>; Alan Stern <[email protected]>; [email protected]; Yoshihiro Shimoda <[email protected]>; Lee Jones <[email protected]>; Mark Brown <[email protected]>; Charles Keepax <[email protected]>; [email protected]; Linux PM list <[email protected]>; USB <[email protected]>; [email protected]; LKML <linux- [email protected]>
Subject: Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

Quoted 16 lines · Jun Li

Quoted 15 lines

Quoted 7 lines

Quoted 3 lines

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.

Quoted 3 lines

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.

Quoted 2 lines

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.

Quoted 3 lines

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

Hi

Quoted 17 lines · Jun Li

Quoted 16 lines

Quoted 15 lines

Quoted 7 lines

Quoted 3 lines

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.

Quoted 5 lines

Quoted 3 lines

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&after gadget connect (pullup DP), you should already know the charger type.

Li Jun

Quoted 4 lines

Quoted 2 lines

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

gadget.

Quoted 4 lines

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);
}

Quoted 7 lines

Quoted 3 lines

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

Quoted 57 lines

-----Original Message-----
From: Baolin Wang [mailto:[email protected]]
Sent: Wednesday, March 30, 2016 5:31 PM
To: Jun Li <[email protected]>
Cc: Peter Chen <[email protected]>; Felipe Balbi <[email protected]>; Greg KH <[email protected]>; Sebastian Reichel <[email protected]>; Dmitry Eremin-Solenikov <[email protected]>; David Woodhouse <[email protected]>; Peter Chen <[email protected]>; Alan Stern <[email protected]>; [email protected]; Yoshihiro Shimoda <[email protected]>; Lee Jones <[email protected]>; Mark Brown <[email protected]>; Charles Keepax <[email protected]>; [email protected]; Linux PM list <[email protected]>; USB <[email protected]>; [email protected]; LKML <linux- [email protected]>
Subject: Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

Quoted 37 lines · Jun Li

Hi

Quoted 18 lines · Jun Li

Quoted 17 lines

Quoted 16 lines

Quoted 8 lines

Quoted 3 lines

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.

Quoted 5 lines

Quoted 3 lines

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&after gadget connect (pullup DP), you should already know the charger type.

Quoted 3 lines

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

Quoted 36 lines

Quoted 35 lines

Quoted 23 lines

Quoted 14 lines

Quoted 14 lines

Quoted 13 lines

Quoted 5 lines

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.

Quoted 5 lines

Quoted 3 lines

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:

Quoted 8 lines

"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&after 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.

Quoted 2 lines

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

[ text/plain ]

Quoted 39 lines · Felipe Balbi

Quoted 38 lines

Quoted 28 lines

Quoted 28 lines

Quoted 14 lines

Quoted 14 lines

Quoted 13 lines

Quoted 5 lines

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.

Quoted 5 lines

Quoted 3 lines

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."

Quoted 7 lines

Quoted 3 lines

As Peter mentioned, the charger detection should happen between VBUS detection and gadget pull up DP for first plug in case. So when&after 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

Quoted 3 lines

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.

Hi,

Quoted 41 lines · Baolin Wang

[ text/plain ]

Quoted 39 lines · Felipe Balbi

Quoted 38 lines

Quoted 28 lines

Quoted 28 lines

Quoted 14 lines

Quoted 14 lines

Quoted 13 lines

Quoted 5 lines

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.

Quoted 5 lines

Quoted 3 lines

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."

Quoted 7 lines

Quoted 3 lines

As Peter mentioned, the charger detection should happen between VBUS detection and gadget pull up DP for first plug in case. So when&after 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

Quoted 8 lines

Quoted 3 lines

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.

Quoted 3 lines

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

Quoted 31 lines

Quoted 30 lines

Quoted 13 lines

Quoted 13 lines

Quoted 12 lines

Quoted 4 lines

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.

Quoted 5 lines

Quoted 3 lines

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&after gadget connect (pullup DP), you should already know the charger type.

Quoted 3 lines

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.

Hi

Quoted 55 lines

-----Original Message-----
From: Baolin Wang [mailto:[email protected]]
Sent: Thursday, March 31, 2016 1:23 PM
To: Jun Li <[email protected]>
Cc: Peter Chen <[email protected]>; Felipe Balbi <[email protected]>; Greg KH <[email protected]>; Sebastian Reichel <[email protected]>; Dmitry Eremin-Solenikov <[email protected]>; David Woodhouse <[email protected]>; Peter Chen <[email protected]>; Alan Stern <[email protected]>; [email protected]; Yoshihiro Shimoda <[email protected]>; Lee Jones <[email protected]>; Mark Brown <[email protected]>; Charles Keepax <[email protected]>; [email protected]; Linux PM list <[email protected]>; USB <[email protected]>; [email protected]; LKML <linux- [email protected]>
Subject: Re: [PATCH v8 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

Quoted 22 lines · Jun Li

Quoted 22 lines

Quoted 22 lines

Quoted 13 lines

Quoted 13 lines

Quoted 12 lines

Quoted 4 lines

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.

Quoted 5 lines

Quoted 3 lines

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:

Quoted 8 lines

Quoted 8 lines

Quoted 8 lines

"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&after gadget connect (pullup DP), you should already know the

charger type.

Quoted 5 lines

Quoted 4 lines

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.

Quoted 8 lines

Quoted 3 lines

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().

Quoted 4 lines

--
Baolin.wang
Best Regards

Quoted 13 lines

Quoted 4 lines

Quoted 4 lines

Quoted 3 lines

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.

Quoted 8 lines

Quoted 3 lines

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.

Quoted 6 lines

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.

Hi Baolin,

Quoted 20 lines · Baolin Wang

Quoted 13 lines

Quoted 4 lines

Quoted 4 lines

Quoted 3 lines

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.

Quoted 8 lines

Quoted 3 lines

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.

Quoted 6 lines

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().

Quoted 3 lines

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