| Message ID | 1545134386-5528-1-git-send-email-macpaul.lin@mediatek.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v5] cdc-acm: fix abnormal DATA RX issue for Mediatek Preloader. | expand |
On Tue, Dec 18, 2018 at 07:59:46PM +0800, Macpaul Lin wrote: > Mediatek Preloader is a proprietary embedded boot loader for loading > Little Kernel and Linux into device DRAM. > > This boot loader also handle firmware update. Mediatek Preloader will be > enumerated as a virtual COM port when the device is connected to Windows > or Linux OS via CDC-ACM class driver. When the USB enumeration has been > done, Mediatek Preloader will send out handshake command "READY" to PC > actively instead of waiting command from the download tool. > > Since Linux 4.12, the commit "tty: reset termios state on device > registration" (93857edd9829e144acb6c7e72d593f6e01aead66) causes Mediatek > Preloader receiving some abnoraml command like "READYXX" as it sent. > This will be recognized as an incorrect response. The behavior change > also causes the download handshake fail. This change only affects > subsequent connects if the reconnected device happens to get the same minor > number. > > By disabling the ECHO termios flag could avoid this problem. However, it > cannot be done by user space configuration when download tool open > /dev/ttyACM0. This is because the device running Mediatek Preloader will > send handshake command "READY" immediately once the CDC-ACM driver is > ready. > > This patch wants to fix above problem by introducing "DISABLE_ECHO" > property in driver_info. When Mediatek Preloader is connected, the > CDC-ACM driver could disable ECHO flag in termios to avoid the problem. > > Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com> > Cc: stable@vger.kernel.org > --- > Changes for v2: > - Move quirks testing of DISABLE_ECHO flag into acm_tty_install(). > - Change quirks testing into bitwise comparison. > Changes for v3: > - Replace quirks testing from init_termios to tty->termios. > - Remove parenthesis for ECHO flag. > Changes for v4: > - Drop quirks varible to simplify the patch. > - Move termios operation right after the driver_data has been installed. > - Write general style comment for suppressing initial echoing. > Changes for v5: > - Fix: termios operation right abover the driver_data has been installed. > - Update commit comment about this patch affects the reconnected device > which get the same minor numbers. Thanks for sticking with it. After addressing a comment below you have my: Reviewed-by: Johan Hovold <johan@kernel.org> > > drivers/usb/class/cdc-acm.c | 12 +++++++++++- > drivers/usb/class/cdc-acm.h | 1 + > 2 files changed, 12 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c > index 1b68fed..336cf13 100644 > --- a/drivers/usb/class/cdc-acm.c > +++ b/drivers/usb/class/cdc-acm.c > @@ -581,6 +581,13 @@ static int acm_tty_install(struct tty_driver *driver, struct tty_struct *tty) > if (retval) > goto error_init_termios; > > + /* > + * Suppress initial echoing for some devices which might send data > + * immediately after acm driver has been installed. > + */ > + if (acm->quirks & DISABLE_ECHO) > + tty->termios.c_lflag &= ~ECHO; > + > tty->driver_data = acm; > > return 0; > @@ -1655,7 +1662,10 @@ static int acm_pre_reset(struct usb_interface *intf) > .driver_info = NO_UNION_NORMAL, /* has no union descriptor */ > }, > { USB_DEVICE(0x0e8d, 0x0003), /* FIREFLY, MediaTek Inc; andrey.arapov@gmail.com */ > - .driver_info = NO_UNION_NORMAL, /* has no union descriptor */ > + .driver_info = DISABLE_ECHO, /* DISABLE ECHO in termios flag */ I just noticed that you remove the NO_UNION_NORMAL here, which looks wrong and definitely requires a motivation. Thanks, Johan
On Di, 2018-12-18 at 13:38 +0100, Johan Hovold wrote: > On > > @@ -1655,7 +1662,10 @@ static int acm_pre_reset(struct usb_interface *intf) > > .driver_info = NO_UNION_NORMAL, /* has no union descriptor */ > > }, > > { USB_DEVICE(0x0e8d, 0x0003), /* FIREFLY, MediaTek Inc; andrey.arapov@gmail.com */ > > - .driver_info = NO_UNION_NORMAL, /* has no union descriptor */ > > + .driver_info = DISABLE_ECHO, /* DISABLE ECHO in termios flag */ > > I just noticed that you remove the NO_UNION_NORMAL here, which looks > wrong and definitely requires a motivation. > > Thanks, > Johan > Hi, thank you and thank you Johan. Unfortunately I cannot take this until the issue with the removed quirk is clarified. Regards Oliver
On Tue, 2018-12-18 at 14:37 +0100, Oliver Neukum wrote: > On Di, 2018-12-18 at 13:38 +0100, Johan Hovold wrote: > > On > > > @@ -1655,7 +1662,10 @@ static int acm_pre_reset(struct usb_interface *intf) > > > .driver_info = NO_UNION_NORMAL, /* has no union descriptor */ > > > }, > > > { USB_DEVICE(0x0e8d, 0x0003), /* FIREFLY, MediaTek Inc; andrey.arapov@gmail.com */ > > > - .driver_info = NO_UNION_NORMAL, /* has no union descriptor */ > > > + .driver_info = DISABLE_ECHO, /* DISABLE ECHO in termios flag */ > > > > I just noticed that you remove the NO_UNION_NORMAL here, which looks > > wrong and definitely requires a motivation. > > > > Thanks, > > Johan > > > > Hi, > > thank you and thank you Johan. > Unfortunately I cannot take this until the issue with the removed > quirk is clarified. > > Regards > Oliver > Hi, Thanks Johan's help on reviewing the updated patches. According to Andrey's patch, the commit said this change is for "Samsung X180 China cellphone" which might be also a Mediatek SoC based phone using Mediatek VID. https://goo.gl/a9ddNq (I've add a url for reference here). But I'm not sure the PID is for Mediatek's BROM or customized Preloader. (Both should be able to apply DISABLE_ECHO flag). Maybe I can simply update PATCH v6 by removing FIREFLY if any one has problem here. Let's also loop Andrey for clarification. Thanks! Regards, Macpaul Lin
On Tue, 2018-12-18 at 22:26 +0800, Macpaul Lin wrote: > On Tue, 2018-12-18 at 14:37 +0100, Oliver Neukum wrote: > > On Di, 2018-12-18 at 13:38 +0100, Johan Hovold wrote: > > > On > > > > @@ -1655,7 +1662,10 @@ static int acm_pre_reset(struct usb_interface *intf) > > > > .driver_info = NO_UNION_NORMAL, /* has no union descriptor */ > > > > }, > > > > { USB_DEVICE(0x0e8d, 0x0003), /* FIREFLY, MediaTek Inc; andrey.arapov@gmail.com */ > > > > - .driver_info = NO_UNION_NORMAL, /* has no union descriptor */ > > > > + .driver_info = DISABLE_ECHO, /* DISABLE ECHO in termios flag */ > > > > > > I just noticed that you remove the NO_UNION_NORMAL here, which looks > > > wrong and definitely requires a motivation. > > > > > > Thanks, > > > Johan > > > > > > > Hi, > > > > thank you and thank you Johan. > > Unfortunately I cannot take this until the issue with the removed > > quirk is clarified. > > > > Regards > > Oliver > > > > Hi, > Thanks Johan's help on reviewing the updated patches. > According to Andrey's patch, the commit said this change is for > "Samsung X180 China cellphone" which might be also a Mediatek SoC based > phone using Mediatek VID. > https://goo.gl/a9ddNq (I've add a url for reference here). > > But I'm not sure the PID is for Mediatek's BROM or customized Preloader. > (Both should be able to apply DISABLE_ECHO flag). > Maybe I can simply update PATCH v6 by removing FIREFLY if any one has > problem here. > Let's also loop Andrey for clarification. Hi all, After double check the PID/VID officially used (registered) by Mediatek Inc. The following VID/PID maps to the corresponding USB interface of the firmware of cell phone. VID:0x0e8d, PID:0x0003: Mediatek BROM. VID:0x0e8d, PID:0x2000: Mediatek Preloader. I will update Patch v6 for changing Andrey's previous comments. Thanks. Regards, Macpaul Lin.
On 12/18/2018 22:19, Macpaul Lin wrote: > Hi all, > > After double check the PID/VID officially used (registered) by Mediatek > Inc. The following VID/PID maps to the corresponding USB interface of > the firmware of cell phone. > VID:0x0e8d, PID:0x0003: Mediatek BROM. > VID:0x0e8d, PID:0x2000: Mediatek Preloader. > > I will update Patch v6 for changing Andrey's previous comments. > You should show us a verbose lsusb of your 0e8d:0003 before changing Andrey's submission. Season's Greeting Lars
On Tue, 2018-12-18 at 23:42 +0700, Lars Melin wrote: > On 12/18/2018 22:19, Macpaul Lin wrote: > > > > Hi all, > > > > After double check the PID/VID officially used (registered) by Mediatek > > Inc. The following VID/PID maps to the corresponding USB interface of > > the firmware of cell phone. > > VID:0x0e8d, PID:0x0003: Mediatek BROM. > > VID:0x0e8d, PID:0x2000: Mediatek Preloader. > > > > I will update Patch v6 for changing Andrey's previous comments. > > > > You should show us a verbose lsusb of your 0e8d:0003 before changing > Andrey's submission. > > > Season's Greeting > Lars Hi Lars, Oliver, and Johan, Well, it's about 1:30 a.m. here in Taiwan. I can do lsusb dump tomorrow in Mediatek's office. But there are a lot of tutorials about hacking and flash method of customized firmware over the WWW. I can find one of the tutorial shows these VID/PID combinations has been used since 2014. (Actually they've been registered officially earlier than 2014.) For example, please check the reference tutorial: https://mattboyer.github.io/PYaffs/2014/07/31/Hacklog%233.html You can check lsusb dump in this picture. https://mattboyer.github.io/PYaffs/images/03_device_usbview.png Both VID/PID of Mediatek BROM and Preloader can be found in INF file pasted in this tutorial, too. Sorry for the proprietary information policy I cannot paste them all, but the VID/PID set of BROM and Preloader are just exactly the same as described in this tutorial. Thanks! Regards, Macpaul Lin
On 12/19/2018 00:48, Macpaul Lin wrote: > Hi Lars, Oliver, and Johan, > > Well, it's about 1:30 a.m. here in Taiwan. I can do lsusb dump tomorrow > in Mediatek's office. > But there are a lot of tutorials about hacking and flash method of > customized firmware over the WWW. I can find one of the tutorial shows > these VID/PID combinations has been used since 2014. > (Actually they've been registered officially earlier than 2014.) > > For example, please check the reference tutorial: > https://mattboyer.github.io/PYaffs/2014/07/31/Hacklog%233.html > You can check lsusb dump in this picture. > https://mattboyer.github.io/PYaffs/images/03_device_usbview.png > Both VID/PID of Mediatek BROM and Preloader can be found in INF file Hi Macpaul, the picture of an lsusb dump is not complete and does not show if there are union descriptors or if they are missing. Since you remove Andrey's NO UNION DESCRIPTOR quirk then you have to show that there are union descriptors in your device. If there are union descriptors in it then Mediatek has used the same vid:pid for two devices that differs from each other. /Lars
On Wed, 2018-12-19 at 01:48 +0800, Macpaul Lin wrote: > On Tue, 2018-12-18 at 23:42 +0700, Lars Melin wrote: > > On 12/18/2018 22:19, Macpaul Lin wrote: > > > > > > > Hi all, > > > > > > After double check the PID/VID officially used (registered) by Mediatek > > > Inc. The following VID/PID maps to the corresponding USB interface of > > > the firmware of cell phone. > > > VID:0x0e8d, PID:0x0003: Mediatek BROM. > > > VID:0x0e8d, PID:0x2000: Mediatek Preloader. > > > > > > I will update Patch v6 for changing Andrey's previous comments. > > > > > > > You should show us a verbose lsusb of your 0e8d:0003 before changing > > Andrey's submission. > > > > > > Season's Greeting > > Lars > > Hi Lars, Oliver, and Johan, > > Well, it's about 1:30 a.m. here in Taiwan. I can do lsusb dump tomorrow > in Mediatek's office. > But there are a lot of tutorials about hacking and flash method of > customized firmware over the WWW. I can find one of the tutorial shows > these VID/PID combinations has been used since 2014. > (Actually they've been registered officially earlier than 2014.) > > For example, please check the reference tutorial: > https://mattboyer.github.io/PYaffs/2014/07/31/Hacklog%233.html > You can check lsusb dump in this picture. > https://mattboyer.github.io/PYaffs/images/03_device_usbview.png > Both VID/PID of Mediatek BROM and Preloader can be found in INF file > pasted in this tutorial, too. > Sorry for the proprietary information policy I cannot paste them all, > but the VID/PID set of BROM and Preloader are just exactly the same > as described in this tutorial. > Hi all, Here is the lsusb dump of the BROM and Preloader state of the phone. lsusb of BROM: Bus 001 Device 012: ID 0e8d:0003 MediaTek Inc. MT6227 phone lsusb of Preloader: Bus 001 Device 011: ID 0e8d:2000 MediaTek Inc. MT65xx Preloader dmesg of BROM: [52138.849233] usb 1-1: new high-speed USB device number 12 using ehci-pci [52139.252557] usb 1-1: New USB device found, idVendor=0e8d, idProduct=0003 [52139.252559] usb 1-1: New USB device strings: Mfr=0, Product=0, SerialNumber=0 [52139.260179] cdc_acm 1-1:1.0: ttyACM0: USB ACM device dmesg of Preloader: [52145.052952] usb 1-1: new high-speed USB device number 13 using ehci-pci [52145.410432] usb 1-1: New USB device found, idVendor=0e8d, idProduct=2000 [52145.410434] usb 1-1: New USB device strings: Mfr=1, Product=2, SerialNumber=0 [52145.410435] usb 1-1: Product: MT65xx Preloader [52145.410437] usb 1-1: Manufacturer: MediaTek Regards, Macpaul Lin
On Wed, 2018-12-19 at 10:22 +0800, Macpaul Lin wrote: > On Wed, 2018-12-19 at 01:48 +0800, Macpaul Lin wrote: > > On Tue, 2018-12-18 at 23:42 +0700, Lars Melin wrote: > > > On 12/18/2018 22:19, Macpaul Lin wrote: > > > > > > > > > > Hi all, > > > > > > > > After double check the PID/VID officially used (registered) by Mediatek > > > > Inc. The following VID/PID maps to the corresponding USB interface of > > > > the firmware of cell phone. > > > > VID:0x0e8d, PID:0x0003: Mediatek BROM. > > > > VID:0x0e8d, PID:0x2000: Mediatek Preloader. > > > > > > > > I will update Patch v6 for changing Andrey's previous comments. > > > > > > > > > > You should show us a verbose lsusb of your 0e8d:0003 before changing > > > Andrey's submission. > > > > > > > > > Season's Greeting > > > Lars > > Hi Lars, Sorry I just missed your explain about the UNION descriptor and did not found in my mail box. Here comes the verbose lsusb -v dump. Bus 001 Device 003: ID 0e8d:0003 MediaTek Inc. MT6227 phone [432/3160] Device Descriptor: bLength 18 bDescriptorType 1 bcdUSB 1.10 bDeviceClass 2 Communications bDeviceSubClass 0 bDeviceProtocol 0 bMaxPacketSize0 64 idVendor 0x0e8d MediaTek Inc. idProduct 0x0003 MT6227 phone bcdDevice 1.00 iManufacturer 0 iProduct 0 iSerial 0 bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 67 bNumInterfaces 2 bConfigurationValue 1 iConfiguration 0 bmAttributes 0x80 (Bus Powered) MaxPower 0mA Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 0 bAlternateSetting 0 bNumEndpoints 1 bInterfaceClass 2 Communications bInterfaceSubClass 2 Abstract (modem) bInterfaceProtocol 1 AT-commands (v.25ter) iInterface 1 (error) CDC Header: bcdCDC 1.10 CDC ACM: bmCapabilities 0x0f connection notifications sends break line coding and serial state get/set/clear comm features CDC Union: bMasterInterface 0 bSlaveInterface 1 CDC Call Management: bmCapabilities 0x03 call management use DataInterface bDataInterface 1 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x84 EP 4 IN bmAttributes 3 Transfer Type Interrupt Synch Type None Usage Type Data wMaxPacketSize 0x0040 1x 64 bytes bInterval 1 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 1 bAlternateSetting 0 bNumEndpoints 2 bInterfaceClass 10 CDC Data bInterfaceSubClass 0 Unused bInterfaceProtocol 0 iInterface 2 (error) Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes 2 Transfer Type Bulk Synch Type None Usage Type Data wMaxPacketSize 0x0200 1x 512 bytes bInterval 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x01 EP 1 OUT bmAttributes 2 Transfer Type Bulk Synch Type None Usage Type Data wMaxPacketSize 0x0200 1x 512 bytes bInterval 0 Device Status: 0x7410 (Bus Powered) Bus 001 Device 002: ID 0e8d:2000 MediaTek Inc. MT65xx Preloader Device Descriptor: bLength 18 bDescriptorType 1 bcdUSB 2.00 bDeviceClass 2 Communications bDeviceSubClass 0 bDeviceProtocol 0 bMaxPacketSize0 64 idVendor 0x0e8d MediaTek Inc. idProduct 0x2000 MT65xx Preloader bcdDevice 1.00 iManufacturer 1 (error) iProduct 2 (error) iSerial 0 bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 70 bNumInterfaces 2 bConfigurationValue 1 iConfiguration 3 (error) bmAttributes 0xc0 Self Powered MaxPower 500mA Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 1 bAlternateSetting 0 bNumEndpoints 2 bInterfaceClass 10 CDC Data bInterfaceSubClass 0 Unused bInterfaceProtocol 0 iInterface 4 (error) Endpoint Descriptor: bLength 8 bDescriptorType 5 bEndpointAddress 0x01 EP 1 OUT bmAttributes 2 Transfer Type Bulk Synch Type None Usage Type Data wMaxPacketSize 0x0200 1x 512 bytes bInterval 0 Endpoint Descriptor: bLength 8 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes 2 Transfer Type Bulk Synch Type None Usage Type Data wMaxPacketSize 0x0200 1x 512 bytes bInterval 0 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 0 bAlternateSetting 0 bNumEndpoints 1 bInterfaceClass 2 Communications bInterfaceSubClass 2 Abstract (modem) bInterfaceProtocol 1 AT-commands (v.25ter) iInterface 5 (error) CDC Header: bcdCDC 1.10 CDC ACM: bmCapabilities 0x0f connection notifications sends break line coding and serial state get/set/clear comm features CDC Union: bMasterInterface 1 bSlaveInterface 0 CDC Call Management: bmCapabilities 0x03 call management use DataInterface bDataInterface 0 Endpoint Descriptor: bLength 8 bDescriptorType 5 bEndpointAddress 0x83 EP 3 IN bmAttributes 3 Transfer Type Interrupt Synch Type None Usage Type Data wMaxPacketSize 0x0040 1x 64 bytes bInterval 16 Device Status: 0x13f0 (Bus Powered) Debug Mode Regards, Macpaul Lin
On 12/19/2018 10:16, Macpaul Lin wrote: . > Here comes the verbose lsusb -v dump. > > Bus 001 Device 003: ID 0e8d:0003 MediaTek Inc. MT6227 phone > [432/3160] > Device Descriptor: > bLength 18 > bDescriptorType 1 > bcdUSB 1.10 > bDeviceClass 2 Communications > bDeviceSubClass 0 > bDeviceProtocol 0 > bMaxPacketSize0 64 > idVendor 0x0e8d MediaTek Inc. > idProduct 0x0003 MT6227 phone > bcdDevice 1.00 > iManufacturer 0 > iProduct 0 > iSerial 0 > bNumConfigurations 1 > Configuration Descriptor: > bLength 9 > bDescriptorType 2 > wTotalLength 67 > bNumInterfaces 2 > bConfigurationValue 1 > iConfiguration 0 > bmAttributes 0x80 > (Bus Powered) > MaxPower 0mA > Interface Descriptor: > bLength 9 > bDescriptorType 4 > bInterfaceNumber 0 > bAlternateSetting 0 > bNumEndpoints 1 > bInterfaceClass 2 Communications > bInterfaceSubClass 2 Abstract (modem) > bInterfaceProtocol 1 AT-commands (v.25ter) > iInterface 1 (error) > CDC Header: > bcdCDC 1.10 > CDC ACM: > bmCapabilities 0x0f > connection notifications > sends break > line coding and serial state > get/set/clear comm features > CDC Union: > bMasterInterface 0 > bSlaveInterface 1 > CDC Call Management: > bmCapabilities 0x03 > call management > use DataInterface > bDataInterface 1 > Endpoint Descriptor: > bLength 7 > bDescriptorType 5 > bEndpointAddress 0x84 EP 4 IN > bmAttributes 3 > Transfer Type Interrupt > Synch Type None > Usage Type Data > wMaxPacketSize 0x0040 1x 64 bytes > bInterval 1 > Interface Descriptor: > bLength 9 > bDescriptorType 4 > bInterfaceNumber 1 > bAlternateSetting 0 > bNumEndpoints 2 > bInterfaceClass 10 CDC Data > bInterfaceSubClass 0 Unused > bInterfaceProtocol 0 > iInterface 2 (error) > Endpoint Descriptor: > bLength 7 > bDescriptorType 5 > bEndpointAddress 0x81 EP 1 IN > bmAttributes 2 > Transfer Type Bulk > Synch Type None > Usage Type Data > wMaxPacketSize 0x0200 1x 512 bytes > bInterval 0 > Endpoint Descriptor: > bLength 7 > bDescriptorType 5 > bEndpointAddress 0x01 EP 1 OUT > bmAttributes 2 > Transfer Type Bulk > Synch Type None > Usage Type Data > wMaxPacketSize 0x0200 1x 512 bytes > bInterval 0 > Device Status: 0x7410 > (Bus Powered) > Regards, > Macpaul Lin > Hi Macpaul, your verbose usb listing show me that Mediatek has made two different 0e8d:003 devices, see my verbose lsusb listing below. (Notice also the reverse order for cmd and data interfaces in it compared to yours). USB id's are intended to identify a device and its needs so there should never be more than one unique device per id. Fairphone FP-1, MT6227 (no CDC union !!!) MI_00 USB Single Port Bus 001 Device 004: ID 0e8d:0003 Device Descriptor: bLength 18 bDescriptorType 1 bcdUSB 1.10 bDeviceClass 2 Communications bDeviceSubClass 0 bDeviceProtocol 0 bMaxPacketSize0 8 idVendor 0x0e8d idProduct 0x0003 bcdDevice 0.01 iManufacturer 3 MediaTek Inc iProduct 4 SEATTLE iSerial 5 534574001004 bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 67 bNumInterfaces 2 bConfigurationValue 1 iConfiguration 0 bmAttributes 0x80 (Bus Powered) MaxPower 500mA Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 0 bAlternateSetting 0 bNumEndpoints 2 bInterfaceClass 10 Data bInterfaceSubClass 0 Unused bInterfaceProtocol 0 iInterface 1 6218B COM Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes 2 Transfer Type Bulk Synch Type None Usage Type Data wMaxPacketSize 0x0040 1x 64 bytes bInterval 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x01 EP 1 OUT bmAttributes 2 Transfer Type Bulk Synch Type None Usage Type Data wMaxPacketSize 0x0040 1x 64 bytes bInterval 0 Interface Descriptor: bLength 28 bDescriptorType 4 bInterfaceNumber 1 bAlternateSetting 0 bNumEndpoints 1 bInterfaceClass 2 Communications bInterfaceSubClass 2 Abstract (modem) bInterfaceProtocol 1 AT-commands (v.25ter) iInterface 2 6218B COM Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x83 EP 3 IN bmAttributes 3 Transfer Type Interrupt Synch Type None Usage Type Data wMaxPacketSize 0x0008 1x 8 bytes bInterval 1 Device Status: 0x0000 (Bus Powered) /Lars
On Wed, 2018-12-19 at 10:31 +0700, Lars Melin wrote: > On 12/19/2018 10:16, Macpaul Lin wrote: > > Hi Macpaul, > your verbose usb listing show me that Mediatek has made two different > 0e8d:003 devices, see my verbose lsusb listing below. > (Notice also the reverse order for cmd and data interfaces in it > compared to yours). > USB id's are intended to identify a device and its needs so there should > never be more than one unique device per id. > > > Fairphone FP-1, MT6227 (no CDC union !!!) > Hi Lars, Ha ha ha, it is a little bit embarrassing. What I've used to capture verbose log is MT6765 platform. Then I've checked Fairphone FP-1, which is MT6589 a pretty old platform. The BROM (boot ROM) has been maintained by other teams and will vary by different SoC project in Mediatek. I'm not sure why they changed the descriptors. For the consistency of BROM's behavior, I'll update a new patch keeps PID:0003 remain untouched. I'll trying to report it to BROM team and see if they have any action on this issue. Regards, Macpaul Lin
On Mi, 2018-12-19 at 12:03 +0800, Macpaul Lin wrote: > On Wed, 2018-12-19 at 10:31 +0700, Lars Melin wrote: > > On 12/19/2018 10:16, Macpaul Lin wrote: > > > > Hi Macpaul, > > your verbose usb listing show me that Mediatek has made two different > > 0e8d:003 devices, see my verbose lsusb listing below. > > (Notice also the reverse order for cmd and data interfaces in it > > compared to yours). > > USB id's are intended to identify a device and its needs so there should > > never be more than one unique device per id. > > > > > > Fairphone FP-1, MT6227 (no CDC union !!!) > > > > Hi Lars, > > Ha ha ha, it is a little bit embarrassing. > What I've used to capture verbose log is MT6765 platform. > Then I've checked Fairphone FP-1, which is MT6589 a pretty old platform. > The BROM (boot ROM) has been maintained by other teams and will vary by > different SoC project in Mediatek. I'm not sure why they changed the > descriptors. > > For the consistency of BROM's behavior, I'll update a new patch keeps > PID:0003 remain untouched. I'll trying to report it to BROM team and see > if they have any action on this issue. Thank you all for taking care of these important issues. Please submit that new patch. Regards Oliver
On Wed, Dec 19, 2018 at 09:56:22AM +0100, Oliver Neukum wrote: > On Mi, 2018-12-19 at 12:03 +0800, Macpaul Lin wrote: > > On Wed, 2018-12-19 at 10:31 +0700, Lars Melin wrote: > > > On 12/19/2018 10:16, Macpaul Lin wrote: > > > > > > Hi Macpaul, > > > your verbose usb listing show me that Mediatek has made two different > > > 0e8d:003 devices, see my verbose lsusb listing below. > > > (Notice also the reverse order for cmd and data interfaces in it > > > compared to yours). > > > USB id's are intended to identify a device and its needs so there should > > > never be more than one unique device per id. > > > > > > > > > Fairphone FP-1, MT6227 (no CDC union !!!) > > > > > > > Hi Lars, > > > > Ha ha ha, it is a little bit embarrassing. > > What I've used to capture verbose log is MT6765 platform. > > Then I've checked Fairphone FP-1, which is MT6589 a pretty old platform. > > The BROM (boot ROM) has been maintained by other teams and will vary by > > different SoC project in Mediatek. I'm not sure why they changed the > > descriptors. > > > > For the consistency of BROM's behavior, I'll update a new patch keeps > > PID:0003 remain untouched. I'll trying to report it to BROM team and see > > if they have any action on this issue. > > Thank you all for taking care of these important issues. > Please submit that new patch. Can you review v7 please? thanks, greg k-h
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c index 1b68fed..336cf13 100644 --- a/drivers/usb/class/cdc-acm.c +++ b/drivers/usb/class/cdc-acm.c @@ -581,6 +581,13 @@ static int acm_tty_install(struct tty_driver *driver, struct tty_struct *tty) if (retval) goto error_init_termios; + /* + * Suppress initial echoing for some devices which might send data + * immediately after acm driver has been installed. + */ + if (acm->quirks & DISABLE_ECHO) + tty->termios.c_lflag &= ~ECHO; + tty->driver_data = acm; return 0; @@ -1655,7 +1662,10 @@ static int acm_pre_reset(struct usb_interface *intf) .driver_info = NO_UNION_NORMAL, /* has no union descriptor */ }, { USB_DEVICE(0x0e8d, 0x0003), /* FIREFLY, MediaTek Inc; andrey.arapov@gmail.com */ - .driver_info = NO_UNION_NORMAL, /* has no union descriptor */ + .driver_info = DISABLE_ECHO, /* DISABLE ECHO in termios flag */ + }, + { USB_DEVICE(0x0e8d, 0x2000), /* FIREFLY, MediaTek Inc; Preloader */ + .driver_info = DISABLE_ECHO, /* DISABLE ECHO in termios flag */ }, { USB_DEVICE(0x0e8d, 0x3329), /* MediaTek Inc GPS */ .driver_info = NO_UNION_NORMAL, /* has no union descriptor */ diff --git a/drivers/usb/class/cdc-acm.h b/drivers/usb/class/cdc-acm.h index ca06b20..515aad0 100644 --- a/drivers/usb/class/cdc-acm.h +++ b/drivers/usb/class/cdc-acm.h @@ -140,3 +140,4 @@ struct acm { #define QUIRK_CONTROL_LINE_STATE BIT(6) #define CLEAR_HALT_CONDITIONS BIT(7) #define SEND_ZERO_PACKET BIT(8) +#define DISABLE_ECHO BIT(9)
Mediatek Preloader is a proprietary embedded boot loader for loading Little Kernel and Linux into device DRAM. This boot loader also handle firmware update. Mediatek Preloader will be enumerated as a virtual COM port when the device is connected to Windows or Linux OS via CDC-ACM class driver. When the USB enumeration has been done, Mediatek Preloader will send out handshake command "READY" to PC actively instead of waiting command from the download tool. Since Linux 4.12, the commit "tty: reset termios state on device registration" (93857edd9829e144acb6c7e72d593f6e01aead66) causes Mediatek Preloader receiving some abnoraml command like "READYXX" as it sent. This will be recognized as an incorrect response. The behavior change also causes the download handshake fail. This change only affects subsequent connects if the reconnected device happens to get the same minor number. By disabling the ECHO termios flag could avoid this problem. However, it cannot be done by user space configuration when download tool open /dev/ttyACM0. This is because the device running Mediatek Preloader will send handshake command "READY" immediately once the CDC-ACM driver is ready. This patch wants to fix above problem by introducing "DISABLE_ECHO" property in driver_info. When Mediatek Preloader is connected, the CDC-ACM driver could disable ECHO flag in termios to avoid the problem. Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com> Cc: stable@vger.kernel.org --- Changes for v2: - Move quirks testing of DISABLE_ECHO flag into acm_tty_install(). - Change quirks testing into bitwise comparison. Changes for v3: - Replace quirks testing from init_termios to tty->termios. - Remove parenthesis for ECHO flag. Changes for v4: - Drop quirks varible to simplify the patch. - Move termios operation right after the driver_data has been installed. - Write general style comment for suppressing initial echoing. Changes for v5: - Fix: termios operation right abover the driver_data has been installed. - Update commit comment about this patch affects the reconnected device which get the same minor numbers. drivers/usb/class/cdc-acm.c | 12 +++++++++++- drivers/usb/class/cdc-acm.h | 1 + 2 files changed, 12 insertions(+), 1 deletion(-)