Re: [PATCH] usb: musb: only set test mode once

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Mar 24, 2015 at 12:16:25PM -0500, Liu, Bin wrote:
> Felipe,
> 
> > -----Original Message-----
> > From: Balbi, Felipe
> > Sent: Tuesday, March 24, 2015 12:11 PM
> > To: Liu, Bin
> > Cc: Balbi, Felipe; linux-usb@xxxxxxxxxxxxxxx
> > Subject: Re: [PATCH] usb: musb: only set test mode once
> > 
> > On Tue, Mar 24, 2015 at 12:05:20PM -0500, Liu, Bin wrote:
> > > Felipe,
> > >
> > > > -----Original Message-----
> > > > From: Balbi, Felipe
> > > > Sent: Tuesday, March 24, 2015 11:54 AM
> > > > To: Liu, Bin
> > > > Cc: Balbi, Felipe; linux-usb@xxxxxxxxxxxxxxx
> > > > Subject: Re: [PATCH] usb: musb: only set test mode once
> > > >
> > > > On Tue, Mar 24, 2015 at 11:52:04AM -0500, Liu, Bin wrote:
> > > > > Felipe,
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Balbi, Felipe
> > > > > > Sent: Tuesday, March 24, 2015 11:31 AM
> > > > > > To: Liu, Bin
> > > > > > Cc: linux-usb@xxxxxxxxxxxxxxx; Balbi, Felipe
> > > > > > Subject: Re: [PATCH] usb: musb: only set test mode once
> > > > > >
> > > > > > On Fri, Mar 20, 2015 at 04:49:58PM -0500, Bin Liu wrote:
> > > > > > > The MUSB test mode register can only be set once, otherwise
> > > > > > > the result is undefined.
> > > > > > >
> > > > > > > This prevents the debugfs testmode entry to set the register
> > > > > > > more than once which causes test failure.
> > > > > > >
> > > > > > > Signed-off-by: Bin Liu <b-liu@xxxxxx>
> > > > > > > ---
> > > > > > >  drivers/usb/musb/musb_debugfs.c | 6 ++++++
> > > > > > >  1 file changed, 6 insertions(+)
> > > > > > >
> > > > > > > diff --git a/drivers/usb/musb/musb_debugfs.c
> > > > > > > b/drivers/usb/musb/musb_debugfs.c index 78a283e..cd0c381
> > > > > > > 100644
> > > > > > > --- a/drivers/usb/musb/musb_debugfs.c
> > > > > > > +++ b/drivers/usb/musb/musb_debugfs.c
> > > > > > > @@ -194,6 +194,12 @@ static ssize_t
> > > > > > > musb_test_mode_write(struct file
> > > > *file,
> > > > > > >  	u8			test = 0;
> > > > > > >  	char			buf[18];
> > > > > > >
> > > > > > > +	test = musb_readb(musb->mregs, MUSB_TESTMODE);
> > > > > > > +	if (test) {
> > > > > > > +		dev_err(musb->controller, "Error: test mode is
> > running\n");
> > > > > >
> > > > > > with this message, user doesn't really know exactly what to do.
> > > > > > Perhaps mention that a USB Bus Reset is needed to get the thing
> > > > > > out of test
> > > > mode?
> > > > >
> > > > > Ok, will change it. How about only print the error message when
> > > > > the new test mode is different from the running one, and silently
> > > > > do nothing if the new test mode is the same. Something like
> > > > >
> > > > > if (test) {
> > > > >     if (new test is different) {
> > > > >         dev_err(...);
> > > > >         return -EINVAL;
> > > > >     } else {
> > > > >         return 0;
> > > > >     }
> > > > > }
> > > >
> > > > USB2 spec says you *must* go through a bus reset in order to get out
> > > > of testmode. So if you want to change test_j to test_k, you must first bus
> > reset.
> > >
> > > Yes, this will be handled in if branch - dev_err():
> > >
> > > 	dev_err(musb->controller, "Error: test mode is already running. "
> > > 		"Please do USB Bus Reset to start a new test\n");
> > 
> > that's alright, but that if (new test is different) check is pointless
> > ;-)
> 
> Ok, I see your point now. What I wanted to enhance is to not print the
> annoying message when user runs the *same* test mode command multiple
> times.
> 
> I don't have a strong opinion on either one. You make the call ;)

let the user get annoyed so he quits the nonsense :-)

-- 
balbi

Attachment: signature.asc
Description: Digital signature


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux