Re: [PATCH v1] i2c: img-scb: remove duplicate dev_err()

Re: [PATCH v1] i2c: img-scb: remove duplicate dev_err()

[Markus Elfring]

> it will print an error message by itself when platform_get_irq()
> goes wrong. so don't need dev_err() in here again.

I suggest to improve the change description.
Can you get any further inspiration by previous patches from
other contributors according to the presented transformation pattern?

Would you like to adjust any more source files in the mentioned
software area?

Regards,
Markus

Re: [PATCH v1] i2c: img-scb: remove duplicate dev_err()

[Dejin Zheng]

On Tue, Apr 14, 2020 at 05:48:41PM +0200, Markus Elfring wrote:
> > it will print an error message by itself when platform_get_irq()
> > goes wrong. so don't need dev_err() in here again.
> 
> I suggest to improve the change description.
> Can you get any further inspiration by previous patches from
> other contributors according to the presented transformation pattern?
> 
> Would you like to adjust any more source files in the mentioned
> software area?
>
Hi Markus:

Thanks for your comments, and maybe we can use coccinelle tools for
more source files do this change in the mentioned software area.

BR,
Dejin
> Regards,
> Markus

Re: i2c: img-scb: remove duplicate dev_err()

[Markus Elfring]

> Thanks for your comments, and maybe we can use coccinelle tools for
> more source files do this change in the mentioned software area.

I found 19 source files (for example in the directory “drivers/i2c”
of the software “Linux next-20200408”) which seem to contain similar
update candidates according to a known transformation pattern.
Would you like to take another look at corresponding change possibilities?

Regards,
Markus

Re: i2c: img-scb: remove duplicate dev_err()

[Dejin Zheng]

On Wed, Apr 15, 2020 at 07:47:41AM +0200, Markus Elfring wrote:
> > Thanks for your comments, and maybe we can use coccinelle tools for
> > more source files do this change in the mentioned software area.
> 
> I found 19 source files (for example in the directory “drivers/i2c”
> of the software “Linux next-20200408”) which seem to contain similar
> update candidates according to a known transformation pattern.
> Would you like to take another look at corresponding change possibilities?
>
yes, I want to do it and thanks for your info.

BR,
Dejin

> Regards,
> Markus

Re: i2c: img-scb: remove duplicate dev_err()

[Markus Elfring]

>> Would you like to take another look at corresponding change possibilities?
>>
> yes, I want to do it and thanks for your info.

This positive feedback can lead to corresponding software development opportunities.
Which directions would you like try out next?

Regards,
Markus

Re: i2c: img-scb: remove duplicate dev_err()

[Markus Elfring]

>>> Would you like to take another look at corresponding change possibilities?
>>>
>> yes, I want to do it and thanks for your info.

How do you think about to improve the clarification around potentially
“odd fixes” (because of collateral evolution)?


I noticed your update suggestion “i2c: busses: convert to devm_platform_ioremap_resource”.
https://lore.kernel.org/linux-i2c/20200409135224.7426-1-zhengdejin5@gmail.com/
https://lore.kernel.org/patchwork/patch/1222122/
https://lkml.org/lkml/2020/4/9/431

It seems that you got encouraged to contribute changes according to another
transformation approach by a single patch for a directory hierarchy.
How likely is it that such adjustments touch source code places
where related error messages would be found for further software
development considerations?
Would you like to wait until the integration of this update succeeded?

Will it be interesting in the meantime to become more familiar with
applications around scripts for the semantic patch language?
Can the Coccinelle software make the discussed transformations more convenient
also for your needs?

Regards,
Markus

Re: i2c: img-scb: remove duplicate dev_err()

[Dejin Zheng]

On Wed, Apr 15, 2020 at 06:24:09PM +0200, Markus Elfring wrote:
> >> Would you like to take another look at corresponding change possibilities?
> >>
> > yes, I want to do it and thanks for your info.
> 
> This positive feedback can lead to corresponding software development opportunities.
> Which directions would you like try out next?
>
May be related to usb, ethernet and pcie. My current job is related to
these. Thanks!

BR,
Dejin

> Regards,
> Markus

Re: [PATCH v1] i2c: img-scb: remove duplicate dev_err()

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 307 bytes --]


> > The patch is already in patchwork, so no need to resend from my side.
> 
> Good to know how you handle the (orphaned?) ones.

Kind of. Those are set to "odd fixes" in MAINTAINERS.

Having per-driver maintainers worked quite good so far, so I keep asking
whenever I see potential candidates.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v1] i2c: img-scb: remove duplicate dev_err()

[Andy Shevchenko]

On Tue, Apr 14, 2020 at 06:42:59PM +0200, Wolfram Sang wrote:
> 
> > > scripts/get_maintainer.pl --git --git-min-percent=67 v1-0001-i2c-img-scb-remove-duplicate-dev_err.patch
> > > linux-i2c@vger.kernel.org (open list:I2C SUBSYSTEM HOST DRIVERS)
> > > linux-kernel@vger.kernel.org (open list)
> > 
> > Nice, you found a bug in MAINTAINERS database.
> 
> No, all fine. We don't have a maintainer for that driver.

I realized that there are many drivers like that in the tree...

> > I recommend to resend a new version with those mailings lists followed by
> > Wolfram included in the Cc list. Don't forget to add my tag in the commit
> > message and bump the version.
> 
> The patch is already in patchwork, so no need to resend from my side.

Good to know how you handle the (orphaned?) ones.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1] i2c: img-scb: remove duplicate dev_err()

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 606 bytes --]


> > scripts/get_maintainer.pl --git --git-min-percent=67 v1-0001-i2c-img-scb-remove-duplicate-dev_err.patch
> > linux-i2c@vger.kernel.org (open list:I2C SUBSYSTEM HOST DRIVERS)
> > linux-kernel@vger.kernel.org (open list)
> 
> Nice, you found a bug in MAINTAINERS database.

No, all fine. We don't have a maintainer for that driver.

> I recommend to resend a new version with those mailings lists followed by
> Wolfram included in the Cc list. Don't forget to add my tag in the commit
> message and bump the version.

The patch is already in patchwork, so no need to resend from my side.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v1] i2c: img-scb: remove duplicate dev_err()

[Andy Shevchenko]

On Tue, Apr 14, 2020 at 11:50:47PM +0800, Dejin Zheng wrote:
> On Tue, Apr 14, 2020 at 05:38:37PM +0300, Andy Shevchenko wrote:
> > On Tue, Apr 14, 2020 at 10:26:50PM +0800, Dejin Zheng wrote:
> > > it will print an error message by itself when platform_get_irq()
> > > goes wrong. so don't need dev_err() in here again.
> > 
> > In the future, please use something like this
> > 	scripts/get_maintainer.pl --git --git-min-percent=67
> > when retrieve Cc list for the mail.
> >
> Andy, Thank you for reminding me again, if I used it, There are only two
> email addresses. 
> 
> scripts/get_maintainer.pl --git --git-min-percent=67 v1-0001-i2c-img-scb-remove-duplicate-dev_err.patch
> linux-i2c@vger.kernel.org (open list:I2C SUBSYSTEM HOST DRIVERS)
> linux-kernel@vger.kernel.org (open list)

Nice, you found a bug in MAINTAINERS database.

> > FWIW,
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >
> Thanks very much for your review.

I recommend to resend a new version with those mailings lists followed by
Wolfram included in the Cc list. Don't forget to add my tag in the commit
message and bump the version.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1] i2c: img-scb: remove duplicate dev_err()

[Dejin Zheng]

On Tue, Apr 14, 2020 at 05:38:37PM +0300, Andy Shevchenko wrote:
> On Tue, Apr 14, 2020 at 10:26:50PM +0800, Dejin Zheng wrote:
> > it will print an error message by itself when platform_get_irq()
> > goes wrong. so don't need dev_err() in here again.
> 
> In the future, please use something like this
> 	scripts/get_maintainer.pl --git --git-min-percent=67
> when retrieve Cc list for the mail.
>
Andy, Thank you for reminding me again, if I used it, There are only two
email addresses. 

scripts/get_maintainer.pl --git --git-min-percent=67 v1-0001-i2c-img-scb-remove-duplicate-dev_err.patch
linux-i2c@vger.kernel.org (open list:I2C SUBSYSTEM HOST DRIVERS)
linux-kernel@vger.kernel.org (open list)

> 
> FWIW,
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>
Thanks very much for your review.

BR,
Dejin

> > Signed-off-by: Dejin Zheng <zhengdejin5@gmail.com>
> > ---
> >  drivers/i2c/busses/i2c-img-scb.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
> > index 422097a31c95..2f6de763816a 100644
> > --- a/drivers/i2c/busses/i2c-img-scb.c
> > +++ b/drivers/i2c/busses/i2c-img-scb.c
> > @@ -1344,10 +1344,8 @@ static int img_i2c_probe(struct platform_device *pdev)
> >  		return PTR_ERR(i2c->base);
> >  
> >  	irq = platform_get_irq(pdev, 0);
> > -	if (irq < 0) {
> > -		dev_err(&pdev->dev, "can't get irq number\n");
> > +	if (irq < 0)
> >  		return irq;
> > -	}
> >  
> >  	i2c->sys_clk = devm_clk_get(&pdev->dev, "sys");
> >  	if (IS_ERR(i2c->sys_clk)) {
> > -- 
> > 2.25.0
> > 
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

Re: [PATCH v1] i2c: img-scb: remove duplicate dev_err()

[Andy Shevchenko]

On Tue, Apr 14, 2020 at 10:26:50PM +0800, Dejin Zheng wrote:
> it will print an error message by itself when platform_get_irq()
> goes wrong. so don't need dev_err() in here again.

In the future, please use something like this
	scripts/get_maintainer.pl --git --git-min-percent=67
when retrieve Cc list for the mail.


FWIW,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

> Signed-off-by: Dejin Zheng <zhengdejin5@gmail.com>
> ---
>  drivers/i2c/busses/i2c-img-scb.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
> index 422097a31c95..2f6de763816a 100644
> --- a/drivers/i2c/busses/i2c-img-scb.c
> +++ b/drivers/i2c/busses/i2c-img-scb.c
> @@ -1344,10 +1344,8 @@ static int img_i2c_probe(struct platform_device *pdev)
>  		return PTR_ERR(i2c->base);
>  
>  	irq = platform_get_irq(pdev, 0);
> -	if (irq < 0) {
> -		dev_err(&pdev->dev, "can't get irq number\n");
> +	if (irq < 0)
>  		return irq;
> -	}
>  
>  	i2c->sys_clk = devm_clk_get(&pdev->dev, "sys");
>  	if (IS_ERR(i2c->sys_clk)) {
> -- 
> 2.25.0
> 

-- 
With Best Regards,
Andy Shevchenko

[PATCH v1] i2c: img-scb: remove duplicate dev_err()

[Dejin Zheng]

it will print an error message by itself when platform_get_irq()
goes wrong. so don't need dev_err() in here again.

Signed-off-by: Dejin Zheng <zhengdejin5@gmail.com>
---
 drivers/i2c/busses/i2c-img-scb.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index 422097a31c95..2f6de763816a 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -1344,10 +1344,8 @@ static int img_i2c_probe(struct platform_device *pdev)
 		return PTR_ERR(i2c->base);
 
 	irq = platform_get_irq(pdev, 0);
-	if (irq < 0) {
-		dev_err(&pdev->dev, "can't get irq number\n");
+	if (irq < 0)
 		return irq;
-	}
 
 	i2c->sys_clk = devm_clk_get(&pdev->dev, "sys");
 	if (IS_ERR(i2c->sys_clk)) {
-- 
2.25.0