Change ABI of mii_readreg() and mii_writereg()

[Masanobu SAITOH]

Hi.

/*
* Callbacks from MII layer into network interface device driver.
*/
typedef int (*mii_readreg_t)(device_t, int, int);
typedef void (*mii_writereg_t)(device_t, int, int, int);

There is no formal way to tell any error (e.g. protocol error or timeout).
By this flaw, we had some bugs and fixed them many times.

On some driver, if the return value of mii_readreg() is all 0 or all 1,
it regards as error. It's not good because some MII PHY registers are
counter.

In 802.3 spec also says that the PHY shall not respond to read/write
transaction to the unimplemented register(22.2.4.3), so the device timeout
is exist in real. It can be used to detect whether a register is implemented
or not (if the register conforms to the spec).

Some Ethernet controller or system are required to take a semaphore
while accessing the PHY. It might be failed to get the semaphore.

So, I propose to change the API as follows
(I also change read/write register value from int to uint16_t because
all of the register is 16bit (and it should be unsigned)):

int (*mii_readreg_t)(device_t, int, int, uint16_t *);
int (*mii_writereg_t)(device_t, int, int, uint16_t);

OK?

[Masanobu SAITOH]

 Hi.

/*
 * Callbacks from MII layer into network interface device driver.
 */
typedef int (*mii_readreg_t)(device_t, int, int);
typedef void (*mii_writereg_t)(device_t, int, int, int);

 There is no formal way to tell any error (e.g. protocol error or timeout).
By this flaw, we had some bugs and fixed them many times.
 On some driver, if the return value of mii_readreg() is all 0 or all 1,
it regards as error. It's not good because some MII PHY registers are
counter.
 In 802.3 spec also says that the PHY shall not respond to read/write
transaction to the unimplemented register(22.2.4.3), so the device timeout
is exist in real. It can be used to detect whether a register is implemented
or not (if the register conforms to the spec).

s/the register/the PHY/

 Some Ethernet controller or system are required to take a semaphore
while accessing the PHY. It might be failed to get the semaphore.
So, I propose to change the API as follows
(I also change read/write register value from int to uint16_t because
int (*mii_readreg_t)(device_t, int, int, uint16_t *);
int (*mii_writereg_t)(device_t, int, int, uint16_t);
 OK?

[Masanobu SAITOH]

Post by Masanobu SAITOH

  Hi.

/*
 * Callbacks from MII layer into network interface device driver.
 */
typedef int (*mii_readreg_t)(device_t, int, int);
typedef void (*mii_writereg_t)(device_t, int, int, int);

  There is no formal way to tell any error (e.g. protocol error or timeout).
By this flaw, we had some bugs and fixed them many times.
  On some driver, if the return value of mii_readreg() is all 0 or all 1,
it regards as error. It's not good because some MII PHY registers are
counter.
  In 802.3 spec also says that the PHY shall not respond to read/write
transaction to the unimplemented register(22.2.4.3), so the device timeout
is exist in real. It can be used to detect whether a register is implemented
or not (if the register conforms to the spec).

s/the register/the PHY/

  Some Ethernet controller or system are required to take a semaphore
while accessing the PHY. It might be failed to get the semaphore.
So, I propose to change the API as follows
(I also change read/write register value from int to uint16_t because
int (*mii_readreg_t)(device_t, int, int, uint16_t *);
int (*mii_writereg_t)(device_t, int, int, uint16_t);
  OK?

Done:

http://mail-index.netbsd.org/source-changes/2019/01/22/msg102560.html

Post by Masanobu SAITOH
Note that I noticed that the following code do infinite loop in the
read/wirte function. If it accesses unimplemented PHY register, it will hang.
arm/at91/at91emac.c
arm/ep93xx/epe.c
arm/omap/omapl1x_emac.c
mips/ralink/ralink_eth.c
arch/powerpc/booke/dev/pq3etsec.c(read)
dev/cadence/if_cemac.c
dev/ic/lan9118.c

This was filed as PR#53902:

http://gnats.netbsd.org/53902