Re: [PATCH 2/4] tty: serial: 8250: Add support for MOXA Mini PCIe boards

[Crescent CY Hsieh]

On Mon, Oct 02, 2023 at 09:05:31AM +0200, Jiri Slaby wrote:
> On 02. 10. 23, 3:57, Crescent CY Hsieh wrote:
> > +	 * This hardware buffer is only supported on Mini PCIe series.
> > +	 */
> > +	if (device == 0x1027 ||	/* MOXA_CP102N */
> > +	    device == 0x1046 ||	/* MOXA_CP104N */
> > +	    device == 0x1121 ||	/* MOXA_CP112N */
> > +	    device == 0x1145 ||	/* MOXA_CP114N */
> > +	    device == 0x1323 ||	/* MOXA_CP132N */
> > +	    device == 0x1343) {	/* MOXA_CP134N */
> 
> Why not use the definitions below? You should define a function for this
> anyway.

I cannot use macros here, unless I put the definitions before this
function or into pci_ids.h.

> > @@ -1973,6 +2009,13 @@ static int pci_moxa_setup(struct serial_private *priv,
> >   #define	PCI_DEVICE_ID_MOXA_CP134EL_A	0x1342
> >   #define	PCI_DEVICE_ID_MOXA_CP138E_A	0x1381
> >   #define	PCI_DEVICE_ID_MOXA_CP168EL_A	0x1683
> > +/* MOXA Mini PCIe */
> > +#define PCI_DEVICE_ID_MOXA_CP102N	0x1027
> > +#define PCI_DEVICE_ID_MOXA_CP104N	0x1046
> > +#define PCI_DEVICE_ID_MOXA_CP112N	0x1121
> > +#define PCI_DEVICE_ID_MOXA_CP114N	0x1145
> > +#define PCI_DEVICE_ID_MOXA_CP132N	0x1323
> > +#define PCI_DEVICE_ID_MOXA_CP134N	0x1343
> 
> I am not sure it matters they are mini PCIe. I would sort them into the
> above preexisting list instead.

For these lines, classify the type of boards is just for clarity, but
sort them into preexisting list is also feasible.

---
Sincerely,
Crescent CY Hsieh