Alan Stern wrote: > On Wed, 18 Apr 2012, Norman Diamond wrote: >> Alan Stern wrote: >>> On Tue, 17 Apr 2012, Norman Diamond wrote: >>>> Yes it does look like changing this bridge to CAPACITY_HEURISTICS from FIX_CAPACITY is likely to help a bit. >>> >>> That's easy enough. >> >> Yeah I know, even I could do that one ^_^ >> >>>> I think that heuristics should also accept multiples of 63 even if the total is odd. This doesn't solve everything but will probably help more. >>> >>> Okay, if nobody objects. >> >> I'm nobody and I don't object ^_^ >> (Of course we have to wait to see if anyone knows a real reason that this would be a problem.) >> >>>> I think that handling of FIX_CAPACITY should check for multiples of 63 and report the apparent likelihood that FIX_CAPACITY probably wasn't the right setting for the bridge (if it's a bridge). This doesn't solve everything but will probably help more. >>> >>> The FIX_CAPACITY code runs in a different module. It has no way to know whether the device is a bridge or not. >> >> The quirks table could be made available, but values stored in the table don't say if the device is a bridge. OK, then I still think this: I think that handling of FIX_CAPACITY should check for multiples of 63 and report the apparent likelihood that FIX_CAPACITY probably wasn't the right setting for the device. >> >> In addition, I have a feeling that if a device has FIX_CAPACITY but the number of reported blocks is even then I doubt the accuracy of FIX_CAPACITY. The reason is that flash memory devices tend to have physical sectors that are even multiples of logical sectors. > > FIX_CAPACITY applies to all kinds of devices, not just those using flash memory. There have been examples posted in the archives of devices which really do have an odd number of blocks. (I don't remember whether any of these devices needs the FIX_CAPACITY flag, though.) Actually I now recall jumpering a hard drive in or around 1998. When the drive reported its full capacity, the PC's BIOS hanged and would not boot. I jumpered the drive to report 15 heads instead of 16, reducing the capacity of the drive so that the PC's BIOS would boot. I don't recall the number of reported cylinders, but if that was also odd then the total number of reported blocks would be odd. If the drive were connected to a USB cable then jumpering would not be necessary. However, if the drive already contained data and we wanted to use a USB cable to retrieve the data, then the jumper would have to remain in place. I no longer have that drive, but I'm going to report a bug later in this message. >> I do think the handling of FIX_CAPACITY should report when devices report capacities that are either multiples of 63 or 2 (or both), so that further testing can be done. The existing code to do the decrement anyway can remain the same until further testing is done. >> >>>> The addition of TEST_CAPACITY doesn't solve everything but will probably help more. >>> >>> That will require a certain amount of work, with the possibility of breaking some systems. I'd rather not do it if there's no clear and pressing need. >> >> Two indistinguishable versions of the Prolific bridge show that there is a need. One version needs the decrement (probably) and the other version doesn't (definitely). > > No, the two indistinguishable versions show there is a need to change the existing FIX_CAPACITY flag to CAPACITY_HEURISTICS. OK, the new code for heuristics makes it PROBABLY good enough to change the Prolific bridge from FIX_CAPACITY to CAPACITY_HEURISTICS. > Below are two patches that do pretty much what you are asking for. Please test just the first and then both together. Ouch. Yeah I should expect it to be my responsibility to test, but rebuilding the live Linux system that I use is particularly painful. (Of course I have to be grateful that it is even possible in the first place, regardless of being painful.) Would it be acceptable for me to donate my USB-to-ATA cable to you? > + (sdp->guess_capacity && (n % 2 == 1 || n % 63 == 1))) { Imagine a device with 63 blocks. guess_capacity is true and n % 2 == 1 so this is going to truncate when we don't want to. How is one of these, either in my style: (sdp->guess_capacity && (n % 2 != 0) && (n % 63 != 0))) { or a popular coding style: (sdp->guess_capacity && n % 2 n % 63)) { > + sd_printk(KERN_INFO, sdkp, "(This adjustment may be incorrect)\n"); I'd like to suggest: "(This adjustment may be incorrect. If this adjustment truncates your drive when it should not, please report the error and your device's details to linux-usb@xxxxxxxxxxxxxxx)" -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html