[PATCH v2 0/4] Convert macros to static inline functions

[PATCH v2 0/4] Convert macros to static inline functions

[Bhaktipriya Shridhar]

Convert macros CLK_CONFIG and GAT_CONFIG into static inline
functions as static inline functions are preferred over macros.

Bhaktipriya Shridhar (4):
  staging: comedi: amplc_pci230: Convert macro CLK_CONFIG to static
    inline function
  staging: comedi: amplc_pci224: Convert macro CLK_CONFIG to static
    inline function
  staging: comedi: amplc_pci224: Convert macro GAT_CONFIG to static
    inline function
  staging: comedi: amplc_pci230: Convert macro GAT_CONFIG to static
    inline function

 drivers/staging/comedi/drivers/amplc_pci224.c | 24 +++++++++-----
 drivers/staging/comedi/drivers/amplc_pci230.c | 48 ++++++++++++++++-----------
 2 files changed, 45 insertions(+), 27 deletions(-)

--
2.1.4

[PATCH v2 1/4] staging: comedi: amplc_pci230: Convert macro CLK_CONFIG to static inline function

[Bhaktipriya Shridhar]

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 pci230_clk_config(struct comedi_device *dev,
+                                      unsigned int reg,unsigned int chan,
+                                      unsigned int src)
+{
+       outb(((chan & 3) << 3) | (src & 7), dev->iobase + reg);
+}

@r1@
expression dev,reg,chan,src;
@@
-outb(CLK_CONFIG(chan, src), dev->iobase + reg);
+pci230_clk_config(dev, 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 usage of this macro passes the argument into
	  the outb() function, with (dev->iobase + ...) as the address,
	  the two calls have been merged into one.

 drivers/staging/comedi/drivers/amplc_pci230.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/comedi/drivers/amplc_pci230.c b/drivers/staging/comedi/drivers/amplc_pci230.c
index 907c39c..635cd46 100644
--- a/drivers/staging/comedi/drivers/amplc_pci230.c
+++ b/drivers/staging/comedi/drivers/amplc_pci230.c
@@ -369,8 +369,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 pci230_clk_config(struct comedi_device *dev,
+				     unsigned int reg, unsigned int chan,
+				     unsigned int src)
+{
+	outb(((chan & 3) << 3) | (src & 7), dev->iobase + reg);
+}

 /*
  * Counter/timer gate input configuration sources.
@@ -687,7 +691,7 @@ static void pci230_ct_setup_ns_mode(struct comedi_device *dev, unsigned int ct,
 	/* Determine clock source and count. */
 	clk_src = pci230_choose_clk_count(ns, &count, flags);
 	/* Program clock source. */
-	outb(CLK_CONFIG(ct, clk_src), dev->iobase + PCI230_ZCLK_SCE);
+	pci230_clk_config(dev, PCI230_ZCLK_SCE, ct, clk_src);
 	/* Set initial count. */
 	if (count >= 65536)
 		count = 0;
--
2.1.4

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

[Bhaktipriya Shridhar]

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);
+}

 /*
  * Counter/timer gate input configuration sources.
@@ -817,9 +821,9 @@ static void pci224_ao_start_pacer(struct comedi_device *dev,
 	/* Make sure Z2-2 is gated on.  */
 	outb(GAT_CONFIG(2, GAT_VCC), devpriv->iobase1 + PCI224_ZGAT_SCE);
 	/* Z2-2 needs 10 MHz clock. */
-	outb(CLK_CONFIG(2, CLK_10MHZ), devpriv->iobase1 + PCI224_ZCLK_SCE);
+	pci224_clk_config(devpriv, PCI224_ZCLK_SCE, 2, CLK_10MHZ);
 	/* Z2-0 is clocked from Z2-2's output. */
-	outb(CLK_CONFIG(0, CLK_OUTNM1), devpriv->iobase1 + PCI224_ZCLK_SCE);
+	pci224_clk_config(devpriv, PCI224_ZCLK_SCE, 0, CLK_OUTNM1);

 	comedi_8254_pacer_enable(dev->pacer, 2, 0, false);
 }
--
2.1.4

[PATCH v2 3/4] staging: comedi: amplc_pci224: Convert macro GAT_CONFIG to static inline function

[Bhaktipriya Shridhar]

Convert macro GAT_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
(devpriv->iobase1 + ...) as the address, the two calls have been merged
into one.

This was done using Coccinelle:

@r@
identifier f;
expression e;
@@
- #define GAT_CONFIG(chan, src) e
+ static inline void pci224_gat_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(GAT_CONFIG(chan, src), devp->iobase1 + reg);
+pci224_gat_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 5bc1377..5046b94 100644
--- a/drivers/staging/comedi/drivers/amplc_pci224.c
+++ b/drivers/staging/comedi/drivers/amplc_pci224.c
@@ -215,8 +215,12 @@ static inline void pci224_clk_config(struct pci224_private *devpriv,
 #define GAT_GND		1	/* GND (i.e. disabled) */
 #define GAT_EXT		2	/* reserved (external gate input) */
 #define GAT_NOUTNM2	3	/* inverted output of channel-2 modulo total */
-/* Macro to construct gate input configuration register value. */
-#define GAT_CONFIG(chan, src)	((((chan) & 3) << 3) | ((src) & 7))
+static inline void pci224_gat_config(struct pci224_private *devpriv,
+				     unsigned int reg, unsigned int chan,
+				     unsigned int src)
+{
+	outb(((chan & 3) << 3) | (src & 7), devpriv->iobase1 + reg);
+}

 /*
  * Summary of CLK_OUTNM1 and GAT_NOUTNM2 connections for PCI224 and PCI234:
@@ -816,10 +820,10 @@ static void pci224_ao_start_pacer(struct comedi_device *dev,
 	 * source.
 	 */
 	/* Make sure Z2-0 is gated on.  */
-	outb(GAT_CONFIG(0, GAT_VCC), devpriv->iobase1 + PCI224_ZGAT_SCE);
+	pci224_gat_config(devpriv, PCI224_ZGAT_SCE, 0, GAT_VCC);
 	/* Cascading with Z2-2. */
 	/* Make sure Z2-2 is gated on.  */
-	outb(GAT_CONFIG(2, GAT_VCC), devpriv->iobase1 + PCI224_ZGAT_SCE);
+	pci224_gat_config(devpriv, PCI224_ZGAT_SCE, 2, GAT_VCC);
 	/* Z2-2 needs 10 MHz clock. */
 	pci224_clk_config(devpriv, PCI224_ZCLK_SCE, 2, CLK_10MHZ);
 	/* Z2-0 is clocked from Z2-2's output. */
--
2.1.4

[PATCH v2 4/4] staging: comedi: amplc_pci230: Convert macro GAT_CONFIG to static inline function

[Bhaktipriya Shridhar]

Convert macro GAT_CONFIG to static inline function as static inline
functions are preferred over macros. This change is possible since the
arguments at all call sites have the same type.

This was done using Coccinelle:

@r@
expression e;
@@
- #define GAT_CONFIG(chan, src) e
+ static inline unsigned int pci230_gat_config(unsigned int
chan,unsigned int src)
+{
+       return ((chan & 3) << 3) | (src & 7);
+}

@r1@
expression dev,reg,chan,src;
@@
-GAT_CONFIG(chan, src)
+pci230_gat_config(chan, src)

Signed-off-by: Bhaktipriya Shridhar <bhaktipriya96@gmail.com>
---
 Changes in v2:
	- Removed the extra () around the arguments.
	- Renamed the function and made the changes consistent

 drivers/staging/comedi/drivers/amplc_pci230.c | 38 ++++++++++++++++-----------
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/comedi/drivers/amplc_pci230.c b/drivers/staging/comedi/drivers/amplc_pci230.c
index 635cd46..6117fb7 100644
--- a/drivers/staging/comedi/drivers/amplc_pci230.c
+++ b/drivers/staging/comedi/drivers/amplc_pci230.c
@@ -383,8 +383,12 @@ static inline void pci230_clk_config(struct comedi_device *dev,
 #define GAT_GND		1	/* GND (i.e. disabled) */
 #define GAT_EXT		2	/* external gate input (PPCn on PCI230) */
 #define GAT_NOUTNM2	3	/* inverted output of channel-2 modulo total */
-/* Macro to construct gate input configuration register value. */
-#define GAT_CONFIG(chan, src)	((((chan) & 3) << 3) | ((src) & 7))
+
+static inline unsigned int pci230_gat_config(unsigned int chan,
+					     unsigned int src)
+{
+	return ((chan & 3) << 3) | (src & 7);
+}

 /*
  * Summary of CLK_OUTNM1 and GAT_NOUTNM2 connections for PCI230 and PCI260:
@@ -1267,7 +1271,8 @@ static void pci230_ao_start(struct comedi_device *dev,
 					       irqflags);
 		}
 		/* Set CT1 gate high to start counting. */
-		outb(GAT_CONFIG(1, GAT_VCC), dev->iobase + PCI230_ZGAT_SCE);
+		outb(pci230_gat_config(1, GAT_VCC),
+		     dev->iobase + PCI230_ZGAT_SCE);
 		break;
 	case TRIG_INT:
 		async->inttrig = pci230_ao_inttrig_scan_begin;
@@ -1355,7 +1360,8 @@ static int pci230_ao_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
 		 * cmd->scan_begin_arg is sampling period in ns.
 		 * Gate it off for now.
 		 */
-		outb(GAT_CONFIG(1, GAT_GND), dev->iobase + PCI230_ZGAT_SCE);
+		outb(pci230_gat_config(1, GAT_GND),
+		     dev->iobase + PCI230_ZGAT_SCE);
 		pci230_ct_setup_ns_mode(dev, 1, I8254_MODE3,
 					cmd->scan_begin_arg,
 					cmd->flags);
@@ -1796,9 +1802,9 @@ static int pci230_ai_inttrig_scan_begin(struct comedi_device *dev,
 	spin_lock_irqsave(&devpriv->ai_stop_spinlock, irqflags);
 	if (devpriv->ai_cmd_started) {
 		/* Trigger scan by waggling CT0 gate source. */
-		zgat = GAT_CONFIG(0, GAT_GND);
+		zgat = pci230_gat_config(0, GAT_GND);
 		outb(zgat, dev->iobase + PCI230_ZGAT_SCE);
-		zgat = GAT_CONFIG(0, GAT_VCC);
+		zgat = pci230_gat_config(0, GAT_VCC);
 		outb(zgat, dev->iobase + PCI230_ZGAT_SCE);
 	}
 	spin_unlock_irqrestore(&devpriv->ai_stop_spinlock, irqflags);
@@ -1930,20 +1936,20 @@ static void pci230_ai_start(struct comedi_device *dev,
 			 * Conversion timer CT2 needs to be gated by
 			 * inverted output of monostable CT2.
 			 */
-			zgat = GAT_CONFIG(2, GAT_NOUTNM2);
+			zgat = pci230_gat_config(2, GAT_NOUTNM2);
 		} else {
 			/*
 			 * Conversion timer CT2 needs to be gated on
 			 * continuously.
 			 */
-			zgat = GAT_CONFIG(2, GAT_VCC);
+			zgat = pci230_gat_config(2, GAT_VCC);
 		}
 		outb(zgat, dev->iobase + PCI230_ZGAT_SCE);
 		if (cmd->scan_begin_src != TRIG_FOLLOW) {
 			/* Set monostable CT0 trigger source. */
 			switch (cmd->scan_begin_src) {
 			default:
-				zgat = GAT_CONFIG(0, GAT_VCC);
+				zgat = pci230_gat_config(0, GAT_VCC);
 				break;
 			case TRIG_EXT:
 				/*
@@ -1954,21 +1960,21 @@ static void pci230_ai_start(struct comedi_device *dev,
 				 * input in order to use it as an external scan
 				 * trigger.
 				 */
-				zgat = GAT_CONFIG(0, GAT_EXT);
+				zgat = pci230_gat_config(0, GAT_EXT);
 				break;
 			case TRIG_TIMER:
 				/*
 				 * Monostable CT0 triggered by rising edge on
 				 * inverted output of CT1 (falling edge on CT1).
 				 */
-				zgat = GAT_CONFIG(0, GAT_NOUTNM2);
+				zgat = pci230_gat_config(0, GAT_NOUTNM2);
 				break;
 			case TRIG_INT:
 				/*
 				 * Monostable CT0 is triggered by inttrig
 				 * function waggling the CT0 gate source.
 				 */
-				zgat = GAT_CONFIG(0, GAT_VCC);
+				zgat = pci230_gat_config(0, GAT_VCC);
 				break;
 			}
 			outb(zgat, dev->iobase + PCI230_ZGAT_SCE);
@@ -1978,7 +1984,7 @@ static void pci230_ai_start(struct comedi_device *dev,
 				 * Scan period timer CT1 needs to be
 				 * gated on to start counting.
 				 */
-				zgat = GAT_CONFIG(1, GAT_VCC);
+				zgat = pci230_gat_config(1, GAT_VCC);
 				outb(zgat, dev->iobase + PCI230_ZGAT_SCE);
 				break;
 			case TRIG_INT:
@@ -2220,7 +2226,7 @@ static int pci230_ai_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
 		 * Note, counter/timer output 2 can be monitored on the
 		 * connector: PCI230 pin 21, PCI260 pin 18.
 		 */
-		zgat = GAT_CONFIG(2, GAT_GND);
+		zgat = pci230_gat_config(2, GAT_GND);
 		outb(zgat, dev->iobase + PCI230_ZGAT_SCE);
 		/* Set counter/timer 2 to the specified conversion period. */
 		pci230_ct_setup_ns_mode(dev, 2, I8254_MODE3, cmd->convert_arg,
@@ -2238,7 +2244,7 @@ static int pci230_ai_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
 			 * monostable to stop it triggering.  The trigger
 			 * source will be changed later.
 			 */
-			zgat = GAT_CONFIG(0, GAT_VCC);
+			zgat = pci230_gat_config(0, GAT_VCC);
 			outb(zgat, dev->iobase + PCI230_ZGAT_SCE);
 			pci230_ct_setup_ns_mode(dev, 0, I8254_MODE1,
 						((uint64_t)cmd->convert_arg *
@@ -2251,7 +2257,7 @@ static int pci230_ai_cmd(struct comedi_device *dev, struct comedi_subdevice *s)
 				 *
 				 * Set up CT1 but gate it off for now.
 				 */
-				zgat = GAT_CONFIG(1, GAT_GND);
+				zgat = pci230_gat_config(1, GAT_GND);
 				outb(zgat, dev->iobase + PCI230_ZGAT_SCE);
 				pci230_ct_setup_ns_mode(dev, 1, I8254_MODE3,
 							cmd->scan_begin_arg,
--
2.1.4

Re: [Outreachy kernel] [PATCH v2 4/4] staging: comedi: amplc_pci230: Convert macro GAT_CONFIG to static inline function

[Greg KH]

On Mon, Mar 21, 2016 at 12:50:25AM +0530, Bhaktipriya Shridhar wrote:
> Convert macro GAT_CONFIG to static inline function as static inline
> functions are preferred over macros. This change is possible since the
> arguments at all call sites have the same type.
> 
> This was done using Coccinelle:
> 
> @r@
> expression e;
> @@
> - #define GAT_CONFIG(chan, src) e
> + static inline unsigned int pci230_gat_config(unsigned int
> chan,unsigned int src)
> +{
> +       return ((chan & 3) << 3) | (src & 7);
> +}
> 
> @r1@
> expression dev,reg,chan,src;
> @@
> -GAT_CONFIG(chan, src)
> +pci230_gat_config(chan, src)
> 
> Signed-off-by: Bhaktipriya Shridhar <bhaktipriya96@gmail.com>
> ---
>  Changes in v2:
> 	- Removed the extra () around the arguments.
> 	- Renamed the function and made the changes consistent
> 
>  drivers/staging/comedi/drivers/amplc_pci230.c | 38 ++++++++++++++++-----------
>  1 file changed, 22 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/staging/comedi/drivers/amplc_pci230.c b/drivers/staging/comedi/drivers/amplc_pci230.c
> index 635cd46..6117fb7 100644
> --- a/drivers/staging/comedi/drivers/amplc_pci230.c
> +++ b/drivers/staging/comedi/drivers/amplc_pci230.c
> @@ -383,8 +383,12 @@ static inline void pci230_clk_config(struct comedi_device *dev,
>  #define GAT_GND		1	/* GND (i.e. disabled) */
>  #define GAT_EXT		2	/* external gate input (PPCn on PCI230) */
>  #define GAT_NOUTNM2	3	/* inverted output of channel-2 modulo total */
> -/* Macro to construct gate input configuration register value. */
> -#define GAT_CONFIG(chan, src)	((((chan) & 3) << 3) | ((src) & 7))
> +
> +static inline unsigned int pci230_gat_config(unsigned int chan,
> +					     unsigned int src)
> +{
> +	return ((chan & 3) << 3) | (src & 7);
> +}

This looks good, but can you properly format this?

thanks,

greg k-h

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

[Greg KH]

On Mon, Mar 21, 2016 at 12:46:26AM +0530, Bhaktipriya Shridhar wrote:
> Convert macro GAT_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
> (devpriv->iobase1 + ...) as the address, the two calls have been merged
> into one.
> 
> This was done using Coccinelle:
> 
> @r@
> identifier f;
> expression e;
> @@
> - #define GAT_CONFIG(chan, src) e
> + static inline void pci224_gat_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(GAT_CONFIG(chan, src), devp->iobase1 + reg);
> +pci224_gat_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(-)

Again, I'm not going to take this, sorry.

greg k-h

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

[Greg KH]

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

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

[Greg KH]

On Mon, Mar 21, 2016 at 12:41:27AM +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 pci230_clk_config(struct comedi_device *dev,
> +                                      unsigned int reg,unsigned int chan,
> +                                      unsigned int src)
> +{
> +       outb(((chan & 3) << 3) | (src & 7), dev->iobase + reg);
> +}
> 
> @r1@
> expression dev,reg,chan,src;
> @@
> -outb(CLK_CONFIG(chan, src), dev->iobase + reg);
> +pci230_clk_config(dev, 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 usage of this macro passes the argument into
> 	  the outb() function, with (dev->iobase + ...) as the address,
> 	  the two calls have been merged into one.
> 
>  drivers/staging/comedi/drivers/amplc_pci230.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/comedi/drivers/amplc_pci230.c b/drivers/staging/comedi/drivers/amplc_pci230.c
> index 907c39c..635cd46 100644
> --- a/drivers/staging/comedi/drivers/amplc_pci230.c
> +++ b/drivers/staging/comedi/drivers/amplc_pci230.c
> @@ -369,8 +369,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 pci230_clk_config(struct comedi_device *dev,
> +				     unsigned int reg, unsigned int chan,
> +				     unsigned int src)
> +{
> +	outb(((chan & 3) << 3) | (src & 7), dev->iobase + reg);
> +}
> 
>  /*
>   * Counter/timer gate input configuration sources.
> @@ -687,7 +691,7 @@ static void pci230_ct_setup_ns_mode(struct comedi_device *dev, unsigned int ct,
>  	/* Determine clock source and count. */
>  	clk_src = pci230_choose_clk_count(ns, &count, flags);
>  	/* Program clock source. */
> -	outb(CLK_CONFIG(ct, clk_src), dev->iobase + PCI230_ZCLK_SCE);
> +	pci230_clk_config(dev, PCI230_ZCLK_SCE, ct, clk_src);

This is only done in one place, just "open code" it, no need for a
separate function at all here.

Actually, the original code is nicer, it makes more sense, and a macro
is just fine, no need to change it.

thanks,

greg k-h