| Message ID | 262613802.2236747.1344549018378.JavaMail.root@advansee.com |
|---|---|
| State | Superseded |
| Delegated to: | Marek Vasut |
| Headers | show |
Dear Benoît Thébaudeau, > Signed-off-by: Benoît Thébaudeau <benoit.thebaudeau@advansee.com> > Cc: Marek Vasut <marex@denx.de> > Cc: Ilya Yanok <ilya.yanok@cogentembedded.com> > Cc: Stefan Herbrechtsmeier <stefan@herbrechtsmeier.net> > --- > Changes for v2: N/A. > Changes for v3: > - New patch. > > .../drivers/usb/host/ehci-hcd.c | 135 > +++++++++++--------- .../drivers/usb/host/ehci.h | > 75 ++++++++++- 2 files changed, 150 insertions(+), 60 deletions(-) > > diff --git u-boot-usb-8d5fb14.orig/drivers/usb/host/ehci-hcd.c > u-boot-usb-8d5fb14/drivers/usb/host/ehci-hcd.c index 5b3b906..1977c28 > 100644 > --- u-boot-usb-8d5fb14.orig/drivers/usb/host/ehci-hcd.c > +++ u-boot-usb-8d5fb14/drivers/usb/host/ehci-hcd.c > @@ -163,7 +163,7 @@ static int ehci_reset(void) > > #ifdef CONFIG_USB_EHCI_TXFIFO_THRESH > cmd = ehci_readl(&hcor->or_txfilltuning); > - cmd &= ~TXFIFO_THRESH(0x3f); > + cmd &= ~TXFIFO_THRESH_MASK; > cmd |= TXFIFO_THRESH(CONFIG_USB_EHCI_TXFIFO_THRESH); > ehci_writel(&hcor->or_txfilltuning, cmd); > #endif > @@ -186,7 +186,7 @@ static int ehci_td_buffer(struct qTD *td, void *buf, > size_t sz) while (idx < QT_BUFFER_CNT) { > td->qt_buffer[idx] = cpu_to_hc32(addr); > td->qt_buffer_hi[idx] = 0; > - next = (addr + 4096) & ~4095; > + next = (addr + EHCI_PAGE_SIZE) & ~(EHCI_PAGE_SIZE - 1); > delta = next - addr; > if (delta >= sz) > break; > @@ -208,7 +208,8 @@ ehci_submit_async(struct usb_device *dev, unsigned long > pipe, void *buffer, int length, struct devrequest *req) > { > ALLOC_ALIGN_BUFFER(struct QH, qh, 1, USB_DMA_MINALIGN); > - ALLOC_ALIGN_BUFFER(struct qTD, qtd, 3, USB_DMA_MINALIGN); > +#define QTD_COUNT 3 > + ALLOC_ALIGN_BUFFER(struct qTD, qtd, QTD_COUNT, USB_DMA_MINALIGN); > int qtd_counter = 0; > > volatile struct qTD *vtd; > @@ -230,7 +231,7 @@ ehci_submit_async(struct usb_device *dev, unsigned long > pipe, void *buffer, le16_to_cpu(req->index)); > > memset(qh, 0, sizeof(struct QH)); > - memset(qtd, 0, 3 * sizeof(*qtd)); > + memset(qtd, 0, QTD_COUNT * sizeof(*qtd)); > > toggle = usb_gettoggle(dev, usb_pipeendpoint(pipe), usb_pipeout(pipe)); > > @@ -246,19 +247,20 @@ ehci_submit_async(struct usb_device *dev, unsigned > long pipe, void *buffer, */ > qh->qh_link = cpu_to_hc32((uint32_t)qh_list | QH_LINK_TYPE_QH); > c = (usb_pipespeed(pipe) != USB_SPEED_HIGH && > - usb_pipeendpoint(pipe) == 0) ? 1 : 0; > - endpt = (8 << 28) | > - (c << 27) | > - (usb_maxpacket(dev, pipe) << 16) | > - (0 << 15) | > - (1 << 14) | > - (usb_pipespeed(pipe) << 12) | > - (usb_pipeendpoint(pipe) << 8) | > - (0 << 7) | (usb_pipedevice(pipe) << 0); > + usb_pipeendpoint(pipe) == 0); > + endpt = (8 << QH_ENDPT1_RL) | > + (c << QH_ENDPT1_C) | Maybe define it as #deifne QH_ENDPT1(x) ((x) << SEOMTHING) ? [...] > @@ -398,50 +408,53 @@ ehci_submit_async(struct usb_device *dev, unsigned > long pipe, void *buffer, ALIGN((uint32_t)buffer + length, > ARCH_DMA_MINALIGN)); > > /* Check that the TD processing happened */ > - if (token & 0x80) { > + if (token & (QT_TOKEN_STATUS_ACTIVE << QT_TOKEN_STATUS)) > printf("EHCI timed out on TD - token=%#x\n", token); > - } > > /* Disable async schedule. */ > cmd = ehci_readl(&hcor->or_usbcmd); > cmd &= ~CMD_ASE; > ehci_writel(&hcor->or_usbcmd, cmd); > > - ret = handshake((uint32_t *)&hcor->or_usbsts, STD_ASS, 0, > + ret = handshake((uint32_t *)&hcor->or_usbsts, STS_ASS, 0, Ooooh, nice catch :) [...] The rest is cool. btw when (I hope you will) resubmitting next time, just submit the whole series under 0/8 patch (or 1/8) of the old one to make it a nice thread.
Dear Marek Vasut, > Dear Benoît Thébaudeau, > > > Signed-off-by: Benoît Thébaudeau <benoit.thebaudeau@advansee.com> > > Cc: Marek Vasut <marex@denx.de> > > Cc: Ilya Yanok <ilya.yanok@cogentembedded.com> > > Cc: Stefan Herbrechtsmeier <stefan@herbrechtsmeier.net> > > --- > > Changes for v2: N/A. > > Changes for v3: > > - New patch. > > > > .../drivers/usb/host/ehci-hcd.c | 135 > > +++++++++++--------- .../drivers/usb/host/ehci.h > > | > > 75 ++++++++++- 2 files changed, 150 insertions(+), 60 deletions(-) > > > > diff --git u-boot-usb-8d5fb14.orig/drivers/usb/host/ehci-hcd.c > > u-boot-usb-8d5fb14/drivers/usb/host/ehci-hcd.c index > > 5b3b906..1977c28 > > 100644 > > --- u-boot-usb-8d5fb14.orig/drivers/usb/host/ehci-hcd.c > > +++ u-boot-usb-8d5fb14/drivers/usb/host/ehci-hcd.c > > @@ -163,7 +163,7 @@ static int ehci_reset(void) > > > > #ifdef CONFIG_USB_EHCI_TXFIFO_THRESH > > cmd = ehci_readl(&hcor->or_txfilltuning); > > - cmd &= ~TXFIFO_THRESH(0x3f); > > + cmd &= ~TXFIFO_THRESH_MASK; > > cmd |= TXFIFO_THRESH(CONFIG_USB_EHCI_TXFIFO_THRESH); > > ehci_writel(&hcor->or_txfilltuning, cmd); > > #endif > > @@ -186,7 +186,7 @@ static int ehci_td_buffer(struct qTD *td, void > > *buf, > > size_t sz) while (idx < QT_BUFFER_CNT) { > > td->qt_buffer[idx] = cpu_to_hc32(addr); > > td->qt_buffer_hi[idx] = 0; > > - next = (addr + 4096) & ~4095; > > + next = (addr + EHCI_PAGE_SIZE) & ~(EHCI_PAGE_SIZE - 1); > > delta = next - addr; > > if (delta >= sz) > > break; > > @@ -208,7 +208,8 @@ ehci_submit_async(struct usb_device *dev, > > unsigned long > > pipe, void *buffer, int length, struct devrequest *req) > > { > > ALLOC_ALIGN_BUFFER(struct QH, qh, 1, USB_DMA_MINALIGN); > > - ALLOC_ALIGN_BUFFER(struct qTD, qtd, 3, USB_DMA_MINALIGN); > > +#define QTD_COUNT 3 > > + ALLOC_ALIGN_BUFFER(struct qTD, qtd, QTD_COUNT, USB_DMA_MINALIGN); > > int qtd_counter = 0; > > > > volatile struct qTD *vtd; > > @@ -230,7 +231,7 @@ ehci_submit_async(struct usb_device *dev, > > unsigned long > > pipe, void *buffer, le16_to_cpu(req->index)); > > > > memset(qh, 0, sizeof(struct QH)); > > - memset(qtd, 0, 3 * sizeof(*qtd)); > > + memset(qtd, 0, QTD_COUNT * sizeof(*qtd)); > > > > toggle = usb_gettoggle(dev, usb_pipeendpoint(pipe), > > usb_pipeout(pipe)); > > > > @@ -246,19 +247,20 @@ ehci_submit_async(struct usb_device *dev, > > unsigned > > long pipe, void *buffer, */ > > qh->qh_link = cpu_to_hc32((uint32_t)qh_list | QH_LINK_TYPE_QH); > > c = (usb_pipespeed(pipe) != USB_SPEED_HIGH && > > - usb_pipeendpoint(pipe) == 0) ? 1 : 0; > > - endpt = (8 << 28) | > > - (c << 27) | > > - (usb_maxpacket(dev, pipe) << 16) | > > - (0 << 15) | > > - (1 << 14) | > > - (usb_pipespeed(pipe) << 12) | > > - (usb_pipeendpoint(pipe) << 8) | > > - (0 << 7) | (usb_pipedevice(pipe) << 0); > > + usb_pipeendpoint(pipe) == 0); > > + endpt = (8 << QH_ENDPT1_RL) | > > + (c << QH_ENDPT1_C) | > > Maybe define it as #deifne QH_ENDPT1(x) ((x) << SEOMTHING) ? > [...] For all of these? > > @@ -398,50 +408,53 @@ ehci_submit_async(struct usb_device *dev, > > unsigned > > long pipe, void *buffer, ALIGN((uint32_t)buffer + length, > > ARCH_DMA_MINALIGN)); > > > > /* Check that the TD processing happened */ > > - if (token & 0x80) { > > + if (token & (QT_TOKEN_STATUS_ACTIVE << QT_TOKEN_STATUS)) > > printf("EHCI timed out on TD - token=%#x\n", token); > > - } > > > > /* Disable async schedule. */ > > cmd = ehci_readl(&hcor->or_usbcmd); > > cmd &= ~CMD_ASE; > > ehci_writel(&hcor->or_usbcmd, cmd); > > > > - ret = handshake((uint32_t *)&hcor->or_usbsts, STD_ASS, 0, > > + ret = handshake((uint32_t *)&hcor->or_usbsts, STS_ASS, 0, > > Ooooh, nice catch :) > > [...] > The rest is cool. > > btw when (I hope you will) resubmitting next time, just submit the > whole series > under 0/8 patch (or 1/8) of the old one to make it a nice thread. OK, so with 2/8 removed since you have applied it. What is the rule here? I thought that a new version of a patch should be posted as a reply to the previous version so that patchwork could mark the old version as superseded (even if this feature is not yet available for U-Boot). Does it apply only to single patches while series should be replied to the previous 0/n? And should 3/n be a reply to 2/n, to 0/n (or to 1/n if no 0/n), or to nothing? I will wait for your review of 8/8. Best regards, Benoît
Dear Benoît Thébaudeau, [...] > > > @@ -246,19 +247,20 @@ ehci_submit_async(struct usb_device *dev, > > > unsigned > > > long pipe, void *buffer, */ > > > > > > qh->qh_link = cpu_to_hc32((uint32_t)qh_list | QH_LINK_TYPE_QH); > > > c = (usb_pipespeed(pipe) != USB_SPEED_HIGH && > > > > > > - usb_pipeendpoint(pipe) == 0) ? 1 : 0; > > > - endpt = (8 << 28) | > > > - (c << 27) | > > > - (usb_maxpacket(dev, pipe) << 16) | > > > - (0 << 15) | > > > - (1 << 14) | > > > - (usb_pipespeed(pipe) << 12) | > > > - (usb_pipeendpoint(pipe) << 8) | > > > - (0 << 7) | (usb_pipedevice(pipe) << 0); > > > + usb_pipeendpoint(pipe) == 0); > > > + endpt = (8 << QH_ENDPT1_RL) | > > > + (c << QH_ENDPT1_C) | > > > > Maybe define it as #deifne QH_ENDPT1(x) ((x) << SEOMTHING) ? > > [...] > > For all of these? Yes, do you not think it's better than define the offsets only? > > > @@ -398,50 +408,53 @@ ehci_submit_async(struct usb_device *dev, > > > unsigned > > > long pipe, void *buffer, ALIGN((uint32_t)buffer + length, > > > ARCH_DMA_MINALIGN)); > > > > > > /* Check that the TD processing happened */ > > > > > > - if (token & 0x80) { > > > + if (token & (QT_TOKEN_STATUS_ACTIVE << QT_TOKEN_STATUS)) > > > > > > printf("EHCI timed out on TD - token=%#x\n", token); > > > > > > - } > > > > > > /* Disable async schedule. */ > > > cmd = ehci_readl(&hcor->or_usbcmd); > > > cmd &= ~CMD_ASE; > > > ehci_writel(&hcor->or_usbcmd, cmd); > > > > > > - ret = handshake((uint32_t *)&hcor->or_usbsts, STD_ASS, 0, > > > + ret = handshake((uint32_t *)&hcor->or_usbsts, STS_ASS, 0, > > > > Ooooh, nice catch :) > > > > [...] > > The rest is cool. > > > > btw when (I hope you will) resubmitting next time, just submit the > > whole series > > under 0/8 patch (or 1/8) of the old one to make it a nice thread. > > OK, so with 2/8 removed since you have applied it. Yes > What is the rule here? I > thought that a new version of a patch should be posted as a reply to the > previous version so that patchwork could mark the old version as superseded Yes, unless the series changed so much (aka. too much reordering etc), that it's easier to repost it in reply to the first patch in the series. > (even if this feature is not yet available for U-Boot). Does it apply only > to single patches while series should be replied to the previous 0/n? I didn't figure out these gems in patchwork myself. I'm not a big fan of PW either. > And should 3/n be a reply to 2/n, to 0/n (or to 1/n if no 0/n), or to > nothing? Just use the standard threading git send-email does. > I will wait for your review of 8/8. I think it's fine to just resend it so I'll either pick first 7 or all 8. First 7 are certain once we agree on the above stuff :) > Best regards, > Benoît Best regards, Marek Vasut
Dear Benoît Thébaudeau, > Dear Marek Vasut, > > > > > > @@ -246,19 +247,20 @@ ehci_submit_async(struct usb_device *dev, > > > > > unsigned > > > > > long pipe, void *buffer, */ > > > > > > > > > > qh->qh_link = cpu_to_hc32((uint32_t)qh_list | > > > > > QH_LINK_TYPE_QH); > > > > > c = (usb_pipespeed(pipe) != USB_SPEED_HIGH && > > > > > > > > > > - usb_pipeendpoint(pipe) == 0) ? 1 : 0; > > > > > - endpt = (8 << 28) | > > > > > - (c << 27) | > > > > > - (usb_maxpacket(dev, pipe) << 16) | > > > > > - (0 << 15) | > > > > > - (1 << 14) | > > > > > - (usb_pipespeed(pipe) << 12) | > > > > > - (usb_pipeendpoint(pipe) << 8) | > > > > > - (0 << 7) | (usb_pipedevice(pipe) << 0); > > > > > + usb_pipeendpoint(pipe) == 0); > > > > > + endpt = (8 << QH_ENDPT1_RL) | > > > > > + (c << QH_ENDPT1_C) | > > > > > > > > Maybe define it as #deifne QH_ENDPT1(x) ((x) << SEOMTHING) ? > > > > [...] > > > > > > For all of these? > > > > Yes, do you not think it's better than define the offsets only? > > Yes, I hesitated with this solution when writing this code. > > Where the offset is needed, I can instead write additional extraction > macros. Oh, there're places where the offset is needed? What about adding both BIT(x) and BIT_OFFSET in such cases ? And BIT(x) (x << BIT_OFFSET) in that case. [...] Best regards, Marek Vasut
Dear Marek Vasut, > > > > @@ -246,19 +247,20 @@ ehci_submit_async(struct usb_device *dev, > > > > unsigned > > > > long pipe, void *buffer, */ > > > > > > > > qh->qh_link = cpu_to_hc32((uint32_t)qh_list | > > > > QH_LINK_TYPE_QH); > > > > c = (usb_pipespeed(pipe) != USB_SPEED_HIGH && > > > > > > > > - usb_pipeendpoint(pipe) == 0) ? 1 : 0; > > > > - endpt = (8 << 28) | > > > > - (c << 27) | > > > > - (usb_maxpacket(dev, pipe) << 16) | > > > > - (0 << 15) | > > > > - (1 << 14) | > > > > - (usb_pipespeed(pipe) << 12) | > > > > - (usb_pipeendpoint(pipe) << 8) | > > > > - (0 << 7) | (usb_pipedevice(pipe) << 0); > > > > + usb_pipeendpoint(pipe) == 0); > > > > + endpt = (8 << QH_ENDPT1_RL) | > > > > + (c << QH_ENDPT1_C) | > > > > > > Maybe define it as #deifne QH_ENDPT1(x) ((x) << SEOMTHING) ? > > > [...] > > > > For all of these? > > Yes, do you not think it's better than define the offsets only? Yes, I hesitated with this solution when writing this code. Where the offset is needed, I can instead write additional extraction macros. > > > > @@ -398,50 +408,53 @@ ehci_submit_async(struct usb_device *dev, > > > > unsigned > > > > long pipe, void *buffer, ALIGN((uint32_t)buffer + length, > > > > ARCH_DMA_MINALIGN)); > > > > > > > > /* Check that the TD processing happened */ > > > > > > > > - if (token & 0x80) { > > > > + if (token & (QT_TOKEN_STATUS_ACTIVE << QT_TOKEN_STATUS)) > > > > > > > > printf("EHCI timed out on TD - token=%#x\n", token); > > > > > > > > - } > > > > > > > > /* Disable async schedule. */ > > > > cmd = ehci_readl(&hcor->or_usbcmd); > > > > cmd &= ~CMD_ASE; > > > > ehci_writel(&hcor->or_usbcmd, cmd); > > > > > > > > - ret = handshake((uint32_t *)&hcor->or_usbsts, STD_ASS, 0, > > > > + ret = handshake((uint32_t *)&hcor->or_usbsts, STS_ASS, 0, > > > > > > Ooooh, nice catch :) > > > > > > [...] > > > The rest is cool. > > > > > > btw when (I hope you will) resubmitting next time, just submit > > > the > > > whole series > > > under 0/8 patch (or 1/8) of the old one to make it a nice thread. > > > > OK, so with 2/8 removed since you have applied it. > > Yes > > > What is the rule here? I > > thought that a new version of a patch should be posted as a reply > > to the > > previous version so that patchwork could mark the old version as > > superseded > > Yes, unless the series changed so much (aka. too much reordering > etc), that it's > easier to repost it in reply to the first patch in the series. > > > (even if this feature is not yet available for U-Boot). Does it > > apply only > > to single patches while series should be replied to the previous > > 0/n? > > I didn't figure out these gems in patchwork myself. I'm not a big fan > of PW > either. > > > And should 3/n be a reply to 2/n, to 0/n (or to 1/n if no 0/n), or > > to > > nothing? > > Just use the standard threading git send-email does. > > > I will wait for your review of 8/8. > > I think it's fine to just resend it so I'll either pick first 7 or > all 8. First > 7 are certain once we agree on the above stuff :) OK. Best regards, Benoît
Dear Marek Vasut, > > > > > > @@ -246,19 +247,20 @@ ehci_submit_async(struct usb_device > > > > > > *dev, > > > > > > unsigned > > > > > > long pipe, void *buffer, */ > > > > > > > > > > > > qh->qh_link = cpu_to_hc32((uint32_t)qh_list | > > > > > > QH_LINK_TYPE_QH); > > > > > > c = (usb_pipespeed(pipe) != USB_SPEED_HIGH && > > > > > > > > > > > > - usb_pipeendpoint(pipe) == 0) ? 1 : 0; > > > > > > - endpt = (8 << 28) | > > > > > > - (c << 27) | > > > > > > - (usb_maxpacket(dev, pipe) << 16) | > > > > > > - (0 << 15) | > > > > > > - (1 << 14) | > > > > > > - (usb_pipespeed(pipe) << 12) | > > > > > > - (usb_pipeendpoint(pipe) << 8) | > > > > > > - (0 << 7) | (usb_pipedevice(pipe) << 0); > > > > > > + usb_pipeendpoint(pipe) == 0); > > > > > > + endpt = (8 << QH_ENDPT1_RL) | > > > > > > + (c << QH_ENDPT1_C) | > > > > > > > > > > Maybe define it as #deifne QH_ENDPT1(x) ((x) << SEOMTHING) ? > > > > > [...] > > > > > > > > For all of these? > > > > > > Yes, do you not think it's better than define the offsets only? > > > > Yes, I hesitated with this solution when writing this code. > > > > Where the offset is needed, I can instead write additional > > extraction > > macros. > > Oh, there're places where the offset is needed? Yes, I think. > What about adding > both BIT(x) > and BIT_OFFSET in such cases ? And BIT(x) (x << BIT_OFFSET) in that > case. I would prefer: #define FIELD(fldval) (((fldval) & MASK) << OFFSET) #define GET_FIELD(regval) (((regval) >> OFFSET) & MASK) Are you OK with that? Best regards, Benoît
Dear Benoît Thébaudeau, > Dear Marek Vasut, > > > > > > > > @@ -246,19 +247,20 @@ ehci_submit_async(struct usb_device > > > > > > > *dev, > > > > > > > unsigned > > > > > > > long pipe, void *buffer, */ > > > > > > > > > > > > > > qh->qh_link = cpu_to_hc32((uint32_t)qh_list | > > > > > > > QH_LINK_TYPE_QH); > > > > > > > c = (usb_pipespeed(pipe) != USB_SPEED_HIGH && > > > > > > > > > > > > > > - usb_pipeendpoint(pipe) == 0) ? 1 : 0; > > > > > > > - endpt = (8 << 28) | > > > > > > > - (c << 27) | > > > > > > > - (usb_maxpacket(dev, pipe) << 16) | > > > > > > > - (0 << 15) | > > > > > > > - (1 << 14) | > > > > > > > - (usb_pipespeed(pipe) << 12) | > > > > > > > - (usb_pipeendpoint(pipe) << 8) | > > > > > > > - (0 << 7) | (usb_pipedevice(pipe) << 0); > > > > > > > + usb_pipeendpoint(pipe) == 0); > > > > > > > + endpt = (8 << QH_ENDPT1_RL) | > > > > > > > + (c << QH_ENDPT1_C) | > > > > > > > > > > > > Maybe define it as #deifne QH_ENDPT1(x) ((x) << SEOMTHING) ? > > > > > > [...] > > > > > > > > > > For all of these? > > > > > > > > Yes, do you not think it's better than define the offsets only? > > > > > > Yes, I hesitated with this solution when writing this code. > > > > > > Where the offset is needed, I can instead write additional > > > extraction > > > macros. > > > > Oh, there're places where the offset is needed? > > Yes, I think. > > > What about adding > > both BIT(x) > > and BIT_OFFSET in such cases ? And BIT(x) (x << BIT_OFFSET) in that > > case. > > I would prefer: > #define FIELD(fldval) (((fldval) & MASK) << OFFSET) > #define GET_FIELD(regval) (((regval) >> OFFSET) & MASK) Yes of course. > Are you OK with that? > > Best regards, > Benoît Best regards, Marek Vasut
Hi all, This series aims at improving EHCI performance. There is also some code cleanup BTW. Best regards, Benoît
Benoît Thébaudeau Application Engineer benoit.thebaudeau@advansee.com +33 2 40 50 21 73 ADVANSEE SARL 9, rue Alfred Kastler CS 30750 44307 Nantes CEDEX 3 France ----- Original Message ----- > From: "Benoît Thébaudeau" <benoit.thebaudeau@advansee.com> > To: u-boot@lists.denx.de > Cc: "Marek Vasut" <marex@denx.de>, "Ilya Yanok" <ilya.yanok@cogentembedded.com>, "Stefan Herbrechtsmeier" > <stefan@herbrechtsmeier.net> > Sent: Friday, August 10, 2012 6:21:37 PM > Subject: [PATCH v4 0/7] ehci: Improve performance > > Hi all, > > This series aims at improving EHCI performance. There is also some > code cleanup > BTW. > > Best regards, > Benoît >
Dear Benoît Thébaudeau, > Hi all, > > This series aims at improving EHCI performance. There is also some code > cleanup BTW. > > Best regards, > Benoît Applied all but 5/7 and pushed. Since we all have vacations etc etc, this will appear in the mainline around start of september. Thanks a lot for this awesome patchset! _Great_ work guys! Best regards, Marek Vasut
Dear Marek Vasut, > Dear Benoît Thébaudeau, > > > Hi all, > > > > This series aims at improving EHCI performance. There is also some > > code > > cleanup BTW. > > > > Best regards, > > Benoît > > Applied all but 5/7 and pushed. Since we all have vacations etc etc, > this will > appear in the mainline around start of september. Thanks. But will it make it for next release since September is after the official end of the current merge window? I also have series for ehci-mxc and ehci-mx5 that I will try to post next week. They fix many things, add new features and add support for i.MX25 and i.MX35. > Thanks a lot for this awesome patchset! _Great_ work guys! Thanks. Best regards, Benoît
Dear Benoît Thébaudeau, > Dear Marek Vasut, > > > Dear Benoît Thébaudeau, > > > > > Hi all, > > > > > > This series aims at improving EHCI performance. There is also some > > > code > > > cleanup BTW. > > > > > > Best regards, > > > Benoît > > > > Applied all but 5/7 and pushed. Since we all have vacations etc etc, > > this will > > appear in the mainline around start of september. > > Thanks. But will it make it for next release since September is after the > official end of the current merge window? If it's properly tested, I'll try to persuade WD to pull it in (and then the same round of horror as during the last release will repeat ;-) ) > I also have series for ehci-mxc and ehci-mx5 that I will try to post next > week. They fix many things, add new features and add support for i.MX25 > and i.MX35. Cool! > > Thanks a lot for this awesome patchset! _Great_ work guys! > > Thanks. > > Best regards, > Benoît Best regards, Marek Vasut
diff --git u-boot-usb-8d5fb14.orig/drivers/usb/host/ehci-hcd.c u-boot-usb-8d5fb14/drivers/usb/host/ehci-hcd.c index 5b3b906..1977c28 100644 --- u-boot-usb-8d5fb14.orig/drivers/usb/host/ehci-hcd.c +++ u-boot-usb-8d5fb14/drivers/usb/host/ehci-hcd.c @@ -163,7 +163,7 @@ static int ehci_reset(void) #ifdef CONFIG_USB_EHCI_TXFIFO_THRESH cmd = ehci_readl(&hcor->or_txfilltuning); - cmd &= ~TXFIFO_THRESH(0x3f); + cmd &= ~TXFIFO_THRESH_MASK; cmd |= TXFIFO_THRESH(CONFIG_USB_EHCI_TXFIFO_THRESH); ehci_writel(&hcor->or_txfilltuning, cmd); #endif @@ -186,7 +186,7 @@ static int ehci_td_buffer(struct qTD *td, void *buf, size_t sz) while (idx < QT_BUFFER_CNT) { td->qt_buffer[idx] = cpu_to_hc32(addr); td->qt_buffer_hi[idx] = 0; - next = (addr + 4096) & ~4095; + next = (addr + EHCI_PAGE_SIZE) & ~(EHCI_PAGE_SIZE - 1); delta = next - addr; if (delta >= sz) break; @@ -208,7 +208,8 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer, int length, struct devrequest *req) { ALLOC_ALIGN_BUFFER(struct QH, qh, 1, USB_DMA_MINALIGN); - ALLOC_ALIGN_BUFFER(struct qTD, qtd, 3, USB_DMA_MINALIGN); +#define QTD_COUNT 3 + ALLOC_ALIGN_BUFFER(struct qTD, qtd, QTD_COUNT, USB_DMA_MINALIGN); int qtd_counter = 0; volatile struct qTD *vtd; @@ -230,7 +231,7 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer, le16_to_cpu(req->index)); memset(qh, 0, sizeof(struct QH)); - memset(qtd, 0, 3 * sizeof(*qtd)); + memset(qtd, 0, QTD_COUNT * sizeof(*qtd)); toggle = usb_gettoggle(dev, usb_pipeendpoint(pipe), usb_pipeout(pipe)); @@ -246,19 +247,20 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer, */ qh->qh_link = cpu_to_hc32((uint32_t)qh_list | QH_LINK_TYPE_QH); c = (usb_pipespeed(pipe) != USB_SPEED_HIGH && - usb_pipeendpoint(pipe) == 0) ? 1 : 0; - endpt = (8 << 28) | - (c << 27) | - (usb_maxpacket(dev, pipe) << 16) | - (0 << 15) | - (1 << 14) | - (usb_pipespeed(pipe) << 12) | - (usb_pipeendpoint(pipe) << 8) | - (0 << 7) | (usb_pipedevice(pipe) << 0); + usb_pipeendpoint(pipe) == 0); + endpt = (8 << QH_ENDPT1_RL) | + (c << QH_ENDPT1_C) | + (usb_maxpacket(dev, pipe) << QH_ENDPT1_MAXPKTLEN) | + (0 << QH_ENDPT1_H) | + (QH_ENDPT1_DTC_DT_FROM_QTD << QH_ENDPT1_DTC) | + (usb_pipespeed(pipe) << QH_ENDPT1_EPS) | + (usb_pipeendpoint(pipe) << QH_ENDPT1_ENDPT) | + (0 << QH_ENDPT1_I) | (usb_pipedevice(pipe) << QH_ENDPT1_DEVADDR); qh->qh_endpt1 = cpu_to_hc32(endpt); - endpt = (1 << 30) | - (dev->portnr << 23) | - (dev->parent->devnum << 16) | (0 << 8) | (0 << 0); + endpt = (1 << QH_ENDPT2_MULT) | + (dev->portnr << QH_ENDPT2_PORTNUM) | + (dev->parent->devnum << QH_ENDPT2_HUBADDR) | + (0 << QH_ENDPT2_UFRAMECMASK) | (0 << QH_ENDPT2_UFRAMESMASK); qh->qh_endpt2 = cpu_to_hc32(endpt); qh->qh_overlay.qt_next = cpu_to_hc32(QT_NEXT_TERMINATE); @@ -276,9 +278,12 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer, */ qtd[qtd_counter].qt_next = cpu_to_hc32(QT_NEXT_TERMINATE); qtd[qtd_counter].qt_altnext = cpu_to_hc32(QT_NEXT_TERMINATE); - token = (0 << 31) | - (sizeof(*req) << 16) | - (0 << 15) | (0 << 12) | (3 << 10) | (2 << 8) | (0x80 << 0); + token = (0 << QT_TOKEN_DT) | + (sizeof(*req) << QT_TOKEN_TOTALBYTES) | + (0 << QT_TOKEN_IOC) | (0 << QT_TOKEN_CPAGE) | + (3 << QT_TOKEN_CERR) | + (QT_TOKEN_PID_SETUP << QT_TOKEN_PID) | + (QT_TOKEN_STATUS_ACTIVE << QT_TOKEN_STATUS); qtd[qtd_counter].qt_token = cpu_to_hc32(token); if (ehci_td_buffer(&qtd[qtd_counter], req, sizeof(*req)) != 0) { printf("unable construct SETUP td\n"); @@ -302,12 +307,14 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer, */ qtd[qtd_counter].qt_next = cpu_to_hc32(QT_NEXT_TERMINATE); qtd[qtd_counter].qt_altnext = cpu_to_hc32(QT_NEXT_TERMINATE); - token = (toggle << 31) | - (length << 16) | - ((req == NULL ? 1 : 0) << 15) | - (0 << 12) | - (3 << 10) | - ((usb_pipein(pipe) ? 1 : 0) << 8) | (0x80 << 0); + token = (toggle << QT_TOKEN_DT) | + (length << QT_TOKEN_TOTALBYTES) | + ((req == NULL) << QT_TOKEN_IOC) | + (0 << QT_TOKEN_CPAGE) | + (3 << QT_TOKEN_CERR) | + ((usb_pipein(pipe) ? QT_TOKEN_PID_IN : QT_TOKEN_PID_OUT) << + QT_TOKEN_PID) | + (QT_TOKEN_STATUS_ACTIVE << QT_TOKEN_STATUS); qtd[qtd_counter].qt_token = cpu_to_hc32(token); if (ehci_td_buffer(&qtd[qtd_counter], buffer, length) != 0) { printf("unable construct DATA td\n"); @@ -328,12 +335,14 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer, */ qtd[qtd_counter].qt_next = cpu_to_hc32(QT_NEXT_TERMINATE); qtd[qtd_counter].qt_altnext = cpu_to_hc32(QT_NEXT_TERMINATE); - token = (toggle << 31) | - (0 << 16) | - (1 << 15) | - (0 << 12) | - (3 << 10) | - ((usb_pipein(pipe) ? 0 : 1) << 8) | (0x80 << 0); + token = (toggle << QT_TOKEN_DT) | + (0 << QT_TOKEN_TOTALBYTES) | + (1 << QT_TOKEN_IOC) | + (0 << QT_TOKEN_CPAGE) | + (3 << QT_TOKEN_CERR) | + ((usb_pipein(pipe) ? QT_TOKEN_PID_OUT : QT_TOKEN_PID_IN) << + QT_TOKEN_PID) | + (QT_TOKEN_STATUS_ACTIVE << QT_TOKEN_STATUS); qtd[qtd_counter].qt_token = cpu_to_hc32(token); /* Update previous qTD! */ *tdp = cpu_to_hc32((uint32_t)&qtd[qtd_counter]); @@ -346,7 +355,8 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer, flush_dcache_range((uint32_t)qh_list, ALIGN_END_ADDR(struct QH, qh_list, 1)); flush_dcache_range((uint32_t)qh, ALIGN_END_ADDR(struct QH, qh, 1)); - flush_dcache_range((uint32_t)qtd, ALIGN_END_ADDR(struct qTD, qtd, 3)); + flush_dcache_range((uint32_t)qtd, + ALIGN_END_ADDR(struct qTD, qtd, QTD_COUNT)); /* Set async. queue head pointer. */ ehci_writel(&hcor->or_asynclistaddr, (uint32_t)qh_list); @@ -359,10 +369,10 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer, cmd |= CMD_ASE; ehci_writel(&hcor->or_usbcmd, cmd); - ret = handshake((uint32_t *)&hcor->or_usbsts, STD_ASS, STD_ASS, + ret = handshake((uint32_t *)&hcor->or_usbsts, STS_ASS, STS_ASS, 100 * 1000); if (ret < 0) { - printf("EHCI fail timeout STD_ASS set\n"); + printf("EHCI fail timeout STS_ASS set\n"); goto fail; } @@ -377,10 +387,10 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer, invalidate_dcache_range((uint32_t)qh, ALIGN_END_ADDR(struct QH, qh, 1)); invalidate_dcache_range((uint32_t)qtd, - ALIGN_END_ADDR(struct qTD, qtd, 3)); + ALIGN_END_ADDR(struct qTD, qtd, QTD_COUNT)); token = hc32_to_cpu(vtd->qt_token); - if (!(token & 0x80)) + if (!(token & (QT_TOKEN_STATUS_ACTIVE << QT_TOKEN_STATUS))) break; WATCHDOG_RESET(); } while (get_timer(ts) < timeout); @@ -398,50 +408,53 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer, ALIGN((uint32_t)buffer + length, ARCH_DMA_MINALIGN)); /* Check that the TD processing happened */ - if (token & 0x80) { + if (token & (QT_TOKEN_STATUS_ACTIVE << QT_TOKEN_STATUS)) printf("EHCI timed out on TD - token=%#x\n", token); - } /* Disable async schedule. */ cmd = ehci_readl(&hcor->or_usbcmd); cmd &= ~CMD_ASE; ehci_writel(&hcor->or_usbcmd, cmd); - ret = handshake((uint32_t *)&hcor->or_usbsts, STD_ASS, 0, + ret = handshake((uint32_t *)&hcor->or_usbsts, STS_ASS, 0, 100 * 1000); if (ret < 0) { - printf("EHCI fail timeout STD_ASS reset\n"); + printf("EHCI fail timeout STS_ASS reset\n"); goto fail; } token = hc32_to_cpu(qh->qh_overlay.qt_token); - if (!(token & 0x80)) { + if (!(token & (QT_TOKEN_STATUS_ACTIVE << QT_TOKEN_STATUS))) { debug("TOKEN=%#x\n", token); - switch (token & 0xfc) { + switch ((token >> QT_TOKEN_STATUS) & (QT_TOKEN_STATUS_ACTIVE | + QT_TOKEN_STATUS_HALTED | QT_TOKEN_STATUS_DATBUFERR | + QT_TOKEN_STATUS_BABBLEDET | QT_TOKEN_STATUS_XACTERR | + QT_TOKEN_STATUS_MISSEDUFRAME)) { case 0: - toggle = token >> 31; + toggle = token >> QT_TOKEN_DT; usb_settoggle(dev, usb_pipeendpoint(pipe), usb_pipeout(pipe), toggle); dev->status = 0; break; - case 0x40: + case QT_TOKEN_STATUS_HALTED: dev->status = USB_ST_STALLED; break; - case 0xa0: - case 0x20: + case QT_TOKEN_STATUS_ACTIVE | QT_TOKEN_STATUS_DATBUFERR: + case QT_TOKEN_STATUS_DATBUFERR: dev->status = USB_ST_BUF_ERR; break; - case 0x50: - case 0x10: + case QT_TOKEN_STATUS_HALTED | QT_TOKEN_STATUS_BABBLEDET: + case QT_TOKEN_STATUS_BABBLEDET: dev->status = USB_ST_BABBLE_DET; break; default: dev->status = USB_ST_CRC_ERR; - if ((token & 0x40) == 0x40) + if (token & (QT_TOKEN_STATUS_HALTED << QT_TOKEN_STATUS)) dev->status |= USB_ST_STALLED; break; } - dev->act_len = length - ((token >> 16) & 0x7fff); + dev->act_len = length - ((token & QT_TOKEN_TOTALBYTES_MASK) >> + QT_TOKEN_TOTALBYTES); } else { dev->act_len = 0; debug("dev=%u, usbsts=%#x, p[1]=%#x, p[2]=%#x\n", @@ -499,12 +512,14 @@ ehci_submit_root(struct usb_device *dev, unsigned long pipe, void *buffer, case USB_DT_DEVICE: debug("USB_DT_DEVICE request\n"); srcptr = &descriptor.device; - srclen = 0x12; + srclen = descriptor.device.bLength; break; case USB_DT_CONFIG: debug("USB_DT_CONFIG config\n"); srcptr = &descriptor.config; - srclen = 0x19; + srclen = descriptor.config.bLength + + descriptor.interface.bLength + + descriptor.endpoint.bLength; break; case USB_DT_STRING: debug("USB_DT_STRING config\n"); @@ -539,7 +554,7 @@ ehci_submit_root(struct usb_device *dev, unsigned long pipe, void *buffer, case USB_DT_HUB: debug("USB_DT_HUB config\n"); srcptr = &descriptor.hub; - srclen = 0x8; + srclen = descriptor.hub.bLength; break; default: debug("unknown value %x\n", le16_to_cpu(req->value)); @@ -577,13 +592,13 @@ ehci_submit_root(struct usb_device *dev, unsigned long pipe, void *buffer, tmpbuf[1] |= USB_PORT_STAT_POWER >> 8; if (ehci_is_TDI()) { - switch ((reg >> 26) & 3) { - case 0: + switch ((reg & PORTSC_PSPD_MASK) >> PORTSC_PSPD_OFFS) { + case PORTSC_PSPD_FS: break; - case 1: + case PORTSC_PSPD_LS: tmpbuf[1] |= USB_PORT_STAT_LOW_SPEED >> 8; break; - case 2: + case PORTSC_PSPD_HS: default: tmpbuf[1] |= USB_PORT_STAT_HIGH_SPEED >> 8; break; @@ -744,11 +759,13 @@ int usb_lowlevel_init(void) /* Set head of reclaim list */ memset(qh_list, 0, sizeof(*qh_list)); qh_list->qh_link = cpu_to_hc32((uint32_t)qh_list | QH_LINK_TYPE_QH); - qh_list->qh_endpt1 = cpu_to_hc32((1 << 15) | (USB_SPEED_HIGH << 12)); + qh_list->qh_endpt1 = cpu_to_hc32((1 << QH_ENDPT1_H) | + (USB_SPEED_HIGH << QH_ENDPT1_EPS)); qh_list->qh_curtd = cpu_to_hc32(QT_NEXT_TERMINATE); qh_list->qh_overlay.qt_next = cpu_to_hc32(QT_NEXT_TERMINATE); qh_list->qh_overlay.qt_altnext = cpu_to_hc32(QT_NEXT_TERMINATE); - qh_list->qh_overlay.qt_token = cpu_to_hc32(0x40); + qh_list->qh_overlay.qt_token = cpu_to_hc32(QT_TOKEN_STATUS_HALTED << + QT_TOKEN_STATUS); reg = ehci_readl(&hccr->cr_hcsparams); descriptor.hub.bNbrPorts = HCS_N_PORTS(reg); diff --git u-boot-usb-8d5fb14.orig/drivers/usb/host/ehci.h u-boot-usb-8d5fb14/drivers/usb/host/ehci.h index 7992983..c6f1794 100644 --- u-boot-usb-8d5fb14.orig/drivers/usb/host/ehci.h +++ u-boot-usb-8d5fb14/drivers/usb/host/ehci.h @@ -68,7 +68,7 @@ struct ehci_hcor { #define CMD_RESET (1 << 1) /* reset HC not bus */ #define CMD_RUN (1 << 0) /* start/stop HC */ uint32_t or_usbsts; -#define STD_ASS (1 << 15) +#define STS_ASS (1 << 15) #define STS_HALT (1 << 12) uint32_t or_usbintr; #define INTR_UE (1 << 0) /* USB interrupt enable */ @@ -83,11 +83,44 @@ struct ehci_hcor { uint32_t _reserved_0_; uint32_t or_burstsize; uint32_t or_txfilltuning; +#define TXFIFO_THRESH_MASK (0x3f << 16) #define TXFIFO_THRESH(p) ((p & 0x3f) << 16) uint32_t _reserved_1_[6]; uint32_t or_configflag; #define FLAG_CF (1 << 0) /* true: we'll support "high speed" */ uint32_t or_portsc[CONFIG_SYS_USB_EHCI_MAX_ROOT_PORTS]; +#define PORTSC_PTS_OFFS 30 +#define PORTSC_PTS_MASK (0x3 << PORTSC_PTS_OFFS) +#define PORTSC_STS (1 << 29) +#define PORTSC_PTW (1 << 28) +#define PORTSC_PSPD_OFFS 26 +#define PORTSC_PSPD_MASK (0x3 << PORTSC_PSPD_OFFS) +#define PORTSC_PSPD_FS 0x0 +#define PORTSC_PSPD_LS 0x1 +#define PORTSC_PSPD_HS 0x2 +#define PORTSC_PFSC (1 << 24) +#define PORTSC_PHCD (1 << 23) +#define PORTSC_WKOC (1 << 22) +#define PORTSC_WKDS (1 << 21) +#define PORTSC_WKCN (1 << 20) +#define PORTSC_PTC_OFFS 16 +#define PORTSC_PTC_MASK (0xf << PORTSC_PTC_OFFS) +#define PORTSC_PIC_OFFS 14 +#define PORTSC_PIC_MASK (0x3 << PORTSC_PIC_OFFS) +#define PORTSC_PO (1 << 13) +#define PORTSC_PP (1 << 12) +#define PORTSC_LS_OFFS 10 +#define PORTSC_LS_MASK (0x3 << PORTSC_LS_OFFS) +#define PORTSC_HSP (1 << 9) +#define PORTSC_PR (1 << 8) +#define PORTSC_SUSP (1 << 7) +#define PORTSC_FPR (1 << 6) +#define PORTSC_OCC (1 << 5) +#define PORTSC_OCA (1 << 4) +#define PORTSC_PEC (1 << 3) +#define PORTSC_PE (1 << 2) +#define PORTSC_OSC (1 << 1) +#define PORTSC_CCS (1 << 0) uint32_t or_systune; } __attribute__ ((packed, aligned(4))); @@ -175,6 +208,25 @@ struct qTD { #define QT_NEXT_TERMINATE 1 uint32_t qt_altnext; /* see EHCI 3.5.2 */ uint32_t qt_token; /* see EHCI 3.5.3 */ +#define QT_TOKEN_DT 31 /* Data Toggle */ +#define QT_TOKEN_TOTALBYTES 16 /* Total Bytes to Transfer */ +#define QT_TOKEN_TOTALBYTES_MASK (0x7fff << QT_TOKEN_TOTALBYTES) +#define QT_TOKEN_IOC 15 /* Interrupt On Complete */ +#define QT_TOKEN_CPAGE 12 /* Current Page */ +#define QT_TOKEN_CERR 10 /* Error Counter */ +#define QT_TOKEN_PID 8 /* PID Code */ +#define QT_TOKEN_PID_OUT 0x0 +#define QT_TOKEN_PID_IN 0x1 +#define QT_TOKEN_PID_SETUP 0x2 +#define QT_TOKEN_STATUS 0 /* Status */ +#define QT_TOKEN_STATUS_ACTIVE 0x80 +#define QT_TOKEN_STATUS_HALTED 0x40 +#define QT_TOKEN_STATUS_DATBUFERR 0x20 +#define QT_TOKEN_STATUS_BABBLEDET 0x10 +#define QT_TOKEN_STATUS_XACTERR 0x08 +#define QT_TOKEN_STATUS_MISSEDUFRAME 0x04 +#define QT_TOKEN_STATUS_SPLITXSTATE 0x02 +#define QT_TOKEN_STATUS_PERR 0x01 #define QT_BUFFER_CNT 5 uint32_t qt_buffer[QT_BUFFER_CNT]; /* see EHCI 3.5.4 */ uint32_t qt_buffer_hi[QT_BUFFER_CNT]; /* Appendix B */ @@ -182,6 +234,8 @@ struct qTD { uint32_t unused[3]; }; +#define EHCI_PAGE_SIZE 4096 + /* Queue Head (QH). */ struct QH { uint32_t qh_link; @@ -191,7 +245,26 @@ struct QH { #define QH_LINK_TYPE_SITD 4 #define QH_LINK_TYPE_FSTN 6 uint32_t qh_endpt1; +#define QH_ENDPT1_RL 28 /* NAK Count Reload */ +#define QH_ENDPT1_C 27 /* Control Endpoint Flag */ +#define QH_ENDPT1_MAXPKTLEN 16 /* Maximum Packet Length */ +#define QH_ENDPT1_H 15 /* Head of Reclamation List Flag */ +#define QH_ENDPT1_DTC 14 /* Data Toggle Control */ +#define QH_ENDPT1_DTC_IGNORE_QTD_TD 0x0 +#define QH_ENDPT1_DTC_DT_FROM_QTD 0x1 +#define QH_ENDPT1_EPS 12 /* Endpoint Speed */ +#define QH_ENDPT1_EPS_FS 0x0 +#define QH_ENDPT1_EPS_LS 0x1 +#define QH_ENDPT1_EPS_HS 0x2 +#define QH_ENDPT1_ENDPT 8 /* Endpoint Number */ +#define QH_ENDPT1_I 7 /* Inactivate on Next Transaction */ +#define QH_ENDPT1_DEVADDR 0 /* Device Address */ uint32_t qh_endpt2; +#define QH_ENDPT2_MULT 30 /* High-Bandwidth Pipe Multiplier */ +#define QH_ENDPT2_PORTNUM 23 /* Port Number */ +#define QH_ENDPT2_HUBADDR 16 /* Hub Address */ +#define QH_ENDPT2_UFRAMECMASK 8 /* Split Completion Mask */ +#define QH_ENDPT2_UFRAMESMASK 0 /* Interrupt Schedule Mask */ uint32_t qh_curtd; struct qTD qh_overlay; /*
Signed-off-by: Benoît Thébaudeau <benoit.thebaudeau@advansee.com> Cc: Marek Vasut <marex@denx.de> Cc: Ilya Yanok <ilya.yanok@cogentembedded.com> Cc: Stefan Herbrechtsmeier <stefan@herbrechtsmeier.net> --- Changes for v2: N/A. Changes for v3: - New patch. .../drivers/usb/host/ehci-hcd.c | 135 +++++++++++--------- .../drivers/usb/host/ehci.h | 75 ++++++++++- 2 files changed, 150 insertions(+), 60 deletions(-)