[PATCH] USB: serial: simple-serial: spell out the ordering

[PATCH] USB: serial: simple-serial: spell out the ordering

[Oliver Neukum]

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
+ *
+ */
+
 /* Medtronic CareLink USB driver */
 #define CARELINK_IDS()			\
 	{ USB_DEVICE(0x0a21, 0x8001) }	/* MMT-7305WW */
-- 
2.41.0

Re: [PATCH] USB: serial: simple-serial: spell out the ordering

[Johan Hovold]

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

Re: [PATCH] USB: serial: simple-serial: spell out the ordering

[Johan Hovold]

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

Re: [PATCH] USB: serial: simple-serial: spell out the ordering

[Oliver Neukum]

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

Re: [PATCH] USB: serial: simple-serial: spell out the ordering

[Johan Hovold]

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