On Thu, Jun 30, 2022 at 09:41:37AM +0000, Phil Edworthy wrote: > On 28 June 2022 22:08 Andy Shevchenko wrote: > > On Tue, Jun 28, 2022 at 08:45:26PM +0100, Phil Edworthy wrote: ... > > > + if (ret == 0) { > > > > This is a bit confusing if it's only comparison with "no error code" > > condition. Use if (!ret) if there is no meaning of positive value. > > Same applies to other cases in the code. ^^^ > > > + if (read) > > > + ret = rzv2m_i2c_receive(priv, msg, &count); > > > + else > > > + ret = rzv2m_i2c_send(priv, msg, &count); > > > + > > > + if ((ret == 0) && stop) > > > > Like here. > Sorry, I don't follow you. How is (ret == 0) any different to (!ret) ? See above. ... > > > + adap->dev.of_node = dev->of_node; > > > > device_set_node() > Since we don't need to set dev->fwnode, why is device_set_node() any > better? We may go for I2C subsystem that supports only fwnode (as others already or almost about to do). This will reduce a lot of unneeded churn in the future. -- With Best Regards, Andy Shevchenko