Re: [PATCH] fix tulip suspend/resume

From: Adam Belay (ambx1_at_neo.rr.com)
Date: 06/09/05

  • Next message: Greg Stark: "Re: SMART support for libata"
    Date:	Wed, 8 Jun 2005 21:48:03 -0400
    To: Andrew Morton <akpm@osdl.org>, Davide Rossetti <davide.rossetti@roma1.infn.it>, Karsten Keil <kkeil@suse.de>
    
    

    On Wed, Jun 08, 2005 at 08:11:36PM +0200, Davide Rossetti wrote:
    >
    > >Looks OK and also work on my HW.
    > >
    > >Andrew, can you please take this updated version, it is much cleaner and
    > >safer, because it handle the IRQ correctly.
    > >
    > >
    > >
    > >>--- a/drivers/net/tulip/tulip_core.c 2005-05-27 22:06:00.000000000 -0400
    > >>+++ b/drivers/net/tulip/tulip_core.c 2005-06-07 16:34:13.641177456 -0400
    > >>@@ -1756,11 +1756,19 @@
    > >>{
    > >> struct net_device *dev = pci_get_drvdata(pdev);
    > >>
    > >>- if (dev && netif_running (dev) && netif_device_present (dev)) {
    > >>- netif_device_detach (dev);
    > >>- tulip_down (dev);
    > >>- /* pci_power_off(pdev, -1); */
    > >>- }
    > >>+ if (!dev)
    > >>+ return -EINVAL;
    > >>+
    > >>+ if (netif_running(dev))
    > >>+ tulip_down(dev);
    > >>+
    > >>+ netif_device_detach(dev);
    > >>+ pci_save_state(pdev);
    > >>+
    > >>+ free_irq(dev->irq, dev);
    > >>
    > >>
    > isn't it safer to swap these last two guys ?? in theory there could be
    > an IRQ betwen pci_save_state() and free_irq()...

    Well, so not exactly. The issue here was that we might access the hardware
    while it's:
    a.) physically powered down
    b.) making a power transition

    In this case, the device is still powered on, so there shouldn't be a problem.
    However, changing the order shouldn't break anything. If you think it would
    be cleaner this way and someone is willing to confirm that it still works,
    then we can change it to the patch below.

    Thanks,
    Adam

    Here's an updated patch:

    [PM] fix suspend/resume for tulip based cards

    This patch allows the tulip driver to suspend and resume properly. It was
    originally written by Karsten Keil and then modified by Adam Belay.

    Signed-off-by: Karsten Keil <kkeil@suse.de>
    Signed-off-by: Adam Belay <abelay@novell.com>

    --- a/drivers/net/tulip/tulip_core.c 2005-05-27 22:06:00.000000000 -0400
    +++ b/drivers/net/tulip/tulip_core.c 2005-06-08 21:28:26.111797528 -0400
    @@ -1756,11 +1756,19 @@
     {
             struct net_device *dev = pci_get_drvdata(pdev);
     
    - if (dev && netif_running (dev) && netif_device_present (dev)) {
    - netif_device_detach (dev);
    - tulip_down (dev);
    - /* pci_power_off(pdev, -1); */
    - }
    + if (!dev)
    + return -EINVAL;
    +
    + if (netif_running(dev))
    + tulip_down(dev);
    +
    + netif_device_detach(dev);
    + free_irq(dev->irq, dev);
    +
    + pci_save_state(pdev);
    + pci_disable_device(pdev);
    + pci_set_power_state(pdev, pci_choose_state(pdev, state));
    +
             return 0;
     }
     
    @@ -1768,15 +1776,26 @@
     static int tulip_resume(struct pci_dev *pdev)
     {
             struct net_device *dev = pci_get_drvdata(pdev);
    + int retval;
     
    - if (dev && netif_running (dev) && !netif_device_present (dev)) {
    -#if 1
    - pci_enable_device (pdev);
    -#endif
    - /* pci_power_on(pdev); */
    - tulip_up (dev);
    - netif_device_attach (dev);
    + if (!dev)
    + return -EINVAL;
    +
    + pci_set_power_state(pdev, PCI_D0);
    + pci_restore_state(pdev);
    +
    + pci_enable_device(pdev);
    +
    + if ((retval = request_irq(dev->irq, &tulip_interrupt, SA_SHIRQ, dev->name, dev))) {
    + printk (KERN_ERR "tulip: request_irq failed in resume\n");
    + return retval;
             }
    +
    + netif_device_attach(dev);
    +
    + if (netif_running(dev))
    + tulip_up(dev);
    +
             return 0;
     }
     
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/


  • Next message: Greg Stark: "Re: SMART support for libata"

    Relevant Pages

    • Re: [PATCH 2/3] block cleanups: Add kconfig default iosched submenu
      ... > anticipatory is not compiled in or another default is preferred. ... This version is cleaner, since it eliminates all the #ifdef's in the ... Can you rebase this against latest -mm, it has the other patch you ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)
    • Re: [PATCH] silence spinlock/rwlock uninitialized break_lock member warnings
      ... > to add the patch himself. ... those are the ones I aim to fix. ... initializers could result in something cleaner - I'll look at that)... ... To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ ...
      (Linux-Kernel)
    • Re: [Fwd: Re: [patch] Real-Time Preemption, -RT-2.6.9-mm1-V0.4]
      ... > let me try some more hacks to make this a little bit safer. ... Please apply this patch and do the changeall-tree ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)
    • Re: [PATCH] ib700wdt watchdog driver for 2.6.6
      ... things will be cleaner if we use the following patch. ... else printk(KERN_CRIT PFX "WDT device closed unexpectedly. ... To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ ...
      (Linux-Kernel)
    • Re: [PATCH] x86_64 io_apic.c: Memorize at bootup where the i8259 is connected
      ... All my comments on the x86 side also apply to this x86-64 version of the ... patch. ... I think the code would be cleaner too, if we did this inside the existing ... send the line "unsubscribe linux-kernel" in ...
      (Linux-Kernel)