[PATCH v2 1/2] irq: Add EXPORT_SYMBOL_GPL to function of irq generic-chip

[PATCH v2 1/2] irq: Add EXPORT_SYMBOL_GPL to function of irq generic-chip

[Nobuhiro Iwamatsu]

Some functions of irq generic-chip is undefined, because
EXPORT_SYMBOL_GPL is not set to these.

-----
ERROR: "irq_setup_generic_chip" [drivers/gpio/gpio-pch.ko] undefined!
ERROR: "irq_alloc_generic_chip" [drivers/gpio/gpio-pch.ko] undefined!
ERROR: "irq_setup_generic_chip" [drivers/gpio/gpio-ml-ioh.ko] undefined!
ERROR: "irq_alloc_generic_chip" [drivers/gpio/gpio-ml-ioh.ko] undefined!
-----

This is revised that EXPORT_SYMBOL_GPL can be added and referred
to in functions.

Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas.com>
---

V2: Add EXPORT_SYMBOL_GPL to irq_setup_generic_chip, irq_alloc_generic_chip,
irq_setup_alt_chip and irq_remove_generic_chip only.

 kernel/irq/generic-chip.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
index e38544d..6cb7613 100644
--- a/kernel/irq/generic-chip.c
+++ b/kernel/irq/generic-chip.c
@@ -211,6 +211,7 @@ irq_alloc_generic_chip(const char *name, int num_ct, unsigned int irq_base,
 	}
 	return gc;
 }
+EXPORT_SYMBOL_GPL(irq_alloc_generic_chip);
 
 /*
  * Separate lockdep class for interrupt chip which can nest irq_desc
@@ -258,6 +259,7 @@ void irq_setup_generic_chip(struct irq_chip_generic *gc, u32 msk,
 	}
 	gc->irq_cnt = i - gc->irq_base;
 }
+EXPORT_SYMBOL_GPL(irq_setup_generic_chip);
 
 /**
  * irq_setup_alt_chip - Switch to alternative chip
@@ -281,6 +283,7 @@ int irq_setup_alt_chip(struct irq_data *d, unsigned int type)
 	}
 	return -EINVAL;
 }
+EXPORT_SYMBOL_GPL(irq_setup_alt_chip);
 
 /**
  * irq_remove_generic_chip - Remove a chip
@@ -311,6 +314,7 @@ void irq_remove_generic_chip(struct irq_chip_generic *gc, u32 msk,
 		irq_modify_status(i, clr, set);
 	}
 }
+EXPORT_SYMBOL_GPL(irq_remove_generic_chip);
 
 #ifdef CONFIG_PM
 static int irq_gc_suspend(void)
-- 
1.7.7

[PATCH 2/2] irq: Add function pointer table for generic-chip

[Nobuhiro Iwamatsu]

This adds the function table to access it with function pointer
by some functions providedd in generic-chip.
The driver who uses the function offered in generic-chip can use
this via this function table.

Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas.com>
---
 include/linux/irq.h       |   27 ++++++++++++++++-----------
 kernel/irq/generic-chip.c |   31 +++++++++++++++++++++----------
 2 files changed, 37 insertions(+), 21 deletions(-)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index bff29c5..06ed817 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -638,6 +638,20 @@ struct irq_chip_type {
 	u32			type;
 };
 
+/* Generic chip callback functions table */
+struct irq_gc_functions {
+	void (* gc_noop)(struct irq_data *d);
+	void (* gc_mask_disable_reg)(struct irq_data *d);
+	void (* gc_mask_set_bit)(struct irq_data *d);
+	void (* gc_mask_clr_bit)(struct irq_data *d);
+	void (* gc_unmask_enable_reg)(struct irq_data *d);
+	void (* gc_ack_set_bit)(struct irq_data *d);
+	void (* gc_ack_clr_bit)(struct irq_data *d);
+	void (* gc_mask_disable_reg_and_ack)(struct irq_data *d);
+	void (* gc_eoi)(struct irq_data *d);
+	int (* gc_set_wake)(struct irq_data *d, unsigned int on);
+};
+
 /**
  * struct irq_chip_generic - Generic irq chip data structure
  * @lock:		Lock to protect register and cache data access
@@ -653,6 +667,7 @@ struct irq_chip_type {
  * @private:		Private data for non generic chip callbacks
  * @list:		List head for keeping track of instances
  * @chip_types:		Array of interrupt irq_chip_types
+ * @functions:		Generic chip callback functions
  *
  * Note, that irq_chip_generic can have multiple irq_chip_type
  * implementations which can be associated to a particular irq line of
@@ -674,6 +689,7 @@ struct irq_chip_generic {
 	void			*private;
 	struct list_head	list;
 	struct irq_chip_type	chip_types[0];
+	struct irq_gc_functions	functions;
 };
 
 /**
@@ -688,17 +704,6 @@ enum irq_gc_flags {
 	IRQ_GC_INIT_NESTED_LOCK		= 1 << 1,
 };
 
-/* Generic chip callback functions */
-void irq_gc_noop(struct irq_data *d);
-void irq_gc_mask_disable_reg(struct irq_data *d);
-void irq_gc_mask_set_bit(struct irq_data *d);
-void irq_gc_mask_clr_bit(struct irq_data *d);
-void irq_gc_unmask_enable_reg(struct irq_data *d);
-void irq_gc_ack_set_bit(struct irq_data *d);
-void irq_gc_ack_clr_bit(struct irq_data *d);
-void irq_gc_mask_disable_reg_and_ack(struct irq_data *d);
-void irq_gc_eoi(struct irq_data *d);
-int irq_gc_set_wake(struct irq_data *d, unsigned int on);
 
 /* Setup functions for irq_chip_generic */
 struct irq_chip_generic *
diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
index 6cb7613..8aec156 100644
--- a/kernel/irq/generic-chip.c
+++ b/kernel/irq/generic-chip.c
@@ -24,7 +24,7 @@ static inline struct irq_chip_regs *cur_regs(struct irq_data *d)
  * irq_gc_noop - NOOP function
  * @d: irq_data
  */
-void irq_gc_noop(struct irq_data *d)
+static void irq_gc_noop(struct irq_data *d)
 {
 }
 
@@ -35,7 +35,7 @@ void irq_gc_noop(struct irq_data *d)
  * Chip has separate enable/disable registers instead of a single mask
  * register.
  */
-void irq_gc_mask_disable_reg(struct irq_data *d)
+static void irq_gc_mask_disable_reg(struct irq_data *d)
 {
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
 	u32 mask = 1 << (d->irq - gc->irq_base);
@@ -53,7 +53,7 @@ void irq_gc_mask_disable_reg(struct irq_data *d)
  * Chip has a single mask register. Values of this register are cached
  * and protected by gc->lock
  */
-void irq_gc_mask_set_bit(struct irq_data *d)
+static void irq_gc_mask_set_bit(struct irq_data *d)
 {
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
 	u32 mask = 1 << (d->irq - gc->irq_base);
@@ -71,7 +71,7 @@ void irq_gc_mask_set_bit(struct irq_data *d)
  * Chip has a single mask register. Values of this register are cached
  * and protected by gc->lock
  */
-void irq_gc_mask_clr_bit(struct irq_data *d)
+static void irq_gc_mask_clr_bit(struct irq_data *d)
 {
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
 	u32 mask = 1 << (d->irq - gc->irq_base);
@@ -89,7 +89,7 @@ void irq_gc_mask_clr_bit(struct irq_data *d)
  * Chip has separate enable/disable registers instead of a single mask
  * register.
  */
-void irq_gc_unmask_enable_reg(struct irq_data *d)
+static void irq_gc_unmask_enable_reg(struct irq_data *d)
 {
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
 	u32 mask = 1 << (d->irq - gc->irq_base);
@@ -104,7 +104,7 @@ void irq_gc_unmask_enable_reg(struct irq_data *d)
  * irq_gc_ack_set_bit - Ack pending interrupt via setting bit
  * @d: irq_data
  */
-void irq_gc_ack_set_bit(struct irq_data *d)
+static void irq_gc_ack_set_bit(struct irq_data *d)
 {
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
 	u32 mask = 1 << (d->irq - gc->irq_base);
@@ -118,7 +118,7 @@ void irq_gc_ack_set_bit(struct irq_data *d)
  * irq_gc_ack_clr_bit - Ack pending interrupt via clearing bit
  * @d: irq_data
  */
-void irq_gc_ack_clr_bit(struct irq_data *d)
+static void irq_gc_ack_clr_bit(struct irq_data *d)
 {
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
 	u32 mask = ~(1 << (d->irq - gc->irq_base));
@@ -132,7 +132,7 @@ void irq_gc_ack_clr_bit(struct irq_data *d)
  * irq_gc_mask_disable_reg_and_ack- Mask and ack pending interrupt
  * @d: irq_data
  */
-void irq_gc_mask_disable_reg_and_ack(struct irq_data *d)
+static void irq_gc_mask_disable_reg_and_ack(struct irq_data *d)
 {
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
 	u32 mask = 1 << (d->irq - gc->irq_base);
@@ -147,7 +147,7 @@ void irq_gc_mask_disable_reg_and_ack(struct irq_data *d)
  * irq_gc_eoi - EOI interrupt
  * @d: irq_data
  */
-void irq_gc_eoi(struct irq_data *d)
+static void irq_gc_eoi(struct irq_data *d)
 {
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
 	u32 mask = 1 << (d->irq - gc->irq_base);
@@ -165,7 +165,7 @@ void irq_gc_eoi(struct irq_data *d)
  * configured in a separate register and the wakeup active state is
  * just stored in a bitmask.
  */
-int irq_gc_set_wake(struct irq_data *d, unsigned int on)
+static int irq_gc_set_wake(struct irq_data *d, unsigned int on)
 {
 	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
 	u32 mask = 1 << (d->irq - gc->irq_base);
@@ -208,6 +208,17 @@ irq_alloc_generic_chip(const char *name, int num_ct, unsigned int irq_base,
 		gc->reg_base = reg_base;
 		gc->chip_types->chip.name = name;
 		gc->chip_types->handler = handler;
+
+		gc->functions.gc_noop = irq_gc_noop;
+		gc->functions.gc_mask_disable_reg = irq_gc_mask_disable_reg;
+		gc->functions.gc_mask_set_bit = irq_gc_mask_set_bit;
+		gc->functions.gc_mask_clr_bit = irq_gc_mask_clr_bit;
+		gc->functions.gc_unmask_enable_reg = irq_gc_unmask_enable_reg;
+		gc->functions.gc_ack_set_bit = irq_gc_ack_set_bit;
+		gc->functions.gc_ack_clr_bit = irq_gc_ack_clr_bit;
+		gc->functions.gc_mask_disable_reg_and_ack = irq_gc_mask_disable_reg_and_ack;
+		gc->functions.gc_eoi = irq_gc_eoi;
+		gc->functions.gc_set_wake = irq_gc_set_wake;
 	}
 	return gc;
 }
-- 
1.7.7

Re: [PATCH 2/2] irq: Add function pointer table for generic-chip

[Thomas Gleixner]

On Mon, 17 Oct 2011, Nobuhiro Iwamatsu wrote:

> This adds the function table to access it with function pointer
> by some functions providedd in generic-chip.
> The driver who uses the function offered in generic-chip can use
> this via this function table.
>  
> -/* Generic chip callback functions */
> -void irq_gc_noop(struct irq_data *d);
> -void irq_gc_mask_disable_reg(struct irq_data *d);
> -void irq_gc_mask_set_bit(struct irq_data *d);
> -void irq_gc_mask_clr_bit(struct irq_data *d);
> -void irq_gc_unmask_enable_reg(struct irq_data *d);
> -void irq_gc_ack_set_bit(struct irq_data *d);
> -void irq_gc_ack_clr_bit(struct irq_data *d);
> -void irq_gc_mask_disable_reg_and_ack(struct irq_data *d);
> -void irq_gc_eoi(struct irq_data *d);
> -int irq_gc_set_wake(struct irq_data *d, unsigned int on);

This breaks the world and some more. We have code which references
those functions directly.
  
> +		gc->functions.gc_noop = irq_gc_noop;
> +		gc->functions.gc_mask_disable_reg = irq_gc_mask_disable_reg;
> +		gc->functions.gc_mask_set_bit = irq_gc_mask_set_bit;
> +		gc->functions.gc_mask_clr_bit = irq_gc_mask_clr_bit;
> +		gc->functions.gc_unmask_enable_reg = irq_gc_unmask_enable_reg;
> +		gc->functions.gc_ack_set_bit = irq_gc_ack_set_bit;
> +		gc->functions.gc_ack_clr_bit = irq_gc_ack_clr_bit;
> +		gc->functions.gc_mask_disable_reg_and_ack = irq_gc_mask_disable_reg_and_ack;
> +		gc->functions.gc_eoi = irq_gc_eoi;
> +		gc->functions.gc_set_wake = irq_gc_set_wake;

Why do you want to add that to every instance of generic irq chip?
What I asked for is:

struct bla {
       .noop = irq_gc_noop,
       ...
};
EXPORT_SYMBOL_GPL(bla);

Now when we have this, we can run a coccinelle script over the tree
and convert all current users to use bla.fun instead of the direct
functions and then make those static.

Thanks,

	tglx

Re: [PATCH 2/2] irq: Add function pointer table for generic-chip

[Nobuhiro Iwamatsu]

Hi,

2011/10/17 Thomas Gleixner <tglx@linutronix.de>:
> On Mon, 17 Oct 2011, Nobuhiro Iwamatsu wrote:
>
>> This adds the function table to access it with function pointer
>> by some functions providedd in generic-chip.
>> The driver who uses the function offered in generic-chip can use
>> this via this function table.
>>
>> -/* Generic chip callback functions */
>> -void irq_gc_noop(struct irq_data *d);
>> -void irq_gc_mask_disable_reg(struct irq_data *d);
>> -void irq_gc_mask_set_bit(struct irq_data *d);
>> -void irq_gc_mask_clr_bit(struct irq_data *d);
>> -void irq_gc_unmask_enable_reg(struct irq_data *d);
>> -void irq_gc_ack_set_bit(struct irq_data *d);
>> -void irq_gc_ack_clr_bit(struct irq_data *d);
>> -void irq_gc_mask_disable_reg_and_ack(struct irq_data *d);
>> -void irq_gc_eoi(struct irq_data *d);
>> -int irq_gc_set_wake(struct irq_data *d, unsigned int on);
>
> This breaks the world and some more. We have code which references
> those functions directly.
>
>> +             gc->functions.gc_noop = irq_gc_noop;
>> +             gc->functions.gc_mask_disable_reg = irq_gc_mask_disable_reg;
>> +             gc->functions.gc_mask_set_bit = irq_gc_mask_set_bit;
>> +             gc->functions.gc_mask_clr_bit = irq_gc_mask_clr_bit;
>> +             gc->functions.gc_unmask_enable_reg = irq_gc_unmask_enable_reg;
>> +             gc->functions.gc_ack_set_bit = irq_gc_ack_set_bit;
>> +             gc->functions.gc_ack_clr_bit = irq_gc_ack_clr_bit;
>> +             gc->functions.gc_mask_disable_reg_and_ack = irq_gc_mask_disable_reg_and_ack;
>> +             gc->functions.gc_eoi = irq_gc_eoi;
>> +             gc->functions.gc_set_wake = irq_gc_set_wake;
>
> Why do you want to add that to every instance of generic irq chip?
> What I asked for is:
>
> struct bla {
>       .noop = irq_gc_noop,
>       ...
> };
> EXPORT_SYMBOL_GPL(bla);
>
> Now when we have this, we can run a coccinelle script over the tree
> and convert all current users to use bla.fun instead of the direct
> functions and then make those static.
>

OK, I misled about your point.
You do not make many symbols by EXPORT_SYMBOL,
genetic_chip makes the object with the function pointer, and you
suggest that each
driver uses the function defined in the object.
Is it correct?

I created new patch. could you check this?

-----

>From c8b422d8bf3d1575a48d62ac6f785e86b36a8709 Mon Sep 17 00:00:00 2001
From: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas.com>
Date: Wed, 19 Oct 2011 12:23:19 +0900
Subject: [PATCH] irq: Add function pointer table for generic-chip

This adds the function table to access it with function pointer
by some functions providedd in generic-chip.
The driver who uses the function offered in generic-chip can use
this via this function table.

For example, driver is used as follows.
...
chip.irq_ack = &irq_gc_function_tbl.ack_set_bit;
chip.irq_mask = &irq_gc_function_tbl.mask_clr_bit;
...

Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas.com>
---
 include/linux/irq.h       |   16 ++++++++++++++++
 kernel/irq/generic-chip.c |   18 ++++++++++++++++++
 2 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index bff29c5..8b271cc 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -700,6 +700,22 @@ void irq_gc_mask_disable_reg_and_ack(struct irq_data *d);
 void irq_gc_eoi(struct irq_data *d);
 int irq_gc_set_wake(struct irq_data *d, unsigned int on);

+/* Generic chip callback function table */
+struct irq_gc_functions {
+	void (* noop)(struct irq_data *d);
+	void (* mask_disable_reg)(struct irq_data *d);
+	void (* mask_set_bit)(struct irq_data *d);
+	void (* mask_clr_bit)(struct irq_data *d);
+	void (* unmask_enable_reg)(struct irq_data *d);
+	void (* ack_set_bit)(struct irq_data *d);
+	void (* ack_clr_bit)(struct irq_data *d);
+	void (* mask_disable_reg_and_ack)(struct irq_data *d);
+	void (* eoi)(struct irq_data *d);
+	int (* set_wake)(struct irq_data *d, unsigned int on);
+};
+
+extern struct irq_gc_functions irq_gc_function_tbl;
+
 /* Setup functions for irq_chip_generic */
 struct irq_chip_generic *
 irq_alloc_generic_chip(const char *name, int nr_ct, unsigned int irq_base,
diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
index 6cb7613..03df526 100644
--- a/kernel/irq/generic-chip.c
+++ b/kernel/irq/generic-chip.c
@@ -183,6 +183,24 @@ int irq_gc_set_wake(struct irq_data *d, unsigned int on)
 }

 /**
+ * irq generic chip user needs to access irq_gc_XXX function using
+ * irq_gc_function_tbl.
+ */
+struct irq_gc_functions irq_gc_function_tbl ={
+	.noop = irq_gc_noop,
+	.mask_disable_reg = irq_gc_mask_disable_reg,
+	.mask_set_bit = irq_gc_mask_set_bit,
+	.mask_clr_bit = irq_gc_mask_clr_bit,
+	.unmask_enable_reg = irq_gc_unmask_enable_reg,
+	.ack_set_bit = irq_gc_ack_set_bit,
+	.ack_clr_bit = irq_gc_ack_clr_bit,
+	.mask_disable_reg_and_ack = irq_gc_mask_disable_reg_and_ack,
+	.eoi = irq_gc_eoi,
+	.set_wake = irq_gc_set_wake,
+};
+EXPORT_SYMBOL_GPL(irq_gc_function_tbl);
+
+/**
  * irq_alloc_generic_chip - Allocate a generic chip and initialize it
  * @name:	Name of the irq chip
  * @num_ct:	Number of irq_chip_type instances associated with this
-- 
1.7.7

Re: [PATCH v2 1/2] irq: Add EXPORT_SYMBOL_GPL to function of irq generic-chip

[Grant Likely]

On Mon, Oct 17, 2011 at 11:08:46AM +0900, Nobuhiro Iwamatsu wrote:
> Some functions of irq generic-chip is undefined, because
> EXPORT_SYMBOL_GPL is not set to these.
> 
> -----
> ERROR: "irq_setup_generic_chip" [drivers/gpio/gpio-pch.ko] undefined!
> ERROR: "irq_alloc_generic_chip" [drivers/gpio/gpio-pch.ko] undefined!
> ERROR: "irq_setup_generic_chip" [drivers/gpio/gpio-ml-ioh.ko] undefined!
> ERROR: "irq_alloc_generic_chip" [drivers/gpio/gpio-ml-ioh.ko] undefined!
> -----
> 
> This is revised that EXPORT_SYMBOL_GPL can be added and referred
> to in functions.
> 
> Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas.com>

After getting verbal ack from Thomas, I've merged this via the gpio tree.

g.

> ---
> 
> V2: Add EXPORT_SYMBOL_GPL to irq_setup_generic_chip, irq_alloc_generic_chip,
> irq_setup_alt_chip and irq_remove_generic_chip only.
> 
>  kernel/irq/generic-chip.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
> index e38544d..6cb7613 100644
> --- a/kernel/irq/generic-chip.c
> +++ b/kernel/irq/generic-chip.c
> @@ -211,6 +211,7 @@ irq_alloc_generic_chip(const char *name, int num_ct, unsigned int irq_base,
>  	}
>  	return gc;
>  }
> +EXPORT_SYMBOL_GPL(irq_alloc_generic_chip);
>  
>  /*
>   * Separate lockdep class for interrupt chip which can nest irq_desc
> @@ -258,6 +259,7 @@ void irq_setup_generic_chip(struct irq_chip_generic *gc, u32 msk,
>  	}
>  	gc->irq_cnt = i - gc->irq_base;
>  }
> +EXPORT_SYMBOL_GPL(irq_setup_generic_chip);
>  
>  /**
>   * irq_setup_alt_chip - Switch to alternative chip
> @@ -281,6 +283,7 @@ int irq_setup_alt_chip(struct irq_data *d, unsigned int type)
>  	}
>  	return -EINVAL;
>  }
> +EXPORT_SYMBOL_GPL(irq_setup_alt_chip);
>  
>  /**
>   * irq_remove_generic_chip - Remove a chip
> @@ -311,6 +314,7 @@ void irq_remove_generic_chip(struct irq_chip_generic *gc, u32 msk,
>  		irq_modify_status(i, clr, set);
>  	}
>  }
> +EXPORT_SYMBOL_GPL(irq_remove_generic_chip);
>  
>  #ifdef CONFIG_PM
>  static int irq_gc_suspend(void)
> -- 
> 1.7.7
> 
> --
> 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/