Erroneous MDIO register documentation or definitions for mEMAC?

[met]

Hi all,

I have come across something extremely confusing with the #defines at the end of fsl_memac.h, available in FreeScale's U-Boot Git repository here: http://git.freescale.com/git/cgit.cgi/ppc/sdk/u-boot.git/tree/include/fsl_memac.h

If you compare the defines with the register definitions in f.ex. "QorIQ LS1046A Data Path Acceleration Architecture (DPAA) Reference Manual", section "6.4.3.4 MDIO Ethernet Management Interface Registers", almost all of them appear to be "backwards".
Allow me to use MDIO_STAT as an example:

#define MDIO_STAT_CLKDIV(x) (((x>>1) & 0xff) << 8) /* Wrong! Should be 16 */
#define MDIO_STAT_BSY (1 << 0) /* Correct! */
#define MDIO_STAT_RD_ER (1 << 1) /* Wrong! Should be 30 */
#define MDIO_STAT_PRE (1 << 5) /* Wrong! Should be 26 */
#define MDIO_STAT_ENC (1 << 6) /* Wrong! Should be 25 */
#define MDIO_STAT_HOLD_15_CLK (7 << 2) /* Wrong! Should be 27 */
#define MDIO_STAT_NEG (1 << 23) /* Wrong! Should be 8 */

The only bit definition that appears correct according to documentation is MDIO_STAT_BSY.
All the other definitions are backwards, i.e., singular bit N is "moved" to 31-N.

There is also a #define for MDIO_DATA_BSY, which I can't find any mention of in the MDIO_DATA register definition.
Does this bit exist at all?

What's going on here? Is the documentation for the MDIO registers wrong?

Or am I misunderstanding something?

Best regards,
    Martin Etnestad

[r8070z · Accepted solution]

Have a great day,

In the DPAA manual bit 0 is the most significant bit of the 32-bit registers. There was patch net/memac_phy: reuse driver for little endian SoCs. It says “The memac for PHY management on little endian SoCs is similar on big endian SoCs, so we modify the driver by using I/O accessor function to handle the endianness, so the driver can be reused on little endian SoCs, we introduce CONFIG_SYS_MEMAC_LITTLE_ENDIAN for little endian SoCs , if not, the I/O access is big endian. Move fsl_memac.h out of powerpc include.”

I have checked some powerpc DPAA manual. In all manuals  the lsb bit of the MDIO_CFG is reserved and cleared. In the 32-bit MDIO_DATA only 16 lsb bits are set as data i.e. the msb of MDIO_DATA is also zero. Hence all timeout checks based on

   while ((memac_in_32(&regs->mdio_data)) & MDIO_DATA_BSY) //read 32-bit reg and check the msb bit

   while ((memac_in_32(&regs->mdio_stat)) & MDIO_STAT_BSY) //read 32-bit reg and check the lsb bit

silently return without waiting for busy. Such waiting for the MDIO_DATA_BSY is harmless. Waiting for MDIO_STAT_BSY can lead to error  if the next operation is not delayed properly. May be there was patch for MDIO_STAT_BSY bit or there are some delays in software in-between mdio accesses which neglect this bug.

-----------------------------------------------------------------------------------------------------------------------
Note: If this post answers your question, please click the Correct Answer button. Thank you!
-----------------------------------------------------------------------------------------------------------------------

[met]

Thank you for the confirmation, Serguei!

[cyrilstrejc]

Martin,

without looking exactly to the register documentation you describe, this took me one hour last week:

1. QorIQ LS documentation follows PowerPC documentation tradition and use MSB 0 bit numbering, specially bit 0 is 2^31 and bit 31 is 1.
2. Documentation of the registers (all registers, not only DPAA) is for big endian ARM, since present day ARM architecture gerenally can run in big or little endian mode. And more over, again, big endian follow the PowerPC tradition. UPDATE: sometimes little endian, sometimes big endian, it's described in reference manual -- little historical mess
3. Typical Linux situation is ARM in little endian mode, it was my case, I guess it's your case. 

Hence, you have to do byte swap when reading/writing registers UPDATE: some registers with swap, some without. Finally, I found the hint even in a manual. 

[met]

Cyril,

Thank you so much for sharing! :-)  I was not aware of this quirky notation.

So that means that the busy-bit (MDIO_STAT_BSY) has been defined wrong, as it should actually be bit 31, not 0.

A bit surprising that it has not been detected earlier, if it is in fact wrong.

There is still the data busy-bit (MDIO_DATA_BSY), which I can't find any documentation for, though.

Does anyone else know anything about this?