Re: [PATCH v3 07/10] media: synopsys: hdmirx: skip the 5V detect interrupt without a GPIO

[Dmitry Osipenko <dmitry.osipenko@collabora.com>]

On 10/5/26 16:28, Sascha Hauer wrote:
> From: Gerald Loacker <gerald.loacker@wolfvision.net>
> 
> The hpd GPIO is fetched with devm_gpiod_get_optional(), so a board that
> does not wire 5V detect to the receiver leaves detect_5v_gpio NULL.
> hdmirx_setup_irq() then calls gpiod_to_irq() on it anyway, which returns
> -EINVAL for a NULL descriptor, and probe fails. The GPIO is only
> optional as far as the fetch goes.
> 
> Set the interrupt up only when the GPIO is there. det_irq then keeps the
> zero devm_kzalloc() gave it, so hdmirx_enable_irq() and
> hdmirx_disable_irq() skip it as well.
> 
> Nothing else has to change for such a board to probe: 5V is then never
> reported as present, which is what gpiod_get_value_cansleep() on a NULL
> descriptor already returns. Getting the state from somewhere else is a
> separate matter.
> 
> Assisted-by: LLM
> Signed-off-by: Gerald Loacker <gerald.loacker@wolfvision.net>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>   .../media/platform/synopsys/hdmirx/snps_hdmirx.c   | 34 ++++++++++++----------
>   1 file changed, 19 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
> index a28eb86e0b83c..a937ee23e230c 100644
> --- a/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
> +++ b/drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
> @@ -2521,7 +2521,8 @@ static void hdmirx_disable_irq(struct device *dev)
>   {
>   	struct snps_hdmirx_dev *hdmirx_dev = dev_get_drvdata(dev);
>   
> -	disable_irq(hdmirx_dev->det_irq);
> +	if (hdmirx_dev->det_irq > 0)
> +		disable_irq(hdmirx_dev->det_irq);
>   	disable_irq(hdmirx_dev->dma_irq);
>   	disable_irq(hdmirx_dev->hdmi_irq);
>   
> @@ -2535,7 +2536,8 @@ static void hdmirx_enable_irq(struct device *dev)
>   
>   	enable_irq(hdmirx_dev->hdmi_irq);
>   	enable_irq(hdmirx_dev->dma_irq);
> -	enable_irq(hdmirx_dev->det_irq);
> +	if (hdmirx_dev->det_irq > 0)
> +		enable_irq(hdmirx_dev->det_irq);
>   
>   	queue_delayed_work(system_dfl_wq,
>   			   &hdmirx_dev->delayed_work_hotplug,
> @@ -2619,21 +2621,23 @@ static int hdmirx_setup_irq(struct snps_hdmirx_dev *hdmirx_dev,
>   		return ret;
>   	}
>   
> -	irq = gpiod_to_irq(hdmirx_dev->detect_5v_gpio);
> -	if (irq < 0) {
> -		dev_err_probe(dev, irq, "failed to get hdmirx-5v irq\n");
> -		return irq;
> -	}
> +	if (hdmirx_dev->detect_5v_gpio) {
> +		irq = gpiod_to_irq(hdmirx_dev->detect_5v_gpio);
> +		if (irq < 0) {
> +			dev_err_probe(dev, irq, "failed to get hdmirx-5v irq\n");
> +			return irq;
> +		}
>   
> -	irq_set_status_flags(irq, IRQ_NOAUTOEN);
> +		irq_set_status_flags(irq, IRQ_NOAUTOEN);
>   
> -	hdmirx_dev->det_irq = irq;
> -	ret = devm_request_irq(dev, irq, hdmirx_5v_det_irq_handler,
> -			       IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
> -			       "rk_hdmirx-5v", hdmirx_dev);
> -	if (ret) {
> -		dev_err_probe(dev, ret, "failed to request hdmirx-5v irq\n");
> -		return ret;
> +		hdmirx_dev->det_irq = irq;
> +		ret = devm_request_irq(dev, irq, hdmirx_5v_det_irq_handler,
> +				       IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
> +				       "rk_hdmirx-5v", hdmirx_dev);
> +		if (ret) {
> +			dev_err_probe(dev, ret, "failed to request hdmirx-5v irq\n");
> +			return ret;
> +		}
>   	}
>   
>   	return 0;
> 

Acked-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>

-- 
Best regards,
Dmitry