* [PATCH RFC 0/2] hwmon: Add RPi PoE HAT fan driver @ 2018-09-02 11:20 ` Stefan Wahren 0 siblings, 0 replies; 25+ messages in thread From: Stefan Wahren @ 2018-09-02 11:20 UTC (permalink / raw) To: Jean Delvare, Guenter Roeck, Rob Herring, Mark Rutland Cc: Eric Anholt, Florian Fainelli, Phil Elwell, bcm-kernel-feedback-list, devicetree, linux-arm-kernel, linux-rpi-kernel, linux-hwmon, linux-kernel, Stefan Wahren This series is an early stage of the hwmon driver for the fan on the Raspberry Pi Power over Ethernet HAT [1]. At the end this should use a Device Tree Overlay. Changes by Stefan based on [2]: - reformat the downstream patches for submission - drop reboot notification - fix remaining checkpatch issues - add COMPILE_TEST to Kconfig The driver is mostly copy & paste from pwm-fan, which isn't good. Personally i see two options: 1) integrate the driver function into the pwm-fan driver (new compatible) 2) implement the core function as a PWM driver and use the pwm-fan driver on top [1] - https://www.raspberrypi.org/products/poe-hat/ [2] - https://github.com/raspberrypi/linux/commit/0f937c8dc3201ebffa6c617c616fd7c65db65959 Serge Schneider (2): dt-bindings: hwmon: Add RPi PoE HAT documentation hwmon: Add RPi PoE HAT fan driver .../devicetree/bindings/hwmon/rpi-poe-fan.txt | 55 +++ Documentation/hwmon/rpi-poe-fan | 15 + drivers/hwmon/Kconfig | 11 + drivers/hwmon/Makefile | 1 + drivers/hwmon/rpi-poe-fan.c | 414 +++++++++++++++++++++ include/soc/bcm2835/raspberrypi-firmware.h | 2 + 6 files changed, 498 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/rpi-poe-fan.txt create mode 100644 Documentation/hwmon/rpi-poe-fan create mode 100644 drivers/hwmon/rpi-poe-fan.c -- 2.7.4 ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH RFC 0/2] hwmon: Add RPi PoE HAT fan driver @ 2018-09-02 11:20 ` Stefan Wahren 0 siblings, 0 replies; 25+ messages in thread From: Stefan Wahren @ 2018-09-02 11:20 UTC (permalink / raw) To: linux-arm-kernel This series is an early stage of the hwmon driver for the fan on the Raspberry Pi Power over Ethernet HAT [1]. At the end this should use a Device Tree Overlay. Changes by Stefan based on [2]: - reformat the downstream patches for submission - drop reboot notification - fix remaining checkpatch issues - add COMPILE_TEST to Kconfig The driver is mostly copy & paste from pwm-fan, which isn't good. Personally i see two options: 1) integrate the driver function into the pwm-fan driver (new compatible) 2) implement the core function as a PWM driver and use the pwm-fan driver on top [1] - https://www.raspberrypi.org/products/poe-hat/ [2] - https://github.com/raspberrypi/linux/commit/0f937c8dc3201ebffa6c617c616fd7c65db65959 Serge Schneider (2): dt-bindings: hwmon: Add RPi PoE HAT documentation hwmon: Add RPi PoE HAT fan driver .../devicetree/bindings/hwmon/rpi-poe-fan.txt | 55 +++ Documentation/hwmon/rpi-poe-fan | 15 + drivers/hwmon/Kconfig | 11 + drivers/hwmon/Makefile | 1 + drivers/hwmon/rpi-poe-fan.c | 414 +++++++++++++++++++++ include/soc/bcm2835/raspberrypi-firmware.h | 2 + 6 files changed, 498 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/rpi-poe-fan.txt create mode 100644 Documentation/hwmon/rpi-poe-fan create mode 100644 drivers/hwmon/rpi-poe-fan.c -- 2.7.4 ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH RFC 1/2] dt-bindings: hwmon: Add RPi PoE HAT documentation 2018-09-02 11:20 ` Stefan Wahren (?) @ 2018-09-02 11:20 ` Stefan Wahren -1 siblings, 0 replies; 25+ messages in thread From: Stefan Wahren @ 2018-09-02 11:20 UTC (permalink / raw) To: Jean Delvare, Guenter Roeck, Rob Herring, Mark Rutland Cc: Eric Anholt, Florian Fainelli, Phil Elwell, bcm-kernel-feedback-list, devicetree, linux-arm-kernel, linux-rpi-kernel, linux-hwmon, linux-kernel, Serge Schneider, Stefan Wahren From: Serge Schneider <serge@raspberrypi.org> This patch adds the binding for the fan on the Raspberry Pi Power over Ethernet HAT. Signed-off-by: Serge Schneider <serge@raspberrypi.org> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> --- .../devicetree/bindings/hwmon/rpi-poe-fan.txt | 55 ++++++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/rpi-poe-fan.txt diff --git a/Documentation/devicetree/bindings/hwmon/rpi-poe-fan.txt b/Documentation/devicetree/bindings/hwmon/rpi-poe-fan.txt new file mode 100644 index 0000000..c71f856 --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/rpi-poe-fan.txt @@ -0,0 +1,55 @@ +Bindings for the Raspberry Pi PoE HAT fan + +Required properties: +- compatible : "raspberrypi,rpi-poe-fan" +- firmware : Reference to the RPi firmware device node +- pwms : the PWM that is used to control the PWM fan +- cooling-levels : PWM duty cycle values in a range from 0 to 255 + which correspond to thermal cooling states + +Example: + fan0: rpi-poe-fan@0 { + compatible = "raspberrypi,rpi-poe-fan"; + firmware = <&firmware>; + cooling-min-state = <0>; + cooling-max-state = <3>; + #cooling-cells = <2>; + cooling-levels = <0 50 150 255>; + status = "okay"; + }; + + thermal-zones { + cpu_thermal: cpu-thermal { + trips { + threshold: trip-point@0 { + temperature = <45000>; + hysteresis = <5000>; + type = "active"; + }; + target: trip-point@1 { + temperature = <50000>; + hysteresis = <2000>; + type = "active"; + }; + cpu_hot: cpu_hot@0 { + temperature = <55000>; + hysteresis = <2000>; + type = "active"; + }; + }; + cooling-maps { + map0 { + trip = <&threshold>; + cooling-device = <&fan0 0 1>; + }; + map1 { + trip = <&target>; + cooling-device = <&fan0 1 2>; + }; + map2 { + trip = <&cpu_hot>; + cooling-device = <&fan0 2 3>; + }; + }; + }; + }; -- 2.7.4 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH RFC 1/2] dt-bindings: hwmon: Add RPi PoE HAT documentation @ 2018-09-02 11:20 ` Stefan Wahren 0 siblings, 0 replies; 25+ messages in thread From: Stefan Wahren @ 2018-09-02 11:20 UTC (permalink / raw) To: linux-arm-kernel From: Serge Schneider <serge@raspberrypi.org> This patch adds the binding for the fan on the Raspberry Pi Power over Ethernet HAT. Signed-off-by: Serge Schneider <serge@raspberrypi.org> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> --- .../devicetree/bindings/hwmon/rpi-poe-fan.txt | 55 ++++++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/rpi-poe-fan.txt diff --git a/Documentation/devicetree/bindings/hwmon/rpi-poe-fan.txt b/Documentation/devicetree/bindings/hwmon/rpi-poe-fan.txt new file mode 100644 index 0000000..c71f856 --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/rpi-poe-fan.txt @@ -0,0 +1,55 @@ +Bindings for the Raspberry Pi PoE HAT fan + +Required properties: +- compatible : "raspberrypi,rpi-poe-fan" +- firmware : Reference to the RPi firmware device node +- pwms : the PWM that is used to control the PWM fan +- cooling-levels : PWM duty cycle values in a range from 0 to 255 + which correspond to thermal cooling states + +Example: + fan0: rpi-poe-fan at 0 { + compatible = "raspberrypi,rpi-poe-fan"; + firmware = <&firmware>; + cooling-min-state = <0>; + cooling-max-state = <3>; + #cooling-cells = <2>; + cooling-levels = <0 50 150 255>; + status = "okay"; + }; + + thermal-zones { + cpu_thermal: cpu-thermal { + trips { + threshold: trip-point at 0 { + temperature = <45000>; + hysteresis = <5000>; + type = "active"; + }; + target: trip-point at 1 { + temperature = <50000>; + hysteresis = <2000>; + type = "active"; + }; + cpu_hot: cpu_hot at 0 { + temperature = <55000>; + hysteresis = <2000>; + type = "active"; + }; + }; + cooling-maps { + map0 { + trip = <&threshold>; + cooling-device = <&fan0 0 1>; + }; + map1 { + trip = <&target>; + cooling-device = <&fan0 1 2>; + }; + map2 { + trip = <&cpu_hot>; + cooling-device = <&fan0 2 3>; + }; + }; + }; + }; -- 2.7.4 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH RFC 1/2] dt-bindings: hwmon: Add RPi PoE HAT documentation @ 2018-09-02 11:20 ` Stefan Wahren 0 siblings, 0 replies; 25+ messages in thread From: Stefan Wahren @ 2018-09-02 11:20 UTC (permalink / raw) To: Jean Delvare, Guenter Roeck, Rob Herring, Mark Rutland Cc: linux-hwmon, devicetree, Florian Fainelli, Serge Schneider, Stefan Wahren, Phil Elwell, linux-kernel, Eric Anholt, bcm-kernel-feedback-list, linux-rpi-kernel, linux-arm-kernel From: Serge Schneider <serge@raspberrypi.org> This patch adds the binding for the fan on the Raspberry Pi Power over Ethernet HAT. Signed-off-by: Serge Schneider <serge@raspberrypi.org> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> --- .../devicetree/bindings/hwmon/rpi-poe-fan.txt | 55 ++++++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/rpi-poe-fan.txt diff --git a/Documentation/devicetree/bindings/hwmon/rpi-poe-fan.txt b/Documentation/devicetree/bindings/hwmon/rpi-poe-fan.txt new file mode 100644 index 0000000..c71f856 --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/rpi-poe-fan.txt @@ -0,0 +1,55 @@ +Bindings for the Raspberry Pi PoE HAT fan + +Required properties: +- compatible : "raspberrypi,rpi-poe-fan" +- firmware : Reference to the RPi firmware device node +- pwms : the PWM that is used to control the PWM fan +- cooling-levels : PWM duty cycle values in a range from 0 to 255 + which correspond to thermal cooling states + +Example: + fan0: rpi-poe-fan@0 { + compatible = "raspberrypi,rpi-poe-fan"; + firmware = <&firmware>; + cooling-min-state = <0>; + cooling-max-state = <3>; + #cooling-cells = <2>; + cooling-levels = <0 50 150 255>; + status = "okay"; + }; + + thermal-zones { + cpu_thermal: cpu-thermal { + trips { + threshold: trip-point@0 { + temperature = <45000>; + hysteresis = <5000>; + type = "active"; + }; + target: trip-point@1 { + temperature = <50000>; + hysteresis = <2000>; + type = "active"; + }; + cpu_hot: cpu_hot@0 { + temperature = <55000>; + hysteresis = <2000>; + type = "active"; + }; + }; + cooling-maps { + map0 { + trip = <&threshold>; + cooling-device = <&fan0 0 1>; + }; + map1 { + trip = <&target>; + cooling-device = <&fan0 1 2>; + }; + map2 { + trip = <&cpu_hot>; + cooling-device = <&fan0 2 3>; + }; + }; + }; + }; -- 2.7.4 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH RFC 2/2] hwmon: Add RPi PoE HAT fan driver 2018-09-02 11:20 ` Stefan Wahren @ 2018-09-02 11:20 ` Stefan Wahren -1 siblings, 0 replies; 25+ messages in thread From: Stefan Wahren @ 2018-09-02 11:20 UTC (permalink / raw) To: Jean Delvare, Guenter Roeck, Rob Herring, Mark Rutland Cc: Eric Anholt, Florian Fainelli, Phil Elwell, bcm-kernel-feedback-list, devicetree, linux-arm-kernel, linux-rpi-kernel, linux-hwmon, linux-kernel, Serge Schneider, Stefan Wahren From: Serge Schneider <serge@raspberrypi.org> This adds support for the fan located on the Raspberry Pi Power over Ethernet HAT. The driver passes commands the Raspberry Pi firmware through the mailbox property interface. The firmware then forwards the commands to the board over I2C on the ID_EEPROM pins. Signed-off-by: Serge Schneider <serge@raspberrypi.org> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> --- Documentation/hwmon/rpi-poe-fan | 15 ++ drivers/hwmon/Kconfig | 11 + drivers/hwmon/Makefile | 1 + drivers/hwmon/rpi-poe-fan.c | 414 +++++++++++++++++++++++++++++ include/soc/bcm2835/raspberrypi-firmware.h | 2 + 5 files changed, 443 insertions(+) create mode 100644 Documentation/hwmon/rpi-poe-fan create mode 100644 drivers/hwmon/rpi-poe-fan.c diff --git a/Documentation/hwmon/rpi-poe-fan b/Documentation/hwmon/rpi-poe-fan new file mode 100644 index 0000000..9182ab6 --- /dev/null +++ b/Documentation/hwmon/rpi-poe-fan @@ -0,0 +1,15 @@ +Kernel driver rpi-poe-fan +===================== + +This driver enables the use of the Raspberry Pi PoE HAT fan. + +Author: Serge Schneider <serge@raspberrypi.org> + +Description +----------- + +The driver implements a simple interface for driving the Raspberry Pi PoE +(Power over Ethernet) HAT fan. The driver passes commands to the Raspberry Pi +firmware through the mailbox property interface. The firmware then forwards +the commands to the board over I2C on the ID_EEPROM pins. The driver exposes +the fan to the user space through the hwmon sysfs interface. diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index 81da17a..f8b5572 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -1330,6 +1330,17 @@ config SENSORS_RASPBERRYPI_HWMON This driver can also be built as a module. If so, the module will be called raspberrypi-hwmon. +config SENSORS_RPI_POE_FAN + tristate "Raspberry Pi PoE HAT fan" + depends on RASPBERRYPI_FIRMWARE || (COMPILE_TEST && !RASPBERRYPI_FIRMWARE) + depends on THERMAL || THERMAL=n + help + If you say yes here you get support for Raspberry Pi PoE (Power over + Ethernet) HAT fan. + + This driver can also be built as a module. If so, the module + will be called rpi-poe-fan. + config SENSORS_SHT15 tristate "Sensiron humidity and temperature sensors. SHT15 and compat." depends on GPIOLIB || COMPILE_TEST diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index 93f7f41..3e2a504 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -144,6 +144,7 @@ obj-$(CONFIG_SENSORS_PCF8591) += pcf8591.o obj-$(CONFIG_SENSORS_POWR1220) += powr1220.o obj-$(CONFIG_SENSORS_PWM_FAN) += pwm-fan.o obj-$(CONFIG_SENSORS_RASPBERRYPI_HWMON) += raspberrypi-hwmon.o +obj-$(CONFIG_SENSORS_RPI_POE_FAN) += rpi-poe-fan.o obj-$(CONFIG_SENSORS_S3C) += s3c-hwmon.o obj-$(CONFIG_SENSORS_SCH56XX_COMMON)+= sch56xx-common.o obj-$(CONFIG_SENSORS_SCH5627) += sch5627.o diff --git a/drivers/hwmon/rpi-poe-fan.c b/drivers/hwmon/rpi-poe-fan.c new file mode 100644 index 0000000..192afa4 --- /dev/null +++ b/drivers/hwmon/rpi-poe-fan.c @@ -0,0 +1,414 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * rpi-poe-fan.c - Hwmon driver for Raspberry Pi PoE HAT fan. + * + * Copyright (C) 2018 Raspberry Pi (Trading) Ltd. + * Based on pwm-fan.c by Kamil Debski <k.debski@samsung.com> + * + * Author: Serge Schneider <serge@raspberrypi.org> + */ + +#include <linux/hwmon.h> +#include <linux/hwmon-sysfs.h> +#include <linux/module.h> +#include <linux/mutex.h> +#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/sysfs.h> +#include <linux/thermal.h> +#include <soc/bcm2835/raspberrypi-firmware.h> + +#define MAX_PWM 255 + +#define POE_CUR_PWM 0x0 +#define POE_DEF_PWM 0x1 + +struct rpi_poe_fan_ctx { + struct mutex lock; + struct rpi_firmware *fw; + unsigned int pwm_value; + unsigned int def_pwm_value; + unsigned int rpi_poe_fan_state; + unsigned int rpi_poe_fan_max_state; + unsigned int *rpi_poe_fan_cooling_levels; + struct thermal_cooling_device *cdev; +}; + +struct fw_tag_data_s { + u32 reg; + u32 val; + u32 ret; +}; + +static int write_reg(struct rpi_firmware *fw, u32 reg, u32 *val) +{ + struct fw_tag_data_s fw_tag_data = { + .reg = reg, + .val = *val + }; + int ret; + + ret = rpi_firmware_property(fw, RPI_FIRMWARE_SET_POE_HAT_VAL, + &fw_tag_data, sizeof(fw_tag_data)); + if (ret) + return ret; + else if (fw_tag_data.ret) + return -EIO; + + return 0; +} + +static int read_reg(struct rpi_firmware *fw, u32 reg, u32 *val) +{ + struct fw_tag_data_s fw_tag_data = { + .reg = reg, + }; + int ret; + + ret = rpi_firmware_property(fw, RPI_FIRMWARE_GET_POE_HAT_VAL, + &fw_tag_data, sizeof(fw_tag_data)); + if (ret) + return ret; + else if (fw_tag_data.ret) + return -EIO; + + *val = fw_tag_data.val; + return 0; +} + +static int __set_pwm(struct rpi_poe_fan_ctx *ctx, u32 pwm) +{ + int ret = 0; + + mutex_lock(&ctx->lock); + if (ctx->pwm_value == pwm) + goto exit_set_pwm_err; + + ret = write_reg(ctx->fw, POE_CUR_PWM, &pwm); + if (!ret) + ctx->pwm_value = pwm; +exit_set_pwm_err: + mutex_unlock(&ctx->lock); + return ret; +} + +static int __set_def_pwm(struct rpi_poe_fan_ctx *ctx, u32 def_pwm) +{ + int ret = 0; + + mutex_lock(&ctx->lock); + if (ctx->def_pwm_value == def_pwm) + goto exit_set_def_pwm_err; + + ret = write_reg(ctx->fw, POE_CUR_PWM, &def_pwm); + if (!ret) + ctx->def_pwm_value = def_pwm; +exit_set_def_pwm_err: + mutex_unlock(&ctx->lock); + return ret; +} + +static void rpi_poe_fan_update_state(struct rpi_poe_fan_ctx *ctx, + unsigned long pwm) +{ + int i; + + for (i = 0; i < ctx->rpi_poe_fan_max_state; ++i) + if (pwm < ctx->rpi_poe_fan_cooling_levels[i + 1]) + break; + + ctx->rpi_poe_fan_state = i; +} + +static ssize_t set_pwm(struct device *dev, struct device_attribute *attr, + const char *buf, size_t count) +{ + struct rpi_poe_fan_ctx *ctx = dev_get_drvdata(dev); + unsigned long pwm; + int ret; + + if (kstrtoul(buf, 10, &pwm) || pwm > MAX_PWM) + return -EINVAL; + + ret = __set_pwm(ctx, pwm); + if (ret) + return ret; + + rpi_poe_fan_update_state(ctx, pwm); + return count; +} + +static ssize_t set_def_pwm(struct device *dev, struct device_attribute *attr, + const char *buf, size_t count) +{ + struct rpi_poe_fan_ctx *ctx = dev_get_drvdata(dev); + unsigned long def_pwm; + int ret; + + if (kstrtoul(buf, 10, &def_pwm) || def_pwm > MAX_PWM) + return -EINVAL; + + ret = __set_def_pwm(ctx, def_pwm); + if (ret) + return ret; + return count; +} + +static ssize_t show_pwm(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct rpi_poe_fan_ctx *ctx = dev_get_drvdata(dev); + + return sprintf(buf, "%u\n", ctx->pwm_value); +} + +static ssize_t show_def_pwm(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct rpi_poe_fan_ctx *ctx = dev_get_drvdata(dev); + + return sprintf(buf, "%u\n", ctx->def_pwm_value); +} + + +static SENSOR_DEVICE_ATTR(pwm1, 0644, show_pwm, set_pwm, 0); +static SENSOR_DEVICE_ATTR(def_pwm1, 0644, show_def_pwm, set_def_pwm, 1); + +static struct attribute *rpi_poe_fan_attrs[] = { + &sensor_dev_attr_pwm1.dev_attr.attr, + &sensor_dev_attr_def_pwm1.dev_attr.attr, + NULL, +}; + +ATTRIBUTE_GROUPS(rpi_poe_fan); + +/* thermal cooling device callbacks */ +static int rpi_poe_fan_get_max_state(struct thermal_cooling_device *cdev, + unsigned long *state) +{ + struct rpi_poe_fan_ctx *ctx = cdev->devdata; + + if (!ctx) + return -EINVAL; + + *state = ctx->rpi_poe_fan_max_state; + + return 0; +} + +static int rpi_poe_fan_get_cur_state(struct thermal_cooling_device *cdev, + unsigned long *state) +{ + struct rpi_poe_fan_ctx *ctx = cdev->devdata; + + if (!ctx) + return -EINVAL; + + *state = ctx->rpi_poe_fan_state; + + return 0; +} + +static int rpi_poe_fan_set_cur_state(struct thermal_cooling_device *cdev, + unsigned long state) +{ + struct rpi_poe_fan_ctx *ctx = cdev->devdata; + int ret; + + if (!ctx || (state > ctx->rpi_poe_fan_max_state)) + return -EINVAL; + + if (state == ctx->rpi_poe_fan_state) + return 0; + + ret = __set_pwm(ctx, ctx->rpi_poe_fan_cooling_levels[state]); + if (ret) { + dev_err(&cdev->device, "Cannot set pwm!\n"); + return ret; + } + + ctx->rpi_poe_fan_state = state; + + return ret; +} + +static const struct thermal_cooling_device_ops rpi_poe_fan_cooling_ops = { + .get_max_state = rpi_poe_fan_get_max_state, + .get_cur_state = rpi_poe_fan_get_cur_state, + .set_cur_state = rpi_poe_fan_set_cur_state, +}; + +static int rpi_poe_fan_of_get_cooling_data(struct device *dev, + struct rpi_poe_fan_ctx *ctx) +{ + struct device_node *np = dev->of_node; + int num, i, ret; + + if (!of_find_property(np, "cooling-levels", NULL)) + return 0; + + ret = of_property_count_u32_elems(np, "cooling-levels"); + if (ret <= 0) { + dev_err(dev, "cooling-levels property missing or invalid: %d\n", + ret); + return ret ? : -EINVAL; + } + + num = ret; + ctx->rpi_poe_fan_cooling_levels = devm_kzalloc(dev, num * sizeof(u32), + GFP_KERNEL); + if (!ctx->rpi_poe_fan_cooling_levels) + return -ENOMEM; + + ret = of_property_read_u32_array(np, "cooling-levels", + ctx->rpi_poe_fan_cooling_levels, num); + if (ret) { + dev_err(dev, "Property 'cooling-levels' cannot be read!\n"); + return ret; + } + + for (i = 0; i < num; i++) { + if (ctx->rpi_poe_fan_cooling_levels[i] > MAX_PWM) { + dev_err(dev, "PWM fan state[%d]:%d > %d\n", i, + ctx->rpi_poe_fan_cooling_levels[i], MAX_PWM); + return -EINVAL; + } + } + + ctx->rpi_poe_fan_max_state = num - 1; + + return 0; +} + +static int rpi_poe_fan_probe(struct platform_device *pdev) +{ + struct thermal_cooling_device *cdev; + struct rpi_poe_fan_ctx *ctx; + struct device *hwmon; + struct device_node *np = pdev->dev.of_node; + struct device_node *fw_node; + int ret; + + fw_node = of_parse_phandle(np, "firmware", 0); + if (!fw_node) { + dev_err(&pdev->dev, "Missing firmware node\n"); + return -ENOENT; + } + + ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_KERNEL); + if (!ctx) + return -ENOMEM; + + mutex_init(&ctx->lock); + + ctx->fw = rpi_firmware_get(fw_node); + if (!ctx->fw) + return -EPROBE_DEFER; + + platform_set_drvdata(pdev, ctx); + + ret = read_reg(ctx->fw, POE_DEF_PWM, &ctx->def_pwm_value); + if (ret) { + dev_err(&pdev->dev, "Failed to get default PWM value: %i\n", + ret); + return ret; + } + ret = read_reg(ctx->fw, POE_CUR_PWM, &ctx->pwm_value); + if (ret) { + dev_err(&pdev->dev, "Failed to get current PWM value: %i\n", + ret); + return ret; + } + + hwmon = devm_hwmon_device_register_with_groups(&pdev->dev, "rpipoefan", + ctx, rpi_poe_fan_groups); + if (IS_ERR(hwmon)) { + dev_err(&pdev->dev, "Failed to register hwmon device\n"); + return PTR_ERR(hwmon); + } + + ret = rpi_poe_fan_of_get_cooling_data(&pdev->dev, ctx); + if (ret) + return ret; + + rpi_poe_fan_update_state(ctx, ctx->pwm_value); + if (!IS_ENABLED(CONFIG_THERMAL)) + return 0; + + cdev = thermal_of_cooling_device_register(np, + "rpi-poe-fan", ctx, + &rpi_poe_fan_cooling_ops); + if (IS_ERR(cdev)) { + dev_err(&pdev->dev, + "Failed to register rpi-poe-fan as cooling device"); + return PTR_ERR(cdev); + } + ctx->cdev = cdev; + thermal_cdev_update(cdev); + + return 0; +} + +static int rpi_poe_fan_remove(struct platform_device *pdev) +{ + struct rpi_poe_fan_ctx *ctx = platform_get_drvdata(pdev); + u32 value = ctx->def_pwm_value; + + thermal_cooling_device_unregister(ctx->cdev); + if (ctx->pwm_value != value) + write_reg(ctx->fw, POE_CUR_PWM, &value); + + return 0; +} + +#ifdef CONFIG_PM_SLEEP +static int rpi_poe_fan_suspend(struct device *dev) +{ + struct rpi_poe_fan_ctx *ctx = dev_get_drvdata(dev); + u32 value = 0; + int ret = 0; + + if (ctx->pwm_value != value) + ret = write_reg(ctx->fw, POE_CUR_PWM, &value); + + return ret; +} + +static int rpi_poe_fan_resume(struct device *dev) +{ + struct rpi_poe_fan_ctx *ctx = dev_get_drvdata(dev); + u32 value = ctx->pwm_value; + int ret = 0; + + if (value != 0) + ret = write_reg(ctx->fw, POE_CUR_PWM, &value); + + return ret; +} +#endif + +static SIMPLE_DEV_PM_OPS(rpi_poe_fan_pm, rpi_poe_fan_suspend, + rpi_poe_fan_resume); + +static const struct of_device_id of_rpi_poe_fan_match[] = { + { .compatible = "raspberrypi,rpi-poe-fan", }, + {}, +}; +MODULE_DEVICE_TABLE(of, of_rpi_poe_fan_match); + +static struct platform_driver rpi_poe_fan_driver = { + .probe = rpi_poe_fan_probe, + .remove = rpi_poe_fan_remove, + .driver = { + .name = "rpi-poe-fan", + .pm = &rpi_poe_fan_pm, + .of_match_table = of_rpi_poe_fan_match, + }, +}; + +module_platform_driver(rpi_poe_fan_driver); + +MODULE_AUTHOR("Serge Schneider <serge@raspberrypi.org>"); +MODULE_ALIAS("platform:rpi-poe-fan"); +MODULE_DESCRIPTION("Raspberry Pi PoE HAT fan driver"); +MODULE_LICENSE("GPL"); diff --git a/include/soc/bcm2835/raspberrypi-firmware.h b/include/soc/bcm2835/raspberrypi-firmware.h index c4a5c9e..f70d08a 100644 --- a/include/soc/bcm2835/raspberrypi-firmware.h +++ b/include/soc/bcm2835/raspberrypi-firmware.h @@ -89,6 +89,8 @@ enum rpi_firmware_property_tag { RPI_FIRMWARE_SET_GPIO_CONFIG = 0x00038043, RPI_FIRMWARE_GET_PERIPH_REG = 0x00030045, RPI_FIRMWARE_SET_PERIPH_REG = 0x00038045, + RPI_FIRMWARE_GET_POE_HAT_VAL = 0x00030049, + RPI_FIRMWARE_SET_POE_HAT_VAL = 0x00030050, /* Dispmanx TAGS */ -- 2.7.4 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH RFC 2/2] hwmon: Add RPi PoE HAT fan driver @ 2018-09-02 11:20 ` Stefan Wahren 0 siblings, 0 replies; 25+ messages in thread From: Stefan Wahren @ 2018-09-02 11:20 UTC (permalink / raw) To: linux-arm-kernel From: Serge Schneider <serge@raspberrypi.org> This adds support for the fan located on the Raspberry Pi Power over Ethernet HAT. The driver passes commands the Raspberry Pi firmware through the mailbox property interface. The firmware then forwards the commands to the board over I2C on the ID_EEPROM pins. Signed-off-by: Serge Schneider <serge@raspberrypi.org> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> --- Documentation/hwmon/rpi-poe-fan | 15 ++ drivers/hwmon/Kconfig | 11 + drivers/hwmon/Makefile | 1 + drivers/hwmon/rpi-poe-fan.c | 414 +++++++++++++++++++++++++++++ include/soc/bcm2835/raspberrypi-firmware.h | 2 + 5 files changed, 443 insertions(+) create mode 100644 Documentation/hwmon/rpi-poe-fan create mode 100644 drivers/hwmon/rpi-poe-fan.c diff --git a/Documentation/hwmon/rpi-poe-fan b/Documentation/hwmon/rpi-poe-fan new file mode 100644 index 0000000..9182ab6 --- /dev/null +++ b/Documentation/hwmon/rpi-poe-fan @@ -0,0 +1,15 @@ +Kernel driver rpi-poe-fan +===================== + +This driver enables the use of the Raspberry Pi PoE HAT fan. + +Author: Serge Schneider <serge@raspberrypi.org> + +Description +----------- + +The driver implements a simple interface for driving the Raspberry Pi PoE +(Power over Ethernet) HAT fan. The driver passes commands to the Raspberry Pi +firmware through the mailbox property interface. The firmware then forwards +the commands to the board over I2C on the ID_EEPROM pins. The driver exposes +the fan to the user space through the hwmon sysfs interface. diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index 81da17a..f8b5572 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -1330,6 +1330,17 @@ config SENSORS_RASPBERRYPI_HWMON This driver can also be built as a module. If so, the module will be called raspberrypi-hwmon. +config SENSORS_RPI_POE_FAN + tristate "Raspberry Pi PoE HAT fan" + depends on RASPBERRYPI_FIRMWARE || (COMPILE_TEST && !RASPBERRYPI_FIRMWARE) + depends on THERMAL || THERMAL=n + help + If you say yes here you get support for Raspberry Pi PoE (Power over + Ethernet) HAT fan. + + This driver can also be built as a module. If so, the module + will be called rpi-poe-fan. + config SENSORS_SHT15 tristate "Sensiron humidity and temperature sensors. SHT15 and compat." depends on GPIOLIB || COMPILE_TEST diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index 93f7f41..3e2a504 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -144,6 +144,7 @@ obj-$(CONFIG_SENSORS_PCF8591) += pcf8591.o obj-$(CONFIG_SENSORS_POWR1220) += powr1220.o obj-$(CONFIG_SENSORS_PWM_FAN) += pwm-fan.o obj-$(CONFIG_SENSORS_RASPBERRYPI_HWMON) += raspberrypi-hwmon.o +obj-$(CONFIG_SENSORS_RPI_POE_FAN) += rpi-poe-fan.o obj-$(CONFIG_SENSORS_S3C) += s3c-hwmon.o obj-$(CONFIG_SENSORS_SCH56XX_COMMON)+= sch56xx-common.o obj-$(CONFIG_SENSORS_SCH5627) += sch5627.o diff --git a/drivers/hwmon/rpi-poe-fan.c b/drivers/hwmon/rpi-poe-fan.c new file mode 100644 index 0000000..192afa4 --- /dev/null +++ b/drivers/hwmon/rpi-poe-fan.c @@ -0,0 +1,414 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * rpi-poe-fan.c - Hwmon driver for Raspberry Pi PoE HAT fan. + * + * Copyright (C) 2018 Raspberry Pi (Trading) Ltd. + * Based on pwm-fan.c by Kamil Debski <k.debski@samsung.com> + * + * Author: Serge Schneider <serge@raspberrypi.org> + */ + +#include <linux/hwmon.h> +#include <linux/hwmon-sysfs.h> +#include <linux/module.h> +#include <linux/mutex.h> +#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/sysfs.h> +#include <linux/thermal.h> +#include <soc/bcm2835/raspberrypi-firmware.h> + +#define MAX_PWM 255 + +#define POE_CUR_PWM 0x0 +#define POE_DEF_PWM 0x1 + +struct rpi_poe_fan_ctx { + struct mutex lock; + struct rpi_firmware *fw; + unsigned int pwm_value; + unsigned int def_pwm_value; + unsigned int rpi_poe_fan_state; + unsigned int rpi_poe_fan_max_state; + unsigned int *rpi_poe_fan_cooling_levels; + struct thermal_cooling_device *cdev; +}; + +struct fw_tag_data_s { + u32 reg; + u32 val; + u32 ret; +}; + +static int write_reg(struct rpi_firmware *fw, u32 reg, u32 *val) +{ + struct fw_tag_data_s fw_tag_data = { + .reg = reg, + .val = *val + }; + int ret; + + ret = rpi_firmware_property(fw, RPI_FIRMWARE_SET_POE_HAT_VAL, + &fw_tag_data, sizeof(fw_tag_data)); + if (ret) + return ret; + else if (fw_tag_data.ret) + return -EIO; + + return 0; +} + +static int read_reg(struct rpi_firmware *fw, u32 reg, u32 *val) +{ + struct fw_tag_data_s fw_tag_data = { + .reg = reg, + }; + int ret; + + ret = rpi_firmware_property(fw, RPI_FIRMWARE_GET_POE_HAT_VAL, + &fw_tag_data, sizeof(fw_tag_data)); + if (ret) + return ret; + else if (fw_tag_data.ret) + return -EIO; + + *val = fw_tag_data.val; + return 0; +} + +static int __set_pwm(struct rpi_poe_fan_ctx *ctx, u32 pwm) +{ + int ret = 0; + + mutex_lock(&ctx->lock); + if (ctx->pwm_value == pwm) + goto exit_set_pwm_err; + + ret = write_reg(ctx->fw, POE_CUR_PWM, &pwm); + if (!ret) + ctx->pwm_value = pwm; +exit_set_pwm_err: + mutex_unlock(&ctx->lock); + return ret; +} + +static int __set_def_pwm(struct rpi_poe_fan_ctx *ctx, u32 def_pwm) +{ + int ret = 0; + + mutex_lock(&ctx->lock); + if (ctx->def_pwm_value == def_pwm) + goto exit_set_def_pwm_err; + + ret = write_reg(ctx->fw, POE_CUR_PWM, &def_pwm); + if (!ret) + ctx->def_pwm_value = def_pwm; +exit_set_def_pwm_err: + mutex_unlock(&ctx->lock); + return ret; +} + +static void rpi_poe_fan_update_state(struct rpi_poe_fan_ctx *ctx, + unsigned long pwm) +{ + int i; + + for (i = 0; i < ctx->rpi_poe_fan_max_state; ++i) + if (pwm < ctx->rpi_poe_fan_cooling_levels[i + 1]) + break; + + ctx->rpi_poe_fan_state = i; +} + +static ssize_t set_pwm(struct device *dev, struct device_attribute *attr, + const char *buf, size_t count) +{ + struct rpi_poe_fan_ctx *ctx = dev_get_drvdata(dev); + unsigned long pwm; + int ret; + + if (kstrtoul(buf, 10, &pwm) || pwm > MAX_PWM) + return -EINVAL; + + ret = __set_pwm(ctx, pwm); + if (ret) + return ret; + + rpi_poe_fan_update_state(ctx, pwm); + return count; +} + +static ssize_t set_def_pwm(struct device *dev, struct device_attribute *attr, + const char *buf, size_t count) +{ + struct rpi_poe_fan_ctx *ctx = dev_get_drvdata(dev); + unsigned long def_pwm; + int ret; + + if (kstrtoul(buf, 10, &def_pwm) || def_pwm > MAX_PWM) + return -EINVAL; + + ret = __set_def_pwm(ctx, def_pwm); + if (ret) + return ret; + return count; +} + +static ssize_t show_pwm(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct rpi_poe_fan_ctx *ctx = dev_get_drvdata(dev); + + return sprintf(buf, "%u\n", ctx->pwm_value); +} + +static ssize_t show_def_pwm(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct rpi_poe_fan_ctx *ctx = dev_get_drvdata(dev); + + return sprintf(buf, "%u\n", ctx->def_pwm_value); +} + + +static SENSOR_DEVICE_ATTR(pwm1, 0644, show_pwm, set_pwm, 0); +static SENSOR_DEVICE_ATTR(def_pwm1, 0644, show_def_pwm, set_def_pwm, 1); + +static struct attribute *rpi_poe_fan_attrs[] = { + &sensor_dev_attr_pwm1.dev_attr.attr, + &sensor_dev_attr_def_pwm1.dev_attr.attr, + NULL, +}; + +ATTRIBUTE_GROUPS(rpi_poe_fan); + +/* thermal cooling device callbacks */ +static int rpi_poe_fan_get_max_state(struct thermal_cooling_device *cdev, + unsigned long *state) +{ + struct rpi_poe_fan_ctx *ctx = cdev->devdata; + + if (!ctx) + return -EINVAL; + + *state = ctx->rpi_poe_fan_max_state; + + return 0; +} + +static int rpi_poe_fan_get_cur_state(struct thermal_cooling_device *cdev, + unsigned long *state) +{ + struct rpi_poe_fan_ctx *ctx = cdev->devdata; + + if (!ctx) + return -EINVAL; + + *state = ctx->rpi_poe_fan_state; + + return 0; +} + +static int rpi_poe_fan_set_cur_state(struct thermal_cooling_device *cdev, + unsigned long state) +{ + struct rpi_poe_fan_ctx *ctx = cdev->devdata; + int ret; + + if (!ctx || (state > ctx->rpi_poe_fan_max_state)) + return -EINVAL; + + if (state == ctx->rpi_poe_fan_state) + return 0; + + ret = __set_pwm(ctx, ctx->rpi_poe_fan_cooling_levels[state]); + if (ret) { + dev_err(&cdev->device, "Cannot set pwm!\n"); + return ret; + } + + ctx->rpi_poe_fan_state = state; + + return ret; +} + +static const struct thermal_cooling_device_ops rpi_poe_fan_cooling_ops = { + .get_max_state = rpi_poe_fan_get_max_state, + .get_cur_state = rpi_poe_fan_get_cur_state, + .set_cur_state = rpi_poe_fan_set_cur_state, +}; + +static int rpi_poe_fan_of_get_cooling_data(struct device *dev, + struct rpi_poe_fan_ctx *ctx) +{ + struct device_node *np = dev->of_node; + int num, i, ret; + + if (!of_find_property(np, "cooling-levels", NULL)) + return 0; + + ret = of_property_count_u32_elems(np, "cooling-levels"); + if (ret <= 0) { + dev_err(dev, "cooling-levels property missing or invalid: %d\n", + ret); + return ret ? : -EINVAL; + } + + num = ret; + ctx->rpi_poe_fan_cooling_levels = devm_kzalloc(dev, num * sizeof(u32), + GFP_KERNEL); + if (!ctx->rpi_poe_fan_cooling_levels) + return -ENOMEM; + + ret = of_property_read_u32_array(np, "cooling-levels", + ctx->rpi_poe_fan_cooling_levels, num); + if (ret) { + dev_err(dev, "Property 'cooling-levels' cannot be read!\n"); + return ret; + } + + for (i = 0; i < num; i++) { + if (ctx->rpi_poe_fan_cooling_levels[i] > MAX_PWM) { + dev_err(dev, "PWM fan state[%d]:%d > %d\n", i, + ctx->rpi_poe_fan_cooling_levels[i], MAX_PWM); + return -EINVAL; + } + } + + ctx->rpi_poe_fan_max_state = num - 1; + + return 0; +} + +static int rpi_poe_fan_probe(struct platform_device *pdev) +{ + struct thermal_cooling_device *cdev; + struct rpi_poe_fan_ctx *ctx; + struct device *hwmon; + struct device_node *np = pdev->dev.of_node; + struct device_node *fw_node; + int ret; + + fw_node = of_parse_phandle(np, "firmware", 0); + if (!fw_node) { + dev_err(&pdev->dev, "Missing firmware node\n"); + return -ENOENT; + } + + ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_KERNEL); + if (!ctx) + return -ENOMEM; + + mutex_init(&ctx->lock); + + ctx->fw = rpi_firmware_get(fw_node); + if (!ctx->fw) + return -EPROBE_DEFER; + + platform_set_drvdata(pdev, ctx); + + ret = read_reg(ctx->fw, POE_DEF_PWM, &ctx->def_pwm_value); + if (ret) { + dev_err(&pdev->dev, "Failed to get default PWM value: %i\n", + ret); + return ret; + } + ret = read_reg(ctx->fw, POE_CUR_PWM, &ctx->pwm_value); + if (ret) { + dev_err(&pdev->dev, "Failed to get current PWM value: %i\n", + ret); + return ret; + } + + hwmon = devm_hwmon_device_register_with_groups(&pdev->dev, "rpipoefan", + ctx, rpi_poe_fan_groups); + if (IS_ERR(hwmon)) { + dev_err(&pdev->dev, "Failed to register hwmon device\n"); + return PTR_ERR(hwmon); + } + + ret = rpi_poe_fan_of_get_cooling_data(&pdev->dev, ctx); + if (ret) + return ret; + + rpi_poe_fan_update_state(ctx, ctx->pwm_value); + if (!IS_ENABLED(CONFIG_THERMAL)) + return 0; + + cdev = thermal_of_cooling_device_register(np, + "rpi-poe-fan", ctx, + &rpi_poe_fan_cooling_ops); + if (IS_ERR(cdev)) { + dev_err(&pdev->dev, + "Failed to register rpi-poe-fan as cooling device"); + return PTR_ERR(cdev); + } + ctx->cdev = cdev; + thermal_cdev_update(cdev); + + return 0; +} + +static int rpi_poe_fan_remove(struct platform_device *pdev) +{ + struct rpi_poe_fan_ctx *ctx = platform_get_drvdata(pdev); + u32 value = ctx->def_pwm_value; + + thermal_cooling_device_unregister(ctx->cdev); + if (ctx->pwm_value != value) + write_reg(ctx->fw, POE_CUR_PWM, &value); + + return 0; +} + +#ifdef CONFIG_PM_SLEEP +static int rpi_poe_fan_suspend(struct device *dev) +{ + struct rpi_poe_fan_ctx *ctx = dev_get_drvdata(dev); + u32 value = 0; + int ret = 0; + + if (ctx->pwm_value != value) + ret = write_reg(ctx->fw, POE_CUR_PWM, &value); + + return ret; +} + +static int rpi_poe_fan_resume(struct device *dev) +{ + struct rpi_poe_fan_ctx *ctx = dev_get_drvdata(dev); + u32 value = ctx->pwm_value; + int ret = 0; + + if (value != 0) + ret = write_reg(ctx->fw, POE_CUR_PWM, &value); + + return ret; +} +#endif + +static SIMPLE_DEV_PM_OPS(rpi_poe_fan_pm, rpi_poe_fan_suspend, + rpi_poe_fan_resume); + +static const struct of_device_id of_rpi_poe_fan_match[] = { + { .compatible = "raspberrypi,rpi-poe-fan", }, + {}, +}; +MODULE_DEVICE_TABLE(of, of_rpi_poe_fan_match); + +static struct platform_driver rpi_poe_fan_driver = { + .probe = rpi_poe_fan_probe, + .remove = rpi_poe_fan_remove, + .driver = { + .name = "rpi-poe-fan", + .pm = &rpi_poe_fan_pm, + .of_match_table = of_rpi_poe_fan_match, + }, +}; + +module_platform_driver(rpi_poe_fan_driver); + +MODULE_AUTHOR("Serge Schneider <serge@raspberrypi.org>"); +MODULE_ALIAS("platform:rpi-poe-fan"); +MODULE_DESCRIPTION("Raspberry Pi PoE HAT fan driver"); +MODULE_LICENSE("GPL"); diff --git a/include/soc/bcm2835/raspberrypi-firmware.h b/include/soc/bcm2835/raspberrypi-firmware.h index c4a5c9e..f70d08a 100644 --- a/include/soc/bcm2835/raspberrypi-firmware.h +++ b/include/soc/bcm2835/raspberrypi-firmware.h @@ -89,6 +89,8 @@ enum rpi_firmware_property_tag { RPI_FIRMWARE_SET_GPIO_CONFIG = 0x00038043, RPI_FIRMWARE_GET_PERIPH_REG = 0x00030045, RPI_FIRMWARE_SET_PERIPH_REG = 0x00038045, + RPI_FIRMWARE_GET_POE_HAT_VAL = 0x00030049, + RPI_FIRMWARE_SET_POE_HAT_VAL = 0x00030050, /* Dispmanx TAGS */ -- 2.7.4 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH RFC 0/2] hwmon: Add RPi PoE HAT fan driver 2018-09-02 11:20 ` Stefan Wahren @ 2018-09-02 14:23 ` Guenter Roeck -1 siblings, 0 replies; 25+ messages in thread From: Guenter Roeck @ 2018-09-02 14:23 UTC (permalink / raw) To: Stefan Wahren, Jean Delvare, Rob Herring, Mark Rutland Cc: Eric Anholt, Florian Fainelli, Phil Elwell, bcm-kernel-feedback-list, devicetree, linux-arm-kernel, linux-rpi-kernel, linux-hwmon, linux-kernel On 09/02/2018 04:20 AM, Stefan Wahren wrote: > This series is an early stage of the hwmon driver for the fan on the > Raspberry Pi Power over Ethernet HAT [1]. At the end this should use a > Device Tree Overlay. > > Changes by Stefan based on [2]: > - reformat the downstream patches for submission > - drop reboot notification > - fix remaining checkpatch issues > - add COMPILE_TEST to Kconfig > > The driver is mostly copy & paste from pwm-fan, which isn't good. Personally > i see two options: > > 1) integrate the driver function into the pwm-fan driver (new compatible) > 2) implement the core function as a PWM driver and use the pwm-fan driver on top > I don't really see the point of thise driver. Why not implement either of those ? 2) sounds like a perfect fit to me. Guenter > [1] - https://www.raspberrypi.org/products/poe-hat/ > [2] - https://github.com/raspberrypi/linux/commit/0f937c8dc3201ebffa6c617c616fd7c65db65959 > > Serge Schneider (2): > dt-bindings: hwmon: Add RPi PoE HAT documentation > hwmon: Add RPi PoE HAT fan driver > > .../devicetree/bindings/hwmon/rpi-poe-fan.txt | 55 +++ > Documentation/hwmon/rpi-poe-fan | 15 + > drivers/hwmon/Kconfig | 11 + > drivers/hwmon/Makefile | 1 + > drivers/hwmon/rpi-poe-fan.c | 414 +++++++++++++++++++++ > include/soc/bcm2835/raspberrypi-firmware.h | 2 + > 6 files changed, 498 insertions(+) > create mode 100644 Documentation/devicetree/bindings/hwmon/rpi-poe-fan.txt > create mode 100644 Documentation/hwmon/rpi-poe-fan > create mode 100644 drivers/hwmon/rpi-poe-fan.c > ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH RFC 0/2] hwmon: Add RPi PoE HAT fan driver @ 2018-09-02 14:23 ` Guenter Roeck 0 siblings, 0 replies; 25+ messages in thread From: Guenter Roeck @ 2018-09-02 14:23 UTC (permalink / raw) To: linux-arm-kernel On 09/02/2018 04:20 AM, Stefan Wahren wrote: > This series is an early stage of the hwmon driver for the fan on the > Raspberry Pi Power over Ethernet HAT [1]. At the end this should use a > Device Tree Overlay. > > Changes by Stefan based on [2]: > - reformat the downstream patches for submission > - drop reboot notification > - fix remaining checkpatch issues > - add COMPILE_TEST to Kconfig > > The driver is mostly copy & paste from pwm-fan, which isn't good. Personally > i see two options: > > 1) integrate the driver function into the pwm-fan driver (new compatible) > 2) implement the core function as a PWM driver and use the pwm-fan driver on top > I don't really see the point of thise driver. Why not implement either of those ? 2) sounds like a perfect fit to me. Guenter > [1] - https://www.raspberrypi.org/products/poe-hat/ > [2] - https://github.com/raspberrypi/linux/commit/0f937c8dc3201ebffa6c617c616fd7c65db65959 > > Serge Schneider (2): > dt-bindings: hwmon: Add RPi PoE HAT documentation > hwmon: Add RPi PoE HAT fan driver > > .../devicetree/bindings/hwmon/rpi-poe-fan.txt | 55 +++ > Documentation/hwmon/rpi-poe-fan | 15 + > drivers/hwmon/Kconfig | 11 + > drivers/hwmon/Makefile | 1 + > drivers/hwmon/rpi-poe-fan.c | 414 +++++++++++++++++++++ > include/soc/bcm2835/raspberrypi-firmware.h | 2 + > 6 files changed, 498 insertions(+) > create mode 100644 Documentation/devicetree/bindings/hwmon/rpi-poe-fan.txt > create mode 100644 Documentation/hwmon/rpi-poe-fan > create mode 100644 drivers/hwmon/rpi-poe-fan.c > ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH RFC 0/2] hwmon: Add RPi PoE HAT fan driver 2018-09-02 14:23 ` Guenter Roeck (?) @ 2018-09-02 16:26 ` Stefan Wahren -1 siblings, 0 replies; 25+ messages in thread From: Stefan Wahren @ 2018-09-02 16:26 UTC (permalink / raw) To: Guenter Roeck Cc: Florian Fainelli, linux-rpi-kernel, Eric Anholt, Phil Elwell, Jean Delvare, Mark Rutland, Rob Herring, bcm-kernel-feedback-list, devicetree, linux-hwmon, linux-kernel, linux-arm-kernel, serge Hi Guenter, > Guenter Roeck <linux@roeck-us.net> hat am 2. September 2018 um 16:23 geschrieben: > > > On 09/02/2018 04:20 AM, Stefan Wahren wrote: > > This series is an early stage of the hwmon driver for the fan on the > > Raspberry Pi Power over Ethernet HAT [1]. At the end this should use a > > Device Tree Overlay. > > > > Changes by Stefan based on [2]: > > - reformat the downstream patches for submission > > - drop reboot notification > > - fix remaining checkpatch issues > > - add COMPILE_TEST to Kconfig > > > > The driver is mostly copy & paste from pwm-fan, which isn't good. Personally > > i see two options: > > > > 1) integrate the driver function into the pwm-fan driver (new compatible) > > 2) implement the core function as a PWM driver and use the pwm-fan driver on top > > > > I don't really see the point of thise driver. Why not implement either of those ? i'm not sure about your question. Since the fan is placed over the SoC, the fan should takes care of the SoC temperature. AFAIK the firmware should have exclusive access to the I2C. So why we need this mailbox interface instead of a I2C driver. > 2) sounds like a perfect fit to me. > > Guenter > > > [1] - https://www.raspberrypi.org/products/poe-hat/ > > [2] - https://github.com/raspberrypi/linux/commit/0f937c8dc3201ebffa6c617c616fd7c65db65959 > > > > Serge Schneider (2): > > dt-bindings: hwmon: Add RPi PoE HAT documentation > > hwmon: Add RPi PoE HAT fan driver > > > > .../devicetree/bindings/hwmon/rpi-poe-fan.txt | 55 +++ > > Documentation/hwmon/rpi-poe-fan | 15 + > > drivers/hwmon/Kconfig | 11 + > > drivers/hwmon/Makefile | 1 + > > drivers/hwmon/rpi-poe-fan.c | 414 +++++++++++++++++++++ > > include/soc/bcm2835/raspberrypi-firmware.h | 2 + > > 6 files changed, 498 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/hwmon/rpi-poe-fan.txt > > create mode 100644 Documentation/hwmon/rpi-poe-fan > > create mode 100644 drivers/hwmon/rpi-poe-fan.c > > > ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH RFC 0/2] hwmon: Add RPi PoE HAT fan driver @ 2018-09-02 16:26 ` Stefan Wahren 0 siblings, 0 replies; 25+ messages in thread From: Stefan Wahren @ 2018-09-02 16:26 UTC (permalink / raw) To: linux-arm-kernel Hi Guenter, > Guenter Roeck <linux@roeck-us.net> hat am 2. September 2018 um 16:23 geschrieben: > > > On 09/02/2018 04:20 AM, Stefan Wahren wrote: > > This series is an early stage of the hwmon driver for the fan on the > > Raspberry Pi Power over Ethernet HAT [1]. At the end this should use a > > Device Tree Overlay. > > > > Changes by Stefan based on [2]: > > - reformat the downstream patches for submission > > - drop reboot notification > > - fix remaining checkpatch issues > > - add COMPILE_TEST to Kconfig > > > > The driver is mostly copy & paste from pwm-fan, which isn't good. Personally > > i see two options: > > > > 1) integrate the driver function into the pwm-fan driver (new compatible) > > 2) implement the core function as a PWM driver and use the pwm-fan driver on top > > > > I don't really see the point of thise driver. Why not implement either of those ? i'm not sure about your question. Since the fan is placed over the SoC, the fan should takes care of the SoC temperature. AFAIK the firmware should have exclusive access to the I2C. So why we need this mailbox interface instead of a I2C driver. > 2) sounds like a perfect fit to me. > > Guenter > > > [1] - https://www.raspberrypi.org/products/poe-hat/ > > [2] - https://github.com/raspberrypi/linux/commit/0f937c8dc3201ebffa6c617c616fd7c65db65959 > > > > Serge Schneider (2): > > dt-bindings: hwmon: Add RPi PoE HAT documentation > > hwmon: Add RPi PoE HAT fan driver > > > > .../devicetree/bindings/hwmon/rpi-poe-fan.txt | 55 +++ > > Documentation/hwmon/rpi-poe-fan | 15 + > > drivers/hwmon/Kconfig | 11 + > > drivers/hwmon/Makefile | 1 + > > drivers/hwmon/rpi-poe-fan.c | 414 +++++++++++++++++++++ > > include/soc/bcm2835/raspberrypi-firmware.h | 2 + > > 6 files changed, 498 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/hwmon/rpi-poe-fan.txt > > create mode 100644 Documentation/hwmon/rpi-poe-fan > > create mode 100644 drivers/hwmon/rpi-poe-fan.c > > > ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH RFC 0/2] hwmon: Add RPi PoE HAT fan driver @ 2018-09-02 16:26 ` Stefan Wahren 0 siblings, 0 replies; 25+ messages in thread From: Stefan Wahren @ 2018-09-02 16:26 UTC (permalink / raw) To: Guenter Roeck Cc: Mark Rutland, devicetree, Jean Delvare, serge, Phil Elwell, linux-kernel, Rob Herring, Eric Anholt, Florian Fainelli, bcm-kernel-feedback-list, linux-rpi-kernel, linux-hwmon, linux-arm-kernel Hi Guenter, > Guenter Roeck <linux@roeck-us.net> hat am 2. September 2018 um 16:23 geschrieben: > > > On 09/02/2018 04:20 AM, Stefan Wahren wrote: > > This series is an early stage of the hwmon driver for the fan on the > > Raspberry Pi Power over Ethernet HAT [1]. At the end this should use a > > Device Tree Overlay. > > > > Changes by Stefan based on [2]: > > - reformat the downstream patches for submission > > - drop reboot notification > > - fix remaining checkpatch issues > > - add COMPILE_TEST to Kconfig > > > > The driver is mostly copy & paste from pwm-fan, which isn't good. Personally > > i see two options: > > > > 1) integrate the driver function into the pwm-fan driver (new compatible) > > 2) implement the core function as a PWM driver and use the pwm-fan driver on top > > > > I don't really see the point of thise driver. Why not implement either of those ? i'm not sure about your question. Since the fan is placed over the SoC, the fan should takes care of the SoC temperature. AFAIK the firmware should have exclusive access to the I2C. So why we need this mailbox interface instead of a I2C driver. > 2) sounds like a perfect fit to me. > > Guenter > > > [1] - https://www.raspberrypi.org/products/poe-hat/ > > [2] - https://github.com/raspberrypi/linux/commit/0f937c8dc3201ebffa6c617c616fd7c65db65959 > > > > Serge Schneider (2): > > dt-bindings: hwmon: Add RPi PoE HAT documentation > > hwmon: Add RPi PoE HAT fan driver > > > > .../devicetree/bindings/hwmon/rpi-poe-fan.txt | 55 +++ > > Documentation/hwmon/rpi-poe-fan | 15 + > > drivers/hwmon/Kconfig | 11 + > > drivers/hwmon/Makefile | 1 + > > drivers/hwmon/rpi-poe-fan.c | 414 +++++++++++++++++++++ > > include/soc/bcm2835/raspberrypi-firmware.h | 2 + > > 6 files changed, 498 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/hwmon/rpi-poe-fan.txt > > create mode 100644 Documentation/hwmon/rpi-poe-fan > > create mode 100644 drivers/hwmon/rpi-poe-fan.c > > > ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH RFC 0/2] hwmon: Add RPi PoE HAT fan driver 2018-09-02 16:26 ` Stefan Wahren @ 2018-09-02 16:49 ` Guenter Roeck -1 siblings, 0 replies; 25+ messages in thread From: Guenter Roeck @ 2018-09-02 16:49 UTC (permalink / raw) To: Stefan Wahren Cc: Florian Fainelli, linux-rpi-kernel, Eric Anholt, Phil Elwell, Jean Delvare, Mark Rutland, Rob Herring, bcm-kernel-feedback-list, devicetree, linux-hwmon, linux-kernel, linux-arm-kernel, serge On 09/02/2018 09:26 AM, Stefan Wahren wrote: > Hi Guenter, > >> Guenter Roeck <linux@roeck-us.net> hat am 2. September 2018 um 16:23 geschrieben: >> >> >> On 09/02/2018 04:20 AM, Stefan Wahren wrote: >>> This series is an early stage of the hwmon driver for the fan on the >>> Raspberry Pi Power over Ethernet HAT [1]. At the end this should use a >>> Device Tree Overlay. >>> >>> Changes by Stefan based on [2]: >>> - reformat the downstream patches for submission >>> - drop reboot notification >>> - fix remaining checkpatch issues >>> - add COMPILE_TEST to Kconfig >>> >>> The driver is mostly copy & paste from pwm-fan, which isn't good. Personally >>> i see two options: >>> >>> 1) integrate the driver function into the pwm-fan driver (new compatible) >>> 2) implement the core function as a PWM driver and use the pwm-fan driver on top >>> >> >> I don't really see the point of thise driver. Why not implement either of those ? > > i'm not sure about your question. Since the fan is placed over the SoC, the fan should takes care of the SoC temperature. AFAIK the firmware should have exclusive access to the I2C. So why we need this mailbox interface instead of a I2C driver. > The driver sets pwm values. The pwm-fan driver sets pwm values. A pwm driver sets pwm values. The pwm-fan driver uses a pwm driver to set pwm values. You appear to be arguing that the pwm-fan driver for Rpi is different than a pwm-fan driver for all other hardware and should _not_ use a pwm driver to set pwm values. As you don't understand my question, I don't understand your rationale either. I will require detailed explanations why 2) is not feasible, and why 1) is not feasible either. Until then, NACK. Guenter >> 2) sounds like a perfect fit to me. >> >> Guenter >> >>> [1] - https://www.raspberrypi.org/products/poe-hat/ >>> [2] - https://github.com/raspberrypi/linux/commit/0f937c8dc3201ebffa6c617c616fd7c65db65959 >>> >>> Serge Schneider (2): >>> dt-bindings: hwmon: Add RPi PoE HAT documentation >>> hwmon: Add RPi PoE HAT fan driver >>> >>> .../devicetree/bindings/hwmon/rpi-poe-fan.txt | 55 +++ >>> Documentation/hwmon/rpi-poe-fan | 15 + >>> drivers/hwmon/Kconfig | 11 + >>> drivers/hwmon/Makefile | 1 + >>> drivers/hwmon/rpi-poe-fan.c | 414 +++++++++++++++++++++ >>> include/soc/bcm2835/raspberrypi-firmware.h | 2 + >>> 6 files changed, 498 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/hwmon/rpi-poe-fan.txt >>> create mode 100644 Documentation/hwmon/rpi-poe-fan >>> create mode 100644 drivers/hwmon/rpi-poe-fan.c >>> >> > ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH RFC 0/2] hwmon: Add RPi PoE HAT fan driver @ 2018-09-02 16:49 ` Guenter Roeck 0 siblings, 0 replies; 25+ messages in thread From: Guenter Roeck @ 2018-09-02 16:49 UTC (permalink / raw) To: linux-arm-kernel On 09/02/2018 09:26 AM, Stefan Wahren wrote: > Hi Guenter, > >> Guenter Roeck <linux@roeck-us.net> hat am 2. September 2018 um 16:23 geschrieben: >> >> >> On 09/02/2018 04:20 AM, Stefan Wahren wrote: >>> This series is an early stage of the hwmon driver for the fan on the >>> Raspberry Pi Power over Ethernet HAT [1]. At the end this should use a >>> Device Tree Overlay. >>> >>> Changes by Stefan based on [2]: >>> - reformat the downstream patches for submission >>> - drop reboot notification >>> - fix remaining checkpatch issues >>> - add COMPILE_TEST to Kconfig >>> >>> The driver is mostly copy & paste from pwm-fan, which isn't good. Personally >>> i see two options: >>> >>> 1) integrate the driver function into the pwm-fan driver (new compatible) >>> 2) implement the core function as a PWM driver and use the pwm-fan driver on top >>> >> >> I don't really see the point of thise driver. Why not implement either of those ? > > i'm not sure about your question. Since the fan is placed over the SoC, the fan should takes care of the SoC temperature. AFAIK the firmware should have exclusive access to the I2C. So why we need this mailbox interface instead of a I2C driver. > The driver sets pwm values. The pwm-fan driver sets pwm values. A pwm driver sets pwm values. The pwm-fan driver uses a pwm driver to set pwm values. You appear to be arguing that the pwm-fan driver for Rpi is different than a pwm-fan driver for all other hardware and should _not_ use a pwm driver to set pwm values. As you don't understand my question, I don't understand your rationale either. I will require detailed explanations why 2) is not feasible, and why 1) is not feasible either. Until then, NACK. Guenter >> 2) sounds like a perfect fit to me. >> >> Guenter >> >>> [1] - https://www.raspberrypi.org/products/poe-hat/ >>> [2] - https://github.com/raspberrypi/linux/commit/0f937c8dc3201ebffa6c617c616fd7c65db65959 >>> >>> Serge Schneider (2): >>> dt-bindings: hwmon: Add RPi PoE HAT documentation >>> hwmon: Add RPi PoE HAT fan driver >>> >>> .../devicetree/bindings/hwmon/rpi-poe-fan.txt | 55 +++ >>> Documentation/hwmon/rpi-poe-fan | 15 + >>> drivers/hwmon/Kconfig | 11 + >>> drivers/hwmon/Makefile | 1 + >>> drivers/hwmon/rpi-poe-fan.c | 414 +++++++++++++++++++++ >>> include/soc/bcm2835/raspberrypi-firmware.h | 2 + >>> 6 files changed, 498 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/hwmon/rpi-poe-fan.txt >>> create mode 100644 Documentation/hwmon/rpi-poe-fan >>> create mode 100644 drivers/hwmon/rpi-poe-fan.c >>> >> > ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH RFC 0/2] hwmon: Add RPi PoE HAT fan driver 2018-09-02 16:49 ` Guenter Roeck (?) @ 2018-09-02 17:13 ` Stefan Wahren -1 siblings, 0 replies; 25+ messages in thread From: Stefan Wahren @ 2018-09-02 17:13 UTC (permalink / raw) To: Guenter Roeck Cc: Rob Herring, Eric Anholt, bcm-kernel-feedback-list, devicetree, linux-kernel, serge, Florian Fainelli, linux-rpi-kernel, Mark Rutland, Phil Elwell, linux-hwmon, Jean Delvare, linux-arm-kernel Hi Guenter, > Guenter Roeck <linux@roeck-us.net> hat am 2. September 2018 um 18:49 geschrieben: > > > On 09/02/2018 09:26 AM, Stefan Wahren wrote: > > Hi Guenter, > > > >> Guenter Roeck <linux@roeck-us.net> hat am 2. September 2018 um 16:23 geschrieben: > >> > >> > >> On 09/02/2018 04:20 AM, Stefan Wahren wrote: > >>> This series is an early stage of the hwmon driver for the fan on the > >>> Raspberry Pi Power over Ethernet HAT [1]. At the end this should use a > >>> Device Tree Overlay. > >>> > >>> Changes by Stefan based on [2]: > >>> - reformat the downstream patches for submission > >>> - drop reboot notification > >>> - fix remaining checkpatch issues > >>> - add COMPILE_TEST to Kconfig > >>> > >>> The driver is mostly copy & paste from pwm-fan, which isn't good. Personally > >>> i see two options: > >>> > >>> 1) integrate the driver function into the pwm-fan driver (new compatible) > >>> 2) implement the core function as a PWM driver and use the pwm-fan driver on top > >>> > >> > >> I don't really see the point of thise driver. Why not implement either of those ? > > > > i'm not sure about your question. Since the fan is placed over the SoC, the fan should takes care of the SoC temperature. AFAIK the firmware should have exclusive access to the I2C. So why we need this mailbox interface instead of a I2C driver. > > > > The driver sets pwm values. The pwm-fan driver sets pwm values. A pwm driver > sets pwm values. The pwm-fan driver uses a pwm driver to set pwm values. > You appear to be arguing that the pwm-fan driver for Rpi is different than > a pwm-fan driver for all other hardware and should _not_ use a pwm driver > to set pwm values. thanks for your explanation. Now i think i understand and sorry for the confusion. We need a driver which translate the pwm values into the mailbox properties. "My" RFC series is only a starting point (not intended for merge and not an option) for a discussion and i'm perfectly fine with 2). Both options would be feasible in general. I only wanted to know your opinion before i start to implement one of them. Stefan > > As you don't understand my question, I don't understand your rationale either. > I will require detailed explanations why 2) is not feasible, and why 1) is not > feasible either. Until then, NACK. > > Guenter > > >> 2) sounds like a perfect fit to me. > >> > >> Guenter > >> > >>> [1] - https://www.raspberrypi.org/products/poe-hat/ > >>> [2] - https://github.com/raspberrypi/linux/commit/0f937c8dc3201ebffa6c617c616fd7c65db65959 > >>> > >>> Serge Schneider (2): > >>> dt-bindings: hwmon: Add RPi PoE HAT documentation > >>> hwmon: Add RPi PoE HAT fan driver > >>> > >>> .../devicetree/bindings/hwmon/rpi-poe-fan.txt | 55 +++ > >>> Documentation/hwmon/rpi-poe-fan | 15 + > >>> drivers/hwmon/Kconfig | 11 + > >>> drivers/hwmon/Makefile | 1 + > >>> drivers/hwmon/rpi-poe-fan.c | 414 +++++++++++++++++++++ > >>> include/soc/bcm2835/raspberrypi-firmware.h | 2 + > >>> 6 files changed, 498 insertions(+) > >>> create mode 100644 Documentation/devicetree/bindings/hwmon/rpi-poe-fan.txt > >>> create mode 100644 Documentation/hwmon/rpi-poe-fan > >>> create mode 100644 drivers/hwmon/rpi-poe-fan.c > >>> > >> > > > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH RFC 0/2] hwmon: Add RPi PoE HAT fan driver @ 2018-09-02 17:13 ` Stefan Wahren 0 siblings, 0 replies; 25+ messages in thread From: Stefan Wahren @ 2018-09-02 17:13 UTC (permalink / raw) To: linux-arm-kernel Hi Guenter, > Guenter Roeck <linux@roeck-us.net> hat am 2. September 2018 um 18:49 geschrieben: > > > On 09/02/2018 09:26 AM, Stefan Wahren wrote: > > Hi Guenter, > > > >> Guenter Roeck <linux@roeck-us.net> hat am 2. September 2018 um 16:23 geschrieben: > >> > >> > >> On 09/02/2018 04:20 AM, Stefan Wahren wrote: > >>> This series is an early stage of the hwmon driver for the fan on the > >>> Raspberry Pi Power over Ethernet HAT [1]. At the end this should use a > >>> Device Tree Overlay. > >>> > >>> Changes by Stefan based on [2]: > >>> - reformat the downstream patches for submission > >>> - drop reboot notification > >>> - fix remaining checkpatch issues > >>> - add COMPILE_TEST to Kconfig > >>> > >>> The driver is mostly copy & paste from pwm-fan, which isn't good. Personally > >>> i see two options: > >>> > >>> 1) integrate the driver function into the pwm-fan driver (new compatible) > >>> 2) implement the core function as a PWM driver and use the pwm-fan driver on top > >>> > >> > >> I don't really see the point of thise driver. Why not implement either of those ? > > > > i'm not sure about your question. Since the fan is placed over the SoC, the fan should takes care of the SoC temperature. AFAIK the firmware should have exclusive access to the I2C. So why we need this mailbox interface instead of a I2C driver. > > > > The driver sets pwm values. The pwm-fan driver sets pwm values. A pwm driver > sets pwm values. The pwm-fan driver uses a pwm driver to set pwm values. > You appear to be arguing that the pwm-fan driver for Rpi is different than > a pwm-fan driver for all other hardware and should _not_ use a pwm driver > to set pwm values. thanks for your explanation. Now i think i understand and sorry for the confusion. We need a driver which translate the pwm values into the mailbox properties. "My" RFC series is only a starting point (not intended for merge and not an option) for a discussion and i'm perfectly fine with 2). Both options would be feasible in general. I only wanted to know your opinion before i start to implement one of them. Stefan > > As you don't understand my question, I don't understand your rationale either. > I will require detailed explanations why 2) is not feasible, and why 1) is not > feasible either. Until then, NACK. > > Guenter > > >> 2) sounds like a perfect fit to me. > >> > >> Guenter > >> > >>> [1] - https://www.raspberrypi.org/products/poe-hat/ > >>> [2] - https://github.com/raspberrypi/linux/commit/0f937c8dc3201ebffa6c617c616fd7c65db65959 > >>> > >>> Serge Schneider (2): > >>> dt-bindings: hwmon: Add RPi PoE HAT documentation > >>> hwmon: Add RPi PoE HAT fan driver > >>> > >>> .../devicetree/bindings/hwmon/rpi-poe-fan.txt | 55 +++ > >>> Documentation/hwmon/rpi-poe-fan | 15 + > >>> drivers/hwmon/Kconfig | 11 + > >>> drivers/hwmon/Makefile | 1 + > >>> drivers/hwmon/rpi-poe-fan.c | 414 +++++++++++++++++++++ > >>> include/soc/bcm2835/raspberrypi-firmware.h | 2 + > >>> 6 files changed, 498 insertions(+) > >>> create mode 100644 Documentation/devicetree/bindings/hwmon/rpi-poe-fan.txt > >>> create mode 100644 Documentation/hwmon/rpi-poe-fan > >>> create mode 100644 drivers/hwmon/rpi-poe-fan.c > >>> > >> > > > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH RFC 0/2] hwmon: Add RPi PoE HAT fan driver @ 2018-09-02 17:13 ` Stefan Wahren 0 siblings, 0 replies; 25+ messages in thread From: Stefan Wahren @ 2018-09-02 17:13 UTC (permalink / raw) To: Guenter Roeck Cc: Mark Rutland, devicetree, Jean Delvare, Florian Fainelli, Phil Elwell, serge, linux-kernel, Eric Anholt, Rob Herring, bcm-kernel-feedback-list, linux-rpi-kernel, linux-hwmon, linux-arm-kernel Hi Guenter, > Guenter Roeck <linux@roeck-us.net> hat am 2. September 2018 um 18:49 geschrieben: > > > On 09/02/2018 09:26 AM, Stefan Wahren wrote: > > Hi Guenter, > > > >> Guenter Roeck <linux@roeck-us.net> hat am 2. September 2018 um 16:23 geschrieben: > >> > >> > >> On 09/02/2018 04:20 AM, Stefan Wahren wrote: > >>> This series is an early stage of the hwmon driver for the fan on the > >>> Raspberry Pi Power over Ethernet HAT [1]. At the end this should use a > >>> Device Tree Overlay. > >>> > >>> Changes by Stefan based on [2]: > >>> - reformat the downstream patches for submission > >>> - drop reboot notification > >>> - fix remaining checkpatch issues > >>> - add COMPILE_TEST to Kconfig > >>> > >>> The driver is mostly copy & paste from pwm-fan, which isn't good. Personally > >>> i see two options: > >>> > >>> 1) integrate the driver function into the pwm-fan driver (new compatible) > >>> 2) implement the core function as a PWM driver and use the pwm-fan driver on top > >>> > >> > >> I don't really see the point of thise driver. Why not implement either of those ? > > > > i'm not sure about your question. Since the fan is placed over the SoC, the fan should takes care of the SoC temperature. AFAIK the firmware should have exclusive access to the I2C. So why we need this mailbox interface instead of a I2C driver. > > > > The driver sets pwm values. The pwm-fan driver sets pwm values. A pwm driver > sets pwm values. The pwm-fan driver uses a pwm driver to set pwm values. > You appear to be arguing that the pwm-fan driver for Rpi is different than > a pwm-fan driver for all other hardware and should _not_ use a pwm driver > to set pwm values. thanks for your explanation. Now i think i understand and sorry for the confusion. We need a driver which translate the pwm values into the mailbox properties. "My" RFC series is only a starting point (not intended for merge and not an option) for a discussion and i'm perfectly fine with 2). Both options would be feasible in general. I only wanted to know your opinion before i start to implement one of them. Stefan > > As you don't understand my question, I don't understand your rationale either. > I will require detailed explanations why 2) is not feasible, and why 1) is not > feasible either. Until then, NACK. > > Guenter > > >> 2) sounds like a perfect fit to me. > >> > >> Guenter > >> > >>> [1] - https://www.raspberrypi.org/products/poe-hat/ > >>> [2] - https://github.com/raspberrypi/linux/commit/0f937c8dc3201ebffa6c617c616fd7c65db65959 > >>> > >>> Serge Schneider (2): > >>> dt-bindings: hwmon: Add RPi PoE HAT documentation > >>> hwmon: Add RPi PoE HAT fan driver > >>> > >>> .../devicetree/bindings/hwmon/rpi-poe-fan.txt | 55 +++ > >>> Documentation/hwmon/rpi-poe-fan | 15 + > >>> drivers/hwmon/Kconfig | 11 + > >>> drivers/hwmon/Makefile | 1 + > >>> drivers/hwmon/rpi-poe-fan.c | 414 +++++++++++++++++++++ > >>> include/soc/bcm2835/raspberrypi-firmware.h | 2 + > >>> 6 files changed, 498 insertions(+) > >>> create mode 100644 Documentation/devicetree/bindings/hwmon/rpi-poe-fan.txt > >>> create mode 100644 Documentation/hwmon/rpi-poe-fan > >>> create mode 100644 drivers/hwmon/rpi-poe-fan.c > >>> > >> > > > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH RFC 0/2] hwmon: Add RPi PoE HAT fan driver 2018-09-02 17:13 ` Stefan Wahren @ 2018-09-03 18:31 ` Florian Fainelli -1 siblings, 0 replies; 25+ messages in thread From: Florian Fainelli @ 2018-09-03 18:31 UTC (permalink / raw) To: Stefan Wahren, Guenter Roeck Cc: Rob Herring, Eric Anholt, bcm-kernel-feedback-list, devicetree, linux-kernel, serge, linux-rpi-kernel, Mark Rutland, Phil Elwell, linux-hwmon, Jean Delvare, linux-arm-kernel On 9/2/2018 10:13 AM, Stefan Wahren wrote: > Hi Guenter, > >> Guenter Roeck <linux@roeck-us.net> hat am 2. September 2018 um 18:49 geschrieben: >> >> >> On 09/02/2018 09:26 AM, Stefan Wahren wrote: >>> Hi Guenter, >>> >>>> Guenter Roeck <linux@roeck-us.net> hat am 2. September 2018 um 16:23 geschrieben: >>>> >>>> >>>> On 09/02/2018 04:20 AM, Stefan Wahren wrote: >>>>> This series is an early stage of the hwmon driver for the fan on the >>>>> Raspberry Pi Power over Ethernet HAT [1]. At the end this should use a >>>>> Device Tree Overlay. >>>>> >>>>> Changes by Stefan based on [2]: >>>>> - reformat the downstream patches for submission >>>>> - drop reboot notification >>>>> - fix remaining checkpatch issues >>>>> - add COMPILE_TEST to Kconfig >>>>> >>>>> The driver is mostly copy & paste from pwm-fan, which isn't good. Personally >>>>> i see two options: >>>>> >>>>> 1) integrate the driver function into the pwm-fan driver (new compatible) >>>>> 2) implement the core function as a PWM driver and use the pwm-fan driver on top >>>>> >>>> >>>> I don't really see the point of thise driver. Why not implement either of those ? >>> >>> i'm not sure about your question. Since the fan is placed over the SoC, the fan should takes care of the SoC temperature. AFAIK the firmware should have exclusive access to the I2C. So why we need this mailbox interface instead of a I2C driver. >>> >> >> The driver sets pwm values. The pwm-fan driver sets pwm values. A pwm driver >> sets pwm values. The pwm-fan driver uses a pwm driver to set pwm values. >> You appear to be arguing that the pwm-fan driver for Rpi is different than >> a pwm-fan driver for all other hardware and should _not_ use a pwm driver >> to set pwm values. > > thanks for your explanation. Now i think i understand and sorry for the confusion. We need a driver which translate the pwm values into the mailbox properties. "My" RFC series is only a starting point (not intended for merge and not an option) for a discussion and i'm perfectly fine with 2). > Both options would be feasible in general. I only wanted to know your opinion before i start to implement one of them. Is not there a way to expose the PWM pins directly to the kernel instead of going through the firmware to do that for us? I am just asking because sometimes this appears to be possible, guess not in this case? -- Florian ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH RFC 0/2] hwmon: Add RPi PoE HAT fan driver @ 2018-09-03 18:31 ` Florian Fainelli 0 siblings, 0 replies; 25+ messages in thread From: Florian Fainelli @ 2018-09-03 18:31 UTC (permalink / raw) To: linux-arm-kernel On 9/2/2018 10:13 AM, Stefan Wahren wrote: > Hi Guenter, > >> Guenter Roeck <linux@roeck-us.net> hat am 2. September 2018 um 18:49 geschrieben: >> >> >> On 09/02/2018 09:26 AM, Stefan Wahren wrote: >>> Hi Guenter, >>> >>>> Guenter Roeck <linux@roeck-us.net> hat am 2. September 2018 um 16:23 geschrieben: >>>> >>>> >>>> On 09/02/2018 04:20 AM, Stefan Wahren wrote: >>>>> This series is an early stage of the hwmon driver for the fan on the >>>>> Raspberry Pi Power over Ethernet HAT [1]. At the end this should use a >>>>> Device Tree Overlay. >>>>> >>>>> Changes by Stefan based on [2]: >>>>> - reformat the downstream patches for submission >>>>> - drop reboot notification >>>>> - fix remaining checkpatch issues >>>>> - add COMPILE_TEST to Kconfig >>>>> >>>>> The driver is mostly copy & paste from pwm-fan, which isn't good. Personally >>>>> i see two options: >>>>> >>>>> 1) integrate the driver function into the pwm-fan driver (new compatible) >>>>> 2) implement the core function as a PWM driver and use the pwm-fan driver on top >>>>> >>>> >>>> I don't really see the point of thise driver. Why not implement either of those ? >>> >>> i'm not sure about your question. Since the fan is placed over the SoC, the fan should takes care of the SoC temperature. AFAIK the firmware should have exclusive access to the I2C. So why we need this mailbox interface instead of a I2C driver. >>> >> >> The driver sets pwm values. The pwm-fan driver sets pwm values. A pwm driver >> sets pwm values. The pwm-fan driver uses a pwm driver to set pwm values. >> You appear to be arguing that the pwm-fan driver for Rpi is different than >> a pwm-fan driver for all other hardware and should _not_ use a pwm driver >> to set pwm values. > > thanks for your explanation. Now i think i understand and sorry for the confusion. We need a driver which translate the pwm values into the mailbox properties. "My" RFC series is only a starting point (not intended for merge and not an option) for a discussion and i'm perfectly fine with 2). > Both options would be feasible in general. I only wanted to know your opinion before i start to implement one of them. Is not there a way to expose the PWM pins directly to the kernel instead of going through the firmware to do that for us? I am just asking because sometimes this appears to be possible, guess not in this case? -- Florian ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH RFC 0/2] hwmon: Add RPi PoE HAT fan driver 2018-09-03 18:31 ` Florian Fainelli (?) @ 2018-09-03 18:55 ` Stefan Wahren -1 siblings, 0 replies; 25+ messages in thread From: Stefan Wahren @ 2018-09-03 18:55 UTC (permalink / raw) To: Florian Fainelli, Guenter Roeck Cc: Rob Herring, serge, linux-rpi-kernel, Eric Anholt, Mark Rutland, bcm-kernel-feedback-list, Phil Elwell, devicetree, linux-hwmon, linux-kernel, Jean Delvare, linux-arm-kernel > Florian Fainelli <f.fainelli@gmail.com> hat am 3. September 2018 um 20:31 geschrieben: > > > > > On 9/2/2018 10:13 AM, Stefan Wahren wrote: > > Hi Guenter, > > > >> Guenter Roeck <linux@roeck-us.net> hat am 2. September 2018 um 18:49 geschrieben: > >> > >> > >> On 09/02/2018 09:26 AM, Stefan Wahren wrote: > >>> Hi Guenter, > >>> > >>>> Guenter Roeck <linux@roeck-us.net> hat am 2. September 2018 um 16:23 geschrieben: > >>>> > >>>> > >>>> On 09/02/2018 04:20 AM, Stefan Wahren wrote: > >>>>> This series is an early stage of the hwmon driver for the fan on the > >>>>> Raspberry Pi Power over Ethernet HAT [1]. At the end this should use a > >>>>> Device Tree Overlay. > >>>>> > >>>>> Changes by Stefan based on [2]: > >>>>> - reformat the downstream patches for submission > >>>>> - drop reboot notification > >>>>> - fix remaining checkpatch issues > >>>>> - add COMPILE_TEST to Kconfig > >>>>> > >>>>> The driver is mostly copy & paste from pwm-fan, which isn't good. Personally > >>>>> i see two options: > >>>>> > >>>>> 1) integrate the driver function into the pwm-fan driver (new compatible) > >>>>> 2) implement the core function as a PWM driver and use the pwm-fan driver on top > >>>>> > >>>> > >>>> I don't really see the point of thise driver. Why not implement either of those ? > >>> > >>> i'm not sure about your question. Since the fan is placed over the SoC, the fan should takes care of the SoC temperature. AFAIK the firmware should have exclusive access to the I2C. So why we need this mailbox interface instead of a I2C driver. > >>> > >> > >> The driver sets pwm values. The pwm-fan driver sets pwm values. A pwm driver > >> sets pwm values. The pwm-fan driver uses a pwm driver to set pwm values. > >> You appear to be arguing that the pwm-fan driver for Rpi is different than > >> a pwm-fan driver for all other hardware and should _not_ use a pwm driver > >> to set pwm values. > > > > thanks for your explanation. Now i think i understand and sorry for the confusion. We need a driver which translate the pwm values into the mailbox properties. "My" RFC series is only a starting point (not intended for merge and not an option) for a discussion and i'm perfectly fine with 2). > > Both options would be feasible in general. I only wanted to know your opinion before i start to implement one of them. > > Is not there a way to expose the PWM pins directly to the kernel instead > of going through the firmware to do that for us? I am just asking > because sometimes this appears to be possible, guess not in this case? According to this blog entry (no schematics available so far) [1], this is my understanding of the fan control: | Raspberry Pi 3B+ | PoE HAT | ARM core -Mailbox-> VideoCore4 -I2C-> Atmel MCU -PWM-> FAN The only chance would be to bypass VC4 and talk to the Atmel microcontroller directly. But this would require a specification of the I2C protocol or some time of reverse engineering. Maybe the foundation guys can say something about that? [1] - https://www.raspberrypi.org/blog/introducing-power-over-ethernet-poe-hat/ > -- > Florian ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH RFC 0/2] hwmon: Add RPi PoE HAT fan driver @ 2018-09-03 18:55 ` Stefan Wahren 0 siblings, 0 replies; 25+ messages in thread From: Stefan Wahren @ 2018-09-03 18:55 UTC (permalink / raw) To: linux-arm-kernel > Florian Fainelli <f.fainelli@gmail.com> hat am 3. September 2018 um 20:31 geschrieben: > > > > > On 9/2/2018 10:13 AM, Stefan Wahren wrote: > > Hi Guenter, > > > >> Guenter Roeck <linux@roeck-us.net> hat am 2. September 2018 um 18:49 geschrieben: > >> > >> > >> On 09/02/2018 09:26 AM, Stefan Wahren wrote: > >>> Hi Guenter, > >>> > >>>> Guenter Roeck <linux@roeck-us.net> hat am 2. September 2018 um 16:23 geschrieben: > >>>> > >>>> > >>>> On 09/02/2018 04:20 AM, Stefan Wahren wrote: > >>>>> This series is an early stage of the hwmon driver for the fan on the > >>>>> Raspberry Pi Power over Ethernet HAT [1]. At the end this should use a > >>>>> Device Tree Overlay. > >>>>> > >>>>> Changes by Stefan based on [2]: > >>>>> - reformat the downstream patches for submission > >>>>> - drop reboot notification > >>>>> - fix remaining checkpatch issues > >>>>> - add COMPILE_TEST to Kconfig > >>>>> > >>>>> The driver is mostly copy & paste from pwm-fan, which isn't good. Personally > >>>>> i see two options: > >>>>> > >>>>> 1) integrate the driver function into the pwm-fan driver (new compatible) > >>>>> 2) implement the core function as a PWM driver and use the pwm-fan driver on top > >>>>> > >>>> > >>>> I don't really see the point of thise driver. Why not implement either of those ? > >>> > >>> i'm not sure about your question. Since the fan is placed over the SoC, the fan should takes care of the SoC temperature. AFAIK the firmware should have exclusive access to the I2C. So why we need this mailbox interface instead of a I2C driver. > >>> > >> > >> The driver sets pwm values. The pwm-fan driver sets pwm values. A pwm driver > >> sets pwm values. The pwm-fan driver uses a pwm driver to set pwm values. > >> You appear to be arguing that the pwm-fan driver for Rpi is different than > >> a pwm-fan driver for all other hardware and should _not_ use a pwm driver > >> to set pwm values. > > > > thanks for your explanation. Now i think i understand and sorry for the confusion. We need a driver which translate the pwm values into the mailbox properties. "My" RFC series is only a starting point (not intended for merge and not an option) for a discussion and i'm perfectly fine with 2). > > Both options would be feasible in general. I only wanted to know your opinion before i start to implement one of them. > > Is not there a way to expose the PWM pins directly to the kernel instead > of going through the firmware to do that for us? I am just asking > because sometimes this appears to be possible, guess not in this case? According to this blog entry (no schematics available so far) [1], this is my understanding of the fan control: | Raspberry Pi 3B+ | PoE HAT | ARM core -Mailbox-> VideoCore4 -I2C-> Atmel MCU -PWM-> FAN The only chance would be to bypass VC4 and talk to the Atmel microcontroller directly. But this would require a specification of the I2C protocol or some time of reverse engineering. Maybe the foundation guys can say something about that? [1] - https://www.raspberrypi.org/blog/introducing-power-over-ethernet-poe-hat/ > -- > Florian ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH RFC 0/2] hwmon: Add RPi PoE HAT fan driver @ 2018-09-03 18:55 ` Stefan Wahren 0 siblings, 0 replies; 25+ messages in thread From: Stefan Wahren @ 2018-09-03 18:55 UTC (permalink / raw) To: Florian Fainelli, Guenter Roeck Cc: Mark Rutland, devicetree, Jean Delvare, Phil Elwell, serge, linux-kernel, Eric Anholt, Rob Herring, bcm-kernel-feedback-list, linux-rpi-kernel, linux-hwmon, linux-arm-kernel > Florian Fainelli <f.fainelli@gmail.com> hat am 3. September 2018 um 20:31 geschrieben: > > > > > On 9/2/2018 10:13 AM, Stefan Wahren wrote: > > Hi Guenter, > > > >> Guenter Roeck <linux@roeck-us.net> hat am 2. September 2018 um 18:49 geschrieben: > >> > >> > >> On 09/02/2018 09:26 AM, Stefan Wahren wrote: > >>> Hi Guenter, > >>> > >>>> Guenter Roeck <linux@roeck-us.net> hat am 2. September 2018 um 16:23 geschrieben: > >>>> > >>>> > >>>> On 09/02/2018 04:20 AM, Stefan Wahren wrote: > >>>>> This series is an early stage of the hwmon driver for the fan on the > >>>>> Raspberry Pi Power over Ethernet HAT [1]. At the end this should use a > >>>>> Device Tree Overlay. > >>>>> > >>>>> Changes by Stefan based on [2]: > >>>>> - reformat the downstream patches for submission > >>>>> - drop reboot notification > >>>>> - fix remaining checkpatch issues > >>>>> - add COMPILE_TEST to Kconfig > >>>>> > >>>>> The driver is mostly copy & paste from pwm-fan, which isn't good. Personally > >>>>> i see two options: > >>>>> > >>>>> 1) integrate the driver function into the pwm-fan driver (new compatible) > >>>>> 2) implement the core function as a PWM driver and use the pwm-fan driver on top > >>>>> > >>>> > >>>> I don't really see the point of thise driver. Why not implement either of those ? > >>> > >>> i'm not sure about your question. Since the fan is placed over the SoC, the fan should takes care of the SoC temperature. AFAIK the firmware should have exclusive access to the I2C. So why we need this mailbox interface instead of a I2C driver. > >>> > >> > >> The driver sets pwm values. The pwm-fan driver sets pwm values. A pwm driver > >> sets pwm values. The pwm-fan driver uses a pwm driver to set pwm values. > >> You appear to be arguing that the pwm-fan driver for Rpi is different than > >> a pwm-fan driver for all other hardware and should _not_ use a pwm driver > >> to set pwm values. > > > > thanks for your explanation. Now i think i understand and sorry for the confusion. We need a driver which translate the pwm values into the mailbox properties. "My" RFC series is only a starting point (not intended for merge and not an option) for a discussion and i'm perfectly fine with 2). > > Both options would be feasible in general. I only wanted to know your opinion before i start to implement one of them. > > Is not there a way to expose the PWM pins directly to the kernel instead > of going through the firmware to do that for us? I am just asking > because sometimes this appears to be possible, guess not in this case? According to this blog entry (no schematics available so far) [1], this is my understanding of the fan control: | Raspberry Pi 3B+ | PoE HAT | ARM core -Mailbox-> VideoCore4 -I2C-> Atmel MCU -PWM-> FAN The only chance would be to bypass VC4 and talk to the Atmel microcontroller directly. But this would require a specification of the I2C protocol or some time of reverse engineering. Maybe the foundation guys can say something about that? [1] - https://www.raspberrypi.org/blog/introducing-power-over-ethernet-poe-hat/ > -- > Florian ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH RFC 0/2] hwmon: Add RPi PoE HAT fan driver 2018-09-03 18:55 ` Stefan Wahren (?) @ 2018-09-03 19:05 ` Serge Schneider -1 siblings, 0 replies; 25+ messages in thread From: Serge Schneider @ 2018-09-03 19:05 UTC (permalink / raw) To: stefan.wahren Cc: f.fainelli, linux, robh+dt, linux-rpi-kernel, eric, mark.rutland, bcm-kernel-feedback-list, Phil Elwell, devicetree, linux-hwmon, linux-kernel, jdelvare, linux-arm-kernel On Mon, Sep 3, 2018 at 7:55 PM Stefan Wahren <stefan.wahren@i2se.com> wrote: > > > > Florian Fainelli <f.fainelli@gmail.com> hat am 3. September 2018 um 20:31 geschrieben: > > > > > > > > > > On 9/2/2018 10:13 AM, Stefan Wahren wrote: > > > Hi Guenter, > > > > > >> Guenter Roeck <linux@roeck-us.net> hat am 2. September 2018 um 18:49 geschrieben: > > >> > > >> > > >> On 09/02/2018 09:26 AM, Stefan Wahren wrote: > > >>> Hi Guenter, > > >>> > > >>>> Guenter Roeck <linux@roeck-us.net> hat am 2. September 2018 um 16:23 geschrieben: > > >>>> > > >>>> > > >>>> On 09/02/2018 04:20 AM, Stefan Wahren wrote: > > >>>>> This series is an early stage of the hwmon driver for the fan on the > > >>>>> Raspberry Pi Power over Ethernet HAT [1]. At the end this should use a > > >>>>> Device Tree Overlay. > > >>>>> > > >>>>> Changes by Stefan based on [2]: > > >>>>> - reformat the downstream patches for submission > > >>>>> - drop reboot notification > > >>>>> - fix remaining checkpatch issues > > >>>>> - add COMPILE_TEST to Kconfig > > >>>>> > > >>>>> The driver is mostly copy & paste from pwm-fan, which isn't good. Personally > > >>>>> i see two options: > > >>>>> > > >>>>> 1) integrate the driver function into the pwm-fan driver (new compatible) > > >>>>> 2) implement the core function as a PWM driver and use the pwm-fan driver on top > > >>>>> > > >>>> > > >>>> I don't really see the point of thise driver. Why not implement either of those ? > > >>> > > >>> i'm not sure about your question. Since the fan is placed over the SoC, the fan should takes care of the SoC temperature. AFAIK the firmware should have exclusive access to the I2C. So why we need this mailbox interface instead of a I2C driver. > > >>> > > >> > > >> The driver sets pwm values. The pwm-fan driver sets pwm values. A pwm driver > > >> sets pwm values. The pwm-fan driver uses a pwm driver to set pwm values. > > >> You appear to be arguing that the pwm-fan driver for Rpi is different than > > >> a pwm-fan driver for all other hardware and should _not_ use a pwm driver > > >> to set pwm values. > > > > > > thanks for your explanation. Now i think i understand and sorry for the confusion. We need a driver which translate the pwm values into the mailbox properties. "My" RFC series is only a starting point (not intended for merge and not an option) for a discussion and i'm perfectly fine with 2). > > > Both options would be feasible in general. I only wanted to know your opinion before i start to implement one of them. > > > > Is not there a way to expose the PWM pins directly to the kernel instead > > of going through the firmware to do that for us? I am just asking > > because sometimes this appears to be possible, guess not in this case? > > According to this blog entry (no schematics available so far) [1], this is my understanding of the fan control: > > | Raspberry Pi 3B+ | PoE HAT | > ARM core -Mailbox-> VideoCore4 -I2C-> Atmel MCU -PWM-> FAN > > The only chance would be to bypass VC4 and talk to the Atmel microcontroller directly. But this would require a specification of the I2C protocol or some time of reverse engineering. > > Maybe the foundation guys can say something about that? That's spot on. The communication is done through the mailbox because it's connected to the I2C periperal that's normally reserved for the firmware. The interface itself is very simple. Device address is 0x51. Register 0xF0 contains the current PWM value. 0xF1 contains the default PWM value that's loaded on power-on. > > > [1] - https://www.raspberrypi.org/blog/introducing-power-over-ethernet-poe-hat/ > > > -- > > Florian ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH RFC 0/2] hwmon: Add RPi PoE HAT fan driver @ 2018-09-03 19:05 ` Serge Schneider 0 siblings, 0 replies; 25+ messages in thread From: Serge Schneider @ 2018-09-03 19:05 UTC (permalink / raw) To: linux-arm-kernel On Mon, Sep 3, 2018 at 7:55 PM Stefan Wahren <stefan.wahren@i2se.com> wrote: > > > > Florian Fainelli <f.fainelli@gmail.com> hat am 3. September 2018 um 20:31 geschrieben: > > > > > > > > > > On 9/2/2018 10:13 AM, Stefan Wahren wrote: > > > Hi Guenter, > > > > > >> Guenter Roeck <linux@roeck-us.net> hat am 2. September 2018 um 18:49 geschrieben: > > >> > > >> > > >> On 09/02/2018 09:26 AM, Stefan Wahren wrote: > > >>> Hi Guenter, > > >>> > > >>>> Guenter Roeck <linux@roeck-us.net> hat am 2. September 2018 um 16:23 geschrieben: > > >>>> > > >>>> > > >>>> On 09/02/2018 04:20 AM, Stefan Wahren wrote: > > >>>>> This series is an early stage of the hwmon driver for the fan on the > > >>>>> Raspberry Pi Power over Ethernet HAT [1]. At the end this should use a > > >>>>> Device Tree Overlay. > > >>>>> > > >>>>> Changes by Stefan based on [2]: > > >>>>> - reformat the downstream patches for submission > > >>>>> - drop reboot notification > > >>>>> - fix remaining checkpatch issues > > >>>>> - add COMPILE_TEST to Kconfig > > >>>>> > > >>>>> The driver is mostly copy & paste from pwm-fan, which isn't good. Personally > > >>>>> i see two options: > > >>>>> > > >>>>> 1) integrate the driver function into the pwm-fan driver (new compatible) > > >>>>> 2) implement the core function as a PWM driver and use the pwm-fan driver on top > > >>>>> > > >>>> > > >>>> I don't really see the point of thise driver. Why not implement either of those ? > > >>> > > >>> i'm not sure about your question. Since the fan is placed over the SoC, the fan should takes care of the SoC temperature. AFAIK the firmware should have exclusive access to the I2C. So why we need this mailbox interface instead of a I2C driver. > > >>> > > >> > > >> The driver sets pwm values. The pwm-fan driver sets pwm values. A pwm driver > > >> sets pwm values. The pwm-fan driver uses a pwm driver to set pwm values. > > >> You appear to be arguing that the pwm-fan driver for Rpi is different than > > >> a pwm-fan driver for all other hardware and should _not_ use a pwm driver > > >> to set pwm values. > > > > > > thanks for your explanation. Now i think i understand and sorry for the confusion. We need a driver which translate the pwm values into the mailbox properties. "My" RFC series is only a starting point (not intended for merge and not an option) for a discussion and i'm perfectly fine with 2). > > > Both options would be feasible in general. I only wanted to know your opinion before i start to implement one of them. > > > > Is not there a way to expose the PWM pins directly to the kernel instead > > of going through the firmware to do that for us? I am just asking > > because sometimes this appears to be possible, guess not in this case? > > According to this blog entry (no schematics available so far) [1], this is my understanding of the fan control: > > | Raspberry Pi 3B+ | PoE HAT | > ARM core -Mailbox-> VideoCore4 -I2C-> Atmel MCU -PWM-> FAN > > The only chance would be to bypass VC4 and talk to the Atmel microcontroller directly. But this would require a specification of the I2C protocol or some time of reverse engineering. > > Maybe the foundation guys can say something about that? That's spot on. The communication is done through the mailbox because it's connected to the I2C periperal that's normally reserved for the firmware. The interface itself is very simple. Device address is 0x51. Register 0xF0 contains the current PWM value. 0xF1 contains the default PWM value that's loaded on power-on. > > > [1] - https://www.raspberrypi.org/blog/introducing-power-over-ethernet-poe-hat/ > > > -- > > Florian ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH RFC 0/2] hwmon: Add RPi PoE HAT fan driver @ 2018-09-03 19:05 ` Serge Schneider 0 siblings, 0 replies; 25+ messages in thread From: Serge Schneider @ 2018-09-03 19:05 UTC (permalink / raw) To: stefan.wahren Cc: mark.rutland, devicetree, jdelvare, f.fainelli, Phil Elwell, linux-kernel, eric, robh+dt, bcm-kernel-feedback-list, linux-rpi-kernel, linux-hwmon, linux, linux-arm-kernel On Mon, Sep 3, 2018 at 7:55 PM Stefan Wahren <stefan.wahren@i2se.com> wrote: > > > > Florian Fainelli <f.fainelli@gmail.com> hat am 3. September 2018 um 20:31 geschrieben: > > > > > > > > > > On 9/2/2018 10:13 AM, Stefan Wahren wrote: > > > Hi Guenter, > > > > > >> Guenter Roeck <linux@roeck-us.net> hat am 2. September 2018 um 18:49 geschrieben: > > >> > > >> > > >> On 09/02/2018 09:26 AM, Stefan Wahren wrote: > > >>> Hi Guenter, > > >>> > > >>>> Guenter Roeck <linux@roeck-us.net> hat am 2. September 2018 um 16:23 geschrieben: > > >>>> > > >>>> > > >>>> On 09/02/2018 04:20 AM, Stefan Wahren wrote: > > >>>>> This series is an early stage of the hwmon driver for the fan on the > > >>>>> Raspberry Pi Power over Ethernet HAT [1]. At the end this should use a > > >>>>> Device Tree Overlay. > > >>>>> > > >>>>> Changes by Stefan based on [2]: > > >>>>> - reformat the downstream patches for submission > > >>>>> - drop reboot notification > > >>>>> - fix remaining checkpatch issues > > >>>>> - add COMPILE_TEST to Kconfig > > >>>>> > > >>>>> The driver is mostly copy & paste from pwm-fan, which isn't good. Personally > > >>>>> i see two options: > > >>>>> > > >>>>> 1) integrate the driver function into the pwm-fan driver (new compatible) > > >>>>> 2) implement the core function as a PWM driver and use the pwm-fan driver on top > > >>>>> > > >>>> > > >>>> I don't really see the point of thise driver. Why not implement either of those ? > > >>> > > >>> i'm not sure about your question. Since the fan is placed over the SoC, the fan should takes care of the SoC temperature. AFAIK the firmware should have exclusive access to the I2C. So why we need this mailbox interface instead of a I2C driver. > > >>> > > >> > > >> The driver sets pwm values. The pwm-fan driver sets pwm values. A pwm driver > > >> sets pwm values. The pwm-fan driver uses a pwm driver to set pwm values. > > >> You appear to be arguing that the pwm-fan driver for Rpi is different than > > >> a pwm-fan driver for all other hardware and should _not_ use a pwm driver > > >> to set pwm values. > > > > > > thanks for your explanation. Now i think i understand and sorry for the confusion. We need a driver which translate the pwm values into the mailbox properties. "My" RFC series is only a starting point (not intended for merge and not an option) for a discussion and i'm perfectly fine with 2). > > > Both options would be feasible in general. I only wanted to know your opinion before i start to implement one of them. > > > > Is not there a way to expose the PWM pins directly to the kernel instead > > of going through the firmware to do that for us? I am just asking > > because sometimes this appears to be possible, guess not in this case? > > According to this blog entry (no schematics available so far) [1], this is my understanding of the fan control: > > | Raspberry Pi 3B+ | PoE HAT | > ARM core -Mailbox-> VideoCore4 -I2C-> Atmel MCU -PWM-> FAN > > The only chance would be to bypass VC4 and talk to the Atmel microcontroller directly. But this would require a specification of the I2C protocol or some time of reverse engineering. > > Maybe the foundation guys can say something about that? That's spot on. The communication is done through the mailbox because it's connected to the I2C periperal that's normally reserved for the firmware. The interface itself is very simple. Device address is 0x51. Register 0xF0 contains the current PWM value. 0xF1 contains the default PWM value that's loaded on power-on. > > > [1] - https://www.raspberrypi.org/blog/introducing-power-over-ethernet-poe-hat/ > > > -- > > Florian ^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2018-09-03 23:37 UTC | newest] Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-09-02 11:20 [PATCH RFC 0/2] hwmon: Add RPi PoE HAT fan driver Stefan Wahren 2018-09-02 11:20 ` Stefan Wahren 2018-09-02 11:20 ` [PATCH RFC 1/2] dt-bindings: hwmon: Add RPi PoE HAT documentation Stefan Wahren 2018-09-02 11:20 ` Stefan Wahren 2018-09-02 11:20 ` Stefan Wahren 2018-09-02 11:20 ` [PATCH RFC 2/2] hwmon: Add RPi PoE HAT fan driver Stefan Wahren 2018-09-02 11:20 ` Stefan Wahren 2018-09-02 14:23 ` [PATCH RFC 0/2] " Guenter Roeck 2018-09-02 14:23 ` Guenter Roeck 2018-09-02 16:26 ` Stefan Wahren 2018-09-02 16:26 ` Stefan Wahren 2018-09-02 16:26 ` Stefan Wahren 2018-09-02 16:49 ` Guenter Roeck 2018-09-02 16:49 ` Guenter Roeck 2018-09-02 17:13 ` Stefan Wahren 2018-09-02 17:13 ` Stefan Wahren 2018-09-02 17:13 ` Stefan Wahren 2018-09-03 18:31 ` Florian Fainelli 2018-09-03 18:31 ` Florian Fainelli 2018-09-03 18:55 ` Stefan Wahren 2018-09-03 18:55 ` Stefan Wahren 2018-09-03 18:55 ` Stefan Wahren 2018-09-03 19:05 ` Serge Schneider 2018-09-03 19:05 ` Serge Schneider 2018-09-03 19:05 ` Serge Schneider
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.