On Wed, Dec 21, 2005 at 06:56:37PM +0000, Christoph Hellwig wrote:
> On Wed, Dec 21, 2005 at 12:42:41PM -0600, Mark Maule wrote:
> > Abstract portions of the MSI core for platforms that do not use standard
> > APIC interrupt controllers. This is implemented through a set of callouts
> > which default to current behavior, but which can be overridden by calling
> > msi_register_callouts() in the platform msi init code.
>
> we tend to calls these _ops or _operations instead of _callouts.
> Also I'd suggest to not keep the generic ones where they are but
> in a separate file and let the existing plattforms calls msi_register()
> with the ops table for those. This keeps the interface symmetric instead
> of favouring the first implementation.
ok.
>
> > @@ -89,10 +91,25 @@
> > }
> >
> > #ifdef CONFIG_SMP
> > +static void msi_target_generic(unsigned int vector,
> > + unsigned int dest_cpu,
> > + uint32_t *address_hi, /* in/out */
> > + uint32_t *address_lo) /* in/out */
>
> Please try to use u32 instead of uint32_t everywhere. Dito for other
> sizes and signed types.
ok.
>
> > +{
> > + struct msg_address address;
> > +
> > + address.lo_address.value = *address_lo;
> > + address.lo_address.value &= MSI_ADDRESS_DEST_ID_MASK;
> > + address.lo_address.value |=
> > + (cpu_physical_id(dest_cpu) << MSI_TARGET_CPU_SHIFT);
> > +
> > + *address_lo = address.lo_address.value;
> > +}
>
> Why do we need the full struct msg_address here? What about just:
>
> static void msi_target_apic(unsigned int vector, unsigned int dest_cpu,
> u32 *address_hi, u32 *address_lo)
> {
> u32 addr = *address_lo;
>
> addr &= MSI_ADDRESS_DEST_ID_MASK;
> addr |= (cpu_physical_id(dest_cpu) << MSI_TARGET_CPU_SHIFT);
>
> *address_lo = addr;
> }
Right.
>
> > + (*msi_callouts.msi_teardown)(vector);
> > +
>
> just
> msi_ops.teardown(vector);
>
ok.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
