[PATCH 0/7] irq: fix checkpatch errors and warnings

[PATCH 0/7] irq: fix checkpatch errors and warnings

[Kefeng Wang]

Fix all the checkpath errors in kernel/irq dir, and some warnings
also fixed.

Kefeng Wang (7):
  irq: fix checkpatch warnings
  irq: fix checkpatch error
  irq: fix checkpatch error
  irq: fix all checkpatch errors and warnings
  irq: fix all checkpatch errors and warnings
  irq: fix checkpatch warnings
  irq: fix all checkpatch errors and warnings

 kernel/irq/chip.c      |  6 ++++--
 kernel/irq/handle.c    |  7 ++++---
 kernel/irq/irqdesc.c   | 15 ++++++++++-----
 kernel/irq/irqdomain.c |  6 +++---
 kernel/irq/manage.c    | 31 +++++++++++++++++++------------
 kernel/irq/proc.c      | 11 ++++++-----
 kernel/irq/spurious.c  | 33 ++++++++++++++++-----------------
 7 files changed, 62 insertions(+), 47 deletions(-)

-- 
1.8.2.1

[PATCH 1/7] irq: fix checkpatch warnings

[Kefeng Wang]

Fixes line over 80 characters warnings.

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 kernel/irq/chip.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index cbd97ce..f19865a 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -56,7 +56,8 @@ EXPORT_SYMBOL(irq_set_chip);
 int irq_set_irq_type(unsigned int irq, unsigned int type)
 {
 	unsigned long flags;
-	struct irq_desc *desc = irq_get_desc_buslock(irq, &flags, IRQ_GET_DESC_CHECK_GLOBAL);
+	struct irq_desc *desc = irq_get_desc_buslock(irq, &flags,
+					IRQ_GET_DESC_CHECK_GLOBAL);
 	int ret = 0;
 
 	if (!desc)
@@ -101,7 +102,8 @@ int irq_set_msi_desc_off(unsigned int irq_base, unsigned int irq_offset,
 			 struct msi_desc *entry)
 {
 	unsigned long flags;
-	struct irq_desc *desc = irq_get_desc_lock(irq_base + irq_offset, &flags, IRQ_GET_DESC_CHECK_GLOBAL);
+	struct irq_desc *desc = irq_get_desc_lock(irq_base + irq_offset, &flags,
+						IRQ_GET_DESC_CHECK_GLOBAL);
 
 	if (!desc)
 		return -EINVAL;
-- 
1.8.2.1

[PATCH 2/7] irq: fix checkpatch error

[Kefeng Wang]

Change printk(KERN_WARNING ... to pr_warn(...
Fix ERROR: space required after that ',' (ctx:VxV)

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 kernel/irq/handle.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/kernel/irq/handle.c b/kernel/irq/handle.c
index 131ca17..07a1c15 100644
--- a/kernel/irq/handle.c
+++ b/kernel/irq/handle.c
@@ -47,7 +47,7 @@ static void warn_no_thread(unsigned int irq, struct irqaction *action)
 	if (test_and_set_bit(IRQTF_WARNED, &action->thread_flags))
 		return;
 
-	printk(KERN_WARNING "IRQ %d device %s returned IRQ_WAKE_THREAD "
+	pr_warn("IRQ %d device %s returned IRQ_WAKE_THREAD "
 	       "but no thread function available.", irq, action->name);
 }
 
@@ -142,8 +142,9 @@ handle_irq_event_percpu(struct irq_desc *desc, struct irqaction *action)
 		res = action->handler(irq, action->dev_id);
 		trace_irq_handler_exit(irq, action, res);
 
-		if (WARN_ONCE(!irqs_disabled(),"irq %u handler %pF enabled interrupts\n",
-			      irq, action->handler))
+		if (WARN_ONCE(!irqs_disabled(),
+				"irq %u handler %pF enabled interrupts\n",
+				irq, action->handler))
 			local_irq_disable();
 
 		switch (res) {
-- 
1.8.2.1

[PATCH 3/7] irq: fix checkpatch error

[Kefeng Wang]

ERROR: space required before the open parenthesis '('
WARNING: Prefer pr_warn(... to pr_warning(...
Just fix above 2 issue.

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 kernel/irq/irqdomain.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 5a83dde..fd47f33 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -419,7 +419,7 @@ static void irq_domain_disassociate_many(struct irq_domain *domain,
 		irq_data->hwirq = 0;
 
 		/* Clear reverse map */
-		switch(domain->revmap_type) {
+		switch (domain->revmap_type) {
 		case IRQ_DOMAIN_MAP_LINEAR:
 			if (hwirq < domain->revmap_data.linear.size)
 				domain->revmap_data.linear.revmap[hwirq] = 0;
@@ -570,7 +570,7 @@ unsigned int irq_create_mapping(struct irq_domain *domain,
 	if (domain == NULL)
 		domain = irq_default_domain;
 	if (domain == NULL) {
-		pr_warning("irq_create_mapping called for"
+		pr_warn("irq_create_mapping called for"
 			   " NULL domain, hwirq=%lx\n", hwirq);
 		WARN_ON(1);
 		return 0;
@@ -672,7 +672,7 @@ unsigned int irq_create_of_mapping(struct device_node *controller,
 		if (intsize > 0)
 			return intspec[0];
 #endif
-		pr_warning("no irq domain found for %s !\n",
+		pr_warn("no irq domain found for %s !\n",
 			   of_node_full_name(controller));
 		return 0;
 	}
-- 
1.8.2.1

[PATCH 4/7] irq: fix all checkpatch errors and warnings

[Kefeng Wang]

Fixes following errors and warnings:
WARNING: line over 80 characters
ERROR: spaces required around that '=' (ctx:WxV)
WARNING: Avoid unnecessary line continuations
WARNING: Prefer pr_warn(... to pr_warning(...
WARNING: suspect code indent for conditional statements (8, 12)

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 kernel/irq/manage.c | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index fa17855..0d6446e 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -211,7 +211,8 @@ int irq_set_affinity(unsigned int irq, const struct cpumask *mask)
 int irq_set_affinity_hint(unsigned int irq, const struct cpumask *m)
 {
 	unsigned long flags;
-	struct irq_desc *desc = irq_get_desc_lock(irq, &flags, IRQ_GET_DESC_CHECK_GLOBAL);
+	struct irq_desc *desc = irq_get_desc_lock(irq, &flags,
+					IRQ_GET_DESC_CHECK_GLOBAL);
 
 	if (!desc)
 		return -EINVAL;
@@ -372,7 +373,8 @@ void __disable_irq(struct irq_desc *desc, unsigned int irq, bool suspend)
 static int __disable_irq_nosync(unsigned int irq)
 {
 	unsigned long flags;
-	struct irq_desc *desc = irq_get_desc_buslock(irq, &flags, IRQ_GET_DESC_CHECK_GLOBAL);
+	struct irq_desc *desc = irq_get_desc_buslock(irq, &flags,
+					IRQ_GET_DESC_CHECK_GLOBAL);
 
 	if (!desc)
 		return -EINVAL;
@@ -464,7 +466,8 @@ void __enable_irq(struct irq_desc *desc, unsigned int irq, bool resume)
 void enable_irq(unsigned int irq)
 {
 	unsigned long flags;
-	struct irq_desc *desc = irq_get_desc_buslock(irq, &flags, IRQ_GET_DESC_CHECK_GLOBAL);
+	struct irq_desc *desc = irq_get_desc_buslock(irq, &flags,
+					IRQ_GET_DESC_CHECK_GLOBAL);
 
 	if (!desc)
 		return;
@@ -507,7 +510,8 @@ static int set_irq_wake_real(unsigned int irq, unsigned int on)
 int irq_set_irq_wake(unsigned int irq, unsigned int on)
 {
 	unsigned long flags;
-	struct irq_desc *desc = irq_get_desc_buslock(irq, &flags, IRQ_GET_DESC_CHECK_GLOBAL);
+	struct irq_desc *desc = irq_get_desc_buslock(irq, &flags,
+					IRQ_GET_DESC_CHECK_GLOBAL);
 	int ret = 0;
 
 	if (!desc)
@@ -557,7 +561,7 @@ int can_request_irq(unsigned int irq, unsigned long irqflags)
 	if (irq_settings_can_request(desc)) {
 		if (desc->action)
 			if (irqflags & desc->action->flags & IRQF_SHARED)
-				canrequest =1;
+				canrequest = 1;
 	}
 	irq_put_desc_unlock(desc, flags);
 	return canrequest;
@@ -1102,7 +1106,7 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new)
 				goto out_mask;
 		}
 
-		desc->istate &= ~(IRQS_AUTODETECT | IRQS_SPURIOUS_DISABLED | \
+		desc->istate &= ~(IRQS_AUTODETECT | IRQS_SPURIOUS_DISABLED |
 				  IRQS_ONESHOT | IRQS_WAITING);
 		irqd_clear(&desc->irq_data, IRQD_IRQ_INPROGRESS);
 
@@ -1135,7 +1139,7 @@ __setup_irq(unsigned int irq, struct irq_desc *desc, struct irqaction *new)
 
 		if (nmsk != omsk)
 			/* hope the handler works with current  trigger mode */
-			pr_warning("irq %d uses trigger mode %u; requested %u\n",
+			pr_warn("irq %d uses trigger mode %u; requested %u\n",
 				   irq, nmsk, omsk);
 	}
 
@@ -1314,7 +1318,7 @@ void remove_irq(unsigned int irq, struct irqaction *act)
 	struct irq_desc *desc = irq_to_desc(irq);
 
 	if (desc && !WARN_ON(irq_settings_is_per_cpu_devid(desc)))
-	    __free_irq(irq, act->dev_id);
+		__free_irq(irq, act->dev_id);
 }
 EXPORT_SYMBOL_GPL(remove_irq);
 
@@ -1504,7 +1508,8 @@ void enable_percpu_irq(unsigned int irq, unsigned int type)
 {
 	unsigned int cpu = smp_processor_id();
 	unsigned long flags;
-	struct irq_desc *desc = irq_get_desc_lock(irq, &flags, IRQ_GET_DESC_CHECK_PERCPU);
+	struct irq_desc *desc = irq_get_desc_lock(irq, &flags,
+					IRQ_GET_DESC_CHECK_PERCPU);
 
 	if (!desc)
 		return;
@@ -1531,7 +1536,8 @@ void disable_percpu_irq(unsigned int irq)
 {
 	unsigned int cpu = smp_processor_id();
 	unsigned long flags;
-	struct irq_desc *desc = irq_get_desc_lock(irq, &flags, IRQ_GET_DESC_CHECK_PERCPU);
+	struct irq_desc *desc = irq_get_desc_lock(irq, &flags,
+					IRQ_GET_DESC_CHECK_PERCPU);
 
 	if (!desc)
 		return;
@@ -1544,7 +1550,8 @@ EXPORT_SYMBOL_GPL(disable_percpu_irq);
 /*
  * Internal function to unregister a percpu irqaction.
  */
-static struct irqaction *__free_percpu_irq(unsigned int irq, void __percpu *dev_id)
+static struct irqaction *
+__free_percpu_irq(unsigned int irq, void __percpu *dev_id)
 {
 	struct irq_desc *desc = irq_to_desc(irq);
 	struct irqaction *action;
@@ -1596,7 +1603,7 @@ void remove_percpu_irq(unsigned int irq, struct irqaction *act)
 	struct irq_desc *desc = irq_to_desc(irq);
 
 	if (desc && irq_settings_is_per_cpu_devid(desc))
-	    __free_percpu_irq(irq, act->percpu_dev_id);
+		__free_percpu_irq(irq, act->percpu_dev_id);
 }
 
 /**
-- 
1.8.2.1

[PATCH 5/7] irq: fix all checkpatch errors and warnings

[Kefeng Wang]

Fix following errors and warnings:
WARNING: space prohibited before semicolon
ERROR: space prohibited before open square bracket '['
WARNING: line over 80 characters

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 kernel/irq/proc.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/kernel/irq/proc.c b/kernel/irq/proc.c
index 19ed5c4..291e383 100644
--- a/kernel/irq/proc.c
+++ b/kernel/irq/proc.c
@@ -276,7 +276,7 @@ static int name_unique(unsigned int irq, struct irqaction *new_action)
 	int ret = 1;
 
 	raw_spin_lock_irqsave(&desc->lock, flags);
-	for (action = desc->action ; action; action = action->next) {
+	for (action = desc->action; action; action = action->next) {
 		if ((action != new_action) && action->name &&
 				!strcmp(new_action->name, action->name)) {
 			ret = 0;
@@ -289,7 +289,7 @@ static int name_unique(unsigned int irq, struct irqaction *new_action)
 
 void register_handler_proc(unsigned int irq, struct irqaction *action)
 {
-	char name [MAX_NAMELEN];
+	char name[MAX_NAMELEN];
 	struct irq_desc *desc = irq_to_desc(irq);
 
 	if (!desc->dir || action->dir || !action->name ||
@@ -309,7 +309,7 @@ void register_handler_proc(unsigned int irq, struct irqaction *action)
 
 void register_irq_proc(unsigned int irq, struct irq_desc *desc)
 {
-	char name [MAX_NAMELEN];
+	char name[MAX_NAMELEN];
 
 	if (!root_irq_dir || (desc->irq_data.chip == &no_irq_chip) || desc->dir)
 		return;
@@ -345,7 +345,7 @@ void register_irq_proc(unsigned int irq, struct irq_desc *desc)
 
 void unregister_irq_proc(unsigned int irq, struct irq_desc *desc)
 {
-	char name [MAX_NAMELEN];
+	char name[MAX_NAMELEN];
 
 	if (!root_irq_dir || !desc->dir)
 		return;
@@ -463,7 +463,8 @@ int show_interrupts(struct seq_file *p, void *v)
 		seq_printf(p, " %8s", "None");
 	}
 #ifdef CONFIG_GENERIC_IRQ_SHOW_LEVEL
-	seq_printf(p, " %-8s", irqd_is_level_type(&desc->irq_data) ? "Level" : "Edge");
+	seq_printf(p, " %-8s",
+			irqd_is_level_type(&desc->irq_data) ? "Level" : "Edge");
 #endif
 	if (desc->name)
 		seq_printf(p, "-%-8s", desc->name);
-- 
1.8.2.1

[PATCH 6/7] irq: fix checkpatch warnings

[Kefeng Wang]

Fix line over and code indent warning, and use pr_foo().

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 kernel/irq/spurious.c | 33 ++++++++++++++++-----------------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/kernel/irq/spurious.c b/kernel/irq/spurious.c
index 7b5f012..84cf547 100644
--- a/kernel/irq/spurious.c
+++ b/kernel/irq/spurious.c
@@ -68,7 +68,8 @@ static int try_one_irq(int irq, struct irq_desc *desc, bool force)
 	raw_spin_lock(&desc->lock);
 
 	/* PER_CPU and nested thread interrupts are never polled */
-	if (irq_settings_is_per_cpu(desc) || irq_settings_is_nested_thread(desc))
+	if (irq_settings_is_per_cpu(desc) ||
+			irq_settings_is_nested_thread(desc))
 		goto out;
 
 	/*
@@ -123,7 +124,7 @@ static int misrouted_irq(int irq)
 
 	for_each_irq_desc(i, desc) {
 		if (!i)
-			 continue;
+			continue;
 
 		if (i == irq)	/* Already tried */
 			continue;
@@ -150,7 +151,7 @@ static void poll_spurious_irqs(unsigned long dummy)
 		unsigned int state;
 
 		if (!i)
-			 continue;
+			continue;
 
 		/* Racy but it doesn't matter */
 		state = desc->istate;
@@ -191,14 +192,14 @@ __report_bad_irq(unsigned int irq, struct irq_desc *desc,
 	unsigned long flags;
 
 	if (bad_action_ret(action_ret)) {
-		printk(KERN_ERR "irq event %d: bogus return value %x\n",
+		pr_err("irq event %d: bogus return value %x\n",
 				irq, action_ret);
 	} else {
-		printk(KERN_ERR "irq %d: nobody cared (try booting with "
+		pr_err("irq %d: nobody cared (try booting with "
 				"the \"irqpoll\" option)\n", irq);
 	}
 	dump_stack();
-	printk(KERN_ERR "handlers:\n");
+	pr_err("handlers:\n");
 
 	/*
 	 * We need to take desc->lock here. note_interrupt() is called
@@ -209,11 +210,11 @@ __report_bad_irq(unsigned int irq, struct irq_desc *desc,
 	raw_spin_lock_irqsave(&desc->lock, flags);
 	action = desc->action;
 	while (action) {
-		printk(KERN_ERR "[<%p>] %pf", action->handler, action->handler);
+		pr_err("[<%p>] %pf", action->handler, action->handler);
 		if (action->thread_fn)
-			printk(KERN_CONT " threaded [<%p>] %pf",
+			pr_cont(" threaded [<%p>] %pf",
 					action->thread_fn, action->thread_fn);
-		printk(KERN_CONT "\n");
+		pr_cont("\n");
 		action = action->next;
 	}
 	raw_spin_unlock_irqrestore(&desc->lock, flags);
@@ -313,7 +314,7 @@ void note_interrupt(unsigned int irq, struct irq_desc *desc,
 		/*
 		 * Now kill the IRQ
 		 */
-		printk(KERN_EMERG "Disabling IRQ #%d\n", irq);
+		pr_emerg("Disabling IRQ #%d\n", irq);
 		desc->istate |= IRQS_SPURIOUS_DISABLED;
 		desc->depth++;
 		irq_disable(desc);
@@ -329,7 +330,7 @@ bool noirqdebug __read_mostly;
 int noirqdebug_setup(char *str)
 {
 	noirqdebug = 1;
-	printk(KERN_INFO "IRQ lockup detection disabled\n");
+	pr_info("IRQ lockup detection disabled\n");
 
 	return 1;
 }
@@ -341,8 +342,8 @@ MODULE_PARM_DESC(noirqdebug, "Disable irq lockup detection when true");
 static int __init irqfixup_setup(char *str)
 {
 	irqfixup = 1;
-	printk(KERN_WARNING "Misrouted IRQ fixup support enabled.\n");
-	printk(KERN_WARNING "This may impact system performance.\n");
+	pr_warn("Misrouted IRQ fixup support enabled.\n");
+	pr_warn("This may impact system performance.\n");
 
 	return 1;
 }
@@ -353,10 +354,8 @@ module_param(irqfixup, int, 0644);
 static int __init irqpoll_setup(char *str)
 {
 	irqfixup = 2;
-	printk(KERN_WARNING "Misrouted IRQ fixup and polling support "
-				"enabled\n");
-	printk(KERN_WARNING "This may significantly impact system "
-				"performance\n");
+	pr_warn("Misrouted IRQ fixup and polling support enabled\n");
+	pr_warn("This may significantly impact system performance\n");
 	return 1;
 }
 
-- 
1.8.2.1

[PATCH 7/7] irq: fix all checkpatch errors and warnings

[Kefeng Wang]

Fix following issue, and use pr_foo().
WARNING: line over 80 characters
ERROR: spaces required around that '>=' (ctx:WxV)

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 kernel/irq/irqdesc.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
index 192a302..9ba01f5 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -7,6 +7,9 @@
  * Detailed information is available in Documentation/DocBook/genericirq
  *
  */
+
+#define pr_fmt(fmt)  "NR_IRQS: " fmt
+
 #include <linux/irq.h>
 #include <linux/slab.h>
 #include <linux/export.h>
@@ -219,7 +222,7 @@ int __init early_irq_init(void)
 
 	/* Let arch update nr_irqs and return the nr of preallocated irqs */
 	initcnt = arch_probe_nr_irqs();
-	printk(KERN_INFO "NR_IRQS:%d nr_irqs:%d %d\n", NR_IRQS, nr_irqs, initcnt);
+	pr_info("%d nr_irqs:%d %d\n", NR_IRQS, nr_irqs, initcnt);
 
 	if (WARN_ON(nr_irqs > IRQ_BITMAP_BITS))
 		nr_irqs = IRQ_BITMAP_BITS;
@@ -255,7 +258,7 @@ int __init early_irq_init(void)
 
 	init_irq_default_affinity();
 
-	printk(KERN_INFO "NR_IRQS:%d\n", NR_IRQS);
+	pr_info("%d\n", NR_IRQS);
 
 	desc = irq_desc;
 	count = ARRAY_SIZE(irq_desc);
@@ -369,7 +372,7 @@ __irq_alloc_descs(int irq, unsigned int from, unsigned int cnt, int node,
 	start = bitmap_find_next_zero_area(allocated_irqs, IRQ_BITMAP_BITS,
 					   from, cnt, 0);
 	ret = -EEXIST;
-	if (irq >=0 && start != irq)
+	if (irq >= 0 && start != irq)
 		goto err;
 
 	if (start + cnt > nr_irqs) {
@@ -404,7 +407,8 @@ int irq_reserve_irqs(unsigned int from, unsigned int cnt)
 		return -EINVAL;
 
 	mutex_lock(&sparse_irq_lock);
-	start = bitmap_find_next_zero_area(allocated_irqs, nr_irqs, from, cnt, 0);
+	start = bitmap_find_next_zero_area(allocated_irqs, nr_irqs,
+						from, cnt, 0);
 	if (start == from)
 		bitmap_set(allocated_irqs, start, cnt);
 	else
@@ -465,7 +469,8 @@ int irq_set_percpu_devid(unsigned int irq)
 	if (desc->percpu_enabled)
 		return -EINVAL;
 
-	desc->percpu_enabled = kzalloc(sizeof(*desc->percpu_enabled), GFP_KERNEL);
+	desc->percpu_enabled = kzalloc(sizeof(*desc->percpu_enabled),
+					GFP_KERNEL);
 
 	if (!desc->percpu_enabled)
 		return -ENOMEM;
-- 
1.8.2.1

Re: [PATCH 0/7] irq: fix checkpatch errors and warnings

[Thomas Gleixner]

On Thu, 6 Jun 2013, Kefeng Wang wrote:

> Fix all the checkpath errors in kernel/irq dir, and some warnings
> also fixed.

Sorry, I'm not really interested in this kind of patches. To be
honest, it would be way more exciting if you had taught checkpatch to
actually fix the missing space after the comma.

Aside of that your mechanical fixups are mostly making the code worse
to read. Just a few examples:

--- linux-2.6.orig/kernel/irq/chip.c
+++ linux-2.6/kernel/irq/chip.c
@@ -56,7 +56,8 @@ EXPORT_SYMBOL(irq_set_chip);
 int irq_set_irq_type(unsigned int irq, unsigned int type)
 {
 	unsigned long flags;
-	struct irq_desc *desc = irq_get_desc_buslock(irq, &flags, IRQ_GET_DESC_CHECK_GLOBAL);
+	struct irq_desc *desc = irq_get_desc_buslock(irq, &flags,
+					IRQ_GET_DESC_CHECK_GLOBAL);

This is horrible to parse. If we enforce the 80 character limit here
at all, which I doubt it has a value for this particular case, then
please adhere to the coding style used in this file and make it 

	struct irq_desc *desc = irq_get_desc_buslock(irq, &flags,
						     IRQ_GET_DESC_CHECK_GLOBAL);

Aligning the first arguments of the first and the second line makes it
way simpler to read.

Aside of that, there is no real value to make changes like this:

--- linux-2.6.orig/kernel/irq/irqdomain.c
+++ linux-2.6/kernel/irq/irqdomain.c
@@ -419,7 +419,7 @@ static void irq_domain_disassociate_many
 		irq_data->hwirq = 0;
 
 		/* Clear reverse map */
-		switch(domain->revmap_type) {
+		switch (domain->revmap_type) {

And this:

--- linux-2.6.orig/kernel/irq/proc.c
+++ linux-2.6/kernel/irq/proc.c
@@ -276,7 +276,7 @@ static int name_unique(unsigned int irq,
 	int ret = 1;
 
 	raw_spin_lock_irqsave(&desc->lock, flags);
-	for (action = desc->action ; action; action = action->next) {
+	for (action = desc->action; action; action = action->next) {

Now there is another category:

@@ -47,7 +47,7 @@ static void warn_no_thread(unsigned int 
 	if (test_and_set_bit(IRQTF_WARNED, &action->thread_flags))
 		return;
 
-	printk(KERN_WARNING "IRQ %d device %s returned IRQ_WAKE_THREAD "
+	pr_warn("IRQ %d device %s returned IRQ_WAKE_THREAD "
 	       "but no thread function available.", irq, action->name);

The checkpatch warning is to prevent new code to use the old style
printk format, but it's not intended to force that on existing code.

We can write a coccinelle script to fix such issues in one go all over
the kernel source tree. Though I doubt that it has much value.

It does not make the source better. It does not fix any bugs. It's
just pointless entries in the changelogs

Please sit down and read and understand the code and try to find real
issues which cannot be detected and solved by scripts and
tools. That's what's kernel hacking is about. You are not becoming a
kernel developer by running tools and blindly fixing the complaints of
the tools. You have to understand the code and you have to learn to
judge the output of tools.

Thanks,

	tglx

Re: [PATCH 0/7] irq: fix checkpatch errors and warnings

[Joe Perches]

On Fri, 2013-06-07 at 22:42 +0200, Thomas Gleixner wrote:
> On Thu, 6 Jun 2013, Kefeng Wang wrote:
> 
> > Fix all the checkpath errors in kernel/irq dir, and some warnings
> > also fixed.
> 
> Sorry, I'm not really interested in this kind of patches. To be
> honest, it would be way more exciting if you had taught checkpatch to
> actually fix the missing space after the comma.

Yup.  Kefeng, if you want to take that up,
I'd be happy to help/guide you.

> Aside of that your mechanical fixups are mostly making the code worse
> to read. Just a few examples:
> 
> --- linux-2.6.orig/kernel/irq/chip.c
[]
>         struct irq_desc *desc = irq_get_desc_buslock(irq, &flags,
>                                                      IRQ_GET_DESC_CHECK_GLOBAL);
> 
> Aligning the first arguments of the first and the second line makes it
> way simpler to read.

or maybe use

	struct irq_desc *desc =
		irq_get_desc_buslock(irq, &flags, IRQ_GET_DESC_CHECK_GLOBAL);

and satisfy 80 cols too.

Existing uses that exceed the 80 column text limit shouldn't
be changed without making other useful changes at the same
time.  Make 80 column changes as part of a larger patch set,

> --- linux-2.6.orig/kernel/irq/handle.c
[]
> @@ -47,7 +47,7 @@ static void warn_no_thread(unsigned int 

> -       printk(KERN_WARNING "IRQ %d device %s returned IRQ_WAKE_THREAD "
> +       pr_warn("IRQ %d device %s returned IRQ_WAKE_THREAD "
>                "but no thread function available.", irq, action->name);

It'd be useful to add a terminating \n though to avoid
interleaving by other thread pr_cont/naked printks.

> The checkpatch warning is to prevent new code to use the old style
> printk format, but it's not intended to force that on existing code.

Yup, and checkpatch will never be a perfect tool.

> Please sit down and read and understand the code and try to find real
> issues which cannot be detected and solved by scripts and
> tools. That's what's kernel hacking is about. You are not becoming a
> kernel developer by running tools and blindly fixing the complaints of
> the tools. You have to understand the code and you have to learn to
> judge the output of tools.

Quite right.

Maybe add something like it to SubmittingPatches in
Section 2 ala:

5) "Don't be a checkpatch monkey - How not to write patches"

Re: [PATCH 0/7] irq: fix checkpatch errors and warnings

[Kefeng Wang]

On 2013-06-08 6:24, Joe Perches wrote:
> On Fri, 2013-06-07 at 22:42 +0200, Thomas Gleixner wrote:
>> On Thu, 6 Jun 2013, Kefeng Wang wrote:
>>
>>> Fix all the checkpath errors in kernel/irq dir, and some warnings
>>> also fixed.
>>
>> Sorry, I'm not really interested in this kind of patches. To be
>> honest, it would be way more exciting if you had taught checkpatch to
>> actually fix the missing space after the comma.
> 
> Yup.  Kefeng, if you want to take that up,
> I'd be happy to help/guide you.
> 
>> Aside of that your mechanical fixups are mostly making the code worse
>> to read. Just a few examples:
>>
>> --- linux-2.6.orig/kernel/irq/chip.c
> []
>>         struct irq_desc *desc = irq_get_desc_buslock(irq, &flags,
>>                                                      IRQ_GET_DESC_CHECK_GLOBAL);
>>
>> Aligning the first arguments of the first and the second line makes it
>> way simpler to read.
> 
> or maybe use
> 
> 	struct irq_desc *desc =
> 		irq_get_desc_buslock(irq, &flags, IRQ_GET_DESC_CHECK_GLOBAL);
> 
> and satisfy 80 cols too.
> 
> Existing uses that exceed the 80 column text limit shouldn't
> be changed without making other useful changes at the same
> time.  Make 80 column changes as part of a larger patch set,
> 
>> --- linux-2.6.orig/kernel/irq/handle.c
> []
>> @@ -47,7 +47,7 @@ static void warn_no_thread(unsigned int 
> 
>> -       printk(KERN_WARNING "IRQ %d device %s returned IRQ_WAKE_THREAD "
>> +       pr_warn("IRQ %d device %s returned IRQ_WAKE_THREAD "
>>                "but no thread function available.", irq, action->name);
> 
> It'd be useful to add a terminating \n though to avoid
> interleaving by other thread pr_cont/naked printks.
> 
>> The checkpatch warning is to prevent new code to use the old style
>> printk format, but it's not intended to force that on existing code.
> 
> Yup, and checkpatch will never be a perfect tool.
> 
>> Please sit down and read and understand the code and try to find real
>> issues which cannot be detected and solved by scripts and
>> tools. That's what's kernel hacking is about. You are not becoming a
>> kernel developer by running tools and blindly fixing the complaints of
>> the tools. You have to understand the code and you have to learn to
>> judge the output of tools.
> 
> Quite right.
> 
> Maybe add something like it to SubmittingPatches in
> Section 2 ala:
> 
> 5) "Don't be a checkpatch monkey - How not to write patches"
> 

hi Thomas and Joe， thank very much for your reply and remind,
I will pay more attention to study kernel， like your said,
read more code and be an good kernel developer.

thanks,
kefeng


> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 
> .
> 

Re: [PATCH 3/7] irq: fix checkpatch error

[Grant Likely]

On Thu, 6 Jun 2013 19:20:27 +0800, Kefeng Wang <wangkefeng.wang@huawei.com> wrote:
> ERROR: space required before the open parenthesis '('
> WARNING: Prefer pr_warn(... to pr_warning(...
> Just fix above 2 issue.
> 
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>

Applied, thanks.

g.

> ---
>  kernel/irq/irqdomain.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index 5a83dde..fd47f33 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -419,7 +419,7 @@ static void irq_domain_disassociate_many(struct irq_domain *domain,
>  		irq_data->hwirq = 0;
>  
>  		/* Clear reverse map */
> -		switch(domain->revmap_type) {
> +		switch (domain->revmap_type) {
>  		case IRQ_DOMAIN_MAP_LINEAR:
>  			if (hwirq < domain->revmap_data.linear.size)
>  				domain->revmap_data.linear.revmap[hwirq] = 0;
> @@ -570,7 +570,7 @@ unsigned int irq_create_mapping(struct irq_domain *domain,
>  	if (domain == NULL)
>  		domain = irq_default_domain;
>  	if (domain == NULL) {
> -		pr_warning("irq_create_mapping called for"
> +		pr_warn("irq_create_mapping called for"
>  			   " NULL domain, hwirq=%lx\n", hwirq);
>  		WARN_ON(1);
>  		return 0;
> @@ -672,7 +672,7 @@ unsigned int irq_create_of_mapping(struct device_node *controller,
>  		if (intsize > 0)
>  			return intspec[0];
>  #endif
> -		pr_warning("no irq domain found for %s !\n",
> +		pr_warn("no irq domain found for %s !\n",
>  			   of_node_full_name(controller));
>  		return 0;
>  	}
> -- 
> 1.8.2.1
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

-- 
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.