On Wed, Aug 24, 2022 at 6:04 PM <Tharunkumar.Pasumarthi@xxxxxxxxxxxxx> wrote: > On Tue, 2022-08-23 at 18:05 +0300, Andy Shevchenko wrote: > > On Tue, Aug 23, 2022 at 08:26:03PM +0530, Tharun Kumar P wrote: ... > > > + * Author: Tharun Kumar P <tharunkumar.pasumarthi@xxxxxxxxxxxxx> > > > + * Kumaravel Thiagarajan <kumaravel.thiagarajan@xxxxxxxxxxxxx> > > > > Single or many? > > There are 2 authors for this file Do you see the issue now? ... > > > +#define SMB_CORE_CTRL_ESO 0x40 > > > +#define SMB_CORE_CTRL_FW_ACK 0x10 > > > > Are they bits or numbers? > > These are bits Use BIT() then. Ditto for the rest of the bits. ... > > > +#define SMB_IDLE_SCALING_100KHZ 0x03E803C9 > > > +#define SMB_IDLE_SCALING_400KHZ 0x01F4009D > > > +#define SMB_IDLE_SCALING_1000KHZ 0x01F4009D > > > > Shouldn't these magics be decimals? This Q seems unanswered. > > Ditto for the rest similar stuff. > > There is no direct correlation between the hex value and time. Configuring > registers with these values in driver will set the time in device. ... > > > +#define I2C_DIR_WRITE 0 > > > +#define I2C_DIR_READ 1 https://elixir.bootlin.com/linux/v6.0-rc2/source/include/uapi/linux/i2c.h#L24 > > Namespace collision. Doesn't I²C core provide these? > > I am unable to find any existing MACROs for WRITE and READ in I2C core. Kindly > let me know the MACROs ... > > > +#define PCI1XXXX_I2C_TIMEOUT 1000 > > > > Units? Same to the rest similar cases. > > Unit is milliseconds So you know what to do, right? ... > > > +#define SMBUS_PERI_LOCK BIT(3) > > > > BIT() out of a sudden. See above. > > Will use hex value for this like in other places to maintain uniformity See above. ... > > > + pci1xxxx_i2c_shutdown(i2c); > > > > Do you really need this in ->remove()? I would expect something in > > the ->suspend() / ->shutdown(). > > pci1xxxx_i2c_shutdown API will reset the registers that are set as part of > pci1xxxx_i2c_init. So, this API is present in ->remove() and not in ->suspend() > callback I understand that, but it doesn't really answer my question. -- With Best Regards, Andy Shevchenko
