netdev - Re: [PATCH v3 02/11] HID: hexLIN: Add support for USB LIN bus adapter

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] [day] [month] [year] [list]

Message-ID: <f558341c-288a-533c-1fcc-6c18f68e311e@linux.intel.com>
Date: Fri, 10 May 2024 12:31:31 +0300 (EEST)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Christoph Fritz <christoph.fritz@...dev.de>
cc: Jiri Slaby <jirislaby@...nel.org>, 
    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>, 
    Andreas Lauser <andreas.lauser@...cedes-benz.com>, 
    Jonathan Corbet <corbet@....net>, Pavel Pisa <pisa@....felk.cvut.cz>, 
    linux-can@...r.kernel.org, Netdev <netdev@...r.kernel.org>, 
    devicetree@...r.kernel.org, linux-input@...r.kernel.org, 
    linux-serial <linux-serial@...r.kernel.org>
Subject: Re: [PATCH v3 02/11] HID: hexLIN: Add support for USB LIN bus
 adapter

On Thu, 9 May 2024, Christoph Fritz wrote:

> On Mon, 2024-05-06 at 19:53 +0300, Ilpo Järvinen wrote:
> > On Thu, 2 May 2024, Christoph Fritz wrote:
> > 
> > > This patch introduces driver support for the hexLIN USB LIN bus adapter,
> > > enabling LIN communication over USB for both controller and responder
> > > modes. The driver interfaces with the CAN_LIN framework for userland
> > > connectivity.
> > > 
> > > For more details on the adapter, visit: https://hexdev.de/hexlin/
> > > 
> > > Tested-by: Andreas Lauser <andreas.lauser@...cedes-benz.com>
> > > Signed-off-by: Christoph Fritz <christoph.fritz@...dev.de>
> > > ---

> > > +	le32_to_cpus(hxf.flags);
> > 
> > You must use correct endianess typing. The struct hexlin_frame should have 
> > __le32 flags so sparse's endianness check is happy.
> 
> OK
> 
> > 
> > But .flags are not used at all so why is this required in the first place?
> 
> Was necessary in the development process and will be used in hid_dbg()
> below in v4.

Ok, I was expecting you'd print it out there but since it wasn't, I made 
the unused comment.

BTW, you don't need to reply "OK" to me for the review comments which 
you're going to do in the next version. I trust you'll address those 
comments which are not replied into. It saves us both time :-).

> > > +	lf.len = hxf.len;
> > > +	lf.lin_id = hxf.lin_id;
> > > +	memcpy(lf.data, hxf.data, LIN_MAX_DLEN);
> > > +	lf.checksum = hxf.checksum;
> > > +	lf.checksum_mode = hxf.checksum_mode;
> > > +
> > > +	hid_dbg(hdev, "id:%02x, len:%u, data:%*ph, checksum:%02x (%s)\n",
> > > +		   lf.lin_id, lf.len, lf.len, lf.data, lf.checksum,
> > > +		   lf.checksum_mode ? "enhanced" : "classic");
> > > +
> > > +	lin_rx(priv->ldev, &lf);
> > > +
> > > +	return 0;
> > > +}


-- 
 i.