Re: [PATCH 1/2] drm/bridge: adv7511: Allow IRQ to share GPIO pins

[Laurent Pinchart]

Hi Adam,

Thank you for the patch.

On Wed, Feb 28, 2024 at 05:37:35AM -0600, Adam Ford wrote:
> The IRQ registration currently assumes that the GPIO is
> dedicated to it, but that may not necessarily be the case.
> If the board has another device sharing the IRQ, it won't be
> registered and the hot-plug detect fails.  This is easily
> fixed by add the IRQF_SHARED flag.
> 
> Signed-off-by: Adam Ford <aford...@gmail.com>
> 
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c 
> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> index b5518ff97165..21f08b2ae265 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -1318,7 +1318,8 @@ static int adv7511_probe(struct i2c_client *i2c)
>  
>               ret = devm_request_threaded_irq(dev, i2c->irq, NULL,
>                                               adv7511_irq_handler,
> -                                             IRQF_ONESHOT, dev_name(dev),
> +                                             IRQF_ONESHOT | IRQF_SHARED,
> +                                             dev_name(dev),

This looks fine, but the IRQ handler doesn't.

static int adv7511_irq_process(struct adv7511 *adv7511, bool process_hpd)
{
        unsigned int irq0, irq1;
        int ret;

        ret = regmap_read(adv7511->regmap, ADV7511_REG_INT(0), &irq0);
        if (ret < 0)
                return ret;

        ret = regmap_read(adv7511->regmap, ADV7511_REG_INT(1), &irq1);
        if (ret < 0)
                return ret;

        regmap_write(adv7511->regmap, ADV7511_REG_INT(0), irq0);
        regmap_write(adv7511->regmap, ADV7511_REG_INT(1), irq1);

        if (process_hpd && irq0 & ADV7511_INT0_HPD && adv7511->bridge.encoder)
                schedule_work(&adv7511->hpd_work);

        if (irq0 & ADV7511_INT0_EDID_READY || irq1 & ADV7511_INT1_DDC_ERROR) {
                adv7511->edid_read = true;

                if (adv7511->i2c_main->irq)
                        wake_up_all(&adv7511->wq);
        }

#ifdef CONFIG_DRM_I2C_ADV7511_CEC
        adv7511_cec_irq_process(adv7511, irq1);
#endif

        return 0;
}

static irqreturn_t adv7511_irq_handler(int irq, void *devid)
{
        struct adv7511 *adv7511 = devid;
        int ret;

        ret = adv7511_irq_process(adv7511, true);
        return ret < 0 ? IRQ_NONE : IRQ_HANDLED;
}

The function will return IRQ_HANDLED as long as the registers can be
read, even if they don't report any interrupt.

>                                               adv7511);
>               if (ret)
>                       goto err_unregister_audio;

-- 
Regards,

Laurent Pinchart