Re: [PATCH 3/4] atl1: Main C file for Attansic L1 driver

Jan Engelhardt
2006-11-19T21:52:56Z

>+char at_driver_name[] = "atl1";
>+static const char at_driver_string[] = "Attansic(R) L1 Ethernet Network Driver";
>+const char at_driver_version[] = DRV_VERSION;
>+static const char at_copyright[] =
>+ "Copyright(c) 2005-2006 Attansic Corporation.";
>+

>+extern s32 at_read_mac_addr(struct at_hw *hw);
>+extern s32 at_init_hw(struct at_hw *hw);
>+extern s32 at_get_speed_and_duplex(struct at_hw *hw, u16 * speed, u16 * duplex);
>+extern s32 at_set_speed_and_duplex(struct at_hw *hw, u16 speed, u16 duplex);
>+extern u32 at_auto_get_fc(struct at_adapter *adapter, u16 duplex);
>+extern u32 at_hash_mc_addr(struct at_hw *hw, u8 * mc_addr);
>+extern void at_hash_set(struct at_hw *hw, u32 hash_value);
>+extern s32 at_read_phy_reg(struct at_hw *hw, u16 reg_addr, u16 * phy_data);
>+extern s32 at_write_phy_reg(struct at_hw *hw, u32 reg_addr, u16 phy_data);
>+extern s32 at_validate_mdi_setting(struct at_hw *hw);
>+extern void set_mac_addr(struct at_hw *hw);
>+extern int get_permanent_address(struct at_hw *hw);
>+extern s32 at_phy_enter_power_saving(struct at_hw *hw);
>+extern s32 at_reset_hw(struct at_hw *hw);
>+extern void at_check_options(struct at_adapter *adapter);
>+void at_set_ethtool_ops(struct net_device *netdev);

Put externs in a .h file.

>+static u16 at_alloc_rx_buffers(struct at_adapter *adapter)
>+{
...
>+ u16 rfd_next_to_use, next_next;
>+ struct rx_free_desc *rfd_desc;
>+
>+ next_next = rfd_next_to_use = (u16) atomic_read(&rfd_ring->next_to_use);

Cast not needed.

>+ buffer_info->length = (u16) adapter->rx_buffer_len;

>+ rrd_next_to_clean = (u16) atomic_read(&rrd_ring->next_to_clean);

Same (check the others too)

>+ if (++rfd_ring->next_to_clean == rfd_ring->count) {
>+ rfd_ring->next_to_clean = 0;
>+ }
>+ }
>+
>+ buffer_info = &rfd_ring->buffer_info[rrd->buf_indx];
>+ if (++rfd_ring->next_to_clean == rfd_ring->count) {
>+ rfd_ring->next_to_clean = 0;
>+ }

Here is a stylistic one: {} is not needed for single-statememnt ifs.

>+static irqreturn_t at_intr(int irq, void *data)
>+{
>+ struct at_adapter *adapter = ((struct net_device *)data)->priv;

(Ahem)

>+ if (0 == (status = adapter->cmb.cmb->int_stats))

Someone else is probably going to complain about this one...

I have not looked through all of it, so there are sure some more places.

-`J'
--