linux-ide.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tejun Heo <htejun@gmail.com>
To: Mark Lord <liml@rtr.ca>
Cc: Jeff Garzik <jgarzik@pobox.com>,
	IDE/ATA development list <linux-ide@vger.kernel.org>
Subject: Re: [PATCH 04/05] sata_mv: msi masking fix (v2)
Date: Thu, 22 Jan 2009 10:02:29 +0900	[thread overview]
Message-ID: <4977C5A5.8070202@gmail.com> (raw)
In-Reply-To: <49773FD1.3040101@rtr.ca>

Hello, Mark.

Mark Lord wrote:
> Enable reliable use of Message-Signaled Interrupts (MSI) in sata_mv
> by masking further chip interrupts within the main interrupt handler.
> 
> Based upon a suggestion by Grant Grundler.
> MSI is working reliably in all of my test systems here now.
> 
> Signed-off-by: Mark Lord <mlord@pobox.com>

Generally looks good, but...

> +    /* Enable message-switched interrupts, if requested */
> +    if (msi && 0 == pci_enable_msi(pdev))
> +        hpriv->hp_flags |= MV_HP_FLAG_MSI;

I just don't like CONSTANT OP EXPRESSION construct.  The only possible
upside it has is that it would make the compiler fail if '=' is used
instead of '==' but compiler has been smart enough to whine about
LVALUE = EXPRESSION for a very long time now.  How about plain
!pci_enable_msi(pdev) or pci_enable_msi(pdev) == 0?

Thanks.

-- 
tejun

  parent reply	other threads:[~2009-01-22  1:02 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-01-19 23:04 [PATCH 01/04] sata_mv: fix 8-port timeouts on 508x/6081 chips Mark Lord
2009-01-19 23:05 ` [PATCH 02/04] sata_mv: don't read hc_irq_cause Mark Lord
2009-01-19 23:06   ` [PATCH 03/04] sata_mv: remove bogus nsect restriction Mark Lord
2009-01-19 23:07     ` [PATCH 04/04] sata_mv: msi masking Mark Lord
2009-01-20 18:19       ` [PATCH 05/04] sata_mv: msi fix and cleanup Mark Lord
2009-01-20 18:49         ` [PATCH 06/06] sata_mv: remove experimental designation Mark Lord
2009-01-21  1:54       ` [PATCH 04/04] sata_mv: msi masking Grant Grundler
2009-01-21 15:15       ` Mark Lord
2009-01-21 15:31     ` [PATCH 04/05] sata_mv: msi masking fix (v2) Mark Lord
2009-01-21 15:34       ` [PATCH 05/05] sata_mv: no longer experimental (v2) Mark Lord
2009-01-26 11:43         ` Jeff Garzik
2009-01-21 17:03       ` [PATCH 04/05] sata_mv: msi masking fix (v2) Grant Grundler
2009-01-22  1:02       ` Tejun Heo [this message]
2009-01-22  5:17         ` Mark Lord
2009-01-22  5:19           ` Mark Lord
2009-01-22  5:22             ` Tejun Heo
2009-01-23 17:20               ` sata_mv: basic PIO-only ATAPI support? Mark Lord
2009-01-23 17:23                 ` Mark Lord
2009-01-23 17:52                   ` Alan Cox
2009-01-23 18:37                     ` Mark Lord
2009-01-26 11:42       ` [PATCH 04/05] sata_mv: msi masking fix (v2) Jeff Garzik
2009-01-26 11:39 ` [PATCH 01/04] sata_mv: fix 8-port timeouts on 508x/6081 chips Jeff Garzik

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4977C5A5.8070202@gmail.com \
    --to=htejun@gmail.com \
    --cc=jgarzik@pobox.com \
    --cc=liml@rtr.ca \
    --cc=linux-ide@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).