On Fri, 16 Aug 2019 at 10:19, John Wang <wangzqbj@xxxxxxxxxx> wrote: > > Add the driver to monitor Inspur Power System power supplies > with hwmon over pmbus. > > This driver adds sysfs attributes for additional power supply data, > including vendor, model, part_number, serial number, > firmware revision, hardware revision, and psu mode(active/standby). > > Signed-off-by: John Wang <wangzqbj@xxxxxxxxxx> > +static const struct i2c_device_id ipsps_id[] = { > + { "inspur_ipsps1", 0 }, Convention would be to use "ipsps" here, instead of "vendor_device"? > + {} > +}; > +MODULE_DEVICE_TABLE(i2c, ipsps_id); > + > +static const struct of_device_id ipsps_of_match[] = { > + { .compatible = "inspur,ipsps1" }, > + {} > +}; > +MODULE_DEVICE_TABLE(of, ipsps_of_match); Do we need the of match table? I thought the match on the device name from the i2c table would be enough. I will defer to Guenter here though. Assuming the device tables are okay: Reviewed-by: Joel Stanley <joel@xxxxxxxxx> Cheers, Joel > + > +static struct i2c_driver ipsps_driver = { > + .driver = { > + .name = "inspur-ipsps", > + .of_match_table = ipsps_of_match, > + }, > + .probe = ipsps_probe, > + .remove = pmbus_do_remove, > + .id_table = ipsps_id, > +};