Re: Handling RTS with an UART that doesn't directly drives the RTS pin

["Bernard Fouché" <bernard.fouche@kuantic.com>]

Le 13/06/2012 12:10, Nick Garnett a écrit :
>
> But there is still a way to do something similar, I think. In the
> board-specific serial configuration package instead of passing
> pc_serial_funs to the SERIAL_CHANNEL() macro, pass your own serial
> functions structure. Most of the entries can just call straight through
> pc_serial_funs to the 16x5x functions, but the set_config entry can be
> your function which handles GPIO flow control and calls
> pc_serial_funs.set_config() for the rest.
Beside having indirect calls for all instances of 16x5x, even for UART 
not concerned by flow control or providing it already, I think it will 
be difficult to avoid modifications to many config files and it will end 
in a 16x5x board-specific version, in that case it's much simpler to 
copy 16x5x and hack it directly. But users not concerned by 16x5x will 
get no benefit from this while missing hw management of RTS/CTS pins is 
a very common problem.
>
> This way all the generic code stays unchanged, and the board-specific
> parts are isolated in a board-specific package.
>
>> Another solution would be to add config keys to get/set the pointers of
>> the low level functions of a serial channel which are exposed by a
>> hardware driver and then one could insert any kind of middle level
>> driver (like hooks): this would be more comfortable in the long run, it
>> could help for debug or statistics. The xxx_serial_channelN definition
>> ends in .data so I guess this is possible.
> I think if you do what I suggest above you wouldn't need to do this.
> Also the serial function tables ought to be declared const and thus be
> in read-only memory. At present they are not, but in the future this
> might change. These tables should be treated as read-only.
>
>
Well this is the approach I took ( I also made the initial version that 
uses indicate_status() but this later system seems more general and can 
be extended), I have it working now:

-----
serialio.h:

#ifdef CYGOPT_IO_SERIAL_SUPPORT_HOOKS

typedef void (*any_low_level_func_t)(void);

typedef struct {
     any_low_level_func_t fp;
     cyg_int32            func_id;
} cyg_serial_hook_t;


// arguments for CYG_IO_GET_CONFIG_SERIAL_FUNCTION and 
CYG_IO_SET_CONFIG_SERIAL_FUNCTION
#define CYGNUM_SERIAL_FUNCTION_PUTC          0
#define CYGNUM_SERIAL_FUNCTION_GETC          1
#define CYGNUM_SERIAL_FUNCTION_SET_CONFIG    2
#define CYGNUM_SERIAL_FUNCTION_START_XMIT    3
#define CYGNUM_SERIAL_FUNCTION_STOP_XMIT     4
-----

serial.c, excerpt for CYG_IO_SET_CONFIG_SERIAL_FUNCTION for putc:

#ifdef CYGOPT_IO_SERIAL_SUPPORT_HOOKS
     case CYG_IO_SET_CONFIG_SERIAL_FUNCTION:
        {
            any_low_level_func_t old;
            cyg_serial_hook_t *h=(cyg_serial_hook_t*)xbuf;

            if (*len < sizeof(*h)) {
              return -EINVAL;
            }

            // prevent calls while we do this
            cyg_drv_dsr_lock();

            switch(h->func_id){
                case CYGNUM_SERIAL_FUNCTION_PUTC:
                    old=(any_low_level_func_t)funs->putc;
                    funs->putc=(typeof(funs->putc))(h->fp);
                    break;
            .....


User code, showing only installing a hook for putc, but 
getc/set_config/{start|stop}_xmit are also available, as _get_config() 
can return the current pointer of one of these functions:

void uart_hook_install(cyg_io_handle_t hnd)
{
     cyg_uint32 l;
     cyg_serial_hook_t h;

     // install hook for putc
     h.func_id=CYGNUM_SERIAL_FUNCTION_PUTC;
     h.fp=(any_low_level_func_t)&uart_putc; // <- user provided putc
     l=sizeof(h);
     
if(ENOERR!=cyg_io_set_config(hnd,CYG_IO_SET_CONFIG_SERIAL_FUNCTION,&h,&l))
       fprintf(OutPc,"Can't set putc hook.\n");
     else{
       fprintf(OutPc,"Old ptr for putc: %p\r\n",h.fp);
       uart_putc_org=(typeof(uart_putc_org))(h.fp); // <- get 'real' putc
     }
     ...
}

// user provided function
bool uart_putc(serial_channel *priv, unsigned char c)
{
   uart_putc_cpt++; // makes our own stats

   return uart_putc_org(priv,c);
}
-----

The advantages seem to me:

- can work with any serial driver, regardless of the underlying hardware.
- hooks installed only for the needed function and the concerned 
UART/driver.
- hooks can be stacked if ever this is of some use.
- allow installing/removing hooks on the fly (debugging purposes, 
statistics (real available outgoing bandwidth, number of RTS event), 
cloning outgoing traffic, etc.
- impacts a single CDL file by providing an option in serial/cdl 
(CYGOPT_IO_SERIAL_SUPPORT_HOOKS)

Drawbacks:

- not a complete system, it should be possible to use a similar system 
between serial.c and the hardware driver mostly for incoming traffic 
(chan->callbacks->*), I did not look at it yet (since I don't need it at 
the moment)
- need serial function tables in RAM. If ever they are defined at a 
later time as 'const', can it be an option :-) ?

I'll make this available as a bugzilla bug/patch if ever someone else 
wants it...

   Bernard