Mbed-os: Typos/thinkos in smsc9220_eth_drv.h

Created on 10 Jan 2019 · 8Comments · Source: ARMmbed/mbed-os

Description

I'm working on MPS2 board Ethernet driver for Zephyr RTOS. We started with mbedOS driver as a starting point, which as far as I can tell, by now is "old" driver, we used files:

targets/TARGET_ARM_SSG/TARGET_CM3DS_MPS2/device/drivers/smsc9220_eth.h
targets/TARGET_ARM_SSG/TARGET_CM3DS_MPS2/device/drivers/smsc9220_eth.c

As far as I can tell, those are by now replaced by other (C++) driver and header. In either case, I spotted that both old and new header has an issue in definition of constants for PHY registers: https://github.com/ARMmbed/mbed-os/blob/dee3506fe4b9ebd86fe2fc276e3fd3f07d20bb82/targets/TARGET_ARM_SSG/TARGET_CM3DS_MPS2/device/drivers/smsc9220_eth_drv.h#L128

Looking at the datasheet, I have here, 00002417A.pdf from MicroChip, page 104, says: "Index (In Decimal)". In other words, unlike most other register addresses in the datasheet which are in hex, PHY registers are in decimal in range 0-31 (0x0-0x1f). As can be seen at the link above, the mbedOS header has hex numbers instead.

The old driver with which we started didn't really use PHY registers with offsets >9, for which there would be difference. I didn't check the new driver, maybe it's the same. But it doesn't seem a good idea to have incorrect values in the header, someone may want to extend the driver, etc. So, as we fixed that in Zephyr header, I thought it makes sense to report this issue to the original upstream project. (Note that we had to rewrite and refactor driver for Zephyr heavily, so there's no reusable patch.)