lkml.org 
[lkml]   [2008]   [Feb]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH2] Basic braille screen reader support
    On Fri, 22 Feb 2008 01:28:22 +0100 Samuel Thibault <samuel.thibault@ens-lyon.org> wrote:

    > Samuel Thibault, le Wed 06 Feb 2008 02:04:23 +0000, a __crit :
    > > Andrew Morton, le Tue 05 Feb 2008 16:58:53 -0800, a __crit :
    > > > On Tue, 5 Feb 2008 22:00:54 +0000
    > > > Samuel Thibault <samuel.thibault@ens-lyon.org> wrote:
    > > > > + serial8250_console.write(braille_co, data, c - data);
    > > >
    > > > hm. Is it appropriate that this driver wire itself directly into
    > > > serial8250?
    > > > What if the screen reader is attached to some other sort of
    > > > uart, or a terminal server, or...
    > >
    > > Indeed that's an issue. For now, there is no clean way to attach to the
    > > early serial drivers, that's why I chose 8250,
    >
    > In the patch below, I hook into kernel/printk.c's console= parser, which
    > now gives me attachment to any kind of console.
    >
    >
    >
    >
    > This adds a minimalistic braille screen reader support.
    > This is meant to be used by blind people e.g. on boot failures or when /
    > cannot be mounted etc and thus the userland screen readers can not work.
    >

    Sorry, but I can't say that this is my favoritest ever patch. But I can't
    immediately think of any way of significantly improving it :( Jiri and
    Dmitry appear to be hiding.

    > +++ linux-2.6.24.1-perso/kernel/printk.c 2008-02-21 12:09:06.000000000 +0100
    > @@ -105,6 +105,7 @@

    Please use `diff -p'

    > char name[8]; /* Name of the driver */
    > int index; /* Minor dev. to use */
    > char *options; /* Options for the driver */
    > + char *brl_options; /* Options for braille driver */
    > };

    Should this depend on CONFIG_A11Y_BRAILLE_CONSOLE or whatever?

    > #define MAX_CMDLINECONSOLES 8
    > @@ -766,15 +767,59 @@
    >
    > #endif
    >
    > +static int __add_preferred_console(char *name, int idx, char *options,
    > + char *brl_options)
    > +{
    > + struct console_cmdline *c;
    > + int i;
    > +
    > + /*
    > + * See if this tty is not yet registered, and
    > + * if we have a slot free.
    > + */
    > + for (i = 0; i < MAX_CMDLINECONSOLES && console_cmdline[i].name[0]; i++)
    > + if (strcmp(console_cmdline[i].name, name) == 0 &&
    > + console_cmdline[i].index == idx) {
    > + if (!brl_options)
    > + selected_console = i;
    > + return 0;
    > + }
    > + if (i == MAX_CMDLINECONSOLES)
    > + return -E2BIG;

    Does the array of consles have any locking here?

    > + if (!brl_options)
    > + selected_console = i;
    > + c = &console_cmdline[i];
    > + memcpy(c->name, name, sizeof(c->name));
    > + c->name[sizeof(c->name) - 1] = 0;

    strlcpy()?

    > + c->options = options;
    > + c->brl_options = brl_options;
    > + c->index = idx;
    > + return 0;
    > +}
    > /*
    > * Set up a list of consoles. Called from init/main.c
    > */
    > static int __init console_setup(char *str)
    > {
    > char buf[sizeof(console_cmdline[0].name) + 4]; /* 4 for index */
    > - char *s, *options;
    > + char *s, *options, *brl_options = NULL;
    > int idx;
    >
    > +#ifdef CONFIG_A11Y_BRAILLE_CONSOLE
    > + if (!memcmp(str, "brl,", 4)) {
    > + brl_options = "";
    > + str += 4;
    > + } else if (!memcmp(str, "brl=", 4)) {
    > + brl_options = str + 4;
    > + str = strchr(brl_options, ',');
    > + if (!str) {
    > + printk(KERN_ERR "need port name after brl=\n");
    > + return 1;
    > + }
    > + *(str++) = 0;
    > + }
    > +#endif

    Update Documentation/kernel-parameters.txt, please. And additional
    documentation would be nice, if justified?

    > /*
    > * Decode str into name, index, options.
    > */
    > @@ -799,7 +844,7 @@
    > idx = simple_strtoul(s, NULL, 10);
    > *s = 0;
    >
    > - add_preferred_console(buf, idx, options);
    > + __add_preferred_console(buf, idx, options, brl_options);
    > return 1;
    > }
    >
    > ...
    >
    > +static void beep(unsigned int freq)
    > +{
    > + if (sound)
    > + kd_mksound(freq, HZ/10);
    > +}

    hm, do we have enough Kconfig dependencies here to ensure that kd_mksound()
    is always available?




    \
     
     \ /
      Last update: 2008-02-23 09:19    [W:6.400 / U:0.384 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site