Re: [PATCH] USB:serial:pl2303:Add new PID to support PL2303HXN (TYPE_HXN)
On Wed, Feb 13, 2019 at 08:30:00PM +0800, Charles Yeh wrote: > Prolific has developed a new USB to UART chip: PL2303HXN > (PL2303GC/PL2303GS/PL2303GT/PL2303GL/PL2303GE) Ok, let's get back to this one. First a general comment; please make sure to address all review comments. I've already pointed out some issues that still hasn't been fixed and I've asked questions that have gone unanswered. If you disagree on something then just say so, but ignoring feedback is just going to make this take longer than necessary. For a start, please fix up the Subject line as we already discussed, and make sure to wrap your commit messages at 72 columns or so (I've reflown the rest of the message below). Always include a changelog (below the cut-off line) when resending so we know what changed when you update your patches. > The Vendor request used by the PL2303HXN (TYPE_HXN) is different from > the existing PL2303 series (TYPE_HX & TYPE_01). > Therefore, different Vendor requests are used to issue related commands. > > 1. Added a new TYPE_HXN type in pl2303_type_data, and then executes > new Vendor request, new flow control and other related instructions if > TYPE_HXN is recognized. > > 2. Because the new PL2303HXN can only accept the new Vendor request, > the old Vendor request cannot be accepted (the error message will be > returned) So first determine the TYPE_HX or TYPE_HXN through > TYPE_HX_READ_STATUS_REG in pl2303_startup. > > 2.1 If the return message is "1", then the PL2303 is the existing > TYPE_HX/ TYPE_01 series. The other settings in pl2303_startup are > to continue execution. > > 2.2 If the return message is "not 1", then the PL2303 is the new > TYPE_HXN series. The other settings in pl2303_startup are ignored. > (PL2303HXN will directly use the default value in the hardware, no > need to add additional settings through the software) > > 3. In pl2303_open: Because TYPE_HXN is different from the instruction > of down/up stream used by TYPE_HX. Therefore, we will also execute > different instructions here. > > 4. In pl2303_set_termios: The UART flow control instructions used by > TYPE_HXN/TYPE_HX/TYPE_01 are different. Therefore, we will also > execute different instructions here. > > 5. In pl2303_vendor_read & pl2303_vendor_write, since TYPE_HXN is > different from the vendor request instruction used by TYPE_HX/TYPE_01, > it will also execute different instructions here. That's a good summary of the differences, but on a more general level, can you confirm the following: 1. The HXN register layout is entirely different from HX and earlier devices. 2. HXN use the same CDC class requests (line encoding, etc) as earlier revisions. Can you send me documentation for the HXN protocol? That would help a lot in finding the right abstraction level for this. > Signed-off-by: Charles Yeh > --- > drivers/usb/serial/pl2303.c | 131 +--- > drivers/usb/serial/pl2303.h | 7 ++ > 2 files changed, 113 insertions(+), 25 deletions(-) > > diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c > index bb3f9aa4a909..d7d557e01390 100644 > --- a/drivers/usb/serial/pl2303.c > +++ b/drivers/usb/serial/pl2303.c > @@ -47,6 +47,12 @@ static const struct usb_device_id id_table[] = { > { USB_DEVICE(PL2303_VENDOR_ID, PL2303_PRODUCT_ID_MOTOROLA) }, > { USB_DEVICE(PL2303_VENDOR_ID, PL2303_PRODUCT_ID_ZTEK) }, > { USB_DEVICE(PL2303_VENDOR_ID, PL2303_PRODUCT_ID_TB) }, > + { USB_DEVICE(PL2303_VENDOR_ID, PL2303G_PRODUCT_ID_GC) }, > + { USB_DEVICE(PL2303_VENDOR_ID, PL2303G_PRODUCT_ID_GB) }, > + { USB_DEVICE(PL2303_VENDOR_ID, PL2303G_PRODUCT_ID_GT) }, > + { USB_DEVICE(PL2303_VENDOR_ID, PL2303G_PRODUCT_ID_GL) }, > + { USB_DEVICE(PL2303_VENDOR_ID, PL2303G_PRODUCT_ID_GE) }, > + { USB_DEVICE(PL2303_VENDOR_ID, PL2303G_PRODUCT_ID_GS) }, > { USB_DEVICE(IODATA_VENDOR_ID, IODATA_PRODUCT_ID) }, > { USB_DEVICE(IODATA_VENDOR_ID, IODATA_PRODUCT_ID_RSAQ5) }, > { USB_DEVICE(ATEN_VENDOR_ID, ATEN_PRODUCT_ID), > @@ -129,9 +135,11 @@ MODULE_DEVICE_TABLE(usb, id_table); > > #define VENDOR_WRITE_REQUEST_TYPE0x40 > #define VENDOR_WRITE_REQUEST 0x01 > +#define VENDOR_WRITE_NREQUEST0x80 > > #define VENDOR_READ_REQUEST_TYPE 0xc0 > #define VENDOR_READ_REQUEST 0x01 > +#define VENDOR_READ_NREQUEST 0x81 > > #define UART_STATE_INDEX 8 > #define UART_STATE_MSR_MASK 0x8b > @@ -145,11 +153,30 @@ MODULE_DEVICE_TABLE(usb, id_table); > #define UART_OVERRUN_ERROR 0x40 > #define UART_CTS 0x80 > > +#define TYPE_HX_READ_STATUS_REG 0x8080 > +#define TYPE_HXN_FLOWCONTROL_REG 0x0A > +#define TYPE_HXN_HARDWAREFLOW_DATA 0xFA > +#define TYPE_HXN_SOFTWAREFLOW_DATA 0xEE > +#define TYPE_HXN_NOFLOW_DATA 0xFF What exactly does bits 0x15 (bits 4, 2 and 0) do? Is register 0x0
Re: [PATCH] [v2]USB:serial:pl2303:add new Pull-Up mode to support PL2303HXD (TYPE_HX)
On Tue, Feb 12, 2019 at 08:50:49PM +0800, Charles Yeh wrote: > Pull-Up mode is disabled (default) in PL2303HXD. > When the Pull-Up mode is activated, its TX/DTR/RTS external resistor will > start the output function. > > How to enable the Pull-Up mode of PL2303HXD > 1.TX/DTR/RTS external resistor is required on the circuit diagram (PCB) > 2.PL2303HXD OTP needs to be programmed to have a Pull-Up mode through 6.5V > (USB_VCC,5V->6.5V) > > The patch driver will read whether the PL2303HXD has a Pull-Up mode,and if so, > it will be set to enable Pull-Up mode function. > > Signed-off-by: Charles Yeh > --- > drivers/usb/serial/pl2303.c | 24 > 1 file changed, 24 insertions(+) > > diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c > index bb3f9aa4a909..e5d00e4a495d 100644 > --- a/drivers/usb/serial/pl2303.c > +++ b/drivers/usb/serial/pl2303.c > @@ -145,6 +145,16 @@ MODULE_DEVICE_TABLE(usb, id_table); > #define UART_OVERRUN_ERROR 0x40 > #define UART_CTS 0x80 > > +#define TYPE_HX_READ_PUM_STATUS_REG 0x8484 > +#define TYPE_HX_READ_PUM_ADD0x0404 > +#define TYPE_HX_READ_PUM_DATA_REG 0x8383 Again, the defines need to reflect what the registers are for, not what use happen to use them for in on specific code path. These are used to access the EEPROM/OTP, right? > +#define TYPE_HX_PULLUP_MODE_DATA0x08 What is bit 0x08? > +#define TYPE_HX_PULLUP_MODE_REG 0x09 And this register isn't just used for pull-up mode as far as I can tell. > +#define TYPE_HX_PUM_ADD00x00 > +#define TYPE_HX_PUM_DATA0 0x31 > +#define TYPE_HX_PUM_ADD10x01 > +#define TYPE_HX_PUM_DATA1 0x08 Same for these, we don't want multiple defines for register 0 for example. What are bits 0x31 of register 0? > + > static void pl2303_set_break(struct usb_serial_port *port, bool enable); > > enum pl2303_type { > @@ -688,6 +698,20 @@ static void pl2303_set_termios(struct tty_struct *tty, > pl2303_vendor_write(serial, 0x0, 0x0); > } > > + if (spriv->type == &pl2303_type_data[TYPE_HX]) { > + pl2303_vendor_read(serial, TYPE_HX_READ_PUM_STATUS_REG, buf); > + pl2303_vendor_write(serial, TYPE_HX_READ_PUM_ADD, > + TYPE_HX_PULLUP_MODE_REG); > + pl2303_vendor_read(serial, TYPE_HX_READ_PUM_STATUS_REG, buf); > + pl2303_vendor_read(serial, TYPE_HX_READ_PUM_DATA_REG, buf); Why do you need to access the OTP on every set_termios() call? I thought those settings where copied to the corresponding control registers during boot? > + if (*buf == TYPE_HX_PULLUP_MODE_DATA) { Don't you want to just check bit 0x80 here? > + pl2303_vendor_write(serial, TYPE_HX_PUM_ADD0, > + TYPE_HX_PUM_DATA0); So this looks broken since you're overwriting the flow control settings that were just set above. And again, what are bits 0x31 of register 0? Doesn't 0x30 control auto-rts? > + pl2303_vendor_write(serial, TYPE_HX_PUM_ADD1, > + TYPE_HX_PUM_DATA1); And why do you need to (over-)write register 2? Either way, configuring pull-up mode should be done at probe and not on every set_termios() call. > + } > + } > + > kfree(buf); > } Thanks, Johan
Re: [PATCH] usb: typec: Registering real device entries for the muxes
Hi Heikki, I love your patch! Perhaps something to improve: [auto build test WARNING on usb/usb-testing] [also build test WARNING on v5.1-rc3 next-20190401] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Heikki-Krogerus/usb-typec-Registering-real-device-entries-for-the-muxes/20190402-030003 base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing reproduce: make htmldocs All warnings (new ones prefixed by >>): WARNING: convert(1) not found, for SVG to PDF conversion install ImageMagick (https://www.imagemagick.org) include/linux/generic-radix-tree.h:1: warning: no structured comments found kernel/rcu/tree_plugin.h:1: warning: no structured comments found kernel/rcu/tree_plugin.h:1: warning: no structured comments found include/linux/firmware/intel/stratix10-svc-client.h:1: warning: no structured comments found include/linux/gpio/driver.h:371: warning: Function parameter or member 'init_valid_mask' not described in 'gpio_chip' include/linux/i2c.h:343: warning: Function parameter or member 'init_irq' not described in 'i2c_client' include/linux/iio/hw-consumer.h:1: warning: no structured comments found include/linux/input/sparse-keymap.h:46: warning: Function parameter or member 'sw' not described in 'key_entry' include/linux/regulator/machine.h:199: warning: Function parameter or member 'max_uV_step' not described in 'regulation_constraints' include/linux/regulator/driver.h:228: warning: Function parameter or member 'resume' not described in 'regulator_ops' drivers/slimbus/stream.c:1: warning: no structured comments found include/linux/spi/spi.h:188: warning: Function parameter or member 'driver_override' not described in 'spi_device' drivers/target/target_core_device.c:1: warning: no structured comments found >> drivers/usb/typec/mux.c:106: warning: Function parameter or member 'parent' >> not described in 'typec_switch_register' >> drivers/usb/typec/mux.c:106: warning: Function parameter or member 'desc' >> not described in 'typec_switch_register' drivers/usb/typec/mux.c:106: warning: Excess function parameter 'sw' description in 'typec_switch_register' >> drivers/usb/typec/mux.c:281: warning: Function parameter or member 'parent' >> not described in 'typec_mux_register' >> drivers/usb/typec/mux.c:281: warning: Function parameter or member 'desc' >> not described in 'typec_mux_register' drivers/usb/typec/mux.c:281: warning: Excess function parameter 'mux' description in 'typec_mux_register' drivers/usb/typec/bus.c:1: warning: no structured comments found drivers/usb/typec/class.c:1: warning: no structured comments found include/linux/w1.h:281: warning: Function parameter or member 'of_match_table' not described in 'w1_family' fs/direct-io.c:257: warning: Excess function parameter 'offset' description in 'dio_complete' fs/file_table.c:1: warning: no structured comments found fs/libfs.c:477: warning: Excess function parameter 'available' description in 'simple_write_end' fs/posix_acl.c:646: warning: Function parameter or member 'inode' not described in 'posix_acl_update_mode' fs/posix_acl.c:646: warning: Function parameter or member 'mode_p' not described in 'posix_acl_update_mode' fs/posix_acl.c:646: warning: Function parameter or member 'acl' not described in 'posix_acl_update_mode' drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:294: warning: Excess function parameter 'mm' description in 'amdgpu_mn_invalidate_range_start_hsa' drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:294: warning: Excess function parameter 'start' description in 'amdgpu_mn_invalidate_range_start_hsa' drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:294: warning: Excess function parameter 'end' description in 'amdgpu_mn_invalidate_range_start_hsa' drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:343: warning: Excess function parameter 'mm' description in 'amdgpu_mn_invalidate_range_end' drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:343: warning: Excess function parameter 'start' description in 'amdgpu_mn_invalidate_range_end' drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:343: warning: Excess function parameter 'end' description in 'amdgpu_mn_invalidate_range_end' drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c:183: warning: Function parameter or member 'blockable' not described in 'amdgpu_mn_re
Re: Mass Storage Gadget Device Falls from SuperSpeed to High Speed
Hi Felipe, On Tue, Apr 02, 2019 at 08:46:16AM +0300, Felipe Balbi wrote: > Felipe Balbi writes: > >>> modphy is the USB PHY integrated in your SoC. There's no control for > >>> that from OS side, only BIOS unfortunately. There is, however, one thing > >>> we can try. DWC3 has several quirk flags for known quirky PHYs; perhaps > >>> CHT needs one of those. Can you try with this patch and let me know > >>> whether it helps? > >> > >> Sure thing, I will try tomorrow. Could you possibly explain what a quirk > >> is as it relates to the kernel? I see this all over the source tree but > >> never knew how it was used. Does the dwc3 also know about "quirks" and > >> these particular flags? or are these flags just specific to the kernel > >> and its functionality? > >> > >>> modified drivers/usb/dwc3/dwc3-pci.c > >>> @@ -105,6 +105,8 @@ static int dwc3_byt_enable_ulpi_refclock(struct > >>> pci_dev *pci) > >>> static const struct property_entry dwc3_pci_intel_properties[] = { > >>> PROPERTY_ENTRY_STRING("dr_mode", "peripheral"), > >>> PROPERTY_ENTRY_BOOL("linux,sysdev_is_parent"), > >>> + PROPERTY_ENTRY_BOOL("snps,dis_u3_susphy_quirk"), > >>> + PROPERTY_ENTRY_BOOL("snps,dis_u2_susphy_quirk"), > >>> {} > >>> }; > >>> > >>> These two quirks will PHY suspend. There are other relevant quirk flags > > > > will DISABLE phy suspend :-) > > > >> What do you mean by PHY suspend? Will it disable U2/U3 for the dwc3? I > > > > No, no. DWC3 can still enter U1/U2/U3, but the PHY will not enter the > > matching P1/P2/P3 state. > > > >> see it modified the DWC3_GUSB2PHYCFG_SUSPHY bit in the configuration > >> register, but I don't have access to the dwc3 databook to dig deeper > >> into this. > > > > The second quirk flag (snps,dis_u2_susphy_quirk) will tell dwc3 to *NOT* > > request USB2 PHY to enter low power state. While the first flag > > (snps,dis_u3_susphy_quirk) will tell dwc3 to *NOT* request USB3 PHY to > > enter low power state. > > > > In reality, they are a single block of HW but they _can_ be different > > and even if they are a single block, they _can_ have separate clock > > domains and we _may_ be able to control them separately. I haven't read > > documentation for modphy because the OS doesn't touch that. If > > necessary, I can try to find out more details about it, but I will > > probably take some time to find the correct documentation. > > > >>> which we can try in case these two don't help. I'd like to figure out > >>> exactly which quirk flag helps (if any). After that, we would need to > >>> check if a similar problem happens on any CHT system or just your > >>> design. > >>> > >>> If it happens on any other system, then I can make sure we add a quirk > >>> flag to all CHTs. > >> > >> Sounds good! > >> > >> Thanks for taking the time to answer my questions! It's definitely > >> helpful for my understanding of USB. I'm learning quite a bit of > >> new information with each email and it's pretty awesome. > > > > No problems at all, happy to help. > > Any updates here? Hopefully the quirk flags above helped. Thanks for following up. My team and I were doing a bunch of testing today as well as this past weekend. We are still seeing mixed results with the suggestion you provided to us earlier. I unfortunately do not have any logs, traces, or analyzer captures to back this up yet, but I hope to gather more concrete data tomorrow. There were two main issues we observed with the quirk flags above: * Host mode no longer worked with these quirks applied. Please keep in mind that we backported part of a patchset [1] to support switching the internal SS mux from the xHCI controller to the xDCI controller. At a quick glance, do you see anything in this patchset that you think might cause a problem with the role switch driver communicating with the xHCI controller when the quirks are applied? * We still saw the drop from SS to HS on a Windows laptop. Linux and Mac seemed to be fine, but there's definitely a chance we did not test with enough samples to confidently say this quirk is stable with those hosts. So tomorrow I hope to gather more concrete data to back up these findings. Our USB protocol analyzer is also out-of-order at the moment, so we might not be able to get analyzer data for a day or two either. Are there some other quirks you would like us to try out? Over the weekend, one of our team members was hacking in the dwc3 driver to try and disable LPM at various points in the chain of events. I've attached a patch that we believe is pretty stable. We have never seen it drop from SS to HS. USB type A to type C connections from any host to our device seem pretty stable. I've attached an archive called "gnarbox-success-a-to-c.tar.xz" That contains the trace events for a successful run where we connect the device to a Windows host and it enumerates at SuperSpeed. This patch isn't entirely stable as I haven't been able to get a
Re: problem with Synopsys DesignWare Core SuperSpeed USB driver
Hi, On 4/1/2019 9:01 PM, Felipe Balbi wrote: > > Hi, > > (I can't answer private queries. Please, always Cc the public mailing > list) > > Antonio Santagiuliana writes: > >> Hello >> I am using RaspberryPI for a project and we want to connect 16 USB to UART >> bridges ( at the moment we are using 8 dual UART cp2105 from Silicon Labs ). >> I have seen your page at : >> https://urldefense.proofpoint.com/v2/url?u=https-3A__dri.freedesktop.org_docs_drm_driver-2Dapi_usb_dwc3.html&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=cQBKt4q-qzNVC53rNAwuwplH23V61rHQhhULvdLA0U8&m=RfK-uhEeZZ8yf79ch5you3BpE1JNPDHRzgmOGkeLBss&s=rGrgFKpJ6-CEmAtR-hiae64T80Z7oCTx3r4xHn-lf4I&e= >> so I was trying to get tracefs output. >> Problem is that on the /t/events subfolder I cannot find the >> dwc3/enable because >> there is not any /dwc3 in /t/events. >> Because of this I cannot set echo 1 > /t/events/dwc3/enable >> as instead your page says I should do. >> I can only find */t/events/enable* should I activate that ? >> running usb-devices command I get this about the drivers loaded : >> >> T: Bus=01 Lev=00 Prnt=00 Port=00 Cnt=00 Dev#= 1 Spd=480 MxCh= 1 >> D: Ver= 2.00 Cls=09(hub ) Sub=00 Prot=01 MxPS=64 #Cfgs= 1 >> P: Vendor=1d6b ProdID=0002 Rev=04.14 >> S: Manufacturer=Linux 4.14.39 dwc_otg_hcd > > You're not using dwc3. That's dwc2, instead. Also, dwc3 tracepoints are > peripheral side only. Host side is a standard xhci, so you would use > xhci tracepoints for that. > > Still, you're using dwc2 from what I can tell. Minas is the maintainer > of that driver. > > It's not dwc2 driver. It's dwc_otg Synopsys reference driver which supplying along with HSOTG core. But what the issue? > > > > >> S: Product=DWC OTG Controller >> S: SerialNumber=3f98.usb >> C: #Ifs= 1 Cfg#= 1 Atr=e0 MxPwr=0mA >> I: If#=0x0 Alt= 0 #EPs= 1 Cls=09(hub ) Sub=00 Prot=00 Driver=hub >> >> T: Bus=01 Lev=01 Prnt=01 Port=00 Cnt=01 Dev#= 2 Spd=480 MxCh= 5 >> D: Ver= 2.00 Cls=09(hub ) Sub=00 Prot=02 MxPS=64 #Cfgs= 1 >> P: Vendor=0424 ProdID=9514 Rev=02.00 >> C: #Ifs= 1 Cfg#= 1 Atr=e0 MxPwr=2mA >> I: If#=0x0 Alt= 1 #EPs= 1 Cls=09(hub ) Sub=00 Prot=02 Driver=hub >> >> T: Bus=01 Lev=02 Prnt=02 Port=00 Cnt=01 Dev#= 3 Spd=480 MxCh= 0 >> D: Ver= 2.00 Cls=ff(vend.) Sub=00 Prot=01 MxPS=64 #Cfgs= 1 >> P: Vendor=0424 ProdID=ec00 Rev=02.00 >> C: #Ifs= 1 Cfg#= 1 Atr=e0 MxPwr=2mA >> I: If#=0x0 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=ff Driver=smsc95xx >> >> T: Bus=01 Lev=02 Prnt=02 Port=01 Cnt=02 Dev#= 14 Spd=480 MxCh= 4 >> D: Ver= 2.00 Cls=09(hub ) Sub=00 Prot=01 MxPS=64 #Cfgs= 1 >> P: Vendor=0409 ProdID=005a Rev=01.00 >> C: #Ifs= 1 Cfg#= 1 Atr=e0 MxPwr=100mA >> I: If#=0x0 Alt= 0 #EPs= 1 Cls=09(hub ) Sub=00 Prot=00 Driver=hub >> >> T: Bus=01 Lev=03 Prnt=14 Port=00 Cnt=01 Dev#= 15 Spd=12 MxCh= 0 >> D: Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs= 1 >> P: Vendor=10c4 ProdID=ea70 Rev=01.00 >> S: Manufacturer=Silicon Labs >> S: Product=CP2105 Dual USB to UART Bridge Controller >> S: SerialNumber=008714B9 >> C: #Ifs= 2 Cfg#= 1 Atr=80 MxPwr=100mA >> I: If#=0x0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=00 Prot=00 Driver=vcpsilabs >> I: If#=0x1 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=00 Prot=00 Driver=vcpsilabs >> >> T: Bus=01 Lev=03 Prnt=14 Port=01 Cnt=02 Dev#= 16 Spd=12 MxCh= 0 >> D: Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs= 1 >> P: Vendor=10c4 ProdID=ea70 Rev=01.00 >> S: Manufacturer=Silicon Labs >> S: Product=CP2105 Dual USB to UART Bridge Controller >> S: SerialNumber=0086F956 >> C: #Ifs= 2 Cfg#= 1 Atr=80 MxPwr=100mA >> I: If#=0x0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=00 Prot=00 Driver=vcpsilabs >> I: If#=0x1 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=00 Prot=00 Driver=vcpsilabs >> >> T: Bus=01 Lev=03 Prnt=14 Port=02 Cnt=03 Dev#= 17 Spd=12 MxCh= 0 >> D: Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs= 1 >> P: Vendor=10c4 ProdID=ea70 Rev=01.00 >> S: Manufacturer=Silicon Labs >> S: Product=CP2105 Dual USB to UART Bridge Controller >> S: SerialNumber=00870FBD >> C: #Ifs= 2 Cfg#= 1 Atr=80 MxPwr=100mA >> I: If#=0x0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=00 Prot=00 Driver=vcpsilabs >> I: If#=0x1 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=00 Prot=00 Driver=vcpsilabs >> >> T: Bus=01 Lev=03 Prnt=14 Port=03 Cnt=04 Dev#= 18 Spd=480 MxCh= 4 >> D: Ver= 2.00 Cls=09(hub ) Sub=00 Prot=01 MxPS=64 #Cfgs= 1 >> P: Vendor=0409 ProdID=005a Rev=01.00 >> C: #Ifs= 1 Cfg#= 1 Atr=e0 MxPwr=100mA >> I: If#=0x0 Alt= 0 #EPs= 1 Cls=09(hub ) Sub=00 Prot=00 Driver=hub >> >> T: Bus=01 Lev=04 Prnt=18 Port=00 Cnt=01 Dev#= 19 Spd=12 MxCh= 0 >> D: Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs= 1 >> P: Vendor=10c4 ProdID=ea70 Rev=01.00 >> S: Manufacturer=Silicon Labs >> S: Product=CP2105 Dual USB to UART Bridge Controller >> S: SerialNumber=00870E54 >> C: #Ifs= 2 Cfg#= 1 Atr=80 MxPwr=100mA >> I: If#=0x0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=00 Prot=00 Driver=vcpsilabs >> I: If#=0x1 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=00 Prot=00 Driver=vcpsilabs >> >
Re: Mass Storage Gadget Device Falls from SuperSpeed to High Speed
Hi Rob, Rob Weber writes: > On Tue, Apr 02, 2019 at 08:46:16AM +0300, Felipe Balbi wrote: >> Felipe Balbi writes: >> >>> modphy is the USB PHY integrated in your SoC. There's no control for >> >>> that from OS side, only BIOS unfortunately. There is, however, one thing >> >>> we can try. DWC3 has several quirk flags for known quirky PHYs; perhaps >> >>> CHT needs one of those. Can you try with this patch and let me know >> >>> whether it helps? >> >> >> >> Sure thing, I will try tomorrow. Could you possibly explain what a quirk >> >> is as it relates to the kernel? I see this all over the source tree but >> >> never knew how it was used. Does the dwc3 also know about "quirks" and >> >> these particular flags? or are these flags just specific to the kernel >> >> and its functionality? >> >> >> >>> modified drivers/usb/dwc3/dwc3-pci.c >> >>> @@ -105,6 +105,8 @@ static int dwc3_byt_enable_ulpi_refclock(struct >> >>> pci_dev *pci) >> >>> static const struct property_entry dwc3_pci_intel_properties[] = { >> >>> PROPERTY_ENTRY_STRING("dr_mode", "peripheral"), >> >>> PROPERTY_ENTRY_BOOL("linux,sysdev_is_parent"), >> >>> +PROPERTY_ENTRY_BOOL("snps,dis_u3_susphy_quirk"), >> >>> +PROPERTY_ENTRY_BOOL("snps,dis_u2_susphy_quirk"), >> >>> {} >> >>> }; >> >>> >> >>> These two quirks will PHY suspend. There are other relevant quirk flags >> > >> > will DISABLE phy suspend :-) >> > >> >> What do you mean by PHY suspend? Will it disable U2/U3 for the dwc3? I >> > >> > No, no. DWC3 can still enter U1/U2/U3, but the PHY will not enter the >> > matching P1/P2/P3 state. >> > >> >> see it modified the DWC3_GUSB2PHYCFG_SUSPHY bit in the configuration >> >> register, but I don't have access to the dwc3 databook to dig deeper >> >> into this. >> > >> > The second quirk flag (snps,dis_u2_susphy_quirk) will tell dwc3 to *NOT* >> > request USB2 PHY to enter low power state. While the first flag >> > (snps,dis_u3_susphy_quirk) will tell dwc3 to *NOT* request USB3 PHY to >> > enter low power state. >> > >> > In reality, they are a single block of HW but they _can_ be different >> > and even if they are a single block, they _can_ have separate clock >> > domains and we _may_ be able to control them separately. I haven't read >> > documentation for modphy because the OS doesn't touch that. If >> > necessary, I can try to find out more details about it, but I will >> > probably take some time to find the correct documentation. >> > >> >>> which we can try in case these two don't help. I'd like to figure out >> >>> exactly which quirk flag helps (if any). After that, we would need to >> >>> check if a similar problem happens on any CHT system or just your >> >>> design. >> >>> >> >>> If it happens on any other system, then I can make sure we add a quirk >> >>> flag to all CHTs. >> >> >> >> Sounds good! >> >> >> >> Thanks for taking the time to answer my questions! It's definitely >> >> helpful for my understanding of USB. I'm learning quite a bit of >> >> new information with each email and it's pretty awesome. >> > >> > No problems at all, happy to help. >> >> Any updates here? Hopefully the quirk flags above helped. > > Thanks for following up. My team and I were doing a bunch of testing no problem. > today as well as this past weekend. We are still seeing mixed results > with the suggestion you provided to us earlier. I unfortunately do not > have any logs, traces, or analyzer captures to back this up yet, but I > hope to gather more concrete data tomorrow. cool, thanks > There were two main issues we observed with the quirk flags above: > > * Host mode no longer worked with these quirks applied. Please keep in Interesting. This is rather unexpected. > mind that we backported part of a patchset [1] to support switching > the internal SS mux from the xHCI controller to the xDCI controller. > At a quick glance, do you see anything in this patchset that you think > might cause a problem with the role switch driver communicating with > the xHCI controller when the quirks are applied? nope, that shouldn't cause the behavior you're seeing. > * We still saw the drop from SS to HS on a Windows laptop. Linux and Mac > seemed to be fine, but there's definitely a chance we did not test > with enough samples to confidently say this quirk is stable with those > hosts. okay, so still not good enough. Let's try to prevent the device side from ever initiating U1/U2 entry and from ever accepting LGO_U1 and LGO_U2 from the host. Here's a (hack) patch for that modified drivers/usb/dwc3/ep0.c @@ -382,7 +382,7 @@ static int dwc3_ep0_handle_u1(struct dwc3 *dwc, enum usb_device_state state, reg = dwc3_readl(dwc->regs, DWC3_DCTL); if (set) - reg |= DWC3_DCTL_INITU1ENA; + return -EINVAL; else reg &= ~DWC3_DCTL_INITU1ENA; dwc3_writel(dwc->regs, DWC3_DCTL,
[PATCH 2/2] USB: serial: pl2303: fix tranceiver suspend mode
Add helper function to update register bits instead of overwriting the entire control register when updating the flow-control settings. This specifically avoids having the tranceiver suspend mode (bit 0) depend on the flow control setting. The tranceiver is currently configured at probe to be disabled during suspend, but this was overridden when disabling flow control or enabling xon/xoff. Fixes: 715f9527c1c1 ("USB: flow control fix for pl2303") Fixes: 7041d9c3f01b ("USB: serial: pl2303: add support for tx xon/xoff flow control") Signed-off-by: Johan Hovold --- drivers/usb/serial/pl2303.c | 35 ++- 1 file changed, 30 insertions(+), 5 deletions(-) diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c index e7ccd06df802..55122ac84518 100644 --- a/drivers/usb/serial/pl2303.c +++ b/drivers/usb/serial/pl2303.c @@ -145,6 +145,8 @@ MODULE_DEVICE_TABLE(usb, id_table); #define UART_OVERRUN_ERROR 0x40 #define UART_CTS 0x80 +#define PL2303_FLOWCTRL_MASK 0xf0 + static void pl2303_set_break(struct usb_serial_port *port, bool enable); enum pl2303_type { @@ -176,7 +178,7 @@ static const struct pl2303_type_data pl2303_type_data[TYPE_COUNT] = { [TYPE_01] = { .max_baud_rate = 1228800, .quirks = PL2303_QUIRK_LEGACY, - .no_autoxonxoff = 1, + .no_autoxonxoff = true, }, [TYPE_HX] = { .max_baud_rate = 1200, @@ -225,6 +227,29 @@ static int pl2303_vendor_write(struct usb_serial *serial, u16 value, u16 index) return 0; } +static int pl2303_update_reg(struct usb_serial *serial, u8 reg, u8 mask, u8 val) +{ + int ret = 0; + u8 *buf; + + buf = kmalloc(1, GFP_KERNEL); + if (!buf) + return -ENOMEM; + + ret = pl2303_vendor_read(serial, reg | 0x80, buf); + if (ret) + goto out_free; + + *buf &= ~mask; + *buf |= val & mask; + + ret = pl2303_vendor_write(serial, reg, *buf); +out_free: + kfree(buf); + + return ret; +} + static int pl2303_probe(struct usb_serial *serial, const struct usb_device_id *id) { @@ -694,13 +719,13 @@ static void pl2303_set_termios(struct tty_struct *tty, if (C_CRTSCTS(tty)) { if (spriv->quirks & PL2303_QUIRK_LEGACY) - pl2303_vendor_write(serial, 0x0, 0x41); + pl2303_update_reg(serial, 0, PL2303_FLOWCTRL_MASK, 0x40); else - pl2303_vendor_write(serial, 0x0, 0x61); + pl2303_update_reg(serial, 0, PL2303_FLOWCTRL_MASK, 0x60); } else if (pl2303_enable_xonxoff(tty, spriv->type)) { - pl2303_vendor_write(serial, 0x0, 0xc0); + pl2303_update_reg(serial, 0, PL2303_FLOWCTRL_MASK, 0xc0); } else { - pl2303_vendor_write(serial, 0x0, 0x0); + pl2303_update_reg(serial, 0, PL2303_FLOWCTRL_MASK, 0); } kfree(buf); -- 2.21.0
[PATCH 1/2] USB: serial: pl2303: fix non-supported xon/xoff
Older pl2303 devices do not support automatic xon/xoff flow control, so add add a flag to prevent trying to enable it for legacy device types. Refactor the IXON test into a helper function to improve readability. Fixes: 7041d9c3f01b ("USB: serial: pl2303: add support for tx xon/xoff flow control") Signed-off-by: Johan Hovold --- drivers/usb/serial/pl2303.c | 25 - 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c index bb3f9aa4a909..e7ccd06df802 100644 --- a/drivers/usb/serial/pl2303.c +++ b/drivers/usb/serial/pl2303.c @@ -156,6 +156,7 @@ enum pl2303_type { struct pl2303_type_data { speed_t max_baud_rate; unsigned long quirks; + unsigned int no_autoxonxoff:1; }; struct pl2303_serial_private { @@ -173,11 +174,12 @@ struct pl2303_private { static const struct pl2303_type_data pl2303_type_data[TYPE_COUNT] = { [TYPE_01] = { - .max_baud_rate =1228800, - .quirks = PL2303_QUIRK_LEGACY, + .max_baud_rate = 1228800, + .quirks = PL2303_QUIRK_LEGACY, + .no_autoxonxoff = 1, }, [TYPE_HX] = { - .max_baud_rate =1200, + .max_baud_rate = 1200, }, }; @@ -552,6 +554,20 @@ static bool pl2303_termios_change(const struct ktermios *a, const struct ktermio return tty_termios_hw_change(a, b) || ixon_change; } +static bool pl2303_enable_xonxoff(struct tty_struct *tty, const struct pl2303_type_data *type) +{ + if (!I_IXON(tty) || I_IXANY(tty)) + return false; + + if (START_CHAR(tty) != 0x11 || STOP_CHAR(tty) != 0x13) + return false; + + if (type->no_autoxonxoff) + return false; + + return true; +} + static void pl2303_set_termios(struct tty_struct *tty, struct usb_serial_port *port, struct ktermios *old_termios) { @@ -681,8 +697,7 @@ static void pl2303_set_termios(struct tty_struct *tty, pl2303_vendor_write(serial, 0x0, 0x41); else pl2303_vendor_write(serial, 0x0, 0x61); - } else if (I_IXON(tty) && !I_IXANY(tty) && START_CHAR(tty) == 0x11 && - STOP_CHAR(tty) == 0x13) { + } else if (pl2303_enable_xonxoff(tty, spriv->type)) { pl2303_vendor_write(serial, 0x0, 0xc0); } else { pl2303_vendor_write(serial, 0x0, 0x0); -- 2.21.0
[PATCH 0/2] USB: serial: pl2303: fix flow-control handling
These patches fix two issues with the current flow-control implementation. Tested using an HXD device. Johan Johan Hovold (2): USB: serial: pl2303: fix non-supported xon/xoff USB: serial: pl2303: fix tranceiver suspend mode drivers/usb/serial/pl2303.c | 58 +++-- 1 file changed, 49 insertions(+), 9 deletions(-) -- 2.21.0
Re: [PATCH 2/2] USB: serial: pl2303: fix tranceiver suspend mode
On Tue, Apr 02, 2019 at 10:19:31AM +0200, Johan Hovold wrote: > Add helper function to update register bits instead of overwriting the > entire control register when updating the flow-control settings. > > This specifically avoids having the tranceiver suspend mode (bit 0) > depend on the flow control setting. > > The tranceiver is currently configured at probe to be disabled during > suspend, but this was overridden when disabling flow control or enabling > xon/xoff. > > Fixes: 715f9527c1c1 ("USB: flow control fix for pl2303") > Fixes: 7041d9c3f01b ("USB: serial: pl2303: add support for tx xon/xoff flow > control") > Signed-off-by: Johan Hovold > --- > @@ -176,7 +178,7 @@ static const struct pl2303_type_data > pl2303_type_data[TYPE_COUNT] = { > [TYPE_01] = { > .max_baud_rate = 1228800, > .quirks = PL2303_QUIRK_LEGACY, > - .no_autoxonxoff = 1, > + .no_autoxonxoff = true, This chunk was supposed to go in the first patch. I'll fix that up before applying (or resending). Johan
Why there is a refcnt check when we change configfs property
Hi Felipe, Do you know why? In that case, I can't change property, eg changing qmult at NCM like below: mkdir functions/ncm.0 ln -s functions/ncm.0 configs/c.1 echo 10 > functions/ncm.0/qmult The code: drivers/usb/gadget/function/u_ether_configfs.h static ssize_t _f_##_opts_qmult_store(struct config_item *item, \ const char *page, size_t len)\ { \ struct f_##_f_##_opts *opts = to_f_##_f_##_opts(item); \ u8 val; \ int ret;\ \ mutex_lock(&opts->lock);\ if (opts->refcnt) { \ ret = -EBUSY; \ goto out; \ } \ \ ret = kstrtou8(page, 0, &val); \ if (ret)\ goto out; \ \ gether_set_qmult(opts->net, val); \ ret = len; \ out:\ mutex_unlock(&opts->lock); \ return ret; \ } \ \ CONFIGFS_ATTR(_f_##_opts_, qmult) Thanks. Best regards, Peter Chen
Re: drivers: usb: serial: ftdi_sio.c error flagging
Hi, and sorry about the late reply. On Fri, Mar 08, 2019 at 09:43:35AM -0700, egaugesyst...@gmail.com wrote: > [Resend with From address corrected. Sorry about that] > > Johan, > > Some of our customers are experiencing communication issues on RS485 > that could be solved quite nicely by turning on termios.PARMRK. I'd > be happy to go into details, but I don't think they're necessary for > the discussion below. > > The problem we encountered is that the error flagging produced by > ftdi_sio.c over-marks errors to the point that PARMRK becomes unusable > (in our cases, everything ends up being flagged as errors, even the > actual, good data). > > The issue in particular is that the driver marks *all* characters > received in a single USB packet with an error flag based on whether a > BREAK, frame error, or parity error condition is reported by that USB > packet. In actuality, the FTDI chip's error condition seems to apply > to the last byte in the received packet (I tested with an FT230X > chip). Unfortunately, it's more complicated than that. I was unable > to find any useful documentation on the USB packets the FTDI chips > generate, but from trial and error, the BREAK condition handling seems > to work something like this: > > When a break condition is reported (FTDI_RS_BI is set), the > last byte received MAY be a BREAK. To confirm, wait for the > next status packet. IF that status packet has no data AND > FTDI_RS_BI is still set, then the previous character was > indeed a BREAK. Otherwise, the previous character was a > normal data byte. > > This seems rather byzantine, but in my testing, it's been the only > algorithm that was able to accurately identify BREAKs. > > I attached some code that worked for the test cases I tried. Of > course, I'm not overly confident that this will work in all cases. > > As for parity errors: I have not been able to figure out how to mark > ALL parity errors characters without also sometimes accidentally > marking correctly received bytes as erroneous. > > My sense is that the FTDI chips simply can't reliably flag all error > characters (and only error characters) and we're probably not going to > use PARMRK to try to handle the communications issue mentioned at the > outset, so I'm on the fence whether is worthwhile to apply a patch > along the lines of the below. However, I'd suggest that at least a > comment be added in the driver making it clear that the error flagging > is not accurate as implemented and that it will mark too many > characters as erroneous, but that, perhaps, it's the best that can be > done. > > Thoughts? I'm aware that the current implementation flags all characters in the receive buffer when we detect an error, but I'm not sure we can do much better either. Apparently, you get similar behaviour using their own drivers: https://highfieldtales.wordpress.com/2014/09/27/lets-dig-into-an-issue-of-the-ft232-chip/ Having FTDI provide the required documentation would help (perhaps you can ask about this specific issue), otherwise it's down to tedious reverse engineering of behaviour which may not even be consistent between device types. Johan
Re: Why there is a refcnt check when we change configfs property
Hi, Peter Chen writes: > Hi Felipe, > > Do you know why? In that case, I can't change property, eg changing qmult at > NCM > like below: > > mkdir functions/ncm.0 > ln -s functions/ncm.0 configs/c.1 > echo 10 > functions/ncm.0/qmult > > The code: > drivers/usb/gadget/function/u_ether_configfs.h > > static ssize_t _f_##_opts_qmult_store(struct config_item *item, \ > const char *page, size_t len)\ > { \ > struct f_##_f_##_opts *opts = to_f_##_f_##_opts(item); \ > u8 val; \ > int ret;\ > \ > mutex_lock(&opts->lock);\ > if (opts->refcnt) { \ > ret = -EBUSY; \ > goto out; \ > } \ That check looks wrong to me. I think, we should prevent changes after cable is already plugged, but if gadget state is NOTATTACHED, we should be able to change qmult, mac address, etc. -- balbi signature.asc Description: PGP signature
[PATCH 0/5] usb: dwc2: Improve gadget phy init
Hi, Theses patches tries to clean a bit dwc2's phy initialization and fix an issue in gadget mode where the utmi phy width is set regardless of utmi being used or not. I believe that when using ulpi a phy width of 8 bits must be used, but this wasn't the case as the variable phyif was set by default to 16 bits. Best, Jules --- Jules Maselbas (5): usb: dwc2: Move UTMI_PHY_DATA defines closer usb: dwc2: Move phy init into core usb: dwc2: gadget: Remove duplicated phy init usb: dwc2: gadget: Replace phyif with phy_utmi_width usb: dwc2: gadget: Move gadget phy init into core phy init drivers/usb/dwc2/core.c | 199 drivers/usb/dwc2/core.h | 3 +- drivers/usb/dwc2/gadget.c | 34 ++ drivers/usb/dwc2/hcd.c | 190 -- drivers/usb/dwc2/hw.h | 6 +- drivers/usb/dwc2/platform.c | 5 +- 6 files changed, 211 insertions(+), 226 deletions(-) -- 2.21.0.196.g041f5ea
[PATCH 2/5] usb: dwc2: Move phy init into core
As the phy initialization is almost the same in host and gadget mode. This only move the phy initialization functions into core.c for now, the goal is to share theses functions between the two modes. Signed-off-by: Jules Maselbas --- drivers/usb/dwc2/core.c | 190 drivers/usb/dwc2/core.h | 1 + drivers/usb/dwc2/hcd.c | 190 3 files changed, 191 insertions(+), 190 deletions(-) diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c index 55d5ae2a7ec7..0dc1ab89092b 100644 --- a/drivers/usb/dwc2/core.c +++ b/drivers/usb/dwc2/core.c @@ -1020,6 +1020,196 @@ int dwc2_hsotg_wait_bit_clear(struct dwc2_hsotg *hsotg, u32 offset, u32 mask, return -ETIMEDOUT; } +/* + * Initializes the FSLSPClkSel field of the HCFG register depending on the + * PHY type + */ +static void dwc2_init_fs_ls_pclk_sel(struct dwc2_hsotg *hsotg) +{ + u32 hcfg, val; + + if ((hsotg->hw_params.hs_phy_type == GHWCFG2_HS_PHY_TYPE_ULPI && +hsotg->hw_params.fs_phy_type == GHWCFG2_FS_PHY_TYPE_DEDICATED && +hsotg->params.ulpi_fs_ls) || + hsotg->params.phy_type == DWC2_PHY_TYPE_PARAM_FS) { + /* Full speed PHY */ + val = HCFG_FSLSPCLKSEL_48_MHZ; + } else { + /* High speed PHY running at full speed or high speed */ + val = HCFG_FSLSPCLKSEL_30_60_MHZ; + } + + dev_dbg(hsotg->dev, "Initializing HCFG.FSLSPClkSel to %08x\n", val); + hcfg = dwc2_readl(hsotg, HCFG); + hcfg &= ~HCFG_FSLSPCLKSEL_MASK; + hcfg |= val << HCFG_FSLSPCLKSEL_SHIFT; + dwc2_writel(hsotg, hcfg, HCFG); +} + +static int dwc2_fs_phy_init(struct dwc2_hsotg *hsotg, bool select_phy) +{ + u32 usbcfg, ggpio, i2cctl; + int retval = 0; + + /* +* core_init() is now called on every switch so only call the +* following for the first time through +*/ + if (select_phy) { + dev_dbg(hsotg->dev, "FS PHY selected\n"); + + usbcfg = dwc2_readl(hsotg, GUSBCFG); + if (!(usbcfg & GUSBCFG_PHYSEL)) { + usbcfg |= GUSBCFG_PHYSEL; + dwc2_writel(hsotg, usbcfg, GUSBCFG); + + /* Reset after a PHY select */ + retval = dwc2_core_reset(hsotg, false); + + if (retval) { + dev_err(hsotg->dev, + "%s: Reset failed, aborting", __func__); + return retval; + } + } + + if (hsotg->params.activate_stm_fs_transceiver) { + ggpio = dwc2_readl(hsotg, GGPIO); + if (!(ggpio & GGPIO_STM32_OTG_GCCFG_PWRDWN)) { + dev_dbg(hsotg->dev, "Activating transceiver\n"); + /* +* STM32F4x9 uses the GGPIO register as general +* core configuration register. +*/ + ggpio |= GGPIO_STM32_OTG_GCCFG_PWRDWN; + dwc2_writel(hsotg, ggpio, GGPIO); + } + } + } + + /* +* Program DCFG.DevSpd or HCFG.FSLSPclkSel to 48Mhz in FS. Also +* do this on HNP Dev/Host mode switches (done in dev_init and +* host_init). +*/ + if (dwc2_is_host_mode(hsotg)) + dwc2_init_fs_ls_pclk_sel(hsotg); + + if (hsotg->params.i2c_enable) { + dev_dbg(hsotg->dev, "FS PHY enabling I2C\n"); + + /* Program GUSBCFG.OtgUtmiFsSel to I2C */ + usbcfg = dwc2_readl(hsotg, GUSBCFG); + usbcfg |= GUSBCFG_OTG_UTMI_FS_SEL; + dwc2_writel(hsotg, usbcfg, GUSBCFG); + + /* Program GI2CCTL.I2CEn */ + i2cctl = dwc2_readl(hsotg, GI2CCTL); + i2cctl &= ~GI2CCTL_I2CDEVADDR_MASK; + i2cctl |= 1 << GI2CCTL_I2CDEVADDR_SHIFT; + i2cctl &= ~GI2CCTL_I2CEN; + dwc2_writel(hsotg, i2cctl, GI2CCTL); + i2cctl |= GI2CCTL_I2CEN; + dwc2_writel(hsotg, i2cctl, GI2CCTL); + } + + return retval; +} + +static int dwc2_hs_phy_init(struct dwc2_hsotg *hsotg, bool select_phy) +{ + u32 usbcfg, usbcfg_old; + int retval = 0; + + if (!select_phy) + return 0; + + usbcfg = dwc2_readl(hsotg, GUSBCFG); + usbcfg_old = usbcfg; + + /* +* HS PHY parameters. These parameters are preserved during soft reset +* so only program the first time. Do a soft reset immediately after +* setting phyif. +*/ + switch (hsotg->params.phy_type) { + case DWC2_PHY_TYPE_PARAM_ULPI: + /* ULPI interface */ + dev_dbg(hsotg->dev,
[PATCH 1/5] usb: dwc2: Move UTMI_PHY_DATA defines closer
Makes GHWCFG4_UTMI_PHY_DATA* defines closer to their relative shift and mask defines to improve readability. Signed-off-by: Jules Maselbas --- drivers/usb/dwc2/hw.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/usb/dwc2/hw.h b/drivers/usb/dwc2/hw.h index 98af924a9a5c..7dbf392783c4 100644 --- a/drivers/usb/dwc2/hw.h +++ b/drivers/usb/dwc2/hw.h @@ -310,12 +310,12 @@ #define GHWCFG4_NUM_DEV_MODE_CTRL_EP_SHIFT 16 #define GHWCFG4_UTMI_PHY_DATA_WIDTH_MASK (0x3 << 14) #define GHWCFG4_UTMI_PHY_DATA_WIDTH_SHIFT 14 -#define GHWCFG4_ACG_SUPPORTED BIT(12) -#define GHWCFG4_IPG_ISOC_SUPPORTED BIT(11) -#define GHWCFG4_SERVICE_INTERVAL_SUPPORTED BIT(10) #define GHWCFG4_UTMI_PHY_DATA_WIDTH_8 0 #define GHWCFG4_UTMI_PHY_DATA_WIDTH_16 1 #define GHWCFG4_UTMI_PHY_DATA_WIDTH_8_OR_162 +#define GHWCFG4_ACG_SUPPORTED BIT(12) +#define GHWCFG4_IPG_ISOC_SUPPORTED BIT(11) +#define GHWCFG4_SERVICE_INTERVAL_SUPPORTED BIT(10) #define GHWCFG4_XHIBER BIT(7) #define GHWCFG4_HIBER BIT(6) #define GHWCFG4_MIN_AHB_FREQ BIT(5) -- 2.21.0.196.g041f5ea
[PATCH 5/5] usb: dwc2: gadget: Move gadget phy init into core phy init
Most of the phy initialization is shared between host and gadget, this adds the turnaround configuration only used by gadgets to the global phy init. Signed-off-by: Jules Maselbas --- drivers/usb/dwc2/core.c | 9 + drivers/usb/dwc2/gadget.c | 21 +++-- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c index 0dc1ab89092b..8e667b10f224 100644 --- a/drivers/usb/dwc2/core.c +++ b/drivers/usb/dwc2/core.c @@ -1152,6 +1152,15 @@ static int dwc2_hs_phy_init(struct dwc2_hsotg *hsotg, bool select_phy) usbcfg &= ~(GUSBCFG_ULPI_UTMI_SEL | GUSBCFG_PHYIF16); if (hsotg->params.phy_utmi_width == 16) usbcfg |= GUSBCFG_PHYIF16; + + /* Set turnaround time */ + if (dwc2_is_device_mode(hsotg)) { + usbcfg &= ~GUSBCFG_USBTRDTIM_MASK; + if (hsotg->params.phy_utmi_width == 16) + usbcfg |= 5 << GUSBCFG_USBTRDTIM_SHIFT; + else + usbcfg |= 9 << GUSBCFG_USBTRDTIM_SHIFT; + } break; default: dev_err(hsotg->dev, "FS PHY selected at HS!\n"); diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c index 41efe1b859ff..2fbd8db6057b 100644 --- a/drivers/usb/dwc2/gadget.c +++ b/drivers/usb/dwc2/gadget.c @@ -3314,22 +3314,15 @@ void dwc2_hsotg_core_init_disconnected(struct dwc2_hsotg *hsotg, /* keep other bits untouched (so e.g. forced modes are not lost) */ usbcfg = dwc2_readl(hsotg, GUSBCFG); - usbcfg &= ~(GUSBCFG_TOUTCAL_MASK | GUSBCFG_PHYIF16 | GUSBCFG_SRPCAP | - GUSBCFG_HNPCAP | GUSBCFG_USBTRDTIM_MASK); - - if (hsotg->params.phy_type == DWC2_PHY_TYPE_PARAM_FS && - (hsotg->params.speed == DWC2_SPEED_PARAM_FULL || -hsotg->params.speed == DWC2_SPEED_PARAM_LOW)) { - /* FS/LS Dedicated Transceiver Interface */ - usbcfg |= GUSBCFG_PHYSEL; - } else { - /* set the PLL on, remove the HNP/SRP and set the PHY */ - val = (hsotg->params.phy_utmi_width == GUSBCFG_PHYIF8) ? 9 : 5; - usbcfg |= hsotg->params.phy_utmi_width | GUSBCFG_TOUTCAL(7) | - (val << GUSBCFG_USBTRDTIM_SHIFT); - } + usbcfg &= ~GUSBCFG_TOUTCAL_MASK; + usbcfg |= GUSBCFG_TOUTCAL(7); + + /* remove the HNP/SRP and set the PHY */ + usbcfg &= ~(GUSBCFG_SRPCAP | GUSBCFG_HNPCAP); dwc2_writel(hsotg, usbcfg, GUSBCFG); + dwc2_phy_init(hsotg, true); + dwc2_hsotg_init_fifo(hsotg); if (!is_usb_reset) -- 2.21.0.196.g041f5ea
[PATCH 4/5] usb: dwc2: gadget: Replace phyif with phy_utmi_width
The phy utmi width information is already set in hsotg params, phyif is only used in few places and I don't see any reason to not use hsotg's params. Moreover the utmi width was being forced to 16 bits by platform initialization which doesn't take in account HW configuration. Signed-off-by: Jules Maselbas --- drivers/usb/dwc2/core.h | 2 -- drivers/usb/dwc2/gadget.c | 4 ++-- drivers/usb/dwc2/platform.c | 5 + 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h index 1fad5dcbcd81..96cf645e53fe 100644 --- a/drivers/usb/dwc2/core.h +++ b/drivers/usb/dwc2/core.h @@ -869,7 +869,6 @@ struct dwc2_hregs_backup { * removed once all SoCs support usb transceiver. * @supplies: Definition of USB power supplies * @vbus_supply:Regulator supplying vbus. - * @phyif: PHY interface width * @lock: Spinlock that protects all the driver data structures * @priv: Stores a pointer to the struct usb_hcd * @queuing_high_bandwidth: True if multiple packets of a high-bandwidth @@ -1052,7 +1051,6 @@ struct dwc2_hsotg { struct dwc2_hsotg_plat *plat; struct regulator_bulk_data supplies[DWC2_NUM_SUPPLIES]; struct regulator *vbus_supply; - u32 phyif; spinlock_t lock; void *priv; diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c index 36ba391a715f..41efe1b859ff 100644 --- a/drivers/usb/dwc2/gadget.c +++ b/drivers/usb/dwc2/gadget.c @@ -3324,8 +3324,8 @@ void dwc2_hsotg_core_init_disconnected(struct dwc2_hsotg *hsotg, usbcfg |= GUSBCFG_PHYSEL; } else { /* set the PLL on, remove the HNP/SRP and set the PHY */ - val = (hsotg->phyif == GUSBCFG_PHYIF8) ? 9 : 5; - usbcfg |= hsotg->phyif | GUSBCFG_TOUTCAL(7) | + val = (hsotg->params.phy_utmi_width == GUSBCFG_PHYIF8) ? 9 : 5; + usbcfg |= hsotg->params.phy_utmi_width | GUSBCFG_TOUTCAL(7) | (val << GUSBCFG_USBTRDTIM_SHIFT); } dwc2_writel(hsotg, usbcfg, GUSBCFG); diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c index c0b64d483552..f126591f06b0 100644 --- a/drivers/usb/dwc2/platform.c +++ b/drivers/usb/dwc2/platform.c @@ -230,9 +230,6 @@ static int dwc2_lowlevel_hw_init(struct dwc2_hsotg *hsotg) reset_control_deassert(hsotg->reset_ecc); - /* Set default UTMI width */ - hsotg->phyif = GUSBCFG_PHYIF16; - /* * Attempt to find a generic PHY, then look for an old style * USB PHY and then fall back to pdata @@ -280,7 +277,7 @@ static int dwc2_lowlevel_hw_init(struct dwc2_hsotg *hsotg) * width is 8-bit and set the phyif appropriately. */ if (phy_get_bus_width(hsotg->phy) == 8) - hsotg->phyif = GUSBCFG_PHYIF8; + hsotg->params.phy_utmi_width = 8; } /* Clock */ -- 2.21.0.196.g041f5ea
[PATCH 3/5] usb: dwc2: gadget: Remove duplicated phy init
The function dwc2_hsotg_init is only called once just before calling dwc2_hsotg_core_init_disconnected which does the same initialization: setting the usbcfg register with turnaround time, timeout calibration and phy width. Signed-off-by: Jules Maselbas --- drivers/usb/dwc2/gadget.c | 13 - 1 file changed, 13 deletions(-) diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c index 6812a8a3a98b..36ba391a715f 100644 --- a/drivers/usb/dwc2/gadget.c +++ b/drivers/usb/dwc2/gadget.c @@ -4328,8 +4328,6 @@ static const struct usb_ep_ops dwc2_hsotg_ep_ops = { */ static void dwc2_hsotg_init(struct dwc2_hsotg *hsotg) { - u32 trdtim; - u32 usbcfg; /* unmask subset of endpoint interrupts */ dwc2_writel(hsotg, DIEPMSK_TIMEOUTMSK | DIEPMSK_AHBERRMSK | @@ -4353,17 +4351,6 @@ static void dwc2_hsotg_init(struct dwc2_hsotg *hsotg) dwc2_hsotg_init_fifo(hsotg); - /* keep other bits untouched (so e.g. forced modes are not lost) */ - usbcfg = dwc2_readl(hsotg, GUSBCFG); - usbcfg &= ~(GUSBCFG_TOUTCAL_MASK | GUSBCFG_PHYIF16 | GUSBCFG_SRPCAP | - GUSBCFG_HNPCAP | GUSBCFG_USBTRDTIM_MASK); - - /* set the PLL on, remove the HNP/SRP and set the PHY */ - trdtim = (hsotg->phyif == GUSBCFG_PHYIF8) ? 9 : 5; - usbcfg |= hsotg->phyif | GUSBCFG_TOUTCAL(7) | - (trdtim << GUSBCFG_USBTRDTIM_SHIFT); - dwc2_writel(hsotg, usbcfg, GUSBCFG); - if (using_dma(hsotg)) dwc2_set_bit(hsotg, GAHBCFG, GAHBCFG_DMA_EN); } -- 2.21.0.196.g041f5ea
Re: problem with Synopsys DesignWare Core SuperSpeed USB driver
Hello, sending again in plain text mode.. On Tue, Apr 2, 2019 at 11:37 AM Antonio Santagiuliana wrote: > > Hello, > thank you for the reply. > We transmit at 46800 bps from 16 devices to the UART side of the UART to USB > bridges ( at the moment cp2105 UART to USB bridges ) . > If we transmit from all of these devices at low throughput small packets of > some bytes ( 16 to 64 bytes ) let's say every 20 ms , at the receiver side > on the USB host device ( Raspberry PI ) we can follow all of the messages > from all of the 16 UART to USB bridges without any problem. > We can then activate a particular mode by which 1 of the devices connected to > one of the UART to USB bridges starts sending data still at the same > baudrate, but at much higher throughput, sending packets of 1460 bytes with > silence interval between packets of around 20 ms. > When this happens the normal messages from the other 15 devices appear to be > retrieved correctly while these longer messages from the high throughput mode > device sometimes miss some bytes , let's say I receive fewer bytes than > expected. > It doesn't appear the problem is the latency of the application layer as I > verified it wakes up with short delay. > if I connect only 1 or 2 devices rather than all of 16 and I activate this > high throughput mode on one device , then the system works perfectly. Problem > happens when all of the 16 devices are connected and high throughput mode on > one device is enabled. > So I was wondering if we got into a limitation of USB system due to polling > rate from the host linked to the fact that the buffers USB 2.0 specified > inside these UART-USB bridges are very limited in size ( I think 64 bytes and > 32 bytes on the two USB ports of the UART to USB dual bridge ) and the are > also translation buffers on the USB hub that could delay further the whole > thing. > I was wondering also if I could raise priority of USB polling that > particular device more frequently from the host rather than lopping to the > others and I was checking if it could be useful to log events so to see if I > can get some clues of what is going on. > any idea would be welcome. > thank you > > > > On Tue, Apr 2, 2019 at 8:56 AM Minas Harutyunyan > wrote: >> >> Hi, >> >> On 4/1/2019 9:01 PM, Felipe Balbi wrote: >> > >> > Hi, >> > >> > (I can't answer private queries. Please, always Cc the public mailing >> > list) >> > >> > Antonio Santagiuliana writes: >> > >> >> Hello >> >> I am using RaspberryPI for a project and we want to connect 16 USB to UART >> >> bridges ( at the moment we are using 8 dual UART cp2105 from Silicon Labs >> >> ). >> >> I have seen your page at : >> >> https://urldefense.proofpoint.com/v2/url?u=https-3A__dri.freedesktop.org_docs_drm_driver-2Dapi_usb_dwc3.html&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=cQBKt4q-qzNVC53rNAwuwplH23V61rHQhhULvdLA0U8&m=RfK-uhEeZZ8yf79ch5you3BpE1JNPDHRzgmOGkeLBss&s=rGrgFKpJ6-CEmAtR-hiae64T80Z7oCTx3r4xHn-lf4I&e= >> >> so I was trying to get tracefs output. >> >> Problem is that on the /t/events subfolder I cannot find the >> >> dwc3/enable because >> >> there is not any /dwc3 in /t/events. >> >> Because of this I cannot set echo 1 > /t/events/dwc3/enable >> >> as instead your page says I should do. >> >> I can only find */t/events/enable* should I activate that ? >> >> running usb-devices command I get this about the drivers loaded : >> >> >> >> T: Bus=01 Lev=00 Prnt=00 Port=00 Cnt=00 Dev#= 1 Spd=480 MxCh= 1 >> >> D: Ver= 2.00 Cls=09(hub ) Sub=00 Prot=01 MxPS=64 #Cfgs= 1 >> >> P: Vendor=1d6b ProdID=0002 Rev=04.14 >> >> S: Manufacturer=Linux 4.14.39 dwc_otg_hcd >> > >> > You're not using dwc3. That's dwc2, instead. Also, dwc3 tracepoints are >> > peripheral side only. Host side is a standard xhci, so you would use >> > xhci tracepoints for that. >> > >> > Still, you're using dwc2 from what I can tell. Minas is the maintainer >> > of that driver. >> > >> > >> It's not dwc2 driver. It's dwc_otg Synopsys reference driver which >> supplying along with HSOTG core. >> >> But what the issue? >> > >> > >> > >> > >> >> S: Product=DWC OTG Controller >> >> S: SerialNumber=3f98.usb >> >> C: #Ifs= 1 Cfg#= 1 Atr=e0 MxPwr=0mA >> >> I: If#=0x0 Alt= 0 #EPs= 1 Cls=09(hub ) Sub=00 Prot=00 Driver=hub >> >> >> >> T: Bus=01 Lev=01 Prnt=01 Port=00 Cnt=01 Dev#= 2 Spd=480 MxCh= 5 >> >> D: Ver= 2.00 Cls=09(hub ) Sub=00 Prot=02 MxPS=64 #Cfgs= 1 >> >> P: Vendor=0424 ProdID=9514 Rev=02.00 >> >> C: #Ifs= 1 Cfg#= 1 Atr=e0 MxPwr=2mA >> >> I: If#=0x0 Alt= 1 #EPs= 1 Cls=09(hub ) Sub=00 Prot=02 Driver=hub >> >> >> >> T: Bus=01 Lev=02 Prnt=02 Port=00 Cnt=01 Dev#= 3 Spd=480 MxCh= 0 >> >> D: Ver= 2.00 Cls=ff(vend.) Sub=00 Prot=01 MxPS=64 #Cfgs= 1 >> >> P: Vendor=0424 ProdID=ec00 Rev=02.00 >> >> C: #Ifs= 1 Cfg#= 1 Atr=e0 MxPwr=2mA >> >> I: If#=0x0 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=ff Driver=smsc95xx >> >> >> >> T: Bus=01 Lev=02 Prnt=02 Port=01 Cnt=02 Dev#= 14 Spd=480 MxCh= 4 >> >> D:
Re: [PATCH 1/2] USB: serial: pl2303: fix non-supported xon/xoff
On Tue, Apr 02, 2019 at 10:19:30AM +0200, Johan Hovold wrote: > Older pl2303 devices do not support automatic xon/xoff flow control, so > add add a flag to prevent trying to enable it for legacy device types. > > Refactor the IXON test into a helper function to improve readability. > > Fixes: 7041d9c3f01b ("USB: serial: pl2303: add support for tx xon/xoff flow > control") > Signed-off-by: Johan Hovold > --- > drivers/usb/serial/pl2303.c | 25 - > 1 file changed, 20 insertions(+), 5 deletions(-) Reviewed-by: Greg Kroah-Hartman
Re: [PATCH 2/2] USB: serial: pl2303: fix tranceiver suspend mode
On Tue, Apr 02, 2019 at 10:22:06AM +0200, Johan Hovold wrote: > On Tue, Apr 02, 2019 at 10:19:31AM +0200, Johan Hovold wrote: > > Add helper function to update register bits instead of overwriting the > > entire control register when updating the flow-control settings. > > > > This specifically avoids having the tranceiver suspend mode (bit 0) > > depend on the flow control setting. > > > > The tranceiver is currently configured at probe to be disabled during > > suspend, but this was overridden when disabling flow control or enabling > > xon/xoff. > > > > Fixes: 715f9527c1c1 ("USB: flow control fix for pl2303") > > Fixes: 7041d9c3f01b ("USB: serial: pl2303: add support for tx xon/xoff flow > > control") > > Signed-off-by: Johan Hovold > > --- > > > @@ -176,7 +178,7 @@ static const struct pl2303_type_data > > pl2303_type_data[TYPE_COUNT] = { > > [TYPE_01] = { > > .max_baud_rate = 1228800, > > .quirks = PL2303_QUIRK_LEGACY, > > - .no_autoxonxoff = 1, > > + .no_autoxonxoff = true, > > This chunk was supposed to go in the first patch. I'll fix that up > before applying (or resending). With that fixup: Reviewed-by: Greg Kroah-Hartman
Re: [PATCH] usb: typec: Registering real device entries for the muxes
On Mon, Apr 01, 2019 at 02:45:11PM +0200, Hans de Goede wrote: > HI, > > On 01-04-19 14:40, Heikki Krogerus wrote: > > On Mon, Apr 01, 2019 at 12:34:29PM +0200, Greg KH wrote: > > > On Mon, Apr 01, 2019 at 01:15:53PM +0300, Heikki Krogerus wrote: > > > > Registering real device entries (struct device) for the mode > > > > muxes as well as for the orientation switches. > > > > > > > > The Type-C mux code was deliberately attempting to avoid > > > > creation of separate device entries for the orientation > > > > switch and the mode switch (alternate modes) because they > > > > are not physical devices. They are functions of a single > > > > physical multiplexer/demultiplexer switch device. > > > > > > > > Unfortunately because of the dependency we still have on the > > > > underlying mux device driver, we had to put in hacks like > > > > the one in the commit 3e3b81965cbf ("usb: typec: mux: Take > > > > care of driver module reference counting") to make sure the > > > > driver does not disappear from underneath us. Even with > > > > those hacks we were still left with a potential NUll pointer > > > > dereference scenario, so just creating the device entries, > > > > and letting the core take care of the dependencies. No more > > > > hacks needed. > > > > > > > > Fixes: 3e3b81965cbf ("usb: typec: mux: Take care of driver module > > > > reference counting") > > > > Cc: v4.19.x # v4.19.x+ > > > > Signed-off-by: Heikki Krogerus > > > > > > This looks good to me, nice work! > > > > > > But, it would be nice if someone who has this hardware can test it to > > > verify it does actually work :) > > > > This alone does not work on Intel Cherrytrail platforms. I need to make > > the Intel Cherrytrail MFD driver (intel_cht_int33fe.c) to use the new > > device names that we now have for the muxes. Sorry for the mistake. > > > > I'll resend this and include the needed modifications to > > intel_cht_int33fe.c. Hans should be able to test this once I do that. I > > hope he has time. > > Yes I need to get back to the CherryTrail Type-C stuff we discussed a while > back anyways. On which tree/branch should I test v2 of this patch(series) ? Greg's usb-next. thanks, -- heikki
Re: Problem with USB2 devices connected to a hub
On 28.3.2019 15.41, Mathias Nyman wrote: The issue happens when only a single hub and a single device is connected to the USB subsystem. On my previous board, I had two USB dongles and two sound devices connected to the same hub and it worked without issues. So I guess it is not a real bandwidth limit. Somehow the hub seems to eat the bandwidth. Can/will you investigate this problem further or do I have to live with it? Or is there anything more I can do? This is one of the items I'm looking at. xHC hw calculates bandwidth usage when endpoints are added based on values in endpoint context. The traces unfortunately didn't support this, so new tracing support was needed, added here to my bandwith-debug branch: git://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git bandwidth-debug xHC hw can also report available bandwidth, but we don't yet have driver support. I was planning on adding a hack that checks the available bandwidth after every endpoint change and add it to the testbranch, but that part is not yet finished. New traces using the bandwidth-debug testbranch could be useful, even without the "available bandwidth" support available bandwidth tracking added to bandwidth-debug branch, any chance you could take new traces? -Mathias
Re: [PATCH 2/5] usb: dwc2: Move phy init into core
Hi Jules, Thank you for the patch! Yet something to improve: [auto build test ERROR on balbi-usb/next] [also build test ERROR on v5.1-rc3 next-20190402] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Jules-Maselbas/usb-dwc2-Improve-gadget-phy-init/20190403-103251 base: https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next config: i386-randconfig-x017-201913 (attached as .config) compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=i386 All errors (new ones prefixed by >>): drivers/usb//dwc2/hcd.c: In function 'dwc2_core_host_init': >> drivers/usb//dwc2/hcd.c:2185:2: error: implicit declaration of function >> 'dwc2_init_fs_ls_pclk_sel'; did you mean 'dwc2_init_params'? >> [-Werror=implicit-function-declaration] dwc2_init_fs_ls_pclk_sel(hsotg); ^~~~ dwc2_init_params cc1: some warnings being treated as errors vim +2185 drivers/usb//dwc2/hcd.c b02038fa John Youn 2016-02-23 2152 b02038fa John Youn 2016-02-23 2153 /** b02038fa John Youn 2016-02-23 2154 * dwc2_core_host_init() - Initializes the DWC_otg controller registers for b02038fa John Youn 2016-02-23 2155 * Host mode b02038fa John Youn 2016-02-23 2156 * b02038fa John Youn 2016-02-23 2157 * @hsotg: Programming view of DWC_otg controller b02038fa John Youn 2016-02-23 2158 * b02038fa John Youn 2016-02-23 2159 * This function flushes the Tx and Rx FIFOs and flushes any entries in the b02038fa John Youn 2016-02-23 2160 * request queues. Host channels are reset to ensure that they are ready for b02038fa John Youn 2016-02-23 2161 * performing transfers. b02038fa John Youn 2016-02-23 2162 */ b02038fa John Youn 2016-02-23 2163 static void dwc2_core_host_init(struct dwc2_hsotg *hsotg) b02038fa John Youn 2016-02-23 2164 { 92a8dd26 Minas Harutyunyan 2018-01-19 2165 u32 hcfg, hfir, otgctl, usbcfg; b02038fa John Youn 2016-02-23 2166 b02038fa John Youn 2016-02-23 2167 dev_dbg(hsotg->dev, "%s(%p)\n", __func__, hsotg); b02038fa John Youn 2016-02-23 2168 92a8dd26 Minas Harutyunyan 2018-01-19 2169 /* Set HS/FS Timeout Calibration to 7 (max available value). 92a8dd26 Minas Harutyunyan 2018-01-19 2170 * The number of PHY clocks that the application programs in 92a8dd26 Minas Harutyunyan 2018-01-19 2171 * this field is added to the high/full speed interpacket timeout 92a8dd26 Minas Harutyunyan 2018-01-19 2172 * duration in the core to account for any additional delays 92a8dd26 Minas Harutyunyan 2018-01-19 2173 * introduced by the PHY. This can be required, because the delay 92a8dd26 Minas Harutyunyan 2018-01-19 2174 * introduced by the PHY in generating the linestate condition 92a8dd26 Minas Harutyunyan 2018-01-19 2175 * can vary from one PHY to another. 92a8dd26 Minas Harutyunyan 2018-01-19 2176 */ f25c42b8 Gevorg Sahakyan 2018-07-26 2177 usbcfg = dwc2_readl(hsotg, GUSBCFG); 92a8dd26 Minas Harutyunyan 2018-01-19 2178 usbcfg |= GUSBCFG_TOUTCAL(7); f25c42b8 Gevorg Sahakyan 2018-07-26 2179 dwc2_writel(hsotg, usbcfg, GUSBCFG); 92a8dd26 Minas Harutyunyan 2018-01-19 2180 b02038fa John Youn 2016-02-23 2181 /* Restart the Phy Clock */ f25c42b8 Gevorg Sahakyan 2018-07-26 2182 dwc2_writel(hsotg, 0, PCGCTL); b02038fa John Youn 2016-02-23 2183 b02038fa John Youn 2016-02-23 2184 /* Initialize Host Configuration Register */ b02038fa John Youn 2016-02-23 @2185 dwc2_init_fs_ls_pclk_sel(hsotg); 38e9002b Vardan Mikayelyan 2016-11-14 2186 if (hsotg->params.speed == DWC2_SPEED_PARAM_FULL || 38e9002b Vardan Mikayelyan 2016-11-14 2187 hsotg->params.speed == DWC2_SPEED_PARAM_LOW) { f25c42b8 Gevorg Sahakyan 2018-07-26 2188 hcfg = dwc2_readl(hsotg, HCFG); b02038fa John Youn 2016-02-23 2189 hcfg |= HCFG_FSLSSUPP; f25c42b8 Gevorg Sahakyan 2018-07-26 2190 dwc2_writel(hsotg, hcfg, HCFG); b02038fa John Youn 2016-02-23 2191 } b02038fa John Youn 2016-02-23 2192 b02038fa John Youn 2016-02-23 2193 /* b02038fa John Youn 2016-02-23 2194 * This bit allows dynamic reloading of the HFIR register during b02038fa John Youn 2016-02-23 2195 * runtime. This bit needs to be programmed during initial configuration b02038fa John Youn 2016-02-23 2196 * and its value must not be changed during runtime. b02038fa John Youn 2016-02-23 2197 */ 95832c00 John Youn 2017-01-23 2198 if (hsotg->params.reload_ctl) { f25c42b8 G
Re: [PATCH] USB:serial:pl2303:Add new PID to support PL2303HXN (TYPE_HXN)
Hi Johan, Thanks for you check the patch.. I will reply to you on the next Monday. Because I am currently on a business trip in China (3/28~4/6) Johan Hovold 於 2019年4月2日 週二 下午3:22寫道: > > On Wed, Feb 13, 2019 at 08:30:00PM +0800, Charles Yeh wrote: > > Prolific has developed a new USB to UART chip: PL2303HXN > > (PL2303GC/PL2303GS/PL2303GT/PL2303GL/PL2303GE) > > Ok, let's get back to this one. > > First a general comment; please make sure to address all review > comments. I've already pointed out some issues that still hasn't been > fixed and I've asked questions that have gone unanswered. > > If you disagree on something then just say so, but ignoring feedback is > just going to make this take longer than necessary. > > For a start, please fix up the Subject line as we already discussed, and > make sure to wrap your commit messages at 72 columns or so (I've reflown > the rest of the message below). > > Always include a changelog (below the cut-off line) when resending so we > know what changed when you update your patches. > > > The Vendor request used by the PL2303HXN (TYPE_HXN) is different from > > the existing PL2303 series (TYPE_HX & TYPE_01). > > Therefore, different Vendor requests are used to issue related commands. > > > > 1. Added a new TYPE_HXN type in pl2303_type_data, and then executes > > new Vendor request, new flow control and other related instructions if > > TYPE_HXN is recognized. > > > > 2. Because the new PL2303HXN can only accept the new Vendor request, > > the old Vendor request cannot be accepted (the error message will be > > returned) So first determine the TYPE_HX or TYPE_HXN through > > TYPE_HX_READ_STATUS_REG in pl2303_startup. > > > > 2.1 If the return message is "1", then the PL2303 is the existing > > TYPE_HX/ TYPE_01 series. The other settings in pl2303_startup are > > to continue execution. > > > > 2.2 If the return message is "not 1", then the PL2303 is the new > > TYPE_HXN series. The other settings in pl2303_startup are ignored. > > (PL2303HXN will directly use the default value in the hardware, no > > need to add additional settings through the software) > > > > 3. In pl2303_open: Because TYPE_HXN is different from the instruction > > of down/up stream used by TYPE_HX. Therefore, we will also execute > > different instructions here. > > > > 4. In pl2303_set_termios: The UART flow control instructions used by > > TYPE_HXN/TYPE_HX/TYPE_01 are different. Therefore, we will also > > execute different instructions here. > > > > 5. In pl2303_vendor_read & pl2303_vendor_write, since TYPE_HXN is > > different from the vendor request instruction used by TYPE_HX/TYPE_01, > > it will also execute different instructions here. > > That's a good summary of the differences, but on a more general level, > can you confirm the following: > > 1. The HXN register layout is entirely different from HX and >earlier devices. > > 2. HXN use the same CDC class requests (line encoding, etc) as >earlier revisions. > > Can you send me documentation for the HXN protocol? That would help a > lot in finding the right abstraction level for this. > > > Signed-off-by: Charles Yeh > > --- > > drivers/usb/serial/pl2303.c | 131 +--- > > drivers/usb/serial/pl2303.h | 7 ++ > > 2 files changed, 113 insertions(+), 25 deletions(-) > > > > diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c > > index bb3f9aa4a909..d7d557e01390 100644 > > --- a/drivers/usb/serial/pl2303.c > > +++ b/drivers/usb/serial/pl2303.c > > @@ -47,6 +47,12 @@ static const struct usb_device_id id_table[] = { > > { USB_DEVICE(PL2303_VENDOR_ID, PL2303_PRODUCT_ID_MOTOROLA) }, > > { USB_DEVICE(PL2303_VENDOR_ID, PL2303_PRODUCT_ID_ZTEK) }, > > { USB_DEVICE(PL2303_VENDOR_ID, PL2303_PRODUCT_ID_TB) }, > > + { USB_DEVICE(PL2303_VENDOR_ID, PL2303G_PRODUCT_ID_GC) }, > > + { USB_DEVICE(PL2303_VENDOR_ID, PL2303G_PRODUCT_ID_GB) }, > > + { USB_DEVICE(PL2303_VENDOR_ID, PL2303G_PRODUCT_ID_GT) }, > > + { USB_DEVICE(PL2303_VENDOR_ID, PL2303G_PRODUCT_ID_GL) }, > > + { USB_DEVICE(PL2303_VENDOR_ID, PL2303G_PRODUCT_ID_GE) }, > > + { USB_DEVICE(PL2303_VENDOR_ID, PL2303G_PRODUCT_ID_GS) }, > > { USB_DEVICE(IODATA_VENDOR_ID, IODATA_PRODUCT_ID) }, > > { USB_DEVICE(IODATA_VENDOR_ID, IODATA_PRODUCT_ID_RSAQ5) }, > > { USB_DEVICE(ATEN_VENDOR_ID, ATEN_PRODUCT_ID), > > @@ -129,9 +135,11 @@ MODULE_DEVICE_TABLE(usb, id_table); > > > > #define VENDOR_WRITE_REQUEST_TYPE0x40 > > #define VENDOR_WRITE_REQUEST 0x01 > > +#define VENDOR_WRITE_NREQUEST0x80 > > > > #define VENDOR_READ_REQUEST_TYPE 0xc0 > > #define VENDOR_READ_REQUEST 0x01 > > +#define VENDOR_READ_NREQUEST 0x81 > > > > #define UART_STATE_INDEX 8 > > #define UART_STATE_MSR_MASK 0x8b > > @@ -145,11 +153,30 @@ MODULE_DEVICE_TABLE(usb, id_table); > > #define UART