* [PATCH] extcon-intel-cht-wc: Don't reset USB data connection at probe @ 2019-08-08 22:01 Yauhen Kharuzhy 2019-08-09 11:06 ` Hans de Goede 0 siblings, 1 reply; 5+ messages in thread From: Yauhen Kharuzhy @ 2019-08-08 22:01 UTC (permalink / raw) To: linux-kernel, Chanwoo Choi, MyungJoo Ham Cc: Hans de Goede, Andy Shevchenko, Yauhen Kharuzhy Intel Cherry Trail Whiskey Cove extcon driver connect USB data lines to PMIC at driver probing for further charger detection. This causes reset of USB data sessions and removing all devices from bus. If system was booted from Live CD or USB dongle, this makes system unusable. Check if USB ID pin is floating and re-route data lines in this case only, don't touch otherwise. Signed-off-by: Yauhen Kharuzhy <jekhor@gmail.com> --- drivers/extcon/extcon-intel-cht-wc.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/drivers/extcon/extcon-intel-cht-wc.c b/drivers/extcon/extcon-intel-cht-wc.c index 9d32150e68db..3ae573e93e6e 100644 --- a/drivers/extcon/extcon-intel-cht-wc.c +++ b/drivers/extcon/extcon-intel-cht-wc.c @@ -338,6 +338,7 @@ static int cht_wc_extcon_probe(struct platform_device *pdev) struct intel_soc_pmic *pmic = dev_get_drvdata(pdev->dev.parent); struct cht_wc_extcon_data *ext; unsigned long mask = ~(CHT_WC_PWRSRC_VBUS | CHT_WC_PWRSRC_USBID_MASK); + int pwrsrc_sts, id; int irq, ret; irq = platform_get_irq(pdev, 0); @@ -387,8 +388,19 @@ static int cht_wc_extcon_probe(struct platform_device *pdev) goto disable_sw_control; } - /* Route D+ and D- to PMIC for initial charger detection */ - cht_wc_extcon_set_phymux(ext, MUX_SEL_PMIC); + ret = regmap_read(ext->regmap, CHT_WC_PWRSRC_STS, &pwrsrc_sts); + if (ret) { + dev_err(ext->dev, "Error reading pwrsrc status: %d\n", ret); + goto disable_sw_control; + } + + id = cht_wc_extcon_get_id(ext, pwrsrc_sts); + + /* If no USB host or device connected, route D+ and D- to PMIC for + * initial charger detection + */ + if (id == INTEL_USB_ID_FLOAT) + cht_wc_extcon_set_phymux(ext, MUX_SEL_PMIC); /* Get initial state */ cht_wc_extcon_pwrsrc_event(ext); -- 2.23.0.rc0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] extcon-intel-cht-wc: Don't reset USB data connection at probe 2019-08-08 22:01 [PATCH] extcon-intel-cht-wc: Don't reset USB data connection at probe Yauhen Kharuzhy @ 2019-08-09 11:06 ` Hans de Goede 2019-08-09 15:11 ` Yauhen Kharuzhy 0 siblings, 1 reply; 5+ messages in thread From: Hans de Goede @ 2019-08-09 11:06 UTC (permalink / raw) To: Yauhen Kharuzhy, linux-kernel, Chanwoo Choi, MyungJoo Ham; +Cc: Andy Shevchenko Hi, On 8/9/19 12:01 AM, Yauhen Kharuzhy wrote: > Intel Cherry Trail Whiskey Cove extcon driver connect USB data lines to > PMIC at driver probing for further charger detection. This causes reset of > USB data sessions and removing all devices from bus. If system was > booted from Live CD or USB dongle, this makes system unusable. > > Check if USB ID pin is floating and re-route data lines in this case > only, don't touch otherwise. > > Signed-off-by: Yauhen Kharuzhy <jekhor@gmail.com> > --- > drivers/extcon/extcon-intel-cht-wc.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/drivers/extcon/extcon-intel-cht-wc.c b/drivers/extcon/extcon-intel-cht-wc.c > index 9d32150e68db..3ae573e93e6e 100644 > --- a/drivers/extcon/extcon-intel-cht-wc.c > +++ b/drivers/extcon/extcon-intel-cht-wc.c > @@ -338,6 +338,7 @@ static int cht_wc_extcon_probe(struct platform_device *pdev) > struct intel_soc_pmic *pmic = dev_get_drvdata(pdev->dev.parent); > struct cht_wc_extcon_data *ext; > unsigned long mask = ~(CHT_WC_PWRSRC_VBUS | CHT_WC_PWRSRC_USBID_MASK); > + int pwrsrc_sts, id; > int irq, ret; > > irq = platform_get_irq(pdev, 0); > @@ -387,8 +388,19 @@ static int cht_wc_extcon_probe(struct platform_device *pdev) > goto disable_sw_control; > } > > - /* Route D+ and D- to PMIC for initial charger detection */ > - cht_wc_extcon_set_phymux(ext, MUX_SEL_PMIC); > + ret = regmap_read(ext->regmap, CHT_WC_PWRSRC_STS, &pwrsrc_sts); > + if (ret) { > + dev_err(ext->dev, "Error reading pwrsrc status: %d\n", ret); > + goto disable_sw_control; > + } > + > + id = cht_wc_extcon_get_id(ext, pwrsrc_sts); > + > + /* If no USB host or device connected, route D+ and D- to PMIC for > + * initial charger detection > + */ > + if (id == INTEL_USB_ID_FLOAT) > + cht_wc_extcon_set_phymux(ext, MUX_SEL_PMIC); The check here should be != INTEL_USB_ID_GND, when we are connected as device we are charging from the host we are connected to and the port we are connected to may be a CDP (charging downstream port) instead of a SDP (standard downstream port) allowing us to charge at 1.5A instead of 0.5A, also != INTEL_USB_ID_GND matches the condition used in cht_wc_extcon_pwrsrc_event to determine if we should continue with charger detection there. Like your other patch I will try to give this one a test-run tomorrow. Regards, Hans > > /* Get initial state */ > cht_wc_extcon_pwrsrc_event(ext); > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] extcon-intel-cht-wc: Don't reset USB data connection at probe 2019-08-09 11:06 ` Hans de Goede @ 2019-08-09 15:11 ` Yauhen Kharuzhy 2019-08-12 17:18 ` Hans de Goede 0 siblings, 1 reply; 5+ messages in thread From: Yauhen Kharuzhy @ 2019-08-09 15:11 UTC (permalink / raw) To: Hans de Goede; +Cc: linux-kernel, Chanwoo Choi, MyungJoo Ham, Andy Shevchenko On Fri, Aug 09, 2019 at 01:06:01PM +0200, Hans de Goede wrote: > Hi, > > On 8/9/19 12:01 AM, Yauhen Kharuzhy wrote: > > Intel Cherry Trail Whiskey Cove extcon driver connect USB data lines to > > PMIC at driver probing for further charger detection. This causes reset of > > USB data sessions and removing all devices from bus. If system was > > booted from Live CD or USB dongle, this makes system unusable. > > > > Check if USB ID pin is floating and re-route data lines in this case > > only, don't touch otherwise. > > > > Signed-off-by: Yauhen Kharuzhy <jekhor@gmail.com> > > --- > > drivers/extcon/extcon-intel-cht-wc.c | 16 ++++++++++++++-- > > 1 file changed, 14 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/extcon/extcon-intel-cht-wc.c b/drivers/extcon/extcon-intel-cht-wc.c > > index 9d32150e68db..3ae573e93e6e 100644 > > --- a/drivers/extcon/extcon-intel-cht-wc.c > > +++ b/drivers/extcon/extcon-intel-cht-wc.c > > @@ -338,6 +338,7 @@ static int cht_wc_extcon_probe(struct platform_device *pdev) > > struct intel_soc_pmic *pmic = dev_get_drvdata(pdev->dev.parent); > > struct cht_wc_extcon_data *ext; > > unsigned long mask = ~(CHT_WC_PWRSRC_VBUS | CHT_WC_PWRSRC_USBID_MASK); > > + int pwrsrc_sts, id; > > int irq, ret; > > irq = platform_get_irq(pdev, 0); > > @@ -387,8 +388,19 @@ static int cht_wc_extcon_probe(struct platform_device *pdev) > > goto disable_sw_control; > > } > > - /* Route D+ and D- to PMIC for initial charger detection */ > > - cht_wc_extcon_set_phymux(ext, MUX_SEL_PMIC); > > + ret = regmap_read(ext->regmap, CHT_WC_PWRSRC_STS, &pwrsrc_sts); > > + if (ret) { > > + dev_err(ext->dev, "Error reading pwrsrc status: %d\n", ret); > > + goto disable_sw_control; > > + } > > + > > + id = cht_wc_extcon_get_id(ext, pwrsrc_sts); > > + > > + /* If no USB host or device connected, route D+ and D- to PMIC for > > + * initial charger detection > > + */ > > + if (id == INTEL_USB_ID_FLOAT) > > + cht_wc_extcon_set_phymux(ext, MUX_SEL_PMIC); > > The check here should be != INTEL_USB_ID_GND, when we are connected as > device we are charging from the host we are connected to and the port > we are connected to may be a CDP (charging downstream port) instead of > a SDP (standard downstream port) allowing us to charge at 1.5A instead > of 0.5A, also != INTEL_USB_ID_GND matches the condition used in > cht_wc_extcon_pwrsrc_event to determine if we should continue with > charger detection there. Actually it should be checked as ((id != INTEL_USB_ID_GND) && (id != INTEL_USB_RID_A)), to not interrupt connection if ACA used and our host is OTG A device (for example, if LiveCD boots from USB flash inserted into an ACA USB hub). > > Like your other patch I will try to give this one a test-run tomorrow. > > Regards, > > Hans > > > > /* Get initial state */ > > cht_wc_extcon_pwrsrc_event(ext); > > -- Yauhen Kharuzhy ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] extcon-intel-cht-wc: Don't reset USB data connection at probe 2019-08-09 15:11 ` Yauhen Kharuzhy @ 2019-08-12 17:18 ` Hans de Goede 2019-09-16 19:12 ` Yauhen Kharuzhy 0 siblings, 1 reply; 5+ messages in thread From: Hans de Goede @ 2019-08-12 17:18 UTC (permalink / raw) To: Yauhen Kharuzhy; +Cc: linux-kernel, Chanwoo Choi, MyungJoo Ham, Andy Shevchenko Hi, On 09-08-19 17:11, Yauhen Kharuzhy wrote: > On Fri, Aug 09, 2019 at 01:06:01PM +0200, Hans de Goede wrote: >> Hi, >> >> On 8/9/19 12:01 AM, Yauhen Kharuzhy wrote: >>> Intel Cherry Trail Whiskey Cove extcon driver connect USB data lines to >>> PMIC at driver probing for further charger detection. This causes reset of >>> USB data sessions and removing all devices from bus. If system was >>> booted from Live CD or USB dongle, this makes system unusable. >>> >>> Check if USB ID pin is floating and re-route data lines in this case >>> only, don't touch otherwise. >>> >>> Signed-off-by: Yauhen Kharuzhy <jekhor@gmail.com> >>> --- >>> drivers/extcon/extcon-intel-cht-wc.c | 16 ++++++++++++++-- >>> 1 file changed, 14 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/extcon/extcon-intel-cht-wc.c b/drivers/extcon/extcon-intel-cht-wc.c >>> index 9d32150e68db..3ae573e93e6e 100644 >>> --- a/drivers/extcon/extcon-intel-cht-wc.c >>> +++ b/drivers/extcon/extcon-intel-cht-wc.c >>> @@ -338,6 +338,7 @@ static int cht_wc_extcon_probe(struct platform_device *pdev) >>> struct intel_soc_pmic *pmic = dev_get_drvdata(pdev->dev.parent); >>> struct cht_wc_extcon_data *ext; >>> unsigned long mask = ~(CHT_WC_PWRSRC_VBUS | CHT_WC_PWRSRC_USBID_MASK); >>> + int pwrsrc_sts, id; >>> int irq, ret; >>> irq = platform_get_irq(pdev, 0); >>> @@ -387,8 +388,19 @@ static int cht_wc_extcon_probe(struct platform_device *pdev) >>> goto disable_sw_control; >>> } >>> - /* Route D+ and D- to PMIC for initial charger detection */ >>> - cht_wc_extcon_set_phymux(ext, MUX_SEL_PMIC); >>> + ret = regmap_read(ext->regmap, CHT_WC_PWRSRC_STS, &pwrsrc_sts); >>> + if (ret) { >>> + dev_err(ext->dev, "Error reading pwrsrc status: %d\n", ret); >>> + goto disable_sw_control; >>> + } >>> + >>> + id = cht_wc_extcon_get_id(ext, pwrsrc_sts); >>> + >>> + /* If no USB host or device connected, route D+ and D- to PMIC for >>> + * initial charger detection >>> + */ >>> + if (id == INTEL_USB_ID_FLOAT) >>> + cht_wc_extcon_set_phymux(ext, MUX_SEL_PMIC); >> >> The check here should be != INTEL_USB_ID_GND, when we are connected as >> device we are charging from the host we are connected to and the port >> we are connected to may be a CDP (charging downstream port) instead of >> a SDP (standard downstream port) allowing us to charge at 1.5A instead >> of 0.5A, also != INTEL_USB_ID_GND matches the condition used in >> cht_wc_extcon_pwrsrc_event to determine if we should continue with >> charger detection there. > > Actually it should be checked as ((id != INTEL_USB_ID_GND) && (id != > INTEL_USB_RID_A)), to not interrupt connection if ACA used and our host > is OTG A device (for example, if LiveCD boots from USB flash inserted > into an ACA USB hub). Right, except that cht_wc_extcon_get_id never returns INTEL_USB_RID_A, so checking for that would be superfluous and also would be inconsistent with what cht_wc_extcon_pwrsrc_event is doing, so for now please just test for != INTEL_USB_ID_GND, if cht_wc_extcon_get_id ever starts supporting RID_A then we should fix both cht_wc_extcon_pwrsrc_event and you new check at that time. Hmm, I now see that cht_wc_extcon_get_id currently only reports one of: INTEL_USB_ID_GND or INTEL_USB_ID_FLOAT so == INTEL_USB_ID_FLOAT is equivalent to != INTEL_USB_ID_GND still please check for 1= INTEL_USB_ID_GND as that is what cht_wc_extcon_pwrsrc_event is doing. >> Like your other patch I will try to give this one a test-run tomorrow. I've given this patch a test-run today and I can confirm that BC1.2 charger detection still works fine on my device using this driver. Regards, Hans ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] extcon-intel-cht-wc: Don't reset USB data connection at probe 2019-08-12 17:18 ` Hans de Goede @ 2019-09-16 19:12 ` Yauhen Kharuzhy 0 siblings, 0 replies; 5+ messages in thread From: Yauhen Kharuzhy @ 2019-09-16 19:12 UTC (permalink / raw) To: Hans de Goede; +Cc: linux-kernel, Chanwoo Choi, MyungJoo Ham, Andy Shevchenko On Mon, Aug 12, 2019 at 07:18:45PM +0200, Hans de Goede wrote: > Hi, > > On 09-08-19 17:11, Yauhen Kharuzhy wrote: > > On Fri, Aug 09, 2019 at 01:06:01PM +0200, Hans de Goede wrote: > > > Hi, > > > > > > On 8/9/19 12:01 AM, Yauhen Kharuzhy wrote: > > > > Intel Cherry Trail Whiskey Cove extcon driver connect USB data lines to > > > > PMIC at driver probing for further charger detection. This causes reset of > > > > USB data sessions and removing all devices from bus. If system was > > > > booted from Live CD or USB dongle, this makes system unusable. > > > > > > > > Check if USB ID pin is floating and re-route data lines in this case > > > > only, don't touch otherwise. > > > > > > > > Signed-off-by: Yauhen Kharuzhy <jekhor@gmail.com> > > > > --- > > > > drivers/extcon/extcon-intel-cht-wc.c | 16 ++++++++++++++-- > > > > 1 file changed, 14 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/drivers/extcon/extcon-intel-cht-wc.c b/drivers/extcon/extcon-intel-cht-wc.c > > > > index 9d32150e68db..3ae573e93e6e 100644 > > > > --- a/drivers/extcon/extcon-intel-cht-wc.c > > > > +++ b/drivers/extcon/extcon-intel-cht-wc.c > > > > @@ -338,6 +338,7 @@ static int cht_wc_extcon_probe(struct platform_device *pdev) > > > > struct intel_soc_pmic *pmic = dev_get_drvdata(pdev->dev.parent); > > > > struct cht_wc_extcon_data *ext; > > > > unsigned long mask = ~(CHT_WC_PWRSRC_VBUS | CHT_WC_PWRSRC_USBID_MASK); > > > > + int pwrsrc_sts, id; > > > > int irq, ret; > > > > irq = platform_get_irq(pdev, 0); > > > > @@ -387,8 +388,19 @@ static int cht_wc_extcon_probe(struct platform_device *pdev) > > > > goto disable_sw_control; > > > > } > > > > - /* Route D+ and D- to PMIC for initial charger detection */ > > > > - cht_wc_extcon_set_phymux(ext, MUX_SEL_PMIC); > > > > + ret = regmap_read(ext->regmap, CHT_WC_PWRSRC_STS, &pwrsrc_sts); > > > > + if (ret) { > > > > + dev_err(ext->dev, "Error reading pwrsrc status: %d\n", ret); > > > > + goto disable_sw_control; > > > > + } > > > > + > > > > + id = cht_wc_extcon_get_id(ext, pwrsrc_sts); > > > > + > > > > + /* If no USB host or device connected, route D+ and D- to PMIC for > > > > + * initial charger detection > > > > + */ > > > > + if (id == INTEL_USB_ID_FLOAT) > > > > + cht_wc_extcon_set_phymux(ext, MUX_SEL_PMIC); > > > > > > The check here should be != INTEL_USB_ID_GND, when we are connected as > > > device we are charging from the host we are connected to and the port > > > we are connected to may be a CDP (charging downstream port) instead of > > > a SDP (standard downstream port) allowing us to charge at 1.5A instead > > > of 0.5A, also != INTEL_USB_ID_GND matches the condition used in > > > cht_wc_extcon_pwrsrc_event to determine if we should continue with > > > charger detection there. > > > > Actually it should be checked as ((id != INTEL_USB_ID_GND) && (id != > > INTEL_USB_RID_A)), to not interrupt connection if ACA used and our host > > is OTG A device (for example, if LiveCD boots from USB flash inserted > > into an ACA USB hub). > > Right, except that cht_wc_extcon_get_id never returns INTEL_USB_RID_A, > so checking for that would be superfluous and also would be inconsistent > with what cht_wc_extcon_pwrsrc_event is doing, so for now please just test > for != INTEL_USB_ID_GND, if cht_wc_extcon_get_id ever starts supporting > RID_A then we should fix both cht_wc_extcon_pwrsrc_event and you new check > at that time. > > Hmm, I now see that cht_wc_extcon_get_id currently only reports one of: > INTEL_USB_ID_GND or INTEL_USB_ID_FLOAT so == INTEL_USB_ID_FLOAT is > equivalent to != INTEL_USB_ID_GND still please check for 1= INTEL_USB_ID_GND > as that is what cht_wc_extcon_pwrsrc_event is doing. > > > > Like your other patch I will try to give this one a test-run tomorrow. > > I've given this patch a test-run today and I can confirm that BC1.2 charger > detection still works fine on my device using this driver. OK. -- Yauhen Kharuzhy ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-09-16 19:12 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-08-08 22:01 [PATCH] extcon-intel-cht-wc: Don't reset USB data connection at probe Yauhen Kharuzhy 2019-08-09 11:06 ` Hans de Goede 2019-08-09 15:11 ` Yauhen Kharuzhy 2019-08-12 17:18 ` Hans de Goede 2019-09-16 19:12 ` Yauhen Kharuzhy
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).