All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] USB: DWC2: Add VBUS overcurrent detection control.
@ 2021-03-23 15:24 周琰杰 (Zhou Yanjie)
  2021-03-23 15:31 ` Greg KH
  0 siblings, 1 reply; 8+ messages in thread
From: 周琰杰 (Zhou Yanjie) @ 2021-03-23 15:24 UTC (permalink / raw)
  To: hminas, gregkh, paul
  Cc: linux-mips, linux-usb, linux-kernel, dongsheng.qiu, aric.pzqi,
	sernia.zhou, Dragan Čečavac

Introduce configurable option for enabling GOTGCTL register
bits VbvalidOvEn and VbvalidOvVal. Once selected it disables
VBUS overcurrent detection.

This patch is derived from Dragan Čečavac (in the kernel 3.18
tree of CI20). It is very useful for the MIPS Creator CI20(r1).
Without this patch, CI20's OTG port has a great probability to
face overcurrent warning, which breaks the OTG functionality.

Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
Signed-off-by: Dragan Čečavac <dragancecavac@yahoo.com>
---
 drivers/usb/dwc2/Kconfig | 6 ++++++
 drivers/usb/dwc2/core.c  | 9 +++++++++
 2 files changed, 15 insertions(+)

diff --git a/drivers/usb/dwc2/Kconfig b/drivers/usb/dwc2/Kconfig
index c131719..e40d187 100644
--- a/drivers/usb/dwc2/Kconfig
+++ b/drivers/usb/dwc2/Kconfig
@@ -94,4 +94,10 @@ config USB_DWC2_DEBUG_PERIODIC
 	  non-periodic transfers, but of course the debug logs will be
 	  incomplete. Note that this also disables some debug messages
 	  for which the transfer type cannot be deduced.
+
+config USB_DWC2_DISABLE_VOD
+	bool "Disable VBUS overcurrent detection"
+	help
+	  Say Y here to switch off VBUS overcurrent detection. It enables USB
+	  functionality blocked by overcurrent detection.
 endif
diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index fec17a2..c629dc97 100644
--- a/drivers/usb/dwc2/core.c
+++ b/drivers/usb/dwc2/core.c
@@ -1200,6 +1200,7 @@ static int dwc2_hs_phy_init(struct dwc2_hsotg *hsotg, bool select_phy)
 int dwc2_phy_init(struct dwc2_hsotg *hsotg, bool select_phy)
 {
 	u32 usbcfg;
+	u32 otgctl;
 	int retval = 0;
 
 	if ((hsotg->params.speed == DWC2_SPEED_PARAM_FULL ||
@@ -1231,6 +1232,14 @@ int dwc2_phy_init(struct dwc2_hsotg *hsotg, bool select_phy)
 		dwc2_writel(hsotg, usbcfg, GUSBCFG);
 	}
 
+	if (IS_ENABLED(CONFIG_USB_DWC2_DISABLE_VOD)) {
+		if (dwc2_is_host_mode(hsotg)) {
+			otgctl = readl(hsotg->regs + GOTGCTL);
+			otgctl |= GOTGCTL_VBVALOEN | GOTGCTL_VBVALOVAL;
+			writel(otgctl, hsotg->regs + GOTGCTL);
+		}
+	}
+
 	return retval;
 }
 
-- 
2.7.4


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

* Re: [PATCH] USB: DWC2: Add VBUS overcurrent detection control.
  2021-03-23 15:24 [PATCH] USB: DWC2: Add VBUS overcurrent detection control 周琰杰 (Zhou Yanjie)
@ 2021-03-23 15:31 ` Greg KH
  2021-06-15  8:16   ` 周琰杰
  0 siblings, 1 reply; 8+ messages in thread
From: Greg KH @ 2021-03-23 15:31 UTC (permalink / raw)
  To: 周琰杰 (Zhou Yanjie)
  Cc: hminas, paul, linux-mips, linux-usb, linux-kernel, dongsheng.qiu,
	aric.pzqi, sernia.zhou, Dragan Čečavac

On Tue, Mar 23, 2021 at 11:24:26PM +0800, 周琰杰 (Zhou Yanjie) wrote:
> Introduce configurable option for enabling GOTGCTL register
> bits VbvalidOvEn and VbvalidOvVal. Once selected it disables
> VBUS overcurrent detection.
> 
> This patch is derived from Dragan Čečavac (in the kernel 3.18
> tree of CI20). It is very useful for the MIPS Creator CI20(r1).
> Without this patch, CI20's OTG port has a great probability to
> face overcurrent warning, which breaks the OTG functionality.
> 
> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
> Signed-off-by: Dragan Čečavac <dragancecavac@yahoo.com>
> ---
>  drivers/usb/dwc2/Kconfig | 6 ++++++
>  drivers/usb/dwc2/core.c  | 9 +++++++++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/drivers/usb/dwc2/Kconfig b/drivers/usb/dwc2/Kconfig
> index c131719..e40d187 100644
> --- a/drivers/usb/dwc2/Kconfig
> +++ b/drivers/usb/dwc2/Kconfig
> @@ -94,4 +94,10 @@ config USB_DWC2_DEBUG_PERIODIC
>  	  non-periodic transfers, but of course the debug logs will be
>  	  incomplete. Note that this also disables some debug messages
>  	  for which the transfer type cannot be deduced.
> +
> +config USB_DWC2_DISABLE_VOD
> +	bool "Disable VBUS overcurrent detection"
> +	help
> +	  Say Y here to switch off VBUS overcurrent detection. It enables USB
> +	  functionality blocked by overcurrent detection.

Why would this be a configuration option?  Shouldn't this be dynamic and
just work properly automatically?

You should not have to do this on a build-time basis, it should be able
to be detected and handled properly at run-time for all devices.

If you know this is needed for a specific type of device, detect it and
make the change then, otherwise this could break working systems, right?

thanks,

greg k-h

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

* Re: [PATCH] USB: DWC2: Add VBUS overcurrent detection control.
  2021-03-23 15:31 ` Greg KH
@ 2021-06-15  8:16   ` 周琰杰
  2021-06-15  8:52     ` Paul Cercueil
  2021-06-15  9:46     ` Greg KH
  0 siblings, 2 replies; 8+ messages in thread
From: 周琰杰 @ 2021-06-15  8:16 UTC (permalink / raw)
  To: Greg KH
  Cc: hminas, paul, linux-mips, linux-usb, linux-kernel, dongsheng.qiu,
	aric.pzqi, sernia.zhou, Dragan Čečavac

Hi Greg,

Sorry for taking so long to reply.

于 Tue, 23 Mar 2021 16:31:29 +0100
Greg KH <gregkh@linuxfoundation.org> 写道:

> On Tue, Mar 23, 2021 at 11:24:26PM +0800, 周琰杰 (Zhou Yanjie) wrote:
> > Introduce configurable option for enabling GOTGCTL register
> > bits VbvalidOvEn and VbvalidOvVal. Once selected it disables
> > VBUS overcurrent detection.
> > 
> > This patch is derived from Dragan Čečavac (in the kernel 3.18
> > tree of CI20). It is very useful for the MIPS Creator CI20(r1).
> > Without this patch, CI20's OTG port has a great probability to
> > face overcurrent warning, which breaks the OTG functionality.
> > 
> > Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
> > Signed-off-by: Dragan Čečavac <dragancecavac@yahoo.com>
> > ---
> >  drivers/usb/dwc2/Kconfig | 6 ++++++
> >  drivers/usb/dwc2/core.c  | 9 +++++++++
> >  2 files changed, 15 insertions(+)
> > 
> > diff --git a/drivers/usb/dwc2/Kconfig b/drivers/usb/dwc2/Kconfig
> > index c131719..e40d187 100644
> > --- a/drivers/usb/dwc2/Kconfig
> > +++ b/drivers/usb/dwc2/Kconfig
> > @@ -94,4 +94,10 @@ config USB_DWC2_DEBUG_PERIODIC
> >  	  non-periodic transfers, but of course the debug logs
> > will be incomplete. Note that this also disables some debug messages
> >  	  for which the transfer type cannot be deduced.
> > +
> > +config USB_DWC2_DISABLE_VOD
> > +	bool "Disable VBUS overcurrent detection"
> > +	help
> > +	  Say Y here to switch off VBUS overcurrent detection. It
> > enables USB
> > +	  functionality blocked by overcurrent detection.  
> 
> Why would this be a configuration option?  Shouldn't this be dynamic
> and just work properly automatically?
> 
> You should not have to do this on a build-time basis, it should be
> able to be detected and handled properly at run-time for all devices.
> 

I consulted the original author Dragan Čečavac, he think since this is
a feature which disables overcurrent detection, so we are not sure if
it could be harmful for some devices. Therefore he advise against
enabling it in runtime, and in favor that user explicitely has to
enable it.

> If you know this is needed for a specific type of device, detect it
> and make the change then, otherwise this could break working systems,
> right?

According to the information provided by Dragan Čečavac, this function
(select whether to enable over-current detection through the otgctl
register) don't seem to be available for all dwc2 controllers, so it
might make sense to add MACH_INGENIC dependency to
USB_DWC2_DISABLE_VOD, which could provide additional protection from
unwanted usage.

Thanks and best regards!

> 
> thanks,
> 
> greg k-h

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

* Re: [PATCH] USB: DWC2: Add VBUS overcurrent detection control.
  2021-06-15  8:16   ` 周琰杰
@ 2021-06-15  8:52     ` Paul Cercueil
  2021-06-15  9:14       ` 周琰杰
  2021-06-15  9:48       ` Greg KH
  2021-06-15  9:46     ` Greg KH
  1 sibling, 2 replies; 8+ messages in thread
From: Paul Cercueil @ 2021-06-15  8:52 UTC (permalink / raw)
  To: 周琰杰
  Cc: Greg KH, hminas, paul, linux-mips, linux-usb, linux-kernel,
	dongsheng.qiu, aric.pzqi, sernia.zhou, Dragan Čečavac

Hi Zhou,

Le mar., juin 15 2021 at 16:16:39 +0800, 周琰杰 
<zhouyanjie@wanyeetech.com> a écrit :
> Hi Greg,
> 
> Sorry for taking so long to reply.
> 
> 于 Tue, 23 Mar 2021 16:31:29 +0100
> Greg KH <gregkh@linuxfoundation.org> 写道:
> 
>>  On Tue, Mar 23, 2021 at 11:24:26PM +0800, 周琰杰 (Zhou Yanjie) 
>> wrote:
>>  > Introduce configurable option for enabling GOTGCTL register
>>  > bits VbvalidOvEn and VbvalidOvVal. Once selected it disables
>>  > VBUS overcurrent detection.
>>  >
>>  > This patch is derived from Dragan Čečavac (in the kernel 3.18
>>  > tree of CI20). It is very useful for the MIPS Creator CI20(r1).
>>  > Without this patch, CI20's OTG port has a great probability to
>>  > face overcurrent warning, which breaks the OTG functionality.
>>  >
>>  > Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
>>  > Signed-off-by: Dragan Čečavac <dragancecavac@yahoo.com>
>>  > ---
>>  >  drivers/usb/dwc2/Kconfig | 6 ++++++
>>  >  drivers/usb/dwc2/core.c  | 9 +++++++++
>>  >  2 files changed, 15 insertions(+)
>>  >
>>  > diff --git a/drivers/usb/dwc2/Kconfig b/drivers/usb/dwc2/Kconfig
>>  > index c131719..e40d187 100644
>>  > --- a/drivers/usb/dwc2/Kconfig
>>  > +++ b/drivers/usb/dwc2/Kconfig
>>  > @@ -94,4 +94,10 @@ config USB_DWC2_DEBUG_PERIODIC
>>  >  	  non-periodic transfers, but of course the debug logs
>>  > will be incomplete. Note that this also disables some debug 
>> messages
>>  >  	  for which the transfer type cannot be deduced.
>>  > +
>>  > +config USB_DWC2_DISABLE_VOD
>>  > +	bool "Disable VBUS overcurrent detection"
>>  > +	help
>>  > +	  Say Y here to switch off VBUS overcurrent detection. It
>>  > enables USB
>>  > +	  functionality blocked by overcurrent detection.
>> 
>>  Why would this be a configuration option?  Shouldn't this be dynamic
>>  and just work properly automatically?
>> 
>>  You should not have to do this on a build-time basis, it should be
>>  able to be detected and handled properly at run-time for all 
>> devices.
>> 
> 
> I consulted the original author Dragan Čečavac, he think since this 
> is
> a feature which disables overcurrent detection, so we are not sure if
> it could be harmful for some devices. Therefore he advise against
> enabling it in runtime, and in favor that user explicitely has to
> enable it.

This could still be enabled at runtime, though, via a module parameter. 
Leave it enabled by default, and those who want to disable it can do it.

Also, overcurrent detection is just "detection", so enabling or 
disabling it won't change the fact that you can get overcurrent 
conditions, right?

-Paul

>>  If you know this is needed for a specific type of device, detect it
>>  and make the change then, otherwise this could break working 
>> systems,
>>  right?
> 
> According to the information provided by Dragan Čečavac, this 
> function
> (select whether to enable over-current detection through the otgctl
> register) don't seem to be available for all dwc2 controllers, so it
> might make sense to add MACH_INGENIC dependency to
> USB_DWC2_DISABLE_VOD, which could provide additional protection from
> unwanted usage.
> 
> Thanks and best regards!
> 
>> 
>>  thanks,
>> 
>>  greg k-h



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

* Re: [PATCH] USB: DWC2: Add VBUS overcurrent detection control.
  2021-06-15  8:52     ` Paul Cercueil
@ 2021-06-15  9:14       ` 周琰杰
  2021-06-15  9:48       ` Greg KH
  1 sibling, 0 replies; 8+ messages in thread
From: 周琰杰 @ 2021-06-15  9:14 UTC (permalink / raw)
  To: Paul Cercueil
  Cc: Greg KH, hminas, paul, linux-mips, linux-usb, linux-kernel,
	dongsheng.qiu, aric.pzqi, sernia.zhou, Dragan Čečavac

于 Tue, 15 Jun 2021 09:52:20 +0100
Paul Cercueil <paul@opendingux.net> 写道:

> Hi Zhou,
> 
> Le mar., juin 15 2021 at 16:16:39 +0800, 周琰杰 
> <zhouyanjie@wanyeetech.com> a écrit :
> > Hi Greg,
> > 
> > Sorry for taking so long to reply.
> > 
> > 于 Tue, 23 Mar 2021 16:31:29 +0100
> > Greg KH <gregkh@linuxfoundation.org> 写道:
> >   
> >>  On Tue, Mar 23, 2021 at 11:24:26PM +0800, 周琰杰 (Zhou Yanjie) 
> >> wrote:  
> >>  > Introduce configurable option for enabling GOTGCTL register
> >>  > bits VbvalidOvEn and VbvalidOvVal. Once selected it disables
> >>  > VBUS overcurrent detection.
> >>  >
> >>  > This patch is derived from Dragan Čečavac (in the kernel 3.18
> >>  > tree of CI20). It is very useful for the MIPS Creator CI20(r1).
> >>  > Without this patch, CI20's OTG port has a great probability to
> >>  > face overcurrent warning, which breaks the OTG functionality.
> >>  >
> >>  > Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
> >>  > Signed-off-by: Dragan Čečavac <dragancecavac@yahoo.com>
> >>  > ---
> >>  >  drivers/usb/dwc2/Kconfig | 6 ++++++
> >>  >  drivers/usb/dwc2/core.c  | 9 +++++++++
> >>  >  2 files changed, 15 insertions(+)
> >>  >
> >>  > diff --git a/drivers/usb/dwc2/Kconfig b/drivers/usb/dwc2/Kconfig
> >>  > index c131719..e40d187 100644
> >>  > --- a/drivers/usb/dwc2/Kconfig
> >>  > +++ b/drivers/usb/dwc2/Kconfig
> >>  > @@ -94,4 +94,10 @@ config USB_DWC2_DEBUG_PERIODIC
> >>  >  	  non-periodic transfers, but of course the debug logs
> >>  > will be incomplete. Note that this also disables some debug   
> >> messages  
> >>  >  	  for which the transfer type cannot be deduced.
> >>  > +
> >>  > +config USB_DWC2_DISABLE_VOD
> >>  > +	bool "Disable VBUS overcurrent detection"
> >>  > +	help
> >>  > +	  Say Y here to switch off VBUS overcurrent detection.
> >>  > It enables USB
> >>  > +	  functionality blocked by overcurrent detection.  
> >> 
> >>  Why would this be a configuration option?  Shouldn't this be
> >> dynamic and just work properly automatically?
> >> 
> >>  You should not have to do this on a build-time basis, it should be
> >>  able to be detected and handled properly at run-time for all 
> >> devices.
> >>   
> > 
> > I consulted the original author Dragan Čečavac, he think since this 
> > is
> > a feature which disables overcurrent detection, so we are not sure
> > if it could be harmful for some devices. Therefore he advise against
> > enabling it in runtime, and in favor that user explicitely has to
> > enable it.  
> 
> This could still be enabled at runtime, though, via a module
> parameter. Leave it enabled by default, and those who want to disable
> it can do it.
> 
> Also, overcurrent detection is just "detection", so enabling or 
> disabling it won't change the fact that you can get overcurrent 
> conditions, right?

emmm, the main problem now is that there is a phenomenon on CI20 r1
(JZ4780) and CU1000 (X1000) that even if there is no overcurrent
(nothing is connected), there is still a high probability (about every
10 times it will be 6 to 7 times) an overcurrent warning appears (even
when just finish detect the OTG driver during the kernel startup),
which then causes the OTG function to not work normally before the
system restarts.

Thanks and best regards!

> 
> -Paul
> 
> >>  If you know this is needed for a specific type of device, detect
> >> it and make the change then, otherwise this could break working 
> >> systems,
> >>  right?  
> > 
> > According to the information provided by Dragan Čečavac, this 
> > function
> > (select whether to enable over-current detection through the otgctl
> > register) don't seem to be available for all dwc2 controllers, so it
> > might make sense to add MACH_INGENIC dependency to
> > USB_DWC2_DISABLE_VOD, which could provide additional protection from
> > unwanted usage.
> > 
> > Thanks and best regards!
> >   
> >> 
> >>  thanks,
> >> 
> >>  greg k-h  
> 


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

* Re: [PATCH] USB: DWC2: Add VBUS overcurrent detection control.
  2021-06-15  8:16   ` 周琰杰
  2021-06-15  8:52     ` Paul Cercueil
@ 2021-06-15  9:46     ` Greg KH
  1 sibling, 0 replies; 8+ messages in thread
From: Greg KH @ 2021-06-15  9:46 UTC (permalink / raw)
  To: 周琰杰
  Cc: hminas, paul, linux-mips, linux-usb, linux-kernel, dongsheng.qiu,
	aric.pzqi, sernia.zhou, Dragan Čečavac

On Tue, Jun 15, 2021 at 04:16:39PM +0800, 周琰杰 wrote:
> Hi Greg,
> 
> Sorry for taking so long to reply.
> 
> 于 Tue, 23 Mar 2021 16:31:29 +0100
> Greg KH <gregkh@linuxfoundation.org> 写道:
> 
> > On Tue, Mar 23, 2021 at 11:24:26PM +0800, 周琰杰 (Zhou Yanjie) wrote:
> > > Introduce configurable option for enabling GOTGCTL register
> > > bits VbvalidOvEn and VbvalidOvVal. Once selected it disables
> > > VBUS overcurrent detection.
> > > 
> > > This patch is derived from Dragan Čečavac (in the kernel 3.18
> > > tree of CI20). It is very useful for the MIPS Creator CI20(r1).
> > > Without this patch, CI20's OTG port has a great probability to
> > > face overcurrent warning, which breaks the OTG functionality.
> > > 
> > > Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
> > > Signed-off-by: Dragan Čečavac <dragancecavac@yahoo.com>
> > > ---
> > >  drivers/usb/dwc2/Kconfig | 6 ++++++
> > >  drivers/usb/dwc2/core.c  | 9 +++++++++
> > >  2 files changed, 15 insertions(+)
> > > 
> > > diff --git a/drivers/usb/dwc2/Kconfig b/drivers/usb/dwc2/Kconfig
> > > index c131719..e40d187 100644
> > > --- a/drivers/usb/dwc2/Kconfig
> > > +++ b/drivers/usb/dwc2/Kconfig
> > > @@ -94,4 +94,10 @@ config USB_DWC2_DEBUG_PERIODIC
> > >  	  non-periodic transfers, but of course the debug logs
> > > will be incomplete. Note that this also disables some debug messages
> > >  	  for which the transfer type cannot be deduced.
> > > +
> > > +config USB_DWC2_DISABLE_VOD
> > > +	bool "Disable VBUS overcurrent detection"
> > > +	help
> > > +	  Say Y here to switch off VBUS overcurrent detection. It
> > > enables USB
> > > +	  functionality blocked by overcurrent detection.  
> > 
> > Why would this be a configuration option?  Shouldn't this be dynamic
> > and just work properly automatically?
> > 
> > You should not have to do this on a build-time basis, it should be
> > able to be detected and handled properly at run-time for all devices.
> > 
> 
> I consulted the original author Dragan Čečavac, he think since this is
> a feature which disables overcurrent detection, so we are not sure if
> it could be harmful for some devices. Therefore he advise against
> enabling it in runtime, and in favor that user explicitely has to
> enable it.

That will not work AT ALL for systems that build a generic kernel for
multiple devices (i.e. Android kernels), so please make this a runtime
determination based on the hardware capabilities.

> > If you know this is needed for a specific type of device, detect it
> > and make the change then, otherwise this could break working systems,
> > right?
> 
> According to the information provided by Dragan Čečavac, this function
> (select whether to enable over-current detection through the otgctl
> register) don't seem to be available for all dwc2 controllers, so it
> might make sense to add MACH_INGENIC dependency to
> USB_DWC2_DISABLE_VOD, which could provide additional protection from
> unwanted usage.

Again, make this dynamic and only happen for devices that know it will
work on, based on some hardware information (device type of DT value).
Do not make this happen based on a Kconfig option.

thanks,

greg k-h

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

* Re: [PATCH] USB: DWC2: Add VBUS overcurrent detection control.
  2021-06-15  8:52     ` Paul Cercueil
  2021-06-15  9:14       ` 周琰杰
@ 2021-06-15  9:48       ` Greg KH
  2021-06-15 10:06         ` Paul Cercueil
  1 sibling, 1 reply; 8+ messages in thread
From: Greg KH @ 2021-06-15  9:48 UTC (permalink / raw)
  To: Paul Cercueil
  Cc: 周琰杰,
	hminas, paul, linux-mips, linux-usb, linux-kernel, dongsheng.qiu,
	aric.pzqi, sernia.zhou, Dragan Čečavac

On Tue, Jun 15, 2021 at 09:52:20AM +0100, Paul Cercueil wrote:
> Hi Zhou,
> 
> Le mar., juin 15 2021 at 16:16:39 +0800, 周琰杰 <zhouyanjie@wanyeetech.com>
> a écrit :
> > Hi Greg,
> > 
> > Sorry for taking so long to reply.
> > 
> > 于 Tue, 23 Mar 2021 16:31:29 +0100
> > Greg KH <gregkh@linuxfoundation.org> 写道:
> > 
> > >  On Tue, Mar 23, 2021 at 11:24:26PM +0800, 周琰杰 (Zhou Yanjie)
> > > wrote:
> > >  > Introduce configurable option for enabling GOTGCTL register
> > >  > bits VbvalidOvEn and VbvalidOvVal. Once selected it disables
> > >  > VBUS overcurrent detection.
> > >  >
> > >  > This patch is derived from Dragan Čečavac (in the kernel 3.18
> > >  > tree of CI20). It is very useful for the MIPS Creator CI20(r1).
> > >  > Without this patch, CI20's OTG port has a great probability to
> > >  > face overcurrent warning, which breaks the OTG functionality.
> > >  >
> > >  > Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
> > >  > Signed-off-by: Dragan Čečavac <dragancecavac@yahoo.com>
> > >  > ---
> > >  >  drivers/usb/dwc2/Kconfig | 6 ++++++
> > >  >  drivers/usb/dwc2/core.c  | 9 +++++++++
> > >  >  2 files changed, 15 insertions(+)
> > >  >
> > >  > diff --git a/drivers/usb/dwc2/Kconfig b/drivers/usb/dwc2/Kconfig
> > >  > index c131719..e40d187 100644
> > >  > --- a/drivers/usb/dwc2/Kconfig
> > >  > +++ b/drivers/usb/dwc2/Kconfig
> > >  > @@ -94,4 +94,10 @@ config USB_DWC2_DEBUG_PERIODIC
> > >  >  	  non-periodic transfers, but of course the debug logs
> > >  > will be incomplete. Note that this also disables some debug
> > > messages
> > >  >  	  for which the transfer type cannot be deduced.
> > >  > +
> > >  > +config USB_DWC2_DISABLE_VOD
> > >  > +	bool "Disable VBUS overcurrent detection"
> > >  > +	help
> > >  > +	  Say Y here to switch off VBUS overcurrent detection. It
> > >  > enables USB
> > >  > +	  functionality blocked by overcurrent detection.
> > > 
> > >  Why would this be a configuration option?  Shouldn't this be dynamic
> > >  and just work properly automatically?
> > > 
> > >  You should not have to do this on a build-time basis, it should be
> > >  able to be detected and handled properly at run-time for all
> > > devices.
> > > 
> > 
> > I consulted the original author Dragan Čečavac, he think since this is
> > a feature which disables overcurrent detection, so we are not sure if
> > it could be harmful for some devices. Therefore he advise against
> > enabling it in runtime, and in favor that user explicitely has to
> > enable it.
> 
> This could still be enabled at runtime, though, via a module parameter.
> Leave it enabled by default, and those who want to disable it can do it.

This is not the 1990's, please NEVER add new module parameters,
especially ones that are somehow supposed to be device-specific.

Remember, module options are code-wide, not device-specific.

Just do this based on the device type, or something else dynamic, do NOT
make this be forced to be selected by a kernel configuration option or a
random module option at runtime.

thanks,

greg k-h

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

* Re: [PATCH] USB: DWC2: Add VBUS overcurrent detection control.
  2021-06-15  9:48       ` Greg KH
@ 2021-06-15 10:06         ` Paul Cercueil
  0 siblings, 0 replies; 8+ messages in thread
From: Paul Cercueil @ 2021-06-15 10:06 UTC (permalink / raw)
  To: Greg KH
  Cc: 周琰杰,
	hminas, paul, linux-mips, linux-usb, linux-kernel, dongsheng.qiu,
	aric.pzqi, sernia.zhou, Dragan Čečavac

Hi Greg,

Le mar., juin 15 2021 at 11:48:30 +0200, Greg KH 
<gregkh@linuxfoundation.org> a écrit :
> On Tue, Jun 15, 2021 at 09:52:20AM +0100, Paul Cercueil wrote:
>>  Hi Zhou,
>> 
>>  Le mar., juin 15 2021 at 16:16:39 +0800, 周琰杰 
>> <zhouyanjie@wanyeetech.com>
>>  a écrit :
>>  > Hi Greg,
>>  >
>>  > Sorry for taking so long to reply.
>>  >
>>  > 于 Tue, 23 Mar 2021 16:31:29 +0100
>>  > Greg KH <gregkh@linuxfoundation.org> 写道:
>>  >
>>  > >  On Tue, Mar 23, 2021 at 11:24:26PM +0800, 周琰杰 (Zhou 
>> Yanjie)
>>  > > wrote:
>>  > >  > Introduce configurable option for enabling GOTGCTL register
>>  > >  > bits VbvalidOvEn and VbvalidOvVal. Once selected it disables
>>  > >  > VBUS overcurrent detection.
>>  > >  >
>>  > >  > This patch is derived from Dragan Čečavac (in the kernel 
>> 3.18
>>  > >  > tree of CI20). It is very useful for the MIPS Creator 
>> CI20(r1).
>>  > >  > Without this patch, CI20's OTG port has a great probability 
>> to
>>  > >  > face overcurrent warning, which breaks the OTG functionality.
>>  > >  >
>>  > >  > Signed-off-by: 周琰杰 (Zhou Yanjie) 
>> <zhouyanjie@wanyeetech.com>
>>  > >  > Signed-off-by: Dragan Čečavac <dragancecavac@yahoo.com>
>>  > >  > ---
>>  > >  >  drivers/usb/dwc2/Kconfig | 6 ++++++
>>  > >  >  drivers/usb/dwc2/core.c  | 9 +++++++++
>>  > >  >  2 files changed, 15 insertions(+)
>>  > >  >
>>  > >  > diff --git a/drivers/usb/dwc2/Kconfig 
>> b/drivers/usb/dwc2/Kconfig
>>  > >  > index c131719..e40d187 100644
>>  > >  > --- a/drivers/usb/dwc2/Kconfig
>>  > >  > +++ b/drivers/usb/dwc2/Kconfig
>>  > >  > @@ -94,4 +94,10 @@ config USB_DWC2_DEBUG_PERIODIC
>>  > >  >  	  non-periodic transfers, but of course the debug logs
>>  > >  > will be incomplete. Note that this also disables some debug
>>  > > messages
>>  > >  >  	  for which the transfer type cannot be deduced.
>>  > >  > +
>>  > >  > +config USB_DWC2_DISABLE_VOD
>>  > >  > +	bool "Disable VBUS overcurrent detection"
>>  > >  > +	help
>>  > >  > +	  Say Y here to switch off VBUS overcurrent detection. It
>>  > >  > enables USB
>>  > >  > +	  functionality blocked by overcurrent detection.
>>  > >
>>  > >  Why would this be a configuration option?  Shouldn't this be 
>> dynamic
>>  > >  and just work properly automatically?
>>  > >
>>  > >  You should not have to do this on a build-time basis, it 
>> should be
>>  > >  able to be detected and handled properly at run-time for all
>>  > > devices.
>>  > >
>>  >
>>  > I consulted the original author Dragan Čečavac, he think since 
>> this is
>>  > a feature which disables overcurrent detection, so we are not 
>> sure if
>>  > it could be harmful for some devices. Therefore he advise against
>>  > enabling it in runtime, and in favor that user explicitely has to
>>  > enable it.
>> 
>>  This could still be enabled at runtime, though, via a module 
>> parameter.
>>  Leave it enabled by default, and those who want to disable it can 
>> do it.
> 
> This is not the 1990's, please NEVER add new module parameters,

First time I hear this.

> especially ones that are somehow supposed to be device-specific.
> 
> Remember, module options are code-wide, not device-specific.

Right. I thought "just make the option available on devices that 
support it" but that's not how it works.

-Paul

> Just do this based on the device type, or something else dynamic, do 
> NOT
> make this be forced to be selected by a kernel configuration option 
> or a
> random module option at runtime.
> 
> thanks,
> 
> greg k-h



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

end of thread, other threads:[~2021-06-15 10:15 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-23 15:24 [PATCH] USB: DWC2: Add VBUS overcurrent detection control 周琰杰 (Zhou Yanjie)
2021-03-23 15:31 ` Greg KH
2021-06-15  8:16   ` 周琰杰
2021-06-15  8:52     ` Paul Cercueil
2021-06-15  9:14       ` 周琰杰
2021-06-15  9:48       ` Greg KH
2021-06-15 10:06         ` Paul Cercueil
2021-06-15  9:46     ` Greg KH

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.