On 11. 10. 26, 21:55, Magnus Lindholm wrote:
...
--- /dev/null
+++ b/drivers/tty/hvc/hvc_m3000.c
@@ -0,0 +1,205 @@
+struct m3000_console {
+ int stdin;
+ bool ready;
+ u8 buffer[256];
+ raw_spinlock_t buffer_lock;
+ struct timer_list poll_timer;
+ struct mutex poll_mutex; /* Serialize open against synchronous close. */
+ spinlock_t poll_lock; /* Protect active state and timer rearming. */
+ bool poll_active;
Perhaps group these a bit better, so that there are not such holes?
+};
...
+/*
+ * HVC's idle backoff can exceed the firmware input FIFO's capacity.
+ * Wake its worker each tick while open; keep PROM calls out of the timer.
+ */
+static void m3000_poll_tick(struct timer_list *timer)
+{
+ struct m3000_console *con = timer_container_of(con, timer, poll_timer);
+ unsigned long flags;
+
+ spin_lock_irqsave(&con->poll_lock, flags);
Any reason not to use guard()s?
+ if (con->poll_active) {
+ hvc_kick();
+ mod_timer(&con->poll_timer, jiffies + 1);
+ }
+ spin_unlock_irqrestore(&con->poll_lock, flags);
+}
+
+static int m3000_open(struct hvc_struct *hp, int data)
+{
+ struct m3000_console *con = m3000_console(hp->vtermno);
+ unsigned long flags;
+
+ if (!con)
+ return -ENODEV;
+ mutex_lock(&con->poll_mutex);
+ spin_lock_irqsave(&con->poll_lock, flags);
Same here.
+ con->poll_active = true;
+ mod_timer(&con->poll_timer, jiffies + 1);
+ spin_unlock_irqrestore(&con->poll_lock, flags);
+ mutex_unlock(&con->poll_mutex);
+ return 0;
+}
+
+static void m3000_close(struct hvc_struct *hp, int data)
+{
+ struct m3000_console *con = m3000_console(hp->vtermno);
+ unsigned long flags;
+
+ if (!con)
+ return;
+ mutex_lock(&con->poll_mutex);
+ spin_lock_irqsave(&con->poll_lock, flags);
And here.
+ con->poll_active = false;
+ spin_unlock_irqrestore(&con->poll_lock, flags);
+ /* Block reopen until the callback has finished and cannot rearm. */
+ timer_delete_sync(&con->poll_timer);
+ mutex_unlock(&con->poll_mutex);
+}
+
+static ssize_t m3000_io(struct m3000_console *con, u8 *in,
+ const u8 *out, size_t count)
+{
+ unsigned long args[7], flags;
+ bool input = in;
You can test "in" below directly, right?
+ int ret;
+
+ if (!count)
+ return 0;
+ count = min(count, input ? 1 : sizeof(con->buffer));
+ /* Console writes can run with interrupts disabled; this lock cannot
sleep. */
+ raw_spin_lock_irqsave(&con->buffer_lock, flags);
scoped_guard()
+ if (!input)
+ memcpy(con->buffer, out, count);
+ args[0] = (unsigned long)(input ? "read" : "write");
+ args[1] = 3;
+ args[2] = 1;
+ args[3] = (unsigned int)(input ? con->stdin : prom_stdout);
+ args[4] = (unsigned long)con->buffer;
+ args[5] = count;
+ args[6] = (unsigned long)-1;
+ p1275_cmd_direct(args);
+ ret = (int)args[6];
+ if (ret > 0 && ret <= count && input)
+ memcpy(in, con->buffer, ret);
+ raw_spin_unlock_irqrestore(&con->buffer_lock, flags);
+ if (ret == -2 || ret == 0)
That's full of magic constants. Can we have defines for those?
+ return 0;
+ if (ret < 0 || ret > count)
+ return -EIO;
+ return ret;
thanks,
--
js
suse labs
