From: Johan Hovold <johan@kernel.org>
To: Crescent Hsieh <crescentcy.hsieh@moxa.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
FangpingFP.Cheng@moxa.com, Epson.Chiang@moxa.com
Subject: Re: [PATCH v2 2/4] USB: serial: mxuport: add MUX50U-based device support
Date: Mon, 3 Aug 2026 09:08:15 +0200 [thread overview]
Message-ID: <anA-X-QE79_hMR4I@hovoldconsulting.com> (raw)
In-Reply-To: <amcyri8fi8YPrEW4@moxa-ThinkCentre-M90t>
On Mon, Jul 27, 2026 at 06:27:58PM +0800, Crescent Hsieh wrote:
> On Tue, Jul 21, 2026 at 04:59:05PM +0200, Johan Hovold wrote:
> > On Tue, Jun 23, 2026 at 04:01:37PM +0800, Crescent Hsieh wrote:
> > > /* Table of devices that work with this driver */
> > > static const struct usb_device_id mxuport_idtable[] = {
> > > { USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1250_PID),
> > > - .driver_info = MX_PORTS(2) },
> > > + .driver_info = MX_DEVICE_INFO(2, MX_FW_UPORT_G1) },
> > > { USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1251_PID),
> > > - .driver_info = MX_PORTS(2) },
> > > + .driver_info = MX_DEVICE_INFO(2, MX_FW_UPORT_G1) },
> > > { USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1410_PID),
> > > - .driver_info = MX_PORTS(4) },
> > > + .driver_info = MX_DEVICE_INFO(4, MX_FW_UPORT_G1) },
> > > { USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1450_PID),
> > > - .driver_info = MX_PORTS(4) },
> > > + .driver_info = MX_DEVICE_INFO(4, MX_FW_UPORT_G1) },
> > > { USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1451_PID),
> > > - .driver_info = MX_PORTS(4) },
> > > + .driver_info = MX_DEVICE_INFO(4, MX_FW_UPORT_G1) },
> > > { USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1618_PID),
> > > - .driver_info = MX_PORTS(8) },
> > > + .driver_info = MX_DEVICE_INFO(8, MX_FW_UPORT_G1) },
> > > { USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1658_PID),
> > > - .driver_info = MX_PORTS(8) },
> > > + .driver_info = MX_DEVICE_INFO(8, MX_FW_UPORT_G1) },
> > > { USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1613_PID),
> > > - .driver_info = MX_PORTS(16) },
> > > + .driver_info = MX_DEVICE_INFO(16, MX_FW_UPORT_G1) },
> > > { USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1653_PID),
> > > - .driver_info = MX_PORTS(16) },
> >
> > My initial reaction was that it would be better to just leave the gen1
> > entries alone (with family implicitly set to MX_FW_UPORT_G1), but I
> > guess this is fine too.
> >
> > > + .driver_info = MX_DEVICE_INFO(16, MX_FW_UPORT_G1) },
> > > + { USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1252_PID),
> > > + .driver_info = MX_DEVICE_INFO(2, MX_FW_UPORT_G2) },
> > > + { USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1253_PID),
> > > + .driver_info = MX_DEVICE_INFO(2, MX_FW_UPORT_G2) },
> > > + { USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1411_PID),
> > > + .driver_info = MX_DEVICE_INFO(4, MX_FW_UPORT_G2) },
> > > + { USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1452_PID),
> > > + .driver_info = MX_DEVICE_INFO(4, MX_FW_UPORT_G2) },
> > > + { USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1453_PID),
> > > + .driver_info = MX_DEVICE_INFO(4, MX_FW_UPORT_G2) },
> > > + { USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1619_PID),
> > > + .driver_info = MX_DEVICE_INFO(8, MX_FW_UPORT_G2) },
> > > + { USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT1659_PID),
> > > + .driver_info = MX_DEVICE_INFO(8, MX_FW_UPORT_G2) },
> > > + { USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT165A_PID),
> > > + .driver_info = MX_DEVICE_INFO(8, MX_FW_UPORT_G2) },
> > > + { USB_DEVICE(MX_USBSERIAL_VID, MX_UPORT165B_PID),
> > > + .driver_info = MX_DEVICE_INFO(8, MX_FW_UPORT_G2) },
> > > + { USB_DEVICE(MX_USBSERIAL_VID, MX_MU250U_PID),
> > > + .driver_info = MX_DEVICE_INFO(2, MX_FW_PLATFORM_UART) },
> > > + { USB_DEVICE(MX_USBSERIAL_VID, MX_MU450U_PID),
> > > + .driver_info = MX_DEVICE_INFO(4, MX_FW_PLATFORM_UART) },
> > > + { USB_DEVICE(MX_USBSERIAL_VID, MX_MU850U_PID),
> > > + .driver_info = MX_DEVICE_INFO(8, MX_FW_PLATFORM_UART) },
> > > + { USB_DEVICE(MX_USBSERIAL_VID, MX_MU850U_6PORT_PID),
> > > + .driver_info = MX_DEVICE_INFO(6, MX_FW_PLATFORM_UART) },
> > > + { USB_DEVICE(MX_USBSERIAL_VID, MX_MUX50U_3PORT_PID),
> > > + .driver_info = MX_DEVICE_INFO(3, MX_FW_PLATFORM_UART) },
> > > + { USB_DEVICE(MX_USBSERIAL_VID, MX_MU850U_5PORT_PID),
> > > + .driver_info = MX_DEVICE_INFO(5, MX_FW_PLATFORM_UART) },
> > > + { USB_DEVICE(MX_USBSERIAL_VID, MX_MU850U_7PORT_PID),
> > > + .driver_info = MX_DEVICE_INFO(7, MX_FW_PLATFORM_UART) },
> > > {} /* Terminating entry */
> > > };
> >
> > Perhaps the family defines can be shortened to make this a bit more
> > readable, for example:
> >
> > MX_FW_UP_G1
> > MX_FW_UP_G2
> > MX_FW_PF
> >
> > or even
> >
> > MX_UP_G1
> > MX_UP_G2
> > MX_PF
> >
> > I didn't try and see what the result looks like. Perhaps the more
> > verbose names are preferred.
>
> I used the verbose names to keep the firmware-family naming explicit,
> but I agree that readability is important as well.
>
> Listing each device-table entry on a single line would make the table
> easier to read, but doing so directly would result in overly long lines.
> Another option would be to introduce a small helper macro:
>
> #define MX_DEVICE(pid, ports, fw) \
> { USB_DEVICE(MX_USBSERIAL_VID, pid), \
> .driver_info = MX_DEVICE_INFO(ports, fw) }
>
> The ID table could then be written as:
>
> MX_DEVICE(MX_UPORT1250_PID, 2, MX_FW_UPORT_G1),
> MX_DEVICE(MX_UPORT1250I_PID, 2, MX_FW_UPORT_G1),
> ...
> MX_DEVICE(MX_UPORT1250_G2_PID, 2, MX_FW_UPORT_G2),
> MX_DEVICE(MX_UPORT12150I_G2_PID, 2, MX_FW_UPORT_G2),
> ...
> MX_DEVICE(MX_MU850U, 8, MX_FW_PLATFORM_UART),
> MX_DEVICE(MX_MUX50U_6PORT_PID, 6, MX_FW_PLATFORM_UART),
>
> This would allow the firmware-family names to remain explicit while
> keeping each device entry on a single line.
That sounds like a good solution.
Johan
next prev parent reply other threads:[~2026-08-03 7:08 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-23 8:01 [PATCH v2 0/4] USB: serial: mxuport: add MUX50U support and updates Crescent Hsieh
2026-06-23 8:01 ` [PATCH v2 1/4] USB: serial: mxuport: clean up firmware version handling Crescent Hsieh
2026-07-21 14:34 ` Johan Hovold
2026-07-27 10:27 ` Crescent Hsieh
2026-06-23 8:01 ` [PATCH v2 2/4] USB: serial: mxuport: add MUX50U-based device support Crescent Hsieh
2026-07-21 14:59 ` Johan Hovold
2026-07-27 10:27 ` Crescent Hsieh
2026-08-03 7:08 ` Johan Hovold [this message]
2026-06-23 8:01 ` [PATCH v2 3/4] USB: serial: mxuport: handle SEND_NEXT transmit flow control Crescent Hsieh
2026-07-21 15:51 ` Johan Hovold
2026-07-27 10:28 ` Crescent Hsieh
2026-08-03 8:35 ` Johan Hovold
2026-06-23 8:01 ` [PATCH v2 4/4] USB: serial: mxuport: support RS485 mode configuration Crescent Hsieh
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=anA-X-QE79_hMR4I@hovoldconsulting.com \
--to=johan@kernel.org \
--cc=Epson.Chiang@moxa.com \
--cc=FangpingFP.Cheng@moxa.com \
--cc=crescentcy.hsieh@moxa.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox