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

[Grant Likely <grant.likely@secretlab.ca>]

On Thu, Jan 14, 2010 at 1:13 AM, Liu Yu <yu.liu@freescale.com> wrote:
> Old method only set tbi-phy for eth0.
>
> Signed-off-by: Liu Yu <yu.liu@freescale.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@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>



-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.