linux-kernel - Re: [PATCH] USB: DWC2: Add VBUS overcurrent detection control.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210615171414.2020ad9b@zhouyanjie-virtual-machine>
Date:   Tue, 15 Jun 2021 17:14:14 +0800
From:   周琰杰 <zhouyanjie@...yeetech.com>
To:     Paul Cercueil <paul@...ndingux.net>
Cc:     Greg KH <gregkh@...uxfoundation.org>, hminas@...opsys.com,
        paul@...pouillou.net, linux-mips@...r.kernel.org,
        linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
        dongsheng.qiu@...enic.com, aric.pzqi@...enic.com,
        sernia.zhou@...mail.com,
        Dragan Čečavac <dragancecavac@...oo.com>
Subject: Re: [PATCH] USB: DWC2: Add VBUS overcurrent detection control.

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

> Hi Zhou,
> 
> Le mar., juin 15 2021 at 16:16:39 +0800, 周琰杰 
> <zhouyanjie@...yeetech.com> a écrit :
> > Hi Greg,
> > 
> > Sorry for taking so long to reply.
> > 
> > 于 Tue, 23 Mar 2021 16:31:29 +0100
> > Greg KH <gregkh@...uxfoundation.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@...yeetech.com>
> >>  > Signed-off-by: Dragan Čečavac <dragancecavac@...oo.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  
>