On 10/30/23 18:40, Alan Stern wrote:
On Sat, Oct 28, 2023 at 07:41:45PM +0200, Milan Broz wrote:
The USB mass storage quirks flags can be stored in driver_info in
a 32-bit integer (unsigned long on 32-bit platforms).
As this attribute cannot be enlarged, we need to use some form
of translation of 64-bit quirk bits.
This problem was discussed on the USB list
https://lore.kernel.org/linux-usb/f9e8acb5-32d5-4a30-859f-d4336a86b31a@xxxxxxxxx/
The initial solution to use a static array extensively increased the size
of the kernel module, so I decided to try the second suggested solution:
generate a table by host-compiled program and use bit 31 to indicate
that the value is an index, not the actual value.
This patch adds a host-compiled program that processes unusual_devs.h
(and unusual_uas.h) and generates files usb-ids.c and usb-ids-uas.c
(for pre-processed USB device table with 32-bit device info).
These files also contain a generated translation table for driver_info
to 64-bit values.
The translation function is used only in usb-storage and uas modules; all
other USB storage modules store flags directly, using only 32-bit flags.
For 64-bit platforms, where unsigned long is 64-bit, we do not need to
convert quirk flags to 32-bit index; the translation function there uses
flags directly.
Signed-off-by: Milan Broz <gmazyland@xxxxxxxxx>
---
diff --git a/drivers/usb/storage/Makefile b/drivers/usb/storage/Makefile
index 46635fa4a340..b8c5daeb8ff3 100644
--- a/drivers/usb/storage/Makefile
+++ b/drivers/usb/storage/Makefile
@@ -45,3 +45,34 @@ ums-realtek-y := realtek_cr.o
ums-sddr09-y := sddr09.o
ums-sddr55-y := sddr55.o
ums-usbat-y := shuttle_usbat.o
+
+# The mkflags host-compiled generator produces usb-ids.c (usb-storage)
+# and usb-ids-uas.c (uas) with USB device tables.
+# These tables include pre-computed 32-bit values, as USB driver_info
+# (where the value is stored) can be only 32-bit.
+# The most significant bit means it is index to 64-bit pre-computed table
+# generated by mkflags host-compiled program.
+# Currently used only by mass-storage and uas driver.
+
+$(obj)/usual-tables.o: $(obj)/usb-ids.c
+$(obj)/uas.o: $(obj)/usb-ids-uas.c
Quick test: After those two lines were commented out from the Makefile,
the compiler still knew to rebuild unusual-tables.o and uas.o when
usb-ids.c and usb-ids-uas.c were changed. So these lines aren't needed.
ok, thx, this is perhaps relict of some previous try (I tried different
approaches) - will remove it.
Apart from this, I tried running the patch through checkpatch.pl, and it
flagged a bunch of issues. Some of them were false positives, but
others really should be changed to match the kernel's style:
The SPDX license line in .c files is supposed to be a
C++-style comment, i.e., use // instead of /* ... */.
Perhaps just copied from header file, it is different format there.
(And my bad, I forget to run checkpatch after many last changes...)
We aren't supposed to add new typedefs. Instead of writing:
typedef enum {...} dev_type;
write:
enum dev_type {...};
and the same for dev_flags_set.
checkpatch would like the FLAGS_END macro value to be enclosed
in parentheses, since it's a complex expression. Same for the
HI32 macro. These don't really matter, but you may as well do
it just to get rid of the warnings.
Quoted strings (line 117 in mkflags.c) aren't supposed to be
broken across lines. It should just be one very long line.
Contrariwise, some other lines are longer than checkpatch likes
to see (its maximum is 100 columns). They should be wrapped.
These issues ought to be fixed. But it's all stylistic stuff; I don't
see any actual errors or things to improve in the patch.
ok, I'll send next version once I have time to do it.
Thanks,
Milan
