Hi,
John Youn <John.Youn@xxxxxxxxxxxx> writes:
>> John Youn <John.Youn@xxxxxxxxxxxx> writes:
>>>>> @@ -1812,17 +1812,17 @@ static u32 dwc2_hsotg_ep0_mps(unsigned int mps)
>>>>> * @hsotg: The driver state.
>>>>> * @ep: The index number of the endpoint
>>>>> * @mps: The maximum packet size in bytes
>>>>> + * @mc: The multicount value
>>>>> *
>>>>> * Configure the maximum packet size for the given endpoint, updating
>>>>> * the hardware control registers to reflect this.
>>>>> */
>>>>> static void dwc2_hsotg_set_ep_maxpacket(struct dwc2_hsotg *hsotg,
>>>>> - unsigned int ep, unsigned int mps, unsigned int dir_in)
>>>>> + unsigned int ep, unsigned int mps,
>>>>> + unsigned int mc, unsigned int dir_in)
>>>>
>>>> this has an odd set of arguments. You pass the ep index, mps, direction
>>>> and mult value, when you could just pass hsotg_ep and descriptor instead.
>>>
>>> Yes looks like we can do some simplification here. And you probably
>>> don't need to pass a descriptor either since it must be set in the
>>> usb_ep before enable.
>>>
>>> However this is also called in some contexts where a descriptor is not
>>> available (initialization and ep0). So we have to think about this a
>>> bit.
>>>
>>> I think dwc3 can make similar simplification on the
>>> __dwc3_gadget_ep_enable().
>>
>> __dwc3_gadget_ep_enable() takes the actual descriptors as arguments:
>>
>> static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep,
>> const struct usb_endpoint_descriptor *desc,
>> const struct usb_ss_ep_comp_descriptor *comp_desc,
>> bool modify, bool restore)
>>
>> I fail to see how much simpler we can make this. Perhaps we can turn
>> bool and restore into a single argument if we use a bitfield instead of
>> a bool.
>>
>
> Hi Felipe,
>
> I mean that there shouldn't be any need to pass in descriptors with
> usb_ep since the ones in usb_ep should be set properly from ep enable
> to ep disable.
>
> I think that's the case anyways.
__dwc3_gadget_ep_enable() is the one assigning the descriptor to the ep:
static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep,
const struct usb_endpoint_descriptor *desc,
const struct usb_ss_ep_comp_descriptor *comp_desc,
bool modify, bool restore)
{
struct dwc3 *dwc = dep->dwc;
u32 reg;
int ret;
if (!(dep->flags & DWC3_EP_ENABLED)) {
ret = dwc3_gadget_start_config(dwc, dep);
if (ret)
return ret;
}
ret = dwc3_gadget_set_ep_config(dwc, dep, desc, comp_desc, modify,
restore);
if (ret)
return ret;
if (!(dep->flags & DWC3_EP_ENABLED)) {
struct dwc3_trb *trb_st_hw;
struct dwc3_trb *trb_link;
dep->endpoint.desc = desc;
dep->comp_desc = comp_desc;
[...]
--
balbi
Attachment:
signature.asc
Description: PGP signature
