W dniu 2014-09-25 17:29, Richard Genoud pisze:

2014-09-20 10:08 GMT+02:00 Janusz Użycki [off-list ref]:
Hi,

Hi Richard,

first, thanks for the very useful answers.

quoted

Could you look on my patch series [1...4]
serial: mxs-auart:
use mctrl_gpio helpers for handling modem signals (v2.2c)
at http://news.gmane.org/gmane.linux.serial ?

If you resend them, put me in the CC list.
That will be easier for me to have a look at them.

sure

quoted

I added some comments in patch's code.
In atmel_serial mctrl_gpio_free() is ommited,
is it useless?

If you take a look at mctrl_gpio_init(), you'll see that all the
allocations are done with devm_* functions.
So that they are automatically deallocated when the module unloads.
I still wrote mctrl_gpio_free() for some cases where we want to free
memory without unloading the module.

So you don't have to call mctrl_gpio_free() at the end of
mxs_auart_probe() and mxs_auart_remove()
(cf Documentation/serial/driver )

It is clear now, thanks.

quoted

Atmel_serial also does not parse modem
line status in mxs_auart_get_mctrl(),
8250 driver does it. Which solution is ok?

Well, in atmel_get_mctrl(), we have to retrieve the line status from
the uart controller :
that's done with :
status = UART_GET_CSR(port);
Then, for the lines controlled via GPIO, we are calling
mctrl_gpio_get() that updates the mctrl flags with gpios states.

It doesn't seems different from 8250.

But serial8250_get_mctrl() in 8250_core.c calls serial8250_modem_status()
which calls eg. uart_handle_cts_change() even if enable_ms() wasn't called.
This is the difference.
The serial8250_modem_status() is also called in the interrupt
and, what I don't understand, in serial8250_console_write().

quoted

Do you happen to know if tty layer
modifies mctrl between mxs_auart_get_mctrl(),
mxs_auart_set_mctrl() and mxs_auart_get_mctrl() again?

I don't quite understand what you mean.
The only way for the kernel to get line status (DCD, CTS,DSR,RI) is
via get_mctrl.
And the only way for the kernel to set line status
(RTS,DTR,out1,out2,loop) is via set_mctrl.

Yes, but:
* mxs_auart_set_mctrl() in the mxs-auart stores the lastest mctrl
    in private ctrl field
* the stored value is used to supply initial returned value
    by mxs_auart_get_mctrl()
* uart_update_mctrl() in serial_core.c modifies only selected bits
    in port->mctrl field - OK, input bits stay untouched
* uart_port_startup() in serial_code.c sets RTS and DTR
    and zeroes others (also input bits)
* uart_set_termios() in serial_core.c modifies the bits too
* uart_suspend_port() in serial_core.c sets all mctrl bits to 0

It looks that mxs_auart_set_mctrl() doesn't have to store mctrl at all
because uart_tiocmget() in serial_core.c restores the output bits
from port->mctrl field. So the code is duplicated.
Do you agree?
P.S. It was even implemented in serial_core.c in 2.6.12.

best regards
Janusz

best regards,
Richard.

--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html