Re: [PATCHv2] usb: musb: Fix unbalanced platform_disable

From: Bin Liu <b-liu-l0cyMroinI0@public.gmane.org>
To: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
Cc: Andreas Kemnade <andreas-cLv4Z9ELZ06ZuzBka8ofvg@public.gmane.org>,
	Greg Kroah-Hartman
	<gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>,
	Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org>,
	linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Laurent Pinchart
	<laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>,
	Rolf Peukert <rolf.peukert-cSCHQGRfioA@public.gmane.org>
Subject: Re: [PATCHv2] usb: musb: Fix unbalanced platform_disable
Date: Tue, 13 Sep 2016 09:14:48 -0500	[thread overview]
Message-ID: <20160913141448.GN18340@uda0271908> (raw)
In-Reply-To: <20160913031805.m5nzyqyclh4pyj6k-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>

Hi,

On Mon, Sep 12, 2016 at 08:18:05PM -0700, Tony Lindgren wrote:
> * Bin Liu <b-liu-l0cyMroinI0@public.gmane.org> [160912 11:36]:
> > On Mon, Sep 12, 2016 at 08:05:30PM +0200, Andreas Kemnade wrote:
> > > Hmm, then the question is: Couldn't the X_musb_disable simply be called
> > > from X_probe if needed to be an the safe side?
> > 
> > In general, we try not to do so if all possible. We want to put common
> > code in the core, not repleat them in glue layers.
> > 
> > In this specific case, we cannot do it. For example in dsps glue, the
> > musb reset is done in dsps_musb_init(), so no place in dsps_probe() to
> > call dsps_musb_disable().
> 
> OK yeah if we need to do something, then disabling the irq in
> musb_core.c until we're done should be enough.

I don't think we can disable it in musb core directly. *_musb_disable()
in glue layers disables the musb wrapper's irq, not the core's irq.

But I guess we can let *_musb_init() directly calls *_musb_disable() in
the glue drivers right after musb reset. This would have to touch almost
all glue drivers, but trivial change. Thoughts?

> 
> Anyways, I tested the $subject patch also with am35x code with the
> following patch and no issues. So I've now tested with omap3,
> omap4, am335x, and am35x.

I don't expect your v2 breaks am35x either, as I believe its default of
out-of-reset already disables the irq. I just don't like that SW relies
on unguaranteed HW default state.

> There are also am35x musb device tree patches from last year by
> Rolf Peukert <rolf.peukert-cSCHQGRfioA@public.gmane.org>:
> 
> https://patchwork.kernel.org/project/linux-omap/list/?submitter=144061

Good to know this work.

> 
> And we now have new drivers/phy/phy-da8xx-usb.c that might be the
> same phy on am35x except with register bits moved around.
> 
> So maybe we'll have it working with device tree quite easily.
> Meanwhile, we should probably apply the following patch so we
> get things working again.

Yup.

Regards,
-Bin.
--
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