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.
Signed-off-by: Tang Bin <[email protected]>
---
drivers/mfd/asic3.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/mfd/asic3.c b/drivers/mfd/asic3.c
index a6bd2134c..cee7454b3 100644
--- a/drivers/mfd/asic3.c
+++ b/drivers/mfd/asic3.c
@@ -924,7 +924,6 @@ static int __init asic3_mfd_probe(struct platform_device *pdev,
goto out;
}
- ret = 0;
if (pdata->leds) {
int i;
--
2.20.1.windows.1
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?
> Signed-off-by: Tang Bin <[email protected]>
> ---
> drivers/mfd/asic3.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/mfd/asic3.c b/drivers/mfd/asic3.c
> index a6bd2134c..cee7454b3 100644
> --- a/drivers/mfd/asic3.c
> +++ b/drivers/mfd/asic3.c
> @@ -924,7 +924,6 @@ static int __init asic3_mfd_probe(struct platform_device *pdev,
> goto out;
> }
>
> - ret = 0;
> if (pdata->leds) {
> int i;
>
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
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.
Thanks,
Tang Bin
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