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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 15-09-16 16:43, Tony Lindgren wrote:
* Hans de Goede <hdegoede@xxxxxxxxxx> [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 <andreas@xxxxxxxxxxxx>.

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().

Not really, musb_init_controller does little of interest before
calling musb_platform_init(), which is a function pointer
to sunxi_musb_init().

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?

Nope, I no the call chain, but nothing interesting happens between
sunxi_musb_probe() and sunxi_musb_init().

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.

Ah is this patch for 4.8-rc, yeah then definitely keep it as
is, as I tried to explain in my previous mail, my remark
was only made in case you need to respin the patch for other
reasons, if not then it is fine as is.

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux