[lkml]   [2006]   [Oct]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    Subjectpci_set_power_state() failure and breaking suspend
    So I noticed a small regression that I think might uncover a deeper

    Recently, ohci1394 grew some "proper" error handling in its suspend
    function, something that looks like:

    err = pci_set_power_state(pdev, pci_choose_state(pdev, state));
    if (err)
    goto out;

    First, it breaks some old PowerBooks where the internal OHCI had PM
    feature exposed on PCI (the pmac specific code that follows those lines
    is enough on those machines).

    That can easily be fixed by removing the if (err) goto out; statement
    and having the pmac code set err to 0 in certain conditions, and I'll be
    happy to submit a patch for this.

    However, this raises the question of do we actually want to prevent
    machines to suspend when they have a PCI device that don't have the PCI
    PM capability ? I'm asking that because I can easily imagine that sort
    of construct growing into more drivers (sounds logical if you don't
    think) and I can even imagine somebody thinking it's a good idea to slap
    a __must_check on pci_set_power_state() ...

    I think the answer is not simple. For a lot of devices, it will be
    harmless, they will be either unaffected by the sleep transition or
    powered down and back up, and the driver will cope just fine. I suppose
    there can be issues for devices that do not support it, though none
    obvious comes to mind.

    It's one of those policies that are hard to define. But I think the best
    approach for now is to not fail the suspend when pci_set_power_state()
    returns an error.


    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to
    More majordomo info at
    Please read the FAQ at

     \ /
      Last update: 2006-10-24 08:59    [W:0.021 / U:14.448 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site