Re: [PATCH] gpio: tegra186: Check PMC driver status before any request

[Thierry Reding]

On Mon, Sep 12, 2022 at 07:03:09PM +0530, Prathamesh Shete wrote:
> This patch fixes the issue where even if pmc driver
> status is disabled still we are invoking pmc driver
> to process some request

s/pmc/PMC/ twice above. Also, this is slightly misleading. We're not
"invoking" the PMC driver, but rather we're trying to look up the IRQ
domain that the PMC driver would've registered if it had been enabled.
So perhaps reword this to more accurately reflect that.

> 
> Signed-off-by: Manish Bhardwaj <mbhardwaj@xxxxxxxxxx>
> Signed-off-by: Prathamesh Shete <pshete@xxxxxxxxxx>
> ---
>  drivers/gpio/gpio-tegra186.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpio/gpio-tegra186.c b/drivers/gpio/gpio-tegra186.c
> index 54d9fa7da9c1..efd508ba07a6 100644
> --- a/drivers/gpio/gpio-tegra186.c
> +++ b/drivers/gpio/gpio-tegra186.c
> @@ -895,7 +895,7 @@ static int tegra186_gpio_probe(struct platform_device *pdev)
>  		tegra186_gpio_init_route_mapping(gpio);
>  
>  	np = of_find_matching_node(NULL, tegra186_pmc_of_match);
> -	if (np) {
> +	if (of_device_is_available(np)) {
>  		irq->parent_domain = irq_find_host(np);
>  		of_node_put(np);

This now leaks a reference to np if np is found but not available. So
this should be something like:

	if (np) {
		if (of_device_is_available(np)) {
			irq->parent_domain = irq_find_host(np);
			of_node_put(np);

			if (!irq->parent_domain)
				return -EPROBE_DEFER;
		} else {
			of_node_put(np);
		}
	}

or:

	if (np) {
		if (of_device_is_available(np))
			irq->parent_domain = irq_find_host(np);

		of_node_put(np);

		if (of_device_is_available(np) && !irq->parent_domain)
			return -EPROBE_DEFER;
	}

The former is a little nicer because it doesn't check availability
twice.

Thierry

Attachment: signature.asc
Description: PGP signature