On 30/07/2020 08:43:03+0200, Greg KH wrote: > > > > > + s32 vendor, product; > > > + > > > + vendor = i2c_smbus_read_word_data(isp1301_i2c_client, 0x00); > > > + product = i2c_smbus_read_word_data(isp1301_i2c_client, 0x02); > > Why are these signed 32bit numbers? Shouldn't they be unsigned? Because i2c_smbus_read_word_data returns an s32 and should be checked for errors but because the whole driver is never checking, I'll leave that as an exercise for outreachy interns. > > > + > > > + if (vendor == 0x0483 && product == 0xa0c4) > > No endian flips anywhere? > The whole driver makes the assumption that it will only run on lpc32xx with an isp1301. I don't believe we will ever see an other platform using it. -- Alexandre Belloni, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
