[PATCH] pinctrl-adi2: Use a signed return type for adi_gpio_irq_startup()

[PATCH] pinctrl-adi2: Use a signed return type for adi_gpio_irq_startup()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 19 Dec 2015 17:55:39 +0100

The return type "unsigned int" was used by the adi_gpio_irq_startup()
function despite of the aspect that it will eventually return a negative
error code.
Improve this implementation detail by deletion of the type modifier then.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/pinctrl/pinctrl-adi2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/pinctrl-adi2.c b/drivers/pinctrl/pinctrl-adi2.c
index fd342df..4032004 100644
--- a/drivers/pinctrl/pinctrl-adi2.c
+++ b/drivers/pinctrl/pinctrl-adi2.c
@@ -329,7 +329,7 @@ static void adi_gpio_unmask_irq(struct irq_data *d)
 	spin_unlock_irqrestore(&port->lock, flags);
 }
 
-static unsigned int adi_gpio_irq_startup(struct irq_data *d)
+static int adi_gpio_irq_startup(struct irq_data *d)
 {
 	unsigned long flags;
 	struct gpio_port *port = irq_data_get_irq_chip_data(d);
-- 
2.6.3

[PATCH] pinctrl-adi2: Use a signed return type for adi_gpio_irq_startup()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 19 Dec 2015 17:55:39 +0100

The return type "unsigned int" was used by the adi_gpio_irq_startup()
function despite of the aspect that it will eventually return a negative
error code.
Improve this implementation detail by deletion of the type modifier then.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/pinctrl/pinctrl-adi2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pinctrl/pinctrl-adi2.c b/drivers/pinctrl/pinctrl-adi2.c
index fd342df..4032004 100644
--- a/drivers/pinctrl/pinctrl-adi2.c
+++ b/drivers/pinctrl/pinctrl-adi2.c
@@ -329,7 +329,7 @@ static void adi_gpio_unmask_irq(struct irq_data *d)
 	spin_unlock_irqrestore(&port->lock, flags);
 }
 
-static unsigned int adi_gpio_irq_startup(struct irq_data *d)
+static int adi_gpio_irq_startup(struct irq_data *d)
 {
 	unsigned long flags;
 	struct gpio_port *port = irq_data_get_irq_chip_data(d);
-- 
2.6.3

Re: [PATCH] pinctrl-adi2: Use a signed return type for adi_gpio_irq_startup()

[Dan Carpenter]

This introduces a compile warning.  These functions are supposed to
return 1 if there is an IRQ pending.  Change the -EINVAL to 0.

regards,
dan carpenter

Re: [PATCH] pinctrl-adi2: Use a signed return type for adi_gpio_irq_startup()

[Dan Carpenter]

This introduces a compile warning.  These functions are supposed to
return 1 if there is an IRQ pending.  Change the -EINVAL to 0.

regards,
dan carpenter

Re: pinctrl-adi2: Use a signed return type for adi_gpio_irq_startup()

[SF Markus Elfring]

> This introduces a compile warning.

How do you think about to show the exact message you get?


> These functions are supposed to return 1 if there is an IRQ pending.
> Change the -EINVAL to 0.

Is there any more source code clean-up needed around the comment "FIXME"
in the affected function?

Regards,
Markus

Re: pinctrl-adi2: Use a signed return type for adi_gpio_irq_startup()

[SF Markus Elfring]

> This introduces a compile warning.

How do you think about to show the exact message you get?


> These functions are supposed to return 1 if there is an IRQ pending.
> Change the -EINVAL to 0.

Is there any more source code clean-up needed around the comment "FIXME"
in the affected function?

Regards,
Markus

Re: pinctrl-adi2: Use a signed return type for adi_gpio_irq_startup()

[Dan Carpenter]

On Sat, Dec 19, 2015 at 09:40:27PM +0100, SF Markus Elfring wrote:
> > This introduces a compile warning.
> 
> How do you think about to show the exact message you get?
> 

I can't actually compile it myself.

> 
> > These functions are supposed to return 1 if there is an IRQ pending.
> > Change the -EINVAL to 0.
> 
> Is there any more source code clean-up needed around the comment "FIXME"
> in the affected function?

Delete the FIXME comment.

regards,
dan carpenter

Re: pinctrl-adi2: Use a signed return type for adi_gpio_irq_startup()

[Dan Carpenter]

On Sat, Dec 19, 2015 at 09:40:27PM +0100, SF Markus Elfring wrote:
> > This introduces a compile warning.
> 
> How do you think about to show the exact message you get?
> 

I can't actually compile it myself.

> 
> > These functions are supposed to return 1 if there is an IRQ pending.
> > Change the -EINVAL to 0.
> 
> Is there any more source code clean-up needed around the comment "FIXME"
> in the affected function?

Delete the FIXME comment.

regards,
dan carpenter

Re: [PATCH] pinctrl-adi2: Use a signed return type for adi_gpio_irq_startup()

[kbuild test robot]

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

Hi Markus,

[auto build test WARNING on pinctrl/for-next]
[also build test WARNING on v4.4-rc5 next-20151218]

url:    https://github.com/0day-ci/linux/commits/SF-Markus-Elfring/pinctrl-adi2-Use-a-signed-return-type-for-adi_gpio_irq_startup/20151220-010253
base:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git for-next
config: blackfin-CM-BF548_defconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=blackfin 

All warnings (new ones prefixed by >>):

>> drivers/pinctrl/pinctrl-adi2.c:584:2: warning: initialization from incompatible pointer type [enabled by default]
   drivers/pinctrl/pinctrl-adi2.c:584:2: warning: (near initialization for 'adi_gpio_irqchip.irq_startup') [enabled by default]

vim +584 drivers/pinctrl/pinctrl-adi2.c

e9a03add Sonic Zhang 2013-09-03  568  		request >>= 1;
e9a03add Sonic Zhang 2013-09-03  569  	}
e9a03add Sonic Zhang 2013-09-03  570  
e9a03add Sonic Zhang 2013-09-03  571  	if (!umask)
e9a03add Sonic Zhang 2013-09-03  572  		chained_irq_exit(chip, desc);
e9a03add Sonic Zhang 2013-09-03  573  }
e9a03add Sonic Zhang 2013-09-03  574  
e9a03add Sonic Zhang 2013-09-03  575  static struct irq_chip adi_gpio_irqchip = {
e9a03add Sonic Zhang 2013-09-03  576  	.name = "GPIO",
e9a03add Sonic Zhang 2013-09-03  577  	.irq_ack = adi_gpio_ack_irq,
e9a03add Sonic Zhang 2013-09-03  578  	.irq_mask = adi_gpio_mask_irq,
e9a03add Sonic Zhang 2013-09-03  579  	.irq_mask_ack = adi_gpio_mask_ack_irq,
e9a03add Sonic Zhang 2013-09-03  580  	.irq_unmask = adi_gpio_unmask_irq,
e9a03add Sonic Zhang 2013-09-03  581  	.irq_disable = adi_gpio_mask_irq,
e9a03add Sonic Zhang 2013-09-03  582  	.irq_enable = adi_gpio_unmask_irq,
e9a03add Sonic Zhang 2013-09-03  583  	.irq_set_type = adi_gpio_irq_type,
e9a03add Sonic Zhang 2013-09-03 @584  	.irq_startup = adi_gpio_irq_startup,
e9a03add Sonic Zhang 2013-09-03  585  	.irq_shutdown = adi_gpio_irq_shutdown,
e9a03add Sonic Zhang 2013-09-03  586  	.irq_set_wake = adi_gpio_set_wake,
e9a03add Sonic Zhang 2013-09-03  587  };
e9a03add Sonic Zhang 2013-09-03  588  
e9a03add Sonic Zhang 2013-09-03  589  static int adi_get_groups_count(struct pinctrl_dev *pctldev)
e9a03add Sonic Zhang 2013-09-03  590  {
e9a03add Sonic Zhang 2013-09-03  591  	struct adi_pinctrl *pinctrl = pinctrl_dev_get_drvdata(pctldev);
e9a03add Sonic Zhang 2013-09-03  592  

:::::: The code at line 584 was first introduced by commit
:::::: e9a03add0c6ed5341fc59ff9c76843c2888a33fa pinctrl: ADI PIN control driver for the GPIO controller on bf54x and bf60x.

:::::: TO: Sonic Zhang <sonic.zhang@analog.com>
:::::: CC: Linus Walleij <linus.walleij@linaro.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 13805 bytes --]

Re: [PATCH] pinctrl-adi2: Use a signed return type for adi_gpio_irq_startup()

[kbuild test robot]

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

Hi Markus,

[auto build test WARNING on pinctrl/for-next]
[also build test WARNING on v4.4-rc5 next-20151218]

url:    https://github.com/0day-ci/linux/commits/SF-Markus-Elfring/pinctrl-adi2-Use-a-signed-return-type-for-adi_gpio_irq_startup/20151220-010253
base:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git for-next
config: blackfin-CM-BF548_defconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=blackfin 

All warnings (new ones prefixed by >>):

>> drivers/pinctrl/pinctrl-adi2.c:584:2: warning: initialization from incompatible pointer type [enabled by default]
   drivers/pinctrl/pinctrl-adi2.c:584:2: warning: (near initialization for 'adi_gpio_irqchip.irq_startup') [enabled by default]

vim +584 drivers/pinctrl/pinctrl-adi2.c

e9a03add Sonic Zhang 2013-09-03  568  		request >>= 1;
e9a03add Sonic Zhang 2013-09-03  569  	}
e9a03add Sonic Zhang 2013-09-03  570  
e9a03add Sonic Zhang 2013-09-03  571  	if (!umask)
e9a03add Sonic Zhang 2013-09-03  572  		chained_irq_exit(chip, desc);
e9a03add Sonic Zhang 2013-09-03  573  }
e9a03add Sonic Zhang 2013-09-03  574  
e9a03add Sonic Zhang 2013-09-03  575  static struct irq_chip adi_gpio_irqchip = {
e9a03add Sonic Zhang 2013-09-03  576  	.name = "GPIO",
e9a03add Sonic Zhang 2013-09-03  577  	.irq_ack = adi_gpio_ack_irq,
e9a03add Sonic Zhang 2013-09-03  578  	.irq_mask = adi_gpio_mask_irq,
e9a03add Sonic Zhang 2013-09-03  579  	.irq_mask_ack = adi_gpio_mask_ack_irq,
e9a03add Sonic Zhang 2013-09-03  580  	.irq_unmask = adi_gpio_unmask_irq,
e9a03add Sonic Zhang 2013-09-03  581  	.irq_disable = adi_gpio_mask_irq,
e9a03add Sonic Zhang 2013-09-03  582  	.irq_enable = adi_gpio_unmask_irq,
e9a03add Sonic Zhang 2013-09-03  583  	.irq_set_type = adi_gpio_irq_type,
e9a03add Sonic Zhang 2013-09-03 @584  	.irq_startup = adi_gpio_irq_startup,
e9a03add Sonic Zhang 2013-09-03  585  	.irq_shutdown = adi_gpio_irq_shutdown,
e9a03add Sonic Zhang 2013-09-03  586  	.irq_set_wake = adi_gpio_set_wake,
e9a03add Sonic Zhang 2013-09-03  587  };
e9a03add Sonic Zhang 2013-09-03  588  
e9a03add Sonic Zhang 2013-09-03  589  static int adi_get_groups_count(struct pinctrl_dev *pctldev)
e9a03add Sonic Zhang 2013-09-03  590  {
e9a03add Sonic Zhang 2013-09-03  591  	struct adi_pinctrl *pinctrl = pinctrl_dev_get_drvdata(pctldev);
e9a03add Sonic Zhang 2013-09-03  592  

:::::: The code at line 584 was first introduced by commit
:::::: e9a03add0c6ed5341fc59ff9c76843c2888a33fa pinctrl: ADI PIN control driver for the GPIO controller on bf54x and bf60x.

:::::: TO: Sonic Zhang <sonic.zhang@analog.com>
:::::: CC: Linus Walleij <linus.walleij@linaro.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 13805 bytes --]

Re: pinctrl-adi2: Use a signed return type for adi_gpio_irq_startup()

[SF Markus Elfring]

>>> This introduces a compile warning.
>>
>> How do you think about to show the exact message you get?
> 
> I can't actually compile it myself.

It seems that I can understand your feedback also a bit better
since I received the information from a background process
like "kbuild test robot".

Will it become acceptable to adjust the data structure "irq_chip"
for the variable "adi_gpio_irqchip"?

Should I just skip my update suggestion for this specific source
code area?
http://lxr.free-electrons.com/source/include/linux/irq.h?v=4.3#L320

Regards,
Markus

Re: pinctrl-adi2: Use a signed return type for adi_gpio_irq_startup()

[SF Markus Elfring]

>>> This introduces a compile warning.
>>
>> How do you think about to show the exact message you get?
> 
> I can't actually compile it myself.

It seems that I can understand your feedback also a bit better
since I received the information from a background process
like "kbuild test robot".

Will it become acceptable to adjust the data structure "irq_chip"
for the variable "adi_gpio_irqchip"?

Should I just skip my update suggestion for this specific source
code area?
http://lxr.free-electrons.com/source/include/linux/irq.h?v=4.3#L320

Regards,
Markus