On Fri, 2013-09-27 at 15:03 -0600, Bjorn Helgaas wrote:
> [+cc Ben]
>
> On Thu, Sep 12, 2013 at 2:20 AM, Yijing Wang <wangyijing@xxxxxxxxxx> wrote:
> > When we setup MSI, if device power state is not equal to PCI_D0,
> > system will silently ignore writing MSI message, but pci_enable_msi()
> > still return 0 which seems setup successfully. So we should warn here
> > to help diagnose this issue.
> >
> > Signed-off-by: Yijing Wang <wangyijing@xxxxxxxxxx>
> > ---
> > drivers/pci/msi.c | 2 ++
> > 1 files changed, 2 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> > index aca7578..25ed59d 100644
> > --- a/drivers/pci/msi.c
> > +++ b/drivers/pci/msi.c
> > @@ -291,6 +291,8 @@ void __write_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
> > {
> > if (entry->dev->current_state != PCI_D0) {
> > /* Don't touch the hardware now */
> > + dev_warn(&entry->dev->dev,
> > + "current_state != PCI_D0, ignore writing MSI message!\n");
>
> Is there a real problem here? If there is a real problem, a printk
> doesn't help fix it. If there's no problem, I don't see the point of
> this printk.
>
> I would expect that if the device is not in D0, we should remember the
> hardware updates that need to be made, and if the device returns to
> D0, we should apply the updates then. If that's the case this is not
> an error and we shouldn't warn about it.
[...]
During system suspend all IRQ affinities are changed to the boot CPU as
other CPUs are disabled, and then I think they are reverted during
system resume. This certainly used to be done while most devices were
suspended. That's why we have to check for the device power state here.
If suspend/resume is still done in the same order then we shouldn't log
a warning about this.
Yijing, why not add a check for this in pci_enable_msi() itself if you
think it's that important to warn about?
Ben.
--
Ben Hutchings
Humans are not rational beings; they are rationalising beings.
Attachment:
signature.asc
Description: This is a digitally signed message part
