[U-Boot] [PATCH V4 3/8] imx: fec: Resolve speed before configuring gasket

[Timo Ketola]

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