linux-kernel - Re: [Letux-kernel] [PATCH v2] musb: omap2430: do not assume balanced enable()/disable()

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: <20160805172039.6dac0aeb@aktux>
Date:	Fri, 5 Aug 2016 17:20:39 +0200
From:	Andreas Kemnade <andreas@...nade.info>
To:	Tony Lindgren <tony@...mide.com>
Cc:	Discussions about the Letux Kernel <letux-kernel@...nphoenux.org>,
	Linux USB Mailing List <linux-usb@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	linux-omap <linux-omap@...r.kernel.org>, Bin Liu <b-liu@...com>
Subject: Re: [Letux-kernel] [PATCH v2] musb: omap2430: do not assume
 balanced enable()/disable()

On Fri, 5 Aug 2016 06:55:01 -0700
Tony Lindgren <tony@...mide.com> wrote:

> * Andreas Kemnade <andreas@...nade.info> [160804 09:44]:
> > Nothing happens here, so the previous state of the phy remains.
> > It would be disabled by the generic phy layer in
> > drivers/phy/phy-core.c
> >
> > > gadget driver is loaded.
> > > musb_start() is called
> > >     omap2430_musb_enable() is called
> > > 	calls phy_power_on(),
> > > 	phy->power_count goes to 0,
> > > 	phy is not powered on because power_count != 1
> > > -> no gadget working, no charging.
> > > 
> > ... if not configured by u-boot before. USB gadget might work when
> > initialized earlier in the boot process (u-boot/x-loader/mlo ...)
> > and phy-twl4030 cannot do anything about it besides if we change
> > drivers/phy/phy-core.c
> 
> Ssounds like an issue in the phy-twl4030-usb.c. Let's try to figure
> out what all we need set there for the various components there.
> 
> PM runtime for phy-twl4030-usb.c should be for the whole PHY
> device including all it's components. Then the charger and
> MUSB should separately increment the PM runtime count and then
> enable the component specific things. And then we have the
> errata to deal with when VBUS is enabled.

I repeat the subject line of the patch:
[PATCH v2] musb: omap2430: do not assume balanced enable()/disable()
It is *not* fix charging.

So you mean that the phy should magically know at which refcounter
it should power on and off if power on/off is not called in the
balanced way?

Maybe the phy-twl4030 uses the phy layer wrong. 
Now the relevant part of power on/off in phy-twl4030 is done via struct
phy_ops.power_off/power_on (*not* via pm_runtime). Maybe that is wrong
and more parts should be moved to the pm_runtime stuff (which is also
present). 
Then the phy subsystem has its own power refcounter in struct
phy.power_count. It it handled via phy_power_off()/phy_power_on().
And that is called from musb/omap2430.c 
But that is another story. 

> 
> If there are MUSB specific PM runtime issues then let's fix
> those separately.
> 
And that exactly tries my patch to do. For that task it does not
even use the PM runtime system. Again please read the subject line of
the patch. Maybe it unveils some other pm issues in musb
which should first be fixed in a complete patch series.

Regards,
Andreas