Re: [PATCH 10/11] tty: Add NVIDIA LSTP UART driver

[Jiri Slaby <jirislaby@kernel.org>]

Hi,

On 12. 10. 26, 19:36, Adrian Ambrozewicz wrote:
> --- /dev/null
> +++ b/drivers/tty/lstp_tty.c
> @@ -0,0 +1,829 @@
...
> +/* clang-format off */
> +static speed_t lstp_baud_to_speed(u32 baud)

Maybe implement this in drivers/tty/tty_baudrate.c on the top of 
baud_table + baud_bits?

> +{
> +	static const struct {
> +		u32     baud;
> +		speed_t speed;
> +	} map[] = {
> +		{      50, B50      }, {      75, B75      }, {     110, B110     },
> +		{     134, B134     }, {     150, B150     }, {     200, B200     },
> +		{     300, B300     }, {     600, B600     }, {    1200, B1200    },
> +		{    1800, B1800    }, {    2400, B2400    }, {    4800, B4800    },
> +		{    9600, B9600    }, {   19200, B19200   }, {   38400, B38400   },
> +		{   57600, B57600   }, {  115200, B115200  }, {  230400, B230400  },
> +		{  460800, B460800  }, {  500000, B500000  }, {  576000, B576000  },
> +		{  921600, B921600  }, { 1000000, B1000000 }, { 1152000, B1152000 },
> +		{ 1500000, B1500000 }, { 2000000, B2000000 }, { 2500000, B2500000 },
> +		{ 3000000, B3000000 }, { 3500000, B3500000 }, { 4000000, B4000000 },
> +	};
> +
> +	for (size_t i = 0; i < ARRAY_SIZE(map); i++)
> +		if (map[i].baud == baud)
> +			return map[i].speed;
> +
> +	return B0;
> +}
...
> +static int lstp_uart_start(struct lstp_channel *ch)
> +{
> +	struct lstp_uart_ctx *ctx = ch->priv;
> +
> +	/*
> +	 * Mark registered before publishing: tty_port_register_device() adds
> +	 * the cdev and emits the uevent that sends udev straight to open(),
> +	 * and lstp_tty_install() refuses a minor that is not marked. No node
> +	 * exists at this index yet, so nothing can reach the map early.
> +	 */
> +	scoped_guard(mutex, &lstp_minors_lock)
> +		ctx->registered = true;
> +
> +	struct device *tty_dev =
> +		tty_port_register_device(&ctx->port, lstp_tty_driver, ctx->minor, ch->dev);
> +
> +	if (IS_ERR(tty_dev)) {
> +		dev_err(ch->dev, "%s: ch_%d: Could not register TTY (%pe)\n", __func__, ch->ch_id,
> +			tty_dev);
> +		scoped_guard(mutex, &lstp_minors_lock)
> +			ctx->registered = false;
> +		return PTR_ERR(tty_dev);
> +	}
> +
> +	/* Nothing below may fail: really_probe() skips .remove() when .probe() errors. */
> +	dev_info(ch->dev, "%s: ch_%d: Started as %s\n", __func__, ch->ch_id, ch->display_name);

We usually print nothing in case everything went well.

> +	return 0;
> +}
...
> +static void lstp_uart_stop(struct lstp_channel *ch)
> +{
> +	struct lstp_uart_ctx *ctx = ch->priv;
> +
> +	/*
> +	 * Clear @registered before sampling @port->tty, not after:
> +	 * tty_port_open() publishes the tty before it calls .activate(), so an
> +	 * open can go live between a sample and a later clear and never be
> +	 * hung up here.
> +	 */
> +	scoped_guard(mutex, &lstp_minors_lock)
> +		ctx->registered = false;
> +
> +	struct tty_struct *tty = tty_port_tty_get(&ctx->port);
> +
> +	if (tty) {
> +		/*
> +		 * Synchronous: fds move to hung_up_tty_fops and lstp_port_shutdown()
> +		 * cancels write_work, so no write is submitted after this returns.
> +		 */
> +		tty_vhangup(tty);
> +		tty_kref_put(tty);

tty_port_tty_vhangup()?

> +	}
> +
> +	tty_unregister_device(lstp_tty_driver, ctx->minor);
> +}
thanks,
-- 
js
suse labs