Re: [PATCH v7 2/3] net: Add Keystone NetCP ethernet driver
From: Joe Perches
Date: Thu Dec 11 2014 - 13:35:14 EST
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
(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);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/