usb: chipidea: Don't select EXTCON

PATCH RESEND In review Jisheng Zhang 2 participants 6 messages Apr 19, 2018 – Apr 25, 2018

Subsystems (2)

USB Chipidea USB high speed dual role controller

lore

Share & tools

lore

Jisheng Zhang

author

2018-04-19 08:01 UTC #0 lore raw

PATCH RESEND 1/1 usb: chipidea: Don't select EXTCON

Not all chipidea users need EXTCON, so it's better to avoid unconditionally select EXTCON, this could save us 2KB kernel Image size.

Signed-off-by Jisheng Zhang <[email protected]>

---
 drivers/usb/chipidea/Kconfig | 1 -
 1 file changed, 1 deletion(-)

drivers/usb/chipidea/Kconfig +0 -1

index 785f0ed037f7..97509172d536 100644
@@ -1,7 +1,6 @@
 config USB_CHIPIDEA
 	tristate "ChipIdea Highspeed Dual Role Controller"
 	depends on ((USB_EHCI_HCD && USB_GADGET) || (USB_EHCI_HCD && !USB_GADGET) || (!USB_EHCI_HCD && USB_GADGET)) && HAS_DMA
-	select EXTCON
 	select RESET_CONTROLLER
 	help
 	  Say Y here if your system has a dual role high speed USB

Signature · 1 line

2.17.0

Peter Chen

2018-04-20 01:38 UTC #1 lore raw

Quoted 18 lines

 
 drivers/usb/chipidea/Kconfig | 1 -
 1 file changed, 1 deletion(-)
drivers/usb/chipidea/Kconfig index renamed from drivers/usb/chipidea/Kconfig +0 -0
785f0ed037f7..97509172d536 100644
drivers/usb/chipidea/Kconfig +0 -0
@@ -1,7 +1,6 @@
 config USB_CHIPIDEA
 	tristate "ChipIdea Highspeed Dual Role Controller"
 	depends on ((USB_EHCI_HCD && USB_GADGET) || (USB_EHCI_HCD
&& !USB_GADGET) || (!USB_EHCI_HCD && USB_GADGET)) && HAS_DMA
drivers/usb/chipidea/Kconfig +0 -2
-	select EXTCON
 	select RESET_CONTROLLER
 	help
 	  Say Y here if your system has a dual role high speed USB
--

Quoted 1 line

2.17.0

Hi Jisheng,

Sorry to reply late, are you really care 2KB code side? Since many users use EXTCON to handle vbus and id, it is hard just delete it. I could accept patch for your specific platforms, like:

+ select EXTCON if !ARCH_XXXX

But please note, even your board uses SoC id/vbus pin to detect related external signal, the other boards use the same SoC may use external gpios to do it.

Peter

Jisheng Zhang

author

2018-04-20 09:01 UTC #2 lore raw

Hi Peter

Quoted 26 lines · On Fri, 20 Apr 2018 01:38:42 +0000 Peter Chen wrote:

Quoted 18 lines
 
 drivers/usb/chipidea/Kconfig | 1 -
 1 file changed, 1 deletion(-)
drivers/usb/chipidea/Kconfig index renamed from drivers/usb/chipidea/Kconfig +0 -0
785f0ed037f7..97509172d536 100644
drivers/usb/chipidea/Kconfig +0 -0
@@ -1,7 +1,6 @@
 config USB_CHIPIDEA
 	tristate "ChipIdea Highspeed Dual Role Controller"
 	depends on ((USB_EHCI_HCD && USB_GADGET) || (USB_EHCI_HCD
&& !USB_GADGET) || (!USB_EHCI_HCD && USB_GADGET)) && HAS_DMA
drivers/usb/chipidea/Kconfig +0 -2
-	select EXTCON
 	select RESET_CONTROLLER
 	help
 	  Say Y here if your system has a dual role high speed USB
--
Quoted 1 line
2.17.0
Hi Jisheng,
Sorry to reply late, are you really care 2KB code side? Since many users use EXTCON to handle vbus and id, it is hard just delete it. I could accept patch for your specific platforms, like:

Review

Quoted 1 line

(quoted diff) +1 -0
+	select EXTCON if !ARCH_XXXX

The patch doesn't remove extcon support from chipidea driver. I just want to not select EXTCON unconditionally, but let the users choose. If the users need extcon, they could enable EXTCON themselves

I just searched all the dts in arch/arm/boot/dts and arch/arm64/boot/dts only the four dts give extcon phandle to chipidea host, other users don't make use of it:

arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi

arch/arm/boot/dts/qcom-apq8074-dragonboard.dts

arch/arm/boot/dts/qcom-msm8974-fairphone-fp2.dts

arch/arm/boot/dts/qcom-msm8974-sony-xperia-castor.dts

Thanks

Peter Chen

2018-04-20 09:35 UTC #3 lore raw

Quoted 15 lines

Review
Quoted 7 lines
Sorry to reply late, are you really care 2KB code side? Since many users use EXTCON to handle vbus and id, it is hard just delete it. I could accept patch for your specific platforms, like:
(quoted diff) +1 -0
+	select EXTCON if !ARCH_XXXX
The patch doesn't remove extcon support from chipidea driver.
I just want to not select EXTCON unconditionally, but let the users choose. If the users need extcon, they could enable EXTCON themselves
I just searched all the dts in arch/arm/boot/dts and arch/arm64/boot/dts only the four dts give extcon phandle to chipidea host, other users don't make use of it:

Quoted 8 lines

arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
arch/arm/boot/dts/qcom-apq8074-dragonboard.dts
arch/arm/boot/dts/qcom-msm8974-fairphone-fp2.dts
arch/arm/boot/dts/qcom-msm8974-sony-xperia-castor.dts

I see, but I do not want to break msm platforms. You may try to create Glue driver Kconfig entry for chipidea like dwc3, and let msm depends on EXTCON.

Peter

Jisheng Zhang

author

2018-04-23 07:33 UTC #4 lore raw

Quoted 19 lines · On Fri, 20 Apr 2018 09:35:54 +0000 Peter Chen wrote:

Quoted 19 lines
Review
Quoted 7 lines
Sorry to reply late, are you really care 2KB code side? Since many users use EXTCON to handle vbus and id, it is hard just delete it. I could accept patch for your specific platforms, like:
(quoted diff) +1 -0
+	select EXTCON if !ARCH_XXXX  
The patch doesn't remove extcon support from chipidea driver.
I just want to not select EXTCON unconditionally, but let the users choose. If the users need extcon, they could enable EXTCON themselves
I just searched all the dts in arch/arm/boot/dts and arch/arm64/boot/dts only the four dts give extcon phandle to chipidea host, other users don't make use of it:
arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
arch/arm/boot/dts/qcom-apq8074-dragonboard.dts

Quoted 7 lines

Quoted 4 lines
arch/arm/boot/dts/qcom-msm8974-fairphone-fp2.dts
arch/arm/boot/dts/qcom-msm8974-sony-xperia-castor.dts
I see, but I do not want to break msm platforms. You may try to create Glue driver Kconfig entry for chipidea like dwc3, and let msm depends on EXTCON.

Got your points. Since multi_v7_defconfig has selected EXTCON, and EXTCON_USB_GPIO(which depends on EXTCON) is enabled in arm64 defconfig, so what about:

enable EXTCON explicitly in arm64 defconfig?
then add this patch?

Is it acceptable?

Thanks,
Jisheng

Peter Chen

2018-04-25 01:47 UTC #5 lore raw

Quoted 23 lines

Quoted 9 lines
Quoted 9 lines
The patch doesn't remove extcon support from chipidea driver. I just want to not select EXTCON unconditionally, but let the users choose. If the users need extcon, they could enable EXTCON themselves
I just searched all the dts in arch/arm/boot/dts and arch/arm64/boot/dts only the four dts give extcon phandle to chipidea host, other
users don't make use of it:
Quoted 5 lines
Quoted 5 lines
arch/arm64/boot/dts/qcom/apq8016-sbc.dtsi
arch/arm/boot/dts/qcom-apq8074-dragonboard.dts
Quoted 7 lines
Quoted 4 lines
arch/arm/boot/dts/qcom-msm8974-fairphone-fp2.dts
arch/arm/boot/dts/qcom-msm8974-sony-xperia-castor.dts
I see, but I do not want to break msm platforms. You may try to create Glue driver Kconfig entry for chipidea like dwc3, and let msm depends on EXTCON.

Quoted 7 lines

Got your points. Since multi_v7_defconfig has selected EXTCON, and EXTCON_USB_GPIO(which depends on EXTCON) is enabled in arm64 defconfig, so what about:
enable EXTCON explicitly in arm64 defconfig?
then add this patch?

I am not sure if Qualcomm platforms use these, add Qualcomm guys to confirm it.

Peter