| 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
| ||
|
Message-ID: <45D763CC.4090507@garzik.org>
Date: Sat, 17 Feb 2007 15:21:32 -0500
From: Jeff Garzik <jeff@...zik.org>
To: Mark Brown <broonie@...ena.org.uk>
CC: Tim Hockin <thockin@...kin.org>, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [patch 1/2] natsemi: Add support for using MII port with no PHY
Mark Brown wrote:
> This patch provides code paths which allow the natsemi driver to use the
> external MII port on the chip but ignore any PHYs that may be attached to it.
> The link state will be left as it was when the driver started and can be
> configured via ethtool. Any PHYs that are present can be accessed via the MII
> ioctl()s.
>
> This is useful for systems where the device is connected without a PHY
> or where either information or actions outside the scope of the driver
> are required in order to use the PHYs.
>
> Signed-Off-By: Mark Brown <broonie@...ena.org.uk>
>
> ---
>
> Previous versions of this patch exposed the new functionality as a module
> option. This has been removed. Any hardware that needs this should be
> identifiable by a quirk since it unlikely to behave correctly with an
> unmodified driver.
I ACK this general concept. Please update for the minor issues and resend.
>
> Index: linux/drivers/net/natsemi.c
> ===================================================================
> --- linux.orig/drivers/net/natsemi.c 2007-02-12 17:44:33.000000000 +0000
> +++ linux/drivers/net/natsemi.c 2007-02-12 18:09:44.000000000 +0000
> @@ -568,6 +568,8 @@
> u32 intr_status;
> /* Do not touch the nic registers */
> int hands_off;
> + /* Don't pay attention to the reported link state. */
> + int ignore_phy;
> /* external phy that is used: only valid if dev->if_port != PORT_TP */
> int mii;
> int phy_addr_external;
> @@ -696,7 +698,10 @@
> struct netdev_private *np = netdev_priv(dev);
> u32 tmp;
>
> - netif_carrier_off(dev);
> + if (np->ignore_phy)
> + netif_carrier_on(dev);
> + else
> + netif_carrier_off(dev);
>
> /* get the initial settings from hardware */
> tmp = mdio_read(dev, MII_BMCR);
> @@ -806,8 +811,10 @@
> np->hands_off = 0;
> np->intr_status = 0;
> np->eeprom_size = natsemi_pci_info[chip_idx].eeprom_size;
> + np->ignore_phy = 0;
>
> /* Initial port:
> + * - If configured to ignore the PHY set up for external.
> * - If the nic was configured to use an external phy and if find_mii
> * finds a phy: use external port, first phy that replies.
> * - Otherwise: internal port.
> @@ -815,7 +822,7 @@
> * The address would be used to access a phy over the mii bus, but
> * the internal phy is accessed through mapped registers.
> */
> - if (readl(ioaddr + ChipConfig) & CfgExtPhy)
> + if (np->ignore_phy || readl(ioaddr + ChipConfig) & CfgExtPhy)
> dev->if_port = PORT_MII;
> else
> dev->if_port = PORT_TP;
> @@ -825,7 +832,9 @@
>
> if (dev->if_port != PORT_TP) {
> np->phy_addr_external = find_mii(dev);
> - if (np->phy_addr_external == PHY_ADDR_NONE) {
> + /* If we're ignoring the PHY it doesn't matter if we can't
> + * find one. */
> + if (!np->ignore_phy && np->phy_addr_external == PHY_ADDR_NONE) {
> dev->if_port = PORT_TP;
> np->phy_addr_external = PHY_ADDR_INTERNAL;
> }
> @@ -891,6 +900,8 @@
> printk("%02x, IRQ %d", dev->dev_addr[i], irq);
> if (dev->if_port == PORT_TP)
> printk(", port TP.\n");
> + else if (np->ignore_phy)
> + printk(", port MII, ignoring PHY\n");
> else
> printk(", port MII, phy ad %d.\n", np->phy_addr_external);
> }
> @@ -1571,42 +1582,45 @@
> {
> struct netdev_private *np = netdev_priv(dev);
> void __iomem * ioaddr = ns_ioaddr(dev);
> - int duplex;
> + int duplex = np->duplex;
> u16 bmsr;
>
> - /* The link status field is latched: it remains low after a temporary
> - * link failure until it's read. We need the current link status,
> - * thus read twice.
> - */
> - mdio_read(dev, MII_BMSR);
> - bmsr = mdio_read(dev, MII_BMSR);
> + /* If we are ignoring the PHY then don't try reading it. */
> + if (!np->ignore_phy) {
This change causes a lot of needless indentation changes. I would
prefer something like
if (np->ignore_phy)
return;
or
if (np->ignore_phy)
goto step_2;
> + /* The link status field is latched: it remains low
> + * after a temporary link failure until it's read. We
> + * need the current link status, thus read twice.
> + */
> + mdio_read(dev, MII_BMSR);
> + bmsr = mdio_read(dev, MII_BMSR);
>
> - if (!(bmsr & BMSR_LSTATUS)) {
> - if (netif_carrier_ok(dev)) {
> + if (!(bmsr & BMSR_LSTATUS)) {
> + if (netif_carrier_ok(dev)) {
> + if (netif_msg_link(np))
> + printk(KERN_NOTICE "%s: link down.\n",
> + dev->name);
> + netif_carrier_off(dev);
> + undo_cable_magic(dev);
> + }
> + return;
> + }
> + if (!netif_carrier_ok(dev)) {
> if (netif_msg_link(np))
> - printk(KERN_NOTICE "%s: link down.\n",
> - dev->name);
> - netif_carrier_off(dev);
> - undo_cable_magic(dev);
> + printk(KERN_NOTICE "%s: link up.\n", dev->name);
> + netif_carrier_on(dev);
> + do_cable_magic(dev);
> }
> - return;
> - }
> - if (!netif_carrier_ok(dev)) {
> - if (netif_msg_link(np))
> - printk(KERN_NOTICE "%s: link up.\n", dev->name);
> - netif_carrier_on(dev);
> - do_cable_magic(dev);
> - }
>
> - duplex = np->full_duplex;
> - if (!duplex) {
> - if (bmsr & BMSR_ANEGCOMPLETE) {
> - int tmp = mii_nway_result(
> - np->advertising & mdio_read(dev, MII_LPA));
> - if (tmp == LPA_100FULL || tmp == LPA_10FULL)
> + duplex = np->full_duplex;
> + if (!duplex) {
> + if (bmsr & BMSR_ANEGCOMPLETE) {
> + int tmp = mii_nway_result(
> + np->advertising & mdio_read(dev, MII_LPA));
> + if (tmp == LPA_100FULL || tmp == LPA_10FULL)
> + duplex = 1;
> + } else if (mdio_read(dev, MII_BMCR) & BMCR_FULLDPLX)
> duplex = 1;
> - } else if (mdio_read(dev, MII_BMCR) & BMCR_FULLDPLX)
> - duplex = 1;
> + }
> }
>
> /* if duplex is set then bit 28 must be set, too */
> @@ -2819,6 +2833,16 @@
> }
>
> /*
> + * If we're ignoring the PHY then autoneg and the internal
> + * transciever are really not going to work so don't let the
> + * user select them.
> + */
> + if (np->ignore_phy && (ecmd->autoneg == AUTONEG_ENABLE ||
> + ecmd->port == PORT_TP)) {
> + return -EINVAL;
> + }
> +
> + /*
always kill the braces surrounding a single C statement
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists