Bluetooth: hci: spi: host doesn't work on targets which do not support level triggered interrupts

[lucasdietrich]

Bug Description
With zephyr v3.4, an application implementing an HCI HOST will constantly fail on targets which do not support GPIO level-triggered interrupts. However, this was working with versions of Zephyr prior to 3.3.

This change from edge to level-triggered interrupts was introduced in the following PR #57427 which addresses issue #53980.

Steps for Reproduction
To replicate the issue, build a Bluetooth sample configure as a HCI HOST (i.e. with CONFIG_BT_HCI=y, CONFIG_BT_SPI=y and CONFIG_BT_CTLR=n) on a board that doesn't support level-triggered GPIO interrupts (e.g., any stm32 board, nucleo -f429zi in my particular instance).

The problem arises when the host can't configure the IRQ pin as a level-triggered interrupt via gpio_pin_interrupt_configure_dt(&irq_gpio, GPIO_INT_LEVEL_ACTIVE); .

The stm32 driver explicitly tells level-triggered interrupts are not supported here.

Expected Result
HCI HOST should ideally be functional even with targets that don't support level-triggered interrupts.

Consequences
As it stands, the HCI HOST is incompatible with targets that don't support level-triggered interrupts.

Environment Details:

• Linux Fedora 37
• Zephyr v3.4.0
• Zephyr SDK 0.16.1

Additional Information
I imagine that only few targets support level-triggered interrupts.
Consequently, it might be more effective to design the HCI driver with the assumption that only edge-triggered interrupts are supported.

While I haven't yet formulated a resolution, I plan to think about it.

Edit:
I edited my message to highlight that this issue was introduced by version 3.4 of zephyr.

[jori-nordic]

tagging @JordanYates since he did the original change.

[JordanYates]

I appreciate the issue, but simply reverting the change just re-introduces the original issue.
Presumably this sort of issue has been encountered before. Is the standard solution to only use lowest common denominator features in drivers or to introduce some form of emulation for chips missing the feature?
I note that GPIO_INT_LEVEL_ACTIVE is used in a few other sensor drivers, which would be affected by the same problem.

[erwango]

Is the standard solution to only use lowest common denominator features in drivers or to introduce some form of emulation for chips missing the feature?

I would opt for "lowest common denominator features".
Alternative would mean each driver as to provide emulation for any missing feature. Not sure this would scale.

[JordanYates]

I would opt for "lowest common denominator features".
Alternative would mean each driver as to provide emulation for any missing feature. Not sure this would scale.

The same argument also applies to lowest common denominator. If Zephyr starts supporting some crazy micro that only supports level triggered interrupts, where does that leave us?

[JordanYates]

It's also not really feasible to expect driver developers to check each implementation of peripherals they use to see what is and isn't supported. My first expectation is that if its in a public header like zephyr/drivers/gpio.h then it's fair game to use, and failure to support such a feature is more a problem for the micro implementation than the device driver.

[erwango]

The same argument also applies to lowest common denominator. If Zephyr starts supporting some crazy micro that only supports level triggered interrupts, where does that leave us?

Fair point.

and failure to support such a feature is more a problem for the micro implementation than the device driver.

This doesn't justify in any case to break a previously working functionality.

[jori-nordic]

This doesn't justify in any case to break a previously working functionality.

I think 'previously working' is up for debate, since @JordanYates did fix a bug.

We're planning to put the SPI and UART (H4) drivers in regression at Nordic. But this will only be done for Nordic chips, so if other members of the community / SoC vendors could do the same with their hardware, that would be great.

That way anyone can NAK the problematic PRs before the functionality is broken (I agree, it's not great) and we can try to come up with a solution that satisfies everyone.

A week of delay for those driver PRs would seem like a fair amount of time for those tests to be run, either in CI or manually. Passed that, I think it's ok to merge.