[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <cab86814-4ab9-4c52-86b5-c35c4f5ed590@roeck-us.net>
Date: Tue, 29 Oct 2024 07:15:11 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Cedric Encarnacion <cedricjustine.encarnacion@...log.com>,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-i2c@...r.kernel.org, linux-doc@...r.kernel.org,
linux-hwmon@...r.kernel.org
Cc: Jean Delvare <jdelvare@...e.com>, Jonathan Corbet <corbet@....net>,
Delphine CC Chiu <Delphine_CC_Chiu@...ynn.com>, Rob Herring
<robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Peter Yin <peteryin.openbmc@...il.com>,
Noah Wang <noahwang.wang@...look.com>, Marek Vasut <marex@...x.de>,
Lukas Wunner <lukas@...ner.de>
Subject: Re: [PATCH 2/2] hwmon: pmbus: add driver for ltp8800-1a, ltp8800-4a,
and ltp8800-2
On 10/29/24 06:01, Cedric Encarnacion wrote:
> LTP8800-1A 54V, 150A DC/DC µModule Regulator with PMBus Interface
> LTP8800-4A 54V, 200A DC/DC µModule Regulator with PMBus Interface
> LTP8800-2 54V, 135A DC/DC μModule Regulator with PMBus Interface
>
Please provide a short description.
> Signed-off-by: Cedric Encarnacion <cedricjustine.encarnacion@...log.com>
> ---
> Documentation/hwmon/index.rst | 1 +
> Documentation/hwmon/ltp8800.rst | 103 ++++++++++++++++++++++++++++++++
> MAINTAINERS | 2 +
> drivers/hwmon/pmbus/Kconfig | 18 ++++++
> drivers/hwmon/pmbus/Makefile | 1 +
> drivers/hwmon/pmbus/ltp8800.c | 74 +++++++++++++++++++++++
> 6 files changed, 199 insertions(+)
> create mode 100644 Documentation/hwmon/ltp8800.rst
> create mode 100644 drivers/hwmon/pmbus/ltp8800.c
>
> diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
> index 4d15664bc41e..d51960f58e43 100644
> --- a/Documentation/hwmon/index.rst
> +++ b/Documentation/hwmon/index.rst
> @@ -136,6 +136,7 @@ Hardware Monitoring Kernel Drivers
> ltc4261
> ltc4282
> ltc4286
> + ltp8800
> max127
> max15301
> max16064
> diff --git a/Documentation/hwmon/ltp8800.rst b/Documentation/hwmon/ltp8800.rst
> new file mode 100644
> index 000000000000..dea73f60c3d7
> --- /dev/null
> +++ b/Documentation/hwmon/ltp8800.rst
> @@ -0,0 +1,103 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +Kernel driver ltp8800
> +=====================
> +
> +Supported chips:
> +
> + * Analog Devices LTP8800-1A
> +
> + Prefix: 'ltp8800-1a'
> +
> + Addresses scanned: I2C 0x40 - 0x4F
> +
No, the chips are not scanned.
> + Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ltp8800-1a.pdf
> +
> + * Analog Devices LTP8800-4A
> +
> + Prefix: 'ltp8800-4a'
> +
> + Addresses scanned: I2C 0x40 - 0x4F
> +
> + Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ltp8800-4a.pdf
> +
> + * Analog Devices LTP8800-2
> +
> + Prefix: 'ltp8800-2'
> +
> + Addresses scanned: I2C 0x40 - 0x4F
> +
> + Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ltp8800-2.pdf
> +
> +Authors:
> + - Cedric Encarnacion <cedricjustine.encarnacion@...log.com>
> +
> +
> +Description
> +-----------
> +
> +The LTP8800 is a family of step-down μModule regulators that provides
> +microprocessor core voltage from 54V power distribution architecture. LTP8800
> +features telemetry monitoring of input/output voltage, input current, output
> +power, and temperature over PMBus.
> +
> +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.
> +
> +Platform data support
> +---------------------
> +
> +The driver supports standard PMBus driver platform data. Please see
> +Documentation/hwmon/pmbus.rst for details.
> +
> +Sysfs Attributes
> +----------------
> +
> +======================= ===========================
> +curr1_label "iin"
> +curr1_input Measured input current
> +curr1_crit Critical maximum current
> +curr1_crit_alarm Current critical high alarm
> +
> +curr2_label "iout1"
> +curr2_input Measured output current
> +curr2_lcrit Critical minimum current
> +curr2_crit Critical maximum current
> +curr2_max Maximum output current
> +curr2_alarm Current alarm
> +
> +in1_label "vin"
> +in1_input Measured input voltage
> +in1_lcrit Critical minimum input voltage
> +in1_lcrit_alarm Input voltage critical low alarm
> +in1_crit Critical maximum input voltage
> +in1_crit_alarm Input voltage critical high 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
> +power1_crit Critical maximum output power
> +
> +temp1_input Measured temperature
> +temp1_lcrit Critical low temperature
> +temp1_lcrit_alarm Chip temperature critical low alarm
> +temp1_crit Critical high temperature
> +temp1_crit_alarm Chip temperature critical high alarm
> +======================= ===========================
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a6abf7243b94..5e7df53eb4a0 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -13559,6 +13559,8 @@ LTP8800 HARDWARE MONITOR DRIVER
> M: Cedric Encarnacion <cedricjustine.encarnacion@...log.com>
> L: linux-hwmon@...r.kernel.org
> S: Supported
> +F: Documentation/hwmon/ltp8800.rst
> +F: drivers/hwmon/pmbus/ltp8800.c
>
> LYNX 28G SERDES PHY DRIVER
> M: Ioana Ciornei <ioana.ciornei@....com>
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index a4f02cad92fd..33e6da249ac8 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -247,6 +247,24 @@ config SENSORS_LTC4286
> If you say yes here you get hardware monitoring support for Analog
> Devices LTC4286.
>
> +config SENSORS_LTP8800
> + tristate "Analog Devices LTP8800 and compatibles"
> + help
> + If you say yes here you get hardware monitoring support for Analog
> + Devices LTP8800-1A, LTP8800-4A, and LTP8800-2.
> +
> + This driver can also be built as a module. If so, the module will
> + be called ltp8800.
> +
> +config SENSORS_LTP8800_REGULATOR
> + bool "Regulator support for LTP8800 and compatibles"
> + depends on SENSORS_LTP8800 && REGULATOR
> + help
> + If you say yes here you get regulator support for Analog Devices
> + LTP8800-1A, LTP8800-4A, and LTP8800-2. LTP8800 is a family of DC/DC
> + µModule regulators that can provide microprocessor power from 54V
> + power distribution architecture.
> +
> config SENSORS_MAX15301
> tristate "Maxim MAX15301"
> help
> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
> index d00bcc758b97..aa5bbdb4a806 100644
> --- a/drivers/hwmon/pmbus/Makefile
> +++ b/drivers/hwmon/pmbus/Makefile
> @@ -26,6 +26,7 @@ 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_LTP8800) += ltp8800.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/ltp8800.c b/drivers/hwmon/pmbus/ltp8800.c
> new file mode 100644
> index 000000000000..a377f2e2b001
> --- /dev/null
> +++ b/drivers/hwmon/pmbus/ltp8800.c
> @@ -0,0 +1,74 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Hardware monitoring driver for Analog Devices LTP8800
> + *
> + * Copyright (C) 2024 Analog Devices, Inc.
> + */
> +#include <linux/bits.h>
Is this used anywhere ? I don''t immediately see it.
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include "pmbus.h"
> +
> +static const struct regulator_desc ltp8800_reg_desc[] = {
> + PMBUS_REGULATOR("vout", 0),
> +};
> +
> +static struct pmbus_driver_info ltp8800_info = {
> + .pages = 1,
> + .format[PSC_VOLTAGE_IN] = linear,
> + .format[PSC_VOLTAGE_OUT] = linear,
> + .format[PSC_CURRENT_IN] = linear,
> + .format[PSC_TEMPERATURE] = linear,
> + .func[0] = PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT |
> + PMBUS_HAVE_VIN | PMBUS_HAVE_STATUS_INPUT |
> + PMBUS_HAVE_IIN | PMBUS_HAVE_IOUT |
> + PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP |
> + PMBUS_HAVE_POUT,
> +};
> +
> +static int ltp8800_probe(struct i2c_client *client)
> +{
> + if (!i2c_check_functionality(client->adapter,
> + I2C_FUNC_SMBUS_READ_BYTE_DATA |
> + I2C_FUNC_SMBUS_READ_WORD_DATA))
> + return -ENODEV;
> +
This is also checked in pmbus_do_probe().
> + if (IS_ENABLED(CONFIG_SENSORS_LTP8800_REGULATOR)) {
> + ltp8800_info.num_regulators = 1;
> + ltp8800_info.reg_desc = ltp8800_reg_desc;
> + }
This can be set directly in ltp8800_info.
> +
> + return pmbus_do_probe(client, <p8800_info);
> +}
> +
> +static const struct i2c_device_id ltp8800_id[] = {
> + {"ltp8800-1a", 0},
> + {"ltp8800-2", 0},
> + {"ltp8800-4a", 0},
> + {}
> +};
> +MODULE_DEVICE_TABLE(i2c, ltp8800_id);
> +
> +static const struct of_device_id ltp8800_of_match[] = {
> + { .compatible = "adi,ltp8800-1a"},
> + { .compatible = "adi,ltp8800-2"},
> + { .compatible = "adi,ltp8800-4a"},
As mentioned in the other patch, I don't see the point of having three
compatible entries and three device IDs.
Guenter
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, ltp8800_of_match);
> +
> +static struct i2c_driver ltp8800_driver = {
> + .driver = {
> + .name = "ltp8800",
> + .of_match_table = ltp8800_of_match,
> + },
> + .probe = ltp8800_probe,
> + .id_table = ltp8800_id,
> +};
> +module_i2c_driver(ltp8800_driver);
> +
> +MODULE_AUTHOR("Cedric Encarnacion <cedricjustine.encarnacion@...log.com>");
> +MODULE_DESCRIPTION("Analog Devices LTP8800 HWMON PMBus Driver");
> +MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS(PMBUS);
Powered by blists - more mailing lists