lkml.org 
[lkml]   [2022]   [Jul]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH] USB: serial: option: adding support for OPPO R11 diag port
    From
    Date

    > On Jul 15, 2022, at 14:53, Greg KH <gregkh@linuxfoundation.org> wrote:
    >
    > On Fri, Jul 15, 2022 at 08:45:34AM +0200, Greg KH wrote:
    >> On Fri, Jul 15, 2022 at 02:39:06PM +0800, sdlyyxy wrote:
    >>> Hi Greg,
    >>> Thanks for your comments!
    >>>
    >>>> On Jul 14, 2022, at 18:56, Greg KH <gregkh@linuxfoundation.org> wrote:
    >>>>
    >>>> On Thu, Jul 14, 2022 at 06:20:37PM +0800, sdlyyxy wrote:
    >>>>> From: Yan Xinyu <sdlyyxy@bupt.edu.cn>
    >>>>>
    >>>>> This patch adds support for OPPO R11 USB diag serial port to option
    >>>>> driver. This phone uses Qualcomm Snapdragon 660 SoC.
    >>>>>
    >>>>> 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 Driver=option
    >>>>
    >>>> I do not think this has an option usb-serial chip in the device, this is
    >>>> a phone with a debug port instead.
    >>>>
    >>> Yeah, this phone uses a Qualcomm chip, not an option usb-serial chip.
    >>> It has the functionality to enter into a special mode, which provides
    >>> a QCDM-capable diag port as the same behaviour of USB modems. For
    >>> Qualcomm devices, there are several drivers: qcserial, qcaux, and
    >>> option. According to qcserial.c, qcaux.c source code and mailing list
    >>> conversations [1], this device with diag+adb layout should be driven
    >>> by option.
    >>
    >> No, this is not an option chip, and does not follow the option device
    >> protocols at all. So this is not the driver to use here.
    >>
    >> We should probably switch those other devices as well, they aren't
    >> really option devices either.
    >>
    >>>>> I: If#=0x1 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=42 Prot=01 Driver=usbfs
    >>>>
    >>>> What userspace program is bound to this endpoint?
    >>>>
    >>> I think it is used by adb via libusb.
    >>>>>
    >>>>> Signed-off-by: Yan Xinyu <sdlyyxy@bupt.edu.cn>
    >>>>> ---
    >>>>> drivers/usb/serial/option.c | 5 +++++
    >>>>> 1 file changed, 5 insertions(+)
    >>>>>
    >>>>> diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
    >>>>> index de59fa919540..cf65cb84c3ca 100644
    >>>>> --- a/drivers/usb/serial/option.c
    >>>>> +++ b/drivers/usb/serial/option.c
    >>>>> @@ -573,6 +573,10 @@ static void option_instat_callback(struct urb *urb);
    >>>>> #define WETELECOM_PRODUCT_6802 0x6802
    >>>>> #define WETELECOM_PRODUCT_WMD300 0x6803
    >>>>>
    >>>>> +/* OPPO products */
    >>>>> +#define OPPO_VENDOR_ID 0x22d9
    >>>>> +#define OPPO_PRODUCT_R11 0x276c
    >>>>> +
    >>>>>
    >>>>> /* Device flags */
    >>>>>
    >>>>> @@ -2155,6 +2159,7 @@ static const struct usb_device_id option_ids[] = {
    >>>>> { USB_DEVICE_INTERFACE_CLASS(0x305a, 0x1404, 0xff) }, /* GosunCn GM500 RNDIS */
    >>>>> { USB_DEVICE_INTERFACE_CLASS(0x305a, 0x1405, 0xff) }, /* GosunCn GM500 MBIM */
    >>>>> { USB_DEVICE_INTERFACE_CLASS(0x305a, 0x1406, 0xff) }, /* GosunCn GM500 ECM/NCM */
    >>>>> + { USB_DEVICE_AND_INTERFACE_INFO(OPPO_VENDOR_ID, OPPO_PRODUCT_R11, 0xff, 0xff, 0x30) },
    >>>>
    >>>> This does not look correct, sorry. Try using the usbserial generic
    >>>> driver instead to transmit and recieve?
    >>>>
    >>> Yes I have tried using usbserial generic driver. As for the interface
    >>> #0x0 diag port, it seems working. However, in the same time the
    >>> generic driver will also be attached to interface #0x1, which causes
    >>> nonfunction of adb. Using this patch, diag and adb can run
    >>> simultaneously. So it's better than the generic driver?
    >>
    >> Ah, we should just bind the simple usb-serial driver to this interface
    >> and not bind the generic usb-serial driver to this interface.
    >>
    >> Let me make up a simple patch for this for you to test...
    >
    > Can you try the patch here for this, it should work the same as your
    > patch:
    >
    >
    > 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 */
    >
    Great, this patch works as intended. The VendorID 0x22d9 does not
    belong to Google, shall I add a separate section for OPPO in
    usb-serial-simple.c and create a new patch?

    Thanks,
    sdlyyxy



    \
     
     \ /
      Last update: 2022-07-15 12:10    [W:3.844 / U:0.400 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site