| Message ID | 20230720114406.14587-1-oneukum@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | USB: serial: simple-serial: spell out the ordering | expand |
On Thu, Jul 20, 2023 at 01:44:06PM +0200, Oliver Neukum wrote: > keeping a list ordered alphabetically instead > nummerically be vendor/product ID is unusual. > This is so odd that examples do not help. > It needs to be clearly stated with strong words. > > Signed-off-by: Oliver Neukum <oneukum@suse.com> > --- > drivers/usb/serial/usb-serial-simple.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/usb/serial/usb-serial-simple.c b/drivers/usb/serial/usb-serial-simple.c > index 24b8772a345e..954b3be7403d 100644 > --- a/drivers/usb/serial/usb-serial-simple.c > +++ b/drivers/usb/serial/usb-serial-simple.c > @@ -33,6 +33,16 @@ static struct usb_serial_driver vendor##_device = { \ > > #define DEVICE(vendor, IDS) DEVICE_N(vendor, IDS, 1) > > +/* > + * These tables are NOT in order of device id by intention > + * > + * Keep them and add new entries sorted by > + * > + * 1. Alphabetical order of the vendor name > + * 2. Alphabetical order of the product name > + * > + */ No, this is not correct. The tables are sorted alphabetically based on the symbol names, but the entries in each table is sorted by VID/PID as usual. The table ordering was not there from the start, but I just moved the offending tables here: https://lore.kernel.org/lkml/20230720080049.14032-1-johan@kernel.org/ Guess I should update the sloppy terminology in that commit message, though. Johan
On Thu, Jul 20, 2023 at 02:01:37PM +0200, Johan Hovold wrote: > On Thu, Jul 20, 2023 at 01:44:06PM +0200, Oliver Neukum wrote: > No, this is not correct. The tables are sorted alphabetically based on > the symbol names, but the entries in each table is sorted by VID/PID as > usual. > > The table ordering was not there from the start, but I just moved the > offending tables here: > > https://lore.kernel.org/lkml/20230720080049.14032-1-johan@kernel.org/ > > Guess I should update the sloppy terminology in that commit message, > though. This should hopefully be clear enough: https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial.git/commit/?h=usb-linus&id=a1ff1edb9251e1151fb78eb9b226d5e4bc6c2810 Johan
On 20.07.23 14:01, Johan Hovold wrote: > On Thu, Jul 20, 2023 at 01:44:06PM +0200, Oliver Neukum wrote: >> + * These tables are NOT in order of device id by intention >> + * >> + * Keep them and add new entries sorted by >> + * >> + * 1. Alphabetical order of the vendor name >> + * 2. Alphabetical order of the product name >> + * >> + */ > > No, this is not correct. The tables are sorted alphabetically based on > the symbol names, but the entries in each table is sorted by VID/PID as > usual. OK, I stand corrected. You do understand that this ordering is extremely peculiar? I am sorry, but deducing that from the example, which the current order is, will just not work. Nobody gets that without a clear and present explanation. This just needs a comment in the text. Regards Oliver
On Thu, Jul 20, 2023 at 02:42:49PM +0200, Oliver Neukum wrote: > On 20.07.23 14:01, Johan Hovold wrote: > > On Thu, Jul 20, 2023 at 01:44:06PM +0200, Oliver Neukum wrote: > > >> + * These tables are NOT in order of device id by intention > >> + * > >> + * Keep them and add new entries sorted by > >> + * > >> + * 1. Alphabetical order of the vendor name > >> + * 2. Alphabetical order of the product name > >> + * > >> + */ > > > > No, this is not correct. The tables are sorted alphabetically based on > > the symbol names, but the entries in each table is sorted by VID/PID as > > usual. > > OK, I stand corrected. You do understand that this ordering is > extremely peculiar? I am sorry, but deducing that from the example, > which the current order is, will just not work. Nobody gets that > without a clear and present explanation. It's not peculiar. And if this wasn't implemented using macros, I'm pretty sure you would not think so. Take a look at io_edgeport.c, for example, where edgeport_2port_device comes before edgeport_4port_device, etc. > This just needs a comment in the text. I disagree, with the last few entries sorted this should be clear enough. Johan
diff --git a/drivers/usb/serial/usb-serial-simple.c b/drivers/usb/serial/usb-serial-simple.c index 24b8772a345e..954b3be7403d 100644 --- a/drivers/usb/serial/usb-serial-simple.c +++ b/drivers/usb/serial/usb-serial-simple.c @@ -33,6 +33,16 @@ static struct usb_serial_driver vendor##_device = { \ #define DEVICE(vendor, IDS) DEVICE_N(vendor, IDS, 1) +/* + * These tables are NOT in order of device id by intention + * + * Keep them and add new entries sorted by + * + * 1. Alphabetical order of the vendor name + * 2. Alphabetical order of the product name + * + */ + /* Medtronic CareLink USB driver */ #define CARELINK_IDS() \ { USB_DEVICE(0x0a21, 0x8001) } /* MMT-7305WW */
keeping a list ordered alphabetically instead nummerically be vendor/product ID is unusual. This is so odd that examples do not help. It needs to be clearly stated with strong words. Signed-off-by: Oliver Neukum <oneukum@suse.com> --- drivers/usb/serial/usb-serial-simple.c | 10 ++++++++++ 1 file changed, 10 insertions(+)