Hi Brown, Thanks for your review > -----Original Message----- > From: Mark Brown <broonie@xxxxxxxxxx> > Sent: Friday, May 12, 2023 12:18 PM > > On Fri, May 12, 2023 at 01:58:44AM +0800, Ye Xiang wrote: > > > +++ b/drivers/spi/spi-ljca.c > > @@ -0,0 +1,290 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * Intel La Jolla Cove Adapter USB-SPI driver > > Please make the entire comment a C++ one so things look more intentional. > I see lots of drivers are commenting like current one. But sorry, you mean the entire comment start with /* and end with */ ? Thanks BR, Wentong > > +struct spi_init_packet { > > + u8 index; > > + u8 speed; > > + u8 mode; > > +} __packed; > > + > > +struct spi_xfer_packet { > > These should be namespaced, especially since they look likely to collide with > other things. Otherwise this looks fine. Ack, these two structs will be like below in next version, thanks struct ljca_spi_init_packet { u8 index; u8 index; u8 mode; } __packed; struct ljca_spi_xfer_packet { u8 indicator; s8 len; u8 data[]; } __packed;