On Thu, 7 Sep 2017, Arnd Bergmann wrote:
> gcc-8 points out two comparisons that are clearly bogus
> and almost certainly not what the author intended to write:
>
> drivers/usb/gadget/udc/dummy_hcd.c: In function 'set_link_state_by_speed':
> drivers/usb/gadget/udc/dummy_hcd.c:379:31: error: bitwise comparison always evaluates to false [-Werror=tautological-compare]
> USB_PORT_STAT_ENABLE) == 1 &&
> ^~
> drivers/usb/gadget/udc/dummy_hcd.c:381:25: error: bitwise comparison always evaluates to false [-Werror=tautological-compare]
> USB_SS_PORT_LS_U0) == 1 &&
> ^~
>
> I looked at the code for a bit and came up with a change that makes
> it look like what the author probably meant here. This makes it
> look reasonable to me and to gcc, shutting up the warning.
>
> It does of course change behavior as the two conditions are actually
> evaluated rather than being hardcoded to false, and I have made no
> attempt at verifying that the changed logic makes sense in the context
> of a USB HCD, so that part needs to be reviewed carefully.
>
> Fixes: 1cd8fd2887e1 ("usb: gadget: dummy_hcd: add SuperSpeed support")
> Cc: Tatyana Brokhman <tlinder@xxxxxxxxxxxxxx>
> Cc: Felipe Balbi <balbi@xxxxxxxxxx>
> Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>
> ---
> v2: simplify the expression as suggested by Alan Stern
> ---
> drivers/usb/gadget/udc/dummy_hcd.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c
> index a030d7923d7d..b1e21b3be6e1 100644
> --- a/drivers/usb/gadget/udc/dummy_hcd.c
> +++ b/drivers/usb/gadget/udc/dummy_hcd.c
> @@ -375,11 +375,10 @@ static void set_link_state_by_speed(struct dummy_hcd *dum_hcd)
> USB_PORT_STAT_CONNECTION) == 0)
> dum_hcd->port_status |=
> (USB_PORT_STAT_C_CONNECTION << 16);
> - if ((dum_hcd->port_status &
> - USB_PORT_STAT_ENABLE) == 1 &&
> - (dum_hcd->port_status &
> - USB_SS_PORT_LS_U0) == 1 &&
> - dum_hcd->rh_state != DUMMY_RH_SUSPENDED)
> + if ((dum_hcd->port_status & USB_PORT_STAT_ENABLE) &&
> + (dum_hcd->port_status &
> + USB_PORT_STAT_LINK_STATE) == USB_SS_PORT_LS_U0 &&
> + dum_hcd->rh_state != DUMMY_RH_SUSPENDED)
> dum_hcd->active = 1;
> }
> } else {
Acked-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
