Re: [PATCH v3 2/8] genirq: irqdomain: Remove irqdomain dependency on struct device_node

[Rafael J. Wysocki]

On Monday, September 14, 2015 05:44:01 PM Marc Zyngier wrote:
> struct device_node is very much DT specific, and the original authors
> of the irqdomain subsystem recognized that tie, and went as far as
> mentionning that this could be replaced by some "void *token",
> should another firmware infrastructure be using it.
> 
> As we move ACPI on arm64 towards this model too, it makes a lot of sense
> to perform that particular move.
> 
> We replace "struct device_node *of_node" with "void *domain_token", which
> is a benign enough transformation. A non DT user of irqdomain can now
> identify its domains using this pointer.
> 
> Also, in order to prevent the introduction of sideband type information,
> only DT is allowed to store a valid kernel pointer in domain_token
> (a pointer that passes the virt_addr_valid() test will be considered
> as a valid device_node).
> 
> non-DT users that wish to store valid pointers in domain_token are
> required to use another structure such as an IDR.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
> ---
>  include/linux/irqdomain.h | 68 +++++++++++++++++++-----------------
>  kernel/irq/irqdomain.c    | 89 ++++++++++++++++++++++++++++++++++-------------
>  2 files changed, 101 insertions(+), 56 deletions(-)
> 
> diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
> index f644fdb..ac7041b 100644
> --- a/include/linux/irqdomain.h
> +++ b/include/linux/irqdomain.h
> @@ -17,16 +17,14 @@
>   * model). It's the domain callbacks that are responsible for setting the
>   * irq_chip on a given irq_desc after it's been mapped.
>   *
> - * The host code and data structures are agnostic to whether or not
> - * we use an open firmware device-tree. We do have references to struct
> - * device_node in two places: in irq_find_host() to find the host matching
> - * a given interrupt controller node, and of course as an argument to its
> - * counterpart domain->ops->match() callback. However, those are treated as
> - * generic pointers by the core and the fact that it's actually a device-node
> - * pointer is purely a convention between callers and implementation. This
> - * code could thus be used on other architectures by replacing those two
> - * by some sort of arch-specific void * "token" used to identify interrupt
> - * controllers.
> + * The host code and data structures are agnostic to whether or not we
> + * use an open firmware device-tree. Domains can be assigned a
> + * "void *domain_token" identifier, which is assumed to represent a
> + * "struct device_node" if the pointer is a valid virtual address.
> + *
> + * Otherwise, the value is assumed to be an opaque identifier. Should
> + * a pointer to a non "struct device_node" be required, another data
> + * structure should be used to indirect it (an IDR for example).
>   */
>  
>  #ifndef _LINUX_IRQDOMAIN_H
> @@ -108,8 +106,8 @@ struct irq_domain_chip_generic;
>   * @flags: host per irq_domain flags
>   *
>   * Optional elements
> - * @of_node: Pointer to device tree nodes associated with the irq_domain. Used
> - *           when decoding device tree interrupt specifiers.
> + * @domain_token: optional firmware-specific identifier for
> + *                the interrupt controller
>   * @gc: Pointer to a list of generic chips. There is a helper function for
>   *      setting up one or more generic chips for interrupt controllers
>   *      drivers using the generic chip library which uses this pointer.
> @@ -130,7 +128,7 @@ struct irq_domain {
>  	unsigned int flags;
>  
>  	/* Optional data */
> -	struct device_node *of_node;
> +	void *domain_token;

I'm wondering if that may be something which isn't (void *), but a specific
pointer type, so the compiler warns us when something suspicious is assigned
to it?

[Somewhat along the lines struct fwnode_handle is used elsewehere.]

>  	enum irq_domain_bus_token bus_token;
>  	struct irq_domain_chip_generic *gc;
>  #ifdef	CONFIG_IRQ_DOMAIN_HIERARCHY
> @@ -161,70 +159,73 @@ enum {

Thanks,
Rafael

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