Skip to contentSkip to conversation

Convert macros to static inline functions

PATCH Superseded Bhaktipriya Shridhar 2 participants 4 messages Mar 18, 2016 Staging

v1 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Bhaktipriya Shridhar

author
#0 lore raw

cover Convert macros to static inline functions

Convert macros CLK_CONFIG and GAT_CONFIG into static inline functions as static inline functions are preferred over macros. This change is possible since the arguments at all call sites have the same type. Also, both the macros have same type of arguments and return values. They have been renamed in lowecase.

Bhaktipriya Shridhar (2):
staging: comedi: drivers: amplc_pci224: Convert macros to static
inline functions
staging: comedi: drivers: amplc_pci230: Convert macros to static
inline functions

 drivers/staging/comedi/drivers/amplc_pci224.c | 16 ++++++-----
 drivers/staging/comedi/drivers/amplc_pci230.c | 38 +++++++++++++++------------
 2 files changed, 31 insertions(+), 23 deletions(-)

--
2.1.4

author
#1 lore raw

PATCH 1/2 staging: comedi: drivers: amplc_pci224: Convert macros to static inline functions

Convert macros CLK_CONFIG and GAT_CONFIG into static inline functions as static inline functions are preferred over macros. This change is possible since the arguments at all call sites have the same type. Also, both the macros have same type of arguments and return values. They have been renamed in lowecase.

This was done using coccinelle:

@r@
identifier f;
expression e;
@@
- #define f(chan, src) e
+ static inline unsigned int f(unsigned int chan,unsigned int src)
+ {
+ return e;
+ }

@r1@
@@

-GAT_CONFIG
+gat_config

@r2@
@@
-CLK_CONFIG
+clk_config

Signed-off-by Bhaktipriya Shridhar <[email protected]>
---
 drivers/staging/comedi/drivers/amplc_pci224.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)
drivers/staging/comedi/drivers/amplc_pci224.c +10 -6
index cac011f..21bc6b3 100644
@@ -202,7 +202,9 @@
 #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 unsigned int CLK_CONFIG(unsigned int chan, unsigned int src) {
+	return ((((chan) & 3) << 3) | ((src) & 7));
+}
 
 /*
  * Counter/timer gate input configuration sources.

@@ -212,7 +214,9 @@
 #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 unsigned int GAT_CONFIG(unsigned int chan, unsigned int src) {
+	return ((((chan) & 3) << 3) | ((src) & 7));
+}
 
 /*
  * Summary of CLK_OUTNM1 and GAT_NOUTNM2 connections for PCI224 and PCI234:

@@ -812,14 +816,14 @@ 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);
+	outb(gat_config(0, GAT_VCC), devpriv->iobase1 + PCI224_ZGAT_SCE);
 	/* Cascading with Z2-2. */
 	/* Make sure Z2-2 is gated on.  */
-	outb(GAT_CONFIG(2, GAT_VCC), devpriv->iobase1 + PCI224_ZGAT_SCE);
+	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);
+	outb(clk_config(2, CLK_10MHZ), devpriv->iobase1 + PCI224_ZCLK_SCE);
 	/* Z2-0 is clocked from Z2-2's output. */
-	outb(CLK_CONFIG(0, CLK_OUTNM1), devpriv->iobase1 + PCI224_ZCLK_SCE);
+	outb(clk_config(0, CLK_OUTNM1), devpriv->iobase1 + PCI224_ZCLK_SCE);
 
 	comedi_8254_pacer_enable(dev->pacer, 2, 0, false);
 }

--
2.1.4

author
#2 lore raw

PATCH 2/2 staging: comedi: drivers: amplc_pci230: Convert macros to static inline functions

In reply to Bhaktipriya Shridhar

Convert macros CLK_CONFIG and GAT_CONFIG into static inline functions as static inline functions are preferred over macros. This change is possible since the arguments at all call sites have the same type. Also, both the macros have same type of arguments and return values. They have been renamed in lowecase.

This was done using coccinelle:

@r@
identifier f;
expression e;
@@
- #define f(chan, src) e
+ static inline unsigned int f(unsigned int chan,unsigned int src)
+ {
+ return e;
+ }

@r1@
@@

-GAT_CONFIG
+gat_config

@r2@
@@
-CLK_CONFIG
+clk_config

Signed-off-by Bhaktipriya Shridhar <[email protected]>
---
 drivers/staging/comedi/drivers/amplc_pci230.c | 38 +++++++++++++++------------
 1 file changed, 21 insertions(+), 17 deletions(-)
drivers/staging/comedi/drivers/amplc_pci230.c +21 -17
index 907c39c..051787e 100644
@@ -370,7 +370,9 @@
 #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 unsigned int CLK_CONFIG(unsigned int chan, unsigned int src) {
+	return ((((chan) & 3) << 3) | ((src) & 7));
+}
 
 /*
  * Counter/timer gate input configuration sources.

@@ -380,7 +382,9 @@
 #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 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:

@@ -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);
+	outb(clk_config(ct, clk_src), dev->iobase + PCI230_ZCLK_SCE);
 	/* Set initial count. */
 	if (count >= 65536)
 		count = 0;

@@ -1263,7 +1267,7 @@ 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(gat_config(1, GAT_VCC), dev->iobase + PCI230_ZGAT_SCE);
 		break;
 	case TRIG_INT:
 		async->inttrig = pci230_ao_inttrig_scan_begin;

@@ -1351,7 +1355,7 @@ 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(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);

@@ -1792,9 +1796,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 = gat_config(0, GAT_GND);
 		outb(zgat, dev->iobase + PCI230_ZGAT_SCE);
-		zgat = GAT_CONFIG(0, GAT_VCC);
+		zgat = gat_config(0, GAT_VCC);
 		outb(zgat, dev->iobase + PCI230_ZGAT_SCE);
 	}
 	spin_unlock_irqrestore(&devpriv->ai_stop_spinlock, irqflags);

@@ -1926,20 +1930,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 = gat_config(2, GAT_NOUTNM2);
 		} else {
 			/*
 			 * Conversion timer CT2 needs to be gated on
 			 * continuously.
 			 */
-			zgat = GAT_CONFIG(2, GAT_VCC);
+			zgat = 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 = gat_config(0, GAT_VCC);
 				break;
 			case TRIG_EXT:
 				/*

@@ -1950,21 +1954,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 = 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 = 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 = gat_config(0, GAT_VCC);
 				break;
 			}
 			outb(zgat, dev->iobase + PCI230_ZGAT_SCE);

@@ -1974,7 +1978,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 = gat_config(1, GAT_VCC);
 				outb(zgat, dev->iobase + PCI230_ZGAT_SCE);
 				break;
 			case TRIG_INT:

@@ -2216,7 +2220,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 = 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,

@@ -2234,7 +2238,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 = 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 *

@@ -2247,7 +2251,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 = 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

#3 lore raw

Re: [Outreachy kernel] [PATCH 2/2] staging: comedi: drivers: amplc_pci230: Convert macros to static inline functions

drivers/staging/comedi/drivers/amplc_pci230.c line 375

Quoted 13 lines · On Saturday 19 March 2016 01:16:32 Bhaktipriya Shridhar wrote:
drivers/staging/comedi/drivers/amplc_pci230.c +3 -1
index 907c39c..051787e 100644
@@ -370,7 +370,9 @@
 #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 unsigned int CLK_CONFIG(unsigned int chan, unsigned int src) {
+	return ((((chan) & 3) << 3) | ((src) & 7));
+}

We often use macros for cases like this, as this really just puts some bits in a particular position.

With the conversion above, you should remove the extra () around the arguments.

drivers/staging/comedi/drivers/amplc_pci230.c static void pci230_ct_setup_ns_mode(struct comedi_device *dev, unsigned int ct, · line 697

Quoted 10 lines
drivers/staging/comedi/drivers/amplc_pci230.c +1 -1
 /*

@@ -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);
+	outb(clk_config(ct, clk_src), dev->iobase + PCI230_ZCLK_SCE);
 	/* Set initial count. */
 	if (count >= 65536)
 		count = 0;

This won't work: you have changed the user from uppercase to lowercase, but not the definition. The change is ok, but you have to do it consistently.

It looks like you did not build-test this correctly, otherwise you would have run into the problem.

I also see that every single user of this macro passes the argument into the outb() function, with (dev->iobase + ...) as the address. I would suggest merging the two calls into one when you convert it, and do

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

which lets the caller above get simplified to

	pci230_clk_config(dev, PCI230_ZCLK_SCE, ct, clk_src);

	Arnd