Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 30 Oct 2020 23:15:40 +0800
From:   Dejin Zheng <zhengdejin5@gmail.com>
To:     Felipe Balbi <balbi@kernel.org>
Cc:     gregkh@linuxfoundation.org, linux-usb@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Thinh Nguyen <Thinh.Nguyen@synopsys.com>
Subject: Re: [PATCH v3] usb: dwc3: core: fix a issue about clear connect state
Message-ID: <20201030151540.GA37515@nuc8i5>
References: <20201020135806.30268-1-zhengdejin5@gmail.com>
 <875z6wdq62.fsf@kernel.org>
 <20201028125812.GA59692@nuc8i5>
 <87y2jqlahc.fsf@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <87y2jqlahc.fsf@kernel.org>
Precedence: bulk

On Wed, Oct 28, 2020 at 03:57:03PM +0200, Felipe Balbi wrote:
Hi Balbi and all:
> 
> Hi,
> 
> Dejin Zheng <zhengdejin5@gmail.com> writes:
> >> Dejin Zheng <zhengdejin5@gmail.com> writes:
> >> > According to Synopsys Programming Guide chapter 2.2 Register Resets,
> >> > it cannot reset the DCTL register by setting DCTL.CSFTRST for core soft
> >> > reset, if DWC3 controller as a slave device and stay connected with a usb
> >> > host, then, while rebooting linux, it will fail to reinitialize dwc3 as a
> >> > slave device when the DWC3 controller did not power off. because the
> >> > connection status is incorrect, so we also need to clear DCTL.RUN_STOP
> >> > bit for disabling connect when doing core soft reset. There will still
> >> > be other stale configuration in DCTL, so reset the other fields of DCTL
> >> > to the default value 0.
> >> 
> >> This commit log is a bit hard to understand. When does this problem
> >> actually happen? It seems like it's in the case of, perhaps, kexecing
> >> into a new kernel, is that right?
> >> 
> > It happens when entering the kernel for the second time after the reboot
> > command.
> >
> >> At the time dwc3_core_soft_reset() is called, the assumption is that
> >> we're starting with a clean core, from power up. If we have stale
> >> configuration from a previous run, we should fix this on the exit
> >> path. Note that if we're reaching probe with pull up connected, we
> >> already have issues elsewhere.
> >> 
> >> I think this is not the right fix for the problem.
> >>
> > I think you are right, Thinh also suggested me fix it on the exit path
> > in the previous patch v2. Do you think I can do these cleanups in the
> > shutdown hook of this driver? Balbi, is there a more suitable place to
> > do this by your rich experience? Thanks!
> 
> I don't think shutdown is called during removal, I'm not sure. I think
> we had some fixes done in shutdown time, though. Test it out, but make
> sure there are no issues with a regular modprobe cycle.
> 
It has some errors in my commit message, I describe the process of linux
restart is wrong. A PC is connected to our arm soc development board
through the usb cable, the adb program runs via usb connection. there is
a very important application in our linux system. when it goes wrong(halt
or kernel panic), we want to restart linux. my wrong description happened
here, when I manually kill this important application for testing, I
thought it was calling the reboot command to restart linux, which is wrong.
our real implementation is through watchdog, when the application no
longer sets the watchdog and the watchdog times out, but watchdog can't 
reset the whole soc. our soc has 3 cpu clusters, one cluster has a arm 
Cortex R5 cpu for boot and security. one cluster has 2 arm Cortex A55 for
linux system. the other cluster for android. when the Cortex R5 detect
the watchdog timeout and want to restart linux system, it will stop Cortex
A55 cpu to run, and load linux image to DDR memory from eMMC flash, then
set Cortex A55 cpu to run new linux system, but it was not reset usb
controller. so the usb controller's status is incorrect for boot new linux
system.

   ------------------------------------------------------------------
   |                                                                |
   |	Boot and Security          Linux             Android        |
   |	----------------     ----------------    ----------------   |
   |	|  1 Cortex R5  |    | 2 Cortex A55 |    | 4 Cortex A72 |   |
   |	|    cluster    |    |    cluster   |    |   cluster    |   |
   |	|---------------|    |--------------|    |--------------|   |
   |                                                                |
   |                           SOC                                  |
   |-----------------------------------------------------------------

Under normal circumstances, run the reboot command and rmmod the
corresponding usb module, it will carry out the corresponding state
processing, all of which can work well.

Balbi, for this case, Currently, the way I can think of is to reset the
DCTL register every initial time. Could you help me and give me some
suggestions? thank you very much!

BR,
Dejin

> -- 
> balbi