diff mbox

[resend] usb: chipidea: Don't select EXTCON

Message ID 20180419160144.0412a510@xhacker.debian (mailing list archive)
State New, archived
Headers show

Commit Message

Jisheng Zhang April 19, 2018, 8:01 a.m. UTC
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 <Jisheng.Zhang@synaptics.com>
---
 drivers/usb/chipidea/Kconfig | 1 -
 1 file changed, 1 deletion(-)

Comments

Peter Chen April 20, 2018, 1:38 a.m. UTC | #1
>  drivers/usb/chipidea/Kconfig | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/usb/chipidea/Kconfig b/drivers/usb/chipidea/Kconfig index
> 785f0ed037f7..97509172d536 100644
> --- a/drivers/usb/chipidea/Kconfig
> +++ b/drivers/usb/chipidea/Kconfig
> @@ -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
> --
> 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 April 20, 2018, 9:01 a.m. UTC | #2
Hi Peter

On Fri, 20 Apr 2018 01:38:42 +0000 Peter Chen wrote:

>  
> >  drivers/usb/chipidea/Kconfig | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/drivers/usb/chipidea/Kconfig b/drivers/usb/chipidea/Kconfig index
> > 785f0ed037f7..97509172d536 100644
> > --- a/drivers/usb/chipidea/Kconfig
> > +++ b/drivers/usb/chipidea/Kconfig
> > @@ -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
> > --
> > 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

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 April 20, 2018, 9:35 a.m. UTC | #3
> >
> > 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
> 
> 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
> 

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 April 23, 2018, 7:33 a.m. UTC | #4
On Fri, 20 Apr 2018 09:35:54 +0000 Peter Chen wrote:

>  
> > >
> > > 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  
> > 
> > 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
> >   
> 
> 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 April 25, 2018, 1:47 a.m. UTC | #5
> > >
> > > 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
> > >
> >
> > 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?
> 

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

Peter
diff mbox

Patch

diff --git a/drivers/usb/chipidea/Kconfig b/drivers/usb/chipidea/Kconfig
index 785f0ed037f7..97509172d536 100644
--- a/drivers/usb/chipidea/Kconfig
+++ b/drivers/usb/chipidea/Kconfig
@@ -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