From 430784b084ff35aee5ada77252034d413e416b84 Mon Sep 17 00:00:00 2001 From: Paul Thompson Date: Wed, 11 Apr 2018 14:26:23 -0700 Subject: [PATCH 1/5] Initial work was for unsigned-signed comparison fix. Current work fixes negative number issues Compile: stm32f7xx_hal_pcd.c ../targets/TARGET_STM/TARGET_STM32F7/device/stm32f7xx_hal_pcd.c: In function 'PCD_WriteEmptyTxFifo': ../targets/TARGET_STM/TARGET_STM32F7/device/stm32f7xx_hal_pcd.c:1310:11: warning: comparison between signed and unsigned integer expressions [-Wsign-compare] if (len > ep->maxpacket) ^ ../targets/TARGET_STM/TARGET_STM32F7/device/stm32f7xx_hal_pcd.c:1325:13: warning: comparison between signed and unsigned integer expressions [-Wsign-compare] if (len > ep->maxpacket) ^ --- .../TARGET_STM32F7/device/stm32f7xx_hal_pcd.c | 78 ++++++++++--------- 1 file changed, 42 insertions(+), 36 deletions(-) diff --git a/targets/TARGET_STM/TARGET_STM32F7/device/stm32f7xx_hal_pcd.c b/targets/TARGET_STM/TARGET_STM32F7/device/stm32f7xx_hal_pcd.c index 66599a979ce..bcb0015c45d 100644 --- a/targets/TARGET_STM/TARGET_STM32F7/device/stm32f7xx_hal_pcd.c +++ b/targets/TARGET_STM/TARGET_STM32F7/device/stm32f7xx_hal_pcd.c @@ -145,7 +145,7 @@ HAL_StatusTypeDef HAL_PCD_Init(PCD_HandleTypeDef *hpcd) // MBED: added if(hpcd->State == HAL_PCD_STATE_RESET) - { + { /* Allocate lock resource and initialize it */ hpcd->Lock = HAL_UNLOCKED; for (i = 0; i < hpcd->Init.dev_endpoints ; i++) @@ -1298,50 +1298,56 @@ PCD_StateTypeDef HAL_PCD_GetState(PCD_HandleTypeDef *hpcd) */ static HAL_StatusTypeDef PCD_WriteEmptyTxFifo(PCD_HandleTypeDef *hpcd, uint32_t epnum) { - USB_OTG_GlobalTypeDef *USBx = hpcd->Instance; - USB_OTG_EPTypeDef *ep; - int32_t len = 0; - uint32_t len32b; - uint32_t fifoemptymsk = 0; - - ep = &hpcd->IN_ep[epnum]; - len = ep->xfer_len - ep->xfer_count; - - if (len > ep->maxpacket) - { - len = ep->maxpacket; - } + USB_OTG_GlobalTypeDef *USBx = hpcd->Instance; + USB_OTG_EPTypeDef *ep; + uint32_t len; + uint32_t len32b; + uint32_t fifoemptymsk = 0U; + ep = &hpcd->IN_ep[epnum]; - len32b = (len + 3) / 4; + if (ep->xfer_len >= ep->xfer_count) + { + len = ep->xfer_len - ep->xfer_count; - while ( (USBx_INEP(epnum)->DTXFSTS & USB_OTG_DTXFSTS_INEPTFSAV) > len32b && - ep->xfer_count < ep->xfer_len && - ep->xfer_len != 0) - { - /* Write the FIFO */ - len = ep->xfer_len - ep->xfer_count; + if (len > ep->maxpacket) + { + len = ep->maxpacket; + } - if (len > ep->maxpacket) - { - len = ep->maxpacket; - } - len32b = (len + 3) / 4; + len32b = (len + 3U) / 4U; - USB_WritePacket(USBx, ep->xfer_buff, epnum, len, hpcd->Init.dma_enable); + while (((USBx_INEP(epnum)->DTXFSTS & USB_OTG_DTXFSTS_INEPTFSAV) > len32b) && + (ep->xfer_count < ep->xfer_len) && + (ep->xfer_len != 0U)) + { + /* Write the FIFO */ + if (ep->xfer_len >= ep->xfer_count) + { + len = ep->xfer_len - ep->xfer_count; - ep->xfer_buff += len; - ep->xfer_count += len; - } + if (len > ep->maxpacket) + { + len = ep->maxpacket; + } + len32b = (len + 3U) / 4U; - if(len <= 0) - { - fifoemptymsk = 0x1 << epnum; - atomic_clr_u32(&USBx_DEVICE->DIEPEMPMSK, fifoemptymsk); // MBED: changed + USB_WritePacket(USBx, ep->xfer_buff, epnum, len, hpcd->Init.dma_enable); - } + ep->xfer_buff += len; + ep->xfer_count += len; + } + } + } + else + { + fifoemptymsk = 0x1U << epnum; + /* MBED */ + atomic_clr_u32(&USBx_DEVICE->DIEPEMPMSK, fifoemptymsk); + /* MBED */ + } - return HAL_OK; + return HAL_OK; } /** From 8f4a5e209352c1a55f5ea13a56b3fc50d713b113 Mon Sep 17 00:00:00 2001 From: Paul Thompson Date: Thu, 12 Apr 2018 10:09:53 -0700 Subject: [PATCH 2/5] Revert to original fix concentrating on type correctness --- .../TARGET_STM32F7/device/stm32f7xx_hal_pcd.c | 79 +++++++++---------- 1 file changed, 38 insertions(+), 41 deletions(-) diff --git a/targets/TARGET_STM/TARGET_STM32F7/device/stm32f7xx_hal_pcd.c b/targets/TARGET_STM/TARGET_STM32F7/device/stm32f7xx_hal_pcd.c index bcb0015c45d..d9ca6ee67d7 100644 --- a/targets/TARGET_STM/TARGET_STM32F7/device/stm32f7xx_hal_pcd.c +++ b/targets/TARGET_STM/TARGET_STM32F7/device/stm32f7xx_hal_pcd.c @@ -1298,56 +1298,53 @@ PCD_StateTypeDef HAL_PCD_GetState(PCD_HandleTypeDef *hpcd) */ static HAL_StatusTypeDef PCD_WriteEmptyTxFifo(PCD_HandleTypeDef *hpcd, uint32_t epnum) { - USB_OTG_GlobalTypeDef *USBx = hpcd->Instance; - USB_OTG_EPTypeDef *ep; - uint32_t len; - uint32_t len32b; - uint32_t fifoemptymsk = 0U; - - ep = &hpcd->IN_ep[epnum]; - - if (ep->xfer_len >= ep->xfer_count) - { - len = ep->xfer_len - ep->xfer_count; + USB_OTG_GlobalTypeDef *USBx = hpcd->Instance; + USB_OTG_EPTypeDef *ep; + uint32_t len; + int32_t ilen; + uint32_t len32b; + uint32_t fifoemptymsk = 0; - if (len > ep->maxpacket) - { - len = ep->maxpacket; - } + ep = &hpcd->IN_ep[epnum]; + ilen = ep->xfer_len - ep->xfer_count; + len = ilen; - len32b = (len + 3U) / 4U; + if ((ilen > 0) && (len > ep->maxpacket)) + { + len = ep->maxpacket; + } - while (((USBx_INEP(epnum)->DTXFSTS & USB_OTG_DTXFSTS_INEPTFSAV) > len32b) && - (ep->xfer_count < ep->xfer_len) && - (ep->xfer_len != 0U)) - { - /* Write the FIFO */ - if (ep->xfer_len >= ep->xfer_count) - { - len = ep->xfer_len - ep->xfer_count; - if (len > ep->maxpacket) - { - len = ep->maxpacket; - } - len32b = (len + 3U) / 4U; + len32b = (len + 3) / 4; - USB_WritePacket(USBx, ep->xfer_buff, epnum, len, hpcd->Init.dma_enable); + while ( (USBx_INEP(epnum)->DTXFSTS & USB_OTG_DTXFSTS_INEPTFSAV) > len32b && + ep->xfer_count < ep->xfer_len && + ep->xfer_len != 0) + { + /* Write the FIFO */ + ilen = ep->xfer_len - ep->xfer_count; + len = ilen; - ep->xfer_buff += len; - ep->xfer_count += len; - } - } - } - else + if ((ilen > 0) && (len > ep->maxpacket)) { - fifoemptymsk = 0x1U << epnum; - /* MBED */ - atomic_clr_u32(&USBx_DEVICE->DIEPEMPMSK, fifoemptymsk); - /* MBED */ + len = ep->maxpacket; } + len32b = (len + 3) / 4; + + USB_WritePacket(USBx, ep->xfer_buff, epnum, len, hpcd->Init.dma_enable); + + ep->xfer_buff += len; + ep->xfer_count += len; + } - return HAL_OK; + if(ilen <= 0) + { + fifoemptymsk = 0x1 << epnum; + atomic_clr_u32(&USBx_DEVICE->DIEPEMPMSK, fifoemptymsk); // MBED: changed + + } + + return HAL_OK; } /** From 2211a27f53d762ead9e1386eec25d8127d89ca38 Mon Sep 17 00:00:00 2001 From: Paul Thompson Date: Fri, 13 Apr 2018 00:27:00 -0700 Subject: [PATCH 3/5] Drop usage of ilen, just use len and cast it to int32_t as appropriate --- .../TARGET_STM32F7/device/stm32f7xx_hal_pcd.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/targets/TARGET_STM/TARGET_STM32F7/device/stm32f7xx_hal_pcd.c b/targets/TARGET_STM/TARGET_STM32F7/device/stm32f7xx_hal_pcd.c index d9ca6ee67d7..cf942dc7a10 100644 --- a/targets/TARGET_STM/TARGET_STM32F7/device/stm32f7xx_hal_pcd.c +++ b/targets/TARGET_STM/TARGET_STM32F7/device/stm32f7xx_hal_pcd.c @@ -1301,15 +1301,13 @@ static HAL_StatusTypeDef PCD_WriteEmptyTxFifo(PCD_HandleTypeDef *hpcd, uint32_t USB_OTG_GlobalTypeDef *USBx = hpcd->Instance; USB_OTG_EPTypeDef *ep; uint32_t len; - int32_t ilen; uint32_t len32b; uint32_t fifoemptymsk = 0; ep = &hpcd->IN_ep[epnum]; - ilen = ep->xfer_len - ep->xfer_count; - len = ilen; + len = ep->xfer_len - ep->xfer_count; - if ((ilen > 0) && (len > ep->maxpacket)) + if (((int32_t)len > 0) && (len > ep->maxpacket)) { len = ep->maxpacket; } @@ -1322,10 +1320,9 @@ static HAL_StatusTypeDef PCD_WriteEmptyTxFifo(PCD_HandleTypeDef *hpcd, uint32_t ep->xfer_len != 0) { /* Write the FIFO */ - ilen = ep->xfer_len - ep->xfer_count; - len = ilen; + len = ep->xfer_len - ep->xfer_count; - if ((ilen > 0) && (len > ep->maxpacket)) + if (((int32_t)len > 0) && (len > ep->maxpacket)) { len = ep->maxpacket; } @@ -1337,7 +1334,7 @@ static HAL_StatusTypeDef PCD_WriteEmptyTxFifo(PCD_HandleTypeDef *hpcd, uint32_t ep->xfer_count += len; } - if(ilen <= 0) + if ((int32_t)len <= 0) { fifoemptymsk = 0x1 << epnum; atomic_clr_u32(&USBx_DEVICE->DIEPEMPMSK, fifoemptymsk); // MBED: changed From b45d4233e11f135229e8ee9b3e64fd0b8589b913 Mon Sep 17 00:00:00 2001 From: Paul Thompson Date: Fri, 13 Apr 2018 04:44:43 -0700 Subject: [PATCH 4/5] Make the atomic_clr_u32 conditional use raw values rather than computed, remove need for guards --- .../TARGET_STM/TARGET_STM32F7/device/stm32f7xx_hal_pcd.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/targets/TARGET_STM/TARGET_STM32F7/device/stm32f7xx_hal_pcd.c b/targets/TARGET_STM/TARGET_STM32F7/device/stm32f7xx_hal_pcd.c index cf942dc7a10..2785ae76bd7 100644 --- a/targets/TARGET_STM/TARGET_STM32F7/device/stm32f7xx_hal_pcd.c +++ b/targets/TARGET_STM/TARGET_STM32F7/device/stm32f7xx_hal_pcd.c @@ -1307,7 +1307,7 @@ static HAL_StatusTypeDef PCD_WriteEmptyTxFifo(PCD_HandleTypeDef *hpcd, uint32_t ep = &hpcd->IN_ep[epnum]; len = ep->xfer_len - ep->xfer_count; - if (((int32_t)len > 0) && (len > ep->maxpacket)) + if (len > ep->maxpacket) { len = ep->maxpacket; } @@ -1322,7 +1322,7 @@ static HAL_StatusTypeDef PCD_WriteEmptyTxFifo(PCD_HandleTypeDef *hpcd, uint32_t /* Write the FIFO */ len = ep->xfer_len - ep->xfer_count; - if (((int32_t)len > 0) && (len > ep->maxpacket)) + if (len > ep->maxpacket) { len = ep->maxpacket; } @@ -1334,7 +1334,7 @@ static HAL_StatusTypeDef PCD_WriteEmptyTxFifo(PCD_HandleTypeDef *hpcd, uint32_t ep->xfer_count += len; } - if ((int32_t)len <= 0) + if (ep->xfer_count >= ep->xfer_len) { fifoemptymsk = 0x1 << epnum; atomic_clr_u32(&USBx_DEVICE->DIEPEMPMSK, fifoemptymsk); // MBED: changed From 20f11bc13f66c42c4a9bd95e746cd8fc948e5ab4 Mon Sep 17 00:00:00 2001 From: Paul Thompson Date: Fri, 13 Apr 2018 05:27:03 -0700 Subject: [PATCH 5/5] Extend changes to other STM32 devices that have the PCD_WriteEmptyTxFifo() function --- targets/TARGET_STM/TARGET_STM32F1/device/stm32f1xx_hal_pcd.c | 4 ++-- targets/TARGET_STM/TARGET_STM32F2/device/stm32f2xx_hal_pcd.c | 4 ++-- targets/TARGET_STM/TARGET_STM32F4/device/stm32f4xx_hal_pcd.c | 4 ++-- targets/TARGET_STM/TARGET_STM32L4/device/stm32l4xx_hal_pcd.c | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/targets/TARGET_STM/TARGET_STM32F1/device/stm32f1xx_hal_pcd.c b/targets/TARGET_STM/TARGET_STM32F1/device/stm32f1xx_hal_pcd.c index 8e0ddbe30bd..11c636a8b9a 100644 --- a/targets/TARGET_STM/TARGET_STM32F1/device/stm32f1xx_hal_pcd.c +++ b/targets/TARGET_STM/TARGET_STM32F1/device/stm32f1xx_hal_pcd.c @@ -1152,7 +1152,7 @@ static HAL_StatusTypeDef PCD_WriteEmptyTxFifo(PCD_HandleTypeDef *hpcd, uint32_t { USB_OTG_GlobalTypeDef *USBx = hpcd->Instance; USB_OTG_EPTypeDef *ep = NULL; - int32_t len = 0; + uint32_t len; uint32_t len32b = 0U; uint32_t fifoemptymsk = 0U; @@ -1185,7 +1185,7 @@ static HAL_StatusTypeDef PCD_WriteEmptyTxFifo(PCD_HandleTypeDef *hpcd, uint32_t ep->xfer_count += len; } - if(len <= 0) + if (ep->xfer_count >= ep->xfer_len) { fifoemptymsk = 0x01U << epnum; USBx_DEVICE->DIEPEMPMSK &= ~fifoemptymsk; diff --git a/targets/TARGET_STM/TARGET_STM32F2/device/stm32f2xx_hal_pcd.c b/targets/TARGET_STM/TARGET_STM32F2/device/stm32f2xx_hal_pcd.c index 218c10a362a..ee1703bbd10 100644 --- a/targets/TARGET_STM/TARGET_STM32F2/device/stm32f2xx_hal_pcd.c +++ b/targets/TARGET_STM/TARGET_STM32F2/device/stm32f2xx_hal_pcd.c @@ -1213,7 +1213,7 @@ static HAL_StatusTypeDef PCD_WriteEmptyTxFifo(PCD_HandleTypeDef *hpcd, uint32_t { USB_OTG_GlobalTypeDef *USBx = hpcd->Instance; USB_OTG_EPTypeDef *ep; - int32_t len = 0U; + uint32_t len; uint32_t len32b; uint32_t fifoemptymsk = 0U; @@ -1247,7 +1247,7 @@ static HAL_StatusTypeDef PCD_WriteEmptyTxFifo(PCD_HandleTypeDef *hpcd, uint32_t ep->xfer_count += len; } - if(len <= 0U) + if (ep->xfer_count >= ep->xfer_len) { fifoemptymsk = 0x1U << epnum; atomic_clr_u32(&USBx_DEVICE->DIEPEMPMSK, fifoemptymsk); diff --git a/targets/TARGET_STM/TARGET_STM32F4/device/stm32f4xx_hal_pcd.c b/targets/TARGET_STM/TARGET_STM32F4/device/stm32f4xx_hal_pcd.c index 44e191553c5..a6ed3193fe8 100644 --- a/targets/TARGET_STM/TARGET_STM32F4/device/stm32f4xx_hal_pcd.c +++ b/targets/TARGET_STM/TARGET_STM32F4/device/stm32f4xx_hal_pcd.c @@ -1311,7 +1311,7 @@ static HAL_StatusTypeDef PCD_WriteEmptyTxFifo(PCD_HandleTypeDef *hpcd, uint32_t { USB_OTG_GlobalTypeDef *USBx = hpcd->Instance; USB_OTG_EPTypeDef *ep; - int32_t len = 0U; + uint32_t len; uint32_t len32b; uint32_t fifoemptymsk = 0U; @@ -1345,7 +1345,7 @@ static HAL_StatusTypeDef PCD_WriteEmptyTxFifo(PCD_HandleTypeDef *hpcd, uint32_t ep->xfer_count += len; } - if(len <= 0U) + if (ep->xfer_count >= ep->xfer_len) { fifoemptymsk = 0x1U << epnum; /* MBED */ diff --git a/targets/TARGET_STM/TARGET_STM32L4/device/stm32l4xx_hal_pcd.c b/targets/TARGET_STM/TARGET_STM32L4/device/stm32l4xx_hal_pcd.c index 453922e7409..5879b406988 100644 --- a/targets/TARGET_STM/TARGET_STM32L4/device/stm32l4xx_hal_pcd.c +++ b/targets/TARGET_STM/TARGET_STM32L4/device/stm32l4xx_hal_pcd.c @@ -1428,7 +1428,7 @@ static HAL_StatusTypeDef PCD_WriteEmptyTxFifo(PCD_HandleTypeDef *hpcd, uint32_t { USB_OTG_GlobalTypeDef *USBx = hpcd->Instance; USB_OTG_EPTypeDef *ep = NULL; - int32_t len = 0U; + uint32_t len; uint32_t len32b = 0; uint32_t fifoemptymsk = 0; @@ -1462,7 +1462,7 @@ static HAL_StatusTypeDef PCD_WriteEmptyTxFifo(PCD_HandleTypeDef *hpcd, uint32_t ep->xfer_count += len; } - if(len <= 0) + if (ep->xfer_count >= ep->xfer_len) { fifoemptymsk = 0x1 << epnum; // Added for MBED PR #3062