Re: [RFC PATCH 07/33] irqchip/gic-v3-its: Macro-ize its_send_single_command

[Auger Eric]

Hi,
On 17/01/2017 11:20, Marc Zyngier wrote:
> Most ITS commands do operate on a collection object, and require
> a SYNC command to be performed on that collection in order to
> guarantee the execution of the first command.
> 
> With GICv4 ITS, another set of commands perform similar operations
> on a VPE object, and a VSYNC operations must be executed to guarantee
> their execution.
> 
> Given the similarities (post a command, perform a synchronization
> operation on a sync object), it makes sense to reuse the same
> mechanism for both class of commands.
> 
> Let's start with turning its_send_single_command into a huge macro
> that performs the bulk of the work, and a set of helpers that
> make this macro useeable for the GICv3 ITS commands.
s/useeable/usable

Eric
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
> ---
>  drivers/irqchip/irq-gic-v3-its.c | 82 ++++++++++++++++++++++------------------
>  1 file changed, 45 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 99f6130..520b764 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -484,43 +484,51 @@ static void its_wait_for_range_completion(struct its_node *its,
>  	}
>  }
>  
> -static void its_send_single_command(struct its_node *its,
> -				    its_cmd_builder_t builder,
> -				    struct its_cmd_desc *desc)
> -{
> -	struct its_cmd_block *cmd, *sync_cmd, *next_cmd;
> -	struct its_collection *sync_col;
> -	unsigned long flags;
> -
> -	raw_spin_lock_irqsave(&its->lock, flags);
> -
> -	cmd = its_allocate_entry(its);
> -	if (!cmd) {		/* We're soooooo screewed... */
> -		pr_err_ratelimited("ITS can't allocate, dropping command\n");
> -		raw_spin_unlock_irqrestore(&its->lock, flags);
> -		return;
> -	}
> -	sync_col = builder(cmd, desc);
> -	its_flush_cmd(its, cmd);
> -
> -	if (sync_col) {
> -		sync_cmd = its_allocate_entry(its);
> -		if (!sync_cmd) {
> -			pr_err_ratelimited("ITS can't SYNC, skipping\n");
> -			goto post;
> -		}
> -		its_encode_cmd(sync_cmd, GITS_CMD_SYNC);
> -		its_encode_target(sync_cmd, sync_col->target_address);
> -		its_fixup_cmd(sync_cmd);
> -		its_flush_cmd(its, sync_cmd);
> -	}
> -
> -post:
> -	next_cmd = its_post_commands(its);
> -	raw_spin_unlock_irqrestore(&its->lock, flags);
> -
> -	its_wait_for_range_completion(its, cmd, next_cmd);
> -}
> +#define __its_send_single_cmd(name, buildtype, synctype, buildfn)	\
> +void name(struct its_node *its,						\
> +	  buildtype builder,						\
> +	  struct its_cmd_desc *desc)					\
> +{									\
> +	struct its_cmd_block *cmd, *sync_cmd, *next_cmd;		\
> +	synctype *sync_obj;						\
> +	unsigned long flags;						\
> +									\
> +	raw_spin_lock_irqsave(&its->lock, flags);			\
> +									\
> +	cmd = its_allocate_entry(its);					\
> +	if (!cmd) {		/* We're soooooo screewed... */		\
> +		raw_spin_unlock_irqrestore(&its->lock, flags);		\
> +		return;							\
> +	}								\
> +	sync_obj = builder(cmd, desc);					\
> +	its_flush_cmd(its, cmd);					\
> +									\
> +	if (sync_obj) {							\
> +		sync_cmd = its_allocate_entry(its);			\
> +		if (!sync_cmd)						\
> +			goto post;					\
> +									\
> +		buildfn(sync_cmd, sync_obj);				\
> +		its_fixup_cmd(sync_cmd);				\
> +		its_flush_cmd(its, sync_cmd);				\
> +	}								\
> +									\
> +post:									\
> +	next_cmd = its_post_commands(its);				\
> +	raw_spin_unlock_irqrestore(&its->lock, flags);			\
> +									\
> +	its_wait_for_range_completion(its, cmd, next_cmd);		\
> +}
> +
> +static void its_build_sync_cmd(struct its_cmd_block *sync_cmd,
> +			       struct its_collection *sync_col)
> +{
> +	its_encode_cmd(sync_cmd, GITS_CMD_SYNC);
> +	its_encode_target(sync_cmd, sync_col->target_address);
> +}
> +
> +static __its_send_single_cmd(its_send_single_command, its_cmd_builder_t,
> +			     struct its_collection, its_build_sync_cmd)
>  
>  static void its_send_inv(struct its_device *dev, u32 event_id)
>  {
>