[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