Hi, >Hi, > >Jayshri Pawar wrote: >> This patch adds additional parameter alt to config_ep_by_speed function. >> This additional parameter allows to improve this function and >> find proper usb_ss_ep_comp_descriptor. >> >> Problem has appeared during testing f_tcm (BOT/UAS) driver function. >> >> f_tcm function for SS use array of headers for both BOT/UAS alternate >> setting: >> >> static struct usb_descriptor_header *uasp_ss_function_desc[] = { >> (struct usb_descriptor_header *) &bot_intf_desc, >> (struct usb_descriptor_header *) &bot_uasp_ss_bi_desc, >> (struct usb_descriptor_header *) &bot_bi_ep_comp_desc, >> (struct usb_descriptor_header *) &bot_uasp_ss_bo_desc, >> (struct usb_descriptor_header *) &bot_bo_ep_comp_desc, >> >> (struct usb_descriptor_header *) &uasp_intf_desc, >> (struct usb_descriptor_header *) &bot_uasp_ss_bi_desc, >> (struct usb_descriptor_header *) &uasp_bi_ep_comp_desc, >> (struct usb_descriptor_header *) &uasp_bi_pipe_desc, >> (struct usb_descriptor_header *) &bot_uasp_ss_bo_desc, >> (struct usb_descriptor_header *) &uasp_bo_ep_comp_desc, >> (struct usb_descriptor_header *) &uasp_bo_pipe_desc, >> (struct usb_descriptor_header *) &uasp_ss_status_desc, >> (struct usb_descriptor_header *) &uasp_status_in_ep_comp_desc, >> (struct usb_descriptor_header *) &uasp_status_pipe_desc, >> (struct usb_descriptor_header *) &uasp_ss_cmd_desc, >> (struct usb_descriptor_header *) &uasp_cmd_comp_desc, >> (struct usb_descriptor_header *) &uasp_cmd_pipe_desc, >> NULL, >> }; >> >> The first 5 descriptors are associated with BOT alternate setting, >> and others are associated with UAS. >> >> During handling UAS alternate setting f_tcm drivr invokes >> config_ep_by_speed and this function sets incorrect companion endpoint >> descriptor in usb_ep object. >> >> Instead setting ep->comp_desc to uasp_bi_ep_comp_desc function in this >> case set ep->comp_desc to bot_uasp_ss_bi_desc. >> >> This is due to the fact that it search endpoint based on endpoint >> address: >> >> for_each_ep_desc(speed_desc, d_spd) { >> chosen_desc = (struct usb_endpoint_descriptor *)*d_spd; >> if (chosen_desc->bEndpoitAddress == _ep->address) >> goto ep_found; >> } >> >> And in result it uses the descriptor from BOT alternate setting >> instead UAS. >> >> Finally, it causes that controller driver during enabling endpoints >> detect that just enabled endpoint for bot. >> >> Signed-off-by: Jayshri Pawar <jpawar@xxxxxxxxxxx> >> Signed-off-by: Pawel Laszczak <pawell@xxxxxxxxxxx> >> --- >> drivers/usb/gadget/composite.c | 46 ++++++++++++++------ >> drivers/usb/gadget/function/f_acm.c | 7 +-- >> drivers/usb/gadget/function/f_ecm.c | 7 +-- >> drivers/usb/gadget/function/f_eem.c | 4 +- >> drivers/usb/gadget/function/f_fs.c | 3 +- >> drivers/usb/gadget/function/f_hid.c | 4 +- >> drivers/usb/gadget/function/f_loopback.c | 2 +- >> drivers/usb/gadget/function/f_mass_storage.c | 5 ++- >> drivers/usb/gadget/function/f_midi.c | 2 +- >> drivers/usb/gadget/function/f_ncm.c | 7 +-- >> drivers/usb/gadget/function/f_obex.c | 4 +- >> drivers/usb/gadget/function/f_phonet.c | 4 +- >> drivers/usb/gadget/function/f_rndis.c | 7 +-- >> drivers/usb/gadget/function/f_serial.c | 4 +- >> drivers/usb/gadget/function/f_sourcesink.c | 11 +++-- >> drivers/usb/gadget/function/f_subset.c | 4 +- >> drivers/usb/gadget/function/f_tcm.c | 36 +++++++-------- >> drivers/usb/gadget/function/f_uac1_legacy.c | 2 +- >> drivers/usb/gadget/function/f_uvc.c | 5 ++- >> drivers/usb/gadget/function/u_audio.c | 4 +- >> include/linux/usb/composite.h | 2 +- >> 21 files changed, 99 insertions(+), 71 deletions(-) >> > >I think we should create a new function and keep the old >config_ep_by_speed() for default alt-setting (e.i. have >config_ep_by_speed calls the new function with default alt-setting 0). >This way, we can keep compatibility with old function drivers and >minimize changes. At least that's what we did. > I don't think we need the separate function. If we set last parameter alt=0 then this function will work in the same way as old one. Regards, Pawel >BR, >Thinh