Hi,
On 2/5/2020 3:56 PM, John Keeping wrote:
> Hi Minas,
>
> On Wed, 5 Feb 2020 07:59:48 +0000
> Minas Harutyunyan <Minas.Harutyunyan@xxxxxxxxxxxx> wrote:
>
>> 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@xxxxxxxxxxxx>
>>
Acked-by: Minas Harutyunyan <hminas@xxxxxxxxxxxx>
>> 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.
>
> I don't think this is actually a fix for your patch, it's a separate fix
> which happens to touch the same code.
>
> Since dwc2 has never supported the set_selfpowered() operation, I'm not
> really sure if this counts as a bugfix or a feature.
>
> I'm happy to re-send with a fixes tag if you think it's necessary, but I
> don't think it's accurate in this case - your patch did not introduce a
> bug here :-)
>
>
> Regards,
> John
>
>>> ---
>>> 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,
>>>
>
