On Fri, Apr 12, 2019 at 1:28 PM Guenter Roeck <linux@xxxxxxxxxxxx> wrote: > > On 4/12/19 12:43 PM, Patrick Venture wrote: > > From: Maxim Sloyko <maxims@xxxxxxxxxx> > > > > Add the pmbus driver for the Infineon ir38064 voltage regulator. > > > > Signed-off-by: Maxim Sloyko <maxims@xxxxxxxxxx> > > Signed-off-by: Patrick Venture <venture@xxxxxxxxxx> > > --- > > v2: > > - Cleaned up weird tabbing behavior in doc. > > v1: > > - Initial patchset. > > --- > > Documentation/hwmon/ir38064 | 64 +++++++++++++++++++++++++++++++++++ > > drivers/hwmon/pmbus/Kconfig | 9 +++++ > > drivers/hwmon/pmbus/Makefile | 1 + > > drivers/hwmon/pmbus/ir38064.c | 60 ++++++++++++++++++++++++++++++++ > > 4 files changed, 134 insertions(+) > > create mode 100644 Documentation/hwmon/ir38064 > > create mode 100644 drivers/hwmon/pmbus/ir38064.c > > > > diff --git a/Documentation/hwmon/ir38064 b/Documentation/hwmon/ir38064 > > new file mode 100644 > > index 0000000000000..618779a329da0 > > --- /dev/null > > +++ b/Documentation/hwmon/ir38064 > > @@ -0,0 +1,64 @@ > > +Kernel driver ir38064 > > +===================== > > + > > +Supported chips: > > + * Infineon IR38064 > > + Prefix: 'ir38064' > > + Addresses scanned: - > > + Datasheet: Publicly available at the Infineon webiste > > + https://www.infineon.com/dgdl/Infineon-IR38064MTRPBF-DS-v03_07-EN.pdf?fileId=5546d462584d1d4a0158db0d9efb67ca > > + > > +Datasheet is not publicly available. > > + > > + > > +Authors: > > + Maxim Sloyko <maxims@xxxxxxxxxx> > > + Patrick Venture <venture@xxxxxxxxxx> > > + > > +Description > > +----------- > > + > > +IR38064 is a Single-input Voltage, Synchronous Buck Regulator, DC-DC Converter. > > + > > +Usage Notes > > +----------- > > + > > +This driver does not probe for PMBus devices. You will have to instantiate > > +devices explicitly. > > + > > +Sysfs attributes > > +---------------- > > + > > +curr1_label "iout1" > > +curr1_input Measured output current > > +curr1_crit Critical maximum current > > +curr1_crit_alarm Current critical high alarm > > +curr1_max Maximum current > > +curr1_max_alarm Current high alarm > > + > > +in1_label "vin" > > +in1_input Measured input voltage > > +in1_crit Critical maximum input voltage > > +in1_crit_alarm Input voltage critical high alarm > > +in1_min Minimum input voltage > > +in1_min_alarm Input voltage low alarm > > + > > +in2_label "vout1" > > +in2_input Measured output voltage > > +in2_lcrit Critical minimum output voltage > > +in2_lcrit_alarm Output voltage critical low alarm > > +in2_crit Critical maximum output voltage > > +in2_crit_alarm Output voltage critical high alarm > > +in2_max Maximum output voltage > > +in2_max_alarm Output voltage high alarm > > +in2_min Minimum output voltage > > +in2_min_alarm Output voltage low alarm > > + > > +power1_label "pout1" > > +power1_input Measured output power > > + > > +temp1_input Measured temperature > > +temp1_crit Critical high temperature > > +temp1_crit_alarm Chip temperature critical high alarm > > +temp1_max Maximum temperature > > +temp1_max_alarm Chip temperature high alarm > > diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig > > index 629cb45f8557a..de87abab990e5 100644 > > --- a/drivers/hwmon/pmbus/Kconfig > > +++ b/drivers/hwmon/pmbus/Kconfig > > @@ -54,6 +54,15 @@ config SENSORS_IR35221 > > This driver can also be built as a module. If so, the module will > > be called ir35521. > > > > +config SENSORS_IR38064 > > + tristate "Infineon IR38064" > > + help > > + If you say yes here you get hardware monitoring support for Infineon > > + IR38064. > > + > > + This driver can also be built as a module. If so, the module will > > + be called ir38064. > > + > > config SENSORS_LM25066 > > tristate "National Semiconductor LM25066 and compatibles" > > help > > diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile > > index ea0e39518c216..7d1fa6b3c8c93 100644 > > --- a/drivers/hwmon/pmbus/Makefile > > +++ b/drivers/hwmon/pmbus/Makefile > > @@ -8,6 +8,7 @@ obj-$(CONFIG_SENSORS_PMBUS) += pmbus.o > > obj-$(CONFIG_SENSORS_ADM1275) += adm1275.o > > obj-$(CONFIG_SENSORS_IBM_CFFPS) += ibm-cffps.o > > obj-$(CONFIG_SENSORS_IR35221) += ir35221.o > > +obj-$(CONFIG_SENSORS_IR38064) += ir38064.o > > obj-$(CONFIG_SENSORS_LM25066) += lm25066.o > > obj-$(CONFIG_SENSORS_LTC2978) += ltc2978.o > > obj-$(CONFIG_SENSORS_LTC3815) += ltc3815.o > > diff --git a/drivers/hwmon/pmbus/ir38064.c b/drivers/hwmon/pmbus/ir38064.c > > new file mode 100644 > > index 0000000000000..4b957ebb6fa71 > > --- /dev/null > > +++ b/drivers/hwmon/pmbus/ir38064.c > > @@ -0,0 +1,60 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * Hardware monitoring driver for Infineon IR38064 > > + * > > + * Copyright (c) 2017 Google Inc > > + * > > + */ > > + > > +#include <linux/err.h> > > +#include <linux/i2c.h> > > +#include <linux/init.h> > > +#include <linux/kernel.h> > > +#include <linux/module.h> > > +#include "pmbus.h" > > + > > +static struct pmbus_driver_info ir38064_info = { > > + .pages = 1, > > + .format[PSC_VOLTAGE_IN] = linear, > > + .format[PSC_VOLTAGE_OUT] = direct, > > The datasheet says this is linear16 format with exponent=-8. > Oddly enough, the datasheet also suggests that VOUT_MODE is > not supported, which would normally report the exponent. > I think it would be is better to implement a local read function > and fake VOUT_MODE instead of declaring direct mode, but then > I understand that it would make the code more complex. I'll > leave it up to you, but if you want to keep it as-is, > please explain in a comment that you are faking linear16 > mode with exponent -8. Added to commit message in v3. > > > + .format[PSC_CURRENT_OUT] = linear, > > + .format[PSC_POWER] = linear, > > + .format[PSC_TEMPERATURE] = linear, > > + .m[PSC_VOLTAGE_OUT] = 256, > > + .b[PSC_VOLTAGE_OUT] = 0, > > + .R[PSC_VOLTAGE_OUT] = 0, > > + .func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_STATUS_INPUT > > + | PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP > > + | PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT > > + | PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT > > + | PMBUS_HAVE_POUT, > > +}; > > + > > Not mandatory, but the chip does support VOUT_PEAK, IOUT_PEAK, > and TEMPERATURE_PEAK, just in case you are interested adding > support for it. Added to commit message in v3. > > Thanks, > Guenter Thanks. > > > +static int ir38064_probe(struct i2c_client *client, > > + const struct i2c_device_id *id) > > +{ > > + return pmbus_do_probe(client, id, &ir38064_info); > > +} > > + > > +static const struct i2c_device_id ir38064_id[] = { > > + {"ir38064", 0}, > > + {} > > +}; > > + > > +MODULE_DEVICE_TABLE(i2c, ir38064_id); > > + > > +/* This is the driver that will be inserted */ > > +static struct i2c_driver ir38064_driver = { > > + .driver = { > > + .name = "ir38064", > > + }, > > + .probe = ir38064_probe, > > + .remove = pmbus_do_remove, > > + .id_table = ir38064_id, > > +}; > > + > > +module_i2c_driver(ir38064_driver); > > + > > +MODULE_AUTHOR("Maxim Sloyko <maxims@xxxxxxxxxx>"); > > +MODULE_DESCRIPTION("PMBus driver for Infineon IR38064"); > > +MODULE_LICENSE("GPL"); > > >