[U-Boot-Users] [PATCH 1/2] Add support for 802.3ae to MII-PHY code

Ben Warren bwarren at qstreams.com
Thu Jan 25 20:01:34 CET 2007


On Thu, 2007-01-25 at 19:14 +0100, Haavard Skinnemoen wrote:
> On 1/23/07, Ben Warren <bwarren at qstreams.com> wrote:
> 
> > +int miiphy_set_attribute(char *devname, enum miiphy_attr attr, void *value)
> 
> Maybe it's just me, but the need for a function like this just screams
> "give ownership of struct mii_dev back to the driver". And it's not
> really about "attributes" at all -- it's about callbacks, so I think
> the function is misnamed.
> 
This is a general method for setting any of the structure's attributes
(callbacks and data fields).  I'm open to suggestions for a better name,
but 'callback' isn't it. 
 
> Why not simply let the driver allocate its own struct mii_dev and
> initialize it as it sees fit? It would simplify the interface a lot.

I started out going down that path, but quickly realized that it was
better to augment the API rather than refactoring it.  Here's why
(please look past my horrid shell skills):

saruman$ cd u-boot
saruman$ grep -R miiphy_read * | sed s/:/\ / | awk '{print $1}' | uniq |
wc -l
33
saruman$ grep -R miiphy_write * | sed s/:/\ / | awk '{print $1}' | uniq
| wc -l
40
saruman$ grep -R miiphy_register * | sed s/:/\ / | awk '{print $1}' |
uniq | wc -l
18

The 'struct mii_dev' definition currently has private scope within
'miiphy_util.c'.  I guess we could put it in a header file and add
another register function that takes a struct as argument.  What is it
about the current implementation that doesn't meet your needs, though?

regards,
Ben





More information about the U-Boot mailing list