| 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
| ||
|
Message-ID: <6ae3c1af-4368-4a3e-bfb5-366080048dac@kernel.org>
Date: Thu, 2 May 2024 10:44:46 +0200
From: Jiri Slaby <jirislaby@...nel.org>
To: Christoph Fritz <christoph.fritz@...dev.de>,
Oliver Hartkopp <socketcan@...tkopp.net>,
Marc Kleine-Budde <mkl@...gutronix.de>,
Vincent Mailhol <mailhol.vincent@...adoo.fr>,
"David S . Miller" <davem@...emloft.net>, Eric Dumazet
<edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, Jiri Kosina <jikos@...nel.org>,
Benjamin Tissoires <bentiss@...nel.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Sebastian Reichel <sre@...nel.org>, Linus Walleij <linus.walleij@...aro.org>
Cc: Andreas Lauser <andreas.lauser@...cedes-benz.com>,
Jonathan Corbet <corbet@....net>, Pavel Pisa <pisa@....felk.cvut.cz>,
linux-can@...r.kernel.org, netdev@...r.kernel.org,
devicetree@...r.kernel.org, linux-input@...r.kernel.org,
linux-serial@...r.kernel.org
Subject: Re: [PATCH v2 07/12] can: Add support for serdev LIN adapters
On 02. 05. 24, 9:55, Christoph Fritz wrote:
> This commit introduces LIN-Bus support for UART devices equipped with
> LIN transceivers, utilizing the Serial Device Bus (serdev) interface.
>
> For more details on an adapter, visit: https://hexdev.de/hexlin#tty
...
> --- /dev/null
> +++ b/drivers/net/can/lin-serdev.c
> @@ -0,0 +1,514 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/* Copyright (C) 2024 hexDEV GmbH - https://hexdev.de */
> +
> +#include <linux/module.h>
> +#include <linux/wait.h>
> +#include <linux/init.h>
> +#include <linux/errno.h>
> +#include <linux/string.h>
> +#include <linux/kernel.h>
What do you need kernel.h for? You should explicitly require what you
need (you apparently do), so kernel.h should not be needed.
> +#include <net/lin.h>
> +#include <linux/of.h>
> +#include <linux/serdev.h>
> +#include <linux/slab.h>
> +#include <linux/kfifo.h>
> +#include <linux/workqueue.h>
> +#include <linux/tty.h>
Might be eaier to maintain if you sort them.
> +#define LINSER_SAMPLES_PER_CHAR 10
> +#define LINSER_TX_BUFFER_SIZE 11
> +#define LINSER_RX_FIFO_SIZE 256
> +#define LINSER_PARSE_BUFFER 24
> +
> +struct linser_rx {
> + u8 data;
> + u8 flag;
> +};
> +
> +enum linser_rx_status {
> + NEED_MORE = -1,
> + MODE_OK = 0,
> + NEED_FORCE,
> +};
> +
> +struct linser_priv {
> + struct lin_device *lin_dev;
> + struct serdev_device *serdev;
> + DECLARE_KFIFO_PTR(rx_fifo, struct linser_rx);
> + struct delayed_work rx_work;
> + ulong break_usleep_min;
> + ulong break_usleep_max;
> + ulong post_break_usleep_min;
> + ulong post_break_usleep_max;
> + ulong force_timeout_jfs;
The same as for uint :)
> + struct lin_responder_answer respond_answ[LIN_NUM_IDS];
> + struct mutex resp_lock; /* protects respond_answ */
> + bool is_stopped;
> +};
...
> +static void linser_derive_timings(struct linser_priv *priv, u16 bitrate)
> +{
> + unsigned long break_baud = (bitrate * 2) / 3;
> + unsigned long timeout_us;
> +
Are those 1000000UL USEC_PER_SEC?
> + priv->break_usleep_min = (1000000UL * LINSER_SAMPLES_PER_CHAR) /
> + break_baud;
> + priv->break_usleep_max = priv->break_usleep_min + 50;
> + priv->post_break_usleep_min = (1000000UL * 1 /* 1 bit */) / break_baud;
> + priv->post_break_usleep_max = priv->post_break_usleep_min + 30;
> +
> + timeout_us = DIV_ROUND_CLOSEST(1000000UL * 256 /* bit */, bitrate);
> + priv->force_timeout_jfs = usecs_to_jiffies(timeout_us);
> +}
...
> +static bool linser_tx_frame_as_responder(struct linser_priv *priv, u8 id)
> +{
> + struct lin_responder_answer *answ = &priv->respond_answ[id];
> + struct serdev_device *serdev = priv->serdev;
> + u8 buf[LINSER_TX_BUFFER_SIZE];
> + u8 checksum, count, n;
> + ssize_t write_len;
> +
> + mutex_lock(&priv->resp_lock);
> +
> + if (!answ->is_active)
> + goto unlock_and_exit_false;
> +
> + if (answ->is_event_frame) {
> + struct lin_responder_answer *e_answ;
> +
> + e_answ = &priv->respond_answ[answ->event_associated_id];
> + n = min(e_answ->lf.len, LIN_MAX_DLEN);
> + if (memcmp(answ->lf.data, e_answ->lf.data, n) != 0) {
> + memcpy(answ->lf.data, e_answ->lf.data, n);
> + checksum = lin_get_checksum(LIN_FORM_PID(answ->lf.lin_id),
> + n, e_answ->lf.data,
> + answ->lf.checksum_mode);
> + answ = e_answ;
> + } else {
> + goto unlock_and_exit_false;
Can't you simply use guard(mutex) above and avoid the error-prone
gotos/cleanup completely?
> + }
> + } else {
> + checksum = answ->lf.checksum;
> + }
> +
> + count = min(answ->lf.len, LIN_MAX_DLEN);
> + memcpy(&buf[0], answ->lf.data, count);
> + buf[count] = checksum;
> +
> + mutex_unlock(&priv->resp_lock);
> +
> + write_len = serdev_device_write(serdev, buf, count + 1, 0);
> + if (write_len < count + 1)
> + return false;
> +
> + serdev_device_wait_until_sent(serdev, 0);
> +
> + return true;
> +
> +unlock_and_exit_false:
> + mutex_unlock(&priv->resp_lock);
> + return false;
> +}
> +
> +static void linser_pop_fifo(struct linser_priv *priv, size_t n)
> +{
> + struct serdev_device *serdev = priv->serdev;
> + struct linser_rx dummy;
> + size_t ret, i;
> +
> + for (i = 0; i < n; i++) {
> + ret = kfifo_out(&priv->rx_fifo, &dummy, 1);
Does kfifo_skip() not work for records? (I added it recently for serial.)
> + if (ret != 1) {
> + dev_err(&serdev->dev, "Failed to pop from FIFO\n");
> + break;
> + }
> + }
> +}
thanks,
--
js
suse labs
Powered by blists - more mailing lists