[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20121018024523.GA2867@netboy.at.omicron.at>
Date: Thu, 18 Oct 2012 04:45:23 +0200
From: Richard Cochran <richardcochran@...il.com>
To: Mugunthan V N <mugunthanvnm@...com>
Cc: netdev@...r.kernel.org, davem@...emloft.net
Subject: Re: [PATCH 1/6] drivers: net: ethernet: cpsw: add support for CPSW
register offset changes in different IP version
On Wed, Oct 17, 2012 at 04:15:13AM +0530, Mugunthan V N wrote:
>
> +#define CPSW_VERSION_1 0x19010a
> +#define CPSW_VERSION_2 0x19010c
Are you sure about these codes? What about 0x19010b?
I could not find them documented anywhere.
> +#define cpsw_slave_reg(priv, slave, reg) \
> + ((slave)->regs + (priv)->slave_reg_ofs[(reg)])
> +
> #define CPSW_MAJOR_VERSION(reg) (reg >> 8 & 0x7)
> #define CPSW_MINOR_VERSION(reg) (reg & 0xff)
> #define CPSW_RTL_VERSION(reg) ((reg >> 11) & 0x1f)
> @@ -117,6 +122,48 @@ do { \
> disable_irq_nosync(priv->irqs_table[i]); \
> } while (0);
>
> +enum CPSW_SLAVE_REG_OFS {
> + MAX_BLKS,
> + BLK_CNT,
> + FLOW_THRESH,
> + PORT_VLAN,
> + TX_PRI_MAP,
> + TS_CTL,
> + TS_SEQ_LTYPE,
> + TS_VLAN,
> + SA_LO,
> + SA_HI,
> + PORT_CONTROL,
> + TS_SEQ_MTYPE,
> + TS_CONTROL,
> +};
> +
> +static const u32 slave_reg_map_ip_v1[] = {
> + [MAX_BLKS] = 0x00,
> + [BLK_CNT] = 0x04,
> + [FLOW_THRESH] = 0x08,
> + [PORT_VLAN] = 0x0c,
> + [TX_PRI_MAP] = 0x10,
> + [TS_CTL] = 0x14,
> + [TS_SEQ_LTYPE] = 0x18,
> + [TS_VLAN] = 0x1c,
> + [SA_LO] = 0x20,
> + [SA_HI] = 0x24,
> +};
> +
> +static const u32 slave_reg_map_ip_v2[] = {
> + [PORT_CONTROL] = 0x00,
> + [TS_CONTROL] = 0x04,
You don't make use of this register in your driver, so what is the
point?
> + [MAX_BLKS] = 0x08,
> + [BLK_CNT] = 0x0c,
> + [FLOW_THRESH] = 0x10,
> + [PORT_VLAN] = 0x14,
> + [TX_PRI_MAP] = 0x18,
> + [TS_SEQ_MTYPE] = 0x1c,
> + [SA_LO] = 0x20,
> + [SA_HI] = 0x24,
> +};
> +
This is wasting memory with unused static stables. There is a better
way to handle this issue.
> static int debug_level;
> module_param(debug_level, int, 0);
> MODULE_PARM_DESC(debug_level, "cpsw debug level (NETIF_MSG bits)");
> @@ -146,19 +193,13 @@ struct cpsw_regs {
> u32 soft_reset;
> u32 stat_port_en;
> u32 ptype;
> -};
> -
> -struct cpsw_slave_regs {
> - u32 max_blks;
> - u32 blk_cnt;
> - u32 flow_thresh;
> - u32 port_vlan;
> - u32 tx_pri_map;
> - u32 ts_ctl;
> - u32 ts_seq_ltype;
> - u32 ts_vlan;
> - u32 sa_lo;
> - u32 sa_hi;
> + u32 soft_idle;
> + u32 thru_rate;
> + u32 gap_thresh;
> + u32 tx_start_wds;
> + u32 flow_control;
> + u32 vlan_ltype;
> + u32 ts_ltype;
> };
>
> struct cpsw_host_regs {
> @@ -185,7 +226,7 @@ struct cpsw_sliver_regs {
> };
>
> struct cpsw_slave {
> - struct cpsw_slave_regs __iomem *regs;
> + void __iomem *regs;
> struct cpsw_sliver_regs __iomem *sliver;
> int slave_num;
> u32 mac_control;
> @@ -215,6 +256,8 @@ struct cpsw_priv {
> struct cpdma_ctlr *dma;
> struct cpdma_chan *txch, *rxch;
> struct cpsw_ale *ale;
> + u32 cpsw_version;
> + u32 *slave_reg_ofs;
> /* snapshot of IRQ numbers */
> u32 irqs_table[4];
> u32 num_irqs;
> @@ -359,8 +402,8 @@ static inline void soft_reset(const char *module, void __iomem *reg)
> static void cpsw_set_slave_mac(struct cpsw_slave *slave,
> struct cpsw_priv *priv)
> {
> - __raw_writel(mac_hi(priv->mac_addr), &slave->regs->sa_hi);
> - __raw_writel(mac_lo(priv->mac_addr), &slave->regs->sa_lo);
> + writel(mac_hi(priv->mac_addr), cpsw_slave_reg(priv, slave, SA_HI));
> + writel(mac_lo(priv->mac_addr), cpsw_slave_reg(priv, slave, SA_LO));
> }
>
> static void _cpsw_adjust_link(struct cpsw_slave *slave,
> @@ -445,8 +488,8 @@ static void cpsw_slave_open(struct cpsw_slave *slave, struct cpsw_priv *priv)
> soft_reset(name, &slave->sliver->soft_reset);
>
> /* setup priority mapping */
> - __raw_writel(RX_PRIORITY_MAPPING, &slave->sliver->rx_pri_map);
> - __raw_writel(TX_PRIORITY_MAPPING, &slave->regs->tx_pri_map);
> + writel(RX_PRIORITY_MAPPING, &slave->sliver->rx_pri_map);
> + writel(TX_PRIORITY_MAPPING, cpsw_slave_reg(priv, slave, TX_PRI_MAP));
>
> /* setup max packet size, and mac address */
> __raw_writel(priv->rx_packet_max, &slave->sliver->rx_maxlen);
> @@ -505,7 +548,12 @@ static int cpsw_ndo_open(struct net_device *ndev)
>
> pm_runtime_get_sync(&priv->pdev->dev);
>
> - reg = __raw_readl(&priv->regs->id_ver);
> + reg = readl(&priv->regs->id_ver);
> + priv->cpsw_version = reg;
> + if (reg == CPSW_VERSION_1)
> + priv->slave_reg_ofs = (u32 *)slave_reg_map_ip_v1;
> + else
> + priv->slave_reg_ofs = (u32 *)slave_reg_map_ip_v2;
You didn't provide a way to even use this code, like a dts for a
non-am335x board with the older version.
I think it would be better to start off supporting one version and
have that fully working, and then add the older version, but *really*
add it so that it is actually working.
Thanks,
Richard
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists