Re: [Outreachy kernel] [PATCH v2 2/4] staging: comedi: amplc_pci224: Convert macro CLK_CONFIG to static inline function

[Greg KH <gregkh@linuxfoundation.org>]

On Mon, Mar 21, 2016 at 12:45:06AM +0530, Bhaktipriya Shridhar wrote:
> Convert macro CLK_CONFIG to static inline function as static inline
> functions are preferred over macros. Since every single user of this
> macro passes the argument into the outb() function, with
> (dev->iobase + ...) as the address, the two calls have been merged
> into one.
> 
> This was done using Coccinelle:
> 
> @r@
> identifier f;
> expression e;
> @@
> - #define CLK_CONFIG(chan, src) e
> + static inline void pci224_clk_config(struct pci224_private *devpriv,
> +                                      unsigned int reg, unsigned int chan,
> +                                      unsigned int src)
> +{
> +       outb(((chan & 3) << 3) | (src & 7), devpriv->iobase1 + reg);
> +}
> 
> @r1@
> expression devp,reg,chan,src;
> @@
> -outb(CLK_CONFIG(chan, src), devp->iobase1 + reg);
> +pci224_clk_config(devp, reg, chan, src);
> 
> Also, the comment describing the macro has been removed manually.
> 
> Signed-off-by: Bhaktipriya Shridhar <bhaktipriya96@gmail.com>
> ---
>  Changes in v2:
> 	-Since every single user of this macro passes the argument
> 	 into the outb() function, with (devpriv->iobase1 + ...)
> 	 as the address, the two calls have been merged into one.
> 
>  drivers/staging/comedi/drivers/amplc_pci224.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/comedi/drivers/amplc_pci224.c b/drivers/staging/comedi/drivers/amplc_pci224.c
> index cac011f..5bc1377 100644
> --- a/drivers/staging/comedi/drivers/amplc_pci224.c
> +++ b/drivers/staging/comedi/drivers/amplc_pci224.c
> @@ -201,8 +201,12 @@
>  #define CLK_1KHZ	5	/* internal 1 kHz clock */
>  #define CLK_OUTNM1	6	/* output of channel-1 modulo total */
>  #define CLK_EXT		7	/* external clock */
> -/* Macro to construct clock input configuration register value. */
> -#define CLK_CONFIG(chan, src)	((((chan) & 3) << 3) | ((src) & 7))
> +static inline void pci224_clk_config(struct pci224_private *devpriv,
> +				     unsigned int reg, unsigned int chan,
> +				     unsigned int src)
> +{
> +	outb(((chan & 3) << 3) | (src & 7), devpriv->iobase1 + reg);
> +}

If you were to do this, you need a blank line before the function.  But
again, I don't think this is a good change, it doesn't help much...

thanks,

greg k-h