Re: [PATCH v2 1/2] hwmon: pmbus: adm1266: add support

Guenter Roeck
2020-03-25T13:58:20Z

Hi,

On 3/25/20 6:06 AM, alexandru.tachici@analog.com wrote:
> From: Alexandru Tachici <alexandru.tachici@analog.com>
>
> Add pmbus probing driver for the adm1266 Cascadable
> Super Sequencer with Margin Control and Fault Recording.
> Driver is using the pmbus_core, creating sysfs files
> under hwmon for inputs: vh1->vh4 and vp1->vp13.
>
> Signed-off-by: Alexandru Tachici <alexandru.tachici@analog.com>

It looks like the driver is reduced to just standard functionality.
Can you try just adding the device ID to pmbus.c ?

Thanks,
Guenter

> ---
> Documentation/hwmon/adm1266.rst | 35 +++++++++++++++++++
> drivers/hwmon/pmbus/Kconfig | 9 +++++
> drivers/hwmon/pmbus/Makefile | 1 +
> drivers/hwmon/pmbus/adm1266.c | 62 +++++++++++++++++++++++++++++++++
> 4 files changed, 107 insertions(+)
> create mode 100644 Documentation/hwmon/adm1266.rst
> create mode 100644 drivers/hwmon/pmbus/adm1266.c
>
> diff --git a/Documentation/hwmon/adm1266.rst b/Documentation/hwmon/adm1266.rst
> new file mode 100644
> index 000000000000..65662115750c
> --- /dev/null
> +++ b/Documentation/hwmon/adm1266.rst
> @@ -0,0 +1,35 @@
> +Kernel driver adm1266
> +=====================
> +
> +Supported chips:
> + * Analog Devices ADM1266
> + Prefix: 'adm1266'
> + Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/ADM1266.pdf
> +
> +Author: Alexandru Tachici <alexandru.tachici@analog.com>
> +
> +
> +Description
> +-----------
> +
> +This driver supports hardware monitoring for Analog Devices ADM1266 sequencer.
> +
> +ADM1266 is a sequencer that features voltage readback from 17 channels via an
> +integrated 12 bit SAR ADC, accessed using a PMBus interface.
> +
> +The driver is a client driver to the core PMBus driver. Please see
> +Documentation/hwmon/pmbus for details on PMBus client drivers.
> +
> +
> +Sysfs entries
> +-------------
> +
> +The following attributes are supported. Limits are read-write, history reset
> +attributes are write-only, all other attributes are read-only.
> +
> +inX_label "voutx"
> +inX_input Measured voltage.
> +inX_min Minimum Voltage.
> +inX_max Maximum voltage.
> +inX_min_alarm Voltage low alarm.
> +inX_max_alarm Voltage high alarm.
> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> index a9ea06204767..3096e46e2212 100644
> --- a/drivers/hwmon/pmbus/Kconfig
> +++ b/drivers/hwmon/pmbus/Kconfig
> @@ -26,6 +26,15 @@ config SENSORS_PMBUS
> This driver can also be built as a module. If so, the module will
> be called pmbus.
>
> +config SENSORS_ADM1266
> + tristate "Analog Devices ADM1266"
> + help
> + If you say yes here you get hardware monitoring support for Analog
> + Devices ADM1266 Cascadable Super Sequencer.
> +
> + This driver can also be built as a module. If so, the module will
> + be called adm1266.
> +
> config SENSORS_ADM1275
> tristate "Analog Devices ADM1275 and compatibles"
> help
> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
> index 5feb45806123..ed38f6d6f845 100644
> --- a/drivers/hwmon/pmbus/Makefile
> +++ b/drivers/hwmon/pmbus/Makefile
> @@ -5,6 +5,7 @@
>
> obj-$(CONFIG_PMBUS) += pmbus_core.o
> obj-$(CONFIG_SENSORS_PMBUS) += pmbus.o
> +obj-$(CONFIG_SENSORS_ADM1266) += adm1266.o
> obj-$(CONFIG_SENSORS_ADM1275) += adm1275.o
> obj-$(CONFIG_SENSORS_BEL_PFE) += bel-pfe.o
> obj-$(CONFIG_SENSORS_IBM_CFFPS) += ibm-cffps.o
> diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
> new file mode 100644
> index 000000000000..c1df21d564f3
> --- /dev/null
> +++ b/drivers/hwmon/pmbus/adm1266.c
> @@ -0,0 +1,62 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * ADM1266 - Cascadable Super Sequencer with Margin
> + * Control and Fault Recording
> + *
> + * Copyright 2020 Analog Devices Inc.
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +
> +#include "pmbus.h"
> +
> +static int adm1266_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct pmbus_driver_info *info;
> + u32 funcs;
> + int i;
> +
> + info = devm_kzalloc(&client->dev, sizeof(struct pmbus_driver_info),
> + GFP_KERNEL);
> +
> + info->pages = 17;
> + info->format[PSC_VOLTAGE_OUT] = linear;
> + funcs = PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT;
> + for (i = 0; i < info->pages; i++)
> + info->func[i] = funcs;
> +
> + return pmbus_do_probe(client, id, info);
> +}
> +
> +static const struct of_device_id adm1266_of_match[] = {
> + { .compatible = "adi,adm1266" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, adm1266_of_match);
> +
> +static const struct i2c_device_id adm1266_id[] = {
> + { "adm1266", 0 },
> + { },
> +};
> +MODULE_DEVICE_TABLE(i2c, adm1266_id);
> +
> +static struct i2c_driver adm1266_driver = {
> + .driver = {
> + .name = "adm1266",
> + .of_match_table = adm1266_of_match,
> + },
> + .probe = adm1266_probe,
> + .remove = pmbus_do_remove,
> + .id_table = adm1266_id,
> +};
> +
> +module_i2c_driver(adm1266_driver);
> +
> +MODULE_AUTHOR("Alexandru Tachici <alexandru.tachici@analog.com>");
> +MODULE_DESCRIPTION("PMBus driver for Analog Devices ADM1266");
> +MODULE_LICENSE("GPL v2");
>