Date:   Tue, 4 Jan 2022 16:21:46 +0100
From:   Miquel Raynal <miquel.raynal@bootlin.com>
To:     Jiasheng Jiang <jiasheng@iscas.ac.cn>
Cc:     kyungmin.park@samsung.com, richard@nod.at, vigneshr@ti.com,
        linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] mtd: onenand: Check for error irq
Message-ID: <20220104162146.38c41e06@xps13>
In-Reply-To: <20220104145854.1987853-1-jiasheng@iscas.ac.cn>
References: <20220104145854.1987853-1-jiasheng@iscas.ac.cn>
Organization: Bootlin
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8BIT
Precedence: bulk

Hi Jiasheng,

jiasheng@iscas.ac.cn wrote on Tue,  4 Jan 2022 22:58:54 +0800:

> As it is possible that there is no suitable irq, platform_get_irq()
> will return error number.

             an

> Therefore, it might be better to check it and return error if fails

                                                      an        failed

> to guarantee the success of the probe.

in order to

Actually in case of error it will not guarantee the success of the
probe so I'm not sure what you are trying to solve here?

Either the irq is mandatory and you may want to check its presence
earlier, or it is not mandatory and you could use get_irq_optional()
maybe.

> Fixes: 26777d37216c ("mtd: Move onenand code base to drivers/mtd/nand/onenand")

This Fixes tag is still wrong.

> Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
> ---
> Changelog:
> 
> v1 -> v2
> 
> * Change 1. Change the commit message.
> ---
>  drivers/mtd/nand/onenand/generic.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/onenand/generic.c b/drivers/mtd/nand/onenand/generic.c
> index 8b6f4da5d720..a4b8b65fe15f 100644
> --- a/drivers/mtd/nand/onenand/generic.c
> +++ b/drivers/mtd/nand/onenand/generic.c
> @@ -53,7 +53,12 @@ static int generic_onenand_probe(struct platform_device *pdev)
>  	}
>  
>  	info->onenand.mmcontrol = pdata ? pdata->mmcontrol : NULL;
> -	info->onenand.irq = platform_get_irq(pdev, 0);
> +
> +	err = platform_get_irq(pdev, 0);
> +	if (err < 0)
> +		goto out_iounmap;
> +
> +	info->onenand.irq = err;
>  
>  	info->mtd.dev.parent = &pdev->dev;
>  	info->mtd.priv = &info->onenand;


Thanks,
Miquèl