Re: [v2] musb: omap2430: do not assume balanced enable()/disable()

From: Andreas Kemnade <andreas@kemnade.info>
To: Tony Lindgren <tony@atomide.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Bin Liu <b-liu@ti.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-usb@vger.kernel.org, linux-omap@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [v2] musb: omap2430: do not assume balanced enable()/disable()
Date: Fri, 9 Sep 2016 23:22:06 +0200	[thread overview]
Message-ID: <20160909232206.70ee2558@aktux> (raw)
In-Reply-To: <20160909205103.sqonsprxbb7i6zth@atomide.com>

On Fri, 9 Sep 2016 13:51:04 -0700
Tony Lindgren <tony@atomide.com> wrote:

> * Laurent Pinchart <laurent.pinchart@ideasonboard.com> [160909 13:21]:
> > On Friday 09 Sep 2016 13:08:03 Tony Lindgren wrote:
> > > This patch has a side effect of fixing the issue by breaking PM
> > > runtime, not a good fix as discussed.
> > 
> > How exactly is it worse breaking runtime PM than breaking USB
> > gadget completely ? :-)
> 
> Yeah sorry to break it, I obviously did not test it on all
> platforms :( I'm mostly using omap3 with the 2430 glue layer and
> am335x for the dsps glue layer and did not know that omap4 is broken.
> I guess I've recently just used the EHCI ports on panda.
> 
> > The issue here is that the .disable() platform operation is called
> > by musb with the PHY already powered off, leading to the PHY power
> > reference count becoming negative. The next call to the .enable()
> > operation restores the reference count to 0 without enabling the
> > PHY.
> 
> Well for the phy-twl4030-usb.c, AFAIK the right fix is to fix the PHY
> driver as done in "[PATCH v2] phy-twl4030-usb: initialize
> charging-related stuff via pm_runtime". I suspect something similar
> is happening here also with the omap4 legacy phy.
> 
No, the fix is for making charging work independant of musb.
Gadget is working because charging is enabled and enables all parts in
the phy needed for it. And you can charge without musb (only musb_hdrc
for the mailbox but not the omap2430 glue module).

We have two independant things:
1. phy-twl4030-usb (and perhaps others) do not enable
  the phy enough to allow charging on pm_runtime_get().
  That is fixed by my phy-related patches.

2. phy_power_off/on() in called in an unbalanced way if
   it is called behind musb_platform_enable()/disable()
   as it happens in omap2430.c. Two ways to fix it:
   a) prevent phy_power_off()/on() to be called in
      an unbalanced way in omap240.c
   b) prevent musb_platform_enable()
	      musb_platform_disable() to be called in an
	      unbalanced way by fixing musb_core.c

Fixing 1. is enough on gta04 to fix charging and hide 2. enough to
have gadget working for the most common usecases. (not using
twl4030-charger would not work yet)
But in the longer term 2. has to be fixed too.

Regards,
Andreas Kemnade