* [PATCH v3 0/2] hwmon: (pmbus) Add Delta AHE-50DC fan control module driver @ 2021-12-07 7:15 ` Zev Weiss 0 siblings, 0 replies; 28+ messages in thread From: Zev Weiss @ 2021-12-07 7:15 UTC (permalink / raw) To: linux-hwmon; +Cc: Guenter Roeck, Jean Delvare, openbmc, Zev Weiss Hello, These patches add a hwmon pmbus driver for the integrated fan control module in the Delta AHE-50DC Open19 power shelf. I don't know of a datasheet or any proper documentation for it; the registers it has available have deen deduced from a combination of an existing GPL driver in a code release from LinkedIn [0] and some manual experimentation with i2cdump. In addition to some standard PMBus sensors (three temperature readings, four fan speeds, and one voltage), it also has a manufacturer-specific fourth temperature reading available, which is supported via a virtual second page (the device itself is not paged, but the driver remaps PMBUS_READ_TEMPERATURE_1 in page 1 to the manufacturer-specific TEMPERATURE_4). Thanks, Zev Weiss Changes since v2 [2]: - converted to pmbus driver Changes since v1 [1]: - fixed invalid name warning from __hwmon_device_register() [0] https://github.com/linkedin/o19-bmc-firmware/blob/master/meta-openbmc/meta-linkedin/meta-deltapower/recipes-kernel/fancontrol-mod/files/fancontrol.c [1] https://lore.kernel.org/linux-hwmon/20211206224419.15736-1-zev@bewilderbeest.net/ [2] https://lore.kernel.org/linux-hwmon/20211206230153.16891-1-zev@bewilderbeest.net/ Zev Weiss (2): hwmon: (pmbus) Add Delta AHE-50DC fan control module driver dt-bindings: add Delta AHE-50DC fan control module .../devicetree/bindings/trivial-devices.yaml | 2 + MAINTAINERS | 6 ++ drivers/hwmon/pmbus/Kconfig | 12 +++ drivers/hwmon/pmbus/Makefile | 1 + drivers/hwmon/pmbus/delta-ahe50dc-fan.c | 84 +++++++++++++++++++ 5 files changed, 105 insertions(+) create mode 100644 drivers/hwmon/pmbus/delta-ahe50dc-fan.c -- 2.34.1 ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v3 0/2] hwmon: (pmbus) Add Delta AHE-50DC fan control module driver @ 2021-12-07 7:15 ` Zev Weiss 0 siblings, 0 replies; 28+ messages in thread From: Zev Weiss @ 2021-12-07 7:15 UTC (permalink / raw) To: linux-hwmon; +Cc: openbmc, Jean Delvare, Zev Weiss, Guenter Roeck Hello, These patches add a hwmon pmbus driver for the integrated fan control module in the Delta AHE-50DC Open19 power shelf. I don't know of a datasheet or any proper documentation for it; the registers it has available have deen deduced from a combination of an existing GPL driver in a code release from LinkedIn [0] and some manual experimentation with i2cdump. In addition to some standard PMBus sensors (three temperature readings, four fan speeds, and one voltage), it also has a manufacturer-specific fourth temperature reading available, which is supported via a virtual second page (the device itself is not paged, but the driver remaps PMBUS_READ_TEMPERATURE_1 in page 1 to the manufacturer-specific TEMPERATURE_4). Thanks, Zev Weiss Changes since v2 [2]: - converted to pmbus driver Changes since v1 [1]: - fixed invalid name warning from __hwmon_device_register() [0] https://github.com/linkedin/o19-bmc-firmware/blob/master/meta-openbmc/meta-linkedin/meta-deltapower/recipes-kernel/fancontrol-mod/files/fancontrol.c [1] https://lore.kernel.org/linux-hwmon/20211206224419.15736-1-zev@bewilderbeest.net/ [2] https://lore.kernel.org/linux-hwmon/20211206230153.16891-1-zev@bewilderbeest.net/ Zev Weiss (2): hwmon: (pmbus) Add Delta AHE-50DC fan control module driver dt-bindings: add Delta AHE-50DC fan control module .../devicetree/bindings/trivial-devices.yaml | 2 + MAINTAINERS | 6 ++ drivers/hwmon/pmbus/Kconfig | 12 +++ drivers/hwmon/pmbus/Makefile | 1 + drivers/hwmon/pmbus/delta-ahe50dc-fan.c | 84 +++++++++++++++++++ 5 files changed, 105 insertions(+) create mode 100644 drivers/hwmon/pmbus/delta-ahe50dc-fan.c -- 2.34.1 ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v3 1/2] hwmon: (pmbus) Add Delta AHE-50DC fan control module driver 2021-12-07 7:15 ` Zev Weiss @ 2021-12-07 7:15 ` Zev Weiss -1 siblings, 0 replies; 28+ messages in thread From: Zev Weiss @ 2021-12-07 7:15 UTC (permalink / raw) To: linux-hwmon; +Cc: Guenter Roeck, Jean Delvare, openbmc, Zev Weiss, linux-kernel This device is an integrated module of the Delta AHE-50DC Open19 power shelf. For lack of proper documentation, this driver has been developed referencing an existing (GPL) driver that was included in a code release from LinkedIn [1]. It provides four fan speeds, four temperatures, and one voltage reading, as well as a handful of warning and fault indicators. [1] https://github.com/linkedin/o19-bmc-firmware/blob/master/meta-openbmc/meta-linkedin/meta-deltapower/recipes-kernel/fancontrol-mod/files/fancontrol.c Signed-off-by: Zev Weiss <zev@bewilderbeest.net> --- MAINTAINERS | 6 ++ drivers/hwmon/pmbus/Kconfig | 12 ++++ drivers/hwmon/pmbus/Makefile | 1 + drivers/hwmon/pmbus/delta-ahe50dc-fan.c | 84 +++++++++++++++++++++++++ 4 files changed, 103 insertions(+) create mode 100644 drivers/hwmon/pmbus/delta-ahe50dc-fan.c diff --git a/MAINTAINERS b/MAINTAINERS index 0ac052200ecb..8bb7ba52d2f5 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -5425,6 +5425,12 @@ W: https://linuxtv.org T: git git://linuxtv.org/media_tree.git F: drivers/media/platform/sti/delta +DELTA AHE-50DC FAN CONTROL MODULE DRIVER +M: Zev Weiss <zev@bewilderbeest.net> +L: linux-hwmon@vger.kernel.org +S: Maintained +F: drivers/hwmon/pmbus/delta-ahe50dc-fan.c + DELTA DPS920AB PSU DRIVER M: Robert Marko <robert.marko@sartura.hr> L: linux-hwmon@vger.kernel.org diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig index ffb609cee3a4..937e1c2c11e7 100644 --- a/drivers/hwmon/pmbus/Kconfig +++ b/drivers/hwmon/pmbus/Kconfig @@ -66,6 +66,18 @@ config SENSORS_BPA_RS600 This driver can also be built as a module. If so, the module will be called bpa-rs600. +config SENSORS_DELTA_AHE50DC_FAN + tristate "Delta AHE-50DC fan control module" + depends on I2C + select REGMAP_I2C + help + If you say yes here you get hardware monitoring support for + the integrated fan control module of the Delta AHE-50DC + Open19 power shelf. + + This driver can also be built as a module. If so, the module + will be called delta-ahe50dc-fan. + config SENSORS_FSP_3Y tristate "FSP/3Y-Power power supplies" help diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile index 0ed4d596a948..a56b2897288d 100644 --- a/drivers/hwmon/pmbus/Makefile +++ b/drivers/hwmon/pmbus/Makefile @@ -9,6 +9,7 @@ obj-$(CONFIG_SENSORS_ADM1266) += adm1266.o obj-$(CONFIG_SENSORS_ADM1275) += adm1275.o obj-$(CONFIG_SENSORS_BEL_PFE) += bel-pfe.o obj-$(CONFIG_SENSORS_BPA_RS600) += bpa-rs600.o +obj-$(CONFIG_SENSORS_DELTA_AHE50DC_FAN) += delta-ahe50dc-fan.o obj-$(CONFIG_SENSORS_FSP_3Y) += fsp-3y.o obj-$(CONFIG_SENSORS_IBM_CFFPS) += ibm-cffps.o obj-$(CONFIG_SENSORS_DPS920AB) += dps920ab.o diff --git a/drivers/hwmon/pmbus/delta-ahe50dc-fan.c b/drivers/hwmon/pmbus/delta-ahe50dc-fan.c new file mode 100644 index 000000000000..07b1e7c5f5f5 --- /dev/null +++ b/drivers/hwmon/pmbus/delta-ahe50dc-fan.c @@ -0,0 +1,84 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Delta AHE-50DC power shelf fan control module driver + * + * Copyright 2021 Zev Weiss <zev@bewilderbeest.net> + */ + +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/i2c.h> +#include <linux/pmbus.h> + +#include "pmbus.h" + +#define AHE50DC_PMBUS_READ_TEMP4 0xd0 + +static int ahe50dc_fan_read_word_data(struct i2c_client *client, int page, int phase, int reg) +{ + /* temp1 in (virtual) page 1 is remapped to mfr-specific temp4 */ + if (page == 1) { + if (reg == PMBUS_READ_TEMPERATURE_1) + return i2c_smbus_read_word_data(client, AHE50DC_PMBUS_READ_TEMP4); + return -EOPNOTSUPP; + } + return -ENODATA; +} + +static struct pmbus_driver_info ahe50dc_fan_info = { + .pages = 2, + .format[PSC_FAN] = direct, + .format[PSC_TEMPERATURE] = direct, + .format[PSC_VOLTAGE_IN] = direct, + .m[PSC_FAN] = 1, + .b[PSC_FAN] = 0, + .R[PSC_FAN] = 0, + .m[PSC_TEMPERATURE] = 1, + .b[PSC_TEMPERATURE] = 0, + .R[PSC_TEMPERATURE] = 1, + .m[PSC_VOLTAGE_IN] = 1, + .b[PSC_VOLTAGE_IN] = 0, + .R[PSC_VOLTAGE_IN] = 3, + .func[0] = PMBUS_HAVE_TEMP | PMBUS_HAVE_TEMP2 | PMBUS_HAVE_TEMP3 | + PMBUS_HAVE_VIN | PMBUS_HAVE_FAN12 | PMBUS_HAVE_FAN34 | + PMBUS_HAVE_STATUS_FAN12 | PMBUS_HAVE_STATUS_FAN34 | PMBUS_PAGE_VIRTUAL, + .func[1] = PMBUS_HAVE_TEMP | PMBUS_PAGE_VIRTUAL, + .read_word_data = ahe50dc_fan_read_word_data, +}; + +static struct pmbus_platform_data ahe50dc_fan_data = { + .flags = PMBUS_NO_CAPABILITY, +}; + +static int ahe50dc_fan_probe(struct i2c_client *client) +{ + client->dev.platform_data = &ahe50dc_fan_data; + return pmbus_do_probe(client, &ahe50dc_fan_info); +} + +static const struct i2c_device_id ahe50dc_fan_id[] = { + { "ahe50dc_fan" }, + { } +}; +MODULE_DEVICE_TABLE(i2c, ahe50dc_fan_id); + +static const struct of_device_id __maybe_unused ahe50dc_fan_of_match[] = { + { .compatible = "delta,ahe50dc-fan" }, + { } +}; +MODULE_DEVICE_TABLE(of, ahe50dc_fan_of_match); + +static struct i2c_driver ahe50dc_fan_driver = { + .driver = { + .name = "ahe50dc_fan", + .of_match_table = of_match_ptr(ahe50dc_fan_of_match), + }, + .probe_new = ahe50dc_fan_probe, + .id_table = ahe50dc_fan_id, +}; +module_i2c_driver(ahe50dc_fan_driver); + +MODULE_AUTHOR("Zev Weiss <zev@bewilderbeest.net>"); +MODULE_DESCRIPTION("Driver for Delta AHE-50DC power shelf fan control module"); +MODULE_LICENSE("GPL"); +MODULE_IMPORT_NS(PMBUS); -- 2.34.1 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v3 1/2] hwmon: (pmbus) Add Delta AHE-50DC fan control module driver @ 2021-12-07 7:15 ` Zev Weiss 0 siblings, 0 replies; 28+ messages in thread From: Zev Weiss @ 2021-12-07 7:15 UTC (permalink / raw) To: linux-hwmon; +Cc: openbmc, Jean Delvare, Zev Weiss, Guenter Roeck, linux-kernel This device is an integrated module of the Delta AHE-50DC Open19 power shelf. For lack of proper documentation, this driver has been developed referencing an existing (GPL) driver that was included in a code release from LinkedIn [1]. It provides four fan speeds, four temperatures, and one voltage reading, as well as a handful of warning and fault indicators. [1] https://github.com/linkedin/o19-bmc-firmware/blob/master/meta-openbmc/meta-linkedin/meta-deltapower/recipes-kernel/fancontrol-mod/files/fancontrol.c Signed-off-by: Zev Weiss <zev@bewilderbeest.net> --- MAINTAINERS | 6 ++ drivers/hwmon/pmbus/Kconfig | 12 ++++ drivers/hwmon/pmbus/Makefile | 1 + drivers/hwmon/pmbus/delta-ahe50dc-fan.c | 84 +++++++++++++++++++++++++ 4 files changed, 103 insertions(+) create mode 100644 drivers/hwmon/pmbus/delta-ahe50dc-fan.c diff --git a/MAINTAINERS b/MAINTAINERS index 0ac052200ecb..8bb7ba52d2f5 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -5425,6 +5425,12 @@ W: https://linuxtv.org T: git git://linuxtv.org/media_tree.git F: drivers/media/platform/sti/delta +DELTA AHE-50DC FAN CONTROL MODULE DRIVER +M: Zev Weiss <zev@bewilderbeest.net> +L: linux-hwmon@vger.kernel.org +S: Maintained +F: drivers/hwmon/pmbus/delta-ahe50dc-fan.c + DELTA DPS920AB PSU DRIVER M: Robert Marko <robert.marko@sartura.hr> L: linux-hwmon@vger.kernel.org diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig index ffb609cee3a4..937e1c2c11e7 100644 --- a/drivers/hwmon/pmbus/Kconfig +++ b/drivers/hwmon/pmbus/Kconfig @@ -66,6 +66,18 @@ config SENSORS_BPA_RS600 This driver can also be built as a module. If so, the module will be called bpa-rs600. +config SENSORS_DELTA_AHE50DC_FAN + tristate "Delta AHE-50DC fan control module" + depends on I2C + select REGMAP_I2C + help + If you say yes here you get hardware monitoring support for + the integrated fan control module of the Delta AHE-50DC + Open19 power shelf. + + This driver can also be built as a module. If so, the module + will be called delta-ahe50dc-fan. + config SENSORS_FSP_3Y tristate "FSP/3Y-Power power supplies" help diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile index 0ed4d596a948..a56b2897288d 100644 --- a/drivers/hwmon/pmbus/Makefile +++ b/drivers/hwmon/pmbus/Makefile @@ -9,6 +9,7 @@ obj-$(CONFIG_SENSORS_ADM1266) += adm1266.o obj-$(CONFIG_SENSORS_ADM1275) += adm1275.o obj-$(CONFIG_SENSORS_BEL_PFE) += bel-pfe.o obj-$(CONFIG_SENSORS_BPA_RS600) += bpa-rs600.o +obj-$(CONFIG_SENSORS_DELTA_AHE50DC_FAN) += delta-ahe50dc-fan.o obj-$(CONFIG_SENSORS_FSP_3Y) += fsp-3y.o obj-$(CONFIG_SENSORS_IBM_CFFPS) += ibm-cffps.o obj-$(CONFIG_SENSORS_DPS920AB) += dps920ab.o diff --git a/drivers/hwmon/pmbus/delta-ahe50dc-fan.c b/drivers/hwmon/pmbus/delta-ahe50dc-fan.c new file mode 100644 index 000000000000..07b1e7c5f5f5 --- /dev/null +++ b/drivers/hwmon/pmbus/delta-ahe50dc-fan.c @@ -0,0 +1,84 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Delta AHE-50DC power shelf fan control module driver + * + * Copyright 2021 Zev Weiss <zev@bewilderbeest.net> + */ + +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/i2c.h> +#include <linux/pmbus.h> + +#include "pmbus.h" + +#define AHE50DC_PMBUS_READ_TEMP4 0xd0 + +static int ahe50dc_fan_read_word_data(struct i2c_client *client, int page, int phase, int reg) +{ + /* temp1 in (virtual) page 1 is remapped to mfr-specific temp4 */ + if (page == 1) { + if (reg == PMBUS_READ_TEMPERATURE_1) + return i2c_smbus_read_word_data(client, AHE50DC_PMBUS_READ_TEMP4); + return -EOPNOTSUPP; + } + return -ENODATA; +} + +static struct pmbus_driver_info ahe50dc_fan_info = { + .pages = 2, + .format[PSC_FAN] = direct, + .format[PSC_TEMPERATURE] = direct, + .format[PSC_VOLTAGE_IN] = direct, + .m[PSC_FAN] = 1, + .b[PSC_FAN] = 0, + .R[PSC_FAN] = 0, + .m[PSC_TEMPERATURE] = 1, + .b[PSC_TEMPERATURE] = 0, + .R[PSC_TEMPERATURE] = 1, + .m[PSC_VOLTAGE_IN] = 1, + .b[PSC_VOLTAGE_IN] = 0, + .R[PSC_VOLTAGE_IN] = 3, + .func[0] = PMBUS_HAVE_TEMP | PMBUS_HAVE_TEMP2 | PMBUS_HAVE_TEMP3 | + PMBUS_HAVE_VIN | PMBUS_HAVE_FAN12 | PMBUS_HAVE_FAN34 | + PMBUS_HAVE_STATUS_FAN12 | PMBUS_HAVE_STATUS_FAN34 | PMBUS_PAGE_VIRTUAL, + .func[1] = PMBUS_HAVE_TEMP | PMBUS_PAGE_VIRTUAL, + .read_word_data = ahe50dc_fan_read_word_data, +}; + +static struct pmbus_platform_data ahe50dc_fan_data = { + .flags = PMBUS_NO_CAPABILITY, +}; + +static int ahe50dc_fan_probe(struct i2c_client *client) +{ + client->dev.platform_data = &ahe50dc_fan_data; + return pmbus_do_probe(client, &ahe50dc_fan_info); +} + +static const struct i2c_device_id ahe50dc_fan_id[] = { + { "ahe50dc_fan" }, + { } +}; +MODULE_DEVICE_TABLE(i2c, ahe50dc_fan_id); + +static const struct of_device_id __maybe_unused ahe50dc_fan_of_match[] = { + { .compatible = "delta,ahe50dc-fan" }, + { } +}; +MODULE_DEVICE_TABLE(of, ahe50dc_fan_of_match); + +static struct i2c_driver ahe50dc_fan_driver = { + .driver = { + .name = "ahe50dc_fan", + .of_match_table = of_match_ptr(ahe50dc_fan_of_match), + }, + .probe_new = ahe50dc_fan_probe, + .id_table = ahe50dc_fan_id, +}; +module_i2c_driver(ahe50dc_fan_driver); + +MODULE_AUTHOR("Zev Weiss <zev@bewilderbeest.net>"); +MODULE_DESCRIPTION("Driver for Delta AHE-50DC power shelf fan control module"); +MODULE_LICENSE("GPL"); +MODULE_IMPORT_NS(PMBUS); -- 2.34.1 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v3 1/2] hwmon: (pmbus) Add Delta AHE-50DC fan control module driver 2021-12-07 7:15 ` Zev Weiss @ 2021-12-07 17:50 ` Guenter Roeck -1 siblings, 0 replies; 28+ messages in thread From: Guenter Roeck @ 2021-12-07 17:50 UTC (permalink / raw) To: Zev Weiss; +Cc: linux-hwmon, Jean Delvare, openbmc, linux-kernel On Mon, Dec 06, 2021 at 11:15:20PM -0800, Zev Weiss wrote: > This device is an integrated module of the Delta AHE-50DC Open19 power > shelf. For lack of proper documentation, this driver has been developed > referencing an existing (GPL) driver that was included in a code release > from LinkedIn [1]. It provides four fan speeds, four temperatures, and > one voltage reading, as well as a handful of warning and fault > indicators. > > [1] https://github.com/linkedin/o19-bmc-firmware/blob/master/meta-openbmc/meta-linkedin/meta-deltapower/recipes-kernel/fancontrol-mod/files/fancontrol.c > Hmm, that reference isn't really accurate anymore. I think it would be better to just say that the device was found to be PMBus compliant. > Signed-off-by: Zev Weiss <zev@bewilderbeest.net> > --- > MAINTAINERS | 6 ++ > drivers/hwmon/pmbus/Kconfig | 12 ++++ > drivers/hwmon/pmbus/Makefile | 1 + > drivers/hwmon/pmbus/delta-ahe50dc-fan.c | 84 +++++++++++++++++++++++++ > 4 files changed, 103 insertions(+) > create mode 100644 drivers/hwmon/pmbus/delta-ahe50dc-fan.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 0ac052200ecb..8bb7ba52d2f5 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -5425,6 +5425,12 @@ W: https://linuxtv.org > T: git git://linuxtv.org/media_tree.git > F: drivers/media/platform/sti/delta > > +DELTA AHE-50DC FAN CONTROL MODULE DRIVER > +M: Zev Weiss <zev@bewilderbeest.net> > +L: linux-hwmon@vger.kernel.org > +S: Maintained > +F: drivers/hwmon/pmbus/delta-ahe50dc-fan.c > + > DELTA DPS920AB PSU DRIVER > M: Robert Marko <robert.marko@sartura.hr> > L: linux-hwmon@vger.kernel.org > diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig > index ffb609cee3a4..937e1c2c11e7 100644 > --- a/drivers/hwmon/pmbus/Kconfig > +++ b/drivers/hwmon/pmbus/Kconfig > @@ -66,6 +66,18 @@ config SENSORS_BPA_RS600 > This driver can also be built as a module. If so, the module will > be called bpa-rs600. > > +config SENSORS_DELTA_AHE50DC_FAN > + tristate "Delta AHE-50DC fan control module" > + depends on I2C > + select REGMAP_I2C > + help > + If you say yes here you get hardware monitoring support for > + the integrated fan control module of the Delta AHE-50DC > + Open19 power shelf. > + > + This driver can also be built as a module. If so, the module > + will be called delta-ahe50dc-fan. > + > config SENSORS_FSP_3Y > tristate "FSP/3Y-Power power supplies" > help > diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile > index 0ed4d596a948..a56b2897288d 100644 > --- a/drivers/hwmon/pmbus/Makefile > +++ b/drivers/hwmon/pmbus/Makefile > @@ -9,6 +9,7 @@ obj-$(CONFIG_SENSORS_ADM1266) += adm1266.o > obj-$(CONFIG_SENSORS_ADM1275) += adm1275.o > obj-$(CONFIG_SENSORS_BEL_PFE) += bel-pfe.o > obj-$(CONFIG_SENSORS_BPA_RS600) += bpa-rs600.o > +obj-$(CONFIG_SENSORS_DELTA_AHE50DC_FAN) += delta-ahe50dc-fan.o > obj-$(CONFIG_SENSORS_FSP_3Y) += fsp-3y.o > obj-$(CONFIG_SENSORS_IBM_CFFPS) += ibm-cffps.o > obj-$(CONFIG_SENSORS_DPS920AB) += dps920ab.o > diff --git a/drivers/hwmon/pmbus/delta-ahe50dc-fan.c b/drivers/hwmon/pmbus/delta-ahe50dc-fan.c > new file mode 100644 > index 000000000000..07b1e7c5f5f5 > --- /dev/null > +++ b/drivers/hwmon/pmbus/delta-ahe50dc-fan.c > @@ -0,0 +1,84 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Delta AHE-50DC power shelf fan control module driver > + * > + * Copyright 2021 Zev Weiss <zev@bewilderbeest.net> > + */ > + > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/i2c.h> > +#include <linux/pmbus.h> Alphabetic include file order please. > + > +#include "pmbus.h" > + > +#define AHE50DC_PMBUS_READ_TEMP4 0xd0 > + > +static int ahe50dc_fan_read_word_data(struct i2c_client *client, int page, int phase, int reg) > +{ > + /* temp1 in (virtual) page 1 is remapped to mfr-specific temp4 */ > + if (page == 1) { > + if (reg == PMBUS_READ_TEMPERATURE_1) > + return i2c_smbus_read_word_data(client, AHE50DC_PMBUS_READ_TEMP4); > + return -EOPNOTSUPP; > + } > + return -ENODATA; > +} > + > +static struct pmbus_driver_info ahe50dc_fan_info = { > + .pages = 2, > + .format[PSC_FAN] = direct, > + .format[PSC_TEMPERATURE] = direct, > + .format[PSC_VOLTAGE_IN] = direct, > + .m[PSC_FAN] = 1, > + .b[PSC_FAN] = 0, > + .R[PSC_FAN] = 0, > + .m[PSC_TEMPERATURE] = 1, > + .b[PSC_TEMPERATURE] = 0, > + .R[PSC_TEMPERATURE] = 1, > + .m[PSC_VOLTAGE_IN] = 1, > + .b[PSC_VOLTAGE_IN] = 0, > + .R[PSC_VOLTAGE_IN] = 3, How did you determine the exponents ? The referenced driver doesn't seem to make a difference between voltage and temperature exponents (nor fan speed, which is a bit odd). > + .func[0] = PMBUS_HAVE_TEMP | PMBUS_HAVE_TEMP2 | PMBUS_HAVE_TEMP3 | > + PMBUS_HAVE_VIN | PMBUS_HAVE_FAN12 | PMBUS_HAVE_FAN34 | > + PMBUS_HAVE_STATUS_FAN12 | PMBUS_HAVE_STATUS_FAN34 | PMBUS_PAGE_VIRTUAL, > + .func[1] = PMBUS_HAVE_TEMP | PMBUS_PAGE_VIRTUAL, > + .read_word_data = ahe50dc_fan_read_word_data, > +}; > + > +static struct pmbus_platform_data ahe50dc_fan_data = { > + .flags = PMBUS_NO_CAPABILITY, > +}; Is that necessary ? It should only be used if trying to read CAPABILITY reports bad data. Thanks, Guenter > + > +static int ahe50dc_fan_probe(struct i2c_client *client) > +{ > + client->dev.platform_data = &ahe50dc_fan_data; > + return pmbus_do_probe(client, &ahe50dc_fan_info); > +} > + > +static const struct i2c_device_id ahe50dc_fan_id[] = { > + { "ahe50dc_fan" }, > + { } > +}; > +MODULE_DEVICE_TABLE(i2c, ahe50dc_fan_id); > + > +static const struct of_device_id __maybe_unused ahe50dc_fan_of_match[] = { > + { .compatible = "delta,ahe50dc-fan" }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, ahe50dc_fan_of_match); > + > +static struct i2c_driver ahe50dc_fan_driver = { > + .driver = { > + .name = "ahe50dc_fan", > + .of_match_table = of_match_ptr(ahe50dc_fan_of_match), > + }, > + .probe_new = ahe50dc_fan_probe, > + .id_table = ahe50dc_fan_id, > +}; > +module_i2c_driver(ahe50dc_fan_driver); > + > +MODULE_AUTHOR("Zev Weiss <zev@bewilderbeest.net>"); > +MODULE_DESCRIPTION("Driver for Delta AHE-50DC power shelf fan control module"); > +MODULE_LICENSE("GPL"); > +MODULE_IMPORT_NS(PMBUS); > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 1/2] hwmon: (pmbus) Add Delta AHE-50DC fan control module driver @ 2021-12-07 17:50 ` Guenter Roeck 0 siblings, 0 replies; 28+ messages in thread From: Guenter Roeck @ 2021-12-07 17:50 UTC (permalink / raw) To: Zev Weiss; +Cc: linux-hwmon, openbmc, Jean Delvare, linux-kernel On Mon, Dec 06, 2021 at 11:15:20PM -0800, Zev Weiss wrote: > This device is an integrated module of the Delta AHE-50DC Open19 power > shelf. For lack of proper documentation, this driver has been developed > referencing an existing (GPL) driver that was included in a code release > from LinkedIn [1]. It provides four fan speeds, four temperatures, and > one voltage reading, as well as a handful of warning and fault > indicators. > > [1] https://github.com/linkedin/o19-bmc-firmware/blob/master/meta-openbmc/meta-linkedin/meta-deltapower/recipes-kernel/fancontrol-mod/files/fancontrol.c > Hmm, that reference isn't really accurate anymore. I think it would be better to just say that the device was found to be PMBus compliant. > Signed-off-by: Zev Weiss <zev@bewilderbeest.net> > --- > MAINTAINERS | 6 ++ > drivers/hwmon/pmbus/Kconfig | 12 ++++ > drivers/hwmon/pmbus/Makefile | 1 + > drivers/hwmon/pmbus/delta-ahe50dc-fan.c | 84 +++++++++++++++++++++++++ > 4 files changed, 103 insertions(+) > create mode 100644 drivers/hwmon/pmbus/delta-ahe50dc-fan.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 0ac052200ecb..8bb7ba52d2f5 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -5425,6 +5425,12 @@ W: https://linuxtv.org > T: git git://linuxtv.org/media_tree.git > F: drivers/media/platform/sti/delta > > +DELTA AHE-50DC FAN CONTROL MODULE DRIVER > +M: Zev Weiss <zev@bewilderbeest.net> > +L: linux-hwmon@vger.kernel.org > +S: Maintained > +F: drivers/hwmon/pmbus/delta-ahe50dc-fan.c > + > DELTA DPS920AB PSU DRIVER > M: Robert Marko <robert.marko@sartura.hr> > L: linux-hwmon@vger.kernel.org > diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig > index ffb609cee3a4..937e1c2c11e7 100644 > --- a/drivers/hwmon/pmbus/Kconfig > +++ b/drivers/hwmon/pmbus/Kconfig > @@ -66,6 +66,18 @@ config SENSORS_BPA_RS600 > This driver can also be built as a module. If so, the module will > be called bpa-rs600. > > +config SENSORS_DELTA_AHE50DC_FAN > + tristate "Delta AHE-50DC fan control module" > + depends on I2C > + select REGMAP_I2C > + help > + If you say yes here you get hardware monitoring support for > + the integrated fan control module of the Delta AHE-50DC > + Open19 power shelf. > + > + This driver can also be built as a module. If so, the module > + will be called delta-ahe50dc-fan. > + > config SENSORS_FSP_3Y > tristate "FSP/3Y-Power power supplies" > help > diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile > index 0ed4d596a948..a56b2897288d 100644 > --- a/drivers/hwmon/pmbus/Makefile > +++ b/drivers/hwmon/pmbus/Makefile > @@ -9,6 +9,7 @@ obj-$(CONFIG_SENSORS_ADM1266) += adm1266.o > obj-$(CONFIG_SENSORS_ADM1275) += adm1275.o > obj-$(CONFIG_SENSORS_BEL_PFE) += bel-pfe.o > obj-$(CONFIG_SENSORS_BPA_RS600) += bpa-rs600.o > +obj-$(CONFIG_SENSORS_DELTA_AHE50DC_FAN) += delta-ahe50dc-fan.o > obj-$(CONFIG_SENSORS_FSP_3Y) += fsp-3y.o > obj-$(CONFIG_SENSORS_IBM_CFFPS) += ibm-cffps.o > obj-$(CONFIG_SENSORS_DPS920AB) += dps920ab.o > diff --git a/drivers/hwmon/pmbus/delta-ahe50dc-fan.c b/drivers/hwmon/pmbus/delta-ahe50dc-fan.c > new file mode 100644 > index 000000000000..07b1e7c5f5f5 > --- /dev/null > +++ b/drivers/hwmon/pmbus/delta-ahe50dc-fan.c > @@ -0,0 +1,84 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Delta AHE-50DC power shelf fan control module driver > + * > + * Copyright 2021 Zev Weiss <zev@bewilderbeest.net> > + */ > + > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/i2c.h> > +#include <linux/pmbus.h> Alphabetic include file order please. > + > +#include "pmbus.h" > + > +#define AHE50DC_PMBUS_READ_TEMP4 0xd0 > + > +static int ahe50dc_fan_read_word_data(struct i2c_client *client, int page, int phase, int reg) > +{ > + /* temp1 in (virtual) page 1 is remapped to mfr-specific temp4 */ > + if (page == 1) { > + if (reg == PMBUS_READ_TEMPERATURE_1) > + return i2c_smbus_read_word_data(client, AHE50DC_PMBUS_READ_TEMP4); > + return -EOPNOTSUPP; > + } > + return -ENODATA; > +} > + > +static struct pmbus_driver_info ahe50dc_fan_info = { > + .pages = 2, > + .format[PSC_FAN] = direct, > + .format[PSC_TEMPERATURE] = direct, > + .format[PSC_VOLTAGE_IN] = direct, > + .m[PSC_FAN] = 1, > + .b[PSC_FAN] = 0, > + .R[PSC_FAN] = 0, > + .m[PSC_TEMPERATURE] = 1, > + .b[PSC_TEMPERATURE] = 0, > + .R[PSC_TEMPERATURE] = 1, > + .m[PSC_VOLTAGE_IN] = 1, > + .b[PSC_VOLTAGE_IN] = 0, > + .R[PSC_VOLTAGE_IN] = 3, How did you determine the exponents ? The referenced driver doesn't seem to make a difference between voltage and temperature exponents (nor fan speed, which is a bit odd). > + .func[0] = PMBUS_HAVE_TEMP | PMBUS_HAVE_TEMP2 | PMBUS_HAVE_TEMP3 | > + PMBUS_HAVE_VIN | PMBUS_HAVE_FAN12 | PMBUS_HAVE_FAN34 | > + PMBUS_HAVE_STATUS_FAN12 | PMBUS_HAVE_STATUS_FAN34 | PMBUS_PAGE_VIRTUAL, > + .func[1] = PMBUS_HAVE_TEMP | PMBUS_PAGE_VIRTUAL, > + .read_word_data = ahe50dc_fan_read_word_data, > +}; > + > +static struct pmbus_platform_data ahe50dc_fan_data = { > + .flags = PMBUS_NO_CAPABILITY, > +}; Is that necessary ? It should only be used if trying to read CAPABILITY reports bad data. Thanks, Guenter > + > +static int ahe50dc_fan_probe(struct i2c_client *client) > +{ > + client->dev.platform_data = &ahe50dc_fan_data; > + return pmbus_do_probe(client, &ahe50dc_fan_info); > +} > + > +static const struct i2c_device_id ahe50dc_fan_id[] = { > + { "ahe50dc_fan" }, > + { } > +}; > +MODULE_DEVICE_TABLE(i2c, ahe50dc_fan_id); > + > +static const struct of_device_id __maybe_unused ahe50dc_fan_of_match[] = { > + { .compatible = "delta,ahe50dc-fan" }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, ahe50dc_fan_of_match); > + > +static struct i2c_driver ahe50dc_fan_driver = { > + .driver = { > + .name = "ahe50dc_fan", > + .of_match_table = of_match_ptr(ahe50dc_fan_of_match), > + }, > + .probe_new = ahe50dc_fan_probe, > + .id_table = ahe50dc_fan_id, > +}; > +module_i2c_driver(ahe50dc_fan_driver); > + > +MODULE_AUTHOR("Zev Weiss <zev@bewilderbeest.net>"); > +MODULE_DESCRIPTION("Driver for Delta AHE-50DC power shelf fan control module"); > +MODULE_LICENSE("GPL"); > +MODULE_IMPORT_NS(PMBUS); > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 1/2] hwmon: (pmbus) Add Delta AHE-50DC fan control module driver 2021-12-07 17:50 ` Guenter Roeck @ 2021-12-07 19:22 ` Zev Weiss -1 siblings, 0 replies; 28+ messages in thread From: Zev Weiss @ 2021-12-07 19:22 UTC (permalink / raw) To: Guenter Roeck; +Cc: linux-hwmon, Jean Delvare, openbmc, linux-kernel On Tue, Dec 07, 2021 at 09:50:15AM PST, Guenter Roeck wrote: >On Mon, Dec 06, 2021 at 11:15:20PM -0800, Zev Weiss wrote: >> This device is an integrated module of the Delta AHE-50DC Open19 power >> shelf. For lack of proper documentation, this driver has been developed >> referencing an existing (GPL) driver that was included in a code release >> from LinkedIn [1]. It provides four fan speeds, four temperatures, and >> one voltage reading, as well as a handful of warning and fault >> indicators. >> >> [1] https://github.com/linkedin/o19-bmc-firmware/blob/master/meta-openbmc/meta-linkedin/meta-deltapower/recipes-kernel/fancontrol-mod/files/fancontrol.c >> > >Hmm, that reference isn't really accurate anymore. I think it would be >better to just say that the device was found to be PMBus compliant. Sure, will do. > >> Signed-off-by: Zev Weiss <zev@bewilderbeest.net> >> --- >> MAINTAINERS | 6 ++ >> drivers/hwmon/pmbus/Kconfig | 12 ++++ >> drivers/hwmon/pmbus/Makefile | 1 + >> drivers/hwmon/pmbus/delta-ahe50dc-fan.c | 84 +++++++++++++++++++++++++ >> 4 files changed, 103 insertions(+) >> create mode 100644 drivers/hwmon/pmbus/delta-ahe50dc-fan.c >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index 0ac052200ecb..8bb7ba52d2f5 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -5425,6 +5425,12 @@ W: https://linuxtv.org >> T: git git://linuxtv.org/media_tree.git >> F: drivers/media/platform/sti/delta >> >> +DELTA AHE-50DC FAN CONTROL MODULE DRIVER >> +M: Zev Weiss <zev@bewilderbeest.net> >> +L: linux-hwmon@vger.kernel.org >> +S: Maintained >> +F: drivers/hwmon/pmbus/delta-ahe50dc-fan.c >> + >> DELTA DPS920AB PSU DRIVER >> M: Robert Marko <robert.marko@sartura.hr> >> L: linux-hwmon@vger.kernel.org >> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig >> index ffb609cee3a4..937e1c2c11e7 100644 >> --- a/drivers/hwmon/pmbus/Kconfig >> +++ b/drivers/hwmon/pmbus/Kconfig >> @@ -66,6 +66,18 @@ config SENSORS_BPA_RS600 >> This driver can also be built as a module. If so, the module will >> be called bpa-rs600. >> >> +config SENSORS_DELTA_AHE50DC_FAN >> + tristate "Delta AHE-50DC fan control module" >> + depends on I2C >> + select REGMAP_I2C And I realize I neglected to drop the depends & select lines here when moving this bit into the pmbus directory... >> + help >> + If you say yes here you get hardware monitoring support for >> + the integrated fan control module of the Delta AHE-50DC >> + Open19 power shelf. >> + >> + This driver can also be built as a module. If so, the module >> + will be called delta-ahe50dc-fan. >> + >> config SENSORS_FSP_3Y >> tristate "FSP/3Y-Power power supplies" >> help >> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile >> index 0ed4d596a948..a56b2897288d 100644 >> --- a/drivers/hwmon/pmbus/Makefile >> +++ b/drivers/hwmon/pmbus/Makefile >> @@ -9,6 +9,7 @@ obj-$(CONFIG_SENSORS_ADM1266) += adm1266.o >> obj-$(CONFIG_SENSORS_ADM1275) += adm1275.o >> obj-$(CONFIG_SENSORS_BEL_PFE) += bel-pfe.o >> obj-$(CONFIG_SENSORS_BPA_RS600) += bpa-rs600.o >> +obj-$(CONFIG_SENSORS_DELTA_AHE50DC_FAN) += delta-ahe50dc-fan.o >> obj-$(CONFIG_SENSORS_FSP_3Y) += fsp-3y.o >> obj-$(CONFIG_SENSORS_IBM_CFFPS) += ibm-cffps.o >> obj-$(CONFIG_SENSORS_DPS920AB) += dps920ab.o >> diff --git a/drivers/hwmon/pmbus/delta-ahe50dc-fan.c b/drivers/hwmon/pmbus/delta-ahe50dc-fan.c >> new file mode 100644 >> index 000000000000..07b1e7c5f5f5 >> --- /dev/null >> +++ b/drivers/hwmon/pmbus/delta-ahe50dc-fan.c >> @@ -0,0 +1,84 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Delta AHE-50DC power shelf fan control module driver >> + * >> + * Copyright 2021 Zev Weiss <zev@bewilderbeest.net> >> + */ >> + >> +#include <linux/kernel.h> >> +#include <linux/module.h> >> +#include <linux/i2c.h> >> +#include <linux/pmbus.h> > >Alphabetic include file order please. Ack, will fix. > >> + >> +#include "pmbus.h" >> + >> +#define AHE50DC_PMBUS_READ_TEMP4 0xd0 >> + >> +static int ahe50dc_fan_read_word_data(struct i2c_client *client, int page, int phase, int reg) >> +{ >> + /* temp1 in (virtual) page 1 is remapped to mfr-specific temp4 */ >> + if (page == 1) { >> + if (reg == PMBUS_READ_TEMPERATURE_1) >> + return i2c_smbus_read_word_data(client, AHE50DC_PMBUS_READ_TEMP4); >> + return -EOPNOTSUPP; >> + } >> + return -ENODATA; >> +} >> + >> +static struct pmbus_driver_info ahe50dc_fan_info = { >> + .pages = 2, >> + .format[PSC_FAN] = direct, >> + .format[PSC_TEMPERATURE] = direct, >> + .format[PSC_VOLTAGE_IN] = direct, >> + .m[PSC_FAN] = 1, >> + .b[PSC_FAN] = 0, >> + .R[PSC_FAN] = 0, >> + .m[PSC_TEMPERATURE] = 1, >> + .b[PSC_TEMPERATURE] = 0, >> + .R[PSC_TEMPERATURE] = 1, >> + .m[PSC_VOLTAGE_IN] = 1, >> + .b[PSC_VOLTAGE_IN] = 0, >> + .R[PSC_VOLTAGE_IN] = 3, > >How did you determine the exponents ? The referenced driver >doesn't seem to make a difference between voltage and temperature >exponents (nor fan speed, which is a bit odd). Lacking documentation, the honest answer here is that I just sort of eyeballed it. However, after doing so, I dug through the code dump a bit more and found some userspace unit-conversion bits that appear to confirm what I arrived at: https://github.com/linkedin/o19-bmc-firmware/blob/master/meta-openbmc/meta-linkedin/meta-deltapower/recipes-deltapower/platform-lib/files/powershelf/powershelf_fan.c (Lines 107, and 153/161/169/177.) > >> + .func[0] = PMBUS_HAVE_TEMP | PMBUS_HAVE_TEMP2 | PMBUS_HAVE_TEMP3 | >> + PMBUS_HAVE_VIN | PMBUS_HAVE_FAN12 | PMBUS_HAVE_FAN34 | >> + PMBUS_HAVE_STATUS_FAN12 | PMBUS_HAVE_STATUS_FAN34 | PMBUS_PAGE_VIRTUAL, >> + .func[1] = PMBUS_HAVE_TEMP | PMBUS_PAGE_VIRTUAL, >> + .read_word_data = ahe50dc_fan_read_word_data, >> +}; >> + >> +static struct pmbus_platform_data ahe50dc_fan_data = { >> + .flags = PMBUS_NO_CAPABILITY, >> +}; > >Is that necessary ? It should only be used if trying to read CAPABILITY >reports bad data. Unfortunately it does seem to be needed -- CAPABILITY returns 0xff, but despite having bit 7 set the device mangles all subsequent transactions by returning bad PEC bytes if we enable I2C_CLIENT_PEC. Actually, none of the fields in that CAPABILITY value seem to accurately reflect the device's observed behavior; I'm pretty sure 0xff is just what it returns when it doesn't support something (I think the bad PECs it sends are all 0xff too). I'll add an explanatory comment in v4. Thanks for the review! Zev ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 1/2] hwmon: (pmbus) Add Delta AHE-50DC fan control module driver @ 2021-12-07 19:22 ` Zev Weiss 0 siblings, 0 replies; 28+ messages in thread From: Zev Weiss @ 2021-12-07 19:22 UTC (permalink / raw) To: Guenter Roeck; +Cc: linux-hwmon, openbmc, Jean Delvare, linux-kernel On Tue, Dec 07, 2021 at 09:50:15AM PST, Guenter Roeck wrote: >On Mon, Dec 06, 2021 at 11:15:20PM -0800, Zev Weiss wrote: >> This device is an integrated module of the Delta AHE-50DC Open19 power >> shelf. For lack of proper documentation, this driver has been developed >> referencing an existing (GPL) driver that was included in a code release >> from LinkedIn [1]. It provides four fan speeds, four temperatures, and >> one voltage reading, as well as a handful of warning and fault >> indicators. >> >> [1] https://github.com/linkedin/o19-bmc-firmware/blob/master/meta-openbmc/meta-linkedin/meta-deltapower/recipes-kernel/fancontrol-mod/files/fancontrol.c >> > >Hmm, that reference isn't really accurate anymore. I think it would be >better to just say that the device was found to be PMBus compliant. Sure, will do. > >> Signed-off-by: Zev Weiss <zev@bewilderbeest.net> >> --- >> MAINTAINERS | 6 ++ >> drivers/hwmon/pmbus/Kconfig | 12 ++++ >> drivers/hwmon/pmbus/Makefile | 1 + >> drivers/hwmon/pmbus/delta-ahe50dc-fan.c | 84 +++++++++++++++++++++++++ >> 4 files changed, 103 insertions(+) >> create mode 100644 drivers/hwmon/pmbus/delta-ahe50dc-fan.c >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index 0ac052200ecb..8bb7ba52d2f5 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -5425,6 +5425,12 @@ W: https://linuxtv.org >> T: git git://linuxtv.org/media_tree.git >> F: drivers/media/platform/sti/delta >> >> +DELTA AHE-50DC FAN CONTROL MODULE DRIVER >> +M: Zev Weiss <zev@bewilderbeest.net> >> +L: linux-hwmon@vger.kernel.org >> +S: Maintained >> +F: drivers/hwmon/pmbus/delta-ahe50dc-fan.c >> + >> DELTA DPS920AB PSU DRIVER >> M: Robert Marko <robert.marko@sartura.hr> >> L: linux-hwmon@vger.kernel.org >> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig >> index ffb609cee3a4..937e1c2c11e7 100644 >> --- a/drivers/hwmon/pmbus/Kconfig >> +++ b/drivers/hwmon/pmbus/Kconfig >> @@ -66,6 +66,18 @@ config SENSORS_BPA_RS600 >> This driver can also be built as a module. If so, the module will >> be called bpa-rs600. >> >> +config SENSORS_DELTA_AHE50DC_FAN >> + tristate "Delta AHE-50DC fan control module" >> + depends on I2C >> + select REGMAP_I2C And I realize I neglected to drop the depends & select lines here when moving this bit into the pmbus directory... >> + help >> + If you say yes here you get hardware monitoring support for >> + the integrated fan control module of the Delta AHE-50DC >> + Open19 power shelf. >> + >> + This driver can also be built as a module. If so, the module >> + will be called delta-ahe50dc-fan. >> + >> config SENSORS_FSP_3Y >> tristate "FSP/3Y-Power power supplies" >> help >> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile >> index 0ed4d596a948..a56b2897288d 100644 >> --- a/drivers/hwmon/pmbus/Makefile >> +++ b/drivers/hwmon/pmbus/Makefile >> @@ -9,6 +9,7 @@ obj-$(CONFIG_SENSORS_ADM1266) += adm1266.o >> obj-$(CONFIG_SENSORS_ADM1275) += adm1275.o >> obj-$(CONFIG_SENSORS_BEL_PFE) += bel-pfe.o >> obj-$(CONFIG_SENSORS_BPA_RS600) += bpa-rs600.o >> +obj-$(CONFIG_SENSORS_DELTA_AHE50DC_FAN) += delta-ahe50dc-fan.o >> obj-$(CONFIG_SENSORS_FSP_3Y) += fsp-3y.o >> obj-$(CONFIG_SENSORS_IBM_CFFPS) += ibm-cffps.o >> obj-$(CONFIG_SENSORS_DPS920AB) += dps920ab.o >> diff --git a/drivers/hwmon/pmbus/delta-ahe50dc-fan.c b/drivers/hwmon/pmbus/delta-ahe50dc-fan.c >> new file mode 100644 >> index 000000000000..07b1e7c5f5f5 >> --- /dev/null >> +++ b/drivers/hwmon/pmbus/delta-ahe50dc-fan.c >> @@ -0,0 +1,84 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Delta AHE-50DC power shelf fan control module driver >> + * >> + * Copyright 2021 Zev Weiss <zev@bewilderbeest.net> >> + */ >> + >> +#include <linux/kernel.h> >> +#include <linux/module.h> >> +#include <linux/i2c.h> >> +#include <linux/pmbus.h> > >Alphabetic include file order please. Ack, will fix. > >> + >> +#include "pmbus.h" >> + >> +#define AHE50DC_PMBUS_READ_TEMP4 0xd0 >> + >> +static int ahe50dc_fan_read_word_data(struct i2c_client *client, int page, int phase, int reg) >> +{ >> + /* temp1 in (virtual) page 1 is remapped to mfr-specific temp4 */ >> + if (page == 1) { >> + if (reg == PMBUS_READ_TEMPERATURE_1) >> + return i2c_smbus_read_word_data(client, AHE50DC_PMBUS_READ_TEMP4); >> + return -EOPNOTSUPP; >> + } >> + return -ENODATA; >> +} >> + >> +static struct pmbus_driver_info ahe50dc_fan_info = { >> + .pages = 2, >> + .format[PSC_FAN] = direct, >> + .format[PSC_TEMPERATURE] = direct, >> + .format[PSC_VOLTAGE_IN] = direct, >> + .m[PSC_FAN] = 1, >> + .b[PSC_FAN] = 0, >> + .R[PSC_FAN] = 0, >> + .m[PSC_TEMPERATURE] = 1, >> + .b[PSC_TEMPERATURE] = 0, >> + .R[PSC_TEMPERATURE] = 1, >> + .m[PSC_VOLTAGE_IN] = 1, >> + .b[PSC_VOLTAGE_IN] = 0, >> + .R[PSC_VOLTAGE_IN] = 3, > >How did you determine the exponents ? The referenced driver >doesn't seem to make a difference between voltage and temperature >exponents (nor fan speed, which is a bit odd). Lacking documentation, the honest answer here is that I just sort of eyeballed it. However, after doing so, I dug through the code dump a bit more and found some userspace unit-conversion bits that appear to confirm what I arrived at: https://github.com/linkedin/o19-bmc-firmware/blob/master/meta-openbmc/meta-linkedin/meta-deltapower/recipes-deltapower/platform-lib/files/powershelf/powershelf_fan.c (Lines 107, and 153/161/169/177.) > >> + .func[0] = PMBUS_HAVE_TEMP | PMBUS_HAVE_TEMP2 | PMBUS_HAVE_TEMP3 | >> + PMBUS_HAVE_VIN | PMBUS_HAVE_FAN12 | PMBUS_HAVE_FAN34 | >> + PMBUS_HAVE_STATUS_FAN12 | PMBUS_HAVE_STATUS_FAN34 | PMBUS_PAGE_VIRTUAL, >> + .func[1] = PMBUS_HAVE_TEMP | PMBUS_PAGE_VIRTUAL, >> + .read_word_data = ahe50dc_fan_read_word_data, >> +}; >> + >> +static struct pmbus_platform_data ahe50dc_fan_data = { >> + .flags = PMBUS_NO_CAPABILITY, >> +}; > >Is that necessary ? It should only be used if trying to read CAPABILITY >reports bad data. Unfortunately it does seem to be needed -- CAPABILITY returns 0xff, but despite having bit 7 set the device mangles all subsequent transactions by returning bad PEC bytes if we enable I2C_CLIENT_PEC. Actually, none of the fields in that CAPABILITY value seem to accurately reflect the device's observed behavior; I'm pretty sure 0xff is just what it returns when it doesn't support something (I think the bad PECs it sends are all 0xff too). I'll add an explanatory comment in v4. Thanks for the review! Zev ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 1/2] hwmon: (pmbus) Add Delta AHE-50DC fan control module driver 2021-12-07 19:22 ` Zev Weiss @ 2021-12-07 19:44 ` Guenter Roeck -1 siblings, 0 replies; 28+ messages in thread From: Guenter Roeck @ 2021-12-07 19:44 UTC (permalink / raw) To: Zev Weiss; +Cc: linux-hwmon, Jean Delvare, openbmc, linux-kernel On 12/7/21 11:22 AM, Zev Weiss wrote: > On Tue, Dec 07, 2021 at 09:50:15AM PST, Guenter Roeck wrote: >> On Mon, Dec 06, 2021 at 11:15:20PM -0800, Zev Weiss wrote: >>> This device is an integrated module of the Delta AHE-50DC Open19 power >>> shelf. For lack of proper documentation, this driver has been developed >>> referencing an existing (GPL) driver that was included in a code release >>> from LinkedIn [1]. It provides four fan speeds, four temperatures, and >>> one voltage reading, as well as a handful of warning and fault >>> indicators. >>> >>> [1] https://github.com/linkedin/o19-bmc-firmware/blob/master/meta-openbmc/meta-linkedin/meta-deltapower/recipes-kernel/fancontrol-mod/files/fancontrol.c >>> >> >> Hmm, that reference isn't really accurate anymore. I think it would be >> better to just say that the device was found to be PMBus compliant. > > Sure, will do. > >> >>> Signed-off-by: Zev Weiss <zev@bewilderbeest.net> >>> --- >>> MAINTAINERS | 6 ++ >>> drivers/hwmon/pmbus/Kconfig | 12 ++++ >>> drivers/hwmon/pmbus/Makefile | 1 + >>> drivers/hwmon/pmbus/delta-ahe50dc-fan.c | 84 +++++++++++++++++++++++++ >>> 4 files changed, 103 insertions(+) >>> create mode 100644 drivers/hwmon/pmbus/delta-ahe50dc-fan.c >>> >>> diff --git a/MAINTAINERS b/MAINTAINERS >>> index 0ac052200ecb..8bb7ba52d2f5 100644 >>> --- a/MAINTAINERS >>> +++ b/MAINTAINERS >>> @@ -5425,6 +5425,12 @@ W: https://linuxtv.org >>> T: git git://linuxtv.org/media_tree.git >>> F: drivers/media/platform/sti/delta >>> >>> +DELTA AHE-50DC FAN CONTROL MODULE DRIVER >>> +M: Zev Weiss <zev@bewilderbeest.net> >>> +L: linux-hwmon@vger.kernel.org >>> +S: Maintained >>> +F: drivers/hwmon/pmbus/delta-ahe50dc-fan.c >>> + >>> DELTA DPS920AB PSU DRIVER >>> M: Robert Marko <robert.marko@sartura.hr> >>> L: linux-hwmon@vger.kernel.org >>> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig >>> index ffb609cee3a4..937e1c2c11e7 100644 >>> --- a/drivers/hwmon/pmbus/Kconfig >>> +++ b/drivers/hwmon/pmbus/Kconfig >>> @@ -66,6 +66,18 @@ config SENSORS_BPA_RS600 >>> This driver can also be built as a module. If so, the module will >>> be called bpa-rs600. >>> >>> +config SENSORS_DELTA_AHE50DC_FAN >>> + tristate "Delta AHE-50DC fan control module" >>> + depends on I2C >>> + select REGMAP_I2C > > And I realize I neglected to drop the depends & select lines here when moving this bit into the pmbus directory... > >>> + help >>> + If you say yes here you get hardware monitoring support for >>> + the integrated fan control module of the Delta AHE-50DC >>> + Open19 power shelf. >>> + >>> + This driver can also be built as a module. If so, the module >>> + will be called delta-ahe50dc-fan. >>> + >>> config SENSORS_FSP_3Y >>> tristate "FSP/3Y-Power power supplies" >>> help >>> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile >>> index 0ed4d596a948..a56b2897288d 100644 >>> --- a/drivers/hwmon/pmbus/Makefile >>> +++ b/drivers/hwmon/pmbus/Makefile >>> @@ -9,6 +9,7 @@ obj-$(CONFIG_SENSORS_ADM1266) += adm1266.o >>> obj-$(CONFIG_SENSORS_ADM1275) += adm1275.o >>> obj-$(CONFIG_SENSORS_BEL_PFE) += bel-pfe.o >>> obj-$(CONFIG_SENSORS_BPA_RS600) += bpa-rs600.o >>> +obj-$(CONFIG_SENSORS_DELTA_AHE50DC_FAN) += delta-ahe50dc-fan.o >>> obj-$(CONFIG_SENSORS_FSP_3Y) += fsp-3y.o >>> obj-$(CONFIG_SENSORS_IBM_CFFPS) += ibm-cffps.o >>> obj-$(CONFIG_SENSORS_DPS920AB) += dps920ab.o >>> diff --git a/drivers/hwmon/pmbus/delta-ahe50dc-fan.c b/drivers/hwmon/pmbus/delta-ahe50dc-fan.c >>> new file mode 100644 >>> index 000000000000..07b1e7c5f5f5 >>> --- /dev/null >>> +++ b/drivers/hwmon/pmbus/delta-ahe50dc-fan.c >>> @@ -0,0 +1,84 @@ >>> +// SPDX-License-Identifier: GPL-2.0 >>> +/* >>> + * Delta AHE-50DC power shelf fan control module driver >>> + * >>> + * Copyright 2021 Zev Weiss <zev@bewilderbeest.net> >>> + */ >>> + >>> +#include <linux/kernel.h> >>> +#include <linux/module.h> >>> +#include <linux/i2c.h> >>> +#include <linux/pmbus.h> >> >> Alphabetic include file order please. > > Ack, will fix. > >> >>> + >>> +#include "pmbus.h" >>> + >>> +#define AHE50DC_PMBUS_READ_TEMP4 0xd0 >>> + >>> +static int ahe50dc_fan_read_word_data(struct i2c_client *client, int page, int phase, int reg) >>> +{ >>> + /* temp1 in (virtual) page 1 is remapped to mfr-specific temp4 */ >>> + if (page == 1) { >>> + if (reg == PMBUS_READ_TEMPERATURE_1) >>> + return i2c_smbus_read_word_data(client, AHE50DC_PMBUS_READ_TEMP4); >>> + return -EOPNOTSUPP; >>> + } >>> + return -ENODATA; >>> +} >>> + >>> +static struct pmbus_driver_info ahe50dc_fan_info = { >>> + .pages = 2, >>> + .format[PSC_FAN] = direct, >>> + .format[PSC_TEMPERATURE] = direct, >>> + .format[PSC_VOLTAGE_IN] = direct, >>> + .m[PSC_FAN] = 1, >>> + .b[PSC_FAN] = 0, >>> + .R[PSC_FAN] = 0, >>> + .m[PSC_TEMPERATURE] = 1, >>> + .b[PSC_TEMPERATURE] = 0, >>> + .R[PSC_TEMPERATURE] = 1, >>> + .m[PSC_VOLTAGE_IN] = 1, >>> + .b[PSC_VOLTAGE_IN] = 0, >>> + .R[PSC_VOLTAGE_IN] = 3, >> >> How did you determine the exponents ? The referenced driver >> doesn't seem to make a difference between voltage and temperature >> exponents (nor fan speed, which is a bit odd). > > Lacking documentation, the honest answer here is that I just sort of eyeballed it. However, after doing so, I dug through the code dump a bit more and found some userspace unit-conversion bits that appear to confirm what I arrived at: > > https://github.com/linkedin/o19-bmc-firmware/blob/master/meta-openbmc/meta-linkedin/meta-deltapower/recipes-deltapower/platform-lib/files/powershelf/powershelf_fan.c > > (Lines 107, and 153/161/169/177.) > Brr. Well, why use a standard API/ABI if one can invent a non-standard one. Can you check this with real hardware, by any chance ? >> >>> + .func[0] = PMBUS_HAVE_TEMP | PMBUS_HAVE_TEMP2 | PMBUS_HAVE_TEMP3 | >>> + PMBUS_HAVE_VIN | PMBUS_HAVE_FAN12 | PMBUS_HAVE_FAN34 | >>> + PMBUS_HAVE_STATUS_FAN12 | PMBUS_HAVE_STATUS_FAN34 | PMBUS_PAGE_VIRTUAL, >>> + .func[1] = PMBUS_HAVE_TEMP | PMBUS_PAGE_VIRTUAL, >>> + .read_word_data = ahe50dc_fan_read_word_data, >>> +}; >>> + >>> +static struct pmbus_platform_data ahe50dc_fan_data = { >>> + .flags = PMBUS_NO_CAPABILITY, >>> +}; >> >> Is that necessary ? It should only be used if trying to read CAPABILITY >> reports bad data. > > Unfortunately it does seem to be needed -- CAPABILITY returns 0xff, but despite having bit 7 set the device mangles all subsequent transactions by returning bad PEC bytes if we enable I2C_CLIENT_PEC. Actually, none of the fields in that CAPABILITY value seem to accurately reflect the device's observed behavior; I'm pretty sure 0xff is just what it returns when it doesn't support something (I think the bad PECs it sends are all 0xff too). > > I'll add an explanatory comment in v4. > > Please do. Thanks, Guenter ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 1/2] hwmon: (pmbus) Add Delta AHE-50DC fan control module driver @ 2021-12-07 19:44 ` Guenter Roeck 0 siblings, 0 replies; 28+ messages in thread From: Guenter Roeck @ 2021-12-07 19:44 UTC (permalink / raw) To: Zev Weiss; +Cc: linux-hwmon, openbmc, Jean Delvare, linux-kernel On 12/7/21 11:22 AM, Zev Weiss wrote: > On Tue, Dec 07, 2021 at 09:50:15AM PST, Guenter Roeck wrote: >> On Mon, Dec 06, 2021 at 11:15:20PM -0800, Zev Weiss wrote: >>> This device is an integrated module of the Delta AHE-50DC Open19 power >>> shelf. For lack of proper documentation, this driver has been developed >>> referencing an existing (GPL) driver that was included in a code release >>> from LinkedIn [1]. It provides four fan speeds, four temperatures, and >>> one voltage reading, as well as a handful of warning and fault >>> indicators. >>> >>> [1] https://github.com/linkedin/o19-bmc-firmware/blob/master/meta-openbmc/meta-linkedin/meta-deltapower/recipes-kernel/fancontrol-mod/files/fancontrol.c >>> >> >> Hmm, that reference isn't really accurate anymore. I think it would be >> better to just say that the device was found to be PMBus compliant. > > Sure, will do. > >> >>> Signed-off-by: Zev Weiss <zev@bewilderbeest.net> >>> --- >>> MAINTAINERS | 6 ++ >>> drivers/hwmon/pmbus/Kconfig | 12 ++++ >>> drivers/hwmon/pmbus/Makefile | 1 + >>> drivers/hwmon/pmbus/delta-ahe50dc-fan.c | 84 +++++++++++++++++++++++++ >>> 4 files changed, 103 insertions(+) >>> create mode 100644 drivers/hwmon/pmbus/delta-ahe50dc-fan.c >>> >>> diff --git a/MAINTAINERS b/MAINTAINERS >>> index 0ac052200ecb..8bb7ba52d2f5 100644 >>> --- a/MAINTAINERS >>> +++ b/MAINTAINERS >>> @@ -5425,6 +5425,12 @@ W: https://linuxtv.org >>> T: git git://linuxtv.org/media_tree.git >>> F: drivers/media/platform/sti/delta >>> >>> +DELTA AHE-50DC FAN CONTROL MODULE DRIVER >>> +M: Zev Weiss <zev@bewilderbeest.net> >>> +L: linux-hwmon@vger.kernel.org >>> +S: Maintained >>> +F: drivers/hwmon/pmbus/delta-ahe50dc-fan.c >>> + >>> DELTA DPS920AB PSU DRIVER >>> M: Robert Marko <robert.marko@sartura.hr> >>> L: linux-hwmon@vger.kernel.org >>> diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig >>> index ffb609cee3a4..937e1c2c11e7 100644 >>> --- a/drivers/hwmon/pmbus/Kconfig >>> +++ b/drivers/hwmon/pmbus/Kconfig >>> @@ -66,6 +66,18 @@ config SENSORS_BPA_RS600 >>> This driver can also be built as a module. If so, the module will >>> be called bpa-rs600. >>> >>> +config SENSORS_DELTA_AHE50DC_FAN >>> + tristate "Delta AHE-50DC fan control module" >>> + depends on I2C >>> + select REGMAP_I2C > > And I realize I neglected to drop the depends & select lines here when moving this bit into the pmbus directory... > >>> + help >>> + If you say yes here you get hardware monitoring support for >>> + the integrated fan control module of the Delta AHE-50DC >>> + Open19 power shelf. >>> + >>> + This driver can also be built as a module. If so, the module >>> + will be called delta-ahe50dc-fan. >>> + >>> config SENSORS_FSP_3Y >>> tristate "FSP/3Y-Power power supplies" >>> help >>> diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile >>> index 0ed4d596a948..a56b2897288d 100644 >>> --- a/drivers/hwmon/pmbus/Makefile >>> +++ b/drivers/hwmon/pmbus/Makefile >>> @@ -9,6 +9,7 @@ obj-$(CONFIG_SENSORS_ADM1266) += adm1266.o >>> obj-$(CONFIG_SENSORS_ADM1275) += adm1275.o >>> obj-$(CONFIG_SENSORS_BEL_PFE) += bel-pfe.o >>> obj-$(CONFIG_SENSORS_BPA_RS600) += bpa-rs600.o >>> +obj-$(CONFIG_SENSORS_DELTA_AHE50DC_FAN) += delta-ahe50dc-fan.o >>> obj-$(CONFIG_SENSORS_FSP_3Y) += fsp-3y.o >>> obj-$(CONFIG_SENSORS_IBM_CFFPS) += ibm-cffps.o >>> obj-$(CONFIG_SENSORS_DPS920AB) += dps920ab.o >>> diff --git a/drivers/hwmon/pmbus/delta-ahe50dc-fan.c b/drivers/hwmon/pmbus/delta-ahe50dc-fan.c >>> new file mode 100644 >>> index 000000000000..07b1e7c5f5f5 >>> --- /dev/null >>> +++ b/drivers/hwmon/pmbus/delta-ahe50dc-fan.c >>> @@ -0,0 +1,84 @@ >>> +// SPDX-License-Identifier: GPL-2.0 >>> +/* >>> + * Delta AHE-50DC power shelf fan control module driver >>> + * >>> + * Copyright 2021 Zev Weiss <zev@bewilderbeest.net> >>> + */ >>> + >>> +#include <linux/kernel.h> >>> +#include <linux/module.h> >>> +#include <linux/i2c.h> >>> +#include <linux/pmbus.h> >> >> Alphabetic include file order please. > > Ack, will fix. > >> >>> + >>> +#include "pmbus.h" >>> + >>> +#define AHE50DC_PMBUS_READ_TEMP4 0xd0 >>> + >>> +static int ahe50dc_fan_read_word_data(struct i2c_client *client, int page, int phase, int reg) >>> +{ >>> + /* temp1 in (virtual) page 1 is remapped to mfr-specific temp4 */ >>> + if (page == 1) { >>> + if (reg == PMBUS_READ_TEMPERATURE_1) >>> + return i2c_smbus_read_word_data(client, AHE50DC_PMBUS_READ_TEMP4); >>> + return -EOPNOTSUPP; >>> + } >>> + return -ENODATA; >>> +} >>> + >>> +static struct pmbus_driver_info ahe50dc_fan_info = { >>> + .pages = 2, >>> + .format[PSC_FAN] = direct, >>> + .format[PSC_TEMPERATURE] = direct, >>> + .format[PSC_VOLTAGE_IN] = direct, >>> + .m[PSC_FAN] = 1, >>> + .b[PSC_FAN] = 0, >>> + .R[PSC_FAN] = 0, >>> + .m[PSC_TEMPERATURE] = 1, >>> + .b[PSC_TEMPERATURE] = 0, >>> + .R[PSC_TEMPERATURE] = 1, >>> + .m[PSC_VOLTAGE_IN] = 1, >>> + .b[PSC_VOLTAGE_IN] = 0, >>> + .R[PSC_VOLTAGE_IN] = 3, >> >> How did you determine the exponents ? The referenced driver >> doesn't seem to make a difference between voltage and temperature >> exponents (nor fan speed, which is a bit odd). > > Lacking documentation, the honest answer here is that I just sort of eyeballed it. However, after doing so, I dug through the code dump a bit more and found some userspace unit-conversion bits that appear to confirm what I arrived at: > > https://github.com/linkedin/o19-bmc-firmware/blob/master/meta-openbmc/meta-linkedin/meta-deltapower/recipes-deltapower/platform-lib/files/powershelf/powershelf_fan.c > > (Lines 107, and 153/161/169/177.) > Brr. Well, why use a standard API/ABI if one can invent a non-standard one. Can you check this with real hardware, by any chance ? >> >>> + .func[0] = PMBUS_HAVE_TEMP | PMBUS_HAVE_TEMP2 | PMBUS_HAVE_TEMP3 | >>> + PMBUS_HAVE_VIN | PMBUS_HAVE_FAN12 | PMBUS_HAVE_FAN34 | >>> + PMBUS_HAVE_STATUS_FAN12 | PMBUS_HAVE_STATUS_FAN34 | PMBUS_PAGE_VIRTUAL, >>> + .func[1] = PMBUS_HAVE_TEMP | PMBUS_PAGE_VIRTUAL, >>> + .read_word_data = ahe50dc_fan_read_word_data, >>> +}; >>> + >>> +static struct pmbus_platform_data ahe50dc_fan_data = { >>> + .flags = PMBUS_NO_CAPABILITY, >>> +}; >> >> Is that necessary ? It should only be used if trying to read CAPABILITY >> reports bad data. > > Unfortunately it does seem to be needed -- CAPABILITY returns 0xff, but despite having bit 7 set the device mangles all subsequent transactions by returning bad PEC bytes if we enable I2C_CLIENT_PEC. Actually, none of the fields in that CAPABILITY value seem to accurately reflect the device's observed behavior; I'm pretty sure 0xff is just what it returns when it doesn't support something (I think the bad PECs it sends are all 0xff too). > > I'll add an explanatory comment in v4. > > Please do. Thanks, Guenter ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 1/2] hwmon: (pmbus) Add Delta AHE-50DC fan control module driver 2021-12-07 19:44 ` Guenter Roeck @ 2021-12-07 21:53 ` Zev Weiss -1 siblings, 0 replies; 28+ messages in thread From: Zev Weiss @ 2021-12-07 21:53 UTC (permalink / raw) To: Guenter Roeck; +Cc: linux-hwmon, Jean Delvare, openbmc, linux-kernel On Tue, Dec 07, 2021 at 11:44:01AM PST, Guenter Roeck wrote: >On 12/7/21 11:22 AM, Zev Weiss wrote: >>On Tue, Dec 07, 2021 at 09:50:15AM PST, Guenter Roeck wrote: >>>On Mon, Dec 06, 2021 at 11:15:20PM -0800, Zev Weiss wrote: >>>>This device is an integrated module of the Delta AHE-50DC Open19 power >>>>shelf. For lack of proper documentation, this driver has been developed >>>>referencing an existing (GPL) driver that was included in a code release >>>>from LinkedIn [1]. It provides four fan speeds, four temperatures, and >>>>one voltage reading, as well as a handful of warning and fault >>>>indicators. >>>> >>>>[1] https://github.com/linkedin/o19-bmc-firmware/blob/master/meta-openbmc/meta-linkedin/meta-deltapower/recipes-kernel/fancontrol-mod/files/fancontrol.c >>>> >>> >>>Hmm, that reference isn't really accurate anymore. I think it would be >>>better to just say that the device was found to be PMBus compliant. >> >>Sure, will do. >> > > Makes me wonder: How do you know that the referenced driver is for > Delta AHE-50DC ? We'd been waiting for the source code for the software it ships with for a while, and were finally provided with that repo; everything I've observed from the factory software is consistent with the code in that driver. A sampling: # no modinfo command available, but... root@bmc-oob:~# strings -n8 /lib/modules/4.1.51-deltapower/extra/fancontrol.ko | head fancontrol license=GPL description=FANCTRL Driver author=Ping Mao <pmao@linkedin.com> alias=i2c:fancontrol depends=i2c_dev_sysfs vermagic=4.1.51-deltapower mod_unload ARMv5 p2v8 fancontrol status_word bit 2: Temparature Warning root@bmc-oob:~# cd /sys/bus/i2c/drivers/fancontrol/8-0030 root@bmc-oob:/sys/bus/i2c/drivers/fancontrol/8-0030# grep . fan* temp* vin fan1_2_status:0 fan1_speed:7860 fan2_speed:7860 fan3_4_status:0 fan3_speed:7680 fan4_speed:7560 temperature1:292 temperature2:278 temperature3:286 temperature4:302 vin:12251 >>> >>>>Signed-off-by: Zev Weiss <zev@bewilderbeest.net> >>>>--- >>>> MAINTAINERS | 6 ++ >>>> drivers/hwmon/pmbus/Kconfig | 12 ++++ >>>> drivers/hwmon/pmbus/Makefile | 1 + >>>> drivers/hwmon/pmbus/delta-ahe50dc-fan.c | 84 +++++++++++++++++++++++++ >>>> 4 files changed, 103 insertions(+) >>>> create mode 100644 drivers/hwmon/pmbus/delta-ahe50dc-fan.c >>>> >>>>diff --git a/MAINTAINERS b/MAINTAINERS >>>>index 0ac052200ecb..8bb7ba52d2f5 100644 >>>>--- a/MAINTAINERS >>>>+++ b/MAINTAINERS >>>>@@ -5425,6 +5425,12 @@ W: https://linuxtv.org >>>> T: git git://linuxtv.org/media_tree.git >>>> F: drivers/media/platform/sti/delta >>>> >>>>+DELTA AHE-50DC FAN CONTROL MODULE DRIVER >>>>+M: Zev Weiss <zev@bewilderbeest.net> >>>>+L: linux-hwmon@vger.kernel.org >>>>+S: Maintained >>>>+F: drivers/hwmon/pmbus/delta-ahe50dc-fan.c >>>>+ >>>> DELTA DPS920AB PSU DRIVER >>>> M: Robert Marko <robert.marko@sartura.hr> >>>> L: linux-hwmon@vger.kernel.org >>>>diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig >>>>index ffb609cee3a4..937e1c2c11e7 100644 >>>>--- a/drivers/hwmon/pmbus/Kconfig >>>>+++ b/drivers/hwmon/pmbus/Kconfig >>>>@@ -66,6 +66,18 @@ config SENSORS_BPA_RS600 >>>> This driver can also be built as a module. If so, the module will >>>> be called bpa-rs600. >>>> >>>>+config SENSORS_DELTA_AHE50DC_FAN >>>>+ tristate "Delta AHE-50DC fan control module" >>>>+ depends on I2C >>>>+ select REGMAP_I2C >> >>And I realize I neglected to drop the depends & select lines here when moving this bit into the pmbus directory... >> >>>>+ help >>>>+ If you say yes here you get hardware monitoring support for >>>>+ the integrated fan control module of the Delta AHE-50DC >>>>+ Open19 power shelf. >>>>+ >>>>+ This driver can also be built as a module. If so, the module >>>>+ will be called delta-ahe50dc-fan. >>>>+ >>>> config SENSORS_FSP_3Y >>>> tristate "FSP/3Y-Power power supplies" >>>> help >>>>diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile >>>>index 0ed4d596a948..a56b2897288d 100644 >>>>--- a/drivers/hwmon/pmbus/Makefile >>>>+++ b/drivers/hwmon/pmbus/Makefile >>>>@@ -9,6 +9,7 @@ obj-$(CONFIG_SENSORS_ADM1266) += adm1266.o >>>> obj-$(CONFIG_SENSORS_ADM1275) += adm1275.o >>>> obj-$(CONFIG_SENSORS_BEL_PFE) += bel-pfe.o >>>> obj-$(CONFIG_SENSORS_BPA_RS600) += bpa-rs600.o >>>>+obj-$(CONFIG_SENSORS_DELTA_AHE50DC_FAN) += delta-ahe50dc-fan.o >>>> obj-$(CONFIG_SENSORS_FSP_3Y) += fsp-3y.o >>>> obj-$(CONFIG_SENSORS_IBM_CFFPS) += ibm-cffps.o >>>> obj-$(CONFIG_SENSORS_DPS920AB) += dps920ab.o >>>>diff --git a/drivers/hwmon/pmbus/delta-ahe50dc-fan.c b/drivers/hwmon/pmbus/delta-ahe50dc-fan.c >>>>new file mode 100644 >>>>index 000000000000..07b1e7c5f5f5 >>>>--- /dev/null >>>>+++ b/drivers/hwmon/pmbus/delta-ahe50dc-fan.c >>>>@@ -0,0 +1,84 @@ >>>>+// SPDX-License-Identifier: GPL-2.0 >>>>+/* >>>>+ * Delta AHE-50DC power shelf fan control module driver >>>>+ * >>>>+ * Copyright 2021 Zev Weiss <zev@bewilderbeest.net> >>>>+ */ >>>>+ >>>>+#include <linux/kernel.h> >>>>+#include <linux/module.h> >>>>+#include <linux/i2c.h> >>>>+#include <linux/pmbus.h> >>> >>>Alphabetic include file order please. >> >>Ack, will fix. >> >>> >>>>+ >>>>+#include "pmbus.h" >>>>+ >>>>+#define AHE50DC_PMBUS_READ_TEMP4 0xd0 >>>>+ >>>>+static int ahe50dc_fan_read_word_data(struct i2c_client *client, int page, int phase, int reg) >>>>+{ >>>>+ /* temp1 in (virtual) page 1 is remapped to mfr-specific temp4 */ >>>>+ if (page == 1) { >>>>+ if (reg == PMBUS_READ_TEMPERATURE_1) >>>>+ return i2c_smbus_read_word_data(client, AHE50DC_PMBUS_READ_TEMP4); >>>>+ return -EOPNOTSUPP; >>>>+ } >>>>+ return -ENODATA; >>>>+} >>>>+ >>>>+static struct pmbus_driver_info ahe50dc_fan_info = { >>>>+ .pages = 2, >>>>+ .format[PSC_FAN] = direct, >>>>+ .format[PSC_TEMPERATURE] = direct, >>>>+ .format[PSC_VOLTAGE_IN] = direct, >>>>+ .m[PSC_FAN] = 1, >>>>+ .b[PSC_FAN] = 0, >>>>+ .R[PSC_FAN] = 0, >>>>+ .m[PSC_TEMPERATURE] = 1, >>>>+ .b[PSC_TEMPERATURE] = 0, >>>>+ .R[PSC_TEMPERATURE] = 1, >>>>+ .m[PSC_VOLTAGE_IN] = 1, >>>>+ .b[PSC_VOLTAGE_IN] = 0, >>>>+ .R[PSC_VOLTAGE_IN] = 3, >>> >>>How did you determine the exponents ? The referenced driver >>>doesn't seem to make a difference between voltage and temperature >>>exponents (nor fan speed, which is a bit odd). >> >>Lacking documentation, the honest answer here is that I just sort of eyeballed it. However, after doing so, I dug through the code dump a bit more and found some userspace unit-conversion bits that appear to confirm what I arrived at: >> >>https://github.com/linkedin/o19-bmc-firmware/blob/master/meta-openbmc/meta-linkedin/meta-deltapower/recipes-deltapower/platform-lib/files/powershelf/powershelf_fan.c >> >>(Lines 107, and 153/161/169/177.) >> > >Brr. Well, why use a standard API/ABI if one can invent a non-standard one. > It's...not the only bit of gratuitous wheel-reinvention we've come across in this particular system. >Can you check this with real hardware, by any chance ? > If you mean running that code on it, yes -- here's the userspace utility that invokes that library routine: root@bmc-oob:~# fan-util.sh fan1 speed: 7860 RPM fan2 speed: 7860 RPM fan3 speed: 7620 RPM fan4 speed: 7560 RPM temperature1: 29.20 C temperature2: 27.80 C temperature3: 28.50 C temperature4: 30.20 C vin_undervolt_fault: no overtemperature_warning: no fan_fault: no fan_warning: no fan_status: ok That fan-util.sh script being this: https://github.com/linkedin/o19-bmc-firmware/blob/master/meta-openbmc/meta-linkedin/meta-deltapower/recipes-deltapower/liutils/files/fan-util.sh which invokes this: https://github.com/linkedin/o19-bmc-firmware/blob/master/meta-openbmc/meta-linkedin/meta-deltapower/recipes-deltapower/liutils/files/fan-ctrl/fan-util.c (which calls into the routine in powershelf_fan.c linked above). Zev ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 1/2] hwmon: (pmbus) Add Delta AHE-50DC fan control module driver @ 2021-12-07 21:53 ` Zev Weiss 0 siblings, 0 replies; 28+ messages in thread From: Zev Weiss @ 2021-12-07 21:53 UTC (permalink / raw) To: Guenter Roeck; +Cc: linux-hwmon, openbmc, Jean Delvare, linux-kernel On Tue, Dec 07, 2021 at 11:44:01AM PST, Guenter Roeck wrote: >On 12/7/21 11:22 AM, Zev Weiss wrote: >>On Tue, Dec 07, 2021 at 09:50:15AM PST, Guenter Roeck wrote: >>>On Mon, Dec 06, 2021 at 11:15:20PM -0800, Zev Weiss wrote: >>>>This device is an integrated module of the Delta AHE-50DC Open19 power >>>>shelf. For lack of proper documentation, this driver has been developed >>>>referencing an existing (GPL) driver that was included in a code release >>>>from LinkedIn [1]. It provides four fan speeds, four temperatures, and >>>>one voltage reading, as well as a handful of warning and fault >>>>indicators. >>>> >>>>[1] https://github.com/linkedin/o19-bmc-firmware/blob/master/meta-openbmc/meta-linkedin/meta-deltapower/recipes-kernel/fancontrol-mod/files/fancontrol.c >>>> >>> >>>Hmm, that reference isn't really accurate anymore. I think it would be >>>better to just say that the device was found to be PMBus compliant. >> >>Sure, will do. >> > > Makes me wonder: How do you know that the referenced driver is for > Delta AHE-50DC ? We'd been waiting for the source code for the software it ships with for a while, and were finally provided with that repo; everything I've observed from the factory software is consistent with the code in that driver. A sampling: # no modinfo command available, but... root@bmc-oob:~# strings -n8 /lib/modules/4.1.51-deltapower/extra/fancontrol.ko | head fancontrol license=GPL description=FANCTRL Driver author=Ping Mao <pmao@linkedin.com> alias=i2c:fancontrol depends=i2c_dev_sysfs vermagic=4.1.51-deltapower mod_unload ARMv5 p2v8 fancontrol status_word bit 2: Temparature Warning root@bmc-oob:~# cd /sys/bus/i2c/drivers/fancontrol/8-0030 root@bmc-oob:/sys/bus/i2c/drivers/fancontrol/8-0030# grep . fan* temp* vin fan1_2_status:0 fan1_speed:7860 fan2_speed:7860 fan3_4_status:0 fan3_speed:7680 fan4_speed:7560 temperature1:292 temperature2:278 temperature3:286 temperature4:302 vin:12251 >>> >>>>Signed-off-by: Zev Weiss <zev@bewilderbeest.net> >>>>--- >>>> MAINTAINERS | 6 ++ >>>> drivers/hwmon/pmbus/Kconfig | 12 ++++ >>>> drivers/hwmon/pmbus/Makefile | 1 + >>>> drivers/hwmon/pmbus/delta-ahe50dc-fan.c | 84 +++++++++++++++++++++++++ >>>> 4 files changed, 103 insertions(+) >>>> create mode 100644 drivers/hwmon/pmbus/delta-ahe50dc-fan.c >>>> >>>>diff --git a/MAINTAINERS b/MAINTAINERS >>>>index 0ac052200ecb..8bb7ba52d2f5 100644 >>>>--- a/MAINTAINERS >>>>+++ b/MAINTAINERS >>>>@@ -5425,6 +5425,12 @@ W: https://linuxtv.org >>>> T: git git://linuxtv.org/media_tree.git >>>> F: drivers/media/platform/sti/delta >>>> >>>>+DELTA AHE-50DC FAN CONTROL MODULE DRIVER >>>>+M: Zev Weiss <zev@bewilderbeest.net> >>>>+L: linux-hwmon@vger.kernel.org >>>>+S: Maintained >>>>+F: drivers/hwmon/pmbus/delta-ahe50dc-fan.c >>>>+ >>>> DELTA DPS920AB PSU DRIVER >>>> M: Robert Marko <robert.marko@sartura.hr> >>>> L: linux-hwmon@vger.kernel.org >>>>diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig >>>>index ffb609cee3a4..937e1c2c11e7 100644 >>>>--- a/drivers/hwmon/pmbus/Kconfig >>>>+++ b/drivers/hwmon/pmbus/Kconfig >>>>@@ -66,6 +66,18 @@ config SENSORS_BPA_RS600 >>>> This driver can also be built as a module. If so, the module will >>>> be called bpa-rs600. >>>> >>>>+config SENSORS_DELTA_AHE50DC_FAN >>>>+ tristate "Delta AHE-50DC fan control module" >>>>+ depends on I2C >>>>+ select REGMAP_I2C >> >>And I realize I neglected to drop the depends & select lines here when moving this bit into the pmbus directory... >> >>>>+ help >>>>+ If you say yes here you get hardware monitoring support for >>>>+ the integrated fan control module of the Delta AHE-50DC >>>>+ Open19 power shelf. >>>>+ >>>>+ This driver can also be built as a module. If so, the module >>>>+ will be called delta-ahe50dc-fan. >>>>+ >>>> config SENSORS_FSP_3Y >>>> tristate "FSP/3Y-Power power supplies" >>>> help >>>>diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile >>>>index 0ed4d596a948..a56b2897288d 100644 >>>>--- a/drivers/hwmon/pmbus/Makefile >>>>+++ b/drivers/hwmon/pmbus/Makefile >>>>@@ -9,6 +9,7 @@ obj-$(CONFIG_SENSORS_ADM1266) += adm1266.o >>>> obj-$(CONFIG_SENSORS_ADM1275) += adm1275.o >>>> obj-$(CONFIG_SENSORS_BEL_PFE) += bel-pfe.o >>>> obj-$(CONFIG_SENSORS_BPA_RS600) += bpa-rs600.o >>>>+obj-$(CONFIG_SENSORS_DELTA_AHE50DC_FAN) += delta-ahe50dc-fan.o >>>> obj-$(CONFIG_SENSORS_FSP_3Y) += fsp-3y.o >>>> obj-$(CONFIG_SENSORS_IBM_CFFPS) += ibm-cffps.o >>>> obj-$(CONFIG_SENSORS_DPS920AB) += dps920ab.o >>>>diff --git a/drivers/hwmon/pmbus/delta-ahe50dc-fan.c b/drivers/hwmon/pmbus/delta-ahe50dc-fan.c >>>>new file mode 100644 >>>>index 000000000000..07b1e7c5f5f5 >>>>--- /dev/null >>>>+++ b/drivers/hwmon/pmbus/delta-ahe50dc-fan.c >>>>@@ -0,0 +1,84 @@ >>>>+// SPDX-License-Identifier: GPL-2.0 >>>>+/* >>>>+ * Delta AHE-50DC power shelf fan control module driver >>>>+ * >>>>+ * Copyright 2021 Zev Weiss <zev@bewilderbeest.net> >>>>+ */ >>>>+ >>>>+#include <linux/kernel.h> >>>>+#include <linux/module.h> >>>>+#include <linux/i2c.h> >>>>+#include <linux/pmbus.h> >>> >>>Alphabetic include file order please. >> >>Ack, will fix. >> >>> >>>>+ >>>>+#include "pmbus.h" >>>>+ >>>>+#define AHE50DC_PMBUS_READ_TEMP4 0xd0 >>>>+ >>>>+static int ahe50dc_fan_read_word_data(struct i2c_client *client, int page, int phase, int reg) >>>>+{ >>>>+ /* temp1 in (virtual) page 1 is remapped to mfr-specific temp4 */ >>>>+ if (page == 1) { >>>>+ if (reg == PMBUS_READ_TEMPERATURE_1) >>>>+ return i2c_smbus_read_word_data(client, AHE50DC_PMBUS_READ_TEMP4); >>>>+ return -EOPNOTSUPP; >>>>+ } >>>>+ return -ENODATA; >>>>+} >>>>+ >>>>+static struct pmbus_driver_info ahe50dc_fan_info = { >>>>+ .pages = 2, >>>>+ .format[PSC_FAN] = direct, >>>>+ .format[PSC_TEMPERATURE] = direct, >>>>+ .format[PSC_VOLTAGE_IN] = direct, >>>>+ .m[PSC_FAN] = 1, >>>>+ .b[PSC_FAN] = 0, >>>>+ .R[PSC_FAN] = 0, >>>>+ .m[PSC_TEMPERATURE] = 1, >>>>+ .b[PSC_TEMPERATURE] = 0, >>>>+ .R[PSC_TEMPERATURE] = 1, >>>>+ .m[PSC_VOLTAGE_IN] = 1, >>>>+ .b[PSC_VOLTAGE_IN] = 0, >>>>+ .R[PSC_VOLTAGE_IN] = 3, >>> >>>How did you determine the exponents ? The referenced driver >>>doesn't seem to make a difference between voltage and temperature >>>exponents (nor fan speed, which is a bit odd). >> >>Lacking documentation, the honest answer here is that I just sort of eyeballed it. However, after doing so, I dug through the code dump a bit more and found some userspace unit-conversion bits that appear to confirm what I arrived at: >> >>https://github.com/linkedin/o19-bmc-firmware/blob/master/meta-openbmc/meta-linkedin/meta-deltapower/recipes-deltapower/platform-lib/files/powershelf/powershelf_fan.c >> >>(Lines 107, and 153/161/169/177.) >> > >Brr. Well, why use a standard API/ABI if one can invent a non-standard one. > It's...not the only bit of gratuitous wheel-reinvention we've come across in this particular system. >Can you check this with real hardware, by any chance ? > If you mean running that code on it, yes -- here's the userspace utility that invokes that library routine: root@bmc-oob:~# fan-util.sh fan1 speed: 7860 RPM fan2 speed: 7860 RPM fan3 speed: 7620 RPM fan4 speed: 7560 RPM temperature1: 29.20 C temperature2: 27.80 C temperature3: 28.50 C temperature4: 30.20 C vin_undervolt_fault: no overtemperature_warning: no fan_fault: no fan_warning: no fan_status: ok That fan-util.sh script being this: https://github.com/linkedin/o19-bmc-firmware/blob/master/meta-openbmc/meta-linkedin/meta-deltapower/recipes-deltapower/liutils/files/fan-util.sh which invokes this: https://github.com/linkedin/o19-bmc-firmware/blob/master/meta-openbmc/meta-linkedin/meta-deltapower/recipes-deltapower/liutils/files/fan-ctrl/fan-util.c (which calls into the routine in powershelf_fan.c linked above). Zev ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 1/2] hwmon: (pmbus) Add Delta AHE-50DC fan control module driver 2021-12-07 21:53 ` Zev Weiss @ 2021-12-07 22:39 ` Zev Weiss -1 siblings, 0 replies; 28+ messages in thread From: Zev Weiss @ 2021-12-07 22:39 UTC (permalink / raw) To: Guenter Roeck; +Cc: linux-hwmon, Jean Delvare, openbmc, linux-kernel On Tue, Dec 07, 2021 at 01:53:44PM PST, Zev Weiss wrote: >On Tue, Dec 07, 2021 at 11:44:01AM PST, Guenter Roeck wrote: >>On 12/7/21 11:22 AM, Zev Weiss wrote: >>>On Tue, Dec 07, 2021 at 09:50:15AM PST, Guenter Roeck wrote: >>>>On Mon, Dec 06, 2021 at 11:15:20PM -0800, Zev Weiss wrote: >>>>>This device is an integrated module of the Delta AHE-50DC Open19 power >>>>>shelf. For lack of proper documentation, this driver has been developed >>>>>referencing an existing (GPL) driver that was included in a code release >>>>>from LinkedIn [1]. It provides four fan speeds, four temperatures, and >>>>>one voltage reading, as well as a handful of warning and fault >>>>>indicators. >>>>> >>>>>[1] https://github.com/linkedin/o19-bmc-firmware/blob/master/meta-openbmc/meta-linkedin/meta-deltapower/recipes-kernel/fancontrol-mod/files/fancontrol.c >>>>> >>>> >>>>Hmm, that reference isn't really accurate anymore. I think it would be >>>>better to just say that the device was found to be PMBus compliant. >>> >>>Sure, will do. >>> >> >>Makes me wonder: How do you know that the referenced driver is for >>Delta AHE-50DC ? > >We'd been waiting for the source code for the software it ships with >for a while, and were finally provided with that repo; everything I've >observed from the factory software is consistent with the code in that >driver. A sampling: > > # no modinfo command available, but... > root@bmc-oob:~# strings -n8 /lib/modules/4.1.51-deltapower/extra/fancontrol.ko | head > fancontrol > license=GPL > description=FANCTRL Driver > author=Ping Mao <pmao@linkedin.com> > alias=i2c:fancontrol > depends=i2c_dev_sysfs > vermagic=4.1.51-deltapower mod_unload ARMv5 p2v8 > fancontrol > status_word > bit 2: Temparature Warning > root@bmc-oob:~# cd /sys/bus/i2c/drivers/fancontrol/8-0030 > root@bmc-oob:/sys/bus/i2c/drivers/fancontrol/8-0030# grep . fan* temp* vin > fan1_2_status:0 > fan1_speed:7860 > fan2_speed:7860 > fan3_4_status:0 > fan3_speed:7680 > fan4_speed:7560 > temperature1:292 > temperature2:278 > temperature3:286 > temperature4:302 > vin:12251 > > >>>> >>>>>Signed-off-by: Zev Weiss <zev@bewilderbeest.net> >>>>>--- >>>>> MAINTAINERS | 6 ++ >>>>> drivers/hwmon/pmbus/Kconfig | 12 ++++ >>>>> drivers/hwmon/pmbus/Makefile | 1 + >>>>> drivers/hwmon/pmbus/delta-ahe50dc-fan.c | 84 +++++++++++++++++++++++++ >>>>> 4 files changed, 103 insertions(+) >>>>> create mode 100644 drivers/hwmon/pmbus/delta-ahe50dc-fan.c >>>>> >>>>>diff --git a/MAINTAINERS b/MAINTAINERS >>>>>index 0ac052200ecb..8bb7ba52d2f5 100644 >>>>>--- a/MAINTAINERS >>>>>+++ b/MAINTAINERS >>>>>@@ -5425,6 +5425,12 @@ W: https://linuxtv.org >>>>> T: git git://linuxtv.org/media_tree.git >>>>> F: drivers/media/platform/sti/delta >>>>> >>>>>+DELTA AHE-50DC FAN CONTROL MODULE DRIVER >>>>>+M: Zev Weiss <zev@bewilderbeest.net> >>>>>+L: linux-hwmon@vger.kernel.org >>>>>+S: Maintained >>>>>+F: drivers/hwmon/pmbus/delta-ahe50dc-fan.c >>>>>+ >>>>> DELTA DPS920AB PSU DRIVER >>>>> M: Robert Marko <robert.marko@sartura.hr> >>>>> L: linux-hwmon@vger.kernel.org >>>>>diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig >>>>>index ffb609cee3a4..937e1c2c11e7 100644 >>>>>--- a/drivers/hwmon/pmbus/Kconfig >>>>>+++ b/drivers/hwmon/pmbus/Kconfig >>>>>@@ -66,6 +66,18 @@ config SENSORS_BPA_RS600 >>>>> This driver can also be built as a module. If so, the module will >>>>> be called bpa-rs600. >>>>> >>>>>+config SENSORS_DELTA_AHE50DC_FAN >>>>>+ tristate "Delta AHE-50DC fan control module" >>>>>+ depends on I2C >>>>>+ select REGMAP_I2C >>> >>>And I realize I neglected to drop the depends & select lines here when moving this bit into the pmbus directory... >>> >>>>>+ help >>>>>+ If you say yes here you get hardware monitoring support for >>>>>+ the integrated fan control module of the Delta AHE-50DC >>>>>+ Open19 power shelf. >>>>>+ >>>>>+ This driver can also be built as a module. If so, the module >>>>>+ will be called delta-ahe50dc-fan. >>>>>+ >>>>> config SENSORS_FSP_3Y >>>>> tristate "FSP/3Y-Power power supplies" >>>>> help >>>>>diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile >>>>>index 0ed4d596a948..a56b2897288d 100644 >>>>>--- a/drivers/hwmon/pmbus/Makefile >>>>>+++ b/drivers/hwmon/pmbus/Makefile >>>>>@@ -9,6 +9,7 @@ obj-$(CONFIG_SENSORS_ADM1266) += adm1266.o >>>>> obj-$(CONFIG_SENSORS_ADM1275) += adm1275.o >>>>> obj-$(CONFIG_SENSORS_BEL_PFE) += bel-pfe.o >>>>> obj-$(CONFIG_SENSORS_BPA_RS600) += bpa-rs600.o >>>>>+obj-$(CONFIG_SENSORS_DELTA_AHE50DC_FAN) += delta-ahe50dc-fan.o >>>>> obj-$(CONFIG_SENSORS_FSP_3Y) += fsp-3y.o >>>>> obj-$(CONFIG_SENSORS_IBM_CFFPS) += ibm-cffps.o >>>>> obj-$(CONFIG_SENSORS_DPS920AB) += dps920ab.o >>>>>diff --git a/drivers/hwmon/pmbus/delta-ahe50dc-fan.c b/drivers/hwmon/pmbus/delta-ahe50dc-fan.c >>>>>new file mode 100644 >>>>>index 000000000000..07b1e7c5f5f5 >>>>>--- /dev/null >>>>>+++ b/drivers/hwmon/pmbus/delta-ahe50dc-fan.c >>>>>@@ -0,0 +1,84 @@ >>>>>+// SPDX-License-Identifier: GPL-2.0 >>>>>+/* >>>>>+ * Delta AHE-50DC power shelf fan control module driver >>>>>+ * >>>>>+ * Copyright 2021 Zev Weiss <zev@bewilderbeest.net> >>>>>+ */ >>>>>+ >>>>>+#include <linux/kernel.h> >>>>>+#include <linux/module.h> >>>>>+#include <linux/i2c.h> >>>>>+#include <linux/pmbus.h> >>>> >>>>Alphabetic include file order please. >>> >>>Ack, will fix. >>> >>>> >>>>>+ >>>>>+#include "pmbus.h" >>>>>+ >>>>>+#define AHE50DC_PMBUS_READ_TEMP4 0xd0 >>>>>+ >>>>>+static int ahe50dc_fan_read_word_data(struct i2c_client *client, int page, int phase, int reg) >>>>>+{ >>>>>+ /* temp1 in (virtual) page 1 is remapped to mfr-specific temp4 */ >>>>>+ if (page == 1) { >>>>>+ if (reg == PMBUS_READ_TEMPERATURE_1) >>>>>+ return i2c_smbus_read_word_data(client, AHE50DC_PMBUS_READ_TEMP4); >>>>>+ return -EOPNOTSUPP; >>>>>+ } >>>>>+ return -ENODATA; >>>>>+} >>>>>+ >>>>>+static struct pmbus_driver_info ahe50dc_fan_info = { >>>>>+ .pages = 2, >>>>>+ .format[PSC_FAN] = direct, >>>>>+ .format[PSC_TEMPERATURE] = direct, >>>>>+ .format[PSC_VOLTAGE_IN] = direct, >>>>>+ .m[PSC_FAN] = 1, >>>>>+ .b[PSC_FAN] = 0, >>>>>+ .R[PSC_FAN] = 0, >>>>>+ .m[PSC_TEMPERATURE] = 1, >>>>>+ .b[PSC_TEMPERATURE] = 0, >>>>>+ .R[PSC_TEMPERATURE] = 1, >>>>>+ .m[PSC_VOLTAGE_IN] = 1, >>>>>+ .b[PSC_VOLTAGE_IN] = 0, >>>>>+ .R[PSC_VOLTAGE_IN] = 3, >>>> >>>>How did you determine the exponents ? The referenced driver >>>>doesn't seem to make a difference between voltage and temperature >>>>exponents (nor fan speed, which is a bit odd). >>> >>>Lacking documentation, the honest answer here is that I just sort of eyeballed it. However, after doing so, I dug through the code dump a bit more and found some userspace unit-conversion bits that appear to confirm what I arrived at: >>> >>>https://github.com/linkedin/o19-bmc-firmware/blob/master/meta-openbmc/meta-linkedin/meta-deltapower/recipes-deltapower/platform-lib/files/powershelf/powershelf_fan.c >>> >>>(Lines 107, and 153/161/169/177.) >>> >> >>Brr. Well, why use a standard API/ABI if one can invent a non-standard one. >> > >It's...not the only bit of gratuitous wheel-reinvention we've come >across in this particular system. > >>Can you check this with real hardware, by any chance ? >> > >If you mean running that code on it, yes -- here's the userspace >utility that invokes that library routine: > > root@bmc-oob:~# fan-util.sh > fan1 speed: 7860 RPM > fan2 speed: 7860 RPM > fan3 speed: 7620 RPM > fan4 speed: 7560 RPM > temperature1: 29.20 C > temperature2: 27.80 C > temperature3: 28.50 C > temperature4: 30.20 C > vin_undervolt_fault: no > overtemperature_warning: no > fan_fault: no > fan_warning: no > fan_status: ok > >That fan-util.sh script being this: > >https://github.com/linkedin/o19-bmc-firmware/blob/master/meta-openbmc/meta-linkedin/meta-deltapower/recipes-deltapower/liutils/files/fan-util.sh > >which invokes this: > >https://github.com/linkedin/o19-bmc-firmware/blob/master/meta-openbmc/meta-linkedin/meta-deltapower/recipes-deltapower/liutils/files/fan-ctrl/fan-util.c > >(which calls into the routine in powershelf_fan.c linked above). > ...though I think I accidentally glossed over some additional steps in beteween there; looks like there's actually another daemon involved that does the read from the sysfs files and the unit normalization and then drops the resulting data into a file that fan-util.c then reads: https://github.com/linkedin/o19-bmc-firmware/blob/master/meta-openbmc/meta-linkedin/meta-deltapower/recipes-core/health-mon/files/healthmon.c Anyway, the upshot is that I'm fairly confident that the code there is in fact what's running on the device as shipped from the factory, and I think the coefficients I determined for the pmbus driver should handle it appropriately. (It produces readings that align with what the factory software gives and are generally sane-looking.) Zev ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 1/2] hwmon: (pmbus) Add Delta AHE-50DC fan control module driver @ 2021-12-07 22:39 ` Zev Weiss 0 siblings, 0 replies; 28+ messages in thread From: Zev Weiss @ 2021-12-07 22:39 UTC (permalink / raw) To: Guenter Roeck; +Cc: linux-hwmon, openbmc, Jean Delvare, linux-kernel On Tue, Dec 07, 2021 at 01:53:44PM PST, Zev Weiss wrote: >On Tue, Dec 07, 2021 at 11:44:01AM PST, Guenter Roeck wrote: >>On 12/7/21 11:22 AM, Zev Weiss wrote: >>>On Tue, Dec 07, 2021 at 09:50:15AM PST, Guenter Roeck wrote: >>>>On Mon, Dec 06, 2021 at 11:15:20PM -0800, Zev Weiss wrote: >>>>>This device is an integrated module of the Delta AHE-50DC Open19 power >>>>>shelf. For lack of proper documentation, this driver has been developed >>>>>referencing an existing (GPL) driver that was included in a code release >>>>>from LinkedIn [1]. It provides four fan speeds, four temperatures, and >>>>>one voltage reading, as well as a handful of warning and fault >>>>>indicators. >>>>> >>>>>[1] https://github.com/linkedin/o19-bmc-firmware/blob/master/meta-openbmc/meta-linkedin/meta-deltapower/recipes-kernel/fancontrol-mod/files/fancontrol.c >>>>> >>>> >>>>Hmm, that reference isn't really accurate anymore. I think it would be >>>>better to just say that the device was found to be PMBus compliant. >>> >>>Sure, will do. >>> >> >>Makes me wonder: How do you know that the referenced driver is for >>Delta AHE-50DC ? > >We'd been waiting for the source code for the software it ships with >for a while, and were finally provided with that repo; everything I've >observed from the factory software is consistent with the code in that >driver. A sampling: > > # no modinfo command available, but... > root@bmc-oob:~# strings -n8 /lib/modules/4.1.51-deltapower/extra/fancontrol.ko | head > fancontrol > license=GPL > description=FANCTRL Driver > author=Ping Mao <pmao@linkedin.com> > alias=i2c:fancontrol > depends=i2c_dev_sysfs > vermagic=4.1.51-deltapower mod_unload ARMv5 p2v8 > fancontrol > status_word > bit 2: Temparature Warning > root@bmc-oob:~# cd /sys/bus/i2c/drivers/fancontrol/8-0030 > root@bmc-oob:/sys/bus/i2c/drivers/fancontrol/8-0030# grep . fan* temp* vin > fan1_2_status:0 > fan1_speed:7860 > fan2_speed:7860 > fan3_4_status:0 > fan3_speed:7680 > fan4_speed:7560 > temperature1:292 > temperature2:278 > temperature3:286 > temperature4:302 > vin:12251 > > >>>> >>>>>Signed-off-by: Zev Weiss <zev@bewilderbeest.net> >>>>>--- >>>>> MAINTAINERS | 6 ++ >>>>> drivers/hwmon/pmbus/Kconfig | 12 ++++ >>>>> drivers/hwmon/pmbus/Makefile | 1 + >>>>> drivers/hwmon/pmbus/delta-ahe50dc-fan.c | 84 +++++++++++++++++++++++++ >>>>> 4 files changed, 103 insertions(+) >>>>> create mode 100644 drivers/hwmon/pmbus/delta-ahe50dc-fan.c >>>>> >>>>>diff --git a/MAINTAINERS b/MAINTAINERS >>>>>index 0ac052200ecb..8bb7ba52d2f5 100644 >>>>>--- a/MAINTAINERS >>>>>+++ b/MAINTAINERS >>>>>@@ -5425,6 +5425,12 @@ W: https://linuxtv.org >>>>> T: git git://linuxtv.org/media_tree.git >>>>> F: drivers/media/platform/sti/delta >>>>> >>>>>+DELTA AHE-50DC FAN CONTROL MODULE DRIVER >>>>>+M: Zev Weiss <zev@bewilderbeest.net> >>>>>+L: linux-hwmon@vger.kernel.org >>>>>+S: Maintained >>>>>+F: drivers/hwmon/pmbus/delta-ahe50dc-fan.c >>>>>+ >>>>> DELTA DPS920AB PSU DRIVER >>>>> M: Robert Marko <robert.marko@sartura.hr> >>>>> L: linux-hwmon@vger.kernel.org >>>>>diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig >>>>>index ffb609cee3a4..937e1c2c11e7 100644 >>>>>--- a/drivers/hwmon/pmbus/Kconfig >>>>>+++ b/drivers/hwmon/pmbus/Kconfig >>>>>@@ -66,6 +66,18 @@ config SENSORS_BPA_RS600 >>>>> This driver can also be built as a module. If so, the module will >>>>> be called bpa-rs600. >>>>> >>>>>+config SENSORS_DELTA_AHE50DC_FAN >>>>>+ tristate "Delta AHE-50DC fan control module" >>>>>+ depends on I2C >>>>>+ select REGMAP_I2C >>> >>>And I realize I neglected to drop the depends & select lines here when moving this bit into the pmbus directory... >>> >>>>>+ help >>>>>+ If you say yes here you get hardware monitoring support for >>>>>+ the integrated fan control module of the Delta AHE-50DC >>>>>+ Open19 power shelf. >>>>>+ >>>>>+ This driver can also be built as a module. If so, the module >>>>>+ will be called delta-ahe50dc-fan. >>>>>+ >>>>> config SENSORS_FSP_3Y >>>>> tristate "FSP/3Y-Power power supplies" >>>>> help >>>>>diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile >>>>>index 0ed4d596a948..a56b2897288d 100644 >>>>>--- a/drivers/hwmon/pmbus/Makefile >>>>>+++ b/drivers/hwmon/pmbus/Makefile >>>>>@@ -9,6 +9,7 @@ obj-$(CONFIG_SENSORS_ADM1266) += adm1266.o >>>>> obj-$(CONFIG_SENSORS_ADM1275) += adm1275.o >>>>> obj-$(CONFIG_SENSORS_BEL_PFE) += bel-pfe.o >>>>> obj-$(CONFIG_SENSORS_BPA_RS600) += bpa-rs600.o >>>>>+obj-$(CONFIG_SENSORS_DELTA_AHE50DC_FAN) += delta-ahe50dc-fan.o >>>>> obj-$(CONFIG_SENSORS_FSP_3Y) += fsp-3y.o >>>>> obj-$(CONFIG_SENSORS_IBM_CFFPS) += ibm-cffps.o >>>>> obj-$(CONFIG_SENSORS_DPS920AB) += dps920ab.o >>>>>diff --git a/drivers/hwmon/pmbus/delta-ahe50dc-fan.c b/drivers/hwmon/pmbus/delta-ahe50dc-fan.c >>>>>new file mode 100644 >>>>>index 000000000000..07b1e7c5f5f5 >>>>>--- /dev/null >>>>>+++ b/drivers/hwmon/pmbus/delta-ahe50dc-fan.c >>>>>@@ -0,0 +1,84 @@ >>>>>+// SPDX-License-Identifier: GPL-2.0 >>>>>+/* >>>>>+ * Delta AHE-50DC power shelf fan control module driver >>>>>+ * >>>>>+ * Copyright 2021 Zev Weiss <zev@bewilderbeest.net> >>>>>+ */ >>>>>+ >>>>>+#include <linux/kernel.h> >>>>>+#include <linux/module.h> >>>>>+#include <linux/i2c.h> >>>>>+#include <linux/pmbus.h> >>>> >>>>Alphabetic include file order please. >>> >>>Ack, will fix. >>> >>>> >>>>>+ >>>>>+#include "pmbus.h" >>>>>+ >>>>>+#define AHE50DC_PMBUS_READ_TEMP4 0xd0 >>>>>+ >>>>>+static int ahe50dc_fan_read_word_data(struct i2c_client *client, int page, int phase, int reg) >>>>>+{ >>>>>+ /* temp1 in (virtual) page 1 is remapped to mfr-specific temp4 */ >>>>>+ if (page == 1) { >>>>>+ if (reg == PMBUS_READ_TEMPERATURE_1) >>>>>+ return i2c_smbus_read_word_data(client, AHE50DC_PMBUS_READ_TEMP4); >>>>>+ return -EOPNOTSUPP; >>>>>+ } >>>>>+ return -ENODATA; >>>>>+} >>>>>+ >>>>>+static struct pmbus_driver_info ahe50dc_fan_info = { >>>>>+ .pages = 2, >>>>>+ .format[PSC_FAN] = direct, >>>>>+ .format[PSC_TEMPERATURE] = direct, >>>>>+ .format[PSC_VOLTAGE_IN] = direct, >>>>>+ .m[PSC_FAN] = 1, >>>>>+ .b[PSC_FAN] = 0, >>>>>+ .R[PSC_FAN] = 0, >>>>>+ .m[PSC_TEMPERATURE] = 1, >>>>>+ .b[PSC_TEMPERATURE] = 0, >>>>>+ .R[PSC_TEMPERATURE] = 1, >>>>>+ .m[PSC_VOLTAGE_IN] = 1, >>>>>+ .b[PSC_VOLTAGE_IN] = 0, >>>>>+ .R[PSC_VOLTAGE_IN] = 3, >>>> >>>>How did you determine the exponents ? The referenced driver >>>>doesn't seem to make a difference between voltage and temperature >>>>exponents (nor fan speed, which is a bit odd). >>> >>>Lacking documentation, the honest answer here is that I just sort of eyeballed it. However, after doing so, I dug through the code dump a bit more and found some userspace unit-conversion bits that appear to confirm what I arrived at: >>> >>>https://github.com/linkedin/o19-bmc-firmware/blob/master/meta-openbmc/meta-linkedin/meta-deltapower/recipes-deltapower/platform-lib/files/powershelf/powershelf_fan.c >>> >>>(Lines 107, and 153/161/169/177.) >>> >> >>Brr. Well, why use a standard API/ABI if one can invent a non-standard one. >> > >It's...not the only bit of gratuitous wheel-reinvention we've come >across in this particular system. > >>Can you check this with real hardware, by any chance ? >> > >If you mean running that code on it, yes -- here's the userspace >utility that invokes that library routine: > > root@bmc-oob:~# fan-util.sh > fan1 speed: 7860 RPM > fan2 speed: 7860 RPM > fan3 speed: 7620 RPM > fan4 speed: 7560 RPM > temperature1: 29.20 C > temperature2: 27.80 C > temperature3: 28.50 C > temperature4: 30.20 C > vin_undervolt_fault: no > overtemperature_warning: no > fan_fault: no > fan_warning: no > fan_status: ok > >That fan-util.sh script being this: > >https://github.com/linkedin/o19-bmc-firmware/blob/master/meta-openbmc/meta-linkedin/meta-deltapower/recipes-deltapower/liutils/files/fan-util.sh > >which invokes this: > >https://github.com/linkedin/o19-bmc-firmware/blob/master/meta-openbmc/meta-linkedin/meta-deltapower/recipes-deltapower/liutils/files/fan-ctrl/fan-util.c > >(which calls into the routine in powershelf_fan.c linked above). > ...though I think I accidentally glossed over some additional steps in beteween there; looks like there's actually another daemon involved that does the read from the sysfs files and the unit normalization and then drops the resulting data into a file that fan-util.c then reads: https://github.com/linkedin/o19-bmc-firmware/blob/master/meta-openbmc/meta-linkedin/meta-deltapower/recipes-core/health-mon/files/healthmon.c Anyway, the upshot is that I'm fairly confident that the code there is in fact what's running on the device as shipped from the factory, and I think the coefficients I determined for the pmbus driver should handle it appropriately. (It produces readings that align with what the factory software gives and are generally sane-looking.) Zev ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 1/2] hwmon: (pmbus) Add Delta AHE-50DC fan control module driver 2021-12-07 21:53 ` Zev Weiss @ 2021-12-07 23:15 ` Guenter Roeck -1 siblings, 0 replies; 28+ messages in thread From: Guenter Roeck @ 2021-12-07 23:15 UTC (permalink / raw) To: Zev Weiss; +Cc: linux-hwmon, Jean Delvare, openbmc, linux-kernel On 12/7/21 1:53 PM, Zev Weiss wrote: > On Tue, Dec 07, 2021 at 11:44:01AM PST, Guenter Roeck wrote: >> On 12/7/21 11:22 AM, Zev Weiss wrote: >>> On Tue, Dec 07, 2021 at 09:50:15AM PST, Guenter Roeck wrote: >>>> On Mon, Dec 06, 2021 at 11:15:20PM -0800, Zev Weiss wrote: >>>>> This device is an integrated module of the Delta AHE-50DC Open19 power >>>>> shelf. For lack of proper documentation, this driver has been developed >>>>> referencing an existing (GPL) driver that was included in a code release >>>>> from LinkedIn [1]. It provides four fan speeds, four temperatures, and >>>>> one voltage reading, as well as a handful of warning and fault >>>>> indicators. >>>>> >>>>> [1] https://github.com/linkedin/o19-bmc-firmware/blob/master/meta-openbmc/meta-linkedin/meta-deltapower/recipes-kernel/fancontrol-mod/files/fancontrol.c >>>>> >>>> >>>> Hmm, that reference isn't really accurate anymore. I think it would be >>>> better to just say that the device was found to be PMBus compliant. >>> >>> Sure, will do. >>> >> >> Makes me wonder: How do you know that the referenced driver is for Delta AHE-50DC ? > > We'd been waiting for the source code for the software it ships with for a while, and were finally provided with that repo; everything I've observed from the factory software is consistent with the code in that driver. A sampling: > I assume you mean "Delta AHE-50DC" when you refer to "it". [ ... ] >> Can you check this with real hardware, by any chance ? >> > > If you mean running that code on it, yes -- here's the userspace utility that invokes that library routine: > > root@bmc-oob:~# fan-util.sh > fan1 speed: 7860 RPM > fan2 speed: 7860 RPM > fan3 speed: 7620 RPM > fan4 speed: 7560 RPM > temperature1: 29.20 C > temperature2: 27.80 C > temperature3: 28.50 C > temperature4: 30.20 C > vin_undervolt_fault: no > overtemperature_warning: no > fan_fault: no > fan_warning: no > fan_status: ok > That doesn't really tell me anything in the context of the driver you submitted. Would it be possible to install your driver and provide the output from the "sensors" command ? It should match the information from the proprietary driver/tool. Thanks, Guenter ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 1/2] hwmon: (pmbus) Add Delta AHE-50DC fan control module driver @ 2021-12-07 23:15 ` Guenter Roeck 0 siblings, 0 replies; 28+ messages in thread From: Guenter Roeck @ 2021-12-07 23:15 UTC (permalink / raw) To: Zev Weiss; +Cc: linux-hwmon, openbmc, Jean Delvare, linux-kernel On 12/7/21 1:53 PM, Zev Weiss wrote: > On Tue, Dec 07, 2021 at 11:44:01AM PST, Guenter Roeck wrote: >> On 12/7/21 11:22 AM, Zev Weiss wrote: >>> On Tue, Dec 07, 2021 at 09:50:15AM PST, Guenter Roeck wrote: >>>> On Mon, Dec 06, 2021 at 11:15:20PM -0800, Zev Weiss wrote: >>>>> This device is an integrated module of the Delta AHE-50DC Open19 power >>>>> shelf. For lack of proper documentation, this driver has been developed >>>>> referencing an existing (GPL) driver that was included in a code release >>>>> from LinkedIn [1]. It provides four fan speeds, four temperatures, and >>>>> one voltage reading, as well as a handful of warning and fault >>>>> indicators. >>>>> >>>>> [1] https://github.com/linkedin/o19-bmc-firmware/blob/master/meta-openbmc/meta-linkedin/meta-deltapower/recipes-kernel/fancontrol-mod/files/fancontrol.c >>>>> >>>> >>>> Hmm, that reference isn't really accurate anymore. I think it would be >>>> better to just say that the device was found to be PMBus compliant. >>> >>> Sure, will do. >>> >> >> Makes me wonder: How do you know that the referenced driver is for Delta AHE-50DC ? > > We'd been waiting for the source code for the software it ships with for a while, and were finally provided with that repo; everything I've observed from the factory software is consistent with the code in that driver. A sampling: > I assume you mean "Delta AHE-50DC" when you refer to "it". [ ... ] >> Can you check this with real hardware, by any chance ? >> > > If you mean running that code on it, yes -- here's the userspace utility that invokes that library routine: > > root@bmc-oob:~# fan-util.sh > fan1 speed: 7860 RPM > fan2 speed: 7860 RPM > fan3 speed: 7620 RPM > fan4 speed: 7560 RPM > temperature1: 29.20 C > temperature2: 27.80 C > temperature3: 28.50 C > temperature4: 30.20 C > vin_undervolt_fault: no > overtemperature_warning: no > fan_fault: no > fan_warning: no > fan_status: ok > That doesn't really tell me anything in the context of the driver you submitted. Would it be possible to install your driver and provide the output from the "sensors" command ? It should match the information from the proprietary driver/tool. Thanks, Guenter ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 1/2] hwmon: (pmbus) Add Delta AHE-50DC fan control module driver 2021-12-07 23:15 ` Guenter Roeck @ 2021-12-08 0:41 ` Zev Weiss -1 siblings, 0 replies; 28+ messages in thread From: Zev Weiss @ 2021-12-08 0:41 UTC (permalink / raw) To: Guenter Roeck; +Cc: linux-hwmon, Jean Delvare, openbmc, linux-kernel On Tue, Dec 07, 2021 at 03:15:18PM PST, Guenter Roeck wrote: >On 12/7/21 1:53 PM, Zev Weiss wrote: >>On Tue, Dec 07, 2021 at 11:44:01AM PST, Guenter Roeck wrote: >>>On 12/7/21 11:22 AM, Zev Weiss wrote: >>>>On Tue, Dec 07, 2021 at 09:50:15AM PST, Guenter Roeck wrote: >>>>>On Mon, Dec 06, 2021 at 11:15:20PM -0800, Zev Weiss wrote: >>>>>>This device is an integrated module of the Delta AHE-50DC Open19 power >>>>>>shelf. For lack of proper documentation, this driver has been developed >>>>>>referencing an existing (GPL) driver that was included in a code release >>>>>>from LinkedIn [1]. It provides four fan speeds, four temperatures, and >>>>>>one voltage reading, as well as a handful of warning and fault >>>>>>indicators. >>>>>> >>>>>>[1] https://github.com/linkedin/o19-bmc-firmware/blob/master/meta-openbmc/meta-linkedin/meta-deltapower/recipes-kernel/fancontrol-mod/files/fancontrol.c >>>>>> >>>>> >>>>>Hmm, that reference isn't really accurate anymore. I think it would be >>>>>better to just say that the device was found to be PMBus compliant. >>>> >>>>Sure, will do. >>>> >>> >>>Makes me wonder: How do you know that the referenced driver is for Delta AHE-50DC ? >> >>We'd been waiting for the source code for the software it ships with for a while, and were finally provided with that repo; everything I've observed from the factory software is consistent with the code in that driver. A sampling: >> > >I assume you mean "Delta AHE-50DC" when you refer to "it". > Yes. >[ ... ] >>>Can you check this with real hardware, by any chance ? >>> >> >>If you mean running that code on it, yes -- here's the userspace utility that invokes that library routine: >> >> root@bmc-oob:~# fan-util.sh >> fan1 speed: 7860 RPM >> fan2 speed: 7860 RPM >> fan3 speed: 7620 RPM >> fan4 speed: 7560 RPM >> temperature1: 29.20 C >> temperature2: 27.80 C >> temperature3: 28.50 C >> temperature4: 30.20 C >> vin_undervolt_fault: no >> overtemperature_warning: no >> fan_fault: no >> fan_warning: no >> fan_status: ok >> > >That doesn't really tell me anything in the context of the driver you submitted. >Would it be possible to install your driver and provide the output from the >"sensors" command ? It should match the information from the proprietary >driver/tool. > Thanks, in doing so I realized I'd neglected to prevent reads from unsupported registers in the read_word_data function, which was leading to the driver producing sysfs files for meaningless sensor limits that the device doesn't actually support. With that fix (which I'll include in v4): root@ahe-50dc:~# /tmp/sensors 'ahe50dc_fan-*' ahe50dc_fan-i2c-28-30 Adapter: i2c-8-mux (chan_id 0) vin: 12.29 V fan1: 7680 RPM fan2: 7860 RPM fan3: 7680 RPM fan4: 7380 RPM temp1: +27.8 C temp2: +23.4 C temp3: +25.3 C temp4: +24.5 C Zev ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 1/2] hwmon: (pmbus) Add Delta AHE-50DC fan control module driver @ 2021-12-08 0:41 ` Zev Weiss 0 siblings, 0 replies; 28+ messages in thread From: Zev Weiss @ 2021-12-08 0:41 UTC (permalink / raw) To: Guenter Roeck; +Cc: linux-hwmon, openbmc, Jean Delvare, linux-kernel On Tue, Dec 07, 2021 at 03:15:18PM PST, Guenter Roeck wrote: >On 12/7/21 1:53 PM, Zev Weiss wrote: >>On Tue, Dec 07, 2021 at 11:44:01AM PST, Guenter Roeck wrote: >>>On 12/7/21 11:22 AM, Zev Weiss wrote: >>>>On Tue, Dec 07, 2021 at 09:50:15AM PST, Guenter Roeck wrote: >>>>>On Mon, Dec 06, 2021 at 11:15:20PM -0800, Zev Weiss wrote: >>>>>>This device is an integrated module of the Delta AHE-50DC Open19 power >>>>>>shelf. For lack of proper documentation, this driver has been developed >>>>>>referencing an existing (GPL) driver that was included in a code release >>>>>>from LinkedIn [1]. It provides four fan speeds, four temperatures, and >>>>>>one voltage reading, as well as a handful of warning and fault >>>>>>indicators. >>>>>> >>>>>>[1] https://github.com/linkedin/o19-bmc-firmware/blob/master/meta-openbmc/meta-linkedin/meta-deltapower/recipes-kernel/fancontrol-mod/files/fancontrol.c >>>>>> >>>>> >>>>>Hmm, that reference isn't really accurate anymore. I think it would be >>>>>better to just say that the device was found to be PMBus compliant. >>>> >>>>Sure, will do. >>>> >>> >>>Makes me wonder: How do you know that the referenced driver is for Delta AHE-50DC ? >> >>We'd been waiting for the source code for the software it ships with for a while, and were finally provided with that repo; everything I've observed from the factory software is consistent with the code in that driver. A sampling: >> > >I assume you mean "Delta AHE-50DC" when you refer to "it". > Yes. >[ ... ] >>>Can you check this with real hardware, by any chance ? >>> >> >>If you mean running that code on it, yes -- here's the userspace utility that invokes that library routine: >> >> root@bmc-oob:~# fan-util.sh >> fan1 speed: 7860 RPM >> fan2 speed: 7860 RPM >> fan3 speed: 7620 RPM >> fan4 speed: 7560 RPM >> temperature1: 29.20 C >> temperature2: 27.80 C >> temperature3: 28.50 C >> temperature4: 30.20 C >> vin_undervolt_fault: no >> overtemperature_warning: no >> fan_fault: no >> fan_warning: no >> fan_status: ok >> > >That doesn't really tell me anything in the context of the driver you submitted. >Would it be possible to install your driver and provide the output from the >"sensors" command ? It should match the information from the proprietary >driver/tool. > Thanks, in doing so I realized I'd neglected to prevent reads from unsupported registers in the read_word_data function, which was leading to the driver producing sysfs files for meaningless sensor limits that the device doesn't actually support. With that fix (which I'll include in v4): root@ahe-50dc:~# /tmp/sensors 'ahe50dc_fan-*' ahe50dc_fan-i2c-28-30 Adapter: i2c-8-mux (chan_id 0) vin: 12.29 V fan1: 7680 RPM fan2: 7860 RPM fan3: 7680 RPM fan4: 7380 RPM temp1: +27.8 C temp2: +23.4 C temp3: +25.3 C temp4: +24.5 C Zev ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 1/2] hwmon: (pmbus) Add Delta AHE-50DC fan control module driver 2021-12-08 0:41 ` Zev Weiss @ 2021-12-08 0:54 ` Guenter Roeck -1 siblings, 0 replies; 28+ messages in thread From: Guenter Roeck @ 2021-12-08 0:54 UTC (permalink / raw) To: Zev Weiss; +Cc: linux-hwmon, Jean Delvare, openbmc, linux-kernel On 12/7/21 4:41 PM, Zev Weiss wrote: > On Tue, Dec 07, 2021 at 03:15:18PM PST, Guenter Roeck wrote: >> On 12/7/21 1:53 PM, Zev Weiss wrote: >>> On Tue, Dec 07, 2021 at 11:44:01AM PST, Guenter Roeck wrote: >>>> On 12/7/21 11:22 AM, Zev Weiss wrote: >>>>> On Tue, Dec 07, 2021 at 09:50:15AM PST, Guenter Roeck wrote: >>>>>> On Mon, Dec 06, 2021 at 11:15:20PM -0800, Zev Weiss wrote: >>>>>>> This device is an integrated module of the Delta AHE-50DC Open19 power >>>>>>> shelf. For lack of proper documentation, this driver has been developed >>>>>>> referencing an existing (GPL) driver that was included in a code release >>>>>>> from LinkedIn [1]. It provides four fan speeds, four temperatures, and >>>>>>> one voltage reading, as well as a handful of warning and fault >>>>>>> indicators. >>>>>>> >>>>>>> [1] https://github.com/linkedin/o19-bmc-firmware/blob/master/meta-openbmc/meta-linkedin/meta-deltapower/recipes-kernel/fancontrol-mod/files/fancontrol.c >>>>>>> >>>>>> >>>>>> Hmm, that reference isn't really accurate anymore. I think it would be >>>>>> better to just say that the device was found to be PMBus compliant. >>>>> >>>>> Sure, will do. >>>>> >>>> >>>> Makes me wonder: How do you know that the referenced driver is for Delta AHE-50DC ? >>> >>> We'd been waiting for the source code for the software it ships with for a while, and were finally provided with that repo; everything I've observed from the factory software is consistent with the code in that driver. A sampling: >>> >> >> I assume you mean "Delta AHE-50DC" when you refer to "it". >> > > Yes. > >> [ ... ] >>>> Can you check this with real hardware, by any chance ? >>>> >>> >>> If you mean running that code on it, yes -- here's the userspace utility that invokes that library routine: >>> >>> root@bmc-oob:~# fan-util.sh >>> fan1 speed: 7860 RPM >>> fan2 speed: 7860 RPM >>> fan3 speed: 7620 RPM >>> fan4 speed: 7560 RPM >>> temperature1: 29.20 C >>> temperature2: 27.80 C >>> temperature3: 28.50 C >>> temperature4: 30.20 C >>> vin_undervolt_fault: no >>> overtemperature_warning: no >>> fan_fault: no >>> fan_warning: no >>> fan_status: ok >>> >> >> That doesn't really tell me anything in the context of the driver you submitted. >> Would it be possible to install your driver and provide the output from the >> "sensors" command ? It should match the information from the proprietary >> driver/tool. >> > > Thanks, in doing so I realized I'd neglected to prevent reads from unsupported registers in the read_word_data function, which was leading to the driver producing sysfs files for meaningless sensor limits that the device doesn't actually support. With that fix (which I'll include in v4): > What a surprise. See drivers/hwmon/pmbus/dps920ab.c; that seems to be a common theme for devices from Delta. I would suggest to add a similar comment. Guenter ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 1/2] hwmon: (pmbus) Add Delta AHE-50DC fan control module driver @ 2021-12-08 0:54 ` Guenter Roeck 0 siblings, 0 replies; 28+ messages in thread From: Guenter Roeck @ 2021-12-08 0:54 UTC (permalink / raw) To: Zev Weiss; +Cc: linux-hwmon, openbmc, Jean Delvare, linux-kernel On 12/7/21 4:41 PM, Zev Weiss wrote: > On Tue, Dec 07, 2021 at 03:15:18PM PST, Guenter Roeck wrote: >> On 12/7/21 1:53 PM, Zev Weiss wrote: >>> On Tue, Dec 07, 2021 at 11:44:01AM PST, Guenter Roeck wrote: >>>> On 12/7/21 11:22 AM, Zev Weiss wrote: >>>>> On Tue, Dec 07, 2021 at 09:50:15AM PST, Guenter Roeck wrote: >>>>>> On Mon, Dec 06, 2021 at 11:15:20PM -0800, Zev Weiss wrote: >>>>>>> This device is an integrated module of the Delta AHE-50DC Open19 power >>>>>>> shelf. For lack of proper documentation, this driver has been developed >>>>>>> referencing an existing (GPL) driver that was included in a code release >>>>>>> from LinkedIn [1]. It provides four fan speeds, four temperatures, and >>>>>>> one voltage reading, as well as a handful of warning and fault >>>>>>> indicators. >>>>>>> >>>>>>> [1] https://github.com/linkedin/o19-bmc-firmware/blob/master/meta-openbmc/meta-linkedin/meta-deltapower/recipes-kernel/fancontrol-mod/files/fancontrol.c >>>>>>> >>>>>> >>>>>> Hmm, that reference isn't really accurate anymore. I think it would be >>>>>> better to just say that the device was found to be PMBus compliant. >>>>> >>>>> Sure, will do. >>>>> >>>> >>>> Makes me wonder: How do you know that the referenced driver is for Delta AHE-50DC ? >>> >>> We'd been waiting for the source code for the software it ships with for a while, and were finally provided with that repo; everything I've observed from the factory software is consistent with the code in that driver. A sampling: >>> >> >> I assume you mean "Delta AHE-50DC" when you refer to "it". >> > > Yes. > >> [ ... ] >>>> Can you check this with real hardware, by any chance ? >>>> >>> >>> If you mean running that code on it, yes -- here's the userspace utility that invokes that library routine: >>> >>> root@bmc-oob:~# fan-util.sh >>> fan1 speed: 7860 RPM >>> fan2 speed: 7860 RPM >>> fan3 speed: 7620 RPM >>> fan4 speed: 7560 RPM >>> temperature1: 29.20 C >>> temperature2: 27.80 C >>> temperature3: 28.50 C >>> temperature4: 30.20 C >>> vin_undervolt_fault: no >>> overtemperature_warning: no >>> fan_fault: no >>> fan_warning: no >>> fan_status: ok >>> >> >> That doesn't really tell me anything in the context of the driver you submitted. >> Would it be possible to install your driver and provide the output from the >> "sensors" command ? It should match the information from the proprietary >> driver/tool. >> > > Thanks, in doing so I realized I'd neglected to prevent reads from unsupported registers in the read_word_data function, which was leading to the driver producing sysfs files for meaningless sensor limits that the device doesn't actually support. With that fix (which I'll include in v4): > What a surprise. See drivers/hwmon/pmbus/dps920ab.c; that seems to be a common theme for devices from Delta. I would suggest to add a similar comment. Guenter ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 1/2] hwmon: (pmbus) Add Delta AHE-50DC fan control module driver 2021-12-07 19:22 ` Zev Weiss @ 2021-12-07 19:53 ` Guenter Roeck -1 siblings, 0 replies; 28+ messages in thread From: Guenter Roeck @ 2021-12-07 19:53 UTC (permalink / raw) To: Zev Weiss; +Cc: linux-hwmon, Jean Delvare, openbmc, linux-kernel On 12/7/21 11:22 AM, Zev Weiss wrote: > On Tue, Dec 07, 2021 at 09:50:15AM PST, Guenter Roeck wrote: >> On Mon, Dec 06, 2021 at 11:15:20PM -0800, Zev Weiss wrote: >>> This device is an integrated module of the Delta AHE-50DC Open19 power >>> shelf. For lack of proper documentation, this driver has been developed >>> referencing an existing (GPL) driver that was included in a code release >>> from LinkedIn [1]. It provides four fan speeds, four temperatures, and >>> one voltage reading, as well as a handful of warning and fault >>> indicators. >>> >>> [1] https://github.com/linkedin/o19-bmc-firmware/blob/master/meta-openbmc/meta-linkedin/meta-deltapower/recipes-kernel/fancontrol-mod/files/fancontrol.c >>> >> >> Hmm, that reference isn't really accurate anymore. I think it would be >> better to just say that the device was found to be PMBus compliant. > > Sure, will do. > Makes me wonder: How do you know that the referenced driver is for Delta AHE-50DC ? Guenter ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 1/2] hwmon: (pmbus) Add Delta AHE-50DC fan control module driver @ 2021-12-07 19:53 ` Guenter Roeck 0 siblings, 0 replies; 28+ messages in thread From: Guenter Roeck @ 2021-12-07 19:53 UTC (permalink / raw) To: Zev Weiss; +Cc: linux-hwmon, openbmc, Jean Delvare, linux-kernel On 12/7/21 11:22 AM, Zev Weiss wrote: > On Tue, Dec 07, 2021 at 09:50:15AM PST, Guenter Roeck wrote: >> On Mon, Dec 06, 2021 at 11:15:20PM -0800, Zev Weiss wrote: >>> This device is an integrated module of the Delta AHE-50DC Open19 power >>> shelf. For lack of proper documentation, this driver has been developed >>> referencing an existing (GPL) driver that was included in a code release >>> from LinkedIn [1]. It provides four fan speeds, four temperatures, and >>> one voltage reading, as well as a handful of warning and fault >>> indicators. >>> >>> [1] https://github.com/linkedin/o19-bmc-firmware/blob/master/meta-openbmc/meta-linkedin/meta-deltapower/recipes-kernel/fancontrol-mod/files/fancontrol.c >>> >> >> Hmm, that reference isn't really accurate anymore. I think it would be >> better to just say that the device was found to be PMBus compliant. > > Sure, will do. > Makes me wonder: How do you know that the referenced driver is for Delta AHE-50DC ? Guenter ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v3 2/2] dt-bindings: add Delta AHE-50DC fan control module 2021-12-07 7:15 ` Zev Weiss @ 2021-12-07 7:15 ` Zev Weiss -1 siblings, 0 replies; 28+ messages in thread From: Zev Weiss @ 2021-12-07 7:15 UTC (permalink / raw) To: linux-hwmon Cc: Guenter Roeck, Jean Delvare, openbmc, Zev Weiss, Rob Herring, Krzysztof Kozlowski, Jonathan Cameron, Alexandre Belloni, Chris Packham, devicetree, linux-kernel This is the integrated fan control module of the Delta AHE-50DC Open19 power shelf. Signed-off-by: Zev Weiss <zev@bewilderbeest.net> --- Documentation/devicetree/bindings/trivial-devices.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml index 791079021f1b..1c43cc91f804 100644 --- a/Documentation/devicetree/bindings/trivial-devices.yaml +++ b/Documentation/devicetree/bindings/trivial-devices.yaml @@ -73,6 +73,8 @@ properties: - dallas,ds4510 # Digital Thermometer and Thermostat - dallas,ds75 + # Delta AHE-50DC Open19 power shelf fan control module + - delta,ahe50dc-fan # Delta Electronics DPS-650-AB power supply - delta,dps650ab # Delta Electronics DPS920AB 920W 54V Power Supply -- 2.34.1 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v3 2/2] dt-bindings: add Delta AHE-50DC fan control module @ 2021-12-07 7:15 ` Zev Weiss 0 siblings, 0 replies; 28+ messages in thread From: Zev Weiss @ 2021-12-07 7:15 UTC (permalink / raw) To: linux-hwmon Cc: devicetree, Alexandre Belloni, Jean Delvare, Zev Weiss, Krzysztof Kozlowski, openbmc, linux-kernel, Rob Herring, Chris Packham, Jonathan Cameron, Guenter Roeck This is the integrated fan control module of the Delta AHE-50DC Open19 power shelf. Signed-off-by: Zev Weiss <zev@bewilderbeest.net> --- Documentation/devicetree/bindings/trivial-devices.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml index 791079021f1b..1c43cc91f804 100644 --- a/Documentation/devicetree/bindings/trivial-devices.yaml +++ b/Documentation/devicetree/bindings/trivial-devices.yaml @@ -73,6 +73,8 @@ properties: - dallas,ds4510 # Digital Thermometer and Thermostat - dallas,ds75 + # Delta AHE-50DC Open19 power shelf fan control module + - delta,ahe50dc-fan # Delta Electronics DPS-650-AB power supply - delta,dps650ab # Delta Electronics DPS920AB 920W 54V Power Supply -- 2.34.1 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v3 2/2] dt-bindings: add Delta AHE-50DC fan control module 2021-12-07 7:15 ` Zev Weiss @ 2021-12-07 7:58 ` Krzysztof Kozlowski -1 siblings, 0 replies; 28+ messages in thread From: Krzysztof Kozlowski @ 2021-12-07 7:58 UTC (permalink / raw) To: Zev Weiss, linux-hwmon Cc: Guenter Roeck, Jean Delvare, openbmc, Rob Herring, Jonathan Cameron, Alexandre Belloni, Chris Packham, devicetree, linux-kernel On 07/12/2021 08:15, Zev Weiss wrote: > This is the integrated fan control module of the Delta AHE-50DC Open19 > power shelf. > > Signed-off-by: Zev Weiss <zev@bewilderbeest.net> > --- > Documentation/devicetree/bindings/trivial-devices.yaml | 2 ++ > 1 file changed, 2 insertions(+) This is a third version sent within one night, without changelog and any indication why sending the same three times. Please do not send multiple versions the same day and give people some time to respond. When creating a new version, add a changelog under ---. > > diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml > index 791079021f1b..1c43cc91f804 100644 > --- a/Documentation/devicetree/bindings/trivial-devices.yaml > +++ b/Documentation/devicetree/bindings/trivial-devices.yaml > @@ -73,6 +73,8 @@ properties: > - dallas,ds4510 > # Digital Thermometer and Thermostat > - dallas,ds75 > + # Delta AHE-50DC Open19 power shelf fan control module > + - delta,ahe50dc-fan > # Delta Electronics DPS-650-AB power supply > - delta,dps650ab > # Delta Electronics DPS920AB 920W 54V Power Supply > Best regards, Krzysztof ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 2/2] dt-bindings: add Delta AHE-50DC fan control module @ 2021-12-07 7:58 ` Krzysztof Kozlowski 0 siblings, 0 replies; 28+ messages in thread From: Krzysztof Kozlowski @ 2021-12-07 7:58 UTC (permalink / raw) To: Zev Weiss, linux-hwmon Cc: devicetree, Alexandre Belloni, Jean Delvare, openbmc, linux-kernel, Rob Herring, Chris Packham, Jonathan Cameron, Guenter Roeck On 07/12/2021 08:15, Zev Weiss wrote: > This is the integrated fan control module of the Delta AHE-50DC Open19 > power shelf. > > Signed-off-by: Zev Weiss <zev@bewilderbeest.net> > --- > Documentation/devicetree/bindings/trivial-devices.yaml | 2 ++ > 1 file changed, 2 insertions(+) This is a third version sent within one night, without changelog and any indication why sending the same three times. Please do not send multiple versions the same day and give people some time to respond. When creating a new version, add a changelog under ---. > > diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml > index 791079021f1b..1c43cc91f804 100644 > --- a/Documentation/devicetree/bindings/trivial-devices.yaml > +++ b/Documentation/devicetree/bindings/trivial-devices.yaml > @@ -73,6 +73,8 @@ properties: > - dallas,ds4510 > # Digital Thermometer and Thermostat > - dallas,ds75 > + # Delta AHE-50DC Open19 power shelf fan control module > + - delta,ahe50dc-fan > # Delta Electronics DPS-650-AB power supply > - delta,dps650ab > # Delta Electronics DPS920AB 920W 54V Power Supply > Best regards, Krzysztof ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 2/2] dt-bindings: add Delta AHE-50DC fan control module 2021-12-07 7:58 ` Krzysztof Kozlowski @ 2021-12-07 8:48 ` Zev Weiss -1 siblings, 0 replies; 28+ messages in thread From: Zev Weiss @ 2021-12-07 8:48 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: linux-hwmon, Guenter Roeck, Jean Delvare, openbmc, Rob Herring, Jonathan Cameron, Alexandre Belloni, Chris Packham, devicetree, linux-kernel On Mon, Dec 06, 2021 at 11:58:14PM PST, Krzysztof Kozlowski wrote: >On 07/12/2021 08:15, Zev Weiss wrote: >> This is the integrated fan control module of the Delta AHE-50DC Open19 >> power shelf. >> >> Signed-off-by: Zev Weiss <zev@bewilderbeest.net> >> --- >> Documentation/devicetree/bindings/trivial-devices.yaml | 2 ++ >> 1 file changed, 2 insertions(+) > >This is a third version sent within one night, without changelog and any >indication why sending the same three times. Please do not send multiple >versions the same day and give people some time to respond. When >creating a new version, add a changelog under ---. > Sorry for the noise -- I wrote changelogs in the cover letter (https://lore.kernel.org/linux-hwmon/20211207071521.543-1-zev@bewilderbeest.net/), but forgot to include all the recipients of the component patches in the CC list of the cover (which I presume would have been preferred), I just used git send-email's --cc-cmd flag with get_maintainer.pl. Zev ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v3 2/2] dt-bindings: add Delta AHE-50DC fan control module @ 2021-12-07 8:48 ` Zev Weiss 0 siblings, 0 replies; 28+ messages in thread From: Zev Weiss @ 2021-12-07 8:48 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: linux-hwmon, devicetree, Alexandre Belloni, Jean Delvare, openbmc, linux-kernel, Rob Herring, Chris Packham, Jonathan Cameron, Guenter Roeck On Mon, Dec 06, 2021 at 11:58:14PM PST, Krzysztof Kozlowski wrote: >On 07/12/2021 08:15, Zev Weiss wrote: >> This is the integrated fan control module of the Delta AHE-50DC Open19 >> power shelf. >> >> Signed-off-by: Zev Weiss <zev@bewilderbeest.net> >> --- >> Documentation/devicetree/bindings/trivial-devices.yaml | 2 ++ >> 1 file changed, 2 insertions(+) > >This is a third version sent within one night, without changelog and any >indication why sending the same three times. Please do not send multiple >versions the same day and give people some time to respond. When >creating a new version, add a changelog under ---. > Sorry for the noise -- I wrote changelogs in the cover letter (https://lore.kernel.org/linux-hwmon/20211207071521.543-1-zev@bewilderbeest.net/), but forgot to include all the recipients of the component patches in the CC list of the cover (which I presume would have been preferred), I just used git send-email's --cc-cmd flag with get_maintainer.pl. Zev ^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2021-12-08 0:55 UTC | newest] Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-12-07 7:15 [PATCH v3 0/2] hwmon: (pmbus) Add Delta AHE-50DC fan control module driver Zev Weiss 2021-12-07 7:15 ` Zev Weiss 2021-12-07 7:15 ` [PATCH v3 1/2] " Zev Weiss 2021-12-07 7:15 ` Zev Weiss 2021-12-07 17:50 ` Guenter Roeck 2021-12-07 17:50 ` Guenter Roeck 2021-12-07 19:22 ` Zev Weiss 2021-12-07 19:22 ` Zev Weiss 2021-12-07 19:44 ` Guenter Roeck 2021-12-07 19:44 ` Guenter Roeck 2021-12-07 21:53 ` Zev Weiss 2021-12-07 21:53 ` Zev Weiss 2021-12-07 22:39 ` Zev Weiss 2021-12-07 22:39 ` Zev Weiss 2021-12-07 23:15 ` Guenter Roeck 2021-12-07 23:15 ` Guenter Roeck 2021-12-08 0:41 ` Zev Weiss 2021-12-08 0:41 ` Zev Weiss 2021-12-08 0:54 ` Guenter Roeck 2021-12-08 0:54 ` Guenter Roeck 2021-12-07 19:53 ` Guenter Roeck 2021-12-07 19:53 ` Guenter Roeck 2021-12-07 7:15 ` [PATCH v3 2/2] dt-bindings: add Delta AHE-50DC fan control module Zev Weiss 2021-12-07 7:15 ` Zev Weiss 2021-12-07 7:58 ` Krzysztof Kozlowski 2021-12-07 7:58 ` Krzysztof Kozlowski 2021-12-07 8:48 ` Zev Weiss 2021-12-07 8:48 ` Zev Weiss
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.