From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tony Lindgren Subject: Re: [PATCHv3] usb: musb: Fix unbalanced platform_disable Date: Thu, 15 Sep 2016 07:43:46 -0700 Message-ID: <20160915144343.jpeoizgbw2o23iuc@atomide.com> References: <20160914181019.22937-1-tony@atomide.com> <79edbb45-cf85-1aba-1d42-1897021e0904@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <79edbb45-cf85-1aba-1d42-1897021e0904-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> Sender: linux-usb-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Hans de Goede Cc: Bin Liu , Greg Kroah-Hartman , Andreas Kemnade , Felipe Balbi , George Cherian , Kishon Vijay Abraham I , Ivaylo Dimitrov , Ladislav Michl , Sergei Shtylyov , linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: linux-omap@vger.kernel.org * Hans de Goede [160915 07:00]: > Hi, > > On 14-09-16 20:10, Tony Lindgren wrote: > > Commit a83e17d0f73b ("usb: musb: Improve PM runtime and phy handling > > for 2430 glue layer") moved PHY enable/disable calls to happen from > > omap2430_musb_enable/disable(). That broke enumeration for several > > devices as PM runtime in the PHY will never enable it. > > > > The root cause of the problem is unpaired calls from musb_core.c to > > musb_platform_enable/disable in musb_core.c as reported by > > Andreas Kemnade . > > > > As musb_platform_enable/disable are being called from various functions, > > let's not attempt to make them paiered immediately. This would require > > fixing all the callers like musb_remove. > > Yeah, the sunxi glue actually has: > > static void sunxi_musb_enable(struct musb *musb) > { > struct sunxi_glue *glue = dev_get_drvdata(musb->controller->parent); > > glue->musb = musb; > > /* musb_core does not call us in a balanced manner */ > if (test_and_set_bit(SUNXI_MUSB_FL_ENABLED, &glue->flags)) > return; Heh we're trying to get to the "balanced manner" point hopefully soon :) > schedule_work(&glue->work); > } > > static void sunxi_musb_disable(struct musb *musb) > { > struct sunxi_glue *glue = dev_get_drvdata(musb->controller->parent); > > clear_bit(SUNXI_MUSB_FL_ENABLED, &glue->flags); > } > > Note that since the glue struct is kzalloc-ed, this makes calling > sunxi_musb_disable() from sunxi_musb_init() a nop, so if this > needs a respin please drop the sunxi changes, they are not > necessary. Hmm we have sunxi_musb_probe() do platform_device_register_full(), then musb_probe() happens, and calls musb_init_controller(). In musb_init_controller() we do musb_platform_init() that finally calls sunxi_musb_init(). So the sunxi glue is kzalloc'ed and initialized already in sunxi_musb_probe(), and musb parts are already initialized in musb_init_controller(). I don't quite follow what you mean how it's a nop.. Care to clarify that a bit? Maybe you're thinking we're calling it from sunxi_musb_probe() instead? Anyways, calling sunxi_musb_disable() seems unnecessary from sunxi_musb_init() because it does reset_control_deassert(). But that can be cleaned up later on if no other reasons for changes for v4.8-rc cycle. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html