Re: [PATCH v7 2/3] net: Add Keystone NetCP ethernet driver

Murali Karicheri <m-karicheri2@xxxxxx> · Thu, 11 Dec 2014 15:21:27 -0500

On 12/11/2014 01:34 PM, Joe Perches wrote:
On Thu, 2014-12-11 at 12:17 -0500, Murali Karicheri wrote:
On 12/11/2014 12:01 PM, David Miller wrote:
From: Murali Karicheri<m-karicheri2@xxxxxx>
Date: Thu, 11 Dec 2014 09:14:37 -0500

BTW, could you provide any suggestions that would help us merge this
series to upstream? This has been sitting on this list for a while
now.

You simply have to continue going through the review and revision
process until people no longer find problems with your changes.

This could take several more weeks, you simply must be patient.

Ok. Thanks. That is encouraging to hear!

Perhaps these trivial things might be considered
Joe, David,

I will address the comments and repost a new version.

Murali

(uncompiled)
---
  drivers/net/ethernet/ti/netcp_core.c     |  2 +-
  drivers/net/ethernet/ti/netcp_ethss.c    | 83 +++++++++++++++++---------------
  drivers/net/ethernet/ti/netcp_xgbepcsr.c | 48 +++++++++---------
  3 files changed, 71 insertions(+), 62 deletions(-)

diff --git a/drivers/net/ethernet/ti/netcp_core.c b/drivers/net/ethernet/ti/netcp_core.c
index 60ad299..8f38fe8 100644
--- a/drivers/net/ethernet/ti/netcp_core.c
+++ b/drivers/net/ethernet/ti/netcp_core.c
@@ -1094,7 +1094,7 @@ static void netcp_tx_notify(void *arg)
  	napi_schedule(&netcp->tx_napi);
  }

-static struct knav_dma_desc*
+static struct knav_dma_desc *
  netcp_tx_map_skb(struct sk_buff *skb, struct netcp_intf *netcp)
  {
  	struct knav_dma_desc *desc, *ndesc, *pdesc;
diff --git a/drivers/net/ethernet/ti/netcp_ethss.c b/drivers/net/ethernet/ti/netcp_ethss.c
index 036b886..3757957 100644
--- a/drivers/net/ethernet/ti/netcp_ethss.c
+++ b/drivers/net/ethernet/ti/netcp_ethss.c
@@ -486,21 +486,29 @@ struct netcp_ethtool_stat {
  	int offset;
  };

-#define GBE_STATSA_INFO(field)		"GBE_A:"#field, GBE_STATSA_MODULE,\
-				FIELD_SIZEOF(struct gbe_hw_stats, field), \
-				offsetof(struct gbe_hw_stats, field)
-
-#define GBE_STATSB_INFO(field)		"GBE_B:"#field, GBE_STATSB_MODULE,\
-				FIELD_SIZEOF(struct gbe_hw_stats, field), \
-				offsetof(struct gbe_hw_stats, field)
-
-#define GBE_STATSC_INFO(field)		"GBE_C:"#field, GBE_STATSC_MODULE,\
-				FIELD_SIZEOF(struct gbe_hw_stats, field), \
-				offsetof(struct gbe_hw_stats, field)
-
-#define GBE_STATSD_INFO(field)		"GBE_D:"#field, GBE_STATSD_MODULE,\
-				FIELD_SIZEOF(struct gbe_hw_stats, field), \
-				offsetof(struct gbe_hw_stats, field)
+#define GBE_STATSA_INFO(field)						\
+	.desc = "GBE_A:"#field,						\
+	.type = GBE_STATSA_MODULE,					\
+	.size = FIELD_SIZEOF(struct gbe_hw_stats, field),		\
+	.offset = offsetof(struct gbe_hw_stats, field)
+
+#define GBE_STATSB_INFO(field)						\
+	.desc = "GBE_B:"#field,						\
+	.type = GBE_STATSB_MODULE,					\
+	.size = FIELD_SIZEOF(struct gbe_hw_stats, field),		\
+	.offset = offsetof(struct gbe_hw_stats, field)
+
+#define GBE_STATSC_INFO(field)						\
+	.desc = "GBE_C:"#field,						\
+	.type = GBE_STATSC_MODULE,					\
+	.size = FIELD_SIZEOF(struct gbe_hw_stats, field),		\
+	.offset = offsetof(struct gbe_hw_stats, field)
+
+#define GBE_STATSD_INFO(field)						\
+	.desc = "GBE_D:"#field,						\
+	.type = GBE_STATSD_MODULE,					\
+	.size = FIELD_SIZEOF(struct gbe_hw_stats, field),		\
+	.offset = offsetof(struct gbe_hw_stats, field)

  static const struct netcp_ethtool_stat gbe13_et_stats[] = {
  	/* GBE module A */
@@ -645,17 +653,23 @@ static const struct netcp_ethtool_stat gbe13_et_stats[] = {
  	{GBE_STATSD_INFO(rx_dma_overruns)},
  };

-#define XGBE_STATS0_INFO(field)	"GBE_0:"#field, XGBE_STATS0_MODULE, \
-				FIELD_SIZEOF(struct xgbe_hw_stats, field), \
-				offsetof(struct xgbe_hw_stats, field)
+#define XGBE_STATS0_INFO(field)						\
+	.desc = "GBE_0:"#field,						\
+	.type = GBE_STATS0_MODULE,					\
+	.size = FIELD_SIZEOF(struct gbe_hw_stats, field),		\
+	.offset = offsetof(struct gbe_hw_stats, field)

-#define XGBE_STATS1_INFO(field)	"GBE_1:"#field, XGBE_STATS1_MODULE, \
-				FIELD_SIZEOF(struct xgbe_hw_stats, field), \
-				offsetof(struct xgbe_hw_stats, field)
+#define XGBE_STATS1_INFO(field)						\
+	.desc = "GBE_1:"#field,						\
+	.type = GBE_STATS1_MODULE,					\
+	.size = FIELD_SIZEOF(struct gbe_hw_stats, field),		\
+	.offset = offsetof(struct gbe_hw_stats, field)

-#define XGBE_STATS2_INFO(field)	"GBE_2:"#field, XGBE_STATS2_MODULE, \
-				FIELD_SIZEOF(struct xgbe_hw_stats, field), \
-				offsetof(struct xgbe_hw_stats, field)
+#define XGBE_STATS2_INFO(field)
+	.desc = "GBE_2:"#field,						\
+	.type = GBE_STATS2_MODULE,					\
+	.size = FIELD_SIZEOF(struct gbe_hw_stats, field),		\
+	.offset = offsetof(struct gbe_hw_stats, field)

  static const struct netcp_ethtool_stat xgbe10_et_stats[] = {
  	/* GBE module 0 */
@@ -883,11 +897,11 @@ static void gbe_update_stats_ver14(struct gbe_priv *gbe_dev, uint64_t *data)
  			case GBE_STATSA_MODULE:
  			case GBE_STATSC_MODULE:
  				base = gbe_statsa;
-			break;
+				break;
  			case GBE_STATSB_MODULE:
  			case GBE_STATSD_MODULE:
  				base  = gbe_statsb;
-			break;
+				break;
  			}

  			p = base + gbe_dev->et_stats[j].offset;
@@ -1639,11 +1653,8 @@ static void init_secondary_ports(struct gbe_priv *gbe_dev,

  	for_each_child_of_node(node, port) {
  		slave = devm_kzalloc(dev, sizeof(*slave), GFP_KERNEL);
-		if (!slave) {
-			dev_err(dev, "memomry alloc failed for secondary port(%s), skipping...\n",
-				port->name);
+		if (!slave)
  			continue;
-		}

  		if (init_slave(gbe_dev, slave, port)) {
  			dev_err(dev, "Failed to initialize secondary port(%s), skipping...\n",
@@ -1763,10 +1774,8 @@ static int set_xgbe_ethss10_priv(struct gbe_priv *gbe_dev,
  					  XGBE10_NUM_STAT_ENTRIES *
  					  XGBE10_NUM_SLAVES * sizeof(u64),
  					  GFP_KERNEL);
-	if (!gbe_dev->hw_stats) {
-		dev_err(gbe_dev->dev, "hw_stats memory allocation failed\n");
+	if (!gbe_dev->hw_stats)
  		return -ENOMEM;
-	}

  	gbe_dev->ss_version = XGBE_SS_VERSION_10;
  	gbe_dev->sgmii_port_regs = gbe_dev->ss_regs +
@@ -1836,10 +1845,8 @@ static int set_gbe_ethss14_priv(struct gbe_priv *gbe_dev,
  					  GBE13_NUM_HW_STAT_ENTRIES *
  					  GBE13_NUM_SLAVES * sizeof(u64),
  					  GFP_KERNEL);
-	if (!gbe_dev->hw_stats) {
-		dev_err(gbe_dev->dev, "hw_stats memory allocation failed\n");
+	if (!gbe_dev->hw_stats)
  		return -ENOMEM;
-	}

  	gbe_dev->ss_version = GBE_SS_VERSION_14;
  	gbe_dev->sgmii_port_regs = regs + GBE13_SGMII_MODULE_OFFSET;
@@ -1995,10 +2002,10 @@ static int gbe_probe(struct netcp_device *netcp_device, struct device *dev,
  		dev_err(gbe_dev->dev, "error initializing ale engine\n");
  		ret = -ENODEV;
  		goto quit;
-	} else {
-		dev_dbg(gbe_dev->dev, "Created a gbe ale engine\n");
  	}

+	dev_dbg(gbe_dev->dev, "Created a gbe ale engine\n");
+
  	/* initialize host port */
  	gbe_init_host_port(gbe_dev);

diff --git a/drivers/net/ethernet/ti/netcp_xgbepcsr.c b/drivers/net/ethernet/ti/netcp_xgbepcsr.c
index 33571ac..d93a6a4 100644
--- a/drivers/net/ethernet/ti/netcp_xgbepcsr.c
+++ b/drivers/net/ethernet/ti/netcp_xgbepcsr.c
@@ -14,6 +14,9 @@
   * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
   * GNU General Public License for more details.
   */
+
+#define pr_fmt(fmt) "XGBE: " fmt
+
  #include "netcp.h"

  /* XGBE registers */
@@ -43,7 +46,7 @@ struct serdes_cfg {
  	u32 mask;
  };

-static struct serdes_cfg cfg_phyb_1p25g_156p25mhz_cmu0[] = {
+static const struct serdes_cfg cfg_phyb_1p25g_156p25mhz_cmu0[] = {
  	{0x0000, 0x00800002, 0x00ff00ff},
  	{0x0014, 0x00003838, 0x0000ffff},
  	{0x0060, 0x1c44e438, 0xffffffff},
@@ -54,7 +57,7 @@ static struct serdes_cfg cfg_phyb_1p25g_156p25mhz_cmu0[] = {
  	{0x0000, 0x00000003, 0x000000ff},
  };

-static struct serdes_cfg cfg_phyb_10p3125g_156p25mhz_cmu1[] = {
+static const struct serdes_cfg cfg_phyb_10p3125g_156p25mhz_cmu1[] = {
  	{0x0c00, 0x00030002, 0x00ff00ff},
  	{0x0c14, 0x00005252, 0x0000ffff},
  	{0x0c28, 0x80000000, 0xff000000},
@@ -76,7 +79,7 @@ static struct serdes_cfg cfg_phyb_10p3125g_156p25mhz_cmu1[] = {
  	{0x0c00, 0x00000003, 0x000000ff},
  };

-static struct serdes_cfg cfg_phyb_10p3125g_16bit_lane[] = {
+static const struct serdes_cfg cfg_phyb_10p3125g_16bit_lane[] = {
  	{0x0204, 0x00000080, 0x000000ff},
  	{0x0208, 0x0000920d, 0x0000ffff},
  	{0x0204, 0xfc000000, 0xff000000},
@@ -106,7 +109,7 @@ static struct serdes_cfg cfg_phyb_10p3125g_16bit_lane[] = {
  	{0x03cc, 0x00000000, 0x000000ff},
  };

-static struct serdes_cfg cfg_phyb_10p3125g_comlane[] = {
+static const struct serdes_cfg cfg_phyb_10p3125g_comlane[] = {
  	{0x0a00, 0x00000800, 0x0000ff00},
  	{0x0a84, 0x00000000, 0x000000ff},
  	{0x0a8c, 0x00130000, 0x00ff0000},
@@ -124,7 +127,7 @@ static struct serdes_cfg cfg_phyb_10p3125g_comlane[] = {
  	{0x0ac0, 0x0000008b, 0x000000ff},
  };

-static struct serdes_cfg cfg_cm_c1_c2[] = {
+static const struct serdes_cfg cfg_cm_c1_c2[] = {
  	{0x0208, 0x00000000, 0x00000f00},
  	{0x0208, 0x00000000, 0x0000001f},
  	{0x0204, 0x00000000, 0x00040000},
@@ -185,8 +188,8 @@ static void netcp_xgbe_serdes_com_enable(void __iomem *serdes_regs)
  	}
  }

-static void netcp_xgbe_serdes_lane_enable(
-			void __iomem *serdes_regs, int lane)
+static void netcp_xgbe_serdes_lane_enable(void __iomem *serdes_regs,
+					  int lane)
  {
  	/* Set Lane Control Rate */
  	writel(0xe0e9e038, serdes_regs + 0x1fe0 + (4 * lane));
@@ -230,7 +233,7 @@ static int netcp_xgbe_wait_pll_locked(void __iomem *sw_regs)
  		cpu_relax();
  	} while (true);

-	pr_err("XGBE serdes not locked: time out.\n");
+	pr_err("serdes not locked: time out\n");
  	return ret;
  }

@@ -292,8 +295,7 @@ static void netcp_xgbe_serdes_reset_cdr(void __iomem *serdes_regs,
  	u32 tmp, dlpf, tbus;

  	/*Get the DLPF values */
-	tmp = netcp_xgbe_serdes_read_select_tbus(
-			serdes_regs, lane + 1, 5);
+	tmp = netcp_xgbe_serdes_read_select_tbus(serdes_regs, lane + 1, 5);

  	dlpf = tmp>>  2;

@@ -302,10 +304,10 @@ static void netcp_xgbe_serdes_reset_cdr(void __iomem *serdes_regs,
  		mdelay(1);
  		reg_rmw(sig_detect_reg, VAL_SH(0, 1), MASK_WID_SH(2, 1));
  	} else {
-		tbus = netcp_xgbe_serdes_read_select_tbus(serdes_regs, lane +
-							  1, 0xe);
+		tbus = netcp_xgbe_serdes_read_select_tbus(serdes_regs,
+							  lane + 1, 0xe);

-		pr_debug("XGBE: CDR centered, DLPF: %4d,%d,%d.\n",
+		pr_debug("CDR centered, DLPF: %4d,%d,%d\n",
  			 tmp>>  2, tmp&  3, (tbus>>  2)&  3);
  	}
  }
@@ -340,13 +342,13 @@ static int netcp_xgbe_check_link_status(void __iomem *serdes_regs,
  		case 0:
  			/* if good link lock the signal detect ON! */
  			if (!loss&&  blk_lock) {
-				pr_debug("XGBE PCSR Linked Lane: %d\n", i);
+				pr_debug("PCSR Linked Lane: %d\n", i);
  				reg_rmw(sig_detect_reg, VAL_SH(3, 1),
  					MASK_WID_SH(2, 1));
  				current_state[i] = 1;
  			} else if (!blk_lock) {
  				/* if no lock, then reset CDR */
-				pr_debug("XGBE PCSR Recover Lane: %d\n", i);
+				pr_debug("PCSR Recover Lane: %d\n", i);
  				netcp_xgbe_serdes_reset_cdr(serdes_regs,
  							    sig_detect_reg, i);
  			}
@@ -361,10 +363,10 @@ static int netcp_xgbe_check_link_status(void __iomem *serdes_regs,
  			break;

  		case 2:
-			if (blk_lock)
+			if (blk_lock) {
  				/* Nope just noise */
  				current_state[i] = 1;
-			else {
+			} else {
  				/* Lost the block lock, reset CDR if it is
  				 * not centered and go back to sync state
  				 */
@@ -375,7 +377,7 @@ static int netcp_xgbe_check_link_status(void __iomem *serdes_regs,
  			break;

  		default:
-			pr_err("XGBE: unknown current_state[%d] %d\n",
+			pr_err("unknown current_state[%d] %d\n",
  			       i, current_state[i]);
  			break;
  		}
@@ -417,19 +419,19 @@ static int netcp_xgbe_serdes_check_lane(void __iomem *serdes_regs,
  			break;

  		if (lane_down[0])
-			pr_debug("XGBE: detected link down on lane 0\n");
+			pr_debug("detected link down on lane 0\n");

  		if (lane_down[1])
-			pr_debug("XGBE: detected link down on lane 1\n");
+			pr_debug("detected link down on lane 1\n");

  		if (++retries>  1) {
-			pr_debug("XGBE: timeout waiting for serdes link up\n");
+			pr_debug("timeout waiting for serdes link up\n");
  			return -ETIMEDOUT;
  		}
  		mdelay(100);
  	} while (!link_up);

-	pr_debug("XGBE: PCSR link is up\n");
+	pr_debug("PCSR link is up\n");
  	return 0;
  }

@@ -494,7 +496,7 @@ int netcp_xgbe_serdes_init(void __iomem *serdes_regs, void __iomem *xgbe_regs)
  	/* read COMLANE bits 4:0 */
  	val = readl(serdes_regs + 0xa00);
  	if (val&  0x1f) {
-		pr_debug("XGBE: serdes already in operation - reset\n");
+		pr_debug("serdes already in operation - reset\n");
  		netcp_xgbe_reset_serdes(serdes_regs);
  	}
  	return netcp_xgbe_serdes_config(serdes_regs, xgbe_regs);





--
Murali Karicheri
Linux Kernel, Texas Instruments
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html







