Re: [PATCH v1] treewide, serdev: change receive_buf() return type to size_t
From: Johan Hovold <johan@kernel.org>
Date: 2023-12-15 13:51:14
Also in:
chrome-platform, linux-arm-kernel, linux-bluetooth, linux-iio, linux-mediatek, linux-serial, linux-sound, lkml, platform-driver-x86
On Thu, Dec 14, 2023 at 06:01:46PM +0100, Francesco Dolcini wrote:
From: Francesco Dolcini <redacted> receive_buf() is called from ttyport_receive_buf() that expects values ">= 0" from serdev_controller_receive_buf(), change its return type from ssize_t to size_t. Suggested-by: Jiri Slaby <jirislaby@kernel.org> Link: https://lore.kernel.org/all/087be419-ec6b-47ad-851a-5e1e3ea5cfcc@kernel.org/ (local) Signed-off-by: Francesco Dolcini <redacted> --- hello, patch is based on current linux next. It has an obvious problem, it touches files from multiple subsystem in a single patch that is complicated to review and eventually merge, just splitting this would however not work, it will break bisectability and the build. I am looking for advise on the best way to move forward. I see the following options: - keep it as it is - break it down with a patch with each subsystem, and squash before applying from a single (tty?) subsystem - go for a multi stage approach, defining a new callback, move to it and in the end remove the original one, likewise it was done for i2c lately --- drivers/bluetooth/btmtkuart.c | 4 ++-- drivers/bluetooth/btnxpuart.c | 4 ++--
quoted hunk ↗ jump to hunk
diff --git a/drivers/bluetooth/btmtkuart.c b/drivers/bluetooth/btmtkuart.c index 3c84fcbda01a..e6bc4a73c9fc 100644 --- a/drivers/bluetooth/btmtkuart.c +++ b/drivers/bluetooth/btmtkuart.c@@ -383,8 +383,8 @@ static void btmtkuart_recv(struct hci_dev *hdev, const u8 *data, size_t count) } } -static ssize_t btmtkuart_receive_buf(struct serdev_device *serdev, - const u8 *data, size_t count) +static size_t btmtkuart_receive_buf(struct serdev_device *serdev, + const u8 *data, size_t count) { struct btmtkuart_dev *bdev = serdev_device_get_drvdata(serdev);diff --git a/drivers/bluetooth/btnxpuart.c b/drivers/bluetooth/btnxpuart.c index 1d592ac413d1..056bef5b2919 100644 --- a/drivers/bluetooth/btnxpuart.c +++ b/drivers/bluetooth/btnxpuart.c@@ -1264,8 +1264,8 @@ static const struct h4_recv_pkt nxp_recv_pkts[] = { { NXP_RECV_FW_REQ_V3, .recv = nxp_recv_fw_req_v3 }, }; -static ssize_t btnxpuart_receive_buf(struct serdev_device *serdev, - const u8 *data, size_t count) +static size_t btnxpuart_receive_buf(struct serdev_device *serdev, + const u8 *data, size_t count) { struct btnxpuart_dev *nxpdev = serdev_device_get_drvdata(serdev);
A quick check of just the first two functions here shows that they can return negative values.
quoted hunk ↗ jump to hunk
diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c index e94e090cf0a1..3d7ae7fa5018 100644 --- a/drivers/tty/serdev/serdev-ttyport.c +++ b/drivers/tty/serdev/serdev-ttyport.c@@ -27,19 +27,17 @@ static size_t ttyport_receive_buf(struct tty_port *port, const u8 *cp, { struct serdev_controller *ctrl = port->client_data; struct serport *serport = serdev_controller_get_drvdata(ctrl); - int ret; + size_t ret; if (!test_bit(SERPORT_ACTIVE, &serport->flags)) return 0; ret = serdev_controller_receive_buf(ctrl, cp, count); - dev_WARN_ONCE(&ctrl->dev, ret < 0 || ret > count, - "receive_buf returns %d (count = %zu)\n", + dev_WARN_ONCE(&ctrl->dev, ret > count, + "receive_buf returns %zu (count = %zu)\n", ret, count); - if (ret < 0) - return 0; - else if (ret > count) + if (ret > count) return count; return ret;
So please do not apply this patch until the various implementations have been fixed. Johan