linux-kernel - Re: [PATCH v4] NDI FTDI USB driver support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aG48PYdWZXwbrd5S@hovoldconsulting.com>
Date: Wed, 9 Jul 2025 11:54:05 +0200
From: Johan Hovold <johan@...nel.org>
To: Ryan Mann <rmann@...gital.com>
Cc: "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
	"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4] NDI FTDI USB driver support

On Fri, Jul 04, 2025 at 05:25:43PM +0000, Ryan Mann wrote:
> From: Ryan Mann (NDI) <rmann@...gital.com>
> 
> NDI (Northern Digital Inc.) is introducing a new product called the 
> EMGUIDE GEMINI that will use an FTDI chip for USB serial communications.

> Remove the use of the baud rate mapping for NDI Aurora.

Your commit message should explain *why* a change is done.

This bit also looks like an unrelated change that should go in it's own
patch. It also no longer reflects what the patch does.

> Add the NDI VID definition as "FTDI_NDI_VID" into the ftdi_sio_ids.h.
> Add the NDI EMGUIDE GEMINI PID definition into the ftdi_sio_ids.h.

No need to be this specific, just say something about the adding the
device id which uses a new vendor define unlike the older products.

> Add the NDI VID/EMGUIDE GEMINI PID combination to the USB_DEVICE list.

Not needed.

> Signed-off-by: Ryan Mann <rmann@...gital.com>
> ---
> V1 -> V2: Email-to issues fixed
> V2 -> V3: Email formatting issues fixed
> V3 -> V4: Email formatting issues fixed

You clearly left out some changes here since your initial submission
also added ids for "future" devices.

There was also some changes related to the two NID quirks.

>  drivers/usb/serial/ftdi_sio.c     | 4 ++--
>  drivers/usb/serial/ftdi_sio_ids.h | 2 ++
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
> index 6ac7a0a5cf07..e5d7cce83a72 100644
> --- a/drivers/usb/serial/ftdi_sio.c
> +++ b/drivers/usb/serial/ftdi_sio.c
> @@ -801,8 +801,8 @@ static const struct usb_device_id id_table_combined[] = {
>                 .driver_info = (kernel_ulong_t)&ftdi_NDI_device_quirk },
>         { USB_DEVICE(FTDI_VID, FTDI_NDI_FUTURE_3_PID),
>                 .driver_info = (kernel_ulong_t)&ftdi_NDI_device_quirk },
> -       { USB_DEVICE(FTDI_VID, FTDI_NDI_AURORA_SCU_PID),
> -               .driver_info = (kernel_ulong_t)&ftdi_NDI_device_quirk },

Why are you removing the old product from the id table which will
prevent the driver from binding to it?

> +       { USB_DEVICE(NDI_VID, FTDI_NDI_EMGUIDE_GEMINI_PID),
> +         .driver_info = (kernel_ulong_t)&ftdi_NDI_device_quirk },

Your patch is also white space damaged (e.g. tabs replaced with spaces,
which prevents it from being applied.

Try sending the patch to yourself first and make sure you can apply it
with git am.

>         { USB_DEVICE(TELLDUS_VID, TELLDUS_TELLSTICK_PID) },
>         { USB_DEVICE(NOVITUS_VID, NOVITUS_BONO_E_PID) },
>         { USB_DEVICE(FTDI_VID, RTSYSTEMS_USB_VX8_PID) },
> diff --git a/drivers/usb/serial/ftdi_sio_ids.h b/drivers/usb/serial/ftdi_sio_ids.h
> index 9acb6f837327..0cb33d257973 100644
> --- a/drivers/usb/serial/ftdi_sio_ids.h
> +++ b/drivers/usb/serial/ftdi_sio_ids.h
> @@ -203,6 +203,8 @@
>  #define FTDI_NDI_FUTURE_2_PID          0xDA72  /* NDI future device #2 */
>  #define FTDI_NDI_FUTURE_3_PID          0xDA73  /* NDI future device #3 */
>  #define FTDI_NDI_AURORA_SCU_PID                0xDA74  /* NDI Aurora SCU */

Please include a newline here to separate the two sections.

> +#define FTDI_NDI_VID                   0x23F2  /* NDI Vendor ID */

And drop the comment here.

> +#define FTDI_NDI_EMGUIDE_GEMINI_PID    0x0003  /* NDI Emguide Gemini */
> 
>  /*
>   * ChamSys Limited (www.chamsys.co.uk) USB wing/interface product IDs

Johan