From patchwork Thu Dec 11 18:34:57 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Joe Perches X-Patchwork-Id: 5477161 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 25382BEEA8 for ; Thu, 11 Dec 2014 18:37:51 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id C39372013A for ; Thu, 11 Dec 2014 18:37:49 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 76C382010B for ; Thu, 11 Dec 2014 18:37:48 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1Xz8aK-0000wQ-I7; Thu, 11 Dec 2014 18:35:32 +0000 Received: from smtprelay0150.hostedemail.com ([216.40.44.150] helo=smtprelay.hostedemail.com) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1Xz8aF-0000As-4q for linux-arm-kernel@lists.infradead.org; Thu, 11 Dec 2014 18:35:29 +0000 Received: from filter.hostedemail.com (unknown [216.40.38.60]) by smtprelay02.hostedemail.com (Postfix) with ESMTP id 101F912BCFE; Thu, 11 Dec 2014 18:35:01 +0000 (UTC) X-Session-Marker: 6A6F6540706572636865732E636F6D X-Spam-Summary: 2, 0, 0, , d41d8cd98f00b204, joe@perches.com, :::::::::::::::::, RULES_HIT:1:41:69:355:379:541:599:800:960:973:988:989:1260:1277:1311:1313:1314:1345:1359:1373:1437:1515:1516:1518:1593:1594:1605:1730:1747:1777:1792:2194:2198:2199:2200:2393:2553:2559:2562:2638:2828:2898:2902:2911:3138:3139:3140:3141:3142:3622:3865:3866:3867:3868:3870:3871:3872:3873:3874:4250:4321:4425:4605:5007:6117:6119:6261:6755:7576:7875:7903:8603:9010:9040:9592:10004:10848:10967:11026:11232:11473:11657:11658:11914:12043:12050:12219:12296:12438:12517:12519:12555:12683:12740:13019:13138:13231:13972:21080, 0, RBL:none, CacheIP:none, Bayesian:0.5, 0.5, 0.5, Netcheck:none, DomainCache:0, MSF:not bulk, SPF:fn, MSBL:0, DNSBL:none, Custom_rules:0:0:0 X-HE-Tag: tooth28_82d0409e98d34 X-Filterd-Recvd-Size: 13895 Received: from joe-X200MA.home (pool-71-103-235-196.lsanca.fios.verizon.net [71.103.235.196]) (Authenticated sender: joe@perches.com) by omf11.hostedemail.com (Postfix) with ESMTPA; Thu, 11 Dec 2014 18:34:58 +0000 (UTC) Message-ID: <1418322897.18092.37.camel@perches.com> Subject: Re: [PATCH v7 2/3] net: Add Keystone NetCP ethernet driver From: Joe Perches To: Murali Karicheri Date: Thu, 11 Dec 2014 10:34:57 -0800 In-Reply-To: <5489D196.4070703@ti.com> References: <5488BB96.9070609@ti.com> <20141210.204110.618599360537141819.davem@davemloft.net> <5489A6CD.20909@ti.com> <20141211.120104.312460507509497826.davem@davemloft.net> <5489D196.4070703@ti.com> X-Mailer: Evolution 3.12.7-0ubuntu1 Mime-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20141211_103527_346542_8DC3B7DD X-CRM114-Status: GOOD ( 25.68 ) X-Spam-Score: -0.0 (/) Cc: devicetree@vger.kernel.org, WingMan Kwok , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, robh+dt@kernel.org, grant.likely@linaro.org, David Miller , linux-arm-kernel@lists.infradead.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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 > > 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);