Re: Change ABI of mii_readreg() and mii_writereg()

[Masanobu SAITOH <msaitoh%execsw.org@localhost>]

On 2018/12/25 17:10, Masanobu SAITOH wrote:
> On 2018/12/25 17:04, Masanobu SAITOH wrote:
> >   Hi.
> >
> >   Our mii(4)'s mii_readreg() and mii_writereg() are defined as below:
> >
> > > /*
> > >  * 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
> > 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?

Done:

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

In the commit message:
>  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.
> It should be fixed:
>
>         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

--
-----------------------------------------------
                SAITOH Masanobu (msaitoh%execsw.org@localhost
                                 msaitoh%netbsd.org@localhost)