From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755018AbcIIUVU (ORCPT ); Fri, 9 Sep 2016 16:21:20 -0400 Received: from galahad.ideasonboard.com ([185.26.127.97]:59636 "EHLO galahad.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753192AbcIIUVQ (ORCPT ); Fri, 9 Sep 2016 16:21:16 -0400 From: Laurent Pinchart To: Tony Lindgren Cc: Andreas Kemnade , Bin Liu , Greg Kroah-Hartman , 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, 09 Sep 2016 23:21:50 +0300 Message-ID: <1538976.gm2HNISj8k@avalon> User-Agent: KMail/4.14.10 (Linux/4.4.6-gentoo; KDE/4.14.20; x86_64; ; ) In-Reply-To: <20160909200803.4cngkfhgkki4e7o3@atomide.com> References: <1470238731-32358-1-git-send-email-andreas@kemnade.info> <1946895.vi37Pmm05X@avalon> <20160909200803.4cngkfhgkki4e7o3@atomide.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Tony, On Friday 09 Sep 2016 13:08:03 Tony Lindgren wrote: > * Laurent Pinchart [160909 12:27]: > > On Wednesday 03 Aug 2016 17:38:51 Andreas Kemnade wrote: > >> The code assumes that omap2430_musb_enable() and > >> omap2430_musb_disable() are called in a balanced way. > >> That fact is broken by the fact that musb_init_controller() calls > >> musb_platform_disable() to switch from unknown state to off state > >> on initialisation. > >> > >> That means that phy_power_off() is called first so that > >> phy->power_count gets -1 and the phy is not enabled on phy_power_on(). > >> So when usb gadget is started the phy is not powered on. > >> Depending on the phy used that caused various problems. > >> Besides of causing usb problems, that can also have side effects. > >> > >> In the case of using the phy_twl4030, that prevents also charging > >> the battery via usb (using twl4030_charger) and so makes further > >> kernel debugging hard. > >> The problem was seen with 4.7 on an openphoenux gta04. It has a DM3730 > >> SoC and a TPS65950 companion. phy->power never became 1 > >> and so the usb did get powered on. > >> > >> The patch prevents phy_power_off() from being called when > >> it is already off. > >> > >> Signed-off-by: Andreas Kemnade > > > > This fixes USB gadget operation on the Panda board. > > > > Fixes: a83e17d0f73b ("usb: musb: Improve PM runtime and phy handling for > > 2430 glue layer") > > Tested-by: Laurent Pinchart > > 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 ? :-) 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. Feel free to send me a better fix and I will test it. -- Regards, Laurent Pinchart