On Fri, Feb 28, 2020 at 05:55:44PM -0800, Guenter Roeck wrote: > On 2/28/20 3:52 PM, Grant Peltier wrote: > > Hi Guenter, > > > > Thank you for your expedient review. I will need to consult with my > > coworkers to determine a more appropriate driver name. In the meantime I > > will make the desired changes and I will also create a document for the > > driver, which I will submit as a linked but separate patch. > > > > With regard to the part numbers, this family of parts is currently in > > the process of being released and we have not yet published all of the > > corresponding datasheets. However, I have been assured that all of the > > parts listed are slated to have a datasheet published publicly in the near > > future. > > > That would be great. > > As for the driver name, I had a look into drivers/hwmon/pmbus/isl68137.c, > and I don't immediately see why the new chips would warrant a new driver. > The only differences seem to be that VMON is a new command, and of course > only the ISL68137 supports AVL. But then there is, for example, ISL68127, > which is again quite similar. The only other difference as far as I can > see is input voltage scaling, but that doesn't warrant a separate driver > (and, of course, I have no means to validate if input voltage scaling > is indeed different for all the new chips). > > Overall I would suggest to extend the isl68137 driver. I would also > suggest to not add separate tables for each of the rail configurations > but use the three-phase entry as starting point, copy it, and adjust its > values as needed. > > For the multi-phase chips, I question if reporting the input voltage > for each phase make sense. Is it really a different voltage ? For IIN > and PIN, the question is if the registers are indeed paged, since they > are not paged in the older chips. > > Guenter The ISL68137 is part of the first generation of our digital multiphase parts which are all exclusively 2-rail (2-page) devices. There are a couple of reasons that we are opting for a new driver for the new generation of devices: 1) Gen 2 has multiple rail configurations (1, 2, or 3) with different scaling parameters than Gen 1 2) We are planning to support some of the non-generic PMBus functions of the Gen 2 devices using the debugfs interface. I am currently working on point 2 and those features are not quite ready to be included in a patch set but we wanted to move forward with the hwmon functionality for now as that is useful on it's own. Fair point on the global vs paged commands. I will modify the page functions so that global commands are only read from page 0. Thank you, Grant
