On Thu, Feb 20, 2014 at 12:55:12PM -0500, Murali Karicheri wrote:
From: Vitaly Andrianov vitalya@ti.com
Ethernet driver configures the CPSW, SGMI and Phy and uses the the Navigator APIs. The driver supports 4 Ethernet ports and can work with only one port at a time.
First, can we just use things in a "dumb" mode and use the existing CPSW driver? Or are there use cases we need to worry about here with a bigger / more robust network driver?
Port configurations are defined in board.c.
Signed-off-by: Vitaly Andrianov vitalya@ti.com Signed-off-by: Murali Karicheri m-karicheri2@ti.com Signed-off-by: WingMan Kwok w-kwok2@ti.com
[snip]
+/*#define chipLmbd(x,y) _lmbd(x,y) */
Don't add unused code.
+/* Marvell 88E1111 PHY ID */ +#define PHY_MARVELL_88E1111 (0x01410cc0)
+/* EMAC MDIO Registers Structure */ +struct mdio_regs {
- dv_reg VERSION;
- dv_reg CONTROL;
Why caps?
+++ b/drivers/net/keystone_net.c
Generic problem, please use phylib.
+#define debug_emac(fmt, args...) if (emac_dbg) printf(fmt, ##args)
We have debug() already.
+#ifdef KEYSTONE2_EMAC_GIG_ENABLE +#define emac_gigabit_enable(x) keystone2_eth_gigabit_enable(x) +#else +#define emac_gigabit_enable(x) /* no gigabit to enable */ +#endif
Do we need to do this?
+static void chip_delay(u32 del) +{
- volatile unsigned int i;
- for (i = 0; i < (del / 8); i++)
;+}
Use one of the generic delays please.
+/* PHY functions for a generic PHY */ +static int __attribute__((unused)) gen_init_phy(int phy_addr)
Why are we adding tons of functions that we mark as unused?
+#if CONFIG_GET_LINK_STATUS_ATTEMPTS > 1
- int j;
- for (j = 0; (j < CONFIG_GET_LINK_STATUS_ATTEMPTS) && (link_state == 0);
j++) {+#endif
New config option, do we really need this as an option?
+void sgmii_serdes_setup_156p25mhz() +{
- unsigned int cnt;
- reg_rmw(0x0232a000, 0x00800000, 0xffff0000);
Please comment on what we're doing here, and why we aren't using a struct to describe whatever is at 0x0232a000 ?
+void sgmii_serdes_shutdown() +{
- reg_rmw(0x0232bfe0, 0, 3 << 29 | 3 << 13);
Same.
Thanks!