Signed-off-by: Milan Broz <gmazyland@xxxxxxxxx>
---
drivers/usb/storage/Makefile | 25 ++++
drivers/usb/storage/mkflags.c | 212 +++++++++++++++++++++++++++++
drivers/usb/storage/uas-detect.h | 2 +-
drivers/usb/storage/uas.c | 17 +--
drivers/usb/storage/usb.c | 7 +-
drivers/usb/storage/usual-tables.c | 23 +---
6 files changed, 248 insertions(+), 38 deletions(-)
create mode 100644 drivers/usb/storage/mkflags.c
diff --git a/drivers/usb/storage/Makefile b/drivers/usb/storage/Makefile
index 46635fa4a340..1eacdbb387cd 100644
--- a/drivers/usb/storage/Makefile
+++ b/drivers/usb/storage/Makefile
@@ -45,3 +45,28 @@ ums-realtek-y := realtek_cr.o
ums-sddr09-y := sddr09.o
ums-sddr55-y := sddr55.o
ums-usbat-y := shuttle_usbat.o
+
Suggestion: Add a comment here, explaining what the following code does
and why it is necessary.
+$(obj)/usb.o: $(obj)/unusual-flags.h
+$(obj)/usual-tables.o: $(obj)/unusual-flags.c
+$(obj)/uas.o: $(obj)/unusual-flags.h $(obj)/unusual-flags-uas.c
+clean-files := unusual-flags.h unusual-flags.c unusual-flags-uas.c
+HOSTCFLAGS_mkflags.o := -I $(srctree)/include/
+hostprogs += mkflags
+
+quiet_cmd_mkflag_flags = FLAGS $@
+ cmd_mkflag_flags = $(obj)/mkflags flags > $@
+
+quiet_cmd_mkflag_storage = FLAGS $@
+ cmd_mkflag_storage = $(obj)/mkflags storage > $@
+
+quiet_cmd_mkflag_uas = FLAGS $@
+ cmd_mkflag_uas = $(obj)/mkflags uas > $@
+
+$(obj)/unusual-flags.h: $(obj)/mkflags FORCE
+ $(call if_changed,mkflag_flags)
+
+$(obj)/unusual-flags.c: $(obj)/mkflags FORCE
+ $(call if_changed,mkflag_storage)
+
+$(obj)/unusual-flags-uas.c: $(obj)/mkflags FORCE
+ $(call if_changed,mkflag_uas)
My make-fu isn't so hot. Do you really need to use this indirect way of
specifying whether and how to rebuild the new files?
diff --git a/drivers/usb/storage/mkflags.c b/drivers/usb/storage/mkflags.c
new file mode 100644
index 000000000000..11aa6579e7e1
--- /dev/null
+++ b/drivers/usb/storage/mkflags.c
@@ -0,0 +1,212 @@
+// SPDX-License-Identifier: GPL-2.0+
There needs to be a big comment here, explaining why this program is
needed and exactly what it does.
+
+#include <stdio.h>
+#include <string.h>
+
+/*
+ * Cannot use userspace <inttypes.h> as code below
+ * processes internal kernel headers
+ */
+#include <linux/types.h>
+
+/*
+ * Silence warning for definitions in headers we do not use
+ */
+struct usb_device_id {};
+struct usb_interface;
+
+#include <linux/usb_usual.h>
+
+struct svals {
+ unsigned int type;
+
+ /*interface */
+ uint8_t bDeviceSubClass;
+ uint8_t bDeviceProtocol;
+
+ /* device */
+ uint16_t idVendor;
+ uint16_t idProduct;
+ uint16_t bcdDevice_lo;
+ uint16_t bcdDevice_hi;
+
+ uint64_t flags;
+ unsigned int set;
+ unsigned int idx;
+};
+
+enum { TYPE_DEVICE_STORAGE, TYPE_DEVICE_UAS, TYPE_CLASS };
+enum { FLAGS_NOT_SET, FLAGS_SET, FLAGS_DUPLICATE };
+#define FLAGS_END (uint64_t)-1
+
+#define IS_ENABLED(x) 0
+
+static struct svals vals[] = {
+#define USUAL_DEV(useProto, useTrans) \
+{ TYPE_CLASS, useProto, useTrans, 0, 0, 0, 0, 0, FLAGS_NOT_SET, 0 }
+
+#define COMPLIANT_DEV UNUSUAL_DEV
+#define UNUSUAL_DEV(id_vendor, id_product, bcdDeviceMin, bcdDeviceMax, \
+ vendorName, productName, useProtocol, useTransport, \
+ initFunction, flags) \
+{ TYPE_DEVICE_STORAGE, 0, 0, id_vendor, id_product, bcdDeviceMin, bcdDeviceMax, flags, FLAGS_NOT_SET, 0 }
+
+#include "unusual_devs.h"
+
+/* UAS */
If you're going to put this comment line here, why isn't there a similar
comment line "/* Mass-Storage */" at the start of the structure
initializer?
+#undef UNUSUAL_DEV
+#define UNUSUAL_DEV(id_vendor, id_product, bcdDeviceMin, bcdDeviceMax, \
+ vendorName, productName, useProtocol, useTransport, \
+ initFunction, flags) \
+{ TYPE_DEVICE_UAS, 0, 0, id_vendor, id_product, bcdDeviceMin, bcdDeviceMax, flags, FLAGS_NOT_SET, 0 }
+
+#include "unusual_uas.h"
+
+{ .flags = FLAGS_END }
+};
+#undef UNUSUAL_DEV
+#undef USUAL_DEV
+#undef COMPLIANT_DEV
+#undef IS_ENABLED
+
+#define HI32 (uint32_t)0x80000000
+
+static unsigned long get_device_info(uint64_t flags, unsigned int idx)
+{
+ if (flags < HI32)
+ return (unsigned long)flags;
+
+ /* Use index that will be processed in usb_stor_di2flags */
+ return HI32 + idx;
+}
+
+static void print_class(uint8_t bDeviceSubClass, uint8_t bDeviceProtocol)
+{
+ printf("\t{ .match_flags = USB_DEVICE_ID_MATCH_INT_INFO, ");
+ printf(".bInterfaceClass = USB_CLASS_MASS_STORAGE, ");
+ printf(".bInterfaceSubClass = 0x%x, .bInterfaceProtocol = 0x%x },\n",
+ bDeviceSubClass, bDeviceProtocol);
+}
+static void print_type(unsigned int type)
+{
+ for (int i = 0; vals[i].flags != FLAGS_END; i++) {
+ if (vals[i].type != type)
+ continue;
+
+ if (type == TYPE_DEVICE_STORAGE || type == TYPE_DEVICE_UAS) {
+ printf("\t{ .match_flags = USB_DEVICE_ID_MATCH_DEVICE_AND_VERSION, ");
+ printf(".idVendor = 0x%x, .idProduct =0x%x, "
+ ".bcdDevice_lo = 0x%x, .bcdDevice_hi = 0x%x, .driver_info = 0x%lx },\n",
+ vals[i].idVendor, vals[i].idProduct,
+ vals[i].bcdDevice_lo, vals[i].bcdDevice_hi,
+ get_device_info(vals[i].flags, vals[i].idx));
+ } else if (type == TYPE_CLASS)
+ print_class(vals[i].bDeviceSubClass, vals[i].bDeviceProtocol);
+ }
+}
+
+static void print_usb_flags(void)
+{
+ int i;
+
+ printf("#include <linux/types.h>\n\n");
+
+ /* usb_stor_di2flags */
+ printf("static u64 usb_stor_di2flags(unsigned long idx)\n{\n");
+ printf("\tu64 flags = 0;\n\n");
+ printf("\tif (idx < 0x%x) \n\t\treturn idx;\n\n", HI32);
+ printf("\tswitch(idx - 0x%x) {\n", HI32);
+ for (i = 0; vals[i].flags != FLAGS_END; i++) {
+ if (vals[i].set == FLAGS_SET)
+ printf("\tcase %u: flags = 0x%llx; break;\n", vals[i].idx, vals[i].flags);
+ }
+ printf("\t}\n\n");
+ printf("\treturn flags;\n");
+ printf("}\n");
+}
I suspect the usb_stor_di2flags() function doesn't have to be created by
this preprocessor. It ought to be possible to put a slightly altered
version directly into uas-detect.h or some similar place (again, along
with a comment explaining just what it does and why), and then generate
here a simple array of 64-bit flags values which the function can index
into rather than looking values up in a large "switch" statement.
+static void print_usb_storage(void)
+{
+ printf("#include <linux/usb.h>\n\n");
+
+ /* usb_storage_usb_ids */
+ printf("const struct usb_device_id usb_storage_usb_ids[] = {\n");
+
+ /* USB storage devices */
+ print_type(TYPE_DEVICE_STORAGE);
+
+ /* UAS storage devices */
+ printf("#if IS_ENABLED(CONFIG_USB_UAS)\n");
+ print_type(TYPE_DEVICE_UAS);
+ printf("#endif\n");
+
+ /* transport subclasses */
+ print_type(TYPE_CLASS);
+
+ printf("\t{ }\t\t/* Terminating entry */\n};\n");
+ printf("MODULE_DEVICE_TABLE(usb, usb_storage_usb_ids);\n");
+}
+
+static void print_usb_uas(void)
+{
+ printf("#include <linux/usb.h>\n\n");
+
+ /* uas_usb_ids */
+ printf("const struct usb_device_id uas_usb_ids[] = {\n");
+
+ /* UAS storage devices */
+ print_type(TYPE_DEVICE_UAS);
+
+ /* transport subclasses */
+ print_class(USB_SC_SCSI, USB_PR_BULK);
+ print_class(USB_SC_SCSI, USB_PR_UAS);
+
+ printf("\t{ }\t\t/* Terminating entry */\n};\n");
+ printf("MODULE_DEVICE_TABLE(usb, uas_usb_ids);\n");
+}
+
+int main(int argc, char *argv[])
+{
+ int i, j, idx = 0, idx_old, skip = 0;
+
+ if (argc != 2 || (strcmp(argv[1], "flags") &&
+ strcmp(argv[1], "storage") && strcmp(argv[1], "uas"))) {
+ printf("Please specify type: storage, uas or flags.\n");
+ return 1;
+ }
+
+ for (i = 0; vals[i].flags != FLAGS_END; i++) {
+ if (vals[i].type == TYPE_CLASS)
+ continue;
+ skip = 0;
+ if (vals[i].flags >= HI32) {
+ for (j = 0; j < i; j++) {
+ if (vals[j].flags == vals[i].flags &&
+ vals[j].set == FLAGS_SET) {
+ skip = 1;
+ idx_old = vals[j].idx;
+ break;
+ }
+ }
This de-duplication may be a little premature. But I guess it doesn't
hurt.
+ if (skip) {
+ vals[i].idx = idx_old;
+ vals[i].set = FLAGS_DUPLICATE;
+ } else {
+ vals[i].idx = idx;
+ vals[i].set = FLAGS_SET;
+ idx++;
+ }
+ }
+ }
+
+ if (!strcmp(argv[1], "flags"))
+ print_usb_flags();
+ else if (!strcmp(argv[1], "storage"))
+ print_usb_storage();
+ else if (!strcmp(argv[1], "uas"))
+ print_usb_uas();
+ else
+ return 1;
+
+ return 0;
+}
The rest of the patch looks pretty straightforward.
Alan Stern
