On Sat, Dec 20, 2014 at 09:08:51AM -0800, Florian Fainelli wrote:
> 2014-12-18 19:49 GMT-08:00 Lennart Sorensen <lsore...@csclub.uwaterloo.ca>:
> > I have been trying to move an 8360 based system from a 3.0 kernel to a
> > 3.12 (on the way to 3.14 with ipipe/xenomai) kernel and encountered an
> > oops in the ucc_geth driver when using RTBI mode on one of the ucc
> > ports. I haven't managed to find any commits to of_mdio or ucc_geth or
> > fsl_pq_mdio that would appear to address this problem, so I believe it
> > is still present in the latest kernel, but have not confirmed that with
> > testing yet.
> >
> > Commit 058112c7efc9ef43bb511c137293dddbe6e42908 appears to have broken
> > ucc support for tbi phy detection.
> >
> > With the patch in place, I am unable to get the mdio bus to create phy
> > devices for the tbi phy in the ucc on an 8360e, and the ucc_geth driver
> > causes a kernel oops, while with the patch reverted, it does create them
> > and the driver comes up and works.
> >
> > The tbi phy is needed when using a ucc in RTBI, TBI or SGMII mode.
> >
> > I am not convinced that the tbi phy really behaves quite like a real phy,
> > which may be why get_phy_device does not work with it. Perhaps there
> > is a better way to deal with the tbi phy on the ucc for this purpose.
>
> There are some comments in ucc_geth that also lead me to believe this
> is a just a hack instead of a real Ethernet PHY device. Part of what I
> think got broken is because of this comment:
>
> /* Initialize TBI PHY interface for communicating with the
> * SERDES lynx PHY on the chip. We communicate with this PHY
> * through the MDIO bus on each controller, treating it as a
> * "normal" PHY at the address found in the UTBIPA register. We assume
> * that the UTBIPA register is valid. Either the MDIO bus code will set
> * it to a value that doesn't conflict with other PHYs on the bus, or the
> * value doesn't matter, as there are no other PHYs on the bus.
> */
>
> In particular this one:
>
> "Either the MDIO bus code will set
> * it to a value that doesn't conflict with other PHYs on the bus, or the
> * value doesn't matter, as there are no other PHYs on the bus."
>
> and what Sebastian removed did exactly that, we used the special MDIO
> broadcast address 0 to provide this "whatever". If this is such a
> requirement from the ucc_geth driver and TBI PHYs, maybe we should
> have this hack somewhere in the actual MDIO driver used by the
> ucc_geth driver instead, or set a flag/read the PHY connection mode
> and do this in drivers/of/of_mdio.c
>
I discovered a problem with the tbi address handling on ucc_geth.
In get_ucc_tbipa, the passed in pointer is expecting a pointer to a struct
fsl_pq_mdio, but on ucc the pointer is actually to the start of the mii
area, since it doesn't have all the stuff that the etsec2 has, so as a
result the address returned for tbipa is actually 1312 bytes too high,
which means the address never gets set of course. In fact the driver
prints out cr=0 and sr=0, while with the older working driver it printed
cr=140 and sr=149.
As a quick test I did:
}
tbipa = data->get_tbipa(priv->map - offsetof(struct
fsl_pq_mdio, mii));
out_be32(tbipa, be32_to_cpup(prop));
and that made it work, but of course is ugly and would break etsec2.
Any suggestion for a clean way to make get_ucc_tbipa able to dereference
the structure correctly?
I suppose I could do:
/*
* Return the TBIPAR address for a QE MDIO node
*/
static uint32_t __iomem *get_ucc_tbipa(void __iomem *p)
{
struct fsl_pq_mdio __iomem *mdio = p - offsetof(struct fsl_pq_mdio,
mii);
return &mdio->utbipar;
}
but it seems like just putting more hacks in place. The use of the
mii_offset in the first place seems like a clue that defining one
structure for etsec2 and ucc and such even though it doesn't apply to
both is probably an error. It would just be using mii_offset in reverse
for the ucc, versus the etsec2.
--
Len Sorensen
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
