Dear Prafulla Wadaskar,
In message 1238798370-9245-3-git-send-email-prafulla@marvell.com you wrote:
From: prafulla_wadaskar prafulla@marvell.com
Contributors: Yotam Admon yotam@marvell.com Michael Blostein <michaelbl@marvell.com
Signed-off-by: prafulla_wadaskar prafulla@marvell.com Reviewed by: Ronen Shitrit rshitrit@marvell.com
...
diff --git a/cpu/arm926ejs/kirkwood/Makefile b/cpu/arm926ejs/kirkwood/Makefile index 41ac8d7..3e20898 100644 --- a/cpu/arm926ejs/kirkwood/Makefile +++ b/cpu/arm926ejs/kirkwood/Makefile @@ -30,6 +30,7 @@ COBJS-y = timer.o COBJS-y += serial.o COBJS-y += kwcore.o COBJS-y += dram.o +COBJS-y += egiga.o
Please sort such lists.
...
+int eth_smi_reg_read(u32 eth_port_num, u32 phy_adr, u32 reg_ofs, u16 * data) +{
- u32 smi_reg;
- volatile u32 timeout;
- /* check parameters */
- if ((phy_adr << ETH_PHY_SMI_DEV_ADDR_OFFS) & ~ETH_PHY_SMI_DEV_ADDR_MASK) {
Line too long. (Hint: consider using shorter names! :-)
...
- /* fill the phy address and regiser offset and read opcode */
- smi_reg =
(phy_adr << ETH_PHY_SMI_DEV_ADDR_OFFS) | (reg_ofs <<KW_ETH_SMI_REG_ADDR_OFFS)| ETH_PHY_SMI_OPCODE_READ;
Please clean up indentation here.
...
+static int egiga_free_rx_rings(struct eth_device *dev) +{
- u32 queue;
- ETH_PORT_INFO *ethernet_private;
- struct egiga_priv *port_private;
- u32 port_num;
- volatile ETH_RX_DESC *p_rx_curr_desc;
- ethernet_private = (ETH_PORT_INFO *) dev->priv;
- port_private = (struct egiga_priv *)ethernet_private->port_private;
- port_num = port_private->port_num;
- /* Stop RX Queues */
- KW_REG_WRITE(KW_ETH_RECEIVE_QUEUE_COMMAND_REG(port_num), 0x0000ff00);
- /* Free RX rings */
- debug_print("Clearing previously allocated RX queues... ");
- for (queue = 0; queue < KW_RX_QUEUE_NUM; queue++) {
/* Free preallocated skb's on RX rings */for (p_rx_curr_desc =ethernet_private->p_rx_desc_area_base[queue];(((u32)p_rx_curr_desc <((u32)ethernet_private->p_rx_desc_area_base[queue] +ethernet_private->rx_desc_area_size[queue])));p_rx_curr_desc =(ETH_RX_DESC *) ((u32)p_rx_curr_desc +RX_DESC_ALIGNED_SIZE)) {if (p_rx_curr_desc->return_info != 0) {p_rx_curr_desc->return_info = 0;debug_print("freed");}
... and here. Such code is basicly unreadable.
...
+static ETH_FUNC_RET_STATUS eth_port_send(ETH_PORT_INFO * p_eth_port_ctrl,
ETH_QUEUE tx_queue,PKT_INFO * p_pkt_info)+{
- volatile ETH_TX_DESC *p_tx_desc_first;
- volatile ETH_TX_DESC *p_tx_desc_curr;
- volatile ETH_TX_DESC *p_tx_next_desc_curr;
- volatile ETH_TX_DESC *p_tx_desc_used;
- u32 command_status;
+#ifdef CONFIG_TX_PKT_DISPLAY
- {
u16 pcnt = p_pkt_info->byte_cnt;u8 *prndt = (char *)p_pkt_info->buf_ptr;printf("cnt=%d ", pcnt);
Blank line after declarations, please (check gloabally, please).
Also, pelase try to acoid such nested blocks with additional declarations.
while (pcnt) {printf("%02x,", prndt[0]);prndt++;pcnt--;}printf(" pckend \n");- }
+#endif
...
diff --git a/cpu/arm926ejs/kirkwood/egiga.h b/cpu/arm926ejs/kirkwood/egiga.h new file mode 100644 index 0000000..83dc4b3 --- /dev/null +++ b/cpu/arm926ejs/kirkwood/egiga.h @@ -0,0 +1,707 @@
...
+#ifndef TRUE +#define TRUE 1 +#endif +#ifndef FALSE +#define FALSE 0 +#endif
Omit?
...
+struct net_device_stats {
- u32 rx_packets; /* total packets received */
- u32 tx_packets; /* total packets transmitted */
- u32 rx_bytes; /* total bytes received */
- u32 tx_bytes; /* total bytes transmitted */
- u32 rx_errors; /* bad packets received */
- u32 tx_errors; /* packet transmit problems */
- u32 rx_dropped; /* no space in linux buffers */
- u32 tx_dropped; /* no space available in linux */
- u32 multicast; /* multicast packets received */
- u32 collisions;
- /* detailed rx_errors: */
- u32 rx_length_errors;
- u32 rx_over_errors; /* receiver ring buff overflow */
- u32 rx_crc_errors; /* recved pkt with crc error */
- u32 rx_frame_errors; /* recv'd frame alignment error */
- u32 rx_fifo_errors; /* recv'r fifo overrun */
- u32 rx_missed_errors; /* receiver missed packet */
- /* detailed tx_errors */
- u32 tx_aborted_errors;
- u32 tx_carrier_errors;
- u32 tx_fifo_errors;
- u32 tx_heartbeat_errors;
- u32 tx_window_errors;
- /* for cslip etc */
- u32 rx_compressed;
- u32 tx_compressed;
+};
+/* Private data structure used for ethernet device */ +struct egiga_priv {
- u32 port_num;
- struct net_device_stats *stats;
- /* to buffer area aligned */
- char *p_eth_tx_buffer[KW_TX_QUEUE_SIZE + 1];
- char *p_eth_rx_buffer[KW_RX_QUEUE_SIZE + 1];
- /* Size of Tx Ring per queue */
- u32 tx_ring_size[MAX_TX_QUEUE_NUM];
- /* Size of Rx Ring per queue */
- u32 rx_ring_size[MAX_RX_QUEUE_NUM];
- /* Magic Number for Ethernet running */
- u32 eth_running;
+};
Please use TABs for vertical alignment to make the code readable.
...
+/* Gap define */ +#define ETH_BAR_GAP 0x8 +#define ETH_SIZE_REG_GAP 0x8 +#define ETH_HIGH_ADDR_REMAP_REG_GAP 0x4 +#define ETH_PORT_ACCESS_CTRL_GAP 0x4 +/* MIB Counters register definitions */ +#define ETH_MIB_GOOD_OCTETS_RECEIVED_LOW 0x0 +#define ETH_MIB_GOOD_OCTETS_RECEIVED_HIGH 0x4 +#define ETH_MIB_BAD_OCTETS_RECEIVED 0x8 +#define ETH_MIB_INTERNAL_MAC_TRANSMIT_ERR 0xc +#define ETH_MIB_GOOD_FRAMES_RECEIVED 0x10 +#define ETH_MIB_BAD_FRAMES_RECEIVED 0x14 +#define ETH_MIB_BROADCAST_FRAMES_RECEIVED 0x18 +#define ETH_MIB_MULTICAST_FRAMES_RECEIVED 0x1c
... and so on:
Please use TABs for vertical alignment to make the code better readable.
This applies to the rest of file, too.
diff --git a/cpu/arm926ejs/kirkwood/egiga_regs.h b/cpu/arm926ejs/kirkwood/egiga_regs.h new file mode 100644 index 0000000..a24fa04 --- /dev/null +++ b/cpu/arm926ejs/kirkwood/egiga_regs.h
...
+/*
- *Ethernet Controller Registers
- */
+#define KW_ETH_PHY_ADDR_REG(port) (0x72000 + (port<<14)) +#define KW_ETH_SMI_REG(port) (0x72004 + (port<<14)) +#define KW_ETH_UNIT_DEFAULT_ADDR_REG(port) (0x72008 + (port<<14)) +#define KW_ETH_UNIT_DEFAULTID_REG(port) (0x7200c + (port<<14)) +#define KW_ETH_UNIT_INTERRUPT_CAUSE_REG(port) (0x72080 + (port<<14)) +#define KW_ETH_UNIT_INTERRUPT_MASK_REG(port) (0x72084 + (port<<14))
...
Use C structures to describe the device layout, and you can get rid of this ugly code.
Best regards,
Wolfgang Denk