linux-kernel - Re: [PATCH] usb: musb: fix pm

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: <CAMP44s1GCPRjV4weNN1GpboDPbzUobBLVP+u2Y4QvHrt2KoG_Q@mail.gmail.com>
Date:	Fri, 16 Dec 2011 03:13:13 +0200
From:	Felipe Contreras <felipe.contreras@...il.com>
To:	balbi@...com
Cc:	linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
	Greg Kroah-Hartman <gregkh@...e.de>,
	Jarkko Nikula <jarkko.nikula@...mer.com>,
	stable@...r.kernel.org, Hema Kalliguddi <hemahk@...com>
Subject: Re: [PATCH] usb: musb: fix pm_runtime mismatch

On Fri, Dec 16, 2011 at 1:50 AM, Felipe Balbi <balbi@...com> wrote:
> On Fri, Dec 16, 2011 at 01:31:02AM +0200, Felipe Contreras wrote:
>> On Fri, Dec 16, 2011 at 1:01 AM, Felipe Balbi <balbi@...com> wrote:
>> > On Fri, Dec 16, 2011 at 12:42:14AM +0200, Felipe Contreras wrote:
>> >> --- a/drivers/usb/musb/musb_core.c
>> >> +++ b/drivers/usb/musb/musb_core.c
>> >> @@ -2012,8 +2012,6 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
>> >>       if (status < 0)
>> >>               goto fail3;
>> >>
>> >> -     pm_runtime_put(musb->controller);
>> >
>> > To me the real fix would be add the missing pm_runtime_get_sync(). On
>> > probe() we're actually accessing MUSB's address space which needs it's
>> > clocks turned on. I guess it's only working now by chance, probably
>> > because glue layer calls pm_runtime_get_sync() to access it's own
>> > address space and that uses the same clocks.
>>
>> Are you sure it's "musb-hdrc", and not "musb-omap2430" the one
>> accessing the relevant address-space? From the runtime_pm
>> documentation it looks like only the probe function should deal with
>> this.
>>
>> If "musb-hdrc" was truly accessing these registers, then I would get
>> the same failure because the clocks are turned off, but I don't...
>
> see musb_core_init(); You don't see any problems when accessing those
> addresses because musb_platform_init() will fall into
> omap2430_musb_init() which calls pm_runtime_get_sync(), and the same
> clock actually enables both address spaces (musb-omap2430 and
> musb-hdrc).

That's true, but how would I go test this theory? Call
pm_runtime_put_sync() at the end of omap2430_musb_init()?

Also, I think most pm_runtime_disable() calls shouldn't be there...
That would tell PM to activate power for the device, which is not what
we want. The core drivers code will take care of truly disabling the
pm_runtime stuff _without_ activating the device.

Cheers.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/