linux-kernel - Re: Nokia N900: musb is in wrong state after boot

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: <20160529103824.GM29844@pali>
Date:	Sun, 29 May 2016 12:38:24 +0200
From:	Pali Rohár <pali.rohar@...il.com>
To:	Tony Lindgren <tony@...mide.com>
Cc:	joerg Reisenweber <joerg@...nmoko.org>,
	Felipe Balbi <balbi@...com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-omap@...r.kernel.org,
	Ivaylo Dimitrov <ivo.g.dimitrov.75@...il.com>,
	Sebastian Reichel <sre@...nel.org>,
	Aaro Koskinen <aaro.koskinen@....fi>,
	Pavel Machek <pavel@....cz>, Nishanth Menon <nm@...com>
Subject: Re: Nokia N900: musb is in wrong state after boot

On Saturday 23 January 2016 13:57:32 Pali Rohár wrote:
> On Thursday 21 January 2016 20:21:13 Tony Lindgren wrote:
> > * joerg Reisenweber <joerg@...nmoko.org> [160121 10:45]:
> > > On Thu 21 January 2016 09:41:46 Tony Lindgren wrote:
> > > > Then for supporting the USB host mode.. We should add regulator
> > > > support to the USB PHY driver so if the ID pin is grounded, the
> > > > PHY driver enables the VBUS regulator. That too seems to need
> > > > some coordination between the drivers/phy/phy-twl4030-usb.c and
> > > > 1707 driver if the ID pin interrupt is only detected in
> > > > drivers/phy/phy-twl4030-usb.c.
> > > 
> > > Note that, while this is probably a good thing to do, it needs to
> > > be sufficiently loose coupling to allow user to 'intercept' this
> > > VBOOS regulator enabling and instead allow device charging while
> > > in externally powered hostmode. There's even a spec for this in
> > > USB-docs-foo iirc, something along a certain resistor value on ID
> > > to GND - alas I guess the twl4030 is not capable to detect such
> > > sophisticated signaling, and anyway it's always desirable to allow
> > > user to manually override the VBOOST and enable VBUS-charging
> > > while in hostmode.
> > 
> > OK, I think this is what's happening with the Motorola LapDock BTW.
> > It always feeds the VBUS, well most of the time. Do you have some
> > pointer to the "certain resistor value on ID to GND" spec? Is it
> > maybe part of the carkit related parts of the USB spec?
> > 
> > > On N900 the situation is even more complex since the 1707 doesn't
> > > support genuine ID detection, neither does it support emulated ID
> > > grounding. And there's no other method than a ID=GND message from
> > > PHY to musb core to make the musb core state engine transfer into
> > > proper hostmode. Thus my H-E-N hostmode botch abuses debug flags
> > > to force the musb core into a "emulated" hostmode and this mode
> > > doesn't support USB speed detection. Thus speed settings are
> > > forced onto musb core and PHY by software, and the musb core speed
> > > bits are only effective before session enabled.
> > > Bottom line: you need VBUS to try and negotiate speed with the
> > > attached device in hostmode, but to actually set this speed you
> > > detected by software means, you need to disable and discharge VBUS
> > > again, or musb core won't care about the speed you set. To be
> > > utterly clear: unconditional enabling of VBUS in ID=GND won't
> > > work.
> > > 
> > > This is quite complex and it's questionable if it could get handled
> > > reasonably in kernel space. *Very* N900 specific niche solution,
> > > I'd not think it's suited for upstreaming.
> > 
> > Yeah OK. I think we should be able to support the aux VBUS regulator
> > part with mainline kernel though.
> > 
> > Regards,
> > 
> > Tony
> 
> Hello, attached patch for musb debugfs adds option to force both 
> hostmode with speed. It is just example, I tested only compilation.
> 
> Something like that will be needed for usb host mode on Nokia N900.
> 
> -- 
> Pali Rohár
> pali.rohar@...il.com

> From fd67b58e3538c0732750ecad915cde736da099dc Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali.rohar@...il.com>
> Date: Sat, 9 Jan 2016 16:57:59 +0100
> Subject: [PATCH] musb: debugfs: Add support in testmode for forcing host mode
>  together with speed
> 
> ---
>  drivers/usb/musb/musb_debugfs.c |   44 +++++++++++++++++++++++++--------------
>  1 file changed, 28 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/usb/musb/musb_debugfs.c b/drivers/usb/musb/musb_debugfs.c
> index 9b22d94..62c13a2 100644
> --- a/drivers/usb/musb/musb_debugfs.c
> +++ b/drivers/usb/musb/musb_debugfs.c
> @@ -147,28 +147,34 @@ static int musb_test_mode_show(struct seq_file *s, void *unused)
>  
>  	test = musb_readb(musb->mregs, MUSB_TESTMODE);
>  
> -	if (test & MUSB_TEST_FORCE_HOST)
> +	if (test & (MUSB_TEST_FORCE_HOST | MUSB_TEST_FORCE_FS))
> +		seq_printf(s, "force host full-speed\n");
> +
> +	else if (test & (MUSB_TEST_FORCE_HOST | MUSB_TEST_FORCE_HS))
> +		seq_printf(s, "force host high-speed\n");
> +
> +	else if (test & MUSB_TEST_FORCE_HOST)
>  		seq_printf(s, "force host\n");
>  
> -	if (test & MUSB_TEST_FIFO_ACCESS)
> +	else if (test & MUSB_TEST_FIFO_ACCESS)
>  		seq_printf(s, "fifo access\n");
>  
> -	if (test & MUSB_TEST_FORCE_FS)
> +	else if (test & MUSB_TEST_FORCE_FS)
>  		seq_printf(s, "force full-speed\n");
>  
> -	if (test & MUSB_TEST_FORCE_HS)
> +	else if (test & MUSB_TEST_FORCE_HS)
>  		seq_printf(s, "force high-speed\n");
>  
> -	if (test & MUSB_TEST_PACKET)
> +	else if (test & MUSB_TEST_PACKET)
>  		seq_printf(s, "test packet\n");
>  
> -	if (test & MUSB_TEST_K)
> +	else if (test & MUSB_TEST_K)
>  		seq_printf(s, "test K\n");
>  
> -	if (test & MUSB_TEST_J)
> +	else if (test & MUSB_TEST_J)
>  		seq_printf(s, "test J\n");
>  
> -	if (test & MUSB_TEST_SE0_NAK)
> +	else if (test & MUSB_TEST_SE0_NAK)
>  		seq_printf(s, "test SE0 NAK\n");
>  
>  	return 0;
> @@ -206,30 +212,36 @@ static ssize_t musb_test_mode_write(struct file *file,
>  	if (copy_from_user(buf, ubuf, min_t(size_t, sizeof(buf) - 1, count)))
>  		return -EFAULT;
>  
> -	if (strstarts(buf, "force host"))
> +	if (strstarts(buf, "force host full-speed"))
> +		test = MUSB_TEST_FORCE_HOST | MUSB_TEST_FORCE_FS;
> +
> +	else if (strstarts(buf, "force host high-speed"))
> +		test = MUSB_TEST_FORCE_HOST | MUSB_TEST_FORCE_HS;
> +
> +	else if (strstarts(buf, "force host"))
>  		test = MUSB_TEST_FORCE_HOST;
>  
> -	if (strstarts(buf, "fifo access"))
> +	else if (strstarts(buf, "fifo access"))
>  		test = MUSB_TEST_FIFO_ACCESS;
>  
> -	if (strstarts(buf, "force full-speed"))
> +	else if (strstarts(buf, "force full-speed"))
>  		test = MUSB_TEST_FORCE_FS;
>  
> -	if (strstarts(buf, "force high-speed"))
> +	else if (strstarts(buf, "force high-speed"))
>  		test = MUSB_TEST_FORCE_HS;
>  
> -	if (strstarts(buf, "test packet")) {
> +	else if (strstarts(buf, "test packet")) {
>  		test = MUSB_TEST_PACKET;
>  		musb_load_testpacket(musb);
>  	}
>  
> -	if (strstarts(buf, "test K"))
> +	else if (strstarts(buf, "test K"))
>  		test = MUSB_TEST_K;
>  
> -	if (strstarts(buf, "test J"))
> +	else if (strstarts(buf, "test J"))
>  		test = MUSB_TEST_J;
>  
> -	if (strstarts(buf, "test SE0 NAK"))
> +	else if (strstarts(buf, "test SE0 NAK"))
>  		test = MUSB_TEST_SE0_NAK;
>  
>  	musb_writeb(musb->mregs, MUSB_TESTMODE, test);
> -- 
> 1.7.9.5
> 

Tony, what do you think about that patch?

-- 
Pali Rohár
pali.rohar@...il.com