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:   Sun, 17 Jul 2022 17:48:33 +0200
From:   Reinhard Speyerer <rspmn@arcor.de>
To:     sdlyyxy <sdlyyxy@bupt.edu.cn>
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
Message-ID: <YtQuMJz+0MCxMAPk@arcor.de>
References: <20220715142444.4173681-1-gregkh@linuxfoundation.org>
 <119D7B0F-7809-464A-AFF1-DF72FFF9E63F@bupt.edu.cn>
 <YtKrbucYNulPEKUp@arcor.de>
 <333E5B85-7534-4CE5-8AB6-464571CBF61E@bupt.edu.cn>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <333E5B85-7534-4CE5-8AB6-464571CBF61E@bupt.edu.cn>
Precedence: bulk

Hi Yan,

On Sat, Jul 16, 2022 at 09:36:27PM +0800, sdlyyxy wrote:
> 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?
> 

in my experience activating all message logs on the device with a
mask value like 0xf or 0x1f is a good way to generate more diag traffic.
Please refer to https://source.codeaurora.org/quic/imm/imm/sources/diag
(DIAG_CMD_OP_SET_ALL_MSG_MASK) for details.

Regards,
Reinhard