Re: [Xenomai-core] new features for 16550A driver
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.
* IC
Re: [Xenomai-core] new features for 16550A driver
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?
> 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?
Jan
--
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux
___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] new features for 16550A driver
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.
2007/9/26, Guillaume Gaudonville <[EMAIL PROTECTED]>:
>
>
>
> 2007/9/26, Jan Kiszka <[EMAIL PROTECTED]>:
> >
> > Guillaume Gaudonville wrote:
> > > Hello,
> > >
> > > I'm working on a project which aims at porting an application
> > > running under VxWorks to Linux. This application uses the serial
> > > port.
> > >
> > > I would like to add an ioctl or a field in the config structure
> > > to permit to strip or not the parity bit of each byte on reception.
> > This
> > > feature is implemented on Linux and VxWorks.
> > >
> > > Is it something I can do and then submit to Xenomai or your driver is
> > only
> > > an example and will never be improved?
> >
> > The driver is far from being just an example, it's used in real
> > applications.
>
>
> It was just to be sure... I'm using it for a real application.
>
> If your feature is useful and doesn't turn the existing code upside
> > down, it will surely be considered for merging.
> >
> > >
> > > I think that I will have more feature to add to the driver, that's why
> > I'm
> > > asking this.
> >
> > I don't want to exclude anything (specifically as long as I haven't seen
> > some patch yet), but I also don't want to overload the driver over the
> > time with corner-case features. We need a good balance.
> >
> > But we first of all need more details about what you plan to add, what
> > application scenario is behind it, why you need the driver to implement
> > the feature, and what impact it may have on the code. You are welcome to
> >
> > elaborate on this or, even better, post some patch drafts here.
>
>
> I don't know which features I'll need. For the ISTRIP feature the goal
> is only to
> remove the eigth'th bit of every byte received: I've got a VxWorks
> application which communicate
> with an external clock on the serial port. The clock uses the parity bit
> but my application
> do not. In the original code an ioctl was done to say the serial driver to
> remove the parity bit on
> reception. This feature is also offered under Linux and seems to be posix
> (man tcsetattr, search ISTRIP).
> In order to reduce the code modifications, I would like to implement it in
> the driver.
>
> If so, I think the better way would be to add a field in the rtser_config
> struct, do you agree? And then depending
> upon this field to apply a mask (0x7f) to the character in the interrupt
> handler rt_16550_rx_interrupt() or only
> to the characters passed to user space depending upon what is done in
> other driver.
> to reproduce the same behaviour).
>
> Jan
> >
> > --
> > Siemens AG, Corporate Technology, CT SE 2
> > Corporate Competence Center Embedded Linux
> >
>
>
>
> --
> Guillaume Gaudonville
> E-mail: [EMAIL PROTECTED]
>
--
Guillaume Gaudonville
E-mail: [EMAIL PROTECTED]
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;
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;
/**
___
Xenomai-core mailin
Re: [Xenomai-core] new features for 16550A driver
2007/9/26, Jan Kiszka <[EMAIL PROTECTED]>: > > Guillaume Gaudonville wrote: > > Hello, > > > > I'm working on a project which aims at porting an application > > running under VxWorks to Linux. This application uses the serial > > port. > > > > I would like to add an ioctl or a field in the config structure > > to permit to strip or not the parity bit of each byte on reception. This > > feature is implemented on Linux and VxWorks. > > > > Is it something I can do and then submit to Xenomai or your driver is > only > > an example and will never be improved? > > The driver is far from being just an example, it's used in real > applications. It was just to be sure... I'm using it for a real application. If your feature is useful and doesn't turn the existing code upside > down, it will surely be considered for merging. > > > > > I think that I will have more feature to add to the driver, that's why > I'm > > asking this. > > I don't want to exclude anything (specifically as long as I haven't seen > some patch yet), but I also don't want to overload the driver over the > time with corner-case features. We need a good balance. > > But we first of all need more details about what you plan to add, what > application scenario is behind it, why you need the driver to implement > the feature, and what impact it may have on the code. You are welcome to > elaborate on this or, even better, post some patch drafts here. I don't know which features I'll need. For the ISTRIP feature the goal is only to remove the eigth'th bit of every byte received: I've got a VxWorks application which communicate with an external clock on the serial port. The clock uses the parity bit but my application do not. In the original code an ioctl was done to say the serial driver to remove the parity bit on reception. This feature is also offered under Linux and seems to be posix (man tcsetattr, search ISTRIP). In order to reduce the code modifications, I would like to implement it in the driver. If so, I think the better way would be to add a field in the rtser_config struct, do you agree? And then depending upon this field to apply a mask (0x7f) to the character in the interrupt handler rt_16550_rx_interrupt() or only to the characters passed to user space depending upon what is done in other driver. to reproduce the same behaviour). 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
Re: [Xenomai-core] new features for 16550A driver
Guillaume Gaudonville wrote: > Hello, > > I'm working on a project which aims at porting an application > running under VxWorks to Linux. This application uses the serial > port. > > I would like to add an ioctl or a field in the config structure > to permit to strip or not the parity bit of each byte on reception. This > feature is implemented on Linux and VxWorks. > > Is it something I can do and then submit to Xenomai or your driver is only > an example and will never be improved? The driver is far from being just an example, it's used in real applications. If your feature is useful and doesn't turn the existing code upside down, it will surely be considered for merging. > > I think that I will have more feature to add to the driver, that's why I'm > asking this. I don't want to exclude anything (specifically as long as I haven't seen some patch yet), but I also don't want to overload the driver over the time with corner-case features. We need a good balance. But we first of all need more details about what you plan to add, what application scenario is behind it, why you need the driver to implement the feature, and what impact it may have on the code. You are welcome to elaborate on this or, even better, post some patch drafts here. Jan -- Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
[Xenomai-core] new features for 16550A driver
Hello, I'm working on a project which aims at porting an application running under VxWorks to Linux. This application uses the serial port. I would like to add an ioctl or a field in the config structure to permit to strip or not the parity bit of each byte on reception. This feature is implemented on Linux and VxWorks. Is it something I can do and then submit to Xenomai or your driver is only an example and will never be improved? I think that I will have more feature to add to the driver, that's why I'm asking this. -- Guillaume Gaudonville E-mail: [EMAIL PROTECTED] ___ Xenomai-core mailing list Xenomai-core@gna.org https://mail.gna.org/listinfo/xenomai-core
