Linux Serial subsystem development
 help / color / mirror / Atom feed

* [PATCH] tty: Use raw spin lock to protect RX flip buffer
From: Ivo Sieben @ 2012-09-20 14:08 UTC (permalink / raw)
  To: Alan Cox, Greg KH, linux-serial, RT; +Cc: Ivo Sieben

The "normal" spin lock that guards the RX flip buffer is replaced by a raw
spin lock.

On a PREEMP_RT system this prevents unwanted scheduling overhead when data is
read at the same time as data is being received: while RX IRQ threaded handling
is busy a TTY read call is performed from a RT priority > threaded IRQ priority.
The read call tries to take the flip buffer spin lock (held by the threaded IRQ)
which blocks and causes a context switch to/from the threaded IRQ handler until
the spin lock is unlocked.

On a 240 MHz AT91SAM9261 processor setup this fixes about 100us of scheduling
overhead on the TTY read call.

Signed-off-by: Ivo Sieben <meltedpianoman@gmail.com>
---
 drivers/tty/tty_buffer.c |   44 ++++++++++++++++++++++----------------------
 include/linux/kbd_kern.h |    4 ++--
 include/linux/tty.h      |    2 +-
 3 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index b8e90b8..fd4c242 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -135,20 +135,20 @@ static void __tty_buffer_flush(struct tty_struct *tty)
 void tty_buffer_flush(struct tty_struct *tty)
 {
 	unsigned long flags;
-	spin_lock_irqsave(&tty->buf.lock, flags);
+	raw_spin_lock_irqsave(&tty->buf.lock, flags);
 
 	/* If the data is being pushed to the tty layer then we can't
 	   process it here. Instead set a flag and the flush_to_ldisc
 	   path will process the flush request before it exits */
 	if (test_bit(TTY_FLUSHING, &tty->flags)) {
 		set_bit(TTY_FLUSHPENDING, &tty->flags);
-		spin_unlock_irqrestore(&tty->buf.lock, flags);
+		raw_spin_unlock_irqrestore(&tty->buf.lock, flags);
 		wait_event(tty->read_wait,
 				test_bit(TTY_FLUSHPENDING, &tty->flags) == 0);
 		return;
 	} else
 		__tty_buffer_flush(tty);
-	spin_unlock_irqrestore(&tty->buf.lock, flags);
+	raw_spin_unlock_irqrestore(&tty->buf.lock, flags);
 }
 
 /**
@@ -238,9 +238,9 @@ int tty_buffer_request_room(struct tty_struct *tty, size_t size)
 	unsigned long flags;
 	int length;
 
-	spin_lock_irqsave(&tty->buf.lock, flags);
+	raw_spin_lock_irqsave(&tty->buf.lock, flags);
 	length = __tty_buffer_request_room(tty, size);
-	spin_unlock_irqrestore(&tty->buf.lock, flags);
+	raw_spin_unlock_irqrestore(&tty->buf.lock, flags);
 	return length;
 }
 EXPORT_SYMBOL_GPL(tty_buffer_request_room);
@@ -268,18 +268,18 @@ int tty_insert_flip_string_fixed_flag(struct tty_struct *tty,
 		unsigned long flags;
 		struct tty_buffer *tb;
 
-		spin_lock_irqsave(&tty->buf.lock, flags);
+		raw_spin_lock_irqsave(&tty->buf.lock, flags);
 		space = __tty_buffer_request_room(tty, goal);
 		tb = tty->buf.tail;
 		/* If there is no space then tb may be NULL */
 		if (unlikely(space == 0)) {
-			spin_unlock_irqrestore(&tty->buf.lock, flags);
+			raw_spin_unlock_irqrestore(&tty->buf.lock, flags);
 			break;
 		}
 		memcpy(tb->char_buf_ptr + tb->used, chars, space);
 		memset(tb->flag_buf_ptr + tb->used, flag, space);
 		tb->used += space;
-		spin_unlock_irqrestore(&tty->buf.lock, flags);
+		raw_spin_unlock_irqrestore(&tty->buf.lock, flags);
 		copied += space;
 		chars += space;
 		/* There is a small chance that we need to split the data over
@@ -313,18 +313,18 @@ int tty_insert_flip_string_flags(struct tty_struct *tty,
 		unsigned long __flags;
 		struct tty_buffer *tb;
 
-		spin_lock_irqsave(&tty->buf.lock, __flags);
+		raw_spin_lock_irqsave(&tty->buf.lock, __flags);
 		space = __tty_buffer_request_room(tty, goal);
 		tb = tty->buf.tail;
 		/* If there is no space then tb may be NULL */
 		if (unlikely(space == 0)) {
-			spin_unlock_irqrestore(&tty->buf.lock, __flags);
+			raw_spin_unlock_irqrestore(&tty->buf.lock, __flags);
 			break;
 		}
 		memcpy(tb->char_buf_ptr + tb->used, chars, space);
 		memcpy(tb->flag_buf_ptr + tb->used, flags, space);
 		tb->used += space;
-		spin_unlock_irqrestore(&tty->buf.lock, __flags);
+		raw_spin_unlock_irqrestore(&tty->buf.lock, __flags);
 		copied += space;
 		chars += space;
 		flags += space;
@@ -357,7 +357,7 @@ int tty_prepare_flip_string(struct tty_struct *tty, unsigned char **chars,
 	unsigned long flags;
 	struct tty_buffer *tb;
 
-	spin_lock_irqsave(&tty->buf.lock, flags);
+	raw_spin_lock_irqsave(&tty->buf.lock, flags);
 	space = __tty_buffer_request_room(tty, size);
 
 	tb = tty->buf.tail;
@@ -366,7 +366,7 @@ int tty_prepare_flip_string(struct tty_struct *tty, unsigned char **chars,
 		memset(tb->flag_buf_ptr + tb->used, TTY_NORMAL, space);
 		tb->used += space;
 	}
-	spin_unlock_irqrestore(&tty->buf.lock, flags);
+	raw_spin_unlock_irqrestore(&tty->buf.lock, flags);
 	return space;
 }
 EXPORT_SYMBOL_GPL(tty_prepare_flip_string);
@@ -394,7 +394,7 @@ int tty_prepare_flip_string_flags(struct tty_struct *tty,
 	unsigned long __flags;
 	struct tty_buffer *tb;
 
-	spin_lock_irqsave(&tty->buf.lock, __flags);
+	raw_spin_lock_irqsave(&tty->buf.lock, __flags);
 	space = __tty_buffer_request_room(tty, size);
 
 	tb = tty->buf.tail;
@@ -403,7 +403,7 @@ int tty_prepare_flip_string_flags(struct tty_struct *tty,
 		*flags = tb->flag_buf_ptr + tb->used;
 		tb->used += space;
 	}
-	spin_unlock_irqrestore(&tty->buf.lock, __flags);
+	raw_spin_unlock_irqrestore(&tty->buf.lock, __flags);
 	return space;
 }
 EXPORT_SYMBOL_GPL(tty_prepare_flip_string_flags);
@@ -433,7 +433,7 @@ static void flush_to_ldisc(struct work_struct *work)
 	if (disc == NULL)	/*  !TTY_LDISC */
 		return;
 
-	spin_lock_irqsave(&tty->buf.lock, flags);
+	raw_spin_lock_irqsave(&tty->buf.lock, flags);
 
 	if (!test_and_set_bit(TTY_FLUSHING, &tty->flags)) {
 		struct tty_buffer *head;
@@ -462,10 +462,10 @@ static void flush_to_ldisc(struct work_struct *work)
 			char_buf = head->char_buf_ptr + head->read;
 			flag_buf = head->flag_buf_ptr + head->read;
 			head->read += count;
-			spin_unlock_irqrestore(&tty->buf.lock, flags);
+			raw_spin_unlock_irqrestore(&tty->buf.lock, flags);
 			disc->ops->receive_buf(tty, char_buf,
 							flag_buf, count);
-			spin_lock_irqsave(&tty->buf.lock, flags);
+			raw_spin_lock_irqsave(&tty->buf.lock, flags);
 		}
 		clear_bit(TTY_FLUSHING, &tty->flags);
 	}
@@ -477,7 +477,7 @@ static void flush_to_ldisc(struct work_struct *work)
 		clear_bit(TTY_FLUSHPENDING, &tty->flags);
 		wake_up(&tty->read_wait);
 	}
-	spin_unlock_irqrestore(&tty->buf.lock, flags);
+	raw_spin_unlock_irqrestore(&tty->buf.lock, flags);
 
 	tty_ldisc_deref(disc);
 }
@@ -519,10 +519,10 @@ void tty_flush_to_ldisc(struct tty_struct *tty)
 void tty_flip_buffer_push(struct tty_struct *tty)
 {
 	unsigned long flags;
-	spin_lock_irqsave(&tty->buf.lock, flags);
+	raw_spin_lock_irqsave(&tty->buf.lock, flags);
 	if (tty->buf.tail != NULL)
 		tty->buf.tail->commit = tty->buf.tail->used;
-	spin_unlock_irqrestore(&tty->buf.lock, flags);
+	raw_spin_unlock_irqrestore(&tty->buf.lock, flags);
 
 	if (tty->low_latency)
 		flush_to_ldisc(&tty->buf.work);
@@ -543,7 +543,7 @@ EXPORT_SYMBOL(tty_flip_buffer_push);
 
 void tty_buffer_init(struct tty_struct *tty)
 {
-	spin_lock_init(&tty->buf.lock);
+	raw_spin_lock_init(&tty->buf.lock);
 	tty->buf.head = NULL;
 	tty->buf.tail = NULL;
 	tty->buf.free = NULL;
diff --git a/include/linux/kbd_kern.h b/include/linux/kbd_kern.h
index daf4a3a..507af24 100644
--- a/include/linux/kbd_kern.h
+++ b/include/linux/kbd_kern.h
@@ -150,10 +150,10 @@ extern unsigned int keymap_count;
 static inline void con_schedule_flip(struct tty_struct *t)
 {
 	unsigned long flags;
-	spin_lock_irqsave(&t->buf.lock, flags);
+	raw_spin_lock_irqsave(&t->buf.lock, flags);
 	if (t->buf.tail != NULL)
 		t->buf.tail->commit = t->buf.tail->used;
-	spin_unlock_irqrestore(&t->buf.lock, flags);
+	raw_spin_unlock_irqrestore(&t->buf.lock, flags);
 	schedule_work(&t->buf.work);
 }
 
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 9f47ab5..e728ecc 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -86,7 +86,7 @@ struct tty_buffer {
 
 struct tty_bufhead {
 	struct work_struct work;
-	spinlock_t lock;
+	raw_spinlock_t lock;
 	struct tty_buffer *head;	/* Queue head */
 	struct tty_buffer *tail;	/* Active buffer */
 	struct tty_buffer *free;	/* Free queue head */
-- 
1.7.9.5



^ permalink raw reply related

* [PATCH] tty: Use raw spin lock to protect the TTY read section
From: Ivo Sieben @ 2012-09-20 14:09 UTC (permalink / raw)
  To: Alan Cox, Greg KH, linux-serial, RT; +Cc: Ivo Sieben

The "normal" spin lock that guards the N_TTY line discipline read section
is replaced by a raw spin lock.

On a PREEMP_RT system this prevents unwanted scheduling overhead when data is
read at the same time as data is being received: while RX IRQ threaded handling
is busy a TTY read call is performed from a RT priority > threaded IRQ priority.
The read call tries to take the read section spin lock (held by the threaded
IRQ) which blocks and causes a context switch to/from the threaded IRQ handler
until the spin lock is unlocked.

On a 240 MHz AT91SAM9261 processor setup this fixes about 100us of scheduling
overhead on the TTY read call.

Signed-off-by: Ivo Sieben <meltedpianoman@gmail.com>
---
 drivers/tty/n_tty.c  |   44 ++++++++++++++++++++++----------------------
 drivers/tty/tty_io.c |    2 +-
 include/linux/tty.h  |    2 +-
 3 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index ee1c268..743ef45 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -138,9 +138,9 @@ static void put_tty_queue(unsigned char c, struct tty_struct *tty)
 	 *	The problem of stomping on the buffers ends here.
 	 *	Why didn't anyone see this one coming? --AJK
 	*/
-	spin_lock_irqsave(&tty->read_lock, flags);
+	raw_spin_lock_irqsave(&tty->read_lock, flags);
 	put_tty_queue_nolock(c, tty);
-	spin_unlock_irqrestore(&tty->read_lock, flags);
+	raw_spin_unlock_irqrestore(&tty->read_lock, flags);
 }
 
 /**
@@ -173,9 +173,9 @@ static void reset_buffer_flags(struct tty_struct *tty)
 {
 	unsigned long flags;
 
-	spin_lock_irqsave(&tty->read_lock, flags);
+	raw_spin_lock_irqsave(&tty->read_lock, flags);
 	tty->read_head = tty->read_tail = tty->read_cnt = 0;
-	spin_unlock_irqrestore(&tty->read_lock, flags);
+	raw_spin_unlock_irqrestore(&tty->read_lock, flags);
 
 	mutex_lock(&tty->echo_lock);
 	tty->echo_pos = tty->echo_cnt = tty->echo_overrun = 0;
@@ -230,7 +230,7 @@ static ssize_t n_tty_chars_in_buffer(struct tty_struct *tty)
 	unsigned long flags;
 	ssize_t n = 0;
 
-	spin_lock_irqsave(&tty->read_lock, flags);
+	raw_spin_lock_irqsave(&tty->read_lock, flags);
 	if (!tty->icanon) {
 		n = tty->read_cnt;
 	} else if (tty->canon_data) {
@@ -238,7 +238,7 @@ static ssize_t n_tty_chars_in_buffer(struct tty_struct *tty)
 			tty->canon_head - tty->read_tail :
 			tty->canon_head + (N_TTY_BUF_SIZE - tty->read_tail);
 	}
-	spin_unlock_irqrestore(&tty->read_lock, flags);
+	raw_spin_unlock_irqrestore(&tty->read_lock, flags);
 	return n;
 }
 
@@ -868,19 +868,19 @@ static void eraser(unsigned char c, struct tty_struct *tty)
 		kill_type = WERASE;
 	else {
 		if (!L_ECHO(tty)) {
-			spin_lock_irqsave(&tty->read_lock, flags);
+			raw_spin_lock_irqsave(&tty->read_lock, flags);
 			tty->read_cnt -= ((tty->read_head - tty->canon_head) &
 					  (N_TTY_BUF_SIZE - 1));
 			tty->read_head = tty->canon_head;
-			spin_unlock_irqrestore(&tty->read_lock, flags);
+			raw_spin_unlock_irqrestore(&tty->read_lock, flags);
 			return;
 		}
 		if (!L_ECHOK(tty) || !L_ECHOKE(tty) || !L_ECHOE(tty)) {
-			spin_lock_irqsave(&tty->read_lock, flags);
+			raw_spin_lock_irqsave(&tty->read_lock, flags);
 			tty->read_cnt -= ((tty->read_head - tty->canon_head) &
 					  (N_TTY_BUF_SIZE - 1));
 			tty->read_head = tty->canon_head;
-			spin_unlock_irqrestore(&tty->read_lock, flags);
+			raw_spin_unlock_irqrestore(&tty->read_lock, flags);
 			finish_erasing(tty);
 			echo_char(KILL_CHAR(tty), tty);
 			/* Add a newline if ECHOK is on and ECHOKE is off. */
@@ -914,10 +914,10 @@ static void eraser(unsigned char c, struct tty_struct *tty)
 				break;
 		}
 		cnt = (tty->read_head - head) & (N_TTY_BUF_SIZE-1);
-		spin_lock_irqsave(&tty->read_lock, flags);
+		raw_spin_lock_irqsave(&tty->read_lock, flags);
 		tty->read_head = head;
 		tty->read_cnt -= cnt;
-		spin_unlock_irqrestore(&tty->read_lock, flags);
+		raw_spin_unlock_irqrestore(&tty->read_lock, flags);
 		if (L_ECHO(tty)) {
 			if (L_ECHOPRT(tty)) {
 				if (!tty->erasing) {
@@ -1290,12 +1290,12 @@ send_signal:
 				put_tty_queue(c, tty);
 
 handle_newline:
-			spin_lock_irqsave(&tty->read_lock, flags);
+			raw_spin_lock_irqsave(&tty->read_lock, flags);
 			set_bit(tty->read_head, tty->read_flags);
 			put_tty_queue_nolock(c, tty);
 			tty->canon_head = tty->read_head;
 			tty->canon_data++;
-			spin_unlock_irqrestore(&tty->read_lock, flags);
+			raw_spin_unlock_irqrestore(&tty->read_lock, flags);
 			kill_fasync(&tty->fasync, SIGIO, POLL_IN);
 			if (waitqueue_active(&tty->read_wait))
 				wake_up_interruptible(&tty->read_wait);
@@ -1371,7 +1371,7 @@ static void n_tty_receive_buf(struct tty_struct *tty, const unsigned char *cp,
 		return;
 
 	if (tty->real_raw) {
-		spin_lock_irqsave(&tty->read_lock, cpuflags);
+		raw_spin_lock_irqsave(&tty->read_lock, cpuflags);
 		i = min(N_TTY_BUF_SIZE - tty->read_cnt,
 			N_TTY_BUF_SIZE - tty->read_head);
 		i = min(count, i);
@@ -1387,7 +1387,7 @@ static void n_tty_receive_buf(struct tty_struct *tty, const unsigned char *cp,
 		memcpy(tty->read_buf + tty->read_head, cp, i);
 		tty->read_head = (tty->read_head + i) & (N_TTY_BUF_SIZE-1);
 		tty->read_cnt += i;
-		spin_unlock_irqrestore(&tty->read_lock, cpuflags);
+		raw_spin_unlock_irqrestore(&tty->read_lock, cpuflags);
 	} else {
 		for (i = count, p = cp, f = fp; i; i--, p++) {
 			if (f)
@@ -1633,23 +1633,23 @@ static int copy_from_read_buf(struct tty_struct *tty,
 	bool is_eof;
 
 	retval = 0;
-	spin_lock_irqsave(&tty->read_lock, flags);
+	raw_spin_lock_irqsave(&tty->read_lock, flags);
 	n = min(tty->read_cnt, N_TTY_BUF_SIZE - tty->read_tail);
 	n = min(*nr, n);
-	spin_unlock_irqrestore(&tty->read_lock, flags);
+	raw_spin_unlock_irqrestore(&tty->read_lock, flags);
 	if (n) {
 		retval = copy_to_user(*b, &tty->read_buf[tty->read_tail], n);
 		n -= retval;
 		is_eof = n == 1 &&
 			tty->read_buf[tty->read_tail] == EOF_CHAR(tty);
 		tty_audit_add_data(tty, &tty->read_buf[tty->read_tail], n);
-		spin_lock_irqsave(&tty->read_lock, flags);
+		raw_spin_lock_irqsave(&tty->read_lock, flags);
 		tty->read_tail = (tty->read_tail + n) & (N_TTY_BUF_SIZE-1);
 		tty->read_cnt -= n;
 		/* Turn single EOF into zero-length read */
 		if (L_EXTPROC(tty) && tty->icanon && is_eof && !tty->read_cnt)
 			n = 0;
-		spin_unlock_irqrestore(&tty->read_lock, flags);
+		raw_spin_unlock_irqrestore(&tty->read_lock, flags);
 		*b += n;
 		*nr -= n;
 	}
@@ -1838,7 +1838,7 @@ do_it_again:
 				eol = test_and_clear_bit(tty->read_tail,
 						tty->read_flags);
 				c = tty->read_buf[tty->read_tail];
-				spin_lock_irqsave(&tty->read_lock, flags);
+				raw_spin_lock_irqsave(&tty->read_lock, flags);
 				tty->read_tail = ((tty->read_tail+1) &
 						  (N_TTY_BUF_SIZE-1));
 				tty->read_cnt--;
@@ -1850,7 +1850,7 @@ do_it_again:
 					if (--tty->canon_data < 0)
 						tty->canon_data = 0;
 				}
-				spin_unlock_irqrestore(&tty->read_lock, flags);
+				raw_spin_unlock_irqrestore(&tty->read_lock, flags);
 
 				if (!eol || (c != __DISABLED_CHAR)) {
 					if (tty_put_user(tty, c, b++)) {
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index b425c79..a3ef00b 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -2943,7 +2943,7 @@ void initialize_tty_struct(struct tty_struct *tty,
 	mutex_init(&tty->atomic_write_lock);
 	mutex_init(&tty->output_lock);
 	mutex_init(&tty->echo_lock);
-	spin_lock_init(&tty->read_lock);
+	raw_spin_lock_init(&tty->read_lock);
 	spin_lock_init(&tty->ctrl_lock);
 	INIT_LIST_HEAD(&tty->tty_files);
 	INIT_WORK(&tty->SAK_work, do_SAK_work);
diff --git a/include/linux/tty.h b/include/linux/tty.h
index e728ecc..21bceef 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -327,7 +327,7 @@ struct tty_struct {
 	struct mutex echo_lock;
 	unsigned char *write_buf;
 	int write_cnt;
-	spinlock_t read_lock;
+	raw_spinlock_t read_lock;
 	/* If the tty has a pending do_SAK, queue it here - akpm */
 	struct work_struct SAK_work;
 	struct tty_port *port;
-- 
1.7.9.5



^ permalink raw reply related

* Re: [PATCH] tty: prevent unnecessary work queue lock checking on flip buffer copy
From: Alan Cox @ 2012-09-20 16:33 UTC (permalink / raw)
  To: Ivo Sieben; +Cc: Greg KH, linux-serial, RT
In-Reply-To: <1348149762-4596-1-git-send-email-meltedpianoman@gmail.com>

> This prevents unnecessary spin lock/unlock on the workqueue spin lock
> that can cause additional scheduling overhead on a PREEMPT_RT system.
> On a 240 MHz AT91SAM9261 processor setup this fixes about 100us of
> scheduling overhead on the TTY read call.

This seems reasonable, but given its also relevant for upstream it
would be nice to get it without the ifdef in upstream.

The corner case is when the tty->low_latency flag is flipped but the
drivers should handle that gracefully and if not we should fix them so
you can get your 100uS.

Alan

^ permalink raw reply

* Re: [PATCH] tty: cleanup duplicate functions in tty_buffer
From: Alan Cox @ 2012-09-20 16:17 UTC (permalink / raw)
  To: Ivo Sieben; +Cc: Greg KH, linux-serial, RT, Jiri Slaby, Heiko Carstens
In-Reply-To: <1348149223-4281-1-git-send-email-meltedpianoman@gmail.com>

> Therefor the the tty_scedule_flip() function and replaced it
> everywhere with the tty_flip_buffer_push() beause that function is
> used the most by serial drivers and implements the work queue bypass
> mechanism.

So now when the user sets tty->low_latency on these devices the machine
crashes ?

This is the wrong direction - in fact we have a pile we need to move
the other way !

If they resolve to the same thing in hard RT patches fine, that's a
different question.

Alan

^ permalink raw reply

* Re: [PATCH] tty: Use raw spin lock to protect RX flip buffer
From: Alan Cox @ 2012-09-20 16:37 UTC (permalink / raw)
  To: Ivo Sieben; +Cc: Greg KH, linux-serial, RT
In-Reply-To: <1348150080-4794-1-git-send-email-meltedpianoman@gmail.com>

On Thu, 20 Sep 2012 16:08:00 +0200
Ivo Sieben <meltedpianoman@gmail.com> wrote:

> The "normal" spin lock that guards the RX flip buffer is replaced by
> a raw spin lock.

Our current goal is to get the buffers into the tty_port structure so
we can try and get rid of some of the locks entirely. We have lockless
kfifos and the port lifetime is the device lifetime plus so hopefully
it'll go away entirely eventually.

In the meantime I've no real opinion on the spin/raw_spin choices as
it's basically an RT question not a tty one so if the rt folks are
happy with it so am I.


Alan

^ permalink raw reply

* [PATCH updated 07/11] tty/serial: Add kgdb_nmi driver
From: Anton Vorontsov @ 2012-09-20 18:30 UTC (permalink / raw)
  To: Jason Wessel
  Cc: Andrew Morton, Russell King, Greg Kroah-Hartman, Alan Cox,
	Arve Hjønnevåg, Colin Cross, Brian Swetland,
	John Stultz, Thomas Gleixner, linux-kernel, linux-arm-kernel,
	linaro-kernel, patches, kernel-team, kgdb-bugreport, linux-serial
In-Reply-To: <505B20C6.5090308@windriver.com>

This special driver makes it possible to temporary use NMI debugger port
as a normal console by issuing 'nmi_console' command (assuming that the
port is attached to KGDB).

Unlike KDB's disable_nmi command, with this driver you are always able
to go back to the debugger using KGDB escape sequence ($3#33).  This is
because this console driver processes the input in NMI context, and thus
is able to intercept the magic sequence.

Note that since the console interprets input and uses polling
communication methods, for things like PPP it is still better to fully
detach debugger port from the KGDB NMI (i.e. disable_nmi), and use raw
console.

Usually, to enter the debugger one have to type the magic sequence, so
initially the kernel will print the following prompt on the NMI debugger
console:

	Type $3#33 to enter the debugger>

For convenience, there is a kgdb_fiq.knock kernel command line option,
when set to 0, this turns the special command to just a return key
press, so the kernel will be printing this:

	Hit <return> to enter the debugger>

This is more convenient for long debugging sessions, although it makes
nmi_console feature somewhat useless.

And for the cases when NMI connected to a dedicated button, the knocking
can be disabled altogether by setting kgdb_fiq.knock to -1.

Suggested-by: Colin Cross <ccross@android.com>
Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
Acked-by: Alan Cox <alan@linux.intel.com>
---

On Thu, Sep 20, 2012 at 08:57:26AM -0500, Jason Wessel wrote:
> On 09/19/2012 06:41 PM, Anton Vorontsov wrote:
> > This special driver makes it possible to temporary use NMI debugger port
> > as a normal console by issuing 'nmi_console' command (assuming that the
> > port is attached to KGDB).
> > 
> 
> 
> The kgdb regression compiler also does checkpatch, so the "check patch
> police" don't come chasing us let's get rid of this warning.  This
> comes with the disclaimer that I believe checkpatch is a bit overly
> agressive in the first place. :-)

Yup, I saw that warning, but decided to ignore as it was not
legitimate.

But sure, I'm fine either way, let's silence it.

[....]
> I have the kgdb regression builder running right now (it will be done in about 1 hour), and so far it picked up one new warning.
> 
> drivers/tty/serial/kgdb_nmi.c: In function 'kgdb_nmi_poll_one_knock':
> drivers/tty/serial/kgdb_nmi.c:161: warning: field width should have type 'int', but argument 4 has type 'size_t'

Ouch, 64 bit. This is something I forgot to test, as I work mostly with
32 bit ARM builds.

This small hunk fixes it up:

 -                  kgdb_nmi_knock ? magic  : "<return>", m, "");
 +                  kgdb_nmi_knock ? magic  : "<return>", (int)m, "");
 
Thanks for catching this!

These two fixups included in the patch down below.

 drivers/tty/serial/Kconfig    |  19 ++
 drivers/tty/serial/Makefile   |   1 +
 drivers/tty/serial/kgdb_nmi.c | 402 ++++++++++++++++++++++++++++++++++++++++++
 drivers/tty/serial/kgdboc.c   |   9 +
 include/linux/kgdb.h          |  10 ++
 5 files changed, 441 insertions(+)
 create mode 100644 drivers/tty/serial/kgdb_nmi.c

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 2a9659d..233fbaa 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -141,6 +141,25 @@ config SERIAL_ATMEL_TTYAT
 
 	  Say Y if you have an external 8250/16C550 UART.  If unsure, say N.
 
+config SERIAL_KGDB_NMI
+	bool "Serial console over KGDB NMI debugger port"
+	depends on KGDB_SERIAL_CONSOLE
+	help
+	  This special driver allows you to temporary use NMI debugger port
+	  as a normal console (assuming that the port is attached to KGDB).
+
+	  Unlike KDB's disable_nmi command, with this driver you are always
+	  able to go back to the debugger using KGDB escape sequence ($3#33).
+	  This is because this console driver processes the input in NMI
+	  context, and thus is able to intercept the magic sequence.
+
+	  Note that since the console interprets input and uses polling
+	  communication methods, for things like PPP you still must fully
+	  detach debugger port from the KGDB NMI (i.e. disable_nmi), and
+	  use raw console.
+
+	  If unsure, say N.
+
 config SERIAL_KS8695
 	bool "Micrel KS8695 (Centaur) serial port support"
 	depends on ARCH_KS8695
diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
index ce88667..4f694da 100644
--- a/drivers/tty/serial/Makefile
+++ b/drivers/tty/serial/Makefile
@@ -61,6 +61,7 @@ obj-$(CONFIG_SERIAL_MSM_HS) += msm_serial_hs.o
 obj-$(CONFIG_SERIAL_NETX) += netx-serial.o
 obj-$(CONFIG_SERIAL_OF_PLATFORM) += of_serial.o
 obj-$(CONFIG_SERIAL_OF_PLATFORM_NWPSERIAL) += nwpserial.o
+obj-$(CONFIG_SERIAL_KGDB_NMI) += kgdb_nmi.o
 obj-$(CONFIG_SERIAL_KS8695) += serial_ks8695.o
 obj-$(CONFIG_SERIAL_OMAP) += omap-serial.o
 obj-$(CONFIG_SERIAL_ALTERA_UART) += altera_uart.o
diff --git a/drivers/tty/serial/kgdb_nmi.c b/drivers/tty/serial/kgdb_nmi.c
new file mode 100644
index 0000000..d185247
--- /dev/null
+++ b/drivers/tty/serial/kgdb_nmi.c
@@ -0,0 +1,402 @@
+/*
+ * KGDB NMI serial console
+ *
+ * Copyright 2010 Google, Inc.
+ *		  Arve Hjønnevåg <arve@android.com>
+ *		  Colin Cross <ccross@android.com>
+ * Copyright 2012 Linaro Ltd.
+ *		  Anton Vorontsov <anton.vorontsov@linaro.org>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/compiler.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/errno.h>
+#include <linux/atomic.h>
+#include <linux/console.h>
+#include <linux/tty.h>
+#include <linux/tty_driver.h>
+#include <linux/tty_flip.h>
+#include <linux/interrupt.h>
+#include <linux/hrtimer.h>
+#include <linux/tick.h>
+#include <linux/kfifo.h>
+#include <linux/kgdb.h>
+#include <linux/kdb.h>
+
+static int kgdb_nmi_knock = 1;
+module_param_named(knock, kgdb_nmi_knock, int, 0600);
+MODULE_PARM_DESC(knock, "if set to 1 (default), the special '$3#33' command " \
+			"must be used to enter the debugger; when set to 0, " \
+			"hitting return key is enough to enter the debugger; " \
+			"when set to -1, the debugger is entered immediately " \
+			"upon NMI");
+
+static char *kgdb_nmi_magic = "$3#33";
+module_param_named(magic, kgdb_nmi_magic, charp, 0600);
+MODULE_PARM_DESC(magic, "magic sequence to enter NMI debugger (default $3#33)");
+
+static bool kgdb_nmi_tty_enabled;
+
+static void kgdb_nmi_console_write(struct console *co, const char *s, uint c)
+{
+	int i;
+
+	if (!kgdb_nmi_tty_enabled || atomic_read(&kgdb_active) >= 0)
+		return;
+
+	for (i = 0; i < c; i++)
+		dbg_io_ops->write_char(s[i]);
+}
+
+static struct tty_driver *kgdb_nmi_tty_driver;
+
+static struct tty_driver *kgdb_nmi_console_device(struct console *co, int *idx)
+{
+	*idx = co->index;
+	return kgdb_nmi_tty_driver;
+}
+
+static struct console kgdb_nmi_console = {
+	.name	= "ttyNMI",
+	.write	= kgdb_nmi_console_write,
+	.device	= kgdb_nmi_console_device,
+	.flags	= CON_PRINTBUFFER | CON_ANYTIME | CON_ENABLED,
+	.index	= -1,
+};
+
+/*
+ * This is usually the maximum rate on debug ports. We make fifo large enough
+ * to make copy-pasting to the terminal usable.
+ */
+#define KGDB_NMI_BAUD		115200
+#define KGDB_NMI_FIFO_SIZE	roundup_pow_of_two(KGDB_NMI_BAUD / 8 / HZ)
+
+struct kgdb_nmi_tty_priv {
+	struct tty_port port;
+	struct tasklet_struct tlet;
+	STRUCT_KFIFO(char, KGDB_NMI_FIFO_SIZE) fifo;
+};
+
+static struct kgdb_nmi_tty_priv *kgdb_nmi_port_to_priv(struct tty_port *port)
+{
+	return container_of(port, struct kgdb_nmi_tty_priv, port);
+}
+
+/*
+ * Our debugging console is polled in a tasklet, so we'll check for input
+ * every tick. In HZ-less mode, we should program the next tick.  We have
+ * to use the lowlevel stuff as no locks should be grabbed.
+ */
+#ifdef CONFIG_HIGH_RES_TIMERS
+static void kgdb_tty_poke(void)
+{
+	tick_program_event(ktime_get(), 0);
+}
+#else
+static inline void kgdb_tty_poke(void) {}
+#endif
+
+static struct tty_port *kgdb_nmi_port;
+
+static void kgdb_tty_recv(int ch)
+{
+	struct kgdb_nmi_tty_priv *priv;
+	char c = ch;
+
+	if (!kgdb_nmi_port || ch < 0)
+		return;
+	/*
+	 * Can't use port->tty->driver_data as tty might be not there. Tasklet
+	 * will check for tty and will get the ref, but here we don't have to
+	 * do that, and actually, we can't: we're in NMI context, no locks are
+	 * possible.
+	 */
+	priv = kgdb_nmi_port_to_priv(kgdb_nmi_port);
+	kfifo_in(&priv->fifo, &c, 1);
+	kgdb_tty_poke();
+}
+
+static int kgdb_nmi_poll_one_knock(void)
+{
+	static int n;
+	int c = -1;
+	const char *magic = kgdb_nmi_magic;
+	size_t m = strlen(magic);
+	bool printch = 0;
+
+	c = dbg_io_ops->read_char();
+	if (c == NO_POLL_CHAR)
+		return c;
+
+	if (!kgdb_nmi_knock && (c == '\r' || c == '\n')) {
+		return 1;
+	} else if (c == magic[n]) {
+		n = (n + 1) % m;
+		if (!n)
+			return 1;
+		printch = 1;
+	} else {
+		n = 0;
+	}
+
+	if (kgdb_nmi_tty_enabled) {
+		kgdb_tty_recv(c);
+		return 0;
+	}
+
+	if (printch) {
+		kdb_printf("%c", c);
+		return 0;
+	}
+
+	kdb_printf("\r%s %s to enter the debugger> %*s",
+		   kgdb_nmi_knock ? "Type" : "Hit",
+		   kgdb_nmi_knock ? magic  : "<return>", (int)m, "");
+	while (m--)
+		kdb_printf("\b");
+	return 0;
+}
+
+/**
+ * kgdb_nmi_poll_knock - Check if it is time to enter the debugger
+ *
+ * "Serial ports are often noisy, especially when muxed over another port (we
+ * often use serial over the headset connector). Noise on the async command
+ * line just causes characters that are ignored, on a command line that blocked
+ * execution noise would be catastrophic." -- Colin Cross
+ *
+ * So, this function implements KGDB/KDB knocking on the serial line: we won't
+ * enter the debugger until we receive a known magic phrase (which is actually
+ * "$3#33", known as "escape to KDB" command. There is also a relaxed variant
+ * of knocking, i.e. just pressing the return key is enough to enter the
+ * debugger. And if knocking is disabled, the function always returns 1.
+ */
+bool kgdb_nmi_poll_knock(void)
+{
+	if (kgdb_nmi_knock < 0)
+		return 1;
+
+	while (1) {
+		int ret;
+
+		ret = kgdb_nmi_poll_one_knock();
+		if (ret == NO_POLL_CHAR)
+			return 0;
+		else if (ret == 1)
+			break;
+	}
+	return 1;
+}
+
+/*
+ * The tasklet is cheap, it does not cause wakeups when reschedules itself,
+ * instead it waits for the next tick.
+ */
+static void kgdb_nmi_tty_receiver(unsigned long data)
+{
+	struct kgdb_nmi_tty_priv *priv = (void *)data;
+	struct tty_struct *tty;
+	char ch;
+
+	tasklet_schedule(&priv->tlet);
+
+	if (likely(!kgdb_nmi_tty_enabled || !kfifo_len(&priv->fifo)))
+		return;
+
+	/* Port is there, but tty might be hung up, check. */
+	tty = tty_port_tty_get(kgdb_nmi_port);
+	if (!tty)
+		return;
+
+	while (kfifo_out(&priv->fifo, &ch, 1))
+		tty_insert_flip_char(priv->port.tty, ch, TTY_NORMAL);
+	tty_flip_buffer_push(priv->port.tty);
+
+	tty_kref_put(tty);
+}
+
+static int kgdb_nmi_tty_activate(struct tty_port *port, struct tty_struct *tty)
+{
+	struct kgdb_nmi_tty_priv *priv = tty->driver_data;
+
+	kgdb_nmi_port = port;
+	tasklet_schedule(&priv->tlet);
+	return 0;
+}
+
+static void kgdb_nmi_tty_shutdown(struct tty_port *port)
+{
+	struct kgdb_nmi_tty_priv *priv = port->tty->driver_data;
+
+	tasklet_kill(&priv->tlet);
+	kgdb_nmi_port = NULL;
+}
+
+static const struct tty_port_operations kgdb_nmi_tty_port_ops = {
+	.activate	= kgdb_nmi_tty_activate,
+	.shutdown	= kgdb_nmi_tty_shutdown,
+};
+
+static int kgdb_nmi_tty_install(struct tty_driver *drv, struct tty_struct *tty)
+{
+	struct kgdb_nmi_tty_priv *priv;
+	int ret;
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	INIT_KFIFO(priv->fifo);
+	tasklet_init(&priv->tlet, kgdb_nmi_tty_receiver, (unsigned long)priv);
+	tty_port_init(&priv->port);
+	priv->port.ops = &kgdb_nmi_tty_port_ops;
+	tty->driver_data = priv;
+
+	ret = tty_port_install(&priv->port, drv, tty);
+	if (ret) {
+		pr_err("%s: can't install tty port: %d\n", __func__, ret);
+		goto err;
+	}
+	return 0;
+err:
+	kfree(priv);
+	return ret;
+}
+
+static void kgdb_nmi_tty_cleanup(struct tty_struct *tty)
+{
+	struct kgdb_nmi_tty_priv *priv = tty->driver_data;
+
+	tty->driver_data = NULL;
+	kfree(priv);
+}
+
+static int kgdb_nmi_tty_open(struct tty_struct *tty, struct file *file)
+{
+	struct kgdb_nmi_tty_priv *priv = tty->driver_data;
+
+	return tty_port_open(&priv->port, tty, file);
+}
+
+static void kgdb_nmi_tty_close(struct tty_struct *tty, struct file *file)
+{
+	struct kgdb_nmi_tty_priv *priv = tty->driver_data;
+
+	tty_port_close(&priv->port, tty, file);
+}
+
+static void kgdb_nmi_tty_hangup(struct tty_struct *tty)
+{
+	struct kgdb_nmi_tty_priv *priv = tty->driver_data;
+
+	tty_port_hangup(&priv->port);
+}
+
+static int kgdb_nmi_tty_write_room(struct tty_struct *tty)
+{
+	/* Actually, we can handle any amount as we use polled writes. */
+	return 2048;
+}
+
+static int kgdb_nmi_tty_write(struct tty_struct *tty, const unchar *buf, int c)
+{
+	int i;
+
+	for (i = 0; i < c; i++)
+		dbg_io_ops->write_char(buf[i]);
+	return c;
+}
+
+static const struct tty_operations kgdb_nmi_tty_ops = {
+	.open		= kgdb_nmi_tty_open,
+	.close		= kgdb_nmi_tty_close,
+	.install	= kgdb_nmi_tty_install,
+	.cleanup	= kgdb_nmi_tty_cleanup,
+	.hangup		= kgdb_nmi_tty_hangup,
+	.write_room	= kgdb_nmi_tty_write_room,
+	.write		= kgdb_nmi_tty_write,
+};
+
+static int kgdb_nmi_enable_console(int argc, const char *argv[])
+{
+	kgdb_nmi_tty_enabled = !(argc == 1 && !strcmp(argv[1], "off"));
+	return 0;
+}
+
+int kgdb_register_nmi_console(void)
+{
+	int ret;
+
+	if (!arch_kgdb_ops.enable_nmi)
+		return 0;
+
+	kgdb_nmi_tty_driver = alloc_tty_driver(1);
+	if (!kgdb_nmi_tty_driver) {
+		pr_err("%s: cannot allocate tty\n", __func__);
+		return -ENOMEM;
+	}
+	kgdb_nmi_tty_driver->driver_name	= "ttyNMI";
+	kgdb_nmi_tty_driver->name		= "ttyNMI";
+	kgdb_nmi_tty_driver->num		= 1;
+	kgdb_nmi_tty_driver->type		= TTY_DRIVER_TYPE_SERIAL;
+	kgdb_nmi_tty_driver->subtype		= SERIAL_TYPE_NORMAL;
+	kgdb_nmi_tty_driver->flags		= TTY_DRIVER_REAL_RAW;
+	kgdb_nmi_tty_driver->init_termios	= tty_std_termios;
+	tty_termios_encode_baud_rate(&kgdb_nmi_tty_driver->init_termios,
+				     KGDB_NMI_BAUD, KGDB_NMI_BAUD);
+	tty_set_operations(kgdb_nmi_tty_driver, &kgdb_nmi_tty_ops);
+
+	ret = tty_register_driver(kgdb_nmi_tty_driver);
+	if (ret) {
+		pr_err("%s: can't register tty driver: %d\n", __func__, ret);
+		goto err_drv_reg;
+	}
+
+	ret = kdb_register("nmi_console", kgdb_nmi_enable_console, "[off]",
+			   "switch to Linux NMI console", 0);
+	if (ret) {
+		pr_err("%s: can't register kdb command: %d\n", __func__, ret);
+		goto err_kdb_reg;
+	}
+
+	register_console(&kgdb_nmi_console);
+	arch_kgdb_ops.enable_nmi(1);
+
+	return 0;
+err_kdb_reg:
+	tty_unregister_driver(kgdb_nmi_tty_driver);
+err_drv_reg:
+	put_tty_driver(kgdb_nmi_tty_driver);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(kgdb_register_nmi_console);
+
+int kgdb_unregister_nmi_console(void)
+{
+	int ret;
+
+	if (!arch_kgdb_ops.enable_nmi)
+		return 0;
+	arch_kgdb_ops.enable_nmi(0);
+
+	kdb_unregister("nmi_console");
+
+	ret = unregister_console(&kgdb_nmi_console);
+	if (ret)
+		return ret;
+
+	ret = tty_unregister_driver(kgdb_nmi_tty_driver);
+	if (ret)
+		return ret;
+	put_tty_driver(kgdb_nmi_tty_driver);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(kgdb_unregister_nmi_console);
diff --git a/drivers/tty/serial/kgdboc.c b/drivers/tty/serial/kgdboc.c
index 2b42a01..3f63d83 100644
--- a/drivers/tty/serial/kgdboc.c
+++ b/drivers/tty/serial/kgdboc.c
@@ -145,6 +145,8 @@ __setup("kgdboc=", kgdboc_option_setup);
 
 static void cleanup_kgdboc(void)
 {
+	if (kgdb_unregister_nmi_console())
+		return;
 	kgdboc_unregister_kbd();
 	if (configured == 1)
 		kgdb_unregister_io_module(&kgdboc_io_ops);
@@ -198,11 +200,18 @@ do_register:
 	if (err)
 		goto noconfig;
 
+	err = kgdb_register_nmi_console();
+	if (err)
+		goto nmi_con_failed;
+
 	configured = 1;
 
 	return 0;
 
+nmi_con_failed:
+	kgdb_unregister_io_module(&kgdboc_io_ops);
 noconfig:
+	kgdboc_unregister_kbd();
 	config[0] = 0;
 	configured = 0;
 	cleanup_kgdboc();
diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h
index 7800cce..4dff0c6 100644
--- a/include/linux/kgdb.h
+++ b/include/linux/kgdb.h
@@ -286,6 +286,16 @@ extern struct kgdb_arch		arch_kgdb_ops;
 
 extern unsigned long __weak kgdb_arch_pc(int exception, struct pt_regs *regs);
 
+#ifdef CONFIG_SERIAL_KGDB_NMI
+extern int kgdb_register_nmi_console(void);
+extern int kgdb_unregister_nmi_console(void);
+extern bool kgdb_nmi_poll_knock(void);
+#else
+static inline int kgdb_register_nmi_console(void) { return 0; }
+static inline int kgdb_unregister_nmi_console(void) { return 0; }
+static inline bool kgdb_nmi_poll_knock(void) { return 1; }
+#endif
+
 extern int kgdb_register_io_module(struct kgdb_io *local_kgdb_io_ops);
 extern void kgdb_unregister_io_module(struct kgdb_io *local_kgdb_io_ops);
 extern struct kgdb_io *dbg_io_ops;
-- 
1.7.11.5

--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* Re: [PATCH 1/3] serial: pl011: safeguard against impossible baudrates
From: Russell King - ARM Linux @ 2012-09-20 18:57 UTC (permalink / raw)
  To: Linus Walleij
  Cc: linux-serial, Greg Kroah-Hartman, Linus Walleij, Guillaume Jaunet,
	Par-Gunnar Hjalmdahl, Anmar Oueja, Matthias Locher,
	Rajanikanth H.V, Christophe Arnal, linux-arm-kernel
In-Reply-To: <1348134346-25593-1-git-send-email-linus.walleij@stericsson.com>

On Thu, Sep 20, 2012 at 11:45:46AM +0200, Linus Walleij wrote:
> +	if ((termios->c_ispeed > max_baud) ||
> +	    (termios->c_ospeed > max_baud)) {
> +		dev_err(port->dev,
> +			"requested a baud rate > clock/mindivisor\n");
> +		return;
> +	}

Why do we need this check?  In any case, this is incorrect behaviour
just to 'return' from this function without doing anything.  You just
produce an error message, and userspace believes that it's request
has been satisfied.

uart_get_baud_rate() is there precisely to do the right thing for invalid
baud rates, which is to fallback to the old termios setting (and update
the new termios with that) if the old termios setting satisifies the
limits, if not it falls back to something within the limited range.

^ permalink raw reply

* Re: [PATCH 3/3] serial: pl011: allow very high baudrates
From: Russell King - ARM Linux @ 2012-09-20 19:00 UTC (permalink / raw)
  To: Linus Walleij
  Cc: linux-serial, Greg Kroah-Hartman, Linus Walleij, Guillaume Jaunet,
	Par-Gunnar Hjalmdahl, Anmar Oueja, Matthias Locher,
	Rajanikanth H.V, Christophe Arnal, linux-arm-kernel
In-Reply-To: <1348134368-25663-1-git-send-email-linus.walleij@stericsson.com>

On Thu, Sep 20, 2012 at 11:46:08AM +0200, Linus Walleij wrote:
> From: Linus Walleij <linus.walleij@linaro.org>
> 
> The ST Microelectronics variant of the PL011 is capable of supporting
> very high non-standard baud rates, even above 4 Mbps. However the
> uart_get_baud_rate() will not allow us to set these, so override that
> calculation on very high speeds.

You don't explain why it doesn't.  It should in theory allow you to,
because there's no limits within that function other than those which
you pass in as the minimum and maximum.

If your userspace hasn't been updated to use the integer baud rate
setting mechanisms, then that could be where the problem lies.
Alternatively, if some Bxxxx setting is not being respected by
tty_termios_baud_rate(), that also would need fixing.

But the fix you propose in this patch just looks wrong.

^ permalink raw reply

* Re: [PATCHv4 7/9] arm: vt8500: doc: Add device tree bindings for arch-vt8500 devices
From: Rob Herring @ 2012-09-20 23:13 UTC (permalink / raw)
  To: Tony Prisk
  Cc: vt8500-wm8505-linux-kernel-/JYPxA39Uh5TLH3MbocFFw,
	Alessandro Zummo, linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	Russell King, Linus Walleij, Florian Tobias Schandinat,
	Greg Kroah-Hartman, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
	linux-serial-u79uwXL29TY76Z2rM5mHXA,
	rtc-linux-/JYPxA39Uh5TLH3MbocFFw, Stephen Warren, Mike Turquette,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Alan Cox,
	Olof Johansson
In-Reply-To: <1345707346-9035-8-git-send-email-linux-ci5G2KO2hbZ+pU9mqzGVBQ@public.gmane.org>

On 08/23/2012 02:35 AM, Tony Prisk wrote:
> Bindings for gpio, interrupt controller, power management controller,
> timer, realtime clock, serial uart, ehci and uhci controllers and
> framebuffer controllers used on the arch-vt8500 platform.
> 
> Framebuffer binding also specifies a 'display' node which is required
> for determining the lcd panel data.

Overall, these bindings look okay, but there is some effort to
standardize a binding for displays:

http://comments.gmane.org/gmane.linux.drivers.devicetree/21386

I would like to see that used rather than something custom.

Rob

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCHv7 04/12] ARM: OMAP: hwmod: Add support for per hwmod/module context lost count
From: Paul Walmsley @ 2012-09-21  0:38 UTC (permalink / raw)
  To: Tero Kristo, balbi, sourav.poddar, gregkh
  Cc: linux-omap, nm, khilman, rnayak, linux-arm-kernel, linux-serial
In-Reply-To: <alpine.DEB.2.00.1209121946410.17903@utopia.booyaka.com>

Hi 

On Wed, 12 Sep 2012, Paul Walmsley wrote:

> On Thu, 19 Jul 2012, Tero Kristo wrote:
> 
> > From: Rajendra Nayak <rnayak@ti.com>
> > 
> > OMAP4 has module specific context lost registers which makes it now
> > possible to have module level context loss count, instead of relying
> > on the powerdomain level context count.
> > 
> > Add 2 private hwmod api's to update/clear the hwmod/module specific
> > context lost counters/register.
> 
> This one has been modified to align with the changes on patch 3, and 
> queued for 3.7.

Am sorry to say that I had to drop the code changes from this patch, due 
to the OMAP serial bugs addressed by these two patches:

    http://comments.gmane.org/gmane.linux.ports.arm.omap/84729
    http://www.spinics.net/lists/arm-kernel/msg196034.html

Without those two patches, OMAP4430ES2 Panda crashes after Tero's patches 
are applied, with the same symptom as with the N800 crash.  Applying 
those two patches fixes the problem.

It would be good if you guys could ask Greg to pick up the first patch 
ASAP so we can queue the second one during the early 3.7-rc time frame.


- Paul

^ permalink raw reply

* Re: [PATCH] serial: omap: Remove unnecessary checks from suspend/resume
From: Poddar, Sourav @ 2012-09-21  5:10 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: balbi, gregkh, alan, tony, linux-omap, linux-serial,
	linux-arm-kernel, santosh.shilimkar
In-Reply-To: <87r4pxelje.fsf@deeprootsystems.com>

Hi Kevin,

On Thu, Sep 20, 2012 at 3:13 AM, Kevin Hilman
<khilman@deeprootsystems.com> wrote:
> "Poddar, Sourav" <sourav.poddar@ti.com> writes:
>
>> Hi Felipe,
>>
>> On Tue, Sep 18, 2012 at 5:04 PM, Felipe Balbi <balbi@ti.com> wrote:
>>> On Tue, Sep 18, 2012 at 05:05:54PM +0530, Sourav Poddar wrote:
>>>> Drop the check for "up" being valid on suspend/resume callbacks.
>>>> It should be valid always. Get rid of the "pdata" check also as
>>>> serial_omap_get_context_loss_count() checks for it.
>>>>
>>>> Tested on omap4 panda and 3630 based Beagle board.
>>>
>>> you need a blank line here. Other than that:
>>>
>>> Reviewed-by: Felipe Balbi <balbi@ti.com>
>>>
>>> ps: what kind of tests did you run, btw ?
>>>
>> Boot tested on Panda.
>> Boot and PM tested(hitiing Off) on Beagle.
>
> What does "PM tested" mean.  hitting off in idle?  hitting off in
> suspend?  both?
>
Hitting off in both idle and suspend.
> Please clarify.
>
> Thanks,
>
> Kevin
~Sourav

^ permalink raw reply

* [PATCHv2] serial: omap: Remove unnecessary checks from suspend/resume
From: Sourav Poddar @ 2012-09-21  6:56 UTC (permalink / raw)
  To: gregkh
  Cc: alan, tony, khilman, linux-omap, linux-serial, linux-arm-kernel,
	balbi, santosh.shilimkar, paul, Sourav Poddar

Drop the check for "up" being valid on suspend/resume callbacks.
It should be valid always. Get rid of the "pdata" check also as
serial_omap_get_context_loss_count() checks for it.

Boot tested on omap4 panda and PM tested(hitting off in Idle
and suspend) on 3630 based Beagle board.

Signed-off-by: Sourav Poddar <sourav.poddar@ti.com>
---
v1->v2
Not a functionality change, just
Introduce a blank line before "signed off by" line.
 drivers/tty/serial/omap-serial.c |   23 +++++++++--------------
 1 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index f175385..3c05c5e 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -1222,10 +1222,8 @@ static int serial_omap_suspend(struct device *dev)
 {
 	struct uart_omap_port *up = dev_get_drvdata(dev);
 
-	if (up) {
-		uart_suspend_port(&serial_omap_reg, &up->port);
-		flush_work_sync(&up->qos_work);
-	}
+	uart_suspend_port(&serial_omap_reg, &up->port);
+	flush_work_sync(&up->qos_work);
 
 	return 0;
 }
@@ -1234,8 +1232,8 @@ static int serial_omap_resume(struct device *dev)
 {
 	struct uart_omap_port *up = dev_get_drvdata(dev);
 
-	if (up)
-		uart_resume_port(&serial_omap_reg, &up->port);
+	uart_resume_port(&serial_omap_reg, &up->port);
+
 	return 0;
 }
 #endif
@@ -1553,17 +1551,14 @@ static int serial_omap_runtime_suspend(struct device *dev)
 static int serial_omap_runtime_resume(struct device *dev)
 {
 	struct uart_omap_port *up = dev_get_drvdata(dev);
-	struct omap_uart_port_info *pdata = dev->platform_data;
 
-	if (up && pdata) {
-			u32 loss_cnt = serial_omap_get_context_loss_count(up);
+	u32 loss_cnt = serial_omap_get_context_loss_count(up);
 
-			if (up->context_loss_cnt != loss_cnt)
-				serial_omap_restore_context(up);
+	if (up->context_loss_cnt != loss_cnt)
+		serial_omap_restore_context(up);
 
-		up->latency = up->calc_latency;
-		schedule_work(&up->qos_work);
-	}
+	up->latency = up->calc_latency;
+	schedule_work(&up->qos_work);
 
 	return 0;
 }
-- 
1.7.1


^ permalink raw reply related

* [Resend/PATCHv2] serial: omap: Remove unnecessary checks from suspend/resume
From: Sourav Poddar @ 2012-09-21  7:04 UTC (permalink / raw)
  To: gregkh
  Cc: alan, tony, khilman, linux-omap, linux-serial, linux-arm-kernel,
	balbi, santosh.shilimkar, paul, Sourav Poddar

Drop the check for "up" being valid on suspend/resume callbacks.
It should be valid always. Get rid of the "pdata" check also as
serial_omap_get_context_loss_count() checks for it.

Boot tested on omap4 panda and PM tested(hitting off in Idle
and suspend) on 3630 based Beagle board.

Reviewed-by: Felipe Balbi <balbi@ti.com>
Signed-off-by: Sourav Poddar <sourav.poddar@ti.com>
---
Added Felipe's Reviewed by
v1->v2
Not a functionality change, just
Introduce a blank line before "signed off by" line.
 drivers/tty/serial/omap-serial.c |   23 +++++++++--------------
 1 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index f175385..3c05c5e 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -1222,10 +1222,8 @@ static int serial_omap_suspend(struct device *dev)
 {
 	struct uart_omap_port *up = dev_get_drvdata(dev);
 
-	if (up) {
-		uart_suspend_port(&serial_omap_reg, &up->port);
-		flush_work_sync(&up->qos_work);
-	}
+	uart_suspend_port(&serial_omap_reg, &up->port);
+	flush_work_sync(&up->qos_work);
 
 	return 0;
 }
@@ -1234,8 +1232,8 @@ static int serial_omap_resume(struct device *dev)
 {
 	struct uart_omap_port *up = dev_get_drvdata(dev);
 
-	if (up)
-		uart_resume_port(&serial_omap_reg, &up->port);
+	uart_resume_port(&serial_omap_reg, &up->port);
+
 	return 0;
 }
 #endif
@@ -1553,17 +1551,14 @@ static int serial_omap_runtime_suspend(struct device *dev)
 static int serial_omap_runtime_resume(struct device *dev)
 {
 	struct uart_omap_port *up = dev_get_drvdata(dev);
-	struct omap_uart_port_info *pdata = dev->platform_data;
 
-	if (up && pdata) {
-			u32 loss_cnt = serial_omap_get_context_loss_count(up);
+	u32 loss_cnt = serial_omap_get_context_loss_count(up);
 
-			if (up->context_loss_cnt != loss_cnt)
-				serial_omap_restore_context(up);
+	if (up->context_loss_cnt != loss_cnt)
+		serial_omap_restore_context(up);
 
-		up->latency = up->calc_latency;
-		schedule_work(&up->qos_work);
-	}
+	up->latency = up->calc_latency;
+	schedule_work(&up->qos_work);
 
 	return 0;
 }
-- 
1.7.1


^ permalink raw reply related

* Re: [PATCH 1/3] serial: pl011: safeguard against impossible baudrates
From: Linus Walleij @ 2012-09-21  8:26 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Linus Walleij, linux-serial, Greg Kroah-Hartman, Guillaume Jaunet,
	Par-Gunnar Hjalmdahl, Anmar Oueja, Matthias Locher,
	Rajanikanth H.V, Christophe Arnal, linux-arm-kernel
In-Reply-To: <20120920185748.GA15609@n2100.arm.linux.org.uk>

On Thu, Sep 20, 2012 at 8:57 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:

> uart_get_baud_rate() is there precisely to do the right thing for invalid
> baud rates, which is to fallback to the old termios setting (and update
> the new termios with that) if the old termios setting satisifies the
> limits, if not it falls back to something within the limited range.

Yeah, hm, passing 4800000 as max rate to this function and
requesting 4050000 returns 4000000... (Other high baud rates
such as 3250000 or 3000000 works fine) so there is some fishy
bug in there we're just duct-taping around, I'll go hunt the real bug
instead.

Thanks!
Linus Walleij

^ permalink raw reply

* [RFC 0/3] Move some ARM header files to more appropriate locations
From: Russell King - ARM Linux @ 2012-09-21  9:35 UTC (permalink / raw)
  To: linux-arm-kernel, linux-serial, linux-usb
  Cc: Alan Cox, Eric Miao, Felipe Balbi, Greg Kroah-Hartman,
	Haojian Zhuang, Imre Kaloz, Kristoffer Ericson, Krzysztof Halasa,
	Nicolas Ferre

This series moves some of the misplaced ARM header files, which
contain driver platform data, out of arch/arm/include/asm/mach
and into include/linux/platform_data.

arch/arm/include/asm/mach is supposed to be for include files
exporting generic ARM code structures and functions to ARM platforms,
not for platforms to export platform data definitions to the rest
of the kernel.

This patch series addresses three of the files: the SA11x0 serial
driver platform data, the PXA2xx UDC driver, and the Atmel serial
driver.  Out of those three, I ended up deleting the Atmel serial
driver header as I could not find any user of the exported function
and its data structure anywhere in the kernel.

 arch/arm/include/asm/mach/serial_at91.h            |   33 --------------------
 arch/arm/mach-ixp4xx/include/mach/udc.h            |    2 +-
 arch/arm/mach-pxa/include/mach/udc.h               |    2 +-
 arch/arm/mach-sa1100/assabet.c                     |    2 +-
 arch/arm/mach-sa1100/badge4.c                      |    2 +-
 arch/arm/mach-sa1100/cerf.c                        |    2 +-
 arch/arm/mach-sa1100/collie.c                      |    2 +-
 arch/arm/mach-sa1100/h3xxx.c                       |    2 +-
 arch/arm/mach-sa1100/hackkit.c                     |    2 +-
 arch/arm/mach-sa1100/jornada720.c                  |    2 +-
 arch/arm/mach-sa1100/lart.c                        |    2 +-
 arch/arm/mach-sa1100/nanoengine.c                  |    2 +-
 arch/arm/mach-sa1100/neponset.c                    |    2 +-
 arch/arm/mach-sa1100/pleb.c                        |    2 +-
 arch/arm/mach-sa1100/shannon.c                     |    2 +-
 arch/arm/mach-sa1100/simpad.c                      |    2 +-
 arch/avr32/include/asm/mach/serial_at91.h          |   33 --------------------
 drivers/tty/serial/atmel_serial.c                  |   18 -----------
 drivers/tty/serial/sa1100.c                        |    2 +-
 drivers/usb/gadget/pxa25x_udc.c                    |    4 +--
 .../linux/platform_data/pxa2xx_udc.h               |    5 ++-
 .../linux/platform_data/sa11x0-serial.h            |    6 ++-
 22 files changed, 24 insertions(+), 107 deletions(-)
 delete mode 100644 arch/arm/include/asm/mach/serial_at91.h
 delete mode 100644 arch/avr32/include/asm/mach/serial_at91.h
 rename arch/arm/include/asm/mach/udc_pxa2xx.h => include/linux/platform_data/pxa2xx_udc.h (94%)
 rename arch/arm/include/asm/mach/serial_sa1100.h => include/linux/platform_data/sa11x0-serial.h (93%)

^ permalink raw reply

* [RFC 1/3] ARM: move serial_sa1100.h header file to linux/platform_data
From: Russell King @ 2012-09-21  9:36 UTC (permalink / raw)
  To: linux-arm-kernel, linux-serial, linux-usb
  Cc: Kristoffer Ericson, Alan Cox, Greg Kroah-Hartman
In-Reply-To: <20120921093530.GB31374@n2100.arm.linux.org.uk>

This is really driver platform data, so move it to the appropriate
directory.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 arch/arm/include/asm/mach/serial_sa1100.h   |   31 -------------------------
 arch/arm/mach-sa1100/assabet.c              |    2 +-
 arch/arm/mach-sa1100/badge4.c               |    2 +-
 arch/arm/mach-sa1100/cerf.c                 |    2 +-
 arch/arm/mach-sa1100/collie.c               |    2 +-
 arch/arm/mach-sa1100/h3xxx.c                |    2 +-
 arch/arm/mach-sa1100/hackkit.c              |    2 +-
 arch/arm/mach-sa1100/jornada720.c           |    2 +-
 arch/arm/mach-sa1100/lart.c                 |    2 +-
 arch/arm/mach-sa1100/nanoengine.c           |    2 +-
 arch/arm/mach-sa1100/neponset.c             |    2 +-
 arch/arm/mach-sa1100/pleb.c                 |    2 +-
 arch/arm/mach-sa1100/shannon.c              |    2 +-
 arch/arm/mach-sa1100/simpad.c               |    2 +-
 drivers/tty/serial/sa1100.c                 |    2 +-
 include/linux/platform_data/sa11x0-serial.h |   33 +++++++++++++++++++++++++++
 16 files changed, 47 insertions(+), 45 deletions(-)
 delete mode 100644 arch/arm/include/asm/mach/serial_sa1100.h
 create mode 100644 include/linux/platform_data/sa11x0-serial.h

diff --git a/arch/arm/include/asm/mach/serial_sa1100.h b/arch/arm/include/asm/mach/serial_sa1100.h
deleted file mode 100644
index d09064b..0000000
--- a/arch/arm/include/asm/mach/serial_sa1100.h
+++ /dev/null
@@ -1,31 +0,0 @@
-/*
- *  arch/arm/include/asm/mach/serial_sa1100.h
- *
- *  Author: Nicolas Pitre
- *
- * Moved and changed lots, Russell King
- *
- * Low level machine dependent UART functions.
- */
-
-struct uart_port;
-struct uart_info;
-
-/*
- * This is a temporary structure for registering these
- * functions; it is intended to be discarded after boot.
- */
-struct sa1100_port_fns {
-	void	(*set_mctrl)(struct uart_port *, u_int);
-	u_int	(*get_mctrl)(struct uart_port *);
-	void	(*pm)(struct uart_port *, u_int, u_int);
-	int	(*set_wake)(struct uart_port *, u_int);
-};
-
-#ifdef CONFIG_SERIAL_SA1100
-void sa1100_register_uart_fns(struct sa1100_port_fns *fns);
-void sa1100_register_uart(int idx, int port);
-#else
-#define sa1100_register_uart_fns(fns) do { } while (0)
-#define sa1100_register_uart(idx,port) do { } while (0)
-#endif
diff --git a/arch/arm/mach-sa1100/assabet.c b/arch/arm/mach-sa1100/assabet.c
index d673211..4a0e6b8 100644
--- a/arch/arm/mach-sa1100/assabet.c
+++ b/arch/arm/mach-sa1100/assabet.c
@@ -14,6 +14,7 @@
 #include <linux/module.h>
 #include <linux/errno.h>
 #include <linux/ioport.h>
+#include <linux/platform_data/sa11x0-serial.h>
 #include <linux/serial_core.h>
 #include <linux/mfd/ucb1x00.h>
 #include <linux/mtd/mtd.h>
@@ -35,7 +36,6 @@
 #include <asm/mach/flash.h>
 #include <asm/mach/irda.h>
 #include <asm/mach/map.h>
-#include <asm/mach/serial_sa1100.h>
 #include <mach/assabet.h>
 #include <mach/mcp.h>
 #include <mach/irqs.h>
diff --git a/arch/arm/mach-sa1100/badge4.c b/arch/arm/mach-sa1100/badge4.c
index b30fb99..1d60b7e 100644
--- a/arch/arm/mach-sa1100/badge4.c
+++ b/arch/arm/mach-sa1100/badge4.c
@@ -16,6 +16,7 @@
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
+#include <linux/platform_data/sa11x0-serial.h>
 #include <linux/platform_device.h>
 #include <linux/delay.h>
 #include <linux/tty.h>
@@ -32,7 +33,6 @@
 #include <asm/mach/flash.h>
 #include <asm/mach/map.h>
 #include <asm/hardware/sa1111.h>
-#include <asm/mach/serial_sa1100.h>
 
 #include <mach/badge4.h>
 
diff --git a/arch/arm/mach-sa1100/cerf.c b/arch/arm/mach-sa1100/cerf.c
index 09d7f4b..1e03937 100644
--- a/arch/arm/mach-sa1100/cerf.c
+++ b/arch/arm/mach-sa1100/cerf.c
@@ -13,6 +13,7 @@
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/tty.h>
+#include <linux/platform_data/sa11x0-serial.h>
 #include <linux/platform_device.h>
 #include <linux/irq.h>
 #include <linux/mtd/mtd.h>
@@ -25,7 +26,6 @@
 #include <asm/mach/arch.h>
 #include <asm/mach/flash.h>
 #include <asm/mach/map.h>
-#include <asm/mach/serial_sa1100.h>
 
 #include <mach/cerf.h>
 #include <mach/mcp.h>
diff --git a/arch/arm/mach-sa1100/collie.c b/arch/arm/mach-sa1100/collie.c
index ea5cff3..5d1e10c 100644
--- a/arch/arm/mach-sa1100/collie.c
+++ b/arch/arm/mach-sa1100/collie.c
@@ -21,6 +21,7 @@
 #include <linux/kernel.h>
 #include <linux/tty.h>
 #include <linux/delay.h>
+#include <linux/platform_data/sa11x0-serial.h>
 #include <linux/platform_device.h>
 #include <linux/mfd/ucb1x00.h>
 #include <linux/mtd/mtd.h>
@@ -40,7 +41,6 @@
 #include <asm/mach/arch.h>
 #include <asm/mach/flash.h>
 #include <asm/mach/map.h>
-#include <asm/mach/serial_sa1100.h>
 
 #include <asm/hardware/scoop.h>
 #include <asm/mach/sharpsl_param.h>
diff --git a/arch/arm/mach-sa1100/h3xxx.c b/arch/arm/mach-sa1100/h3xxx.c
index 63150e1..f17e738 100644
--- a/arch/arm/mach-sa1100/h3xxx.c
+++ b/arch/arm/mach-sa1100/h3xxx.c
@@ -17,12 +17,12 @@
 #include <linux/mfd/htc-egpio.h>
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/partitions.h>
+#include <linux/platform_data/sa11x0-serial.h>
 #include <linux/platform_device.h>
 #include <linux/serial_core.h>
 
 #include <asm/mach/flash.h>
 #include <asm/mach/map.h>
-#include <asm/mach/serial_sa1100.h>
 
 #include <mach/h3xxx.h>
 
diff --git a/arch/arm/mach-sa1100/hackkit.c b/arch/arm/mach-sa1100/hackkit.c
index 7f86bd9..2da6d08 100644
--- a/arch/arm/mach-sa1100/hackkit.c
+++ b/arch/arm/mach-sa1100/hackkit.c
@@ -18,6 +18,7 @@
 #include <linux/module.h>
 #include <linux/errno.h>
 #include <linux/cpufreq.h>
+#include <linux/platform_data/sa11x0-serial.h>
 #include <linux/serial_core.h>
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/partitions.h>
@@ -31,7 +32,6 @@
 #include <asm/mach/flash.h>
 #include <asm/mach/map.h>
 #include <asm/mach/irq.h>
-#include <asm/mach/serial_sa1100.h>
 
 #include <mach/hardware.h>
 #include <mach/irqs.h>
diff --git a/arch/arm/mach-sa1100/jornada720.c b/arch/arm/mach-sa1100/jornada720.c
index e3084f4..35cfc42 100644
--- a/arch/arm/mach-sa1100/jornada720.c
+++ b/arch/arm/mach-sa1100/jornada720.c
@@ -17,6 +17,7 @@
 #include <linux/kernel.h>
 #include <linux/tty.h>
 #include <linux/delay.h>
+#include <linux/platform_data/sa11x0-serial.h>
 #include <linux/platform_device.h>
 #include <linux/ioport.h>
 #include <linux/mtd/mtd.h>
@@ -30,7 +31,6 @@
 #include <asm/mach/arch.h>
 #include <asm/mach/flash.h>
 #include <asm/mach/map.h>
-#include <asm/mach/serial_sa1100.h>
 
 #include <mach/hardware.h>
 #include <mach/irqs.h>
diff --git a/arch/arm/mach-sa1100/lart.c b/arch/arm/mach-sa1100/lart.c
index b775a0a..39aed9b 100644
--- a/arch/arm/mach-sa1100/lart.c
+++ b/arch/arm/mach-sa1100/lart.c
@@ -4,6 +4,7 @@
 
 #include <linux/init.h>
 #include <linux/kernel.h>
+#include <linux/platform_data/sa11x0-serial.h>
 #include <linux/tty.h>
 
 #include <video/sa1100fb.h>
@@ -15,7 +16,6 @@
 
 #include <asm/mach/arch.h>
 #include <asm/mach/map.h>
-#include <asm/mach/serial_sa1100.h>
 #include <mach/mcp.h>
 #include <mach/irqs.h>
 
diff --git a/arch/arm/mach-sa1100/nanoengine.c b/arch/arm/mach-sa1100/nanoengine.c
index 41f69d9..102e08f 100644
--- a/arch/arm/mach-sa1100/nanoengine.c
+++ b/arch/arm/mach-sa1100/nanoengine.c
@@ -13,6 +13,7 @@
 
 #include <linux/init.h>
 #include <linux/kernel.h>
+#include <linux/platform_data/sa11x0-serial.h>
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/partitions.h>
 #include <linux/root_dev.h>
@@ -24,7 +25,6 @@
 #include <asm/mach/arch.h>
 #include <asm/mach/flash.h>
 #include <asm/mach/map.h>
-#include <asm/mach/serial_sa1100.h>
 
 #include <mach/hardware.h>
 #include <mach/nanoengine.h>
diff --git a/arch/arm/mach-sa1100/neponset.c b/arch/arm/mach-sa1100/neponset.c
index 266db87..88be047 100644
--- a/arch/arm/mach-sa1100/neponset.c
+++ b/arch/arm/mach-sa1100/neponset.c
@@ -7,6 +7,7 @@
 #include <linux/irq.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/platform_data/sa11x0-serial.h>
 #include <linux/platform_device.h>
 #include <linux/pm.h>
 #include <linux/serial_core.h>
@@ -14,7 +15,6 @@
 
 #include <asm/mach-types.h>
 #include <asm/mach/map.h>
-#include <asm/mach/serial_sa1100.h>
 #include <asm/hardware/sa1111.h>
 #include <asm/sizes.h>
 
diff --git a/arch/arm/mach-sa1100/pleb.c b/arch/arm/mach-sa1100/pleb.c
index 37fe0a0..c51bb63 100644
--- a/arch/arm/mach-sa1100/pleb.c
+++ b/arch/arm/mach-sa1100/pleb.c
@@ -6,6 +6,7 @@
 #include <linux/kernel.h>
 #include <linux/tty.h>
 #include <linux/ioport.h>
+#include <linux/platform_data/sa11x0-serial.h>
 #include <linux/platform_device.h>
 #include <linux/irq.h>
 #include <linux/io.h>
@@ -18,7 +19,6 @@
 #include <asm/mach/arch.h>
 #include <asm/mach/map.h>
 #include <asm/mach/flash.h>
-#include <asm/mach/serial_sa1100.h>
 #include <mach/irqs.h>
 
 #include "generic.h"
diff --git a/arch/arm/mach-sa1100/shannon.c b/arch/arm/mach-sa1100/shannon.c
index 5d33fc3..3dd4416 100644
--- a/arch/arm/mach-sa1100/shannon.c
+++ b/arch/arm/mach-sa1100/shannon.c
@@ -5,6 +5,7 @@
 #include <linux/init.h>
 #include <linux/device.h>
 #include <linux/kernel.h>
+#include <linux/platform_data/sa11x0-serial.h>
 #include <linux/tty.h>
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/partitions.h>
@@ -18,7 +19,6 @@
 #include <asm/mach/arch.h>
 #include <asm/mach/flash.h>
 #include <asm/mach/map.h>
-#include <asm/mach/serial_sa1100.h>
 #include <mach/mcp.h>
 #include <mach/shannon.h>
 #include <mach/irqs.h>
diff --git a/arch/arm/mach-sa1100/simpad.c b/arch/arm/mach-sa1100/simpad.c
index fbd5359..c76038e 100644
--- a/arch/arm/mach-sa1100/simpad.c
+++ b/arch/arm/mach-sa1100/simpad.c
@@ -9,6 +9,7 @@
 #include <linux/proc_fs.h>
 #include <linux/string.h>
 #include <linux/pm.h>
+#include <linux/platform_data/sa11x0-serial.h>
 #include <linux/platform_device.h>
 #include <linux/mfd/ucb1x00.h>
 #include <linux/mtd/mtd.h>
@@ -23,7 +24,6 @@
 #include <asm/mach/arch.h>
 #include <asm/mach/flash.h>
 #include <asm/mach/map.h>
-#include <asm/mach/serial_sa1100.h>
 #include <mach/mcp.h>
 #include <mach/simpad.h>
 #include <mach/irqs.h>
diff --git a/drivers/tty/serial/sa1100.c b/drivers/tty/serial/sa1100.c
index 2ca5959..ecc1e16 100644
--- a/drivers/tty/serial/sa1100.c
+++ b/drivers/tty/serial/sa1100.c
@@ -29,6 +29,7 @@
 #include <linux/init.h>
 #include <linux/console.h>
 #include <linux/sysrq.h>
+#include <linux/platform_data/sa11x0-serial.h>
 #include <linux/platform_device.h>
 #include <linux/tty.h>
 #include <linux/tty_flip.h>
@@ -39,7 +40,6 @@
 #include <asm/irq.h>
 #include <mach/hardware.h>
 #include <mach/irqs.h>
-#include <asm/mach/serial_sa1100.h>
 
 /* We've been assigned a range on the "Low-density serial ports" major */
 #define SERIAL_SA1100_MAJOR	204
diff --git a/include/linux/platform_data/sa11x0-serial.h b/include/linux/platform_data/sa11x0-serial.h
new file mode 100644
index 0000000..4504d5d
--- /dev/null
+++ b/include/linux/platform_data/sa11x0-serial.h
@@ -0,0 +1,33 @@
+/*
+ *  Author: Nicolas Pitre
+ *
+ * Moved and changed lots, Russell King
+ *
+ * Low level machine dependent UART functions.
+ */
+#ifndef SA11X0_SERIAL_H
+#define SA11X0_SERIAL_H
+
+struct uart_port;
+struct uart_info;
+
+/*
+ * This is a temporary structure for registering these
+ * functions; it is intended to be discarded after boot.
+ */
+struct sa1100_port_fns {
+	void	(*set_mctrl)(struct uart_port *, u_int);
+	u_int	(*get_mctrl)(struct uart_port *);
+	void	(*pm)(struct uart_port *, u_int, u_int);
+	int	(*set_wake)(struct uart_port *, u_int);
+};
+
+#ifdef CONFIG_SERIAL_SA1100
+void sa1100_register_uart_fns(struct sa1100_port_fns *fns);
+void sa1100_register_uart(int idx, int port);
+#else
+#define sa1100_register_uart_fns(fns) do { } while (0)
+#define sa1100_register_uart(idx,port) do { } while (0)
+#endif
+
+#endif
-- 
1.7.4.4


^ permalink raw reply related

* [RFC 2/3] ARM: move udc_pxa2xx.h to linux/platform_data
From: Russell King @ 2012-09-21  9:36 UTC (permalink / raw)
  To: linux-arm-kernel, linux-serial, linux-usb
  Cc: Imre Kaloz, Krzysztof Halasa, Eric Miao, Haojian Zhuang,
	Felipe Balbi, Greg Kroah-Hartman
In-Reply-To: <20120921093530.GB31374@n2100.arm.linux.org.uk>

Move the PXA2xx/IXP4xx UDC header file into linux/platform_data as it
only contains a driver platform data structure.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 arch/arm/include/asm/mach/udc_pxa2xx.h   |   26 --------------------------
 arch/arm/mach-ixp4xx/include/mach/udc.h  |    2 +-
 arch/arm/mach-pxa/include/mach/udc.h     |    2 +-
 drivers/usb/gadget/pxa25x_udc.c          |    4 +---
 include/linux/platform_data/pxa2xx_udc.h |   27 +++++++++++++++++++++++++++
 5 files changed, 30 insertions(+), 31 deletions(-)
 delete mode 100644 arch/arm/include/asm/mach/udc_pxa2xx.h
 create mode 100644 include/linux/platform_data/pxa2xx_udc.h

diff --git a/arch/arm/include/asm/mach/udc_pxa2xx.h b/arch/arm/include/asm/mach/udc_pxa2xx.h
deleted file mode 100644
index ea297ac..0000000
--- a/arch/arm/include/asm/mach/udc_pxa2xx.h
+++ /dev/null
@@ -1,26 +0,0 @@
-/*
- * arch/arm/include/asm/mach/udc_pxa2xx.h
- *
- * This supports machine-specific differences in how the PXA2xx
- * USB Device Controller (UDC) is wired.
- *
- * It is set in linux/arch/arm/mach-pxa/<machine>.c or in
- * linux/arch/mach-ixp4xx/<machine>.c and used in
- * the probe routine of linux/drivers/usb/gadget/pxa2xx_udc.c
- */
-
-struct pxa2xx_udc_mach_info {
-        int  (*udc_is_connected)(void);		/* do we see host? */
-        void (*udc_command)(int cmd);
-#define	PXA2XX_UDC_CMD_CONNECT		0	/* let host see us */
-#define	PXA2XX_UDC_CMD_DISCONNECT	1	/* so host won't see us */
-
-	/* Boards following the design guidelines in the developer's manual,
-	 * with on-chip GPIOs not Lubbock's weird hardware, can have a sane
-	 * VBUS IRQ and omit the methods above.  Store the GPIO number
-	 * here.  Note that sometimes the signals go through inverters...
-	 */
-	bool	gpio_pullup_inverted;
-	int	gpio_pullup;			/* high == pullup activated */
-};
-
diff --git a/arch/arm/mach-ixp4xx/include/mach/udc.h b/arch/arm/mach-ixp4xx/include/mach/udc.h
index 80d6da2..b47cc0d 100644
--- a/arch/arm/mach-ixp4xx/include/mach/udc.h
+++ b/arch/arm/mach-ixp4xx/include/mach/udc.h
@@ -2,7 +2,7 @@
  * arch/arm/mach-ixp4xx/include/mach/udc.h
  *
  */
-#include <asm/mach/udc_pxa2xx.h>
+#include <linux/platform_data_pxa2xx_udc.h>
 
 extern void ixp4xx_set_udc_info(struct pxa2xx_udc_mach_info *info);
 
diff --git a/arch/arm/mach-pxa/include/mach/udc.h b/arch/arm/mach-pxa/include/mach/udc.h
index 2f82332..9a827e3 100644
--- a/arch/arm/mach-pxa/include/mach/udc.h
+++ b/arch/arm/mach-pxa/include/mach/udc.h
@@ -2,7 +2,7 @@
  * arch/arm/mach-pxa/include/mach/udc.h
  *
  */
-#include <asm/mach/udc_pxa2xx.h>
+#include <linux/platform_data/pxa2xx_udc.h>
 
 extern void pxa_set_udc_info(struct pxa2xx_udc_mach_info *info);
 
diff --git a/drivers/usb/gadget/pxa25x_udc.c b/drivers/usb/gadget/pxa25x_udc.c
index 907ad3e..06e498d 100644
--- a/drivers/usb/gadget/pxa25x_udc.c
+++ b/drivers/usb/gadget/pxa25x_udc.c
@@ -29,6 +29,7 @@
 #include <linux/list.h>
 #include <linux/interrupt.h>
 #include <linux/mm.h>
+#include <linux/platform_data/pxa2xx_udc.h>
 #include <linux/platform_device.h>
 #include <linux/dma-mapping.h>
 #include <linux/irq.h>
@@ -60,9 +61,6 @@
 #include <mach/lubbock.h>
 #endif
 
-#include <asm/mach/udc_pxa2xx.h>
-
-
 /*
  * This driver handles the USB Device Controller (UDC) in Intel's PXA 25x
  * series processors.  The UDC for the IXP 4xx series is very similar.
diff --git a/include/linux/platform_data/pxa2xx_udc.h b/include/linux/platform_data/pxa2xx_udc.h
new file mode 100644
index 0000000..c6c5e98
--- /dev/null
+++ b/include/linux/platform_data/pxa2xx_udc.h
@@ -0,0 +1,27 @@
+/*
+ * This supports machine-specific differences in how the PXA2xx
+ * USB Device Controller (UDC) is wired.
+ *
+ * It is set in linux/arch/arm/mach-pxa/<machine>.c or in
+ * linux/arch/mach-ixp4xx/<machine>.c and used in
+ * the probe routine of linux/drivers/usb/gadget/pxa2xx_udc.c
+ */
+#ifndef PXA2XX_UDC_H
+#define PXA2XX_UDC_H
+
+struct pxa2xx_udc_mach_info {
+        int  (*udc_is_connected)(void);		/* do we see host? */
+        void (*udc_command)(int cmd);
+#define	PXA2XX_UDC_CMD_CONNECT		0	/* let host see us */
+#define	PXA2XX_UDC_CMD_DISCONNECT	1	/* so host won't see us */
+
+	/* Boards following the design guidelines in the developer's manual,
+	 * with on-chip GPIOs not Lubbock's weird hardware, can have a sane
+	 * VBUS IRQ and omit the methods above.  Store the GPIO number
+	 * here.  Note that sometimes the signals go through inverters...
+	 */
+	bool	gpio_pullup_inverted;
+	int	gpio_pullup;			/* high == pullup activated */
+};
+
+#endif
-- 
1.7.4.4


^ permalink raw reply related

* [RFC 3/3] ARM/AVR32: get rid of serial_at91.h
From: Russell King @ 2012-09-21  9:36 UTC (permalink / raw)
  To: linux-arm-kernel, linux-serial, linux-usb
  Cc: Nicolas Ferre, Alan Cox, Greg Kroah-Hartman
In-Reply-To: <20120921093530.GB31374@n2100.arm.linux.org.uk>

The definitions provided by serial_at91.h are only used by the
atmel_serial driver, and the function that uses it is never called
from anywhere in the kernel.  Therefore, these definitions are unused
and/or obsolete, and can be removed.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 arch/arm/include/asm/mach/serial_at91.h   |   33 -----------------------------
 arch/avr32/include/asm/mach/serial_at91.h |   33 -----------------------------
 drivers/tty/serial/atmel_serial.c         |   18 ---------------
 3 files changed, 0 insertions(+), 84 deletions(-)
 delete mode 100644 arch/arm/include/asm/mach/serial_at91.h
 delete mode 100644 arch/avr32/include/asm/mach/serial_at91.h

diff --git a/arch/arm/include/asm/mach/serial_at91.h b/arch/arm/include/asm/mach/serial_at91.h
deleted file mode 100644
index ea6d063..0000000
--- a/arch/arm/include/asm/mach/serial_at91.h
+++ /dev/null
@@ -1,33 +0,0 @@
-/*
- *  arch/arm/include/asm/mach/serial_at91.h
- *
- *  Based on serial_sa1100.h  by Nicolas Pitre
- *
- *  Copyright (C) 2002 ATMEL Rousset
- *
- *  Low level machine dependent UART functions.
- */
-
-struct uart_port;
-
-/*
- * This is a temporary structure for registering these
- * functions; it is intended to be discarded after boot.
- */
-struct atmel_port_fns {
-	void	(*set_mctrl)(struct uart_port *, u_int);
-	u_int	(*get_mctrl)(struct uart_port *);
-	void	(*enable_ms)(struct uart_port *);
-	void	(*pm)(struct uart_port *, u_int, u_int);
-	int	(*set_wake)(struct uart_port *, u_int);
-	int	(*open)(struct uart_port *);
-	void	(*close)(struct uart_port *);
-};
-
-#if defined(CONFIG_SERIAL_ATMEL)
-void atmel_register_uart_fns(struct atmel_port_fns *fns);
-#else
-#define atmel_register_uart_fns(fns) do { } while (0)
-#endif
-
-
diff --git a/arch/avr32/include/asm/mach/serial_at91.h b/arch/avr32/include/asm/mach/serial_at91.h
deleted file mode 100644
index 55b317a..0000000
--- a/arch/avr32/include/asm/mach/serial_at91.h
+++ /dev/null
@@ -1,33 +0,0 @@
-/*
- *  linux/include/asm-arm/mach/serial_at91.h
- *
- *  Based on serial_sa1100.h  by Nicolas Pitre
- *
- *  Copyright (C) 2002 ATMEL Rousset
- *
- *  Low level machine dependent UART functions.
- */
-
-struct uart_port;
-
-/*
- * This is a temporary structure for registering these
- * functions; it is intended to be discarded after boot.
- */
-struct atmel_port_fns {
-	void	(*set_mctrl)(struct uart_port *, u_int);
-	u_int	(*get_mctrl)(struct uart_port *);
-	void	(*enable_ms)(struct uart_port *);
-	void	(*pm)(struct uart_port *, u_int, u_int);
-	int	(*set_wake)(struct uart_port *, u_int);
-	int	(*open)(struct uart_port *);
-	void	(*close)(struct uart_port *);
-};
-
-#if defined(CONFIG_SERIAL_ATMEL)
-void atmel_register_uart_fns(struct atmel_port_fns *fns);
-#else
-#define atmel_register_uart_fns(fns) do { } while (0)
-#endif
-
-
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 3d7e1ee..a6134c9 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -43,7 +43,6 @@
 #include <asm/io.h>
 #include <asm/ioctls.h>
 
-#include <asm/mach/serial_at91.h>
 #include <mach/board.h>
 
 #ifdef CONFIG_ARM
@@ -1513,23 +1512,6 @@ static void __devinit atmel_init_port(struct atmel_uart_port *atmel_port,
 	}
 }
 
-/*
- * Register board-specific modem-control line handlers.
- */
-void __init atmel_register_uart_fns(struct atmel_port_fns *fns)
-{
-	if (fns->enable_ms)
-		atmel_pops.enable_ms = fns->enable_ms;
-	if (fns->get_mctrl)
-		atmel_pops.get_mctrl = fns->get_mctrl;
-	if (fns->set_mctrl)
-		atmel_pops.set_mctrl = fns->set_mctrl;
-	atmel_open_hook		= fns->open;
-	atmel_close_hook	= fns->close;
-	atmel_pops.pm		= fns->pm;
-	atmel_pops.set_wake	= fns->set_wake;
-}
-
 struct platform_device *atmel_default_console_device;	/* the serial console device */
 
 #ifdef CONFIG_SERIAL_ATMEL_CONSOLE
-- 
1.7.4.4


^ permalink raw reply related

* Re: [RFC 2/3] ARM: move udc_pxa2xx.h to linux/platform_data
From: Felipe Balbi @ 2012-09-21  9:39 UTC (permalink / raw)
  To: Russell King
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-serial-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, Imre Kaloz, Krzysztof Halasa,
	Eric Miao, Haojian Zhuang, Felipe Balbi, Greg Kroah-Hartman
In-Reply-To: <E1TEzet-00077v-VD-eh5Bv4kxaXIANfyc6IWni62ZND6+EDdj@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 5158 bytes --]

On Fri, Sep 21, 2012 at 10:36:27AM +0100, Russell King wrote:
> Move the PXA2xx/IXP4xx UDC header file into linux/platform_data as it
> only contains a driver platform data structure.
> 
> Signed-off-by: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>

after fixing Eric's comment:

Acked-by: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>

> ---
>  arch/arm/include/asm/mach/udc_pxa2xx.h   |   26 --------------------------
>  arch/arm/mach-ixp4xx/include/mach/udc.h  |    2 +-
>  arch/arm/mach-pxa/include/mach/udc.h     |    2 +-
>  drivers/usb/gadget/pxa25x_udc.c          |    4 +---
>  include/linux/platform_data/pxa2xx_udc.h |   27 +++++++++++++++++++++++++++
>  5 files changed, 30 insertions(+), 31 deletions(-)
>  delete mode 100644 arch/arm/include/asm/mach/udc_pxa2xx.h
>  create mode 100644 include/linux/platform_data/pxa2xx_udc.h
> 
> diff --git a/arch/arm/include/asm/mach/udc_pxa2xx.h b/arch/arm/include/asm/mach/udc_pxa2xx.h
> deleted file mode 100644
> index ea297ac..0000000
> --- a/arch/arm/include/asm/mach/udc_pxa2xx.h
> +++ /dev/null
> @@ -1,26 +0,0 @@
> -/*
> - * arch/arm/include/asm/mach/udc_pxa2xx.h
> - *
> - * This supports machine-specific differences in how the PXA2xx
> - * USB Device Controller (UDC) is wired.
> - *
> - * It is set in linux/arch/arm/mach-pxa/<machine>.c or in
> - * linux/arch/mach-ixp4xx/<machine>.c and used in
> - * the probe routine of linux/drivers/usb/gadget/pxa2xx_udc.c
> - */
> -
> -struct pxa2xx_udc_mach_info {
> -        int  (*udc_is_connected)(void);		/* do we see host? */
> -        void (*udc_command)(int cmd);
> -#define	PXA2XX_UDC_CMD_CONNECT		0	/* let host see us */
> -#define	PXA2XX_UDC_CMD_DISCONNECT	1	/* so host won't see us */
> -
> -	/* Boards following the design guidelines in the developer's manual,
> -	 * with on-chip GPIOs not Lubbock's weird hardware, can have a sane
> -	 * VBUS IRQ and omit the methods above.  Store the GPIO number
> -	 * here.  Note that sometimes the signals go through inverters...
> -	 */
> -	bool	gpio_pullup_inverted;
> -	int	gpio_pullup;			/* high == pullup activated */
> -};
> -
> diff --git a/arch/arm/mach-ixp4xx/include/mach/udc.h b/arch/arm/mach-ixp4xx/include/mach/udc.h
> index 80d6da2..b47cc0d 100644
> --- a/arch/arm/mach-ixp4xx/include/mach/udc.h
> +++ b/arch/arm/mach-ixp4xx/include/mach/udc.h
> @@ -2,7 +2,7 @@
>   * arch/arm/mach-ixp4xx/include/mach/udc.h
>   *
>   */
> -#include <asm/mach/udc_pxa2xx.h>
> +#include <linux/platform_data_pxa2xx_udc.h>
>  
>  extern void ixp4xx_set_udc_info(struct pxa2xx_udc_mach_info *info);
>  
> diff --git a/arch/arm/mach-pxa/include/mach/udc.h b/arch/arm/mach-pxa/include/mach/udc.h
> index 2f82332..9a827e3 100644
> --- a/arch/arm/mach-pxa/include/mach/udc.h
> +++ b/arch/arm/mach-pxa/include/mach/udc.h
> @@ -2,7 +2,7 @@
>   * arch/arm/mach-pxa/include/mach/udc.h
>   *
>   */
> -#include <asm/mach/udc_pxa2xx.h>
> +#include <linux/platform_data/pxa2xx_udc.h>
>  
>  extern void pxa_set_udc_info(struct pxa2xx_udc_mach_info *info);
>  
> diff --git a/drivers/usb/gadget/pxa25x_udc.c b/drivers/usb/gadget/pxa25x_udc.c
> index 907ad3e..06e498d 100644
> --- a/drivers/usb/gadget/pxa25x_udc.c
> +++ b/drivers/usb/gadget/pxa25x_udc.c
> @@ -29,6 +29,7 @@
>  #include <linux/list.h>
>  #include <linux/interrupt.h>
>  #include <linux/mm.h>
> +#include <linux/platform_data/pxa2xx_udc.h>
>  #include <linux/platform_device.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/irq.h>
> @@ -60,9 +61,6 @@
>  #include <mach/lubbock.h>
>  #endif
>  
> -#include <asm/mach/udc_pxa2xx.h>
> -
> -
>  /*
>   * This driver handles the USB Device Controller (UDC) in Intel's PXA 25x
>   * series processors.  The UDC for the IXP 4xx series is very similar.
> diff --git a/include/linux/platform_data/pxa2xx_udc.h b/include/linux/platform_data/pxa2xx_udc.h
> new file mode 100644
> index 0000000..c6c5e98
> --- /dev/null
> +++ b/include/linux/platform_data/pxa2xx_udc.h
> @@ -0,0 +1,27 @@
> +/*
> + * This supports machine-specific differences in how the PXA2xx
> + * USB Device Controller (UDC) is wired.
> + *
> + * It is set in linux/arch/arm/mach-pxa/<machine>.c or in
> + * linux/arch/mach-ixp4xx/<machine>.c and used in
> + * the probe routine of linux/drivers/usb/gadget/pxa2xx_udc.c
> + */
> +#ifndef PXA2XX_UDC_H
> +#define PXA2XX_UDC_H
> +
> +struct pxa2xx_udc_mach_info {
> +        int  (*udc_is_connected)(void);		/* do we see host? */
> +        void (*udc_command)(int cmd);
> +#define	PXA2XX_UDC_CMD_CONNECT		0	/* let host see us */
> +#define	PXA2XX_UDC_CMD_DISCONNECT	1	/* so host won't see us */
> +
> +	/* Boards following the design guidelines in the developer's manual,
> +	 * with on-chip GPIOs not Lubbock's weird hardware, can have a sane
> +	 * VBUS IRQ and omit the methods above.  Store the GPIO number
> +	 * here.  Note that sometimes the signals go through inverters...
> +	 */
> +	bool	gpio_pullup_inverted;
> +	int	gpio_pullup;			/* high == pullup activated */
> +};
> +
> +#endif
> -- 
> 1.7.4.4
> 

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [RFC 2/3] ARM: move udc_pxa2xx.h to linux/platform_data
From: Eric Miao @ 2012-09-21  9:40 UTC (permalink / raw)
  To: Russell King
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-serial-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, Imre Kaloz, Krzysztof Halasa,
	Haojian Zhuang, Felipe Balbi, Greg Kroah-Hartman
In-Reply-To: <E1TEzet-00077v-VD-eh5Bv4kxaXIANfyc6IWni62ZND6+EDdj@public.gmane.org>

On Fri, Sep 21, 2012 at 5:36 PM, Russell King
<rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org> wrote:
> Move the PXA2xx/IXP4xx UDC header file into linux/platform_data as it
> only contains a driver platform data structure.
>
> Signed-off-by: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
> ---
>  arch/arm/include/asm/mach/udc_pxa2xx.h   |   26 --------------------------
>  arch/arm/mach-ixp4xx/include/mach/udc.h  |    2 +-
>  arch/arm/mach-pxa/include/mach/udc.h     |    2 +-
>  drivers/usb/gadget/pxa25x_udc.c          |    4 +---
>  include/linux/platform_data/pxa2xx_udc.h |   27 +++++++++++++++++++++++++++
>  5 files changed, 30 insertions(+), 31 deletions(-)
>  delete mode 100644 arch/arm/include/asm/mach/udc_pxa2xx.h
>  create mode 100644 include/linux/platform_data/pxa2xx_udc.h
>
> diff --git a/arch/arm/include/asm/mach/udc_pxa2xx.h b/arch/arm/include/asm/mach/udc_pxa2xx.h
> deleted file mode 100644
> index ea297ac..0000000
> --- a/arch/arm/include/asm/mach/udc_pxa2xx.h
> +++ /dev/null
> @@ -1,26 +0,0 @@
> -/*
> - * arch/arm/include/asm/mach/udc_pxa2xx.h
> - *
> - * This supports machine-specific differences in how the PXA2xx
> - * USB Device Controller (UDC) is wired.
> - *
> - * It is set in linux/arch/arm/mach-pxa/<machine>.c or in
> - * linux/arch/mach-ixp4xx/<machine>.c and used in
> - * the probe routine of linux/drivers/usb/gadget/pxa2xx_udc.c
> - */
> -
> -struct pxa2xx_udc_mach_info {
> -        int  (*udc_is_connected)(void);                /* do we see host? */
> -        void (*udc_command)(int cmd);
> -#define        PXA2XX_UDC_CMD_CONNECT          0       /* let host see us */
> -#define        PXA2XX_UDC_CMD_DISCONNECT       1       /* so host won't see us */
> -
> -       /* Boards following the design guidelines in the developer's manual,
> -        * with on-chip GPIOs not Lubbock's weird hardware, can have a sane
> -        * VBUS IRQ and omit the methods above.  Store the GPIO number
> -        * here.  Note that sometimes the signals go through inverters...
> -        */
> -       bool    gpio_pullup_inverted;
> -       int     gpio_pullup;                    /* high == pullup activated */
> -};
> -
> diff --git a/arch/arm/mach-ixp4xx/include/mach/udc.h b/arch/arm/mach-ixp4xx/include/mach/udc.h
> index 80d6da2..b47cc0d 100644
> --- a/arch/arm/mach-ixp4xx/include/mach/udc.h
> +++ b/arch/arm/mach-ixp4xx/include/mach/udc.h
> @@ -2,7 +2,7 @@
>   * arch/arm/mach-ixp4xx/include/mach/udc.h
>   *
>   */
> -#include <asm/mach/udc_pxa2xx.h>
> +#include <linux/platform_data_pxa2xx_udc.h>

Guess a typo here, "/" instead of "_"? Otherwise looks good to me

>
>  extern void ixp4xx_set_udc_info(struct pxa2xx_udc_mach_info *info);
>
> diff --git a/arch/arm/mach-pxa/include/mach/udc.h b/arch/arm/mach-pxa/include/mach/udc.h
> index 2f82332..9a827e3 100644
> --- a/arch/arm/mach-pxa/include/mach/udc.h
> +++ b/arch/arm/mach-pxa/include/mach/udc.h
> @@ -2,7 +2,7 @@
>   * arch/arm/mach-pxa/include/mach/udc.h
>   *
>   */
> -#include <asm/mach/udc_pxa2xx.h>
> +#include <linux/platform_data/pxa2xx_udc.h>
>
>  extern void pxa_set_udc_info(struct pxa2xx_udc_mach_info *info);
>
> diff --git a/drivers/usb/gadget/pxa25x_udc.c b/drivers/usb/gadget/pxa25x_udc.c
> index 907ad3e..06e498d 100644
> --- a/drivers/usb/gadget/pxa25x_udc.c
> +++ b/drivers/usb/gadget/pxa25x_udc.c
> @@ -29,6 +29,7 @@
>  #include <linux/list.h>
>  #include <linux/interrupt.h>
>  #include <linux/mm.h>
> +#include <linux/platform_data/pxa2xx_udc.h>
>  #include <linux/platform_device.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/irq.h>
> @@ -60,9 +61,6 @@
>  #include <mach/lubbock.h>
>  #endif
>
> -#include <asm/mach/udc_pxa2xx.h>
> -
> -
>  /*
>   * This driver handles the USB Device Controller (UDC) in Intel's PXA 25x
>   * series processors.  The UDC for the IXP 4xx series is very similar.
> diff --git a/include/linux/platform_data/pxa2xx_udc.h b/include/linux/platform_data/pxa2xx_udc.h
> new file mode 100644
> index 0000000..c6c5e98
> --- /dev/null
> +++ b/include/linux/platform_data/pxa2xx_udc.h
> @@ -0,0 +1,27 @@
> +/*
> + * This supports machine-specific differences in how the PXA2xx
> + * USB Device Controller (UDC) is wired.
> + *
> + * It is set in linux/arch/arm/mach-pxa/<machine>.c or in
> + * linux/arch/mach-ixp4xx/<machine>.c and used in
> + * the probe routine of linux/drivers/usb/gadget/pxa2xx_udc.c
> + */
> +#ifndef PXA2XX_UDC_H
> +#define PXA2XX_UDC_H
> +
> +struct pxa2xx_udc_mach_info {
> +        int  (*udc_is_connected)(void);                /* do we see host? */
> +        void (*udc_command)(int cmd);
> +#define        PXA2XX_UDC_CMD_CONNECT          0       /* let host see us */
> +#define        PXA2XX_UDC_CMD_DISCONNECT       1       /* so host won't see us */
> +
> +       /* Boards following the design guidelines in the developer's manual,
> +        * with on-chip GPIOs not Lubbock's weird hardware, can have a sane
> +        * VBUS IRQ and omit the methods above.  Store the GPIO number
> +        * here.  Note that sometimes the signals go through inverters...
> +        */
> +       bool    gpio_pullup_inverted;
> +       int     gpio_pullup;                    /* high == pullup activated */
> +};
> +
> +#endif
> --
> 1.7.4.4
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [RFC 2/3] ARM: move udc_pxa2xx.h to linux/platform_data
From: Russell King - ARM Linux @ 2012-09-21  9:48 UTC (permalink / raw)
  To: Eric Miao
  Cc: linux-arm-kernel, linux-serial, linux-usb, Imre Kaloz,
	Krzysztof Halasa, Haojian Zhuang, Felipe Balbi,
	Greg Kroah-Hartman
In-Reply-To: <CAMPhdO_xqpqUUbSvi1zC4eT6vr0xFO-70+ieknjMWk9h0iWfHg@mail.gmail.com>

On Fri, Sep 21, 2012 at 05:40:07PM +0800, Eric Miao wrote:
> On Fri, Sep 21, 2012 at 5:36 PM, Russell King
> <rmk+kernel@arm.linux.org.uk> wrote:
> > diff --git a/arch/arm/mach-ixp4xx/include/mach/udc.h b/arch/arm/mach-ixp4xx/include/mach/udc.h
> > index 80d6da2..b47cc0d 100644
> > --- a/arch/arm/mach-ixp4xx/include/mach/udc.h
> > +++ b/arch/arm/mach-ixp4xx/include/mach/udc.h
> > @@ -2,7 +2,7 @@
> >   * arch/arm/mach-ixp4xx/include/mach/udc.h
> >   *
> >   */
> > -#include <asm/mach/udc_pxa2xx.h>
> > +#include <linux/platform_data_pxa2xx_udc.h>
> 
> Guess a typo here, "/" instead of "_"? Otherwise looks good to me

Yea, fixed.

^ permalink raw reply

* [PATCH] serial: omap: fix the overrun case
From: Shubhrajyoti D @ 2012-09-21 10:22 UTC (permalink / raw)
  To: linux-serial; +Cc: linux-omap, linux-arm-kernel, sourav.poddar, Shubhrajyoti D

Overrun also causes an internal flag to be set, which disables further
reception. Before the next frame can
be received, the MPU must:
• Reset the RX FIFO.
• clear the internal flag.

In the uart mode a dummy read is needed. Add the same.

Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
---
- functional testing on omap4sdp
- Verified idle and suspend path hits off on beagle.

 drivers/tty/serial/omap-serial.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index a0d4460..bc22a2b 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -334,6 +334,13 @@ static unsigned int check_modem_status(struct uart_omap_port *up)
 static void serial_omap_rlsi(struct uart_omap_port *up, unsigned int lsr)
 {
 	unsigned int flag;
+	unsigned char ch = 0;
+
+	if (!(lsr & UART_LSR_BRK_ERROR_BITS))
+		return;
+
+	if (likely(lsr & UART_LSR_DR))
+		ch = serial_in(up, UART_RX);
 
 	up->port.icount.rx++;
 	flag = TTY_NORMAL;
-- 
1.7.5.4

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* Re: [PATCH] serial: omap: fix the overrun case
From: Felipe Balbi @ 2012-09-21 11:00 UTC (permalink / raw)
  To: Shubhrajyoti D; +Cc: linux-serial, linux-omap, linux-arm-kernel, sourav.poddar
In-Reply-To: <1348222976-7241-1-git-send-email-shubhrajyoti@ti.com>

[-- Attachment #1: Type: text/plain, Size: 1455 bytes --]

On Fri, Sep 21, 2012 at 03:52:56PM +0530, Shubhrajyoti D wrote:
> Overrun also causes an internal flag to be set, which disables further
> reception. Before the next frame can
> be received, the MPU must:
> • Reset the RX FIFO.
> • clear the internal flag.
> 
> In the uart mode a dummy read is needed. Add the same.

Very nice patch but I think commit log can be a bit more verbose.

Please make the problem a little clearer. Why do we even get that
interrupt fired if BRK_ERROR_BITS aren't set ?

> Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
> ---
> - functional testing on omap4sdp
> - Verified idle and suspend path hits off on beagle.
> 
>  drivers/tty/serial/omap-serial.c |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
> index a0d4460..bc22a2b 100644
> --- a/drivers/tty/serial/omap-serial.c
> +++ b/drivers/tty/serial/omap-serial.c
> @@ -334,6 +334,13 @@ static unsigned int check_modem_status(struct uart_omap_port *up)
>  static void serial_omap_rlsi(struct uart_omap_port *up, unsigned int lsr)
>  {
>  	unsigned int flag;
> +	unsigned char ch = 0;
> +
> +	if (!(lsr & UART_LSR_BRK_ERROR_BITS))
> +		return;
> +
> +	if (likely(lsr & UART_LSR_DR))
> +		ch = serial_in(up, UART_RX);

Maybe add a comment before this condition stating why this character
read is necessary ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH] serial: omap: fix the overrun case
From: Shubhrajyoti @ 2012-09-21 11:16 UTC (permalink / raw)
  To: balbi; +Cc: linux-serial, linux-omap, linux-arm-kernel, sourav.poddar
In-Reply-To: <20120921110050.GC16003@arwen.pp.htv.fi>

On Friday 21 September 2012 04:30 PM, Felipe Balbi wrote:
> On Fri, Sep 21, 2012 at 03:52:56PM +0530, Shubhrajyoti D wrote:
>> Overrun also causes an internal flag to be set, which disables further
>> reception. Before the next frame can
>> be received, the MPU must:
>> • Reset the RX FIFO.
>> • clear the internal flag.
>>
>> In the uart mode a dummy read is needed. Add the same.
> Very nice patch but I think commit log can be a bit more verbose.
ok
> Please make the problem a little clearer. Why do we even get that
> interrupt fired if BRK_ERROR_BITS aren't set ?
I did not get this point.

it it is !  BRK_ERROR_BITS I return.

If it is and there is data in the fifo then a read is done.
>> Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
>> ---
>> - functional testing on omap4sdp
>> - Verified idle and suspend path hits off on beagle.
>>
>>  drivers/tty/serial/omap-serial.c |    7 +++++++
>>  1 files changed, 7 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
>> index a0d4460..bc22a2b 100644
>> --- a/drivers/tty/serial/omap-serial.c
>> +++ b/drivers/tty/serial/omap-serial.c
>> @@ -334,6 +334,13 @@ static unsigned int check_modem_status(struct uart_omap_port *up)
>>  static void serial_omap_rlsi(struct uart_omap_port *up, unsigned int lsr)
>>  {
>>  	unsigned int flag;
>> +	unsigned char ch = 0;
>> +
>> +	if (!(lsr & UART_LSR_BRK_ERROR_BITS))
>> +		return;
>> +
>> +	if (likely(lsr & UART_LSR_DR))
>> +		ch = serial_in(up, UART_RX);
> Maybe add a comment before this condition stating why this character
> read is necessary ?
OK.
>

--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] serial: omap: fix the overrun case
From: Poddar, Sourav @ 2012-09-21 11:18 UTC (permalink / raw)
  To: Shubhrajyoti D; +Cc: linux-serial, linux-omap, linux-arm-kernel
In-Reply-To: <1348222976-7241-1-git-send-email-shubhrajyoti@ti.com>

Hi,

On Fri, Sep 21, 2012 at 3:52 PM, Shubhrajyoti D <shubhrajyoti@ti.com> wrote:
> Overrun also causes an internal flag to be set, which disables further
> reception. Before the next frame can
> be received, the MPU must:
> • Reset the RX FIFO.
> • clear the internal flag.
>
> In the uart mode a dummy read is needed. Add the same.
>
> Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
> ---
> - functional testing on omap4sdp
> - Verified idle and suspend path hits off on beagle.
>
>  drivers/tty/serial/omap-serial.c |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
> index a0d4460..bc22a2b 100644
> --- a/drivers/tty/serial/omap-serial.c
> +++ b/drivers/tty/serial/omap-serial.c
> @@ -334,6 +334,13 @@ static unsigned int check_modem_status(struct uart_omap_port *up)
>  static void serial_omap_rlsi(struct uart_omap_port *up, unsigned int lsr)
>  {
>         unsigned int flag;
> +       unsigned char ch = 0;
> +
> +       if (!(lsr & UART_LSR_BRK_ERROR_BITS))
By using this flag, you are trying to take into account not just the
overrun case but also
frame, parity and break condition case as the flag is the OR of all these.

I suppose the commit log should reflect this.
> +               return;
> +
> +       if (likely(lsr & UART_LSR_DR))
> +               ch = serial_in(up, UART_RX);
>
>         up->port.icount.rx++;
>         flag = TTY_NORMAL;
> --
> 1.7.5.4
>
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox