Hi Minas: >-----Original Message----- >From: Minas Harutyunyan [mailto:Minas.Harutyunyan@xxxxxxxxxxxx] >Sent: Wednesday, March 21, 2018 4:08 PM >To: Zengtao (B) <prime.zeng@xxxxxxxxxxxxx>; Minas Harutyunyan ><Minas.Harutyunyan@xxxxxxxxxxxx>; John Youn <John.Youn@xxxxxxxxxxxx>; >Felipe Balbi <balbi@xxxxxxxxxx>; Greg Kroah-Hartman ><gregkh@xxxxxxxxxxxxxxxxxxx>; linux-usb@xxxxxxxxxxxxxxx >Subject: Re: [PATCH v1 2/3] usb: dwc2: Change ISOC DDMA flow > >Hi Zengtao, > >On 3/21/2018 6:17 AM, Zengtao (B) wrote: >>> -----Original Message----- >> >>> From: linux-usb-owner@xxxxxxxxxxxxxxx >> >>> [mailto:linux-usb-owner@xxxxxxxxxxxxxxx] On Behalf Of Minas >>> Harutyunyan >> >>> Sent: Tuesday, March 20, 2018 10:40 PM >> >>> To: Zengtao (B) <prime.zeng@xxxxxxxxxxxxx>; Minas Harutyunyan >> >>> <Minas.Harutyunyan@xxxxxxxxxxxx>; John Youn ><John.Youn@xxxxxxxxxxxx>; >> >>> Felipe Balbi <balbi@xxxxxxxxxx>; Greg Kroah-Hartman >> >>> <gregkh@xxxxxxxxxxxxxxxxxxx>; linux-usb@xxxxxxxxxxxxxxx >> >>> Subject: Re: [PATCH v1 2/3] usb: dwc2: Change ISOC DDMA flow >> >>> >> >>> Hi Zengtao, >> >>> >> >>> On 3/20/2018 6:01 AM, Zengtao (B) wrote: >> >>>> Hi Minas: >> >>>> >> >>>> >> >>>> >> >>>> A few minor comments: >> >>>> >> >>>> >> >>>> >> >>>>> -----Original Message----- >> >>>> >> >>>>> From: linux-usb-owner@xxxxxxxxxxxxxxx >> >>>> >> >>>>> [mailto:linux-usb-owner@xxxxxxxxxxxxxxx] On Behalf Of Minas >> >>>>> Harutyunyan >> >>>> >> >>>>> Sent: Saturday, March 17, 2018 5:10 PM >> >>>> >> >>>>> To: John Youn <John.Youn@xxxxxxxxxxxx>; Felipe Balbi >> >>>>> <balbi@xxxxxxxxxx>; >> >>>> >> >>>>> Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>; >> >>>>> linux-usb@xxxxxxxxxxxxxxx >> >>>> >> >>>>> Cc: Minas Harutyunyan <Minas.Harutyunyan@xxxxxxxxxxxx> >> >>>> >> >>>>> Subject: [PATCH v1 2/3] usb: dwc2: Change ISOC DDMA flow >> >>>> >> >>>>> >> >>>> >> >>>>> Changed existing two descriptor-chain flow to one chain. >> >>>> >> >>>>> >> >>>> >> >>>>> In two-chain implementation BNA interrupt used for switching >>>>> between >> >>>>> two >> >>>> >> >>>>> chains. BNA interrupt asserted because of returning to beginning of >> >>>>> the chain >> >>>> >> >>>>> based on L-bit of last descriptor. >> >>>> >> >>>>> >> >>>> >> >>>>> Because of that we lose packets. This issue resolved by using one >desc-chain. >> >>>> >> >>>>> >> >>>> >> >>>>> Removed all staff related to two desc-chain flow from DDMA ISOC >> >>>>> related >> >>>> >> >>>>> functions. >> >>>> >> >>>>> >> >>>> >> >>>>> Removed request length checking from dwc2_gadget_fill_isoc_desc() >> >>> function. >> >>>> >> >>>>> Request length checking added to dwc2_hsotg_ep_queue() function. If >> >>>>> request >> >>>> >> >>>>> length greater than descriptor limits then request not added to queue. >> >>>> >> >>>>> Additional checking done for High Bandwidth ISOC OUT's which not >> >>>>> supported by >> >>>> >> >>>>> driver. In >> >>>> >> >>>>> dwc2_gadget_fill_isoc_desc() function also checked desc-chain >>>>> status >> >>>>> (full or not) >> >>>> >> >>>>> to avoid of reusing not yet processed descriptors. >> >>>> >> >>>>> >> >>>> >> >>>>> In dwc2_gadget_start_isoc_ddma() function creation of desc-chain >> >>>>> always >> >>>> >> >>>>> started from descriptor 0. Before filling descriptors, they were >> >>>>> initialized by >> >>>> >> >>>>> HOST BUSY status. >> >>>> >> >>>>> >> >>>> >> >>>>> In dwc2_gadget_complete_isoc_request_ddma() added checking for >> >>>>> desc-chain >> >>>> >> >>>>> rollover. Also added checking completion status. >> >>>> >> >>>>> Request completed successfully if DEV_DMA_STS is DEV_DMA_STS_SUCC, >> >>>> >> >>>>> otherwise complete with -ETIMEDOUT. >> >>>> >> >>>>> >> >>>> >> >>>>> Actually removed dwc2_gadget_start_next_isoc_ddma() function >>>>> because >> >>>>> now >> >>>> >> >>>>> driver use only one desc-chain and instead that function added >> >>>> >> >>>>> dwc2_gadget_handle_isoc_bna() function for handling BNA interrupts. >> >>>> >> >>>>> >> >>>> >> >>>>> Handling BNA interrupt done by flushing TxFIFOs for OUT EPs, >> >>>>> completing >> >>>> >> >>>>> request with -EIO and resetting desc-chain number and target frame >>>>> to >> >>>>> initial >> >>>> >> >>>>> values for restarting transfers. >> >>>> >> >>>>> >> >>>> >> >>>>> On handling NAK request completed with -ENODATA. Incremented target >> >>>>> frame >> >>>> >> >>>>> to allow fill desc chain and start transfers. >> >>>> >> >>>>> In DDMA mode avoided of frame number incrementing, because tracking >> >>>>> of >> >>>> >> >>>>> frame number performed in dwc2_gadget_fill_isoc_desc() function. >> >>>> >> >>>>> >> >>>> >> >>>>> When core assert XferCompl along with BNA, we should ignore >>>>> XferCompl >> >>>>> in >> >>>> >> >>>>> dwc2_hsotg_epint() function. >> >>>> >> >>>>> >> >>>> >> >>>>> On BNA interrupt replaced dwc2_gadget_start_next_isoc_ddma() by >>>>> above >> >>>> >> >>>>> mentioned BNA handler. >> >>>> >> >>>>> >> >>>> >> >>>>> In dwc2_hsotg_ep_enable() function added sanity check of bInterval >> >>>>> for ISOC IN >> >>>> >> >>>>> in DDMA mode, because HW not supported EP's with bInterval more than >12. >> >>>> >> >>>>> >> >>>> >> >>>>> Signed-off-by: Minas Harutyunyan <hminas@xxxxxxxxxxxx> >> >>>> >> >>>>> --- >> >>>> >> >>>>> drivers/usb/dwc2/core.h | 2 - >> >>>> >> >>>>> drivers/usb/dwc2/gadget.c | 235 >> >>>>> ++++++++++++++++++++++------------------------ >> >>>> >> >>>>> 2 files changed, 113 insertions(+), 124 deletions(-) >> >>>> >> >>>>> >> >>>> >> >>>>> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h >>>>> index >> >>>> >> >>>>> d83be5651f87..093d078adaf4 100644 >> >>>> >> >>>>> --- a/drivers/usb/dwc2/core.h >> >>>> >> >>>>> +++ b/drivers/usb/dwc2/core.h >> >>>> >> >>>>> @@ -178,7 +178,6 @@ struct dwc2_hsotg_req; >> >>>> >> >>>>> * @desc_list_dma: The DMA address of descriptor chain currently in >use. >> >>>> >> >>>>> * @desc_list: Pointer to descriptor DMA chain head currently in use. >> >>>> >> >>>>> * @desc_count: Count of entries within the DMA descriptor chain of >EP. >> >>>> >> >>>>> - * @isoc_chain_num: Number of ISOC chain currently in use - either 0 or 1. >> >>>> >> >>>>> * @next_desc: index of next free descriptor in the ISOC chain >>>>> under >> >>>>> SW >> >>>> >> >>>>> control. >> >>>> >> >>>>> * @total_data: The total number of data bytes done. >> >>>> >> >>>>> * @fifo_size: The size of the FIFO (for periodic IN endpoints) >>>>> @@ >> >>>>> -231,7 >> >>>> >> >>>>> +230,6 @@ struct dwc2_hsotg_ep { >> >>>> >> >>>>> struct dwc2_dma_desc *desc_list; >> >>>> >> >>>>> u8 desc_count; >> >>>> >> >>>>> >> >>>> >> >>>>> - unsigned char isoc_chain_num; >> >>>> >> >>>>> unsigned int next_desc; >> >>>> >> >>>>> >> >>>> >> >>>>> char name[10]; >> >>>> >> >>>>> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c >> >>>>> index >> >>>> >> >>>>> c231321656f9..1b9c84cb58fb 100644 >> >>>> >> >>>>> --- a/drivers/usb/dwc2/gadget.c >> >>>> >> >>>>> +++ b/drivers/usb/dwc2/gadget.c >> >>>> >> >>>>> @@ -793,9 +793,7 @@ static void >> >>>> >> >>>>> dwc2_gadget_config_nonisoc_xfer_ddma(struct dwc2_hsotg_ep *hs_ep, >> >>>> >> >>>>> * @dma_buff: usb requests dma buffer. >> >>>> >> >>>>> * @len: usb request transfer length. >> >>>> >> >>>>> * >> >>>> >> >>>>> - * Finds out index of first free entry either in the bottom or up >> >>>>> half of >> >>>> >> >>>>> - * descriptor chain depend on which is under SW control and not >> >>>>> processed >> >>>> >> >>>>> - * by HW. Then fills that descriptor with the data of the arrived >> >>>>> usb request, >> >>>> >> >>>>> + * Fills next free descriptor with the data of the arrived usb >> >>>>> + request, >> >>>> >> >>>>> * frame info, sets Last and IOC bits increments next_desc. If >> >>>>> filled >> >>>> >> >>>>> * descriptor is not the first one, removes L bit from the >>>>> previous >> >>>>> descriptor >> >>>> >> >>>>> * status. >> >>>> >> >>>>> @@ -810,34 +808,17 @@ static int dwc2_gadget_fill_isoc_desc(struct >> >>>> >> >>>>> dwc2_hsotg_ep *hs_ep, >> >>>> >> >>>>> u32 mask = 0; >> >>>> >> >>>>> >> >>>> >> >>>>> maxsize = dwc2_gadget_get_desc_params(hs_ep, &mask); >> >>>> >> >>>>> - if (len > maxsize) { >> >>>> >> >>>>> - dev_err(hsotg->dev, "wrong len %d\n", len); >> >>>> >> >>>>> - return -EINVAL; >> >>>> >> >>>>> - } >> >>>> >> >>>>> - >> >>>> >> >>>>> - /* >> >>>> >> >>>>> - * If SW has already filled half of chain, then return and wait for >> >>>> >> >>>>> - * the other chain to be processed by HW. >> >>>> >> >>>>> - */ >> >>>> >> >>>>> - if (hs_ep->next_desc == MAX_DMA_DESC_NUM_GENERIC / 2) >> >>>> >> >>>>> - return -EBUSY; >> >>>> >> >>>>> >> >>>> >> >>>>> - /* Increment frame number by interval for IN */ >> >>>> >> >>>>> - if (hs_ep->dir_in) >> >>>> >> >>>>> - dwc2_gadget_incr_frame_num(hs_ep); >> >>>> >> >>>>> - >> >>>> >> >>>>> - index = (MAX_DMA_DESC_NUM_GENERIC / 2) * >hs_ep->isoc_chain_num + >> >>>> >> >>>>> - hs_ep->next_desc; >> >>>> >> >>>>> + index = hs_ep->next_desc; >> >>>> >> >>>>> + desc = &hs_ep->desc_list[index]; >> >>>> >> >>>>> >> >>>> >> >>>>> - /* Sanity check of calculated index */ >> >>>> >> >>>>> - if ((hs_ep->isoc_chain_num && index > >MAX_DMA_DESC_NUM_GENERIC) >> >>>> >> >>>>> || >> >>>> >> >>>>> - (!hs_ep->isoc_chain_num && index > >> >>>> >> >>>>> MAX_DMA_DESC_NUM_GENERIC / 2)) { >> >>>> >> >>>>> - dev_err(hsotg->dev, "wrong index %d for iso chain\n", index); >> >>>> >> >>>>> - return -EINVAL; >> >>>> >> >>>>> + /* Check if descriptor chain full */ >> >>>> >> >>>>> + if ((desc->status >> DEV_DMA_BUFF_STS_SHIFT) == >> >>>> >> >>>>> + DEV_DMA_BUFF_STS_HREADY) { >> >>>> >> >>>>> + dev_dbg(hsotg->dev, "%s: desc chain full\n", __func__); >> >>>> >> >>>>> + return 1; >> >>>> >> >>>>> } >> >>>> >> >>>>> >> >>>> >> >>>>> - desc = &hs_ep->desc_list[index]; >> >>>> >> >>>>> - >> >>>> >> >>>>> /* Clear L bit of previous desc if more than one entries in the >> >>>>> chain */ >> >>>> >> >>>>> if (hs_ep->next_desc) >> >>>> >> >>>>> hs_ep->desc_list[index - 1].status &= ~DEV_DMA_L; @@ -865,8 >> >>>> >> >>>> >> >>>> >> >>>> When changing the status of the desc, How to sync the SW with the HW? >> >>>> >> >>>> Since the HW and SW is working on the same desc synchronously. >> >>> >> >>> Not clear for what you mean: HW and SW working on same desc >synchronously? >> >>> Descriptor list is like Producer(SW)/Consumer(HW) list. >> >>> Index management allow always keep gap between Producer and Consumer >> >>> indexes. Producer index should be always more than Consumer index. In >>> some >> >>> cases when Consumer index achieved Producer index, which mean no more >> >>> ready descriptor to process then BNA interrupt will be asserted, i.e >>> SW can't >> >>> enough bandwidth to submit new requests. >> >>> >> >> >> >> About my question, >> >> I mean when you clear the DEV_DMA_L bit of last desc in the chain, the >> HW may be >> >> accessing this particular desc at the same time, so perhaps the >> DEV_DMA_L bit >> >> clear action is not sync to the HW, and the HW can't work as we want. >> >> >Core always fetching 1 descriptor based on: >1. ISOC IN - one uf before target uf. >2. ISOC OUT - RxFIFO NonEmpty, EP enabled. >As I mentioned before, between producer and consumer indexes should be >enough gap. If not, then BNA will be asserted, EP will be disabled and will need >to restart transfers from scratch based on IN-NAK (ISOC IN) or OUT-EPDis (ISOC >OUT) interrupts. >> How do you ensure enough gap between the producer and consumer? The producer fill new desc at random time, and when clearing the DEV_DMA_L bit of the last desc in the chain, interrupts is disabled and we may delay or miss the BNA interrupt. Linux is not a real time system, so I think we should consider such corner cases. >> Per your comment, you mentioned the producer and consumer, but >> >> How do you do the synchronization between the producer and consumer? >> >Actually, as soon as consumer complete some desc then dwc2 complete >appropriate producers request to function driver then function driver queuing >new request. So, initial gap will be kept. >> It is not mandatary for the function driver to queuing new request in the complete handler. we can't depend on the function driver to keep the gap. >> >>>> >> >>>>> >> >>>>> +846,14 @@ static int dwc2_gadget_fill_isoc_desc(struct >>>>> +dwc2_hsotg_ep >> >>>> >> >>>>> *hs_ep, >> >>>> >> >>>>> desc->status &= ~DEV_DMA_BUFF_STS_MASK; >> >>>> >> >>>>> desc->status |= (DEV_DMA_BUFF_STS_HREADY << >> >>>> >> >>>>> DEV_DMA_BUFF_STS_SHIFT); >> >>>> >> >>>>> >> >>>> >> >>>>> + /* Increment frame number by interval for IN */ >> >>>> >> >>>>> + if (hs_ep->dir_in) >> >>>> >> >>>>> + dwc2_gadget_incr_frame_num(hs_ep); >> >>>> >> >>>>> + >> >>>> >> >>>>> /* Update index of last configured entry in the chain */ >> >>>> >> >>>>> hs_ep->next_desc++; >> >>>> >> >>>>> + if (hs_ep->next_desc >= MAX_DMA_DESC_NUM_GENERIC) >> >>>> >> >>>>> + hs_ep->next_desc = 0; >> >>>> >> >>>>> >> >>>> >> >>>>> return 0; >> >>>> >> >>>>> } >> >>>> >> >>>>> @@ -875,11 +862,8 @@ static int dwc2_gadget_fill_isoc_desc(struct >> >>>> >> >>>>> dwc2_hsotg_ep *hs_ep, >> >>>> >> >>>>> * dwc2_gadget_start_isoc_ddma - start isochronous transfer in >>>>> DDMA >> >>>> >> >>>>> * @hs_ep: The isochronous endpoint. >> >>>> >> >>>>> * >> >>>> >> >>>>> - * Prepare first descriptor chain for isochronous endpoints. >> >>>>> Afterwards >> >>>> >> >>>>> + * Prepare descriptor chain for isochronous endpoints. Afterwards >> >>>> >> >>>>> * write DMA address to HW and enable the endpoint. >> >>>> >> >>>>> - * >> >>>> >> >>>>> - * Switch between descriptor chains via isoc_chain_num to give SW >> >>>>> opportunity >> >>>> >> >>>>> - * to prepare second descriptor chain while first one is being >>>>> processed by >> >>> HW. >> >>>> >> >>>>> */ >> >>>> >> >>>>> static void dwc2_gadget_start_isoc_ddma(struct dwc2_hsotg_ep >>>>> *hs_ep) >> >>>> >> >>>>> { @@ -890,19 +874,27 @@ static void >> >>>>> dwc2_gadget_start_isoc_ddma(struct >> >>>> >> >>>>> dwc2_hsotg_ep *hs_ep) >> >>>> >> >>>>> u32 dma_reg; >> >>>> >> >>>>> u32 depctl; >> >>>> >> >>>>> u32 ctrl; >> >>>> >> >>>>> + struct dwc2_dma_desc *desc; >> >>>> >> >>>>> >> >>>> >> >>>>> if (list_empty(&hs_ep->queue)) { >> >>>> >> >>>>> dev_dbg(hsotg->dev, "%s: No requests in queue\n", __func__); >> >>>> >> >>>>> return; >> >>>> >> >>>>> } >> >>>> >> >>>>> >> >>>> >> >>>>> + /* Initialize descriptor chain by Host Busy status */ >> >>>> >> >>>>> + for (ret = 0; ret < MAX_DMA_DESC_NUM_GENERIC; ret++) { >> >>>> >> >>>>> + desc = &hs_ep->desc_list[ret]; >> >>>> >> >>>>> + desc->status = 0; >> >>>> >> >>>>> + desc->status |= (DEV_DMA_BUFF_STS_HBUSY >> >>>> >> >>>>> + << DEV_DMA_BUFF_STS_SHIFT); >> >>>> >> >>>>> + } >> >>>> >> >>>>> + >> >>>> >> >>>> >> >>>> >> >>>> Ret is not a good naming as the loop counter. >> >>>> >> >>> >> >>> Could be, but it just reuse existing variable instead to define new one. >> >>> >> >> >> >> From the code reader's perspective, it is a bit strange. >> >Ok, will declare new variable. >> >> >>>> >> >>>> >> >>>>> + hs_ep->next_desc = 0; >> >>>> >> >>>>> list_for_each_entry_safe(hs_req, treq, &hs_ep->queue, queue) { >> >>>> >> >>>> >> >>>> >> >>>> Do we really need safe function here? We have already have the spinlock. >> >>>> >> >>> I left existing list_for_each_entry_safe() not of part of this patch. >> >>> >> >>>> >> >>>> >> >>>>> ret = dwc2_gadget_fill_isoc_desc(hs_ep, hs_req->req.dma, >> >>>> >> >>>>> hs_req->req.length); >> >>>> >> >>>>> - if (ret) { >> >>>> >> >>>>> - dev_dbg(hsotg->dev, "%s: desc chain full\n", __func__); >> >>>> >> >>>>> + if (ret) >> >>>> >> >>>>> break; >> >>>> >> >>>>> - } >> >>>> >> >>>>> } >> >>>> >> >>>>> >> >>>> >> >>>>> depctl = hs_ep->dir_in ? DIEPCTL(index) : DOEPCTL(index); @@ >>>>> -914,10 >> >>>> >> >>>>> +906,6 @@ static void dwc2_gadget_start_isoc_ddma(struct >> >>>>> +dwc2_hsotg_ep >> >>>> >> >>>>> *hs_ep) >> >>>> >> >>>>> ctrl = dwc2_readl(hsotg->regs + depctl); >> >>>> >> >>>>> ctrl |= DXEPCTL_EPENA | DXEPCTL_CNAK; >> >>>> >> >>>>> dwc2_writel(ctrl, hsotg->regs + depctl); >> >>>> >> >>>>> - >> >>>> >> >>>>> - /* Switch ISOC descriptor chain number being processed by SW*/ >> >>>> >> >>>>> - hs_ep->isoc_chain_num = (hs_ep->isoc_chain_num ^ 1) & 0x1; >> >>>> >> >>>>> - hs_ep->next_desc = 0; >> >>>> >> >>>>> } >> >>>> >> >>>>> >> >>>> >> >>>>> /** >> >>>> >> >>>>> @@ -1291,6 +1279,9 @@ static int dwc2_hsotg_ep_queue(struct usb_ep >> >>>>> *ep, >> >>>> >> >>>>> struct usb_request *req, >> >>>> >> >>>>> struct dwc2_hsotg *hs = hs_ep->parent; >> >>>> >> >>>>> bool first; >> >>>> >> >>>>> int ret; >> >>>> >> >>>>> + u32 maxsize = 0; >> >>>> >> >>>>> + u32 mask = 0; >> >>>> >> >>>>> + >> >>>> >> >>>>> >> >>>> >> >>>>> dev_dbg(hs->dev, "%s: req %p: %d@%p, noi=%d, zero=%d, >snok=%d\n", >> >>>> >> >>>>> ep->name, req, req->length, req->buf, req->no_interrupt, @@ >>>>> -1308,6 >> >>>> >> >>>>> +1299,24 @@ static int dwc2_hsotg_ep_queue(struct usb_ep *ep, >>>>> +struct >> >>>> >> >>>>> usb_request *req, >> >>>> >> >>>>> req->actual = 0; >> >>>> >> >>>>> req->status = -EINPROGRESS; >> >>>> >> >>>>> >> >>>> >> >>>>> + /* In DDMA mode for ISOC's don't queue request if length greater >> >>>> >> >>>>> + * than descriptor limits. >> >>>> >> >>>>> + */ >> >>>> >> >>>>> + if (using_desc_dma(hs) && hs_ep->isochronous) { >> >>>> >> >>>>> + maxsize = dwc2_gadget_get_desc_params(hs_ep, &mask); >> >>>> >> >>>>> + if (hs_ep->dir_in && req->length > maxsize) { >> >>>> >> >>>>> + dev_err(hs->dev, "wrong length %d (maxsize=%d)\n", >> >>>> >> >>>>> + req->length, maxsize); >> >>>> >> >>>>> + return -EINVAL; >> >>>> >> >>>>> + } >> >>>> >> >>>>> + /* ISOC OUT high bandwidth not supported */ >> >>>> >> >>>>> + if (!hs_ep->dir_in && req->length > hs_ep->ep.maxpacket) { >> >>>> >> >>>>> + dev_err(hs->dev, "ISOC OUT: wrong length %d (mps=%d)\n", >> >>>> >> >>>>> + req->length, hs_ep->ep.maxpacket); >> >>>> >> >>>>> + return -EINVAL; >> >>>> >> >>>>> + } >> >>>> >> >>>>> + } >> >>>> >> >>>>> + >> >>>> >> >>>>> ret = dwc2_hsotg_handle_unaligned_buf_start(hs, hs_ep, hs_req); >> >>>> >> >>>>> if (ret) >> >>>> >> >>>>> return ret; >> >>>> >> >>>>> @@ -1330,7 +1339,7 @@ static int dwc2_hsotg_ep_queue(struct usb_ep >> >>>>> *ep, >> >>>> >> >>>>> struct usb_request *req, >> >>>> >> >>>>> >> >>>> >> >>>>> /* >> >>>> >> >>>>> * Handle DDMA isochronous transfers separately - just add new >>>>> entry >> >>>> >> >>>>> - * to the half of descriptor chain that is not processed by HW. >> >>>> >> >>>>> + * to the descriptor chain. >> >>>> >> >>>>> * Transfer will be started once SW gets either one of NAK or >> >>>> >> >>>>> * OutTknEpDis interrupts. >> >>>> >> >>>>> */ >> >>>> >> >>>>> @@ -1338,9 +1347,9 @@ static int dwc2_hsotg_ep_queue(struct usb_ep >> >>>>> *ep, >> >>>> >> >>>>> struct usb_request *req, >> >>>> >> >>>>> hs_ep->target_frame != TARGET_FRAME_INITIAL) { >> >>>> >> >>>>> ret = dwc2_gadget_fill_isoc_desc(hs_ep, hs_req->req.dma, >> >>>> >> >>>>> hs_req->req.length); >> >>>> >> >>>>> - if (ret) >> >>>> >> >>>>> - dev_dbg(hs->dev, "%s: ISO desc chain full\n", __func__); >> >>>> >> >>>>> - >> >>>> >> >>>>> + if (ret < 0) >> >>>> >> >>>>> + dev_dbg(hs->dev, "%s: failed to fill isoc desc\n", >> >>>> >> >>>>> + __func__); >> >>>> >> >>>> >> >>>> >> >>>> dwc2_gadget_fill_isoc_desc will never return a negative value, and >>>> at >> >>>> the same >> >>>> >> >>>> time I think there is no need to check the return value, we can work >> >>>> properly even >> >>>> >> >>>> the desc chain is full. >> >>>> >> >>> You are rigth. ret is 0 or 1. So, no need to change existing code. >>> Yes, we can >> >>> continue work properly even if the desc chain full. It just debug >>> message if (ret != >> >>> 0). Do you have objection? >> >>> >> >> >> >> Since we already have debug message inside dwc2_gadget_fill_isoc_desc, >> so here >> >> we don't need additional debug message, what do you think of it? >> >> >Agree with you, will remove. >> >> >> >>> >> >>>> >> >>>> >> >>>>> return 0; >> >>>> >> >>>>> } >> >>>> >> >>>>> >> >>>> >> >>>>> @@ -2011,10 +2020,9 @@ static void >> >>> dwc2_hsotg_complete_request(struct >> >>>> >> >>>>> dwc2_hsotg *hsotg, >> >>>> >> >>>>> * @hs_ep: The endpoint the request was on. >> >>>> >> >>>>> * >> >>>> >> >>>>> * Get first request from the ep queue, determine descriptor on >> >>>>> which >> >>>> >> >>>>> complete >> >>>> >> >>>>> - * happened. SW based on isoc_chain_num discovers which half of >>>>> the >> >>>> >> >>>>> descriptor >> >>>> >> >>>>> - * chain is currently in use by HW, adjusts dma_address and >> >>>>> calculates index >> >>>> >> >>>>> - * of completed descriptor based on the value of DEPDMA register. >> >>>>> Update >> >>>> >> >>>>> actual >> >>>> >> >>>>> - * length of request, giveback to gadget. >> >>>> >> >>>>> + * happened. SW discovers which descriptor currently in use by HW, >> >>>> >> >>>>> + adjusts >> >>>> >> >>>>> + * dma_address and calculates index of completed descriptor based >>>>> + on >> >>>> >> >>>>> + the value >> >>>> >> >>>>> + * of DEPDMA register. Update actual length of request, giveback to >gadget. >> >>>> >> >>>>> */ >> >>>> >> >>>>> static void dwc2_gadget_complete_isoc_request_ddma(struct >> >>>>> dwc2_hsotg_ep >> >>>> >> >>>>> *hs_ep) { @@ -2037,82 +2045,55 @@ static void >> >>>> >> >>>>> dwc2_gadget_complete_isoc_request_ddma(struct dwc2_hsotg_ep >*hs_ep) >> >>>> >> >>>>> >> >>>> >> >>>>> dma_addr = hs_ep->desc_list_dma; >> >>>> >> >>>>> >> >>>> >> >>>>> - /* >> >>>> >> >>>>> - * If lower half of descriptor chain is currently use by SW, >> >>>> >> >>>>> - * that means higher half is being processed by HW, so shift >> >>>> >> >>>>> - * DMA address to higher half of descriptor chain. >> >>>> >> >>>>> - */ >> >>>> >> >>>>> - if (!hs_ep->isoc_chain_num) >> >>>> >> >>>>> - dma_addr += sizeof(struct dwc2_dma_desc) * >> >>>> >> >>>>> - (MAX_DMA_DESC_NUM_GENERIC / 2); >> >>>> >> >>>>> - >> >>>> >> >>>>> dma_reg = hs_ep->dir_in ? DIEPDMA(hs_ep->index) : >> >>>> >> >>>>> DOEPDMA(hs_ep->index); >> >>>> >> >>>>> depdma = dwc2_readl(hsotg->regs + dma_reg); >> >>>> >> >>>>> >> >>>> >> >>>>> index = (depdma - dma_addr) / sizeof(struct dwc2_dma_desc) - 1; >> >>>> >> >>>>> - desc_sts = hs_ep->desc_list[index].status; >> >>>> >> >>>>> + /* Check descriptor chain rollover */ >> >>>> >> >>>>> + if (index < 0) >> >>>> >> >>>>> + index = MAX_DMA_DESC_NUM_GENERIC - 1; >> >>>> >> >>>>> >> >>>> >> >>>> >> >>>> >> >>>> I don't understand here, why setting the index to >> >>>> >> >>>> MAX_DMA_DESC_NUM_GENERIC - 1 when the chain is rollover? >> >>>> >> >>>> If the chain is rollover, the index should point to the latest >> >>>> finished desc, >> >>>> >> >>>> and all the finished descs should be processed. And even the chain >>>> is >> >>>> rollover, >> >>>> >> >>>> the count of descs in chain is maybe less then >> >>> MAX_DMA_DESC_NUM_GENERIC. >> >>>> >> >>> If depdma point to first desc in list, that mean the last processed >>> desc was last >> >>> in the chain. In this case index will be negative. >> >>> In case of desc count less than MAX_DMA_DESC_NUM_GENERIC then >>> rollover >> >>> can be happen because of L-bit set. In this case by processing first >>> desc will be >> >>> asserted BNA interrupt but not XferComplete interrupt. >> >>> >> >> >> >> Can we reach here in BNA interrupt, or can we have both BNA and >> XferComplete >> >> Interrupts asserted at the same time? >> >> Your conclusion is based on the following assumption: >> >> 1. BNA interrupt handle flow can't go into here. >> >> 2. when the execution flow going to here, only because of XferComplete >interrupt. >> >> 3. when the XferComplete is asserted, BNA interrupt can't be asserted. >> >> >In case of both interrupt asserted then XferComplete ignored. See >dwc2_hsotg_epint() function. Actually, in my test cases with bInterval=1 it >happen on last descriptor and no any new requests from function driver, >because EP disabled with some latency in core BNA also asserted. >But its corner case and can be ignored. > Then what about adding some assert here to catch the core case? >> >>>> >> >>>> >> >>>>> - mask = hs_ep->dir_in ? DEV_DMA_ISOC_TX_NBYTES_MASK : >> >>>> >> >>>>> - DEV_DMA_ISOC_RX_NBYTES_MASK; >> >>>> >> >>>>> - ureq->actual = ureq->length - >> >>>> >> >>>>> - ((desc_sts & mask) >> DEV_DMA_ISOC_NBYTES_SHIFT); >> >>>> >> >>>>> - >> >>>> >> >>>>> - /* Adjust actual length for ISOC Out if length is not align of 4 */ >> >>>> >> >>>>> - if (!hs_ep->dir_in && ureq->length & 0x3) >> >>>> >> >>>>> - ureq->actual += 4 - (ureq->length & 0x3); >> >>>> >> >>>>> + desc_sts = hs_ep->desc_list[index].status; >> >>>> >> >>>>> + /* Check completion status */ >> >>>> >> >>>>> + if ((desc_sts & DEV_DMA_STS_MASK) >> DEV_DMA_STS_SHIFT == >> >>>> >> >>>>> + DEV_DMA_STS_SUCC) { >> >>>> >> >>>>> + mask = hs_ep->dir_in ? DEV_DMA_ISOC_TX_NBYTES_MASK : >> >>>> >> >>>>> + DEV_DMA_ISOC_RX_NBYTES_MASK; >> >>>> >> >>>>> + ureq->actual = ureq->length - >> >>>> >> >>>>> + ((desc_sts & mask) >> >> >>>> >> >>>>> + DEV_DMA_ISOC_NBYTES_SHIFT); >> >>>> >> >>>>> + >> >>>> >> >>>>> + /* Adjust actual len for ISOC Out if len is not align of 4 */ >> >>>> >> >>>>> + if (!hs_ep->dir_in && ureq->length & 0x3) >> >>>> >> >>>>> + ureq->actual += 4 - (ureq->length & 0x3); >> >>>> >> >>>>> >> >>>> >> >>>>> - dwc2_hsotg_complete_request(hsotg, hs_ep, hs_req, 0); >> >>>> >> >>>>> + dwc2_hsotg_complete_request(hsotg, hs_ep, hs_req, 0); >> >>>> >> >>>>> + } else { >> >>>> >> >>>>> + dwc2_hsotg_complete_request(hsotg, hs_ep, hs_req, >-ETIMEDOUT); >> >>>> >> >>>>> + } >> >>>> >> >>>>> } >> >>>> >> >>>>> >> >>>> >> >>>>> /* >> >>>> >> >>>>> - * dwc2_gadget_start_next_isoc_ddma - start next isoc request, if any. >> >>>> >> >>>>> - * @hs_ep: The isochronous endpoint to be re-enabled. >> >>>> >> >>>>> + * dwc2_gadget_handle_isoc_bna - handle BNA interrupt for ISOC. >> >>>> >> >>>>> + * @hs_ep: The isochronous endpoint. >> >>>> >> >>>>> * >> >>>> >> >>>>> - * If ep has been disabled due to last descriptor servicing (IN endpoint) or >> >>>> >> >>>>> - * BNA (OUT endpoint) check the status of other half of descriptor chain >that >> >>>> >> >>>>> - * was under SW control till HW was busy and restart the endpoint if >needed. >> >>>> >> >>>>> + * Complete request with -EIO. >> >>>> >> >>>>> + * If EP ISOC OUT then need to flush RX FIFO to remove source of BNA >> >>>> >> >>>>> + * interrupt. Reset target frame and next_desc to allow to start >> >>>> >> >>>>> + * ISOC's on NAK interrupt for IN direction or on OUTTKNEPDIS >> >>>> >> >>>>> + * interrupt for OUT direction. >> >>>> >> >>>>> */ >> >>>> >> >>>>> -static void dwc2_gadget_start_next_isoc_ddma(struct dwc2_hsotg_ep >> >>>> >> >>>>> *hs_ep) >> >>>> >> >>>>> +static void dwc2_gadget_handle_isoc_bna(struct dwc2_hsotg_ep >*hs_ep) >> >>>> >> >>>>> { >> >>>> >> >>>>> struct dwc2_hsotg *hsotg = hs_ep->parent; >> >>>> >> >>>>> - u32 depctl; >> >>>> >> >>>>> - u32 dma_reg; >> >>>> >> >>>>> - u32 ctrl; >> >>>> >> >>>>> - u32 dma_addr = hs_ep->desc_list_dma; >> >>>> >> >>>>> - unsigned char index = hs_ep->index; >> >>>> >> >>>>> - >> >>>> >> >>>>> - dma_reg = hs_ep->dir_in ? DIEPDMA(index) : DOEPDMA(index); >> >>>> >> >>>>> - depctl = hs_ep->dir_in ? DIEPCTL(index) : DOEPCTL(index); >> >>>> >> >>>>> >> >>>> >> >>>>> - ctrl = dwc2_readl(hsotg->regs + depctl); >> >>>> >> >>>>> + if (!hs_ep->dir_in) >> >>>> >> >>>>> + dwc2_flush_rx_fifo(hsotg); >> >>>> >> >>>>> + dwc2_hsotg_complete_request(hsotg, hs_ep, get_ep_head(hs_ep), >> >>>> >> >>>>> + -EIO); >> >>>> >> >>>>> >> >>>> >> >>>>> - /* >> >>>> >> >>>>> - * EP was disabled if HW has processed last descriptor or BNA was >set. >> >>>> >> >>>>> - * So restart ep if SW has prepared new descriptor chain in ep_queue >> >>>> >> >>>>> - * routine while HW was busy. >> >>>> >> >>>>> - */ >> >>>> >> >>>>> - if (!(ctrl & DXEPCTL_EPENA)) { >> >>>> >> >>>>> - if (!hs_ep->next_desc) { >> >>>> >> >>>>> - dev_dbg(hsotg->dev, "%s: No more ISOC requests\n", >> >>>> >> >>>>> - __func__); >> >>>> >> >>>>> - return; >> >>>> >> >>>>> - } >> >>>> >> >>>>> - >> >>>> >> >>>>> - dma_addr += sizeof(struct dwc2_dma_desc) * >> >>>> >> >>>>> - (MAX_DMA_DESC_NUM_GENERIC / 2) * >> >>>> >> >>>>> - hs_ep->isoc_chain_num; >> >>>> >> >>>>> - dwc2_writel(dma_addr, hsotg->regs + dma_reg); >> >>>> >> >>>>> - >> >>>> >> >>>>> - ctrl |= DXEPCTL_EPENA | DXEPCTL_CNAK; >> >>>> >> >>>>> - dwc2_writel(ctrl, hsotg->regs + depctl); >> >>>> >> >>>>> - >> >>>> >> >>>>> - /* Switch ISOC descriptor chain number being processed by SW*/ >> >>>> >> >>>>> - hs_ep->isoc_chain_num = (hs_ep->isoc_chain_num ^ 1) & 0x1; >> >>>> >> >>>>> - hs_ep->next_desc = 0; >> >>>> >> >>>>> - >> >>>> >> >>>>> - dev_dbg(hsotg->dev, "%s: Restarted isochronous endpoint\n", >> >>>> >> >>>>> - __func__); >> >>>> >> >>>>> - } >> >>>> >> >>>>> + hs_ep->target_frame = TARGET_FRAME_INITIAL; >> >>>> >> >>>>> + hs_ep->next_desc = 0; >> >>>> >> >>>>> } >> >>>> >> >>>>> >> >>>> >> >>>>> /** >> >>>> >> >>>>> @@ -2816,18 +2797,25 @@ static void dwc2_gadget_handle_nak(struct >> >>>> >> >>>>> dwc2_hsotg_ep *hs_ep) { >> >>>> >> >>>>> struct dwc2_hsotg *hsotg = hs_ep->parent; >> >>>> >> >>>>> int dir_in = hs_ep->dir_in; >> >>>> >> >>>>> + u32 tmp; >> >>>> >> >>>>> >> >>>> >> >>>>> if (!dir_in || !hs_ep->isochronous) >> >>>> >> >>>>> return; >> >>>> >> >>>>> >> >>>> >> >>>>> if (hs_ep->target_frame == TARGET_FRAME_INITIAL) { >> >>>> >> >>>>> - hs_ep->target_frame = dwc2_hsotg_read_frameno(hsotg); >> >>>> >> >>>>> >> >>>> >> >>>>> + tmp = dwc2_hsotg_read_frameno(hsotg); >> >>>> >> >>>> >> >>>> >> >>>> I think there is no need to introduce tmp, and the original is all right. >> >>>> >> >>> No, tmp required. Reading current frame number should be done as soon as >> >>> possible. This is why it's stored in tmp and then used for below cases: >> >>> 1. DDMA mode. On dwc2_hsotg_complete_request() function driver >> >>> immediatly queued new request and as result target_frame incremented by >> >>> interval. >> >>> 2. BDMA mode. tmp required if interval > 1. >> >>>> >> >> >> >> So for both DDMA or BDMA mode, hs_ep->target_frame should be updated >the >> >> Current frame number in HW. >> >> The original code can cover both branch, right? >> >> >Original code doesn't increment here target frame number (performing in >fill_isoc) to start transfers from next interval and doesn't complete >request. > Maybe you have misunderstood me, I mean there is no need to add the tmp variable. Directly update the hs_ep->target_frame using "hs_ep->target_frame = dwc2_hsotg_read_frameno(hsotg); " instead of "hs_ep->target_frame = tmp" In both branches. >> >>>> >> >>>>> if (using_desc_dma(hsotg)) { >> >>>> >> >>>>> + dwc2_hsotg_complete_request(hsotg, hs_ep, >> >>>> >> >>>>> + get_ep_head(hs_ep), >> >>>> >> >>>>> + -ENODATA); >> >>>> >> >>>>> + hs_ep->target_frame = tmp; >> >>>> >> >>>>> + dwc2_gadget_incr_frame_num(hs_ep); >> >>>> >> >>>>> dwc2_gadget_start_isoc_ddma(hs_ep); >> >>>> >> >>>>> return; >> >>>> >> >>>>> } >> >>>> >> >>>>> >> >>>> >> >>>>> + hs_ep->target_frame = tmp; >> >>>> >> >>>>> if (hs_ep->interval > 1) { >> >>>> >> >>>>> u32 ctrl = dwc2_readl(hsotg->regs + >> >>>> >> >>>>> DIEPCTL(hs_ep->index)); >> >>>> >> >>>>> @@ -2843,7 +2831,8 @@ static void dwc2_gadget_handle_nak(struct >> >>>> >> >>>>> dwc2_hsotg_ep *hs_ep) >> >>>> >> >>>>> get_ep_head(hs_ep), 0); >> >>>> >> >>>>> } >> >>>> >> >>>>> >> >>>> >> >>>>> - dwc2_gadget_incr_frame_num(hs_ep); >> >>>> >> >>>>> + if (!using_desc_dma(hsotg)) >> >>>> >> >>>>> + dwc2_gadget_incr_frame_num(hs_ep); >> >>>> >> >>>>> } >> >>>> >> >>>>> >> >>>> >> >>>>> /** >> >>>> >> >>>>> @@ -2901,9 +2890,9 @@ static void dwc2_hsotg_epint(struct >dwc2_hsotg >> >>>> >> >>>>> *hsotg, unsigned int idx, >> >>>> >> >>>>> >> >>>> >> >>>>> /* In DDMA handle isochronous requests separately */ >> >>>> >> >>>>> if (using_desc_dma(hsotg) && hs_ep->isochronous) { >> >>>> >> >>>>> - dwc2_gadget_complete_isoc_request_ddma(hs_ep); >> >>>> >> >>>>> - /* Try to start next isoc request */ >> >>>> >> >>>>> - dwc2_gadget_start_next_isoc_ddma(hs_ep); >> >>>> >> >>>>> + /* XferCompl set along with BNA */ >> >>>> >> >>>>> + if (!(ints & DXEPINT_BNAINTR)) >> >>>> >> >>>>> + dwc2_gadget_complete_isoc_request_ddma(hs_ep); >> >>>> >> >>>>> } else if (dir_in) { >> >>>> >> >>>>> /* >> >>>> >> >>>>> * We get OutDone from the FIFO, so we only @@ -2978,15 >> >>>> >> >>>>> +2967,8 @@ static void dwc2_hsotg_epint(struct dwc2_hsotg *hsotg, >> >>> unsigned >> >>>> >> >>>>> int idx, >> >>>> >> >>>>> >> >>>> >> >>>>> if (ints & DXEPINT_BNAINTR) { >> >>>> >> >>>>> dev_dbg(hsotg->dev, "%s: BNA interrupt\n", __func__); >> >>>> >> >>>>> - >> >>>> >> >>>>> - /* >> >>>> >> >>>>> - * Try to start next isoc request, if any. >> >>>> >> >>>>> - * Sometimes the endpoint remains enabled after BNA interrupt >> >>>> >> >>>>> - * assertion, which is not expected, hence we can enter here >> >>>> >> >>>>> - * couple of times. >> >>>> >> >>>>> - */ >> >>>> >> >>>>> if (hs_ep->isochronous) >> >>>> >> >>>>> - dwc2_gadget_start_next_isoc_ddma(hs_ep); >> >>>> >> >>>>> + dwc2_gadget_handle_isoc_bna(hs_ep); >> >>>> >> >>>>> } >> >>>> >> >>>>> >> >>>> >> >>>>> if (dir_in && !hs_ep->isochronous) { >> >>>> >> >>>>> @@ -3791,6 +3773,7 @@ static int dwc2_hsotg_ep_enable(struct usb_ep >> >>> *ep, >> >>>> >> >>>>> unsigned int dir_in; >> >>>> >> >>>>> unsigned int i, val, size; >> >>>> >> >>>>> int ret = 0; >> >>>> >> >>>>> + unsigned char ep_type; >> >>>> >> >>>>> >> >>>> >> >>>>> dev_dbg(hsotg->dev, >> >>>> >> >>>>> "%s: ep %s: a 0x%02x, attr 0x%02x, mps 0x%04x, intr %d\n", >@@ >> >>>> >> >>>>> -3809,6 +3792,15 @@ static int dwc2_hsotg_ep_enable(struct usb_ep >*ep, >> >>>> >> >>>>> return -EINVAL; >> >>>> >> >>>>> } >> >>>> >> >>>>> >> >>>> >> >>>>> + ep_type = desc->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK; >> >>>> >> >>>>> + /* ISOC DDMA supported bInterval up to 12 */ >> >>>> >> >>>>> + if (using_desc_dma(hsotg) && ep_type == >USB_ENDPOINT_XFER_ISOC && >> >>>> >> >>>>> + dir_in && desc->bInterval > 12) { >> >>>> >> >>>>> + dev_err(hsotg->dev, >> >>>> >> >>>>> + "%s: ISOC IN: bInterval>12 not supported!\n", __func__); >> >>>> >> >>>>> + return -EINVAL; >> >>>> >> >>>>> + } >> >>>> >> >>>>> + >> >>>> >> >>>>> mps = usb_endpoint_maxp(desc); >> >>>> >> >>>>> mc = usb_endpoint_maxp_mult(desc); >> >>>> >> >>>>> >> >>>> >> >>>>> @@ -3852,14 +3844,13 @@ static int dwc2_hsotg_ep_enable(struct >usb_ep >> >>>> >> >>>>> *ep, >> >>>> >> >>>>> hs_ep->halted = 0; >> >>>> >> >>>>> hs_ep->interval = desc->bInterval; >> >>>> >> >>>>> >> >>>> >> >>>>> - switch (desc->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) { >> >>>> >> >>>>> + switch (ep_type) { >> >>>> >> >>>>> case USB_ENDPOINT_XFER_ISOC: >> >>>> >> >>>>> epctrl |= DXEPCTL_EPTYPE_ISO; >> >>>> >> >>>>> epctrl |= DXEPCTL_SETEVENFR; >> >>>> >> >>>>> hs_ep->isochronous = 1; >> >>>> >> >>>>> hs_ep->interval = 1 << (desc->bInterval - 1); >> >>>> >> >>>>> hs_ep->target_frame = TARGET_FRAME_INITIAL; >> >>>> >> >>>>> - hs_ep->isoc_chain_num = 0; >> >>>> >> >>>>> hs_ep->next_desc = 0; >> >>>> >> >>>>> if (dir_in) { >> >>>> >> >>>>> hs_ep->periodic = 1; >> >>>> >> >>>>> -- >> >>>> >> >>>>> 2.11.0 >> >>>> >> >>>>> >> >>>> >> >>>>> -- >> >>>> >> >>>>> To unsubscribe from this list: send the line "unsubscribe linux-usb" in the >body >> >>> of >> >>>> >> >>>>> a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at >> >>>> >> >>>>> >> >>> >https://urldefense.proofpoint.com/v2/url?u=http-3A__vger.kernel.org_majordo >> >>> >mo-2Dinfo.html&d=DwIGaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=6z9Al9FrHR_Zqb >> >>> >btSAsD16pvOL2S3XHxQnSzq8kusyI&m=aPXXRmF_CWLjH-UpIejOss2qsaYVAMO- >> >>> oeSoNd1iTmg&s=RtquFAV3zcz956KK1FiPPWA43kJl9InZKrc3jWQR59s&e= >> >>>> >> >>>> >> >>> >> >>> Thank you for review. >> >>> >> >>> Minas >> >>> -- >> >>> To unsubscribe from this list: send the line "unsubscribe linux-usb" in >> >>> the body of a message to majordomo@xxxxxxxxxxxxxxx >> >>> More majordomo info at >https://urldefense.proofpoint.com/v2/url?u=http-3A__vger.kernel.org_majordo >mo-2Dinfo.html&d=DwIGaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=6z9Al9FrHR_Zqb >btSAsD16pvOL2S3XHxQnSzq8kusyI&m=WfGU1k4UBRap5Y7jA2LB6Q9JV5b8FXNr >mgg3ZvnV7V4&s=ZtDDEAhbXXQqBblOSfwtEDMXZqRRC3_6wLgCJREr2EQ&e= >> >> >Thanks, >Minas ��.n��������+%������w��{.n�����{���)�� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
