Hi,
Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> writes:
> Felipe Balbi wrote:
>> Hi,
>>
>> Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> writes:
>>> @@ -2476,6 +2506,17 @@ static void dwc3_gadget_set_speed(struct usb_gadget *g,
>>> spin_unlock_irqrestore(&dwc->lock, flags);
>>> }
>>>
>>> +static void dwc3_gadget_set_ssp_rate(struct usb_gadget *g,
>>> + enum usb_ssp_rate rate)
>>> +{
>>> + struct dwc3 *dwc = gadget_to_dwc(g);
>>> + unsigned long flags;
>>> +
>>> + spin_lock_irqsave(&dwc->lock, flags);
>>> + dwc->gadget_ssp_rate = rate;
>>> + spin_unlock_irqrestore(&dwc->lock, flags);
>>> +}
>> it would be best to make this return a value. If udc_set_ssp_rate() is
>> called with invalid rate, UDC can notify core.
>>
>
> The core should know what ssp rate the gadget supports via the
> gadget->max_ssp_rate capability field. Any rate beyond that is invalid.
> Is it necessary to have a return value here? This uses the same logic as
> udc_set_speed()
Yeah, I don't know what I had in mind when I made ->udc_set_speed()
void. Then again, we know exactly who's calling it and we can guarantee
that no invalid values will be passed. There's no way for, for example,
userspace (via ffs) to call it with a bogus value.
Perhaps it's okay, but something to keep an eye for both
->udc_set_ssp_rate() and ->udc_set_speed().
Thanks for pointing me back at ->udc_set_speed().
--
balbi
Attachment:
signature.asc
Description: PGP signature
