lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20070510002740.c4350813.akpm@linux-foundation.org>
Date:	Thu, 10 May 2007 00:27:40 -0700
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Rodolfo Giometti <giometti@...eenne.com>
Cc:	linux-kernel@...r.kernel.org, linuxpps@...enneenne.com
Subject: Re: [PATCH 1/1] LinuxPPS: Pulse per Second support for Linux

On Wed, 2 May 2007 21:33:15 +0200 Rodolfo Giometti <giometti@...eenne.com> wrote:

> Pulse per Second (PPS) support for Linux.

Have a patch:



From: Andrew Morton <akpm@...ux-foundation.org>

Review comments:

- Running a timer once per second will make the super-low-power people upset.

- This uses netlink?  Is that interface documented anywhere?

  Please check with Dave Miller that this:

	#define NETLINK_PPSAPI          20

  reservation is OK.

- This:

	if ((nlpps->tsformat != PPS_TSFMT_TSPEC) != 0 ) {

  is weird.  I changed it to

	if (nlpps->tsformat != PPS_TSFMT_TSPEC) {


- This:

	timeout += nlpps->timeout.tv_nsec/(1000000000/HZ);

  probably won't work on i386.  We use do_div() for 64/32 divides.  I'll
  find out when I compile it.

  It's nice to use NSEC_PER_SEC rather than having to count all those
  zeroes.

- The code uses interruptible_sleep_on_timeout().  That API is deprecated
  and is racy.  Please convert to wait_event_interruptible_timeout().

  Ditto interruptible_sleep_on()

- This:

        memset(pps_source, 0, sizeof(struct pps_s) * PPS_MAX_SOURCES);

  was unneeded.  The C startup code already did that.

- All these separators:

+/* --- Input function ------------------------------------------------------ */

  aren't typical for kernel code.  I left them in, but please consider
  removing them all.

- This:

	static void pps_class_release(struct class_device *cdev)
	{
		/* Nop??? */
	}

  is a bug and it earns you a nastygram from Greg.  These objects must be
  dynamically allocated - this is not optional.

- What's this doing in 8250.c?

+	if (up->port.flags & UPF_HARDPPS_CD)
+		up->ier |= UART_IER_MSI;	/* enable interrupts */

  Please fully describe the reasons for this change in the changelog, and in
  a code comment and then get the change reviewed by Russell King
  <rmk@....linux.org.uk>.

- Please document within the changelog the other changes to the serial code
  and we'll ask Russell to take a look at those as well.

- The Kconfig purports to support CONFIG_PPS=m.  Does that actually work?

  We have a bunch of code in random other drivers which is dependent upon
  CONFIG_PPS_CLIENT_foo.  The problem is that if a kernel was compiled with
  CONFIG_PPS_CLIENT_foo=n and then the pps driver is later built for that
  kernel, it won't actually work because lp, serial etc weren't correctly
  configured when _they_ were built.

  This sort of cross-module coupling is considered to be a bad thing, but
  I'm not really sure it's all that important.

- Please split the patch up into a series of patches: one for pps core and
  one for each of the clients (servers?): one for lp, one for serial, etc.

  Try to arrange for that series of patches to build and run at each stage
  of application.

  Please don't lose my changes when you do so ;)

  Please review the changes I made and a) stick to the same style and b) fix
  up any sites which I missed.

- Please remove all the typedefs:

+typedef struct ntp_fp {
+typedef union pps_timeu {		
+typedef struct pps_info {
+typedef struct pps_params {

  and just use `struct ntp_fp' everywhere.

- The above four structures are communicated with userspace, yes?

  I believe that they will not work correctly when 32-bit userspace is
  communicating with a 64-bit kernel.  Alignments change and sizeof(long)
  changes.

  You don't want to have to write compat code.  I suggest that you redo
  those structures in terms of __u32, __u64, etc.  You probably need to use
  attribute((packed)) too, not sure.

  Then let's get that part carefully reviewed (Arnd Bergmann <arnd@...db.de>
  is my go-to guru on this) and please test it carefully.

  Yeah, you just haven't got a chance that something as huge and as complex
  as struct pps_netlink_msg will survive the 32->64 transition.

- Please ensure that `make headers_check' passes OK (you'll hear from me if
  it doesn't ;))

- Can we get rid of the private dbg, err and info macros?  Surely there are
  generic ones somewhere.

- struct pps_s has volatiles in it.  Please remove them.  There's lots of
  discussion on this topic on linux-kernel just today.

- Why did the

	port->icount.dcd++;

  get moved in uart_handle_dcd_change()?

- In lots of places you do:

+#ifdef CONFIG_PPS_CLIENT_UART
+#include <linux/pps.h>
+#endif

  Please remove the ifdefs at all these sites and make the header file
  handle it.

- It no longer compiles, because netlink_kernel_create now requires a
  `struct mutex *'.  I stuck a NULL in there, which is permitted.  But I don't
  know if that was a good thing - please check this.

  Please also chase the net guys with a pointy stick for failing to document
  their damned APIs.

- Generally: code looks OK and is probably useful.  Please keep going ;)


Code changes:

- fix docs a bit

- uninline lp_pps_echo(): we take its address.

- remove unneeded and undesirable casts of void*'s

- coding-style fixes

- various changes to the use of the timer API.

- Remove lots of inlinings.  The compiler gets this right.

- DEFINE_SPINLOCK is required: SPIN_LOCK_UNLOCKED defeats lockdep.

Cc: Rodolfo Giometti <giometti@...eenne.com>
Cc: john stultz <johnstul@...ibm.com>
Cc: Roman Zippel <zippel@...ux-m68k.org>
Signed-off-by: Andrew Morton <akpm@...ux-foundation.org>
---

 Documentation/pps.txt        |    4 -
 drivers/char/lp.c            |    7 +--
 drivers/pps/clients/Kconfig  |    8 +--
 drivers/pps/clients/ktimer.c |   19 +++-----
 drivers/pps/kapi.c           |   40 ++++++++---------
 drivers/pps/pps.c            |   75 +++++++++++----------------------
 drivers/pps/sysfs.c          |   14 +++---
 drivers/serial/8250.c        |    1 
 include/linux/pps.h          |    9 ++-
 9 files changed, 77 insertions(+), 100 deletions(-)

diff -puN Documentation/pps.txt~linuxpps-pulse-per-second-support-for-linux-fix Documentation/pps.txt
--- a/Documentation/pps.txt~linuxpps-pulse-per-second-support-for-linux-fix
+++ a/Documentation/pps.txt
@@ -49,7 +49,7 @@ problem:
 
 This implies that the source has a /dev/... entry. This assumption is
 ok for the serial and parallel port, where you can do something
-usefull besides(!) the gathering of timestamps as it is the central
+useful besides(!) the gathering of timestamps as it is the central
 task for a PPS-API. But this assumption does not work for a single
 purpose GPIO line. In this case even basic file-related functionality
 (like read() and write()) makes no sense at all and should not be a
@@ -167,7 +167,7 @@ inside you find several files:
 Inside each "assert" and "clear" file you can find the timestamp and a
 sequence number:
 
-    $ cat cat /sys/class/pps/00/assert
+    $ cat /sys/class/pps/00/assert
     1170026870.983207967#8
 
 Where before the "#" is the timestamp in seconds and after it is the
diff -puN drivers/char/lp.c~linuxpps-pulse-per-second-support-for-linux-fix drivers/char/lp.c
--- a/drivers/char/lp.c~linuxpps-pulse-per-second-support-for-linux-fix
+++ a/drivers/char/lp.c
@@ -750,9 +750,9 @@ static struct console lpcons = {
 
 #ifdef CONFIG_PPS_CLIENT_LP
 
-static inline void lp_pps_echo(int source, int event, void *data)
+static void lp_pps_echo(int source, int event, void *data)
 {
-	struct parport *port = (struct parport *) data;
+	struct parport *port = data;
 	unsigned char status = parport_read_status(port);
 
 	/* echo event via SEL bit */
@@ -860,8 +860,7 @@ static int lp_register(int nr, struct pa
 		else
 			info("PPS source #%d \"%s\" added to the system",
 					port->pps_source, port->pps_info.path);
-	}
-	else {
+	} else {
 		port->pps_source = -1;
 		err("PPS support disabled due port \"%s\" is in polling mode",
 			port->pps_info.path);
diff -puN drivers/pps/clients/Kconfig~linuxpps-pulse-per-second-support-for-linux-fix drivers/pps/clients/Kconfig
--- a/drivers/pps/clients/Kconfig~linuxpps-pulse-per-second-support-for-linux-fix
+++ a/drivers/pps/clients/Kconfig
@@ -16,21 +16,21 @@ config PPS_CLIENT_KTIMER
 	  will be called ktimer.o.
 
 comment "UART serial support (forced off)"
-	depends on ! ( SERIAL_CORE != n && ! ( PPS = m && SERIAL_CORE = y ) )
+	depends on ! (SERIAL_CORE != n && !(PPS = m && SERIAL_CORE = y))
 
 config PPS_CLIENT_UART
 	bool "UART serial support"
-	depends on SERIAL_CORE != n && ! ( PPS = m && SERIAL_CORE = y )
+	depends on SERIAL_CORE != n && !(PPS = m && SERIAL_CORE = y)
 	help
 	  If you say yes here you get support for a PPS source connected
 	  with the CD (Carrier Detect) pin of your serial port.
 
 comment "Parallel printer support (forced off)"
-	depends on ! ( PRINTER != n && ! ( PPS = m && PRINTER = y ) )
+	depends on !( PRINTER != n && !(PPS = m && PRINTER = y))
 
 config PPS_CLIENT_LP
 	bool "Parallel printer support"
-	depends on PRINTER != n && ! ( PPS = m && PRINTER = y )
+	depends on PRINTER != n && !(PPS = m && PRINTER = y)
 	help
 	  If you say yes here you get support for a PPS source connected
 	  with the interrupt pin of your parallel port.
diff -puN drivers/pps/clients/ktimer.c~linuxpps-pulse-per-second-support-for-linux-fix drivers/pps/clients/ktimer.c
--- a/drivers/pps/clients/ktimer.c~linuxpps-pulse-per-second-support-for-linux-fix
+++ a/drivers/pps/clients/ktimer.c
@@ -35,14 +35,13 @@ static struct timer_list ktimer;
 
 /* --- The kernel timer ----------------------------------------------------- */
 
-static void pps_ktimer_event(unsigned long ptr) {
+static void pps_ktimer_event(unsigned long ptr)
+{
 	info("PPS event at %lu", jiffies);
 
 	pps_event(source, PPS_CAPTUREASSERT, NULL);
 
-	/* Rescheduling */
-	ktimer.expires = jiffies+HZ;   /* 1 second */
-	add_timer(&ktimer);
+	mod_timer(&ktimer, jiffies + HZ);
 }
 
 /* --- The echo function ---------------------------------------------------- */
@@ -50,9 +49,9 @@ static void pps_ktimer_event(unsigned lo
 static void pps_ktimer_echo(int source, int event, void *data)
 {
 	info("echo %s %s for source %d",
-	 event&PPS_CAPTUREASSERT ? "assert" : "",
-	 event&PPS_CAPTURECLEAR ? "clear" : "",
-	 source);
+		event & PPS_CAPTUREASSERT ? "assert" : "",
+		event & PPS_CAPTURECLEAR ? "clear" : "",
+		source);
 }
 
 /* --- The PPS info struct -------------------------------------------------- */
@@ -88,10 +87,8 @@ static int __init pps_ktimer_init(void)
 	}
 	source = ret;
 
-	init_timer(&ktimer);
-	ktimer.function = pps_ktimer_event;
-	ktimer.expires = jiffies+HZ;   /* 1 second */
-	add_timer(&ktimer);
+	setup_timer(&ktimer, pps_ktimer_event, 0);
+	mod_timer(&ktimer, jiffies + HZ);
 
 	info("ktimer PPS source registered at %d", source);
 
diff -puN drivers/pps/kapi.c~linuxpps-pulse-per-second-support-for-linux-fix drivers/pps/kapi.c
--- a/drivers/pps/kapi.c~linuxpps-pulse-per-second-support-for-linux-fix
+++ a/drivers/pps/kapi.c
@@ -24,7 +24,6 @@
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/time.h>
-
 #include <linux/pps.h>
 
 /* --- Local functions ----------------------------------------------------- */
@@ -44,7 +43,8 @@ static void pps_add_offset(struct timesp
 
 /* --- Exported functions -------------------------------------------------- */
 
-static inline int __pps_register_source(struct pps_source_info_s *info, int default_params, int try_id)
+static int __pps_register_source(struct pps_source_info_s *info,
+				int default_params, int try_id)
 {
 	int i;
 
@@ -54,8 +54,7 @@ static inline int __pps_register_source(
 			return -EBUSY;
 		}
 		i = try_id;
-	}
-	else {
+	} else {
 		for (i = 0; i < PPS_MAX_SOURCES; i++)
 			if (!pps_is_allocated(i))
 				break;
@@ -66,15 +65,16 @@ static inline int __pps_register_source(
 	}
 
 	/* Sanity checks */
-	if ((info->mode&default_params) != default_params) {
+	if ((info->mode & default_params) != default_params) {
 		err("unsupported default parameters");
 		return -EINVAL;
 	}
-	if ((info->mode&(PPS_ECHOASSERT|PPS_ECHOCLEAR)) != 0 && info->echo == NULL) {
+	if ((info->mode & (PPS_ECHOASSERT|PPS_ECHOCLEAR)) != 0 &&
+			info->echo == NULL) {
 		err("echo function is not defined");
 		return -EINVAL;
 	}
-	if ((info->mode&(PPS_TSFMT_TSPEC|PPS_TSFMT_NTPFP)) == 0) {
+	if ((info->mode & (PPS_TSFMT_TSPEC|PPS_TSFMT_NTPFP)) == 0) {
 		err("unspecified time format");
 		return -EINVAL;
 	}
@@ -89,7 +89,8 @@ static inline int __pps_register_source(
 	return i;
 }
 
-int pps_register_source(struct pps_source_info_s *info, int default_params, int try_id)
+int pps_register_source(struct pps_source_info_s *info, int default_params,
+			int try_id)
 {
 	unsigned long flags;
 	int i, ret;
@@ -108,8 +109,9 @@ int pps_register_source(struct pps_sourc
 
 	return i;
 }
+EXPORT_SYMBOL(pps_register_source);
 
-static inline void __pps_unregister_source(struct pps_source_info_s *info)
+static void __pps_unregister_source(struct pps_source_info_s *info)
 {
 	int i;
 
@@ -136,6 +138,7 @@ void pps_unregister_source(struct pps_so
 	__pps_unregister_source(info);
 	spin_unlock_irqrestore(&pps_lock, flags);
 }
+EXPORT_SYMBOL(pps_unregister_source);
 
 void pps_event(int source, int event, void *data)
 {
@@ -153,21 +156,22 @@ void pps_event(int source, int event, vo
 		err("unknow source for event!");
 		return;
 	}
-	if ((event&(PPS_CAPTUREASSERT|PPS_CAPTURECLEAR)) == 0 ) {
+	if ((event & (PPS_CAPTUREASSERT|PPS_CAPTURECLEAR)) == 0 ) {
 		err("unknow event (%x) for source %d", event, source);
 		return;
 	}
 
 	/* Must call the echo function? */
-	if ((pps_source[source].params.mode&(PPS_ECHOASSERT|PPS_ECHOCLEAR)) != 0)
+	if ((pps_source[source].params.mode & (PPS_ECHOASSERT|PPS_ECHOCLEAR)) != 0)
 		pps_source[source].info->echo(source, event, data);
 
 	/* Check the event */
 	pps_source[source].current_mode = pps_source[source].params.mode;
-	if (event&PPS_CAPTUREASSERT) {
+	if (event & PPS_CAPTUREASSERT) {
 		/* We have to add an offset? */
 		if (pps_source[source].params.mode&PPS_OFFSETASSERT)
-			pps_add_offset(&ts, &pps_source[source].params.assert_off_tu.tspec);
+			pps_add_offset(&ts,
+				&pps_source[source].params.assert_off_tu.tspec);
 
 		/* Save the time stamp */
 		pps_source[source].assert_tu.tspec = ts;
@@ -175,10 +179,11 @@ void pps_event(int source, int event, vo
 		dbg("capture assert seq #%lu for source %d",
 			pps_source[source].assert_sequence, source);
 	}
-	if (event&PPS_CAPTURECLEAR) {
+	if (event & PPS_CAPTURECLEAR) {
 		/* We have to add an offset? */
 		if (pps_source[source].params.mode&PPS_OFFSETCLEAR)
-			pps_add_offset(&ts, &pps_source[source].params.clear_off_tu.tspec);
+			pps_add_offset(&ts,
+				&pps_source[source].params.clear_off_tu.tspec);
 
 		/* Save the time stamp */
 		pps_source[source].clear_tu.tspec = ts;
@@ -189,9 +194,4 @@ void pps_event(int source, int event, vo
 
 	wake_up_interruptible(&pps_source[source].queue);
 }
-
-/* --- Exported functions -------------------------------------------------- */
-
-EXPORT_SYMBOL(pps_register_source);
-EXPORT_SYMBOL(pps_unregister_source);
 EXPORT_SYMBOL(pps_event);
diff -puN drivers/pps/pps.c~linuxpps-pulse-per-second-support-for-linux-fix drivers/pps/pps.c
--- a/drivers/pps/pps.c~linuxpps-pulse-per-second-support-for-linux-fix
+++ a/drivers/pps/pps.c
@@ -31,7 +31,7 @@
 /* --- Global variables ---------------------------------------------------- */
 
 struct pps_s pps_source[PPS_MAX_SOURCES];
-spinlock_t pps_lock = SPIN_LOCK_UNLOCKED;
+DEFINE_SPINLOCK(pps_lock);
 
 /* --- Local variables ----------------------------------------------------- */
 
@@ -44,7 +44,7 @@ static inline int pps_check_source(int s
 	return (source < 0 || !pps_is_allocated(source)) ? -EINVAL : 0;
 }
 
-static inline int pps_find_source(int source)
+static int pps_find_source(int source)
 {
 	int i;
 
@@ -65,7 +65,7 @@ static inline int pps_find_source(int so
 	return i;
 }
 
-static inline int pps_find_path(char *path)
+static int pps_find_path(char *path)
 {
 	int i;
 
@@ -90,11 +90,9 @@ static void pps_nl_data_ready(struct soc
 	struct sk_buff *skb;
 	struct nlmsghdr *nlh;
 	struct pps_netlink_msg *nlpps;
-
 	int cmd, source, id;
 	wait_queue_head_t *queue;
 	unsigned long timeout;
-
 	int ret;
 
 	while ((skb = skb_dequeue(&sk->sk_receive_queue)) != NULL) {
@@ -108,7 +106,7 @@ static void pps_nl_data_ready(struct soc
 		source = nlpps->source;
 
 		switch (cmd) {
-		case PPS_CREATE : {
+		case PPS_CREATE:
 			dbg("PPS_CREATE: source %d", source);
 
 			/* Check if the requested source is allocated */
@@ -120,20 +118,16 @@ static void pps_nl_data_ready(struct soc
 
 			nlpps->source = ret;
 			nlpps->ret = 0;
-
 			break;
-		}
 
-		case PPS_DESTROY : {
+		case PPS_DESTROY:
 			dbg("PPS_DESTROY: source %d", source);
 
 			/* Nothing to do here! Just answer ok... */
 			nlpps->ret = 0;
-
 			break;
-		}
 
-		case PPS_SETPARMS : {
+		case PPS_SETPARMS:
 			dbg("PPS_SETPARMS: source %d", source);
 
 			/* Check the capabilities */
@@ -148,17 +142,17 @@ static void pps_nl_data_ready(struct soc
 				nlpps->ret = ret;
 				break;
 			}
-			if ((nlpps->params.mode&~pps_source[source].info->mode) != 0) {
+			if ((nlpps->params.mode & ~pps_source[source].info->mode) != 0) {
 				dbg("unsupported capabilities");
 				nlpps->ret = -EINVAL;
 				break;
 		 	}
-			if ((nlpps->params.mode&(PPS_CAPTUREASSERT|PPS_CAPTURECLEAR)) == 0) {
+			if ((nlpps->params.mode & (PPS_CAPTUREASSERT|PPS_CAPTURECLEAR)) == 0) {
 				dbg("capture mode unspecified");
 				nlpps->ret = -EINVAL;
 				break;
 			}
-			if ((nlpps->params.mode&(PPS_TSFMT_TSPEC|PPS_TSFMT_NTPFP)) == 0) {
+			if ((nlpps->params.mode & (PPS_TSFMT_TSPEC|PPS_TSFMT_NTPFP)) == 0) {
 				/* section 3.3 of RFC 2783 interpreted */
 				dbg("time format unspecified");
 				nlpps->params.mode |= PPS_TSFMT_TSPEC;
@@ -173,11 +167,9 @@ static void pps_nl_data_ready(struct soc
 			pps_source[source].params.api_version = PPS_API_VERS;
 
 			nlpps->ret = 0;
-
 			break;
-		}
 
-		case PPS_GETPARMS : {
+		case PPS_GETPARMS:
 			dbg("PPS_GETPARMS: source %d", source);
 
 			/* Sanity checks */
@@ -189,11 +181,9 @@ static void pps_nl_data_ready(struct soc
 
 			nlpps->params = pps_source[source].params;
 			nlpps->ret = 0;
-
 			break;
-	 	}
 
-	 	case PPS_GETCAP : {
+	 	case PPS_GETCAP:
 			dbg("PPS_GETCAP: source %d", source);
 
 			/* Sanity checks */
@@ -205,11 +195,9 @@ static void pps_nl_data_ready(struct soc
 
 			nlpps->mode = pps_source[source].info->mode;
 			nlpps->ret = 0;
-
 			break;
-	 	}
 
-	 	case PPS_FETCH : {
+	 	case PPS_FETCH:
 			dbg("PPS_FETCH: source %d", source);
 			queue = &pps_source[source].queue;
 
@@ -219,7 +207,7 @@ static void pps_nl_data_ready(struct soc
 				nlpps->ret = ret;
 				break;
 			}
-			if ((nlpps->tsformat != PPS_TSFMT_TSPEC) != 0 ) {
+			if (nlpps->tsformat != PPS_TSFMT_TSPEC) {
 				dbg("unsupported time format");
 				nlpps->ret = -EINVAL;
 				break;
@@ -227,8 +215,9 @@ static void pps_nl_data_ready(struct soc
 
 		 	/* Manage the timeout */
 			if (nlpps->timeout.tv_sec != -1) {
-				timeout = nlpps->timeout.tv_sec*HZ;
-				timeout += nlpps->timeout.tv_nsec/(1000000000/HZ);
+				timeout = nlpps->timeout.tv_sec * HZ;
+				timeout += nlpps->timeout.tv_nsec/
+						(NSEC_PER_SEC/HZ);
 
 				if (timeout != 0) {
 					timeout = interruptible_sleep_on_timeout(queue, timeout);
@@ -238,8 +227,7 @@ static void pps_nl_data_ready(struct soc
 						break;
 					}
 				}
-		 	}
-			else
+		 	} else
 				interruptible_sleep_on(queue);
 
 			/* Return the fetched timestamp */
@@ -250,19 +238,15 @@ static void pps_nl_data_ready(struct soc
 			nlpps->info.current_mode = pps_source[source].current_mode;
 
 			nlpps->ret = 0;
-
 			break;
-	 	}
 
-	 	case PPS_KC_BIND : {
+	 	case PPS_KC_BIND:
 			dbg("PPS_KC_BIND: source %d", source);
 			/* Feature currently not supported */
 			nlpps->ret = -EOPNOTSUPP;
-
 			break;
-	 	}
 
-		case PPS_FIND_SRC : {
+		case PPS_FIND_SRC:
 			dbg("PPS_FIND_SRC: source %d", source);
 			source = pps_find_source(source);
 			if (source < 0) {
@@ -278,11 +262,9 @@ static void pps_nl_data_ready(struct soc
 			strncpy(nlpps->path, pps_source[source].info->path,
 				PPS_MAX_NAME_LEN);
 			nlpps->ret = 0;
-
 			break;
-	 	}
 
-		case PPS_FIND_PATH : {
+		case PPS_FIND_PATH:
 			dbg("PPS_FIND_PATH: source %s", nlpps->path);
 			source = pps_find_path(nlpps->path);
 			if (source < 0) {
@@ -298,17 +280,14 @@ static void pps_nl_data_ready(struct soc
 			strncpy(nlpps->path, pps_source[source].info->path,
 				PPS_MAX_NAME_LEN);
 			nlpps->ret = 0;
-
 			break;
-	 	}
 
-		default : {
+		default:
 			/* Unknow command */
 			dbg("unknow command %d", nlpps->cmd);
 
 			nlpps->ret = -EINVAL;
 		}
-		}
 
 		/* Send an answer to the userland */
 		id = NETLINK_CB(skb).pid;
@@ -336,16 +315,13 @@ static int __init pps_init(void)
 	int ret;
 
 	nl_sk = netlink_kernel_create(NETLINK_PPSAPI, 0,
-					pps_nl_data_ready, THIS_MODULE);
+					pps_nl_data_ready, NULL, THIS_MODULE);
 	if (nl_sk == NULL) {
 		err("unable to create netlink kernel socket");
 		return -EBUSY;
 	}
 	dbg("netlink protocol %d created", NETLINK_PPSAPI);
 
-	/* Init the main struct */
-	memset(pps_source, 0, sizeof(struct pps_s)*PPS_MAX_SOURCES);
-
 	/* Register to sysfs */
 	ret = pps_sysfs_register();
 	if (ret < 0) {
@@ -354,11 +330,12 @@ static int __init pps_init(void)
 	}
 
 	info("LinuxPPS API ver. %d registered", PPS_API_VERS);
-	info("Software ver. %s - Copyright 2005-2007 Rodolfo Giometti <giometti@...ux.it>", PPS_VERSION);
+	info("Software ver. %s - Copyright 2005-2007 Rodolfo Giometti "
+		"<giometti@...ux.it>", PPS_VERSION);
 
-	return  0;
+	return 0;
 
-pps_sysfs_register_error :
+pps_sysfs_register_error:
 
 	sock_release(nl_sk->sk_socket);
 
diff -puN drivers/pps/sysfs.c~linuxpps-pulse-per-second-support-for-linux-fix drivers/pps/sysfs.c
--- a/drivers/pps/sysfs.c~linuxpps-pulse-per-second-support-for-linux-fix
+++ a/drivers/pps/sysfs.c
@@ -112,7 +112,8 @@ static struct class pps_class = {
 
 /* ----- Public functions --------------------------------------------- */
 
-void pps_sysfs_remove_source_entry(struct pps_source_info_s *info) {
+void pps_sysfs_remove_source_entry(struct pps_source_info_s *info)
+{
 	int i;
 
 	/* Sanity checks */
@@ -136,7 +137,8 @@ void pps_sysfs_remove_source_entry(struc
 	class_device_unregister(&info->class_dev);
 }
 
-int pps_sysfs_create_source_entry(struct pps_source_info_s *info, int id) {
+int pps_sysfs_create_source_entry(struct pps_source_info_s *info, int id)
+{
 	char buf[32];
 	int i, ret;
 
@@ -161,14 +163,14 @@ int pps_sysfs_create_source_entry(struct
 	/* Create info files */
 
 	/* Create file "assert" and "clear" according to source capability */
-	if (info->mode&PPS_CAPTUREASSERT) {
+	if (info->mode & PPS_CAPTUREASSERT) {
 		ret = class_device_create_file(&info->class_dev,
 					&pps_class_device_attributes[0]);
 		i = 0;
 		if (unlikely(ret))
 			goto error_class_device_create_file;
 	}
-	if (info->mode&PPS_CAPTURECLEAR) {
+	if (info->mode & PPS_CAPTURECLEAR) {
 		ret = class_device_create_file(&info->class_dev,
 					&pps_class_device_attributes[1]);
 		i = 1;
@@ -185,7 +187,7 @@ int pps_sysfs_create_source_entry(struct
 
 	return 0;
 
-error_class_device_create_file :
+error_class_device_create_file:
 	while (--i >= 0)
 		class_device_remove_file(&info->class_dev,
 					&pps_class_device_attributes[i]);
@@ -193,7 +195,7 @@ error_class_device_create_file :
 	class_device_unregister(&info->class_dev);
 	/* Here the  release() method was already called */
 
-error_class_device_register :
+error_class_device_register:
 
 	return ret;
 }
diff -puN drivers/serial/8250.c~linuxpps-pulse-per-second-support-for-linux-fix drivers/serial/8250.c
--- a/drivers/serial/8250.c~linuxpps-pulse-per-second-support-for-linux-fix
+++ a/drivers/serial/8250.c
@@ -2820,7 +2820,6 @@ void serial8250_unregister_port(int line
 	struct uart_8250_port *uart = &serial8250_ports[line];
 
 	mutex_lock(&serial_mutex);
-
 	uart_remove_one_port(&serial8250_reg, &uart->port);
 	if (serial8250_isa_devs) {
 		uart->port.flags &= ~UPF_BOOT_AUTOCONF;
diff -puN include/linux/pps.h~linuxpps-pulse-per-second-support-for-linux-fix include/linux/pps.h
--- a/include/linux/pps.h~linuxpps-pulse-per-second-support-for-linux-fix
+++ a/include/linux/pps.h
@@ -185,7 +185,8 @@ extern spinlock_t pps_lock;
 
 /* --- Global functions ---------------------------------------------------- */
 
-static inline int pps_is_allocated(int source) {
+static inline int pps_is_allocated(int source)
+{
 	return pps_source[source].info != NULL;
 }
 
@@ -193,11 +194,13 @@ static inline int pps_is_allocated(int s
 
 /* --- Exported functions -------------------------------------------------- */
 
-extern int pps_register_source(struct pps_source_info_s *info, int default_params, int try_id);
+extern int pps_register_source(struct pps_source_info_s *info,
+				int default_params, int try_id);
 extern void pps_unregister_source(struct pps_source_info_s *info);
 extern void pps_event(int source, int event, void *data);
 
-extern int pps_sysfs_create_source_entry(struct pps_source_info_s *info, int id);
+extern int pps_sysfs_create_source_entry(struct pps_source_info_s *info,
+				int id);
 extern void pps_sysfs_remove_source_entry(struct pps_source_info_s *info);
 extern int pps_sysfs_register(void);
 extern void pps_sysfs_unregister(void);
_

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ