lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 15 Dec 2015 11:55:57 +0000 From: Liberman Igal <Igal.Liberman@...escale.com> To: Andy Fleming <afleming@...il.com> CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Scott Wood <scottwood@...escale.com>, Madalin-Cristian Bucur <madalin.bucur@...escale.com>, "pebolle@...cali.nl" <pebolle@...cali.nl>, Joakim Tjernlund <joakim.tjernlund@...nsmode.se>, "ppc@...dchasers.com" <ppc@...dchasers.com>, "stephen@...workplumber.org" <stephen@...workplumber.org>, David Miller <davem@...emloft.net> Subject: RE: [v9, 6/6] fsl/fman: Add FMan MAC driver Hello Andy, Thank you for your feedback. Some inline answers. Regards, Igal Liberman > -----Original Message----- > From: Andy Fleming [mailto:afleming@...il.com] > Sent: Tuesday, December 08, 2015 10:18 PM > To: Liberman Igal-B31950 <Igal.Liberman@...escale.com> > Cc: netdev@...r.kernel.org; linuxppc-dev@...ts.ozlabs.org; linux- > kernel@...r.kernel.org; Wood Scott-B07421 <scottwood@...escale.com>; > Bucur Madalin-Cristian-B32716 <madalin.bucur@...escale.com>; > pebolle@...cali.nl; Joakim Tjernlund <joakim.tjernlund@...nsmode.se>; > ppc@...dchasers.com; stephen@...workplumber.org; David Miller > <davem@...emloft.net> > Subject: Re: [v9, 6/6] fsl/fman: Add FMan MAC driver > > On Thu, Dec 3, 2015 at 1:19 AM, <igal.liberman@...escale.com> wrote: > > From: Igal Liberman <igal.liberman@...escale.com> > > > > This patch adds the Ethernet MAC driver supporting the three different > > types of MACs: dTSEC, tGEC and mEMAC. > > > > Signed-off-by: Igal Liberman <igal.liberman@...escale.com> > > [...] > > > + > > +MODULE_LICENSE("Dual BSD/GPL"); > > + > > +MODULE_AUTHOR("Emil Medve <Emilian.Medve@...escale.com>"); > > 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. > > [...] > I removed MODULE_AUTHOR as per Scott's request. We'll change the way we note contributors. > > +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 * > OK, removed. > [...] > > > +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? > Agree, changed the driver to use priv->phy_if instead of strstr in both places. In both > > > + 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) > According to https://www.kernel.org/doc/Documentation/CodingStyle: The preferred style for long (multi-line) comments is: /* * This is the preferred style for multi-line * comments in the Linux kernel source code. * Please use it consistently. * * Description: A column of asterisks on the left side, * with beginning and ending almost-blank lines. */ For files in net/ and drivers/net/ the preferred style for long (multi-line) comments is a little different. /* The preferred comment style for files in net/ and drivers/net * looks like this. * * It is nearly the same as the generally preferred comment style, * but there is no initial almost-blank line. */ > Andy
Powered by blists - more mailing lists