tests: drivers: udc: Fixed behavior on devices that has 16 endpoints #85024
Conversation
soburi
force-pushed
the
udc_test_failed_ep16
branch
from
b4cf0e2 to
c3a08d4
Compare
In the current test,
```
#define FALSE_EP_ADDR 0x0FU
```
is to check non-existent EP, but if there are 16 EPs,
i.e., the USB standard is fully implemented,
this becomes a valid EP.
In that case, the test item that checks the operation of
a non-existent EP will fail.
Even if this value is set to a larger value, the EP value is rounded
by the following macro, so it cannot point to a non-existent EP.
```
#define USB_EP_LUT_IDX(ep) (USB_EP_DIR_IS_IN(ep) \
? (ep & BIT_MASK(4)) + 16 : ep & BIT_MASK(4))
```
Essentially, I think it would be more appropriate to implement
`udc_get_ep_cfg()` to return NULL when a non-existent EP is specified,
but a major design change is required.
Here, I will refer to the value in the device tree to find the number of
EPs, and if 16 or more EPs are available, we will change it so that the
test for non-existent EPs is not performed.
Signed-off-by: TOKITA Hiroshi <[email protected]>
soburi
force-pushed
the
udc_test_failed_ep16
branch
from
c3a08d4 to
cc5f85c
Compare
| @@ -19,6 +19,13 @@ LOG_MODULE_REGISTER(udc_test, LOG_LEVEL_INF); | |||
| */ | |||
|
|
|||
| #define FALSE_EP_ADDR 0x0FU | |||
| #if DT_NODE_HAS_COMPAT(DT_NODELABEL(zephyr_udc0), snps_dwc2) | |||
| #define FALSE_EP_EXISTS \ | |||
There was a problem hiding this comment.
I haven't been able to test synopsys' dwc2 because I don't have a device. I don't think there's any problem with the logic, but I'd like someone with knowledge to check it.
There was a problem hiding this comment.
Getting a non-existent endpoint on DWC2 is harder than just comparing the number of in and out endpoints.
The 32-bit long GHWCFG1 contains 2 bits per endpoint:
- 00 means the endpoint is bidirectional
- 01 means the endpoint is IN only
- 10 means the endpoint is OUT only
bits 1..0 are for endpoint 0, 3..2 -> endpoint 1, ..., 29..28 -> endpoint 14, 31..30 -> endpoint 15
Then it is possible that there are more IN endpoints from GHWCFG1 than there are TxFIFOs. If such thing is a case, then the num_in_eps is lower than what would be derived from GHWCFG1 (number of 00 or 01 entries).
Depending on the configuration it may be that the non-existent endpoint exists on DWC2 (GHWCFG1 different from 0x00000000) but it some other endpoint than 0xF.
This problem also occurs in the version 9816e87 that you committed, and I would like your cooperation in resolving it. My analysis is as above, but as an expert in this domain, could you please give me some advice? |
The bug report is already assigned to me, I will look into it during the stabilization period. |
Understood. I'll wait for the results first. |
In the current test,
is to check non-existent EP, but if there are 16 EPs, i.e., the USB standard is fully implemented,
this becomes a valid EP.
In that case, the test item that checks the operation of
a non-existent EP will fail.
Even if this value is set to a larger value, the EP value is rounded by the following macro, so it cannot point to a non-existent EP.
Essentially, I think it would be more appropriate to implement
udc_get_ep_cfg()to return NULL when a non-existent EP is specified, but a major design change is required.Here, I will refer to the value in the device tree to find the number of EPs, and if 16 or more EPs are available, we will change it so that the test for non-existent EPs is not performed.
Fix #85025