-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Cypress: FPGA: gpio test looks like not valid #11835
Comments
Isn't it that you write 0 (fpga side), but because of pull up you expect 1 on the actual pin? This comment explains it : |
Yes. But PSoC6 also set initial state 0. |
gpio api supports value even for input, it writes to gpio as it claims "for future use if you switch to output". Although as soon as you change the direction, you write a new value so I dont understand the use case for that condition there). I cant find it in the history why it was there, will need someone else to look at this closer. |
From my point of view:
|
Test scenario:
Maybe pull mode does not work on the cypress board? |
@mprse Cypress support pull mode. This case is passed if replace gpio_init_inout() -> gpio_init_in_ex() Could you please help me understand why? |
This call to |
Looks like no. But why we have initial state for input pin it should be done for output pin. Right? |
That is what I dont understand. 😞 anyway, what happens on the pin if you set it to output (it should be floating, no value set?). Is this documented in the reference manual for that MCU ? What is expected behavior for input pin? In case the pin has different behavior than we expect (@mprse I havent found the behavior defined in the gpio_api header file for this inout function), you can always check in HAL (if pin is output, and something asks to write to the pin, ignore it and keep it floating). |
Looking in
Suggesting that the That test (and the init_inout) seems to be assuming that the Seems like achieving that would need a more advanced |
Maybe something like this? |
I think so, but it seems that needs to be extended so that if you're told to write while an input, you should be remembering the written value (in obj, I guess) to write next time you switch to output. |
(I guess that sort of thing is a valid use case, as you could leave the output set to 0 always, then flip between input and output to bitbang I2C, for example, where you only ever drive low) |
@yarbcy Can you review the proposal above #11835 (comment) ? |
@0xc0170 Looks good. But I m not expert in mbed_hal. |
@yarbcy Can you test this code? |
@mprse Yes. |
@mprse I tested and it failed communication with FPGA tester: "An MbedTester communication channel could not be created" |
The communication with the |
I replaced cy_gpio_api.c line 32 apply_config():
Replaced gpio_object.h line 59 gpio_write(): static inline void gpio_write(gpio_t *obj, int value) Added new after gpio_write(): static inline void gpio_set_pull(gpio_t *obj, int value) <--- new function for setting pull mode |
Internal Jira reference: https://jira.arm.com/browse/MBOTRIAGE-2322 |
@mprse My proposal for code change is: mbed-os/hal/mbed_gpio.c remove line 63-65 because initial value doesn't make sense when pin direction is in input direction. |
As I noted in my example above, there is a use-case for this. Set the output to low, and flip between input and output to manually bit-bang an I2C open-drain output. That would be efficient on a chip with "OUT" and "DIR" registers where OUT was ignored when DIR was set to input. I don't know whether any Mbed application in the word is currently doing this, but it does currently work, those lines 63-65 seem to be intended for that, and that test makes sure it works. |
Annoyingly, the commit that added lines 63-65 (be8bca4) only has the commit message "proposed change of gpio_api", so it's not 100% clear what they were intending. But it was deliberate. |
@kjbracey-arm I don't understand next steps. If you don't agree (line 63-65). Please propose your approach. |
My suggestion is roughly as @mprse suggested above, but with the addition as I stated:
|
@yarbcy @mprse will update the suggestion above with written value storing, it will be good to take this to PR to propose a change. It should fix the failures you have been seeing
👍 , the comment in the PR itself states something similar about the intention #198 (comment) . Sadly, this is not included in the commit itself |
@mprse I don't have clear understanding what needs to be done. Can you do these changes? |
I will create a fix proposition. |
The fix can be found here: PR #11867 |
@mprse I tested. All GPIO tests PASSED. But please fix in file gpio_object.h line 66: change '==' to '='.
|
Great news! 🎉
Fixed! Thanks for catching this. BTW. Probably the previous implementation faulty because the direction check was invalid:
|
Fix for Cypress GPIO driver (fix for issue #11835)
All GPIO tests are PASSED.
|
Description of defect
Cypress: FPGA: gpio test looks like not valid.
In file TESTS/mbed_hal_fpga_ci_test_shield/gpio/main.cpp line 165:
`
// Initialize GPIO pin as an input, pull-up mode.
PSoC6 set 0, tester(FPGA) set 0. But expect 1. Test FAILED. It looks like it doesn't make sense.
This test PASSED if set PSOC6 1, tester(FPGA) set 0.
Target(s) affected by this defect ?
Tested on CY8CKIT_062_WIFI_BT
Toolchain(s) (name and version) displaying this defect ?
Tested on GCC_ARM
What version of Mbed-os are you using (tag or sha) ?
Latest
What version(s) of tools are you using. List all that apply (E.g. mbed-cli)
N/A
How is this defect reproduced ?
Always
The text was updated successfully, but these errors were encountered: