Re: [PATCH 2/4] atl1: Header files for Attansic L1 driver
> diff --git a/drivers/net/atl1/atl1_hw.h b/drivers/net/atl1/atl1_hw.h > new file mode 100644 > index 000..0450b77 > --- /dev/null > +++ b/drivers/net/atl1/atl1_hw.h [...] > +/* MII definition */ > +/* PHY Common Register */ > +#define MII_BMCR 0x00 > +#define MII_BMSR 0x01 > +#define MII_PHYSID1 0x02 > +#define MII_PHYSID2 0x03 > +#define MII_ADVERTISE0x04 > +#define MII_LPA 0x05 > +#define MII_EXPANSION0x06 [snip] It duplicates a lot of #define already available in include/linux/mii.h. -- Ueimor - 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/
Re: [PATCH 2/4] atl1: Header files for Attansic L1 driver
diff --git a/drivers/net/atl1/atl1_hw.h b/drivers/net/atl1/atl1_hw.h new file mode 100644 index 000..0450b77 --- /dev/null +++ b/drivers/net/atl1/atl1_hw.h [...] +/* MII definition */ +/* PHY Common Register */ +#define MII_BMCR 0x00 +#define MII_BMSR 0x01 +#define MII_PHYSID1 0x02 +#define MII_PHYSID2 0x03 +#define MII_ADVERTISE0x04 +#define MII_LPA 0x05 +#define MII_EXPANSION0x06 [snip] It duplicates a lot of #define already available in include/linux/mii.h. -- Ueimor - 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/
Re: [PATCH 2/4] atl1: Header files for Attansic L1 driver
Francois Romieu wrote: Jay Cliburn <[EMAIL PROTECTED]> : [...] I welcome any comments on the rationality of this approach. An URL for the current version of the patch would be welcome too :o) Sorry. Forgot to do that. The current version may be found here: ftp://hogchain.net/pub/linux/m2v/attansic/kernel_driver/atl1-2.0.4/atl1-2.0.4-linux-2.6.20.rc5.patch.bz2 Jay - 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/
Re: [PATCH 2/4] atl1: Header files for Attansic L1 driver
Jay Cliburn <[EMAIL PROTECTED]> : [...] > I welcome any comments on the rationality of this approach. An URL for the current version of the patch would be welcome too :o) -- Ueimor - 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/
Re: [PATCH 2/4] atl1: Header files for Attansic L1 driver
Christoph Hellwig wrote:
On Wed, Jan 10, 2007 at 06:41:37PM -0600, Jay Cliburn wrote:
+struct csum_param {
+ unsigned buf_len:14;
+ unsigned dma_int:1;
+ unsigned pkt_int:1;
+ u16 valan_tag;
+ unsigned eop:1;
+ /* command */
+ unsigned coalese:1;
+ unsigned ins_vlag:1;
+ unsigned custom_chksum:1;
+ unsigned segment:1;
+ unsigned ip_chksum:1;
+ unsigned tcp_chksum:1;
+ unsigned udp_chksum:1;
+ /* packet state */
+ unsigned vlan_tagged:1;
+ unsigned eth_type:1;
+ unsigned iphl:4;
+ unsigned:2;
+ unsigned payload_offset:8;
+ unsigned xsum_offset:8;
+} _ATL1_ATTRIB_PACK_;
Bitfields should not be used for hardware datastructures ever.
Please convert this to explicit masking and shifting.
+/* formerly ATL1_WRITE_REG */
+static inline void atl1_write32(const struct atl1_hw *hw, int reg, u32 val)
+{
+writel(val, hw->hw_addr + reg);
+}
+
+/* formerly ATL1_READ_REG */
+static inline u32 atl1_read32(const struct atl1_hw *hw, int reg)
+{
+return readl(hw->hw_addr + reg);
+}
Just kill all these wrappers. Also you probably want to convert to
pci_iomap + ioread*/iowrite*.
Christoph et al.,
I've incorporated all your comments except the two shown above. I
killed the indicated atl1_write*/atl1_read* wrappers, but I'm not yet
familiar enough with pci_iomap/iowrite*/ioread* to make that particular
conversion, and I'm having trouble getting the bitfield struct converted
to shift/mask semantics (No matter how hard I try, I keep breaking the
transmit side of the adapter).
I'd like to plead for relief on these two items and submit a new version
of the driver containing all your other comments. I need help from a
more experienced netdev hacker, and in my mind, the best way to do that
is to get the driver in the kernel so more people can use it and
contribute changes and make improvements.
I welcome any comments on the rationality of this approach.
Jay
-
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/
Re: [PATCH 2/4] atl1: Header files for Attansic L1 driver
Christoph Hellwig wrote:
On Wed, Jan 10, 2007 at 06:41:37PM -0600, Jay Cliburn wrote:
+struct csum_param {
+ unsigned buf_len:14;
+ unsigned dma_int:1;
+ unsigned pkt_int:1;
+ u16 valan_tag;
+ unsigned eop:1;
+ /* command */
+ unsigned coalese:1;
+ unsigned ins_vlag:1;
+ unsigned custom_chksum:1;
+ unsigned segment:1;
+ unsigned ip_chksum:1;
+ unsigned tcp_chksum:1;
+ unsigned udp_chksum:1;
+ /* packet state */
+ unsigned vlan_tagged:1;
+ unsigned eth_type:1;
+ unsigned iphl:4;
+ unsigned:2;
+ unsigned payload_offset:8;
+ unsigned xsum_offset:8;
+} _ATL1_ATTRIB_PACK_;
Bitfields should not be used for hardware datastructures ever.
Please convert this to explicit masking and shifting.
+/* formerly ATL1_WRITE_REG */
+static inline void atl1_write32(const struct atl1_hw *hw, int reg, u32 val)
+{
+writel(val, hw-hw_addr + reg);
+}
+
+/* formerly ATL1_READ_REG */
+static inline u32 atl1_read32(const struct atl1_hw *hw, int reg)
+{
+return readl(hw-hw_addr + reg);
+}
Just kill all these wrappers. Also you probably want to convert to
pci_iomap + ioread*/iowrite*.
Christoph et al.,
I've incorporated all your comments except the two shown above. I
killed the indicated atl1_write*/atl1_read* wrappers, but I'm not yet
familiar enough with pci_iomap/iowrite*/ioread* to make that particular
conversion, and I'm having trouble getting the bitfield struct converted
to shift/mask semantics (No matter how hard I try, I keep breaking the
transmit side of the adapter).
I'd like to plead for relief on these two items and submit a new version
of the driver containing all your other comments. I need help from a
more experienced netdev hacker, and in my mind, the best way to do that
is to get the driver in the kernel so more people can use it and
contribute changes and make improvements.
I welcome any comments on the rationality of this approach.
Jay
-
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/
Re: [PATCH 2/4] atl1: Header files for Attansic L1 driver
Jay Cliburn [EMAIL PROTECTED] : [...] I welcome any comments on the rationality of this approach. An URL for the current version of the patch would be welcome too :o) -- Ueimor - 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/
Re: [PATCH 2/4] atl1: Header files for Attansic L1 driver
Francois Romieu wrote: Jay Cliburn [EMAIL PROTECTED] : [...] I welcome any comments on the rationality of this approach. An URL for the current version of the patch would be welcome too :o) Sorry. Forgot to do that. The current version may be found here: ftp://hogchain.net/pub/linux/m2v/attansic/kernel_driver/atl1-2.0.4/atl1-2.0.4-linux-2.6.20.rc5.patch.bz2 Jay - 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/
Re: [PATCH 2/4] atl1: Header files for Attansic L1 driver
On Thu, Jan 11, 2007 at 10:53:30AM -0600, Jay Cliburn wrote: > On Thu, Jan 11, 2007 at 09:27:04AM +, Christoph Hellwig wrote: > > On Wed, Jan 10, 2007 at 06:41:37PM -0600, Jay Cliburn wrote: > > > +/** > > > + * atl1.h - atl1 main header > > > > Please remove these kind of comments, they get out of date far too soon > > and don't really help anything. (Also everywhere else in the driver) > > Is your concern here with the filename portion of the comment only, or > with the entire comment including the copyright and other material? Only the line I quoted. Copyright statements and similar things are obviously fine :) - 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/
Re: [PATCH 2/4] atl1: Header files for Attansic L1 driver
On Thu, 11 Jan 2007 10:53:30 -0600 Jay Cliburn <[EMAIL PROTECTED]> wrote: > On Thu, Jan 11, 2007 at 09:27:04AM +, Christoph Hellwig wrote: > > On Wed, Jan 10, 2007 at 06:41:37PM -0600, Jay Cliburn wrote: > > > +/** > > > + * atl1.h - atl1 main header > > > > Please remove these kind of comments, they get out of date far too soon > > and don't really help anything. (Also everywhere else in the driver) > > Is your concern here with the filename portion of the comment only, or > with the entire comment including the copyright and other material? > > Jay Comments should not insult the reader by stating the obvious. - 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/
Re: [PATCH 2/4] atl1: Header files for Attansic L1 driver
On Thu, Jan 11, 2007 at 09:27:04AM +, Christoph Hellwig wrote: > On Wed, Jan 10, 2007 at 06:41:37PM -0600, Jay Cliburn wrote: > > +/** > > + * atl1.h - atl1 main header > > Please remove these kind of comments, they get out of date far too soon > and don't really help anything. (Also everywhere else in the driver) Is your concern here with the filename portion of the comment only, or with the entire comment including the copyright and other material? Jay - 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/
Re: [PATCH 2/4] atl1: Header files for Attansic L1 driver
On Wed, Jan 10, 2007 at 06:41:37PM -0600, Jay Cliburn wrote:
> +/**
> + * atl1.h - atl1 main header
Please remove these kind of comments, they get out of date far too soon
and don't really help anything. (Also everywhere else in the driver)
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
Please always include
>
> +#include
Please only includ headers where you use them - that's mostly the .c
files unless you have lots of inlines or complex structures in the headers.
> +#ifdef NETIF_F_TSO
> +#include
> +#endif
> +
> +#ifdef SIOCGMIIPHY
> +#include
> +#endif
> +
> +#ifdef SIOCETHTOOL
> +#include
> +#endif
Please remove all these ifdefs.
> +#define BAR_00
This looks etirely superflous.
> +#define usec_delay(x)udelay(x)
> +#ifndef msec_delay
> +#define msec_delay(x)do { if(in_interrupt()) { \
> + /* Don't mdelay in interrupt context!*/ \
> + BUG(); \
> + } else { \
> + msleep(x); \
> + }} while(0)
> +/**
> + * Some workarounds require millisecond delays and are run during interrupt
> + * context. Most notably, when establishing link, the phy may need tweaking
> + * but cannot process phy register reads/writes faster than millisecond
> + * intervals...and we establish link due to a "link status change" interrupt.
> + **/
> +#define msec_delay_irq(x) mdelay(x)
> +#endif
Please kill all these wrappers.
> +} _ATL1_ATTRIB_PACK_;
All your structs seems to be properly packed from a first sight. Please
doble-check this and get rid of the attribute packed.
> +struct csum_param {
> + unsigned buf_len:14;
> + unsigned dma_int:1;
> + unsigned pkt_int:1;
> + u16 valan_tag;
> + unsigned eop:1;
> + /* command */
> + unsigned coalese:1;
> + unsigned ins_vlag:1;
> + unsigned custom_chksum:1;
> + unsigned segment:1;
> + unsigned ip_chksum:1;
> + unsigned tcp_chksum:1;
> + unsigned udp_chksum:1;
> + /* packet state */
> + unsigned vlan_tagged:1;
> + unsigned eth_type:1;
> + unsigned iphl:4;
> + unsigned:2;
> + unsigned payload_offset:8;
> + unsigned xsum_offset:8;
> +} _ATL1_ATTRIB_PACK_;
Bitfields should not be used for hardware datastructures ever.
Please convert this to explicit masking and shifting.
> +/* Structure containing variables used by the shared code */
> +struct atl1_hw {
> + u8 __iomem *hw_addr;
> + void *back;
This is definitly a kernel data structure. Shouldn't it be in atl1.h
instead of _hw.h? Also does back really need to be a void pointer or
can it be something typed?
> + u16 dev_rev;
> + u16 device_id;
> + u16 vendor_id;
> + u16 subsystem_id;
> + u16 subsystem_vendor_id;
> + u8 revision_id;
Please just use the values from the pci_dev insead of duplicating them.
> +/* formerly ATL1_WRITE_REG */
> +static inline void atl1_write32(const struct atl1_hw *hw, int reg, u32 val)
> +{
> +writel(val, hw->hw_addr + reg);
> +}
> +
> +/* formerly ATL1_READ_REG */
> +static inline u32 atl1_read32(const struct atl1_hw *hw, int reg)
> +{
> +return readl(hw->hw_addr + reg);
> +}
Just kill all these wrappers. Also you probably want to convert to
pci_iomap + ioread*/iowrite*.
-
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/
Re: [PATCH 2/4] atl1: Header files for Attansic L1 driver
On Wed, Jan 10, 2007 at 06:41:37PM -0600, Jay Cliburn wrote:
+/**
+ * atl1.h - atl1 main header
Please remove these kind of comments, they get out of date far too soon
and don't really help anything. (Also everywhere else in the driver)
+#include linux/tcp.h
+#include linux/skbuff.h
+#include linux/netdevice.h
+#include linux/pci.h
+#include linux/spinlock_types.h
+#include linux/workqueue.h
+#include linux/timer.h
+#include linux/delay.h
+#include linux/if_vlan.h
+
+#include asm/types.h
Please always include linux/types.h
+#include asm/atomic.h
Please only includ headers where you use them - that's mostly the .c
files unless you have lots of inlines or complex structures in the headers.
+#ifdef NETIF_F_TSO
+#include net/checksum.h
+#endif
+
+#ifdef SIOCGMIIPHY
+#include linux/mii.h
+#endif
+
+#ifdef SIOCETHTOOL
+#include linux/ethtool.h
+#endif
Please remove all these ifdefs.
+#define BAR_00
This looks etirely superflous.
+#define usec_delay(x)udelay(x)
+#ifndef msec_delay
+#define msec_delay(x)do { if(in_interrupt()) { \
+ /* Don't mdelay in interrupt context!*/ \
+ BUG(); \
+ } else { \
+ msleep(x); \
+ }} while(0)
+/**
+ * Some workarounds require millisecond delays and are run during interrupt
+ * context. Most notably, when establishing link, the phy may need tweaking
+ * but cannot process phy register reads/writes faster than millisecond
+ * intervals...and we establish link due to a link status change interrupt.
+ **/
+#define msec_delay_irq(x) mdelay(x)
+#endif
Please kill all these wrappers.
+} _ATL1_ATTRIB_PACK_;
All your structs seems to be properly packed from a first sight. Please
doble-check this and get rid of the attribute packed.
+struct csum_param {
+ unsigned buf_len:14;
+ unsigned dma_int:1;
+ unsigned pkt_int:1;
+ u16 valan_tag;
+ unsigned eop:1;
+ /* command */
+ unsigned coalese:1;
+ unsigned ins_vlag:1;
+ unsigned custom_chksum:1;
+ unsigned segment:1;
+ unsigned ip_chksum:1;
+ unsigned tcp_chksum:1;
+ unsigned udp_chksum:1;
+ /* packet state */
+ unsigned vlan_tagged:1;
+ unsigned eth_type:1;
+ unsigned iphl:4;
+ unsigned:2;
+ unsigned payload_offset:8;
+ unsigned xsum_offset:8;
+} _ATL1_ATTRIB_PACK_;
Bitfields should not be used for hardware datastructures ever.
Please convert this to explicit masking and shifting.
+/* Structure containing variables used by the shared code */
+struct atl1_hw {
+ u8 __iomem *hw_addr;
+ void *back;
This is definitly a kernel data structure. Shouldn't it be in atl1.h
instead of _hw.h? Also does back really need to be a void pointer or
can it be something typed?
+ u16 dev_rev;
+ u16 device_id;
+ u16 vendor_id;
+ u16 subsystem_id;
+ u16 subsystem_vendor_id;
+ u8 revision_id;
Please just use the values from the pci_dev insead of duplicating them.
+/* formerly ATL1_WRITE_REG */
+static inline void atl1_write32(const struct atl1_hw *hw, int reg, u32 val)
+{
+writel(val, hw-hw_addr + reg);
+}
+
+/* formerly ATL1_READ_REG */
+static inline u32 atl1_read32(const struct atl1_hw *hw, int reg)
+{
+return readl(hw-hw_addr + reg);
+}
Just kill all these wrappers. Also you probably want to convert to
pci_iomap + ioread*/iowrite*.
-
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/
Re: [PATCH 2/4] atl1: Header files for Attansic L1 driver
On Thu, Jan 11, 2007 at 09:27:04AM +, Christoph Hellwig wrote: On Wed, Jan 10, 2007 at 06:41:37PM -0600, Jay Cliburn wrote: +/** + * atl1.h - atl1 main header Please remove these kind of comments, they get out of date far too soon and don't really help anything. (Also everywhere else in the driver) Is your concern here with the filename portion of the comment only, or with the entire comment including the copyright and other material? Jay - 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/
Re: [PATCH 2/4] atl1: Header files for Attansic L1 driver
On Thu, 11 Jan 2007 10:53:30 -0600 Jay Cliburn [EMAIL PROTECTED] wrote: On Thu, Jan 11, 2007 at 09:27:04AM +, Christoph Hellwig wrote: On Wed, Jan 10, 2007 at 06:41:37PM -0600, Jay Cliburn wrote: +/** + * atl1.h - atl1 main header Please remove these kind of comments, they get out of date far too soon and don't really help anything. (Also everywhere else in the driver) Is your concern here with the filename portion of the comment only, or with the entire comment including the copyright and other material? Jay Comments should not insult the reader by stating the obvious. - 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/
Re: [PATCH 2/4] atl1: Header files for Attansic L1 driver
On Thu, Jan 11, 2007 at 10:53:30AM -0600, Jay Cliburn wrote: On Thu, Jan 11, 2007 at 09:27:04AM +, Christoph Hellwig wrote: On Wed, Jan 10, 2007 at 06:41:37PM -0600, Jay Cliburn wrote: +/** + * atl1.h - atl1 main header Please remove these kind of comments, they get out of date far too soon and don't really help anything. (Also everywhere else in the driver) Is your concern here with the filename portion of the comment only, or with the entire comment including the copyright and other material? Only the line I quoted. Copyright statements and similar things are obviously fine :) - 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/
Re: [PATCH 2/4] atl1: Header files for Attansic L1 driver
On Nov 19 2006 14:30, Jay Cliburn wrote: >+ >+#define LBYTESWAP( a ) ( ( ( (a) & 0x00ff00ff ) << 8 ) | ( ( (a) & >0xff00ff00 ) >> 8 ) ) >+#define LONGSWAP( a ) ( ( LBYTESWAP( a ) << 16 ) | ( LBYTESWAP( a ) >>> 16 ) ) >+#define SHORTSWAP( a ) ( ( (a) << 8 ) | ( (a) >> 8 ) ) Please use swab16/swab32 for these. >+#define AT_DESC_UNUSED(R) \ >+ R)->next_to_clean > (R)->next_to_use) ? 0 : (R)->count) + \ >+ (R)->next_to_clean - (R)->next_to_use - 1) >+ >+#define AT_DESC_USED(R) \ >+ (((R)->next_to_clean > (R)->next_to_use) ? \ >+ ((R)->count+(R)->next_to_use-(R)->next_to_clean+1) : \ >+ ((R)->next_to_use-(R)->next_to_clean+1)) These look like they are on the edge to be written as a static-inline function. What do others think? >+#define AT_WRITE_REG(a, reg, value) ( \ >+ writel((value), ((a)->hw_addr + reg))) >+ >+#define AT_READ_REG(a, reg) ( \ >+ readl((a)->hw_addr + reg )) >+ >+#define AT_WRITE_REGB(a, reg, value) (\ >+ writeb((value), ((a)->hw_addr + reg))) >+ >+#define AT_READ_REGB(a, reg) (\ >+ readb((a)->hw_addr + reg)) >+ >+#define AT_WRITE_REGW(a, reg, value) (\ >+ writew((value), ((a)->hw_addr + reg))) >+ >+#define AT_READ_REGW(a, reg) (\ >+ readw((a)->hw_addr + reg)) >+ >+#define AT_WRITE_REG_ARRAY(a, reg, offset, value) ( \ >+ writel((value), (((a)->hw_addr + reg) + ((offset) << 2 >+ >+#define AT_READ_REG_ARRAY(a, reg, offset) ( \ >+ readl(((a)->hw_addr + reg) + ((offset) << 2))) Possibly similarly. -`J' -- - 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/
Re: [PATCH 2/4] atl1: Header files for Attansic L1 driver
On Nov 19 2006 14:30, Jay Cliburn wrote: + +#define LBYTESWAP( a ) ( ( ( (a) 0x00ff00ff ) 8 ) | ( ( (a) 0xff00ff00 ) 8 ) ) +#define LONGSWAP( a ) ( ( LBYTESWAP( a ) 16 ) | ( LBYTESWAP( a ) 16 ) ) +#define SHORTSWAP( a ) ( ( (a) 8 ) | ( (a) 8 ) ) Please use swab16/swab32 for these. +#define AT_DESC_UNUSED(R) \ + R)-next_to_clean (R)-next_to_use) ? 0 : (R)-count) + \ + (R)-next_to_clean - (R)-next_to_use - 1) + +#define AT_DESC_USED(R) \ + (((R)-next_to_clean (R)-next_to_use) ? \ + ((R)-count+(R)-next_to_use-(R)-next_to_clean+1) : \ + ((R)-next_to_use-(R)-next_to_clean+1)) These look like they are on the edge to be written as a static-inline function. What do others think? +#define AT_WRITE_REG(a, reg, value) ( \ + writel((value), ((a)-hw_addr + reg))) + +#define AT_READ_REG(a, reg) ( \ + readl((a)-hw_addr + reg )) + +#define AT_WRITE_REGB(a, reg, value) (\ + writeb((value), ((a)-hw_addr + reg))) + +#define AT_READ_REGB(a, reg) (\ + readb((a)-hw_addr + reg)) + +#define AT_WRITE_REGW(a, reg, value) (\ + writew((value), ((a)-hw_addr + reg))) + +#define AT_READ_REGW(a, reg) (\ + readw((a)-hw_addr + reg)) + +#define AT_WRITE_REG_ARRAY(a, reg, offset, value) ( \ + writel((value), (((a)-hw_addr + reg) + ((offset) 2 + +#define AT_READ_REG_ARRAY(a, reg, offset) ( \ + readl(((a)-hw_addr + reg) + ((offset) 2))) Possibly similarly. -`J' -- - 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/
