Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 17 Apr 2020 10:27:05 +0100
From:   Lee Jones <lee.jones@linaro.org>
To:     Tang Bin <tangbin@cmss.chinamobile.com>
Cc:     linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mfd:asic3:Delete redundant variable definition
Message-ID: <20200417092705.GG2167633@dell>
References: <20200403042020.17452-1-tangbin@cmss.chinamobile.com>
 <20200416070802.GQ2167633@dell>
 <211e7521-639d-804c-f332-bfed0cf315df@cmss.chinamobile.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <211e7521-639d-804c-f332-bfed0cf315df@cmss.chinamobile.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, 16 Apr 2020, Tang Bin wrote:

> Hi, Lee:
> 
> On 2020/4/16 15:08, Lee Jones wrote:
> > On Fri, 03 Apr 2020, Tang Bin wrote:
> > 
> > > In this function, 'ret' is always assigned, even if 'pdata->leds'
> > > don't carry out,it has already been assigned a value in the above
> > > code, including '0',so it's redundant.
> > Which line initialises/assigns 'ret' before this one?
> 
>     I think it maybe my fault before, because I treat get resource and irq
> succeed. But now I have two questions  to ask you:
> 
>     Q1: About asic3_mfd_probe()?
> 
>           In the function asic3_mfd_probe(), if get resource or irq failed,
> the value returned just detected and dev_dbg() error message, but there were
> no error return. What I think the modify should be as follows:
> 
>     mem_sdio = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>     if (!mem_sdio) {
>         dev_dbg(asic->dev, "no SDIO MEM resource\n");
>         ret = -EINVAL;
>         goto out;
>     }
> 
>     irq = platform_get_irq(pdev, 1);
>     if (irq < 0) {
>         dev_dbg(asic->dev, "no SDIO IRQ resource\n");
>         ret = irq;
>         goto out;
>     }
> 
>     If the function do like this, the 'ret = 0' in line 993 maybe redundant.
> 
> 
>    Q2: About asic3_probe()?
> 
>           In the line 995, if the function asic3_irq_probe() failed, it will
> print error message by the internally called function platform_get_irq(), so
> the dev_err() in the line 997 is redundant, should be delete.
> 
> 
>     I'll wait actively, and submit the corresponding patch according to your
> reply.

Just submit the patch and we can discuss it there.

These kind of messages always require a lot of extra faff to process.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog