2007/9/26, Jan Kiszka <[EMAIL PROTECTED]>:
>
> Guillaume Gaudonville wrote:
> > I've attached the patch drafts. It works fine for me. it prepare the
> work to
> > implement the other iflag values as describe in man tcsetattr.
> >
>
> ...
>
> >
> > Index: ksrc/drivers/serial/16550A.c
> > ===================================================================
> > --- ksrc/drivers/serial/16550A.c      (révision 2765)
> > +++ ksrc/drivers/serial/16550A.c      (copie de travail)
> > @@ -153,7 +153,10 @@ static inline int rt_16550_rx_interrupt(
> >       do {
> >               c = rt_16550_reg_in(mode, base, RHR);   /* read input char
> */
> >
> > -             ctx->in_buf[ctx->in_tail] = c;
> > +             if (testbits(ctx->config.iflags, RTSER_IFLAG_ISTRIP))
> > +                     ctx->in_buf[ctx->in_tail] = c & 0x7f;
> > +             else
> > +                     ctx->in_buf[ctx->in_tail] = c;
>
> OK, things become clearer.
>
> Question: Switching parity checking on in the hardware does not work/is
> no option for you? Shouldn't that clear the parity information as well?

It's not an option for me. I'm not sure if that clear the parity information
but I don't want to have parity error checking. I think this option exist
to permit to manage a remote hardware which communicates over
the serial line with parity enabled when we don't mind of the parity
information and don't want to see parity error.


>               if (ctx->in_history)
> >                       ctx->in_history[ctx->in_tail] = *timestamp;
> >               ctx->in_tail = (ctx->in_tail + 1) & (IN_BUFFER_SIZE - 1);
> > @@ -426,6 +429,10 @@ static int rt_16550_set_config(struct rt
> >               rtdm_lock_put_irqrestore(&ctx->lock, lock_ctx);
> >       }
> >
> > +     if (testbits(config->config_mask, RTSER_SET_IFLAGS)) {
> > +             ctx->config.iflags = config->iflags;
> > +     }
> > +
> >       return err;
> >  }
> >
> > Index: include/rtdm/rtserial.h
> > ===================================================================
> > --- include/rtdm/rtserial.h   (révision 2765)
> > +++ include/rtdm/rtserial.h   (copie de travail)
> > @@ -184,6 +184,7 @@
> >  #define RTSER_SET_TIMEOUT_EVENT              0x0400
> >  #define RTSER_SET_TIMESTAMP_HISTORY  0x0800
> >  #define RTSER_SET_EVENT_MASK         0x1000
> > +#define RTSER_SET_IFLAGS             0x2000
> >  /** @} */
> >
> >
> > @@ -238,6 +239,15 @@
> >  #define RTSER_BREAK_SET                      0x01
> >
> >
> > +/*!
> > + * @anchor RTSER_IFLAG_xxx   @name RTSER_IFLAG_xxx
> > + * Input behaviour (when used every bits will be set by the ioctl
> command)
> > + * @{ */
> > +#define RTSER_IFLAG_NONE             0x00
> > +#define RTSER_IFLAG_ISTRIP           0x01
> > +#define RTSER_IFLAG_DEFAULT          0x00
> > +
> > +
> >  /**
> >   * Serial device configuration
> >   */
> > @@ -280,6 +290,11 @@ typedef struct rtser_config {
> >       /** event mask to be used with @ref RTSER_RTIOC_WAIT_EVENT, see
> >        *  @ref RTSER_EVENT_xxx */
> >       int             event_mask;
> > +
> > +     /** input flags, see @ref RTSER_IFLAG_xxx, RTSER_IFLAG_ISTRIP is
> > +      *  the only one supported for the moment (when used every bits
> > +      *  must be set according to the desired configuration)*/
> > +     int             iflags;
> >  } rtser_config_t;
> >
> >  /**
>
> The patch looks very clean, no question! Assuming my attempt above to
> use the hardware for this is nonsense, I then wonder if the feature is
> worth the additional conditional branch + another potential cache miss
> (for ctx->config.iflags) inside the critical reception path.


This is precisely that question of balance: Is the extension of some
> broader interest? Do we need it inside the driver (I see no reason yet
> why the application cannot do the masking, and we are not forced into
> compatibility with termios)? And does the extension impact critical paths?


I agree with you if the goal was just to manage the ISTRIP option. But we
could
then add some of the other input flag :

       * IGNBRK Ignore BREAK condition on input.
       * BRKINT If  IGNBRK  is set, a BREAK is ignored. If it is not set but
BRKINT is set, then a BREAK causes the input and output queues to be
flushed, and if the terminal is the controlling terminal of a foreground
process group, it will cause a SIGINT to be sent to this foreground process
group.  When neither IGNBRK nor BRKINT are set, a BREAK reads as a null byte
('\0'), except when  PARMRK is set, in which case it reads as the sequence
\377 \0 \0.
       * IGNPAR Ignore framing errors and parity errors.
       * PARMRK If  IGNPAR is not set, prefix a character with a parity
error or framing error with \377 \0.  If neither IGNPAR nor PARMRK is set,
read a character with a parity error or framing error as \0.
       * INPCK  Enable input parity checking.
       * ISTRIP Strip off eighth bit.
       * INLCR  Translate NL to CR on input.
       * IGNCR  Ignore carriage return on input.
       * ICRNL  Translate carriage return to newline on input (unless IGNCR
is set).

Maybe I could implement some of them. Number depending upon the time I have.

This flag could be very useful to Xenomai users. I know some of them are
implemented
both in termios and in the tyLib of VxWorks. So, I see two use case:

           * peoples who will have to port a Linux application or a VxWorks
application
              under Xenomai: those features would permit them to limits the
modifications
              in the original code (which I think is a big constraint of
this type of work).
           * peoples who will have to code an application which need those
features. This features
              seems to be implemented by a lot of OSes, I don't think this
is done for nothing. This
              limits the development work in User Space while not adding a
big coomplexity in the driver.

Just a precision: Under Linux this is not done in the low-level driver but
in the serial line discipline code. But
your driver manage already the communication with User Space which is more
than what Linux serial
drivers do.


Jan
>
> --
> Siemens AG, Corporate Technology, CT SE 2
> Corporate Competence Center Embedded Linux
>



-- 
Guillaume Gaudonville
E-mail: [EMAIL PROTECTED]
_______________________________________________
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core

Reply via email to