can: avoid naming conflict in 2.6.26

PATCH In review Jan Kiszka 3 participants 6 messages Jul 4, 2008 – Aug 17, 2008

Share & tools

Jan Kiszka

author

2008-07-04 18:38 UTC #0 lore raw

PATCH 1/1 can: avoid naming conflict in 2.6.26

Due to a global definition of 'clock' in latest 2.6.26, we now have a problem with two rtcan drivers. Below is a fix that breaks those drivers' user interface by rename the conflicting variable which unfortunately also renames the module parameter name.

An alternative approach would be to add a kernel version dependent fix to the drivers. That would be required as compat_module_param_array cannot be extended to use a different parameter name than the related variable is called. Maybe someone sees a third, better solution, I do not.

Jan

drivers/can/sja1000/rtcan_isa.c +5 -5

Index: ksrc/drivers/can/sja1000/rtcan_isa.c
===================================================================
@@ -47,19 +47,19 @@ MODULE_LICENSE("GPL");
 
 static u16 io[RTCAN_ISA_MAX_DEV];
 static int irq[RTCAN_ISA_MAX_DEV];
-static u32 clock[RTCAN_ISA_MAX_DEV];
+static u32 clock_freq[RTCAN_ISA_MAX_DEV];
 static u8 ocr[RTCAN_ISA_MAX_DEV];
 static u8 cdr[RTCAN_ISA_MAX_DEV];
 
 compat_module_param_array(io, ushort, RTCAN_ISA_MAX_DEV, 0444);
 compat_module_param_array(irq, int, RTCAN_ISA_MAX_DEV, 0444);
-compat_module_param_array(clock, uint, RTCAN_ISA_MAX_DEV, 0444);
+compat_module_param_array(clock_freq, uint, RTCAN_ISA_MAX_DEV, 0444);
 compat_module_param_array(ocr, byte, RTCAN_ISA_MAX_DEV, 0444);
 compat_module_param_array(cdr, byte, RTCAN_ISA_MAX_DEV, 0444);
 
 MODULE_PARM_DESC(io, "The io-port address");
 MODULE_PARM_DESC(irq, "The interrupt number");
-MODULE_PARM_DESC(clock, "External clock frequency (default 16 MHz)");
+MODULE_PARM_DESC(clock_freq, "External clock frequency (default 16 MHz)");
 MODULE_PARM_DESC(ocr, "Value of output control register (default 0x1a)");
 MODULE_PARM_DESC(cdr, "Value of clock divider register (default 0xc8");

@@ -116,8 +116,8 @@ int __init rtcan_isa_init_one(int idx)
 	}
 
 	/* Clock frequency in Hz */
-	if (clock[idx])
-		dev->can_sys_clock = clock[idx] / 2;
+	if (clock_freq[idx])
+		dev->can_sys_clock = clock_freq[idx] / 2;
 	else
 		dev->can_sys_clock = 8000000; /* 16/2 MHz */

drivers/can/sja1000/rtcan_mem.c +5 -5

Index: ksrc/drivers/can/sja1000/rtcan_mem.c
===================================================================
@@ -55,19 +55,19 @@ MODULE_LICENSE("GPL");
 
 static u32 mem[RTCAN_MEM_MAX_DEV];
 static int irq[RTCAN_MEM_MAX_DEV];
-static u32 clock[RTCAN_MEM_MAX_DEV];
+static u32 clock_freq[RTCAN_MEM_MAX_DEV];
 static u8 ocr[RTCAN_MEM_MAX_DEV];
 static u8 cdr[RTCAN_MEM_MAX_DEV];
 
 compat_module_param_array(mem, uint, RTCAN_MEM_MAX_DEV, 0444);
 compat_module_param_array(irq, int, RTCAN_MEM_MAX_DEV, 0444);
-compat_module_param_array(clock, uint, RTCAN_MEM_MAX_DEV, 0444);
+compat_module_param_array(clock_freq, uint, RTCAN_MEM_MAX_DEV, 0444);
 compat_module_param_array(ocr, byte, RTCAN_MEM_MAX_DEV, 0444);
 compat_module_param_array(cdr, byte, RTCAN_MEM_MAX_DEV, 0444);
 
 MODULE_PARM_DESC(mem, "The io-memory address");
 MODULE_PARM_DESC(irq, "The interrupt number");
-MODULE_PARM_DESC(clock, "External clock frequency (default 16 MHz)");
+MODULE_PARM_DESC(clock_freq, "External clock frequency (default 16 MHz)");
 MODULE_PARM_DESC(ocr, "Value of output control register (default 0x1a)");
 MODULE_PARM_DESC(cdr, "Value of clock divider register (default 0xc8");

@@ -125,8 +125,8 @@ int __init rtcan_mem_init_one(int idx)
 	}
 
 	/* Clock frequency in Hz */
-	if (clock[idx])
-		dev->can_sys_clock = clock[idx] / 2;
+	if (clock_freq[idx])
+		dev->can_sys_clock = clock_freq[idx] / 2;
 	else
 		dev->can_sys_clock = 8000000; /* 16/2 MHz */

Wolfgang Grandegger

2008-07-07 07:32 UTC #1 lore raw

Hi Jan,

Quoted 10 lines · Jan Kiszka

Due to a global definition of 'clock' in latest 2.6.26, we now have a problem with two rtcan drivers. Below is a fix that breaks those drivers' user interface by rename the conflicting variable which unfortunately also renames the module parameter name.
An alternative approach would be to add a kernel version dependent fix to the drivers. That would be required as compat_module_param_array cannot be extended to use a different parameter name than the related variable is called. Maybe someone sees a third, better solution, I do not.

Nor do I. But I would prefer prefixing the module parameters with "can_". Who knows what comes next.

Wolfgang.

Jan Kiszka

author

2008-08-13 11:04 UTC #2 lore raw

Quoted 8 lines · Wolfgang Grandegger

Hi Jan,
Quoted 5 lines · Jan Kiszka
Due to a global definition of 'clock' in latest 2.6.26, we now have a problem with two rtcan drivers. Below is a fix that breaks those drivers' user interface by rename the conflicting variable which unfortunately also renames the module parameter name.

Quoted 8 lines

Quoted 5 lines
An alternative approach would be to add a kernel version dependent fix to the drivers. That would be required as compat_module_param_array cannot be extended to use a different parameter name than the related variable is called. Maybe someone sees a third, better solution, I do not.
Nor do I. But I would prefer prefixing the module parameters with "can_". Who knows what comes next.

The issue is still open. Wolfgang, will you take care?

Jan

Paul

2008-08-16 22:15 UTC #3 lore raw

Quoted 15 lines · Jan Kiszka

Quoted 14 lines · Wolfgang Grandegger
Hi Jan,
Quoted 10 lines · Jan Kiszka
Due to a global definition of 'clock' in latest 2.6.26, we now have a problem with two rtcan drivers. Below is a fix that breaks those drivers' user interface by rename the conflicting variable which unfortunately also renames the module parameter name.
An alternative approach would be to add a kernel version dependent fix to the drivers. That would be required as compat_module_param_array cannot be extended to use a different parameter name than the related variable is called. Maybe someone sees a third, better solution, I do not.

Quoted 6 lines

Quoted 2 lines
Nor do I. But I would prefer prefixing the module parameters with "can_". Who knows what comes next.
The issue is still open. Wolfgang, will you take care?
Jan

Just bringing the issue back to the top (had to fix locally before compiling 2.6.26.2).

Attached, a patch in line with Wolfgang's recommendation - Also includes a
trivial spelling correction.

Regards, Paul.

xeno-test.in +1 -1

Index: scripts/xeno-test.in
===================================================================
@@ -439,7 +439,7 @@ exit 0
 #will eventually finish, and should be restarted to keep the load up.
 #Figure out why killall didnt work properly.
 #
-#2. Much more testing.  Heres a weak start. You migth be better off
+#2. Much more testing.  Heres a weak start. You might be better off
 #using and improving test/test-xeno-test.rb.
 #
 ##!/bin/bash

drivers/can/sja1000/rtcan_isa.c +5 -5

Index: ksrc/drivers/can/sja1000/rtcan_isa.c
===================================================================
@@ -47,19 +47,19 @@ MODULE_LICENSE("GPL");
 
 static u16 io[RTCAN_ISA_MAX_DEV];
 static int irq[RTCAN_ISA_MAX_DEV];
-static u32 clock[RTCAN_ISA_MAX_DEV];
+static u32 can_clock[RTCAN_ISA_MAX_DEV];
 static u8 ocr[RTCAN_ISA_MAX_DEV];
 static u8 cdr[RTCAN_ISA_MAX_DEV];
 
 compat_module_param_array(io, ushort, RTCAN_ISA_MAX_DEV, 0444);
 compat_module_param_array(irq, int, RTCAN_ISA_MAX_DEV, 0444);
-compat_module_param_array(clock, uint, RTCAN_ISA_MAX_DEV, 0444);
+compat_module_param_array(can_clock, uint, RTCAN_ISA_MAX_DEV, 0444);
 compat_module_param_array(ocr, byte, RTCAN_ISA_MAX_DEV, 0444);
 compat_module_param_array(cdr, byte, RTCAN_ISA_MAX_DEV, 0444);
 
 MODULE_PARM_DESC(io, "The io-port address");
 MODULE_PARM_DESC(irq, "The interrupt number");
-MODULE_PARM_DESC(clock, "External clock frequency (default 16 MHz)");
+MODULE_PARM_DESC(can_clock, "External clock frequency (default 16 MHz)");
 MODULE_PARM_DESC(ocr, "Value of output control register (default 0x1a)");
 MODULE_PARM_DESC(cdr, "Value of clock divider register (default 0xc8");

@@ -116,8 +116,8 @@ int __init rtcan_isa_init_one(int idx)
 	}
 
 	/* Clock frequency in Hz */
-	if (clock[idx])
-		dev->can_sys_clock = clock[idx] / 2;
+	if (can_clock[idx])
+		dev->can_sys_clock = can_clock[idx] / 2;
 	else
 		dev->can_sys_clock = 8000000; /* 16/2 MHz */

drivers/can/sja1000/rtcan_mem.c +5 -5

Index: ksrc/drivers/can/sja1000/rtcan_mem.c
===================================================================
@@ -55,19 +55,19 @@ MODULE_LICENSE("GPL");
 
 static u32 mem[RTCAN_MEM_MAX_DEV];
 static int irq[RTCAN_MEM_MAX_DEV];
-static u32 clock[RTCAN_MEM_MAX_DEV];
+static u32 can_clock[RTCAN_MEM_MAX_DEV];
 static u8 ocr[RTCAN_MEM_MAX_DEV];
 static u8 cdr[RTCAN_MEM_MAX_DEV];
 
 compat_module_param_array(mem, uint, RTCAN_MEM_MAX_DEV, 0444);
 compat_module_param_array(irq, int, RTCAN_MEM_MAX_DEV, 0444);
-compat_module_param_array(clock, uint, RTCAN_MEM_MAX_DEV, 0444);
+compat_module_param_array(can_clock, uint, RTCAN_MEM_MAX_DEV, 0444);
 compat_module_param_array(ocr, byte, RTCAN_MEM_MAX_DEV, 0444);
 compat_module_param_array(cdr, byte, RTCAN_MEM_MAX_DEV, 0444);
 
 MODULE_PARM_DESC(mem, "The io-memory address");
 MODULE_PARM_DESC(irq, "The interrupt number");
-MODULE_PARM_DESC(clock, "External clock frequency (default 16 MHz)");
+MODULE_PARM_DESC(can_clock, "External clock frequency (default 16 MHz)");
 MODULE_PARM_DESC(ocr, "Value of output control register (default 0x1a)");
 MODULE_PARM_DESC(cdr, "Value of clock divider register (default 0xc8");

@@ -125,8 +125,8 @@ int __init rtcan_mem_init_one(int idx)
 	}
 
 	/* Clock frequency in Hz */
-	if (clock[idx])
-		dev->can_sys_clock = clock[idx] / 2;
+	if (can_clock[idx])
+		dev->can_sys_clock = can_clock[idx] / 2;
 	else
 		dev->can_sys_clock = 8000000; /* 16/2 MHz */

Wolfgang Grandegger

2008-08-17 07:41 UTC #4 lore raw

Quoted 10 lines · Paul

Quoted 9 lines · Jan Kiszka
Quoted 8 lines · Wolfgang Grandegger
Hi Jan,
Quoted 5 lines · Jan Kiszka
Due to a global definition of 'clock' in latest 2.6.26, we now have a problem with two rtcan drivers. Below is a fix that breaks those drivers' user interface by rename the conflicting variable which unfortunately also renames the module parameter name.

Quoted 8 lines

Quoted 8 lines
Quoted 7 lines
Quoted 5 lines
An alternative approach would be to add a kernel version dependent fix to the drivers. That would be required as compat_module_param_array cannot be extended to use a different parameter name than the related variable is called. Maybe someone sees a third, better solution, I do not.
Nor do I. But I would prefer prefixing the module parameters with "can_". Who knows what comes next.
The issue is still open. Wolfgang, will you take care?

Didn't find time yet to fix it, sorry.

Quoted 7 lines

Quoted 1 line
Jan
Just bringing the issue back to the top (had to fix locally before compiling 2.6.26.2).
Attached, a patch in line with Wolfgang's recommendation - Also includes a
trivial spelling correction.

Your patch looks good.

Thanks.

Wolfgang.

Jan Kiszka

author

2008-08-17 22:38 UTC #5 lore raw

Quoted 22 lines · Wolfgang Grandegger

Quoted 10 lines · Paul
Quoted 9 lines · Jan Kiszka
Quoted 8 lines · Wolfgang Grandegger
Hi Jan,
Quoted 5 lines · Jan Kiszka
Due to a global definition of 'clock' in latest 2.6.26, we now have a problem with two rtcan drivers. Below is a fix that breaks those drivers' user interface by rename the conflicting variable which unfortunately also renames the module parameter name.
Quoted 8 lines
Quoted 8 lines
Quoted 7 lines
Quoted 5 lines
An alternative approach would be to add a kernel version dependent fix to the drivers. That would be required as compat_module_param_array cannot be extended to use a different parameter name than the related variable is called. Maybe someone sees a third, better solution, I do not.
Nor do I. But I would prefer prefixing the module parameters with "can_". Who knows what comes next.
The issue is still open. Wolfgang, will you take care?
Didn't find time yet to fix it, sorry.

Quoted 9 lines

Quoted 6 lines
Quoted 1 line
Jan
Just bringing the issue back to the top (had to fix locally before compiling 2.6.26.2).
Attached, a patch in line with Wolfgang's recommendation - Also includes a
trivial spelling correction.
Your patch looks good.

OK, finally committed.

Jan