lkml.org 
[lkml]   [2010]   [Feb]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 6/8] pps: serial clients support.
    On Tue, 16 Feb 2010 10:51:23 +0100
    Rodolfo Giometti <giometti@linux.it> wrote:

    > Adds support, by using the PPS line discipline, for the PPS sources
    > connected with the CD (Carrier Detect) pin of a serial port.
    >
    > ...
    >
    > +static int (*n_tty_open)(struct tty_struct *tty);
    > +
    > +static int pps_tty_open(struct tty_struct *tty)
    > +{
    > + struct pps_source_info info;
    > + struct tty_driver *drv = tty->driver;
    > + int index = tty->index + drv->name_base;
    > + int ret;
    > +
    > + info.owner = THIS_MODULE;
    > + info.dev = NULL;
    > + snprintf(info.name, PPS_MAX_NAME_LEN, "%s%d", drv->driver_name, index);
    > + snprintf(info.path, PPS_MAX_NAME_LEN, "/dev/%s%d", drv->name, index);
    > + info.mode = PPS_CAPTUREBOTH | \
    > + PPS_OFFSETASSERT | PPS_OFFSETCLEAR | \
    > + PPS_CANWAIT | PPS_TSFMT_TSPEC;
    > +
    > + ret = pps_register_source(&info, PPS_CAPTUREBOTH | \
    > + PPS_OFFSETASSERT | PPS_OFFSETCLEAR);
    > + if (ret < 0) {
    > + pr_err("cannot register PPS source \"%s\"\n", info.path);
    > + return ret;
    > + }
    > + tty->disc_data = (void *) ret;
    > +
    > + /* Should open N_TTY ldisc too */
    > + ret = n_tty_open(tty);
    > + if (ret < 0)
    > + pps_unregister_source((int) tty->disc_data);
    > +
    > + pr_info("PPS source #%d \"%s\" added\n", ret, info.path);
    > +
    > + return 0;
    > +}

    hm. The n_tty_open/close layering is a bit grubby but I guess we don't
    have a need to create anything more comprehensive at this stage.

    A small stylistic thing: the code's a bit confusing at present because
    the names n_tty_open/n_tty_close are identical to the global functions
    over in n_tty.c. A user of ctags will be tricked. Also, the _way_ in
    which these function pointers are called:

    ret = n_tty_open(tty);

    make it look like the code is calling
    drivers/char/n_tty.c:n_tty_open(). Except it isn't.

    So I think it's better to do it with

    ret = (*n_tty_open)(tty);

    to avoid misleading the reader.

    And perhaps to rename these variables to avoid the duplication with the
    global functions.




    \
     
     \ /
      Last update: 2010-02-19 22:57    [W:4.422 / U:0.056 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site