Re: PATCH: usb-storage-psc1350-v4.patch (was Linux scsi / usb-mass-storage and HP printer cardreader bug + fix)
Guillaume Bedot wrote: I have tested this time with two PSC 1610 printers, and two SD cards, the same bug occured without the patch. And is fixed with your new patch. Good work ! Hi, Thanks for testing! But it fixes only two models. Do you think other devices (hp or not) can be impacted ? There are hundreds of models with card readers only for hp : http://hplip.sourceforge.net/supported_devices/combined.html Will this be possible to use "LAST_SECTOR_BUG" quirk for testing without recompiling a kernel ? This is not possible AFAIK, I've already wrote a blog post about this asking for people to test this, but got no responses. I think it would be good if the people from the hplip project would take some time to test all the printer models they have got access too. Regards, Hans - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: PATCH: usb-storage-psc1350-v4.patch (was Linux scsi / usb-mass-storage and HP printer cardreader bug + fix)
Hello, On ven, 2008-01-11 at 21:14 +0100, Hans de Goede wrote: > Boaz Harrosh wrote: > > Yes, you're right. in ULDs it is a much proper way to do this. > > > > So I guess you'll have to do that special host flag or device > > flag, and add a check for it in sd.c. You'll see that sd.c is > > already doing bufflen truncation at sd_prep_fn(), just add one > > more case. > > > > Done, thanks for the hint. Patch implementing my fix this way attached, > please > apply. > > Thanks & Regards, > > Hans > I have tested this time with two PSC 1610 printers, and two SD cards, the same bug occured without the patch. And is fixed with your new patch. Good work ! The bug however did not occur with a microSD card in a SD adaptator ?! But it fixes only two models. Do you think other devices (hp or not) can be impacted ? There are hundreds of models with card readers only for hp : http://hplip.sourceforge.net/supported_devices/combined.html Will this be possible to use "LAST_SECTOR_BUG" quirk for testing without recompiling a kernel ? Best regards, Guillaume B. - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/5] USB Kconfig: Reorganize USB Kconfig Menu
Stefan Richter wrote: > Greg KH wrote: > > On Sat, Jan 12, 2008 at 01:20:46PM +0300, Al Boldi wrote: > >> Greg KH wrote: > >>> On Sat, Jan 05, 2008 at 06:40:38PM +0300, Al Boldi wrote: > Reorganize USB Kconfig Menu, and move USB_GADGET out into the Device > Driver Menu. ?This helps the USB Kconfig Menu to be more > logical/usable. > > Patchset against 2.6.23 > >>> > >>> So what was the final verdict in this patch set? > >> > >> IMHO, it's a lot better than what we have right now, and it's split up > >> so that you can pick and choose what you think is useful. > >> > >>> Can you rsend this against 2.6.24-rc7 with the requested changes (if > >>> any) in it? > >> > >> The only critical change is in patch 2/5: > >> > >> menuconfig USB_STORAGE > >> tristate "USB Mass Storage support" > >> - depends on USB && SCSI > >> + depends on USB && BLOCK > >> + select SCSI > >> > >> > >> I was hoping you could take care of it, or maybe wait until 2.6.25 is > >> out. > > > > Well, if you want such a change to go into 2.6.25, I need the patches > > now :) > > > > So, can you respin these against 2.6.24-rc7, with the above fix, so that > > I can apply them and test them out? > > May I repeat my request that nobody does "select SCSI", please? > > Al Boldy, would you please explain like I asked you: > - what is wrong with the current solution which tells the user to > first enable SCSI to get the USB_STORAGE option, > - whether there are frequent end-user requests which demonstrate > that many people currently don't realize how to enable USB_STORAGE. > > Please use "select" carefully, particularly *only* for options which > enable simple compact library-like code which > - is small, > - doesn't itself depend on anything, > - doesn't has further configuration options in some other menu. > None of this applies to CONFIG_SCSI. I thought we discussed this before. We do it here, for the same reasons libata does it. In any case, the patch is optional. Thanks! -- Al - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: SD card reader problem
On 02/01/08 04:48, Matthew Dharm wrote: > On Mon, Dec 31, 2007 at 04:55:21PM -0500, Alan Stern wrote: > >> On Mon, 31 Dec 2007, Bruce Schultz wrote: >> >>> I have a USB SD card reader which has never worked properly and finally got >>> me >>> annoyed enough to try to do something about it. Using a 2.6.22 kernel >>> (gentoo), I get the following output in dmesg when I plug it in (with a >>> 128MB >>> SD card inserted): >>> >>> sd 8:0:0:0: [sdb] 248320 512-byte hardware sectors (127 MB) >>> sd 8:0:0:0: [sdb] Write Protect is off >>> sd 8:0:0:0: [sdb] Mode Sense: 03 00 00 00 >>> sd 8:0:0:0: [sdb] Assuming drive cache: write through >>> sd 8:0:0:0: [sdb] 248320 512-byte hardware sectors (127 MB) >>> sd 8:0:0:0: [sdb] Write Protect is off >>> sd 8:0:0:0: [sdb] Mode Sense: 03 00 00 00 >>> sd 8:0:0:0: [sdb] Assuming drive cache: write through >>> sdb: sdb1 >>> end_request: I/O error, dev sdb, sector 248313 >>> >> ... >> >>> After a bit of googling, I tried patching unusual_devs.h, which got me far >>> enough to be able to mount /dev/sdb1 and I seem to be able to read/write to >>> the SD card, but with errors in the dmesg output still. So I guess I'm on >>> the >>> right track, but maybe there's something else I'm missing? Any pointers out >>> there? >>> >> You need to do a low-level format of the card. Delete and then >> recreate the first partition. And then of course you'll have to >> recreate the VFAT filesystem on it. >> > That will work, but I don't think it's optimal. > Indeed, this does definitely work, but I agree that is not optimal to reformat every new SD card which gets used in it. At least it is mostly usable for its most typical purpose, namely copying photos off of friends' cameras, although I have found a card which refused to copy 1 file completely, I guess because part of the file was in the last sector. > Looking at the logs provided, I think this might be one of those devices > that doesn't like reading the last sector if it's part of a multi-sector > request. > I'll have a closer look into the recent patch I saw on the list for HP printer card readers, and report back if I have any success with that. Thanks for the input, Bruce - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: PATCH: usb-storage-psc1350-v4.patch (was Linux scsi / usb-mass-storage and HP printer cardreader bug + fix)
On Mon, Jan 14, 2008 at 10:46:56AM +0100, Hans de Goede wrote: > Guillaume Bedot wrote: > >But it fixes only two models. > >Do you think other devices (hp or not) can be impacted ? > >There are hundreds of models with card readers only for hp : > >http://hplip.sourceforge.net/supported_devices/combined.html > > > >Will this be possible to use "LAST_SECTOR_BUG" quirk for testing without > >recompiling a kernel ? > > > > This is not possible AFAIK, I've already wrote a blog post about this > asking for people to test this, but got no responses. Once the patches are accepted by the SCSI people, one of the things we can consider doing is enabling this quirk for all USB devices. It should be pretty harmless to all properly working devices, and the performance hit should be pretty minimal. We may be able to convince the SCSI people to enable it for all devices, regardless of HCD. Matt -- Matthew Dharm Home: [EMAIL PROTECTED] Maintainer, Linux USB Mass Storage Driver What the hell are you? -- Pitr to Dust Puppy User Friendly, 12/3/1997 pgpRWo6penKzA.pgp Description: PGP signature
Re: PATCH: usb-storage-psc1350-v4.patch (was Linux scsi / usb-mass-storage and HP printer cardreader bug + fix)
On Mon, 2008-01-14 at 08:03 -0800, Matthew Dharm wrote:
> On Mon, Jan 14, 2008 at 10:46:56AM +0100, Hans de Goede wrote:
> > Guillaume Bedot wrote:
> > >But it fixes only two models.
> > >Do you think other devices (hp or not) can be impacted ?
> > >There are hundreds of models with card readers only for hp :
> > >http://hplip.sourceforge.net/supported_devices/combined.html
> > >
> > >Will this be possible to use "LAST_SECTOR_BUG" quirk for testing without
> > >recompiling a kernel ?
> > >
> >
> > This is not possible AFAIK, I've already wrote a blog post about this
> > asking for people to test this, but got no responses.
>
> Once the patches are accepted by the SCSI people, one of the things we can
> consider doing is enabling this quirk for all USB devices. It should be
> pretty harmless to all properly working devices, and the performance hit
> should be pretty minimal.
The SCSI patches look OK, with the stylistic points fixed below ...
we'll need two separate patches as well (one for SCSI, one for USB).
> We may be able to convince the SCSI people to enable it for all devices,
> regardless of HCD.
No ... I'm not particularly keen to have enterprise vendors after my
blood ...
> + /* Some devices (some sdcards for one) don't like it if the last
> sector
> + gets read in a larger then 1 sector read */
The comment style in sd is
/*
* comment
*/
> + if (sdp->last_sector_bug && rq->nr_sectors > sdp->sector_size / 512 &&
An unlikely() here, please to force the compiler to optimise for the
non-buggy case. Plus what is the rq->nr_sectors > sdp->sector_size /
512 test supposed to be doing? that being true is supposed to be a
guarantee of the block layer (and if something goes wrong there's a
check for this lower down).
> + block + rq->nr_sectors == get_capacity(disk)) {
rq->nr_sectors should be this_count
> + this_count -= sdp->sector_size / 512;
If you relocate this code to after the sector_size/this_count adjustment
code (i.e. about line 442) you can just do --this_count;
James
-
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Frame rate
Thanks for the response David, but somehow I want to recover, I use the asynchronous, usb_submit_urb call and the call back never returns... any idea why that would happen, I have no visibility beyond my layer...! when would the host, think the device is dead? no of PINGS? what is the condition under which the host would think device ceases to exist... but I do see the IN tokens happening in my IN endpoints.. On 1/13/08, David Brownell <[EMAIL PROTECTED]> wrote: > > I am trying to send 64 byte packets from a linux host into a device at > > the rate of 4000 packets/second > > If that rate is critical, then it should be a periodic transfer ... > likely an "interrupt" transfer. > > > > it is a bulk endpoint > > i submit one URB per packet > > and the device is a modem, packet deliantion is critical > > Sounds like a badly designed modem. Notice how it won't work right > at full speed, since it can't possibly get that same rate. > > Also, consider that there are many millions of working USB modems > that have shipped *without* that oddball 4000 packet/sec requirement. > > - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] usb-serial: Sierra driver - add devices and update dtr
> Hm, no, the intrusion into the driver is just too much this late in
the
> release cycle to allow this.
>
> Now I will be glad to only add the new device ids for the devices that
> do not rely on the new changes right now, but that's it.
>
> So, right now I have a separate patch split out of your original one
> that is below. Should I modify it and not include some of these
device
> ids right now? You mention 0023, is that the only one I should remove
> from this patch?
Correct, the 0x0023 is the only newly added device that requires the new
features.
When do you expect the other changes will be propagated to the kernel?
Would it be in a 2.6.24.x point release or will they have to wait until
2.6.25?
Thanks,
-Kevin
-
From: Kevin Lloyd <[EMAIL PROTECTED]>
Subject: USB: sierra driver - add devices
From: Kevin Lloyd <[EMAIL PROTECTED]>
The following improvements were made:
- Added new product support: MC5725, AC 880 U, MP 3G (UMTS & CDMA)
Signed-off-by: Kevin Lloyd <[EMAIL PROTECTED]>
Signed-off-by: Greg Kroah-Hartman <[EMAIL PROTECTED]>
---
drivers/usb/serial/sierra.c |9 +
1 file changed, 9 insertions(+)
--- a/drivers/usb/serial/sierra.c
+++ b/drivers/usb/serial/sierra.c
@@ -104,6 +104,7 @@ static struct usb_device_id id_table []
{ USB_DEVICE(0x1199, 0x0019) }, /* Sierra Wireless AirCard 595
*/
{ USB_DEVICE(0x1199, 0x0021) }, /* Sierra Wireless AirCard 597E
*/
{ USB_DEVICE(0x1199, 0x0120) }, /* Sierra Wireless USB Dongle
595U */
+ { USB_DEVICE(0x1199, 0x0023) }, /* Sierra Wireless AirCard */
{ USB_DEVICE(0x1199, 0x6802) }, /* Sierra Wireless MC8755 */
{ USB_DEVICE(0x1199, 0x6804) }, /* Sierra Wireless MC8755 */
@@ -117,8 +118,12 @@ static struct usb_device_id id_table []
{ USB_DEVICE(0x1199, 0x6851) }, /* Sierra Wireless AirCard 881
*/
{ USB_DEVICE(0x1199, 0x6852) }, /* Sierra Wireless AirCard 880 E
*/
{ USB_DEVICE(0x1199, 0x6853) }, /* Sierra Wireless AirCard 881 E
*/
+ { USB_DEVICE(0x1199, 0x6855) }, /* Sierra Wireless AirCard 880 U
*/
{ USB_DEVICE(0x1199, 0x6856) }, /* Sierra Wireless AirCard 881 U
*/
+ { USB_DEVICE(0x1199, 0x6468) }, /* Sierra Wireless MP3G - EVDO
*/
+ { USB_DEVICE(0x1199, 0x6469) }, /* Sierra Wireless MP3G -
UMTS/HSPA */
+
{ USB_DEVICE(0x1199, 0x0112), .driver_info = DEVICE_1_PORT }, /*
Sierra Wireless AirCard 580 */
{ USB_DEVICE(0x0F3D, 0x0112), .driver_info = DEVICE_1_PORT }, /*
Airprime/Sierra PC 5220 */
@@ -143,6 +148,7 @@ static struct usb_device_id id_table_3po
{ USB_DEVICE(0x1199, 0x0019) }, /* Sierra Wireless AirCard 595
*/
{ USB_DEVICE(0x1199, 0x0021) }, /* Sierra Wireless AirCard 597E
*/
{ USB_DEVICE(0x1199, 0x0120) }, /* Sierra Wireless USB Dongle
595U*/
+ { USB_DEVICE(0x1199, 0x0023) }, /* Sierra Wireless AirCard */
{ USB_DEVICE(0x1199, 0x6802) }, /* Sierra Wireless MC8755 */
{ USB_DEVICE(0x1199, 0x6804) }, /* Sierra Wireless MC8755 */
@@ -156,7 +162,10 @@ static struct usb_device_id id_table_3po
{ USB_DEVICE(0x1199, 0x6851) }, /* Sierra Wireless AirCard 881
*/
{ USB_DEVICE(0x1199, 0x6852) }, /* Sierra Wireless AirCard 880E
*/
{ USB_DEVICE(0x1199, 0x6853) }, /* Sierra Wireless AirCard 881E
*/
+ { USB_DEVICE(0x1199, 0x6855) }, /* Sierra Wireless AirCard 880 U
*/
{ USB_DEVICE(0x1199, 0x6856) }, /* Sierra Wireless AirCard 881U
*/
+ { USB_DEVICE(0x1199, 0x6468) }, /* Sierra Wireless MP3G - EVDO
*/
+ { USB_DEVICE(0x1199, 0x6469) }, /* Sierra Wireless MP3G -
UMTS/HSPA */
{ }
};
-
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/5] USB Kconfig: Reorganize USB Kconfig Menu
On Mon, Jan 14, 2008 at 01:21:51PM +0300, Al Boldi wrote: > Stefan Richter wrote: > > Greg KH wrote: > > > On Sat, Jan 12, 2008 at 01:20:46PM +0300, Al Boldi wrote: > > >> Greg KH wrote: > > >>> On Sat, Jan 05, 2008 at 06:40:38PM +0300, Al Boldi wrote: > > Reorganize USB Kconfig Menu, and move USB_GADGET out into the Device > > Driver Menu. ?This helps the USB Kconfig Menu to be more > > logical/usable. > > > > Patchset against 2.6.23 > > >>> > > >>> So what was the final verdict in this patch set? > > >> > > >> IMHO, it's a lot better than what we have right now, and it's split up > > >> so that you can pick and choose what you think is useful. > > >> > > >>> Can you rsend this against 2.6.24-rc7 with the requested changes (if > > >>> any) in it? > > >> > > >> The only critical change is in patch 2/5: > > >> > > >> menuconfig USB_STORAGE > > >> tristate "USB Mass Storage support" > > >> - depends on USB && SCSI > > >> + depends on USB && BLOCK > > >> + select SCSI > > >> > > >> > > >> I was hoping you could take care of it, or maybe wait until 2.6.25 is > > >> out. > > > > > > Well, if you want such a change to go into 2.6.25, I need the patches > > > now :) > > > > > > So, can you respin these against 2.6.24-rc7, with the above fix, so that > > > I can apply them and test them out? > > > > May I repeat my request that nobody does "select SCSI", please? > > > > Al Boldy, would you please explain like I asked you: > > - what is wrong with the current solution which tells the user to > > first enable SCSI to get the USB_STORAGE option, > > - whether there are frequent end-user requests which demonstrate > > that many people currently don't realize how to enable USB_STORAGE. > > > > Please use "select" carefully, particularly *only* for options which > > enable simple compact library-like code which > > - is small, > > - doesn't itself depend on anything, > > - doesn't has further configuration options in some other menu. > > None of this applies to CONFIG_SCSI. > > I thought we discussed this before. We do it here, for the same reasons > libata does it. In any case, the patch is optional. Yes, I do not want to do this, as we went round and round on it in the past many times over the years. It should be left as-is. thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: PATCH: usb-storage-psc1350-v4.patch (was Linux scsi / usb-mass-storage and HP printer cardreader bug + fix)
James Bottomley wrote:
On Mon, 2008-01-14 at 08:03 -0800, Matthew Dharm wrote:
On Mon, Jan 14, 2008 at 10:46:56AM +0100, Hans de Goede wrote:
Guillaume Bedot wrote:
But it fixes only two models.
Do you think other devices (hp or not) can be impacted ?
There are hundreds of models with card readers only for hp :
http://hplip.sourceforge.net/supported_devices/combined.html
Will this be possible to use "LAST_SECTOR_BUG" quirk for testing without
recompiling a kernel ?
This is not possible AFAIK, I've already wrote a blog post about this
asking for people to test this, but got no responses.
Once the patches are accepted by the SCSI people, one of the things we can
consider doing is enabling this quirk for all USB devices. It should be
pretty harmless to all properly working devices, and the performance hit
should be pretty minimal.
The SCSI patches look OK, with the stylistic points fixed below ...
we'll need two separate patches as well (one for SCSI, one for USB).
Okay,
Thanks for the review! I'll do a new scsi changes only patch once I get an
answer to some questions regarding your review.
+ /* Some devices (some sdcards for one) don't like it if the last sector
+ gets read in a larger then 1 sector read */
The comment style in sd is
/*
* comment
*/
Will fix,
+ if (sdp->last_sector_bug && rq->nr_sectors > sdp->sector_size / 512 &&
An unlikely() here, please to force the compiler to optimise for the
non-buggy case.
Will do.
Plus what is the rq->nr_sectors > sdp->sector_size /
512 test supposed to be doing? that being true is supposed to be a
guarantee of the block layer (and if something goes wrong there's a
check for this lower down).
It first is was just:
rq->nr_sectors > 1
Then I changed it to also do the right thing for 1024 and larger sector disks.
The whole purpose is to not read the last sector in a larger then one sector
read, so the amount of sectors gets reduced by one if (block + rq->nr_sectors
== get_capacity(disk)) but we do want still want to be able to read the last
sector by itself, so we must not reduce the no sectors to be read by one if it
is already one.
+ block + rq->nr_sectors == get_capacity(disk)) {
rq->nr_sectors should be this_count
Fine by me, but in this place in the code they are the same value, and the
check for a read beyond the end of disk a few lines above also uses
rq->nr_sectors, which one should be used when?
+ this_count -= sdp->sector_size / 512;
If you relocate this code to after the sector_size/this_count adjustment
code (i.e. about line 442) you can just do --this_count;
I cannot move the check down as then block has been adjusted for the sector
size, so if I move the check down it becomes:
if (block * (sdp->sector_size / 512) + rq->nr_sectors == get_capacity(disk))
I would rather not have the sdp->sector_size / 512 code in the check (slow) but
rather in the not often entered if block.
Regards,
Hans
-
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: PATCH: usb-storage-psc1350-v4.patch (was Linux scsi / usb-mass-storage and HP printer cardreader bug + fix)
James Bottomley wrote: > On Mon, 2008-01-14 at 08:03 -0800, Matthew Dharm wrote: >> On Mon, Jan 14, 2008 at 10:46:56AM +0100, Hans de Goede wrote: >> > Guillaume Bedot wrote: >> > >Will this be possible to use "LAST_SECTOR_BUG" quirk for testing without >> > >recompiling a kernel ? >> > >> > This is not possible AFAIK, I've already wrote a blog post about this >> > asking for people to test this, but got no responses. >> >> Once the patches are accepted by the SCSI people, one of the things we can >> consider doing is enabling this quirk for all USB devices. ... >> We may be able to convince the SCSI people to enable it for all devices, >> regardless of HCD. > > No ... I'm not particularly keen to have enterprise vendors after my > blood ... Guillaume, you can tell the SCSI core driver at boot time (or module insertion time) and/or at runtime to - switch on default quirk flags, - add quirk flags for selected devices per name matching. Alas I don't know of a good documention how to do either of this, and I am not familiar enough with the procedure to explain it here. -- Stefan Richter -=-==--- ---= -===- http://arcgraph.de/sr/ - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: PATCH: usb-storage-psc1350-v4.patch (was Linux scsi / usb-mass-storage and HP printer cardreader bug + fix)
On Mon, Jan 14, 2008 at 10:33:08AM -0600, James Bottomley wrote: > > On Mon, 2008-01-14 at 08:03 -0800, Matthew Dharm wrote: > > We may be able to convince the SCSI people to enable it for all devices, > > regardless of HCD. > > No ... I'm not particularly keen to have enterprise vendors after my > blood ... Fair enough. Tho, we should probably just enable this blindly for all usb-storage devices. Otherwise, this is just going to become an unusual_devs.h nightmare. Matt -- Matthew Dharm Home: [EMAIL PROTECTED] Maintainer, Linux USB Mass Storage Driver What, are you one of those Microsoft-bashing Linux freaks? -- Customer to Greg User Friendly, 2/10/1999 pgps8kgiJqubH.pgp Description: PGP signature
Re: PATCH: usb-storage-psc1350-v4.patch (was Linux scsi / usb-mass-storage and HP printer cardreader bug + fix)
On Mon, 2008-01-14 at 19:37 +0100, Hans de Goede wrote:
> James Bottomley wrote:
> > On Mon, 2008-01-14 at 08:03 -0800, Matthew Dharm wrote:
> >> On Mon, Jan 14, 2008 at 10:46:56AM +0100, Hans de Goede wrote:
> >>> Guillaume Bedot wrote:
> But it fixes only two models.
> Do you think other devices (hp or not) can be impacted ?
> There are hundreds of models with card readers only for hp :
> http://hplip.sourceforge.net/supported_devices/combined.html
>
> Will this be possible to use "LAST_SECTOR_BUG" quirk for testing without
> recompiling a kernel ?
>
> >>> This is not possible AFAIK, I've already wrote a blog post about this
> >>> asking for people to test this, but got no responses.
> >> Once the patches are accepted by the SCSI people, one of the things we can
> >> consider doing is enabling this quirk for all USB devices. It should be
> >> pretty harmless to all properly working devices, and the performance hit
> >> should be pretty minimal.
> >
> > The SCSI patches look OK, with the stylistic points fixed below ...
> > we'll need two separate patches as well (one for SCSI, one for USB).
> >
>
> Okay,
>
> Thanks for the review! I'll do a new scsi changes only patch once I get an
> answer to some questions regarding your review.
>
> >> + /* Some devices (some sdcards for one) don't like it if the last
> >> sector
> >> + gets read in a larger then 1 sector read */
> >
> > The comment style in sd is
> >
> > /*
> > * comment
> > */
> >
>
> Will fix,
>
> >> + if (sdp->last_sector_bug && rq->nr_sectors > sdp->sector_size /
> >> 512 &&
> >
> > An unlikely() here, please to force the compiler to optimise for the
> > non-buggy case.
>
> Will do.
>
> > Plus what is the rq->nr_sectors > sdp->sector_size /
> > 512 test supposed to be doing? that being true is supposed to be a
> > guarantee of the block layer (and if something goes wrong there's a
> > check for this lower down).
> >
>
> It first is was just:
> rq->nr_sectors > 1
>
> Then I changed it to also do the right thing for 1024 and larger sector
> disks.
> The whole purpose is to not read the last sector in a larger then one sector
> read, so the amount of sectors gets reduced by one if (block + rq->nr_sectors
> == get_capacity(disk)) but we do want still want to be able to read the last
> sector by itself, so we must not reduce the no sectors to be read by one if
> it
> is already one.
Yes, I know that. What I mean is the block subsystem sends reads and
writes down in increments of the sector size. Checking if the current
number of pending sectors is greater than the current block size is
checking that guarantee. The current code already has a check in it to
see if this guarantee is observed ... you don't need to check it again.
> >> + block + rq->nr_sectors == get_capacity(disk)) {
> >
> > rq->nr_sectors should be this_count
> >
>
> Fine by me, but in this place in the code they are the same value, and the
> check for a read beyond the end of disk a few lines above also uses
> rq->nr_sectors, which one should be used when?
this_count is the adjusted sector size. At the moment, there's no size
transformation in the code between your check and the top (where
this_count is set to rq->nr_sectors). But if there were (and someone
might add one one day) you'd be wanting to check the adjusted size (i.e.
this_count).
> >> + this_count -= sdp->sector_size / 512;
> >
> > If you relocate this code to after the sector_size/this_count adjustment
> > code (i.e. about line 442) you can just do --this_count;
> >
>
> I cannot move the check down as then block has been adjusted for the sector
> size, so if I move the check down it becomes:
> if (block * (sdp->sector_size / 512) + rq->nr_sectors == get_capacity(disk))
>
> I would rather not have the sdp->sector_size / 512 code in the check (slow)
> but
> rather in the not often entered if block.
OK, point taken, it would be worse. I think today's compilers are happy
to translate x/512 to x>>9, so it shouldn't be that slow.
James
-
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: PATCH: usb-storage-psc1350-v4.patch (was Linux scsi / usb-mass-storage and HP printer cardreader bug + fix)
Matthew Dharm wrote: On Mon, Jan 14, 2008 at 10:33:08AM -0600, James Bottomley wrote: On Mon, 2008-01-14 at 08:03 -0800, Matthew Dharm wrote: We may be able to convince the SCSI people to enable it for all devices, regardless of HCD. No ... I'm not particularly keen to have enterprise vendors after my blood ... Fair enough. Tho, we should probably just enable this blindly for all usb-storage devices. Otherwise, this is just going to become an unusual_devs.h nightmare. Since this will make all devices with this bug "just work" (tm), I'm all for it! As soon as I get an answer from the scsi people on my questions resulting from there review I'll do a new patch for the scsi layer for this. Regards, Hans - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: PATCH: usb-storage-psc1350-v4.patch (was Linux scsi / usb-mass-storage and HP printer cardreader bug + fix)
James Bottomley wrote: Plus what is the rq->nr_sectors > sdp->sector_size / 512 test supposed to be doing? that being true is supposed to be a guarantee of the block layer (and if something goes wrong there's a check for this lower down). It first is was just: rq->nr_sectors > 1 Then I changed it to also do the right thing for 1024 and larger sector disks. The whole purpose is to not read the last sector in a larger then one sector read, so the amount of sectors gets reduced by one if (block + rq->nr_sectors == get_capacity(disk)) but we do want still want to be able to read the last sector by itself, so we must not reduce the no sectors to be read by one if it is already one. Yes, I know that. What I mean is the block subsystem sends reads and writes down in increments of the sector size. Checking if the current number of pending sectors is greater than the current block size is checking that guarantee. The current code already has a check in it to see if this guarantee is observed ... you don't need to check it again. I'm not checking for the guarantee, I'm checking that the read > 1 realsize_sector, before decrementing the number of _512_bytes_sectors by realsize_sector, this check must be there, as after reading all but the last sector, another request will be send with 1 realsize_sector size, for which (block + rq->nr_sectors) == get_capacity(disk) will still hold true, in this case this_count must not be lowered, otherwise this_count will become 0 in this case and the last sector will never get read. I hope that clarifies why that code is there, if not can you tell how you would want the code to look by just giving the full if statement as it should be, I think we are somehow misunderstanding each other. Regards, Hans - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/5] USB Kconfig: Reorganize USB Kconfig Menu
Greg KH wrote: > On Mon, Jan 14, 2008 at 01:21:51PM +0300, Al Boldi wrote: >> Stefan Richter wrote: >>> would you please explain like I asked you: >>> - what is wrong with the current solution which tells the user to >>> first enable SCSI to get the USB_STORAGE option, >>> - whether there are frequent end-user requests which demonstrate >>> that many people currently don't realize how to enable USB_STORAGE. ... >> I thought we discussed this before. We do it here, for the same reasons >> libata does it. In any case, the patch is optional. Al, I admit I haven't read everything of the discussion. So, sorry if I got on your nerves. (However, http://lkml.org/lkml/2008/1/5/151 and http://lkml.org/lkml/2008/1/5/209 didn't yield responses which addressed the mentioned points. Also, "libata's reasons for doing so apply to usb-storage as well" is not a fact-based answer to these points either. Anyway, sorry if I missed an according response elsewhere.) > Yes, I do not want to do this, as we went round and round on it in the > past many times over the years. It should be left as-is. I'm relieved. :-) -- Stefan Richter -=-==--- ---= -===- http://arcgraph.de/sr/ - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: PATCH: usb-storage-psc1350-v4.patch (was Linux scsi / usb-mass-storage and HP printer cardreader bug + fix)
On Mon, 2008-01-14 at 20:27 +0100, Hans de Goede wrote: > James Bottomley wrote: > >>> Plus what is the rq->nr_sectors > sdp->sector_size / > >>> 512 test supposed to be doing? that being true is supposed to be a > >>> guarantee of the block layer (and if something goes wrong there's a > >>> check for this lower down). > >>> > >> It first is was just: > >> rq->nr_sectors > 1 > >> > >> Then I changed it to also do the right thing for 1024 and larger sector > >> disks. > >> The whole purpose is to not read the last sector in a larger then one > >> sector > >> read, so the amount of sectors gets reduced by one if (block + > >> rq->nr_sectors > >> == get_capacity(disk)) but we do want still want to be able to read the > >> last > >> sector by itself, so we must not reduce the no sectors to be read by one > >> if it > >> is already one. > > > > Yes, I know that. What I mean is the block subsystem sends reads and > > writes down in increments of the sector size. Checking if the current > > number of pending sectors is greater than the current block size is > > checking that guarantee. The current code already has a check in it to > > see if this guarantee is observed ... you don't need to check it again. > > > > I'm not checking for the guarantee, I'm checking that the read > 1 > realsize_sector, before decrementing the number of _512_bytes_sectors by > realsize_sector, this check must be there, as after reading all but the last > sector, another request will be send with 1 realsize_sector size, for which > (block + rq->nr_sectors) == get_capacity(disk) will still hold true, in this > case this_count must not be lowered, otherwise this_count will become 0 in > this > case and the last sector will never get read. > > I hope that clarifies why that code is there, if not can you tell how you > would > want the code to look by just giving the full if statement as it should be, I > think we are somehow misunderstanding each other. Oh, right; it's > rather than >= ... sorry, yes that's fine. James - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Frame rate
On Monday 14 January 2008, Vijay Ramamurthi wrote: > Thanks for the response David, > but somehow I want to recover, I use the asynchronous, usb_submit_urb call and > the call back never returns... > > any idea why that would happen, I have no visibility beyond my layer...! > > when would the host, think the device is dead? I don't seem to recall such a state in the USB spec... There are disconnect states; and there are situations where the device stops behaving according to the spec. But there's no pushing-up-daisies state there, and hence none in the Linux-USB stack. > no of PINGS? what is > the condition under which the host would think device ceases to > exist... > > but I do see the IN tokens happening in my IN endpoints.. If the IN tokens are happening, then and your device is NAKing them, it's hardly dead. > > > On 1/13/08, David Brownell <[EMAIL PROTECTED]> wrote: > > > I am trying to send 64 byte packets from a linux host into a device at > > > the rate of 4000 packets/second > > > > If that rate is critical, then it should be a periodic transfer ... > > likely an "interrupt" transfer. > > > > > > > it is a bulk endpoint > > > i submit one URB per packet > > > and the device is a modem, packet deliantion is critical > > > > Sounds like a badly designed modem. Notice how it won't work right > > at full speed, since it can't possibly get that same rate. > > > > Also, consider that there are many millions of working USB modems > > that have shipped *without* that oddball 4000 packet/sec requirement. > > > > > - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: Problem with NEC PCI USB 720101 (linux 2.6.23.8)
I should mention one additional item - the kozio diagnostics we run in manufacturing ** CAN ** write/read a block from an external thumb drive... -Original Message- From: Morrison, Tom Sent: Monday, January 14, 2008 6:19 PM To: [EMAIL PROTECTED] Cc: linux-usb@vger.kernel.org Subject: Problem with NEC PCI USB 720101 (linux 2.6.23.8) - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] PS3: Fix EHCI ISO transfer bug
On 01/13/2008 09:36 AM, David Brownell wrote: > Yes, please. The appended patch is that updated version. > > === CUT HERE > From: David Brownell <[EMAIL PROTECTED]> > > This adds a workaround for an issue reported with ISO transfers > on some EHCI controllers, most recently with VIA KT800 and PS3 > EHCI silicon. > > The issue is that the silicon doesn't necessarily seem to be done > using ISO DMA descriptors (itd, sitd) when it marks them inactive. > (One theory is that the ill-defined mechanism where hardware caches > periodic transfer descriptors isn't invalidating their state...) > With such silicon, quick re-use of those descriptors makes trouble. > Waiting until the next frame seems to be a sufficient workaround. > > This patch ensures that the relevant descriptors aren't available > for immediate re-use. It does so by not recycling them until after > issuing the completion callback which would reuse them by enqueueing > an URB and thus (re)allocating ISO DMA descriptors. > > Signed-off-by: David Brownell <[EMAIL PROTECTED]> I tested this on PS3, and it seems to work OK. Acked-by: Geoff Levand <[EMAIL PROTECTED]> - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
aiding text editing + RFC: ti_usb-serial: userspace firmware, internal cfg. change, cleanup
@ Wed, Nov 14, 2007 at 08:10:51PM +0100, olecom: > == Wed, Nov 14, 2007 at 05:48:01PM +, Alan Cox == > > > > Please run this through scripts/checkpatch.pl first and fix all of those > > > > warnings. > > > > > > How nice, you have time to write this to me. I wonder how frequently you > > > did same for last, say, 9 years with your USB writings. > > > > We didn't have checkpatch nine years ago, and I've been fixing up a ton > > of USB serial drivers as a result. Please use checkpatch is the right > > thing to say - its what Andrew just told me to go do as well and he's > > right. > > Alan. I know that. And i appreciate style fixes. But if tools are > there for quite some time, and sources still have trivial whitespace > damage, this is not serious. > > Going my usual flame-way, i'd like to say, that unless cause will be > cured, any dealing with consequences will be a waste of time. As history > and current state shows, making usable syntax highlighting and general, > as well as particular, aid for text editors is unsolvable problem. Even > for such simple thing as C. Those CS professors still teaching compilers, > without f*cking trivial application of the parsers to produce (at least) > comprehensive, checking, analyzing, aiding syntax highlighting, not > saying about other simple text processing. MUA problems with Linus > looking on (broken) text via `od` are not funny. It's just complete joke. Actually, there's something academic GUI based for Java. http://www.cs.cmu.edu/~ajko/papers/Ko2006Barista.pdf = Bonus. = > == > > Driver. Even if official maintainer, who wanted 5 blobs in addition to > two, did say something 9 month ago, i see no progress. The only one is no > such driver in Debian. It's not a religion, it's just a style; managing > and programming style. Managing, because any ``userspace'' problem is > IMHO hand-waving. Support is Cc'ed and payed for making information > available. I've choose neutral meaningful filenames, that can be very > easily symlinked or copied ``for users''. Having and extending driver for > per-device blobs is not general usecase, thus there's no driver update so > far. If `udevd --daemon` or `udev --notdaemon` with sysfs are in trouble, > then it's not my problem. A new TIUSB based EZ430-RF2500 device[0] is turned to be a HID, device for pretty much same task -- programmer/debugger interface for TI MSP430 controllers. It now has a back-link, which is a serial link to the uC under development. [0] http://focus.ti.com/graphics/tool/eZ430-rf2500_500x418.jpg `-http://focus.ti.com/docs/toolsw/folders/print/ez430-rf2500.html IMHO this one hits problems, i was trying to discuss last year about Linux USB "drivers". Making main tool's interface different from the serial, which now is an optional back-link, have problems. One actual device (ti usb 3410) by loading different firmware, can become quite different end user usb one. And it has serial link in addition to the main interface. It's obviously for me, that this was made for users to do not mix all-in-one serial interfaces and troubling TI support. But it also shows clearly, that driver must be split into parts: * a real usb driver part (attaching, fw loading) * usb interface part (serial, HID+serial, etc.) > == > > The very small and cute usb-based development tool for MSP430 controllers > (very low power and smart clocks, qualified mixed-signal processing and > good CPU speed) -- most user-(i.e. embedded developer)-friendly device, is > f*cked up by programmers. With such `official' driver, most people can > not deal with udev crap easily, and more importantly with actual hardware > study and work. And after making things basically working one cannot go > further inside same *small* device -- to hack usb interface itself > *easily*, by just making another firmware file under same symlink in > '/lib/firmware'. > > If most of the LKML crowd are happy to hack off on every new -rc, it > doesn't mean every skilled programmer must know how to make that fine > driver in linux work, to patch, to build, to configure. > > But if one going to hack this driver, there will be a big surprise. > 30k of 99% of time useless bloat in the fine kernel, not saying about > obvious efficient programming. > __ - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] PS3: Fix EHCI ISO transfer bug
On Monday 14 January 2008, Geoff Levand wrote: > On 01/13/2008 09:36 AM, David Brownell wrote: > > Yes, please. The appended patch is that updated version. > > > > === CUT HERE > > From: David Brownell <[EMAIL PROTECTED]> > > > > This adds a workaround for an issue reported with ISO transfers > > on some EHCI controllers, most recently with VIA KT800 and PS3 > > EHCI silicon. > > > > The issue is that the silicon doesn't necessarily seem to be done > > using ISO DMA descriptors (itd, sitd) when it marks them inactive. > > ... > > I tested this on PS3, and it seems to work OK. > > Acked-by: Geoff Levand <[EMAIL PROTECTED]> Great!! Thanks for testing, Geoff. - Dave - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
