On Wed, 2010-03-31 at 02:18 +0100, David Woodhouse wrote: > We don't use the normal hotplug mechanism because it doesn't work. It will > load the module some time after the device appears, but that's not good > enough for us -- we need the driver loaded _immediately_ because otherwise > the NIC driver may just abort and then the phy 'device' goes away. > > Instead, we just issue a request_module() directly in phy_device_create(). [...]
Thanks for doing this, David. I had a stab at it earlier when this problem was reported in Debian <http://bugs.debian.org/553024>. I didn't complete this because (a) I didn't understand all the details of adding new device table type, and (b) I tried to avoid duplicating information, which turns out to be rather difficult in modules with multiple drivers. Since you've dealt with (a), and (b) is not really as important, I would just like to suggest some minor changes to your patch 1 (see below). Feel free to fold them in. Your patch 2 would then need the substitutions s/phy_device_id/mdio_device_id/; s/TABLE(phy/TABLE(mdio/. Ben. From: Ben Hutchings <b...@decadent.org.uk> Date: Thu, 1 Apr 2010 05:03:02 +0100 Subject: [PATCH] phylib: Minor cleanup of phylib autoloading Refer to MDIO, consistent with other module aliases using bus names. Change type names to __u32, consistent with the rest of the file. Add kernel-doc comment to struct mdio_device_id. Signed-off-by: Ben Hutchings <b...@decadent.org.uk> --- drivers/net/phy/phy_device.c | 2 +- include/linux/mod_devicetable.h | 22 ++++++++++++++-------- scripts/mod/file2alias.c | 8 ++++---- 3 files changed, 19 insertions(+), 13 deletions(-) diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index b35ec7e..b0e54b4 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -188,7 +188,7 @@ struct phy_device* phy_device_create(struct mii_bus *bus, int addr, int phy_id) around for long enough for the driver to get loaded. With MDIO, the NIC driver will get bored and give up as soon as it finds that there's no driver _already_ loaded. */ - sprintf(modid, "phy:" PHYID_FMT, PHYID_ARGS(phy_id)); + sprintf(modid, MDIO_MODULE_PREFIX MDIO_ID_FMT, MDIO_ID_ARGS(phy_id)); request_module(modid); #endif diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h index 0c3e300..55f1f9c 100644 --- a/include/linux/mod_devicetable.h +++ b/include/linux/mod_devicetable.h @@ -474,10 +474,10 @@ struct platform_device_id { __attribute__((aligned(sizeof(kernel_ulong_t)))); }; -#define PHY_MODULE_PREFIX "phy:" +#define MDIO_MODULE_PREFIX "mdio:" -#define PHYID_FMT "%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d" -#define PHYID_ARGS(_id) \ +#define MDIO_ID_FMT "%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d%d" +#define MDIO_ID_ARGS(_id) \ (_id)>>31, ((_id)>>30) & 1, ((_id)>>29) & 1, ((_id)>>28) & 1, \ ((_id)>>27) & 1, ((_id)>>26) & 1, ((_id)>>25) & 1, ((_id)>>24) & 1, \ ((_id)>>23) & 1, ((_id)>>22) & 1, ((_id)>>21) & 1, ((_id)>>20) & 1, \ @@ -487,11 +487,17 @@ struct platform_device_id { ((_id)>>7) & 1, ((_id)>>6) & 1, ((_id)>>5) & 1, ((_id)>>4) & 1, \ ((_id)>>3) & 1, ((_id)>>2) & 1, ((_id)>>1) & 1, (_id) & 1 - - -struct phy_device_id { - uint32_t phy_id; - uint32_t phy_id_mask; +/** + * struct mdio_device_id - identifies PHY devices on an MDIO/MII bus + * @phy_id: The result of + * (mdio_read(&MII_PHYSID1) << 16 | mdio_read(&PHYSID2)) & @phy_id_mask + * for this PHY type + * @phy_id_mask: Defines the significant bits of @phy_id. A value of 0 + * is used to terminate an array of struct mdio_device_id. + */ +struct mdio_device_id { + __u32 phy_id; + __u32 phy_id_mask; }; #endif /* LINUX_MOD_DEVICETABLE_H */ diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c index b412185..0e08b8b 100644 --- a/scripts/mod/file2alias.c +++ b/scripts/mod/file2alias.c @@ -797,7 +797,7 @@ static int do_platform_entry(const char *filename, } static int do_phy_entry(const char *filename, - struct phy_device_id *id, char *alias) + struct mdio_device_id *id, char *alias) { char str[33]; int i; @@ -813,7 +813,7 @@ static int do_phy_entry(const char *filename, str[i] = '0'; } - sprintf(alias, PHY_MODULE_PREFIX "%s", str); + sprintf(alias, MDIO_MODULE_PREFIX "%s", str); return 1; } @@ -964,9 +964,9 @@ void handle_moddevtable(struct module *mod, struct elf_info *info, do_table(symval, sym->st_size, sizeof(struct platform_device_id), "platform", do_platform_entry, mod); - else if (sym_is(symname, "__mod_phy_device_table")) + else if (sym_is(symname, "__mod_mdio_device_table")) do_table(symval, sym->st_size, - sizeof(struct phy_device_id), "phy", + sizeof(struct mdio_device_id), "phy", do_phy_entry, mod); free(zeros); } -- 1.7.0.3 -- Ben Hutchings Once a job is fouled up, anything done to improve it makes it worse.
signature.asc
Description: This is a digitally signed message part
