From mboxrd@z Thu Jan 1 00:00:00 1970 From: Timo Ketola Date: Fri, 20 Apr 2012 11:54:25 +0300 Subject: [U-Boot] [PATCH V4 3/8] imx: fec: Resolve speed before configuring gasket In-Reply-To: <4F9110AE.5040004@denx.de> References: <1334223234-23383-1-git-send-email-timo@exertus.fi> <1334825735-27992-1-git-send-email-timo@exertus.fi> <1334825735-27992-4-git-send-email-timo@exertus.fi> <4F906714.9030909@boundarydevices.com> <4F907315.7050907@exertus.fi> <4F907FF1.1030103@boundarydevices.com> <4F908240.7040605@boundarydevices.com> <4F90E78D.3070304@exertus.fi> <4F9110AE.5040004@denx.de> Message-ID: <4F912441.2020604@exertus.fi> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Dear Stefano, Troy, Scott, On 20.04.2012 10:30, Stefano Babic wrote: > On 20/04/2012 06:35, Timo Ketola wrote: >> [undeleted Stefano from CC-list] >> > > Hi Timo, hi Troy, > >> On 20.04.2012 00:23, Troy Kisky wrote: >>> On 4/19/2012 2:13 PM, Troy Kisky wrote: >>>> On 4/19/2012 1:18 PM, Timo Ketola wrote: >>>>> On 19.04.2012 22:27, Troy Kisky wrote: >>>>>> On 4/19/2012 1:55 AM, Timo Ketola wrote: >>>>>>> -#if !defined(CONFIG_MII) >>>>>>> - /* configure gasket for RMII, 50 MHz, no loopback, and no echo */ >>>>>>> - writew(MIIGSK_CFGR_IF_MODE_RMII,&fec->eth->miigsk_cfgr); >>>>>>> +#if defined(CONFIG_RMII) >>>>>> >>>>>> While this change seems to make sense, it could break some boards. >>>>> >>>>> Please explain how. Every board using fec_mxc define CONFIG_MII - >>>>> they have to: >>>>> >>>>> #ifndef CONFIG_MII >>>>> #error "CONFIG_MII has to be defined!" >>>>> #endif >>>> Does every board that has a gasket define CONFIG_RMII? > > as far as I can see, there are some inconsistencies. All boards define > CONFIG_MII, but they really need CONFIG_RMII, because only with my last > patch I set the gasket for MII. The driver has always set in a fixed way > the gasket for RMII, independently if CONFIG_RMII or CONFIG_MII was set, > and that is also wrong. Ah, so, to answer Troy, there really is RMII boards (which maybe was obvious to all others than me; I reasoned in wrong direction: because they would be already broken with this code, there could be none) and they were already broken. > I would say that the configuration file of most boards using fec_mxc > must be changed. > > And then fec_mxc.c does not need at all these lines: > #ifndef CONFIG_MII > #error "CONFIG_MII has to be defined!" > #endif Functionally this does nothing of course but I can imagine the reasoning behind that check: If I understand correctly, fec_mxc depends on MII management interface (for example miiphy_wait_aneg). Then, if CONFIG_MII is not defined, there is inconsistency because configuration says "don't use MII" but fec_mxc still uses it. I don't know whether this causes any confusion. > Boards are compiled clean without them. Correct me if I am wrong, but it > seems the correct way to do is to drop the unneeded check in the above > lines and sets CONFIG_RMII for all boards except the only one > (ima3-mx53), that needs really MII. Agreed regarding CONFIG_RMII. With dropping the check I'm OK either way. Furthermore, I might like to propose to change the name of the configuration variable CONFIG_MII to CONFIG_MII_MGM or something like that. That might reduce confusion (at least I have been quite confused). -- Timo