[RFC][PATCH 0/3] add EXTCON_CHG_USB_* cables to MSM USB phy

[RFC][PATCH 0/3] add EXTCON_CHG_USB_* cables to MSM USB phy

[Damien Riegel]

This patchset adds a way for the MSM USB phy to notify a power supply
when the charging state changes. It achieves that using the extcon
subsystem.

The first patch makes sure msm_otg_notify_charger is called after the
charger attributes have been set.
The second one makes sure that function is called when unplugging a
"in-the-wall" charger.
The last one adds EXTCON_CHG_USB_* cables to the phy.


I send this patchset as RFC because it seems a bit peculiar to have
different drivers that generate the EXTCON_USB_* and EXTCON_CHG_USB_*
events, so I want to make sure to get things right.

As far as I can tell, all the drivers in the kernel that have USB
charger events also have the EXTCON_USB one. In this case, this patchset
would make things a bit different for the MSM phy:

       +----------+      +--------------+
       |   gpio   |      |     PMIC     |<-+
       +----------+      +--------------+  |
           |                    |          |
           `--------------------+          |  EXTCON_CHG_USB_*
                                |          |       events
                    EXTCON_USB  |          |
                      events    |          |
                               \|/         |
                         +--------------+  |
                         |   USB PHY    |--+
                         +--------------+
       
Text version: EXTCON_USB comes from a GPIO or a PMIC, that triggers a
notifier in the USB phy. That notifier will determine the new
EXTCON_CHG_USB_XXX state and the PMIC will be notified about it and
determine how much current it can use to charge a battery.

Please let me know if this is the correct way to go.
 

Damien Riegel (3):
  usb: phy: msm: notify charger after setting charger info
  usb: phy: msm: notify charger when power supply is unplugged
  usb: phy: msm: use extcon to notify charger

 drivers/usb/phy/phy-msm-usb.c | 47 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 44 insertions(+), 3 deletions(-)

-- 
2.12.2

[RFC][PATCH 1/3] usb: phy: msm: notify charger after setting charger info

[Damien Riegel]

Move calls to msm_otg_notify_charger after attributes chg_state and
chg_type have been set. That way the function can use them and not rely
only on the "mA" parameter.

Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
---
 drivers/usb/phy/phy-msm-usb.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index 93d9aaad2994..c1460182bc56 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -1392,9 +1392,9 @@ static void msm_otg_sm_work(struct work_struct *w)
 				pm_runtime_put_sync(otg->usb_phy->dev);
 				msm_otg_reset(otg->usb_phy);
 			}
-			msm_otg_notify_charger(motg, 0);
 			motg->chg_state = USB_CHG_STATE_UNDEFINED;
 			motg->chg_type = USB_INVALID_CHARGER;
+			msm_otg_notify_charger(motg, 0);
 		}
 
 		if (otg->state == OTG_STATE_B_IDLE)
@@ -1404,10 +1404,10 @@ static void msm_otg_sm_work(struct work_struct *w)
 		dev_dbg(otg->usb_phy->dev, "OTG_STATE_B_PERIPHERAL state\n");
 		if (!test_bit(B_SESS_VLD, &motg->inputs) ||
 				!test_bit(ID, &motg->inputs)) {
-			msm_otg_notify_charger(motg, 0);
-			msm_otg_start_peripheral(otg->usb_phy, 0);
 			motg->chg_state = USB_CHG_STATE_UNDEFINED;
 			motg->chg_type = USB_INVALID_CHARGER;
+			msm_otg_notify_charger(motg, 0);
+			msm_otg_start_peripheral(otg->usb_phy, 0);
 			otg->state = OTG_STATE_B_IDLE;
 			msm_otg_reset(otg->usb_phy);
 			schedule_work(w);
-- 
2.12.2

[RFC][PATCH 2/3] usb: phy: msm: notify charger when power supply is unplugged

[Damien Riegel]

With the current code, msm_otg_notify_charger doesn't get called when a
power supply identified as a DCP is unplugged. To work around that,
update charger info and call the notify function when switching from
idle to host.

Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
---
 drivers/usb/phy/phy-msm-usb.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index c1460182bc56..f89a2a540f71 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -1349,6 +1349,9 @@ static void msm_otg_sm_work(struct work_struct *w)
 			writel(readl(USB_OTGSC) & ~OTGSC_BSVIE, USB_OTGSC);
 			msm_otg_start_host(otg->usb_phy, 1);
 			otg->state = OTG_STATE_A_HOST;
+			motg->chg_state = USB_CHG_STATE_UNDEFINED;
+			motg->chg_type = USB_INVALID_CHARGER;
+			msm_otg_notify_charger(motg, 0);
 		} else if (test_bit(B_SESS_VLD, &motg->inputs)) {
 			switch (motg->chg_state) {
 			case USB_CHG_STATE_UNDEFINED:
-- 
2.12.2

[RFC][PATCH 3/3] usb: phy: msm: use extcon to notify charger

[Damien Riegel]

Phy already keeps track of the USB charger mode it is in, that
information could be useful to a power supply to let it know how much
current it can draw. So in this case when DCP or CDP is set maximum
current available is 1500mA, and 100mA when SDP is set.

This is a bit peculiar in that this driver only handle EXTCON_CHG_*
states, EXTCON_USB_* events come from another driver.

Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
---
 drivers/usb/phy/phy-msm-usb.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
index f89a2a540f71..f264d5a5ad76 100644
--- a/drivers/usb/phy/phy-msm-usb.c
+++ b/drivers/usb/phy/phy-msm-usb.c
@@ -217,6 +217,7 @@ struct msm_otg {
 
 	struct msm_usb_cable vbus;
 	struct msm_usb_cable id;
+	struct extcon_dev *edev;
 
 	struct gpio_desc *switch_gpio;
 	struct notifier_block reboot;
@@ -248,6 +249,13 @@ enum vdd_levels {
 	VDD_LEVEL_MAX,
 };
 
+static const unsigned int msm_extcon_cables[] = {
+	EXTCON_CHG_USB_SDP,
+	EXTCON_CHG_USB_DCP,
+	EXTCON_CHG_USB_CDP,
+	EXTCON_NONE,
+};
+
 static int msm_hsusb_init_vddcx(struct msm_otg *motg, int init)
 {
 	int ret = 0;
@@ -834,6 +842,21 @@ static int msm_otg_resume(struct msm_otg *motg)
 
 static void msm_otg_notify_charger(struct msm_otg *motg, unsigned mA)
 {
+	const unsigned int extcon_cables[][2] = {
+		{ EXTCON_CHG_USB_SDP, USB_SDP_CHARGER },
+		{ EXTCON_CHG_USB_DCP, USB_DCP_CHARGER },
+		{ EXTCON_CHG_USB_CDP, USB_CDP_CHARGER },
+	};
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(extcon_cables); i++) {
+		unsigned int cable = extcon_cables[i][0];
+		unsigned int chg_type = extcon_cables[i][1];
+
+		extcon_set_cable_state_(motg->edev, cable,
+				motg->chg_type == chg_type);
+	}
+
 	if (motg->cur_power == mA)
 		return;
 
@@ -1861,6 +1884,21 @@ static int msm_otg_probe(struct platform_device *pdev)
 	}
 
 	/*
+	 * Documentation in extcon.h states that EXTCONG_CHG_USB_SDP should
+	 * always appear together with EXTCON_USB, so register extcon cables
+	 * only if we successfully got the vbus extcon.
+	 */
+	if (!IS_ERR(motg->vbus.extcon)) {
+		motg->edev = devm_extcon_dev_allocate(&pdev->dev, msm_extcon_cables);
+		if (IS_ERR(motg->edev))
+			return PTR_ERR(motg->edev);
+
+		ret = devm_extcon_dev_register(&pdev->dev, motg->edev);
+		if (ret)
+			return ret;
+	}
+
+	/*
 	 * NOTE: The PHYs can be multiplexed between the chipidea controller
 	 * and the dwc3 controller, using a single bit. It is important that
 	 * the dwc3 driver does not set this bit in an incompatible way.
-- 
2.12.2

Re: [RFC][PATCH 0/3] add EXTCON_CHG_USB_* cables to MSM USB phy

[Damien Riegel]

Hi,

On Fri, Apr 14, 2017 at 02:43:27PM -0400, Damien Riegel wrote:
> This patchset adds a way for the MSM USB phy to notify a power supply
> when the charging state changes. It achieves that using the extcon
> subsystem.
> 
> The first patch makes sure msm_otg_notify_charger is called after the
> charger attributes have been set.
> The second one makes sure that function is called when unplugging a
> "in-the-wall" charger.
> The last one adds EXTCON_CHG_USB_* cables to the phy.
> 
> 
> I send this patchset as RFC because it seems a bit peculiar to have
> different drivers that generate the EXTCON_USB_* and EXTCON_CHG_USB_*
> events, so I want to make sure to get things right.
> 
> As far as I can tell, all the drivers in the kernel that have USB
> charger events also have the EXTCON_USB one. In this case, this patchset
> would make things a bit different for the MSM phy:
> 
>        +----------+      +--------------+
>        |   gpio   |      |     PMIC     |<-+
>        +----------+      +--------------+  |
>            |                    |          |
>            `--------------------+          |  EXTCON_CHG_USB_*
>                                 |          |       events
>                     EXTCON_USB  |          |
>                       events    |          |
>                                \|/         |
>                          +--------------+  |
>                          |   USB PHY    |--+
>                          +--------------+
>        
> Text version: EXTCON_USB comes from a GPIO or a PMIC, that triggers a
> notifier in the USB phy. That notifier will determine the new
> EXTCON_CHG_USB_XXX state and the PMIC will be notified about it and
> determine how much current it can use to charge a battery.
> 
> Please let me know if this is the correct way to go.

I wanted to know if someone has any comment to make on this patchset?
I'm currently working on the PMIC driver and it uses the EXTCON
notifications, so I just want to make sure it makes sense to do that.

Thanks,
-- 
Damien

Re: [RFC][PATCH 0/3] add EXTCON_CHG_USB_* cables to MSM USB phy

[Chanwoo Choi]

Hi,

On 2017년 05월 17일 22:12, Damien Riegel wrote:
> Hi,
> 
> On Fri, Apr 14, 2017 at 02:43:27PM -0400, Damien Riegel wrote:
>> This patchset adds a way for the MSM USB phy to notify a power supply
>> when the charging state changes. It achieves that using the extcon
>> subsystem.
>>
>> The first patch makes sure msm_otg_notify_charger is called after the
>> charger attributes have been set.
>> The second one makes sure that function is called when unplugging a
>> "in-the-wall" charger.
>> The last one adds EXTCON_CHG_USB_* cables to the phy.
>>
>>
>> I send this patchset as RFC because it seems a bit peculiar to have
>> different drivers that generate the EXTCON_USB_* and EXTCON_CHG_USB_*
>> events, so I want to make sure to get things right.
>>
>> As far as I can tell, all the drivers in the kernel that have USB
>> charger events also have the EXTCON_USB one. In this case, this patchset
>> would make things a bit different for the MSM phy:
>>
>>        +----------+      +--------------+
>>        |   gpio   |      |     PMIC     |<-+
>>        +----------+      +--------------+  |
>>            |                    |          |
>>            `--------------------+          |  EXTCON_CHG_USB_*
>>                                 |          |       events
>>                     EXTCON_USB  |          |
>>                       events    |          |
>>                                \|/         |
>>                          +--------------+  |
>>                          |   USB PHY    |--+
>>                          +--------------+
>>        
>> Text version: EXTCON_USB comes from a GPIO or a PMIC, that triggers a
>> notifier in the USB phy. That notifier will determine the new
>> EXTCON_CHG_USB_XXX state and the PMIC will be notified about it and
>> determine how much current it can use to charge a battery.
>>
>> Please let me know if this is the correct way to go.
> 
> I wanted to know if someone has any comment to make on this patchset?
> I'm currently working on the PMIC driver and it uses the EXTCON
> notifications, so I just want to make sure it makes sense to do that.

It looks like some strange situation. In this case, it seems like
that usb phy just uses the extcon as a notifier chain.


IMHO, USB PHY might handle the regulator provided by PMIC
instead of extcon notifier as following:
But, I'm not sure. It is just my opinion.

        +----------+      +--------------+
        |   gpio   |      |     PMIC     |
        +----------+      +--------------+
            |                    |        
            `----------+         |
                       |      regulator
                     EXTCON_USB  |        
                       events    |        
                      \|/       \|/
                       +--------------+
                       |   USB PHY    |
                       +--------------+
                               |
                              \|/
                       USB PHY may determine how much current
                       it can use to charge a battery according
                       to the charger cable. Because USB PHY
                       know the kind of connected charger cable.

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics