Xilinx / Xilinx/embeddedsw

xusbps_ch9 command verifier tests failing

Open
#277 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
HTML
Stars
1.2k
Forks
1.1k
PR merge metrics
No merged PRs in 30d

Description

In the "/drivers/usbps/examples
/xusbps_ch9.c" I found a few things that make my usb device fail the Chapter 9 tests, I would consider them bugs. Firstly, the endianess of the endpoint status seems to be wrong :
` case XUSBPS_STATUS_ENDPOINT: {
u32 Status;
int EpNum = SetupData->wIndex;

					Status = XUsbPs_ReadReg(InstancePtr->Config.BaseAddress,
								XUSBPS_EPCRn_OFFSET(EpNum & 0xF));

					if (EpNum & 0x80) { /* In EP */
						if (Status & XUSBPS_EPCR_TXS_MASK) {
							*((u16 *) &Reply[0]) = 0x0100;
						} else {
							*((u16 *) &Reply[0]) = 0x0000;
						}
					} else {	/* Out EP */
						if (Status & XUSBPS_EPCR_RXS_MASK) {
							*((u16 *) &Reply[0]) = 0x0100;
						} else {
							*((u16 *) &Reply[0]) = 0x0000;
						}
					}
					break;
				}`

What worked for me was just changing the two bytes around:
if(EpNum & 0x80) { // In EP if(Status & XUSBPS_EPCR_TXS_MASK) { *((u16 *) &Reply[0]) = 0x0001; // Endpoint is halted } else { *((u16 *) &Reply[0]) = 0x0000; // Endpoint is not halted } } else { // Out EP if(Status & XUSBPS_EPCR_RXS_MASK) { *((u16 *) &Reply[0]) = 0x0001; // Endpoint is halted } else { *((u16 *) &Reply[0]) = 0x0000; // Endpoint is not halted } }
Furthermore, for the usb device to be conform, it needs to allow for other states then the configured state, so apart from allowing value 1, value zero also needs to be processed correctly where the device is put into unconfigured state:
` case XUSBPS_REQ_SET_CONFIGURATION:

		/*
		 * Only allow configuration index 1 as this is the only one we
		 * have.
		 */
		if ((SetupData->wValue & 0xff) != 1) {
			Error = 1;
			break;
		}`

And lastly get_config is returning alt settings instead of current config:
` case XUSBPS_REQ_GET_CONFIGURATION:

		if (InstancePtr->AppData != NULL) {

			/* When we run CV test suite application in Windows, need to
			 * add GET_CONFIGURATION command to pass test suite
			 */
			*((u8 *) &Reply[0]) = XUsbPs_GetConfigDone((XUsbPs *)InstancePtr);
			Status = XUsbPs_EpBufferSend((XUsbPs *)InstancePtr, 0, Reply,
						     SetupData->wLength);
		} else {
			Response = (u8)InstancePtr->CurrentAltSetting;
			XUsbPs_EpBufferSend(InstancePtr, 0,
					    &Response, 1);
		}
		break;`

So it should be:
Response = (u8)InstancePtr->CurrentConfig;

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in drivers/usbps/examples/xusbps_ch9.c and review the XUSBPS_STATUS_ENDPOINT, XUSBPS_REQ_SET_CONFIGURATION, and XUSBPS_REQ_GET_CONFIGURATION cases. Verify the endpoint status response byte order, handling of configuration values 0 and 1, and use of CurrentConfig. Done means the USB Chapter 9 command verifier tests pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
c
Domain
embedded-iot, testing-qa
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.