linux-kernel - Re: [PATCH v4 6/8] i3c: target: func: add tty driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Wed, 24 Jan 2024 17:15:01 +0200 (EET)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Frank Li <Frank.Li@....com>
cc: alexandre.belloni@...tlin.com, conor.culhane@...vaco.com, 
    devicetree@...r.kernel.org, gregkh@...uxfoundation.org, 
    imx@...ts.linux.dev, jirislaby@...nel.org, joe@...ches.com, 
    krzysztof.kozlowski+dt@...aro.org, krzysztof.kozlowski@...aro.org, 
    linux-i3c@...ts.infradead.org, linux-kernel@...r.kernel.org, 
    linux-serial@...r.kernel.org, miquel.raynal@...tlin.com, robh@...nel.org, 
    zbigniew.lukwinski@...ux.intel.com
Subject: Re: [PATCH v4 6/8] i3c: target: func: add tty driver

On Tue, 23 Jan 2024, Frank Li wrote:

> Add tty over I3C target function driver.
> 
> Signed-off-by: Frank Li <Frank.Li@....com>

> diff --git a/drivers/i3c/func/tty.c b/drivers/i3c/func/tty.c
> new file mode 100644
> index 0000000000000..bad99c08be0ac
> --- /dev/null
> +++ b/drivers/i3c/func/tty.c
> @@ -0,0 +1,475 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2023 NXP
> + * Author: Frank Li <Frank.Li@....com>
> + */
> +
> +#include <linux/iopoll.h>
> +#include <linux/i3c/target.h>
> +#include <linux/serial_core.h>
> +#include <linux/slab.h>
> +#include <linux/tty_flip.h>
> +
> +static DEFINE_IDR(i3c_tty_minors);
> +
> +static struct tty_driver *i3c_tty_driver;
> +
> +#define I3C_TTY_MINORS		8
> +
> +#define I3C_TX_NOEMPTY		BIT(0)
> +#define I3C_TTY_TRANS_SIZE	16
> +#define I3C_TTY_IBI_TX		BIT(0)

Lacking #include for BIT()


> +	do {
> +		req = i3c_target_ctrl_alloc_request(func->ctrl, GFP_KERNEL);
> +		if (!req)
> +			goto err_alloc_req;
> +
> +		req->buf = (void *) (sport->buffer + offset);

Casting to void is unnecessary.

> +		req->length = rxfifo_size;
> +		req->context = sport;
> +		req->complete = i3c_target_tty_rx_complete;
> +		offset += rxfifo_size;
> +
> +		if (i3c_target_ctrl_queue(req, GFP_KERNEL))
> +			goto err_alloc_req;
> +	} while (req && (offset + rxfifo_size) < UART_XMIT_SIZE);

Extra ().

> +static void i3c_port_shutdown(struct tty_port *port)
> +{
> +	struct ttyi3c_port *sport =
> +		container_of(port, struct ttyi3c_port, port);

Just put this to a single line, nothing important is lost even on 80 char
screen.

> +static void i3c_port_destruct(struct tty_port *port)
> +{
> +	struct ttyi3c_port *sport =
> +		container_of(port, struct ttyi3c_port, port);

Ditto.

> +	if (i3c_target_ctrl_set_config(func->ctrl, func)) {
> +		dev_err(&func->dev, "failure set i3c config\n");

failed to set i3c config


> +	read_poll_timeout(i3c_target_ctrl_fifo_status, val, !val, 100, timeout, false,
> +			  sport->i3cdev->ctrl, true);
> +
> +	i3c_target_ctrl_set_status_format1(sport->i3cdev->ctrl, sport->status & (~I3C_TX_NOEMPTY));

Unnecessary ().

-- 
 i.