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

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

[Bernard Fouché]

Hi,

I'm adding support for RTS/CTS handling for a couple of UART which have 
no hw support for these pins.

Only one UART has real hw flow control handled in silicon in my target 
and I already use this particular UART for another need.

To react to a CTS state change I detect in my GPIO interrupt DSR, I can 
send CYG_IO_SET_CONFIG_SERIAL_FLOW_CONTROL_FORCE using 
CYGNUM_SERIAL_FLOW_{THROTTLE|RESTART}_TX to serial.c . However I don't 
see any way to have serial.c to drive RTS: throttle_rx() and 
restart_rx()  call the underlying hardware driver (generic 16x5xon my 
target) but of course the driver can't do anything interesting in this case.

I think the change is to be done in serial.c, since it's the part of the 
code that takes the decision to call throttle_rx() or restart_rx() and 
there is no interest in having the underlying driver to be made aware of 
pins external to what it can handle itself (particularly for the generic 
serial driver). Modifying serial.c would also allow any existing 
hardware driver to use this feature since the situation I encounter 
occurs with many MCU having many UART but only one doing real hw flow 
control.

I'm about to :

- add definitions for CYGNUM_SERIAL_STATUS_THROTTLE_RX and 
CYGNUM_SERIAL_STATUS_RESTART_RX
- add a cdl option to have the line status callback function, which is 
user defined, to be called with the corresponding value if 
throttle_rx()/restart_rx() are called within serial.c .
- hence the user defined callback can handle RTS (or any other flow 
control pin) the way it wants

Do I break some convention doing this or is it okay?

   Bernard

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

[Stanislav Meduna]

On 12.06.2012 14:34, Bernard Fouché wrote:

> I think the change is to be done in serial.c, since it's the part of the
> code that takes the decision to call throttle_rx() or restart_rx()

Is this the correct place at all? As far as I can tell the
flow-control here depends on channel buffer watermarks. It does
not protect you from overflowing the 16x5x FIFO; it does not
even know how deep it is.

I do not know what UART speeds, UART FIFO sizes and maximum
guaranteed DSR latency do you have in your system, but in case
you want to protect against UART FIFO overflow I'm afraid
the only reliable way is to modify the low level driver's ISR.

Regards
-- 
                                      Stano

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

[Nick Garnett]

On 12/06/12 13:34, Bernard Fouché wrote:

> I'm about to :
> 
> - add definitions for CYGNUM_SERIAL_STATUS_THROTTLE_RX and 
> CYGNUM_SERIAL_STATUS_RESTART_RX
> - add a cdl option to have the line status callback function, which is 
> user defined, to be called with the corresponding value if 
> throttle_rx()/restart_rx() are called within serial.c .
> - hence the user defined callback can handle RTS (or any other flow 
> control pin) the way it wants
> 
> Do I break some convention doing this or is it okay?


I don't think hacking this into the generic code is the right way to do
this. In the past, when I have had to do the same and use GPIO pins for
this, I have added it to the underlying serial driver. Of course this
was for devices that had no other notion of hardware flow control. But
you could define your own variant of the 16x5x driver that did the right
thing.

However, a better approach is to avoid hacking on either the generic
code or the 16x5x driver and make use of the driver stacking mechanism.
Create a serial driver that passes all calls through to the underlying
driver, except for the throttle and flow config options, which do the
right things with the GPIO lines. Take a look at the TTY and TERMIO
drivers for how to set this up.

Note that while RTS is a simple output level, CTS really needs to be
driven by a GPIO line that can interrupt and which will then call the
indicate_status() callback in its DSR. That should then drive the
generic serial transmit engine.


-- 
Nick Garnett                                      eCos Kernel Architect
eCosCentric Limited    http://www.eCosCentric.com      The eCos experts
Barnwell House, Barnwell Drive, Cambridge, UK.     Tel: +44 1223 245571
Registered in England and Wales:                        Reg No: 4422071

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

[Bernard Fouché]

Le 12/06/2012 15:15, Nick Garnett a écrit :
> On 12/06/12 13:34, Bernard Fouché wrote:
>
>> I'm about to :
>>
>> - add definitions for CYGNUM_SERIAL_STATUS_THROTTLE_RX and
>> CYGNUM_SERIAL_STATUS_RESTART_RX
>> - add a cdl option to have the line status callback function, which is
>> user defined, to be called with the corresponding value if
>> throttle_rx()/restart_rx() are called within serial.c .
>> - hence the user defined callback can handle RTS (or any other flow
>> control pin) the way it wants
>>
>> Do I break some convention doing this or is it okay?
> I don't think hacking this into the generic code is the right way to do
> this. In the past, when I have had to do the same and use GPIO pins for
> this, I have added it to the underlying serial driver. Of course this
> was for devices that had no other notion of hardware flow control. But
> you could define your own variant of the 16x5x driver that did the right
> thing.
Modifying the 16x5x driver would mean to add a cdl option to specify a 
function name (what function to call to change RTS) since there is no 
GPIO infrastructure in the 'public' eCos version. For the same reason, 
sending to the driver the information related to CTS pin state has to be 
done from application level (but the infrastructure already exists 
thanks to CYG_IO_SET_CONFIG_SERIAL_FLOW_CONTROL_FORCE, which is used by 
TERMIO btw). But if such a change is made into ser_16x5x.c, anyone 
having the same need with a different hardware driver will have to 
re-invent the wheel.

> However, a better approach is to avoid hacking on either the generic
> code or the 16x5x driver and make use of the driver stacking mechanism.
> Create a serial driver that passes all calls through to the underlying
> driver, except for the throttle and flow config options, which do the
> right things with the GPIO lines. Take a look at the TTY and TERMIO
> drivers for how to set this up.

RTS is the real problem: only serial.c knows when it's time to change 
the pin state because it's related to the use of the RX buffer, which is 
visible only to serial.c (please correct me if I'm wrong!). AFAIK there 
is nothing making the RX throttling information to reach higher levels 
since these higher levels are unaware of the buffering details underneath.

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.
> Note that while RTS is a simple output level, CTS really needs to be
> driven by a GPIO line that can interrupt and which will then call the
> indicate_status() callback in its DSR. That should then drive the
> generic serial transmit engine.
This I already have.

The line_status callback change in serial.c is very simple: only a few 
lines of conditional compilation in throttle_rx() and restart_rx() are 
needed. Adding config keys to get/set the function pointers should be 
not too difficult also.

Le 12/06/2012 15:12, Stanislav Meduna a écrit :
> On 12.06.2012 14:34, Bernard Fouché wrote:
>
>> I think the change is to be done in serial.c, since it's the part of the
>> code that takes the decision to call throttle_rx() or restart_rx()
> Is this the correct place at all? As far as I can tell the
> flow-control here depends on channel buffer watermarks. It does
> not protect you from overflowing the 16x5x FIFO; it does not
> even know how deep it is.
>
> I do not know what UART speeds, UART FIFO sizes and maximum
> guaranteed DSR latency do you have in your system, but in case
> you want to protect against UART FIFO overflow I'm afraid
> the only reliable way is to modify the low level driver's ISR.
I don't have problems with the 16x5x hardware FIFO overflow condition: 
as you write this depends on the whole system characteristics. I 
consider my DSR latency  good enoug to empty the hardware FIFO in time 
(thanks to the patches of bug 1001456 the system does not lose time ;-) 
), and if not, I have to lower the value of 
CYGPKG_IO_SERIAL_GENERIC_16X5X_FIFO_RX_THRESHOLD. With a 16 bytes FIFO 
and a threshold of 8, I have more than 8 characters to have the DSR to 
empty the FIFO. At 115200bps (I don't go faster), that's 694µs: even 
when clocking very slowly there is time for many thousands of instructions.

My problem has never been (yet) the hardware FIFO but serial flow 
control since the consuming threads are less reactive than DSR, and I 
don't know what will do the device connected to the RS232 port.

     Bernard

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

[Nick Garnett]

On 12/06/12 17:55, Bernard Fouché wrote:
>
> 
> RTS is the real problem: only serial.c knows when it's time to change 
> the pin state because it's related to the use of the RX buffer, which is 
> visible only to serial.c (please correct me if I'm wrong!). AFAIK there 
> is nothing making the RX throttling information to reach higher levels 
> since these higher levels are unaware of the buffering details underneath.

You are right. I was thinking, without looking too closely at the code,
that it might be possible to slip another drive in under the serial
driver. But obviously that is not possible.

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.

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.



-- 
Nick Garnett                                      eCos Kernel Architect
eCosCentric Limited    http://www.eCosCentric.com      The eCos experts
Barnwell House, Barnwell Drive, Cambridge, UK.     Tel: +44 1223 245571
Registered in England and Wales:                        Reg No: 4422071

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

[Bernard Fouché]

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

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

[Bernard Fouché]

It took me some time to figure that putc/get/set_config etc. are per 
hardware driver type, not per UART... hence only one 'hook' for any of 
these function for a given driver is available, and when the hook 
function is called it has trouble to figure what UART is concerned since 
it can't know about the 'priv' field setup by the underlying driver. I 
found a hack to know what UART is concerned but this isn't very clean... 
no good solution yet!

Re: Closing devices

[Frank Pagliughi]

Hello All,

I'm looking for ideas on how to close and re-open devices on eCos. The 
needs for this are (1) to support swappable/removable devices, (2) to 
have a consistent way to put devices into a low-power state when they 
are not being used, and (3) to prevent devices from using the CPU when 
they are not needed.

On a current project for a battery-powered device, I have a need for all 
of this: a CompactFlash that can be removed; a GPS that sends data 
constantly, but only needs to be read once every few hours; and many of 
the peripherals need to be put in a consistent state prior to going into 
a low power mode.

I've been able to accomplish all of this with ugly application-level 
code, but thought that a much better solution would be to propagate the 
close() call of a devfs device down to the driver, so you could do this:

    while (true) {
         int fd = open("/dev/ser0", O_RDONLY);
         read(fd, buf, BUF_LEN);

         // power down the port
         close(fd);

         sleep(60);
    }

I think this can be done pretty easily by adding a "shutdown" function 
to the devtab entries:

    typedef struct cyg_devtab_entry {
         // ...
         Cyg_ErrNo        (*lookup)(struct cyg_devtab_entry **tab,
                                    struct cyg_devtab_entry *sub_tab,
                                    const char *name);
         Cyg_ErrNo        (*shutdown)(struct cyg_devtab_entry *tab);
         // ...
    } CYG_HAL_TABLE_TYPE cyg_devtab_entry_t;


The existing macros CHAR_DEVTAB_ENTRY, BLOCK_DEVTAB_ENTRY, etc, can just 
insert an empty function into the shutdown slot, whereas a new set of 
macros can accept the shutdown pointer:

    #define
    CHAR_CLOSABLE_DEVTAB_ENTRY(_l,_name,_dep_name,_handlers,_init,_lookup,_shutdown,_priv)
    ...
    #define
    BLOCK_CLOSABLE_DEVTAB_ENTRY(_l,_name,_dep_name,_handlers,_init,_lookup,_shutdown,_priv)
    ...

The assumption is that anything done in the shutdown would be undone in 
a subsequent lookup so that the devices could bre closed then re-opened, 
and that the shutdown would mark the entry as "unavailable":

    tab->status &= ~CYG_DEVTAB_STATUS_AVAIL;


Assuming that this is a good way to handle this I would need to know how 
to hook this into the upper-level device and file operations. What would 
be needed?

Thanks,
Frank

Re: Closing devices

[Bernard Fouché]

Le 18/06/2012 16:34, Frank Pagliughi a écrit :
> Hello All,
>
> I'm looking for ideas on how to close and re-open devices on eCos. The 
> needs for this are (1) to support swappable/removable devices, (2) to 
> have a consistent way to put devices into a low-power state when they 
> are not being used, and (3) to prevent devices from using the CPU when 
> they are not needed.
>
> On a current project for a battery-powered device, I have a need for 
> all of this: a CompactFlash that can be removed; a GPS that sends data 
> constantly, but only needs to be read once every few hours; and many 
> of the peripherals need to be put in a consistent state prior to going 
> into a low power mode.
>
> I've been able to accomplish all of this with ugly application-level 
> code, but thought that a much better solution would be to propagate 
> the close() call of a devfs device down to the driver, so you could do 
> this:
Hello Frank,

I have the exact same needs and I also made my changes in the 
application code at the moment, for the same reasons. However if we look 
at the low level details:

-  The Init() function of a driver is called at boot time,  a time you 
don't want to initialize much things if you don't know yet if you'll 
need them a few moment later. init() isn't visible from anything else 
than the startup procedure of eCos.

- lookup() is called when the application 'opens' a channel of a driver. 
Usually nothing much is done at low level since the assumption is that 
init() made the job before. However it's possible to rework the drivers 
to change this and future drivers could be written with this in mind.

- Since a devtab entry can be looked up many times, even by different 
threads, it is probably necessary to have a driver to count the number 
of times it is looked up and the number of times it is shutdown. When 
the count reaches zero, then the driver knows it can power off things.

- drivers that are shared between different targets do not know about 
target specific features, by design they focus on the parts that are 
common to all targets they can be used on. Such a driver expects that 
the MCU pin setup (and other details) has already been done earlier in 
the board init code, it has no way to query something to run again this 
procedure. If you need to close an UART, you probably also want to 
reconfigure the MCU's pins. You may also want to power off the UART 
(from the MCU point of view) if the MCU allows it. So even if shutdown() 
is implemented, such a driver wouldn't do much regarding power savings, 
at best it could only mask or disable an interrupt, the most important 
savings must be handled elsewhere. Even if the board init code could be 
accessible, how one could ask this code to perform a partial 
initialization? (for instance to avoid reconfiguring all UARTs while a 
single one is to be re-initialized).

- power management is very MCU/board/application specific and project 
specific code will have different things to do. For instance if you have 
an external RS232 device, you save more power by turning off the level 
converter between the UART and the RS232 connector. Of course it's 
better to be able to turn off both the UART in the MCU and the level 
converter. If you don't have a level converter, you may want to 
reconfigure the MCU pins, for instance to avoid having power drawn from 
the UART TX pin if the connected device is  also powered off. You may 
also want to setup pull-up/down on the pins to stabilize the signals: it 
means changing the pin setup of the MCU to change them from 'UART' to 
'GPIO' and then configure the pull up/down feature. You  may also want 
to change peripheral clock settings for disabled peripheral in the MCU, 
to spare a bit more so you have to re configure also clocking registers 
when the peripheral must come back in line.

- There is CYGPKG_POWER. Each driver implementing some kind of power 
management can be modified to support this package. But I don't see how 
this package can interact with the platform code layer. How can a target 
using a shared driver can make use of this package for the shared driver?

IMHO, beside a shutdown mechanism, one also needs to be able to get 
control of what's going on between the hardware drivers and the packages 
that use them. A low level application initialization routine should be 
able to register callbacks to be triggered when events occur in the 
drivers and in the package code managing them, hence the application 
could handle the board or MCU specifically when some expected event 
occurs. Today only part of this could be done in platform code, but in 
such a way that it is very close to application code, however without 
any clearly defined API.

     Bernard

Re: Closing devices

[Frank Pagliughi]

On 06/19/2012 09:36 AM, Bernard Fouché wrote:
>
> Le 18/06/2012 16:34, Frank Pagliughi a écrit :
>> Hello All,
>>
>> I'm looking for ideas on how to close and re-open devices on eCos. 
>> The needs for this are (1) to support swappable/removable devices, 
>> (2) to have a consistent way to put devices into a low-power state 
>> when they are not being used, and (3) to prevent devices from using 
>> the CPU when they are not needed.
>>
>> On a current project for a battery-powered device, I have a need for 
>> all of this: a CompactFlash that can be removed; a GPS that sends 
>> data constantly, but only needs to be read once every few hours; and 
>> many of the peripherals need to be put in a consistent state prior to 
>> going into a low power mode.
>>
>> I've been able to accomplish all of this with ugly application-level 
>> code, but thought that a much better solution would be to propagate 
>> the close() call of a devfs device down to the driver, so you could 
>> do this:
> Hello Frank,
>
> I have the exact same needs and I also made my changes in the 
> application code at the moment, for the same reasons. However if we 
> look at the low level details:
>
> -  The Init() function of a driver is called at boot time,  a time you 
> don't want to initialize much things if you don't know yet if you'll 
> need them a few moment later. init() isn't visible from anything else 
> than the startup procedure of eCos.
>
> - lookup() is called when the application 'opens' a channel of a 
> driver. Usually nothing much is done at low level since the assumption 
> is that init() made the job before. However it's possible to rework 
> the drivers to change this and future drivers could be written with 
> this in mind.
>
> - Since a devtab entry can be looked up many times, even by different 
> threads, it is probably necessary to have a driver to count the number 
> of times it is looked up and the number of times it is shutdown. When 
> the count reaches zero, then the driver knows it can power off things.
>
> - drivers that are shared between different targets do not know about 
> target specific features, by design they focus on the parts that are 
> common to all targets they can be used on. Such a driver expects that 
> the MCU pin setup (and other details) has already been done earlier in 
> the board init code, it has no way to query something to run again 
> this procedure. If you need to close an UART, you probably also want 
> to reconfigure the MCU's pins. You may also want to power off the UART 
> (from the MCU point of view) if the MCU allows it. So even if 
> shutdown() is implemented, such a driver wouldn't do much regarding 
> power savings, at best it could only mask or disable an interrupt, the 
> most important savings must be handled elsewhere. Even if the board 
> init code could be accessible, how one could ask this code to perform 
> a partial initialization? (for instance to avoid reconfiguring all 
> UARTs while a single one is to be re-initialized).
>
> - power management is very MCU/board/application specific and project 
> specific code will have different things to do. For instance if you 
> have an external RS232 device, you save more power by turning off the 
> level converter between the UART and the RS232 connector. Of course 
> it's better to be able to turn off both the UART in the MCU and the 
> level converter. If you don't have a level converter, you may want to 
> reconfigure the MCU pins, for instance to avoid having power drawn 
> from the UART TX pin if the connected device is  also powered off. You 
> may also want to setup pull-up/down on the pins to stabilize the 
> signals: it means changing the pin setup of the MCU to change them 
> from 'UART' to 'GPIO' and then configure the pull up/down feature. 
> You  may also want to change peripheral clock settings for disabled 
> peripheral in the MCU, to spare a bit more so you have to re configure 
> also clocking registers when the peripheral must come back in line.
>
> - There is CYGPKG_POWER. Each driver implementing some kind of power 
> management can be modified to support this package. But I don't see 
> how this package can interact with the platform code layer. How can a 
> target using a shared driver can make use of this package for the 
> shared driver?
>
> IMHO, beside a shutdown mechanism, one also needs to be able to get 
> control of what's going on between the hardware drivers and the 
> packages that use them. A low level application initialization routine 
> should be able to register callbacks to be triggered when events occur 
> in the drivers and in the package code managing them, hence the 
> application could handle the board or MCU specifically when some 
> expected event occurs. Today only part of this could be done in 
> platform code, but in such a way that it is very close to application 
> code, however without any clearly defined API.
>
>     Bernard
>

Thank you, Bernard.

I had not thought about reference counting in the drivers from multiple 
lookup() calls, but yes, that would probably be required.

In this context - closable devices - it seems unfortunate that the 
lookup() function has the side effect of opening the device. I can 
imagine that some code might just be trying to get the devtab entry for 
other purposes. Perhaps there needs to be another search function that 
returns the entry without triggering the lookup() function?

Does the file I/O code serialize calls to lookup?  I examined a few 
device drivers the implement lookup() and don't see any explicit 
mechanism to prevent race conditions, but that could be due to the 
nature of the calls not requiring it. I suppose the drivers should lock 
the scheduler when manipulating this new reference count.

I acknowledge that the close of the low-level driver may not be 
adequate, and the power management and GPIO pins are problematic.

I saw a recent discussion on the list about layering serial drivers, and 
started wondering if the target-level issues might be handled through 
layered drivers. Could I, perhaps, make a target-specific driver that 
intercepted the shutdown() calls, called the lower-level driver (which 
might just turn off the interrupt), then have the upper-layer driver 
power-down the device and tri-state the GPIO pins?

Would that be similar to the application callbacks that you mention?  If 
not, please give more detail about what you're thinking in this regard.

Thanks,
Frank

Re: Closing devices

[Bernard Fouché]

Le 20/06/2012 15:37, Frank Pagliughi a écrit :
> On 06/19/2012 09:36 AM, Bernard Fouché wrote:
>>
>> IMHO, beside a shutdown mechanism, one also needs to be able to get 
>> control of what's going on between the hardware drivers and the 
>> packages that use them. A low level application initialization 
>> routine should be able to register callbacks to be triggered when 
>> events occur in the drivers and in the package code managing them, 
>> hence the application could handle the board or MCU specifically when 
>> some expected event occurs. Today only part of this could be done in 
>> platform code, but in such a way that it is very close to application 
>> code, however without any clearly defined API.
>>
>>     Bernard
>>
>
> Thank you, Bernard.
>
> I had not thought about reference counting in the drivers from 
> multiple lookup() calls, but yes, that would probably be required.
>
> In this context - closable devices - it seems unfortunate that the 
> lookup() function has the side effect of opening the device. I can 
> imagine that some code might just be trying to get the devtab entry 
> for other purposes. Perhaps there needs to be another search function 
> that returns the entry without triggering the lookup() function?

The problem seems yet more general: anything can call cyg_io_lookup() 
and since the API doesn't have from the beginning a function call to 
report 'I don't need the devtab entry anymore', then even counting the 
number of times lookup() is called won't help much: my suggestion do 
move hw init code from init() to lookup() is very wrong...

>
> Does the file I/O code serialize calls to lookup?  I examined a few 
> device drivers the implement lookup() and don't see any explicit 
> mechanism to prevent race conditions, but that could be due to the 
> nature of the calls not requiring it. I suppose the drivers should 
> lock the scheduler when manipulating this new reference count.

Since when calling lookup() you mostly only get a reference to the 
devtab entry, there isn't any race condition. If you look at 
ser_16x5x.c, when lookup() is called, it calls serial_init() in the 
upper layer, in serial.c . In serial.c, the concerned channel is 
initialized only once by serial_init(), for whatever lower driver calls 
it. I guess this has been done to allow calling lookup() at any time, 
multiple times.

>
> I acknowledge that the close of the low-level driver may not be 
> adequate, and the power management and GPIO pins are problematic.
>
> I saw a recent discussion on the list about layering serial drivers, 
> and started wondering if the target-level issues might be handled 
> through layered drivers. Could I, perhaps, make a target-specific 
> driver that intercepted the shutdown() calls, called the lower-level 
> driver (which might just turn off the interrupt), then have the 
> upper-layer driver power-down the device and tri-state the GPIO pins?
>
> Would that be similar to the application callbacks that you mention?  
> If not, please give more detail about what you're thinking in this 
> regard.
>
>
Well my callback scheme seems all wrong: the more I look, the more it 
seems that nothing is able to track down exactly the hw driver use made 
by upper layers (packages or application code). Since the application 
needs to handle many details like GPIO pin setup, then it can also mask 
the interrupt ;-). So much for a layered software model with low level 
stuff done only in low level code...

Now if you tristate the pins, you are normally disabling the UART 
feature since you put the pins in GPIO mode. And since the UART cell is 
disconnected from the pins, it won't generate any interrupt: this is 
what I do today to avoid modifying ser_16x_5x, but the UART cell remains 
powered in the MCU which is a waste. I guess I'll end making my own copy 
of ser_16x_5x for my specific target, and add  _set_config() options to 
put the driver in whatever state is required to disable or enable it.

   Bernard

Re: Closing devices

[Frank Pagliughi]

>>
>> In this context - closable devices - it seems unfortunate that the 
>> lookup() function has the side effect of opening the device. I can 
>> imagine that some code might just be trying to get the devtab entry 
>> for other purposes. Perhaps there needs to be another search function 
>> that returns the entry without triggering the lookup() function?
>
> The problem seems yet more general: anything can call cyg_io_lookup() 
> and since the API doesn't have from the beginning a function call to 
> report 'I don't need the devtab entry anymore', then even counting the 
> number of times lookup() is called won't help much: my suggestion do 
> move hw init code from init() to lookup() is very wrong...

Well, several device drivers do postpone "final" initialization until 
the lookup() function. But many more don't make a distinction at all.

>
>>
>> Does the file I/O code serialize calls to lookup?  I examined a few 
>> device drivers the implement lookup() and don't see any explicit 
>> mechanism to prevent race conditions, but that could be due to the 
>> nature of the calls not requiring it. I suppose the drivers should 
>> lock the scheduler when manipulating this new reference count.
>
> Since when calling lookup() you mostly only get a reference to the 
> devtab entry, there isn't any race condition. If you look at 
> ser_16x5x.c, when lookup() is called, it calls serial_init() in the 
> upper layer, in serial.c . In serial.c, the concerned channel is 
> initialized only once by serial_init(), for whatever lower driver 
> calls it. I guess this has been done to allow calling lookup() at any 
> time, multiple times.

Sorry. I meant the individual implementations of lookup() calls in the 
drivers that do use it. I didn't see any that locked the scheduler 
before they started fiddling with registers on the device. Though 
admittedly, I didn't look through too many drivers.

>
> Well my callback scheme seems all wrong: the more I look, the more it 
> seems that nothing is able to track down exactly the hw driver use 
> made by upper layers (packages or application code). Since the 
> application needs to handle many details like GPIO pin setup, then it 
> can also mask the interrupt ;-). So much for a layered software model 
> with low level stuff done only in low level code...
>

I just want to reiterate that I am trying to find a mechanism by which I 
can add the capability to write *new* drivers that can close their 
devices, without breaking the existing code.

So all the drivers that already exist would work in the exact same way - 
they are fully opened after init() and the first call to lookup(), and 
they never close.

But if you want to write new drivers that can be closed, or modify 
select ones that are already written than there is a mechanism to do so, 
even if there are some constraints on using these new devices.  For 
example, if multiple threads are using a device and one thread calls the 
yet-to-be-implemented cyg_io_shutdown() funtion, obviously the device 
would be closed for all the threads.

New applications that make use of the device "close" feature would need 
to be conscious of this and properly protect and share the closable objects.

It's just maddening to me that when you use the File IO layer, and you 
call close(fd) on your device, nothing gets propogated to the driver and 
it still keeps chugging away, servicing interrupts, and consuming CPU 
time and electric power, even though the application thought that it 
"closed" the device.

Does that make sense?

Frank

Re: Closing devices

[Bernard Fouché]

Le 21/06/2012 20:20, Frank Pagliughi a écrit :
>
>> Well my callback scheme seems all wrong: the more I look, the more it 
>> seems that nothing is able to track down exactly the hw driver use 
>> made by upper layers (packages or application code). Since the 
>> application needs to handle many details like GPIO pin setup, then it 
>> can also mask the interrupt ;-). So much for a layered software model 
>> with low level stuff done only in low level code...
>>
>
> I just want to reiterate that I am trying to find a mechanism by which 
> I can add the capability to write *new* drivers that can close their 
> devices, without breaking the existing code.

It seems the easiest solution is too process specific config keys: 
serial.c will forward downward the config keys it does not understand 
and there is no change in CYG_IO, SERIAL, or whatever upper package. If 
you look at config_keys.h, there are already keys to enable/disable a 
CAN channel. IMHO what we want is adding similar keys to the serial 
driver system.

>
> So all the drivers that already exist would work in the exact same way 
> - they are fully opened after init() and the first call to lookup(), 
> and they never close.
>
> But if you want to write new drivers that can be closed, or modify 
> select ones that are already written than there is a mechanism to do 
> so, even if there are some constraints on using these new devices.  
> For example, if multiple threads are using a device and one thread 
> calls the yet-to-be-implemented cyg_io_shutdown() funtion, obviously 
> the device would be closed for all the threads.

Using new keys, you make your own power_open() / power_close() functions 
(or whatever name). They don't call close(), they use instead 
cyg_io_set_config() to send the keys to the driver: this keeps the whole 
package /driver organization untouched and this is a very slight 
modification that may even be accepted in the eCos repo (I was able to 
have new CAN keys accepted). While you are at it, what about adding a 
config key to start/stop transmitting the 'break' serial character ;-) .

So your new driver processes these new keys, and if ever someone tries 
to use the keys on a driver not supporting them, cyg_io_set_config() 
will report an error and the developper knows some work is needed at low 
level to add the feature on the concerned driver.

>
> New applications that make use of the device "close" feature would 
> need to be conscious of this and properly protect and share the 
> closable objects.
>
> It's just maddening to me that when you use the File IO layer, and you 
> call close(fd) on your device, nothing gets propogated to the driver 
> and it still keeps chugging away, servicing interrupts, and consuming 
> CPU time and electric power, even though the application thought that 
> it "closed" the device.

I guess part of the initial goals for eCos were to have a functionally 
rich system without being necessarily tied to the Posix world, while 
allowing easy porting of application coming from a Posix like system. 
Otherwise it would have been called eCosnix ;-)

Another possible reason could be that more than 10 years ago eCos was 
not targeted to be used on systems with great power constraints since it 
was assumed the MCU was a 32 bits one and at that time 32 bits MCU were 
consuming much more power than nowadays. MCU had less embedded 
peripherals. Humanity was paying the barrel $20.

Today with cores like the Cortex-M series you can reach power 
consumption very close to a 8/16 bits system while and you have 32 bits 
luxury allowing you to move on from no OS or from a feature poor OS to 
something like eCos. In some way we rant bout having not enough caviar 
in the free lunch :-)

What we miss is some document explaining the design choices (beside the 
source code).

    Bernard

Re: Closing devices

[Frank Pagliughi]

>
> Using new keys, you make your own power_open() / power_close() 
> functions (or whatever name). They don't call close(), they use 
> instead cyg_io_set_config() to send the keys to the driver: this keeps 
> the whole package /driver organization untouched and this is a very 
> slight modification that may even be accepted in the eCos repo (I was 
> able to have new CAN keys accepted). While you are at it, what about 
> adding a config key to start/stop transmitting the 'break' serial 
> character ;-) .
>
> So your new driver processes these new keys, and if ever someone tries 
> to use the keys on a driver not supporting them, cyg_io_set_config() 
> will report an error and the developper knows some work is needed at 
> low level to add the feature on the concerned driver.

I like the idea of some new, consistent keys for putting devices into a 
low power state(s) - like devices that can power down until something 
interesting happens. But to the application they are still "open".

I still think that having a close/shutdown is appropriate for cases 
where you don't want anything from the device for some amount of time.

>
>> I guess part of the initial goals for eCos were to have a 
>> functionally rich system without being necessarily tied to the Posix 
>> world, while allowing easy porting of application coming from a Posix 
>> like system. Otherwise it would have been called eCosnix ;-)

Yes, agreed. But things have certainly changed. The Linux raid on the 
embedded space has been great and yet somewhat devastating. The most 
popular open-source RTOS a few years from now will likely be something 
that is very compatible with Linux.  My current eCos project shared 
about 70% of it's code with an ARM9 Linux application.

So I'm looking for a solution that propagates through all of the eCos 
API's. This may be a personal bias. I don't tend to program eCos with 
full POSIX support, but I almost always use the FileIO package. It keeps 
the code much, much more portable.

Of course, another option is to create an entirely new I/O package that 
can replace (or sit side-by-side) with the existing one. Then let the 
new package provide all the added functionality for removable and 
dynamic devices. But I'm not up for that at the moment!

Well, I've already implemented the basic "shutdown" framework (in about 
an hour), and it is not very intrusive, nor very long. I will do some 
testing, and then upload it for comment some time next week.

Frank