* Handling RTS with an UART that doesn't directly drives the RTS pin @ 2012-06-12 12:34 Bernard Fouché 2012-06-12 13:12 ` Stanislav Meduna ` (2 more replies) 0 siblings, 3 replies; 14+ messages in thread From: Bernard Fouché @ 2012-06-12 12:34 UTC (permalink / raw) To: <ecos-devel@ecos.sourceware.org> 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Handling RTS with an UART that doesn't directly drives the RTS pin 2012-06-12 12:34 Handling RTS with an UART that doesn't directly drives the RTS pin Bernard Fouché @ 2012-06-12 13:12 ` Stanislav Meduna 2012-06-12 13:16 ` Nick Garnett [not found] ` <4FDF355B.2010209@mindspring.com> 2 siblings, 0 replies; 14+ messages in thread From: Stanislav Meduna @ 2012-06-12 13:12 UTC (permalink / raw) To: ecos-devel 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Handling RTS with an UART that doesn't directly drives the RTS pin 2012-06-12 12:34 Handling RTS with an UART that doesn't directly drives the RTS pin Bernard Fouché 2012-06-12 13:12 ` Stanislav Meduna @ 2012-06-12 13:16 ` Nick Garnett 2012-06-12 16:56 ` Bernard Fouché [not found] ` <4FDF355B.2010209@mindspring.com> 2 siblings, 1 reply; 14+ messages in thread From: Nick Garnett @ 2012-06-12 13:16 UTC (permalink / raw) To: Bernard Fouché; +Cc: <ecos-devel@ecos.sourceware.org> 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Handling RTS with an UART that doesn't directly drives the RTS pin 2012-06-12 13:16 ` Nick Garnett @ 2012-06-12 16:56 ` Bernard Fouché 2012-06-13 10:10 ` Nick Garnett 0 siblings, 1 reply; 14+ messages in thread From: Bernard Fouché @ 2012-06-12 16:56 UTC (permalink / raw) To: Nick Garnett; +Cc: <ecos-devel@ecos.sourceware.org>, stano 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Handling RTS with an UART that doesn't directly drives the RTS pin 2012-06-12 16:56 ` Bernard Fouché @ 2012-06-13 10:10 ` Nick Garnett 2012-06-13 16:37 ` Bernard Fouché 0 siblings, 1 reply; 14+ messages in thread From: Nick Garnett @ 2012-06-13 10:10 UTC (permalink / raw) To: Bernard Fouché; +Cc: <ecos-devel@ecos.sourceware.org>, stano 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Handling RTS with an UART that doesn't directly drives the RTS pin 2012-06-13 10:10 ` Nick Garnett @ 2012-06-13 16:37 ` Bernard Fouché 2012-06-14 15:33 ` Bernard Fouché 0 siblings, 1 reply; 14+ messages in thread From: Bernard Fouché @ 2012-06-13 16:37 UTC (permalink / raw) To: Nick Garnett, <ecos-devel@ecos.sourceware.org> 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Handling RTS with an UART that doesn't directly drives the RTS pin 2012-06-13 16:37 ` Bernard Fouché @ 2012-06-14 15:33 ` Bernard Fouché 0 siblings, 0 replies; 14+ messages in thread From: Bernard Fouché @ 2012-06-14 15:33 UTC (permalink / raw) To: ecos-devel 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! ^ permalink raw reply [flat|nested] 14+ messages in thread
[parent not found: <4FDF355B.2010209@mindspring.com>]
* Re: Closing devices [not found] ` <4FDF355B.2010209@mindspring.com> @ 2012-06-18 14:34 ` Frank Pagliughi 2012-06-19 13:36 ` Bernard Fouché 0 siblings, 1 reply; 14+ messages in thread From: Frank Pagliughi @ 2012-06-18 14:34 UTC (permalink / raw) To: eCos 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Closing devices 2012-06-18 14:34 ` Closing devices Frank Pagliughi @ 2012-06-19 13:36 ` Bernard Fouché 2012-06-20 13:38 ` Frank Pagliughi 0 siblings, 1 reply; 14+ messages in thread From: Bernard Fouché @ 2012-06-19 13:36 UTC (permalink / raw) To: ecos-devel 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Closing devices 2012-06-19 13:36 ` Bernard Fouché @ 2012-06-20 13:38 ` Frank Pagliughi 2012-06-20 15:20 ` Bernard Fouché 0 siblings, 1 reply; 14+ messages in thread From: Frank Pagliughi @ 2012-06-20 13:38 UTC (permalink / raw) To: Bernard Fouché; +Cc: ecos-devel 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Closing devices 2012-06-20 13:38 ` Frank Pagliughi @ 2012-06-20 15:20 ` Bernard Fouché 2012-06-21 18:20 ` Frank Pagliughi 0 siblings, 1 reply; 14+ messages in thread From: Bernard Fouché @ 2012-06-20 15:20 UTC (permalink / raw) To: ecos-devel 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Closing devices 2012-06-20 15:20 ` Bernard Fouché @ 2012-06-21 18:20 ` Frank Pagliughi 2012-06-22 8:33 ` Bernard Fouché 0 siblings, 1 reply; 14+ messages in thread From: Frank Pagliughi @ 2012-06-21 18:20 UTC (permalink / raw) To: Bernard Fouché; +Cc: ecos-devel >> >> 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Closing devices 2012-06-21 18:20 ` Frank Pagliughi @ 2012-06-22 8:33 ` Bernard Fouché 2012-06-22 14:48 ` Frank Pagliughi 0 siblings, 1 reply; 14+ messages in thread From: Bernard Fouché @ 2012-06-22 8:33 UTC (permalink / raw) To: ecos-devel 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Closing devices 2012-06-22 8:33 ` Bernard Fouché @ 2012-06-22 14:48 ` Frank Pagliughi 0 siblings, 0 replies; 14+ messages in thread From: Frank Pagliughi @ 2012-06-22 14:48 UTC (permalink / raw) To: Bernard Fouché; +Cc: ecos-devel > > 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2012-06-22 14:48 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-06-12 12:34 Handling RTS with an UART that doesn't directly drives the RTS pin Bernard Fouché 2012-06-12 13:12 ` Stanislav Meduna 2012-06-12 13:16 ` Nick Garnett 2012-06-12 16:56 ` Bernard Fouché 2012-06-13 10:10 ` Nick Garnett 2012-06-13 16:37 ` Bernard Fouché 2012-06-14 15:33 ` Bernard Fouché [not found] ` <4FDF355B.2010209@mindspring.com> 2012-06-18 14:34 ` Closing devices Frank Pagliughi 2012-06-19 13:36 ` Bernard Fouché 2012-06-20 13:38 ` Frank Pagliughi 2012-06-20 15:20 ` Bernard Fouché 2012-06-21 18:20 ` Frank Pagliughi 2012-06-22 8:33 ` Bernard Fouché 2012-06-22 14:48 ` Frank Pagliughi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).