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
From: Serge Schneider <[email protected]>
This patch adds the binding for the fan on the Raspberry Pi Power over
Ethernet HAT.
Signed-off-by: Serge Schneider <[email protected]>
Signed-off-by: Stefan Wahren <[email protected]>
---
.../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
From: Serge Schneider <[email protected]>
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 <[email protected]>
Signed-off-by: Stefan Wahren <[email protected]>
---
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 <[email protected]>
+
+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 <[email protected]>
+ *
+ * Author: Serge Schneider <[email protected]>
+ */
+
+#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 <[email protected]>");
+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
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
>
Hi Guenter,
> Guenter Roeck <[email protected]> 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
> >
>
On 09/02/2018 09:26 AM, Stefan Wahren wrote:
> Hi Guenter,
>
>> Guenter Roeck <[email protected]> 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
>>>
>>
>
Hi Guenter,
> Guenter Roeck <[email protected]> hat am 2. September 2018 um 18:49 geschrieben:
>
>
> On 09/02/2018 09:26 AM, Stefan Wahren wrote:
> > Hi Guenter,
> >
> >> Guenter Roeck <[email protected]> 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
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 9/2/2018 10:13 AM, Stefan Wahren wrote:
> Hi Guenter,
>
>> Guenter Roeck <[email protected]> hat am 2. September 2018 um 18:49 geschrieben:
>>
>>
>> On 09/02/2018 09:26 AM, Stefan Wahren wrote:
>>> Hi Guenter,
>>>
>>>> Guenter Roeck <[email protected]> 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
> Florian Fainelli <[email protected]> hat am 3. September 2018 um 20:31 geschrieben:
>
>
>
>
> On 9/2/2018 10:13 AM, Stefan Wahren wrote:
> > Hi Guenter,
> >
> >> Guenter Roeck <[email protected]> hat am 2. September 2018 um 18:49 geschrieben:
> >>
> >>
> >> On 09/02/2018 09:26 AM, Stefan Wahren wrote:
> >>> Hi Guenter,
> >>>
> >>>> Guenter Roeck <[email protected]> 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
On Mon, Sep 3, 2018 at 7:55 PM Stefan Wahren <[email protected]> wrote:
>
>
> > Florian Fainelli <[email protected]> hat am 3. September 2018 um 20:31 geschrieben:
> >
> >
> >
> >
> > On 9/2/2018 10:13 AM, Stefan Wahren wrote:
> > > Hi Guenter,
> > >
> > >> Guenter Roeck <[email protected]> hat am 2. September 2018 um 18:49 geschrieben:
> > >>
> > >>
> > >> On 09/02/2018 09:26 AM, Stefan Wahren wrote:
> > >>> Hi Guenter,
> > >>>
> > >>>> Guenter Roeck <[email protected]> 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