Re: [PATCH] USB: serial: option: add Telit Cinterion FE910C04 (ECM) composition

[Johan Hovold <johan@kernel.org>]

On Wed, Jul 09, 2025 at 12:50:27PM +0200, Fabio Porcedda wrote:
> Il giorno mer 9 lug 2025 alle ore 12:08 Johan Hovold
> <johan@kernel.org> ha scritto:
> >
> > On Tue, Jul 08, 2025 at 02:00:04PM +0200, Fabio Porcedda wrote:
> > > Add Telit Cinterion FE910C04 (ECM) composition:
> > > 0x10c7: ECM + tty (AT) + tty (AT) + tty (diag)

> > >  /* Interface does not support modem-control requests */
> > >  #define NCTRL(ifnum) ((BIT(ifnum) & 0xff) << 8)
> > > +#define NCTRL_ALL    (0xff << 8)

> > > +     { USB_DEVICE_AND_INTERFACE_INFO(TELIT_VENDOR_ID, 0x10c7, 0xff, 0xff, 0x30),     /* Telit FE910C04 (ECM) */
> > > +       .driver_info = NCTRL_ALL },
> >
> > Please just use NCTRL(4) here. (And remember to mention additions like
> > this in the commit message in the future.)
> 
> Ok, I will send a v2.
> 
> > Or do you have reasons to believe the interface numbering may change? Or
> > is it just to avoid matching on both number and protocol?
> 
> The interface number should not change, it's just to have a more
> generic definition that matches also other pids for the same soc. I
> think it's easier to write and less error prone because the interface
> number changes based on the PID.

Yeah, I can see it having some merit. Maybe I reacted to the naming as
I at first incorrectly read it as no interface supporting the control
request (perhaps naming it "NCTRL_ANY" would have avoided this).

But for consistency I think we can continue using the interface numbers
until we have some better abstraction for these that can be used to
avoid also the explicit per protocol entries.

Johan