Re: Net - new API for handling PHY devices

[Andrew Lunn <andrew dot lunn at ascom dot ch>]

On Tue, Aug 19, 2003 at 11:27:36AM -0600, Gary Thomas wrote:
> Still rough, but at least a framework for handling these devices.

> Index: devs/eth/phy/current/ChangeLog
> +cdl_package CYGPKG_DEVS_ETH_PHY {
> +    display       "Ethernet transciever (PHY) support"
> +    description   "API for ethernet PHY devices"

CYGPKG_DEVS_ETH_PHY seems a rather generic name, when you are only
supporting one way of talking to the phy. This code uses bit banging,
but i could imagine some ethernet chips with built in shift registers
which you just need to give bytes or words to. Maybe you should call
this package .._ETH_PHY_BITBANG, and leave .._ETH_PHY for a generic
layer which goes over all possible PHY access packages?

> +
> +    parent        CYGPKG_IO_ETH_DRIVERS
> +
> +    include_dir   cyg/io

If the parent is in the io tree and the include files go into the io
tree, it seems better to me to put this package also in the io src tree.

> +// Transceiver mode
> +#define PHY_BMCR             0x00    // Register number
> +#define PHY_BMCR_RESET       0x8000
> +#define PHY_BMCR_LOOPBACK    0x4000
> +#define PHY_BMCR_100MB       0x2000
> +#define PHY_BMCR_AUTO_NEG    0x1000
> +#define PHY_BMCR_POWER_DOWN  0x0800
> +#define PHY_BMCR_ISOLATE     0x0400
> +#define PHY_BMCR_RESTART     0x0200
> +#define PHY_BMCR_FULL_DUPLEX 0x0100
> +#define PHY_BMCR_COLL_TEST   0x0080

Where have these come from? Are they part of some standard, or
specific to one PHY? There are some obvious things missing, eg 1000MB,
UTP, BNC. AUI.  

> +// Physical device access - defined by hardware instance
> +typedef struct {
> +    void (*init)(void);
> +    void (*set_data)(int);
> +    int  (*get_data)(void);
> +    void (*set_clock)(int);
> +    void (*set_dir)(int);
> +} eth_phy_access_t;

Some of the device drivers support multiple instances of devices, eg
the i82559. The phy access functions should also support multiple
devices. Its necessary to add an extra parameter to all these
functions, struct eth_drv_sc *sc. 

> +externC void _eth_phy_init(eth_phy_access_t *f);
> +externC void _eth_phy_write(eth_phy_access_t *f, int reg, int unit, unsigned short data);
> +externC bool _eth_phy_read(eth_phy_access_t *f, int reg, int unit, unsigned short *val);

These are the external API functions from the package. Shouldn't they
have cyg prefixes?

     Andrew