| Message ID | 20231109014948.2334465-3-Delphine_CC_Chiu@Wiwynn.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | LTC4286 and LTC4287 driver support | expand |
On 11/8/23 17:49, Delphine CC Chiu wrote: > Add a driver to support ltc4286 chip > > Signed-off-by: Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com> > > Changelog: > v4 - Add empty line before "config SENSORS_LTC4286" in Kconfig > - Add ltc4286 to Documentation/hwmon/index.rst > - Revise comment typo > - Use devm_kmemdup instead of memcpy > - Check MBR value before writting into > v3 - Use dev_err_probe() instead of dev_err() > - The VRANGE_SELECT bit only be written if it actually changed > - Avoid the info pointer being overwritten > - Check the MBR value range to avoid overflow > - Revise ltc4286.rst to corrcet description > v2 - Revise Linear Technologies LTC4286 to > Analog Devices LTC4286 in Kconfig > - Add more description for this driver in Kconfig > - Add some comments for MBR setting in ltc4286.c > - Add ltc4286.rst > --- > Documentation/hwmon/index.rst | 1 + > Documentation/hwmon/ltc4286.rst | 95 +++++++++++++++++ > drivers/hwmon/pmbus/Kconfig | 10 ++ > drivers/hwmon/pmbus/Makefile | 1 + > drivers/hwmon/pmbus/ltc4286.c | 177 ++++++++++++++++++++++++++++++++ > 5 files changed, 284 insertions(+) > create mode 100644 Documentation/hwmon/ltc4286.rst > create mode 100644 drivers/hwmon/pmbus/ltc4286.c > > diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst > index 72f4e6065bae..080827cc4c34 100644 > --- a/Documentation/hwmon/index.rst > +++ b/Documentation/hwmon/index.rst > @@ -128,6 +128,7 @@ Hardware Monitoring Kernel Drivers > ltc4245 > ltc4260 > ltc4261 > + ltc4286 > max127 > max15301 > max16064 > diff --git a/Documentation/hwmon/ltc4286.rst b/Documentation/hwmon/ltc4286.rst > new file mode 100644 > index 000000000000..2cd149676d86 > --- /dev/null > +++ b/Documentation/hwmon/ltc4286.rst > @@ -0,0 +1,95 @@ > +.. SPDX-License-Identifier: GPL-2.0-or-later > + > +Kernel driver ltc4286 > +===================== > + > +Supported chips: > + > + * Analog Devices LTC4286 > + > + Prefix: 'ltc4286' > + > + Addresses scanned: - > + > + Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ltc4286.pdf > + > + * Analog Devices LTC4287 > + > + Prefix: 'ltc4287' > + > + Addresses scanned: - > + > + Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ltc4287.pdf > + > +Author: Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com> > + > + > +Description > +----------- > + > +This driver supports hardware monitoring for Analog Devices LTC4286 > +and LTC4287 Hot-Swap Controller and Digital Power Monitors. > + > +LTC4286 and LTC4287 are hot-swap controllers that allow a circuit board > +to be removed from or inserted into a live backplane. They also feature > +current and voltage readback via an integrated 12 bit analog-to-digital > +converter (ADC), accessed using a PMBus interface. > + > +The driver is a client driver to the core PMBus driver. Please see > +Documentation/hwmon/pmbus.rst for details on PMBus client drivers. > + > + > +Usage Notes > +----------- > + > +This driver does not auto-detect devices. You will have to instantiate the > +devices explicitly. Please see Documentation/i2c/instantiating-devices.rst for > +details. > + > +The shunt value in micro-ohms can be set via device tree at compile-time. Please > +refer to the Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml for bindings > +if the device tree is used. > + > + > +Platform data support > +--------------------- > + > +The driver supports standard PMBus driver platform data. Please see > +Documentation/hwmon/pmbus.rst for details. > + > + > +Sysfs entries > +------------- > + > +The following attributes are supported. Limits are read-write, history reset > +attributes are write-only, all other attributes are read-only. > + > +======================= ======================================================= > +in1_label "vin" > +in1_input Measured voltage. > +in1_alarm Input voltage alarm. > +in1_min Minimum input voltage. > +in1_max Maximum input voltage. > + > +in2_label "vout1" > +in2_input Measured voltage. > +in2_alarm Output voltage alarm. > +in2_min Minimum output voltage. > +in2_max Maximum output voltage. > + > +curr1_label "iout1" > +curr1_input Measured current. > +curr1_alarm Output current alarm. > +curr1_max Maximum current. > + > +power1_label "pin" > +power1_input Input power. > +power1_alarm Input power alarm. > +power1_max Maximum poewr. > + > +temp1_input Chip temperature. > +temp1_min Minimum chip temperature. > +temp1_max Maximum chip temperature. > +temp1_crit Critical chip temperature. > +temp1_alarm Chip temperature alarm. > +======================= ======================================================= > diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig > index b4e93bd5835e..2d4f972e5a65 100644 > --- a/drivers/hwmon/pmbus/Kconfig > +++ b/drivers/hwmon/pmbus/Kconfig > @@ -227,6 +227,16 @@ config SENSORS_LTC3815 > This driver can also be built as a module. If so, the module will > be called ltc3815. > > +config SENSORS_LTC4286 > + bool "Analog Devices LTC4286" > + help > + LTC4286 is an integrated solution for hot swap applications that > + allows a board to be safely inserted and removed from a live > + backplane. > + This chip could be used to monitor voltage, current, ...etc. > + If you say yes here you get hardware monitoring support for Analog > + Devices LTC4286. > + > config SENSORS_MAX15301 > tristate "Maxim MAX15301" > help > diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile > index 84ee960a6c2d..94e28f6d6a61 100644 > --- a/drivers/hwmon/pmbus/Makefile > +++ b/drivers/hwmon/pmbus/Makefile > @@ -24,6 +24,7 @@ obj-$(CONFIG_SENSORS_LM25066) += lm25066.o > obj-$(CONFIG_SENSORS_LT7182S) += lt7182s.o > obj-$(CONFIG_SENSORS_LTC2978) += ltc2978.o > obj-$(CONFIG_SENSORS_LTC3815) += ltc3815.o > +obj-$(CONFIG_SENSORS_LTC4286) += ltc4286.o > obj-$(CONFIG_SENSORS_MAX15301) += max15301.o > obj-$(CONFIG_SENSORS_MAX16064) += max16064.o > obj-$(CONFIG_SENSORS_MAX16601) += max16601.o > diff --git a/drivers/hwmon/pmbus/ltc4286.c b/drivers/hwmon/pmbus/ltc4286.c > new file mode 100644 > index 000000000000..e6690b38349a > --- /dev/null > +++ b/drivers/hwmon/pmbus/ltc4286.c > @@ -0,0 +1,177 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > + > +#include <linux/err.h> > +#include <linux/i2c.h> > +#include <linux/init.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/pmbus.h> > +#include "pmbus.h" > + > +/* LTC4286 register */ > +#define LTC4286_MFR_CONFIG1 0xF2 > + > +/* LTC4286 configuration */ > +#define VRANGE_SELECT_BIT BIT(1) > + > +#define LTC4286_MFR_ID_SIZE 3 > +#define VRANGE_25P6 0 > + > +/* > + * Initialize the MBR as default settings which is referred to LTC4286 datasheet > + * (March 22, 2022 version) table 3 page 16 > + */ > +static struct pmbus_driver_info ltc4286_info = { > + .pages = 1, > + .format[PSC_VOLTAGE_IN] = direct, > + .format[PSC_VOLTAGE_OUT] = direct, > + .format[PSC_CURRENT_OUT] = direct, > + .format[PSC_POWER] = direct, > + .format[PSC_TEMPERATURE] = direct, > + .m[PSC_VOLTAGE_IN] = 32, > + .b[PSC_VOLTAGE_IN] = 0, > + .R[PSC_VOLTAGE_IN] = 1, > + .m[PSC_VOLTAGE_OUT] = 32, > + .b[PSC_VOLTAGE_OUT] = 0, > + .R[PSC_VOLTAGE_OUT] = 1, > + .m[PSC_CURRENT_OUT] = 1024, > + .b[PSC_CURRENT_OUT] = 0, > + /* > + * The rsense value used in MBR formula in LTC4286 datasheet should be ohm unit. > + * However, the rsense value that user input is micro ohm. > + * Thus, the MBR setting which involves rsense should be shifted by 6 digits. > + */ > + .R[PSC_CURRENT_OUT] = 3 - 6, > + .m[PSC_POWER] = 1, > + .b[PSC_POWER] = 0, > + /* > + * The rsense value used in MBR formula in LTC4286 datasheet should be ohm unit. > + * However, the rsense value that user input is micro ohm. > + * Thus, the MBR setting which involves rsense should be shifted by 6 digits. > + */ > + .R[PSC_POWER] = 4 - 6, > + .m[PSC_TEMPERATURE] = 1, > + .b[PSC_TEMPERATURE] = 273, > + .R[PSC_TEMPERATURE] = 0, > + .func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT | PMBUS_HAVE_IOUT | > + PMBUS_HAVE_PIN | PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_VOUT | > + PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_STATUS_TEMP, > +}; > + > +static const struct i2c_device_id ltc4286_id[] = { { "ltc4286", 0 }, > + { "ltc4287", 1 }, > + {} }; Please fix that formatting and use the more common style. static const struct i2c_device_id ltc4286_id[] = { { "ltc4286", 0 }, { "ltc4287", 1 }, {} }; > +MODULE_DEVICE_TABLE(i2c, ltc4286_id); > + > +static int ltc4286_probe(struct i2c_client *client) > +{ > + int ret; > + int temp_setting; > + const struct i2c_device_id *mid; > + u8 block_buffer[I2C_SMBUS_BLOCK_MAX + 1]; > + struct pmbus_driver_info *info; > + u32 rsense; > + > + ret = i2c_smbus_read_block_data(client, PMBUS_MFR_ID, block_buffer); > + if (ret < 0) { > + return dev_err_probe(&client->dev, ret, > + "Failed to read manufacturer id\n"); > + } > + > + /* > + * Refer to ltc4286 datasheet page 20 > + * the manufacturer id is LTC > + */ > + if (ret != LTC4286_MFR_ID_SIZE || > + strncmp(block_buffer, "LTC", LTC4286_MFR_ID_SIZE)) { > + return dev_err_probe(&client->dev, ret, > + "Manufacturer id mismatch\n"); > + } > + > + ret = i2c_smbus_read_block_data(client, PMBUS_MFR_MODEL, block_buffer); > + if (ret < 0) { > + return dev_err_probe(&client->dev, ret, > + "Failed to read manufacturer model\n"); > + } > + > + for (mid = ltc4286_id; mid->name[0]; mid++) { > + if (!strncasecmp(mid->name, block_buffer, strlen(mid->name))) > + break; > + } > + if (!mid->name[0]) > + return dev_err_probe(&client->dev, -ENODEV, > + "Unsupported device\n"); > + > + ret = of_property_read_u32(client->dev.of_node, > + "shunt-resistor-micro-ohms", &rsense); > + if (ret < 0) > + return ret; > + > + if (rsense == 0) > + return -EINVAL; > + > + if (rsense > INT_MAX) > + return -ERANGE; > + This is not "Math result not representable". See below. > + info = devm_kmemdup(&client->dev, <c4286_info, sizeof(*info), > + GFP_KERNEL); > + if (!info) > + return -ENOMEM; > + > + /* Default of VRANGE_SELECT = 1, 102.4V */ > + if (device_property_read_bool(&client->dev, "adi,vrange-low-enable")) { > + /* Setup MFR1 CONFIG register bit 1 VRANGE_SELECT */ > + ret = i2c_smbus_read_word_data(client, LTC4286_MFR_CONFIG1); > + if (ret < 0) > + return dev_err_probe( > + &client->dev, ret, > + "Failed to read manufacturer configuration one\n"); > + > + ret &= ~VRANGE_SELECT_BIT; /* VRANGE_SELECT = 0, 25.6V */ > + ret = i2c_smbus_write_word_data(client, LTC4286_MFR_CONFIG1, > + ret); > + if (ret < 0) > + return dev_err_probe(&client->dev, ret, > + "Failed to set vrange\n"); > + > + info->m[PSC_VOLTAGE_IN] = 128; > + info->m[PSC_VOLTAGE_OUT] = 128; > + > + temp_setting = 4 * rsense; > + if (temp_setting > INT_MAX) This will still overflow for values of rsense larger than INT_MAX / 4. temp_setting is an int, and its value will never be > INT_MAX. > + return dev_err_probe(&client->dev, -ERANGE, > + "Power coefficient overflow\n"); ERANGE is "Math result not representable", which is not the case here. This (and the above above) error needs to be -EINVAL. > + info->m[PSC_POWER] = temp_setting; > + } else { > + info->m[PSC_POWER] = rsense; I told you before, the default range needs to be set. The range may have been changed by the BIOS/ROMMON, or someone could have changed it manually with i2cset or some other application, or some other operating system was loaded earlier which did its own setting. I do understand by now that you don't want to do that, but I won't accept the driver without it, sorry. > + } > + > + temp_setting = 1024 * rsense; > + if (temp_setting > INT_MAX) > + return dev_err_probe(&client->dev, -ERANGE, > + "Current coefficient overflow\n"); Same comments as above for both overflow detection and error code. This also means that rsense must _always_ be <= INT_MAX / 1024. I would suggest to check that once after reading rsense instead of having three different checks in different places. Also, having an error message here but not with the initial check against 0 and INT_MAX is inconsistent. I'd suggest no error message, but if you want to have error messages please make it consistent. > + info->m[PSC_CURRENT_OUT] = temp_setting; > + > + return pmbus_do_probe(client, info); > +} > + > +static const struct of_device_id ltc4286_of_match[] = { > + { .compatible = "lltc,ltc4286" }, > + { .compatible = "lltc,ltc4287" }, > + {} > +}; > + > +static struct i2c_driver ltc4286_driver = { > + .driver = { > + .name = "ltc4286", > + .of_match_table = ltc4286_of_match, > + }, > + .probe = ltc4286_probe, > + .id_table = ltc4286_id, > +}; > + > +module_i2c_driver(ltc4286_driver); > + > +MODULE_AUTHOR("Delphine CC Chiu <Delphine_CC_Chiu@wiwynn.com>"); > +MODULE_DESCRIPTION("PMBUS driver for LTC4286 and compatibles"); > +MODULE_LICENSE("GPL");
On 11/9/23 09:27, Guenter Roeck wrote: > On 11/8/23 17:49, Delphine CC Chiu wrote: >> + info->m[PSC_POWER] = temp_setting; >> + } else { >> + info->m[PSC_POWER] = rsense; > > I told you before, the default range needs to be set. The range may have been > changed by the BIOS/ROMMON, or someone could have changed it manually with > i2cset or some other application, or some other operating system was loaded > earlier which did its own setting. > > I do understand by now that you don't want to do that, but I won't accept > the driver without it, sorry. > As a follow-up on this, it turns out that LTC4287 supports an external EEPROM to store its configuration as well as "STORE_USER_ALL" and "RESTORE_USER_ALL" PMBus commands. It is therefore very well possible and even likely that the voltage range (together with other configurable parameters) was set during system production. With that in mind, it seems to me that not only the default (property not present) case has to be handled, but that a boolean property to reflect the voltage range is insufficient. I think what is needed instead is a property which specifies the requested range and which, if not present, explicitly means "use whatever is currently programmed into the chip". Thanks, Guenter
> -----Original Message----- > From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck > Sent: Friday, November 10, 2023 1:28 AM > To: Delphine_CC_Chiu/WYHQ/Wiwynn <Delphine_CC_Chiu@wiwynn.com>; > patrick@stwcx.xyz; Jean Delvare <jdelvare@suse.com>; Jonathan Corbet > <corbet@lwn.net> > Cc: Rob Herring <robh+dt@kernel.org>; Krzysztof Kozlowski > <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley <conor+dt@kernel.org>; > linux-i2c@vger.kernel.org; linux-hwmon@vger.kernel.org; > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; > linux-doc@vger.kernel.org > Subject: Re: [PATCH v4 2/2] hwmon: pmbus: Add ltc4286 driver > > Security Reminder: Please be aware that this email is sent by an external > sender. > > On 11/8/23 17:49, Delphine CC Chiu wrote: > > Add a driver to support ltc4286 chip > > > > Signed-off-by: Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com> > > > > Changelog: > > v4 - Add empty line before "config SENSORS_LTC4286" in Kconfig > > - Add ltc4286 to Documentation/hwmon/index.rst > > - Revise comment typo > > - Use devm_kmemdup instead of memcpy > > - Check MBR value before writting into > > v3 - Use dev_err_probe() instead of dev_err() > > - The VRANGE_SELECT bit only be written if it actually changed > > - Avoid the info pointer being overwritten > > - Check the MBR value range to avoid overflow > > - Revise ltc4286.rst to corrcet description > > v2 - Revise Linear Technologies LTC4286 to > > Analog Devices LTC4286 in Kconfig > > - Add more description for this driver in Kconfig > > - Add some comments for MBR setting in ltc4286.c > > - Add ltc4286.rst > > --- > > Documentation/hwmon/index.rst | 1 + > > Documentation/hwmon/ltc4286.rst | 95 +++++++++++++++++ > > drivers/hwmon/pmbus/Kconfig | 10 ++ > > drivers/hwmon/pmbus/Makefile | 1 + > > drivers/hwmon/pmbus/ltc4286.c | 177 > ++++++++++++++++++++++++++++++++ > > 5 files changed, 284 insertions(+) > > create mode 100644 Documentation/hwmon/ltc4286.rst > > create mode 100644 drivers/hwmon/pmbus/ltc4286.c > > > > diff --git a/Documentation/hwmon/index.rst > > b/Documentation/hwmon/index.rst index 72f4e6065bae..080827cc4c34 > > 100644 > > --- a/Documentation/hwmon/index.rst > > +++ b/Documentation/hwmon/index.rst > > @@ -128,6 +128,7 @@ Hardware Monitoring Kernel Drivers > > ltc4245 > > ltc4260 > > ltc4261 > > + ltc4286 > > max127 > > max15301 > > max16064 > > diff --git a/Documentation/hwmon/ltc4286.rst > > b/Documentation/hwmon/ltc4286.rst new file mode 100644 index > > 000000000000..2cd149676d86 > > --- /dev/null > > +++ b/Documentation/hwmon/ltc4286.rst > > @@ -0,0 +1,95 @@ > > +.. SPDX-License-Identifier: GPL-2.0-or-later > > + > > +Kernel driver ltc4286 > > +===================== > > + > > +Supported chips: > > + > > + * Analog Devices LTC4286 > > + > > + Prefix: 'ltc4286' > > + > > + Addresses scanned: - > > + > > + Datasheet: > > + https://www.analog.com/media/en/technical-documentation/data-sheets/ > > + ltc4286.pdf > > + > > + * Analog Devices LTC4287 > > + > > + Prefix: 'ltc4287' > > + > > + Addresses scanned: - > > + > > + Datasheet: > > + https://www.analog.com/media/en/technical-documentation/data-sheets/ > > + ltc4287.pdf > > + > > +Author: Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com> > > + > > + > > +Description > > +----------- > > + > > +This driver supports hardware monitoring for Analog Devices LTC4286 > > +and LTC4287 Hot-Swap Controller and Digital Power Monitors. > > + > > +LTC4286 and LTC4287 are hot-swap controllers that allow a circuit > > +board to be removed from or inserted into a live backplane. They also > > +feature current and voltage readback via an integrated 12 bit > > +analog-to-digital converter (ADC), accessed using a PMBus interface. > > + > > +The driver is a client driver to the core PMBus driver. Please see > > +Documentation/hwmon/pmbus.rst for details on PMBus client drivers. > > + > > + > > +Usage Notes > > +----------- > > + > > +This driver does not auto-detect devices. You will have to > > +instantiate the devices explicitly. Please see > > +Documentation/i2c/instantiating-devices.rst for details. > > + > > +The shunt value in micro-ohms can be set via device tree at > > +compile-time. Please refer to the > > +Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml for bindings > if the device tree is used. > > + > > + > > +Platform data support > > +--------------------- > > + > > +The driver supports standard PMBus driver platform data. Please see > > +Documentation/hwmon/pmbus.rst for details. > > + > > + > > +Sysfs entries > > +------------- > > + > > +The following attributes are supported. Limits are read-write, > > +history reset attributes are write-only, all other attributes are read-only. > > + > > +======================= > ======================================================= > > +in1_label "vin" > > +in1_input Measured voltage. > > +in1_alarm Input voltage alarm. > > +in1_min Minimum input voltage. > > +in1_max Maximum input voltage. > > + > > +in2_label "vout1" > > +in2_input Measured voltage. > > +in2_alarm Output voltage alarm. > > +in2_min Minimum output voltage. > > +in2_max Maximum output voltage. > > + > > +curr1_label "iout1" > > +curr1_input Measured current. > > +curr1_alarm Output current alarm. > > +curr1_max Maximum current. > > + > > +power1_label "pin" > > +power1_input Input power. > > +power1_alarm Input power alarm. > > +power1_max Maximum poewr. > > + > > +temp1_input Chip temperature. > > +temp1_min Minimum chip temperature. > > +temp1_max Maximum chip temperature. > > +temp1_crit Critical chip temperature. > > +temp1_alarm Chip temperature alarm. > > +======================= > > +======================================================= > > diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig > > index b4e93bd5835e..2d4f972e5a65 100644 > > --- a/drivers/hwmon/pmbus/Kconfig > > +++ b/drivers/hwmon/pmbus/Kconfig > > @@ -227,6 +227,16 @@ config SENSORS_LTC3815 > > This driver can also be built as a module. If so, the module will > > be called ltc3815. > > > > +config SENSORS_LTC4286 > > + bool "Analog Devices LTC4286" > > + help > > + LTC4286 is an integrated solution for hot swap applications that > > + allows a board to be safely inserted and removed from a live > > + backplane. > > + This chip could be used to monitor voltage, current, ...etc. > > + If you say yes here you get hardware monitoring support for Analog > > + Devices LTC4286. > > + > > config SENSORS_MAX15301 > > tristate "Maxim MAX15301" > > help > > diff --git a/drivers/hwmon/pmbus/Makefile > > b/drivers/hwmon/pmbus/Makefile index 84ee960a6c2d..94e28f6d6a61 > 100644 > > --- a/drivers/hwmon/pmbus/Makefile > > +++ b/drivers/hwmon/pmbus/Makefile > > @@ -24,6 +24,7 @@ obj-$(CONFIG_SENSORS_LM25066) += > lm25066.o > > obj-$(CONFIG_SENSORS_LT7182S) += lt7182s.o > > obj-$(CONFIG_SENSORS_LTC2978) += ltc2978.o > > obj-$(CONFIG_SENSORS_LTC3815) += ltc3815.o > > +obj-$(CONFIG_SENSORS_LTC4286) += ltc4286.o > > obj-$(CONFIG_SENSORS_MAX15301) += max15301.o > > obj-$(CONFIG_SENSORS_MAX16064) += max16064.o > > obj-$(CONFIG_SENSORS_MAX16601) += max16601.o > > diff --git a/drivers/hwmon/pmbus/ltc4286.c > > b/drivers/hwmon/pmbus/ltc4286.c new file mode 100644 index > > 000000000000..e6690b38349a > > --- /dev/null > > +++ b/drivers/hwmon/pmbus/ltc4286.c > > @@ -0,0 +1,177 @@ > > +// SPDX-License-Identifier: GPL-2.0-or-later > > + > > +#include <linux/err.h> > > +#include <linux/i2c.h> > > +#include <linux/init.h> > > +#include <linux/kernel.h> > > +#include <linux/module.h> > > +#include <linux/pmbus.h> > > +#include "pmbus.h" > > + > > +/* LTC4286 register */ > > +#define LTC4286_MFR_CONFIG1 0xF2 > > + > > +/* LTC4286 configuration */ > > +#define VRANGE_SELECT_BIT BIT(1) > > + > > +#define LTC4286_MFR_ID_SIZE 3 > > +#define VRANGE_25P6 0 > > + > > +/* > > + * Initialize the MBR as default settings which is referred to > > +LTC4286 datasheet > > + * (March 22, 2022 version) table 3 page 16 */ static struct > > +pmbus_driver_info ltc4286_info = { > > + .pages = 1, > > + .format[PSC_VOLTAGE_IN] = direct, > > + .format[PSC_VOLTAGE_OUT] = direct, > > + .format[PSC_CURRENT_OUT] = direct, > > + .format[PSC_POWER] = direct, > > + .format[PSC_TEMPERATURE] = direct, > > + .m[PSC_VOLTAGE_IN] = 32, > > + .b[PSC_VOLTAGE_IN] = 0, > > + .R[PSC_VOLTAGE_IN] = 1, > > + .m[PSC_VOLTAGE_OUT] = 32, > > + .b[PSC_VOLTAGE_OUT] = 0, > > + .R[PSC_VOLTAGE_OUT] = 1, > > + .m[PSC_CURRENT_OUT] = 1024, > > + .b[PSC_CURRENT_OUT] = 0, > > + /* > > + * The rsense value used in MBR formula in LTC4286 datasheet > should be ohm unit. > > + * However, the rsense value that user input is micro ohm. > > + * Thus, the MBR setting which involves rsense should be shifted by 6 > digits. > > + */ > > + .R[PSC_CURRENT_OUT] = 3 - 6, > > + .m[PSC_POWER] = 1, > > + .b[PSC_POWER] = 0, > > + /* > > + * The rsense value used in MBR formula in LTC4286 datasheet > should be ohm unit. > > + * However, the rsense value that user input is micro ohm. > > + * Thus, the MBR setting which involves rsense should be shifted by 6 > digits. > > + */ > > + .R[PSC_POWER] = 4 - 6, > > + .m[PSC_TEMPERATURE] = 1, > > + .b[PSC_TEMPERATURE] = 273, > > + .R[PSC_TEMPERATURE] = 0, > > + .func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT | > PMBUS_HAVE_IOUT | > > + PMBUS_HAVE_PIN | PMBUS_HAVE_TEMP | > PMBUS_HAVE_STATUS_VOUT | > > + PMBUS_HAVE_STATUS_IOUT | > PMBUS_HAVE_STATUS_TEMP, }; > > + > > +static const struct i2c_device_id ltc4286_id[] = { { "ltc4286", 0 }, > > + { "ltc4287", 1 }, > > + {} }; > > Please fix that formatting and use the more common style. > > static const struct i2c_device_id ltc4286_id[] = { > { "ltc4286", 0 }, > { "ltc4287", 1 }, > {} > }; We will revise to static const struct i2c_device_id ltc4286_id[] = { { "ltc4286", 0 }, { "ltc4287", 1 }, {} }; > > > +MODULE_DEVICE_TABLE(i2c, ltc4286_id); > > + > > +static int ltc4286_probe(struct i2c_client *client) { > > + int ret; > > + int temp_setting; > > + const struct i2c_device_id *mid; > > + u8 block_buffer[I2C_SMBUS_BLOCK_MAX + 1]; > > + struct pmbus_driver_info *info; > > + u32 rsense; > > + > > + ret = i2c_smbus_read_block_data(client, PMBUS_MFR_ID, > block_buffer); > > + if (ret < 0) { > > + return dev_err_probe(&client->dev, ret, > > + "Failed to read manufacturer > id\n"); > > + } > > + > > + /* > > + * Refer to ltc4286 datasheet page 20 > > + * the manufacturer id is LTC > > + */ > > + if (ret != LTC4286_MFR_ID_SIZE || > > + strncmp(block_buffer, "LTC", LTC4286_MFR_ID_SIZE)) { > > + return dev_err_probe(&client->dev, ret, > > + "Manufacturer id mismatch\n"); > > + } > > + > > + ret = i2c_smbus_read_block_data(client, PMBUS_MFR_MODEL, > block_buffer); > > + if (ret < 0) { > > + return dev_err_probe(&client->dev, ret, > > + "Failed to read manufacturer > model\n"); > > + } > > + > > + for (mid = ltc4286_id; mid->name[0]; mid++) { > > + if (!strncasecmp(mid->name, block_buffer, > strlen(mid->name))) > > + break; > > + } > > + if (!mid->name[0]) > > + return dev_err_probe(&client->dev, -ENODEV, > > + "Unsupported device\n"); > > + > > + ret = of_property_read_u32(client->dev.of_node, > > + "shunt-resistor-micro-ohms", > &rsense); > > + if (ret < 0) > > + return ret; > > + > > + if (rsense == 0) > > + return -EINVAL; > > + > > + if (rsense > INT_MAX) > > + return -ERANGE; > > + > > This is not "Math result not representable". See below. We will revise as below /* Check for the latter MBR value won't overflow */ if (rsense > (INT_MAX / 1024)) return -EINVAL; > > > + info = devm_kmemdup(&client->dev, <c4286_info, sizeof(*info), > > + GFP_KERNEL); > > + if (!info) > > + return -ENOMEM; > > + > > + /* Default of VRANGE_SELECT = 1, 102.4V */ > > + if (device_property_read_bool(&client->dev, "adi,vrange-low-enable")) > { > > + /* Setup MFR1 CONFIG register bit 1 VRANGE_SELECT */ > > + ret = i2c_smbus_read_word_data(client, > LTC4286_MFR_CONFIG1); > > + if (ret < 0) > > + return dev_err_probe( > > + &client->dev, ret, > > + "Failed to read manufacturer > > + configuration one\n"); > > + > > + ret &= ~VRANGE_SELECT_BIT; /* VRANGE_SELECT = 0, > 25.6V */ > > + ret = i2c_smbus_write_word_data(client, > LTC4286_MFR_CONFIG1, > > + ret); > > + if (ret < 0) > > + return dev_err_probe(&client->dev, ret, > > + "Failed to set > vrange\n"); > > + > > + info->m[PSC_VOLTAGE_IN] = 128; > > + info->m[PSC_VOLTAGE_OUT] = 128; > > + > > + temp_setting = 4 * rsense; > > + if (temp_setting > INT_MAX) > > This will still overflow for values of rsense larger than INT_MAX / 4. > temp_setting is an int, and its value will never be > INT_MAX. > > > + return dev_err_probe(&client->dev, -ERANGE, > > + "Power coefficient > > + overflow\n"); > > ERANGE is "Math result not representable", which is not the case here. > This (and the above above) error needs to be -EINVAL. > > > > + info->m[PSC_POWER] = temp_setting; > > + } else { > > + info->m[PSC_POWER] = rsense; > > I told you before, the default range needs to be set. The range may have been > changed by the BIOS/ROMMON, or someone could have changed it manually > with i2cset or some other application, or some other operating system was > loaded earlier which did its own setting. After consulting with our hardware engineer, we will set default rsense value as 300. > > I do understand by now that you don't want to do that, but I won't accept the > driver without it, sorry. > > > + } > > + > > + temp_setting = 1024 * rsense; > > + if (temp_setting > INT_MAX) > > + return dev_err_probe(&client->dev, -ERANGE, > > + "Current coefficient overflow\n"); > > Same comments as above for both overflow detection and error code. This > also means that rsense must _always_ be <= INT_MAX / 1024. I would suggest > to check that once after reading rsense instead of having three different checks > in different places. > > Also, having an error message here but not with the initial check against > 0 and INT_MAX is inconsistent. I'd suggest no error message, but if you want to > have error messages please make it consistent. OK, we will check the rsense right after reading it. > > > + info->m[PSC_CURRENT_OUT] = temp_setting; > > + > > + return pmbus_do_probe(client, info); } > > + > > +static const struct of_device_id ltc4286_of_match[] = { > > + { .compatible = "lltc,ltc4286" }, > > + { .compatible = "lltc,ltc4287" }, > > + {} > > +}; > > + > > +static struct i2c_driver ltc4286_driver = { > > + .driver = { > > + .name = "ltc4286", > > + .of_match_table = ltc4286_of_match, > > + }, > > + .probe = ltc4286_probe, > > + .id_table = ltc4286_id, > > +}; > > + > > +module_i2c_driver(ltc4286_driver); > > + > > +MODULE_AUTHOR("Delphine CC Chiu > <Delphine_CC_Chiu@wiwynn.com>"); > > +MODULE_DESCRIPTION("PMBUS driver for LTC4286 and compatibles"); > > +MODULE_LICENSE("GPL");
diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst index 72f4e6065bae..080827cc4c34 100644 --- a/Documentation/hwmon/index.rst +++ b/Documentation/hwmon/index.rst @@ -128,6 +128,7 @@ Hardware Monitoring Kernel Drivers ltc4245 ltc4260 ltc4261 + ltc4286 max127 max15301 max16064 diff --git a/Documentation/hwmon/ltc4286.rst b/Documentation/hwmon/ltc4286.rst new file mode 100644 index 000000000000..2cd149676d86 --- /dev/null +++ b/Documentation/hwmon/ltc4286.rst @@ -0,0 +1,95 @@ +.. SPDX-License-Identifier: GPL-2.0-or-later + +Kernel driver ltc4286 +===================== + +Supported chips: + + * Analog Devices LTC4286 + + Prefix: 'ltc4286' + + Addresses scanned: - + + Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ltc4286.pdf + + * Analog Devices LTC4287 + + Prefix: 'ltc4287' + + Addresses scanned: - + + Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ltc4287.pdf + +Author: Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com> + + +Description +----------- + +This driver supports hardware monitoring for Analog Devices LTC4286 +and LTC4287 Hot-Swap Controller and Digital Power Monitors. + +LTC4286 and LTC4287 are hot-swap controllers that allow a circuit board +to be removed from or inserted into a live backplane. They also feature +current and voltage readback via an integrated 12 bit analog-to-digital +converter (ADC), accessed using a PMBus interface. + +The driver is a client driver to the core PMBus driver. Please see +Documentation/hwmon/pmbus.rst for details on PMBus client drivers. + + +Usage Notes +----------- + +This driver does not auto-detect devices. You will have to instantiate the +devices explicitly. Please see Documentation/i2c/instantiating-devices.rst for +details. + +The shunt value in micro-ohms can be set via device tree at compile-time. Please +refer to the Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml for bindings +if the device tree is used. + + +Platform data support +--------------------- + +The driver supports standard PMBus driver platform data. Please see +Documentation/hwmon/pmbus.rst for details. + + +Sysfs entries +------------- + +The following attributes are supported. Limits are read-write, history reset +attributes are write-only, all other attributes are read-only. + +======================= ======================================================= +in1_label "vin" +in1_input Measured voltage. +in1_alarm Input voltage alarm. +in1_min Minimum input voltage. +in1_max Maximum input voltage. + +in2_label "vout1" +in2_input Measured voltage. +in2_alarm Output voltage alarm. +in2_min Minimum output voltage. +in2_max Maximum output voltage. + +curr1_label "iout1" +curr1_input Measured current. +curr1_alarm Output current alarm. +curr1_max Maximum current. + +power1_label "pin" +power1_input Input power. +power1_alarm Input power alarm. +power1_max Maximum poewr. + +temp1_input Chip temperature. +temp1_min Minimum chip temperature. +temp1_max Maximum chip temperature. +temp1_crit Critical chip temperature. +temp1_alarm Chip temperature alarm. +======================= ======================================================= diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig index b4e93bd5835e..2d4f972e5a65 100644 --- a/drivers/hwmon/pmbus/Kconfig +++ b/drivers/hwmon/pmbus/Kconfig @@ -227,6 +227,16 @@ config SENSORS_LTC3815 This driver can also be built as a module. If so, the module will be called ltc3815. +config SENSORS_LTC4286 + bool "Analog Devices LTC4286" + help + LTC4286 is an integrated solution for hot swap applications that + allows a board to be safely inserted and removed from a live + backplane. + This chip could be used to monitor voltage, current, ...etc. + If you say yes here you get hardware monitoring support for Analog + Devices LTC4286. + config SENSORS_MAX15301 tristate "Maxim MAX15301" help diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile index 84ee960a6c2d..94e28f6d6a61 100644 --- a/drivers/hwmon/pmbus/Makefile +++ b/drivers/hwmon/pmbus/Makefile @@ -24,6 +24,7 @@ obj-$(CONFIG_SENSORS_LM25066) += lm25066.o obj-$(CONFIG_SENSORS_LT7182S) += lt7182s.o obj-$(CONFIG_SENSORS_LTC2978) += ltc2978.o obj-$(CONFIG_SENSORS_LTC3815) += ltc3815.o +obj-$(CONFIG_SENSORS_LTC4286) += ltc4286.o obj-$(CONFIG_SENSORS_MAX15301) += max15301.o obj-$(CONFIG_SENSORS_MAX16064) += max16064.o obj-$(CONFIG_SENSORS_MAX16601) += max16601.o diff --git a/drivers/hwmon/pmbus/ltc4286.c b/drivers/hwmon/pmbus/ltc4286.c new file mode 100644 index 000000000000..e6690b38349a --- /dev/null +++ b/drivers/hwmon/pmbus/ltc4286.c @@ -0,0 +1,177 @@ +// SPDX-License-Identifier: GPL-2.0-or-later + +#include <linux/err.h> +#include <linux/i2c.h> +#include <linux/init.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/pmbus.h> +#include "pmbus.h" + +/* LTC4286 register */ +#define LTC4286_MFR_CONFIG1 0xF2 + +/* LTC4286 configuration */ +#define VRANGE_SELECT_BIT BIT(1) + +#define LTC4286_MFR_ID_SIZE 3 +#define VRANGE_25P6 0 + +/* + * Initialize the MBR as default settings which is referred to LTC4286 datasheet + * (March 22, 2022 version) table 3 page 16 + */ +static struct pmbus_driver_info ltc4286_info = { + .pages = 1, + .format[PSC_VOLTAGE_IN] = direct, + .format[PSC_VOLTAGE_OUT] = direct, + .format[PSC_CURRENT_OUT] = direct, + .format[PSC_POWER] = direct, + .format[PSC_TEMPERATURE] = direct, + .m[PSC_VOLTAGE_IN] = 32, + .b[PSC_VOLTAGE_IN] = 0, + .R[PSC_VOLTAGE_IN] = 1, + .m[PSC_VOLTAGE_OUT] = 32, + .b[PSC_VOLTAGE_OUT] = 0, + .R[PSC_VOLTAGE_OUT] = 1, + .m[PSC_CURRENT_OUT] = 1024, + .b[PSC_CURRENT_OUT] = 0, + /* + * The rsense value used in MBR formula in LTC4286 datasheet should be ohm unit. + * However, the rsense value that user input is micro ohm. + * Thus, the MBR setting which involves rsense should be shifted by 6 digits. + */ + .R[PSC_CURRENT_OUT] = 3 - 6, + .m[PSC_POWER] = 1, + .b[PSC_POWER] = 0, + /* + * The rsense value used in MBR formula in LTC4286 datasheet should be ohm unit. + * However, the rsense value that user input is micro ohm. + * Thus, the MBR setting which involves rsense should be shifted by 6 digits. + */ + .R[PSC_POWER] = 4 - 6, + .m[PSC_TEMPERATURE] = 1, + .b[PSC_TEMPERATURE] = 273, + .R[PSC_TEMPERATURE] = 0, + .func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT | PMBUS_HAVE_IOUT | + PMBUS_HAVE_PIN | PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_VOUT | + PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_STATUS_TEMP, +}; + +static const struct i2c_device_id ltc4286_id[] = { { "ltc4286", 0 }, + { "ltc4287", 1 }, + {} }; +MODULE_DEVICE_TABLE(i2c, ltc4286_id); + +static int ltc4286_probe(struct i2c_client *client) +{ + int ret; + int temp_setting; + const struct i2c_device_id *mid; + u8 block_buffer[I2C_SMBUS_BLOCK_MAX + 1]; + struct pmbus_driver_info *info; + u32 rsense; + + ret = i2c_smbus_read_block_data(client, PMBUS_MFR_ID, block_buffer); + if (ret < 0) { + return dev_err_probe(&client->dev, ret, + "Failed to read manufacturer id\n"); + } + + /* + * Refer to ltc4286 datasheet page 20 + * the manufacturer id is LTC + */ + if (ret != LTC4286_MFR_ID_SIZE || + strncmp(block_buffer, "LTC", LTC4286_MFR_ID_SIZE)) { + return dev_err_probe(&client->dev, ret, + "Manufacturer id mismatch\n"); + } + + ret = i2c_smbus_read_block_data(client, PMBUS_MFR_MODEL, block_buffer); + if (ret < 0) { + return dev_err_probe(&client->dev, ret, + "Failed to read manufacturer model\n"); + } + + for (mid = ltc4286_id; mid->name[0]; mid++) { + if (!strncasecmp(mid->name, block_buffer, strlen(mid->name))) + break; + } + if (!mid->name[0]) + return dev_err_probe(&client->dev, -ENODEV, + "Unsupported device\n"); + + ret = of_property_read_u32(client->dev.of_node, + "shunt-resistor-micro-ohms", &rsense); + if (ret < 0) + return ret; + + if (rsense == 0) + return -EINVAL; + + if (rsense > INT_MAX) + return -ERANGE; + + info = devm_kmemdup(&client->dev, <c4286_info, sizeof(*info), + GFP_KERNEL); + if (!info) + return -ENOMEM; + + /* Default of VRANGE_SELECT = 1, 102.4V */ + if (device_property_read_bool(&client->dev, "adi,vrange-low-enable")) { + /* Setup MFR1 CONFIG register bit 1 VRANGE_SELECT */ + ret = i2c_smbus_read_word_data(client, LTC4286_MFR_CONFIG1); + if (ret < 0) + return dev_err_probe( + &client->dev, ret, + "Failed to read manufacturer configuration one\n"); + + ret &= ~VRANGE_SELECT_BIT; /* VRANGE_SELECT = 0, 25.6V */ + ret = i2c_smbus_write_word_data(client, LTC4286_MFR_CONFIG1, + ret); + if (ret < 0) + return dev_err_probe(&client->dev, ret, + "Failed to set vrange\n"); + + info->m[PSC_VOLTAGE_IN] = 128; + info->m[PSC_VOLTAGE_OUT] = 128; + + temp_setting = 4 * rsense; + if (temp_setting > INT_MAX) + return dev_err_probe(&client->dev, -ERANGE, + "Power coefficient overflow\n"); + info->m[PSC_POWER] = temp_setting; + } else { + info->m[PSC_POWER] = rsense; + } + + temp_setting = 1024 * rsense; + if (temp_setting > INT_MAX) + return dev_err_probe(&client->dev, -ERANGE, + "Current coefficient overflow\n"); + info->m[PSC_CURRENT_OUT] = temp_setting; + + return pmbus_do_probe(client, info); +} + +static const struct of_device_id ltc4286_of_match[] = { + { .compatible = "lltc,ltc4286" }, + { .compatible = "lltc,ltc4287" }, + {} +}; + +static struct i2c_driver ltc4286_driver = { + .driver = { + .name = "ltc4286", + .of_match_table = ltc4286_of_match, + }, + .probe = ltc4286_probe, + .id_table = ltc4286_id, +}; + +module_i2c_driver(ltc4286_driver); + +MODULE_AUTHOR("Delphine CC Chiu <Delphine_CC_Chiu@wiwynn.com>"); +MODULE_DESCRIPTION("PMBUS driver for LTC4286 and compatibles"); +MODULE_LICENSE("GPL");
Add a driver to support ltc4286 chip Signed-off-by: Delphine CC Chiu <Delphine_CC_Chiu@Wiwynn.com> Changelog: v4 - Add empty line before "config SENSORS_LTC4286" in Kconfig - Add ltc4286 to Documentation/hwmon/index.rst - Revise comment typo - Use devm_kmemdup instead of memcpy - Check MBR value before writting into v3 - Use dev_err_probe() instead of dev_err() - The VRANGE_SELECT bit only be written if it actually changed - Avoid the info pointer being overwritten - Check the MBR value range to avoid overflow - Revise ltc4286.rst to corrcet description v2 - Revise Linear Technologies LTC4286 to Analog Devices LTC4286 in Kconfig - Add more description for this driver in Kconfig - Add some comments for MBR setting in ltc4286.c - Add ltc4286.rst --- Documentation/hwmon/index.rst | 1 + Documentation/hwmon/ltc4286.rst | 95 +++++++++++++++++ drivers/hwmon/pmbus/Kconfig | 10 ++ drivers/hwmon/pmbus/Makefile | 1 + drivers/hwmon/pmbus/ltc4286.c | 177 ++++++++++++++++++++++++++++++++ 5 files changed, 284 insertions(+) create mode 100644 Documentation/hwmon/ltc4286.rst create mode 100644 drivers/hwmon/pmbus/ltc4286.c