linux-kernel - Re: [PATCH] usb: dwc2: Implement set

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <34b74e48-a3ea-f68f-540e-121ae98afb31@synopsys.com>
Date:   Wed, 5 Feb 2020 07:59:48 +0000
From:   Minas Harutyunyan <Minas.Harutyunyan@...opsys.com>
To:     John Keeping <john@...anate.com>
CC:     Felipe Balbi <balbi@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "open list:DESIGNWARE USB2 DRD IP DRIVER" <linux-usb@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] usb: dwc2: Implement set_selfpowered()

Hi John,

On 2/4/2020 7:29 PM, John Keeping wrote:
> dwc2 always reports as self-powered in response to a device status
> request.  Implement the set_selfpowered() operations so that the gadget
> can report as bus-powered when appropriate.
> 
> This is modelled on the dwc3 implementation.
> 
> Signed-off-by: John Keeping <john@...anate.com>

Good catch. Just one concern. Your patch partially fix my patch this is 
why I think you need to add Fixes tag otherwise it can create merge 
conflict if your patch will be merged to next earlier than my.

Thanks,
Minas


> ---
>   drivers/usb/dwc2/gadget.c | 24 +++++++++++++++++++++++-
>   1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index 2717f4401b97..76c0a5242175 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -1646,7 +1646,8 @@ static int dwc2_hsotg_process_req_status(struct dwc2_hsotg *hsotg,
>   
>   	switch (ctrl->bRequestType & USB_RECIP_MASK) {
>   	case USB_RECIP_DEVICE:
> -		status = 1 << USB_DEVICE_SELF_POWERED;
> +		status = hsotg->gadget.is_selfpowered <<
> +			 USB_DEVICE_SELF_POWERED;
>   		status |= hsotg->remote_wakeup_allowed <<
>   			  USB_DEVICE_REMOTE_WAKEUP;
>   		reply = cpu_to_le16(status);
> @@ -4530,6 +4531,26 @@ static int dwc2_hsotg_gadget_getframe(struct usb_gadget *gadget)
>   	return dwc2_hsotg_read_frameno(to_hsotg(gadget));
>   }
>   
> +/**
> + * dwc2_hsotg_set_selfpowered - set if device is self/bus powered
> + * @gadget: The usb gadget state
> + * @is_selfpowered: Whether the device is self-powered
> + *
> + * Set if the device is self or bus powered.
> + */
> +static int dwc2_hsotg_set_selfpowered(struct usb_gadget *gadget,
> +				      int is_selfpowered)
> +{
> +	struct dwc2_hsotg *hsotg = to_hsotg(gadget);
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&hsotg->lock, flags);
> +	gadget->is_selfpowered = !!is_selfpowered;
> +	spin_unlock_irqrestore(&hsotg->lock, flags);
> +
> +	return 0;
> +}
> +
>   /**
>    * dwc2_hsotg_pullup - connect/disconnect the USB PHY
>    * @gadget: The usb gadget state
> @@ -4621,6 +4642,7 @@ static int dwc2_hsotg_vbus_draw(struct usb_gadget *gadget, unsigned int mA)
>   
>   static const struct usb_gadget_ops dwc2_hsotg_gadget_ops = {
>   	.get_frame	= dwc2_hsotg_gadget_getframe,
> +	.set_selfpowered	= dwc2_hsotg_set_selfpowered,
>   	.udc_start		= dwc2_hsotg_udc_start,
>   	.udc_stop		= dwc2_hsotg_udc_stop,
>   	.pullup                 = dwc2_hsotg_pullup,
>