Ar Sul, 2006-10-15 am 16:44 -0700, ysgrifennodd Andrew Morton:
> Let me restore the words from my earlier email which you removed so that
> you could say that:
>
> For you the driver author to make assumptions about what's happening
> inside pci_set_mwi() is a layering violation. Maybe the br
Ar Sul, 2006-10-15 am 17:16 -0700, ysgrifennodd David Brownell:
> Signed-off-by: David Brownell <[EMAIL PROTECTED]>
Acked-by: Alan Cox <[EMAIL PROTECTED]>
>
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -499,7 +499,7 @@ int __must_check pci_enable_device_bars(
> void pci_disable_d
Ar Sul, 2006-10-15 am 18:10 -0700, ysgrifennodd Andrew Morton:
> Question is, should pci_set_mwi() ever return -EFOO? I guess it should, in
> the case where setting the line size didn't work out.
It does no harm, no driver will ever check anything but 0 v !0 because
the handling is no different i
On Sunday 15 October 2006 6:10 pm, Andrew Morton wrote:
> - A printk if something went bad
Where "bad" would I hope exclude cases where the device
doesn't support MWI ... that is, the "go faster if you can"
advice from the driver, where it isn't an error to run into
the common case of _this_ impl
On Mon, 16 Oct 2006 10:44:30 +1000
Paul Mackerras <[EMAIL PROTECTED]> wrote:
> Andrew Morton writes:
>
> > Let me restore the words from my earlier email which you removed so that
> > you could say that:
> >
> > For you the driver author to make assumptions about what's happening
> > inside
Andrew Morton writes:
> Let me restore the words from my earlier email which you removed so that
> you could say that:
>
> For you the driver author to make assumptions about what's happening
> inside pci_set_mwi() is a layering violation. Maybe the bridge got
> hot-unplugged. Maybe the a
On Sun, 15 Oct 2006 17:16:35 -0700
David Brownell <[EMAIL PROTECTED]> wrote:
>
> > You, the driver author _do not know_ what pci_set_mwi() does at present, on
> > all platforms, nor do you know what it does in the future.
>
> I know that it enables MWI accesses ... or fails. Beyond that, there
> > If the drivers doesn't care and if it makes no difference to performance
> > then just delete the call to pci_set_mwi().
> >
> > But if MWI _does_ make a difference to performance then we should tell
> > someone that it isn't working rather than silently misbehaving?
To repeat: it's not "mis
> > I agree that set_mwo() should set MWI if possible, and fail cleanly
> > if it couldn't (for whatever reason). Thing is, choosing to treat
> > that as an error must be the _driver's_ choice ... it'd be wrong to force
> > that policy into the _interface_ by forcing must_check etc.
>
> No. If
On Mon, 16 Oct 2006 10:00:25 +1000 Paul Mackerras <[EMAIL PROTECTED]> wrote:
> Andrew Morton writes:
>
> > If the drivers doesn't care and if it makes no difference to performance
> > then just delete the call to pci_set_mwi().
> >
> > But if MWI _does_ make a difference to performance then we s
Andrew Morton writes:
> If the drivers doesn't care and if it makes no difference to performance
> then just delete the call to pci_set_mwi().
>
> But if MWI _does_ make a difference to performance then we should tell
> someone that it isn't working rather than silently misbehaving?
That sounds
On Mon, 16 Oct 2006 01:02:40 +0100
Alan Cox <[EMAIL PROTECTED]> wrote:
> Ar Sul, 2006-10-15 am 16:18 -0700, ysgrifennodd Andrew Morton:
> > No. If pci_set_mwi() detects an unexpected error then the driver should
> > take some action: report it, recover from it, fail to load, etc. If the
> > driv
Ar Sul, 2006-10-15 am 16:18 -0700, ysgrifennodd Andrew Morton:
> No. If pci_set_mwi() detects an unexpected error then the driver should
> take some action: report it, recover from it, fail to load, etc. If the
> driver fails to do any of this then it's a buggy driver.
Wrong and there are severa
On Sun, 15 Oct 2006 15:45:58 -0700
David Brownell <[EMAIL PROTECTED]> wrote:
> > In that case its interface is misdesigned, because it doesn't discriminate
> > between "yes-it-does/no-it-doesn't" (which we don't want to report, because
> > either is expected and legitimate) and "something screwed
> > Most drivers should be able to say "enable MWI if possible, but
> > don't worry if it's not possible". Only a few controllers need
> > additional setup to make MWI actually work ... if they couldn't
> > do that setup, that'd be worth a warning before they backed off
> > to run in a non-MWI mod
Ar Sul, 2006-10-15 am 10:45 -0700, ysgrifennodd Andrew Morton:
> If the drivers doesn't care and if it makes no difference to performance
> then just delete the call to pci_set_mwi().
>
> But if MWI _does_ make a difference to performance then we should tell
> someone that it isn't working rather
On Sun, 15 Oct 2006 12:16:31 -0700
David Brownell <[EMAIL PROTECTED]> wrote:
> (From Alan Cox:)
> > The underlying bug is that someone marked pci_set_mwi must-check, that's
> > wrong for most of the drivers that use it. If you remove the must check
> > annotation from it then the problem and a tho
(From Alan Cox:)
> The underlying bug is that someone marked pci_set_mwi must-check, that's
> wrong for most of the drivers that use it. If you remove the must check
> annotation from it then the problem and a thousand other spurious
> warnings go away.
Yes, there seems to be abuse of this new "mu
On Sun, 15 Oct 2006 07:54:41 -0600
Matthew Wilcox <[EMAIL PROTECTED]> wrote:
> On Sat, Oct 14, 2006 at 11:53:59PM -0700, Andrew Morton wrote:
> > It seems to have stopped happening. I don't know why.
>
> Argh. Possibly a mistake during the bisect procedure?
I don't think so - I spent a lot of
On Sun, 15 Oct 2006 07:57:56 -0600
Matthew Wilcox <[EMAIL PROTECTED]> wrote:
> On Sun, Oct 15, 2006 at 03:21:22PM +0100, Alan Cox wrote:
> > Ar Sul, 2006-10-15 am 00:08 -0700, ysgrifennodd David Brownell:
> > > Since it's not an error, there should be no such printk ... which
> > > is exactly how
On Sun, Oct 15, 2006 at 03:21:22PM +0100, Alan Cox wrote:
> Ar Sul, 2006-10-15 am 00:08 -0700, ysgrifennodd David Brownell:
> > Since it's not an error, there should be no such printk ... which
> > is exactly how it's coded above.
>
> The underlying bug is that someone marked pci_set_mwi must-chec
On Sun, Oct 15, 2006 at 12:08:09AM -0700, David Brownell wrote:
> > But the only effect of returning EINVAL is a printk (for this particular
> > driver):
> >
> > /* PCI Memory-Write-Invalidate cycle support is optional (uncommon)
> > */
> > retval = pci_set_mwi(pdev);
> > i
Ar Sul, 2006-10-15 am 00:08 -0700, ysgrifennodd David Brownell:
> Since it's not an error, there should be no such printk ... which
> is exactly how it's coded above.
The underlying bug is that someone marked pci_set_mwi must-check, that's
wrong for most of the drivers that use it. If you remove t
On Sat, Oct 14, 2006 at 11:53:59PM -0700, Andrew Morton wrote:
> It seems to have stopped happening. I don't know why.
Argh. Possibly a mistake during the bisect procedure?
> gregkh-pci-pci-prevent-user-config-space-access-during-power-state-transitions.patch
> still breaks suspend though: http
> But the only effect of returning EINVAL is a printk (for this particular
> driver):
>
> /* PCI Memory-Write-Invalidate cycle support is optional (uncommon) */
> retval = pci_set_mwi(pdev);
> if (!retval)
> ehci_dbg(ehci, "MWI active\n");
Erm, I've lost con
On Sat, 14 Oct 2006 21:20:01 -0600
Matthew Wilcox <[EMAIL PROTECTED]> wrote:
> On Sat, Oct 14, 2006 at 01:48:55PM -0700, Andrew Morton wrote:
> > On Sat, 14 Oct 2006 08:02:49 -0600
> > Matthew Wilcox <[EMAIL PROTECTED]> wrote:
> >
> > > On Fri, Oct 13, 2006 at 09:41:35PM -0700, Andrew Morton wrot
On Sat, Oct 14, 2006 at 01:48:55PM -0700, Andrew Morton wrote:
> On Sat, 14 Oct 2006 08:02:49 -0600
> Matthew Wilcox <[EMAIL PROTECTED]> wrote:
>
> > On Fri, Oct 13, 2006 at 09:41:35PM -0700, Andrew Morton wrote:
> > > Bisection shows that this patch
> > > (pci-check-that-mwi-bit-really-did-get-se
On Sat, 14 Oct 2006 08:02:49 -0600
Matthew Wilcox <[EMAIL PROTECTED]> wrote:
> On Fri, Oct 13, 2006 at 09:41:35PM -0700, Andrew Morton wrote:
> > Bisection shows that this patch
> > (pci-check-that-mwi-bit-really-did-get-set.patch in Greg's PCI tree) breaks
> > suspend-to-disk on my Vaio. It writ
On Fri, Oct 13, 2006 at 09:41:35PM -0700, Andrew Morton wrote:
> Bisection shows that this patch
> (pci-check-that-mwi-bit-really-did-get-set.patch in Greg's PCI tree) breaks
> suspend-to-disk on my Vaio. It writes the suspend image and gets to the
> point where it's supposed to power down, but do
On Fri, Oct 13, 2006 at 09:41:35PM -0700, Andrew Morton wrote:
> On Fri, 06 Oct 2006 13:05:18 -0600
> Matthew Wilcox <[EMAIL PROTECTED]> wrote:
>
> > Since some devices may not implement the MWI bit, we should check that
> > the write did set it and return an error if it didn't.
> >
> > Signed-of
On Fri, 06 Oct 2006 13:05:18 -0600
Matthew Wilcox <[EMAIL PROTECTED]> wrote:
> Since some devices may not implement the MWI bit, we should check that
> the write did set it and return an error if it didn't.
>
> Signed-off-by: Matthew Wilcox <[EMAIL PROTECTED]>
>
> diff --git a/drivers/pci/pci.c
Matthew Wilcox wrote:
Since some devices may not implement the MWI bit, we should check that
the write did set it and return an error if it didn't.
Signed-off-by: Matthew Wilcox <[EMAIL PROTECTED]>
ACK, though (as it seems you are doing) you should audit pci_set_mwi()
users to make sure behav
Since some devices may not implement the MWI bit, we should check that
the write did set it and return an error if it didn't.
Signed-off-by: Matthew Wilcox <[EMAIL PROTECTED]>
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index a544997..3d041f4 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pc
33 matches
Mail list logo
