netdev - Re: [PATCH 3/4] ucc_geth: update the tbi-phy setting

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <fa686aa41002011400i7160f360offc5b788e82122ea@mail.gmail.com>
Date:	Mon, 1 Feb 2010 15:00:21 -0700
From:	Grant Likely <grant.likely@...retlab.ca>
To:	Liu Yu <yu.liu@...escale.com>
Cc:	galak@...nel.crashing.org, davem@...emloft.net,
	netdev@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org
Subject: Re: [PATCH 3/4] ucc_geth: update the tbi-phy setting

On Thu, Jan 14, 2010 at 1:13 AM, Liu Yu <yu.liu@...escale.com> wrote:
> Old method only set tbi-phy for eth0.
>
> Signed-off-by: Liu Yu <yu.liu@...escale.com>
> ---
>  drivers/net/ucc_geth.c |   23 +++++++++++++++--------
>  1 files changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
> index 750d86d..0e72013 100644
> --- a/drivers/net/ucc_geth.c
> +++ b/drivers/net/ucc_geth.c
> @@ -37,6 +37,7 @@
>  #include <asm/qe.h>
>  #include <asm/ucc.h>
>  #include <asm/ucc_fast.h>
> +#include <asm/machdep.h>
>
>  #include "ucc_geth.h"
>  #include "fsl_pq_mdio.h"
> @@ -1334,7 +1335,7 @@ static int adjust_enet_interface(struct ucc_geth_private *ugeth)
>        struct ucc_geth __iomem *ug_regs;
>        struct ucc_fast __iomem *uf_regs;
>        int ret_val;
> -       u32 upsmr, maccfg2, tbiBaseAddress;
> +       u32 upsmr, maccfg2;
>        u16 value;
>
>        ugeth_vdbg("%s: IN", __func__);
> @@ -1389,14 +1390,20 @@ static int adjust_enet_interface(struct ucc_geth_private *ugeth)
>        /* Note that this depends on proper setting in utbipar register. */
>        if ((ugeth->phy_interface == PHY_INTERFACE_MODE_TBI) ||
>            (ugeth->phy_interface == PHY_INTERFACE_MODE_RTBI)) {
> -               tbiBaseAddress = in_be32(&ug_regs->utbipar);
> -               tbiBaseAddress &= UTBIPAR_PHY_ADDRESS_MASK;
> -               tbiBaseAddress >>= UTBIPAR_PHY_ADDRESS_SHIFT;
> -               value = ugeth->phydev->bus->read(ugeth->phydev->bus,
> -                               (u8) tbiBaseAddress, ENET_TBI_MII_CR);
> +               struct ucc_geth_info *ug_info = ugeth->ug_info;
> +               struct phy_device *tbiphy;
> +
> +               if (!ug_info->tbi_node)
> +                       ugeth_warn("TBI mode requires that the device "
> +                               "tree specify a tbi-handle\n");

Shouldn't the driver avoid calling of_phy_find_device() if this error
path is taken?

> +
> +               tbiphy = of_phy_find_device(ug_info->tbi_node);
> +               if (!tbiphy)
> +                       ugeth_warn("Could not get TBI device\n");
> +
> +               value = phy_read(tbiphy, ENET_TBI_MII_CR);

Ditto here; the code unconditionally tries to read the tbiphy, even if
it isn't there.

Looks wrong.

g.

>                value &= ~0x1000;       /* Turn off autonegotiation */
> -               ugeth->phydev->bus->write(ugeth->phydev->bus,
> -                               (u8) tbiBaseAddress, ENET_TBI_MII_CR, value);
> +               phy_write(tbiphy, ENET_TBI_MII_CR, value);
>        }
>
>        init_check_frame_length_mode(ug_info->lengthCheckRx, &ug_regs->maccfg2);
> --
> 1.6.4
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@...ts.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>



-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
--
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