Messages in this thread | | | Date | Fri, 21 Mar 2008 12:17:36 +0100 | From | Rodolfo Giometti <> | Subject | Re: [PATCH 5/7] PPS: serial clients support. |
| |
On Thu, Mar 20, 2008 at 01:04:00PM -0700, Andrew Morton wrote: > On Thu, 6 Mar 2008 13:09:04 +0100 > Rodolfo Giometti <giometti@linux.it> wrote: > > > Adds support for the PPS sources connected with the CD (Carrier > > Detect) pin of a serial port. > > > > ... > > > > +#ifdef CONFIG_PPS_CLIENT_UART > > + > > +static int > > +uart_register_pps_port(struct uart_state *state, struct uart_port *port) > > +{ > > + struct tty_driver *drv = port->info->tty->driver; > > + int ret; > > + > > + state->pps_info.owner = THIS_MODULE; > > + state->pps_info.dev = port->dev; > > + snprintf(state->pps_info.name, PPS_MAX_NAME_LEN, "%s%d", > > + drv->driver_name, port->line); > > + snprintf(state->pps_info.path, PPS_MAX_NAME_LEN, "/dev/%s%d", > > + drv->name, port->line); > > umm, why are we hard-wiring "/dev" into the kernel source? Is it just for > user-friendly printks?
Yes... :)
> > + state->pps_info.mode = PPS_CAPTUREBOTH | \ > > + PPS_OFFSETASSERT | PPS_OFFSETCLEAR | \ > > + PPS_CANWAIT | PPS_TSFMT_TSPEC; > > + > > + ret = pps_register_source(&state->pps_info, PPS_CAPTUREBOTH | \ > > + PPS_OFFSETASSERT | PPS_OFFSETCLEAR); > > + if (ret < 0) { > > + dev_err(port->dev, "cannot register PPS source \"%s\"\n", > > + state->pps_info.path); > > + return ret; > > + } > > + port->pps_source = ret; > > + dev_dbg(port->dev, "PPS source #%d \"%s\" added\n", > > + port->pps_source, state->pps_info.path); > > + > > + return 0; > > +} > > + > > +static void > > +uart_unregister_pps_port(struct uart_state *state, struct uart_port *port) > > +{ > > + pps_unregister_source(port->pps_source); > > + dev_dbg(port->dev, "PPS source #%d \"%s\" removed\n", > > + port->pps_source, state->pps_info.path); > > +} > > + > > +#else > > + > > +#define uart_register_pps_port(state, port) do { } while (0) > > +#define uart_unregister_pps_port(state, port) do { } while (0) > > Please never implement in cpp that which can be implemented in C. These > should have been static inlines. That's nicer, matches the > CONFIG_PPS_CLIENT_UART=y code and provides type checking.
Are you saying that I should do:
static inline int uart_register_pps_port(struct uart_state *state, struct uart_port *port) { /* Nop */ }
static inline void uart_unregister_pps_port(struct uart_state *state, struct uart_port *port) { /* Nop */ }
?
Ciao,
Rodolfo
--
GNU/Linux Solutions e-mail: giometti@enneenne.com Linux Device Driver giometti@gnudd.com Embedded Systems giometti@linux.it UNIX programming phone: +39 349 2432127
| |