2020-12-09 08:07:47

by Troy Lee

[permalink] [raw]
Subject: [PATCH 0/4] hwmon: aspeed2600-pwm-tacho: Add driver support

Aspeed AST2600 is a server management SoC which has 16 PWM channels and
16 fan tacho channel.

This series of patch provides AST2600 PWM/Fan tacho support in hwmon class.

The driver provides a sysfs interface, and user can configure PWM duty cycle
and read current FAN speed in RPM.

Troy Lee (4):
dt-bindings: hwmon: Add Aspeed AST2600 PWM/Fan
ARM: dts: aspeed: Add Aspeed AST2600 PWM/Fan node in devicetree
hwmon: Add Aspeed AST2600 support
hwmon: Support Aspeed AST2600 PWM/Fan tachometer

.../bindings/hwmon/aspeed2600-pwm-tacho.txt | 69 ++
Documentation/hwmon/aspeed_pwm_tachometer.rst | 24 +
Documentation/hwmon/index.rst | 1 +
arch/arm/boot/dts/aspeed-ast2600-evb.dts | 149 +++
arch/arm/boot/dts/aspeed-g6.dtsi | 10 +
drivers/hwmon/Kconfig | 10 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/aspeed2600-pwm-tacho.c | 1053 +++++++++++++++++
8 files changed, 1317 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hwmon/aspeed2600-pwm-tacho.txt
create mode 100644 Documentation/hwmon/aspeed_pwm_tachometer.rst
create mode 100644 drivers/hwmon/aspeed2600-pwm-tacho.c

--
2.17.1


2020-12-09 08:13:55

by Troy Lee

[permalink] [raw]
Subject: [PATCH 1/4] dt-bindings: hwmon: Add Aspeed AST2600 PWM/Fan

For supporting a new AST2600 PWM/Fan hwmon driver, we add a new binding.

Signed-off-by: Troy Lee <[email protected]>
---
.../bindings/hwmon/aspeed2600-pwm-tacho.txt | 69 +++++++++++++++++++
1 file changed, 69 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hwmon/aspeed2600-pwm-tacho.txt

diff --git a/Documentation/devicetree/bindings/hwmon/aspeed2600-pwm-tacho.txt b/Documentation/devicetree/bindings/hwmon/aspeed2600-pwm-tacho.txt
new file mode 100644
index 000000000000..61b11914352f
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/aspeed2600-pwm-tacho.txt
@@ -0,0 +1,69 @@
+ASPEED AST2600 PWM and Fan Tacho controller device driver
+
+The ASPEED PWM controller can support upto 16 PWM outputs. The ASPEED Fan Tacho
+controller can support upto 16 Fan tachometer inputs.
+
+There can be upto 16 fans supported. Each fan can have one PWM output and
+one Fan tach inputs.
+
+Required properties for pwm-tacho node:
+- #address-cells : should be 1.
+
+- #size-cells : should be 0.
+
+- #cooling-cells: should be 2.
+
+- reg : address and length of the register set for the device.
+
+- pinctrl-names : a pinctrl state named "default" must be defined.
+
+- pinctrl-0 : phandle referencing pin configuration of the PWM ports.
+
+- compatible : should be "aspeed,ast2600-pwm-tachometer".
+
+- clocks : phandle to clock provider with the clock number in the second cell
+
+- resets : phandle to reset controller with the reset number in the second cell
+
+fan subnode format:
+===================
+Under fan subnode there can upto 16 child nodes, with each child node
+representing a fan. There are 16 fans each fan can have one PWM port and one
+Fan tach inputs.
+For PWM port can be configured cooling-levels to create cooling device.
+Cooling device could be bound to a thermal zone for the thermal control.
+
+Required properties for each child node:
+- reg : should specify PWM source port.
+ integer value in the range 0x00 to 0x0f with 0x00 indicating PWM port 0
+ and 0x0f indicating PWM port F.
+
+- cooling-levels: PWM duty cycle values in a range from 0 to 255
+ which correspond to thermal cooling states.
+
+- aspeed,fan-tach-ch : should specify the Fan tach input channel.
+ integer value in the range 0 through 15, with 0 indicating
+ Fan tach channel 0 and 15 indicating Fan tach channel 15.
+ Atleast one Fan tach input channel is required.
+
+- aspeed,target-pwm : Specify the frequency of PWM. The value range from 24 to
+ 780000. Default value will be set to 25000.
+
+- aspeed,pulse-pr : Specify tacho pulse per revolution of the fan. A general
+ parameter of pulse-pr is 2.
+
+Examples:
+
+&pwm_tacho {
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_pwm0_default &pinctrl_tach0_default>;
+
+ fan@0 {
+ reg = <0x00>;
+ aspeed,target-pwm = <25000>;
+ cooling-levels = /bits/ 8 <125 151 177 203 229 255>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x00>;
+ aspeed,pulse-pr = <2>;
+ };
+};
--
2.17.1

2020-12-09 08:14:07

by Troy Lee

[permalink] [raw]
Subject: [PATCH 2/4] ARM: dts: aspeed: Add Aspeed AST2600 PWM/Fan node in devicetree

Create a common node in aspeed-g6.dtsi and add fan nodes for ast2600-evb
dts file.

Signed-off-by: Troy Lee <[email protected]>
---
arch/arm/boot/dts/aspeed-ast2600-evb.dts | 149 +++++++++++++++++++++++
arch/arm/boot/dts/aspeed-g6.dtsi | 10 ++
2 files changed, 159 insertions(+)

diff --git a/arch/arm/boot/dts/aspeed-ast2600-evb.dts b/arch/arm/boot/dts/aspeed-ast2600-evb.dts
index 89be13197780..51e00cf60749 100644
--- a/arch/arm/boot/dts/aspeed-ast2600-evb.dts
+++ b/arch/arm/boot/dts/aspeed-ast2600-evb.dts
@@ -23,6 +23,155 @@
};
};

+&pwm_tacho {
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_pwm0_default &pinctrl_tach0_default
+ &pinctrl_pwm1_default &pinctrl_tach1_default
+ &pinctrl_pwm2_default &pinctrl_tach2_default
+ &pinctrl_pwm3_default &pinctrl_tach3_default
+ &pinctrl_pwm4_default &pinctrl_tach4_default
+ &pinctrl_pwm5_default &pinctrl_tach5_default
+ &pinctrl_pwm6_default &pinctrl_tach6_default
+ &pinctrl_pwm7_default &pinctrl_tach7_default
+ &pinctrl_pwm8g1_default &pinctrl_tach8_default
+ &pinctrl_pwm9g1_default &pinctrl_tach9_default
+ &pinctrl_pwm10g1_default &pinctrl_tach10_default
+ &pinctrl_pwm11g1_default &pinctrl_tach11_default
+ &pinctrl_pwm12g1_default &pinctrl_tach12_default
+ &pinctrl_pwm13g1_default &pinctrl_tach13_default
+ &pinctrl_pwm14g1_default &pinctrl_tach14_default
+ &pinctrl_pwm15g1_default &pinctrl_tach15_default>;
+
+ fan@0 {
+ reg = <0x00>;
+ aspeed,pwm-freq = <25000>;
+ cooling-levels = /bits/ 8 <125 151 177 203 229 255>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x00>;
+ aspeed,pulse-pr = <2>;
+ };
+
+ fan@1 {
+ reg = <0x01>;
+ aspeed,pwm-freq = <25000>;
+ cooling-levels = /bits/ 8 <125 151 177 203 229 255>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x01>;
+ aspeed,pulse-pr = <2>;
+ };
+
+ fan@2 {
+ reg = <0x02>;
+ aspeed,pwm-freq = <25000>;
+ cooling-levels = /bits/ 8 <125 151 177 203 229 255>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x02>;
+ aspeed,pulse-pr = <2>;
+ };
+
+ fan@3 {
+ reg = <0x03>;
+ aspeed,pwm-freq = <25000>;
+ cooling-levels = /bits/ 8 <125 151 177 203 229 255>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x03>;
+ aspeed,pulse-pr = <2>;
+ };
+
+ fan@4 {
+ reg = <0x04>;
+ aspeed,pwm-freq = <25000>;
+ cooling-levels = /bits/ 8 <125 151 177 203 229 255>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x04>;
+ aspeed,pulse-pr = <2>;
+ };
+
+ fan@5 {
+ reg = <0x05>;
+ aspeed,pwm-freq = <25000>;
+ cooling-levels = /bits/ 8 <125 151 177 203 229 255>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x05>;
+ aspeed,pulse-pr = <2>;
+ };
+
+ fan@6 {
+ reg = <0x06>;
+ aspeed,pwm-freq = <25000>;
+ cooling-levels = /bits/ 8 <125 151 177 203 229 255>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x06>;
+ aspeed,pulse-pr = <2>;
+ };
+
+ fan@7 {
+ reg = <0x07>;
+ aspeed,pwm-freq = <25000>;
+ cooling-levels = /bits/ 8 <125 151 177 203 229 255>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x07>;
+ aspeed,pulse-pr = <2>;
+ };
+
+ fan@8 {
+ reg = <0x08>;
+ aspeed,pwm-freq = <25000>;
+ cooling-levels = /bits/ 8 <125 151 177 203 229 255>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x08>;
+ aspeed,pulse-pr = <2>;
+ };
+
+ fan@9 {
+ reg = <0x09>;
+ aspeed,pwm-freq = <25000>;
+ cooling-levels = /bits/ 8 <125 151 177 203 229 255>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x09>;
+ aspeed,pulse-pr = <2>;
+ };
+
+ fan@10 {
+ reg = <0x0a>;
+ aspeed,pwm-freq = <25000>;
+ cooling-levels = /bits/ 8 <125 151 177 203 229 255>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x0a>;
+ aspeed,pulse-pr = <2>;
+ };
+
+ fan@11 {
+ reg = <0x0b>;
+ aspeed,pwm-freq = <25000>;
+ cooling-levels = /bits/ 8 <125 151 177 203 229 255>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x0b>;
+ aspeed,pulse-pr = <2>;
+ };
+
+ fan@12 {
+ reg = <0x0c>;
+ aspeed,pwm-freq = <25000>;
+ cooling-levels = /bits/ 8 <125 151 177 203 229 255>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x0c>;
+ aspeed,pulse-pr = <2>;
+ };
+
+ fan@13 {
+ reg = <0x0d>;
+ aspeed,pwm-freq = <25000>;
+ cooling-levels = /bits/ 8 <125 151 177 203 229 255>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x0d>;
+ aspeed,pulse-pr = <2>;
+ };
+
+ fan@14 {
+ reg = <0x0e>;
+ aspeed,pwm-freq = <25000>;
+ cooling-levels = /bits/ 8 <125 151 177 203 229 255>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x0e>;
+ aspeed,pulse-pr = <2>;
+ };
+
+ fan@15 {
+ reg = <0x0f>;
+ aspeed,pwm-freq = <25000>;
+ cooling-levels = /bits/ 8 <125 151 177 203 229 255>;
+ aspeed,fan-tach-ch = /bits/ 8 <0x0f>;
+ aspeed,pulse-pr = <2>;
+ };
+};
+
&mdio0 {
status = "okay";

diff --git a/arch/arm/boot/dts/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed-g6.dtsi
index 97ca743363d7..7687d37bba26 100644
--- a/arch/arm/boot/dts/aspeed-g6.dtsi
+++ b/arch/arm/boot/dts/aspeed-g6.dtsi
@@ -298,6 +298,16 @@
#size-cells = <1>;
ranges;

+ pwm_tacho: pwm-tacho-controller@1e610000 {
+ compatible = "aspeed,ast2600-pwm-tachometer";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0x1e610000 0x100>;
+ clocks = <&syscon ASPEED_CLK_AHB>;
+ resets = <&syscon ASPEED_RESET_PWM>;
+ status = "disabled";
+ };
+
syscon: syscon@1e6e2000 {
compatible = "aspeed,ast2600-scu", "syscon", "simple-mfd";
reg = <0x1e6e2000 0x1000>;
--
2.17.1

2020-12-09 08:14:41

by Troy Lee

[permalink] [raw]
Subject: [PATCH 3/4] hwmon: Add Aspeed AST2600 support

Updating index.rst and adding aspeed_pwm_tachometer.rst to address the
driver.

Signed-off-by: Troy Lee <[email protected]>
---
Documentation/hwmon/aspeed_pwm_tachometer.rst | 24 +++++++++++++++++++
Documentation/hwmon/index.rst | 1 +
2 files changed, 25 insertions(+)
create mode 100644 Documentation/hwmon/aspeed_pwm_tachometer.rst

diff --git a/Documentation/hwmon/aspeed_pwm_tachometer.rst b/Documentation/hwmon/aspeed_pwm_tachometer.rst
new file mode 100644
index 000000000000..301448002e6e
--- /dev/null
+++ b/Documentation/hwmon/aspeed_pwm_tachometer.rst
@@ -0,0 +1,24 @@
+Kernel driver aspeed_pwm_tachometer
+===================================
+
+Supported chips:
+ ASPEED AST2600
+
+Authors:
+ Ryan Chen <[email protected]>
+
+Description:
+------------
+This driver implements support for ASPEED AST2600 PWM and Fan Tacho
+controller. The PWM controller supports upto 16 PWM outputs. The Fan tacho
+controller supports up to 16 tachometer inputs.
+
+The driver provides the following sensor accesses in sysfs:
+
+=============== ======= =====================================================
+fanX_input ro provide current fan rotation value in RPM as reported
+ by the fan to the device.
+
+pwmX rw get or set PWM fan control value. This is an integer
+ value between 0(off) and 255(full speed).
+=============== ======= =====================================================
diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
index 89e1a824021f..26d277c1d211 100644
--- a/Documentation/hwmon/index.rst
+++ b/Documentation/hwmon/index.rst
@@ -43,6 +43,7 @@ Hardware Monitoring Kernel Drivers
asb100
asc7621
aspeed-pwm-tacho
+ aspeed_pwm_tachometer
bcm54140
bel-pfe
bt1-pvt
--
2.17.1

2020-12-09 23:00:47

by Troy Lee

[permalink] [raw]
Subject: [PATCH 4/4] hwmon: Support Aspeed AST2600 PWM/Fan tachometer

Add Aspeed AST2600 PWM/Fan tacho driver. AST2600 has 16 PWM channel and
16 FAN tacho channel.

Signed-off-by: Troy Lee <[email protected]>
---
drivers/hwmon/Kconfig | 10 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/aspeed2600-pwm-tacho.c | 1053 ++++++++++++++++++++++++++
3 files changed, 1064 insertions(+)
create mode 100644 drivers/hwmon/aspeed2600-pwm-tacho.c

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 9aa89d7d4193..097c01430259 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -400,6 +400,16 @@ config SENSORS_ASPEED
This driver can also be built as a module. If so, the module
will be called aspeed_pwm_tacho.

+config SENSORS_ASPEED2600_PWM_TACHO
+ tristate "ASPEED AST2600 PWM and Fan Tachometer"
+ depends on THERMAL || THERMAL=n
+ help
+ This driver provides support for ASPEED AST2600 PWM
+ and Fan Tacho controllers.
+
+ This driver can also be built as a module. If so, the module
+ will be called aspeed2600-pwm-tacho.
+
config SENSORS_ATXP1
tristate "Attansic ATXP1 VID controller"
depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index ae41ee71a71b..10be45768d36 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -52,6 +52,7 @@ obj-$(CONFIG_SENSORS_ARM_SCPI) += scpi-hwmon.o
obj-$(CONFIG_SENSORS_AS370) += as370-hwmon.o
obj-$(CONFIG_SENSORS_ASC7621) += asc7621.o
obj-$(CONFIG_SENSORS_ASPEED) += aspeed-pwm-tacho.o
+obj-$(CONFIG_SENSORS_ASPEED2600_PWM_TACHO) += aspeed2600-pwm-tacho.o
obj-$(CONFIG_SENSORS_ATXP1) += atxp1.o
obj-$(CONFIG_SENSORS_AXI_FAN_CONTROL) += axi-fan-control.o
obj-$(CONFIG_SENSORS_BT1_PVT) += bt1-pvt.o
diff --git a/drivers/hwmon/aspeed2600-pwm-tacho.c b/drivers/hwmon/aspeed2600-pwm-tacho.c
new file mode 100644
index 000000000000..083eb3b253ff
--- /dev/null
+++ b/drivers/hwmon/aspeed2600-pwm-tacho.c
@@ -0,0 +1,1053 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) ASPEED Technology Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 or later as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/clk.h>
+#include <linux/errno.h>
+#include <linux/gpio/consumer.h>
+#include <linux/delay.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/sysfs.h>
+#include <linux/reset.h>
+#include <linux/regmap.h>
+#include <linux/thermal.h>
+/**********************************************************
+ * PWM HW register offset define
+ *********************************************************/
+//PWM Control Register
+#define ASPEED_PWM_CTRL_CH(ch) ((ch * 0x10) + 0x00)
+//PWM Duty Cycle Register
+#define ASPEED_PWM_DUTY_CYCLE_CH(ch) ((ch * 0x10) + 0x04)
+//TACH Control Register
+#define ASPEED_TACHO_CTRL_CH(ch) ((ch * 0x10) + 0x08)
+//TACH Status Register
+#define ASPEED_TACHO_STS_CH(x) ((x * 0x10) + 0x0C)
+/**********************************************************
+ * PWM register Bit field
+ *********************************************************/
+/*PWM_CTRL */
+#define PWM_LOAD_SEL_AS_WDT_BIT (19) //load selection as WDT
+#define PWM_DUTY_LOAD_AS_WDT_EN BIT(18) //enable PWM duty load as WDT
+#define PWM_DUTY_SYNC_DIS BIT(17) //disable PWM duty sync
+#define PWM_CLK_ENABLE BIT(16) //enable PWM clock
+#define PWM_LEVEL_OUTPUT BIT(15) //output PWM level
+#define PWM_INVERSE BIT(14) //inverse PWM pin
+#define PWM_OPEN_DRAIN_EN BIT(13) //enable open-drain
+#define PWM_PIN_EN BIT(12) //enable PWM pin
+#define PWM_CLK_DIV_H_MASK (0xf << 8) //PWM clock division H bit [3:0]
+#define PWM_CLK_DIV_L_MASK (0xff) //PWM clock division H bit [3:0]
+/* [19] */
+#define LOAD_SEL_FALLING 0
+#define LOAD_SEL_RIGING 1
+
+/*PWM_DUTY_CYCLE */
+#define PWM_PERIOD_BIT (24) //pwm period bit [7:0]
+#define PWM_PERIOD_BIT_MASK (0xff << 24) //pwm period bit [7:0]
+#define PWM_RISING_FALLING_AS_WDT_BIT (16)
+#define PWM_RISING_FALLING_AS_WDT_MASK (0xff << 16) //pwm rising/falling point bit [7:0] as WDT
+#define PWM_RISING_FALLING_MASK (0xffff)
+#define PWM_FALLING_POINT_BIT (8) //pwm falling point bit [7:0]
+#define PWM_RISING_POINT_BIT (0) //pwm rising point bit [7:0]
+/* [31:24] */
+#define DEFAULT_PWM_PERIOD 0xff
+
+/*PWM_TACHO_CTRL */
+#define TACHO_IER BIT(31) //enable tacho interrupt
+#define TACHO_INVERS_LIMIT BIT(30) //inverse tacho limit comparison
+#define TACHO_LOOPBACK BIT(29) //tacho loopback
+#define TACHO_ENABLE BIT(28) //{enable tacho}
+#define TACHO_DEBOUNCE_MASK (0x3 << 26) //{tacho de-bounce}
+#define TACHO_DEBOUNCE_BIT (26) //{tacho de-bounce}
+#define TECHIO_EDGE_MASK (0x3 << 24) //tacho edge}
+#define TECHIO_EDGE_BIT (24) //tacho edge}
+#define TACHO_CLK_DIV_T_MASK (0xf << 20)
+#define TACHO_CLK_DIV_BIT (20)
+#define TACHO_THRESHOLD_MASK (0xfffff) //tacho threshold bit
+/* [27:26] */
+#define DEBOUNCE_3_CLK 0x00 /* 10b */
+#define DEBOUNCE_2_CLK 0x01 /* 10b */
+#define DEBOUNCE_1_CLK 0x02 /* 10b */
+#define DEBOUNCE_0_CLK 0x03 /* 10b */
+/* [25:24] */
+#define F2F_EDGES 0x00 /* 10b */
+#define R2R_EDGES 0x01 /* 10b */
+#define BOTH_EDGES 0x02 /* 10b */
+/* [23:20] */
+/* Cover rpm range 5~5859375 */
+#define DEFAULT_TACHO_DIV 5
+
+/*PWM_TACHO_STS */
+#define TACHO_ISR BIT(31) //interrupt status and clear
+#define PWM_OUT BIT(25) //{pwm_out}
+#define PWM_OEN BIT(24) //{pwm_oeN}
+#define TACHO_DEB_INPUT BIT(23) //tacho deB input
+#define TACHO_RAW_INPUT BIT(22) //tacho raw input}
+#define TACHO_VALUE_UPDATE BIT(21) //tacho value updated since the last read
+#define TACHO_FULL_MEASUREMENT BIT(20) //{tacho full measurement}
+#define TACHO_VALUE_MASK 0xfffff //tacho value bit [19:0]}
+/**********************************************************
+ * Software setting
+ *********************************************************/
+#define DEFAULT_TARGET_PWM_FREQ 25000
+#define DEFAULT_FAN_PULSE_PR 2
+#define MAX_CDEV_NAME_LEN 16
+
+struct aspeed_pwm_channel_params {
+ int target_freq;
+ int pwm_freq;
+ int load_wdt_rising_falling_pt;
+ int load_wdt_selection; //0: rising , 1: falling
+ int load_wdt_enable;
+ int duty_sync_enable;
+ int invert_pin;
+ u8 rising;
+ u8 falling;
+};
+
+static struct aspeed_pwm_channel_params default_pwm_params[] = {
+ [0] = {
+ .target_freq = 25000,
+ .load_wdt_rising_falling_pt = 0x10,
+ .load_wdt_selection = LOAD_SEL_FALLING,
+ .load_wdt_enable = 1,
+ .duty_sync_enable = 0,
+ .invert_pin = 0,
+ .rising = 0x00,
+ .falling = 0x0a,
+ },
+ [1] = {
+ .target_freq = 25000,
+ .load_wdt_rising_falling_pt = 0x10,
+ .load_wdt_selection = LOAD_SEL_FALLING,
+ .load_wdt_enable = 0,
+ .duty_sync_enable = 0,
+ .invert_pin = 0,
+ .rising = 0x00,
+ .falling = 0x0a,
+ },
+ [2] = {
+ .target_freq = 25000,
+ .load_wdt_rising_falling_pt = 0x10,
+ .load_wdt_selection = LOAD_SEL_FALLING,
+ .load_wdt_enable = 0,
+ .duty_sync_enable = 0,
+ .invert_pin = 0,
+ .rising = 0x00,
+ .falling = 0x0a,
+ },
+ [3] = {
+ .target_freq = 25000,
+ .load_wdt_rising_falling_pt = 0x10,
+ .load_wdt_selection = LOAD_SEL_FALLING,
+ .load_wdt_enable = 0,
+ .duty_sync_enable = 0,
+ .invert_pin = 0,
+ .rising = 0x00,
+ .falling = 0x0a,
+ },
+ [4] = {
+ .target_freq = 25000,
+ .load_wdt_rising_falling_pt = 0x10,
+ .load_wdt_selection = LOAD_SEL_FALLING,
+ .load_wdt_enable = 0,
+ .duty_sync_enable = 0,
+ .invert_pin = 0,
+ .rising = 0x00,
+ .falling = 0x0a,
+ },
+ [5] = {
+ .target_freq = 25000,
+ .load_wdt_rising_falling_pt = 0x10,
+ .load_wdt_selection = LOAD_SEL_FALLING,
+ .load_wdt_enable = 0,
+ .duty_sync_enable = 0,
+ .invert_pin = 0,
+ .rising = 0x00,
+ .falling = 0x0a,
+ },
+ [6] = {
+ .target_freq = 25000,
+ .load_wdt_rising_falling_pt = 0x10,
+ .load_wdt_selection = LOAD_SEL_FALLING,
+ .load_wdt_enable = 0,
+ .duty_sync_enable = 0,
+ .invert_pin = 0,
+ .rising = 0x00,
+ .falling = 0x0a,
+ },
+ [7] = {
+ .target_freq = 25000,
+ .load_wdt_rising_falling_pt = 0x10,
+ .load_wdt_selection = LOAD_SEL_FALLING,
+ .load_wdt_enable = 0,
+ .duty_sync_enable = 0,
+ .invert_pin = 0,
+ .rising = 0x00,
+ .falling = 0x0a,
+ },
+ [8] = {
+ .target_freq = 25000,
+ .load_wdt_rising_falling_pt = 0x10,
+ .load_wdt_selection = LOAD_SEL_FALLING,
+ .load_wdt_enable = 0,
+ .duty_sync_enable = 0,
+ .invert_pin = 0,
+ .rising = 0x00,
+ .falling = 0x0a,
+ },
+ [9] = {
+ .target_freq = 25000,
+ .load_wdt_rising_falling_pt = 0x10,
+ .load_wdt_selection = LOAD_SEL_FALLING,
+ .load_wdt_enable = 0,
+ .duty_sync_enable = 0,
+ .invert_pin = 0,
+ .rising = 0x00,
+ .falling = 0x0a,
+ },
+ [10] = {
+ .target_freq = 25000,
+ .load_wdt_rising_falling_pt = 0x10,
+ .load_wdt_selection = LOAD_SEL_FALLING,
+ .load_wdt_enable = 0,
+ .duty_sync_enable = 0,
+ .invert_pin = 0,
+ .rising = 0x00,
+ .falling = 0x0a,
+ },
+ [11] = {
+ .target_freq = 25000,
+ .load_wdt_rising_falling_pt = 0x10,
+ .load_wdt_selection = LOAD_SEL_FALLING,
+ .load_wdt_enable = 0,
+ .duty_sync_enable = 0,
+ .invert_pin = 0,
+ .rising = 0x00,
+ .falling = 0x0a,
+ },
+ [12] = {
+ .target_freq = 25000,
+ .load_wdt_rising_falling_pt = 0x10,
+ .load_wdt_selection = LOAD_SEL_FALLING,
+ .load_wdt_enable = 0,
+ .duty_sync_enable = 0,
+ .invert_pin = 0,
+ .rising = 0x00,
+ .falling = 0x0a,
+ },
+ [13] = {
+ .target_freq = 25000,
+ .load_wdt_rising_falling_pt = 0x10,
+ .load_wdt_selection = LOAD_SEL_FALLING,
+ .load_wdt_enable = 0,
+ .duty_sync_enable = 0,
+ .invert_pin = 0,
+ .rising = 0x00,
+ .falling = 0x0a,
+ },
+ [14] = {
+ .target_freq = 25000,
+ .load_wdt_rising_falling_pt = 0x10,
+ .load_wdt_selection = LOAD_SEL_FALLING,
+ .load_wdt_enable = 0,
+ .duty_sync_enable = 0,
+ .invert_pin = 0,
+ .rising = 0x00,
+ .falling = 0x0a,
+ },
+ [15] = {
+ .target_freq = 25000,
+ .load_wdt_rising_falling_pt = 0x10,
+ .load_wdt_selection = LOAD_SEL_FALLING,
+ .load_wdt_enable = 0,
+ .duty_sync_enable = 0,
+ .invert_pin = 0,
+ .rising = 0x00,
+ .falling = 0x0a,
+ },
+};
+
+struct aspeed_tacho_channel_params {
+ int limited_inverse;
+ u16 threshold;
+ u8 tacho_edge;
+ u8 tacho_debounce;
+ u8 pulse_pr;
+ u32 divide;
+};
+
+
+static struct aspeed_tacho_channel_params default_tacho_params[] = {
+ [0] = {
+ .limited_inverse = 0,
+ .threshold = 0,
+ .tacho_edge = F2F_EDGES,
+ .tacho_debounce = DEBOUNCE_3_CLK,
+ .pulse_pr = DEFAULT_FAN_PULSE_PR,
+ .divide = 8,
+ },
+ [1] = {
+ .limited_inverse = 0,
+ .threshold = 0,
+ .tacho_edge = F2F_EDGES,
+ .tacho_debounce = DEBOUNCE_3_CLK,
+ .pulse_pr = DEFAULT_FAN_PULSE_PR,
+ .divide = 8,
+ },
+ [2] = {
+ .limited_inverse = 0,
+ .threshold = 0,
+ .tacho_edge = F2F_EDGES,
+ .tacho_debounce = DEBOUNCE_3_CLK,
+ .pulse_pr = DEFAULT_FAN_PULSE_PR,
+ .divide = 8,
+ },
+ [3] = {
+ .limited_inverse = 0,
+ .threshold = 0,
+ .tacho_edge = F2F_EDGES,
+ .tacho_debounce = DEBOUNCE_3_CLK,
+ .pulse_pr = DEFAULT_FAN_PULSE_PR,
+ .divide = 8,
+ },
+ [4] = {
+ .limited_inverse = 0,
+ .threshold = 0,
+ .tacho_edge = F2F_EDGES,
+ .tacho_debounce = DEBOUNCE_3_CLK,
+ .pulse_pr = DEFAULT_FAN_PULSE_PR,
+ .divide = 8,
+ },
+ [5] = {
+ .limited_inverse = 0,
+ .threshold = 0,
+ .tacho_edge = F2F_EDGES,
+ .tacho_debounce = DEBOUNCE_3_CLK,
+ .pulse_pr = DEFAULT_FAN_PULSE_PR,
+ .divide = 8,
+ },
+ [6] = {
+ .limited_inverse = 0,
+ .threshold = 0,
+ .tacho_edge = F2F_EDGES,
+ .tacho_debounce = DEBOUNCE_3_CLK,
+ .pulse_pr = DEFAULT_FAN_PULSE_PR,
+ .divide = 8,
+ },
+ [7] = {
+ .limited_inverse = 0,
+ .threshold = 0,
+ .tacho_edge = F2F_EDGES,
+ .tacho_debounce = DEBOUNCE_3_CLK,
+ .pulse_pr = DEFAULT_FAN_PULSE_PR,
+ .divide = 8,
+ },
+ [8] = {
+ .limited_inverse = 0,
+ .threshold = 0,
+ .tacho_edge = F2F_EDGES,
+ .tacho_debounce = DEBOUNCE_3_CLK,
+ .pulse_pr = DEFAULT_FAN_PULSE_PR,
+ .divide = 8,
+ },
+ [9] = {
+ .limited_inverse = 0,
+ .threshold = 0,
+ .tacho_edge = F2F_EDGES,
+ .tacho_debounce = DEBOUNCE_3_CLK,
+ .pulse_pr = DEFAULT_FAN_PULSE_PR,
+ .divide = 8,
+ },
+ [10] = {
+ .limited_inverse = 0,
+ .threshold = 0,
+ .tacho_edge = F2F_EDGES,
+ .tacho_debounce = DEBOUNCE_3_CLK,
+ .pulse_pr = DEFAULT_FAN_PULSE_PR,
+ .divide = 8,
+ },
+ [11] = {
+ .limited_inverse = 0,
+ .threshold = 0,
+ .tacho_edge = F2F_EDGES,
+ .tacho_debounce = DEBOUNCE_3_CLK,
+ .pulse_pr = DEFAULT_FAN_PULSE_PR,
+ .divide = 8,
+ },
+ [12] = {
+ .limited_inverse = 0,
+ .threshold = 0,
+ .tacho_edge = F2F_EDGES,
+ .tacho_debounce = DEBOUNCE_3_CLK,
+ .pulse_pr = DEFAULT_FAN_PULSE_PR,
+ .divide = 8,
+ },
+ [13] = {
+ .limited_inverse = 0,
+ .threshold = 0,
+ .tacho_edge = F2F_EDGES,
+ .tacho_debounce = DEBOUNCE_3_CLK,
+ .pulse_pr = DEFAULT_FAN_PULSE_PR,
+ .divide = 8,
+ },
+ [14] = {
+ .limited_inverse = 0,
+ .threshold = 0,
+ .tacho_edge = F2F_EDGES,
+ .tacho_debounce = DEBOUNCE_3_CLK,
+ .pulse_pr = DEFAULT_FAN_PULSE_PR,
+ .divide = 8,
+ },
+ [15] = {
+ .limited_inverse = 0,
+ .threshold = 0,
+ .tacho_edge = F2F_EDGES,
+ .tacho_debounce = DEBOUNCE_3_CLK,
+ .pulse_pr = DEFAULT_FAN_PULSE_PR,
+ .divide = 8,
+ },
+};
+
+struct aspeed_pwm_tachometer_data {
+ struct regmap *regmap;
+ unsigned long clk_freq;
+ struct reset_control *reset;
+ bool pwm_present[16];
+ bool fan_tach_present[16];
+ struct aspeed_pwm_channel_params *pwm_channel;
+ struct aspeed_tacho_channel_params *tacho_channel;
+ /* for thermal */
+ struct aspeed_cooling_device *cdev[8];
+ /* for hwmon */
+ const struct attribute_group *groups[3];
+};
+
+struct aspeed_cooling_device {
+ char name[16];
+ struct aspeed_pwm_tachometer_data *priv;
+ struct thermal_cooling_device *tcdev;
+ int pwm_channel;
+ u8 *cooling_levels;
+ u8 max_state;
+ u8 cur_state;
+};
+
+static int regmap_aspeed_pwm_tachometer_reg_write(void *context, unsigned int reg,
+ unsigned int val)
+{
+ void __iomem *regs = (void __iomem *)context;
+
+ writel(val, regs + reg);
+ return 0;
+}
+
+static int regmap_aspeed_pwm_tachometer_reg_read(void *context, unsigned int reg,
+ unsigned int *val)
+{
+ void __iomem *regs = (void __iomem *)context;
+
+ *val = readl(regs + reg);
+ return 0;
+}
+
+static const struct regmap_config aspeed_pwm_tachometer_regmap_config = {
+ .reg_bits = 32,
+ .val_bits = 32,
+ .reg_stride = 4,
+ .max_register = 0x100,
+ .reg_write = regmap_aspeed_pwm_tachometer_reg_write,
+ .reg_read = regmap_aspeed_pwm_tachometer_reg_read,
+ .fast_io = true,
+};
+
+static void aspeed_set_pwm_channel_enable(struct regmap *regmap, u8 pwm_channel,
+ bool enable)
+{
+ regmap_update_bits(regmap, ASPEED_PWM_CTRL_CH(pwm_channel),
+ (PWM_CLK_ENABLE | PWM_PIN_EN),
+ enable ? (PWM_CLK_ENABLE | PWM_PIN_EN) : 0);
+}
+
+static void aspeed_set_fan_tach_ch_enable(struct aspeed_pwm_tachometer_data *priv, u8 fan_tach_ch,
+ bool enable, u32 tacho_div)
+{
+ u32 reg_value = 0;
+
+ if (enable) {
+ /* divide = 2^(tacho_div*2) */
+ priv->tacho_channel[fan_tach_ch].divide = 1 << (tacho_div << 1);
+
+ reg_value = TACHO_ENABLE |
+ (priv->tacho_channel[fan_tach_ch].tacho_edge << TECHIO_EDGE_BIT) |
+ (tacho_div << TACHO_CLK_DIV_BIT) |
+ (priv->tacho_channel[fan_tach_ch].tacho_debounce << TACHO_DEBOUNCE_BIT);
+
+ if (priv->tacho_channel[fan_tach_ch].limited_inverse)
+ reg_value |= TACHO_INVERS_LIMIT;
+
+ if (priv->tacho_channel[fan_tach_ch].threshold)
+ reg_value |= (TACHO_IER | priv->tacho_channel[fan_tach_ch].threshold);
+
+ regmap_write(priv->regmap, ASPEED_TACHO_CTRL_CH(fan_tach_ch), reg_value);
+ } else
+ regmap_update_bits(priv->regmap, ASPEED_TACHO_CTRL_CH(fan_tach_ch), TACHO_ENABLE, 0);
+}
+
+/*
+ * The PWM frequency = HCLK(200Mhz) / (clock division L bit *
+ * clock division H bit * (period bit + 1))
+ */
+static void aspeed_set_pwm_channel_fan_ctrl(struct device *dev,
+ struct aspeed_pwm_tachometer_data *priv,
+ u8 index, u8 fan_ctrl)
+{
+ u32 duty_value, ctrl_value;
+ u32 div_h, div_l, cal_freq;
+ u8 div_found;
+
+ if (fan_ctrl == 0) {
+ aspeed_set_pwm_channel_enable(priv->regmap, index, false);
+ } else {
+ cal_freq = priv->clk_freq / (DEFAULT_PWM_PERIOD + 1);
+ //calculate for target frequence
+ div_found = 0;
+ for (div_h = 0; div_h < 0x10; div_h++) {
+ for (div_l = 0; div_l < 0x100; div_l++) {
+ dev_dbg(dev, "div h %x, l : %x , freq %ld \n", div_h, div_l,
+ (cal_freq / (BIT(div_h) * (div_l + 1))));
+ if ((cal_freq / (BIT(div_h) * (div_l + 1))) < priv->pwm_channel[index].target_freq) {
+ div_found = 1;
+ break;
+ }
+ }
+ if (div_found)
+ break;
+ }
+
+ priv->pwm_channel[index].pwm_freq = cal_freq / (BIT(div_h) * (div_l + 1));
+ dev_dbg(dev, "div h %x, l : %x pwm out clk %d \n", div_h, div_l,
+ priv->pwm_channel[index].pwm_freq);
+ dev_dbg(dev, "hclk %ld, target pwm freq %d, real pwm freq %d\n", priv->clk_freq,
+ priv->pwm_channel[index].target_freq, priv->pwm_channel[index].pwm_freq);
+
+ ctrl_value = (div_h << 8) | div_l;
+
+ duty_value = (DEFAULT_PWM_PERIOD << PWM_PERIOD_BIT) |
+ (0 << PWM_RISING_POINT_BIT) | (fan_ctrl << PWM_FALLING_POINT_BIT);
+
+ if (priv->pwm_channel[index].load_wdt_enable) {
+ ctrl_value |= PWM_DUTY_LOAD_AS_WDT_EN;
+ ctrl_value |= priv->pwm_channel[index].load_wdt_selection << PWM_LOAD_SEL_AS_WDT_BIT;
+ duty_value |= (priv->pwm_channel[index].load_wdt_rising_falling_pt << PWM_RISING_FALLING_AS_WDT_BIT);
+ }
+
+ regmap_write(priv->regmap, ASPEED_PWM_DUTY_CYCLE_CH(index), duty_value);
+ regmap_write(priv->regmap, ASPEED_PWM_CTRL_CH(index), ctrl_value);
+
+ aspeed_set_pwm_channel_enable(priv->regmap, index, true);
+ }
+}
+
+static int aspeed_get_fan_tach_ch_rpm(struct device *dev, struct aspeed_pwm_tachometer_data *priv,
+ u8 fan_tach_ch)
+{
+ u32 raw_data, tach_div, clk_source, val;
+ int i, retries = 3;
+
+ for (i = 0; i < retries; i++) {
+ regmap_read(priv->regmap, ASPEED_TACHO_STS_CH(fan_tach_ch), &val);
+ if (TACHO_FULL_MEASUREMENT & val)
+ break;
+ }
+
+ raw_data = val & TACHO_VALUE_MASK;
+ if (raw_data == 0xfffff)
+ return 0;
+
+ raw_data += 1;
+
+ /*
+ * We need the mode to determine if the raw_data is double (from
+ * counting both edges).
+ */
+ tach_div = raw_data * (priv->tacho_channel[fan_tach_ch].divide) * (priv->tacho_channel[fan_tach_ch].pulse_pr);
+
+ dev_dbg(dev, "clk %ld, raw_data %d , tach_div %d \n", priv->clk_freq, raw_data, tach_div);
+ clk_source = priv->clk_freq;
+
+ if (raw_data == 0)
+ return 0;
+
+ return ((clk_source / tach_div) * 60);
+
+}
+
+static ssize_t set_pwm(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr);
+ int index = sensor_attr->index;
+ int ret;
+ struct aspeed_pwm_tachometer_data *priv = dev_get_drvdata(dev);
+ long fan_ctrl;
+ u8 org_falling = priv->pwm_channel[index].falling;
+
+ ret = kstrtol(buf, 10, &fan_ctrl);
+ if (ret != 0)
+ return ret;
+
+ if (fan_ctrl < 0 || fan_ctrl > DEFAULT_PWM_PERIOD)
+ return -EINVAL;
+
+ if (priv->pwm_channel[index].falling == fan_ctrl)
+ return count;
+
+ priv->pwm_channel[index].falling = fan_ctrl;
+
+ if (fan_ctrl == 0)
+ aspeed_set_pwm_channel_enable(priv->regmap, index, false);
+ else {
+ if (fan_ctrl == DEFAULT_PWM_PERIOD)
+ regmap_update_bits(priv->regmap,
+ ASPEED_PWM_DUTY_CYCLE_CH(index),
+ GENMASK(15, 0), 0);
+ else
+ regmap_update_bits(priv->regmap,
+ ASPEED_PWM_DUTY_CYCLE_CH(index),
+ GENMASK(15, 8),
+ (fan_ctrl << PWM_FALLING_POINT_BIT));
+ }
+
+ if (org_falling == 0)
+ aspeed_set_pwm_channel_enable(priv->regmap, index, true);
+
+ return count;
+}
+
+static ssize_t show_pwm(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr);
+ int index = sensor_attr->index;
+ struct aspeed_pwm_tachometer_data *priv = dev_get_drvdata(dev);
+
+ return sprintf(buf, "%u\n", priv->pwm_channel[index].falling);
+}
+
+static ssize_t show_rpm(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr);
+ int index = sensor_attr->index;
+ int rpm;
+ struct aspeed_pwm_tachometer_data *priv = dev_get_drvdata(dev);
+
+ rpm = aspeed_get_fan_tach_ch_rpm(dev, priv, index);
+ if (rpm < 0)
+ return rpm;
+
+ return sprintf(buf, "%d\n", rpm);
+}
+
+static umode_t pwm_is_visible(struct kobject *kobj,
+ struct attribute *a, int index)
+{
+ struct device *dev = container_of(kobj, struct device, kobj);
+ struct aspeed_pwm_tachometer_data *priv = dev_get_drvdata(dev);
+
+ if (!priv->pwm_present[index])
+ return 0;
+ return a->mode;
+}
+
+static umode_t fan_dev_is_visible(struct kobject *kobj,
+ struct attribute *a, int index)
+{
+ struct device *dev = container_of(kobj, struct device, kobj);
+ struct aspeed_pwm_tachometer_data *priv = dev_get_drvdata(dev);
+
+ if (!priv->fan_tach_present[index])
+ return 0;
+ return a->mode;
+}
+
+static SENSOR_DEVICE_ATTR(pwm0, 0644,
+ show_pwm, set_pwm, 0);
+static SENSOR_DEVICE_ATTR(pwm1, 0644,
+ show_pwm, set_pwm, 1);
+static SENSOR_DEVICE_ATTR(pwm2, 0644,
+ show_pwm, set_pwm, 2);
+static SENSOR_DEVICE_ATTR(pwm3, 0644,
+ show_pwm, set_pwm, 3);
+static SENSOR_DEVICE_ATTR(pwm4, 0644,
+ show_pwm, set_pwm, 4);
+static SENSOR_DEVICE_ATTR(pwm5, 0644,
+ show_pwm, set_pwm, 5);
+static SENSOR_DEVICE_ATTR(pwm6, 0644,
+ show_pwm, set_pwm, 6);
+static SENSOR_DEVICE_ATTR(pwm7, 0644,
+ show_pwm, set_pwm, 7);
+static SENSOR_DEVICE_ATTR(pwm8, 0644,
+ show_pwm, set_pwm, 8);
+static SENSOR_DEVICE_ATTR(pwm9, 0644,
+ show_pwm, set_pwm, 9);
+static SENSOR_DEVICE_ATTR(pwm10, 0644,
+ show_pwm, set_pwm, 10);
+static SENSOR_DEVICE_ATTR(pwm11, 0644,
+ show_pwm, set_pwm, 11);
+static SENSOR_DEVICE_ATTR(pwm12, 0644,
+ show_pwm, set_pwm, 12);
+static SENSOR_DEVICE_ATTR(pwm13, 0644,
+ show_pwm, set_pwm, 13);
+static SENSOR_DEVICE_ATTR(pwm14, 0644,
+ show_pwm, set_pwm, 14);
+static SENSOR_DEVICE_ATTR(pwm15, 0644,
+ show_pwm, set_pwm, 15);
+static struct attribute *pwm_dev_attrs[] = {
+ &sensor_dev_attr_pwm0.dev_attr.attr,
+ &sensor_dev_attr_pwm1.dev_attr.attr,
+ &sensor_dev_attr_pwm2.dev_attr.attr,
+ &sensor_dev_attr_pwm3.dev_attr.attr,
+ &sensor_dev_attr_pwm4.dev_attr.attr,
+ &sensor_dev_attr_pwm5.dev_attr.attr,
+ &sensor_dev_attr_pwm6.dev_attr.attr,
+ &sensor_dev_attr_pwm7.dev_attr.attr,
+ &sensor_dev_attr_pwm8.dev_attr.attr,
+ &sensor_dev_attr_pwm9.dev_attr.attr,
+ &sensor_dev_attr_pwm10.dev_attr.attr,
+ &sensor_dev_attr_pwm11.dev_attr.attr,
+ &sensor_dev_attr_pwm12.dev_attr.attr,
+ &sensor_dev_attr_pwm13.dev_attr.attr,
+ &sensor_dev_attr_pwm14.dev_attr.attr,
+ &sensor_dev_attr_pwm15.dev_attr.attr,
+ NULL,
+};
+
+static const struct attribute_group pwm_dev_group = {
+ .attrs = pwm_dev_attrs,
+ .is_visible = pwm_is_visible,
+};
+
+static SENSOR_DEVICE_ATTR(fan0_input, 0444,
+ show_rpm, NULL, 0);
+static SENSOR_DEVICE_ATTR(fan1_input, 0444,
+ show_rpm, NULL, 1);
+static SENSOR_DEVICE_ATTR(fan2_input, 0444,
+ show_rpm, NULL, 2);
+static SENSOR_DEVICE_ATTR(fan3_input, 0444,
+ show_rpm, NULL, 3);
+static SENSOR_DEVICE_ATTR(fan4_input, 0444,
+ show_rpm, NULL, 4);
+static SENSOR_DEVICE_ATTR(fan5_input, 0444,
+ show_rpm, NULL, 5);
+static SENSOR_DEVICE_ATTR(fan6_input, 0444,
+ show_rpm, NULL, 6);
+static SENSOR_DEVICE_ATTR(fan7_input, 0444,
+ show_rpm, NULL, 7);
+static SENSOR_DEVICE_ATTR(fan8_input, 0444,
+ show_rpm, NULL, 8);
+static SENSOR_DEVICE_ATTR(fan9_input, 0444,
+ show_rpm, NULL, 9);
+static SENSOR_DEVICE_ATTR(fan10_input, 0444,
+ show_rpm, NULL, 10);
+static SENSOR_DEVICE_ATTR(fan11_input, 0444,
+ show_rpm, NULL, 11);
+static SENSOR_DEVICE_ATTR(fan12_input, 0444,
+ show_rpm, NULL, 12);
+static SENSOR_DEVICE_ATTR(fan13_input, 0444,
+ show_rpm, NULL, 13);
+static SENSOR_DEVICE_ATTR(fan14_input, 0444,
+ show_rpm, NULL, 14);
+static SENSOR_DEVICE_ATTR(fan15_input, 0444,
+ show_rpm, NULL, 15);
+static struct attribute *fan_dev_attrs[] = {
+ &sensor_dev_attr_fan0_input.dev_attr.attr,
+ &sensor_dev_attr_fan1_input.dev_attr.attr,
+ &sensor_dev_attr_fan2_input.dev_attr.attr,
+ &sensor_dev_attr_fan3_input.dev_attr.attr,
+ &sensor_dev_attr_fan4_input.dev_attr.attr,
+ &sensor_dev_attr_fan5_input.dev_attr.attr,
+ &sensor_dev_attr_fan6_input.dev_attr.attr,
+ &sensor_dev_attr_fan7_input.dev_attr.attr,
+ &sensor_dev_attr_fan8_input.dev_attr.attr,
+ &sensor_dev_attr_fan9_input.dev_attr.attr,
+ &sensor_dev_attr_fan10_input.dev_attr.attr,
+ &sensor_dev_attr_fan11_input.dev_attr.attr,
+ &sensor_dev_attr_fan12_input.dev_attr.attr,
+ &sensor_dev_attr_fan13_input.dev_attr.attr,
+ &sensor_dev_attr_fan14_input.dev_attr.attr,
+ &sensor_dev_attr_fan15_input.dev_attr.attr,
+ NULL
+};
+
+static const struct attribute_group fan_dev_group = {
+ .attrs = fan_dev_attrs,
+ .is_visible = fan_dev_is_visible,
+};
+
+static void aspeed_create_pwm_channel(struct device *dev, struct aspeed_pwm_tachometer_data *priv,
+ u8 pwm_channel, u32 target_pwm_freq)
+{
+ priv->pwm_present[pwm_channel] = true;
+ priv->pwm_channel[pwm_channel].target_freq = target_pwm_freq;
+
+ //use default
+ aspeed_set_pwm_channel_fan_ctrl(dev,
+ priv,
+ pwm_channel,
+ priv->pwm_channel[pwm_channel].falling);
+}
+
+static void aspeed_create_fan_tach_channel(struct aspeed_pwm_tachometer_data *priv,
+ u8 *fan_tach_ch, int count,
+ u32 fan_pulse_pr, u32 tacho_div)
+{
+ u8 val, index;
+
+ for (val = 0; val < count; val++) {
+ index = fan_tach_ch[val];
+ priv->fan_tach_present[index] = true;
+ priv->tacho_channel[index].pulse_pr = fan_pulse_pr;
+ aspeed_set_fan_tach_ch_enable(priv, index, true, tacho_div);
+ }
+}
+
+static int
+aspeed_pwm_cz_get_max_state(struct thermal_cooling_device *tcdev,
+ unsigned long *state)
+{
+ struct aspeed_cooling_device *cdev = 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 = 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 = tcdev->devdata;
+
+ if (state > cdev->max_state)
+ return -EINVAL;
+
+ cdev->cur_state = state;
+ cdev->priv->pwm_channel[cdev->pwm_channel].falling =
+ cdev->cooling_levels[cdev->cur_state];
+ aspeed_set_pwm_channel_fan_ctrl(&tcdev->device, cdev->priv, cdev->pwm_channel,
+ 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_cooling(struct device *dev,
+ struct device_node *child,
+ struct aspeed_pwm_tachometer_data *priv,
+ u32 pwm_channel, u8 num_levels)
+{
+ int ret;
+ struct aspeed_cooling_device *cdev;
+
+ cdev = devm_kzalloc(dev, sizeof(*cdev), GFP_KERNEL);
+ if (!cdev)
+ return -ENOMEM;
+
+ cdev->cooling_levels = devm_kzalloc(dev, num_levels, GFP_KERNEL);
+ if (!cdev->cooling_levels)
+ return -ENOMEM;
+
+ cdev->max_state = num_levels - 1;
+ ret = of_property_read_u8_array(child, "cooling-levels",
+ cdev->cooling_levels,
+ num_levels);
+ if (ret) {
+ dev_err(dev, "Property 'cooling-levels' cannot be read.\n");
+ return ret;
+ }
+ snprintf(cdev->name, MAX_CDEV_NAME_LEN, "%s%d", child->name, pwm_channel);
+
+ cdev->tcdev = thermal_of_cooling_device_register(child,
+ cdev->name,
+ cdev,
+ &aspeed_pwm_cool_ops);
+ if (IS_ERR(cdev->tcdev))
+ return PTR_ERR(cdev->tcdev);
+
+ cdev->priv = priv;
+ cdev->pwm_channel = pwm_channel;
+
+ priv->cdev[pwm_channel] = cdev;
+
+ return 0;
+}
+
+static int aspeed_pwm_create_fan(struct device *dev,
+ struct device_node *child,
+ struct aspeed_pwm_tachometer_data *priv)
+{
+ u8 *fan_tach_ch;
+ u32 fan_pulse_pr;
+ u32 tacho_div;
+ u32 pwm_channel;
+ u32 target_pwm_freq = 0;
+ int ret, count;
+
+ ret = of_property_read_u32(child, "reg", &pwm_channel);
+ if (ret)
+ return ret;
+
+ ret = of_property_read_u32(child, "aspeed,pwm-freq", &target_pwm_freq);
+ if (ret)
+ target_pwm_freq = DEFAULT_TARGET_PWM_FREQ;
+
+ aspeed_create_pwm_channel(dev, priv, (u8)pwm_channel, target_pwm_freq);
+
+ ret = of_property_count_u8_elems(child, "cooling-levels");
+ if (ret > 0) {
+ if (IS_ENABLED(CONFIG_THERMAL)) {
+ ret = aspeed_create_pwm_cooling(dev, child, priv, pwm_channel,
+ ret);
+ if (ret)
+ return ret;
+ }
+ }
+
+ count = of_property_count_u8_elems(child, "aspeed,fan-tach-ch");
+ if (count < 1)
+ return -EINVAL;
+
+ fan_tach_ch = devm_kzalloc(dev, sizeof(*fan_tach_ch) * count,
+ GFP_KERNEL);
+ if (!fan_tach_ch)
+ return -ENOMEM;
+ ret = of_property_read_u8_array(child, "aspeed,fan-tach-ch",
+ fan_tach_ch, count);
+ if (ret)
+ return ret;
+
+ ret = of_property_read_u32(child, "aspeed,pulse-pr", &fan_pulse_pr);
+ if (ret)
+ fan_pulse_pr = DEFAULT_FAN_PULSE_PR;
+
+ ret = of_property_read_u32(child, "aspeed,tacho-div", &tacho_div);
+ if (ret)
+ tacho_div = DEFAULT_TACHO_DIV;
+
+ aspeed_create_fan_tach_channel(priv, fan_tach_ch, count, fan_pulse_pr, tacho_div);
+
+ return 0;
+}
+
+static int aspeed_pwm_tachometer_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct device_node *np, *child;
+ struct aspeed_pwm_tachometer_data *priv;
+ void __iomem *regs;
+ struct resource *res;
+ struct device *hwmon;
+ struct clk *clk;
+ int ret;
+
+ np = dev->of_node;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res)
+ return -ENOENT;
+ regs = devm_ioremap_resource(dev, res);
+ if (IS_ERR(regs))
+ return PTR_ERR(regs);
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->pwm_channel = default_pwm_params;
+ priv->tacho_channel = default_tacho_params;
+ priv->regmap = devm_regmap_init(dev, NULL, (__force void *)regs,
+ &aspeed_pwm_tachometer_regmap_config);
+ if (IS_ERR(priv->regmap))
+ return PTR_ERR(priv->regmap);
+
+ clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(clk))
+ return -ENODEV;
+ priv->clk_freq = clk_get_rate(clk);
+
+ priv->reset = devm_reset_control_get(&pdev->dev, NULL);
+ if (IS_ERR(priv->reset)) {
+ dev_err(&pdev->dev, "can't get aspeed_pwm_tacho reset\n");
+ return PTR_ERR(priv->reset);
+ }
+
+ //scu init
+ reset_control_assert(priv->reset);
+ reset_control_deassert(priv->reset);
+
+ for_each_child_of_node(np, child) {
+ ret = aspeed_pwm_create_fan(dev, child, priv);
+ if (ret) {
+ of_node_put(child);
+ return ret;
+ }
+ }
+
+ priv->groups[0] = &pwm_dev_group;
+ priv->groups[1] = &fan_dev_group;
+ priv->groups[2] = NULL;
+ dev_info(dev, "pwm tach probe done\n");
+ hwmon = devm_hwmon_device_register_with_groups(dev,
+ "aspeed_pwm_tachometer",
+ priv, priv->groups);
+
+ return PTR_ERR_OR_ZERO(hwmon);
+}
+
+static const struct of_device_id of_pwm_tachometer_match_table[] = {
+ { .compatible = "aspeed,ast2600-pwm-tachometer", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, of_pwm_tachometer_match_table);
+
+static struct platform_driver aspeed_pwm_tachometer_driver = {
+ .probe = aspeed_pwm_tachometer_probe,
+ .driver = {
+ .name = "aspeed_pwm_tachometer",
+ .of_match_table = of_pwm_tachometer_match_table,
+ },
+};
+
+module_platform_driver(aspeed_pwm_tachometer_driver);
+
+MODULE_AUTHOR("Ryan Chen <[email protected]>");
+MODULE_DESCRIPTION("ASPEED PWM and Fan Tachometer device driver");
+MODULE_LICENSE("GPL");
--
2.17.1

2020-12-11 16:25:53

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 1/4] dt-bindings: hwmon: Add Aspeed AST2600 PWM/Fan

On Wed, Dec 09, 2020 at 03:59:17PM +0800, Troy Lee wrote:
> For supporting a new AST2600 PWM/Fan hwmon driver, we add a new binding.
>
> Signed-off-by: Troy Lee <[email protected]>
> ---
> .../bindings/hwmon/aspeed2600-pwm-tacho.txt | 69 +++++++++++++++++++

Bindings are in DT schema format now.

> 1 file changed, 69 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/hwmon/aspeed2600-pwm-tacho.txt
>
> diff --git a/Documentation/devicetree/bindings/hwmon/aspeed2600-pwm-tacho.txt b/Documentation/devicetree/bindings/hwmon/aspeed2600-pwm-tacho.txt
> new file mode 100644
> index 000000000000..61b11914352f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/aspeed2600-pwm-tacho.txt
> @@ -0,0 +1,69 @@
> +ASPEED AST2600 PWM and Fan Tacho controller device driver
> +
> +The ASPEED PWM controller can support upto 16 PWM outputs. The ASPEED Fan Tacho
> +controller can support upto 16 Fan tachometer inputs.
> +
> +There can be upto 16 fans supported. Each fan can have one PWM output and
> +one Fan tach inputs.
> +
> +Required properties for pwm-tacho node:
> +- #address-cells : should be 1.
> +
> +- #size-cells : should be 0.
> +
> +- #cooling-cells: should be 2.
> +
> +- reg : address and length of the register set for the device.
> +
> +- pinctrl-names : a pinctrl state named "default" must be defined.
> +
> +- pinctrl-0 : phandle referencing pin configuration of the PWM ports.
> +
> +- compatible : should be "aspeed,ast2600-pwm-tachometer".
> +
> +- clocks : phandle to clock provider with the clock number in the second cell
> +
> +- resets : phandle to reset controller with the reset number in the second cell
> +
> +fan subnode format:
> +===================
> +Under fan subnode there can upto 16 child nodes, with each child node
> +representing a fan. There are 16 fans each fan can have one PWM port and one
> +Fan tach inputs.
> +For PWM port can be configured cooling-levels to create cooling device.
> +Cooling device could be bound to a thermal zone for the thermal control.
> +
> +Required properties for each child node:
> +- reg : should specify PWM source port.
> + integer value in the range 0x00 to 0x0f with 0x00 indicating PWM port 0
> + and 0x0f indicating PWM port F.
> +
> +- cooling-levels: PWM duty cycle values in a range from 0 to 255
> + which correspond to thermal cooling states.
> +
> +- aspeed,fan-tach-ch : should specify the Fan tach input channel.
> + integer value in the range 0 through 15, with 0 indicating
> + Fan tach channel 0 and 15 indicating Fan tach channel 15.
> + Atleast one Fan tach input channel is required.

Already has 'fan-tach-ch' in npcm750-pwm-fan.txt.

> +
> +- aspeed,target-pwm : Specify the frequency of PWM. The value range from 24 to
> + 780000. Default value will be set to 25000.
> +
> +- aspeed,pulse-pr : Specify tacho pulse per revolution of the fan. A general
> + parameter of pulse-pr is 2.

Already have 'pulses-per-revolution' property in pwm-fan.txt. Use that.

Really, all these should be in a common fan schema that you reference.

> +
> +Examples:
> +
> +&pwm_tacho {
> + status = "okay";

Don't show status in examples.

> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_pwm0_default &pinctrl_tach0_default>;
> +
> + fan@0 {
> + reg = <0x00>;
> + aspeed,target-pwm = <25000>;
> + cooling-levels = /bits/ 8 <125 151 177 203 229 255>;
> + aspeed,fan-tach-ch = /bits/ 8 <0x00>;
> + aspeed,pulse-pr = <2>;
> + };
> +};
> --
> 2.17.1
>

2020-12-15 09:32:06

by Troy Lee

[permalink] [raw]
Subject: Re: [PATCH 1/4] dt-bindings: hwmon: Add Aspeed AST2600 PWM/Fan

The 12/11/2020 11:26, Rob Herring wrote:
> On Wed, Dec 09, 2020 at 03:59:17PM +0800, Troy Lee wrote:
> > For supporting a new AST2600 PWM/Fan hwmon driver, we add a new binding.
> >
> > Signed-off-by: Troy Lee <[email protected]>
> > ---
> > .../bindings/hwmon/aspeed2600-pwm-tacho.txt | 69 +++++++++++++++++++
>
> Bindings are in DT schema format now.
>
I'll submit a new binding with DT schema.

> > 1 file changed, 69 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/hwmon/aspeed2600-pwm-tacho.txt
> >
> > diff --git a/Documentation/devicetree/bindings/hwmon/aspeed2600-pwm-tacho.txt b/Documentation/devicetree/bindings/hwmon/aspeed2600-pwm-tacho.txt
> > new file mode 100644
> > index 000000000000..61b11914352f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/hwmon/aspeed2600-pwm-tacho.txt
> > @@ -0,0 +1,69 @@
> > +ASPEED AST2600 PWM and Fan Tacho controller device driver
> > +
> > +The ASPEED PWM controller can support upto 16 PWM outputs. The ASPEED Fan Tacho
> > +controller can support upto 16 Fan tachometer inputs.
> > +
> > +There can be upto 16 fans supported. Each fan can have one PWM output and
> > +one Fan tach inputs.
> > +
> > +Required properties for pwm-tacho node:
> > +- #address-cells : should be 1.
> > +
> > +- #size-cells : should be 0.
> > +
> > +- #cooling-cells: should be 2.
> > +
> > +- reg : address and length of the register set for the device.
> > +
> > +- pinctrl-names : a pinctrl state named "default" must be defined.
> > +
> > +- pinctrl-0 : phandle referencing pin configuration of the PWM ports.
> > +
> > +- compatible : should be "aspeed,ast2600-pwm-tachometer".
> > +
> > +- clocks : phandle to clock provider with the clock number in the second cell
> > +
> > +- resets : phandle to reset controller with the reset number in the second cell
> > +
> > +fan subnode format:
> > +===================
> > +Under fan subnode there can upto 16 child nodes, with each child node
> > +representing a fan. There are 16 fans each fan can have one PWM port and one
> > +Fan tach inputs.
> > +For PWM port can be configured cooling-levels to create cooling device.
> > +Cooling device could be bound to a thermal zone for the thermal control.
> > +
> > +Required properties for each child node:
> > +- reg : should specify PWM source port.
> > + integer value in the range 0x00 to 0x0f with 0x00 indicating PWM port 0
> > + and 0x0f indicating PWM port F.
> > +
> > +- cooling-levels: PWM duty cycle values in a range from 0 to 255
> > + which correspond to thermal cooling states.
> > +
> > +- aspeed,fan-tach-ch : should specify the Fan tach input channel.
> > + integer value in the range 0 through 15, with 0 indicating
> > + Fan tach channel 0 and 15 indicating Fan tach channel 15.
> > + Atleast one Fan tach input channel is required.
>
> Already has 'fan-tach-ch' in npcm750-pwm-fan.txt.
>
OK.

> > +
> > +- aspeed,target-pwm : Specify the frequency of PWM. The value range from 24 to
> > + 780000. Default value will be set to 25000.
> > +
> > +- aspeed,pulse-pr : Specify tacho pulse per revolution of the fan. A general
> > + parameter of pulse-pr is 2.
>
> Already have 'pulses-per-revolution' property in pwm-fan.txt. Use that.
>
OK.

> Really, all these should be in a common fan schema that you reference.
>
Are you suggesting that I should also update these properties into
pwm-fan.txt with a separeated patch, perhaps?

> > +
> > +Examples:
> > +
> > +&pwm_tacho {
> > + status = "okay";
>
> Don't show status in examples.
>
Understood, it will be removed in v2.

> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pinctrl_pwm0_default &pinctrl_tach0_default>;
> > +
> > + fan@0 {
> > + reg = <0x00>;
> > + aspeed,target-pwm = <25000>;
> > + cooling-levels = /bits/ 8 <125 151 177 203 229 255>;
> > + aspeed,fan-tach-ch = /bits/ 8 <0x00>;
> > + aspeed,pulse-pr = <2>;
> > + };
> > +};
> > --
> > 2.17.1
> >

2020-12-15 09:51:40

by Troy Lee

[permalink] [raw]
Subject: Re: [PATCH 4/4] hwmon: Support Aspeed AST2600 PWM/Fan tachometer

The 12/11/2020 00:16, Guenter Roeck wrote:
> On Wed, Dec 09, 2020 at 03:59:20PM +0800, Troy Lee wrote:
> > Add Aspeed AST2600 PWM/Fan tacho driver. AST2600 has 16 PWM channel and
> > 16 FAN tacho channel.
> >
> > Signed-off-by: Troy Lee <[email protected]>
> > ---
> > drivers/hwmon/Kconfig | 10 +
> > drivers/hwmon/Makefile | 1 +
> > drivers/hwmon/aspeed2600-pwm-tacho.c | 1053 ++++++++++++++++++++++++++
> > 3 files changed, 1064 insertions(+)
> > create mode 100644 drivers/hwmon/aspeed2600-pwm-tacho.c
> >
> > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> > index 9aa89d7d4193..097c01430259 100644
> > --- a/drivers/hwmon/Kconfig
> > +++ b/drivers/hwmon/Kconfig
> > @@ -400,6 +400,16 @@ config SENSORS_ASPEED
> > This driver can also be built as a module. If so, the module
> > will be called aspeed_pwm_tacho.
> >
> > +config SENSORS_ASPEED2600_PWM_TACHO
> > + tristate "ASPEED AST2600 PWM and Fan Tachometer"
> > + depends on THERMAL || THERMAL=n
> > + help
> > + This driver provides support for ASPEED AST2600 PWM
> > + and Fan Tacho controllers.
> > +
> > + This driver can also be built as a module. If so, the module
> > + will be called aspeed2600-pwm-tacho.
> > +
> > config SENSORS_ATXP1
> > tristate "Attansic ATXP1 VID controller"
> > depends on I2C
> > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> > index ae41ee71a71b..10be45768d36 100644
> > --- a/drivers/hwmon/Makefile
> > +++ b/drivers/hwmon/Makefile
> > @@ -52,6 +52,7 @@ obj-$(CONFIG_SENSORS_ARM_SCPI) += scpi-hwmon.o
> > obj-$(CONFIG_SENSORS_AS370) += as370-hwmon.o
> > obj-$(CONFIG_SENSORS_ASC7621) += asc7621.o
> > obj-$(CONFIG_SENSORS_ASPEED) += aspeed-pwm-tacho.o
> > +obj-$(CONFIG_SENSORS_ASPEED2600_PWM_TACHO) += aspeed2600-pwm-tacho.o
> > obj-$(CONFIG_SENSORS_ATXP1) += atxp1.o
> > obj-$(CONFIG_SENSORS_AXI_FAN_CONTROL) += axi-fan-control.o
> > obj-$(CONFIG_SENSORS_BT1_PVT) += bt1-pvt.o
> > diff --git a/drivers/hwmon/aspeed2600-pwm-tacho.c b/drivers/hwmon/aspeed2600-pwm-tacho.c
> > new file mode 100644
> > index 000000000000..083eb3b253ff
> > --- /dev/null
> > +++ b/drivers/hwmon/aspeed2600-pwm-tacho.c
> > @@ -0,0 +1,1053 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright (C) ASPEED Technology Inc.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 or later as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/errno.h>
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/delay.h>
> > +#include <linux/hwmon.h>
> > +#include <linux/hwmon-sysfs.h>
> > +#include <linux/io.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/of_device.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/sysfs.h>
> > +#include <linux/reset.h>
> > +#include <linux/regmap.h>
> > +#include <linux/thermal.h>
> > +/**********************************************************
> > + * PWM HW register offset define
> > + *********************************************************/
> > +//PWM Control Register
>
> Please no C++ comments, and please use standard multi-line comments.
>
Understood.

> > +#define ASPEED_PWM_CTRL_CH(ch) ((ch * 0x10) + 0x00)
> > +//PWM Duty Cycle Register
> > +#define ASPEED_PWM_DUTY_CYCLE_CH(ch) ((ch * 0x10) + 0x04)
> > +//TACH Control Register
> > +#define ASPEED_TACHO_CTRL_CH(ch) ((ch * 0x10) + 0x08)
>
> (ch)
>
> > +//TACH Status Register
> > +#define ASPEED_TACHO_STS_CH(x) ((x * 0x10) + 0x0C)
>
> (x)
>
Good catch.

> > +/**********************************************************
> > + * PWM register Bit field
> > + *********************************************************/
> > +/*PWM_CTRL */
> > +#define PWM_LOAD_SEL_AS_WDT_BIT (19) //load selection as WDT
> > +#define PWM_DUTY_LOAD_AS_WDT_EN BIT(18) //enable PWM duty load as WDT
> > +#define PWM_DUTY_SYNC_DIS BIT(17) //disable PWM duty sync
> > +#define PWM_CLK_ENABLE BIT(16) //enable PWM clock
> > +#define PWM_LEVEL_OUTPUT BIT(15) //output PWM level
> > +#define PWM_INVERSE BIT(14) //inverse PWM pin
> > +#define PWM_OPEN_DRAIN_EN BIT(13) //enable open-drain
> > +#define PWM_PIN_EN BIT(12) //enable PWM pin
> > +#define PWM_CLK_DIV_H_MASK (0xf << 8) //PWM clock division H bit [3:0]
> > +#define PWM_CLK_DIV_L_MASK (0xff) //PWM clock division H bit [3:0]
> > +/* [19] */
> > +#define LOAD_SEL_FALLING 0
> > +#define LOAD_SEL_RIGING 1
> > +
> > +/*PWM_DUTY_CYCLE */
> > +#define PWM_PERIOD_BIT (24) //pwm period bit [7:0]
> > +#define PWM_PERIOD_BIT_MASK (0xff << 24) //pwm period bit [7:0]
> > +#define PWM_RISING_FALLING_AS_WDT_BIT (16)
> > +#define PWM_RISING_FALLING_AS_WDT_MASK (0xff << 16) //pwm rising/falling point bit [7:0] as WDT
> > +#define PWM_RISING_FALLING_MASK (0xffff)
> > +#define PWM_FALLING_POINT_BIT (8) //pwm falling point bit [7:0]
> > +#define PWM_RISING_POINT_BIT (0) //pwm rising point bit [7:0]
> > +/* [31:24] */
> > +#define DEFAULT_PWM_PERIOD 0xff
> > +
> > +/*PWM_TACHO_CTRL */
> > +#define TACHO_IER BIT(31) //enable tacho interrupt
> > +#define TACHO_INVERS_LIMIT BIT(30) //inverse tacho limit comparison
> > +#define TACHO_LOOPBACK BIT(29) //tacho loopback
> > +#define TACHO_ENABLE BIT(28) //{enable tacho}
> > +#define TACHO_DEBOUNCE_MASK (0x3 << 26) //{tacho de-bounce}
> > +#define TACHO_DEBOUNCE_BIT (26) //{tacho de-bounce}
> > +#define TECHIO_EDGE_MASK (0x3 << 24) //tacho edge}
> > +#define TECHIO_EDGE_BIT (24) //tacho edge}
> > +#define TACHO_CLK_DIV_T_MASK (0xf << 20)
> > +#define TACHO_CLK_DIV_BIT (20)
> > +#define TACHO_THRESHOLD_MASK (0xfffff) //tacho threshold bit
> > +/* [27:26] */
> > +#define DEBOUNCE_3_CLK 0x00 /* 10b */
> > +#define DEBOUNCE_2_CLK 0x01 /* 10b */
> > +#define DEBOUNCE_1_CLK 0x02 /* 10b */
> > +#define DEBOUNCE_0_CLK 0x03 /* 10b */
> > +/* [25:24] */
> > +#define F2F_EDGES 0x00 /* 10b */
> > +#define R2R_EDGES 0x01 /* 10b */
> > +#define BOTH_EDGES 0x02 /* 10b */
> > +/* [23:20] */
> > +/* Cover rpm range 5~5859375 */
> > +#define DEFAULT_TACHO_DIV 5
> > +
> > +/*PWM_TACHO_STS */
> > +#define TACHO_ISR BIT(31) //interrupt status and clear
> > +#define PWM_OUT BIT(25) //{pwm_out}
> > +#define PWM_OEN BIT(24) //{pwm_oeN}
> > +#define TACHO_DEB_INPUT BIT(23) //tacho deB input
> > +#define TACHO_RAW_INPUT BIT(22) //tacho raw input}
> > +#define TACHO_VALUE_UPDATE BIT(21) //tacho value updated since the last read
> > +#define TACHO_FULL_MEASUREMENT BIT(20) //{tacho full measurement}
> > +#define TACHO_VALUE_MASK 0xfffff //tacho value bit [19:0]}
> > +/**********************************************************
> > + * Software setting
> > + *********************************************************/
> > +#define DEFAULT_TARGET_PWM_FREQ 25000
> > +#define DEFAULT_FAN_PULSE_PR 2
> > +#define MAX_CDEV_NAME_LEN 16
> > +
> > +struct aspeed_pwm_channel_params {
> > + int target_freq;
> > + int pwm_freq;
> > + int load_wdt_rising_falling_pt;
> > + int load_wdt_selection; //0: rising , 1: falling
> > + int load_wdt_enable;
> > + int duty_sync_enable;
> > + int invert_pin;
> > + u8 rising;
> > + u8 falling;
> > +};
> > +
> > +static struct aspeed_pwm_channel_params default_pwm_params[] = {
> > + [0] = {
> > + .target_freq = 25000,
> > + .load_wdt_rising_falling_pt = 0x10,
> > + .load_wdt_selection = LOAD_SEL_FALLING,
> > + .load_wdt_enable = 1,
> > + .duty_sync_enable = 0,
> > + .invert_pin = 0,
> > + .rising = 0x00,
> > + .falling = 0x0a,
> > + },
>
> I am in general very much opposed to include default configurations
> in hwmon drivers. Configuration should be provided through devicetree,
> or through platform data.
>
I'll move most of these configurations into devicetree.

> > + [1] = {
> > + .target_freq = 25000,
> > + .load_wdt_rising_falling_pt = 0x10,
> > + .load_wdt_selection = LOAD_SEL_FALLING,
> > + .load_wdt_enable = 0,
> > + .duty_sync_enable = 0,
> > + .invert_pin = 0,
> > + .rising = 0x00,
> > + .falling = 0x0a,
> > + },
> > + [2] = {
> > + .target_freq = 25000,
> > + .load_wdt_rising_falling_pt = 0x10,
> > + .load_wdt_selection = LOAD_SEL_FALLING,
> > + .load_wdt_enable = 0,
> > + .duty_sync_enable = 0,
> > + .invert_pin = 0,
> > + .rising = 0x00,
> > + .falling = 0x0a,
> > + },
> > + [3] = {
> > + .target_freq = 25000,
> > + .load_wdt_rising_falling_pt = 0x10,
> > + .load_wdt_selection = LOAD_SEL_FALLING,
> > + .load_wdt_enable = 0,
> > + .duty_sync_enable = 0,
> > + .invert_pin = 0,
> > + .rising = 0x00,
> > + .falling = 0x0a,
> > + },
> > + [4] = {
> > + .target_freq = 25000,
> > + .load_wdt_rising_falling_pt = 0x10,
> > + .load_wdt_selection = LOAD_SEL_FALLING,
> > + .load_wdt_enable = 0,
> > + .duty_sync_enable = 0,
> > + .invert_pin = 0,
> > + .rising = 0x00,
> > + .falling = 0x0a,
> > + },
> > + [5] = {
> > + .target_freq = 25000,
> > + .load_wdt_rising_falling_pt = 0x10,
> > + .load_wdt_selection = LOAD_SEL_FALLING,
> > + .load_wdt_enable = 0,
> > + .duty_sync_enable = 0,
> > + .invert_pin = 0,
> > + .rising = 0x00,
> > + .falling = 0x0a,
> > + },
> > + [6] = {
> > + .target_freq = 25000,
> > + .load_wdt_rising_falling_pt = 0x10,
> > + .load_wdt_selection = LOAD_SEL_FALLING,
> > + .load_wdt_enable = 0,
> > + .duty_sync_enable = 0,
> > + .invert_pin = 0,
> > + .rising = 0x00,
> > + .falling = 0x0a,
> > + },
> > + [7] = {
> > + .target_freq = 25000,
> > + .load_wdt_rising_falling_pt = 0x10,
> > + .load_wdt_selection = LOAD_SEL_FALLING,
> > + .load_wdt_enable = 0,
> > + .duty_sync_enable = 0,
> > + .invert_pin = 0,
> > + .rising = 0x00,
> > + .falling = 0x0a,
> > + },
> > + [8] = {
> > + .target_freq = 25000,
> > + .load_wdt_rising_falling_pt = 0x10,
> > + .load_wdt_selection = LOAD_SEL_FALLING,
> > + .load_wdt_enable = 0,
> > + .duty_sync_enable = 0,
> > + .invert_pin = 0,
> > + .rising = 0x00,
> > + .falling = 0x0a,
> > + },
> > + [9] = {
> > + .target_freq = 25000,
> > + .load_wdt_rising_falling_pt = 0x10,
> > + .load_wdt_selection = LOAD_SEL_FALLING,
> > + .load_wdt_enable = 0,
> > + .duty_sync_enable = 0,
> > + .invert_pin = 0,
> > + .rising = 0x00,
> > + .falling = 0x0a,
> > + },
> > + [10] = {
> > + .target_freq = 25000,
> > + .load_wdt_rising_falling_pt = 0x10,
> > + .load_wdt_selection = LOAD_SEL_FALLING,
> > + .load_wdt_enable = 0,
> > + .duty_sync_enable = 0,
> > + .invert_pin = 0,
> > + .rising = 0x00,
> > + .falling = 0x0a,
> > + },
> > + [11] = {
> > + .target_freq = 25000,
> > + .load_wdt_rising_falling_pt = 0x10,
> > + .load_wdt_selection = LOAD_SEL_FALLING,
> > + .load_wdt_enable = 0,
> > + .duty_sync_enable = 0,
> > + .invert_pin = 0,
> > + .rising = 0x00,
> > + .falling = 0x0a,
> > + },
> > + [12] = {
> > + .target_freq = 25000,
> > + .load_wdt_rising_falling_pt = 0x10,
> > + .load_wdt_selection = LOAD_SEL_FALLING,
> > + .load_wdt_enable = 0,
> > + .duty_sync_enable = 0,
> > + .invert_pin = 0,
> > + .rising = 0x00,
> > + .falling = 0x0a,
> > + },
> > + [13] = {
> > + .target_freq = 25000,
> > + .load_wdt_rising_falling_pt = 0x10,
> > + .load_wdt_selection = LOAD_SEL_FALLING,
> > + .load_wdt_enable = 0,
> > + .duty_sync_enable = 0,
> > + .invert_pin = 0,
> > + .rising = 0x00,
> > + .falling = 0x0a,
> > + },
> > + [14] = {
> > + .target_freq = 25000,
> > + .load_wdt_rising_falling_pt = 0x10,
> > + .load_wdt_selection = LOAD_SEL_FALLING,
> > + .load_wdt_enable = 0,
> > + .duty_sync_enable = 0,
> > + .invert_pin = 0,
> > + .rising = 0x00,
> > + .falling = 0x0a,
> > + },
> > + [15] = {
> > + .target_freq = 25000,
> > + .load_wdt_rising_falling_pt = 0x10,
> > + .load_wdt_selection = LOAD_SEL_FALLING,
> > + .load_wdt_enable = 0,
> > + .duty_sync_enable = 0,
> > + .invert_pin = 0,
> > + .rising = 0x00,
> > + .falling = 0x0a,
> > + },
> > +};
> > +
> > +struct aspeed_tacho_channel_params {
> > + int limited_inverse;
> > + u16 threshold;
> > + u8 tacho_edge;
> > + u8 tacho_debounce;
> > + u8 pulse_pr;
> > + u32 divide;
> > +};
> > +
> > +
> > +static struct aspeed_tacho_channel_params default_tacho_params[] = {
> > + [0] = {
> > + .limited_inverse = 0,
> > + .threshold = 0,
> > + .tacho_edge = F2F_EDGES,
> > + .tacho_debounce = DEBOUNCE_3_CLK,
> > + .pulse_pr = DEFAULT_FAN_PULSE_PR,
> > + .divide = 8,
>
> Same as above.
>
> > + },
> > + [1] = {
> > + .limited_inverse = 0,
> > + .threshold = 0,
> > + .tacho_edge = F2F_EDGES,
> > + .tacho_debounce = DEBOUNCE_3_CLK,
> > + .pulse_pr = DEFAULT_FAN_PULSE_PR,
> > + .divide = 8,
> > + },
> > + [2] = {
> > + .limited_inverse = 0,
> > + .threshold = 0,
> > + .tacho_edge = F2F_EDGES,
> > + .tacho_debounce = DEBOUNCE_3_CLK,
> > + .pulse_pr = DEFAULT_FAN_PULSE_PR,
> > + .divide = 8,
> > + },
> > + [3] = {
> > + .limited_inverse = 0,
> > + .threshold = 0,
> > + .tacho_edge = F2F_EDGES,
> > + .tacho_debounce = DEBOUNCE_3_CLK,
> > + .pulse_pr = DEFAULT_FAN_PULSE_PR,
> > + .divide = 8,
> > + },
> > + [4] = {
> > + .limited_inverse = 0,
> > + .threshold = 0,
> > + .tacho_edge = F2F_EDGES,
> > + .tacho_debounce = DEBOUNCE_3_CLK,
> > + .pulse_pr = DEFAULT_FAN_PULSE_PR,
> > + .divide = 8,
> > + },
> > + [5] = {
> > + .limited_inverse = 0,
> > + .threshold = 0,
> > + .tacho_edge = F2F_EDGES,
> > + .tacho_debounce = DEBOUNCE_3_CLK,
> > + .pulse_pr = DEFAULT_FAN_PULSE_PR,
> > + .divide = 8,
> > + },
> > + [6] = {
> > + .limited_inverse = 0,
> > + .threshold = 0,
> > + .tacho_edge = F2F_EDGES,
> > + .tacho_debounce = DEBOUNCE_3_CLK,
> > + .pulse_pr = DEFAULT_FAN_PULSE_PR,
> > + .divide = 8,
> > + },
> > + [7] = {
> > + .limited_inverse = 0,
> > + .threshold = 0,
> > + .tacho_edge = F2F_EDGES,
> > + .tacho_debounce = DEBOUNCE_3_CLK,
> > + .pulse_pr = DEFAULT_FAN_PULSE_PR,
> > + .divide = 8,
> > + },
> > + [8] = {
> > + .limited_inverse = 0,
> > + .threshold = 0,
> > + .tacho_edge = F2F_EDGES,
> > + .tacho_debounce = DEBOUNCE_3_CLK,
> > + .pulse_pr = DEFAULT_FAN_PULSE_PR,
> > + .divide = 8,
> > + },
> > + [9] = {
> > + .limited_inverse = 0,
> > + .threshold = 0,
> > + .tacho_edge = F2F_EDGES,
> > + .tacho_debounce = DEBOUNCE_3_CLK,
> > + .pulse_pr = DEFAULT_FAN_PULSE_PR,
> > + .divide = 8,
> > + },
> > + [10] = {
> > + .limited_inverse = 0,
> > + .threshold = 0,
> > + .tacho_edge = F2F_EDGES,
> > + .tacho_debounce = DEBOUNCE_3_CLK,
> > + .pulse_pr = DEFAULT_FAN_PULSE_PR,
> > + .divide = 8,
> > + },
> > + [11] = {
> > + .limited_inverse = 0,
> > + .threshold = 0,
> > + .tacho_edge = F2F_EDGES,
> > + .tacho_debounce = DEBOUNCE_3_CLK,
> > + .pulse_pr = DEFAULT_FAN_PULSE_PR,
> > + .divide = 8,
> > + },
> > + [12] = {
> > + .limited_inverse = 0,
> > + .threshold = 0,
> > + .tacho_edge = F2F_EDGES,
> > + .tacho_debounce = DEBOUNCE_3_CLK,
> > + .pulse_pr = DEFAULT_FAN_PULSE_PR,
> > + .divide = 8,
> > + },
> > + [13] = {
> > + .limited_inverse = 0,
> > + .threshold = 0,
> > + .tacho_edge = F2F_EDGES,
> > + .tacho_debounce = DEBOUNCE_3_CLK,
> > + .pulse_pr = DEFAULT_FAN_PULSE_PR,
> > + .divide = 8,
> > + },
> > + [14] = {
> > + .limited_inverse = 0,
> > + .threshold = 0,
> > + .tacho_edge = F2F_EDGES,
> > + .tacho_debounce = DEBOUNCE_3_CLK,
> > + .pulse_pr = DEFAULT_FAN_PULSE_PR,
> > + .divide = 8,
> > + },
> > + [15] = {
> > + .limited_inverse = 0,
> > + .threshold = 0,
> > + .tacho_edge = F2F_EDGES,
> > + .tacho_debounce = DEBOUNCE_3_CLK,
> > + .pulse_pr = DEFAULT_FAN_PULSE_PR,
> > + .divide = 8,
> > + },
> > +};
> > +
> > +struct aspeed_pwm_tachometer_data {
> > + struct regmap *regmap;
> > + unsigned long clk_freq;
> > + struct reset_control *reset;
> > + bool pwm_present[16];
> > + bool fan_tach_present[16];
> > + struct aspeed_pwm_channel_params *pwm_channel;
> > + struct aspeed_tacho_channel_params *tacho_channel;
> > + /* for thermal */
> > + struct aspeed_cooling_device *cdev[8];
>
> This makes me wonder if this should be a thermal driver instead.
> Any thoughts ?
>
> > + /* for hwmon */
> > + const struct attribute_group *groups[3];
> > +};
> > +
> > +struct aspeed_cooling_device {
> > + char name[16];
> > + struct aspeed_pwm_tachometer_data *priv;
> > + struct thermal_cooling_device *tcdev;
> > + int pwm_channel;
> > + u8 *cooling_levels;
> > + u8 max_state;
> > + u8 cur_state;
> > +};
> > +
> > +static int regmap_aspeed_pwm_tachometer_reg_write(void *context, unsigned int reg,
> > + unsigned int val)
> > +{
> > + void __iomem *regs = (void __iomem *)context;
> > +
> > + writel(val, regs + reg);
> > + return 0;
> > +}
> > +
> > +static int regmap_aspeed_pwm_tachometer_reg_read(void *context, unsigned int reg,
> > + unsigned int *val)
> > +{
> > + void __iomem *regs = (void __iomem *)context;
> > +
> > + *val = readl(regs + reg);
> > + return 0;
> > +}
> > +
> > +static const struct regmap_config aspeed_pwm_tachometer_regmap_config = {
> > + .reg_bits = 32,
> > + .val_bits = 32,
> > + .reg_stride = 4,
> > + .max_register = 0x100,
> > + .reg_write = regmap_aspeed_pwm_tachometer_reg_write,
> > + .reg_read = regmap_aspeed_pwm_tachometer_reg_read,
> > + .fast_io = true,
> > +};
> > +
> > +static void aspeed_set_pwm_channel_enable(struct regmap *regmap, u8 pwm_channel,
> > + bool enable)
> > +{
> > + regmap_update_bits(regmap, ASPEED_PWM_CTRL_CH(pwm_channel),
> > + (PWM_CLK_ENABLE | PWM_PIN_EN),
> > + enable ? (PWM_CLK_ENABLE | PWM_PIN_EN) : 0);
>
> Unnecessary ()
>
OK.

> > +}
> > +
> > +static void aspeed_set_fan_tach_ch_enable(struct aspeed_pwm_tachometer_data *priv, u8 fan_tach_ch,
> > + bool enable, u32 tacho_div)
>
> This function is only called with enable == true. Please no unnecessary
> complexity.
>
> > +{
> > + u32 reg_value = 0;
>
> Unnecessary initialization.
>
> > +
> > + if (enable) {
> > + /* divide = 2^(tacho_div*2) */
> > + priv->tacho_channel[fan_tach_ch].divide = 1 << (tacho_div << 1);
> > +
> > + reg_value = TACHO_ENABLE |
> > + (priv->tacho_channel[fan_tach_ch].tacho_edge << TECHIO_EDGE_BIT) |
> > + (tacho_div << TACHO_CLK_DIV_BIT) |
> > + (priv->tacho_channel[fan_tach_ch].tacho_debounce << TACHO_DEBOUNCE_BIT);
> > +
> > + if (priv->tacho_channel[fan_tach_ch].limited_inverse)
> > + reg_value |= TACHO_INVERS_LIMIT;
> > +
> > + if (priv->tacho_channel[fan_tach_ch].threshold)
> > + reg_value |= (TACHO_IER | priv->tacho_channel[fan_tach_ch].threshold);
> > +
> > + regmap_write(priv->regmap, ASPEED_TACHO_CTRL_CH(fan_tach_ch), reg_value);
> > + } else
> > + regmap_update_bits(priv->regmap, ASPEED_TACHO_CTRL_CH(fan_tach_ch), TACHO_ENABLE, 0);
> > +}
> > +
> > +/*
> > + * The PWM frequency = HCLK(200Mhz) / (clock division L bit *
> > + * clock division H bit * (period bit + 1))
> > + */
> > +static void aspeed_set_pwm_channel_fan_ctrl(struct device *dev,
> > + struct aspeed_pwm_tachometer_data *priv,
> > + u8 index, u8 fan_ctrl)
> > +{
> > + u32 duty_value, ctrl_value;
> > + u32 div_h, div_l, cal_freq;
> > + u8 div_found;
>
> div_found is used as boolean. Declaring it u8 makes the code more complex
> on many architectures. Please use bool.
>
> > +
> > + if (fan_ctrl == 0) {
> > + aspeed_set_pwm_channel_enable(priv->regmap, index, false);
>
> Consider using return; here and drop else.
>
> > + } else {
> > + cal_freq = priv->clk_freq / (DEFAULT_PWM_PERIOD + 1);
> > + //calculate for target frequence
> > + div_found = 0;
> > + for (div_h = 0; div_h < 0x10; div_h++) {
> > + for (div_l = 0; div_l < 0x100; div_l++) {
> > + dev_dbg(dev, "div h %x, l : %x , freq %ld \n", div_h, div_l,
> > + (cal_freq / (BIT(div_h) * (div_l + 1))));
> > + if ((cal_freq / (BIT(div_h) * (div_l + 1))) < priv->pwm_channel[index].target_freq) {
> > + div_found = 1;
> > + break;
> > + }
> > + }
> > + if (div_found)
> > + break;
> > + }
>
> This double loop is quite expensive. Are yu sure there is no better means to
> determine the fan divider ? By using a binary search, maybe ?
>
> Also, what happens if div_found is false at the end ? The code below suggests
> that this would be problematic.
>
I'll change the algorithm, so it would not be double loop and remote the need
of div_found.

> > +
> > + priv->pwm_channel[index].pwm_freq = cal_freq / (BIT(div_h) * (div_l + 1));
> > + dev_dbg(dev, "div h %x, l : %x pwm out clk %d \n", div_h, div_l,
> > + priv->pwm_channel[index].pwm_freq);
> > + dev_dbg(dev, "hclk %ld, target pwm freq %d, real pwm freq %d\n", priv->clk_freq,
> > + priv->pwm_channel[index].target_freq, priv->pwm_channel[index].pwm_freq);
> > +
> > + ctrl_value = (div_h << 8) | div_l;
> > +
> > + duty_value = (DEFAULT_PWM_PERIOD << PWM_PERIOD_BIT) |
> > + (0 << PWM_RISING_POINT_BIT) | (fan_ctrl << PWM_FALLING_POINT_BIT);
> > +
> > + if (priv->pwm_channel[index].load_wdt_enable) {
> > + ctrl_value |= PWM_DUTY_LOAD_AS_WDT_EN;
> > + ctrl_value |= priv->pwm_channel[index].load_wdt_selection << PWM_LOAD_SEL_AS_WDT_BIT;
> > + duty_value |= (priv->pwm_channel[index].load_wdt_rising_falling_pt << PWM_RISING_FALLING_AS_WDT_BIT);
> > + }
> > +
> > + regmap_write(priv->regmap, ASPEED_PWM_DUTY_CYCLE_CH(index), duty_value);
> > + regmap_write(priv->regmap, ASPEED_PWM_CTRL_CH(index), ctrl_value);
> > +
> > + aspeed_set_pwm_channel_enable(priv->regmap, index, true);
> > + }
> > +}
> > +
> > +static int aspeed_get_fan_tach_ch_rpm(struct device *dev, struct aspeed_pwm_tachometer_data *priv,
> > + u8 fan_tach_ch)
> > +{
> > + u32 raw_data, tach_div, clk_source, val;
> > + int i, retries = 3;
> > +
> > + for (i = 0; i < retries; i++) {
> > + regmap_read(priv->regmap, ASPEED_TACHO_STS_CH(fan_tach_ch), &val);
> > + if (TACHO_FULL_MEASUREMENT & val)
>
> No Yoda programming please.
>
Understood.

> > + break;
> > + }
> > +
> > + raw_data = val & TACHO_VALUE_MASK;
> > + if (raw_data == 0xfffff)
> > + return 0;
> > +
> > + raw_data += 1;
> > +
> > + /*
> > + * We need the mode to determine if the raw_data is double (from
> > + * counting both edges).
> > + */
> > + tach_div = raw_data * (priv->tacho_channel[fan_tach_ch].divide) * (priv->tacho_channel[fan_tach_ch].pulse_pr);
> > +
> > + dev_dbg(dev, "clk %ld, raw_data %d , tach_div %d \n", priv->clk_freq, raw_data, tach_div);
> > + clk_source = priv->clk_freq;
> > +
> > + if (raw_data == 0)
> > + return 0;
>
> How would raw_data ever be 0 here ? And why check it after using it,
> and not before ?
>
No, it wouldn't be 0 here.

> > +
> > + return ((clk_source / tach_div) * 60);
> > +
> > +}
> > +
> > +static ssize_t set_pwm(struct device *dev, struct device_attribute *attr,
> > + const char *buf, size_t count)
> > +{
> > + struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr);
> > + int index = sensor_attr->index;
> > + int ret;
> > + struct aspeed_pwm_tachometer_data *priv = dev_get_drvdata(dev);
> > + long fan_ctrl;
> > + u8 org_falling = priv->pwm_channel[index].falling;
> > +
> > + ret = kstrtol(buf, 10, &fan_ctrl);
> > + if (ret != 0)
> > + return ret;
> > +
> > + if (fan_ctrl < 0 || fan_ctrl > DEFAULT_PWM_PERIOD)
> > + return -EINVAL;
>
> Please use kstrtoul().
>
After change to use devm_device_hwmon_register_with_info, the kstrtoul doesn't
require anymore, this it will be removed in v2.

> > +
> > + if (priv->pwm_channel[index].falling == fan_ctrl)
> > + return count;
> > +
> > + priv->pwm_channel[index].falling = fan_ctrl;
> > +
> > + if (fan_ctrl == 0)
> > + aspeed_set_pwm_channel_enable(priv->regmap, index, false);
> > + else {
> > + if (fan_ctrl == DEFAULT_PWM_PERIOD)
> > + regmap_update_bits(priv->regmap,
> > + ASPEED_PWM_DUTY_CYCLE_CH(index),
> > + GENMASK(15, 0), 0);
> > + else
> > + regmap_update_bits(priv->regmap,
> > + ASPEED_PWM_DUTY_CYCLE_CH(index),
> > + GENMASK(15, 8),
> > + (fan_ctrl << PWM_FALLING_POINT_BIT));
> > + }
> > +
> > + if (org_falling == 0)
> > + aspeed_set_pwm_channel_enable(priv->regmap, index, true);
> > +
> > + return count;
> > +}
> > +
> > +static ssize_t show_pwm(struct device *dev, struct device_attribute *attr,
> > + char *buf)
> > +{
> > + struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr);
> > + int index = sensor_attr->index;
> > + struct aspeed_pwm_tachometer_data *priv = dev_get_drvdata(dev);
> > +
> > + return sprintf(buf, "%u\n", priv->pwm_channel[index].falling);
> > +}
> > +
> > +static ssize_t show_rpm(struct device *dev, struct device_attribute *attr,
> > + char *buf)
> > +{
> > + struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr);
> > + int index = sensor_attr->index;
> > + int rpm;
> > + struct aspeed_pwm_tachometer_data *priv = dev_get_drvdata(dev);
> > +
> > + rpm = aspeed_get_fan_tach_ch_rpm(dev, priv, index);
> > + if (rpm < 0)
> > + return rpm;
> > +
> > + return sprintf(buf, "%d\n", rpm);
> > +}
> > +
> > +static umode_t pwm_is_visible(struct kobject *kobj,
> > + struct attribute *a, int index)
> > +{
> > + struct device *dev = container_of(kobj, struct device, kobj);
> > + struct aspeed_pwm_tachometer_data *priv = dev_get_drvdata(dev);
> > +
> > + if (!priv->pwm_present[index])
> > + return 0;
> > + return a->mode;
> > +}
> > +
> > +static umode_t fan_dev_is_visible(struct kobject *kobj,
> > + struct attribute *a, int index)
> > +{
> > + struct device *dev = container_of(kobj, struct device, kobj);
> > + struct aspeed_pwm_tachometer_data *priv = dev_get_drvdata(dev);
> > +
> > + if (!priv->fan_tach_present[index])
> > + return 0;
> > + return a->mode;
> > +}
> > +
> > +static SENSOR_DEVICE_ATTR(pwm0, 0644,
> > + show_pwm, set_pwm, 0);
> > +static SENSOR_DEVICE_ATTR(pwm1, 0644,
> > + show_pwm, set_pwm, 1);
> > +static SENSOR_DEVICE_ATTR(pwm2, 0644,
> > + show_pwm, set_pwm, 2);
> > +static SENSOR_DEVICE_ATTR(pwm3, 0644,
> > + show_pwm, set_pwm, 3);
> > +static SENSOR_DEVICE_ATTR(pwm4, 0644,
> > + show_pwm, set_pwm, 4);
> > +static SENSOR_DEVICE_ATTR(pwm5, 0644,
> > + show_pwm, set_pwm, 5);
> > +static SENSOR_DEVICE_ATTR(pwm6, 0644,
> > + show_pwm, set_pwm, 6);
> > +static SENSOR_DEVICE_ATTR(pwm7, 0644,
> > + show_pwm, set_pwm, 7);
> > +static SENSOR_DEVICE_ATTR(pwm8, 0644,
> > + show_pwm, set_pwm, 8);
> > +static SENSOR_DEVICE_ATTR(pwm9, 0644,
> > + show_pwm, set_pwm, 9);
> > +static SENSOR_DEVICE_ATTR(pwm10, 0644,
> > + show_pwm, set_pwm, 10);
> > +static SENSOR_DEVICE_ATTR(pwm11, 0644,
> > + show_pwm, set_pwm, 11);
> > +static SENSOR_DEVICE_ATTR(pwm12, 0644,
> > + show_pwm, set_pwm, 12);
> > +static SENSOR_DEVICE_ATTR(pwm13, 0644,
> > + show_pwm, set_pwm, 13);
> > +static SENSOR_DEVICE_ATTR(pwm14, 0644,
> > + show_pwm, set_pwm, 14);
> > +static SENSOR_DEVICE_ATTR(pwm15, 0644,
> > + show_pwm, set_pwm, 15);
> > +static struct attribute *pwm_dev_attrs[] = {
> > + &sensor_dev_attr_pwm0.dev_attr.attr,
> > + &sensor_dev_attr_pwm1.dev_attr.attr,
> > + &sensor_dev_attr_pwm2.dev_attr.attr,
> > + &sensor_dev_attr_pwm3.dev_attr.attr,
> > + &sensor_dev_attr_pwm4.dev_attr.attr,
> > + &sensor_dev_attr_pwm5.dev_attr.attr,
> > + &sensor_dev_attr_pwm6.dev_attr.attr,
> > + &sensor_dev_attr_pwm7.dev_attr.attr,
> > + &sensor_dev_attr_pwm8.dev_attr.attr,
> > + &sensor_dev_attr_pwm9.dev_attr.attr,
> > + &sensor_dev_attr_pwm10.dev_attr.attr,
> > + &sensor_dev_attr_pwm11.dev_attr.attr,
> > + &sensor_dev_attr_pwm12.dev_attr.attr,
> > + &sensor_dev_attr_pwm13.dev_attr.attr,
> > + &sensor_dev_attr_pwm14.dev_attr.attr,
> > + &sensor_dev_attr_pwm15.dev_attr.attr,
> > + NULL,
> > +};
> > +
> > +static const struct attribute_group pwm_dev_group = {
> > + .attrs = pwm_dev_attrs,
> > + .is_visible = pwm_is_visible,
> > +};
> > +
> > +static SENSOR_DEVICE_ATTR(fan0_input, 0444,
> > + show_rpm, NULL, 0);
> > +static SENSOR_DEVICE_ATTR(fan1_input, 0444,
> > + show_rpm, NULL, 1);
> > +static SENSOR_DEVICE_ATTR(fan2_input, 0444,
> > + show_rpm, NULL, 2);
> > +static SENSOR_DEVICE_ATTR(fan3_input, 0444,
> > + show_rpm, NULL, 3);
> > +static SENSOR_DEVICE_ATTR(fan4_input, 0444,
> > + show_rpm, NULL, 4);
> > +static SENSOR_DEVICE_ATTR(fan5_input, 0444,
> > + show_rpm, NULL, 5);
> > +static SENSOR_DEVICE_ATTR(fan6_input, 0444,
> > + show_rpm, NULL, 6);
> > +static SENSOR_DEVICE_ATTR(fan7_input, 0444,
> > + show_rpm, NULL, 7);
> > +static SENSOR_DEVICE_ATTR(fan8_input, 0444,
> > + show_rpm, NULL, 8);
> > +static SENSOR_DEVICE_ATTR(fan9_input, 0444,
> > + show_rpm, NULL, 9);
> > +static SENSOR_DEVICE_ATTR(fan10_input, 0444,
> > + show_rpm, NULL, 10);
> > +static SENSOR_DEVICE_ATTR(fan11_input, 0444,
> > + show_rpm, NULL, 11);
> > +static SENSOR_DEVICE_ATTR(fan12_input, 0444,
> > + show_rpm, NULL, 12);
> > +static SENSOR_DEVICE_ATTR(fan13_input, 0444,
> > + show_rpm, NULL, 13);
> > +static SENSOR_DEVICE_ATTR(fan14_input, 0444,
> > + show_rpm, NULL, 14);
> > +static SENSOR_DEVICE_ATTR(fan15_input, 0444,
> > + show_rpm, NULL, 15);
> > +static struct attribute *fan_dev_attrs[] = {
> > + &sensor_dev_attr_fan0_input.dev_attr.attr,
> > + &sensor_dev_attr_fan1_input.dev_attr.attr,
> > + &sensor_dev_attr_fan2_input.dev_attr.attr,
> > + &sensor_dev_attr_fan3_input.dev_attr.attr,
> > + &sensor_dev_attr_fan4_input.dev_attr.attr,
> > + &sensor_dev_attr_fan5_input.dev_attr.attr,
> > + &sensor_dev_attr_fan6_input.dev_attr.attr,
> > + &sensor_dev_attr_fan7_input.dev_attr.attr,
> > + &sensor_dev_attr_fan8_input.dev_attr.attr,
> > + &sensor_dev_attr_fan9_input.dev_attr.attr,
> > + &sensor_dev_attr_fan10_input.dev_attr.attr,
> > + &sensor_dev_attr_fan11_input.dev_attr.attr,
> > + &sensor_dev_attr_fan12_input.dev_attr.attr,
> > + &sensor_dev_attr_fan13_input.dev_attr.attr,
> > + &sensor_dev_attr_fan14_input.dev_attr.attr,
> > + &sensor_dev_attr_fan15_input.dev_attr.attr,
> > + NULL
> > +};
> > +
> > +static const struct attribute_group fan_dev_group = {
> > + .attrs = fan_dev_attrs,
> > + .is_visible = fan_dev_is_visible,
> > +};
> > +
> > +static void aspeed_create_pwm_channel(struct device *dev, struct aspeed_pwm_tachometer_data *priv,
> > + u8 pwm_channel, u32 target_pwm_freq)
> > +{
> > + priv->pwm_present[pwm_channel] = true;
> > + priv->pwm_channel[pwm_channel].target_freq = target_pwm_freq;
> > +
> > + //use default
> > + aspeed_set_pwm_channel_fan_ctrl(dev,
> > + priv,
> > + pwm_channel,
> > + priv->pwm_channel[pwm_channel].falling);
> > +}
> > +
> > +static void aspeed_create_fan_tach_channel(struct aspeed_pwm_tachometer_data *priv,
> > + u8 *fan_tach_ch, int count,
> > + u32 fan_pulse_pr, u32 tacho_div)
> > +{
> > + u8 val, index;
> > +
> > + for (val = 0; val < count; val++) {
> > + index = fan_tach_ch[val];
> > + priv->fan_tach_present[index] = true;
> > + priv->tacho_channel[index].pulse_pr = fan_pulse_pr;
> > + aspeed_set_fan_tach_ch_enable(priv, index, true, tacho_div);
> > + }
> > +}
> > +
> > +static int
> > +aspeed_pwm_cz_get_max_state(struct thermal_cooling_device *tcdev,
> > + unsigned long *state)
> > +{
> > + struct aspeed_cooling_device *cdev = 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 = 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 = tcdev->devdata;
> > +
> > + if (state > cdev->max_state)
> > + return -EINVAL;
> > +
> > + cdev->cur_state = state;
> > + cdev->priv->pwm_channel[cdev->pwm_channel].falling =
> > + cdev->cooling_levels[cdev->cur_state];
> > + aspeed_set_pwm_channel_fan_ctrl(&tcdev->device, cdev->priv, cdev->pwm_channel,
> > + 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_cooling(struct device *dev,
> > + struct device_node *child,
> > + struct aspeed_pwm_tachometer_data *priv,
> > + u32 pwm_channel, u8 num_levels)
> > +{
> > + int ret;
> > + struct aspeed_cooling_device *cdev;
> > +
> > + cdev = devm_kzalloc(dev, sizeof(*cdev), GFP_KERNEL);
> > + if (!cdev)
> > + return -ENOMEM;
> > +
> > + cdev->cooling_levels = devm_kzalloc(dev, num_levels, GFP_KERNEL);
> > + if (!cdev->cooling_levels)
> > + return -ENOMEM;
> > +
> > + cdev->max_state = num_levels - 1;
> > + ret = of_property_read_u8_array(child, "cooling-levels",
> > + cdev->cooling_levels,
> > + num_levels);
> > + if (ret) {
> > + dev_err(dev, "Property 'cooling-levels' cannot be read.\n");
> > + return ret;
> > + }
> > + snprintf(cdev->name, MAX_CDEV_NAME_LEN, "%s%d", child->name, pwm_channel);
> > +
> > + cdev->tcdev = thermal_of_cooling_device_register(child,
> > + cdev->name,
> > + cdev,
> > + &aspeed_pwm_cool_ops);
> > + if (IS_ERR(cdev->tcdev))
> > + return PTR_ERR(cdev->tcdev);
> > +
> > + cdev->priv = priv;
> > + cdev->pwm_channel = pwm_channel;
> > +
> > + priv->cdev[pwm_channel] = cdev;
> > +
> > + return 0;
> > +}
> > +
> > +static int aspeed_pwm_create_fan(struct device *dev,
> > + struct device_node *child,
> > + struct aspeed_pwm_tachometer_data *priv)
> > +{
> > + u8 *fan_tach_ch;
> > + u32 fan_pulse_pr;
> > + u32 tacho_div;
> > + u32 pwm_channel;
> > + u32 target_pwm_freq = 0;
> > + int ret, count;
> > +
> > + ret = of_property_read_u32(child, "reg", &pwm_channel);
> > + if (ret)
> > + return ret;
> > +
> > + ret = of_property_read_u32(child, "aspeed,pwm-freq", &target_pwm_freq);
> > + if (ret)
> > + target_pwm_freq = DEFAULT_TARGET_PWM_FREQ;
> > +
> > + aspeed_create_pwm_channel(dev, priv, (u8)pwm_channel, target_pwm_freq);
> > +
> > + ret = of_property_count_u8_elems(child, "cooling-levels");
> > + if (ret > 0) {
> > + if (IS_ENABLED(CONFIG_THERMAL)) {
> > + ret = aspeed_create_pwm_cooling(dev, child, priv, pwm_channel,
> > + ret);
> > + if (ret)
> > + return ret;
> > + }
> > + }
> > +
> > + count = of_property_count_u8_elems(child, "aspeed,fan-tach-ch");
> > + if (count < 1)
> > + return -EINVAL;
> > +
> > + fan_tach_ch = devm_kzalloc(dev, sizeof(*fan_tach_ch) * count,
> > + GFP_KERNEL);
> > + if (!fan_tach_ch)
> > + return -ENOMEM;
> > + ret = of_property_read_u8_array(child, "aspeed,fan-tach-ch",
> > + fan_tach_ch, count);
> > + if (ret)
> > + return ret;
> > +
> > + ret = of_property_read_u32(child, "aspeed,pulse-pr", &fan_pulse_pr);
> > + if (ret)
> > + fan_pulse_pr = DEFAULT_FAN_PULSE_PR;
>
> Are those properties declared as optional ?
>
I'll update the dt-bindings document and make it clearly.

> > +
> > + ret = of_property_read_u32(child, "aspeed,tacho-div", &tacho_div);
> > + if (ret)
> > + tacho_div = DEFAULT_TACHO_DIV;
> > +
> > + aspeed_create_fan_tach_channel(priv, fan_tach_ch, count, fan_pulse_pr, tacho_div);
> > +
> > + return 0;
> > +}
> > +
> > +static int aspeed_pwm_tachometer_probe(struct platform_device *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + struct device_node *np, *child;
> > + struct aspeed_pwm_tachometer_data *priv;
> > + void __iomem *regs;
> > + struct resource *res;
> > + struct device *hwmon;
> > + struct clk *clk;
> > + int ret;
> > +
> > + np = dev->of_node;
> > +
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + if (!res)
> > + return -ENOENT;
>
> Unnecessary error check. devm_ioremap_resource() does that (and
> returns -EINVAL).
>
Change these line-of-codes into devm_platform_ioremap_resource(pdev, 0).

> > + regs = devm_ioremap_resource(dev, res);
> > + if (IS_ERR(regs))
> > + return PTR_ERR(regs);
> > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > + if (!priv)
> > + return -ENOMEM;
> > +
> > + priv->pwm_channel = default_pwm_params;
> > + priv->tacho_channel = default_tacho_params;
> > + priv->regmap = devm_regmap_init(dev, NULL, (__force void *)regs,
> > + &aspeed_pwm_tachometer_regmap_config);
> > + if (IS_ERR(priv->regmap))
> > + return PTR_ERR(priv->regmap);
> > +
> > + clk = devm_clk_get(dev, NULL);
> > + if (IS_ERR(clk))
> > + return -ENODEV;
> > + priv->clk_freq = clk_get_rate(clk);
> > +
> > + priv->reset = devm_reset_control_get(&pdev->dev, NULL);
> > + if (IS_ERR(priv->reset)) {
> > + dev_err(&pdev->dev, "can't get aspeed_pwm_tacho reset\n");
> > + return PTR_ERR(priv->reset);
> > + }
> > +
> > + //scu init
> > + reset_control_assert(priv->reset);
> > + reset_control_deassert(priv->reset);
> > +
> > + for_each_child_of_node(np, child) {
> > + ret = aspeed_pwm_create_fan(dev, child, priv);
> > + if (ret) {
> > + of_node_put(child);
> > + return ret;
> > + }
> > + }
> > +
> > + priv->groups[0] = &pwm_dev_group;
> > + priv->groups[1] = &fan_dev_group;
> > + priv->groups[2] = NULL;
> > + dev_info(dev, "pwm tach probe done\n");
> > + hwmon = devm_hwmon_device_register_with_groups(dev,
> > + "aspeed_pwm_tachometer",
> > + priv, priv->groups);
>
> New drivers must use devm_hwmon_device_register_with_info().
>
I will change using it in v2.

> > +
> > + return PTR_ERR_OR_ZERO(hwmon);
> > +}
> > +
> > +static const struct of_device_id of_pwm_tachometer_match_table[] = {
> > + { .compatible = "aspeed,ast2600-pwm-tachometer", },
> > + {},
> > +};
> > +MODULE_DEVICE_TABLE(of, of_pwm_tachometer_match_table);
> > +
> > +static struct platform_driver aspeed_pwm_tachometer_driver = {
> > + .probe = aspeed_pwm_tachometer_probe,
> > + .driver = {
> > + .name = "aspeed_pwm_tachometer",
> > + .of_match_table = of_pwm_tachometer_match_table,
> > + },
> > +};
> > +
> > +module_platform_driver(aspeed_pwm_tachometer_driver);
> > +
> > +MODULE_AUTHOR("Ryan Chen <[email protected]>");
> > +MODULE_DESCRIPTION("ASPEED PWM and Fan Tachometer device driver");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.17.1
> >

Thanks for you suggestion,
Troy Lee