From owner-freebsd-usb@freebsd.org Sun Mar 8 12:40:52 2020 Return-Path: Delivered-To: freebsd-usb@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 97A5026860A for ; Sun, 8 Mar 2020 12:40:52 +0000 (UTC) (envelope-from hps@selasky.org) Received: from mail.turbocat.net (turbocat.net [88.99.82.50]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 48b1DL3lNkz4M0n for ; Sun, 8 Mar 2020 12:40:49 +0000 (UTC) (envelope-from hps@selasky.org) Received: from hps2020.home.selasky.org (unknown [62.141.129.235]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by mail.turbocat.net (Postfix) with ESMTPSA id BCBF626012C; Sun, 8 Mar 2020 13:40:41 +0100 (CET) Subject: Re: Error bit using of wPortStatus when need usbd_req_warm_reset_port To: Shichun.Ma@dell.com, freebsd-usb@freebsd.org Cc: Shunchao.Hu@dell.com References: <13174e356c0640ab84fd697a1ae28e55@KULX13MDC130.APAC.DELL.COM> From: Hans Petter Selasky Message-ID: <8b8ae413-29d2-1b0b-1037-cf3b49c1baf7@selasky.org> Date: Sun, 8 Mar 2020 13:39:37 +0100 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:68.0) Gecko/20100101 Thunderbird/68.4.2 MIME-Version: 1.0 In-Reply-To: <13174e356c0640ab84fd697a1ae28e55@KULX13MDC130.APAC.DELL.COM> Content-Type: multipart/mixed; boundary="------------C50C2C89E61F94EB24D1116F" Content-Language: en-US X-Rspamd-Queue-Id: 48b1DL3lNkz4M0n X-Spamd-Bar: ----- Authentication-Results: mx1.freebsd.org; dkim=none; dmarc=none; spf=pass (mx1.freebsd.org: domain of hps@selasky.org designates 88.99.82.50 as permitted sender) smtp.mailfrom=hps@selasky.org X-Spamd-Result: default: False [-5.41 / 15.00]; ARC_NA(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[]; NEURAL_HAM_MEDIUM(-0.99)[-0.989,0]; FROM_HAS_DN(0.00)[]; RCPT_COUNT_THREE(0.00)[3]; R_SPF_ALLOW(-0.20)[+a:mail.turbocat.net]; NEURAL_HAM_LONG(-1.00)[-1.000,0]; HAS_ATTACHMENT(0.00)[]; TO_DN_NONE(0.00)[]; DMARC_NA(0.00)[selasky.org]; MIME_GOOD(-0.10)[multipart/mixed,text/plain]; TO_MATCH_ENVRCPT_SOME(0.00)[]; IP_SCORE(-3.12)[ip: (-9.31), ipnet: 88.99.0.0/16(-4.71), asn: 24940(-1.56), country: DE(-0.02)]; FROM_EQ_ENVFROM(0.00)[]; R_DKIM_NA(0.00)[]; MIME_TRACE(0.00)[0:+,1:+,2:~]; ASN(0.00)[asn:24940, ipnet:88.99.0.0/16, country:DE]; MID_RHS_MATCH_FROM(0.00)[]; RCVD_TLS_ALL(0.00)[]; RCVD_COUNT_TWO(0.00)[2] X-BeenThere: freebsd-usb@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FreeBSD support for USB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 08 Mar 2020 12:40:52 -0000 This is a multi-part message in MIME format. --------------C50C2C89E61F94EB24D1116F Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit On 2020-03-08 12:15, Shichun.Ma@dell.com wrote: > Link info need 4 bits and they take bit 5 to bit 9, while UPS_PORT_POWER takes bit 9. > So this will give wrong link state info in function uhub_suspend_resume_port when it check if need usbd_req_warm_reset_port. > Please review and kindly suggest how to fix this issue. Hi, You are absolutely right. Good catch. I think we will just remove the POWER bit from the super speed RH. It is currently not needed for anything. Then the LINK state values won't be clobbered like you found. Does the attached patch work for you? --HPS --------------C50C2C89E61F94EB24D1116F Content-Type: text/x-patch; charset=UTF-8; name="usb_power.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="usb_power.diff" Index: sys/dev/usb/controller/xhci.c =================================================================== --- sys/dev/usb/controller/xhci.c (revision 358693) +++ sys/dev/usb/controller/xhci.c (working copy) @@ -3591,13 +3591,10 @@ i |= UPS_OVERCURRENT_INDICATOR; if (v & XHCI_PS_PR) i |= UPS_RESET; - if (v & XHCI_PS_PP) { - /* - * The USB 3.0 RH is using the - * USB 2.0's power bit - */ - i |= UPS_PORT_POWER; - } +#if 0 + if (v & XHCI_PS_PP) + /* XXX undefined */ +#endif USETW(sc->sc_hub_desc.ps.wPortStatus, i); i = 0; Index: sys/dev/usb/usb_hub.c =================================================================== --- sys/dev/usb/usb_hub.c (revision 358693) +++ sys/dev/usb/usb_hub.c (working copy) @@ -660,7 +660,7 @@ break; case USB_SPEED_SUPER: if (udev->parent_hub == NULL) - power_mask = UPS_PORT_POWER; + power_mask = 0; /* XXX undefined */ else power_mask = UPS_PORT_POWER_SS; break; @@ -668,7 +668,7 @@ power_mask = 0; break; } - if (!(sc->sc_st.port_status & power_mask)) { + if ((sc->sc_st.port_status & power_mask) != power_mask) { DPRINTF("WARNING: strange, connected port %d " "has no power\n", portno); } --------------C50C2C89E61F94EB24D1116F--