Skip to contentSkip to conversation

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

PATCH In review SF Markus Elfring 3 participants 6 messages Dec 19, 2015

Subsystems (3)
Kernel janitors GPIO Pin control
lore
Share & tools
lore

SF Markus Elfring

author
#0 lore raw

PATCH 1/1 pinctrl-adi2: Use a signed return type for adi_gpio_irq_startup()

From: Markus Elfring <[email protected]>
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 <[email protected]>
---
 drivers/pinctrl/pinctrl-adi2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/pinctrl/pinctrl-adi2.c +1 -1
index fd342df..4032004 100644
@@ -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);
Signature · 1 line
2.6.3
#1 lore raw

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

author
#2 lore raw
Quoted 1 line

This introduces a compile warning.

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

Quoted 2 lines

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

#3 lore raw
Quoted 4 lines · SF Markus Elfring
Quoted 1 line

This introduces a compile warning.

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

I can't actually compile it myself.

Quoted 6 lines
Quoted 3 lines

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

build bot
#5 lore raw
In reply to SF Markus Elfring
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…

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 <[email protected]> :::::: CC: Linus Walleij <[email protected]>

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

author
#4 lore raw
In reply to Dan Carpenter
Quoted 5 lines
Quoted 3 lines
Quoted 1 line

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