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

classic Classic list List threaded Threaded
14 messages Options
Reply | Threaded
Open this post in threaded view
|

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


Reply | Threaded
Open this post in threaded view
|

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
Reply | Threaded
Open this post in threaded view
|

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

Nick Garnett-2
In reply to this post by Bernard Fouché


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
Reply | Threaded
Open this post in threaded view
|

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
Reply | Threaded
Open this post in threaded view
|

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

Nick Garnett-2


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
Reply | Threaded
Open this post in threaded view
|

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


Reply | Threaded
Open this post in threaded view
|

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!
Reply | Threaded
Open this post in threaded view
|

Re: Closing devices

Frank Pagliughi
In reply to this post by Bernard Fouché
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

Reply | Threaded
Open this post in threaded view
|

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
Reply | Threaded
Open this post in threaded view
|

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


Reply | Threaded
Open this post in threaded view
|

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

Reply | Threaded
Open this post in threaded view
|

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
Reply | Threaded
Open this post in threaded view
|

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
Reply | Threaded
Open this post in threaded view
|

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