Just a few technical comments on aspects of your patch:

On Fri, 7 Oct 2005, Pete Zaitcev wrote:

> diff -urpN -X dontdiff linux-2.6.14-rc2/drivers/usb/storage/usb.h 
> linux-2.6.14-rc2-wip/drivers/usb/storage/usb.h
> --- linux-2.6.14-rc2/drivers/usb/storage/usb.h        2005-09-24 
> 20:32:56.000000000 -0700
> +++ linux-2.6.14-rc2-wip/drivers/usb/storage/usb.h    2005-10-06 
> 21:37:10.000000000 -0700
...
> -/* Dynamic flag definitions: used in set_bit() etc. */
> -#define US_FLIDX_URB_ACTIVE  18  /* 0x00040000  current_urb is in use  */
> -#define US_FLIDX_SG_ACTIVE   19  /* 0x00080000  current_sg is in use   */
> -#define US_FLIDX_ABORTING    20  /* 0x00100000  abort is in progress   */
> -#define US_FLIDX_DISCONNECTING       21  /* 0x00200000  disconnect in 
> progress */
> -#define ABORTING_OR_DISCONNECTING    ((1UL << US_FLIDX_ABORTING) | \
> -                                      (1UL << US_FLIDX_DISCONNECTING))
> -#define US_FLIDX_RESETTING   22  /* 0x00400000  device reset in progress */
> -#define US_FLIDX_TIMED_OUT   23  /* 0x00800000  SCSI midlayer timed out  */
> -
> -
>  #define USB_STOR_STRING_LEN 32

I would prefer to keep these definitions in the usb-storage driver.  They 
refer to dynamic aspects of an individual device, not static blacklist or 
ID-matching for all devices of a particular type.  As such, they are of no 
interest to ub or libusual.


> diff -urpN -X dontdiff linux-2.6.14-rc2/include/linux/usb_usual.h 
> linux-2.6.14-rc2-wip/include/linux/usb_usual.h
> --- linux-2.6.14-rc2/include/linux/usb_usual.h        1969-12-31 
> 16:00:00.000000000 -0800
> +++ linux-2.6.14-rc2-wip/include/linux/usb_usual.h    2005-09-27 
> 18:05:09.000000000 -0700
> +/*
> + * The bias field for libusual and friends.
> + * Observe that usb-storage blatantly mixes set_bit() and normal
> + * shift and mask operations on flags, which is strictly illegal.
> + * And it probably even works for all flags except GO_SLOW and NO_WP_DETECT.

The two types of operations are used to write these flags at strictly
different times.  The shift/mask operations are used while initializing
the data structures and the set_bit-style operations are used after
everything has been set up and the device is operational.

Once the device is running, the shift/mask operations are used _only_ for
reading, never for writing.  This is true for all of the static flags, not
just GO_SLOW and NO_WP_DETECT.  There shouldn't be anything wrong or
illegal about it.  I don't see how it could possibly cause any kind of
error or problem.

It's true that two separate words could be used for the two kinds of flag
bits.  But since they coexist peacefully now, there doesn't seem to be any
need.

Alan Stern



-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to