On Sun, Aug 24, 2014 at 08:30:16AM -0500, Alan Tull wrote: > On Sat, Aug 23, 2014 at 10:10 AM, Guenter Roeck <linux@xxxxxxxxxxxx> wrote: > > On 08/22/2014 02:45 PM, Mark Brown wrote: > >> > >> On Fri, Aug 22, 2014 at 04:11:34PM -0500, atull@xxxxxxxxxxxxxxxxxxxxx > >> wrote: > >>> > >>> From: Alan Tull <atull@xxxxxxxxxxxxxxxxxxxxx> > >>> > >>> Add regulator with support for enabling or disabling all > >>> supplies. > >> > >> > >> Reviwed-by: Mark Brown <broonie@xxxxxxxxxx> > >> > >> though it still looks like you should be able to create generic > >> functions for the operations. > >> > > Sorry I didn't have time to review the code myself. I'll have > > to check the datasheet about turning regulators on and off. > > Using page 0xff for the lm2978 looks wrong, as the chip supports > > up to 8 channels which should be controlled separately > > (I would assume) instead of turning them all on and off in > > one go. Maybe I am missing something, but my assumption would > > have been to have a separate regulator for each channel, and > > that each channel would have its own regulator which would be > > turned on and off separately. So I don't really understand the > > change between v1 and v2 of the core patch, which dropped the > > per-channel regulators. Someone will have to explain to me > > why that makes sense, especially since it means that I won't > > be able to use the regulator expansion in my system (which > > would require per-channel regulators, and which does not always > > have all channels enabled on a given chip). > > The LTC2978 spec says that the OPERATION command register "responds to > the global page command (PAGE=0xFF)." I originally took it to mean > that it *only* responds to 0xFF. Now I get that yes I could turn > on/off each of 8 channels separately. > > I will make the change have one regulator for each supply. It would > be useful to still have a global regulator for turning them all on/off > together if that is not completely odious. > > > > > In respect to generic functions, that really depends on the scope > > of the regulators. As written, where all regulators are turned on > > in a single operation, per-chip functions are needed. I thought > > we would only need per-chip configuration values, but that only > > applies if regulators are turned on one by one, not all in one go. > > For all the parts supported by ltc2978.c, a banked write of 0x80 to > the OPERATION register turns a regulator on, writing 0x00 turns it > off. I'm not sure how universal that is for other PMBUS parts. I > will look at the PMBUS specs when I get back to the office Tuesday. > It isn't. Other parts need a different value to be written for on/off. However, that does not mean we need a chip specific _function_ to write the values. All we should need is the values to write for on/off. > I'll look into it and see if I can push almost all of this code into > pmbus_core.c and just have per-chip config values in > pmbus_driver_info. If many PMBUS parts have the same scheme (0x80 or > 0x00 written to OPERATION register as a banked write) then a flag bit > that is turned on in pmbus_driver_info could enable this scheme for > this chip. If it is a different register or different on/off values, > then I'll need to add those to pmbus_driver_info. Values should work, and the writes should all be banked, but a flag is unfortunately insufficient. You should include a per-page flag to enable the functionality (something like PMBUS_HAVE_REGULATOR). Some PMBus chips don't have regulators on all pages. > > > > Either case, a wrapper for ltc2978_write_pmbus_operation needs to > > be added to pmbus_core.c as pmbus_write_byte_data and exported > > (as a separate patch). For paged reads, the existing > > pmbus_read_byte_data should be used. In general, avoid direct > > accesses to paged registers and use API functions instead > > if possible. > > OK. Will add pmbus_write_byte_data as a separate patch and use the > existing pmbus_read_byte_data. > If you use per-page flags and values, you should not need those at all. Thanks, Guenter _______________________________________________ lm-sensors mailing list lm-sensors@xxxxxxxxxxxxxx http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
