* [PATCH 7/7] hvc: add an M3000 firmware console backend
[not found] <20261002161515.932316-1-linmag7@gmail.com>
@ 2026-10-02 16:14 ` Magnus Lindholm
2026-10-04 12:50 ` Greg Kroah-Hartman
0 siblings, 1 reply; 3+ messages in thread
From: Magnus Lindholm @ 2026-10-02 16:14 UTC (permalink / raw)
To: sparclinux, David S . Miller, Andreas Larsson
Cc: linux-kernel, Magnus Lindholm, Greg Kroah-Hartman, Jiri Slaby,
linuxppc-dev, linux-serial
Provide hvc0 through the IKKAKU Open Firmware pseudo-console, validating
model and console properties before matching. Serialize firmware calls,
keep the bounce buffer in locked mappings and poll input each open tick.
Keep output buffers const and bound no-progress retries because HVC
retries EAGAIN indefinitely. A stalled firmware call remains unbounded.
Use the existing SPARC p1275 interface and HVC output contract.
Signed-off-by: Magnus Lindholm <linmag7@gmail.com>
---
MAINTAINERS | 6 ++
drivers/tty/hvc/Kconfig | 10 +++
drivers/tty/hvc/Makefile | 2 +
drivers/tty/hvc/hvc_m3000.c | 167 ++++++++++++++++++++++++++++++++++++
4 files changed, 185 insertions(+)
create mode 100644 drivers/tty/hvc/hvc_m3000.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 3a19da74d00c..5bc9b1cabd0a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10728,6 +10728,12 @@ L: platform-driver-x86@vger.kernel.org
S: Maintained
F: drivers/platform/x86/fujitsu-laptop.c
+FUJITSU M3000 FIRMWARE CONSOLE
+M: Magnus Lindholm <linmag7@gmail.com>
+L: linux-serial@vger.kernel.org
+S: Maintained
+F: drivers/tty/hvc/hvc_m3000.c
+
FUJITSU TABLET EXTRAS
M: Robert Gerlach <khnz@gmx.de>
L: platform-driver-x86@vger.kernel.org
diff --git a/drivers/tty/hvc/Kconfig b/drivers/tty/hvc/Kconfig
index 5866195de26a..0feb7ec98f4f 100644
--- a/drivers/tty/hvc/Kconfig
+++ b/drivers/tty/hvc/Kconfig
@@ -134,3 +134,13 @@ config HVCS
will depend on arch specific APIs exported from hvcserver.ko
which will also be compiled when this driver is built as a
module.
+
+config HVC_M3000
+ bool "M3000 firmware pseudo-console"
+ depends on SPARC64_VII
+ select HVC_DRIVER
+ help
+ Provide hvc0 using Open Firmware stdin/stdout on IKKAKU only.
+ Input is polled; no native UART registers are accessed.
+ Firmware calls are serialized and use a locked-image bounce buffer.
+ Say Y to use the M3000 firmware pseudo-console as hvc0.
diff --git a/drivers/tty/hvc/Makefile b/drivers/tty/hvc/Makefile
index 98880e357941..5b0277deb7c9 100644
--- a/drivers/tty/hvc/Makefile
+++ b/drivers/tty/hvc/Makefile
@@ -11,3 +11,5 @@ obj-$(CONFIG_HVC_IUCV) += hvc_iucv.o
obj-$(CONFIG_HVC_UDBG) += hvc_udbg.o
obj-$(CONFIG_HVC_RISCV_SBI) += hvc_riscv_sbi.o
obj-$(CONFIG_HVCS) += hvcs.o
+
+obj-$(CONFIG_HVC_M3000) += hvc_m3000.o
diff --git a/drivers/tty/hvc/hvc_m3000.c b/drivers/tty/hvc/hvc_m3000.c
new file mode 100644
index 000000000000..0ef4c888a6a2
--- /dev/null
+++ b/drivers/tty/hvc/hvc_m3000.c
@@ -0,0 +1,167 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Experimental M3000 Open Firmware pseudo-console backend. */
+#include <linux/console.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/irqflags.h>
+#include <linux/string.h>
+#include <linux/spinlock.h>
+#include <linux/timer.h>
+#include <asm/oplib.h>
+#include <asm/spitfire.h>
+#include "hvc_console.h"
+
+#define M3000_WRITE_ATTEMPTS 8
+
+static int m3000_stdin;
+static bool m3000_ready;
+/* Keep firmware buffers within the locked kernel image mappings. */
+static u8 m3000_buffer[256];
+static DEFINE_RAW_SPINLOCK(m3000_buffer_lock);
+static struct timer_list m3000_poll_timer;
+static bool m3000_poll_active;
+
+/*
+ * 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)
+{
+ if (!READ_ONCE(m3000_poll_active))
+ return;
+ hvc_kick();
+ mod_timer(&m3000_poll_timer, jiffies + 1);
+}
+
+static int m3000_open(struct hvc_struct *hp, int data)
+{
+ WRITE_ONCE(m3000_poll_active, true);
+ mod_timer(&m3000_poll_timer, jiffies + 1);
+ return 0;
+}
+
+static void m3000_close(struct hvc_struct *hp, int data)
+{
+ WRITE_ONCE(m3000_poll_active, false);
+ timer_delete_sync(&m3000_poll_timer);
+}
+
+static ssize_t m3000_io(u8 *in, const u8 *out, size_t count)
+{
+ unsigned long args[7], flags;
+ bool input = in != NULL;
+ int ret;
+
+ if (!count)
+ return 0;
+ count = min_t(size_t, count, input ? 1 : sizeof(m3000_buffer));
+ /* Serialize the bounce buffer; p1275 separately serializes firmware. */
+ raw_spin_lock_irqsave(&m3000_buffer_lock, flags);
+ if (!input)
+ memcpy(m3000_buffer, out, count);
+ args[0] = (unsigned long)(input ? "read" : "write");
+ args[1] = 3;
+ args[2] = 1;
+ args[3] = (unsigned int)(input ? m3000_stdin : prom_stdout);
+ args[4] = (unsigned long)m3000_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, m3000_buffer, ret);
+ raw_spin_unlock_irqrestore(&m3000_buffer_lock, flags);
+ if (ret == -2 || ret == 0)
+ return input ? 0 : -EAGAIN;
+ if (ret < 0 || ret > count)
+ return -EIO;
+ return ret;
+}
+
+static ssize_t m3000_get(u32 termno, u8 *buf, size_t count)
+{
+ return m3000_io(buf, NULL, count);
+}
+
+static ssize_t m3000_put(u32 termno, const u8 *buf, size_t count)
+{
+ ssize_t ret;
+ int attempt;
+
+ /*
+ * HVC retries -EAGAIN forever, so drop this chunk without logging when
+ * the no-progress budget expires. This cannot bound a stalled firmware
+ * call or lock acquisition.
+ */
+ for (attempt = 0; attempt < M3000_WRITE_ATTEMPTS; attempt++) {
+ ret = m3000_io(NULL, buf, count);
+ if (ret != -EAGAIN)
+ return ret;
+ cpu_relax();
+ }
+ return -EIO;
+}
+
+static const struct hv_ops m3000_ops = {
+ .get_chars = m3000_get,
+ .put_chars = m3000_put,
+ .notifier_add = m3000_open,
+ .notifier_del = m3000_close,
+ .notifier_hangup = m3000_close,
+};
+
+static bool __init m3000_property_matches(phandle node, const char *prop,
+ const char *expected)
+{
+ char value[64];
+ int len;
+
+ len = prom_getproperty(node, prop, value, sizeof(value) - 1);
+ if (len <= 0)
+ return false;
+ value[len] = '\0';
+ return !strcmp(value, expected);
+}
+
+static int __init m3000_console_init(void)
+{
+ phandle node;
+ int ret;
+
+ if (tlb_type != sparc64_vii)
+ return -ENODEV;
+ if (!m3000_property_matches(prom_finddevice("/"), "model", "IKKAKU"))
+ return -ENODEV;
+ m3000_stdin = prom_getint(prom_chosen_node, "stdin");
+ if (!m3000_stdin || m3000_stdin == -1 || !prom_stdout || prom_stdout == -1)
+ return -ENODEV;
+ node = prom_inst2pkg(prom_stdout);
+ if (!m3000_property_matches(node, "name", "pseudo-console"))
+ return -ENODEV;
+ node = prom_inst2pkg(m3000_stdin);
+ if (!m3000_property_matches(node, "name", "pseudo-console"))
+ return -ENODEV;
+ ret = hvc_instantiate(0, 0, &m3000_ops);
+ if (ret < 0)
+ return ret;
+ m3000_ready = true;
+ return 0;
+}
+console_initcall(m3000_console_init);
+
+static int __init m3000_tty_init(void)
+{
+ struct hvc_struct *hp;
+
+ if (!m3000_ready)
+ return -ENODEV;
+ timer_setup(&m3000_poll_timer, m3000_poll_tick, 0);
+ hp = hvc_alloc(0, 0, &m3000_ops, sizeof(m3000_buffer));
+ if (IS_ERR(hp))
+ return PTR_ERR(hp);
+ hp->ws.ws_row = 24;
+ hp->ws.ws_col = 80;
+ pr_info("M3000: firmware-backed hvc0 tty ready (serialized, polled input)\n");
+ return 0;
+}
+device_initcall(m3000_tty_init);
--
2.43.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 7/7] hvc: add an M3000 firmware console backend
2026-10-02 16:14 ` [PATCH 7/7] hvc: add an M3000 firmware console backend Magnus Lindholm
@ 2026-10-04 12:50 ` Greg Kroah-Hartman
2026-10-04 15:24 ` Magnus Lindholm
0 siblings, 1 reply; 3+ messages in thread
From: Greg Kroah-Hartman @ 2026-10-04 12:50 UTC (permalink / raw)
To: Magnus Lindholm
Cc: sparclinux, David S . Miller, Andreas Larsson, linux-kernel,
Jiri Slaby, linuxppc-dev, linux-serial
On Fri, Oct 02, 2026 at 06:14:29PM +0200, Magnus Lindholm wrote:
> Provide hvc0 through the IKKAKU Open Firmware pseudo-console, validating
> model and console properties before matching. Serialize firmware calls,
> keep the bounce buffer in locked mappings and poll input each open tick.
>
> Keep output buffers const and bound no-progress retries because HVC
> retries EAGAIN indefinitely. A stalled firmware call remains unbounded.
> Use the existing SPARC p1275 interface and HVC output contract.
>
> Signed-off-by: Magnus Lindholm <linmag7@gmail.com>
> ---
> MAINTAINERS | 6 ++
> drivers/tty/hvc/Kconfig | 10 +++
> drivers/tty/hvc/Makefile | 2 +
> drivers/tty/hvc/hvc_m3000.c | 167 ++++++++++++++++++++++++++++++++++++
> 4 files changed, 185 insertions(+)
> create mode 100644 drivers/tty/hvc/hvc_m3000.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3a19da74d00c..5bc9b1cabd0a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10728,6 +10728,12 @@ L: platform-driver-x86@vger.kernel.org
> S: Maintained
> F: drivers/platform/x86/fujitsu-laptop.c
>
> +FUJITSU M3000 FIRMWARE CONSOLE
> +M: Magnus Lindholm <linmag7@gmail.com>
> +L: linux-serial@vger.kernel.org
> +S: Maintained
> +F: drivers/tty/hvc/hvc_m3000.c
> +
> FUJITSU TABLET EXTRAS
> M: Robert Gerlach <khnz@gmx.de>
> L: platform-driver-x86@vger.kernel.org
> diff --git a/drivers/tty/hvc/Kconfig b/drivers/tty/hvc/Kconfig
> index 5866195de26a..0feb7ec98f4f 100644
> --- a/drivers/tty/hvc/Kconfig
> +++ b/drivers/tty/hvc/Kconfig
> @@ -134,3 +134,13 @@ config HVCS
> will depend on arch specific APIs exported from hvcserver.ko
> which will also be compiled when this driver is built as a
> module.
> +
> +config HVC_M3000
> + bool "M3000 firmware pseudo-console"
> + depends on SPARC64_VII
> + select HVC_DRIVER
> + help
> + Provide hvc0 using Open Firmware stdin/stdout on IKKAKU only.
> + Input is polled; no native UART registers are accessed.
> + Firmware calls are serialized and use a locked-image bounce buffer.
> + Say Y to use the M3000 firmware pseudo-console as hvc0.
> diff --git a/drivers/tty/hvc/Makefile b/drivers/tty/hvc/Makefile
> index 98880e357941..5b0277deb7c9 100644
> --- a/drivers/tty/hvc/Makefile
> +++ b/drivers/tty/hvc/Makefile
> @@ -11,3 +11,5 @@ obj-$(CONFIG_HVC_IUCV) += hvc_iucv.o
> obj-$(CONFIG_HVC_UDBG) += hvc_udbg.o
> obj-$(CONFIG_HVC_RISCV_SBI) += hvc_riscv_sbi.o
> obj-$(CONFIG_HVCS) += hvcs.o
> +
> +obj-$(CONFIG_HVC_M3000) += hvc_m3000.o
No need for a blank line, right? And why no tab used here?
> diff --git a/drivers/tty/hvc/hvc_m3000.c b/drivers/tty/hvc/hvc_m3000.c
> new file mode 100644
> index 000000000000..0ef4c888a6a2
> --- /dev/null
> +++ b/drivers/tty/hvc/hvc_m3000.c
> @@ -0,0 +1,167 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Experimental M3000 Open Firmware pseudo-console backend. */
What will make it not "experimental"?
And no copyright?
> +#include <linux/console.h>
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/irqflags.h>
> +#include <linux/string.h>
> +#include <linux/spinlock.h>
> +#include <linux/timer.h>
> +#include <asm/oplib.h>
> +#include <asm/spitfire.h>
> +#include "hvc_console.h"
> +
> +#define M3000_WRITE_ATTEMPTS 8
why 8?
> +
> +static int m3000_stdin;
> +static bool m3000_ready;
> +/* Keep firmware buffers within the locked kernel image mappings. */
> +static u8 m3000_buffer[256];
> +static DEFINE_RAW_SPINLOCK(m3000_buffer_lock);
> +static struct timer_list m3000_poll_timer;
> +static bool m3000_poll_active;
Shouldn't these be in a structure attached to the console somewhere?
Otherwise you have limited yourself to just one of these.
> +
> +/*
> + * 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)
> +{
> + if (!READ_ONCE(m3000_poll_active))
> + return;
Why are you accessing m3000_poll_active like this in an attempt to not
use a real lock? Are you _sure_ a bool will work this way properly?
> + hvc_kick();
> + mod_timer(&m3000_poll_timer, jiffies + 1);
> +}
> +
> +static int m3000_open(struct hvc_struct *hp, int data)
> +{
> + WRITE_ONCE(m3000_poll_active, true);
> + mod_timer(&m3000_poll_timer, jiffies + 1);
> + return 0;
> +}
> +
> +static void m3000_close(struct hvc_struct *hp, int data)
> +{
> + WRITE_ONCE(m3000_poll_active, false);
> + timer_delete_sync(&m3000_poll_timer);
> +}
> +
> +static ssize_t m3000_io(u8 *in, const u8 *out, size_t count)
> +{
> + unsigned long args[7], flags;
> + bool input = in != NULL;
> + int ret;
> +
> + if (!count)
> + return 0;
> + count = min_t(size_t, count, input ? 1 : sizeof(m3000_buffer));
No need for min_t(), why not just min()?
> + /* Serialize the bounce buffer; p1275 separately serializes firmware. */
> + raw_spin_lock_irqsave(&m3000_buffer_lock, flags);
why is this a "raw" spinlock?
> + if (!input)
> + memcpy(m3000_buffer, out, count);
> + args[0] = (unsigned long)(input ? "read" : "write");
A string being cast to an unsigned long? Are you _sure_?
> + args[1] = 3;
> + args[2] = 1;
> + args[3] = (unsigned int)(input ? m3000_stdin : prom_stdout);
> + args[4] = (unsigned long)m3000_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, m3000_buffer, ret);
> + raw_spin_unlock_irqrestore(&m3000_buffer_lock, flags);
> + if (ret == -2 || ret == 0)
> + return input ? 0 : -EAGAIN;
> + if (ret < 0 || ret > count)
> + return -EIO;
> + return ret;
> +}
> +
> +static ssize_t m3000_get(u32 termno, u8 *buf, size_t count)
> +{
> + return m3000_io(buf, NULL, count);
> +}
> +
> +static ssize_t m3000_put(u32 termno, const u8 *buf, size_t count)
> +{
> + ssize_t ret;
> + int attempt;
> +
> + /*
> + * HVC retries -EAGAIN forever, so drop this chunk without logging when
> + * the no-progress budget expires. This cannot bound a stalled firmware
> + * call or lock acquisition.
> + */
> + for (attempt = 0; attempt < M3000_WRITE_ATTEMPTS; attempt++) {
> + ret = m3000_io(NULL, buf, count);
> + if (ret != -EAGAIN)
> + return ret;
> + cpu_relax();
> + }
> + return -EIO;
> +}
> +
> +static const struct hv_ops m3000_ops = {
> + .get_chars = m3000_get,
> + .put_chars = m3000_put,
> + .notifier_add = m3000_open,
> + .notifier_del = m3000_close,
> + .notifier_hangup = m3000_close,
> +};
> +
> +static bool __init m3000_property_matches(phandle node, const char *prop,
> + const char *expected)
> +{
> + char value[64];
> + int len;
> +
> + len = prom_getproperty(node, prop, value, sizeof(value) - 1);
> + if (len <= 0)
> + return false;
> + value[len] = '\0';
> + return !strcmp(value, expected);
> +}
> +
> +static int __init m3000_console_init(void)
> +{
> + phandle node;
> + int ret;
> +
> + if (tlb_type != sparc64_vii)
> + return -ENODEV;
> + if (!m3000_property_matches(prom_finddevice("/"), "model", "IKKAKU"))
> + return -ENODEV;
> + m3000_stdin = prom_getint(prom_chosen_node, "stdin");
> + if (!m3000_stdin || m3000_stdin == -1 || !prom_stdout || prom_stdout == -1)
> + return -ENODEV;
> + node = prom_inst2pkg(prom_stdout);
> + if (!m3000_property_matches(node, "name", "pseudo-console"))
> + return -ENODEV;
> + node = prom_inst2pkg(m3000_stdin);
> + if (!m3000_property_matches(node, "name", "pseudo-console"))
> + return -ENODEV;
> + ret = hvc_instantiate(0, 0, &m3000_ops);
> + if (ret < 0)
> + return ret;
> + m3000_ready = true;
> + return 0;
> +}
> +console_initcall(m3000_console_init);
> +
> +static int __init m3000_tty_init(void)
> +{
> + struct hvc_struct *hp;
> +
> + if (!m3000_ready)
> + return -ENODEV;
> + timer_setup(&m3000_poll_timer, m3000_poll_tick, 0);
> + hp = hvc_alloc(0, 0, &m3000_ops, sizeof(m3000_buffer));
> + if (IS_ERR(hp))
> + return PTR_ERR(hp);
> + hp->ws.ws_row = 24;
> + hp->ws.ws_col = 80;
> + pr_info("M3000: firmware-backed hvc0 tty ready (serialized, polled input)\n");
When drivers work properly, they are quiet.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 7/7] hvc: add an M3000 firmware console backend
2026-10-04 12:50 ` Greg Kroah-Hartman
@ 2026-10-04 15:24 ` Magnus Lindholm
0 siblings, 0 replies; 3+ messages in thread
From: Magnus Lindholm @ 2026-10-04 15:24 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: sparclinux, David S . Miller, Andreas Larsson, linux-kernel,
Jiri Slaby, linuxppc-dev, linux-serial
Hi Greg,
Thanks alot for taking the time to review this.
On Sun, Oct 4, 2026 at 2:50 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> > @@ -11,3 +11,5 @@ obj-$(CONFIG_HVC_IUCV) += hvc_iucv.o
> > obj-$(CONFIG_HVC_UDBG) += hvc_udbg.o
> > obj-$(CONFIG_HVC_RISCV_SBI) += hvc_riscv_sbi.o
> > obj-$(CONFIG_HVCS) += hvcs.o
> > +
> > +obj-$(CONFIG_HVC_M3000) += hvc_m3000.o
>
> No need for a blank line, right? And why no tab used here?
>
Agreed; I have removed the blank line and matched the Makefile's tab
alignment.
> > diff --git a/drivers/tty/hvc/hvc_m3000.c b/drivers/tty/hvc/hvc_m3000.c
> > new file mode 100644
> > index 000000000000..0ef4c888a6a2
> > --- /dev/null
> > +++ b/drivers/tty/hvc/hvc_m3000.c
> > @@ -0,0 +1,167 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Experimental M3000 Open Firmware pseudo-console backend. */
>
> What will make it not "experimental"?
>
That was stale wording from bring-up, rather than a defined support
status. I have removed it.
> And no copyright?
>
>
I'll add a copyright notice for the v2.
> > +
> > +#define M3000_WRITE_ATTEMPTS 8
>
> why 8?
>
There is no hardware requirement for eight. The intent was to bound
no-progress retries because HVC's console write path retries -EAGAIN
indefinitely; I will revisit the retry policy before sending v2.
> > +
> > +static int m3000_stdin;
> > +static bool m3000_ready;
> > +/* Keep firmware buffers within the locked kernel image mappings. */
> > +static u8 m3000_buffer[256];
> > +static DEFINE_RAW_SPINLOCK(m3000_buffer_lock);
> > +static struct timer_list m3000_poll_timer;
> > +static bool m3000_poll_active;
>
> Shouldn't these be in a structure attached to the console somewhere?
> Otherwise you have limited yourself to just one of these.
>
I can group the state into a structure. This backend uses the single
/chosen stdin/stdout pair, and the buffer must remain in the locked
kernel image mappings for firmware access. I will also review how the
state is associated with HVC before finalizing a v2
> > +
> > +/*
> > + * 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)
> > +{
> > + if (!READ_ONCE(m3000_poll_active))
> > + return;
>
> Why are you accessing m3000_poll_active like this in an attempt to not
> use a real lock? Are you _sure_ a bool will work this way properly?
>
Agreed that READ_ONCE/WRITE_ONCE alone do not explain the timer lifecycle.
I will review synchronization between open, close and timer rearming,
and make that protocol explicit in v2.
> > + hvc_kick();
> > + mod_timer(&m3000_poll_timer, jiffies + 1);
> > +}
> > +
> > +static int m3000_open(struct hvc_struct *hp, int data)
> > +{
> > + WRITE_ONCE(m3000_poll_active, true);
> > + mod_timer(&m3000_poll_timer, jiffies + 1);
> > + return 0;
> > +}
> > +
> > +static void m3000_close(struct hvc_struct *hp, int data)
> > +{
> > + WRITE_ONCE(m3000_poll_active, false);
> > + timer_delete_sync(&m3000_poll_timer);
> > +}
> > +
> > +static ssize_t m3000_io(u8 *in, const u8 *out, size_t count)
> > +{
> > + unsigned long args[7], flags;
> > + bool input = in != NULL;
> > + int ret;
> > +
> > + if (!count)
> > + return 0;
> > + count = min_t(size_t, count, input ? 1 : sizeof(m3000_buffer));
>
> No need for min_t(), why not just min()?
>
Agreed, will change it to min().
> > + /* Serialize the bounce buffer; p1275 separately serializes firmware. */
> > + raw_spin_lock_irqsave(&m3000_buffer_lock, flags);
>
> why is this a "raw" spinlock?
>
The intent is to protect the shared firmware buffer in console output
contexts where sleeping is not allowed. I will check the precise HVC
calling contexts and lock ordering before deciding whether the raw lock
is necessary and documenting that choice.
> > + if (!input)
> > + memcpy(m3000_buffer, out, count);
> > + args[0] = (unsigned long)(input ? "read" : "write");
>
> A string being cast to an unsigned long? Are you _sure_?
>
Yes: p1275_cmd_direct() takes unsigned long firmware argument cells,
and the first cell carries the service-name pointer. The existing
SPARC prom_console_write_buf() uses the same cast for "write".
> > + if (!m3000_ready)
> > + return -ENODEV;
> > + timer_setup(&m3000_poll_timer, m3000_poll_tick, 0);
> > + hp = hvc_alloc(0, 0, &m3000_ops, sizeof(m3000_buffer));
> > + if (IS_ERR(hp))
> > + return PTR_ERR(hp);
> > + hp->ws.ws_row = 24;
> > + hp->ws.ws_col = 80;
> > + pr_info("M3000: firmware-backed hvc0 tty ready (serialized, polled input)\n");
>
> When drivers work properly, they are quiet.
>
Agreed; I have removed the initialization message.
Thanks again for the review. I'll prepare a v2 addressing your
comments and explain the reasoning behind any design choices
that remain.
Regards
Magnus
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-10-04 15:24 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20261002161515.932316-1-linmag7@gmail.com>
2026-10-02 16:14 ` [PATCH 7/7] hvc: add an M3000 firmware console backend Magnus Lindholm
2026-10-04 12:50 ` Greg Kroah-Hartman
2026-10-04 15:24 ` Magnus Lindholm
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox