From: Nick Hawkins <[email protected]>
The GXP SoC can support up to 16 fans through the interface provided by
the CPLD. The fans speeds are controlled via a pwm value 0-255. The fans
are also capable of reporting if they have failed to the CPLD which in
turn reports the status to the GXP SoC. Based on previous feedback the
registers required for fan control have been regmaped individualy to fan
driver. Specifically these registers are the function 2 registers and the
programmable logic registers from the CPLD. Additionally in this patchset
there is support for the SPI driver which already exists as spi-gxp.c in
the SPI driver.
Nick Hawkins (6):
hwmon: (gxp-fan-ctrl) Add GXP fan controller
ABI: sysfs-class-hwmon: add a description for fanY_fault
dt-bindings: hwmon: Add hpe,gxp-fan-ctrl
ARM: dts: add GXP Support for fans and SPI
ARM: multi_v7_defconfig: Add GXP Fan and SPI support
MAINTAINERS: add gxp fan controller and documents
Documentation/ABI/testing/sysfs-class-hwmon | 9 +
.../bindings/hwmon/hpe,gxp-fan-ctrl.yaml | 41 ++
Documentation/hwmon/gxp-fan-ctrl.rst | 36 ++
MAINTAINERS | 2 +
arch/arm/boot/dts/hpe-bmc-dl360gen10.dts | 58 +++
arch/arm/boot/dts/hpe-gxp.dtsi | 64 +++-
arch/arm/configs/multi_v7_defconfig | 2 +
drivers/hwmon/Kconfig | 8 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/gxp-fan-ctrl.c | 362 ++++++++++++++++++
10 files changed, 564 insertions(+), 19 deletions(-)
create mode 100644 Documentation/devicetree/bindings/hwmon/hpe,gxp-fan-ctrl.yaml
create mode 100644 Documentation/hwmon/gxp-fan-ctrl.rst
create mode 100644 drivers/hwmon/gxp-fan-ctrl.c
--
2.17.1
From: Nick Hawkins <[email protected]>
This provides the base registers address, programmable logic registers
address, and the function 2 registers to allow control access of the HPE
fans on the GXP SoC.
Signed-off-by: Nick Hawkins <[email protected]>
---
.../bindings/hwmon/hpe,gxp-fan-ctrl.yaml | 41 +++++++++++++++++++
1 file changed, 41 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hwmon/hpe,gxp-fan-ctrl.yaml
diff --git a/Documentation/devicetree/bindings/hwmon/hpe,gxp-fan-ctrl.yaml b/Documentation/devicetree/bindings/hwmon/hpe,gxp-fan-ctrl.yaml
new file mode 100644
index 000000000000..40a5d9cd0a30
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/hpe,gxp-fan-ctrl.yaml
@@ -0,0 +1,41 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/hwmon/hpe,gxp-fan-ctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: GXP Fan Controller
+
+maintainers:
+ - Nick Hawkins <[email protected]>
+
+properties:
+ compatible:
+ const: hpe,gxp-fan-ctrl
+
+ reg:
+ items:
+ - description: Fan controller base register
+ - description: Programmable logic registers base
+ - description: Function 2 registers base
+
+ reg-names:
+ items:
+ - const: base
+ - const: plreg
+ - const: fn2reg
+
+required:
+ - compatible
+ - reg
+ - reg-names
+
+additionalProperties: false
+
+examples:
+ - |
+ fanctrl@1000c00 {
+ compatible = "hpe,gxp-fan-ctrl";
+ reg = <0x1000c00 0x200>, <0xd1000000 0xff>, <0x80200000 0x100000>;
+ reg-names = "base", "plreg", "fn2reg";
+ };
--
2.17.1
From: Nick Hawkins <[email protected]>
The GXP SoC can support up to 16 fans through the interface provided by
the CPLD. The fans speeds are controlled via a pwm value 0-255. The fans
are also capable of reporting if they have failed to the CPLD which in
turn reports the status to the GXP SoC. There are no tachometers so fan
speeds are reported as a percent of the pwm value.
Signed-off-by: Nick Hawkins <[email protected]>
---
Documentation/hwmon/gxp-fan-ctrl.rst | 36 +++
drivers/hwmon/Kconfig | 8 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/gxp-fan-ctrl.c | 362 +++++++++++++++++++++++++++
4 files changed, 407 insertions(+)
create mode 100644 Documentation/hwmon/gxp-fan-ctrl.rst
create mode 100644 drivers/hwmon/gxp-fan-ctrl.c
diff --git a/Documentation/hwmon/gxp-fan-ctrl.rst b/Documentation/hwmon/gxp-fan-ctrl.rst
new file mode 100644
index 000000000000..fc1709fb113b
--- /dev/null
+++ b/Documentation/hwmon/gxp-fan-ctrl.rst
@@ -0,0 +1,36 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+Kernel driver gxp-fan-ctrl
+==========================
+
+Supported chips:
+
+ * HPE GXP SOC
+
+Author: Nick Hawkins <[email protected]>
+
+
+Description
+-----------
+
+gxp-fan-ctrl is a driver which provides fan control for the hpe gxp soc.
+The driver allows the gathering of fan status and the use of fan
+pwm control.
+
+
+Usage Notes
+-----------
+
+Traditionally fanY_input returns an RPM value, on HPE GXP systems it is
+the pwm value [0-255] due to the fan speeds being reported as
+percentages.
+
+
+Sysfs attributes
+----------------
+
+======================= =================================================
+pwm[0-15] Fan 0 to 15 respective pwm value
+fan[0-15]_input Fan 0 to 15 respective input value: pwm value
+fan[0-15]_fault Fan 0 to 15 respective fault status: 1 fail, 0 ok
+======================= =================================================
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index e70d9614bec2..3d32cd77424c 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -2303,6 +2303,14 @@ config SENSORS_INTEL_M10_BMC_HWMON
sensors monitor various telemetry data of different components on the
card, e.g. board temperature, FPGA core temperature/voltage/current.
+config SENSORS_GXP_FAN_CTRL
+ tristate "GXP Fan Control driver"
+ depends on ARCH_HPE_GXP || COMPILE_TEST
+ help
+ If you say yes here you get support for GXP fan control functionality.
+ The GXP controls fan function via the CPLD through the use of PWM
+ registers. This driver reports status and pwm setting of the fans.
+
if ACPI
comment "ACPI drivers"
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 007e829d1d0d..b474dcc708c4 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -83,6 +83,7 @@ obj-$(CONFIG_SENSORS_GL518SM) += gl518sm.o
obj-$(CONFIG_SENSORS_GL520SM) += gl520sm.o
obj-$(CONFIG_SENSORS_GSC) += gsc-hwmon.o
obj-$(CONFIG_SENSORS_GPIO_FAN) += gpio-fan.o
+obj-$(CONFIG_SENSORS_GXP_FAN_CTRL) += gxp-fan-ctrl.o
obj-$(CONFIG_SENSORS_HIH6130) += hih6130.o
obj-$(CONFIG_SENSORS_ULTRA45) += ultra45_env.o
obj-$(CONFIG_SENSORS_I5500) += i5500_temp.o
diff --git a/drivers/hwmon/gxp-fan-ctrl.c b/drivers/hwmon/gxp-fan-ctrl.c
new file mode 100644
index 000000000000..a01530951d58
--- /dev/null
+++ b/drivers/hwmon/gxp-fan-ctrl.c
@@ -0,0 +1,362 @@
+// SPDX-License-Identifier: GPL-2.0=or-later
+/* Copyright (C) 2022 Hewlett-Packard Enterprise Development Company, L.P. */
+
+#include <linux/err.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
+
+#define OFFSET_PWM0DUTY 0x10
+#define OFFSET_PWM1DUTY 0x11
+#define OFFSET_PWM2DUTY 0x12
+#define OFFSET_PWM3DUTY 0x13
+#define OFFSET_PWM4DUTY 0x14
+#define OFFSET_PWM5DUTY 0x15
+#define OFFSET_PWM6DUTY 0x16
+#define OFFSET_PWM7DUTY 0x17
+
+struct fan_data {
+ u32 inst;
+ u32 fail;
+ u32 id;
+ u32 bit;
+};
+
+struct fan_ctrl_data {
+ struct fan_data fan[16];
+ u32 power_bit;
+};
+
+struct gxp_fan_ctrl_drvdata {
+ struct device *dev;
+ struct device *hwmon_dev;
+ struct regmap *plreg_map; /* Programmable logic register regmap */
+ struct regmap *fn2_map; /* Function 2 regmap */
+ void __iomem *base;
+ const struct fan_ctrl_data *data;
+ struct mutex update_lock; /* To protect the setting of the fan PWM value */
+};
+
+static void address_translation(u32 desired_offset, u32 *offset, u32 *bit_shift)
+{
+ *offset = (desired_offset & 0xffc);
+ *bit_shift = (desired_offset - *offset) * 8;
+}
+
+static bool fan_installed(struct device *dev, int fan)
+{
+ struct gxp_fan_ctrl_drvdata *drvdata = dev_get_drvdata(dev);
+ u32 trans_offset;
+ u32 trans_shift;
+ u32 val;
+
+ address_translation(drvdata->data->fan[fan].inst,
+ &trans_offset,
+ &trans_shift);
+
+ regmap_read(drvdata->plreg_map, trans_offset, &val);
+ val = (val >> trans_shift) & drvdata->data->fan[fan].bit;
+ if (val == drvdata->data->fan[fan].bit)
+ return 1;
+ else
+ return 0;
+}
+
+static bool fan_failed(struct device *dev, int fan)
+{
+ struct gxp_fan_ctrl_drvdata *drvdata = dev_get_drvdata(dev);
+ u32 trans_offset;
+ u32 trans_shift;
+ u32 val;
+
+ address_translation(drvdata->data->fan[fan].fail,
+ &trans_offset,
+ &trans_shift);
+
+ regmap_read(drvdata->plreg_map, trans_offset, &val);
+ val = (val >> trans_shift) & drvdata->data->fan[fan].fail;
+ if (val == drvdata->data->fan[fan].fail)
+ return 1;
+ else
+ return 0;
+}
+
+static ssize_t show_fault(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ int nr = (to_sensor_dev_attr(attr))->index;
+ unsigned char val;
+
+ val = (fan_failed(dev, nr)) ? 1 : 0;
+
+ return sprintf(buf, "%d\n", val);
+}
+
+static ssize_t show_in(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ int nr = (to_sensor_dev_attr(attr))->index;
+ struct gxp_fan_ctrl_drvdata *drvdata = dev_get_drvdata(dev);
+ unsigned char val;
+ unsigned int reg;
+
+ /* Check Power Status */
+ regmap_read(drvdata->fn2_map, 0, ®);
+ if (reg & BIT(drvdata->data->power_bit)) {
+ /* If Fan presents, then read it. */
+ val = (fan_installed(dev, nr)) ? readb(drvdata->base +
+ OFFSET_PWM0DUTY +
+ nr) : 0;
+ } else {
+ /* Power Off */
+ val = 0;
+ }
+
+ return sprintf(buf, "%d\n", val);
+}
+
+static ssize_t show_pwm(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ int nr = (to_sensor_dev_attr(attr))->index;
+ struct gxp_fan_ctrl_drvdata *drvdata = dev_get_drvdata(dev);
+ unsigned char val;
+
+ val = readb(drvdata->base + OFFSET_PWM0DUTY + nr);
+
+ return sprintf(buf, "%d\n", val);
+}
+
+static ssize_t store_pwm(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ int nr = (to_sensor_dev_attr(attr))->index;
+ struct gxp_fan_ctrl_drvdata *drvdata = dev_get_drvdata(dev);
+ unsigned long val;
+ int err;
+
+ err = kstrtoul(buf, 10, &val);
+ if (err)
+ return err;
+
+ if (val > 255)
+ return -1; /* out of range */
+
+ mutex_lock(&drvdata->update_lock);
+
+ writeb(val, drvdata->base + OFFSET_PWM0DUTY + nr);
+
+ mutex_unlock(&drvdata->update_lock);
+ return count;
+}
+
+static SENSOR_DEVICE_ATTR(pwm0, 0200 | 0444, show_pwm, store_pwm, 0);
+static SENSOR_DEVICE_ATTR(pwm1, 0200 | 0444, show_pwm, store_pwm, 1);
+static SENSOR_DEVICE_ATTR(pwm2, 0200 | 0444, show_pwm, store_pwm, 2);
+static SENSOR_DEVICE_ATTR(pwm3, 0200 | 0444, show_pwm, store_pwm, 3);
+static SENSOR_DEVICE_ATTR(pwm4, 0200 | 0444, show_pwm, store_pwm, 4);
+static SENSOR_DEVICE_ATTR(pwm5, 0200 | 0444, show_pwm, store_pwm, 5);
+static SENSOR_DEVICE_ATTR(pwm6, 0200 | 0444, show_pwm, store_pwm, 6);
+static SENSOR_DEVICE_ATTR(pwm7, 0200 | 0444, show_pwm, store_pwm, 7);
+static SENSOR_DEVICE_ATTR(pwm8, 0200 | 0444, show_pwm, store_pwm, 8);
+static SENSOR_DEVICE_ATTR(pwm9, 0200 | 0444, show_pwm, store_pwm, 9);
+static SENSOR_DEVICE_ATTR(pwm10, 0200 | 0444, show_pwm, store_pwm, 10);
+static SENSOR_DEVICE_ATTR(pwm11, 0200 | 0444, show_pwm, store_pwm, 11);
+static SENSOR_DEVICE_ATTR(pwm12, 0200 | 0444, show_pwm, store_pwm, 12);
+static SENSOR_DEVICE_ATTR(pwm13, 0200 | 0444, show_pwm, store_pwm, 13);
+static SENSOR_DEVICE_ATTR(pwm14, 0200 | 0444, show_pwm, store_pwm, 14);
+static SENSOR_DEVICE_ATTR(pwm15, 0200 | 0444, show_pwm, store_pwm, 15);
+
+static struct sensor_device_attribute sda_in_input[] = {
+ SENSOR_ATTR(fan0_input, 0444, show_in, NULL, 0),
+ SENSOR_ATTR(fan1_input, 0444, show_in, NULL, 1),
+ SENSOR_ATTR(fan2_input, 0444, show_in, NULL, 2),
+ SENSOR_ATTR(fan3_input, 0444, show_in, NULL, 3),
+ SENSOR_ATTR(fan4_input, 0444, show_in, NULL, 4),
+ SENSOR_ATTR(fan5_input, 0444, show_in, NULL, 5),
+ SENSOR_ATTR(fan6_input, 0444, show_in, NULL, 6),
+ SENSOR_ATTR(fan7_input, 0444, show_in, NULL, 7),
+ SENSOR_ATTR(fan8_input, 0444, show_in, NULL, 8),
+ SENSOR_ATTR(fan9_input, 0444, show_in, NULL, 9),
+ SENSOR_ATTR(fan10_input, 0444, show_in, NULL, 10),
+ SENSOR_ATTR(fan11_input, 0444, show_in, NULL, 11),
+ SENSOR_ATTR(fan12_input, 0444, show_in, NULL, 12),
+ SENSOR_ATTR(fan13_input, 0444, show_in, NULL, 13),
+ SENSOR_ATTR(fan14_input, 0444, show_in, NULL, 14),
+ SENSOR_ATTR(fan15_input, 0444, show_in, NULL, 15),
+};
+
+static SENSOR_DEVICE_ATTR(fan0_fault, 0444, show_fault, NULL, 0);
+static SENSOR_DEVICE_ATTR(fan1_fault, 0444, show_fault, NULL, 1);
+static SENSOR_DEVICE_ATTR(fan2_fault, 0444, show_fault, NULL, 2);
+static SENSOR_DEVICE_ATTR(fan3_fault, 0444, show_fault, NULL, 3);
+static SENSOR_DEVICE_ATTR(fan4_fault, 0444, show_fault, NULL, 4);
+static SENSOR_DEVICE_ATTR(fan5_fault, 0444, show_fault, NULL, 5);
+static SENSOR_DEVICE_ATTR(fan6_fault, 0444, show_fault, NULL, 6);
+static SENSOR_DEVICE_ATTR(fan7_fault, 0444, show_fault, NULL, 7);
+static SENSOR_DEVICE_ATTR(fan8_fault, 0444, show_fault, NULL, 8);
+static SENSOR_DEVICE_ATTR(fan9_fault, 0444, show_fault, NULL, 9);
+static SENSOR_DEVICE_ATTR(fan10_fault, 0444, show_fault, NULL, 10);
+static SENSOR_DEVICE_ATTR(fan11_fault, 0444, show_fault, NULL, 11);
+static SENSOR_DEVICE_ATTR(fan12_fault, 0444, show_fault, NULL, 12);
+static SENSOR_DEVICE_ATTR(fan13_fault, 0444, show_fault, NULL, 13);
+static SENSOR_DEVICE_ATTR(fan14_fault, 0444, show_fault, NULL, 14);
+static SENSOR_DEVICE_ATTR(fan15_fault, 0444, show_fault, NULL, 15);
+
+static struct attribute *gxp_fan_ctrl_attrs[] = {
+ &sensor_dev_attr_fan0_fault.dev_attr.attr,
+ &sensor_dev_attr_fan1_fault.dev_attr.attr,
+ &sensor_dev_attr_fan2_fault.dev_attr.attr,
+ &sensor_dev_attr_fan3_fault.dev_attr.attr,
+ &sensor_dev_attr_fan4_fault.dev_attr.attr,
+ &sensor_dev_attr_fan5_fault.dev_attr.attr,
+ &sensor_dev_attr_fan6_fault.dev_attr.attr,
+ &sensor_dev_attr_fan7_fault.dev_attr.attr,
+ &sensor_dev_attr_fan8_fault.dev_attr.attr,
+ &sensor_dev_attr_fan9_fault.dev_attr.attr,
+ &sensor_dev_attr_fan10_fault.dev_attr.attr,
+ &sensor_dev_attr_fan11_fault.dev_attr.attr,
+ &sensor_dev_attr_fan12_fault.dev_attr.attr,
+ &sensor_dev_attr_fan13_fault.dev_attr.attr,
+ &sensor_dev_attr_fan14_fault.dev_attr.attr,
+ &sensor_dev_attr_fan15_fault.dev_attr.attr,
+ &sda_in_input[0].dev_attr.attr,
+ &sda_in_input[1].dev_attr.attr,
+ &sda_in_input[2].dev_attr.attr,
+ &sda_in_input[3].dev_attr.attr,
+ &sda_in_input[4].dev_attr.attr,
+ &sda_in_input[5].dev_attr.attr,
+ &sda_in_input[6].dev_attr.attr,
+ &sda_in_input[7].dev_attr.attr,
+ &sda_in_input[8].dev_attr.attr,
+ &sda_in_input[9].dev_attr.attr,
+ &sda_in_input[10].dev_attr.attr,
+ &sda_in_input[11].dev_attr.attr,
+ &sda_in_input[12].dev_attr.attr,
+ &sda_in_input[13].dev_attr.attr,
+ &sda_in_input[14].dev_attr.attr,
+ &sda_in_input[15].dev_attr.attr,
+ &sensor_dev_attr_pwm0.dev_attr.attr,
+ &sensor_dev_attr_pwm1.dev_attr.attr,
+ &sensor_dev_attr_pwm2.dev_attr.attr,
+ &sensor_dev_attr_pwm3.dev_attr.attr,
+ &sensor_dev_attr_pwm4.dev_attr.attr,
+ &sensor_dev_attr_pwm5.dev_attr.attr,
+ &sensor_dev_attr_pwm6.dev_attr.attr,
+ &sensor_dev_attr_pwm7.dev_attr.attr,
+ &sensor_dev_attr_pwm8.dev_attr.attr,
+ &sensor_dev_attr_pwm9.dev_attr.attr,
+ &sensor_dev_attr_pwm10.dev_attr.attr,
+ &sensor_dev_attr_pwm11.dev_attr.attr,
+ &sensor_dev_attr_pwm12.dev_attr.attr,
+ &sensor_dev_attr_pwm13.dev_attr.attr,
+ &sensor_dev_attr_pwm14.dev_attr.attr,
+ &sensor_dev_attr_pwm15.dev_attr.attr,
+ NULL,
+};
+
+ATTRIBUTE_GROUPS(gxp_fan_ctrl);
+
+static struct regmap *gxp_fan_ctrl_init_regmap(struct platform_device *pdev, char *reg_name)
+{
+ struct regmap_config regmap_config = {
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ };
+ void __iomem *base;
+
+ base = devm_platform_ioremap_resource_byname(pdev, reg_name);
+ if (IS_ERR(base))
+ return ERR_CAST(base);
+
+ regmap_config.name = reg_name;
+
+ return devm_regmap_init_mmio(&pdev->dev, base, ®map_config);
+}
+
+static int gxp_fan_ctrl_probe(struct platform_device *pdev)
+{
+ struct gxp_fan_ctrl_drvdata *drvdata;
+ struct resource *res;
+ struct device *dev = &pdev->dev;
+
+ drvdata = devm_kzalloc(&pdev->dev, sizeof(struct gxp_fan_ctrl_drvdata),
+ GFP_KERNEL);
+ if (!drvdata)
+ return -ENOMEM;
+
+ drvdata->dev = &pdev->dev;
+
+ drvdata->data = of_device_get_match_data(&pdev->dev);
+
+ platform_set_drvdata(pdev, drvdata);
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ drvdata->base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(drvdata->base))
+ return dev_err_probe(dev, PTR_ERR(drvdata->base),
+ "failed to map base\n");
+ drvdata->plreg_map = gxp_fan_ctrl_init_regmap(pdev, "plreg");
+ if (IS_ERR(drvdata->plreg_map))
+ return dev_err_probe(dev, PTR_ERR(drvdata->plreg_map),
+ "failed to map plreg_handle\n");
+
+ drvdata->fn2_map = gxp_fan_ctrl_init_regmap(pdev, "fn2reg");
+ if (IS_ERR(drvdata->fn2_map))
+ return dev_err_probe(dev, PTR_ERR(drvdata->fn2_map),
+ "failed to map fn2_handle\n");
+
+ mutex_init(&drvdata->update_lock);
+
+ drvdata->hwmon_dev = devm_hwmon_device_register_with_groups(&pdev->dev,
+ "fan_ctrl",
+ drvdata,
+ gxp_fan_ctrl_groups);
+
+ return PTR_ERR_OR_ZERO(drvdata->hwmon_dev);
+}
+
+static const struct fan_ctrl_data g10_data = {
+ .fan[0] = { .inst = 0x00, .fail = 0x02, .id = 0x04, .bit = 0x01 },
+ .fan[1] = { .inst = 0x00, .fail = 0x02, .id = 0x04, .bit = 0x02 },
+ .fan[2] = { .inst = 0x00, .fail = 0x02, .id = 0x04, .bit = 0x04 },
+ .fan[3] = { .inst = 0x00, .fail = 0x02, .id = 0x04, .bit = 0x08 },
+ .fan[4] = { .inst = 0x00, .fail = 0x02, .id = 0x04, .bit = 0x10 },
+ .fan[5] = { .inst = 0x00, .fail = 0x02, .id = 0x04, .bit = 0x20 },
+ .fan[6] = { .inst = 0x00, .fail = 0x02, .id = 0x04, .bit = 0x40 },
+ .fan[7] = { .inst = 0x00, .fail = 0x02, .id = 0x04, .bit = 0x80 },
+ .fan[8] = { .inst = 0x01, .fail = 0x03, .id = 0x05, .bit = 0x01 },
+ .fan[9] = { .inst = 0x01, .fail = 0x03, .id = 0x05, .bit = 0x02 },
+ .fan[10] = { .inst = 0x01, .fail = 0x03, .id = 0x05, .bit = 0x04 },
+ .fan[11] = { .inst = 0x01, .fail = 0x03, .id = 0x05, .bit = 0x08 },
+ .fan[12] = { .inst = 0x01, .fail = 0x03, .id = 0x05, .bit = 0x10 },
+ .fan[13] = { .inst = 0x01, .fail = 0x03, .id = 0x05, .bit = 0x20 },
+ .fan[14] = { .inst = 0x01, .fail = 0x03, .id = 0x05, .bit = 0x40 },
+ .fan[15] = { .inst = 0x01, .fail = 0x03, .id = 0x05, .bit = 0x80 },
+ .power_bit = 24,
+};
+
+static const struct of_device_id gxp_fan_ctrl_of_match[] = {
+ { .compatible = "hpe,gxp-fan-ctrl", .data = &g10_data },
+ {},
+};
+MODULE_DEVICE_TABLE(of, gxp_fan_ctrl_of_match);
+
+static struct platform_driver gxp_fan_ctrl_driver = {
+ .probe = gxp_fan_ctrl_probe,
+ .driver = {
+ .name = "gxp-fan-ctrl",
+ .of_match_table = gxp_fan_ctrl_of_match,
+ },
+};
+module_platform_driver(gxp_fan_ctrl_driver);
+
+MODULE_AUTHOR("Nick Hawkins <[email protected]>");
+MODULE_DESCRIPTION("HPE GXP Fan Ctrl driver");
+MODULE_LICENSE("GPL");
--
2.17.1
From: Nick Hawkins <[email protected]>
In order for HPE platforms to be supported by linux on GXP it is
necessary for there to be fan and spi driver support. There fan driver
can support up to 16 fans that are driven by pwm through the CPLD. The
SPI driver supports access to the core flash and bios part. The SPI
driver spi-gxp was added previously to linux.
Signed-off-by: Nick Hawkins <[email protected]>
---
arch/arm/configs/multi_v7_defconfig | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index 12b35008571f..92a854e7b01b 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -445,6 +445,7 @@ CONFIG_SPI_CADENCE=y
CONFIG_SPI_DAVINCI=y
CONFIG_SPI_FSL_QUADSPI=m
CONFIG_SPI_GPIO=m
+CONFIG_SPI_GXP=m
CONFIG_SPI_FSL_DSPI=m
CONFIG_SPI_OMAP24XX=y
CONFIG_SPI_ORION=y
@@ -535,6 +536,7 @@ CONFIG_SENSORS_NTC_THERMISTOR=m
CONFIG_SENSORS_PWM_FAN=m
CONFIG_SENSORS_RASPBERRYPI_HWMON=m
CONFIG_SENSORS_INA2XX=m
+CONFIG_SENSORS_GXP_FAN_CTRL=m
CONFIG_CPU_THERMAL=y
CONFIG_DEVFREQ_THERMAL=y
CONFIG_IMX_THERMAL=y
--
2.17.1
On Fri, Nov 04, 2022 at 02:36:52PM -0500, [email protected] wrote:
> From: Nick Hawkins <[email protected]>
>
> The GXP SoC can support up to 16 fans through the interface provided by
> the CPLD. The fans speeds are controlled via a pwm value 0-255. The fans
> are also capable of reporting if they have failed to the CPLD which in
> turn reports the status to the GXP SoC. There are no tachometers so fan
> speeds are reported as a percent of the pwm value.
Drop the last sentence and the associated code. More on that below.
>
> Signed-off-by: Nick Hawkins <[email protected]>
> ---
> Documentation/hwmon/gxp-fan-ctrl.rst | 36 +++
Needs to be added to Documentation/hwmon/index.rst.
> drivers/hwmon/Kconfig | 8 +
> drivers/hwmon/Makefile | 1 +
> drivers/hwmon/gxp-fan-ctrl.c | 362 +++++++++++++++++++++++++++
> 4 files changed, 407 insertions(+)
> create mode 100644 Documentation/hwmon/gxp-fan-ctrl.rst
> create mode 100644 drivers/hwmon/gxp-fan-ctrl.c
>
> diff --git a/Documentation/hwmon/gxp-fan-ctrl.rst b/Documentation/hwmon/gxp-fan-ctrl.rst
> new file mode 100644
> index 000000000000..fc1709fb113b
> --- /dev/null
> +++ b/Documentation/hwmon/gxp-fan-ctrl.rst
> @@ -0,0 +1,36 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +Kernel driver gxp-fan-ctrl
> +==========================
> +
> +Supported chips:
> +
> + * HPE GXP SOC
> +
> +Author: Nick Hawkins <[email protected]>
> +
> +
> +Description
> +-----------
> +
> +gxp-fan-ctrl is a driver which provides fan control for the hpe gxp soc.
> +The driver allows the gathering of fan status and the use of fan
> +pwm control.
> +
> +
> +Usage Notes
> +-----------
> +
> +Traditionally fanY_input returns an RPM value, on HPE GXP systems it is
> +the pwm value [0-255] due to the fan speeds being reported as
> +percentages.
It seems to me what is reported is the pwm value sent to the fan,
and the code stringly suggests that this is the case. If the chip
or controller doesn't report anything else, don't claim that this
has any relation to the fan speed, and just report the pwm value.
> +
> +
> +Sysfs attributes
> +----------------
> +
> +======================= =================================================
> +pwm[0-15] Fan 0 to 15 respective pwm value
> +fan[0-15]_input Fan 0 to 15 respective input value: pwm value
> +fan[0-15]_fault Fan 0 to 15 respective fault status: 1 fail, 0 ok
> +======================= =================================================
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index e70d9614bec2..3d32cd77424c 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -2303,6 +2303,14 @@ config SENSORS_INTEL_M10_BMC_HWMON
> sensors monitor various telemetry data of different components on the
> card, e.g. board temperature, FPGA core temperature/voltage/current.
>
> +config SENSORS_GXP_FAN_CTRL
> + tristate "GXP Fan Control driver"
> + depends on ARCH_HPE_GXP || COMPILE_TEST
> + help
> + If you say yes here you get support for GXP fan control functionality.
> + The GXP controls fan function via the CPLD through the use of PWM
> + registers. This driver reports status and pwm setting of the fans.
> +
> if ACPI
>
> comment "ACPI drivers"
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 007e829d1d0d..b474dcc708c4 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -83,6 +83,7 @@ obj-$(CONFIG_SENSORS_GL518SM) += gl518sm.o
> obj-$(CONFIG_SENSORS_GL520SM) += gl520sm.o
> obj-$(CONFIG_SENSORS_GSC) += gsc-hwmon.o
> obj-$(CONFIG_SENSORS_GPIO_FAN) += gpio-fan.o
> +obj-$(CONFIG_SENSORS_GXP_FAN_CTRL) += gxp-fan-ctrl.o
> obj-$(CONFIG_SENSORS_HIH6130) += hih6130.o
> obj-$(CONFIG_SENSORS_ULTRA45) += ultra45_env.o
> obj-$(CONFIG_SENSORS_I5500) += i5500_temp.o
> diff --git a/drivers/hwmon/gxp-fan-ctrl.c b/drivers/hwmon/gxp-fan-ctrl.c
> new file mode 100644
> index 000000000000..a01530951d58
> --- /dev/null
> +++ b/drivers/hwmon/gxp-fan-ctrl.c
> @@ -0,0 +1,362 @@
> +// SPDX-License-Identifier: GPL-2.0=or-later
> +/* Copyright (C) 2022 Hewlett-Packard Enterprise Development Company, L.P. */
> +
> +#include <linux/err.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/mfd/syscon.h>
> +
> +#define OFFSET_PWM0DUTY 0x10
> +#define OFFSET_PWM1DUTY 0x11
> +#define OFFSET_PWM2DUTY 0x12
> +#define OFFSET_PWM3DUTY 0x13
> +#define OFFSET_PWM4DUTY 0x14
> +#define OFFSET_PWM5DUTY 0x15
> +#define OFFSET_PWM6DUTY 0x16
> +#define OFFSET_PWM7DUTY 0x17
OFFSET_PWM[1-7]DUTY are not used anywhere. Please drop.
> +
> +struct fan_data {
> + u32 inst;
> + u32 fail;
> + u32 id;
> + u32 bit;
> +};
> +
> +struct fan_ctrl_data {
> + struct fan_data fan[16];
> + u32 power_bit;
> +};
> +
> +struct gxp_fan_ctrl_drvdata {
> + struct device *dev;
> + struct device *hwmon_dev;
> + struct regmap *plreg_map; /* Programmable logic register regmap */
> + struct regmap *fn2_map; /* Function 2 regmap */
> + void __iomem *base;
> + const struct fan_ctrl_data *data;
> + struct mutex update_lock; /* To protect the setting of the fan PWM value */
> +};
> +
> +static void address_translation(u32 desired_offset, u32 *offset, u32 *bit_shift)
> +{
> + *offset = (desired_offset & 0xffc);
> + *bit_shift = (desired_offset - *offset) * 8;
> +}
> +
> +static bool fan_installed(struct device *dev, int fan)
> +{
> + struct gxp_fan_ctrl_drvdata *drvdata = dev_get_drvdata(dev);
> + u32 trans_offset;
> + u32 trans_shift;
> + u32 val;
> +
> + address_translation(drvdata->data->fan[fan].inst,
> + &trans_offset,
> + &trans_shift);
> +
> + regmap_read(drvdata->plreg_map, trans_offset, &val);
> + val = (val >> trans_shift) & drvdata->data->fan[fan].bit;
> + if (val == drvdata->data->fan[fan].bit)
> + return 1;
> + else
> + return 0;
return val == drvdata->data->fan[fan].bit;
Those calculations look quite complex. Is there a public datasheet
that would enable me to understand how registers are actually assigned ?
> +}
> +
> +static bool fan_failed(struct device *dev, int fan)
> +{
> + struct gxp_fan_ctrl_drvdata *drvdata = dev_get_drvdata(dev);
> + u32 trans_offset;
> + u32 trans_shift;
> + u32 val;
> +
> + address_translation(drvdata->data->fan[fan].fail,
> + &trans_offset,
> + &trans_shift);
> +
> + regmap_read(drvdata->plreg_map, trans_offset, &val);
> + val = (val >> trans_shift) & drvdata->data->fan[fan].fail;
> + if (val == drvdata->data->fan[fan].fail)
> + return 1;
> + else
> + return 0;
return val == drvdata->data->fan[fan].fail;
> +}
> +
> +static ssize_t show_fault(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + int nr = (to_sensor_dev_attr(attr))->index;
> + unsigned char val;
> +
> + val = (fan_failed(dev, nr)) ? 1 : 0;
This is really unnecessary. bool translates to 1/0. There is no need
for another layer of translation.
> +
> + return sprintf(buf, "%d\n", val);
> +}
> +
> +static ssize_t show_in(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + int nr = (to_sensor_dev_attr(attr))->index;
> + struct gxp_fan_ctrl_drvdata *drvdata = dev_get_drvdata(dev);
> + unsigned char val;
> + unsigned int reg;
> +
> + /* Check Power Status */
> + regmap_read(drvdata->fn2_map, 0, ®);
> + if (reg & BIT(drvdata->data->power_bit)) {
> + /* If Fan presents, then read it. */
is present
> + val = (fan_installed(dev, nr)) ? readb(drvdata->base +
> + OFFSET_PWM0DUTY +
> + nr) : 0;
Various unnecessary ( ) throughout the code.
> + } else {
> + /* Power Off */
> + val = 0;
> + }
What determines power to a fan ? Should the power state be reported
with fanX_enable ? Or possibly the installed state ?
> +
> + return sprintf(buf, "%d\n", val);
> +}
> +
> +static ssize_t show_pwm(struct device *dev, struct device_attribute *attr,
> + char *buf)
> +{
> + int nr = (to_sensor_dev_attr(attr))->index;
> + struct gxp_fan_ctrl_drvdata *drvdata = dev_get_drvdata(dev);
> + unsigned char val;
> +
> + val = readb(drvdata->base + OFFSET_PWM0DUTY + nr);
> +
> + return sprintf(buf, "%d\n", val);
> +}
> +
> +static ssize_t store_pwm(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + int nr = (to_sensor_dev_attr(attr))->index;
> + struct gxp_fan_ctrl_drvdata *drvdata = dev_get_drvdata(dev);
> + unsigned long val;
> + int err;
> +
> + err = kstrtoul(buf, 10, &val);
> + if (err)
> + return err;
> +
> + if (val > 255)
> + return -1; /* out of range */
Please always use standard error codes. This should be -EINVAL,
not -EPERM (-1).
> +
> + mutex_lock(&drvdata->update_lock);
> +
> + writeb(val, drvdata->base + OFFSET_PWM0DUTY + nr);
> +
> + mutex_unlock(&drvdata->update_lock);
Mutex is pointless here.
> + return count;
> +}
> +
> +static SENSOR_DEVICE_ATTR(pwm0, 0200 | 0444, show_pwm, store_pwm, 0);
> +static SENSOR_DEVICE_ATTR(pwm1, 0200 | 0444, show_pwm, store_pwm, 1);
> +static SENSOR_DEVICE_ATTR(pwm2, 0200 | 0444, show_pwm, store_pwm, 2);
> +static SENSOR_DEVICE_ATTR(pwm3, 0200 | 0444, show_pwm, store_pwm, 3);
> +static SENSOR_DEVICE_ATTR(pwm4, 0200 | 0444, show_pwm, store_pwm, 4);
> +static SENSOR_DEVICE_ATTR(pwm5, 0200 | 0444, show_pwm, store_pwm, 5);
> +static SENSOR_DEVICE_ATTR(pwm6, 0200 | 0444, show_pwm, store_pwm, 6);
> +static SENSOR_DEVICE_ATTR(pwm7, 0200 | 0444, show_pwm, store_pwm, 7);
> +static SENSOR_DEVICE_ATTR(pwm8, 0200 | 0444, show_pwm, store_pwm, 8);
> +static SENSOR_DEVICE_ATTR(pwm9, 0200 | 0444, show_pwm, store_pwm, 9);
> +static SENSOR_DEVICE_ATTR(pwm10, 0200 | 0444, show_pwm, store_pwm, 10);
> +static SENSOR_DEVICE_ATTR(pwm11, 0200 | 0444, show_pwm, store_pwm, 11);
> +static SENSOR_DEVICE_ATTR(pwm12, 0200 | 0444, show_pwm, store_pwm, 12);
> +static SENSOR_DEVICE_ATTR(pwm13, 0200 | 0444, show_pwm, store_pwm, 13);
> +static SENSOR_DEVICE_ATTR(pwm14, 0200 | 0444, show_pwm, store_pwm, 14);
> +static SENSOR_DEVICE_ATTR(pwm15, 0200 | 0444, show_pwm, store_pwm, 15);
> +
> +static struct sensor_device_attribute sda_in_input[] = {
> + SENSOR_ATTR(fan0_input, 0444, show_in, NULL, 0),
> + SENSOR_ATTR(fan1_input, 0444, show_in, NULL, 1),
> + SENSOR_ATTR(fan2_input, 0444, show_in, NULL, 2),
> + SENSOR_ATTR(fan3_input, 0444, show_in, NULL, 3),
> + SENSOR_ATTR(fan4_input, 0444, show_in, NULL, 4),
> + SENSOR_ATTR(fan5_input, 0444, show_in, NULL, 5),
> + SENSOR_ATTR(fan6_input, 0444, show_in, NULL, 6),
> + SENSOR_ATTR(fan7_input, 0444, show_in, NULL, 7),
> + SENSOR_ATTR(fan8_input, 0444, show_in, NULL, 8),
> + SENSOR_ATTR(fan9_input, 0444, show_in, NULL, 9),
> + SENSOR_ATTR(fan10_input, 0444, show_in, NULL, 10),
> + SENSOR_ATTR(fan11_input, 0444, show_in, NULL, 11),
> + SENSOR_ATTR(fan12_input, 0444, show_in, NULL, 12),
> + SENSOR_ATTR(fan13_input, 0444, show_in, NULL, 13),
> + SENSOR_ATTR(fan14_input, 0444, show_in, NULL, 14),
> + SENSOR_ATTR(fan15_input, 0444, show_in, NULL, 15),
> +};
> +
> +static SENSOR_DEVICE_ATTR(fan0_fault, 0444, show_fault, NULL, 0);
> +static SENSOR_DEVICE_ATTR(fan1_fault, 0444, show_fault, NULL, 1);
> +static SENSOR_DEVICE_ATTR(fan2_fault, 0444, show_fault, NULL, 2);
> +static SENSOR_DEVICE_ATTR(fan3_fault, 0444, show_fault, NULL, 3);
> +static SENSOR_DEVICE_ATTR(fan4_fault, 0444, show_fault, NULL, 4);
> +static SENSOR_DEVICE_ATTR(fan5_fault, 0444, show_fault, NULL, 5);
> +static SENSOR_DEVICE_ATTR(fan6_fault, 0444, show_fault, NULL, 6);
> +static SENSOR_DEVICE_ATTR(fan7_fault, 0444, show_fault, NULL, 7);
> +static SENSOR_DEVICE_ATTR(fan8_fault, 0444, show_fault, NULL, 8);
> +static SENSOR_DEVICE_ATTR(fan9_fault, 0444, show_fault, NULL, 9);
> +static SENSOR_DEVICE_ATTR(fan10_fault, 0444, show_fault, NULL, 10);
> +static SENSOR_DEVICE_ATTR(fan11_fault, 0444, show_fault, NULL, 11);
> +static SENSOR_DEVICE_ATTR(fan12_fault, 0444, show_fault, NULL, 12);
> +static SENSOR_DEVICE_ATTR(fan13_fault, 0444, show_fault, NULL, 13);
> +static SENSOR_DEVICE_ATTR(fan14_fault, 0444, show_fault, NULL, 14);
> +static SENSOR_DEVICE_ATTR(fan15_fault, 0444, show_fault, NULL, 15);
> +
> +static struct attribute *gxp_fan_ctrl_attrs[] = {
> + &sensor_dev_attr_fan0_fault.dev_attr.attr,
> + &sensor_dev_attr_fan1_fault.dev_attr.attr,
> + &sensor_dev_attr_fan2_fault.dev_attr.attr,
> + &sensor_dev_attr_fan3_fault.dev_attr.attr,
> + &sensor_dev_attr_fan4_fault.dev_attr.attr,
> + &sensor_dev_attr_fan5_fault.dev_attr.attr,
> + &sensor_dev_attr_fan6_fault.dev_attr.attr,
> + &sensor_dev_attr_fan7_fault.dev_attr.attr,
> + &sensor_dev_attr_fan8_fault.dev_attr.attr,
> + &sensor_dev_attr_fan9_fault.dev_attr.attr,
> + &sensor_dev_attr_fan10_fault.dev_attr.attr,
> + &sensor_dev_attr_fan11_fault.dev_attr.attr,
> + &sensor_dev_attr_fan12_fault.dev_attr.attr,
> + &sensor_dev_attr_fan13_fault.dev_attr.attr,
> + &sensor_dev_attr_fan14_fault.dev_attr.attr,
> + &sensor_dev_attr_fan15_fault.dev_attr.attr,
> + &sda_in_input[0].dev_attr.attr,
> + &sda_in_input[1].dev_attr.attr,
> + &sda_in_input[2].dev_attr.attr,
> + &sda_in_input[3].dev_attr.attr,
> + &sda_in_input[4].dev_attr.attr,
> + &sda_in_input[5].dev_attr.attr,
> + &sda_in_input[6].dev_attr.attr,
> + &sda_in_input[7].dev_attr.attr,
> + &sda_in_input[8].dev_attr.attr,
> + &sda_in_input[9].dev_attr.attr,
> + &sda_in_input[10].dev_attr.attr,
> + &sda_in_input[11].dev_attr.attr,
> + &sda_in_input[12].dev_attr.attr,
> + &sda_in_input[13].dev_attr.attr,
> + &sda_in_input[14].dev_attr.attr,
> + &sda_in_input[15].dev_attr.attr,
> + &sensor_dev_attr_pwm0.dev_attr.attr,
> + &sensor_dev_attr_pwm1.dev_attr.attr,
> + &sensor_dev_attr_pwm2.dev_attr.attr,
> + &sensor_dev_attr_pwm3.dev_attr.attr,
> + &sensor_dev_attr_pwm4.dev_attr.attr,
> + &sensor_dev_attr_pwm5.dev_attr.attr,
> + &sensor_dev_attr_pwm6.dev_attr.attr,
> + &sensor_dev_attr_pwm7.dev_attr.attr,
> + &sensor_dev_attr_pwm8.dev_attr.attr,
> + &sensor_dev_attr_pwm9.dev_attr.attr,
> + &sensor_dev_attr_pwm10.dev_attr.attr,
> + &sensor_dev_attr_pwm11.dev_attr.attr,
> + &sensor_dev_attr_pwm12.dev_attr.attr,
> + &sensor_dev_attr_pwm13.dev_attr.attr,
> + &sensor_dev_attr_pwm14.dev_attr.attr,
> + &sensor_dev_attr_pwm15.dev_attr.attr,
> + NULL,
> +};
> +
> +ATTRIBUTE_GROUPS(gxp_fan_ctrl);
> +
> +static struct regmap *gxp_fan_ctrl_init_regmap(struct platform_device *pdev, char *reg_name)
> +{
> + struct regmap_config regmap_config = {
> + .reg_bits = 32,
> + .reg_stride = 4,
> + .val_bits = 32,
> + };
> + void __iomem *base;
> +
> + base = devm_platform_ioremap_resource_byname(pdev, reg_name);
> + if (IS_ERR(base))
> + return ERR_CAST(base);
> +
> + regmap_config.name = reg_name;
> +
> + return devm_regmap_init_mmio(&pdev->dev, base, ®map_config);
> +}
> +
> +static int gxp_fan_ctrl_probe(struct platform_device *pdev)
> +{
> + struct gxp_fan_ctrl_drvdata *drvdata;
> + struct resource *res;
> + struct device *dev = &pdev->dev;
> +
> + drvdata = devm_kzalloc(&pdev->dev, sizeof(struct gxp_fan_ctrl_drvdata),
> + GFP_KERNEL);
> + if (!drvdata)
> + return -ENOMEM;
> +
> + drvdata->dev = &pdev->dev;
> +
> + drvdata->data = of_device_get_match_data(&pdev->dev);
> +
> + platform_set_drvdata(pdev, drvdata);
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + drvdata->base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(drvdata->base))
> + return dev_err_probe(dev, PTR_ERR(drvdata->base),
> + "failed to map base\n");
> + drvdata->plreg_map = gxp_fan_ctrl_init_regmap(pdev, "plreg");
> + if (IS_ERR(drvdata->plreg_map))
> + return dev_err_probe(dev, PTR_ERR(drvdata->plreg_map),
> + "failed to map plreg_handle\n");
> +
> + drvdata->fn2_map = gxp_fan_ctrl_init_regmap(pdev, "fn2reg");
> + if (IS_ERR(drvdata->fn2_map))
> + return dev_err_probe(dev, PTR_ERR(drvdata->fn2_map),
> + "failed to map fn2_handle\n");
> +
> + mutex_init(&drvdata->update_lock);
> +
> + drvdata->hwmon_dev = devm_hwmon_device_register_with_groups(&pdev->dev,
> + "fan_ctrl",
> + drvdata,
> + gxp_fan_ctrl_groups);
New drivers must register with devm_hwmon_device_register_with_info().
> +
> + return PTR_ERR_OR_ZERO(drvdata->hwmon_dev);
> +}
> +
> +static const struct fan_ctrl_data g10_data = {
> + .fan[0] = { .inst = 0x00, .fail = 0x02, .id = 0x04, .bit = 0x01 },
> + .fan[1] = { .inst = 0x00, .fail = 0x02, .id = 0x04, .bit = 0x02 },
> + .fan[2] = { .inst = 0x00, .fail = 0x02, .id = 0x04, .bit = 0x04 },
> + .fan[3] = { .inst = 0x00, .fail = 0x02, .id = 0x04, .bit = 0x08 },
> + .fan[4] = { .inst = 0x00, .fail = 0x02, .id = 0x04, .bit = 0x10 },
> + .fan[5] = { .inst = 0x00, .fail = 0x02, .id = 0x04, .bit = 0x20 },
> + .fan[6] = { .inst = 0x00, .fail = 0x02, .id = 0x04, .bit = 0x40 },
> + .fan[7] = { .inst = 0x00, .fail = 0x02, .id = 0x04, .bit = 0x80 },
> + .fan[8] = { .inst = 0x01, .fail = 0x03, .id = 0x05, .bit = 0x01 },
> + .fan[9] = { .inst = 0x01, .fail = 0x03, .id = 0x05, .bit = 0x02 },
> + .fan[10] = { .inst = 0x01, .fail = 0x03, .id = 0x05, .bit = 0x04 },
> + .fan[11] = { .inst = 0x01, .fail = 0x03, .id = 0x05, .bit = 0x08 },
> + .fan[12] = { .inst = 0x01, .fail = 0x03, .id = 0x05, .bit = 0x10 },
> + .fan[13] = { .inst = 0x01, .fail = 0x03, .id = 0x05, .bit = 0x20 },
> + .fan[14] = { .inst = 0x01, .fail = 0x03, .id = 0x05, .bit = 0x40 },
> + .fan[15] = { .inst = 0x01, .fail = 0x03, .id = 0x05, .bit = 0x80 },
> + .power_bit = 24,
> +};
> +
> +static const struct of_device_id gxp_fan_ctrl_of_match[] = {
> + { .compatible = "hpe,gxp-fan-ctrl", .data = &g10_data },
I don't understand the point of attaching g10_data here.
Why not just access it directly ? There is just one table.
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, gxp_fan_ctrl_of_match);
> +
> +static struct platform_driver gxp_fan_ctrl_driver = {
> + .probe = gxp_fan_ctrl_probe,
> + .driver = {
> + .name = "gxp-fan-ctrl",
> + .of_match_table = gxp_fan_ctrl_of_match,
> + },
> +};
> +module_platform_driver(gxp_fan_ctrl_driver);
> +
> +MODULE_AUTHOR("Nick Hawkins <[email protected]>");
> +MODULE_DESCRIPTION("HPE GXP Fan Ctrl driver");
> +MODULE_LICENSE("GPL");
> --
> 2.17.1
>
On 04/11/2022 20:36, [email protected] wrote:
> From: Nick Hawkins <[email protected]>
>
> In order for HPE platforms to be supported by linux on GXP it is
s/linux/Linux/
> necessary for there to be fan and spi driver support. There fan driver
> can support up to 16 fans that are driven by pwm through the CPLD. The
s/pwm/PWM/
> SPI driver supports access to the core flash and bios part. The SPI
> driver spi-gxp was added previously to linux.
s/linux/Linux/
>
> Signed-off-by: Nick Hawkins <[email protected]>
> ---
> arch/arm/configs/multi_v7_defconfig | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
> index 12b35008571f..92a854e7b01b 100644
> --- a/arch/arm/configs/multi_v7_defconfig
> +++ b/arch/arm/configs/multi_v7_defconfig
> @@ -445,6 +445,7 @@ CONFIG_SPI_CADENCE=y
> CONFIG_SPI_DAVINCI=y
> CONFIG_SPI_FSL_QUADSPI=m
> CONFIG_SPI_GPIO=m
> +CONFIG_SPI_GXP=m
> CONFIG_SPI_FSL_DSPI=m
> CONFIG_SPI_OMAP24XX=y
> CONFIG_SPI_ORION=y
> @@ -535,6 +536,7 @@ CONFIG_SENSORS_NTC_THERMISTOR=m
> CONFIG_SENSORS_PWM_FAN=m
> CONFIG_SENSORS_RASPBERRYPI_HWMON=m
> CONFIG_SENSORS_INA2XX=m
> +CONFIG_SENSORS_GXP_FAN_CTRL=m
> CONFIG_CPU_THERMAL=y
> CONFIG_DEVFREQ_THERMAL=y
> CONFIG_IMX_THERMAL=y
Best regards,
Krzysztof
On 04/11/2022 20:36, [email protected] wrote:
> From: Nick Hawkins <[email protected]>
>
> This provides the base registers address, programmable logic registers
> address, and the function 2 registers to allow control access of the HPE
> fans on the GXP SoC.
What is "This"? If "This patch", then drop it.
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95
If "This hardware" then please instead describe the hardware, not it
components. What are its features? If it controls the fan, then why
there are no PWM-related cells? How do you set the speed?
>
> Signed-off-by: Nick Hawkins <[email protected]>
> ---
> .../bindings/hwmon/hpe,gxp-fan-ctrl.yaml | 41 +++++++++++++++++++
> 1 file changed, 41 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/hwmon/hpe,gxp-fan-ctrl.yaml
>
> diff --git a/Documentation/devicetree/bindings/hwmon/hpe,gxp-fan-ctrl.yaml b/Documentation/devicetree/bindings/hwmon/hpe,gxp-fan-ctrl.yaml
> new file mode 100644
> index 000000000000..40a5d9cd0a30
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/hpe,gxp-fan-ctrl.yaml
> @@ -0,0 +1,41 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwmon/hpe,gxp-fan-ctrl.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: GXP Fan Controller
> +
> +maintainers:
> + - Nick Hawkins <[email protected]>
> +
> +properties:
> + compatible:
> + const: hpe,gxp-fan-ctrl
> +
> + reg:
> + items:
> + - description: Fan controller base register
> + - description: Programmable logic registers base
> + - description: Function 2 registers base
Drop "register" and "base" from all descriptions
> +
> + reg-names:
> + items:
> + - const: base
> + - const: plreg
Drop reg suffix
> + - const: fn2reg
Drop reg suffix
> +
> +required:
> + - compatible
> + - reg
> + - reg-names
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + fanctrl@1000c00 {
fan-controller
> + compatible = "hpe,gxp-fan-ctrl";
> + reg = <0x1000c00 0x200>, <0xd1000000 0xff>, <0x80200000 0x100000>;
> + reg-names = "base", "plreg", "fn2reg";
> + };
Best regards,
Krzysztof
On Fri, Nov 04, 2022 at 02:36:52PM -0500, [email protected] wrote:
> diff --git a/Documentation/hwmon/gxp-fan-ctrl.rst b/Documentation/hwmon/gxp-fan-ctrl.rst
> new file mode 100644
> index 000000000000..fc1709fb113b
> --- /dev/null
> +++ b/Documentation/hwmon/gxp-fan-ctrl.rst
> @@ -0,0 +1,36 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +Kernel driver gxp-fan-ctrl
> +==========================
> +
> +Supported chips:
> +
> + * HPE GXP SOC
> +
> +Author: Nick Hawkins <[email protected]>
> +
> +
> +Description
> +-----------
> +
> +gxp-fan-ctrl is a driver which provides fan control for the hpe gxp soc.
> +The driver allows the gathering of fan status and the use of fan
> +pwm control.
> +
> +
> +Usage Notes
> +-----------
> +
> +Traditionally fanY_input returns an RPM value, on HPE GXP systems it is
> +the pwm value [0-255] due to the fan speeds being reported as
> +percentages.
> +
> +
> +Sysfs attributes
> +----------------
> +
> +======================= =================================================
> +pwm[0-15] Fan 0 to 15 respective pwm value
> +fan[0-15]_input Fan 0 to 15 respective input value: pwm value
> +fan[0-15]_fault Fan 0 to 15 respective fault status: 1 fail, 0 ok
> +======================= =================================================
You need to add the documentation to toctree of hwmon documentation:
---- >8 ----
diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
index ddff3c5713d74e..29ecef3ba4870b 100644
--- a/Documentation/hwmon/index.rst
+++ b/Documentation/hwmon/index.rst
@@ -73,6 +73,7 @@ Hardware Monitoring Kernel Drivers
g762
gsc-hwmon
gl518sm
+ gxp-fan-ctrl
hih6130
ibmaem
ibm-cffps
Thanks.
--
An old man doll... just what I always wanted! - Clara
> > This provides the base registers address, programmable logic registers
> > address, and the function 2 registers to allow control access of the HPE
> > fans on the GXP SoC.
> What is "This"? If "This patch", then drop it.
> https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95
> If "This hardware" then please instead describe the hardware, not it
components. What are its features? If it controls the fan, then why
there are no PWM-related cells? How do you set the speed?
Greetings Krzysztof,
Thank you for the feedback. The intention was this binding.. however, that was an error on my part, and I will correct it to reflect the hardware situation of the GXP with the fan controller and how each of the mapped registers provide control to the system. To answer your questions: The fans speeds are controlled through an external CPLD device which we provide a PWM value (0-255) using the "base" register to the CIF interface. This interface provides access to the CPLD. The CPLD then drives the fan. The CPLD can generate up to 8 unique different PWMs to multiple fans. The CPLD monitors the fans and reports the status back to the SoC through the CIF interface to the "plreg base". The plreg includes the installation, failed, and identification statuses. The function 2 register base is used to check the power state of the system as that influences the PWM values read back.
As the PWM generation happens outside the SoC do we still need pwm-cells? If so, should we have a custom compatible for that?
Thanks,
-Nick
Hi,
I love your patch! Perhaps something to improve:
[auto build test WARNING on groeck-staging/hwmon-next]
[also build test WARNING on robh/for-next linus/master v6.1-rc4 next-20221107]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/nick-hawkins-hpe-com/ARM-Add-GXP-Fan-and-SPI-controllers/20221105-034118
base: https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
patch link: https://lore.kernel.org/r/20221104193657.105130-2-nick.hawkins%40hpe.com
patch subject: [PATCH v1 1/6] hwmon: (gxp-fan-ctrl) Add GXP fan controller
reproduce:
# https://github.com/intel-lab-lkp/linux/commit/bd3528b8ab29e7be698674e7ad68ea85938f9015
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review nick-hawkins-hpe-com/ARM-Add-GXP-Fan-and-SPI-controllers/20221105-034118
git checkout bd3528b8ab29e7be698674e7ad68ea85938f9015
make menuconfig
# enable CONFIG_COMPILE_TEST, CONFIG_WARN_MISSING_DOCUMENTS, CONFIG_WARN_ABI_ERRORS
make htmldocs
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
>> Documentation/hwmon/gxp-fan-ctrl.rst: WARNING: document isn't included in any toctree
--
0-DAY CI Kernel Test Service
https://01.org/lkp
On 07/11/2022 23:36, Hawkins, Nick wrote:
>
> > > This provides the base registers address, programmable logic registers
> > > address, and the function 2 registers to allow control access of the HPE
> > > fans on the GXP SoC.
>
> > What is "This"? If "This patch", then drop it.
> > https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95
>
> > If "This hardware" then please instead describe the hardware, not it
> components. What are its features? If it controls the fan, then why
> there are no PWM-related cells? How do you set the speed?
>
> Greetings Krzysztof,
>
> Thank you for the feedback. The intention was this binding.. however, that was an error on my part, and I will correct it to reflect the hardware situation of the GXP with the fan controller and how each of the mapped registers provide control to the system. To answer your questions: The fans speeds are controlled through an external CPLD device which we provide a PWM value (0-255) using the "base" register to the CIF interface.
Wrap your emails, it's impossible to simply reply to it.
Then your CIF interface is a PWM device?
> This interface provides access to the CPLD. The CPLD then drives the fan. The CPLD can generate up to 8 unique different PWMs to multiple fans.
So you have other CPLD (not external) which generates PWM based on first
CPLD base register? Hm, I think it's one CPLD.
> The CPLD monitors the fans and reports the status back to the SoC through the CIF interface to the "plreg base". The plreg includes the installation, failed, and identification statuses. The function 2 register base is used to check the power state of the system as that influences the PWM values read back.
> As the PWM generation happens outside the SoC do we still need pwm-cells? If so, should we have a custom compatible for that?
>
Depends, if these are actually tightly coupled and you cannot use PWM
for anything else, then you do not need.
> Thanks,
>
> -Nick
>
>
Best regards,
Krzysztof
Greetings Guenter,
> > +static bool fan_installed(struct device *dev, int fan) {
> > + struct gxp_fan_ctrl_drvdata *drvdata = dev_get_drvdata(dev);
> > + u32 trans_offset;
> > + u32 trans_shift;
> > + u32 val;
> > +
> > + address_translation(drvdata->data->fan[fan].inst,
> > + &trans_offset,
> > + &trans_shift);
> > +
> > + regmap_read(drvdata->plreg_map, trans_offset, &val);
> > + val = (val >> trans_shift) & drvdata->data->fan[fan].bit;
> > + if (val == drvdata->data->fan[fan].bit)
> > + return 1;
> > + else
> > + return 0;
> return val == drvdata->data->fan[fan].bit;
> Those calculations look quite complex. Is there a public datasheet that would enable me to understand how registers are actually assigned ?
There is no public datasheet as of yet but there is work ongoing to
create one. I will however document exactly how it is setup in hwmon.
There is so much I/O on our board that most of the inputs and outputs
go through an external CPLD we are interfaced with to save pins. A
memory area in our SoC reflects some of the I/O from CPLD in bytes
ranging from 0 to 0xff. Each byte represents information such as
byte 0x27, which on this particular platform represents the fan
installation status of fans 0 to 7 respectively with bit 0 to 7. The
byte 0x28 represents something else. Regmap_read/write does a word
instead of a single byte which we are interested in so we use
address_translation to keep offsets easier to read.
> > + } else {
> > + /* Power Off */
> > + val = 0;
> > + }
> What determines power to a fan ? Should the power state be reported with fanX_enable ? Or possibly the installed state ?
This actually is the power state of the system, not the fan. When the
system is off we will see a PWM value of 0xFF on the fan. The idea
here was to report a value of 0 if the system was off.
Would you like me to use fanX_enable (read only) to show it as
disabled while the system is off ? From a hardware standpoint
that would be accurate.
> > +static const struct fan_ctrl_data g10_data = {
> > + .fan[0] = { .inst = 0x00, .fail = 0x02, .id = 0x04, .bit = 0x01 },
> > + .fan[1] = { .inst = 0x00, .fail = 0x02, .id = 0x04, .bit = 0x02 },
> > + .fan[2] = { .inst = 0x00, .fail = 0x02, .id = 0x04, .bit = 0x04 },
> > + .fan[3] = { .inst = 0x00, .fail = 0x02, .id = 0x04, .bit = 0x08 },
> > + .fan[4] = { .inst = 0x00, .fail = 0x02, .id = 0x04, .bit = 0x10 },
> > + .fan[5] = { .inst = 0x00, .fail = 0x02, .id = 0x04, .bit = 0x20 },
> > + .fan[6] = { .inst = 0x00, .fail = 0x02, .id = 0x04, .bit = 0x40 },
> > + .fan[7] = { .inst = 0x00, .fail = 0x02, .id = 0x04, .bit = 0x80 },
> > + .fan[8] = { .inst = 0x01, .fail = 0x03, .id = 0x05, .bit = 0x01 },
> > + .fan[9] = { .inst = 0x01, .fail = 0x03, .id = 0x05, .bit = 0x02 },
> > + .fan[10] = { .inst = 0x01, .fail = 0x03, .id = 0x05, .bit = 0x04 },
> > + .fan[11] = { .inst = 0x01, .fail = 0x03, .id = 0x05, .bit = 0x08 },
> > + .fan[12] = { .inst = 0x01, .fail = 0x03, .id = 0x05, .bit = 0x10 },
> > + .fan[13] = { .inst = 0x01, .fail = 0x03, .id = 0x05, .bit = 0x20 },
> > + .fan[14] = { .inst = 0x01, .fail = 0x03, .id = 0x05, .bit = 0x40 },
> > + .fan[15] = { .inst = 0x01, .fail = 0x03, .id = 0x05, .bit = 0x80 },
> > + .power_bit = 24,
> > +};
> > +
> > +static const struct of_device_id gxp_fan_ctrl_of_match[] = {
> > + { .compatible = "hpe,gxp-fan-ctrl", .data = &g10_data },
> I don't understand the point of attaching g10_data here.
> Why not just access it directly ? There is just one table.
The reason for having this data with the of_device_id binding is that
each platform has different byte offsets as mentioned above. We
would like to be able to reuse the driver if possible for this. We will
soon need g11_data that will be added here.
Would a description in Documentation, comments and commit message
allow us to keep this ?
Thank you for your assistance and feedback with this code,
-Nick Hawkins
Note: This is a resend, my email client decided to
Change my paragraph format with 70 char lines.
Apologies.
Greetings Guenter,
> > +static bool fan_installed(struct device *dev, int fan) {
> > + struct gxp_fan_ctrl_drvdata *drvdata = dev_get_drvdata(dev);
> > + u32 trans_offset;
> > + u32 trans_shift;
> > + u32 val;
> > +
> > + address_translation(drvdata->data->fan[fan].inst,
> > + &trans_offset,
> > + &trans_shift);
> > +
> > + regmap_read(drvdata->plreg_map, trans_offset, &val);
> > + val = (val >> trans_shift) & drvdata->data->fan[fan].bit;
> > + if (val == drvdata->data->fan[fan].bit)
> > + return 1;
> > + else
> > + return 0;
> return val == drvdata->data->fan[fan].bit;
> Those calculations look quite complex. Is there a public datasheet that would enable me to understand how registers are actually assigned ?
There is no public datasheet as of yet but there is work ongoing to
create one. I will however document exactly how it is setup in hwmon.
There is so much I/O on our board that most of the inputs and outputs
go through an external CPLD we are interfaced with to save pins. A
memory area in our SoC reflects some of the I/O from CPLD in bytes
ranging from 0 to 0xff. Each byte represents information such as byte
0x27, which on this particular platform represents the fan installation
status of fans 0 to 7 respectively with bit 0 to 7. The byte 0x28 represents
something else. Regmap_read/write does a word instead of a single byte
which we are interested in so we use address_translation to keep offsets
easier to read.
> > + } else {
> > + /* Power Off */
> > + val = 0;
> > + }
> What determines power to a fan ? Should the power state be reported with fanX_enable ? Or possibly the installed state ?
This actually is the power state of the system, not the fan. When the
system is off we will see a PWM value of 0xFF on the fan. The idea
here was to report a value of 0 if the system was off.
Would you like me to use fanX_enable (read only) to show it as
disabled while the system is off ?
From a hardware standpoint that would be accurate.
> > +static const struct fan_ctrl_data g10_data = {
> > + .fan[0] = { .inst = 0x00, .fail = 0x02, .id = 0x04, .bit = 0x01 },
> > + .fan[1] = { .inst = 0x00, .fail = 0x02, .id = 0x04, .bit = 0x02 },
> > + .fan[2] = { .inst = 0x00, .fail = 0x02, .id = 0x04, .bit = 0x04 },
> > + .fan[3] = { .inst = 0x00, .fail = 0x02, .id = 0x04, .bit = 0x08 },
> > + .fan[4] = { .inst = 0x00, .fail = 0x02, .id = 0x04, .bit = 0x10 },
> > + .fan[5] = { .inst = 0x00, .fail = 0x02, .id = 0x04, .bit = 0x20 },
> > + .fan[6] = { .inst = 0x00, .fail = 0x02, .id = 0x04, .bit = 0x40 },
> > + .fan[7] = { .inst = 0x00, .fail = 0x02, .id = 0x04, .bit = 0x80 },
> > + .fan[8] = { .inst = 0x01, .fail = 0x03, .id = 0x05, .bit = 0x01 },
> > + .fan[9] = { .inst = 0x01, .fail = 0x03, .id = 0x05, .bit = 0x02 },
> > + .fan[10] = { .inst = 0x01, .fail = 0x03, .id = 0x05, .bit = 0x04 },
> > + .fan[11] = { .inst = 0x01, .fail = 0x03, .id = 0x05, .bit = 0x08 },
> > + .fan[12] = { .inst = 0x01, .fail = 0x03, .id = 0x05, .bit = 0x10 },
> > + .fan[13] = { .inst = 0x01, .fail = 0x03, .id = 0x05, .bit = 0x20 },
> > + .fan[14] = { .inst = 0x01, .fail = 0x03, .id = 0x05, .bit = 0x40 },
> > + .fan[15] = { .inst = 0x01, .fail = 0x03, .id = 0x05, .bit = 0x80 },
> > + .power_bit = 24,
> > +};
> > +
> > +static const struct of_device_id gxp_fan_ctrl_of_match[] = {
> > + { .compatible = "hpe,gxp-fan-ctrl", .data = &g10_data },
> I don't understand the point of attaching g10_data here.
> Why not just access it directly ? There is just one table.
The reason for having this data with the of_device_id binding is that
each platform has different byte offsets as mentioned above. We
would like to be able to reuse the driver if possible for this. We will
soon need g11_data that will be added here. Would a description in
Documentation, comments and commit message allow us to keep
this ?
Thank you for your assistance and feedback with this code,
-Nick Hawkins