linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: tangbin <tangbin@cmss.chinamobile.com>
To: Jonathan Cameron <jic23@kernel.org>
Cc: knaack.h@gmx.de, lars@metafoo.de, shawnguo@kernel.org,
	s.hauer@pengutronix.de, festevam@gmail.com,
	linux-iio@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] iio: adc: fsl-imx25-gcq: fix the right check and simplify code
Date: Mon, 2 Aug 2021 10:31:58 +0800	[thread overview]
Message-ID: <b84ea3e4-5650-d6ac-36f6-98067b286b45@cmss.chinamobile.com> (raw)
In-Reply-To: <20210731174551.188aee79@jic23-huawei>

Hi Jonathan:

On 2021/8/1 0:45, Jonathan Cameron wrote:
> On Tue, 27 Jul 2021 20:52:09 +0800
> Tang Bin <tangbin@cmss.chinamobile.com> wrote:
>
>> For the function of platform_get_irq(), the example in platform.c is
>> *		int irq = platform_get_irq(pdev, 0);
>> *		if (irq < 0)
>> *			return irq;
>> So the return value of zero is unnecessary to check. And move it
>> up to a little bit can simplify the code jump.
>>
>> Co-developed-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com>
>> Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com>
>> Signed-off-by: Tang Bin <tangbin@cmss.chinamobile.com>
> Hi,
>
> Logically it is better to keep the irq handling all together, so
> I would prefer we didn't move it.
Got it in this place.
>
> Also, platform_get_irq() is documented as never returning 0, so the current
> code is not incorrect.  As such, this looks like noise unless there is
> some plan to make use of the 0 return value?  What benefit do we get from
> this change?

Thanks for your reply, I think the benefit of this change maybe just 
simplify the code.

Because the return value is never equal to 0, so the check in here is 
redundant.

We can make the patch like this:

>> ---
>>   drivers/iio/adc/fsl-imx25-gcq.c | 12 ++++--------
>>   1 file changed, 4 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/iio/adc/fsl-imx25-gcq.c b/drivers/iio/adc/fsl-imx25-gcq.c
>> index 8cb51cf7a..d28976f21 100644
>> --- a/drivers/iio/adc/fsl-imx25-gcq.c
>> +++ b/drivers/iio/adc/fsl-imx25-gcq.c
>> @@ -320,6 +320,10 @@ static int mx25_gcq_probe(struct platform_device *pdev)
>>   	if (ret)
>>   		return ret;
>>   
>> +	priv->irq = platform_get_irq(pdev, 0);
>> +	if (priv->irq < 0)
>> +		return priv->irq;
>> +
>>   	for (i = 0; i != 4; ++i) {
>>   		if (!priv->vref[i])
>>   			continue;
>> @@ -336,14 +340,6 @@ static int mx25_gcq_probe(struct platform_device *pdev)
>>   		goto err_vref_disable;
>>   	}
>>   
>> -	priv->irq = platform_get_irq(pdev, 0);
>> -	if (priv->irq <= 0) {
>> -		ret = priv->irq;
>> -		if (!ret)
>> -			ret = -ENXIO;
>> -		goto err_clk_unprepare;
>> -	}
>> -

	priv->irq = platform_get_irq(pdev, 0);
	if (priv->irq < 0) {
		ret = priv->irq;
		goto err_clk_unprepare;
	}

     If you think this is ok, I will send V2 for you. If you think these 
change is meaningless,

just dropped this.

Thanks

Tang Bin





  reply	other threads:[~2021-08-02  2:33 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-27 12:52 Tang Bin
2021-07-31 16:45 ` Jonathan Cameron
2021-08-02  2:31   ` tangbin [this message]
2021-08-02 10:16     ` Jonathan Cameron
2021-08-02 11:50       ` [PATCH] iio: adc: fsl-imx25-gcq: fix the right check andsimplify code tangbin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b84ea3e4-5650-d6ac-36f6-98067b286b45@cmss.chinamobile.com \
    --to=tangbin@cmss.chinamobile.com \
    --cc=festevam@gmail.com \
    --cc=jic23@kernel.org \
    --cc=knaack.h@gmx.de \
    --cc=lars@metafoo.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=s.hauer@pengutronix.de \
    --cc=shawnguo@kernel.org \
    --subject='Re: [PATCH] iio: adc: fsl-imx25-gcq: fix the right check and simplify code' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).