Skip to content
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

add checks for optional properties of FPGA iCE40 #80909

Conversation

benediktibk
Copy link
Collaborator

Add checks in the GPIO bitbang mode to avoid a fault for missing configuration in the devicetree.
Fixes #80850

Add checks in the GPIO bitbang mode to avoid a fault for missing
configuration in the devicetree.
Fixes zephyrproject-rtos#80850

Signed-off-by: Benedikt Schmidt <[email protected]>
@zephyrbot zephyrbot added the area: FPGA Field-Programmable Gate Array (FPGA) label Nov 5, 2024
Comment on lines +220 to +229
if (config->set == NULL) {
LOG_ERR("%s: set register was not specified", dev->name);
return -EFAULT;
}

if (config->clear == NULL) {
LOG_ERR("%s: clear register was not specified", dev->name);
return -EFAULT;
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be possible to move this check at build-time by removing the _OR from here:

#define FPGA_ICE40_GPIO_PINS(inst, name) (volatile gpio_port_pins_t *)DT_INST_PROP_OR(inst, name, 0)

But it is still allowed to be empty for SPI, so it could be some build assert checking FPGA_ICE40_LOAD_MODE(inst) eventually.

A run-time check is good too!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for build-time checks.

@cfriedt
Copy link
Member

cfriedt commented Nov 5, 2024

@benediktibk - I would fully support moving these checks to build-time if that works. It's possible that the build_all tests would need to have some arbitrary gpio present.

Copy link
Member

@cfriedt cfriedt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix @benediktibk

@benediktibk
Copy link
Collaborator Author

I'm planning on simplifying the usage of the iCE40 in #80846 and #80854 and would prefer to change these into build time check probably in #80854, as this hopefully simplifies the macros.

@mmahadevan108 mmahadevan108 added the bug The issue is a bug, or the PR is fixing a bug label Nov 5, 2024
@mmahadevan108 mmahadevan108 added this to the v4.0.0 milestone Nov 5, 2024
@mmahadevan108 mmahadevan108 merged commit 035b139 into zephyrproject-rtos:main Nov 7, 2024
28 checks passed
@benediktibk benediktibk deleted the fix/fpga_ice40_gpio_bitbang branch November 11, 2024 07:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: FPGA Field-Programmable Gate Array (FPGA) bug The issue is a bug, or the PR is fixing a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

load mode GPIO bitbang for iCE40 is broken
5 participants