Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 9 Jun 2022 14:36:04 +0200
From:   Johan Hovold <johan@kernel.org>
To:     Slark Xiao <slark_xiao@163.com>
Cc:     gregkh@linuxfoundation.org, linux-usb@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] USB: serial: option: add support for Cinterion MV31 with
 new baseline
Message-ID: <YqHpNAaY/Oez8TrJ@hovoldconsulting.com>
References: <20220601034740.5438-1-slark_xiao@163.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220601034740.5438-1-slark_xiao@163.com>
Precedence: bulk

On Wed, Jun 01, 2022 at 11:47:40AM +0800, Slark Xiao wrote:
> Adding support for Cinterion device MV31 with Qualcomm
> new baseline. Use different PIDs to separate it from
> previous base line products.
> All interfaces settings keep same as previous.
> 
> Below is test evidence:
> T:  Bus=03 Lev=01 Prnt=01 Port=00 Cnt=01 Dev#=  6 Spd=480 MxCh= 0
> D:  Ver= 2.10 Cls=ef(misc ) Sub=02 Prot=01 MxPS=64 #Cfgs=  1
> P:  Vendor=1e2d ProdID=00b8 Rev=04.14
> S:  Manufacturer=Cinterion
> S:  Product=Cinterion PID 0x00B8 USB Mobile Broadband
> S:  SerialNumber=90418e79
> C:  #Ifs= 6 Cfg#= 1 Atr=a0 MxPwr=500mA
> I:  If#=0x0 Alt= 0 #EPs= 1 Cls=02(commc) Sub=0e Prot=00 Driver=cdc_mbim
> I:  If#=0x1 Alt= 1 #EPs= 2 Cls=0a(data ) Sub=00 Prot=02 Driver=cdc_mbim
> I:  If#=0x2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=40 Driver=option
> I:  If#=0x3 Alt= 0 #EPs= 1 Cls=ff(vend.) Sub=ff Prot=ff Driver=(none)
> I:  If#=0x4 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=60 Driver=option
> I:  If#=0x5 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=30 Driver=option
> 
> T:  Bus=03 Lev=01 Prnt=01 Port=00 Cnt=01 Dev#=  7 Spd=480 MxCh= 0
> D:  Ver= 2.10 Cls=ef(misc ) Sub=02 Prot=01 MxPS=64 #Cfgs=  1
> P:  Vendor=1e2d ProdID=00b9 Rev=04.14
> S:  Manufacturer=Cinterion
> S:  Product=Cinterion PID 0x00B9 USB Mobile Broadband
> S:  SerialNumber=90418e79
> C:  #Ifs= 4 Cfg#= 1 Atr=a0 MxPwr=500mA
> I:  If#=0x0 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=50 Driver=qmi_wwan
> I:  If#=0x1 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=40 Driver=option
> I:  If#=0x2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=60 Driver=option
> I:  If#=0x3 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=30 Driver=option
> 
> For PID 00b8, interface 3 is GNSS port which don't use serial driver.
> 
> Signed-off-by: Slark Xiao <slark_xiao@163.com>
> ---
>  drivers/usb/serial/option.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
> index e60425bbf537..5e26e90db7d6 100644
> --- a/drivers/usb/serial/option.c
> +++ b/drivers/usb/serial/option.c
> @@ -432,6 +432,8 @@ static void option_instat_callback(struct urb *urb);
>  #define CINTERION_PRODUCT_CLS8			0x00b0
>  #define CINTERION_PRODUCT_MV31_MBIM		0x00b3
>  #define CINTERION_PRODUCT_MV31_RMNET		0x00b7
> +#define CINTERION_PRODUCT_MV31_MBIM2		0x00b8
> +#define CINTERION_PRODUCT_MV31_RMNET2		0x00b9

Thanks for the patch. I've applied it now after renaming the defines to
make the variant an infix instead (e.g. CINTERION_PRODUCT_MV31_2_MBIM).

>  #define CINTERION_PRODUCT_MV32_WA		0x00f1
>  #define CINTERION_PRODUCT_MV32_WB		0x00f2
>  
> @@ -1979,6 +1981,10 @@ static const struct usb_device_id option_ids[] = {
>  	  .driver_info = RSVD(3)},
>  	{ USB_DEVICE_INTERFACE_CLASS(CINTERION_VENDOR_ID, CINTERION_PRODUCT_MV31_RMNET, 0xff),
>  	  .driver_info = RSVD(0)},
> +	{ USB_DEVICE_INTERFACE_CLASS(CINTERION_VENDOR_ID, CINTERION_PRODUCT_MV31_MBIM2, 0xff),
> +	  .driver_info = RSVD(3)},
> +	{ USB_DEVICE_INTERFACE_CLASS(CINTERION_VENDOR_ID, CINTERION_PRODUCT_MV31_RMNET2, 0xff),
> +	  .driver_info = RSVD(0)},
>  	{ USB_DEVICE_INTERFACE_CLASS(CINTERION_VENDOR_ID, CINTERION_PRODUCT_MV32_WA, 0xff),
>  	  .driver_info = RSVD(3)},
>  	{ USB_DEVICE_INTERFACE_CLASS(CINTERION_VENDOR_ID, CINTERION_PRODUCT_MV32_WB, 0xff),

Johan