Re: [PATCH v4] NDI FTDI USB driver support

[Johan Hovold <johan@kernel.org>]

On Fri, Jul 04, 2025 at 05:25:43PM +0000, Ryan Mann wrote:
> From: Ryan Mann (NDI) <rmann@ndigital.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@ndigital.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