* Re: 2.6.12-rc3-mm1 (a-new-10gb-ethernet-driver-by-chelsio-communications.patch)
[not found] <20050429231653.32d2f091.akpm@osdl.org>
[not found] ` <42735E42.9050901@ens-lyon.org>
2005-04-30 11:52 ` [-mm patch] SIS900 must select MII Adrian Bunk
@ 2005-04-30 23:34 ` Alexey Dobriyan
2 siblings, 0 replies; 5+ messages in thread
From: Alexey Dobriyan @ 2005-04-30 23:34 UTC (permalink / raw)
To: Christoph Lameter; +Cc: linux-kernel, netdev, Andrew Morton
[-- Attachment #1: Type: text/plain, Size: 1 bytes --]
[-- Attachment #2: chelsio.txt --]
[-- Type: text/plain, Size: 7308 bytes --]
> --- /dev/null
> +++ 25-akpm/drivers/net/chelsio/common.h
> +#define DIMOF(x) (sizeof(x)/sizeof(x[0]))
ARRAY_SIZE in include/linux/kernel.h
> --- /dev/null
>+++ 25-akpm/drivers/net/chelsio/cpl5_cmd.h
> +struct cpl_tx_pkt {
> + __u16 vlan;
__be16.
> +struct cpl_tx_pkt_lso {
> + __u32 len;
__be32
> + __u16 eth_type_mss;
__be16
> +struct cpl_rx_pkt {
> + __u16 vlan;
_be16 really. You do ntohs(p->vlan).
If any other fields of these structs are in network order I'd mark them as
such.
> --- /dev/null
> +++ 25-akpm/drivers/net/chelsio/cxgb2.c
> +#if BITS_PER_LONG == 64 && !defined(CONFIG_X86_64)
> +# define FMT64 "l"
> +#else
> +# define FMT64 "ll"
> +#endif
FMT64 is unused.
> +#define PCI_DMA_64BIT ~0ULL
> +#define PCI_DMA_32BIT 0xffffffffULL
In include/linux/dma-mapping.h DMA_64BIT_MASK and DMA_32BIT_MASK, respectively.
> +static const char pci_speed[][4] = {
> + "33", "66", "100", "133"
> +};
Unused.
> +static void get_stats(struct net_device *dev, struct ethtool_stats *stats,
> + u64 *data)
> +{
> + *data++ = s->TxOctetsOK;
> + *data++ = s->TxOctetsBad;
> + *data++ = s->TxUnicastFramesOK;
> + *data++ = s->TxMulticastFramesOK;
> + *data++ = s->TxBroadcastFramesOK;
> + *data++ = s->TxPauseFrames;
> + *data++ = s->TxFramesWithDeferredXmissions;
> + *data++ = s->TxLateCollisions;
> + *data++ = s->TxTotalCollisions;
> + *data++ = s->TxFramesAbortedDueToXSCollisions;
> + *data++ = s->TxUnderrun;
> + *data++ = s->TxLengthErrors;
> + *data++ = s->TxInternalMACXmitError;
> + *data++ = s->TxFramesWithExcessiveDeferral;
> + *data++ = s->TxFCSErrors;
> +
> + *data++ = s->RxOctetsOK;
> + *data++ = s->RxOctetsBad;
> + *data++ = s->RxUnicastFramesOK;
> + *data++ = s->RxMulticastFramesOK;
> + *data++ = s->RxBroadcastFramesOK;
> + *data++ = s->RxPauseFrames;
> + *data++ = s->RxFCSErrors;
> + *data++ = s->RxAlignErrors;
> + *data++ = s->RxSymbolErrors;
> + *data++ = s->RxDataErrors;
> + *data++ = s->RxSequenceErrors;
> + *data++ = s->RxRuntErrors;
> + *data++ = s->RxJabberErrors;
> + *data++ = s->RxInternalMACRcvError;
> + *data++ = s->RxInRangeLengthErrors;
> + *data++ = s->RxOutOfRangeLengthField;
> + *data++ = s->RxFrameTooLongErrors;
It seems you want one big memcpy.
> +static int ethtool_ioctl(struct net_device *dev, void *useraddr)
void __user *addr. To make sparse happy.
> +{
> + if (copy_from_user(&cmd, useraddr, sizeof(cmd)))
> +static int __devinit init_one(struct pci_dev *pdev,
> + const struct pci_device_id *ent)
> +{
> + if (!pci_set_dma_mask(pdev, PCI_DMA_64BIT)) {
DMA_64BIT_MASK
> + if (pci_set_consistent_dma_mask(pdev, PCI_DMA_64BIT)) {
DMA_64BIT_MASK
> + } else if ((err = pci_set_dma_mask(pdev, PCI_DMA_32BIT)) != 0) {
DMA_32BIT_MASK
> --- /dev/null
> +++ 25-akpm/drivers/net/chelsio/cxgb2.h
> +struct adapter {
> + u8 *regs;
void __iomem *regs; You do ->regs = ioremap(...);
> --- /dev/null
> +++ 25-akpm/drivers/net/chelsio/espi.c
> +/* 1. Deassert rx_reset_core. */
> +/* 2. Program TRICN_CNFG registers. */
> +/* 3. Deassert rx_reset_link */
> +static int tricn_init(adapter_t *adapter)
> +{
> + /* 1 */
> + timeout=1000;
> + /* 2 */
> + if (sme) {
> + /* 3 */
> + t1_write_reg_4(adapter, A_ESPI_RX_RESET, 0x3);
So move comments where they should be. ;-)
> +/* T2 Init part -- */
> +/* 1. Set T_ESPI_MISCCTRL_ADDR */
> +/* 2. Init ESPI registers. */
> +/* 3. Init TriCN Hard Macro */
> +int t1_espi_init(struct peespi *espi, int mac_type, int nports)
> +{
> --- /dev/null
> +++ 25-akpm/drivers/net/chelsio/mv88x201x.c
> + if (do_enable & LINK_ENABLE_BIT) {
> + led |= LINK_ENABLE_BIT;
> + mdio_write(cphy, 0x1, 0x7, led);
> + } else {
> + led &= ~LINK_ENABLE_BIT;
> + mdio_write(cphy, 0x1, 0x7, led);
> + }
Common mdio_write line.
> --- /dev/null
> +++ 25-akpm/drivers/net/chelsio/pm3393.c
> +static int pm3393_macaddress_set(struct cmac *cmac, u8 ma[6])
> +{
> + u32 val, lo, mid, hi
> + * MAC addr: 00:07:43:00:13:09
> + *
> + * ma[5] = 0x09
> + * ma[4] = 0x13
> + * ma[3] = 0x00
> + * ma[2] = 0x43
> + * ma[1] = 0x07
> + * ma[0] = 0x00
> + *
> + * The PM3393 requires byte swapping and reverse order entry
> + * when programming MAC addresses:
> + *
> + * low_bits[15:0] = ma[1]:ma[0]
> + * mid_bits[31:16] = ma[3]:ma[2]
> + * high_bits[47:32] = ma[5]:ma[4]
> + lo = ((u32) ma[1] << 8) | (u32) ma[0];
> + mid = ((u32) ma[3] << 8) | (u32) ma[2];
> + hi = ((u32) ma[5] << 8) | (u32) ma[4];
This is
lo = (u32) le16_to_cpu((__le16 *) &ma[0]);
mid = (u32) le16_to_cpu((__le16 *) &ma[2]);
hi = (u32) le16_to_cpu((__le16 *) &ma[4]);
Right?
> + /* The following steps are required to properly reset
> + * the PM3393. This information is provided in the
> + * PM3393 datasheet (Issue 2: November 2002)
> + * section 13.1 -- Device Reset.
> + *
> + * The PM3393 has three types of components that are
> + * individually reset:
> + *
> + * DRESETB - Digital circuitry
> + * PL4_ARESETB - PL4 analog circuitry
> + * XAUI_ARESETB - XAUI bus analog circuitry
> + *
> + * Steps to reset PM3393 using RSTB pin:
> + *
> + * 1. Assert RSTB pin low ( write 0 )
> + * 2. Wait at least 1ms to initiate a complete initialization of device.
> + * 3. Wait until all external clocks and REFSEL are stable.
> + * 4. Wait minimum of 1ms. (after external clocks and REFEL are stable)
> + * 5. De-assert RSTB ( write 1 )
> + * 6. Wait until internal timers to expires after ~14ms.
> + * - Allows analog clock synthesizer(PL4CSU) to stabilize to
> + * selected reference frequency before allowing the digital
> + * portion of the device to operate.
> + * 7. Wait at least 200us for XAUI interface to stabilize.
> + * 8. Verify the PM3393 came out of reset successfully.
> + * Set successful reset flag if everything worked else try again
> + * a few more times.
Same comment about comments.
--- /dev/null
+++ 25-akpm/drivers/net/chelsio/regs.h
> +/* Do not edit this file */
OK.
--- /dev/null
+++ 25-akpm/drivers/net/chelsio/subr.c
> +static struct board_info t1_board[] = {
> +
> +{ CHBT_BOARD_N110, 1/*ports#*/,
> + SUPPORTED_10000baseT_Full | SUPPORTED_FIBRE /*caps*/, CHBT_TERM_T1,
> + CHBT_MAC_PM3393, CHBT_PHY_88X2010,
> + 125000000/*clk-core*/, 0/*clk-mc3*/, 0/*clk-mc4*/,
> + 1/*espi-ports*/, 0/*clk-cspi*/, 44/*clk-elmer0*/, 0/*mdien*/,
> + 0/*mdiinv*/, 1/*mdc*/, 0/*phybaseaddr*/, &t1_pm3393_ops,
> + &t1_mv88x201x_ops, &mi1_mdio_ext_ops,
> + "Chelsio N110 1x10GBaseX NIC" },
> +
> +{ CHBT_BOARD_N210, 1/*ports#*/,
> + SUPPORTED_10000baseT_Full | SUPPORTED_FIBRE /*caps*/, CHBT_TERM_T2,
> + CHBT_MAC_PM3393, CHBT_PHY_88X2010,
> + 125000000/*clk-core*/, 0/*clk-mc3*/, 0/*clk-mc4*/,
> + 1/*espi-ports*/, 0/*clk-cspi*/, 44/*clk-elmer0*/, 0/*mdien*/,
> + 0/*mdiinv*/, 1/*mdc*/, 0/*phybaseaddr*/, &t1_pm3393_ops,
> + &t1_mv88x201x_ops, &mi1_mdio_ext_ops,
> + "Chelsio N210 1x10GBaseX NIC" },
> +
> +};
That's what C99 initializers are for:
static struct board_info t1_board[] = {
{
.board = CHBT_BOARD_N110,
.port_number = 1,
.caps = SUPPORTED_10000baseT_Full | SUPPORTED_FIBRE,
.chip_term = CHBT_TERM_T1,
...
}, {
...
},
};
> +const struct board_info *t1_get_board_info(unsigned int board_id)
> +{
> + return board_id < DIMOF(t1_board) ? &t1_board[board_id] : NULL;
ARRAY_SIZE
^ permalink raw reply [flat|nested] 5+ messages in thread