Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752613AbbLHUST (ORCPT ); Tue, 8 Dec 2015 15:18:19 -0500 Received: from mail-io0-f179.google.com ([209.85.223.179]:35399 "EHLO mail-io0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752422AbbLHUSQ (ORCPT ); Tue, 8 Dec 2015 15:18:16 -0500 MIME-Version: 1.0 In-Reply-To: <1449127157-9400-7-git-send-email-igal.liberman@freescale.com> References: <1449127157-9400-1-git-send-email-igal.liberman@freescale.com> <1449127157-9400-7-git-send-email-igal.liberman@freescale.com> Date: Tue, 8 Dec 2015 14:18:16 -0600 Message-ID: Subject: Re: [v9, 6/6] fsl/fman: Add FMan MAC driver From: Andy Fleming To: igal.liberman@freescale.com Cc: "netdev@vger.kernel.org" , "linuxppc-dev@lists.ozlabs.org" , "linux-kernel@vger.kernel.org" , Scott Wood , madalin.bucur@freescale.com, pebolle@tiscali.nl, Joakim Tjernlund , ppc@mindchasers.com, stephen@networkplumber.org, David Miller Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3701 Lines: 110 On Thu, Dec 3, 2015 at 1:19 AM, wrote: > From: Igal Liberman > > This patch adds the Ethernet MAC driver supporting the three > different types of MACs: dTSEC, tGEC and mEMAC. > > Signed-off-by: Igal Liberman [...] > + > +MODULE_LICENSE("Dual BSD/GPL"); > + > +MODULE_AUTHOR("Emil Medve "); I imagine this email address doesn't exist anymore, or won't soon. This is also an issue in the ethernet driver (with my old address). I'm not sure what the right approach is, but we shouldn't be putting obsolete email addresses in the kernel. [...] > +static void mac_exception(void *_mac_dev, enum fman_mac_exceptions ex) > +{ > + struct mac_device *mac_dev; > + struct mac_priv_s *priv; > + > + mac_dev = (struct mac_device *)_mac_dev; Don't cast a void * [...] > +static int mac_probe(struct platform_device *_of_dev) > +{ > + int err, i, lenp, nph; > + struct device *dev; > + struct device_node *mac_node, *dev_node, *tbi_node; > + struct mac_device *mac_dev; > + struct platform_device *of_dev; > + struct resource res; > + struct mac_priv_s *priv; > + const u8 *mac_addr; > + const char *char_prop; [...] > + /* Get the PHY connection type */ > + char_prop = (const char *)of_get_property(mac_node, > + "phy-connection-type", NULL); > + if (!char_prop) { > + dev_warn(dev, > + "of_get_property(%s, phy-connection-type) failed. Defaulting to MII\n", > + mac_node->full_name); > + priv->phy_if = PHY_INTERFACE_MODE_MII; > + } else { > + priv->phy_if = str2phy(char_prop); > + } > + > + priv->speed = phy2speed[priv->phy_if]; > + priv->max_speed = priv->speed; > + mac_dev->if_support = DTSEC_SUPPORTED; > + /* We don't support half-duplex in SGMII mode */ > + if (strstr(char_prop, "sgmii")) This causes a crash if the device tree does not have a "phy-connection-type" for this node. Also, you already have parsed the property, and put the result in priv->phy_if. Why not use this to do all of these determinations? > + mac_dev->if_support &= ~(SUPPORTED_10baseT_Half | > + SUPPORTED_100baseT_Half); > + > + /* Gigabit support (no half-duplex) */ > + if (priv->max_speed == 1000) > + mac_dev->if_support |= SUPPORTED_1000baseT_Full; > + > + /* The 10G interface only supports one mode */ > + if (strstr(char_prop, "xgmii")) > + mac_dev->if_support = SUPPORTED_10000baseT_Full; Here, too. [...] > + err = mac_dev->init(mac_dev); > + if (err < 0) { > + dev_err(dev, "mac_dev->init() = %d\n", err); > + of_node_put(priv->phy_node); > + goto _return_dev_set_drvdata; > + } > + > + /* pause frame autonegotiation enabled */ > + mac_dev->autoneg_pause = true; > + > + /* by intializing the values to false, force FMD to enable PAUSE frames > + * on RX and TX > + */ Minor comment format issue (leave blank line at top of block comment) Andy -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/