On Mon, 19 Jan 2015, Jiang Liu wrote:
> Hi Thomas and Marc,
> During working on the generic MSI support, I have some proposal
> about reorganizing struct irq_data and struct irq_desc. The proposed
> changes are:
> 1) Add a pointer "struct irq_desc *" to struct irq_data, so we could
> quickly get struct irq_desc from struct irq_data.
> 2) Move "node" from struct irq_data into struct irq_desc, NUMA info
> should be per-irq instead of per-chip.
> 3) Move "affinity" from struct irq_data into struct irq_desc, NUMA info
> should be per-irq instead of per-chip.
> 4) Move "msi_desc" from struct irq_data into struct irq_desc. (Not sure
> whether we should do this. Theoretically we should use
> irq_data->handler_data to store msi_desc.)
msi_desc belongs to the msi chip, while handler_data is common data.
I had a look at the usage sites of handler_data. Most of them use it
in combination with chained handlers. Some sites use it instead of
chip data and only a few use it for some random other stuff, where the
x86 sites (hpet, ht, iommu ...) will go away with the irqdomain
conversion.
So in the long run we should provide:
irq_set_chained_handler_and_data(irq, handler, handler_data)
convert everything over and finally remove the direct accessor to
handler_data.
msi_desc in a hierarchical implementation should actually be in
chip_data, but we probably need to keep the msi_desc pointer for
backward compability reasons.
> With above change applied, struct irq_data only hosts per-chip data, and
> struct irq_desc hosts per-irq data. What's your thoughts?
I'm not so happy with exposing irqdesc to random code again. I went a
great way to hide it from abuse.
So I'd rather like to see something like this:
struct irq_common_data {
unsigned int state_use_accessors;
unsigned int node;
void *handler_data;
cpumask_var_t affinity;
};
struct irq_data {
u32 mask;
unsigned int irq;
unsigned long hwirq;
struct irq_chip *chip;
struct irq_domain *domain;
#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
struct irq_data *parent_data;
#endif
void *chip_data;
struct msi_desc *msi_desc;
struct irq_common_data *common_data;
};
struct irq_desc {
struct irq_data irq_data;
struct common_irq_data common_data;
...
};
Thanks,
tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
