drivers: uart_mcux_flexcomm: fix mcux_flexcomm_isr unused for polled uart

[alpsayin]

When UART_INTERRUPT_DRIVEN=n, mcux_flexcomm_isr and the data structure inside is left unused.
This patch turns off the build of the entire ISR.

First noticed as part of a CI run in #52208
CI run link: https://github.com/zephyrproject-rtos/zephyr/actions/runs/5403369165/jobs/9816093378?pr=52208#step:13:469

Fixes #59853

[decsny]

will break async api

[alpsayin]

will break async api

Argh. Apologies.

I'll need to do some more interesting ifdefry then.

[decsny]

I'll need to do some more interesting ifdefry then.

yeah that's probably it

[decsny]

@alpsayin what you are doing here just made me realize there is actually currently a bug in this driver, the irq connect / setup macro near the bottom doesnt seem to happen unless config_interrupt_driven is set. Which means it wouldnt happen for async api. My guess is all the boards tested with the async api using this driver have config_interrupt_driven set in their board _defconfig file which is why it worked in testing.

#59844

[alpsayin]

@alpsayin what you are doing here just made me realize there is actually currently a bug in this driver, the irq connect / setup macro near the bottom doesnt seem to happen unless config_interrupt_driven is set. Which means it wouldnt happen for async api. My guess is all the boards tested with the async api using this driver have config_interrupt_driven set in their board _defconfig file which is why it worked in testing.

To be perfectly honest, I should've checked why this was necessary. But I didn't :/. Async api sounds to me like it'd need ISRs, but I didn't give it too much thought.
That said, irqconnect/setup not happening when NOT interrupt driven sounds correct to me.

But this is all based on the words and their meanings, not the code. I don't know what exactly
async api does, once I do then I can comment more intelligently.

P.S. at zephyr conference now, i have sporadic access to things

[decsny]

That said, irqconnect/setup not happening when NOT interrupt driven sounds correct to me.

Async api requires interrupt, I am responsible for this bug as I wrote about 80% of this driver, which is why I assigned myself to the issue I just made in case it doesn't get fixed in this PR

[alpsayin]

@decsny what do you think of this "highly elegant" solution?
I think this is in alignment with what Async API needs to do.

[decsny]

@decsny what do you think of this "highly elegant" solution?
I think this is in alignment with what Async API needs to do.

woops, I just had a PR to do almost the same thing, minus the original issue you were fixing here... I just closed it as duplicate, so this is good (you did the exact same thing I was going to do)

thanks for the help with this

[decsny]

Fixes #59853

[danieldegrasse]

Could we solve this issue the same way we do in the LPUART driver, with a shared KConfig that is set when the interrupt driven or async API is used? https://github.com/zephyrproject-rtos/zephyr/blob/main/drivers/serial/Kconfig.mcux_lpuart#L19.

I ask because if we end up needing to enable the interrupt when CONFIG_PM=y (can be required to insure that all characters are sent before the device enters sleep), we will have a rather ugly set of #if defined(x) conditionals sprinkled throughout the driver.

[alpsayin]

Could we solve this issue the same way we do in the LPUART driver, with a shared KConfig that is set when the interrupt driven or async API is used? https://github.com/zephyrproject-rtos/zephyr/blob/main/drivers/serial/Kconfig.mcux_lpuart#L19.

I ask because if we end up needing to enable the interrupt when CONFIG_PM=y (can be required to insure that all characters are sent before the device enters sleep), we will have a rather ugly set of #if defined(x) conditionals sprinkled throughout the driver.

If you both agree this is the way, I'm more than happy to make the changes.
I personally do agree with your sentiment.

To be clear, should the new flag be enabled on UART_INTERRUPT_DRIVEN || PM || UART_ASYNC_API ?
Or just UART_INTERRUPT_DRIVEN || UART_ASYNC_API without PM ?

[decsny]

Could we solve this issue the same way we do in the LPUART driver, with a shared KConfig that is set when the interrupt driven or async API is used? https://github.com/zephyrproject-rtos/zephyr/blob/main/drivers/serial/Kconfig.mcux_lpuart#L19.

I ask because if we end up needing to enable the interrupt when CONFIG_PM=y (can be required to insure that all characters are sent before the device enters sleep), we will have a rather ugly set of #if defined(x) conditionals sprinkled throughout the driver.

I prefer to have a minimal set of Kconfigs over code aesthetic. However, the advantage I see to having such a Kconfig is not aesthetics but the benefit of single point of control. I don't have a preference here.

[alpsayin]

@decsny @danieldegrasse I've kept it as a separate commit so that you can inspect the diff partially or fully. I'll fixup or remove the 2nd commit depending on your thoughts.
What do think?

[danieldegrasse]

You are welcome to squash the commits here if you'd prefer, but they are both valid changes on their own (won't break bisectbility)