Hans Petter Selasky wrote:

I am not sure what you mean by this statement, since it can be
interpreted in several ways, some not so friendly.

I mean I need to make another patchset. And that the current patchset will break the kernel compilation if blindly committed after mpsafetty.

OK, thanks for clarifying.

The patch only includes kernel parts, is there any impact on userland?
No. Userland will build like before. The current USB userland utilities
and libusb from ports will only work with the old USB stack. I'm working
on a replacement for "usbdevs" called "usbconfig" which has some more
functionality and a FreeBSD specific libusb, which is compatible with
libusb from sourceforge. I have most of it finished in my private SVN
repository, and will add it to my P4 repository soon.
This raises the question of why the kernel changes need to be committed
now, and what benefit they bring in the absence of a compatible
userland.  Shouldn't the commit happen after both kernel and userland
are ready (and reviewed)?

I think that is correct.

OK, so does this mean we should expect to see a revised commit candidate patch from you and/or alfred later once that is finished?

Another comment: the new code seems to bundle all new drivers into
"subsystems" but not allow them to be loaded individually.  This is
quite contrary to how the rest of the kernel is structured, and seems to
be of questionable benefit.  For example, users will rarely have more
than one USB ethernet driver in use on a given system but "device
usb2_ethernet" will compile in all 7 drivers.

It is still possible to separate the USB drivers. In my experience grouping the drivers gives a more user-friendly experience. For example you have a USB serial port adapter and plug it in. Then all you need to do is to kldload usb2_serial regardless of adapter. It is like a container module. I don't opponent that for kernel compilation you can have a more fine-grained control what drivers are actually included. I will see about fixing that.

You could look at what the sound code does, (I think it is specifically the snd_driver module). This implements auto-loading of the "right" drivers by loading them and then unloading the ones that don't attach. This still has overhead though, so users often will just compile in the ones they know they have.

Also is it safe to drop and reacquire the lock here?

You have to drop the lock, else I get witness warnings.

Yes, and presumably rightly so. It doesn't mean that dropping the lock and later reacquiring it is safe though; it could introduce races.

This seems to say that m->lock now may be NULL in situations where it
was not before (since there is no handling for m->lock == NULL in
existing parts of the code), so the Giant locking is new.

That is just a safety measure, because the Sound code has some ifdef's to enable it to run without mutexes, and in that case Giant must be used.

OK, this seems unrelated to USB then and is something you should discuss with the sound maintainers. It sounds to me like the ability to "run without mutexes" is the real bug here, and "support" for that should be removed completely instead of patching it up downstream.

It's needed for compiling usb2_ndis, but...
Without this patch the usb2_ndis module will not compile on certain
architectures. If you remove this patch you will need to decouple the
usb2_ndis module, which is not complete anyway, from the default build.
Why will it not compile?  This looks like a workaround for some other
issue that should be solved directly.

There are multiple issues:

1) If PAGE_SIZE is 16384 bytes, then the code simply fails.
2) Sometimes PAGE_SHIFT is already defined.

#if PAGE_SIZE == 4096
#define PAGE_SHIFT      12
#elif PAGE_SIZE == 8192
#define PAGE_SHIFT      13
#else
#error PAGE_SHIFT undefined!
#endif

OK, but again, why? If some architecture is not defining the same symbols as the others then maybe that architecture should be fixed, instead of working around the effect downstream.

Kris

P.S. There were a number of questions you didn't answer, can I assume those will follow later?

_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to "[EMAIL PROTECTED]"

Reply via email to