[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CABMQnVKpFnRwy1ujwQ5aeDn8M3iKYTgWTMK13iUY-_0V_Jw1WQ@mail.gmail.com>
Date: Thu, 20 Oct 2011 12:44:55 +0900
From: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@...esas.com>
To: Thomas Gleixner <tglx@...utronix.de>
Cc: linux-kernel@...r.kernel.org, mingo@...nel.org
Subject: Re: [PATCH 2/2] irq: Add function pointer table for generic-chip
Hi,
2011/10/17 Thomas Gleixner <tglx@...utronix.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@...esas.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@...esas.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists