lkml.org 
[lkml]   [2016]   [Mar]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v3] Bluetooth: hci_uart: Support firmware download for Marvell
O
> +/* Get standard baud rate, given the speed */
> +static unsigned int get_baud_rate(unsigned int speed)
> +{
> + switch (speed) {
> + case 9600:
> + return B9600;
> + case 19200:
> + return B19200;
> + case 38400:
> + return B38400;
> + case 57600:
> + return B57600;
> + case 115200:
> + return B115200;
> + case 230400:
> + return B230400;
> + case 460800:
> + return B460800;
> + case 921600:
> + return B921600;
> + case 2000000:
> + return B2000000;
> + case 3000000:
> + return B3000000;
> + default:
> + return -1;
> + }
> +}


NAK. Please use the existing kernel helpers for this


+
> +/* Set terminal properties */
> +static int mrvl_set_term_baud(struct tty_struct *tty, unsigned int speed,
> + unsigned char flow_ctl)
> +{
> + struct ktermios old_termios = tty->termios;
> + int baud;
> +
> + tty->termios.c_cflag &= ~CBAUD;
> + baud = get_baud_rate(speed);
> +
> + if (baud == -1) {
> + BT_ERR("Baud rate not supported");
> + return -1;
> + }
> +
> + tty->termios.c_cflag |= baud;
> +

This isn't the correct way to do any of this, just do

tty_termios_encode_baud_rate(&tty->termios, speed, speed)


> + if (flow_ctl)
> + tty->termios.c_cflag |= CRTSCTS;
> + else
> + tty->termios.c_cflag &= ~CRTSCTS;
> +
> + tty->ops->set_termios(tty, &old_termios);

Call the provided kernel helpers that get the locking right.

tty_set_termios(tty, &new_termios);

You should also do your error checking here and see what baud rate was
actually provided by the hardware (tty_get_baud_rate(tty)) by checking
the value actually selected by the tty.

> + /* restore uart settings */
> + new_termios = tty->termios;
> + tty->termios.c_cflag = old_termios.c_cflag;
> + tty->ops->set_termios(tty, &new_termios);
> + clear_bit(HCI_UART_DNLD_FW, &hu->flags);

Again use the proper helpers

> +
> +set_baud:
> + ret = mrvl_set_baud(hu);
> + if (ret)
> + goto fail;
> +
> + mdelay(MRVL_DNLD_DELAY);

Why not msleep() ?

> +
> + return ret;
> +fail:
> + /* restore uart settings */
> + new_termios = tty->termios;
> + tty->termios.c_cflag = old_termios.c_cflag;
> + tty->ops->set_termios(tty, &new_termios);
> + clear_bit(HCI_UART_DNLD_FW, &hu->flags);
> +

Ditto...

Alan



\
 
 \ /
  Last update: 2016-03-01 23:41    [W:0.069 / U:0.612 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site