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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxxxxx>
> > > Signed-off-by: Christoph Fritz <christoph.fritz@xxxxxxxxx>
> > > ---

> > > +	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.

[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux