Re: [PATCH] serial: qcom-geni: fix enabling deactivated interrupt

From: Stephen Boyd
Date: Fri May 05 2023 - 19:51:26 EST


Quoting Krzysztof Kozlowski (2023-05-05 08:23:01)
> The driver have a race, experienced only with PREEMPT_RT patchset:
>
> CPU0 | CPU1
> ==================================================================
> qcom_geni_serial_probe |
> uart_add_one_port |
> | serdev_drv_probe
> | qca_serdev_probe
> | serdev_device_open
> | uart_open
> | uart_startup
> | qcom_geni_serial_startup
> | enable_irq
> | __irq_startup
> | WARN_ON()
> | IRQ not activated
> request_threaded_irq |
> irq_domain_activate_irq |
>
> The warning:
>
> 894000.serial: ttyHS1 at MMIO 0x894000 (irq = 144, base_baud = 0) is a MSM
> serial serial0: tty port ttyHS1 registered
> WARNING: CPU: 7 PID: 107 at kernel/irq/chip.c:241 __irq_startup+0x78/0xd8
> ...
> qcom_geni_serial 894000.serial: serial engine reports 0 RX bytes in!
>
> Adding UART port triggers probe of child serial devices - serdev and
> eventually Qualcomm Bluetooth hci_qca driver. This opens UART port
> which enables the interrupt before it got activated in
> request_threaded_irq(). The issue originates in commit f3974413cf02
> ("tty: serial: qcom_geni_serial: Wakeup IRQ cleanup") and discussion on
> mailing list [1]. However the above commit does not explain why the
> uart_add_one_port() is moved above requesting interrupt.
>
> [1] https://lore.kernel.org/all/5d9f3dfa.1c69fb81.84c4b.30bf@xxxxxxxxxxxxx/
>
> Fixes: f3974413cf02 ("tty: serial: qcom_geni_serial: Wakeup IRQ cleanup")
> Cc: <stable@xxxxxxxxxxxxxxx>
> Cc: Stephen Boyd <swboyd@xxxxxxxxxxxx>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
> ---

Reviewed-by: Stephen Boyd <swboyd@xxxxxxxxxxxx>