Re: [PATCH] USB: usb-serial-simple: add new device id for OPPO R11

From: sdlyyxy <sdlyyxy@bupt.edu.cn>
To: Reinhard Speyerer <rspmn@arcor.de>
Cc: johan@kernel.org, Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] USB: usb-serial-simple: add new device id for OPPO R11
Date: Sat, 16 Jul 2022 21:36:27 +0800	[thread overview]
Message-ID: <333E5B85-7534-4CE5-8AB6-464571CBF61E@bupt.edu.cn> (raw)
In-Reply-To: <YtKrbucYNulPEKUp@arcor.de>

Hi Reinhard,

> On Jul 16, 2022, at 20:13, Reinhard Speyerer <rspmn@arcor.de> wrote:
> 
> On Fri, Jul 15, 2022 at 10:59:13PM +0800, sdlyyxy wrote:
>> 
>>> On Jul 15, 2022, at 22:24, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
>>> 
>>> The Oppo R11 diagnostic USB connection needs to be bound to the
>>> usb-serial-simple driver as it just wants to use a dumb pipe to
>>> communicate to the host.
>>> 
>>> usb-devices output:
>>> T: Bus=03 Lev=01 Prnt=01 Port=01 Cnt=01 Dev#= 10 Spd=480 MxCh= 0
>>> D: Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs= 1
>>> P: Vendor=22d9 ProdID=276c Rev=04.04
>>> S: Manufacturer=OPPO
>>> S: Product=SDM660-MTP _SN:09C6BCA7
>>> S: SerialNumber=beb2c403
>>> C: #Ifs= 2 Cfg#= 1 Atr=80 MxPwr=500mA
>>> I: If#=0x0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=30
>>> 
>>> Reported-by: Yan Xinyu <sdlyyxy@bupt.edu.cn>
>>> Cc: Johan Hovold <johan@kernel.org>
>>> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>> ---
>>> drivers/usb/serial/usb-serial-simple.c | 4 +++-
>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>> 
>>> diff --git a/drivers/usb/serial/usb-serial-simple.c b/drivers/usb/serial/usb-serial-simple.c
>>> index 4c6747889a19..eb832b94aa3a 100644
>>> --- a/drivers/usb/serial/usb-serial-simple.c
>>> +++ b/drivers/usb/serial/usb-serial-simple.c
>>> @@ -60,7 +60,9 @@ DEVICE(flashloader, FLASHLOADER_IDS);
>>> 	{ USB_VENDOR_AND_INTERFACE_INFO(0x18d1,			\
>>> 					USB_CLASS_VENDOR_SPEC,	\
>>> 					0x50,			\
>>> -					0x01) }
>>> +					0x01) },		\
>>> +	{ USB_DEVICE_AND_INTERFACE_INFO(0x22d9, 0x276c,		\
>>> +					0xff, 0xff, 0x30) }
>>> DEVICE(google, GOOGLE_IDS);
>>> 
>>> /* Libtransistor USB console */
>>> -- 
>>> 2.37.1
>> Tested-by: Yan Xinyu <sdlyyxy@bupt.edu.cn>
> 
> While this may work sufficiently well for real low-volume diag traffic I'd
> expect a significant percentage of diag messages to be lost in practice
> with the usb-serial-simple driver.
> 
> According to the usb-devices output this looks like the Qualcomm USB gadget
> in the DIAG + ADB composition to me.
> 
> Since the option driver uses the usb-wwan framework my suggestion would be
> for the original patch to be applied instead similar to what has been done
> e.g. for the Quectel RM500Q diag port.
> 
> Regards,
> Reinhard
> 
I tested the diag port using two userspace programs: QCSuper[1] 
and scat[2]. Both option and usb-serial-simple drivers generate
similar output, so I cannot comfirm diag message loss. Do you
have any test method suggestions to generate high-volume diag 
traffic and detect message loss?

[1] https://github.com/P1sec/QCSuper
[2] https://github.com/fgsect/scat

Thanks,
sdlyyxy