[PATCH linux dev-4.10] hwmon: (aspeed-pwm-tacho) cooling device support

Patrick Venture venture at google.com
Thu Jul 6 05:42:06 AEST 2017


On Tue, Jul 4, 2017 at 6:56 AM, Mykola Kostenok <c_mykolak at 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 at 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 at 0 {
> +                       reg = <0x00>;
> +                       aspeed,fan-tach-ch = /bits/ 8 <0x00>;
> +               };
> +
> +               fan at 1 {
> +                       reg = <0x00>;
> +                       aspeed,fan-tach-ch = /bits/ 8 <0x01>;
> +               };
> +
> +               fan at 2 {
> +                       reg = <0x00>;
> +                       aspeed,fan-tach-ch = /bits/ 8 <0x02>;
> +               };
> +
> +               fan at 3 {
> +                       reg = <0x00>;
> +                       aspeed,fan-tach-ch = /bits/ 8 <0x03>;
> +               };
> +
> +               fan at 4 {
> +                       reg = <0x00>;
> +                       aspeed,fan-tach-ch = /bits/ 8 <0x04>;
> +               };
> +
> +               fan at 5 {
> +                       reg = <0x00>;
> +                       aspeed,fan-tach-ch = /bits/ 8 <0x05>;
> +               };
> +
> +               fan at 6 {
> +                       reg = <0x00>;
> +                       aspeed,fan-tach-ch = /bits/ 8 <0x06>;
> +               };
> +
> +               fan at 7 {
> +                       reg = <0x00>;
> +                       aspeed,fan-tach-ch = /bits/ 8 <0x07>;
> +               };
> +       };
> +
> +       pwm-channels {
> +               #address-cells = <1>;
> +               #size-cells = <0>;
> +               #cooling-cells = <2>;
> +
> +               pwm at 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 at 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 at 0 {
> -                       reg = <0x00>;
> -                       aspeed,fan-tach-ch = /bits/ 8 <0x00>;
> -               };
> -
> -               fan at 1 {
> -                       reg = <0x01>;
> -                       aspeed,fan-tach-ch = /bits/ 8 <0x01>;
> -               };
> -
> -               fan at 2 {
> -                       reg = <0x02>;
> -                       aspeed,fan-tach-ch = /bits/ 8 <0x02>;
> -               };
> -
> -               fan at 3 {
> -                       reg = <0x03>;
> -                       aspeed,fan-tach-ch = /bits/ 8 <0x03>;
> -               };
> -
> -               fan at 4 {
> -                       reg = <0x00>;
> -                       aspeed,fan-tach-ch = /bits/ 8 <0x04>;
> -               };
> -
> -               fan at 5 {
> -                       reg = <0x01>;
> -                       aspeed,fan-tach-ch = /bits/ 8 <0x05>;
> -               };
> -
> -               fan at 6 {
> -                       reg = <0x02>;
> -                       aspeed,fan-tach-ch = /bits/ 8 <0x06>;
> -               };
> -
> -               fan at 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 at 0 {
> +                       reg = <0x00>;
> +                       aspeed,fan-tach-ch = /bits/ 8 <0x00>;
> +               };
> +
> +               fan at 1 {
> +                       reg = <0x01>;
> +                       aspeed,fan-tach-ch = /bits/ 8 <0x01>;
> +               };
> +
> +               fan at 2 {
> +                       reg = <0x02>;
> +                       aspeed,fan-tach-ch = /bits/ 8 <0x02>;
> +               };
> +
> +               fan at 3 {
> +                       reg = <0x03>;
> +                       aspeed,fan-tach-ch = /bits/ 8 <0x03>;
> +               };
> +
> +               fan at 4 {
> +                       reg = <0x00>;
> +                       aspeed,fan-tach-ch = /bits/ 8 <0x04>;
> +               };
> +
> +               fan at 5 {
> +                       reg = <0x01>;
> +                       aspeed,fan-tach-ch = /bits/ 8 <0x05>;
> +               };
> +
> +               fan at 6 {
> +                       reg = <0x02>;
> +                       aspeed,fan-tach-ch = /bits/ 8 <0x06>;
> +               };
> +
> +               fan at 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 at 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 at 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.


More information about the openbmc mailing list