From: Matthew Wilcox <matthew@wil.cx>
To: Mark Maule <maule@sgi.com>
Cc: linux-ia64@vger.kernel.org, linux-kernel@vger.kernel.org,
Tony Luck <tony.luck@intel.com>
Subject: Re: [PATCH 2/4] msi vector targeting abstractions
Date: Wed, 21 Dec 2005 12:05:58 -0700 [thread overview]
Message-ID: <20051221190558.GD2361@parisc-linux.org> (raw)
In-Reply-To: <20051221184348.5003.7540.53186@attica.americas.sgi.com>
On Wed, Dec 21, 2005 at 12:42:41PM -0600, Mark Maule wrote:
> {
> struct msi_desc *entry;
> - struct msg_address address;
> + uint32_t address_hi, address_lo;
Don't use uint32_t. Use u32 instead.
> @@ -108,28 +125,38 @@
> if (!(pos = pci_find_capability(entry->dev, PCI_CAP_ID_MSI)))
> return;
>
> + pci_read_config_dword(entry->dev, msi_upper_address_reg(pos),
> + &address_hi);
> pci_read_config_dword(entry->dev, msi_lower_address_reg(pos),
> - &address.lo_address.value);
> - address.lo_address.value &= MSI_ADDRESS_DEST_ID_MASK;
> - address.lo_address.value |= (cpu_physical_id(dest_cpu) <<
> - MSI_TARGET_CPU_SHIFT);
> - entry->msi_attrib.current_cpu = cpu_physical_id(dest_cpu);
> + &address_lo);
> +
> + msi_callouts.msi_target(vector, dest_cpu,
> + &address_hi, &address_lo);
> +
> + pci_write_config_dword(entry->dev, msi_upper_address_reg(pos),
> + address_hi);
> pci_write_config_dword(entry->dev, msi_lower_address_reg(pos),
> - address.lo_address.value);
> + address_lo);
But actually, I don't understand why you don't just pass a msg_address
pointer to msi_target instead.
(last two points apply throughtout this patch)
>
> + (*msi_callouts.msi_teardown)(vector);
> +
Yuck. There's a reason C allows you to call through function pointers as if
they were functions.
> +int
> +msi_register_callouts(struct msi_callouts *co)
> +{
> + msi_callouts = *co; /* structure copy */
> + return 0;
Why do it this way instead of having a pointer to a struct?
> -struct msg_data {
> +union msg_data {
> + struct {
How about leaving struct msg_data alone and adding
union good_name {
struct msg_data;
u32 value;
}
Or possibly struct msg_data should just be deleted and we should use
shift/mask to access the contents of it. ISTR GCC handled that much
better.
next prev parent reply other threads:[~2005-12-21 19:06 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2005-12-21 18:42 [PATCH 0/4] msi abstractions and support for altix Mark Maule
2005-12-21 18:42 ` [PATCH 1/4] msi archetecture init hook Mark Maule
2005-12-21 18:53 ` Matthew Wilcox
2005-12-21 19:03 ` Mark Maule
2005-12-21 19:40 ` Matthew Wilcox
2005-12-21 18:42 ` [PATCH 2/4] msi vector targeting abstractions Mark Maule
2005-12-21 18:56 ` Christoph Hellwig
2005-12-21 19:17 ` Mark Maule
2005-12-21 19:05 ` Matthew Wilcox [this message]
2005-12-21 19:17 ` linux-os (Dick Johnson)
2005-12-21 19:33 ` Mark Maule
2005-12-22 10:36 ` Christoph Hellwig
2005-12-22 13:59 ` Mark Maule
2005-12-21 18:42 ` [PATCH 3/4] per-platform IA64_{FIRST,LAST}_DEVICE_VECTOR definitions Mark Maule
2005-12-21 19:09 ` Matthew Wilcox
2005-12-21 19:18 ` Mark Maule
2005-12-21 19:32 ` Matthew Wilcox
2005-12-22 6:26 ` Keith Owens
2005-12-22 13:14 ` Matthew Wilcox
2005-12-21 18:42 ` [PATCH 4/4] altix: msi support Mark Maule
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=20051221190558.GD2361@parisc-linux.org \
--to=matthew@wil.cx \
--cc=linux-ia64@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=maule@sgi.com \
--cc=tony.luck@intel.com \
/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).