| Message ID | 1275913563.1823.1.camel@yio.site |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
On Mon, 2010-06-07 at 14:26 +0200, Kay Sievers wrote: > On Mon, Jun 7, 2010 at 13:41, Johannes Berg <johannes@sipsolutions.net> wrote: > > (mind you, I think we probably need to have the bus/driver assignment, > > but I wanted to try out your suggestion first) > > Class device can never have a driver. And unregistered drivers can > not be used, what you try to do here. That all should just be removed or > properly registered with the core if needed. Yeah but it shouldn't influence the operation? Anyway I see from your patch that I should have assigned the release function and that would've helped I guess. > > So I removed bus/driver assignment from the above code just to try it > > out, and got > > > > Device 'hwsim0' does not have a release() function, it is broken and > > must be fixed. > > The driver core expects the resources of the device to be freed on > release. You can provide an empty release function because you do this > from a global list of devices. Ok, makes sense. > > This has evolved far too much for me right now. Can we just apply the > > initial patch from Eric and be happier for a while? I can't justify > > spending this much time on it right now. Alternatively, you could look > > at hwsim too, since it's all virtual, nothing special is required, I do > > testing in a virtual machine ... > > > > current patch is at > > http://johannes.sipsolutions.net/patches/kernel/all/2010-06-07-11:41/hwsim-bus.patch > > Here is something that seems to work for me. I see you remove the driver. Does this mean that in sysfs these devices wouldn't have a driver symlink? ISTR that we needed that for userspace, but I'm not entirely sure, and I don't have all the relevant userspace in my test setup. johannes -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jun 7, 2010 at 14:36, Johannes Berg <johannes@sipsolutions.net> wrote: > On Mon, 2010-06-07 at 14:26 +0200, Kay Sievers wrote: >> On Mon, Jun 7, 2010 at 13:41, Johannes Berg <johannes@sipsolutions.net> wrote: >> > (mind you, I think we probably need to have the bus/driver assignment, >> > but I wanted to try out your suggestion first) >> >> Class device can never have a driver. And unregistered drivers can >> not be used, what you try to do here. That all should just be removed or >> properly registered with the core if needed. > > Yeah but it shouldn't influence the operation? It does. You can not use "dead static" drivers. They need to be properly registered, and can only work for bus devices. Class devices are not allowed to have drivers. This worked only because the core ignored the value you assigned behind its back. :) >> Here is something that seems to work for me. > > I see you remove the driver. Does this mean that in sysfs these devices > wouldn't have a driver symlink? ISTR that we needed that for userspace, > but I'm not entirely sure, and I don't have all the relevant userspace > in my test setup. Yeah, if you need the driver, you need to use bus device, as you do now. The driver needs to be registered with the core, and the probe function needs to tell to bind to the devices while registered. You can not manually assign this. Kay -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2010-06-07 at 14:54 +0200, Kay Sievers wrote: > > I see you remove the driver. Does this mean that in sysfs these devices > > wouldn't have a driver symlink? ISTR that we needed that for userspace, > > but I'm not entirely sure, and I don't have all the relevant userspace > > in my test setup. > > Yeah, if you need the driver, you need to use bus device, as you do > now. The driver needs to be registered with the core, and the probe > function needs to tell to bind to the devices while registered. You > can not manually assign this. So I spoke to Jouni and he says that he doesn't remember needing the driver, so I guess we don't care. Can I get you to submit that patch properly? Send it to linux-wireless@vger.kernel.org please. johannes -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jun 8, 2010 at 11:27, Johannes Berg <johannes@sipsolutions.net> wrote: > On Mon, 2010-06-07 at 14:54 +0200, Kay Sievers wrote: > >> > I see you remove the driver. Does this mean that in sysfs these devices >> > wouldn't have a driver symlink? ISTR that we needed that for userspace, >> > but I'm not entirely sure, and I don't have all the relevant userspace >> > in my test setup. >> >> Yeah, if you need the driver, you need to use bus device, as you do >> now. The driver needs to be registered with the core, and the probe >> function needs to tell to bind to the devices while registered. You >> can not manually assign this. > > So I spoke to Jouni and he says that he doesn't remember needing the > driver, so I guess we don't care. Can I get you to submit that patch > properly? Send it to linux-wireless@vger.kernel.org please. Sure, you mean the patch that changes the hwsim code, or the one I talked about to properly create a virtual bus parent? Kay -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2010-06-08 at 11:30 +0200, Kay Sievers wrote: > On Tue, Jun 8, 2010 at 11:27, Johannes Berg <johannes@sipsolutions.net> wrote: > > On Mon, 2010-06-07 at 14:54 +0200, Kay Sievers wrote: > > > >> > I see you remove the driver. Does this mean that in sysfs these devices > >> > wouldn't have a driver symlink? ISTR that we needed that for userspace, > >> > but I'm not entirely sure, and I don't have all the relevant userspace > >> > in my test setup. > >> > >> Yeah, if you need the driver, you need to use bus device, as you do > >> now. The driver needs to be registered with the core, and the probe > >> function needs to tell to bind to the devices while registered. You > >> can not manually assign this. > > > > So I spoke to Jouni and he says that he doesn't remember needing the > > driver, so I guess we don't care. Can I get you to submit that patch > > properly? Send it to linux-wireless@vger.kernel.org please. > > Sure, you mean the patch that changes the hwsim code, or the one I > talked about to properly create a virtual bus parent? The hwsim one you pasted earlier. johannes -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c index 6f8cb3e..d210ce6 100644 --- a/drivers/net/wireless/mac80211_hwsim.c +++ b/drivers/net/wireless/mac80211_hwsim.c @@ -194,7 +194,13 @@ static inline void hwsim_clear_sta_magic(struct ieee80211_sta *sta) sp->magic = 0; } -static struct class *hwsim_class; +static struct bus_type hwsim_bus = { + .name = "mac80211_hwsim", +}; + +static struct device hwsim_parent = { + .init_name = "mac80211_hwsim", +}; static struct net_device *hwsim_mon; /* global monitor netdev */ @@ -1071,14 +1077,10 @@ static void mac80211_hwsim_free(void) device_unregister(data->dev); ieee80211_free_hw(data->hw); } - class_destroy(hwsim_class); + device_unregister(&hwsim_parent); + bus_unregister(&hwsim_bus); } - -static struct device_driver mac80211_hwsim_driver = { - .name = "mac80211_hwsim" -}; - static const struct net_device_ops hwsim_netdev_ops = { .ndo_start_xmit = hwsim_mon_xmit, .ndo_change_mtu = eth_change_mtu, @@ -1231,6 +1233,9 @@ DEFINE_SIMPLE_ATTRIBUTE(hwsim_fops_group, hwsim_fops_group_read, hwsim_fops_group_write, "%llx\n"); +/* all devices are kept in out own list and the ressources are freed on exit */ +static void hwsim_dev_release(struct device* dev) {} + static int __init init_mac80211_hwsim(void) { int i, err = 0; @@ -1251,9 +1256,15 @@ static int __init init_mac80211_hwsim(void) spin_lock_init(&hwsim_radio_lock); INIT_LIST_HEAD(&hwsim_radios); - hwsim_class = class_create(THIS_MODULE, "mac80211_hwsim"); - if (IS_ERR(hwsim_class)) - return PTR_ERR(hwsim_class); + err = bus_register(&hwsim_bus); + if (err) + return err; + + err = device_register(&hwsim_parent); + if (err) { + bus_unregister(&hwsim_bus); + return err; + } memset(addr, 0, ETH_ALEN); addr[0] = 0x02; @@ -1271,16 +1282,24 @@ static int __init init_mac80211_hwsim(void) data = hw->priv; data->hw = hw; - data->dev = device_create(hwsim_class, NULL, 0, hw, - "hwsim%d", i); - if (IS_ERR(data->dev)) { - printk(KERN_DEBUG - "mac80211_hwsim: device_create " - "failed (%ld)\n", PTR_ERR(data->dev)); + data->dev = kzalloc(sizeof(struct device), GFP_KERNEL); + if (!data->dev) { err = -ENOMEM; goto failed_drvdata; } - data->dev->driver = &mac80211_hwsim_driver; + + dev_set_name(data->dev, "hwsim%d", i); + data->dev->parent = &hwsim_parent; + data->dev->bus = &hwsim_bus; + data->dev->release = hwsim_dev_release; + + err = device_register(data->dev); + if (err) { + printk(KERN_DEBUG + "mac80211_hwsim: device_register failed (%d)\n", + err); + goto failed_drvdata; + } SET_IEEE80211_DEV(hw, data->dev); addr[3] = i >> 8;