linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4] usb: dwc3: qcom: Remove extcon functionality from glue
@ 2025-07-18  5:38 Krishna Kurapati
  2025-07-18  8:52 ` Konrad Dybcio
  2025-07-20 11:40 ` Greg Kroah-Hartman
  0 siblings, 2 replies; 4+ messages in thread
From: Krishna Kurapati @ 2025-07-18  5:38 UTC (permalink / raw)
  To: Thinh Nguyen, Greg Kroah-Hartman, Bjorn Andersson,
	Dmitry Baryshkov, Konrad Dybcio
  Cc: linux-arm-msm, linux-usb, linux-kernel, Krishna Kurapati

Deprecate usage of extcon functionality from the glue driver. Now
that the glue driver is a flattened implementation, all existing
DTs would eventually move to new bindings. While doing so let them
make use of role-switch/ typec frameworks to provide role data
rather than using extcon. None of the existing in-kernel extcon users
have moved to using new bindings yet, so this change doesn't affect
any existing users.

On upstream, summary of targets/platforms using extcon is as follows:

1. MSM8916 and MSM8939 use Chipidea controller, hence the changes have no
effect on them.

2. Of the other extcon users, most of them use "linux,extcon-usb-gpio"
driver which relies on id/vbus gpios to inform role changes. This can be
transitioned to role switch based driver (usb-conn-gpio) while flattening
those platforms to move away from extcon and rely on role switching.

3. The one target that uses dwc3 controller and extcon and is not based
on reading gpios is "arch/arm64/boot/dts/qcom/msm8996-xiaomi-common.dtsi".
This platform uses TI's Type-C Port controller chip to provide extcon. If
usb on this platform is being flattened, then effort should be put in to
define a usb-c-connector device in DT and make use of role switch in
TUSB320L driver.

Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
---
Changes in v4:
Updated commit text to reflect the patch doesn't affect in-kernel users.
Removed RB tags from v3 since commit text is changed.

Link to v3:
https://lore.kernel.org/all/20250714044703.2091075-1-krishna.kurapati@oss.qualcomm.com/

 drivers/usb/dwc3/dwc3-qcom.c | 90 +-----------------------------------
 1 file changed, 1 insertion(+), 89 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
index ca7e1c02773a..a7eaefaeec4d 100644
--- a/drivers/usb/dwc3/dwc3-qcom.c
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -11,7 +11,6 @@
 #include <linux/of_clk.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
-#include <linux/extcon.h>
 #include <linux/interconnect.h>
 #include <linux/platform_device.h>
 #include <linux/phy/phy.h>
@@ -79,11 +78,6 @@ struct dwc3_qcom {
 	struct dwc3_qcom_port	ports[DWC3_QCOM_MAX_PORTS];
 	u8			num_ports;
 
-	struct extcon_dev	*edev;
-	struct extcon_dev	*host_edev;
-	struct notifier_block	vbus_nb;
-	struct notifier_block	host_nb;
-
 	enum usb_dr_mode	mode;
 	bool			is_suspended;
 	bool			pm_suspended;
@@ -119,8 +113,7 @@ static inline void dwc3_qcom_clrbits(void __iomem *base, u32 offset, u32 val)
 
 /*
  * TODO: Make the in-core role switching code invoke dwc3_qcom_vbus_override_enable(),
- * validate that the in-core extcon support is functional, and drop extcon
- * handling from the glue
+ * validate that the in-core extcon support is functional
  */
 static void dwc3_qcom_vbus_override_enable(struct dwc3_qcom *qcom, bool enable)
 {
@@ -137,80 +130,6 @@ static void dwc3_qcom_vbus_override_enable(struct dwc3_qcom *qcom, bool enable)
 	}
 }
 
-static int dwc3_qcom_vbus_notifier(struct notifier_block *nb,
-				   unsigned long event, void *ptr)
-{
-	struct dwc3_qcom *qcom = container_of(nb, struct dwc3_qcom, vbus_nb);
-
-	/* enable vbus override for device mode */
-	dwc3_qcom_vbus_override_enable(qcom, event);
-	qcom->mode = event ? USB_DR_MODE_PERIPHERAL : USB_DR_MODE_HOST;
-
-	return NOTIFY_DONE;
-}
-
-static int dwc3_qcom_host_notifier(struct notifier_block *nb,
-				   unsigned long event, void *ptr)
-{
-	struct dwc3_qcom *qcom = container_of(nb, struct dwc3_qcom, host_nb);
-
-	/* disable vbus override in host mode */
-	dwc3_qcom_vbus_override_enable(qcom, !event);
-	qcom->mode = event ? USB_DR_MODE_HOST : USB_DR_MODE_PERIPHERAL;
-
-	return NOTIFY_DONE;
-}
-
-static int dwc3_qcom_register_extcon(struct dwc3_qcom *qcom)
-{
-	struct device		*dev = qcom->dev;
-	struct extcon_dev	*host_edev;
-	int			ret;
-
-	if (!of_property_present(dev->of_node, "extcon"))
-		return 0;
-
-	qcom->edev = extcon_get_edev_by_phandle(dev, 0);
-	if (IS_ERR(qcom->edev))
-		return dev_err_probe(dev, PTR_ERR(qcom->edev),
-				     "Failed to get extcon\n");
-
-	qcom->vbus_nb.notifier_call = dwc3_qcom_vbus_notifier;
-
-	qcom->host_edev = extcon_get_edev_by_phandle(dev, 1);
-	if (IS_ERR(qcom->host_edev))
-		qcom->host_edev = NULL;
-
-	ret = devm_extcon_register_notifier(dev, qcom->edev, EXTCON_USB,
-					    &qcom->vbus_nb);
-	if (ret < 0) {
-		dev_err(dev, "VBUS notifier register failed\n");
-		return ret;
-	}
-
-	if (qcom->host_edev)
-		host_edev = qcom->host_edev;
-	else
-		host_edev = qcom->edev;
-
-	qcom->host_nb.notifier_call = dwc3_qcom_host_notifier;
-	ret = devm_extcon_register_notifier(dev, host_edev, EXTCON_USB_HOST,
-					    &qcom->host_nb);
-	if (ret < 0) {
-		dev_err(dev, "Host notifier register failed\n");
-		return ret;
-	}
-
-	/* Update initial VBUS override based on extcon state */
-	if (extcon_get_state(qcom->edev, EXTCON_USB) ||
-	    !extcon_get_state(host_edev, EXTCON_USB_HOST))
-		dwc3_qcom_vbus_notifier(&qcom->vbus_nb, true, qcom->edev);
-	else
-		dwc3_qcom_vbus_notifier(&qcom->vbus_nb, false, qcom->edev);
-
-	return 0;
-}
-
 static int dwc3_qcom_interconnect_enable(struct dwc3_qcom *qcom)
 {
 	int ret;
@@ -737,11 +656,6 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
 	if (qcom->mode != USB_DR_MODE_HOST)
 		dwc3_qcom_vbus_override_enable(qcom, true);
 
-	/* register extcon to override sw_vbus on Vbus change later */
-	ret = dwc3_qcom_register_extcon(qcom);
-	if (ret)
-		goto interconnect_exit;
-
 	wakeup_source = of_property_read_bool(dev->of_node, "wakeup-source");
 	device_init_wakeup(&pdev->dev, wakeup_source);
 
@@ -749,8 +663,6 @@ static int dwc3_qcom_probe(struct platform_device *pdev)
 
 	return 0;
 
-interconnect_exit:
-	dwc3_qcom_interconnect_exit(qcom);
 remove_core:
 	dwc3_core_remove(&qcom->dwc);
 clk_disable:
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v4] usb: dwc3: qcom: Remove extcon functionality from glue
  2025-07-18  5:38 [PATCH v4] usb: dwc3: qcom: Remove extcon functionality from glue Krishna Kurapati
@ 2025-07-18  8:52 ` Konrad Dybcio
  2025-07-20 11:40 ` Greg Kroah-Hartman
  1 sibling, 0 replies; 4+ messages in thread
From: Konrad Dybcio @ 2025-07-18  8:52 UTC (permalink / raw)
  To: Krishna Kurapati, Thinh Nguyen, Greg Kroah-Hartman,
	Bjorn Andersson, Dmitry Baryshkov
  Cc: linux-arm-msm, linux-usb, linux-kernel

On 7/18/25 7:38 AM, Krishna Kurapati wrote:
> Deprecate usage of extcon functionality from the glue driver. Now
> that the glue driver is a flattened implementation, all existing
> DTs would eventually move to new bindings. While doing so let them
> make use of role-switch/ typec frameworks to provide role data
> rather than using extcon. None of the existing in-kernel extcon users
> have moved to using new bindings yet, so this change doesn't affect
> any existing users.
> 
> On upstream, summary of targets/platforms using extcon is as follows:
> 
> 1. MSM8916 and MSM8939 use Chipidea controller, hence the changes have no
> effect on them.
> 
> 2. Of the other extcon users, most of them use "linux,extcon-usb-gpio"
> driver which relies on id/vbus gpios to inform role changes. This can be
> transitioned to role switch based driver (usb-conn-gpio) while flattening
> those platforms to move away from extcon and rely on role switching.
> 
> 3. The one target that uses dwc3 controller and extcon and is not based
> on reading gpios is "arch/arm64/boot/dts/qcom/msm8996-xiaomi-common.dtsi".
> This platform uses TI's Type-C Port controller chip to provide extcon. If
> usb on this platform is being flattened, then effort should be put in to
> define a usb-c-connector device in DT and make use of role switch in
> TUSB320L driver.
> 
> Signed-off-by: Krishna Kurapati <krishna.kurapati@oss.qualcomm.com>
> ---

To an external reviewer who doesn't have the context, 'flattening' doesn't
really mean much. You should instead specifically mention the existence
and purpose of dwc3-qcom-legacy.c

Konrad

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v4] usb: dwc3: qcom: Remove extcon functionality from glue
  2025-07-18  5:38 [PATCH v4] usb: dwc3: qcom: Remove extcon functionality from glue Krishna Kurapati
  2025-07-18  8:52 ` Konrad Dybcio
@ 2025-07-20 11:40 ` Greg Kroah-Hartman
  2025-07-22  7:18   ` Krishna Kurapati
  1 sibling, 1 reply; 4+ messages in thread
From: Greg Kroah-Hartman @ 2025-07-20 11:40 UTC (permalink / raw)
  To: Krishna Kurapati
  Cc: Thinh Nguyen, Bjorn Andersson, Dmitry Baryshkov, Konrad Dybcio,
	linux-arm-msm, linux-usb, linux-kernel

On Fri, Jul 18, 2025 at 11:08:56AM +0530, Krishna Kurapati wrote:
> Deprecate usage of extcon functionality from the glue driver.

It's not "deprecate", it is "delete".  "deprecate" means that you don't
want future users of this, you are flat out deleting it entirely.

> Now
> that the glue driver is a flattened implementation, all existing
> DTs would eventually move to new bindings.

When is this happening?

> While doing so let them
> make use of role-switch/ typec frameworks to provide role data
> rather than using extcon. None of the existing in-kernel extcon users
> have moved to using new bindings yet, so this change doesn't affect
> any existing users.

I don't understand, who does this affect?

> On upstream, summary of targets/platforms using extcon is as follows:

What is "upstream" here?  In-tree?  We only have one development place :)

> 1. MSM8916 and MSM8939 use Chipidea controller, hence the changes have no
> effect on them.
> 
> 2. Of the other extcon users, most of them use "linux,extcon-usb-gpio"
> driver which relies on id/vbus gpios to inform role changes. This can be
> transitioned to role switch based driver (usb-conn-gpio) while flattening
> those platforms to move away from extcon and rely on role switching.

"most" do, but not all.

> 3. The one target that uses dwc3 controller and extcon and is not based
> on reading gpios is "arch/arm64/boot/dts/qcom/msm8996-xiaomi-common.dtsi".
> This platform uses TI's Type-C Port controller chip to provide extcon. If
> usb on this platform is being flattened, then effort should be put in to
> define a usb-c-connector device in DT and make use of role switch in
> TUSB320L driver.

I really still do not understand what is happening here.

Does this break existing in-tree users?  If yes, we can't do that.  If
no, they this is just unused code?  That's all that we should be
concerned about here.

> ---
> Changes in v4:
> Updated commit text to reflect the patch doesn't affect in-kernel users.
> Removed RB tags from v3 since commit text is changed.
> 
> Link to v3:
> https://lore.kernel.org/all/20250714044703.2091075-1-krishna.kurapati@oss.qualcomm.com/

What changed in v3?  v2?  v1?

Please properly document this.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v4] usb: dwc3: qcom: Remove extcon functionality from glue
  2025-07-20 11:40 ` Greg Kroah-Hartman
@ 2025-07-22  7:18   ` Krishna Kurapati
  0 siblings, 0 replies; 4+ messages in thread
From: Krishna Kurapati @ 2025-07-22  7:18 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Thinh Nguyen, Bjorn Andersson, Dmitry Baryshkov, Konrad Dybcio,
	linux-arm-msm, linux-usb, linux-kernel



On 7/20/2025 5:10 PM, Greg Kroah-Hartman wrote:
> On Fri, Jul 18, 2025 at 11:08:56AM +0530, Krishna Kurapati wrote:
>> Deprecate usage of extcon functionality from the glue driver.
> 
> It's not "deprecate", it is "delete".  "deprecate" means that you don't
> want future users of this, you are flat out deleting it entirely.
> 

ACK, will rephrase it to "removing code".

>> Now
>> that the glue driver is a flattened implementation, all existing
>> DTs would eventually move to new bindings.
> 
> When is this happening?
> 
>> While doing so let them
>> make use of role-switch/ typec frameworks to provide role data
>> rather than using extcon. None of the existing in-kernel extcon users
>> have moved to using new bindings yet, so this change doesn't affect
>> any existing users.
> 
> I don't understand, who does this affect?
> 
>> On upstream, summary of targets/platforms using extcon is as follows:
> 
> What is "upstream" here?  In-tree?  We only have one development place :)
> 
>> 1. MSM8916 and MSM8939 use Chipidea controller, hence the changes have no
>> effect on them.
>>
>> 2. Of the other extcon users, most of them use "linux,extcon-usb-gpio"
>> driver which relies on id/vbus gpios to inform role changes. This can be
>> transitioned to role switch based driver (usb-conn-gpio) while flattening
>> those platforms to move away from extcon and rely on role switching.
> 
> "most" do, but not all.
> 
>> 3. The one target that uses dwc3 controller and extcon and is not based
>> on reading gpios is "arch/arm64/boot/dts/qcom/msm8996-xiaomi-common.dtsi".
>> This platform uses TI's Type-C Port controller chip to provide extcon. If
>> usb on this platform is being flattened, then effort should be put in to
>> define a usb-c-connector device in DT and make use of role switch in
>> TUSB320L driver.
> 
> I really still do not understand what is happening here.
> 
> Does this break existing in-tree users?  If yes, we can't do that.  If
> no, they this is just unused code?  That's all that we should be
> concerned about here.
> 

Thanks for the comments Greg. Basically, all extcon users today use 
legacy glue bindings (dwc3-qcom-legacy.c). In the new bindings 
(dwc3-qcom.c), we wanted to remove extcon functionality and enforce 
usage of usb-role-switch framework instead. So no existing users of 
extcon would be affected now. But when the above mentioned platforms 
(the ones that use extcon-usb-gpio driver and 
msm8996-xiaomi-common.dtsi) are being flattened (by anyone), we want to 
enforce usage of role-switch (moving to use usb-conn-gpio driver). Would 
it be clear if I incorporated the following in commit text:

1. Extcon users today use legacy bindings, so this change wont affect 
any existing in-kernel users since no flattened dT today uses extcon.
2. Extcon code is being removed and usage of role-switch is being 
encouraged instead.
3. Effort to be be put in while flattening platforms using extcon based 
on gpio to convert them to use ubs-conn-gpio based on role switch.

Let me know your thoughts on this. And thanks for taking the time to 
review the patch.

Regards,
Krishna,

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-07-22  7:18 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-18  5:38 [PATCH v4] usb: dwc3: qcom: Remove extcon functionality from glue Krishna Kurapati
2025-07-18  8:52 ` Konrad Dybcio
2025-07-20 11:40 ` Greg Kroah-Hartman
2025-07-22  7:18   ` Krishna Kurapati

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