lkml.org 
[lkml]   [2005]   [Oct]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [usb-storage] Re: RFC drivers/usb/storage/libusual
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

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-10-07 16:43    [W:0.293 / U:0.008 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site