[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <200710200736.22129.maximlevitsky@gmail.com>
Date: Sat, 20 Oct 2007 07:36:21 +0200
From: Maxim Levitsky <maximlevitsky@...il.com>
To: benh@...nel.crashing.org
Cc: Linus Torvalds <torvalds@...ux-foundation.org>,
akpm <akpm@...ux-foundation.org>,
Linux Kernel list <linux-kernel@...r.kernel.org>,
linuxppc-dev list <linuxppc-dev@...abs.org>
Subject: Re: [PATCH] synchronize_irq needs a barrier
On Saturday 20 October 2007 07:04:35 Benjamin Herrenschmidt wrote:
>
> > 1) some drivers use pci_disable_device(), and pci_enable_device().
> > should I use it too?
>
> I generally don't do the former, and I would expect the late to be done
> by pci_restore_state() for you. pci_disable_device(), last I looked,
> only cleared the bus master bit though, which might be a good idea to
> do.
>
> People in ACPI/x86 land, are there more good reasons to do one or the
> other ?
>
> That reminds me that I volunteered to write a documentation on how
> drivers should do all that stuff at KS and didn't get to actually doing
> it yet. shame ... I'll try to start something asap.
>
> > 2) I accidentally did this:
> > pci_set_power_state(pci_dev, pci_choose_state(pci_dev, state));
> > pci_save_state(pci_dev);
> >
> > I somehow thought that this is correct, that I should save the pci config state
> > after the power-down, but now I know that it isn't correct.
>
> Right, you need to do the save_state before the power down. You need to
> avoid pretty much any access to the device after the save state other
> than the pending set_power_state on resume.
>
> > I now need to send a patch for dmfe.c network driver that has the same commands written by me.
> > (but it works perfectly anyway)
>
> On x86 desktop... might have surprises on others.
>
> > Is it possible to access pci configuration space in D3?
>
> It's only really safe to access the PM register itself, though I suppose
> you should be able to walk the capability chain to do that. But I
> wouldnt recommend doing anything else.
>
> > And lastly speaking of network drivers, one issue came to my mind:
> > most network drivars has a packet queue and in case of dmfe it is located in main memory,
> > and card does dma from it.
>
> Note that the network stack nowadays does a fair bit of cleaning up for
> you before your suspend routine is called....
> >
> > in .suspend I ignore that some packets may be in that queue, and I want
> > to ask, whenever there are better ways to do that.
> >
> >
> > this is my dmfe .suspend routine.
> >
> > /* Disable upper layer interface */
> > netif_device_detach(dev);
>
>
> Looks allright on a quick glance appart from the bits we already
> discussed.
>
> > I guess, everybody makes mistakes... :-)
> >
> > Other network drivers has a bit more complicated .suspend/.resume routines,
> > but I didn't see a driver waiting for output queue to finish
>
> I think the network stack does that nowadays but we'll have to double
> check, that's based on what DaveM told me at KS.
>
> Ben.
>
>
Hi,
Thanks a lot.
I fix the order of calls in dmfe.c
and in saa7134-core.c.
I probably need to add this synchronize_irq() logic in dmfe.c too, but I probably do it later,
I think I am overestimating this race, since most drivers don't do dev->insuspend checks in IRQ handler.
Maybe even just use free_irq() after all....
Best regards,
Maxim Levitsky
-
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/
Powered by blists - more mailing lists