Re: [PATCH v7 2/2] tty: add rpmsg driver

From: Joe Perches
Date: Tue Mar 24 2020 - 13:25:00 EST


On Tue, 2020-03-24 at 18:04 +0100, Arnaud Pouliquen wrote:
> This driver exposes a standard TTY interface on top of the rpmsg
> framework through a rpmsg service.
>
> This driver supports multi-instances, offering a /dev/ttyRPMSGx entry
> per rpmsg endpoint.

trivial notes:

> diff --git a/Documentation/serial/tty_rpmsg.rst b/Documentation/serial/tty_rpmsg.rst
[]
> +The rpmsg tty driver implements serial communication on the RPMsg bus to makes possible for user-space programs to send and receive rpmsg messages as a standard tty protocol.

Very long text lines missing newlines?

[]
> +To be compliant with this driver, the remote firmware must create its data end point associated with the "rpmsg-tty-raw" service.
[]
> +To be compatible with this driver, the remote firmware must create or use its end point associated with "rpmsg-tty-ctrl" service, plus a second endpoint for the data flow.
> +On Linux rpmsg_tty probes, the data endpoint address and the CTS (set to disable)

[]

> diff --git a/drivers/tty/rpmsg_tty.c b/drivers/tty/rpmsg_tty.c
[]
> +typedef void (*rpmsg_tty_rx_cb_t)(struct rpmsg_device *, void *, int, void *,
> + u32);

unused typedef?

[]

> +static int __init rpmsg_tty_init(void)
> +{
[]
> + err = tty_register_driver(rpmsg_tty_driver);
> + if (err < 0) {
> + pr_err("Couldn't install rpmsg tty driver: err %d\n", err);
> + goto error_put;
> + }

Might use vsprintf extension %pe

pr_err("Couldn't install rpmsg tty driver: %pe\n", ERR_PTR(err));

> + err = register_rpmsg_driver(&rpmsg_tty_rpmsg_drv);
> + if (err < 0) {
> + pr_err("Couldn't register rpmsg tty driver: err %d\n", err);

etc.