Re: [PATCH v3 2/2] spi: qcom-geni: Fix missing error check on pm_runtime_get_sync()

[Rafael J. Wysocki (Intel)]

On Mon, Jul 6, 2026 at 10:53 AM Praveen Talari
<praveen.talari@xxxxxxxxxxxxxxxx> wrote:
>
> spi_geni_init() calls pm_runtime_get_sync() to power up the device
> before accessing hardware registers, but never checks the return value.
> If the runtime resume fails, the function silently proceeds to read and
> write hardware registers on a device that may not be powered up, leading
> to register access faults.
>
> Fix this by replacing pm_runtime_get_sync() with the
> PM_RUNTIME_ACQUIRE_IF_ENABLED() macro and checking the result via
> PM_RUNTIME_ACQUIRE_ERR(), propagating any error back to the caller
> immediately before any hardware access occurs.
>
> Since the macro handles its own cleanup on failure, the out_pm label and
> the corresponding pm_runtime_put() call are no longer needed. Replace
> all goto out_pm paths with direct return ret statements and remove the
> label entirely.
>
> Fixes: 561de45f72bd ("spi: spi-geni-qcom: Add SPI driver support for GENI based QUP")
> Signed-off-by: Praveen Talari <praveen.talari@xxxxxxxxxxxxxxxx>

Reviewed-by: Rafael J. Wysocki (Intel) <rafael@xxxxxxxxxx>

(no issues found)

> ---
>  drivers/spi/spi-geni-qcom.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
> index 26e723cfea61..392accfd8515 100644
> --- a/drivers/spi/spi-geni-qcom.c
> +++ b/drivers/spi/spi-geni-qcom.c
> @@ -613,25 +613,30 @@ static int spi_geni_init(struct spi_geni_master *mas)
>         u32 spi_tx_cfg, fifo_disable;
>         int ret = -ENXIO;
>
> -       pm_runtime_get_sync(mas->dev);
> +       PM_RUNTIME_ACQUIRE_IF_ENABLED(mas->dev, pm);
> +       ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
> +       if (ret < 0) {
> +               dev_err(mas->dev, "Failed to resume and get %d\n", ret);
> +               return ret;
> +       }
>
>         proto = geni_se_read_proto(se);
>
>         if (spi->target) {
>                 if (proto != GENI_SE_SPI_SLAVE) {
>                         dev_err(mas->dev, "Invalid proto %d\n", proto);
> -                       goto out_pm;
> +                       return ret;
>                 }
>                 spi_slv_setup(mas);
>         } else if (proto == GENI_SE_INVALID_PROTO) {
>                 ret = geni_load_se_firmware(se, GENI_SE_SPI);
>                 if (ret) {
>                         dev_err(mas->dev, "spi master firmware load failed ret: %d\n", ret);
> -                       goto out_pm;
> +                       return ret;
>                 }
>         } else if (proto != GENI_SE_SPI) {
>                 dev_err(mas->dev, "Invalid proto %d\n", proto);
> -               goto out_pm;
> +               return ret;
>         }
>         mas->tx_fifo_depth = geni_se_get_tx_fifo_depth(se);
>
> @@ -664,7 +669,7 @@ static int spi_geni_init(struct spi_geni_master *mas)
>                         dev_dbg(mas->dev, "Using GPI DMA mode for SPI\n");
>                         break;
>                 } else if (ret == -EPROBE_DEFER) {
> -                       goto out_pm;
> +                       return ret;
>                 }
>                 /*
>                  * in case of failure to get gpi dma channel, we can still do the
> @@ -693,10 +698,7 @@ static int spi_geni_init(struct spi_geni_master *mas)
>                 writel(spi_tx_cfg, se->base + SE_SPI_TRANS_CFG);
>         }
>
> -out_pm:
> -       pm_runtime_put(mas->dev);
>         return ret;
> -}
>
>  static unsigned int geni_byte_per_fifo_word(struct spi_geni_master *mas)
>  {
>
> --
> 2.34.1
>