-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
libsdio: use libsdio in WHD HAL #78
base: master
Are you sure you want to change the base?
Conversation
JIRA: RTOS-212
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some changes need to be done and we need a further discussion on some of the libsdio API changes.
unsigned int dir : 1; /* 31 */ | ||
} cmd53; | ||
uint32_t uint; | ||
} sdio_cmd_arg_t; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This type is already defined by whd, so we don't have to redefine it. It's named 'sdio_cmd_argument_t' and is placed whd_bus_sdio_protocol.h.
|
||
|
||
/* NOTE: obj is ignored - state is kept in sdio_common */ | ||
/* NOTE: obj is ignored, state instance is held by the SDIO API*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since obj
is used, you could add a line (void)obj;
so that the compiler does not complain about an unused variable. Also a nitpick:
/* NOTE: obj is ignored, state instance is held by the SDIO API*/ | |
/* NOTE: obj is ignored, state instance is held by the SDIO API */ |
cyhal_gpio_init(0, 0, 0, 0); /* all args are ignored */ | ||
usleep(10000); /* 10 ms for regulator discharge */ | ||
cyhal_gpio_write(0, 1); /* only second arg is used */ | ||
usleep(250000); /* 250 ms for WLAN power up */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A newline before the return statement would be cool ;)
} | ||
|
||
|
||
/* NOTE: obj is ignored - state is kept in sdio_common */ | ||
/* NOTE: obj is ignored, state instance is held by the SDIO API*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick:
/* NOTE: obj is ignored, state instance is held by the SDIO API*/ | |
/* NOTE: obj is ignored, state instance is held by the SDIO API */ |
} | ||
|
||
|
||
/* NOTE: obj is ignored - state is kept in sdio_common */ | ||
/* NOTE: obj is ignored, state instance is held by the SDIO API*/ | ||
cy_rslt_t cyhal_sdio_configure(cyhal_sdio_t *obj, const cyhal_sdio_cfg_t *config) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, basically if you don't use this obj
variable you should add a (void)obj
statement everywhere.
*response = val; | ||
|
||
return CY_RSLT_SUCCESS; | ||
/* block size is 0 for some reason, hardcode instead */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand this comment. Does whd always set blocksz to 0? If yes, then how do you know it should be 64?
/* WHD only uses send cmd api call for | ||
* transfers after init commands which are | ||
* issued by sdio_init function, hence | ||
* just indicate success and exit */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm afraid we can't do it this way - assuming a specifiic behaviour of the upper layer, in this case the whd driver. It might change in the future, we need to keep it generic still. Unfortunately I see no other option here than exporting sdio_sendCmd
, which is now static, and maybe check if the cmd is CYHAL_SDIO_CMD_IO_RW_DIRECT and use transferDirect function in this case and sendCmd otherwise.
} | ||
|
||
size_t len; | ||
sdio_cmd_arg_t arg; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could be done in an even smarter way:
sdio_cmd_arg_t arg; | |
whd_bus_sdio_cmd53_argument_t arg = (whd_bus_sdio_cmd53_argument_t)argument; | |
val = *(sdio_common.base + int_status); | ||
if (val & (1 << 8)) | ||
break; | ||
cyhal_sdio_irq_handler_t conv; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At least it should be static. But I think the most portable solution would be to add a structure for keeping the sdio context with only this value here and make use of the (cyhal_sdio_t *obj) pointer, which gets passed to basically every function.
JIRA: RTOS-212
Description
Motivation and Context
Types of changes
How Has This Been Tested?
Checklist:
Special treatment