* [PATCH linux dev-4.10] hwmon: (aspeed-pwm-tacho) cooling device support
@ 2017-07-04 13:56 Mykola Kostenok
2017-07-05 19:42 ` Patrick Venture
2017-07-06 15:57 ` Patrick Williams
0 siblings, 2 replies; 7+ messages in thread
From: Mykola Kostenok @ 2017-07-04 13:56 UTC (permalink / raw)
To: Joel Stanley, openbmc, Patrick Venture; +Cc: Mykola Kostenok
Add support in aspeed-pwm-tacho driver for cooling device creation.
This cooling device could be bound to a thermal zone
for the thermal control. Device will appear in /sys/class/thermal
folder as cooling_deviceX. Then it could be bound to particular
thermal zones. Allow specification of the cooling levels
vector - PWM duty cycle values in a range from 0 to 255
which correspond to thermal cooling states.
Add basic pwm-tacho-controller node to ast-g5 dtsi.
Add basic pwm-tacho-controller node to ast-g4 dtsi.
Change pwm-tacho in aspeed-bmc-quanta-q71l.dts according new
pwm-tacho structure.
Add pwm-tacho to aspeed-bmc-mellanox-msn.dts.
Signed-off-by: Mykola Kostenok <c_mykolak@mellanox.com>
---
arch/arm/boot/dts/aspeed-bmc-mellanox-msn.dts | 61 +++++++++++
arch/arm/boot/dts/aspeed-bmc-quanta-q71l.dts | 103 +++++++++----------
arch/arm/boot/dts/aspeed-g4.dtsi | 9 +-
arch/arm/boot/dts/aspeed-g5.dtsi | 17 ++++
drivers/hwmon/aspeed-pwm-tacho.c | 139 +++++++++++++++++++++++++-
5 files changed, 273 insertions(+), 56 deletions(-)
diff --git a/arch/arm/boot/dts/aspeed-bmc-mellanox-msn.dts b/arch/arm/boot/dts/aspeed-bmc-mellanox-msn.dts
index c71a6db..57d5951 100644
--- a/arch/arm/boot/dts/aspeed-bmc-mellanox-msn.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-mellanox-msn.dts
@@ -144,3 +144,64 @@
status = "okay";
};
+&pwm_tacho {
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_pwm0_default>;
+
+ tach-channels {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ fan@0 {
+ reg = <0x00>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x00>;
+ };
+
+ fan@1 {
+ reg = <0x00>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x01>;
+ };
+
+ fan@2 {
+ reg = <0x00>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x02>;
+ };
+
+ fan@3 {
+ reg = <0x00>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x03>;
+ };
+
+ fan@4 {
+ reg = <0x00>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x04>;
+ };
+
+ fan@5 {
+ reg = <0x00>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x05>;
+ };
+
+ fan@6 {
+ reg = <0x00>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x06>;
+ };
+
+ fan@7 {
+ reg = <0x00>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x07>;
+ };
+ };
+
+ pwm-channels {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ #cooling-cells = <2>;
+
+ pwm@0 {
+ reg = <0x00>;
+ cooling-levels = /bits/ 8 <125 151 177 203 229 255>;
+ };
+ };
+};
diff --git a/arch/arm/boot/dts/aspeed-bmc-quanta-q71l.dts b/arch/arm/boot/dts/aspeed-bmc-quanta-q71l.dts
index bb287fb..43ada5b 100644
--- a/arch/arm/boot/dts/aspeed-bmc-quanta-q71l.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-quanta-q71l.dts
@@ -46,57 +46,6 @@
gpios = <&gpio ASPEED_GPIO(B, 3) GPIO_ACTIVE_LOW>;
};
};
-
- pwm_tacho: pwm-tacho-controller@1e786000 {
- compatible = "aspeed,ast2500-pwm-tacho";
- #address-cells = <1>;
- #size-cells = <0>;
- reg = <0x1e786000 0x1000>;
- clocks = <&pwm_tacho_fixed_clk>;
- pinctrl-names = "default";
- pinctrl-0 = <&pinctrl_pwm0_default &pinctrl_pwm1_default
- &pinctrl_pwm2_default &pinctrl_pwm3_default>;
-
- fan@0 {
- reg = <0x00>;
- aspeed,fan-tach-ch = /bits/ 8 <0x00>;
- };
-
- fan@1 {
- reg = <0x01>;
- aspeed,fan-tach-ch = /bits/ 8 <0x01>;
- };
-
- fan@2 {
- reg = <0x02>;
- aspeed,fan-tach-ch = /bits/ 8 <0x02>;
- };
-
- fan@3 {
- reg = <0x03>;
- aspeed,fan-tach-ch = /bits/ 8 <0x03>;
- };
-
- fan@4 {
- reg = <0x00>;
- aspeed,fan-tach-ch = /bits/ 8 <0x04>;
- };
-
- fan@5 {
- reg = <0x01>;
- aspeed,fan-tach-ch = /bits/ 8 <0x05>;
- };
-
- fan@6 {
- reg = <0x02>;
- aspeed,fan-tach-ch = /bits/ 8 <0x06>;
- };
-
- fan@7 {
- reg = <0x03>;
- aspeed,fan-tach-ch = /bits/ 8 <0x07>;
- };
- };
};
&fmc {
@@ -299,3 +248,55 @@
&wdt2 {
status = "okay";
};
+
+&pwm_tacho {
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_pwm0_default &pinctrl_pwm1_default
+ &pinctrl_pwm2_default &pinctrl_pwm3_default>;
+
+ tach-channels {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ fan@0 {
+ reg = <0x00>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x00>;
+ };
+
+ fan@1 {
+ reg = <0x01>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x01>;
+ };
+
+ fan@2 {
+ reg = <0x02>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x02>;
+ };
+
+ fan@3 {
+ reg = <0x03>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x03>;
+ };
+
+ fan@4 {
+ reg = <0x00>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x04>;
+ };
+
+ fan@5 {
+ reg = <0x01>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x05>;
+ };
+
+ fan@6 {
+ reg = <0x02>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x06>;
+ };
+
+ fan@7 {
+ reg = <0x03>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x07>;
+ };
+ };
+};
diff --git a/arch/arm/boot/dts/aspeed-g4.dtsi b/arch/arm/boot/dts/aspeed-g4.dtsi
index 9cc959f..c69ad77 100644
--- a/arch/arm/boot/dts/aspeed-g4.dtsi
+++ b/arch/arm/boot/dts/aspeed-g4.dtsi
@@ -318,7 +318,14 @@
ranges = <0 0x1e78a000 0x1000>;
};
-
+ pwm_tacho: pwm-tacho-controller@1e786000 {
+ compatible = "aspeed,ast2500-pwm-tacho";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x1e786000 0x1000>;
+ clocks = <&pwm_tacho_fixed_clk>;
+ status = "disabled";
+ };
};
};
};
diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi
index 992242d..1ed29d9 100644
--- a/arch/arm/boot/dts/aspeed-g5.dtsi
+++ b/arch/arm/boot/dts/aspeed-g5.dtsi
@@ -35,6 +35,14 @@
i2c13 = &i2c13;
};
+ clocks {
+ pwm_tacho_fixed_clk: fixedclk {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <24000000>;
+ };
+ };
+
ahb {
compatible = "simple-bus";
#address-cells = <1>;
@@ -366,6 +374,15 @@
#size-cells = <1>;
ranges = <0 0x1e78a000 0x1000>;
};
+
+ pwm_tacho: pwm-tacho-controller@1e786000 {
+ compatible = "aspeed,ast2500-pwm-tacho";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x1e786000 0x1000>;
+ clocks = <&pwm_tacho_fixed_clk>;
+ status = "disabled";
+ };
};
};
};
diff --git a/drivers/hwmon/aspeed-pwm-tacho.c b/drivers/hwmon/aspeed-pwm-tacho.c
index ddfe66b..eab18af 100644
--- a/drivers/hwmon/aspeed-pwm-tacho.c
+++ b/drivers/hwmon/aspeed-pwm-tacho.c
@@ -20,6 +20,7 @@
#include <linux/platform_device.h>
#include <linux/sysfs.h>
#include <linux/regmap.h>
+#include <linux/thermal.h>
/* ASPEED PWM & FAN Tach Register Definition */
#define ASPEED_PTCR_CTRL 0x00
@@ -166,6 +167,16 @@
/* How long we sleep in us while waiting for an RPM result. */
#define ASPEED_RPM_STATUS_SLEEP_USEC 500
+struct aspeed_cooling_device {
+ char name[16];
+ struct aspeed_pwm_tacho_data *priv;
+ struct thermal_cooling_device *tcdev;
+ int pwm_port;
+ u8 *cooling_levels;
+ u8 max_state;
+ u8 cur_state;
+};
+
struct aspeed_pwm_tacho_data {
struct regmap *regmap;
unsigned long clk_freq;
@@ -180,6 +191,7 @@ struct aspeed_pwm_tacho_data {
u8 pwm_port_type[8];
u8 pwm_port_fan_ctrl[8];
u8 fan_tach_ch_source[16];
+ struct aspeed_cooling_device *cdev[8];
const struct attribute_group *groups[3];
};
@@ -794,10 +806,111 @@ static int aspeed_create_fan(struct device *dev,
return 0;
}
+static int
+aspeed_pwm_cz_get_max_state(struct thermal_cooling_device *tcdev,
+ unsigned long *state)
+{
+ struct aspeed_cooling_device *cdev =
+ (struct aspeed_cooling_device *)tcdev->devdata;
+
+ *state = cdev->max_state;
+
+ return 0;
+}
+
+static int
+aspeed_pwm_cz_get_cur_state(struct thermal_cooling_device *tcdev,
+ unsigned long *state)
+{
+ struct aspeed_cooling_device *cdev =
+ (struct aspeed_cooling_device *)tcdev->devdata;
+
+ *state = cdev->cur_state;
+
+ return 0;
+}
+
+static int
+aspeed_pwm_cz_set_cur_state(struct thermal_cooling_device *tcdev,
+ unsigned long state)
+{
+ struct aspeed_cooling_device *cdev =
+ (struct aspeed_cooling_device *)tcdev->devdata;
+
+ if (state > cdev->max_state)
+ return -EINVAL;
+
+ cdev->cur_state = state;
+ cdev->priv->pwm_port_fan_ctrl[cdev->pwm_port] = *(cdev->cooling_levels
+ + cdev->cur_state);
+ aspeed_set_pwm_port_fan_ctrl(cdev->priv, cdev->pwm_port,
+ *(cdev->cooling_levels +
+ cdev->cur_state));
+
+ return 0;
+}
+
+static const struct thermal_cooling_device_ops aspeed_pwm_cool_ops = {
+ .get_max_state = aspeed_pwm_cz_get_max_state,
+ .get_cur_state = aspeed_pwm_cz_get_cur_state,
+ .set_cur_state = aspeed_pwm_cz_set_cur_state,
+};
+
+static int aspeed_create_pwm(struct device *dev,
+ struct device_node *child,
+ struct aspeed_pwm_tacho_data *priv)
+{
+ u32 pwm_port;
+ int ret;
+
+ ret = of_property_read_u32(child, "reg", &pwm_port);
+ if (ret)
+ return ret;
+
+ ret = of_property_count_u8_elems(child, "cooling-levels");
+ if (ret <= 0) {
+ dev_err(dev, "Wrong cooling-levels data.\n");
+ return -EINVAL;
+ }
+
+ priv->cdev[pwm_port] = devm_kzalloc(dev, sizeof(*priv->cdev[pwm_port]),
+ GFP_KERNEL);
+ if (!priv->cdev[pwm_port])
+ return -ENOMEM;
+
+ priv->cdev[pwm_port]->cooling_levels = devm_kzalloc(dev, ret *
+ sizeof(u8),
+ GFP_KERNEL);
+ if (!priv->cdev[pwm_port]->cooling_levels)
+ return -ENOMEM;
+
+ priv->cdev[pwm_port]->max_state = ret - 1;
+ ret = of_property_read_u8_array(child, "cooling-levels",
+ priv->cdev[pwm_port]->cooling_levels,
+ ret);
+ if (ret) {
+ dev_err(dev, "Property 'cooling-levels' cannot be read.\n");
+ return ret;
+ }
+
+ sprintf(priv->cdev[pwm_port]->name, "%s%d", child->name, pwm_port);
+ priv->cdev[pwm_port]->tcdev = thermal_of_cooling_device_register(child,
+ priv->cdev[pwm_port]->name,
+ priv->cdev[pwm_port],
+ &aspeed_pwm_cool_ops);
+ if (PTR_ERR_OR_ZERO(priv->cdev[pwm_port]->tcdev))
+ return PTR_ERR(priv->cdev[pwm_port]->tcdev);
+
+ priv->cdev[pwm_port]->priv = priv;
+ priv->cdev[pwm_port]->pwm_port = pwm_port;
+
+ return 0;
+}
+
static int aspeed_pwm_tacho_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
- struct device_node *np, *child;
+ struct device_node *np, *child, *grandchild;
struct aspeed_pwm_tacho_data *priv;
void __iomem *regs;
struct resource *res;
@@ -833,10 +946,28 @@ static int aspeed_pwm_tacho_probe(struct platform_device *pdev)
aspeed_create_type(priv);
for_each_child_of_node(np, child) {
- ret = aspeed_create_fan(dev, child, priv);
+ if (!of_node_cmp(child->name, "tach-channels")) {
+ for_each_child_of_node(child, grandchild) {
+ ret = aspeed_create_fan(dev, grandchild, priv);
+ of_node_put(grandchild);
+ if (ret) {
+ of_node_put(child);
+ return ret;
+ }
+ }
+ }
+
+ if (!of_node_cmp(child->name, "pwm-channels")) {
+ for_each_child_of_node(child, grandchild) {
+ ret = aspeed_create_pwm(dev, grandchild, priv);
+ of_node_put(grandchild);
+ if (ret) {
+ of_node_put(child);
+ return ret;
+ }
+ }
+ }
of_node_put(child);
- if (ret)
- return ret;
}
priv->groups[0] = &pwm_dev_group;
--
2.8.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH linux dev-4.10] hwmon: (aspeed-pwm-tacho) cooling device support
2017-07-04 13:56 [PATCH linux dev-4.10] hwmon: (aspeed-pwm-tacho) cooling device support Mykola Kostenok
@ 2017-07-05 19:42 ` Patrick Venture
2017-07-05 20:10 ` Vadim Pasternak
2017-07-06 15:57 ` Patrick Williams
1 sibling, 1 reply; 7+ messages in thread
From: Patrick Venture @ 2017-07-05 19:42 UTC (permalink / raw)
To: Mykola Kostenok; +Cc: Joel Stanley, OpenBMC Maillist
On Tue, Jul 4, 2017 at 6:56 AM, Mykola Kostenok <c_mykolak@mellanox.com> wrote:
> Add support in aspeed-pwm-tacho driver for cooling device creation.
> This cooling device could be bound to a thermal zone
> for the thermal control. Device will appear in /sys/class/thermal
> folder as cooling_deviceX. Then it could be bound to particular
> thermal zones. Allow specification of the cooling levels
> vector - PWM duty cycle values in a range from 0 to 255
> which correspond to thermal cooling states.
>
> Add basic pwm-tacho-controller node to ast-g5 dtsi.
> Add basic pwm-tacho-controller node to ast-g4 dtsi.
>
> Change pwm-tacho in aspeed-bmc-quanta-q71l.dts according new
> pwm-tacho structure.
>
> Add pwm-tacho to aspeed-bmc-mellanox-msn.dts.
>
> Signed-off-by: Mykola Kostenok <c_mykolak@mellanox.com>
> ---
> arch/arm/boot/dts/aspeed-bmc-mellanox-msn.dts | 61 +++++++++++
> arch/arm/boot/dts/aspeed-bmc-quanta-q71l.dts | 103 +++++++++----------
> arch/arm/boot/dts/aspeed-g4.dtsi | 9 +-
> arch/arm/boot/dts/aspeed-g5.dtsi | 17 ++++
> drivers/hwmon/aspeed-pwm-tacho.c | 139 +++++++++++++++++++++++++-
> 5 files changed, 273 insertions(+), 56 deletions(-)
>
> diff --git a/arch/arm/boot/dts/aspeed-bmc-mellanox-msn.dts b/arch/arm/boot/dts/aspeed-bmc-mellanox-msn.dts
> index c71a6db..57d5951 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-mellanox-msn.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-mellanox-msn.dts
> @@ -144,3 +144,64 @@
> status = "okay";
> };
>
> +&pwm_tacho {
> + status = "okay";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_pwm0_default>;
> +
> + tach-channels {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + fan@0 {
> + reg = <0x00>;
> + aspeed,fan-tach-ch = /bits/ 8 <0x00>;
> + };
> +
> + fan@1 {
> + reg = <0x00>;
> + aspeed,fan-tach-ch = /bits/ 8 <0x01>;
> + };
> +
> + fan@2 {
> + reg = <0x00>;
> + aspeed,fan-tach-ch = /bits/ 8 <0x02>;
> + };
> +
> + fan@3 {
> + reg = <0x00>;
> + aspeed,fan-tach-ch = /bits/ 8 <0x03>;
> + };
> +
> + fan@4 {
> + reg = <0x00>;
> + aspeed,fan-tach-ch = /bits/ 8 <0x04>;
> + };
> +
> + fan@5 {
> + reg = <0x00>;
> + aspeed,fan-tach-ch = /bits/ 8 <0x05>;
> + };
> +
> + fan@6 {
> + reg = <0x00>;
> + aspeed,fan-tach-ch = /bits/ 8 <0x06>;
> + };
> +
> + fan@7 {
> + reg = <0x00>;
> + aspeed,fan-tach-ch = /bits/ 8 <0x07>;
> + };
> + };
> +
> + pwm-channels {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + #cooling-cells = <2>;
> +
> + pwm@0 {
> + reg = <0x00>;
> + cooling-levels = /bits/ 8 <125 151 177 203 229 255>;
> + };
> + };
> +};
> diff --git a/arch/arm/boot/dts/aspeed-bmc-quanta-q71l.dts b/arch/arm/boot/dts/aspeed-bmc-quanta-q71l.dts
> index bb287fb..43ada5b 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-quanta-q71l.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-quanta-q71l.dts
> @@ -46,57 +46,6 @@
> gpios = <&gpio ASPEED_GPIO(B, 3) GPIO_ACTIVE_LOW>;
> };
> };
> -
> - pwm_tacho: pwm-tacho-controller@1e786000 {
> - compatible = "aspeed,ast2500-pwm-tacho";
> - #address-cells = <1>;
> - #size-cells = <0>;
> - reg = <0x1e786000 0x1000>;
> - clocks = <&pwm_tacho_fixed_clk>;
> - pinctrl-names = "default";
> - pinctrl-0 = <&pinctrl_pwm0_default &pinctrl_pwm1_default
> - &pinctrl_pwm2_default &pinctrl_pwm3_default>;
> -
> - fan@0 {
> - reg = <0x00>;
> - aspeed,fan-tach-ch = /bits/ 8 <0x00>;
> - };
> -
> - fan@1 {
> - reg = <0x01>;
> - aspeed,fan-tach-ch = /bits/ 8 <0x01>;
> - };
> -
> - fan@2 {
> - reg = <0x02>;
> - aspeed,fan-tach-ch = /bits/ 8 <0x02>;
> - };
> -
> - fan@3 {
> - reg = <0x03>;
> - aspeed,fan-tach-ch = /bits/ 8 <0x03>;
> - };
> -
> - fan@4 {
> - reg = <0x00>;
> - aspeed,fan-tach-ch = /bits/ 8 <0x04>;
> - };
> -
> - fan@5 {
> - reg = <0x01>;
> - aspeed,fan-tach-ch = /bits/ 8 <0x05>;
> - };
> -
> - fan@6 {
> - reg = <0x02>;
> - aspeed,fan-tach-ch = /bits/ 8 <0x06>;
> - };
> -
> - fan@7 {
> - reg = <0x03>;
> - aspeed,fan-tach-ch = /bits/ 8 <0x07>;
> - };
> - };
> };
>
> &fmc {
> @@ -299,3 +248,55 @@
> &wdt2 {
> status = "okay";
> };
> +
> +&pwm_tacho {
> + status = "okay";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_pwm0_default &pinctrl_pwm1_default
> + &pinctrl_pwm2_default &pinctrl_pwm3_default>;
> +
> + tach-channels {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + fan@0 {
> + reg = <0x00>;
> + aspeed,fan-tach-ch = /bits/ 8 <0x00>;
> + };
> +
> + fan@1 {
> + reg = <0x01>;
> + aspeed,fan-tach-ch = /bits/ 8 <0x01>;
> + };
> +
> + fan@2 {
> + reg = <0x02>;
> + aspeed,fan-tach-ch = /bits/ 8 <0x02>;
> + };
> +
> + fan@3 {
> + reg = <0x03>;
> + aspeed,fan-tach-ch = /bits/ 8 <0x03>;
> + };
> +
> + fan@4 {
> + reg = <0x00>;
> + aspeed,fan-tach-ch = /bits/ 8 <0x04>;
> + };
> +
> + fan@5 {
> + reg = <0x01>;
> + aspeed,fan-tach-ch = /bits/ 8 <0x05>;
> + };
> +
> + fan@6 {
> + reg = <0x02>;
> + aspeed,fan-tach-ch = /bits/ 8 <0x06>;
> + };
> +
> + fan@7 {
> + reg = <0x03>;
> + aspeed,fan-tach-ch = /bits/ 8 <0x07>;
> + };
> + };
> +};
> diff --git a/arch/arm/boot/dts/aspeed-g4.dtsi b/arch/arm/boot/dts/aspeed-g4.dtsi
> index 9cc959f..c69ad77 100644
> --- a/arch/arm/boot/dts/aspeed-g4.dtsi
> +++ b/arch/arm/boot/dts/aspeed-g4.dtsi
> @@ -318,7 +318,14 @@
> ranges = <0 0x1e78a000 0x1000>;
> };
>
> -
> + pwm_tacho: pwm-tacho-controller@1e786000 {
> + compatible = "aspeed,ast2500-pwm-tacho";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0x1e786000 0x1000>;
> + clocks = <&pwm_tacho_fixed_clk>;
> + status = "disabled";
> + };
> };
> };
> };
> diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi
> index 992242d..1ed29d9 100644
> --- a/arch/arm/boot/dts/aspeed-g5.dtsi
> +++ b/arch/arm/boot/dts/aspeed-g5.dtsi
> @@ -35,6 +35,14 @@
> i2c13 = &i2c13;
> };
>
> + clocks {
> + pwm_tacho_fixed_clk: fixedclk {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <24000000>;
> + };
> + };
> +
> ahb {
> compatible = "simple-bus";
> #address-cells = <1>;
> @@ -366,6 +374,15 @@
> #size-cells = <1>;
> ranges = <0 0x1e78a000 0x1000>;
> };
> +
> + pwm_tacho: pwm-tacho-controller@1e786000 {
> + compatible = "aspeed,ast2500-pwm-tacho";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0x1e786000 0x1000>;
> + clocks = <&pwm_tacho_fixed_clk>;
> + status = "disabled";
> + };
> };
> };
> };
> diff --git a/drivers/hwmon/aspeed-pwm-tacho.c b/drivers/hwmon/aspeed-pwm-tacho.c
> index ddfe66b..eab18af 100644
> --- a/drivers/hwmon/aspeed-pwm-tacho.c
> +++ b/drivers/hwmon/aspeed-pwm-tacho.c
> @@ -20,6 +20,7 @@
> #include <linux/platform_device.h>
> #include <linux/sysfs.h>
> #include <linux/regmap.h>
> +#include <linux/thermal.h>
>
> /* ASPEED PWM & FAN Tach Register Definition */
> #define ASPEED_PTCR_CTRL 0x00
> @@ -166,6 +167,16 @@
> /* How long we sleep in us while waiting for an RPM result. */
> #define ASPEED_RPM_STATUS_SLEEP_USEC 500
>
> +struct aspeed_cooling_device {
> + char name[16];
> + struct aspeed_pwm_tacho_data *priv;
> + struct thermal_cooling_device *tcdev;
> + int pwm_port;
> + u8 *cooling_levels;
> + u8 max_state;
> + u8 cur_state;
> +};
> +
> struct aspeed_pwm_tacho_data {
> struct regmap *regmap;
> unsigned long clk_freq;
> @@ -180,6 +191,7 @@ struct aspeed_pwm_tacho_data {
> u8 pwm_port_type[8];
> u8 pwm_port_fan_ctrl[8];
> u8 fan_tach_ch_source[16];
> + struct aspeed_cooling_device *cdev[8];
> const struct attribute_group *groups[3];
> };
>
> @@ -794,10 +806,111 @@ static int aspeed_create_fan(struct device *dev,
> return 0;
> }
>
> +static int
> +aspeed_pwm_cz_get_max_state(struct thermal_cooling_device *tcdev,
> + unsigned long *state)
> +{
> + struct aspeed_cooling_device *cdev =
> + (struct aspeed_cooling_device *)tcdev->devdata;
> +
> + *state = cdev->max_state;
> +
> + return 0;
> +}
> +
> +static int
> +aspeed_pwm_cz_get_cur_state(struct thermal_cooling_device *tcdev,
> + unsigned long *state)
> +{
> + struct aspeed_cooling_device *cdev =
> + (struct aspeed_cooling_device *)tcdev->devdata;
> +
> + *state = cdev->cur_state;
> +
> + return 0;
> +}
> +
> +static int
> +aspeed_pwm_cz_set_cur_state(struct thermal_cooling_device *tcdev,
> + unsigned long state)
> +{
> + struct aspeed_cooling_device *cdev =
> + (struct aspeed_cooling_device *)tcdev->devdata;
> +
> + if (state > cdev->max_state)
> + return -EINVAL;
> +
> + cdev->cur_state = state;
> + cdev->priv->pwm_port_fan_ctrl[cdev->pwm_port] = *(cdev->cooling_levels
> + + cdev->cur_state);
> + aspeed_set_pwm_port_fan_ctrl(cdev->priv, cdev->pwm_port,
> + *(cdev->cooling_levels +
> + cdev->cur_state));
> +
> + return 0;
> +}
> +
> +static const struct thermal_cooling_device_ops aspeed_pwm_cool_ops = {
> + .get_max_state = aspeed_pwm_cz_get_max_state,
> + .get_cur_state = aspeed_pwm_cz_get_cur_state,
> + .set_cur_state = aspeed_pwm_cz_set_cur_state,
> +};
> +
> +static int aspeed_create_pwm(struct device *dev,
> + struct device_node *child,
> + struct aspeed_pwm_tacho_data *priv)
For this, I would recommend just renaming to aspeed_create_pwm_cooling
> +{
> + u32 pwm_port;
> + int ret;
> +
> + ret = of_property_read_u32(child, "reg", &pwm_port);
> + if (ret)
> + return ret;
> +
> + ret = of_property_count_u8_elems(child, "cooling-levels");
> + if (ret <= 0) {
> + dev_err(dev, "Wrong cooling-levels data.\n");
> + return -EINVAL;
> + }
> +
> + priv->cdev[pwm_port] = devm_kzalloc(dev, sizeof(*priv->cdev[pwm_port]),
> + GFP_KERNEL);
> + if (!priv->cdev[pwm_port])
> + return -ENOMEM;
> +
> + priv->cdev[pwm_port]->cooling_levels = devm_kzalloc(dev, ret *
> + sizeof(u8),
> + GFP_KERNEL);
> + if (!priv->cdev[pwm_port]->cooling_levels)
> + return -ENOMEM;
> +
> + priv->cdev[pwm_port]->max_state = ret - 1;
> + ret = of_property_read_u8_array(child, "cooling-levels",
> + priv->cdev[pwm_port]->cooling_levels,
> + ret);
> + if (ret) {
> + dev_err(dev, "Property 'cooling-levels' cannot be read.\n");
> + return ret;
> + }
> +
> + sprintf(priv->cdev[pwm_port]->name, "%s%d", child->name, pwm_port);
> + priv->cdev[pwm_port]->tcdev = thermal_of_cooling_device_register(child,
> + priv->cdev[pwm_port]->name,
> + priv->cdev[pwm_port],
> + &aspeed_pwm_cool_ops);
> + if (PTR_ERR_OR_ZERO(priv->cdev[pwm_port]->tcdev))
> + return PTR_ERR(priv->cdev[pwm_port]->tcdev);
> +
> + priv->cdev[pwm_port]->priv = priv;
> + priv->cdev[pwm_port]->pwm_port = pwm_port;
> +
> + return 0;
> +}
> +
> static int aspeed_pwm_tacho_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> - struct device_node *np, *child;
> + struct device_node *np, *child, *grandchild;
> struct aspeed_pwm_tacho_data *priv;
> void __iomem *regs;
> struct resource *res;
> @@ -833,10 +946,28 @@ static int aspeed_pwm_tacho_probe(struct platform_device *pdev)
> aspeed_create_type(priv);
>
> for_each_child_of_node(np, child) {
> - ret = aspeed_create_fan(dev, child, priv);
> + if (!of_node_cmp(child->name, "tach-channels")) {
> + for_each_child_of_node(child, grandchild) {
> + ret = aspeed_create_fan(dev, grandchild, priv);
> + of_node_put(grandchild);
> + if (ret) {
> + of_node_put(child);
> + return ret;
> + }
> + }
> + }
> +
> + if (!of_node_cmp(child->name, "pwm-channels")) {
> + for_each_child_of_node(child, grandchild) {
> + ret = aspeed_create_pwm(dev, grandchild, priv);
> + of_node_put(grandchild);
> + if (ret) {
> + of_node_put(child);
> + return ret;
> + }
> + }
> + }
> of_node_put(child);
> - if (ret)
> - return ret;
> }
>
> priv->groups[0] = &pwm_dev_group;
> --
> 2.8.4
>
Presumably it's been tested and won't impact users who don't configure
this behaviour, so I don't have a lot of feedback about functionality.
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH linux dev-4.10] hwmon: (aspeed-pwm-tacho) cooling device support
2017-07-05 19:42 ` Patrick Venture
@ 2017-07-05 20:10 ` Vadim Pasternak
2017-07-05 20:49 ` Patrick Venture
0 siblings, 1 reply; 7+ messages in thread
From: Vadim Pasternak @ 2017-07-05 20:10 UTC (permalink / raw)
To: Patrick Venture, Mykola Kostenok; +Cc: OpenBMC Maillist
> -----Original Message-----
> From: openbmc [mailto:openbmc-
> bounces+yanivab=mellanox.com@lists.ozlabs.org] On Behalf Of Patrick
> Venture
> Sent: Wednesday, July 05, 2017 10:42 PM
> To: Mykola Kostenok <c_mykolak@mellanox.com>
> Cc: OpenBMC Maillist <openbmc@lists.ozlabs.org>
> Subject: Re: [PATCH linux dev-4.10] hwmon: (aspeed-pwm-tacho) cooling
> device support
>
> On Tue, Jul 4, 2017 at 6:56 AM, Mykola Kostenok
> <c_mykolak@mellanox.com> wrote:
> > Add support in aspeed-pwm-tacho driver for cooling device creation.
> > This cooling device could be bound to a thermal zone for the thermal
> > control. Device will appear in /sys/class/thermal folder as
> > cooling_deviceX. Then it could be bound to particular thermal zones.
> > Allow specification of the cooling levels vector - PWM duty cycle
> > values in a range from 0 to 255 which correspond to thermal cooling
> > states.
> >
> > Add basic pwm-tacho-controller node to ast-g5 dtsi.
> > Add basic pwm-tacho-controller node to ast-g4 dtsi.
> >
> > Change pwm-tacho in aspeed-bmc-quanta-q71l.dts according new pwm-
> tacho
> > structure.
> >
> > Add pwm-tacho to aspeed-bmc-mellanox-msn.dts.
> >
> > Signed-off-by: Mykola Kostenok <c_mykolak@mellanox.com>
> > ---
> > arch/arm/boot/dts/aspeed-bmc-mellanox-msn.dts | 61 +++++++++++
> > arch/arm/boot/dts/aspeed-bmc-quanta-q71l.dts | 103 +++++++++---------
> -
> > arch/arm/boot/dts/aspeed-g4.dtsi | 9 +-
> > arch/arm/boot/dts/aspeed-g5.dtsi | 17 ++++
> > drivers/hwmon/aspeed-pwm-tacho.c | 139
> +++++++++++++++++++++++++-
> > 5 files changed, 273 insertions(+), 56 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/aspeed-bmc-mellanox-msn.dts
> > b/arch/arm/boot/dts/aspeed-bmc-mellanox-msn.dts
> > index c71a6db..57d5951 100644
> > --- a/arch/arm/boot/dts/aspeed-bmc-mellanox-msn.dts
> > +++ b/arch/arm/boot/dts/aspeed-bmc-mellanox-msn.dts
> > @@ -144,3 +144,64 @@
> > status = "okay";
> > };
> >
> > +&pwm_tacho {
> > + status = "okay";
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pinctrl_pwm0_default>;
> > +
> > + tach-channels {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + fan@0 {
> > + reg = <0x00>;
> > + aspeed,fan-tach-ch = /bits/ 8 <0x00>;
> > + };
> > +
> > + fan@1 {
> > + reg = <0x00>;
> > + aspeed,fan-tach-ch = /bits/ 8 <0x01>;
> > + };
> > +
> > + fan@2 {
> > + reg = <0x00>;
> > + aspeed,fan-tach-ch = /bits/ 8 <0x02>;
> > + };
> > +
> > + fan@3 {
> > + reg = <0x00>;
> > + aspeed,fan-tach-ch = /bits/ 8 <0x03>;
> > + };
> > +
> > + fan@4 {
> > + reg = <0x00>;
> > + aspeed,fan-tach-ch = /bits/ 8 <0x04>;
> > + };
> > +
> > + fan@5 {
> > + reg = <0x00>;
> > + aspeed,fan-tach-ch = /bits/ 8 <0x05>;
> > + };
> > +
> > + fan@6 {
> > + reg = <0x00>;
> > + aspeed,fan-tach-ch = /bits/ 8 <0x06>;
> > + };
> > +
> > + fan@7 {
> > + reg = <0x00>;
> > + aspeed,fan-tach-ch = /bits/ 8 <0x07>;
> > + };
> > + };
> > +
> > + pwm-channels {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + #cooling-cells = <2>;
> > +
> > + pwm@0 {
> > + reg = <0x00>;
> > + cooling-levels = /bits/ 8 <125 151 177 203 229 255>;
> > + };
> > + };
> > +};
> > diff --git a/arch/arm/boot/dts/aspeed-bmc-quanta-q71l.dts
> > b/arch/arm/boot/dts/aspeed-bmc-quanta-q71l.dts
> > index bb287fb..43ada5b 100644
> > --- a/arch/arm/boot/dts/aspeed-bmc-quanta-q71l.dts
> > +++ b/arch/arm/boot/dts/aspeed-bmc-quanta-q71l.dts
> > @@ -46,57 +46,6 @@
> > gpios = <&gpio ASPEED_GPIO(B, 3) GPIO_ACTIVE_LOW>;
> > };
> > };
> > -
> > - pwm_tacho: pwm-tacho-controller@1e786000 {
> > - compatible = "aspeed,ast2500-pwm-tacho";
> > - #address-cells = <1>;
> > - #size-cells = <0>;
> > - reg = <0x1e786000 0x1000>;
> > - clocks = <&pwm_tacho_fixed_clk>;
> > - pinctrl-names = "default";
> > - pinctrl-0 = <&pinctrl_pwm0_default &pinctrl_pwm1_default
> > - &pinctrl_pwm2_default &pinctrl_pwm3_default>;
> > -
> > - fan@0 {
> > - reg = <0x00>;
> > - aspeed,fan-tach-ch = /bits/ 8 <0x00>;
> > - };
> > -
> > - fan@1 {
> > - reg = <0x01>;
> > - aspeed,fan-tach-ch = /bits/ 8 <0x01>;
> > - };
> > -
> > - fan@2 {
> > - reg = <0x02>;
> > - aspeed,fan-tach-ch = /bits/ 8 <0x02>;
> > - };
> > -
> > - fan@3 {
> > - reg = <0x03>;
> > - aspeed,fan-tach-ch = /bits/ 8 <0x03>;
> > - };
> > -
> > - fan@4 {
> > - reg = <0x00>;
> > - aspeed,fan-tach-ch = /bits/ 8 <0x04>;
> > - };
> > -
> > - fan@5 {
> > - reg = <0x01>;
> > - aspeed,fan-tach-ch = /bits/ 8 <0x05>;
> > - };
> > -
> > - fan@6 {
> > - reg = <0x02>;
> > - aspeed,fan-tach-ch = /bits/ 8 <0x06>;
> > - };
> > -
> > - fan@7 {
> > - reg = <0x03>;
> > - aspeed,fan-tach-ch = /bits/ 8 <0x07>;
> > - };
> > - };
> > };
> >
> > &fmc {
> > @@ -299,3 +248,55 @@
> > &wdt2 {
> > status = "okay";
> > };
> > +
> > +&pwm_tacho {
> > + status = "okay";
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pinctrl_pwm0_default &pinctrl_pwm1_default
> > + &pinctrl_pwm2_default
> > +&pinctrl_pwm3_default>;
> > +
> > + tach-channels {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + fan@0 {
> > + reg = <0x00>;
> > + aspeed,fan-tach-ch = /bits/ 8 <0x00>;
> > + };
> > +
> > + fan@1 {
> > + reg = <0x01>;
> > + aspeed,fan-tach-ch = /bits/ 8 <0x01>;
> > + };
> > +
> > + fan@2 {
> > + reg = <0x02>;
> > + aspeed,fan-tach-ch = /bits/ 8 <0x02>;
> > + };
> > +
> > + fan@3 {
> > + reg = <0x03>;
> > + aspeed,fan-tach-ch = /bits/ 8 <0x03>;
> > + };
> > +
> > + fan@4 {
> > + reg = <0x00>;
> > + aspeed,fan-tach-ch = /bits/ 8 <0x04>;
> > + };
> > +
> > + fan@5 {
> > + reg = <0x01>;
> > + aspeed,fan-tach-ch = /bits/ 8 <0x05>;
> > + };
> > +
> > + fan@6 {
> > + reg = <0x02>;
> > + aspeed,fan-tach-ch = /bits/ 8 <0x06>;
> > + };
> > +
> > + fan@7 {
> > + reg = <0x03>;
> > + aspeed,fan-tach-ch = /bits/ 8 <0x07>;
> > + };
> > + };
> > +};
> > diff --git a/arch/arm/boot/dts/aspeed-g4.dtsi
> > b/arch/arm/boot/dts/aspeed-g4.dtsi
> > index 9cc959f..c69ad77 100644
> > --- a/arch/arm/boot/dts/aspeed-g4.dtsi
> > +++ b/arch/arm/boot/dts/aspeed-g4.dtsi
> > @@ -318,7 +318,14 @@
> > ranges = <0 0x1e78a000 0x1000>;
> > };
> >
> > -
> > + pwm_tacho: pwm-tacho-controller@1e786000 {
> > + compatible = "aspeed,ast2500-pwm-tacho";
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + reg = <0x1e786000 0x1000>;
> > + clocks = <&pwm_tacho_fixed_clk>;
> > + status = "disabled";
> > + };
> > };
> > };
> > };
> > diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi
> > b/arch/arm/boot/dts/aspeed-g5.dtsi
> > index 992242d..1ed29d9 100644
> > --- a/arch/arm/boot/dts/aspeed-g5.dtsi
> > +++ b/arch/arm/boot/dts/aspeed-g5.dtsi
> > @@ -35,6 +35,14 @@
> > i2c13 = &i2c13;
> > };
> >
> > + clocks {
> > + pwm_tacho_fixed_clk: fixedclk {
> > + compatible = "fixed-clock";
> > + #clock-cells = <0>;
> > + clock-frequency = <24000000>;
> > + };
> > + };
> > +
> > ahb {
> > compatible = "simple-bus";
> > #address-cells = <1>;
> > @@ -366,6 +374,15 @@
> > #size-cells = <1>;
> > ranges = <0 0x1e78a000 0x1000>;
> > };
> > +
> > + pwm_tacho: pwm-tacho-controller@1e786000 {
> > + compatible = "aspeed,ast2500-pwm-tacho";
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + reg = <0x1e786000 0x1000>;
> > + clocks = <&pwm_tacho_fixed_clk>;
> > + status = "disabled";
> > + };
> > };
> > };
> > };
> > diff --git a/drivers/hwmon/aspeed-pwm-tacho.c
> > b/drivers/hwmon/aspeed-pwm-tacho.c
> > index ddfe66b..eab18af 100644
> > --- a/drivers/hwmon/aspeed-pwm-tacho.c
> > +++ b/drivers/hwmon/aspeed-pwm-tacho.c
> > @@ -20,6 +20,7 @@
> > #include <linux/platform_device.h>
> > #include <linux/sysfs.h>
> > #include <linux/regmap.h>
> > +#include <linux/thermal.h>
> >
> > /* ASPEED PWM & FAN Tach Register Definition */
> > #define ASPEED_PTCR_CTRL 0x00
> > @@ -166,6 +167,16 @@
> > /* How long we sleep in us while waiting for an RPM result. */
> > #define ASPEED_RPM_STATUS_SLEEP_USEC 500
> >
> > +struct aspeed_cooling_device {
> > + char name[16];
> > + struct aspeed_pwm_tacho_data *priv;
> > + struct thermal_cooling_device *tcdev;
> > + int pwm_port;
> > + u8 *cooling_levels;
> > + u8 max_state;
> > + u8 cur_state;
> > +};
> > +
> > struct aspeed_pwm_tacho_data {
> > struct regmap *regmap;
> > unsigned long clk_freq;
> > @@ -180,6 +191,7 @@ struct aspeed_pwm_tacho_data {
> > u8 pwm_port_type[8];
> > u8 pwm_port_fan_ctrl[8];
> > u8 fan_tach_ch_source[16];
> > + struct aspeed_cooling_device *cdev[8];
> > const struct attribute_group *groups[3]; };
> >
> > @@ -794,10 +806,111 @@ static int aspeed_create_fan(struct device *dev,
> > return 0;
> > }
> >
> > +static int
> > +aspeed_pwm_cz_get_max_state(struct thermal_cooling_device *tcdev,
> > + unsigned long *state) {
> > + struct aspeed_cooling_device *cdev =
> > + (struct aspeed_cooling_device
> > +*)tcdev->devdata;
> > +
> > + *state = cdev->max_state;
> > +
> > + return 0;
> > +}
> > +
> > +static int
> > +aspeed_pwm_cz_get_cur_state(struct thermal_cooling_device *tcdev,
> > + unsigned long *state) {
> > + struct aspeed_cooling_device *cdev =
> > + (struct aspeed_cooling_device
> > +*)tcdev->devdata;
> > +
> > + *state = cdev->cur_state;
> > +
> > + return 0;
> > +}
> > +
> > +static int
> > +aspeed_pwm_cz_set_cur_state(struct thermal_cooling_device *tcdev,
> > + unsigned long state) {
> > + struct aspeed_cooling_device *cdev =
> > + (struct aspeed_cooling_device
> > +*)tcdev->devdata;
> > +
> > + if (state > cdev->max_state)
> > + return -EINVAL;
> > +
> > + cdev->cur_state = state;
> > + cdev->priv->pwm_port_fan_ctrl[cdev->pwm_port] = *(cdev-
> >cooling_levels
> > + + cdev->cur_state);
> > + aspeed_set_pwm_port_fan_ctrl(cdev->priv, cdev->pwm_port,
> > + *(cdev->cooling_levels +
> > + cdev->cur_state));
> > +
> > + return 0;
> > +}
> > +
> > +static const struct thermal_cooling_device_ops aspeed_pwm_cool_ops =
> {
> > + .get_max_state = aspeed_pwm_cz_get_max_state,
> > + .get_cur_state = aspeed_pwm_cz_get_cur_state,
> > + .set_cur_state = aspeed_pwm_cz_set_cur_state, };
> > +
> > +static int aspeed_create_pwm(struct device *dev,
> > + struct device_node *child,
> > + struct aspeed_pwm_tacho_data *priv)
>
> For this, I would recommend just renaming to aspeed_create_pwm_cooling
>
> > +{
> > + u32 pwm_port;
> > + int ret;
> > +
> > + ret = of_property_read_u32(child, "reg", &pwm_port);
> > + if (ret)
> > + return ret;
> > +
> > + ret = of_property_count_u8_elems(child, "cooling-levels");
> > + if (ret <= 0) {
> > + dev_err(dev, "Wrong cooling-levels data.\n");
> > + return -EINVAL;
> > + }
> > +
> > + priv->cdev[pwm_port] = devm_kzalloc(dev, sizeof(*priv-
> >cdev[pwm_port]),
> > + GFP_KERNEL);
> > + if (!priv->cdev[pwm_port])
> > + return -ENOMEM;
> > +
> > + priv->cdev[pwm_port]->cooling_levels = devm_kzalloc(dev, ret *
> > + sizeof(u8),
> > + GFP_KERNEL);
> > + if (!priv->cdev[pwm_port]->cooling_levels)
> > + return -ENOMEM;
> > +
> > + priv->cdev[pwm_port]->max_state = ret - 1;
> > + ret = of_property_read_u8_array(child, "cooling-levels",
> > + priv->cdev[pwm_port]->cooling_levels,
> > + ret);
> > + if (ret) {
> > + dev_err(dev, "Property 'cooling-levels' cannot be read.\n");
> > + return ret;
> > + }
> > +
> > + sprintf(priv->cdev[pwm_port]->name, "%s%d", child->name,
> pwm_port);
> > + priv->cdev[pwm_port]->tcdev =
> thermal_of_cooling_device_register(child,
> > + priv->cdev[pwm_port]->name,
> > + priv->cdev[pwm_port],
> > + &aspeed_pwm_cool_ops);
> > + if (PTR_ERR_OR_ZERO(priv->cdev[pwm_port]->tcdev))
> > + return PTR_ERR(priv->cdev[pwm_port]->tcdev);
> > +
> > + priv->cdev[pwm_port]->priv = priv;
> > + priv->cdev[pwm_port]->pwm_port = pwm_port;
> > +
> > + return 0;
> > +}
> > +
> > static int aspeed_pwm_tacho_probe(struct platform_device *pdev) {
> > struct device *dev = &pdev->dev;
> > - struct device_node *np, *child;
> > + struct device_node *np, *child, *grandchild;
> > struct aspeed_pwm_tacho_data *priv;
> > void __iomem *regs;
> > struct resource *res;
> > @@ -833,10 +946,28 @@ static int aspeed_pwm_tacho_probe(struct
> platform_device *pdev)
> > aspeed_create_type(priv);
> >
> > for_each_child_of_node(np, child) {
> > - ret = aspeed_create_fan(dev, child, priv);
> > + if (!of_node_cmp(child->name, "tach-channels")) {
> > + for_each_child_of_node(child, grandchild) {
> > + ret = aspeed_create_fan(dev, grandchild, priv);
> > + of_node_put(grandchild);
> > + if (ret) {
> > + of_node_put(child);
> > + return ret;
> > + }
> > + }
> > + }
> > +
> > + if (!of_node_cmp(child->name, "pwm-channels")) {
> > + for_each_child_of_node(child, grandchild) {
> > + ret = aspeed_create_pwm(dev, grandchild, priv);
> > + of_node_put(grandchild);
> > + if (ret) {
> > + of_node_put(child);
> > + return ret;
> > + }
> > + }
> > + }
> > of_node_put(child);
> > - if (ret)
> > - return ret;
> > }
> >
> > priv->groups[0] = &pwm_dev_group;
> > --
> > 2.8.4
> >
>
> Presumably it's been tested and won't impact users who don't configure this
> behaviour, so I don't have a lot of feedback about functionality.
Hi Patrick,
Thanks for review.
If it's not configured, cooling device is not created and of course there is no impact of functionality.
If cooling device is created, but not bound to any thermal device there is also no impact.
We are using it along with kernel step wise thermal algorithm which is supposed to set up or down pwm on crossing thermal zone boundary to the relevant cooling level and to perform PWM throttling according to temperature trend.
It also can be enabled/disabled through sysfs.
Cheers,
Vadim.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH linux dev-4.10] hwmon: (aspeed-pwm-tacho) cooling device support
2017-07-05 20:10 ` Vadim Pasternak
@ 2017-07-05 20:49 ` Patrick Venture
2017-07-05 20:58 ` Vadim Pasternak
0 siblings, 1 reply; 7+ messages in thread
From: Patrick Venture @ 2017-07-05 20:49 UTC (permalink / raw)
To: Vadim Pasternak; +Cc: Mykola Kostenok, OpenBMC Maillist
On Wed, Jul 5, 2017 at 1:10 PM, Vadim Pasternak <vadimp@mellanox.com> wrote:
>
>
>> -----Original Message-----
>> From: openbmc [mailto:openbmc-
>> bounces+yanivab=mellanox.com@lists.ozlabs.org] On Behalf Of Patrick
>> Venture
>> Sent: Wednesday, July 05, 2017 10:42 PM
>> To: Mykola Kostenok <c_mykolak@mellanox.com>
>> Cc: OpenBMC Maillist <openbmc@lists.ozlabs.org>
>> Subject: Re: [PATCH linux dev-4.10] hwmon: (aspeed-pwm-tacho) cooling
>> device support
>>
>> On Tue, Jul 4, 2017 at 6:56 AM, Mykola Kostenok
>> <c_mykolak@mellanox.com> wrote:
>> > Add support in aspeed-pwm-tacho driver for cooling device creation.
>> > This cooling device could be bound to a thermal zone for the thermal
>> > control. Device will appear in /sys/class/thermal folder as
>> > cooling_deviceX. Then it could be bound to particular thermal zones.
>> > Allow specification of the cooling levels vector - PWM duty cycle
>> > values in a range from 0 to 255 which correspond to thermal cooling
>> > states.
>> >
>> > Add basic pwm-tacho-controller node to ast-g5 dtsi.
>> > Add basic pwm-tacho-controller node to ast-g4 dtsi.
>> >
>> > Change pwm-tacho in aspeed-bmc-quanta-q71l.dts according new pwm-
>> tacho
>> > structure.
>> >
>> > Add pwm-tacho to aspeed-bmc-mellanox-msn.dts.
>> >
>> > Signed-off-by: Mykola Kostenok <c_mykolak@mellanox.com>
>> > ---
>> > arch/arm/boot/dts/aspeed-bmc-mellanox-msn.dts | 61 +++++++++++
>> > arch/arm/boot/dts/aspeed-bmc-quanta-q71l.dts | 103 +++++++++---------
>> -
>> > arch/arm/boot/dts/aspeed-g4.dtsi | 9 +-
>> > arch/arm/boot/dts/aspeed-g5.dtsi | 17 ++++
>> > drivers/hwmon/aspeed-pwm-tacho.c | 139
>> +++++++++++++++++++++++++-
>> > 5 files changed, 273 insertions(+), 56 deletions(-)
>> >
>> > diff --git a/arch/arm/boot/dts/aspeed-bmc-mellanox-msn.dts
>> > b/arch/arm/boot/dts/aspeed-bmc-mellanox-msn.dts
>> > index c71a6db..57d5951 100644
>> > --- a/arch/arm/boot/dts/aspeed-bmc-mellanox-msn.dts
>> > +++ b/arch/arm/boot/dts/aspeed-bmc-mellanox-msn.dts
>> > @@ -144,3 +144,64 @@
>> > status = "okay";
>> > };
>> >
>> > +&pwm_tacho {
>> > + status = "okay";
>> > + pinctrl-names = "default";
>> > + pinctrl-0 = <&pinctrl_pwm0_default>;
>> > +
>> > + tach-channels {
>> > + #address-cells = <1>;
>> > + #size-cells = <0>;
>> > +
>> > + fan@0 {
>> > + reg = <0x00>;
>> > + aspeed,fan-tach-ch = /bits/ 8 <0x00>;
>> > + };
>> > +
>> > + fan@1 {
>> > + reg = <0x00>;
>> > + aspeed,fan-tach-ch = /bits/ 8 <0x01>;
>> > + };
>> > +
>> > + fan@2 {
>> > + reg = <0x00>;
>> > + aspeed,fan-tach-ch = /bits/ 8 <0x02>;
>> > + };
>> > +
>> > + fan@3 {
>> > + reg = <0x00>;
>> > + aspeed,fan-tach-ch = /bits/ 8 <0x03>;
>> > + };
>> > +
>> > + fan@4 {
>> > + reg = <0x00>;
>> > + aspeed,fan-tach-ch = /bits/ 8 <0x04>;
>> > + };
>> > +
>> > + fan@5 {
>> > + reg = <0x00>;
>> > + aspeed,fan-tach-ch = /bits/ 8 <0x05>;
>> > + };
>> > +
>> > + fan@6 {
>> > + reg = <0x00>;
>> > + aspeed,fan-tach-ch = /bits/ 8 <0x06>;
>> > + };
>> > +
>> > + fan@7 {
>> > + reg = <0x00>;
>> > + aspeed,fan-tach-ch = /bits/ 8 <0x07>;
>> > + };
>> > + };
>> > +
>> > + pwm-channels {
>> > + #address-cells = <1>;
>> > + #size-cells = <0>;
>> > + #cooling-cells = <2>;
>> > +
>> > + pwm@0 {
>> > + reg = <0x00>;
>> > + cooling-levels = /bits/ 8 <125 151 177 203 229 255>;
>> > + };
>> > + };
>> > +};
>> > diff --git a/arch/arm/boot/dts/aspeed-bmc-quanta-q71l.dts
>> > b/arch/arm/boot/dts/aspeed-bmc-quanta-q71l.dts
>> > index bb287fb..43ada5b 100644
>> > --- a/arch/arm/boot/dts/aspeed-bmc-quanta-q71l.dts
>> > +++ b/arch/arm/boot/dts/aspeed-bmc-quanta-q71l.dts
>> > @@ -46,57 +46,6 @@
>> > gpios = <&gpio ASPEED_GPIO(B, 3) GPIO_ACTIVE_LOW>;
>> > };
>> > };
>> > -
>> > - pwm_tacho: pwm-tacho-controller@1e786000 {
>> > - compatible = "aspeed,ast2500-pwm-tacho";
>> > - #address-cells = <1>;
>> > - #size-cells = <0>;
>> > - reg = <0x1e786000 0x1000>;
>> > - clocks = <&pwm_tacho_fixed_clk>;
>> > - pinctrl-names = "default";
>> > - pinctrl-0 = <&pinctrl_pwm0_default &pinctrl_pwm1_default
>> > - &pinctrl_pwm2_default &pinctrl_pwm3_default>;
>> > -
>> > - fan@0 {
>> > - reg = <0x00>;
>> > - aspeed,fan-tach-ch = /bits/ 8 <0x00>;
>> > - };
>> > -
>> > - fan@1 {
>> > - reg = <0x01>;
>> > - aspeed,fan-tach-ch = /bits/ 8 <0x01>;
>> > - };
>> > -
>> > - fan@2 {
>> > - reg = <0x02>;
>> > - aspeed,fan-tach-ch = /bits/ 8 <0x02>;
>> > - };
>> > -
>> > - fan@3 {
>> > - reg = <0x03>;
>> > - aspeed,fan-tach-ch = /bits/ 8 <0x03>;
>> > - };
>> > -
>> > - fan@4 {
>> > - reg = <0x00>;
>> > - aspeed,fan-tach-ch = /bits/ 8 <0x04>;
>> > - };
>> > -
>> > - fan@5 {
>> > - reg = <0x01>;
>> > - aspeed,fan-tach-ch = /bits/ 8 <0x05>;
>> > - };
>> > -
>> > - fan@6 {
>> > - reg = <0x02>;
>> > - aspeed,fan-tach-ch = /bits/ 8 <0x06>;
>> > - };
>> > -
>> > - fan@7 {
>> > - reg = <0x03>;
>> > - aspeed,fan-tach-ch = /bits/ 8 <0x07>;
>> > - };
>> > - };
>> > };
>> >
>> > &fmc {
>> > @@ -299,3 +248,55 @@
>> > &wdt2 {
>> > status = "okay";
>> > };
>> > +
>> > +&pwm_tacho {
>> > + status = "okay";
>> > + pinctrl-names = "default";
>> > + pinctrl-0 = <&pinctrl_pwm0_default &pinctrl_pwm1_default
>> > + &pinctrl_pwm2_default
>> > +&pinctrl_pwm3_default>;
>> > +
>> > + tach-channels {
>> > + #address-cells = <1>;
>> > + #size-cells = <0>;
>> > +
>> > + fan@0 {
>> > + reg = <0x00>;
>> > + aspeed,fan-tach-ch = /bits/ 8 <0x00>;
>> > + };
>> > +
>> > + fan@1 {
>> > + reg = <0x01>;
>> > + aspeed,fan-tach-ch = /bits/ 8 <0x01>;
>> > + };
>> > +
>> > + fan@2 {
>> > + reg = <0x02>;
>> > + aspeed,fan-tach-ch = /bits/ 8 <0x02>;
>> > + };
>> > +
>> > + fan@3 {
>> > + reg = <0x03>;
>> > + aspeed,fan-tach-ch = /bits/ 8 <0x03>;
>> > + };
>> > +
>> > + fan@4 {
>> > + reg = <0x00>;
>> > + aspeed,fan-tach-ch = /bits/ 8 <0x04>;
>> > + };
>> > +
>> > + fan@5 {
>> > + reg = <0x01>;
>> > + aspeed,fan-tach-ch = /bits/ 8 <0x05>;
>> > + };
>> > +
>> > + fan@6 {
>> > + reg = <0x02>;
>> > + aspeed,fan-tach-ch = /bits/ 8 <0x06>;
>> > + };
>> > +
>> > + fan@7 {
>> > + reg = <0x03>;
>> > + aspeed,fan-tach-ch = /bits/ 8 <0x07>;
>> > + };
>> > + };
>> > +};
>> > diff --git a/arch/arm/boot/dts/aspeed-g4.dtsi
>> > b/arch/arm/boot/dts/aspeed-g4.dtsi
>> > index 9cc959f..c69ad77 100644
>> > --- a/arch/arm/boot/dts/aspeed-g4.dtsi
>> > +++ b/arch/arm/boot/dts/aspeed-g4.dtsi
>> > @@ -318,7 +318,14 @@
>> > ranges = <0 0x1e78a000 0x1000>;
>> > };
>> >
>> > -
>> > + pwm_tacho: pwm-tacho-controller@1e786000 {
>> > + compatible = "aspeed,ast2500-pwm-tacho";
>> > + #address-cells = <1>;
>> > + #size-cells = <0>;
>> > + reg = <0x1e786000 0x1000>;
>> > + clocks = <&pwm_tacho_fixed_clk>;
>> > + status = "disabled";
>> > + };
>> > };
>> > };
>> > };
>> > diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi
>> > b/arch/arm/boot/dts/aspeed-g5.dtsi
>> > index 992242d..1ed29d9 100644
>> > --- a/arch/arm/boot/dts/aspeed-g5.dtsi
>> > +++ b/arch/arm/boot/dts/aspeed-g5.dtsi
>> > @@ -35,6 +35,14 @@
>> > i2c13 = &i2c13;
>> > };
>> >
>> > + clocks {
>> > + pwm_tacho_fixed_clk: fixedclk {
>> > + compatible = "fixed-clock";
>> > + #clock-cells = <0>;
>> > + clock-frequency = <24000000>;
>> > + };
>> > + };
>> > +
>> > ahb {
>> > compatible = "simple-bus";
>> > #address-cells = <1>;
>> > @@ -366,6 +374,15 @@
>> > #size-cells = <1>;
>> > ranges = <0 0x1e78a000 0x1000>;
>> > };
>> > +
>> > + pwm_tacho: pwm-tacho-controller@1e786000 {
>> > + compatible = "aspeed,ast2500-pwm-tacho";
>> > + #address-cells = <1>;
>> > + #size-cells = <0>;
>> > + reg = <0x1e786000 0x1000>;
>> > + clocks = <&pwm_tacho_fixed_clk>;
>> > + status = "disabled";
>> > + };
>> > };
>> > };
>> > };
>> > diff --git a/drivers/hwmon/aspeed-pwm-tacho.c
>> > b/drivers/hwmon/aspeed-pwm-tacho.c
>> > index ddfe66b..eab18af 100644
>> > --- a/drivers/hwmon/aspeed-pwm-tacho.c
>> > +++ b/drivers/hwmon/aspeed-pwm-tacho.c
>> > @@ -20,6 +20,7 @@
>> > #include <linux/platform_device.h>
>> > #include <linux/sysfs.h>
>> > #include <linux/regmap.h>
>> > +#include <linux/thermal.h>
>> >
>> > /* ASPEED PWM & FAN Tach Register Definition */
>> > #define ASPEED_PTCR_CTRL 0x00
>> > @@ -166,6 +167,16 @@
>> > /* How long we sleep in us while waiting for an RPM result. */
>> > #define ASPEED_RPM_STATUS_SLEEP_USEC 500
>> >
>> > +struct aspeed_cooling_device {
>> > + char name[16];
>> > + struct aspeed_pwm_tacho_data *priv;
>> > + struct thermal_cooling_device *tcdev;
>> > + int pwm_port;
>> > + u8 *cooling_levels;
>> > + u8 max_state;
>> > + u8 cur_state;
>> > +};
>> > +
>> > struct aspeed_pwm_tacho_data {
>> > struct regmap *regmap;
>> > unsigned long clk_freq;
>> > @@ -180,6 +191,7 @@ struct aspeed_pwm_tacho_data {
>> > u8 pwm_port_type[8];
>> > u8 pwm_port_fan_ctrl[8];
>> > u8 fan_tach_ch_source[16];
>> > + struct aspeed_cooling_device *cdev[8];
>> > const struct attribute_group *groups[3]; };
>> >
>> > @@ -794,10 +806,111 @@ static int aspeed_create_fan(struct device *dev,
>> > return 0;
>> > }
>> >
>> > +static int
>> > +aspeed_pwm_cz_get_max_state(struct thermal_cooling_device *tcdev,
>> > + unsigned long *state) {
>> > + struct aspeed_cooling_device *cdev =
>> > + (struct aspeed_cooling_device
>> > +*)tcdev->devdata;
>> > +
>> > + *state = cdev->max_state;
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +static int
>> > +aspeed_pwm_cz_get_cur_state(struct thermal_cooling_device *tcdev,
>> > + unsigned long *state) {
>> > + struct aspeed_cooling_device *cdev =
>> > + (struct aspeed_cooling_device
>> > +*)tcdev->devdata;
>> > +
>> > + *state = cdev->cur_state;
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +static int
>> > +aspeed_pwm_cz_set_cur_state(struct thermal_cooling_device *tcdev,
>> > + unsigned long state) {
>> > + struct aspeed_cooling_device *cdev =
>> > + (struct aspeed_cooling_device
>> > +*)tcdev->devdata;
>> > +
>> > + if (state > cdev->max_state)
>> > + return -EINVAL;
>> > +
>> > + cdev->cur_state = state;
>> > + cdev->priv->pwm_port_fan_ctrl[cdev->pwm_port] = *(cdev-
>> >cooling_levels
>> > + + cdev->cur_state);
>> > + aspeed_set_pwm_port_fan_ctrl(cdev->priv, cdev->pwm_port,
>> > + *(cdev->cooling_levels +
>> > + cdev->cur_state));
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +static const struct thermal_cooling_device_ops aspeed_pwm_cool_ops =
>> {
>> > + .get_max_state = aspeed_pwm_cz_get_max_state,
>> > + .get_cur_state = aspeed_pwm_cz_get_cur_state,
>> > + .set_cur_state = aspeed_pwm_cz_set_cur_state, };
>> > +
>> > +static int aspeed_create_pwm(struct device *dev,
>> > + struct device_node *child,
>> > + struct aspeed_pwm_tacho_data *priv)
>>
>> For this, I would recommend just renaming to aspeed_create_pwm_cooling
>>
>> > +{
>> > + u32 pwm_port;
>> > + int ret;
>> > +
>> > + ret = of_property_read_u32(child, "reg", &pwm_port);
>> > + if (ret)
>> > + return ret;
>> > +
>> > + ret = of_property_count_u8_elems(child, "cooling-levels");
>> > + if (ret <= 0) {
>> > + dev_err(dev, "Wrong cooling-levels data.\n");
>> > + return -EINVAL;
>> > + }
>> > +
>> > + priv->cdev[pwm_port] = devm_kzalloc(dev, sizeof(*priv-
>> >cdev[pwm_port]),
>> > + GFP_KERNEL);
>> > + if (!priv->cdev[pwm_port])
>> > + return -ENOMEM;
>> > +
>> > + priv->cdev[pwm_port]->cooling_levels = devm_kzalloc(dev, ret *
>> > + sizeof(u8),
>> > + GFP_KERNEL);
>> > + if (!priv->cdev[pwm_port]->cooling_levels)
>> > + return -ENOMEM;
>> > +
>> > + priv->cdev[pwm_port]->max_state = ret - 1;
>> > + ret = of_property_read_u8_array(child, "cooling-levels",
>> > + priv->cdev[pwm_port]->cooling_levels,
>> > + ret);
>> > + if (ret) {
>> > + dev_err(dev, "Property 'cooling-levels' cannot be read.\n");
>> > + return ret;
>> > + }
>> > +
>> > + sprintf(priv->cdev[pwm_port]->name, "%s%d", child->name,
>> pwm_port);
>> > + priv->cdev[pwm_port]->tcdev =
>> thermal_of_cooling_device_register(child,
>> > + priv->cdev[pwm_port]->name,
>> > + priv->cdev[pwm_port],
>> > + &aspeed_pwm_cool_ops);
>> > + if (PTR_ERR_OR_ZERO(priv->cdev[pwm_port]->tcdev))
>> > + return PTR_ERR(priv->cdev[pwm_port]->tcdev);
>> > +
>> > + priv->cdev[pwm_port]->priv = priv;
>> > + priv->cdev[pwm_port]->pwm_port = pwm_port;
>> > +
>> > + return 0;
>> > +}
>> > +
>> > static int aspeed_pwm_tacho_probe(struct platform_device *pdev) {
>> > struct device *dev = &pdev->dev;
>> > - struct device_node *np, *child;
>> > + struct device_node *np, *child, *grandchild;
>> > struct aspeed_pwm_tacho_data *priv;
>> > void __iomem *regs;
>> > struct resource *res;
>> > @@ -833,10 +946,28 @@ static int aspeed_pwm_tacho_probe(struct
>> platform_device *pdev)
>> > aspeed_create_type(priv);
>> >
>> > for_each_child_of_node(np, child) {
>> > - ret = aspeed_create_fan(dev, child, priv);
>> > + if (!of_node_cmp(child->name, "tach-channels")) {
>> > + for_each_child_of_node(child, grandchild) {
>> > + ret = aspeed_create_fan(dev, grandchild, priv);
>> > + of_node_put(grandchild);
>> > + if (ret) {
>> > + of_node_put(child);
>> > + return ret;
>> > + }
>> > + }
>> > + }
>> > +
>> > + if (!of_node_cmp(child->name, "pwm-channels")) {
>> > + for_each_child_of_node(child, grandchild) {
>> > + ret = aspeed_create_pwm(dev, grandchild, priv);
>> > + of_node_put(grandchild);
>> > + if (ret) {
>> > + of_node_put(child);
>> > + return ret;
>> > + }
>> > + }
>> > + }
>> > of_node_put(child);
>> > - if (ret)
>> > - return ret;
>> > }
>> >
>> > priv->groups[0] = &pwm_dev_group;
>> > --
>> > 2.8.4
>> >
>>
>> Presumably it's been tested and won't impact users who don't configure this
>> behaviour, so I don't have a lot of feedback about functionality.
>
> Hi Patrick,
>
> Thanks for review.
>
> If it's not configured, cooling device is not created and of course there is no impact of functionality.
> If cooling device is created, but not bound to any thermal device there is also no impact.
>
> We are using it along with kernel step wise thermal algorithm which is supposed to set up or down pwm on crossing thermal zone boundary to the relevant cooling level and to perform PWM throttling according to temperature trend.
> It also can be enabled/disabled through sysfs.
>
> Cheers,
> Vadim.
Fantastic. I approve this change. Have you sent this driver patch
upstream to hwmon maintainers?
Patrick
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH linux dev-4.10] hwmon: (aspeed-pwm-tacho) cooling device support
2017-07-05 20:49 ` Patrick Venture
@ 2017-07-05 20:58 ` Vadim Pasternak
0 siblings, 0 replies; 7+ messages in thread
From: Vadim Pasternak @ 2017-07-05 20:58 UTC (permalink / raw)
To: Patrick Venture; +Cc: Mykola Kostenok, OpenBMC Maillist
> -----Original Message-----
> From: Patrick Venture [mailto:venture@google.com]
> Sent: Wednesday, July 05, 2017 11:50 PM
> To: Vadim Pasternak <vadimp@mellanox.com>
> Cc: Mykola Kostenok <c_mykolak@mellanox.com>; OpenBMC Maillist
> <openbmc@lists.ozlabs.org>
> Subject: Re: [PATCH linux dev-4.10] hwmon: (aspeed-pwm-tacho) cooling
> device support
>
> On Wed, Jul 5, 2017 at 1:10 PM, Vadim Pasternak <vadimp@mellanox.com>
> wrote:
> >
> >
> >> -----Original Message-----
> >> From: openbmc [mailto:openbmc-
> >> bounces+yanivab=mellanox.com@lists.ozlabs.org] On Behalf Of Patrick
> >> Venture
> >> Sent: Wednesday, July 05, 2017 10:42 PM
> >> To: Mykola Kostenok <c_mykolak@mellanox.com>
> >> Cc: OpenBMC Maillist <openbmc@lists.ozlabs.org>
> >> Subject: Re: [PATCH linux dev-4.10] hwmon: (aspeed-pwm-tacho) cooling
> >> device support
> >>
> >> On Tue, Jul 4, 2017 at 6:56 AM, Mykola Kostenok
> >> <c_mykolak@mellanox.com> wrote:
> >> > Add support in aspeed-pwm-tacho driver for cooling device creation.
> >> > This cooling device could be bound to a thermal zone for the
> >> > thermal control. Device will appear in /sys/class/thermal folder as
> >> > cooling_deviceX. Then it could be bound to particular thermal zones.
> >> > Allow specification of the cooling levels vector - PWM duty cycle
> >> > values in a range from 0 to 255 which correspond to thermal cooling
> >> > states.
> >> >
> >> > Add basic pwm-tacho-controller node to ast-g5 dtsi.
> >> > Add basic pwm-tacho-controller node to ast-g4 dtsi.
> >> >
> >> > Change pwm-tacho in aspeed-bmc-quanta-q71l.dts according new
> pwm-
> >> tacho
> >> > structure.
> >> >
> >> > Add pwm-tacho to aspeed-bmc-mellanox-msn.dts.
> >> >
> >> > Signed-off-by: Mykola Kostenok <c_mykolak@mellanox.com>
> >> > ---
> >> > arch/arm/boot/dts/aspeed-bmc-mellanox-msn.dts | 61 +++++++++++
> >> > arch/arm/boot/dts/aspeed-bmc-quanta-q71l.dts | 103
> >> > +++++++++---------
> >> -
> >> > arch/arm/boot/dts/aspeed-g4.dtsi | 9 +-
> >> > arch/arm/boot/dts/aspeed-g5.dtsi | 17 ++++
> >> > drivers/hwmon/aspeed-pwm-tacho.c | 139
> >> +++++++++++++++++++++++++-
> >> > 5 files changed, 273 insertions(+), 56 deletions(-)
> >> >
> >> > diff --git a/arch/arm/boot/dts/aspeed-bmc-mellanox-msn.dts
> >> > b/arch/arm/boot/dts/aspeed-bmc-mellanox-msn.dts
> >> > index c71a6db..57d5951 100644
> >> > --- a/arch/arm/boot/dts/aspeed-bmc-mellanox-msn.dts
> >> > +++ b/arch/arm/boot/dts/aspeed-bmc-mellanox-msn.dts
> >> > @@ -144,3 +144,64 @@
> >> > status = "okay";
> >> > };
> >> >
> >> > +&pwm_tacho {
> >> > + status = "okay";
> >> > + pinctrl-names = "default";
> >> > + pinctrl-0 = <&pinctrl_pwm0_default>;
> >> > +
> >> > + tach-channels {
> >> > + #address-cells = <1>;
> >> > + #size-cells = <0>;
> >> > +
> >> > + fan@0 {
> >> > + reg = <0x00>;
> >> > + aspeed,fan-tach-ch = /bits/ 8 <0x00>;
> >> > + };
> >> > +
> >> > + fan@1 {
> >> > + reg = <0x00>;
> >> > + aspeed,fan-tach-ch = /bits/ 8 <0x01>;
> >> > + };
> >> > +
> >> > + fan@2 {
> >> > + reg = <0x00>;
> >> > + aspeed,fan-tach-ch = /bits/ 8 <0x02>;
> >> > + };
> >> > +
> >> > + fan@3 {
> >> > + reg = <0x00>;
> >> > + aspeed,fan-tach-ch = /bits/ 8 <0x03>;
> >> > + };
> >> > +
> >> > + fan@4 {
> >> > + reg = <0x00>;
> >> > + aspeed,fan-tach-ch = /bits/ 8 <0x04>;
> >> > + };
> >> > +
> >> > + fan@5 {
> >> > + reg = <0x00>;
> >> > + aspeed,fan-tach-ch = /bits/ 8 <0x05>;
> >> > + };
> >> > +
> >> > + fan@6 {
> >> > + reg = <0x00>;
> >> > + aspeed,fan-tach-ch = /bits/ 8 <0x06>;
> >> > + };
> >> > +
> >> > + fan@7 {
> >> > + reg = <0x00>;
> >> > + aspeed,fan-tach-ch = /bits/ 8 <0x07>;
> >> > + };
> >> > + };
> >> > +
> >> > + pwm-channels {
> >> > + #address-cells = <1>;
> >> > + #size-cells = <0>;
> >> > + #cooling-cells = <2>;
> >> > +
> >> > + pwm@0 {
> >> > + reg = <0x00>;
> >> > + cooling-levels = /bits/ 8 <125 151 177 203 229 255>;
> >> > + };
> >> > + };
> >> > +};
> >> > diff --git a/arch/arm/boot/dts/aspeed-bmc-quanta-q71l.dts
> >> > b/arch/arm/boot/dts/aspeed-bmc-quanta-q71l.dts
> >> > index bb287fb..43ada5b 100644
> >> > --- a/arch/arm/boot/dts/aspeed-bmc-quanta-q71l.dts
> >> > +++ b/arch/arm/boot/dts/aspeed-bmc-quanta-q71l.dts
> >> > @@ -46,57 +46,6 @@
> >> > gpios = <&gpio ASPEED_GPIO(B, 3) GPIO_ACTIVE_LOW>;
> >> > };
> >> > };
> >> > -
> >> > - pwm_tacho: pwm-tacho-controller@1e786000 {
> >> > - compatible = "aspeed,ast2500-pwm-tacho";
> >> > - #address-cells = <1>;
> >> > - #size-cells = <0>;
> >> > - reg = <0x1e786000 0x1000>;
> >> > - clocks = <&pwm_tacho_fixed_clk>;
> >> > - pinctrl-names = "default";
> >> > - pinctrl-0 = <&pinctrl_pwm0_default &pinctrl_pwm1_default
> >> > - &pinctrl_pwm2_default &pinctrl_pwm3_default>;
> >> > -
> >> > - fan@0 {
> >> > - reg = <0x00>;
> >> > - aspeed,fan-tach-ch = /bits/ 8 <0x00>;
> >> > - };
> >> > -
> >> > - fan@1 {
> >> > - reg = <0x01>;
> >> > - aspeed,fan-tach-ch = /bits/ 8 <0x01>;
> >> > - };
> >> > -
> >> > - fan@2 {
> >> > - reg = <0x02>;
> >> > - aspeed,fan-tach-ch = /bits/ 8 <0x02>;
> >> > - };
> >> > -
> >> > - fan@3 {
> >> > - reg = <0x03>;
> >> > - aspeed,fan-tach-ch = /bits/ 8 <0x03>;
> >> > - };
> >> > -
> >> > - fan@4 {
> >> > - reg = <0x00>;
> >> > - aspeed,fan-tach-ch = /bits/ 8 <0x04>;
> >> > - };
> >> > -
> >> > - fan@5 {
> >> > - reg = <0x01>;
> >> > - aspeed,fan-tach-ch = /bits/ 8 <0x05>;
> >> > - };
> >> > -
> >> > - fan@6 {
> >> > - reg = <0x02>;
> >> > - aspeed,fan-tach-ch = /bits/ 8 <0x06>;
> >> > - };
> >> > -
> >> > - fan@7 {
> >> > - reg = <0x03>;
> >> > - aspeed,fan-tach-ch = /bits/ 8 <0x07>;
> >> > - };
> >> > - };
> >> > };
> >> >
> >> > &fmc {
> >> > @@ -299,3 +248,55 @@
> >> > &wdt2 {
> >> > status = "okay";
> >> > };
> >> > +
> >> > +&pwm_tacho {
> >> > + status = "okay";
> >> > + pinctrl-names = "default";
> >> > + pinctrl-0 = <&pinctrl_pwm0_default &pinctrl_pwm1_default
> >> > + &pinctrl_pwm2_default
> >> > +&pinctrl_pwm3_default>;
> >> > +
> >> > + tach-channels {
> >> > + #address-cells = <1>;
> >> > + #size-cells = <0>;
> >> > +
> >> > + fan@0 {
> >> > + reg = <0x00>;
> >> > + aspeed,fan-tach-ch = /bits/ 8 <0x00>;
> >> > + };
> >> > +
> >> > + fan@1 {
> >> > + reg = <0x01>;
> >> > + aspeed,fan-tach-ch = /bits/ 8 <0x01>;
> >> > + };
> >> > +
> >> > + fan@2 {
> >> > + reg = <0x02>;
> >> > + aspeed,fan-tach-ch = /bits/ 8 <0x02>;
> >> > + };
> >> > +
> >> > + fan@3 {
> >> > + reg = <0x03>;
> >> > + aspeed,fan-tach-ch = /bits/ 8 <0x03>;
> >> > + };
> >> > +
> >> > + fan@4 {
> >> > + reg = <0x00>;
> >> > + aspeed,fan-tach-ch = /bits/ 8 <0x04>;
> >> > + };
> >> > +
> >> > + fan@5 {
> >> > + reg = <0x01>;
> >> > + aspeed,fan-tach-ch = /bits/ 8 <0x05>;
> >> > + };
> >> > +
> >> > + fan@6 {
> >> > + reg = <0x02>;
> >> > + aspeed,fan-tach-ch = /bits/ 8 <0x06>;
> >> > + };
> >> > +
> >> > + fan@7 {
> >> > + reg = <0x03>;
> >> > + aspeed,fan-tach-ch = /bits/ 8 <0x07>;
> >> > + };
> >> > + };
> >> > +};
> >> > diff --git a/arch/arm/boot/dts/aspeed-g4.dtsi
> >> > b/arch/arm/boot/dts/aspeed-g4.dtsi
> >> > index 9cc959f..c69ad77 100644
> >> > --- a/arch/arm/boot/dts/aspeed-g4.dtsi
> >> > +++ b/arch/arm/boot/dts/aspeed-g4.dtsi
> >> > @@ -318,7 +318,14 @@
> >> > ranges = <0 0x1e78a000 0x1000>;
> >> > };
> >> >
> >> > -
> >> > + pwm_tacho: pwm-tacho-controller@1e786000 {
> >> > + compatible = "aspeed,ast2500-pwm-tacho";
> >> > + #address-cells = <1>;
> >> > + #size-cells = <0>;
> >> > + reg = <0x1e786000 0x1000>;
> >> > + clocks = <&pwm_tacho_fixed_clk>;
> >> > + status = "disabled";
> >> > + };
> >> > };
> >> > };
> >> > };
> >> > diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi
> >> > b/arch/arm/boot/dts/aspeed-g5.dtsi
> >> > index 992242d..1ed29d9 100644
> >> > --- a/arch/arm/boot/dts/aspeed-g5.dtsi
> >> > +++ b/arch/arm/boot/dts/aspeed-g5.dtsi
> >> > @@ -35,6 +35,14 @@
> >> > i2c13 = &i2c13;
> >> > };
> >> >
> >> > + clocks {
> >> > + pwm_tacho_fixed_clk: fixedclk {
> >> > + compatible = "fixed-clock";
> >> > + #clock-cells = <0>;
> >> > + clock-frequency = <24000000>;
> >> > + };
> >> > + };
> >> > +
> >> > ahb {
> >> > compatible = "simple-bus";
> >> > #address-cells = <1>; @@ -366,6 +374,15 @@
> >> > #size-cells = <1>;
> >> > ranges = <0 0x1e78a000 0x1000>;
> >> > };
> >> > +
> >> > + pwm_tacho: pwm-tacho-controller@1e786000 {
> >> > + compatible = "aspeed,ast2500-pwm-tacho";
> >> > + #address-cells = <1>;
> >> > + #size-cells = <0>;
> >> > + reg = <0x1e786000 0x1000>;
> >> > + clocks = <&pwm_tacho_fixed_clk>;
> >> > + status = "disabled";
> >> > + };
> >> > };
> >> > };
> >> > };
> >> > diff --git a/drivers/hwmon/aspeed-pwm-tacho.c
> >> > b/drivers/hwmon/aspeed-pwm-tacho.c
> >> > index ddfe66b..eab18af 100644
> >> > --- a/drivers/hwmon/aspeed-pwm-tacho.c
> >> > +++ b/drivers/hwmon/aspeed-pwm-tacho.c
> >> > @@ -20,6 +20,7 @@
> >> > #include <linux/platform_device.h> #include <linux/sysfs.h>
> >> > #include <linux/regmap.h>
> >> > +#include <linux/thermal.h>
> >> >
> >> > /* ASPEED PWM & FAN Tach Register Definition */
> >> > #define ASPEED_PTCR_CTRL 0x00
> >> > @@ -166,6 +167,16 @@
> >> > /* How long we sleep in us while waiting for an RPM result. */
> >> > #define ASPEED_RPM_STATUS_SLEEP_USEC 500
> >> >
> >> > +struct aspeed_cooling_device {
> >> > + char name[16];
> >> > + struct aspeed_pwm_tacho_data *priv;
> >> > + struct thermal_cooling_device *tcdev;
> >> > + int pwm_port;
> >> > + u8 *cooling_levels;
> >> > + u8 max_state;
> >> > + u8 cur_state;
> >> > +};
> >> > +
> >> > struct aspeed_pwm_tacho_data {
> >> > struct regmap *regmap;
> >> > unsigned long clk_freq;
> >> > @@ -180,6 +191,7 @@ struct aspeed_pwm_tacho_data {
> >> > u8 pwm_port_type[8];
> >> > u8 pwm_port_fan_ctrl[8];
> >> > u8 fan_tach_ch_source[16];
> >> > + struct aspeed_cooling_device *cdev[8];
> >> > const struct attribute_group *groups[3]; };
> >> >
> >> > @@ -794,10 +806,111 @@ static int aspeed_create_fan(struct device
> *dev,
> >> > return 0;
> >> > }
> >> >
> >> > +static int
> >> > +aspeed_pwm_cz_get_max_state(struct thermal_cooling_device
> *tcdev,
> >> > + unsigned long *state) {
> >> > + struct aspeed_cooling_device *cdev =
> >> > + (struct aspeed_cooling_device
> >> > +*)tcdev->devdata;
> >> > +
> >> > + *state = cdev->max_state;
> >> > +
> >> > + return 0;
> >> > +}
> >> > +
> >> > +static int
> >> > +aspeed_pwm_cz_get_cur_state(struct thermal_cooling_device
> *tcdev,
> >> > + unsigned long *state) {
> >> > + struct aspeed_cooling_device *cdev =
> >> > + (struct aspeed_cooling_device
> >> > +*)tcdev->devdata;
> >> > +
> >> > + *state = cdev->cur_state;
> >> > +
> >> > + return 0;
> >> > +}
> >> > +
> >> > +static int
> >> > +aspeed_pwm_cz_set_cur_state(struct thermal_cooling_device
> *tcdev,
> >> > + unsigned long state) {
> >> > + struct aspeed_cooling_device *cdev =
> >> > + (struct aspeed_cooling_device
> >> > +*)tcdev->devdata;
> >> > +
> >> > + if (state > cdev->max_state)
> >> > + return -EINVAL;
> >> > +
> >> > + cdev->cur_state = state;
> >> > + cdev->priv->pwm_port_fan_ctrl[cdev->pwm_port] = *(cdev-
> >> >cooling_levels
> >> > + + cdev->cur_state);
> >> > + aspeed_set_pwm_port_fan_ctrl(cdev->priv, cdev->pwm_port,
> >> > + *(cdev->cooling_levels +
> >> > + cdev->cur_state));
> >> > +
> >> > + return 0;
> >> > +}
> >> > +
> >> > +static const struct thermal_cooling_device_ops
> aspeed_pwm_cool_ops
> >> > +=
> >> {
> >> > + .get_max_state = aspeed_pwm_cz_get_max_state,
> >> > + .get_cur_state = aspeed_pwm_cz_get_cur_state,
> >> > + .set_cur_state = aspeed_pwm_cz_set_cur_state, };
> >> > +
> >> > +static int aspeed_create_pwm(struct device *dev,
> >> > + struct device_node *child,
> >> > + struct aspeed_pwm_tacho_data *priv)
> >>
> >> For this, I would recommend just renaming to
> >> aspeed_create_pwm_cooling
> >>
> >> > +{
> >> > + u32 pwm_port;
> >> > + int ret;
> >> > +
> >> > + ret = of_property_read_u32(child, "reg", &pwm_port);
> >> > + if (ret)
> >> > + return ret;
> >> > +
> >> > + ret = of_property_count_u8_elems(child, "cooling-levels");
> >> > + if (ret <= 0) {
> >> > + dev_err(dev, "Wrong cooling-levels data.\n");
> >> > + return -EINVAL;
> >> > + }
> >> > +
> >> > + priv->cdev[pwm_port] = devm_kzalloc(dev, sizeof(*priv-
> >> >cdev[pwm_port]),
> >> > + GFP_KERNEL);
> >> > + if (!priv->cdev[pwm_port])
> >> > + return -ENOMEM;
> >> > +
> >> > + priv->cdev[pwm_port]->cooling_levels = devm_kzalloc(dev, ret *
> >> > + sizeof(u8),
> >> > + GFP_KERNEL);
> >> > + if (!priv->cdev[pwm_port]->cooling_levels)
> >> > + return -ENOMEM;
> >> > +
> >> > + priv->cdev[pwm_port]->max_state = ret - 1;
> >> > + ret = of_property_read_u8_array(child, "cooling-levels",
> >> > + priv->cdev[pwm_port]->cooling_levels,
> >> > + ret);
> >> > + if (ret) {
> >> > + dev_err(dev, "Property 'cooling-levels' cannot be read.\n");
> >> > + return ret;
> >> > + }
> >> > +
> >> > + sprintf(priv->cdev[pwm_port]->name, "%s%d", child->name,
> >> pwm_port);
> >> > + priv->cdev[pwm_port]->tcdev =
> >> thermal_of_cooling_device_register(child,
> >> > + priv->cdev[pwm_port]->name,
> >> > + priv->cdev[pwm_port],
> >> > + &aspeed_pwm_cool_ops);
> >> > + if (PTR_ERR_OR_ZERO(priv->cdev[pwm_port]->tcdev))
> >> > + return PTR_ERR(priv->cdev[pwm_port]->tcdev);
> >> > +
> >> > + priv->cdev[pwm_port]->priv = priv;
> >> > + priv->cdev[pwm_port]->pwm_port = pwm_port;
> >> > +
> >> > + return 0;
> >> > +}
> >> > +
> >> > static int aspeed_pwm_tacho_probe(struct platform_device *pdev) {
> >> > struct device *dev = &pdev->dev;
> >> > - struct device_node *np, *child;
> >> > + struct device_node *np, *child, *grandchild;
> >> > struct aspeed_pwm_tacho_data *priv;
> >> > void __iomem *regs;
> >> > struct resource *res;
> >> > @@ -833,10 +946,28 @@ static int aspeed_pwm_tacho_probe(struct
> >> platform_device *pdev)
> >> > aspeed_create_type(priv);
> >> >
> >> > for_each_child_of_node(np, child) {
> >> > - ret = aspeed_create_fan(dev, child, priv);
> >> > + if (!of_node_cmp(child->name, "tach-channels")) {
> >> > + for_each_child_of_node(child, grandchild) {
> >> > + ret = aspeed_create_fan(dev, grandchild, priv);
> >> > + of_node_put(grandchild);
> >> > + if (ret) {
> >> > + of_node_put(child);
> >> > + return ret;
> >> > + }
> >> > + }
> >> > + }
> >> > +
> >> > + if (!of_node_cmp(child->name, "pwm-channels")) {
> >> > + for_each_child_of_node(child, grandchild) {
> >> > + ret = aspeed_create_pwm(dev, grandchild, priv);
> >> > + of_node_put(grandchild);
> >> > + if (ret) {
> >> > + of_node_put(child);
> >> > + return ret;
> >> > + }
> >> > + }
> >> > + }
> >> > of_node_put(child);
> >> > - if (ret)
> >> > - return ret;
> >> > }
> >> >
> >> > priv->groups[0] = &pwm_dev_group;
> >> > --
> >> > 2.8.4
> >> >
> >>
> >> Presumably it's been tested and won't impact users who don't
> >> configure this behaviour, so I don't have a lot of feedback about
> functionality.
> >
> > Hi Patrick,
> >
> > Thanks for review.
> >
> > If it's not configured, cooling device is not created and of course there is no
> impact of functionality.
> > If cooling device is created, but not bound to any thermal device there is
> also no impact.
> >
> > We are using it along with kernel step wise thermal algorithm which is
> supposed to set up or down pwm on crossing thermal zone boundary to the
> relevant cooling level and to perform PWM throttling according to
> temperature trend.
> > It also can be enabled/disabled through sysfs.
> >
> > Cheers,
> > Vadim.
>
> Fantastic. I approve this change. Have you sent this driver patch upstream to
> hwmon maintainers?
Not yet, we'll apply your comment and then send to openbmc and to hwmon.
>
> Patrick
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH linux dev-4.10] hwmon: (aspeed-pwm-tacho) cooling device support
2017-07-04 13:56 [PATCH linux dev-4.10] hwmon: (aspeed-pwm-tacho) cooling device support Mykola Kostenok
2017-07-05 19:42 ` Patrick Venture
@ 2017-07-06 15:57 ` Patrick Williams
2017-07-07 8:27 ` Mykola Kostenok
1 sibling, 1 reply; 7+ messages in thread
From: Patrick Williams @ 2017-07-06 15:57 UTC (permalink / raw)
To: Mykola Kostenok; +Cc: Joel Stanley, openbmc, Patrick Venture
[-- Attachment #1: Type: text/plain, Size: 1571 bytes --]
On Tue, Jul 04, 2017 at 04:56:08PM +0300, Mykola Kostenok wrote:
> Add support in aspeed-pwm-tacho driver for cooling device creation.
> This cooling device could be bound to a thermal zone
> for the thermal control. Device will appear in /sys/class/thermal
> folder as cooling_deviceX. Then it could be bound to particular
> thermal zones. Allow specification of the cooling levels
> vector - PWM duty cycle values in a range from 0 to 255
> which correspond to thermal cooling states.
>
> Add basic pwm-tacho-controller node to ast-g5 dtsi.
> Add basic pwm-tacho-controller node to ast-g4 dtsi.
>
> Change pwm-tacho in aspeed-bmc-quanta-q71l.dts according new
> pwm-tacho structure.
>
> Add pwm-tacho to aspeed-bmc-mellanox-msn.dts.
>
> Signed-off-by: Mykola Kostenok <c_mykolak@mellanox.com>
> ---
> arch/arm/boot/dts/aspeed-bmc-mellanox-msn.dts | 61 +++++++++++
> arch/arm/boot/dts/aspeed-bmc-quanta-q71l.dts | 103 +++++++++----------
> arch/arm/boot/dts/aspeed-g4.dtsi | 9 +-
> arch/arm/boot/dts/aspeed-g5.dtsi | 17 ++++
> drivers/hwmon/aspeed-pwm-tacho.c | 139 +++++++++++++++++++++++++-
This commit is both modifying a set of device trees and adding support
to the hwmon driver. Don't we need these as two separate commits? The
hwmon driver changes should go upstream first to get feedback from the
hwmon subsystem owner and then we can backport them into the current
OpenBMC kernel.
https://github.com/openbmc/docs/blob/master/kernel-development.md
--
Patrick Williams
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH linux dev-4.10] hwmon: (aspeed-pwm-tacho) cooling device support
2017-07-06 15:57 ` Patrick Williams
@ 2017-07-07 8:27 ` Mykola Kostenok
0 siblings, 0 replies; 7+ messages in thread
From: Mykola Kostenok @ 2017-07-07 8:27 UTC (permalink / raw)
To: Patrick Williams; +Cc: Joel Stanley, openbmc, Patrick Venture, Vadim Pasternak
Hi Patrick,
Ok I'll send driver patch to upstream.
And when it will be accepted. I will send separate patches for driver and dts to openbmc.
Thanks for your help.
Best regards. Mykola Kostenok.
> -----Original Message-----
> From: Patrick Williams [mailto:patrick@stwcx.xyz]
> Sent: Thursday, July 6, 2017 6:57 PM
> To: Mykola Kostenok <c_mykolak@mellanox.com>
> Cc: Joel Stanley <joel@jms.id.au>; openbmc@lists.ozlabs.org; Patrick
> Venture <venture@google.com>
> Subject: Re: [PATCH linux dev-4.10] hwmon: (aspeed-pwm-tacho) cooling
> device support
>
> On Tue, Jul 04, 2017 at 04:56:08PM +0300, Mykola Kostenok wrote:
> > Add support in aspeed-pwm-tacho driver for cooling device creation.
> > This cooling device could be bound to a thermal zone for the thermal
> > control. Device will appear in /sys/class/thermal folder as
> > cooling_deviceX. Then it could be bound to particular thermal zones.
> > Allow specification of the cooling levels vector - PWM duty cycle
> > values in a range from 0 to 255 which correspond to thermal cooling
> > states.
> >
> > Add basic pwm-tacho-controller node to ast-g5 dtsi.
> > Add basic pwm-tacho-controller node to ast-g4 dtsi.
> >
> > Change pwm-tacho in aspeed-bmc-quanta-q71l.dts according new pwm-
> tacho
> > structure.
> >
> > Add pwm-tacho to aspeed-bmc-mellanox-msn.dts.
> >
> > Signed-off-by: Mykola Kostenok <c_mykolak@mellanox.com>
> > ---
> > arch/arm/boot/dts/aspeed-bmc-mellanox-msn.dts | 61 +++++++++++
> > arch/arm/boot/dts/aspeed-bmc-quanta-q71l.dts | 103 +++++++++---------
> -
> > arch/arm/boot/dts/aspeed-g4.dtsi | 9 +-
> > arch/arm/boot/dts/aspeed-g5.dtsi | 17 ++++
> > drivers/hwmon/aspeed-pwm-tacho.c | 139
> +++++++++++++++++++++++++-
>
> This commit is both modifying a set of device trees and adding support to the
> hwmon driver. Don't we need these as two separate commits? The hwmon
> driver changes should go upstream first to get feedback from the hwmon
> subsystem owner and then we can backport them into the current
> OpenBMC kernel.
>
> https://github.com/openbmc/docs/blob/master/kernel-development.md
>
> --
> Patrick Williams
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-07-07 8:27 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-04 13:56 [PATCH linux dev-4.10] hwmon: (aspeed-pwm-tacho) cooling device support Mykola Kostenok
2017-07-05 19:42 ` Patrick Venture
2017-07-05 20:10 ` Vadim Pasternak
2017-07-05 20:49 ` Patrick Venture
2017-07-05 20:58 ` Vadim Pasternak
2017-07-06 15:57 ` Patrick Williams
2017-07-07 8:27 ` Mykola Kostenok
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.