[systemd-devel] [usb-storage] Re: Amazon Kindle disconnect after Synchronize Cache

[Hans de Goede]

Hi,

On 3/16/21 5:26 PM, Alan Stern wrote:
> On Tue, Mar 16, 2021 at 06:26:30AM +0100, Matthias Schwarzott wrote:
>> I implemented solution 3b. This is the pullrequest for udev (systemd
>> repository):
>>
>> 	https://github.com/systemd/systemd/pull/19002
>>
>> Now Lennart asks if udev is the best place for such hacks/work-arounds?
>>
>> Well, I implemented it as suggested by Alan (see above). This was the
>> simplest of the considered alternatives. Different quirks in kernel has been
>> considered, but are more effort to be implemented.
> 
> Lennart probably isn't aware how the usb-storage driver works.  It does 
> not create commands on its own; it merely sends the commands that it 
> gets from higher SCSI layers.
> 
> It may be possible to modify the SCSI core, to make it send a TEST UNIT 
> READY command immediately following any SYNCHRONIZE CACHE to a Kindle.
> 
> However, there may be an easier solution.  usb-storage does indeed send 
> a command of its own, REQUEST SENSE, to get error data when a command 
> fails.  The patch below will make it do the same thing whenever it sends 
> a SYNCHRONIZE CACHE to a Kindle, failure or not.
> 
> The only question is whether the Kindle will regard REQUEST SENSE as a 
> sufficient indication that it shouldn't do an eject.  The only way to 
> find out is by testing the patch.
> 
> Alan Stern

Thank you for this patch, yes if this works it would IMHO be
a much better solution then the udev rule.

One question though, if this works to fix the undesired ejects,
will an actual eject (using e.g. the eject utility as say
"sudo eject /dev/sda") still be seen as an eject by the kindle
after this ?

Because that is actually kind of important for everyone using their
Kindle with Calibre, breaking that would not be good.

Regards,

Hans




> 
> 
> 
> Index: usb-devel/drivers/usb/storage/transport.c
> ===================================================================
> --- usb-devel.orig/drivers/usb/storage/transport.c
> +++ usb-devel/drivers/usb/storage/transport.c
> @@ -656,6 +656,13 @@ void usb_stor_invoke_transport(struct sc
>  		need_auto_sense = 1;
>  	}
>  
> +	/* Some devices (Kindle) require another command after SYNC CACHE */
> +	if (us->fflags & US_FL_CHECK_AFTER_SYNC &&
> +			srb->cmnd[0] == SYNCHRONIZE_CACHE) {
> +		usb_stor_dbg(us, "-- sense after SYNC CACHE\n");
> +		need_auto_sense = 1;
> +	}
> +
>  	/*
>  	 * If we have a failure, we're going to do a REQUEST_SENSE 
>  	 * automatically.  Note that we differentiate between a command
> Index: usb-devel/drivers/usb/storage/unusual_devs.h
> ===================================================================
> --- usb-devel.orig/drivers/usb/storage/unusual_devs.h
> +++ usb-devel/drivers/usb/storage/unusual_devs.h
> @@ -2212,6 +2212,18 @@ UNUSUAL_DEV( 0x1908, 0x3335, 0x0200, 0x0
>  		US_FL_NO_READ_DISC_INFO ),
>  
>  /*
> + * Reported by Matthias Schwarzott <zzam at gentoo.org>
> + * The Amazon Kindle treats SYNCHRONIZE CACHE as an indication that
> + * the host may be finished with it, and automatically ejects its
> + * media unless it receives another command within one second.
> + */
> +UNUSUAL_DEV( 0x1949, 0x0004, 0x0000, 0x9999,
> +		"Amazon",
> +		"Kindle",
> +		USB_SC_DEVICE, USB_PR_DEVICE, NULL,
> +		US_FL_CHECK_AFTER_SYNC ),
> +
> +/*
>   * Reported by Oliver Neukum <oneukum at suse.com>
>   * This device morphes spontaneously into another device if the access
>   * pattern of Windows isn't followed. Thus writable media would be dirty
> Index: usb-devel/include/linux/usb_usual.h
> ===================================================================
> --- usb-devel.orig/include/linux/usb_usual.h
> +++ usb-devel/include/linux/usb_usual.h
> @@ -86,6 +86,8 @@
>  		/* lies about caching, so always sync */	\
>  	US_FLAG(NO_SAME, 0x40000000)				\
>  		/* Cannot handle WRITE_SAME */			\
> +	US_FLAG(CHECK_AFTER_SYNC, 0x80000000)			\
> +		/* Check sense after SYNCHRONIZE_CACHE */	\
>  
>  #define US_FLAG(name, value)	US_FL_##name = value ,
>  enum { US_DO_ALL_FLAGS };
> _______________________________________________
> systemd-devel mailing list
> systemd-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/systemd-devel
>