Re: [RFC v1 14/25] genirq: Kill the first parameter 'irq' of irq_flow_handler_t

[Thomas Gleixner]

B1;2802;0cOn Wed, 20 May 2015, Jiang Liu wrote:
> On 2015/5/20 23:25, Thomas Gleixner wrote:
> > On Wed, 20 May 2015, Jiang Liu wrote:
> >> --- a/include/linux/irqdesc.h
> >> +++ b/include/linux/irqdesc.h
> >> @@ -99,6 +99,11 @@ static inline struct irq_desc *irq_data_to_desc(struct irq_data *data)
> >>  	return container_of(data->common, struct irq_desc, irq_common_data);
> >>  }
> >>  
> >> +static inline unsigned int irq_desc_get_irq(struct irq_desc *desc)
> >> +{
> >> +	return desc->irq_data.irq;
> >> +}
> >> +
> > 
> > Does not apply either.
> Hi Thomas,
> 	Will check the base again.
> 	Please do not apply patch after [14/25]. There are here to
> ask for comments, there are still building issues with patch after
> [14/25]. Just help to comment whether it's on the right direction.
> If so, I will send another version for formal review.

I think you are doing that conversion wrong. You should first change
all handlers which use the irq argument to:

    handler(unsigned __irq, *desc)

and add the local variable 

    unsigned irq;

or
	
    unsigned irq = irq_desc_get_irq(desc);

for those.

So that results in a pretty small series of patches.

Once that is done, you can remove the irq argument in one go treewide.

And you should do that with scripting aid. Coccinelle is the proper
tool for this. If you have problems with cocci, you can politely ask
Julia (Cc'ed) for help. She's the master of coccinelle :)

The purpose of the script is to:

    1) Identify all handlers

       This can be done by analyzing the arguments of

       	    __irq_set_handler()
	    irq_set_handler()
	    irq_set_chained_handler()
	    irq_alloc_generic_chip()
	    irq_alloc_domain_generic_chips()
	    irq_set_chip_and_handler_name()
	    irq_set_chip_and_handler()
	    __irq_set_handler_locked()
	    __irq_set_chip_handler_name_locked()
	    __irq_set_preflow_handler()

	If the argument is a function local to the file step 2 can
	take place.

	If it's a extern function, then the script should at least
	print out the function names. Those are in most of the cases
	the generic flow handler functions.

    2) Check the handlers whether they use the irq argument

       For those which do:

          Check whether they just use the argument instead of a local
       	  variable, i.e. the first usage is an assignment. In that
       	  case rename the argument to __irq and add a local variable
       	  
	      unsigned int irq;

	  If they use the irq argument for some computation or as
       	  argument to another function, then you need to rename the
       	  argument as well and add:

       	      unsigned int irq = irq_get_desc_irq(desc);

So this results in a series of patches which can be merged right
away. If that lot has hit Linus tree and right after -rc1, we run the
script again and find and fix stuff which got added throughout the
merge window. That might result in 0 to n patches.

The last step is another script which does:

    1) Identify all handlers

    2) Remove the irq argument

We can send that right after -rc1 so no new users can trickle in.

It's really important to do that with scripts. It seems you try to do
it via compile testing. But that will fail as you CANNOT execute all
possible config combinations.

Thanks,

	tglx

--
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/