On Thu, Dec 23, 2004 at 03:40:31AM +0100, Adrian Bunk wrote:
> On Sun, Dec 19, 2004 at 04:31:46PM -0800, Greg KH wrote:
> > On Mon, Dec 20, 2004 at 01:16:44AM +0100, Adrian Bunk wrote:
> > > I've already seen people crippling their usb-storage driver with
> > > enabling BLK_DEV_UB - and I doubt the warning in the help text added
> > > after 2.6.9 will fix all such problems.
> > >
> > > Is there except for kernel size any good reason for using BLK_DEV_UB
> > > instead of USB_STORAGE?
> >
> > You don't want to use the scsi layer? You like the stability of it at
> > times? :)
> >
> > > If not, I'd suggest the patch below to let BLK_DEV_UB depend
> > > on EMBEDDED.
> >
> > No, it's good for non-embedded boxes too.
>
>
> My current understanding is:
> - BLK_DEV_UB supports a subset of what USB_STORAGE can support
> - for an average user, there's no reason to enable BLK_DEV_UB
> - if you really know what you are doing, there might be several reasons
> why you might want to use BLK_DEV_UB
I have been running with just the code portion of this patch for a while
now, with good results (no Kconfig changes.)
Pete and Matt, do you mind me applying the following portion of the
patch to the kernel tree?
thanks,
greg k-h
> --- linux-2.6.10-rc3-mm1-full/drivers/usb/storage/unusual_devs.h.old 2004-12-23 03:09:42.000000000 +0100
> +++ linux-2.6.10-rc3-mm1-full/drivers/usb/storage/unusual_devs.h 2004-12-23 03:09:53.000000000 +0100
> @@ -549,13 +549,11 @@
> US_SC_SCSI, US_PR_CB, NULL,
> US_FL_SINGLE_LUN ),
>
> -#if !defined(CONFIG_BLK_DEV_UB) && !defined(CONFIG_BLK_DEV_UB_MODULE)
> UNUSUAL_DEV( 0x0781, 0x0002, 0x0009, 0x0009,
> "Sandisk",
> "ImageMate SDDR-31",
> US_SC_DEVICE, US_PR_DEVICE, NULL,
> US_FL_IGNORE_SER ),
> -#endif
>
> UNUSUAL_DEV( 0x0781, 0x0100, 0x0100, 0x0100,
> "Sandisk",
> --- linux-2.6.10-rc3-mm1-full/drivers/usb/storage/usb.c.old 2004-12-23 03:10:00.000000000 +0100
> +++ linux-2.6.10-rc3-mm1-full/drivers/usb/storage/usb.c 2004-12-23 03:10:13.000000000 +0100
> @@ -144,9 +144,7 @@
> { USB_INTERFACE_INFO(USB_CLASS_MASS_STORAGE, US_SC_QIC, US_PR_BULK) },
> { USB_INTERFACE_INFO(USB_CLASS_MASS_STORAGE, US_SC_UFI, US_PR_BULK) },
> { USB_INTERFACE_INFO(USB_CLASS_MASS_STORAGE, US_SC_8070, US_PR_BULK) },
> -#if !defined(CONFIG_BLK_DEV_UB) && !defined(CONFIG_BLK_DEV_UB_MODULE)
> { USB_INTERFACE_INFO(USB_CLASS_MASS_STORAGE, US_SC_SCSI, US_PR_BULK) },
> -#endif
>
> /* Terminating entry */
> { }
> @@ -220,10 +218,8 @@
> .useTransport = US_PR_BULK},
> { .useProtocol = US_SC_8070,
> .useTransport = US_PR_BULK},
> -#if !defined(CONFIG_BLK_DEV_UB) && !defined(CONFIG_BLK_DEV_UB_MODULE)
> { .useProtocol = US_SC_SCSI,
> .useTransport = US_PR_BULK},
> -#endif
>
> /* Terminating entry */
> { NULL }
--
On Wed, Jan 19, 2005 at 02:07:07PM -0800, Greg KH wrote:
> On Thu, Dec 23, 2004 at 03:40:31AM +0100, Adrian Bunk wrote:
> > On Sun, Dec 19, 2004 at 04:31:46PM -0800, Greg KH wrote:
> > > On Mon, Dec 20, 2004 at 01:16:44AM +0100, Adrian Bunk wrote:
> > > > I've already seen people crippling their usb-storage driver with
> > > > enabling BLK_DEV_UB - and I doubt the warning in the help text added
> > > > after 2.6.9 will fix all such problems.
> > > >
> > > > Is there except for kernel size any good reason for using BLK_DEV_UB
> > > > instead of USB_STORAGE?
> > >
> > > You don't want to use the scsi layer? You like the stability of it at
> > > times? :)
> > >
> > > > If not, I'd suggest the patch below to let BLK_DEV_UB depend
> > > > on EMBEDDED.
> > >
> > > No, it's good for non-embedded boxes too.
> >
> >
> > My current understanding is:
> > - BLK_DEV_UB supports a subset of what USB_STORAGE can support
> > - for an average user, there's no reason to enable BLK_DEV_UB
> > - if you really know what you are doing, there might be several reasons
> > why you might want to use BLK_DEV_UB
>
> I have been running with just the code portion of this patch for a while
> now, with good results (no Kconfig changes.)
>
> Pete and Matt, do you mind me applying the following portion of the
> patch to the kernel tree?
I have no objection.
Matt
--
Matthew Dharm Home: [email protected]
Maintainer, Linux USB Mass Storage Driver
E: You run this ship with Windows?! YOU IDIOT!
L: Give me a break, it came bundled with the computer!
-- ESR and Lan Solaris
User Friendly, 12/8/1998
On Wed, Jan 19, 2005 at 06:49:00PM -0800, Matthew Dharm wrote:
> On Wed, Jan 19, 2005 at 02:07:07PM -0800, Greg KH wrote:
> > On Thu, Dec 23, 2004 at 03:40:31AM +0100, Adrian Bunk wrote:
> > > On Sun, Dec 19, 2004 at 04:31:46PM -0800, Greg KH wrote:
> > > > On Mon, Dec 20, 2004 at 01:16:44AM +0100, Adrian Bunk wrote:
> > > > > I've already seen people crippling their usb-storage driver with
> > > > > enabling BLK_DEV_UB - and I doubt the warning in the help text added
> > > > > after 2.6.9 will fix all such problems.
> > > > >
> > > > > Is there except for kernel size any good reason for using BLK_DEV_UB
> > > > > instead of USB_STORAGE?
> > > >
> > > > You don't want to use the scsi layer? You like the stability of it at
> > > > times? :)
> > > >
> > > > > If not, I'd suggest the patch below to let BLK_DEV_UB depend
> > > > > on EMBEDDED.
> > > >
> > > > No, it's good for non-embedded boxes too.
> > >
> > >
> > > My current understanding is:
> > > - BLK_DEV_UB supports a subset of what USB_STORAGE can support
> > > - for an average user, there's no reason to enable BLK_DEV_UB
> > > - if you really know what you are doing, there might be several reasons
> > > why you might want to use BLK_DEV_UB
> >
> > I have been running with just the code portion of this patch for a while
> > now, with good results (no Kconfig changes.)
> >
> > Pete and Matt, do you mind me applying the following portion of the
> > patch to the kernel tree?
>
> I have no objection.
Ok, I've commited the change to my trees, thanks.
greg k-h
On Wed, 2005-01-19 at 14:07 -0800, Greg KH wrote:
> I have been running with just the code portion of this patch for a while
> now, with good results (no Kconfig changes.)
>
> Pete and Matt, do you mind me applying the following portion of the
> patch to the kernel tree?
>
> > -#if !defined(CONFIG_BLK_DEV_UB) && !defined(CONFIG_BLK_DEV_UB_MODULE)
> > UNUSUAL_DEV( 0x0781, 0x0002, 0x0009, 0x0009,
> > "Sandisk",
> > "ImageMate SDDR-31",
> > US_SC_DEVICE, US_PR_DEVICE, NULL,
> > US_FL_IGNORE_SER ),
> > -#endif
Urgh. Please do. Code which may compile differently in the kernel image
according to which _modules_ are configured at the time is horrid, and
should be avoided.
--
dwmw2
On Mon, 24 Jan 2005 11:48:51 +0000, David Woodhouse <[email protected]> wrote:
> On Wed, 2005-01-19 at 14:07 -0800, Greg KH wrote:
> > I have been running with just the code portion of this patch for a while
> > now, with good results (no Kconfig changes.)
> >
> > Pete and Matt, do you mind me applying the following portion of the
> > patch to the kernel tree?
> >
> > > -#if !defined(CONFIG_BLK_DEV_UB) && !defined(CONFIG_BLK_DEV_UB_MODULE)
> > > UNUSUAL_DEV( 0x0781, 0x0002, 0x0009, 0x0009,
> > > "Sandisk",
> > > "ImageMate SDDR-31",
> > > US_SC_DEVICE, US_PR_DEVICE, NULL,
> > > US_FL_IGNORE_SER ),
> > > -#endif
>
> Urgh. Please do. Code which may compile differently in the kernel image
> according to which _modules_ are configured at the time is horrid, and
> should be avoided.
The fallacy of this "urgh" is easy to demonstrate when you consider usb-storage
and ub the one and the same driver. Initially, ub was just a mode for
usb-storage ("threadless"). I only factored them separate for reasons
of clarity. Horrid, indeed. There's no reason to build one statically
and another as a module.
Mind, I didn't disagree with the backout patch as such, but not because
it was a good idea, but because it may help to shut up a few stupid users
(provided that our scripts preserve the link order from drivers/usb/Makefile
in modules.usbmap, or have other way to make sure that usb-storage entries
are ahead of ub entires; did anyone actually check it? if those scripts
sort by name, ub still pops ahead, and the backout is utterly ineffectual).
When we reintroduce ub in Fedora, I'll just put this patch right back,
it's not a problem. But please think about this issue a little more,
you might want to take the Urgh back.
-- Pete