Re: [PATCH 4/6] net: phy: Support Marvell 88EE1545 PHY

From: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
To: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>,
	barebox@lists.infradead.org, Jason Cooper <jason@lakedaemon.net>,
	Willy Tarreau <w@1wt.eu>
Subject: Re: [PATCH 4/6] net: phy: Support Marvell 88EE1545 PHY
Date: Sat, 2 Aug 2014 17:33:28 -0300	[thread overview]
Message-ID: <20140802203328.GA1034@arch.cereza> (raw)
In-Reply-To: <53DD2484.3020101@gmail.com>

On 02 Aug 07:48 PM, Sebastian Hesselbarth wrote:
> On 08/02/2014 07:40 PM, Ezequiel Garcia wrote:
> > On 31 Jul 07:25 PM, Sebastian Hesselbarth wrote:
> >> On 29.07.2014 23:37, Ezequiel Garcia wrote:
> >>> On 29 Jul 05:44 PM, Ezequiel Garcia wrote:
> >>>> This commit adds support for Marvell's 88E1545 PHY chip. In particular, this
> >>>> allows to support QSGMII interfaces.
> >>>>
> >>>> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> >>>> ---
> [...]
> >>>>  /* Mask used for ID comparisons */
> >>>>  #define MARVELL_PHY_ID_MASK		0xfffffff0
> >>>
> >>> Small erratum: given this mask, the ID for the new PHY device should be
> >>> 0x0x01410eb0 instead of 0x01410eb1.
> >>>
> >>> And by the way, Mirabox and Openblocks AX3 network support is not working.
> >>> The link is not detected, so it seems there's some more PHY work to be done.
> > 
> > I've been doing some more tests and it seems that Openblocks AX3 works fine
> > after all (last time I wasn't setting the parameters properly).
> > 
> > I need to do some more tests for Mirabox, but we probably need to setup the
> > pinctrl to make it work. Currently, the board hangs when trying to bring up
> > the network interface.
> > 
> >>
> >> I just confirmed that (after adding support for 88e1318), driver works
> >> on Armada XP ix4-300d on both RGMII ports.
> >>
> >> Armada XP requires some additional magic foo in some retarded register
> >> to enable RGMII:
> >>
> >> #define ARMADA_XP_PUP_ENABLE_BASE	(ARMADA_370_XP_INT_REGS_BASE + 0x1864c)
> >> /* Enable GBE0, GBE1, LCD and NFC PUP */
> >> reg = readl(ARMADA_XP_PUP_ENABLE_BASE);
> >> reg |= 0x17;
> >> writel(reg, ARMADA_XP_PUP_ENABLE_BASE);
> >>
> > 
> > Thanks a lot for this test! Do you think it's OK to just add that
> > initialization to armada_370_xp_init_soc() ?
> 
> I was hoping you'd try to do so on your SGMII board and report back
> if it does any harm. I kind of have the impression that it is
> _required_ only for single ended (R)GMII pins.
> 
> Also, I already have patches ready to separate 370 and XP. They are
> quite compatible, but have some early differences of course. That way
> we have some code duplication but won't pollute the file with early
> setup for both.
> 
> >> I am not yet sure if that is also true for Armada 370 nor if it is also
> >> true for SGMII/QSGMII ports. Took me a while to hunt this down, but
> >> without above foo, there is no txclk on RGMII.
> >>
> > 
> > The register is documented only on Armada XP, and it seems it's not needed
> > for SGMII/QSGMII.
> 
> I'd say, if it does no harm, enable it unconditionally.
> 

Yes, I tested on SGMII and QSGMII (OBS AX3-4 and XP-GP) and found it makes no
difference, and no harm.
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox