| Message ID | 20230511175844.185070-1-xiang.ye@intel.com |
|---|---|
| Headers | show |
| Series | Add Intel LJCA device driver | expand |
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. > +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.
On Fri, May 12, 2023 at 01:58:38AM +0800, Ye Xiang wrote: > Add driver for Intel La Jolla Cove Adapter (LJCA) device. > This is a USB-GPIO, USB-I2C and USB-SPI device. We add 4 > drivers to support this device: a USB driver, a GPIO chip > driver, a I2C controller driver and a SPI controller driver. I am sorry, but you have not followed the required Intel-specific requirements for submitting code like this. Please work with the Linux Intel developer group to resolve this issue and do it properly for your next patch submission as I can not take this one for this obvious reason. thanks, greg k-h
Hi Brown, Thanks for your review > -----Original Message----- > From: Mark Brown <broonie@kernel.org> > 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;
Hi Greg, Thanks for your review. BTW, somehow I will take care this patch set. Thanks Wentong > -----Original Message----- > From: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Sent: Saturday, May 13, 2023 4:50 PM > > On Fri, May 12, 2023 at 01:58:38AM +0800, Ye Xiang wrote: > > Add driver for Intel La Jolla Cove Adapter (LJCA) device. > > This is a USB-GPIO, USB-I2C and USB-SPI device. We add 4 drivers to > > support this device: a USB driver, a GPIO chip driver, a I2C > > controller driver and a SPI controller driver. > > I am sorry, but you have not followed the required Intel-specific requirements > for submitting code like this. Please work with the Linux Intel developer group > to resolve this issue and do it properly for your next patch submission as I can > not take this one for this obvious reason. > > thanks, > > greg k-h
On Tue, Jun 20, 2023 at 06:47:11AM +0000, Wu, Wentong wrote: > > > +// 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 Other way around, use // for all the lines in this comment block please.