@Wolfgang: > The usb drivers use REGISTER_TIMEOUT32(entry->skb->len), evidently > basing the timeout value on the length of the data written. Based on > that it would now be REGISTER_TIMEOUT32(entry->skb->len + padding_len).
Yes, I overlooked that, thanks for pointing it out. > However, that time seems to be rather long already, my beacon e.g. would > have a timeout of 11500ms. padding_len would only add a maximum of > 375ms. I tried to find some indication of how long that timeout really > has to be. It feels like people are using various incarnations of > "plenty". That's a long time, but an extra 375ms is insignificant. I guess it would probably be better to include the extra bytes in the timeout calculation (if we continue to carry the usb changes, more about that below). > It would have been possible to just backport the beacon padding for the > two pci drivers, since only they use the changed register_multiwrite() > from the second patch in 2.6.35, so only they need it as preparation for > that patch. But the usb beacons should be padded anyway, its nice to > have. Hmm, that appears to be true. For some reason I thought I had identified a connection, but looking now that doesn't seem to make sense at all. I was probably mixing up what I saw in the upstream code with what I saw in the maverick tree. I'm probably going to strip the usb changes out then, since this was only reported against pci and as such those changes are unrelated. > Regarding the missing return value check. Good question. No, not sure > that it will always be successful. Normal beacons should always have > enough tailroom so that skb_pad() does not even have to ask for more > memory, but we are not sure that this will always be the case. If an > extra long beacon should need to be padded in an out of memory situation > skb_pad() might fail, not getting the memory. I tried but could not > provoke that. Seems linux does like to give skb_pad() what it asks for > even while oom-killing. If it does happen however skb_pad() would free > the skb and the call to free it again with dev_kfree_skb_any() produces > a hard panic that will leave the user without anything useful to report > (unless he already lay in wait before the console with an HD video > camera). That, plus seeing that this is checked everywhere else gives me > second thoughts, so I am considering proposing a patch upstream (for > linux-next). I'd appreciate your thoughts on this issue. I'd certainly prefer to see the checks there. I'll add a patch to introduce the checks when I refresh the series with the other changes. > I successfully tried your test kernel with rt2800pci. It now loads the > firmware (and also works after that in station mode). Testing against > regressions introduced by the padding patch is trickier as it involves > four different drivers and this code does only get used in ad hoc mode. Thanks for the review and testing! Agree about the regression testing being dificult, it frequently is :) I'll post new patches and test builds as soon as I can get to them, hopefully later today. -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/659143 Title: 64bit-only: regression: kernels >=2.6.34: rt2800pci: load firmware Error with ralink [1814:0781] -- ubuntu-bugs mailing list [email protected] https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
