* [PATCH v2 0/2] hwmon: Add NPCM7xx PWM and Fan driver support
@ 2018-06-19 10:53 Tomer Maimon
2018-06-19 10:53 ` [PATCH v2 1/2] dt-binding: hwmon: Add NPCM7xx PWM and Fan controller documentation Tomer Maimon
2018-06-19 10:53 ` [PATCH v2 2/2] hwmon: npcm750: add NPCM7xx PWM and Fan driver Tomer Maimon
0 siblings, 2 replies; 11+ messages in thread
From: Tomer Maimon @ 2018-06-19 10:53 UTC (permalink / raw)
To: robh+dt, mark.rutland, jdelvare, linux, avifishman70, yuenn,
brendanhiggins, venture, joel
Cc: devicetree, linux-kernel, linux-hwmon, openbmc, Tomer Maimon
This patch set adds Pulse Width Modulation (PWM) and Fan tacho
support for the Nuvoton NPCM7xx Baseboard Management Controller (BMC).
The Nuvoton BMC NPCM7xx has two identical PWM controller modules,
each module has four PWM controller outputs and eight identical Fan
controller modules, each module has two Fan controller inputs.
The hwmon driver provides sysfs entries through which the user can
configure and get the duty cycle between 0 (disable the specific PWM)
to 255 of particular PWM output port.
The driver support cooling device creation, that could be bound
to a thermal zone for the thermal control.
The NPCM7xx PWM and Fan driver tested on NPCM750 evaluation board.
Addressed comments from:.
- Guenter Roeck: https://www.spinics.net/lists/devicetree/msg231982.html
Changes since version 1:
- Rename driver name
- Adding Fan Controller support
- Adding cooling device support
- Modifying dt-binding documentation for Fan and cooling binding
Tomer Maimon (2):
dt-binding: hwmon: Add NPCM7xx PWM and Fan controller documentation
hwmon: npcm750: add NPCM7xx PWM and Fan driver
.../devicetree/bindings/hwmon/npcm750-pwm-fan.txt | 84 ++
drivers/hwmon/Kconfig | 7 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/npcm750-pwm-fan.c | 1099 ++++++++++++++++++++
4 files changed, 1191 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hwmon/npcm750-pwm-fan.txt
create mode 100644 drivers/hwmon/npcm750-pwm-fan.c
--
2.14.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 1/2] dt-binding: hwmon: Add NPCM7xx PWM and Fan controller documentation
2018-06-19 10:53 [PATCH v2 0/2] hwmon: Add NPCM7xx PWM and Fan driver support Tomer Maimon
@ 2018-06-19 10:53 ` Tomer Maimon
2018-06-19 10:53 ` [PATCH v2 2/2] hwmon: npcm750: add NPCM7xx PWM and Fan driver Tomer Maimon
1 sibling, 0 replies; 11+ messages in thread
From: Tomer Maimon @ 2018-06-19 10:53 UTC (permalink / raw)
To: robh+dt, mark.rutland, jdelvare, linux, avifishman70, yuenn,
brendanhiggins, venture, joel
Cc: devicetree, linux-kernel, linux-hwmon, openbmc, Tomer Maimon
Added device tree binding documentation for Nuvoton BMC
NPCM7xx Pulse Width Modulation (PWM) and Fan tach controller.
The PWM controller can support upto 8 PWM output ports.
The Fan tach controller can support upto 16 tachometer inputs.
Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
---
.../devicetree/bindings/hwmon/npcm750-pwm-fan.txt | 84 ++++++++++++++++++++++
1 file changed, 84 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hwmon/npcm750-pwm-fan.txt
diff --git a/Documentation/devicetree/bindings/hwmon/npcm750-pwm-fan.txt b/Documentation/devicetree/bindings/hwmon/npcm750-pwm-fan.txt
new file mode 100644
index 000000000000..a9eacda34f92
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/npcm750-pwm-fan.txt
@@ -0,0 +1,84 @@
+Nuvoton NPCM7xx PWM and Fan Tacho controller device driver
+
+The NPCM7xx has two identical Pulse-width modulation (PWM) controller modules,
+Each PWM module has four PWM controller outputs, Totally 8 PWM controller outputs.
+
+The NPCM7xx has eight identical Fan tachometer controller modules,
+Each Fan module has two Fan controller inputs, Totally 16 Fan controller inputs.
+
+Required properties for pwm-fan node:
+- compatible : "nuvoton,npcm750-pwm-fan" for Poleg NPCM7XX.
+- reg : specifies physical base address and size of the registers.
+- reg-names : must contain:
+ * "pwm-base" for the PWM registers.
+ * "fan-base" for the Fan registers.
+- clocks : phandle of reference clocks.
+- clock-names : must contain
+ * "clk_apb3" for clock of PWM controller.
+ * "clk_apb4" for clock of Fan controller.
+- interrupts : contain the Fan interrupts with flags for falling edge.
+- pinctrl-names : a pinctrl state named "default" must be defined.
+- pinctrl-0 : phandle referencing pin configuration of the PWM and Fan
+ controller ports.
+
+pwm subnode format:
+===================
+Under pwm subnode can be upto 8 child nodes, each child node representing a PWM channel.
+Each pwm subnode must have one PWM channel and atleast one Fan tach channel.
+
+For PWM channel 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:
+- pwm-ch : specify the PWM output channel.
+ integer value in the range 0 through 7, that represent
+ the PWM channel number that used.
+
+- fan-ch : specify the Fan input channel.
+ integer value in the range 0 through 15, that represent
+ the Fan channel number that used.
+
+ At least one Fan tach input channel is required
+
+Optional property for each child node:
+- cooling-levels: PWM duty cycle values in a range from 0 to 255
+ which correspond to thermal cooling states.
+
+Examples:
+
+pwm_fan:pwm_fan_controller@103000 {
+ compatible = "nuvoton,npcm750-pwm-fan";
+ reg = <0x103000 0x2000>,
+ <0x180000 0x8000>;
+ reg-names = "pwm_base", "fan_base";
+ clocks = <&clk NPCM7XX_CLK_APB3>,
+ <&clk NPCM7XX_CLK_APB4>;
+ clock-names = "clk_apb3","clk_apb4";
+ interrupts = <GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pwm0_pins &pwm1_pins &pwm2_pins
+ &fanin0_pins &fanin1_pins &fanin2_pins
+ &fanin3_pins &fanin4_pins>;
+
+ pwm@0 {
+ pwm-ch = /bits/ 8 <0x00>;
+ fan-ch = /bits/ 8 <0x00 0x01>;
+ cooling-levels = <127 255>;
+ };
+ pwm@1 {
+ pwm-ch = /bits/ 8 <0x01>;
+ fan-ch = /bits/ 8 <0x02 0x03>;
+ };
+ pwm@2 {
+ pwm-ch = /bits/ 8 <0x02>;
+ fan-ch = /bits/ 8 <0x04>;
+ };
+
+};
\ No newline at end of file
--
2.14.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 2/2] hwmon: npcm750: add NPCM7xx PWM and Fan driver
2018-06-19 10:53 [PATCH v2 0/2] hwmon: Add NPCM7xx PWM and Fan driver support Tomer Maimon
2018-06-19 10:53 ` [PATCH v2 1/2] dt-binding: hwmon: Add NPCM7xx PWM and Fan controller documentation Tomer Maimon
@ 2018-06-19 10:53 ` Tomer Maimon
2018-06-19 14:31 ` kbuild test robot
` (3 more replies)
1 sibling, 4 replies; 11+ messages in thread
From: Tomer Maimon @ 2018-06-19 10:53 UTC (permalink / raw)
To: robh+dt, mark.rutland, jdelvare, linux, avifishman70, yuenn,
brendanhiggins, venture, joel
Cc: devicetree, linux-kernel, linux-hwmon, openbmc, Tomer Maimon
Add Nuvoton BMC NPCM7xx Pulse Width Modulation (PWM) and Fan tacho driver.
The Nuvoton BMC NPCM7xx has two identical PWM controller modules,
each module has four PWM controller outputs and eight identical Fan
controller modules, each module has two Fan controller inputs.
The driver provides a sysfs entries through which the user can
configure the duty-cycle value (ranging from 0 to 100 percent)
and read the fan tacho rpm value.
The driver support cooling device creation, that could be bound
to a thermal zone for the thermal control.
Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
---
drivers/hwmon/Kconfig | 7 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/npcm750-pwm-fan.c | 1099 +++++++++++++++++++++++++++++++++++++++
3 files changed, 1107 insertions(+)
create mode 100644 drivers/hwmon/npcm750-pwm-fan.c
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index f10840ad465c..f6c2eff9bb7d 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1888,6 +1888,13 @@ config SENSORS_XGENE
If you say yes here you get support for the temperature
and power sensors for APM X-Gene SoC.
+config SENSORS_NPCM7XX
+ tristate "Nuvoton NPCM7XX PWM and Fan driver"
+ depends on THERMAL || THERMAL=n
+ help
+ This driver provides support for Nuvoton NPCM7XX PWM and Fan
+ controllers.
+
if ACPI
comment "ACPI drivers"
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index e7d52a36e6c4..004e2ec5b68f 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -174,6 +174,7 @@ obj-$(CONFIG_SENSORS_W83L786NG) += w83l786ng.o
obj-$(CONFIG_SENSORS_WM831X) += wm831x-hwmon.o
obj-$(CONFIG_SENSORS_WM8350) += wm8350-hwmon.o
obj-$(CONFIG_SENSORS_XGENE) += xgene-hwmon.o
+obj-$(CONFIG_SENSORS_NPCM7XX) += npcm750-pwm-fan.o
obj-$(CONFIG_PMBUS) += pmbus/
diff --git a/drivers/hwmon/npcm750-pwm-fan.c b/drivers/hwmon/npcm750-pwm-fan.c
new file mode 100644
index 000000000000..82898197686f
--- /dev/null
+++ b/drivers/hwmon/npcm750-pwm-fan.c
@@ -0,0 +1,1099 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (c) 2014-2018 Nuvoton Technology corporation.
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/device.h>
+#include <linux/clk.h>
+#include <linux/platform_device.h>
+#include <linux/interrupt.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/sysfs.h>
+#include <linux/spinlock.h>
+#include <linux/of_irq.h>
+#include <linux/of_address.h>
+#include <linux/thermal.h>
+
+/* NPCM7XX PWM registers */
+#define NPCM7XX_PWM_REG_BASE(base, n) (base + ((n) * 0x1000L))
+
+#define NPCM7XX_PWM_REG_PR(base, n) (NPCM7XX_PWM_REG_BASE(base, n) + 0x00)
+#define NPCM7XX_PWM_REG_CSR(base, n) (NPCM7XX_PWM_REG_BASE(base, n) + 0x04)
+#define NPCM7XX_PWM_REG_CR(base, n) (NPCM7XX_PWM_REG_BASE(base, n) + 0x08)
+#define NPCM7XX_PWM_REG_CNRx(base, n, ch) \
+ (NPCM7XX_PWM_REG_BASE(base, n) + 0x0C + (12 * ch))
+#define NPCM7XX_PWM_REG_CMRx(base, n, ch) \
+ (NPCM7XX_PWM_REG_BASE(base, n) + 0x10 + (12 * ch))
+#define NPCM7XX_PWM_REG_PDRx(base, n, ch) \
+ (NPCM7XX_PWM_REG_BASE(base, n) + 0x14 + (12 * ch))
+#define NPCM7XX_PWM_REG_PIER(base, n) (NPCM7XX_PWM_REG_BASE(base, n) + 0x3C)
+#define NPCM7XX_PWM_REG_PIIR(base, n) (NPCM7XX_PWM_REG_BASE(base, n) + 0x40)
+#define NPCM7XX_FAN_REG_BASE(base, n) (base + ((n) * 0x1000L))
+
+#define NPCM7XX_PWM_CTRL_CH0_MODE_BIT BIT(3)
+#define NPCM7XX_PWM_CTRL_CH1_MODE_BIT BIT(11)
+#define NPCM7XX_PWM_CTRL_CH2_MODE_BIT BIT(15)
+#define NPCM7XX_PWM_CTRL_CH3_MODE_BIT BIT(19)
+
+#define NPCM7XX_PWM_CTRL_CH0_INV_BIT BIT(2)
+#define NPCM7XX_PWM_CTRL_CH1_INV_BIT BIT(10)
+#define NPCM7XX_PWM_CTRL_CH2_INV_BIT BIT(14)
+#define NPCM7XX_PWM_CTRL_CH3_INV_BIT BIT(18)
+
+#define NPCM7XX_PWM_CTRL_CH0_EN_BIT BIT(0)
+#define NPCM7XX_PWM_CTRL_CH1_EN_BIT BIT(8)
+#define NPCM7XX_PWM_CTRL_CH2_EN_BIT BIT(12)
+#define NPCM7XX_PWM_CTRL_CH3_EN_BIT BIT(16)
+
+/* Define the maximum PWM channel number */
+#define NPCM7XX_PWM_MAX_CHN_NUM 8
+#define NPCM7XX_PWM_MAX_CHN_NUM_IN_A_MODULE 4
+#define NPCM7XX_PWM_MAX_MODULES 2
+
+/* Define the Counter Register, value = 100 for match 100% */
+#define NPCM7XX_PWM_COUNTER_DEFALUT_NUM 255
+#define NPCM7XX_PWM_COMPARATOR_DEFALUT_NUM 127
+
+#define NPCM7XX_PWM_COMPARATOR_MAX 255
+
+/* default all PWM channels PRESCALE2 = 1 */
+#define NPCM7XX_PWM_PRESCALE2_DEFALUT_CH0 0x4
+#define NPCM7XX_PWM_PRESCALE2_DEFALUT_CH1 0x40
+#define NPCM7XX_PWM_PRESCALE2_DEFALUT_CH2 0x400
+#define NPCM7XX_PWM_PRESCALE2_DEFALUT_CH3 0x4000
+
+#define PWM_OUTPUT_FREQ_25KHZ 25000
+#define PWN_CNT_DEFAULT 256
+#define MIN_PRESCALE1 2
+#define NPCM7XX_PWM_PRESCALE_SHIFT_CH01 8
+
+#define NPCM7XX_PWM_PRESCALE2_DEFALUT (NPCM7XX_PWM_PRESCALE2_DEFALUT_CH0 | \
+ NPCM7XX_PWM_PRESCALE2_DEFALUT_CH1 | \
+ NPCM7XX_PWM_PRESCALE2_DEFALUT_CH2 | \
+ NPCM7XX_PWM_PRESCALE2_DEFALUT_CH3)
+
+#define NPCM7XX_PWM_CTRL_MODE_DEFALUT (NPCM7XX_PWM_CTRL_CH0_MODE_BIT | \
+ NPCM7XX_PWM_CTRL_CH1_MODE_BIT | \
+ NPCM7XX_PWM_CTRL_CH2_MODE_BIT | \
+ NPCM7XX_PWM_CTRL_CH3_MODE_BIT)
+
+#define NPCM7XX_PWM_CTRL_EN_DEFALUT (NPCM7XX_PWM_CTRL_CH0_EN_BIT | \
+ NPCM7XX_PWM_CTRL_CH1_EN_BIT | \
+ NPCM7XX_PWM_CTRL_CH2_EN_BIT | \
+ NPCM7XX_PWM_CTRL_CH3_EN_BIT)
+
+/* NPCM7XX FAN Tacho registers */
+#define NPCM7XX_FAN_REG_BASE(base, n) (base + ((n) * 0x1000L))
+
+#define NPCM7XX_FAN_REG_TCNT1(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x00)
+#define NPCM7XX_FAN_REG_TCRA(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x02)
+#define NPCM7XX_FAN_REG_TCRB(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x04)
+#define NPCM7XX_FAN_REG_TCNT2(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x06)
+#define NPCM7XX_FAN_REG_TPRSC(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x08)
+#define NPCM7XX_FAN_REG_TCKC(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x0A)
+#define NPCM7XX_FAN_REG_TMCTRL(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x0C)
+#define NPCM7XX_FAN_REG_TICTRL(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x0E)
+#define NPCM7XX_FAN_REG_TICLR(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x10)
+#define NPCM7XX_FAN_REG_TIEN(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x12)
+#define NPCM7XX_FAN_REG_TCPA(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x14)
+#define NPCM7XX_FAN_REG_TCPB(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x16)
+#define NPCM7XX_FAN_REG_TCPCFG(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x18)
+#define NPCM7XX_FAN_REG_TINASEL(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x1A)
+#define NPCM7XX_FAN_REG_TINBSEL(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x1C)
+
+#define NPCM7XX_FAN_TCKC_CLKX_NONE 0
+#define NPCM7XX_FAN_TCKC_CLK1_APB BIT(0)
+#define NPCM7XX_FAN_TCKC_CLK2_APB BIT(3)
+
+#define NPCM7XX_FAN_TMCTRL_TBEN BIT(6)
+#define NPCM7XX_FAN_TMCTRL_TAEN BIT(5)
+#define NPCM7XX_FAN_TMCTRL_TBEDG BIT(4)
+#define NPCM7XX_FAN_TMCTRL_TAEDG BIT(3)
+#define NPCM7XX_FAN_TMCTRL_MODE_5 BIT(2)
+
+#define NPCM7XX_FAN_TICLR_CLEAR_ALL GENMASK(5, 0)
+#define NPCM7XX_FAN_TICLR_TFCLR BIT(5)
+#define NPCM7XX_FAN_TICLR_TECLR BIT(4)
+#define NPCM7XX_FAN_TICLR_TDCLR BIT(3)
+#define NPCM7XX_FAN_TICLR_TCCLR BIT(2)
+#define NPCM7XX_FAN_TICLR_TBCLR BIT(1)
+#define NPCM7XX_FAN_TICLR_TACLR BIT(0)
+
+#define NPCM7XX_FAN_TIEN_ENABLE_ALL GENMASK(5, 0)
+#define NPCM7XX_FAN_TIEN_TFIEN BIT(5)
+#define NPCM7XX_FAN_TIEN_TEIEN BIT(4)
+#define NPCM7XX_FAN_TIEN_TDIEN BIT(3)
+#define NPCM7XX_FAN_TIEN_TCIEN BIT(2)
+#define NPCM7XX_FAN_TIEN_TBIEN BIT(1)
+#define NPCM7XX_FAN_TIEN_TAIEN BIT(0)
+
+#define NPCM7XX_FAN_TICTRL_TFPND BIT(5)
+#define NPCM7XX_FAN_TICTRL_TEPND BIT(4)
+#define NPCM7XX_FAN_TICTRL_TDPND BIT(3)
+#define NPCM7XX_FAN_TICTRL_TCPND BIT(2)
+#define NPCM7XX_FAN_TICTRL_TBPND BIT(1)
+#define NPCM7XX_FAN_TICTRL_TAPND BIT(0)
+
+#define NPCM7XX_FAN_TCPCFG_HIBEN BIT(7)
+#define NPCM7XX_FAN_TCPCFG_EQBEN BIT(6)
+#define NPCM7XX_FAN_TCPCFG_LOBEN BIT(5)
+#define NPCM7XX_FAN_TCPCFG_CPBSEL BIT(4)
+#define NPCM7XX_FAN_TCPCFG_HIAEN BIT(3)
+#define NPCM7XX_FAN_TCPCFG_EQAEN BIT(2)
+#define NPCM7XX_FAN_TCPCFG_LOAEN BIT(1)
+#define NPCM7XX_FAN_TCPCFG_CPASEL BIT(0)
+
+/* FAN General Defintion */
+/* Define the maximum FAN channel number */
+#define NPCM7XX_FAN_MAX_MODULE 8
+#define NPCM7XX_FAN_MAX_CHN_NUM_IN_A_MODULE 2
+#define NPCM7XX_FAN_MAX_CHN_NUM 16
+
+/*
+ * Get Fan Tach Timeout (base on clock 214843.75Hz, 1 cnt = 4.654us)
+ * Timeout 94ms ~= 0x5000
+ * (The minimum FAN speed could to support ~640RPM/pulse 1,
+ * 320RPM/pulse 2, ...-- 10.6Hz)
+ */
+#define NPCM7XX_FAN_TIMEOUT 0x5000
+#define NPCM7XX_FAN_TCNT 0xFFFF
+#define NPCM7XX_FAN_TCPA (NPCM7XX_FAN_TCNT - NPCM7XX_FAN_TIMEOUT)
+#define NPCM7XX_FAN_TCPB (NPCM7XX_FAN_TCNT - NPCM7XX_FAN_TIMEOUT)
+
+#define NPCM7XX_FAN_POLL_TIMER_200MS 200
+#define NPCM7XX_FAN_DEFAULT_PULSE_PER_REVOLUTION 2
+#define NPCM7XX_FAN_TINASEL_FANIN_DEFAULT 0
+#define NPCM7XX_FAN_CLK_PRESCALE 255
+
+#define NPCM7XX_FAN_CMPA 0
+#define NPCM7XX_FAN_CMPB 1
+
+/* Obtain the fan number */
+#define NPCM7XX_FAN_INPUT(fan, cmp) ((fan << 1) + (cmp))
+
+/* fan sample status */
+#define FAN_DISABLE 0xFF
+#define FAN_INIT 0x00
+#define FAN_PREPARE_TO_GET_FIRST_CAPTURE 0x01
+#define FAN_ENOUGH_SAMPLE 0x02
+
+struct Fan_Dev {
+ u8 FanStFlag;
+ u8 FanPlsPerRev;
+ u16 FanCnt;
+ u32 FanCntTemp;
+};
+
+struct npcm7xx_cooling_device {
+ char name[THERMAL_NAME_LENGTH];
+ struct npcm7xx_pwm_fan_data *data;
+ struct thermal_cooling_device *tcdev;
+ int pwm_port;
+ u8 *cooling_levels;
+ u8 max_state;
+ u8 cur_state;
+};
+
+struct npcm7xx_pwm_fan_data {
+ void __iomem *pwm_base, *fan_base;
+ unsigned long pwm_clk_freq, fan_clk_freq;
+ struct clk *pwm_clk, *fan_clk;
+ struct mutex npcm7xx_pwm_lock[NPCM7XX_PWM_MAX_MODULES];
+ spinlock_t npcm7xx_fan_lock[NPCM7XX_FAN_MAX_MODULE];
+ int fan_irq[NPCM7XX_FAN_MAX_MODULE];
+ bool pwm_present[NPCM7XX_PWM_MAX_CHN_NUM],
+ fan_present[NPCM7XX_FAN_MAX_CHN_NUM];
+ u32 InputClkFreq;
+ struct timer_list npcm7xx_fan_timer;
+ struct Fan_Dev npcm7xx_fan[NPCM7XX_FAN_MAX_CHN_NUM];
+ struct npcm7xx_cooling_device *cdev[NPCM7XX_PWM_MAX_CHN_NUM];
+ u8 npcm7xx_fan_select;
+};
+
+static int npcm7xx_pwm_config_set(struct npcm7xx_pwm_fan_data *data,
+ int channel, u16 val)
+{
+ u32 PWMCh = (channel % NPCM7XX_PWM_MAX_CHN_NUM_IN_A_MODULE);
+ u32 module = (channel / NPCM7XX_PWM_MAX_CHN_NUM_IN_A_MODULE);
+ u32 u32TmpBuf = 0, ctrl_en_bit, env_bit;
+
+ /*
+ * Config PWM Comparator register for setting duty cycle
+ */
+ if (val < 0 || val > NPCM7XX_PWM_COMPARATOR_MAX)
+ return -EINVAL;
+
+ mutex_lock(&data->npcm7xx_pwm_lock[module]);
+
+ /* write new CMR value */
+ iowrite32(val, NPCM7XX_PWM_REG_CMRx(data->pwm_base, module, PWMCh));
+ u32TmpBuf = ioread32(NPCM7XX_PWM_REG_CR(data->pwm_base, module));
+
+ switch (PWMCh) {
+ case 0:
+ ctrl_en_bit = NPCM7XX_PWM_CTRL_CH0_EN_BIT;
+ env_bit = NPCM7XX_PWM_CTRL_CH0_INV_BIT;
+ break;
+ case 1:
+ ctrl_en_bit = NPCM7XX_PWM_CTRL_CH1_EN_BIT;
+ env_bit = NPCM7XX_PWM_CTRL_CH1_INV_BIT;
+ break;
+ case 2:
+ ctrl_en_bit = NPCM7XX_PWM_CTRL_CH2_EN_BIT;
+ env_bit = NPCM7XX_PWM_CTRL_CH2_INV_BIT;
+ break;
+ case 3:
+ ctrl_en_bit = NPCM7XX_PWM_CTRL_CH3_EN_BIT;
+ env_bit = NPCM7XX_PWM_CTRL_CH3_INV_BIT;
+ break;
+ default:
+ return -ENODEV;
+ }
+
+ if (val == 0) {
+ /* Disable PWM */
+ u32TmpBuf &= ~(ctrl_en_bit);
+ u32TmpBuf |= env_bit;
+ } else {
+ /* Enable PWM */
+ u32TmpBuf |= ctrl_en_bit;
+ u32TmpBuf &= ~(env_bit);
+ }
+
+ iowrite32(u32TmpBuf, NPCM7XX_PWM_REG_CR(data->pwm_base, module));
+ mutex_unlock(&data->npcm7xx_pwm_lock[module]);
+
+ return 0;
+}
+
+static inline void npcm7xx_fan_start_capture(struct npcm7xx_pwm_fan_data *data,
+ u8 fan, u8 cmp)
+{
+ u8 fan_id = 0;
+ u8 reg_mode = 0;
+ u8 reg_int = 0;
+ unsigned long flags;
+
+ fan_id = NPCM7XX_FAN_INPUT(fan, cmp);
+
+ /* to check whether any fan tach is enable */
+ if (data->npcm7xx_fan[fan_id].FanStFlag != FAN_DISABLE) {
+ /* reset status */
+ spin_lock_irqsave(&data->npcm7xx_fan_lock[fan], flags);
+
+ data->npcm7xx_fan[fan_id].FanStFlag = FAN_INIT;
+ reg_int = ioread8(NPCM7XX_FAN_REG_TIEN(data->fan_base, fan));
+
+ if (cmp == NPCM7XX_FAN_CMPA) {
+ /* enable interrupt */
+ iowrite8((u8) (reg_int | (NPCM7XX_FAN_TIEN_TAIEN |
+ NPCM7XX_FAN_TIEN_TEIEN)),
+ NPCM7XX_FAN_REG_TIEN(data->fan_base, fan));
+
+ reg_mode = NPCM7XX_FAN_TCKC_CLK1_APB
+ | ioread8(NPCM7XX_FAN_REG_TCKC(data->fan_base,
+ fan));
+
+ /* start to Capture */
+ iowrite8(reg_mode, NPCM7XX_FAN_REG_TCKC(data->fan_base,
+ fan));
+ } else {
+ /* enable interrupt */
+ iowrite8((u8) (reg_int | (NPCM7XX_FAN_TIEN_TBIEN |
+ NPCM7XX_FAN_TIEN_TFIEN)),
+ NPCM7XX_FAN_REG_TIEN(data->fan_base, fan));
+
+ reg_mode =
+ NPCM7XX_FAN_TCKC_CLK2_APB
+ | ioread8(NPCM7XX_FAN_REG_TCKC(data->fan_base,
+ fan));
+
+ /* start to Capture */
+ iowrite8(reg_mode,
+ NPCM7XX_FAN_REG_TCKC(data->fan_base, fan));
+ }
+
+ spin_unlock_irqrestore(&data->npcm7xx_fan_lock[fan], flags);
+ }
+}
+
+/*
+ * Enable a background timer to poll fan tach value, (200ms * 4)
+ * to polling all fan
+ */
+static void npcm7xx_fan_polling(struct timer_list *t)
+{
+ struct npcm7xx_pwm_fan_data *data;
+ unsigned long flags;
+ int i;
+
+ data = from_timer(data, t, npcm7xx_fan_timer);
+
+ /*
+ * Polling two module per one round,
+ * FAN01 & FAN89 / FAN23 & FAN1011 / FAN45 & FAN1213 / FAN67 & FAN1415
+ */
+ for (i = data->npcm7xx_fan_select; i < NPCM7XX_FAN_MAX_MODULE;
+ i = i+4) {
+ /* clear the flag and reset the counter (TCNT) */
+ spin_lock_irqsave(&data->npcm7xx_fan_lock[i], flags);
+ iowrite8((u8) NPCM7XX_FAN_TICLR_CLEAR_ALL,
+ NPCM7XX_FAN_REG_TICLR(data->fan_base, i));
+ spin_unlock_irqrestore(&data->npcm7xx_fan_lock[i], flags);
+
+ if (data->fan_present[i*2] == true) {
+ spin_lock_irqsave(&data->npcm7xx_fan_lock[i], flags);
+ iowrite16(NPCM7XX_FAN_TCNT,
+ NPCM7XX_FAN_REG_TCNT1(data->fan_base, i));
+ spin_unlock_irqrestore(&data->npcm7xx_fan_lock[i],
+ flags);
+ npcm7xx_fan_start_capture(data, i, NPCM7XX_FAN_CMPA);
+ }
+ if (data->fan_present[(i*2)+1] == true) {
+ spin_lock_irqsave(&data->npcm7xx_fan_lock[i], flags);
+ iowrite16(NPCM7XX_FAN_TCNT,
+ NPCM7XX_FAN_REG_TCNT2(data->fan_base, i));
+ spin_unlock_irqrestore(&data->npcm7xx_fan_lock[i],
+ flags);
+ npcm7xx_fan_start_capture(data, i, NPCM7XX_FAN_CMPB);
+ }
+ }
+
+ data->npcm7xx_fan_select++;
+ data->npcm7xx_fan_select &= 0x3;
+
+ /* reset the timer interval */
+ data->npcm7xx_fan_timer.expires = jiffies +
+ msecs_to_jiffies(NPCM7XX_FAN_POLL_TIMER_200MS);
+ add_timer(&data->npcm7xx_fan_timer);
+}
+
+static inline void npcm7xx_fan_compute(struct npcm7xx_pwm_fan_data *data,
+ u8 fan, u8 cmp, u8 fan_id,
+ u8 flag_int, u8 flag_mode,
+ u8 flag_clear)
+{
+ u8 reg_int = 0;
+ u8 reg_mode = 0;
+ u16 fan_cap = 0;
+
+ if (cmp == NPCM7XX_FAN_CMPA)
+ fan_cap = ioread16(NPCM7XX_FAN_REG_TCRA(data->fan_base, fan));
+ else
+ fan_cap = ioread16(NPCM7XX_FAN_REG_TCRB(data->fan_base, fan));
+
+ /* clear capature flag, H/W will auto reset the NPCM7XX_FAN_TCNTx */
+ iowrite8((u8) flag_clear, NPCM7XX_FAN_REG_TICLR(data->fan_base, fan));
+
+ if (data->npcm7xx_fan[fan_id].FanStFlag == FAN_INIT) {
+ /* First capture, drop it */
+ data->npcm7xx_fan[fan_id].FanStFlag =
+ FAN_PREPARE_TO_GET_FIRST_CAPTURE;
+
+ /* reset counter */
+ data->npcm7xx_fan[fan_id].FanCntTemp = 0;
+ } else if (data->npcm7xx_fan[fan_id].FanStFlag <
+ FAN_ENOUGH_SAMPLE) {
+ /*
+ * collect the enough sample,
+ * (ex: 2 pulse fan need to get 2 sample)
+ */
+ data->npcm7xx_fan[fan_id].FanCntTemp +=
+ (NPCM7XX_FAN_TCNT - fan_cap);
+
+ data->npcm7xx_fan[fan_id].FanStFlag++;
+ } else {
+ /* get enough sample or fan disable */
+ if (data->npcm7xx_fan[fan_id].FanStFlag == FAN_ENOUGH_SAMPLE) {
+ data->npcm7xx_fan[fan_id].FanCntTemp +=
+ (NPCM7XX_FAN_TCNT - fan_cap);
+
+ /* compute finial average cnt per pulse */
+ data->npcm7xx_fan[fan_id].FanCnt
+ = data->npcm7xx_fan[fan_id].FanCntTemp /
+ FAN_ENOUGH_SAMPLE;
+
+ data->npcm7xx_fan[fan_id].FanStFlag = FAN_INIT;
+ }
+
+ reg_int = ioread8(NPCM7XX_FAN_REG_TIEN(data->fan_base, fan));
+
+ /* disable interrupt */
+ iowrite8((u8) (reg_int & ~flag_int),
+ NPCM7XX_FAN_REG_TIEN(data->fan_base, fan));
+ reg_mode = ioread8(NPCM7XX_FAN_REG_TCKC(data->fan_base, fan));
+
+ /* stop capturing */
+ iowrite8((u8) (reg_mode & ~flag_mode),
+ NPCM7XX_FAN_REG_TCKC(data->fan_base, fan));
+
+ }
+}
+
+static inline void npcm7xx_check_cmp(struct npcm7xx_pwm_fan_data *data,
+ u8 fan, u8 cmp, u8 flag)
+{
+ u8 reg_int = 0;
+ u8 reg_mode = 0;
+ u8 flag_timeout;
+ u8 flag_cap;
+ u8 flag_clear;
+ u8 flag_int;
+ u8 flag_mode;
+ u8 fan_id;
+
+ fan_id = NPCM7XX_FAN_INPUT(fan, cmp);
+
+ if (cmp == NPCM7XX_FAN_CMPA) {
+ flag_cap = NPCM7XX_FAN_TICTRL_TAPND;
+ flag_timeout = NPCM7XX_FAN_TICTRL_TEPND;
+ flag_int = NPCM7XX_FAN_TIEN_TAIEN | NPCM7XX_FAN_TIEN_TEIEN;
+ flag_mode = NPCM7XX_FAN_TCKC_CLK1_APB;
+ flag_clear = NPCM7XX_FAN_TICLR_TACLR | NPCM7XX_FAN_TICLR_TECLR;
+ } else {
+ flag_cap = NPCM7XX_FAN_TICTRL_TBPND;
+ flag_timeout = NPCM7XX_FAN_TICTRL_TFPND;
+ flag_int = NPCM7XX_FAN_TIEN_TBIEN | NPCM7XX_FAN_TIEN_TFIEN;
+ flag_mode = NPCM7XX_FAN_TCKC_CLK2_APB;
+ flag_clear = NPCM7XX_FAN_TICLR_TBCLR | NPCM7XX_FAN_TICLR_TFCLR;
+ }
+
+
+ if (flag & flag_timeout) {
+
+ reg_int = ioread8(NPCM7XX_FAN_REG_TIEN(data->fan_base, fan));
+
+ /* disable interrupt */
+ iowrite8((u8) (reg_int & ~flag_int),
+ NPCM7XX_FAN_REG_TIEN(data->fan_base, fan));
+
+ /* clear interrup flag */
+ iowrite8((u8) flag_clear, NPCM7XX_FAN_REG_TICLR(data->fan_base,
+ fan));
+
+ reg_mode = ioread8(NPCM7XX_FAN_REG_TCKC(data->fan_base, fan));
+
+ /* stop capturing */
+ iowrite8((u8) (reg_mode & ~flag_mode),
+ NPCM7XX_FAN_REG_TCKC(data->fan_base, fan));
+
+ /*
+ * If timeout occurs (NPCM7XX_FAN_TIMEOUT), the fan doesn't
+ * connect or speed is lower than 10.6Hz (320RPM/pulse2).
+ * In these situation, the RPM output should be zero.
+ */
+ data->npcm7xx_fan[fan_id].FanCnt = 0;
+ //DEBUG_MSG("%s : it is timeout fan_id %d\n", __func__, fan_id);
+ } else {
+ /* input capture is occurred */
+ if (flag & flag_cap)
+ npcm7xx_fan_compute(data, fan, cmp, fan_id, flag_int,
+ flag_mode, flag_clear);
+ }
+}
+
+static irqreturn_t npcm7xx_fan_isr(int irq, void *dev_id)
+{
+ struct npcm7xx_pwm_fan_data *data = dev_id;
+ u8 flag = 0;
+ int module;
+ unsigned long flags;
+
+ module = irq - data->fan_irq[0];
+ spin_lock_irqsave(&data->npcm7xx_fan_lock[module], flags);
+
+ flag = ioread8(NPCM7XX_FAN_REG_TICTRL(data->fan_base, module));
+ if (flag > 0) {
+ npcm7xx_check_cmp(data, module, NPCM7XX_FAN_CMPA, flag);
+ npcm7xx_check_cmp(data, module, NPCM7XX_FAN_CMPB, flag);
+ spin_unlock_irqrestore(&data->npcm7xx_fan_lock[module], flags);
+ return IRQ_HANDLED;
+ }
+
+ spin_unlock_irqrestore(&data->npcm7xx_fan_lock[module], flags);
+
+ return IRQ_NONE;
+}
+
+static int npcm7xx_read_pwm(struct device *dev, u32 attr, int channel,
+ long *val)
+{
+ struct npcm7xx_pwm_fan_data *data = dev_get_drvdata(dev);
+ u32 PWMCh = (channel % NPCM7XX_PWM_MAX_CHN_NUM_IN_A_MODULE);
+ u32 module = (channel / NPCM7XX_PWM_MAX_CHN_NUM_IN_A_MODULE);
+
+ if (IS_ERR(data))
+ return PTR_ERR(data);
+
+ switch (attr) {
+ case hwmon_pwm_input:
+ mutex_lock(&data->npcm7xx_pwm_lock[module]);
+ *val = (long)ioread32(
+ NPCM7XX_PWM_REG_CMRx(data->pwm_base, module, PWMCh));
+ mutex_unlock(&data->npcm7xx_pwm_lock[module]);
+ return 0;
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+static int npcm7xx_write_pwm(struct device *dev, u32 attr, int channel,
+ long val)
+{
+ struct npcm7xx_pwm_fan_data *data = dev_get_drvdata(dev);
+ int err = 0;
+
+ switch (attr) {
+ case hwmon_pwm_input:
+ err = npcm7xx_pwm_config_set(data, channel, (u16)val);
+ break;
+ default:
+ err = -EOPNOTSUPP;
+ break;
+ }
+
+ return err;
+}
+
+static umode_t npcm7xx_pwm_is_visible(const void *_data, u32 attr, int channel)
+{
+ const struct npcm7xx_pwm_fan_data *data = _data;
+
+ if (!data->pwm_present[channel])
+ return 0;
+
+ switch (attr) {
+ case hwmon_pwm_input:
+ return 0644;
+ default:
+ return 0;
+ }
+}
+
+static int npcm7xx_read_fan(struct device *dev, u32 attr, int channel,
+ long *val)
+{
+ struct npcm7xx_pwm_fan_data *data = dev_get_drvdata(dev);
+
+ switch (attr) {
+ case hwmon_fan_input:
+ *val = 0;
+ if (data->npcm7xx_fan[channel].FanCnt <= 0)
+ return data->npcm7xx_fan[channel].FanCnt;
+
+ /* Convert the raw reading to RPM */
+ if ((data->npcm7xx_fan[channel].FanCnt > 0) &&
+ data->npcm7xx_fan[channel].FanPlsPerRev > 0)
+ *val = (long)((data->InputClkFreq * 60)/
+ (data->npcm7xx_fan[channel].FanCnt *
+ data->npcm7xx_fan[channel].FanPlsPerRev));
+ return 0;
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+static umode_t npcm7xx_fan_is_visible(const void *_data, u32 attr, int channel)
+{
+ const struct npcm7xx_pwm_fan_data *data = _data;
+
+ if (!data->fan_present[channel])
+ return 0;
+
+ switch (attr) {
+ case hwmon_fan_input:
+ return 0444;
+ default:
+ return 0;
+ }
+}
+
+static int npcm7xx_read(struct device *dev, enum hwmon_sensor_types type,
+ u32 attr, int channel, long *val)
+{
+ switch (type) {
+ case hwmon_pwm:
+ return npcm7xx_read_pwm(dev, attr, channel, val);
+ case hwmon_fan:
+ return npcm7xx_read_fan(dev, attr, channel, val);
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+static int npcm7xx_write(struct device *dev, enum hwmon_sensor_types type,
+ u32 attr, int channel, long val)
+{
+ switch (type) {
+ case hwmon_pwm:
+ return npcm7xx_write_pwm(dev, attr, channel, val);
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+static umode_t npcm7xx_is_visible(const void *data,
+ enum hwmon_sensor_types type,
+ u32 attr, int channel)
+{
+ switch (type) {
+ case hwmon_pwm:
+ return npcm7xx_pwm_is_visible(data, attr, channel);
+ case hwmon_fan:
+ return npcm7xx_fan_is_visible(data, attr, channel);
+ default:
+ return 0;
+ }
+}
+
+static const u32 npcm7xx_pwm_config[] = {
+ HWMON_PWM_INPUT,
+ HWMON_PWM_INPUT,
+ HWMON_PWM_INPUT,
+ HWMON_PWM_INPUT,
+ HWMON_PWM_INPUT,
+ HWMON_PWM_INPUT,
+ HWMON_PWM_INPUT,
+ HWMON_PWM_INPUT,
+ 0
+};
+
+static const struct hwmon_channel_info npcm7xx_pwm = {
+ .type = hwmon_pwm,
+ .config = npcm7xx_pwm_config,
+};
+
+static const u32 npcm7xx_fan_config[] = {
+ HWMON_F_INPUT,
+ HWMON_F_INPUT,
+ HWMON_F_INPUT,
+ HWMON_F_INPUT,
+ HWMON_F_INPUT,
+ HWMON_F_INPUT,
+ HWMON_F_INPUT,
+ HWMON_F_INPUT,
+ HWMON_F_INPUT,
+ HWMON_F_INPUT,
+ HWMON_F_INPUT,
+ HWMON_F_INPUT,
+ HWMON_F_INPUT,
+ HWMON_F_INPUT,
+ HWMON_F_INPUT,
+ HWMON_F_INPUT,
+ 0
+};
+
+static const struct hwmon_channel_info npcm7xx_fan = {
+ .type = hwmon_fan,
+ .config = npcm7xx_fan_config,
+};
+
+static const struct hwmon_channel_info *npcm7xx_info[] = {
+ &npcm7xx_pwm,
+ &npcm7xx_fan,
+ NULL
+};
+
+static const struct hwmon_ops npcm7xx_hwmon_ops = {
+ .is_visible = npcm7xx_is_visible,
+ .read = npcm7xx_read,
+ .write = npcm7xx_write,
+};
+
+static const struct hwmon_chip_info npcm7xx_chip_info = {
+ .ops = &npcm7xx_hwmon_ops,
+ .info = npcm7xx_info,
+};
+
+static u32 npcm7xx_pwm_init(struct npcm7xx_pwm_fan_data *data)
+{
+ int m, ch;
+ u32 Prescale_val, output_freq;
+
+ data->pwm_clk_freq = clk_get_rate(data->pwm_clk);
+
+ /* Adjust NPCM7xx PWMs output frequency to ~25Khz */
+ output_freq = data->pwm_clk_freq / PWN_CNT_DEFAULT;
+ Prescale_val = DIV_ROUND_CLOSEST(output_freq, PWM_OUTPUT_FREQ_25KHZ);
+
+ /* If Prescale_val = 0, then the prescale output clock is stopped */
+ if (Prescale_val < MIN_PRESCALE1)
+ Prescale_val = MIN_PRESCALE1;
+ /*
+ * Prescale_val need to decrement in one because in the PWM Prescale
+ * register the Prescale value increment by one
+ */
+ Prescale_val--;
+
+ /* Setting PWM Prescale Register value register to both modules */
+ Prescale_val |= (Prescale_val << NPCM7XX_PWM_PRESCALE_SHIFT_CH01);
+
+ for (m = 0; m < NPCM7XX_PWM_MAX_MODULES ; m++) {
+ iowrite32(Prescale_val, NPCM7XX_PWM_REG_PR(data->pwm_base, m));
+ iowrite32(NPCM7XX_PWM_PRESCALE2_DEFALUT,
+ NPCM7XX_PWM_REG_CSR(data->pwm_base, m));
+ iowrite32(NPCM7XX_PWM_CTRL_MODE_DEFALUT,
+ NPCM7XX_PWM_REG_CR(data->pwm_base, m));
+
+ for (ch = 0; ch < NPCM7XX_PWM_MAX_CHN_NUM_IN_A_MODULE; ch++) {
+ iowrite32(NPCM7XX_PWM_COUNTER_DEFALUT_NUM,
+ NPCM7XX_PWM_REG_CNRx(data->pwm_base, m, ch));
+ }
+ }
+
+ return output_freq / ((Prescale_val & 0xf) + 1);
+}
+
+static void npcm7xx_fan_init(struct npcm7xx_pwm_fan_data *data)
+{
+ int md;
+ int ch;
+ int i;
+ u32 apb_clk_freq;
+
+ for (md = 0; md < NPCM7XX_FAN_MAX_MODULE; md++) {
+
+ /* stop FAN0~7 clock */
+ iowrite8((u8)NPCM7XX_FAN_TCKC_CLKX_NONE,
+ NPCM7XX_FAN_REG_TCKC(data->fan_base, md));
+
+ /* disable all interrupt */
+ iowrite8((u8) 0x00, NPCM7XX_FAN_REG_TIEN(data->fan_base, md));
+
+ /* clear all interrupt */
+ iowrite8((u8) NPCM7XX_FAN_TICLR_CLEAR_ALL,
+ NPCM7XX_FAN_REG_TICLR(data->fan_base, md));
+
+ /* set FAN0~7 clock prescaler */
+ iowrite8((u8) NPCM7XX_FAN_CLK_PRESCALE,
+ NPCM7XX_FAN_REG_TPRSC(data->fan_base, md));
+
+ /* set FAN0~7 mode (high-to-low transition) */
+ iowrite8(
+ (u8) (
+ NPCM7XX_FAN_TMCTRL_MODE_5 |
+ NPCM7XX_FAN_TMCTRL_TBEN |
+ NPCM7XX_FAN_TMCTRL_TAEN
+ ),
+ NPCM7XX_FAN_REG_TMCTRL(data->fan_base, md)
+ );
+
+ /* set FAN0~7 Initial Count/Cap */
+ iowrite16(NPCM7XX_FAN_TCNT,
+ NPCM7XX_FAN_REG_TCNT1(data->fan_base, md));
+ iowrite16(NPCM7XX_FAN_TCNT,
+ NPCM7XX_FAN_REG_TCNT2(data->fan_base, md));
+
+ /* set FAN0~7 compare (equal to count) */
+ iowrite8((u8)(NPCM7XX_FAN_TCPCFG_EQAEN |
+ NPCM7XX_FAN_TCPCFG_EQBEN),
+ NPCM7XX_FAN_REG_TCPCFG(data->fan_base, md));
+
+ /* set FAN0~7 compare value */
+ iowrite16(NPCM7XX_FAN_TCPA,
+ NPCM7XX_FAN_REG_TCPA(data->fan_base, md));
+ iowrite16(NPCM7XX_FAN_TCPB,
+ NPCM7XX_FAN_REG_TCPB(data->fan_base, md));
+
+ /* set FAN0~7 fan input FANIN 0~15 */
+ iowrite8((u8) NPCM7XX_FAN_TINASEL_FANIN_DEFAULT,
+ NPCM7XX_FAN_REG_TINASEL(data->fan_base, md));
+ iowrite8((u8) NPCM7XX_FAN_TINASEL_FANIN_DEFAULT,
+ NPCM7XX_FAN_REG_TINBSEL(data->fan_base, md));
+
+ for (i = 0; i < NPCM7XX_FAN_MAX_CHN_NUM_IN_A_MODULE; i++) {
+ ch = md * NPCM7XX_FAN_MAX_CHN_NUM_IN_A_MODULE + i;
+ data->npcm7xx_fan[ch].FanStFlag = FAN_DISABLE;
+ data->npcm7xx_fan[ch].FanPlsPerRev =
+ NPCM7XX_FAN_DEFAULT_PULSE_PER_REVOLUTION;
+ data->npcm7xx_fan[ch].FanCnt = 0;
+ }
+ }
+
+ apb_clk_freq = clk_get_rate(data->fan_clk);
+
+ /* Fan tach input clock = APB clock / prescalar, default is 255. */
+ data->InputClkFreq = apb_clk_freq / (NPCM7XX_FAN_CLK_PRESCALE + 1);
+}
+
+static int
+npcm7xx_pwm_cz_get_max_state(struct thermal_cooling_device *tcdev,
+ unsigned long *state)
+{
+ struct npcm7xx_cooling_device *cdev = tcdev->devdata;
+
+ *state = cdev->max_state;
+
+ return 0;
+}
+
+static int
+npcm7xx_pwm_cz_get_cur_state(struct thermal_cooling_device *tcdev,
+ unsigned long *state)
+{
+ struct npcm7xx_cooling_device *cdev = tcdev->devdata;
+
+ *state = cdev->cur_state;
+
+ return 0;
+}
+
+static int
+npcm7xx_pwm_cz_set_cur_state(struct thermal_cooling_device *tcdev,
+ unsigned long state)
+{
+ struct npcm7xx_cooling_device *cdev = tcdev->devdata;
+ int ret;
+
+ if (state > cdev->max_state)
+ return -EINVAL;
+
+ cdev->cur_state = state;
+ ret = npcm7xx_pwm_config_set(cdev->data, cdev->pwm_port,
+ cdev->cooling_levels[cdev->cur_state]);
+
+ return ret;
+}
+
+static const struct thermal_cooling_device_ops npcm7xx_pwm_cool_ops = {
+ .get_max_state = npcm7xx_pwm_cz_get_max_state,
+ .get_cur_state = npcm7xx_pwm_cz_get_cur_state,
+ .set_cur_state = npcm7xx_pwm_cz_set_cur_state,
+};
+
+static int npcm7xx_create_pwm_cooling(struct device *dev,
+ struct device_node *child,
+ struct npcm7xx_pwm_fan_data *data,
+ u32 pwm_port, u8 num_levels)
+{
+ int ret;
+ struct npcm7xx_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, THERMAL_NAME_LENGTH, "%s%d", child->name,
+ pwm_port);
+
+ cdev->tcdev = thermal_of_cooling_device_register(child,
+ cdev->name,
+ cdev,
+ &npcm7xx_pwm_cool_ops);
+ if (IS_ERR(cdev->tcdev))
+ return PTR_ERR(cdev->tcdev);
+
+ cdev->data = data;
+ cdev->pwm_port = pwm_port;
+
+ data->cdev[pwm_port] = cdev;
+
+ return 0;
+}
+
+static int npcm7xx_en_pwm_fan(struct device *dev,
+ struct device_node *child,
+ struct npcm7xx_pwm_fan_data *data)
+{
+ u8 *fan_ch;
+ u8 pwm_port;
+ int ret, fan_cnt;
+ u8 index, ch;
+
+ ret = of_property_read_u8(child, "pwm-ch", &pwm_port);
+ if (ret)
+ return ret;
+
+ data->pwm_present[pwm_port] = true;
+ ret = npcm7xx_pwm_config_set(data, pwm_port,
+ NPCM7XX_PWM_COMPARATOR_DEFALUT_NUM);
+
+ ret = of_property_count_u8_elems(child, "cooling-levels");
+ if (ret > 0) {
+ ret = npcm7xx_create_pwm_cooling(dev, child, data, pwm_port,
+ ret);
+ if (ret)
+ return ret;
+ }
+
+ fan_cnt = of_property_count_u8_elems(child, "fan-ch");
+ if (fan_cnt < 1)
+ return -EINVAL;
+
+ fan_ch = devm_kzalloc(dev, sizeof(*fan_ch) * fan_cnt, GFP_KERNEL);
+ if (!fan_ch)
+ return -ENOMEM;
+
+ ret = of_property_read_u8_array(child, "fan-ch", fan_ch, fan_cnt);
+ if (ret)
+ return ret;
+
+ for (ch = 0; ch < fan_cnt; ch++) {
+ index = fan_ch[ch];
+ data->fan_present[index] = true;
+ data->npcm7xx_fan[index].FanStFlag = FAN_INIT;
+ }
+
+ return 0;
+}
+
+static int npcm7xx_pwm_fan_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct device_node *np, *child;
+ struct npcm7xx_pwm_fan_data *data;
+ struct resource *res;
+ struct device *hwmon;
+ char name[20];
+ int ret, cnt;
+ u32 output_freq;
+ u32 i;
+
+ np = dev->of_node;
+
+ data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "pwm_base");
+ if (res == NULL) {
+ pr_err("PWM of_address_to_resource fail ret %d\n", ret);
+ return -ENODEV;
+ }
+
+ data->pwm_base = devm_ioremap_resource(dev, res);
+ pr_debug("pwm base is 0x%08X, res.start 0x%08X , size 0x%08X\n",
+ (u32)data->pwm_base, res->start, resource_size(res));
+ if (!data->pwm_base) {
+ pr_err("pwm probe failed: can't read pwm base address\n");
+ return -ENOMEM;
+ }
+
+ data->pwm_clk = devm_clk_get(dev, "clk_apb3");
+ if (IS_ERR(data->pwm_clk)) {
+ pr_err(" pwm probe failed: can't read clk.\n");
+ return -ENODEV;
+ }
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "fan_base");
+ if (ret) {
+ pr_err("fan of_address_to_resource fail ret %d\n", ret);
+ return -ENODEV;
+ }
+
+ data->fan_base = devm_ioremap_resource(dev, res);
+ pr_debug("fan base is 0x%08X, res.start 0x%08X , size 0x%08X\n",
+ (u32)data->fan_base, res->start, resource_size(res));
+
+ if (!data->fan_base) {
+ pr_err("fan probe failed: can't read fan base address.\n");
+ return -ENOMEM;
+ }
+
+ data->fan_clk = devm_clk_get(dev, "clk_apb4");
+ if (IS_ERR(data->fan_clk)) {
+ pr_err(" FAN probe failed: can't read clk.\n");
+ return -ENODEV;
+ }
+
+ output_freq = npcm7xx_pwm_init(data);
+ npcm7xx_fan_init(data);
+
+ for (cnt = 0; cnt < NPCM7XX_PWM_MAX_MODULES ; cnt++)
+ mutex_init(&data->npcm7xx_pwm_lock[cnt]);
+
+ for (i = 0; i < NPCM7XX_FAN_MAX_MODULE; i++) {
+ spin_lock_init(&data->npcm7xx_fan_lock[i]);
+
+ data->fan_irq[i] = platform_get_irq(pdev, i);
+ if (!data->fan_irq[i]) {
+ pr_err("%s - failed to map irq %d\n", __func__, i);
+ ret = -EAGAIN;
+ goto err_irq;
+ }
+
+ sprintf(name, "NPCM7XX-FAN-MD%d", i);
+
+ if (request_irq(data->fan_irq[i], npcm7xx_fan_isr, 0, name,
+ (void *)data)) {
+ pr_err("NPCM7XX: register irq FAN%d failed\n", i);
+ ret = -EAGAIN;
+ goto err_irq;
+ }
+ }
+
+ for_each_child_of_node(np, child) {
+ ret = npcm7xx_en_pwm_fan(dev, child, data);
+ if (ret) {
+ pr_err("npcm7xx_en_pwm_fan failed ret %d\n", ret);
+ of_node_put(child);
+ goto err_irq;
+ }
+ }
+
+ hwmon = devm_hwmon_device_register_with_info(dev, "npcm7xx_pwm_fan",
+ data, &npcm7xx_chip_info,
+ NULL);
+ if (IS_ERR(hwmon)) {
+ pr_err("PWM Driver failed - devm_hwmon_device_register_with_groups failed\n");
+ ret = PTR_ERR(hwmon);
+ goto err_irq;
+ }
+
+ for (i = 0; i < NPCM7XX_FAN_MAX_CHN_NUM; i++) {
+ if (data->fan_present[i] == true) {
+ /* fan timer initialization */
+ data->npcm7xx_fan_select = 0;
+ data->npcm7xx_fan_timer.expires = jiffies +
+ msecs_to_jiffies(NPCM7XX_FAN_POLL_TIMER_200MS);
+ timer_setup(&data->npcm7xx_fan_timer,
+ npcm7xx_fan_polling, 0);
+ add_timer(&data->npcm7xx_fan_timer);
+ break;
+ }
+ }
+
+ pr_info("NPCM7XX PWM-FAN Driver probed, output Freq %dHz[PWM], input Freq %dHz[FAN]\n",
+ output_freq, data->InputClkFreq);
+
+ return 0;
+
+err_irq:
+ for (; i > 0; i--)
+ free_irq(data->fan_irq[i-1], (void *)data);
+
+ return ret;
+}
+
+static const struct of_device_id of_pwm_fan_match_table[] = {
+ { .compatible = "nuvoton,npcm750-pwm-fan", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, of_pwm_fan_match_table);
+
+static struct platform_driver npcm7xx_pwm_fan_driver = {
+ .probe = npcm7xx_pwm_fan_probe,
+ .driver = {
+ .name = "npcm7xx_pwm_fan",
+ .of_match_table = of_pwm_fan_match_table,
+ },
+};
+
+module_platform_driver(npcm7xx_pwm_fan_driver);
+
+MODULE_DESCRIPTION("Nuvoton NPCM7XX PWM and Fan Tacho driver");
+MODULE_AUTHOR("Tomer Maimon <tomer.maimon@nuvoton.com>");
+MODULE_LICENSE("GPL v2");
--
2.14.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] hwmon: npcm750: add NPCM7xx PWM and Fan driver
2018-06-19 10:53 ` [PATCH v2 2/2] hwmon: npcm750: add NPCM7xx PWM and Fan driver Tomer Maimon
@ 2018-06-19 14:31 ` kbuild test robot
2018-06-19 19:43 ` kbuild test robot
` (2 subsequent siblings)
3 siblings, 0 replies; 11+ messages in thread
From: kbuild test robot @ 2018-06-19 14:31 UTC (permalink / raw)
To: Tomer Maimon
Cc: kbuild-all, robh+dt, mark.rutland, jdelvare, linux, avifishman70,
yuenn, brendanhiggins, venture, joel, devicetree, linux-kernel,
linux-hwmon, openbmc, Tomer Maimon
[-- Attachment #1: Type: text/plain, Size: 10653 bytes --]
Hi Tomer,
I love your patch! Perhaps something to improve:
[auto build test WARNING on hwmon/hwmon-next]
[also build test WARNING on v4.18-rc1 next-20180619]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Tomer-Maimon/hwmon-Add-NPCM7xx-PWM-and-Fan-driver-support/20180619-192033
base: https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 8.1.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=8.1.0 make.cross ARCH=ia64
All warnings (new ones prefixed by >>):
In file included from include/linux/printk.h:332,
from include/linux/kernel.h:14,
from include/linux/list.h:9,
from include/linux/module.h:9,
from drivers/hwmon/npcm750-pwm-fan.c:4:
drivers/hwmon/npcm750-pwm-fan.c: In function 'npcm7xx_pwm_fan_probe':
>> drivers/hwmon/npcm750-pwm-fan.c:979:4: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
(u32)data->pwm_base, res->start, resource_size(res));
^
include/linux/dynamic_debug.h:128:10: note: in definition of macro 'dynamic_pr_debug'
##__VA_ARGS__); \
^~~~~~~~~~~
drivers/hwmon/npcm750-pwm-fan.c:978:2: note: in expansion of macro 'pr_debug'
pr_debug("pwm base is 0x%08X, res.start 0x%08X , size 0x%08X\n",
^~~~~~~~
In file included from include/linux/kernel.h:14,
from include/linux/list.h:9,
from include/linux/module.h:9,
from drivers/hwmon/npcm750-pwm-fan.c:4:
>> drivers/hwmon/npcm750-pwm-fan.c:978:11: warning: format '%X' expects argument of type 'unsigned int', but argument 4 has type 'resource_size_t' {aka 'long long unsigned int'} [-Wformat=]
pr_debug("pwm base is 0x%08X, res.start 0x%08X , size 0x%08X\n",
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/printk.h:288:21: note: in definition of macro 'pr_fmt'
#define pr_fmt(fmt) fmt
^~~
include/linux/printk.h:336:2: note: in expansion of macro 'dynamic_pr_debug'
dynamic_pr_debug(fmt, ##__VA_ARGS__)
^~~~~~~~~~~~~~~~
drivers/hwmon/npcm750-pwm-fan.c:978:2: note: in expansion of macro 'pr_debug'
pr_debug("pwm base is 0x%08X, res.start 0x%08X , size 0x%08X\n",
^~~~~~~~
drivers/hwmon/npcm750-pwm-fan.c:978:11: warning: format '%X' expects argument of type 'unsigned int', but argument 5 has type 'resource_size_t' {aka 'long long unsigned int'} [-Wformat=]
pr_debug("pwm base is 0x%08X, res.start 0x%08X , size 0x%08X\n",
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/printk.h:288:21: note: in definition of macro 'pr_fmt'
#define pr_fmt(fmt) fmt
^~~
include/linux/printk.h:336:2: note: in expansion of macro 'dynamic_pr_debug'
dynamic_pr_debug(fmt, ##__VA_ARGS__)
^~~~~~~~~~~~~~~~
drivers/hwmon/npcm750-pwm-fan.c:978:2: note: in expansion of macro 'pr_debug'
pr_debug("pwm base is 0x%08X, res.start 0x%08X , size 0x%08X\n",
^~~~~~~~
In file included from include/linux/printk.h:332,
from include/linux/kernel.h:14,
from include/linux/list.h:9,
from include/linux/module.h:9,
from drivers/hwmon/npcm750-pwm-fan.c:4:
drivers/hwmon/npcm750-pwm-fan.c:999:4: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
(u32)data->fan_base, res->start, resource_size(res));
^
include/linux/dynamic_debug.h:128:10: note: in definition of macro 'dynamic_pr_debug'
##__VA_ARGS__); \
^~~~~~~~~~~
drivers/hwmon/npcm750-pwm-fan.c:998:2: note: in expansion of macro 'pr_debug'
pr_debug("fan base is 0x%08X, res.start 0x%08X , size 0x%08X\n",
^~~~~~~~
In file included from include/linux/kernel.h:14,
from include/linux/list.h:9,
from include/linux/module.h:9,
from drivers/hwmon/npcm750-pwm-fan.c:4:
drivers/hwmon/npcm750-pwm-fan.c:998:11: warning: format '%X' expects argument of type 'unsigned int', but argument 4 has type 'resource_size_t' {aka 'long long unsigned int'} [-Wformat=]
pr_debug("fan base is 0x%08X, res.start 0x%08X , size 0x%08X\n",
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/printk.h:288:21: note: in definition of macro 'pr_fmt'
#define pr_fmt(fmt) fmt
^~~
include/linux/printk.h:336:2: note: in expansion of macro 'dynamic_pr_debug'
dynamic_pr_debug(fmt, ##__VA_ARGS__)
^~~~~~~~~~~~~~~~
drivers/hwmon/npcm750-pwm-fan.c:998:2: note: in expansion of macro 'pr_debug'
pr_debug("fan base is 0x%08X, res.start 0x%08X , size 0x%08X\n",
^~~~~~~~
drivers/hwmon/npcm750-pwm-fan.c:998:11: warning: format '%X' expects argument of type 'unsigned int', but argument 5 has type 'resource_size_t' {aka 'long long unsigned int'} [-Wformat=]
pr_debug("fan base is 0x%08X, res.start 0x%08X , size 0x%08X\n",
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/printk.h:288:21: note: in definition of macro 'pr_fmt'
#define pr_fmt(fmt) fmt
^~~
include/linux/printk.h:336:2: note: in expansion of macro 'dynamic_pr_debug'
dynamic_pr_debug(fmt, ##__VA_ARGS__)
^~~~~~~~~~~~~~~~
drivers/hwmon/npcm750-pwm-fan.c:998:2: note: in expansion of macro 'pr_debug'
pr_debug("fan base is 0x%08X, res.start 0x%08X , size 0x%08X\n",
^~~~~~~~
>> include/linux/printk.h:304:2: warning: 'ret' may be used uninitialized in this function [-Wmaybe-uninitialized]
printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
^~~~~~
drivers/hwmon/npcm750-pwm-fan.c:961:6: note: 'ret' was declared here
int ret, cnt;
^~~
vim +979 drivers/hwmon/npcm750-pwm-fan.c
952
953 static int npcm7xx_pwm_fan_probe(struct platform_device *pdev)
954 {
955 struct device *dev = &pdev->dev;
956 struct device_node *np, *child;
957 struct npcm7xx_pwm_fan_data *data;
958 struct resource *res;
959 struct device *hwmon;
960 char name[20];
961 int ret, cnt;
962 u32 output_freq;
963 u32 i;
964
965 np = dev->of_node;
966
967 data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
968 if (!data)
969 return -ENOMEM;
970
971 res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "pwm_base");
972 if (res == NULL) {
973 pr_err("PWM of_address_to_resource fail ret %d\n", ret);
974 return -ENODEV;
975 }
976
977 data->pwm_base = devm_ioremap_resource(dev, res);
> 978 pr_debug("pwm base is 0x%08X, res.start 0x%08X , size 0x%08X\n",
> 979 (u32)data->pwm_base, res->start, resource_size(res));
980 if (!data->pwm_base) {
981 pr_err("pwm probe failed: can't read pwm base address\n");
982 return -ENOMEM;
983 }
984
985 data->pwm_clk = devm_clk_get(dev, "clk_apb3");
986 if (IS_ERR(data->pwm_clk)) {
987 pr_err(" pwm probe failed: can't read clk.\n");
988 return -ENODEV;
989 }
990
991 res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "fan_base");
992 if (ret) {
993 pr_err("fan of_address_to_resource fail ret %d\n", ret);
994 return -ENODEV;
995 }
996
997 data->fan_base = devm_ioremap_resource(dev, res);
998 pr_debug("fan base is 0x%08X, res.start 0x%08X , size 0x%08X\n",
999 (u32)data->fan_base, res->start, resource_size(res));
1000
1001 if (!data->fan_base) {
1002 pr_err("fan probe failed: can't read fan base address.\n");
1003 return -ENOMEM;
1004 }
1005
1006 data->fan_clk = devm_clk_get(dev, "clk_apb4");
1007 if (IS_ERR(data->fan_clk)) {
1008 pr_err(" FAN probe failed: can't read clk.\n");
1009 return -ENODEV;
1010 }
1011
1012 output_freq = npcm7xx_pwm_init(data);
1013 npcm7xx_fan_init(data);
1014
1015 for (cnt = 0; cnt < NPCM7XX_PWM_MAX_MODULES ; cnt++)
1016 mutex_init(&data->npcm7xx_pwm_lock[cnt]);
1017
1018 for (i = 0; i < NPCM7XX_FAN_MAX_MODULE; i++) {
1019 spin_lock_init(&data->npcm7xx_fan_lock[i]);
1020
1021 data->fan_irq[i] = platform_get_irq(pdev, i);
1022 if (!data->fan_irq[i]) {
1023 pr_err("%s - failed to map irq %d\n", __func__, i);
1024 ret = -EAGAIN;
1025 goto err_irq;
1026 }
1027
1028 sprintf(name, "NPCM7XX-FAN-MD%d", i);
1029
1030 if (request_irq(data->fan_irq[i], npcm7xx_fan_isr, 0, name,
1031 (void *)data)) {
1032 pr_err("NPCM7XX: register irq FAN%d failed\n", i);
1033 ret = -EAGAIN;
1034 goto err_irq;
1035 }
1036 }
1037
1038 for_each_child_of_node(np, child) {
1039 ret = npcm7xx_en_pwm_fan(dev, child, data);
1040 if (ret) {
1041 pr_err("npcm7xx_en_pwm_fan failed ret %d\n", ret);
1042 of_node_put(child);
1043 goto err_irq;
1044 }
1045 }
1046
1047 hwmon = devm_hwmon_device_register_with_info(dev, "npcm7xx_pwm_fan",
1048 data, &npcm7xx_chip_info,
1049 NULL);
1050 if (IS_ERR(hwmon)) {
1051 pr_err("PWM Driver failed - devm_hwmon_device_register_with_groups failed\n");
1052 ret = PTR_ERR(hwmon);
1053 goto err_irq;
1054 }
1055
1056 for (i = 0; i < NPCM7XX_FAN_MAX_CHN_NUM; i++) {
1057 if (data->fan_present[i] == true) {
1058 /* fan timer initialization */
1059 data->npcm7xx_fan_select = 0;
1060 data->npcm7xx_fan_timer.expires = jiffies +
1061 msecs_to_jiffies(NPCM7XX_FAN_POLL_TIMER_200MS);
1062 timer_setup(&data->npcm7xx_fan_timer,
1063 npcm7xx_fan_polling, 0);
1064 add_timer(&data->npcm7xx_fan_timer);
1065 break;
1066 }
1067 }
1068
1069 pr_info("NPCM7XX PWM-FAN Driver probed, output Freq %dHz[PWM], input Freq %dHz[FAN]\n",
1070 output_freq, data->InputClkFreq);
1071
1072 return 0;
1073
1074 err_irq:
1075 for (; i > 0; i--)
1076 free_irq(data->fan_irq[i-1], (void *)data);
1077
1078 return ret;
1079 }
1080
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 50920 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] hwmon: npcm750: add NPCM7xx PWM and Fan driver
2018-06-19 10:53 ` [PATCH v2 2/2] hwmon: npcm750: add NPCM7xx PWM and Fan driver Tomer Maimon
2018-06-19 14:31 ` kbuild test robot
@ 2018-06-19 19:43 ` kbuild test robot
2018-06-20 16:48 ` Guenter Roeck
2018-06-21 13:46 ` Guenter Roeck
3 siblings, 0 replies; 11+ messages in thread
From: kbuild test robot @ 2018-06-19 19:43 UTC (permalink / raw)
To: Tomer Maimon
Cc: kbuild-all, robh+dt, mark.rutland, jdelvare, linux, avifishman70,
yuenn, brendanhiggins, venture, joel, devicetree, linux-kernel,
linux-hwmon, openbmc, Tomer Maimon
[-- Attachment #1: Type: text/plain, Size: 5593 bytes --]
Hi Tomer,
I love your patch! Perhaps something to improve:
[auto build test WARNING on hwmon/hwmon-next]
[also build test WARNING on v4.18-rc1 next-20180619]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Tomer-Maimon/hwmon-Add-NPCM7xx-PWM-and-Fan-driver-support/20180619-192033
base: https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
config: i386-allyesconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings
All warnings (new ones prefixed by >>):
drivers//hwmon/npcm750-pwm-fan.c: In function 'npcm7xx_pwm_fan_probe':
>> drivers//hwmon/npcm750-pwm-fan.c:973:3: warning: 'ret' may be used uninitialized in this function [-Wmaybe-uninitialized]
pr_err("PWM of_address_to_resource fail ret %d\n", ret);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
vim +/ret +973 drivers//hwmon/npcm750-pwm-fan.c
952
953 static int npcm7xx_pwm_fan_probe(struct platform_device *pdev)
954 {
955 struct device *dev = &pdev->dev;
956 struct device_node *np, *child;
957 struct npcm7xx_pwm_fan_data *data;
958 struct resource *res;
959 struct device *hwmon;
960 char name[20];
961 int ret, cnt;
962 u32 output_freq;
963 u32 i;
964
965 np = dev->of_node;
966
967 data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
968 if (!data)
969 return -ENOMEM;
970
971 res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "pwm_base");
972 if (res == NULL) {
> 973 pr_err("PWM of_address_to_resource fail ret %d\n", ret);
974 return -ENODEV;
975 }
976
977 data->pwm_base = devm_ioremap_resource(dev, res);
978 pr_debug("pwm base is 0x%08X, res.start 0x%08X , size 0x%08X\n",
979 (u32)data->pwm_base, res->start, resource_size(res));
980 if (!data->pwm_base) {
981 pr_err("pwm probe failed: can't read pwm base address\n");
982 return -ENOMEM;
983 }
984
985 data->pwm_clk = devm_clk_get(dev, "clk_apb3");
986 if (IS_ERR(data->pwm_clk)) {
987 pr_err(" pwm probe failed: can't read clk.\n");
988 return -ENODEV;
989 }
990
991 res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "fan_base");
992 if (ret) {
993 pr_err("fan of_address_to_resource fail ret %d\n", ret);
994 return -ENODEV;
995 }
996
997 data->fan_base = devm_ioremap_resource(dev, res);
998 pr_debug("fan base is 0x%08X, res.start 0x%08X , size 0x%08X\n",
999 (u32)data->fan_base, res->start, resource_size(res));
1000
1001 if (!data->fan_base) {
1002 pr_err("fan probe failed: can't read fan base address.\n");
1003 return -ENOMEM;
1004 }
1005
1006 data->fan_clk = devm_clk_get(dev, "clk_apb4");
1007 if (IS_ERR(data->fan_clk)) {
1008 pr_err(" FAN probe failed: can't read clk.\n");
1009 return -ENODEV;
1010 }
1011
1012 output_freq = npcm7xx_pwm_init(data);
1013 npcm7xx_fan_init(data);
1014
1015 for (cnt = 0; cnt < NPCM7XX_PWM_MAX_MODULES ; cnt++)
1016 mutex_init(&data->npcm7xx_pwm_lock[cnt]);
1017
1018 for (i = 0; i < NPCM7XX_FAN_MAX_MODULE; i++) {
1019 spin_lock_init(&data->npcm7xx_fan_lock[i]);
1020
1021 data->fan_irq[i] = platform_get_irq(pdev, i);
1022 if (!data->fan_irq[i]) {
1023 pr_err("%s - failed to map irq %d\n", __func__, i);
1024 ret = -EAGAIN;
1025 goto err_irq;
1026 }
1027
1028 sprintf(name, "NPCM7XX-FAN-MD%d", i);
1029
1030 if (request_irq(data->fan_irq[i], npcm7xx_fan_isr, 0, name,
1031 (void *)data)) {
1032 pr_err("NPCM7XX: register irq FAN%d failed\n", i);
1033 ret = -EAGAIN;
1034 goto err_irq;
1035 }
1036 }
1037
1038 for_each_child_of_node(np, child) {
1039 ret = npcm7xx_en_pwm_fan(dev, child, data);
1040 if (ret) {
1041 pr_err("npcm7xx_en_pwm_fan failed ret %d\n", ret);
1042 of_node_put(child);
1043 goto err_irq;
1044 }
1045 }
1046
1047 hwmon = devm_hwmon_device_register_with_info(dev, "npcm7xx_pwm_fan",
1048 data, &npcm7xx_chip_info,
1049 NULL);
1050 if (IS_ERR(hwmon)) {
1051 pr_err("PWM Driver failed - devm_hwmon_device_register_with_groups failed\n");
1052 ret = PTR_ERR(hwmon);
1053 goto err_irq;
1054 }
1055
1056 for (i = 0; i < NPCM7XX_FAN_MAX_CHN_NUM; i++) {
1057 if (data->fan_present[i] == true) {
1058 /* fan timer initialization */
1059 data->npcm7xx_fan_select = 0;
1060 data->npcm7xx_fan_timer.expires = jiffies +
1061 msecs_to_jiffies(NPCM7XX_FAN_POLL_TIMER_200MS);
1062 timer_setup(&data->npcm7xx_fan_timer,
1063 npcm7xx_fan_polling, 0);
1064 add_timer(&data->npcm7xx_fan_timer);
1065 break;
1066 }
1067 }
1068
1069 pr_info("NPCM7XX PWM-FAN Driver probed, output Freq %dHz[PWM], input Freq %dHz[FAN]\n",
1070 output_freq, data->InputClkFreq);
1071
1072 return 0;
1073
1074 err_irq:
1075 for (; i > 0; i--)
1076 free_irq(data->fan_irq[i-1], (void *)data);
1077
1078 return ret;
1079 }
1080
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 63476 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] hwmon: npcm750: add NPCM7xx PWM and Fan driver
2018-06-19 10:53 ` [PATCH v2 2/2] hwmon: npcm750: add NPCM7xx PWM and Fan driver Tomer Maimon
2018-06-19 14:31 ` kbuild test robot
2018-06-19 19:43 ` kbuild test robot
@ 2018-06-20 16:48 ` Guenter Roeck
2018-06-20 18:25 ` Joe Perches
2018-06-21 13:46 ` Guenter Roeck
3 siblings, 1 reply; 11+ messages in thread
From: Guenter Roeck @ 2018-06-20 16:48 UTC (permalink / raw)
To: Tomer Maimon
Cc: robh+dt, mark.rutland, jdelvare, avifishman70, yuenn,
brendanhiggins, venture, joel, devicetree, linux-kernel,
linux-hwmon, openbmc
On Tue, Jun 19, 2018 at 01:53:52PM +0300, Tomer Maimon wrote:
> Add Nuvoton BMC NPCM7xx Pulse Width Modulation (PWM) and Fan tacho driver.
>
> The Nuvoton BMC NPCM7xx has two identical PWM controller modules,
> each module has four PWM controller outputs and eight identical Fan
> controller modules, each module has two Fan controller inputs.
>
> The driver provides a sysfs entries through which the user can
> configure the duty-cycle value (ranging from 0 to 100 percent)
The ABI expects 0..255 for pwm values. That seems to be what the
chip uses as well, so the above statement is a bit misleading.
> and read the fan tacho rpm value.
>
> The driver support cooling device creation, that could be bound
> to a thermal zone for the thermal control.
>
Please fix all the problems reported by 0day. Also, please run
checkpatch.pl --strict and follow its guidance. I don't expect you
to fix everything (you won't have to sign up as maintainer, for example),
but statements such as "xxx == true" or "xxx == NULL" are not necessary,
and macro parameters should be provided in (). For example, "12 * ch"
would expand to "12 * 2 + 1" if ch is "2 + 1".
This is an incomplete review - too many problems. Please fix and resubmit
for another round.
> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> ---
> drivers/hwmon/Kconfig | 7 +
> drivers/hwmon/Makefile | 1 +
> drivers/hwmon/npcm750-pwm-fan.c | 1099 +++++++++++++++++++++++++++++++++++++++
We would also expect Documentation/hwmon/npcm750-pwm-fan.
> 3 files changed, 1107 insertions(+)
> create mode 100644 drivers/hwmon/npcm750-pwm-fan.c
>
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index f10840ad465c..f6c2eff9bb7d 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -1888,6 +1888,13 @@ config SENSORS_XGENE
> If you say yes here you get support for the temperature
> and power sensors for APM X-Gene SoC.
>
> +config SENSORS_NPCM7XX
> + tristate "Nuvoton NPCM7XX PWM and Fan driver"
> + depends on THERMAL || THERMAL=n
> + help
> + This driver provides support for Nuvoton NPCM7XX PWM and Fan
> + controllers.
> +
> if ACPI
>
> comment "ACPI drivers"
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index e7d52a36e6c4..004e2ec5b68f 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -174,6 +174,7 @@ obj-$(CONFIG_SENSORS_W83L786NG) += w83l786ng.o
> obj-$(CONFIG_SENSORS_WM831X) += wm831x-hwmon.o
> obj-$(CONFIG_SENSORS_WM8350) += wm8350-hwmon.o
> obj-$(CONFIG_SENSORS_XGENE) += xgene-hwmon.o
> +obj-$(CONFIG_SENSORS_NPCM7XX) += npcm750-pwm-fan.o
>
> obj-$(CONFIG_PMBUS) += pmbus/
>
> diff --git a/drivers/hwmon/npcm750-pwm-fan.c b/drivers/hwmon/npcm750-pwm-fan.c
> new file mode 100644
> index 000000000000..82898197686f
> --- /dev/null
> +++ b/drivers/hwmon/npcm750-pwm-fan.c
> @@ -0,0 +1,1099 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2014-2018 Nuvoton Technology corporation.
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/device.h>
> +#include <linux/clk.h>
> +#include <linux/platform_device.h>
> +#include <linux/interrupt.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/sysfs.h>
> +#include <linux/spinlock.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_address.h>
> +#include <linux/thermal.h>
Alphabetic order, please.
> +
> +/* NPCM7XX PWM registers */
> +#define NPCM7XX_PWM_REG_BASE(base, n) (base + ((n) * 0x1000L))
> +
> +#define NPCM7XX_PWM_REG_PR(base, n) (NPCM7XX_PWM_REG_BASE(base, n) + 0x00)
> +#define NPCM7XX_PWM_REG_CSR(base, n) (NPCM7XX_PWM_REG_BASE(base, n) + 0x04)
> +#define NPCM7XX_PWM_REG_CR(base, n) (NPCM7XX_PWM_REG_BASE(base, n) + 0x08)
> +#define NPCM7XX_PWM_REG_CNRx(base, n, ch) \
> + (NPCM7XX_PWM_REG_BASE(base, n) + 0x0C + (12 * ch))
> +#define NPCM7XX_PWM_REG_CMRx(base, n, ch) \
> + (NPCM7XX_PWM_REG_BASE(base, n) + 0x10 + (12 * ch))
> +#define NPCM7XX_PWM_REG_PDRx(base, n, ch) \
> + (NPCM7XX_PWM_REG_BASE(base, n) + 0x14 + (12 * ch))
> +#define NPCM7XX_PWM_REG_PIER(base, n) (NPCM7XX_PWM_REG_BASE(base, n) + 0x3C)
> +#define NPCM7XX_PWM_REG_PIIR(base, n) (NPCM7XX_PWM_REG_BASE(base, n) + 0x40)
> +#define NPCM7XX_FAN_REG_BASE(base, n) (base + ((n) * 0x1000L))
> +
> +#define NPCM7XX_PWM_CTRL_CH0_MODE_BIT BIT(3)
> +#define NPCM7XX_PWM_CTRL_CH1_MODE_BIT BIT(11)
> +#define NPCM7XX_PWM_CTRL_CH2_MODE_BIT BIT(15)
> +#define NPCM7XX_PWM_CTRL_CH3_MODE_BIT BIT(19)
> +
> +#define NPCM7XX_PWM_CTRL_CH0_INV_BIT BIT(2)
> +#define NPCM7XX_PWM_CTRL_CH1_INV_BIT BIT(10)
> +#define NPCM7XX_PWM_CTRL_CH2_INV_BIT BIT(14)
> +#define NPCM7XX_PWM_CTRL_CH3_INV_BIT BIT(18)
> +
> +#define NPCM7XX_PWM_CTRL_CH0_EN_BIT BIT(0)
> +#define NPCM7XX_PWM_CTRL_CH1_EN_BIT BIT(8)
> +#define NPCM7XX_PWM_CTRL_CH2_EN_BIT BIT(12)
> +#define NPCM7XX_PWM_CTRL_CH3_EN_BIT BIT(16)
> +
> +/* Define the maximum PWM channel number */
> +#define NPCM7XX_PWM_MAX_CHN_NUM 8
> +#define NPCM7XX_PWM_MAX_CHN_NUM_IN_A_MODULE 4
> +#define NPCM7XX_PWM_MAX_MODULES 2
> +
> +/* Define the Counter Register, value = 100 for match 100% */
> +#define NPCM7XX_PWM_COUNTER_DEFALUT_NUM 255
> +#define NPCM7XX_PWM_COMPARATOR_DEFALUT_NUM 127
> +
> +#define NPCM7XX_PWM_COMPARATOR_MAX 255
> +
> +/* default all PWM channels PRESCALE2 = 1 */
> +#define NPCM7XX_PWM_PRESCALE2_DEFALUT_CH0 0x4
> +#define NPCM7XX_PWM_PRESCALE2_DEFALUT_CH1 0x40
> +#define NPCM7XX_PWM_PRESCALE2_DEFALUT_CH2 0x400
> +#define NPCM7XX_PWM_PRESCALE2_DEFALUT_CH3 0x4000
> +
> +#define PWM_OUTPUT_FREQ_25KHZ 25000
> +#define PWN_CNT_DEFAULT 256
> +#define MIN_PRESCALE1 2
> +#define NPCM7XX_PWM_PRESCALE_SHIFT_CH01 8
> +
> +#define NPCM7XX_PWM_PRESCALE2_DEFALUT (NPCM7XX_PWM_PRESCALE2_DEFALUT_CH0 | \
> + NPCM7XX_PWM_PRESCALE2_DEFALUT_CH1 | \
> + NPCM7XX_PWM_PRESCALE2_DEFALUT_CH2 | \
> + NPCM7XX_PWM_PRESCALE2_DEFALUT_CH3)
> +
> +#define NPCM7XX_PWM_CTRL_MODE_DEFALUT (NPCM7XX_PWM_CTRL_CH0_MODE_BIT | \
> + NPCM7XX_PWM_CTRL_CH1_MODE_BIT | \
> + NPCM7XX_PWM_CTRL_CH2_MODE_BIT | \
> + NPCM7XX_PWM_CTRL_CH3_MODE_BIT)
> +
> +#define NPCM7XX_PWM_CTRL_EN_DEFALUT (NPCM7XX_PWM_CTRL_CH0_EN_BIT | \
> + NPCM7XX_PWM_CTRL_CH1_EN_BIT | \
> + NPCM7XX_PWM_CTRL_CH2_EN_BIT | \
> + NPCM7XX_PWM_CTRL_CH3_EN_BIT)
> +
> +/* NPCM7XX FAN Tacho registers */
> +#define NPCM7XX_FAN_REG_BASE(base, n) (base + ((n) * 0x1000L))
> +
> +#define NPCM7XX_FAN_REG_TCNT1(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x00)
> +#define NPCM7XX_FAN_REG_TCRA(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x02)
> +#define NPCM7XX_FAN_REG_TCRB(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x04)
> +#define NPCM7XX_FAN_REG_TCNT2(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x06)
> +#define NPCM7XX_FAN_REG_TPRSC(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x08)
> +#define NPCM7XX_FAN_REG_TCKC(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x0A)
> +#define NPCM7XX_FAN_REG_TMCTRL(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x0C)
> +#define NPCM7XX_FAN_REG_TICTRL(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x0E)
> +#define NPCM7XX_FAN_REG_TICLR(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x10)
> +#define NPCM7XX_FAN_REG_TIEN(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x12)
> +#define NPCM7XX_FAN_REG_TCPA(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x14)
> +#define NPCM7XX_FAN_REG_TCPB(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x16)
> +#define NPCM7XX_FAN_REG_TCPCFG(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x18)
> +#define NPCM7XX_FAN_REG_TINASEL(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x1A)
> +#define NPCM7XX_FAN_REG_TINBSEL(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x1C)
> +
> +#define NPCM7XX_FAN_TCKC_CLKX_NONE 0
> +#define NPCM7XX_FAN_TCKC_CLK1_APB BIT(0)
> +#define NPCM7XX_FAN_TCKC_CLK2_APB BIT(3)
> +
> +#define NPCM7XX_FAN_TMCTRL_TBEN BIT(6)
> +#define NPCM7XX_FAN_TMCTRL_TAEN BIT(5)
> +#define NPCM7XX_FAN_TMCTRL_TBEDG BIT(4)
> +#define NPCM7XX_FAN_TMCTRL_TAEDG BIT(3)
> +#define NPCM7XX_FAN_TMCTRL_MODE_5 BIT(2)
> +
> +#define NPCM7XX_FAN_TICLR_CLEAR_ALL GENMASK(5, 0)
> +#define NPCM7XX_FAN_TICLR_TFCLR BIT(5)
> +#define NPCM7XX_FAN_TICLR_TECLR BIT(4)
> +#define NPCM7XX_FAN_TICLR_TDCLR BIT(3)
> +#define NPCM7XX_FAN_TICLR_TCCLR BIT(2)
> +#define NPCM7XX_FAN_TICLR_TBCLR BIT(1)
> +#define NPCM7XX_FAN_TICLR_TACLR BIT(0)
> +
> +#define NPCM7XX_FAN_TIEN_ENABLE_ALL GENMASK(5, 0)
> +#define NPCM7XX_FAN_TIEN_TFIEN BIT(5)
> +#define NPCM7XX_FAN_TIEN_TEIEN BIT(4)
> +#define NPCM7XX_FAN_TIEN_TDIEN BIT(3)
> +#define NPCM7XX_FAN_TIEN_TCIEN BIT(2)
> +#define NPCM7XX_FAN_TIEN_TBIEN BIT(1)
> +#define NPCM7XX_FAN_TIEN_TAIEN BIT(0)
> +
> +#define NPCM7XX_FAN_TICTRL_TFPND BIT(5)
> +#define NPCM7XX_FAN_TICTRL_TEPND BIT(4)
> +#define NPCM7XX_FAN_TICTRL_TDPND BIT(3)
> +#define NPCM7XX_FAN_TICTRL_TCPND BIT(2)
> +#define NPCM7XX_FAN_TICTRL_TBPND BIT(1)
> +#define NPCM7XX_FAN_TICTRL_TAPND BIT(0)
> +
> +#define NPCM7XX_FAN_TCPCFG_HIBEN BIT(7)
> +#define NPCM7XX_FAN_TCPCFG_EQBEN BIT(6)
> +#define NPCM7XX_FAN_TCPCFG_LOBEN BIT(5)
> +#define NPCM7XX_FAN_TCPCFG_CPBSEL BIT(4)
> +#define NPCM7XX_FAN_TCPCFG_HIAEN BIT(3)
> +#define NPCM7XX_FAN_TCPCFG_EQAEN BIT(2)
> +#define NPCM7XX_FAN_TCPCFG_LOAEN BIT(1)
> +#define NPCM7XX_FAN_TCPCFG_CPASEL BIT(0)
> +
> +/* FAN General Defintion */
> +/* Define the maximum FAN channel number */
> +#define NPCM7XX_FAN_MAX_MODULE 8
> +#define NPCM7XX_FAN_MAX_CHN_NUM_IN_A_MODULE 2
> +#define NPCM7XX_FAN_MAX_CHN_NUM 16
> +
> +/*
> + * Get Fan Tach Timeout (base on clock 214843.75Hz, 1 cnt = 4.654us)
> + * Timeout 94ms ~= 0x5000
> + * (The minimum FAN speed could to support ~640RPM/pulse 1,
> + * 320RPM/pulse 2, ...-- 10.6Hz)
> + */
> +#define NPCM7XX_FAN_TIMEOUT 0x5000
> +#define NPCM7XX_FAN_TCNT 0xFFFF
> +#define NPCM7XX_FAN_TCPA (NPCM7XX_FAN_TCNT - NPCM7XX_FAN_TIMEOUT)
> +#define NPCM7XX_FAN_TCPB (NPCM7XX_FAN_TCNT - NPCM7XX_FAN_TIMEOUT)
> +
> +#define NPCM7XX_FAN_POLL_TIMER_200MS 200
> +#define NPCM7XX_FAN_DEFAULT_PULSE_PER_REVOLUTION 2
> +#define NPCM7XX_FAN_TINASEL_FANIN_DEFAULT 0
> +#define NPCM7XX_FAN_CLK_PRESCALE 255
> +
> +#define NPCM7XX_FAN_CMPA 0
> +#define NPCM7XX_FAN_CMPB 1
> +
> +/* Obtain the fan number */
> +#define NPCM7XX_FAN_INPUT(fan, cmp) ((fan << 1) + (cmp))
> +
> +/* fan sample status */
> +#define FAN_DISABLE 0xFF
> +#define FAN_INIT 0x00
> +#define FAN_PREPARE_TO_GET_FIRST_CAPTURE 0x01
> +#define FAN_ENOUGH_SAMPLE 0x02
> +
> +struct Fan_Dev {
> + u8 FanStFlag;
> + u8 FanPlsPerRev;
> + u16 FanCnt;
> + u32 FanCntTemp;
> +};
> +
> +struct npcm7xx_cooling_device {
> + char name[THERMAL_NAME_LENGTH];
> + struct npcm7xx_pwm_fan_data *data;
> + struct thermal_cooling_device *tcdev;
> + int pwm_port;
> + u8 *cooling_levels;
> + u8 max_state;
> + u8 cur_state;
> +};
> +
> +struct npcm7xx_pwm_fan_data {
> + void __iomem *pwm_base, *fan_base;
> + unsigned long pwm_clk_freq, fan_clk_freq;
> + struct clk *pwm_clk, *fan_clk;
> + struct mutex npcm7xx_pwm_lock[NPCM7XX_PWM_MAX_MODULES];
> + spinlock_t npcm7xx_fan_lock[NPCM7XX_FAN_MAX_MODULE];
> + int fan_irq[NPCM7XX_FAN_MAX_MODULE];
> + bool pwm_present[NPCM7XX_PWM_MAX_CHN_NUM],
> + fan_present[NPCM7XX_FAN_MAX_CHN_NUM];
Separate declarations please.
> + u32 InputClkFreq;
No CamelCase.
> + struct timer_list npcm7xx_fan_timer;
> + struct Fan_Dev npcm7xx_fan[NPCM7XX_FAN_MAX_CHN_NUM];
> + struct npcm7xx_cooling_device *cdev[NPCM7XX_PWM_MAX_CHN_NUM];
> + u8 npcm7xx_fan_select;
> +};
> +
> +static int npcm7xx_pwm_config_set(struct npcm7xx_pwm_fan_data *data,
> + int channel, u16 val)
> +{
> + u32 PWMCh = (channel % NPCM7XX_PWM_MAX_CHN_NUM_IN_A_MODULE);
> + u32 module = (channel / NPCM7XX_PWM_MAX_CHN_NUM_IN_A_MODULE);
> + u32 u32TmpBuf = 0, ctrl_en_bit, env_bit;
The u32 in u32TmpBuf is quite unnecessary.
> +
> + /*
> + * Config PWM Comparator register for setting duty cycle
> + */
> + if (val < 0 || val > NPCM7XX_PWM_COMPARATOR_MAX)
> + return -EINVAL;
How can a u32 ever be < 0 ?
> +
> + mutex_lock(&data->npcm7xx_pwm_lock[module]);
> +
> + /* write new CMR value */
> + iowrite32(val, NPCM7XX_PWM_REG_CMRx(data->pwm_base, module, PWMCh));
> + u32TmpBuf = ioread32(NPCM7XX_PWM_REG_CR(data->pwm_base, module));
> +
> + switch (PWMCh) {
> + case 0:
> + ctrl_en_bit = NPCM7XX_PWM_CTRL_CH0_EN_BIT;
> + env_bit = NPCM7XX_PWM_CTRL_CH0_INV_BIT;
> + break;
> + case 1:
> + ctrl_en_bit = NPCM7XX_PWM_CTRL_CH1_EN_BIT;
> + env_bit = NPCM7XX_PWM_CTRL_CH1_INV_BIT;
> + break;
> + case 2:
> + ctrl_en_bit = NPCM7XX_PWM_CTRL_CH2_EN_BIT;
> + env_bit = NPCM7XX_PWM_CTRL_CH2_INV_BIT;
> + break;
> + case 3:
> + ctrl_en_bit = NPCM7XX_PWM_CTRL_CH3_EN_BIT;
> + env_bit = NPCM7XX_PWM_CTRL_CH3_INV_BIT;
> + break;
> + default:
> + return -ENODEV;
> + }
> +
> + if (val == 0) {
> + /* Disable PWM */
> + u32TmpBuf &= ~(ctrl_en_bit);
Unnecessary ( )
> + u32TmpBuf |= env_bit;
> + } else {
> + /* Enable PWM */
> + u32TmpBuf |= ctrl_en_bit;
> + u32TmpBuf &= ~(env_bit);
Unnecessary ( )
> + }
> +
> + iowrite32(u32TmpBuf, NPCM7XX_PWM_REG_CR(data->pwm_base, module));
> + mutex_unlock(&data->npcm7xx_pwm_lock[module]);
> +
> + return 0;
> +}
> +
> +static inline void npcm7xx_fan_start_capture(struct npcm7xx_pwm_fan_data *data,
> + u8 fan, u8 cmp)
> +{
> + u8 fan_id = 0;
> + u8 reg_mode = 0;
> + u8 reg_int = 0;
> + unsigned long flags;
> +
> + fan_id = NPCM7XX_FAN_INPUT(fan, cmp);
> +
> + /* to check whether any fan tach is enable */
> + if (data->npcm7xx_fan[fan_id].FanStFlag != FAN_DISABLE) {
> + /* reset status */
> + spin_lock_irqsave(&data->npcm7xx_fan_lock[fan], flags);
> +
> + data->npcm7xx_fan[fan_id].FanStFlag = FAN_INIT;
> + reg_int = ioread8(NPCM7XX_FAN_REG_TIEN(data->fan_base, fan));
> +
> + if (cmp == NPCM7XX_FAN_CMPA) {
> + /* enable interrupt */
> + iowrite8((u8) (reg_int | (NPCM7XX_FAN_TIEN_TAIEN |
> + NPCM7XX_FAN_TIEN_TEIEN)),
Is the (u8) typecast really necessary ? Seems unlikely.
> + NPCM7XX_FAN_REG_TIEN(data->fan_base, fan));
> +
> + reg_mode = NPCM7XX_FAN_TCKC_CLK1_APB
> + | ioread8(NPCM7XX_FAN_REG_TCKC(data->fan_base,
> + fan));
> +
> + /* start to Capture */
> + iowrite8(reg_mode, NPCM7XX_FAN_REG_TCKC(data->fan_base,
> + fan));
> + } else {
> + /* enable interrupt */
> + iowrite8((u8) (reg_int | (NPCM7XX_FAN_TIEN_TBIEN |
> + NPCM7XX_FAN_TIEN_TFIEN)),
Same as above.
> + NPCM7XX_FAN_REG_TIEN(data->fan_base, fan));
> +
> + reg_mode =
> + NPCM7XX_FAN_TCKC_CLK2_APB
> + | ioread8(NPCM7XX_FAN_REG_TCKC(data->fan_base,
> + fan));
> +
> + /* start to Capture */
> + iowrite8(reg_mode,
> + NPCM7XX_FAN_REG_TCKC(data->fan_base, fan));
> + }
> +
> + spin_unlock_irqrestore(&data->npcm7xx_fan_lock[fan], flags);
> + }
> +}
> +
> +/*
> + * Enable a background timer to poll fan tach value, (200ms * 4)
> + * to polling all fan
> + */
> +static void npcm7xx_fan_polling(struct timer_list *t)
> +{
> + struct npcm7xx_pwm_fan_data *data;
> + unsigned long flags;
> + int i;
> +
> + data = from_timer(data, t, npcm7xx_fan_timer);
> +
> + /*
> + * Polling two module per one round,
> + * FAN01 & FAN89 / FAN23 & FAN1011 / FAN45 & FAN1213 / FAN67 & FAN1415
> + */
> + for (i = data->npcm7xx_fan_select; i < NPCM7XX_FAN_MAX_MODULE;
> + i = i+4) {
i + 4
> + /* clear the flag and reset the counter (TCNT) */
> + spin_lock_irqsave(&data->npcm7xx_fan_lock[i], flags);
> + iowrite8((u8) NPCM7XX_FAN_TICLR_CLEAR_ALL,
> + NPCM7XX_FAN_REG_TICLR(data->fan_base, i));
> + spin_unlock_irqrestore(&data->npcm7xx_fan_lock[i], flags);
> +
> + if (data->fan_present[i*2] == true) {
> + spin_lock_irqsave(&data->npcm7xx_fan_lock[i], flags);
> + iowrite16(NPCM7XX_FAN_TCNT,
> + NPCM7XX_FAN_REG_TCNT1(data->fan_base, i));
> + spin_unlock_irqrestore(&data->npcm7xx_fan_lock[i],
> + flags);
> + npcm7xx_fan_start_capture(data, i, NPCM7XX_FAN_CMPA);
> + }
> + if (data->fan_present[(i*2)+1] == true) {
(i * 2) + 1
same everywhere - add spaces
> + spin_lock_irqsave(&data->npcm7xx_fan_lock[i], flags);
> + iowrite16(NPCM7XX_FAN_TCNT,
> + NPCM7XX_FAN_REG_TCNT2(data->fan_base, i));
> + spin_unlock_irqrestore(&data->npcm7xx_fan_lock[i],
> + flags);
> + npcm7xx_fan_start_capture(data, i, NPCM7XX_FAN_CMPB);
> + }
> + }
> +
> + data->npcm7xx_fan_select++;
> + data->npcm7xx_fan_select &= 0x3;
> +
> + /* reset the timer interval */
> + data->npcm7xx_fan_timer.expires = jiffies +
> + msecs_to_jiffies(NPCM7XX_FAN_POLL_TIMER_200MS);
> + add_timer(&data->npcm7xx_fan_timer);
> +}
> +
> +static inline void npcm7xx_fan_compute(struct npcm7xx_pwm_fan_data *data,
> + u8 fan, u8 cmp, u8 fan_id,
> + u8 flag_int, u8 flag_mode,
> + u8 flag_clear)
> +{
> + u8 reg_int = 0;
> + u8 reg_mode = 0;
> + u16 fan_cap = 0;
> +
> + if (cmp == NPCM7XX_FAN_CMPA)
> + fan_cap = ioread16(NPCM7XX_FAN_REG_TCRA(data->fan_base, fan));
> + else
> + fan_cap = ioread16(NPCM7XX_FAN_REG_TCRB(data->fan_base, fan));
> +
> + /* clear capature flag, H/W will auto reset the NPCM7XX_FAN_TCNTx */
> + iowrite8((u8) flag_clear, NPCM7XX_FAN_REG_TICLR(data->fan_base, fan));
> +
Now this typecast is definitely unnecessary. Please check the entire code
and drop all unnecessary typecasts.
> + if (data->npcm7xx_fan[fan_id].FanStFlag == FAN_INIT) {
> + /* First capture, drop it */
> + data->npcm7xx_fan[fan_id].FanStFlag =
> + FAN_PREPARE_TO_GET_FIRST_CAPTURE;
> +
> + /* reset counter */
> + data->npcm7xx_fan[fan_id].FanCntTemp = 0;
> + } else if (data->npcm7xx_fan[fan_id].FanStFlag <
> + FAN_ENOUGH_SAMPLE) {
> + /*
> + * collect the enough sample,
> + * (ex: 2 pulse fan need to get 2 sample)
> + */
> + data->npcm7xx_fan[fan_id].FanCntTemp +=
> + (NPCM7XX_FAN_TCNT - fan_cap);
> +
> + data->npcm7xx_fan[fan_id].FanStFlag++;
> + } else {
> + /* get enough sample or fan disable */
> + if (data->npcm7xx_fan[fan_id].FanStFlag == FAN_ENOUGH_SAMPLE) {
> + data->npcm7xx_fan[fan_id].FanCntTemp +=
> + (NPCM7XX_FAN_TCNT - fan_cap);
> +
> + /* compute finial average cnt per pulse */
> + data->npcm7xx_fan[fan_id].FanCnt
> + = data->npcm7xx_fan[fan_id].FanCntTemp /
> + FAN_ENOUGH_SAMPLE;
> +
> + data->npcm7xx_fan[fan_id].FanStFlag = FAN_INIT;
> + }
> +
> + reg_int = ioread8(NPCM7XX_FAN_REG_TIEN(data->fan_base, fan));
> +
> + /* disable interrupt */
> + iowrite8((u8) (reg_int & ~flag_int),
> + NPCM7XX_FAN_REG_TIEN(data->fan_base, fan));
> + reg_mode = ioread8(NPCM7XX_FAN_REG_TCKC(data->fan_base, fan));
> +
> + /* stop capturing */
> + iowrite8((u8) (reg_mode & ~flag_mode),
> + NPCM7XX_FAN_REG_TCKC(data->fan_base, fan));
> +
> + }
> +}
> +
> +static inline void npcm7xx_check_cmp(struct npcm7xx_pwm_fan_data *data,
> + u8 fan, u8 cmp, u8 flag)
> +{
> + u8 reg_int = 0;
> + u8 reg_mode = 0;
> + u8 flag_timeout;
> + u8 flag_cap;
> + u8 flag_clear;
> + u8 flag_int;
> + u8 flag_mode;
> + u8 fan_id;
> +
> + fan_id = NPCM7XX_FAN_INPUT(fan, cmp);
> +
> + if (cmp == NPCM7XX_FAN_CMPA) {
> + flag_cap = NPCM7XX_FAN_TICTRL_TAPND;
> + flag_timeout = NPCM7XX_FAN_TICTRL_TEPND;
> + flag_int = NPCM7XX_FAN_TIEN_TAIEN | NPCM7XX_FAN_TIEN_TEIEN;
> + flag_mode = NPCM7XX_FAN_TCKC_CLK1_APB;
> + flag_clear = NPCM7XX_FAN_TICLR_TACLR | NPCM7XX_FAN_TICLR_TECLR;
> + } else {
> + flag_cap = NPCM7XX_FAN_TICTRL_TBPND;
> + flag_timeout = NPCM7XX_FAN_TICTRL_TFPND;
> + flag_int = NPCM7XX_FAN_TIEN_TBIEN | NPCM7XX_FAN_TIEN_TFIEN;
> + flag_mode = NPCM7XX_FAN_TCKC_CLK2_APB;
> + flag_clear = NPCM7XX_FAN_TICLR_TBCLR | NPCM7XX_FAN_TICLR_TFCLR;
> + }
> +
> +
> + if (flag & flag_timeout) {
> +
> + reg_int = ioread8(NPCM7XX_FAN_REG_TIEN(data->fan_base, fan));
> +
> + /* disable interrupt */
> + iowrite8((u8) (reg_int & ~flag_int),
> + NPCM7XX_FAN_REG_TIEN(data->fan_base, fan));
> +
> + /* clear interrup flag */
> + iowrite8((u8) flag_clear, NPCM7XX_FAN_REG_TICLR(data->fan_base,
> + fan));
> +
> + reg_mode = ioread8(NPCM7XX_FAN_REG_TCKC(data->fan_base, fan));
> +
> + /* stop capturing */
> + iowrite8((u8) (reg_mode & ~flag_mode),
> + NPCM7XX_FAN_REG_TCKC(data->fan_base, fan));
> +
> + /*
> + * If timeout occurs (NPCM7XX_FAN_TIMEOUT), the fan doesn't
> + * connect or speed is lower than 10.6Hz (320RPM/pulse2).
> + * In these situation, the RPM output should be zero.
> + */
> + data->npcm7xx_fan[fan_id].FanCnt = 0;
> + //DEBUG_MSG("%s : it is timeout fan_id %d\n", __func__, fan_id);
> + } else {
> + /* input capture is occurred */
> + if (flag & flag_cap)
> + npcm7xx_fan_compute(data, fan, cmp, fan_id, flag_int,
> + flag_mode, flag_clear);
> + }
> +}
> +
> +static irqreturn_t npcm7xx_fan_isr(int irq, void *dev_id)
> +{
> + struct npcm7xx_pwm_fan_data *data = dev_id;
> + u8 flag = 0;
> + int module;
> + unsigned long flags;
> +
> + module = irq - data->fan_irq[0];
> + spin_lock_irqsave(&data->npcm7xx_fan_lock[module], flags);
> +
> + flag = ioread8(NPCM7XX_FAN_REG_TICTRL(data->fan_base, module));
> + if (flag > 0) {
> + npcm7xx_check_cmp(data, module, NPCM7XX_FAN_CMPA, flag);
> + npcm7xx_check_cmp(data, module, NPCM7XX_FAN_CMPB, flag);
> + spin_unlock_irqrestore(&data->npcm7xx_fan_lock[module], flags);
> + return IRQ_HANDLED;
> + }
> +
> + spin_unlock_irqrestore(&data->npcm7xx_fan_lock[module], flags);
> +
> + return IRQ_NONE;
> +}
> +
> +static int npcm7xx_read_pwm(struct device *dev, u32 attr, int channel,
> + long *val)
> +{
> + struct npcm7xx_pwm_fan_data *data = dev_get_drvdata(dev);
> + u32 PWMCh = (channel % NPCM7XX_PWM_MAX_CHN_NUM_IN_A_MODULE);
> + u32 module = (channel / NPCM7XX_PWM_MAX_CHN_NUM_IN_A_MODULE);
> +
> + if (IS_ERR(data))
> + return PTR_ERR(data);
When would this ever be an ERR_PTR ?
> +
> + switch (attr) {
> + case hwmon_pwm_input:
> + mutex_lock(&data->npcm7xx_pwm_lock[module]);
> + *val = (long)ioread32(
Another unnecessary typecast.
> + NPCM7XX_PWM_REG_CMRx(data->pwm_base, module, PWMCh));
> + mutex_unlock(&data->npcm7xx_pwm_lock[module]);
What is this lock for ?
> + return 0;
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static int npcm7xx_write_pwm(struct device *dev, u32 attr, int channel,
> + long val)
> +{
> + struct npcm7xx_pwm_fan_data *data = dev_get_drvdata(dev);
> + int err = 0;
> +
> + switch (attr) {
> + case hwmon_pwm_input:
> + err = npcm7xx_pwm_config_set(data, channel, (u16)val);
If val == 0x10000 this is curring off the upper bit(s), and converts
negative values into large positive ones.
The validation in npcm7xx_pwm_config_set() is thus quite pointless.
> + break;
> + default:
> + err = -EOPNOTSUPP;
> + break;
> + }
> +
> + return err;
> +}
> +
> +static umode_t npcm7xx_pwm_is_visible(const void *_data, u32 attr, int channel)
> +{
> + const struct npcm7xx_pwm_fan_data *data = _data;
> +
> + if (!data->pwm_present[channel])
> + return 0;
> +
> + switch (attr) {
> + case hwmon_pwm_input:
> + return 0644;
> + default:
> + return 0;
> + }
> +}
> +
> +static int npcm7xx_read_fan(struct device *dev, u32 attr, int channel,
> + long *val)
> +{
> + struct npcm7xx_pwm_fan_data *data = dev_get_drvdata(dev);
> +
> + switch (attr) {
> + case hwmon_fan_input:
> + *val = 0;
> + if (data->npcm7xx_fan[channel].FanCnt <= 0)
> + return data->npcm7xx_fan[channel].FanCnt;
> +
> + /* Convert the raw reading to RPM */
> + if ((data->npcm7xx_fan[channel].FanCnt > 0) &&
> + data->npcm7xx_fan[channel].FanPlsPerRev > 0)
> + *val = (long)((data->InputClkFreq * 60)/
> + (data->npcm7xx_fan[channel].FanCnt *
> + data->npcm7xx_fan[channel].FanPlsPerRev));
> + return 0;
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static umode_t npcm7xx_fan_is_visible(const void *_data, u32 attr, int channel)
> +{
> + const struct npcm7xx_pwm_fan_data *data = _data;
> +
> + if (!data->fan_present[channel])
> + return 0;
> +
> + switch (attr) {
> + case hwmon_fan_input:
> + return 0444;
> + default:
> + return 0;
> + }
> +}
> +
> +static int npcm7xx_read(struct device *dev, enum hwmon_sensor_types type,
> + u32 attr, int channel, long *val)
> +{
> + switch (type) {
> + case hwmon_pwm:
> + return npcm7xx_read_pwm(dev, attr, channel, val);
> + case hwmon_fan:
> + return npcm7xx_read_fan(dev, attr, channel, val);
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static int npcm7xx_write(struct device *dev, enum hwmon_sensor_types type,
> + u32 attr, int channel, long val)
> +{
> + switch (type) {
> + case hwmon_pwm:
> + return npcm7xx_write_pwm(dev, attr, channel, val);
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static umode_t npcm7xx_is_visible(const void *data,
> + enum hwmon_sensor_types type,
> + u32 attr, int channel)
> +{
> + switch (type) {
> + case hwmon_pwm:
> + return npcm7xx_pwm_is_visible(data, attr, channel);
> + case hwmon_fan:
> + return npcm7xx_fan_is_visible(data, attr, channel);
> + default:
> + return 0;
> + }
> +}
> +
> +static const u32 npcm7xx_pwm_config[] = {
> + HWMON_PWM_INPUT,
> + HWMON_PWM_INPUT,
> + HWMON_PWM_INPUT,
> + HWMON_PWM_INPUT,
> + HWMON_PWM_INPUT,
> + HWMON_PWM_INPUT,
> + HWMON_PWM_INPUT,
> + HWMON_PWM_INPUT,
> + 0
> +};
> +
> +static const struct hwmon_channel_info npcm7xx_pwm = {
> + .type = hwmon_pwm,
> + .config = npcm7xx_pwm_config,
> +};
> +
> +static const u32 npcm7xx_fan_config[] = {
> + HWMON_F_INPUT,
> + HWMON_F_INPUT,
> + HWMON_F_INPUT,
> + HWMON_F_INPUT,
> + HWMON_F_INPUT,
> + HWMON_F_INPUT,
> + HWMON_F_INPUT,
> + HWMON_F_INPUT,
> + HWMON_F_INPUT,
> + HWMON_F_INPUT,
> + HWMON_F_INPUT,
> + HWMON_F_INPUT,
> + HWMON_F_INPUT,
> + HWMON_F_INPUT,
> + HWMON_F_INPUT,
> + HWMON_F_INPUT,
> + 0
> +};
> +
> +static const struct hwmon_channel_info npcm7xx_fan = {
> + .type = hwmon_fan,
> + .config = npcm7xx_fan_config,
> +};
> +
> +static const struct hwmon_channel_info *npcm7xx_info[] = {
> + &npcm7xx_pwm,
> + &npcm7xx_fan,
> + NULL
> +};
> +
> +static const struct hwmon_ops npcm7xx_hwmon_ops = {
> + .is_visible = npcm7xx_is_visible,
> + .read = npcm7xx_read,
> + .write = npcm7xx_write,
> +};
> +
> +static const struct hwmon_chip_info npcm7xx_chip_info = {
> + .ops = &npcm7xx_hwmon_ops,
> + .info = npcm7xx_info,
> +};
> +
> +static u32 npcm7xx_pwm_init(struct npcm7xx_pwm_fan_data *data)
> +{
> + int m, ch;
> + u32 Prescale_val, output_freq;
> +
> + data->pwm_clk_freq = clk_get_rate(data->pwm_clk);
> +
> + /* Adjust NPCM7xx PWMs output frequency to ~25Khz */
> + output_freq = data->pwm_clk_freq / PWN_CNT_DEFAULT;
> + Prescale_val = DIV_ROUND_CLOSEST(output_freq, PWM_OUTPUT_FREQ_25KHZ);
> +
> + /* If Prescale_val = 0, then the prescale output clock is stopped */
> + if (Prescale_val < MIN_PRESCALE1)
> + Prescale_val = MIN_PRESCALE1;
> + /*
> + * Prescale_val need to decrement in one because in the PWM Prescale
> + * register the Prescale value increment by one
> + */
> + Prescale_val--;
> +
> + /* Setting PWM Prescale Register value register to both modules */
> + Prescale_val |= (Prescale_val << NPCM7XX_PWM_PRESCALE_SHIFT_CH01);
> +
> + for (m = 0; m < NPCM7XX_PWM_MAX_MODULES ; m++) {
> + iowrite32(Prescale_val, NPCM7XX_PWM_REG_PR(data->pwm_base, m));
> + iowrite32(NPCM7XX_PWM_PRESCALE2_DEFALUT,
> + NPCM7XX_PWM_REG_CSR(data->pwm_base, m));
> + iowrite32(NPCM7XX_PWM_CTRL_MODE_DEFALUT,
> + NPCM7XX_PWM_REG_CR(data->pwm_base, m));
> +
> + for (ch = 0; ch < NPCM7XX_PWM_MAX_CHN_NUM_IN_A_MODULE; ch++) {
> + iowrite32(NPCM7XX_PWM_COUNTER_DEFALUT_NUM,
> + NPCM7XX_PWM_REG_CNRx(data->pwm_base, m, ch));
> + }
> + }
> +
> + return output_freq / ((Prescale_val & 0xf) + 1);
> +}
> +
> +static void npcm7xx_fan_init(struct npcm7xx_pwm_fan_data *data)
> +{
> + int md;
> + int ch;
> + int i;
> + u32 apb_clk_freq;
> +
> + for (md = 0; md < NPCM7XX_FAN_MAX_MODULE; md++) {
> +
> + /* stop FAN0~7 clock */
> + iowrite8((u8)NPCM7XX_FAN_TCKC_CLKX_NONE,
> + NPCM7XX_FAN_REG_TCKC(data->fan_base, md));
> +
> + /* disable all interrupt */
> + iowrite8((u8) 0x00, NPCM7XX_FAN_REG_TIEN(data->fan_base, md));
> +
> + /* clear all interrupt */
> + iowrite8((u8) NPCM7XX_FAN_TICLR_CLEAR_ALL,
> + NPCM7XX_FAN_REG_TICLR(data->fan_base, md));
> +
> + /* set FAN0~7 clock prescaler */
> + iowrite8((u8) NPCM7XX_FAN_CLK_PRESCALE,
> + NPCM7XX_FAN_REG_TPRSC(data->fan_base, md));
> +
> + /* set FAN0~7 mode (high-to-low transition) */
> + iowrite8(
> + (u8) (
> + NPCM7XX_FAN_TMCTRL_MODE_5 |
> + NPCM7XX_FAN_TMCTRL_TBEN |
> + NPCM7XX_FAN_TMCTRL_TAEN
> + ),
> + NPCM7XX_FAN_REG_TMCTRL(data->fan_base, md)
> + );
> +
> + /* set FAN0~7 Initial Count/Cap */
> + iowrite16(NPCM7XX_FAN_TCNT,
> + NPCM7XX_FAN_REG_TCNT1(data->fan_base, md));
> + iowrite16(NPCM7XX_FAN_TCNT,
> + NPCM7XX_FAN_REG_TCNT2(data->fan_base, md));
> +
> + /* set FAN0~7 compare (equal to count) */
> + iowrite8((u8)(NPCM7XX_FAN_TCPCFG_EQAEN |
> + NPCM7XX_FAN_TCPCFG_EQBEN),
> + NPCM7XX_FAN_REG_TCPCFG(data->fan_base, md));
> +
> + /* set FAN0~7 compare value */
> + iowrite16(NPCM7XX_FAN_TCPA,
> + NPCM7XX_FAN_REG_TCPA(data->fan_base, md));
> + iowrite16(NPCM7XX_FAN_TCPB,
> + NPCM7XX_FAN_REG_TCPB(data->fan_base, md));
> +
> + /* set FAN0~7 fan input FANIN 0~15 */
> + iowrite8((u8) NPCM7XX_FAN_TINASEL_FANIN_DEFAULT,
> + NPCM7XX_FAN_REG_TINASEL(data->fan_base, md));
> + iowrite8((u8) NPCM7XX_FAN_TINASEL_FANIN_DEFAULT,
> + NPCM7XX_FAN_REG_TINBSEL(data->fan_base, md));
> +
> + for (i = 0; i < NPCM7XX_FAN_MAX_CHN_NUM_IN_A_MODULE; i++) {
> + ch = md * NPCM7XX_FAN_MAX_CHN_NUM_IN_A_MODULE + i;
> + data->npcm7xx_fan[ch].FanStFlag = FAN_DISABLE;
> + data->npcm7xx_fan[ch].FanPlsPerRev =
> + NPCM7XX_FAN_DEFAULT_PULSE_PER_REVOLUTION;
> + data->npcm7xx_fan[ch].FanCnt = 0;
> + }
> + }
> +
> + apb_clk_freq = clk_get_rate(data->fan_clk);
> +
> + /* Fan tach input clock = APB clock / prescalar, default is 255. */
> + data->InputClkFreq = apb_clk_freq / (NPCM7XX_FAN_CLK_PRESCALE + 1);
> +}
> +
> +static int
> +npcm7xx_pwm_cz_get_max_state(struct thermal_cooling_device *tcdev,
> + unsigned long *state)
> +{
> + struct npcm7xx_cooling_device *cdev = tcdev->devdata;
> +
> + *state = cdev->max_state;
> +
> + return 0;
> +}
> +
> +static int
> +npcm7xx_pwm_cz_get_cur_state(struct thermal_cooling_device *tcdev,
> + unsigned long *state)
> +{
> + struct npcm7xx_cooling_device *cdev = tcdev->devdata;
> +
> + *state = cdev->cur_state;
> +
> + return 0;
> +}
> +
> +static int
> +npcm7xx_pwm_cz_set_cur_state(struct thermal_cooling_device *tcdev,
> + unsigned long state)
> +{
> + struct npcm7xx_cooling_device *cdev = tcdev->devdata;
> + int ret;
> +
> + if (state > cdev->max_state)
> + return -EINVAL;
> +
> + cdev->cur_state = state;
> + ret = npcm7xx_pwm_config_set(cdev->data, cdev->pwm_port,
> + cdev->cooling_levels[cdev->cur_state]);
> +
> + return ret;
> +}
> +
> +static const struct thermal_cooling_device_ops npcm7xx_pwm_cool_ops = {
> + .get_max_state = npcm7xx_pwm_cz_get_max_state,
> + .get_cur_state = npcm7xx_pwm_cz_get_cur_state,
> + .set_cur_state = npcm7xx_pwm_cz_set_cur_state,
> +};
> +
> +static int npcm7xx_create_pwm_cooling(struct device *dev,
> + struct device_node *child,
> + struct npcm7xx_pwm_fan_data *data,
> + u32 pwm_port, u8 num_levels)
> +{
> + int ret;
> + struct npcm7xx_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, THERMAL_NAME_LENGTH, "%s%d", child->name,
> + pwm_port);
> +
> + cdev->tcdev = thermal_of_cooling_device_register(child,
> + cdev->name,
> + cdev,
> + &npcm7xx_pwm_cool_ops);
> + if (IS_ERR(cdev->tcdev))
> + return PTR_ERR(cdev->tcdev);
> +
> + cdev->data = data;
> + cdev->pwm_port = pwm_port;
> +
> + data->cdev[pwm_port] = cdev;
> +
> + return 0;
> +}
> +
> +static int npcm7xx_en_pwm_fan(struct device *dev,
> + struct device_node *child,
> + struct npcm7xx_pwm_fan_data *data)
> +{
> + u8 *fan_ch;
> + u8 pwm_port;
> + int ret, fan_cnt;
> + u8 index, ch;
> +
> + ret = of_property_read_u8(child, "pwm-ch", &pwm_port);
> + if (ret)
> + return ret;
> +
> + data->pwm_present[pwm_port] = true;
> + ret = npcm7xx_pwm_config_set(data, pwm_port,
> + NPCM7XX_PWM_COMPARATOR_DEFALUT_NUM);
> +
> + ret = of_property_count_u8_elems(child, "cooling-levels");
> + if (ret > 0) {
> + ret = npcm7xx_create_pwm_cooling(dev, child, data, pwm_port,
> + ret);
> + if (ret)
> + return ret;
> + }
> +
> + fan_cnt = of_property_count_u8_elems(child, "fan-ch");
> + if (fan_cnt < 1)
> + return -EINVAL;
> +
> + fan_ch = devm_kzalloc(dev, sizeof(*fan_ch) * fan_cnt, GFP_KERNEL);
> + if (!fan_ch)
> + return -ENOMEM;
> +
> + ret = of_property_read_u8_array(child, "fan-ch", fan_ch, fan_cnt);
> + if (ret)
> + return ret;
> +
> + for (ch = 0; ch < fan_cnt; ch++) {
> + index = fan_ch[ch];
> + data->fan_present[index] = true;
> + data->npcm7xx_fan[index].FanStFlag = FAN_INIT;
> + }
> +
> + return 0;
> +}
> +
> +static int npcm7xx_pwm_fan_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct device_node *np, *child;
> + struct npcm7xx_pwm_fan_data *data;
> + struct resource *res;
> + struct device *hwmon;
> + char name[20];
> + int ret, cnt;
> + u32 output_freq;
> + u32 i;
> +
> + np = dev->of_node;
> +
> + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "pwm_base");
> + if (res == NULL) {
> + pr_err("PWM of_address_to_resource fail ret %d\n", ret);
Everywhere: pr_XXX -> dev_XXX.
ret is a random number here.
> + return -ENODEV;
> + }
Unnecessary error message. devm_ioremap_resource() checks for that and displays
an error message.
> +
> + data->pwm_base = devm_ioremap_resource(dev, res);
> + pr_debug("pwm base is 0x%08X, res.start 0x%08X , size 0x%08X\n",
> + (u32)data->pwm_base, res->start, resource_size(res));
No need to reinvent %pR.
> + if (!data->pwm_base) {
> + pr_err("pwm probe failed: can't read pwm base address\n");
devm_ioremap_resource() returns an ERR_PTR. The error message is wrong anyway.
This can be EINVAL, EBUSY, or ENOMEM.
> + return -ENOMEM;
... and the reported error should be returned to the caller.
> + }
> +
> + data->pwm_clk = devm_clk_get(dev, "clk_apb3");
> + if (IS_ERR(data->pwm_clk)) {
> + pr_err(" pwm probe failed: can't read clk.\n");
> + return -ENODEV;
return PTR_ERR(data->pwm_clk);
> + }
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "fan_base");
> + if (ret) {
ret is a random number. Have you tested this code ?
> + pr_err("fan of_address_to_resource fail ret %d\n", ret);
Another random number.
> + return -ENODEV;
> + }
> +
> + data->fan_base = devm_ioremap_resource(dev, res);
> + pr_debug("fan base is 0x%08X, res.start 0x%08X , size 0x%08X\n",
> + (u32)data->fan_base, res->start, resource_size(res));
%pR
> +
> + if (!data->fan_base) {
devm_ioremap_resource() returns PTR_ERR()
> + pr_err("fan probe failed: can't read fan base address.\n");
> + return -ENOMEM;
> + }
> +
> + data->fan_clk = devm_clk_get(dev, "clk_apb4");
> + if (IS_ERR(data->fan_clk)) {
> + pr_err(" FAN probe failed: can't read clk.\n");
> + return -ENODEV;
PTR_ERR()
> + }
> +
> + output_freq = npcm7xx_pwm_init(data);
> + npcm7xx_fan_init(data);
> +
> + for (cnt = 0; cnt < NPCM7XX_PWM_MAX_MODULES ; cnt++)
> + mutex_init(&data->npcm7xx_pwm_lock[cnt]);
> +
> + for (i = 0; i < NPCM7XX_FAN_MAX_MODULE; i++) {
> + spin_lock_init(&data->npcm7xx_fan_lock[i]);
> +
> + data->fan_irq[i] = platform_get_irq(pdev, i);
> + if (!data->fan_irq[i]) {
> + pr_err("%s - failed to map irq %d\n", __func__, i);
> + ret = -EAGAIN;
> + goto err_irq;
> + }
> +
> + sprintf(name, "NPCM7XX-FAN-MD%d", i);
> +
> + if (request_irq(data->fan_irq[i], npcm7xx_fan_isr, 0, name,
No remove function. Either use devm_request_irq() or declare one.
> + (void *)data)) {
> + pr_err("NPCM7XX: register irq FAN%d failed\n", i);
> + ret = -EAGAIN;
This is supposed to work next time ? Why ?
> + goto err_irq;
> + }
> + }
> +
> + for_each_child_of_node(np, child) {
> + ret = npcm7xx_en_pwm_fan(dev, child, data);
> + if (ret) {
> + pr_err("npcm7xx_en_pwm_fan failed ret %d\n", ret);
> + of_node_put(child);
> + goto err_irq;
> + }
> + }
> +
> + hwmon = devm_hwmon_device_register_with_info(dev, "npcm7xx_pwm_fan",
> + data, &npcm7xx_chip_info,
> + NULL);
> + if (IS_ERR(hwmon)) {
> + pr_err("PWM Driver failed - devm_hwmon_device_register_with_groups failed\n");
> + ret = PTR_ERR(hwmon);
> + goto err_irq;
> + }
> +
> + for (i = 0; i < NPCM7XX_FAN_MAX_CHN_NUM; i++) {
> + if (data->fan_present[i] == true) {
> + /* fan timer initialization */
> + data->npcm7xx_fan_select = 0;
> + data->npcm7xx_fan_timer.expires = jiffies +
> + msecs_to_jiffies(NPCM7XX_FAN_POLL_TIMER_200MS);
> + timer_setup(&data->npcm7xx_fan_timer,
> + npcm7xx_fan_polling, 0);
> + add_timer(&data->npcm7xx_fan_timer);
> + break;
> + }
> + }
> +
> + pr_info("NPCM7XX PWM-FAN Driver probed, output Freq %dHz[PWM], input Freq %dHz[FAN]\n",
> + output_freq, data->InputClkFreq);
> +
> + return 0;
> +
> +err_irq:
> + for (; i > 0; i--)
> + free_irq(data->fan_irq[i-1], (void *)data);
> +
> + return ret;
> +}
> +
> +static const struct of_device_id of_pwm_fan_match_table[] = {
> + { .compatible = "nuvoton,npcm750-pwm-fan", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, of_pwm_fan_match_table);
> +
> +static struct platform_driver npcm7xx_pwm_fan_driver = {
> + .probe = npcm7xx_pwm_fan_probe,
> + .driver = {
> + .name = "npcm7xx_pwm_fan",
> + .of_match_table = of_pwm_fan_match_table,
> + },
> +};
> +
> +module_platform_driver(npcm7xx_pwm_fan_driver);
> +
> +MODULE_DESCRIPTION("Nuvoton NPCM7XX PWM and Fan Tacho driver");
> +MODULE_AUTHOR("Tomer Maimon <tomer.maimon@nuvoton.com>");
> +MODULE_LICENSE("GPL v2");
> --
> 2.14.1
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] hwmon: npcm750: add NPCM7xx PWM and Fan driver
2018-06-20 16:48 ` Guenter Roeck
@ 2018-06-20 18:25 ` Joe Perches
2018-06-20 19:38 ` Guenter Roeck
2018-06-21 13:17 ` Julia Lawall
0 siblings, 2 replies; 11+ messages in thread
From: Joe Perches @ 2018-06-20 18:25 UTC (permalink / raw)
To: Guenter Roeck, Tomer Maimon, Julia Lawall, cocci
Cc: robh+dt, mark.rutland, jdelvare, avifishman70, yuenn,
brendanhiggins, venture, joel, devicetree, linux-kernel,
linux-hwmon, openbmc
(adding Julia Lawall and cocci mailing list)
On Wed, 2018-06-20 at 09:48 -0700, Guenter Roeck wrote:
[]
> > +static inline void npcm7xx_fan_start_capture(struct npcm7xx_pwm_fan_data *data,
> > + u8 fan, u8 cmp)
> > +{
> > + u8 fan_id = 0;
> > + u8 reg_mode = 0;
> > + u8 reg_int = 0;
> > + unsigned long flags;
> > +
> > + fan_id = NPCM7XX_FAN_INPUT(fan, cmp);
> > +
> > + /* to check whether any fan tach is enable */
> > + if (data->npcm7xx_fan[fan_id].FanStFlag != FAN_DISABLE) {
> > + /* reset status */
> > + spin_lock_irqsave(&data->npcm7xx_fan_lock[fan], flags);
> > +
> > + data->npcm7xx_fan[fan_id].FanStFlag = FAN_INIT;
> > + reg_int = ioread8(NPCM7XX_FAN_REG_TIEN(data->fan_base, fan));
> > +
> > + if (cmp == NPCM7XX_FAN_CMPA) {
> > + /* enable interrupt */
> > + iowrite8((u8) (reg_int | (NPCM7XX_FAN_TIEN_TAIEN |
> > + NPCM7XX_FAN_TIEN_TEIEN)),
>
> Is the (u8) typecast really necessary ? Seems unlikely.
The cast is not really necessary here as there would
be an implicit cast already.
Some might complain about loss of type safety and
"make W=123" would probably emit something here.
But casts to the same type are not necessary.
A possible coccinelle script to find casts to the
same type is below, but there are some false positives
for things like __force and __user casts
Also, spatch (1.0.4) seems to have a defect for this
when the type is used in operations that change a
smaller type to int or unsigned int.
i.e.: (offset is u16, but offset * 2 is int)
While running the cocci script below:
HANDLING: drivers/net/ethernet/intel/igb/e1000_nvm.c
diff =
diff -u -p a/drivers/net/drivers/net/ethernet/intel/igb/e1000_nvm.c b/drivers/net/ethernet/intel/igb/e1000_nvm.c
--- a/drivers/net/ethernet/intel/igb/e1000_nvm.c
+++ b/drivers/net/ethernet/intel/igb/e1000_nvm.c
@@ -335,7 +335,7 @@ s32 igb_read_nvm_spi(struct e1000_hw *hw
/* Send the READ command (opcode + addr) */
igb_shift_out_eec_bits(hw, read_opcode, nvm->opcode_bits);
- igb_shift_out_eec_bits(hw, (u16)(offset*2), nvm->address_bits);
+ igb_shift_out_eec_bits(hw, (offset * 2), nvm->address_bits);
/* Read the data. SPI NVMs increment the address with each byte
* read and will roll over if reading beyond the end. This allows
---
Anyway, here's the cocci script:
$ cat same_typecast.cocci
@@
type T;
T foo;
@@
- (T *)&foo
+ &foo
@@
type T;
T foo;
@@
- (T)foo
+ foo
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] hwmon: npcm750: add NPCM7xx PWM and Fan driver
2018-06-20 18:25 ` Joe Perches
@ 2018-06-20 19:38 ` Guenter Roeck
2018-06-21 13:17 ` Julia Lawall
1 sibling, 0 replies; 11+ messages in thread
From: Guenter Roeck @ 2018-06-20 19:38 UTC (permalink / raw)
To: Joe Perches
Cc: Tomer Maimon, Julia Lawall, cocci, robh+dt, mark.rutland,
jdelvare, avifishman70, yuenn, brendanhiggins, venture, joel,
devicetree, linux-kernel, linux-hwmon, openbmc
On Wed, Jun 20, 2018 at 11:25:08AM -0700, Joe Perches wrote:
> (adding Julia Lawall and cocci mailing list)
>
> On Wed, 2018-06-20 at 09:48 -0700, Guenter Roeck wrote:
> []
> > > +static inline void npcm7xx_fan_start_capture(struct npcm7xx_pwm_fan_data *data,
> > > + u8 fan, u8 cmp)
> > > +{
> > > + u8 fan_id = 0;
> > > + u8 reg_mode = 0;
> > > + u8 reg_int = 0;
> > > + unsigned long flags;
> > > +
> > > + fan_id = NPCM7XX_FAN_INPUT(fan, cmp);
> > > +
> > > + /* to check whether any fan tach is enable */
> > > + if (data->npcm7xx_fan[fan_id].FanStFlag != FAN_DISABLE) {
> > > + /* reset status */
> > > + spin_lock_irqsave(&data->npcm7xx_fan_lock[fan], flags);
> > > +
> > > + data->npcm7xx_fan[fan_id].FanStFlag = FAN_INIT;
> > > + reg_int = ioread8(NPCM7XX_FAN_REG_TIEN(data->fan_base, fan));
> > > +
> > > + if (cmp == NPCM7XX_FAN_CMPA) {
> > > + /* enable interrupt */
> > > + iowrite8((u8) (reg_int | (NPCM7XX_FAN_TIEN_TAIEN |
> > > + NPCM7XX_FAN_TIEN_TEIEN)),
> >
> > Is the (u8) typecast really necessary ? Seems unlikely.
>
> The cast is not really necessary here as there would
> be an implicit cast already.
>
> Some might complain about loss of type safety and
> "make W=123" would probably emit something here.
>
I spent (wasted) some time browsing through the kernel.
Similar typecasts are only used if there is a real type change.
A warning here would not make sense unless NPCM7XX_FAN_TIEN_TAIEN
or NPCM7XX_FAN_TIEN_TEIEN would be outside the u8 range, and then
there would be one anyway.
So, no, I am not going to accept those typecasts. They just make the code
more difficult to read. For example, the code here could have been
simplified to something like
reg_int = ioread8(NPCM7XX_FAN_REG_TIEN(data->fan_base, fan));
reg_mode = ioread8(NPCM7XX_FAN_REG_TCKC(data->fan_base, fan));
if (cmp == NPCM7XX_FAN_CMPA) {
reg_int |= NPCM7XX_FAN_TIEN_TAIEN | NPCM7XX_FAN_TIEN_TEIEN;
reg_mode |= NPCM7XX_FAN_TCKC_CLK1_APB;
} else {
reg_int |= NPCM7XX_FAN_TIEN_TBIEN | NPCM7XX_FAN_TIEN_TFIEN;
reg_mode |= NPCM7XX_FAN_TCKC_CLK2_APB;
}
iowrite8(reg_int, NPCM7XX_FAN_REG_TIEN(data->fan_base, fan);
iowrite8(reg_mode, NPCM7XX_FAN_REG_TCKC(data->fan_base, fan);
This, in turn, leads to the question if it is really not necessary
to _clear_ those mask bits in the same context.
Guenter
> But casts to the same type are not necessary.
>
> A possible coccinelle script to find casts to the
> same type is below, but there are some false positives
> for things like __force and __user casts
>
> Also, spatch (1.0.4) seems to have a defect for this
> when the type is used in operations that change a
> smaller type to int or unsigned int.
>
> i.e.: (offset is u16, but offset * 2 is int)
>
> While running the cocci script below:
>
> HANDLING: drivers/net/ethernet/intel/igb/e1000_nvm.c
> diff =
> diff -u -p a/drivers/net/drivers/net/ethernet/intel/igb/e1000_nvm.c b/drivers/net/ethernet/intel/igb/e1000_nvm.c
> --- a/drivers/net/ethernet/intel/igb/e1000_nvm.c
> +++ b/drivers/net/ethernet/intel/igb/e1000_nvm.c
> @@ -335,7 +335,7 @@ s32 igb_read_nvm_spi(struct e1000_hw *hw
>
> /* Send the READ command (opcode + addr) */
> igb_shift_out_eec_bits(hw, read_opcode, nvm->opcode_bits);
> - igb_shift_out_eec_bits(hw, (u16)(offset*2), nvm->address_bits);
> + igb_shift_out_eec_bits(hw, (offset * 2), nvm->address_bits);
>
> /* Read the data. SPI NVMs increment the address with each byte
> * read and will roll over if reading beyond the end. This allows
>
> ---
>
> Anyway, here's the cocci script:
>
> $ cat same_typecast.cocci
> @@
> type T;
> T foo;
> @@
>
> - (T *)&foo
> + &foo
>
> @@
> type T;
> T foo;
> @@
>
> - (T)foo
> + foo
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] hwmon: npcm750: add NPCM7xx PWM and Fan driver
2018-06-20 18:25 ` Joe Perches
2018-06-20 19:38 ` Guenter Roeck
@ 2018-06-21 13:17 ` Julia Lawall
2018-06-21 14:48 ` Joe Perches
1 sibling, 1 reply; 11+ messages in thread
From: Julia Lawall @ 2018-06-21 13:17 UTC (permalink / raw)
To: Joe Perches
Cc: Guenter Roeck, Tomer Maimon, cocci, robh+dt, mark.rutland,
jdelvare, avifishman70, yuenn, brendanhiggins, venture, joel,
devicetree, linux-kernel, linux-hwmon, openbmc
On Wed, 20 Jun 2018, Joe Perches wrote:
> (adding Julia Lawall and cocci mailing list)
>
> On Wed, 2018-06-20 at 09:48 -0700, Guenter Roeck wrote:
> []
> > > +static inline void npcm7xx_fan_start_capture(struct npcm7xx_pwm_fan_data *data,
> > > + u8 fan, u8 cmp)
> > > +{
> > > + u8 fan_id = 0;
> > > + u8 reg_mode = 0;
> > > + u8 reg_int = 0;
> > > + unsigned long flags;
> > > +
> > > + fan_id = NPCM7XX_FAN_INPUT(fan, cmp);
> > > +
> > > + /* to check whether any fan tach is enable */
> > > + if (data->npcm7xx_fan[fan_id].FanStFlag != FAN_DISABLE) {
> > > + /* reset status */
> > > + spin_lock_irqsave(&data->npcm7xx_fan_lock[fan], flags);
> > > +
> > > + data->npcm7xx_fan[fan_id].FanStFlag = FAN_INIT;
> > > + reg_int = ioread8(NPCM7XX_FAN_REG_TIEN(data->fan_base, fan));
> > > +
> > > + if (cmp == NPCM7XX_FAN_CMPA) {
> > > + /* enable interrupt */
> > > + iowrite8((u8) (reg_int | (NPCM7XX_FAN_TIEN_TAIEN |
> > > + NPCM7XX_FAN_TIEN_TEIEN)),
> >
> > Is the (u8) typecast really necessary ? Seems unlikely.
>
> The cast is not really necessary here as there would
> be an implicit cast already.
>
> Some might complain about loss of type safety and
> "make W=123" would probably emit something here.
>
> But casts to the same type are not necessary.
>
> A possible coccinelle script to find casts to the
> same type is below, but there are some false positives
> for things like __force and __user casts
>
> Also, spatch (1.0.4) seems to have a defect for this
> when the type is used in operations that change a
> smaller type to int or unsigned int.
>
> i.e.: (offset is u16, but offset * 2 is int)
Ah. The rule is that the result type is always the larger one?
Unfortunately, Coccinelle doesn't know the size of any type. I could add
some special cases, but that may be more confusing than helpful.
julia
>
> While running the cocci script below:
>
> HANDLING: drivers/net/ethernet/intel/igb/e1000_nvm.c
> diff =
> diff -u -p a/drivers/net/drivers/net/ethernet/intel/igb/e1000_nvm.c b/drivers/net/ethernet/intel/igb/e1000_nvm.c
> --- a/drivers/net/ethernet/intel/igb/e1000_nvm.c
> +++ b/drivers/net/ethernet/intel/igb/e1000_nvm.c
> @@ -335,7 +335,7 @@ s32 igb_read_nvm_spi(struct e1000_hw *hw
>
> /* Send the READ command (opcode + addr) */
> igb_shift_out_eec_bits(hw, read_opcode, nvm->opcode_bits);
> - igb_shift_out_eec_bits(hw, (u16)(offset*2), nvm->address_bits);
> + igb_shift_out_eec_bits(hw, (offset * 2), nvm->address_bits);
>
> /* Read the data. SPI NVMs increment the address with each byte
> * read and will roll over if reading beyond the end. This allows
>
> ---
>
> Anyway, here's the cocci script:
>
> $ cat same_typecast.cocci
> @@
> type T;
> T foo;
> @@
>
> - (T *)&foo
> + &foo
>
> @@
> type T;
> T foo;
> @@
>
> - (T)foo
> + foo
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] hwmon: npcm750: add NPCM7xx PWM and Fan driver
2018-06-19 10:53 ` [PATCH v2 2/2] hwmon: npcm750: add NPCM7xx PWM and Fan driver Tomer Maimon
` (2 preceding siblings ...)
2018-06-20 16:48 ` Guenter Roeck
@ 2018-06-21 13:46 ` Guenter Roeck
3 siblings, 0 replies; 11+ messages in thread
From: Guenter Roeck @ 2018-06-21 13:46 UTC (permalink / raw)
To: Tomer Maimon, robh+dt, mark.rutland, jdelvare, avifishman70,
yuenn, brendanhiggins, venture, joel
Cc: devicetree, linux-kernel, linux-hwmon, openbmc
On 06/19/2018 03:53 AM, Tomer Maimon wrote:
> Add Nuvoton BMC NPCM7xx Pulse Width Modulation (PWM) and Fan tacho driver.
>
> The Nuvoton BMC NPCM7xx has two identical PWM controller modules,
> each module has four PWM controller outputs and eight identical Fan
> controller modules, each module has two Fan controller inputs.
>
> The driver provides a sysfs entries through which the user can
> configure the duty-cycle value (ranging from 0 to 100 percent)
> and read the fan tacho rpm value.
>
> The driver support cooling device creation, that could be bound
> to a thermal zone for the thermal control.
>
You ignored most of my previous comments. Why should I review your code
if you ignore most of the feedback ?
> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> ---
> drivers/hwmon/Kconfig | 7 +
> drivers/hwmon/Makefile | 1 +
> drivers/hwmon/npcm750-pwm-fan.c | 1099 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 1107 insertions(+)
> create mode 100644 drivers/hwmon/npcm750-pwm-fan.c
>
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index f10840ad465c..f6c2eff9bb7d 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -1888,6 +1888,13 @@ config SENSORS_XGENE
> If you say yes here you get support for the temperature
> and power sensors for APM X-Gene SoC.
>
> +config SENSORS_NPCM7XX
> + tristate "Nuvoton NPCM7XX PWM and Fan driver"
> + depends on THERMAL || THERMAL=n
> + help
> + This driver provides support for Nuvoton NPCM7XX PWM and Fan
> + controllers.
> +
> if ACPI
>
> comment "ACPI drivers"
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index e7d52a36e6c4..004e2ec5b68f 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -174,6 +174,7 @@ obj-$(CONFIG_SENSORS_W83L786NG) += w83l786ng.o
> obj-$(CONFIG_SENSORS_WM831X) += wm831x-hwmon.o
> obj-$(CONFIG_SENSORS_WM8350) += wm8350-hwmon.o
> obj-$(CONFIG_SENSORS_XGENE) += xgene-hwmon.o
> +obj-$(CONFIG_SENSORS_NPCM7XX) += npcm750-pwm-fan.o
>
> obj-$(CONFIG_PMBUS) += pmbus/
>
> diff --git a/drivers/hwmon/npcm750-pwm-fan.c b/drivers/hwmon/npcm750-pwm-fan.c
> new file mode 100644
> index 000000000000..82898197686f
> --- /dev/null
> +++ b/drivers/hwmon/npcm750-pwm-fan.c
> @@ -0,0 +1,1099 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2014-2018 Nuvoton Technology corporation.
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/device.h>
> +#include <linux/clk.h>
> +#include <linux/platform_device.h>
> +#include <linux/interrupt.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/sysfs.h>
> +#include <linux/spinlock.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_address.h>
> +#include <linux/thermal.h>
> +
I am sure I asked for those to be in alphabetic order.
> +/* NPCM7XX PWM registers */
> +#define NPCM7XX_PWM_REG_BASE(base, n) (base + ((n) * 0x1000L))
> +
(base)
> +#define NPCM7XX_PWM_REG_PR(base, n) (NPCM7XX_PWM_REG_BASE(base, n) + 0x00)
> +#define NPCM7XX_PWM_REG_CSR(base, n) (NPCM7XX_PWM_REG_BASE(base, n) + 0x04)
> +#define NPCM7XX_PWM_REG_CR(base, n) (NPCM7XX_PWM_REG_BASE(base, n) + 0x08)
> +#define NPCM7XX_PWM_REG_CNRx(base, n, ch) \
> + (NPCM7XX_PWM_REG_BASE(base, n) + 0x0C + (12 * ch))
> +#define NPCM7XX_PWM_REG_CMRx(base, n, ch) \
> + (NPCM7XX_PWM_REG_BASE(base, n) + 0x10 + (12 * ch))
> +#define NPCM7XX_PWM_REG_PDRx(base, n, ch) \
> + (NPCM7XX_PWM_REG_BASE(base, n) + 0x14 + (12 * ch))
> +#define NPCM7XX_PWM_REG_PIER(base, n) (NPCM7XX_PWM_REG_BASE(base, n) + 0x3C)
> +#define NPCM7XX_PWM_REG_PIIR(base, n) (NPCM7XX_PWM_REG_BASE(base, n) + 0x40)
> +#define NPCM7XX_FAN_REG_BASE(base, n) (base + ((n) * 0x1000L))
> +
(base)
> +#define NPCM7XX_PWM_CTRL_CH0_MODE_BIT BIT(3)
> +#define NPCM7XX_PWM_CTRL_CH1_MODE_BIT BIT(11)
> +#define NPCM7XX_PWM_CTRL_CH2_MODE_BIT BIT(15)
> +#define NPCM7XX_PWM_CTRL_CH3_MODE_BIT BIT(19)
> +
> +#define NPCM7XX_PWM_CTRL_CH0_INV_BIT BIT(2)
> +#define NPCM7XX_PWM_CTRL_CH1_INV_BIT BIT(10)
> +#define NPCM7XX_PWM_CTRL_CH2_INV_BIT BIT(14)
> +#define NPCM7XX_PWM_CTRL_CH3_INV_BIT BIT(18)
> +
> +#define NPCM7XX_PWM_CTRL_CH0_EN_BIT BIT(0)
> +#define NPCM7XX_PWM_CTRL_CH1_EN_BIT BIT(8)
> +#define NPCM7XX_PWM_CTRL_CH2_EN_BIT BIT(12)
> +#define NPCM7XX_PWM_CTRL_CH3_EN_BIT BIT(16)
> +
> +/* Define the maximum PWM channel number */
> +#define NPCM7XX_PWM_MAX_CHN_NUM 8
> +#define NPCM7XX_PWM_MAX_CHN_NUM_IN_A_MODULE 4
> +#define NPCM7XX_PWM_MAX_MODULES 2
> +
> +/* Define the Counter Register, value = 100 for match 100% */
> +#define NPCM7XX_PWM_COUNTER_DEFALUT_NUM 255
> +#define NPCM7XX_PWM_COMPARATOR_DEFALUT_NUM 127
> +
> +#define NPCM7XX_PWM_COMPARATOR_MAX 255
> +
> +/* default all PWM channels PRESCALE2 = 1 */
> +#define NPCM7XX_PWM_PRESCALE2_DEFALUT_CH0 0x4
> +#define NPCM7XX_PWM_PRESCALE2_DEFALUT_CH1 0x40
> +#define NPCM7XX_PWM_PRESCALE2_DEFALUT_CH2 0x400
> +#define NPCM7XX_PWM_PRESCALE2_DEFALUT_CH3 0x4000
> +
> +#define PWM_OUTPUT_FREQ_25KHZ 25000
> +#define PWN_CNT_DEFAULT 256
> +#define MIN_PRESCALE1 2
> +#define NPCM7XX_PWM_PRESCALE_SHIFT_CH01 8
> +
> +#define NPCM7XX_PWM_PRESCALE2_DEFALUT (NPCM7XX_PWM_PRESCALE2_DEFALUT_CH0 | \
> + NPCM7XX_PWM_PRESCALE2_DEFALUT_CH1 | \
> + NPCM7XX_PWM_PRESCALE2_DEFALUT_CH2 | \
> + NPCM7XX_PWM_PRESCALE2_DEFALUT_CH3)
> +
> +#define NPCM7XX_PWM_CTRL_MODE_DEFALUT (NPCM7XX_PWM_CTRL_CH0_MODE_BIT | \
> + NPCM7XX_PWM_CTRL_CH1_MODE_BIT | \
> + NPCM7XX_PWM_CTRL_CH2_MODE_BIT | \
> + NPCM7XX_PWM_CTRL_CH3_MODE_BIT)
> +
> +#define NPCM7XX_PWM_CTRL_EN_DEFALUT (NPCM7XX_PWM_CTRL_CH0_EN_BIT | \
> + NPCM7XX_PWM_CTRL_CH1_EN_BIT | \
> + NPCM7XX_PWM_CTRL_CH2_EN_BIT | \
> + NPCM7XX_PWM_CTRL_CH3_EN_BIT)
> +
:1,$s/DEFALUT/DEFAULT/g
Are those defines used anywhere ?
> +/* NPCM7XX FAN Tacho registers */
> +#define NPCM7XX_FAN_REG_BASE(base, n) (base + ((n) * 0x1000L))
> +
(base)
> +#define NPCM7XX_FAN_REG_TCNT1(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x00)
> +#define NPCM7XX_FAN_REG_TCRA(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x02)
> +#define NPCM7XX_FAN_REG_TCRB(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x04)
> +#define NPCM7XX_FAN_REG_TCNT2(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x06)
> +#define NPCM7XX_FAN_REG_TPRSC(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x08)
> +#define NPCM7XX_FAN_REG_TCKC(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x0A)
> +#define NPCM7XX_FAN_REG_TMCTRL(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x0C)
> +#define NPCM7XX_FAN_REG_TICTRL(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x0E)
> +#define NPCM7XX_FAN_REG_TICLR(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x10)
> +#define NPCM7XX_FAN_REG_TIEN(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x12)
> +#define NPCM7XX_FAN_REG_TCPA(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x14)
> +#define NPCM7XX_FAN_REG_TCPB(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x16)
> +#define NPCM7XX_FAN_REG_TCPCFG(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x18)
> +#define NPCM7XX_FAN_REG_TINASEL(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x1A)
> +#define NPCM7XX_FAN_REG_TINBSEL(base, n) (NPCM7XX_FAN_REG_BASE(base, n) + 0x1C)
> +
> +#define NPCM7XX_FAN_TCKC_CLKX_NONE 0
> +#define NPCM7XX_FAN_TCKC_CLK1_APB BIT(0)
> +#define NPCM7XX_FAN_TCKC_CLK2_APB BIT(3)
> +
> +#define NPCM7XX_FAN_TMCTRL_TBEN BIT(6)
> +#define NPCM7XX_FAN_TMCTRL_TAEN BIT(5)
> +#define NPCM7XX_FAN_TMCTRL_TBEDG BIT(4)
> +#define NPCM7XX_FAN_TMCTRL_TAEDG BIT(3)
> +#define NPCM7XX_FAN_TMCTRL_MODE_5 BIT(2)
> +
> +#define NPCM7XX_FAN_TICLR_CLEAR_ALL GENMASK(5, 0)
> +#define NPCM7XX_FAN_TICLR_TFCLR BIT(5)
> +#define NPCM7XX_FAN_TICLR_TECLR BIT(4)
> +#define NPCM7XX_FAN_TICLR_TDCLR BIT(3)
> +#define NPCM7XX_FAN_TICLR_TCCLR BIT(2)
> +#define NPCM7XX_FAN_TICLR_TBCLR BIT(1)
> +#define NPCM7XX_FAN_TICLR_TACLR BIT(0)
> +
> +#define NPCM7XX_FAN_TIEN_ENABLE_ALL GENMASK(5, 0)
> +#define NPCM7XX_FAN_TIEN_TFIEN BIT(5)
> +#define NPCM7XX_FAN_TIEN_TEIEN BIT(4)
> +#define NPCM7XX_FAN_TIEN_TDIEN BIT(3)
> +#define NPCM7XX_FAN_TIEN_TCIEN BIT(2)
> +#define NPCM7XX_FAN_TIEN_TBIEN BIT(1)
> +#define NPCM7XX_FAN_TIEN_TAIEN BIT(0)
> +
> +#define NPCM7XX_FAN_TICTRL_TFPND BIT(5)
> +#define NPCM7XX_FAN_TICTRL_TEPND BIT(4)
> +#define NPCM7XX_FAN_TICTRL_TDPND BIT(3)
> +#define NPCM7XX_FAN_TICTRL_TCPND BIT(2)
> +#define NPCM7XX_FAN_TICTRL_TBPND BIT(1)
> +#define NPCM7XX_FAN_TICTRL_TAPND BIT(0)
> +
> +#define NPCM7XX_FAN_TCPCFG_HIBEN BIT(7)
> +#define NPCM7XX_FAN_TCPCFG_EQBEN BIT(6)
> +#define NPCM7XX_FAN_TCPCFG_LOBEN BIT(5)
> +#define NPCM7XX_FAN_TCPCFG_CPBSEL BIT(4)
> +#define NPCM7XX_FAN_TCPCFG_HIAEN BIT(3)
> +#define NPCM7XX_FAN_TCPCFG_EQAEN BIT(2)
> +#define NPCM7XX_FAN_TCPCFG_LOAEN BIT(1)
> +#define NPCM7XX_FAN_TCPCFG_CPASEL BIT(0)
> +
> +/* FAN General Defintion */
> +/* Define the maximum FAN channel number */
> +#define NPCM7XX_FAN_MAX_MODULE 8
> +#define NPCM7XX_FAN_MAX_CHN_NUM_IN_A_MODULE 2
> +#define NPCM7XX_FAN_MAX_CHN_NUM 16
> +
> +/*
> + * Get Fan Tach Timeout (base on clock 214843.75Hz, 1 cnt = 4.654us)
> + * Timeout 94ms ~= 0x5000
> + * (The minimum FAN speed could to support ~640RPM/pulse 1,
> + * 320RPM/pulse 2, ...-- 10.6Hz)
> + */
> +#define NPCM7XX_FAN_TIMEOUT 0x5000
> +#define NPCM7XX_FAN_TCNT 0xFFFF
> +#define NPCM7XX_FAN_TCPA (NPCM7XX_FAN_TCNT - NPCM7XX_FAN_TIMEOUT)
> +#define NPCM7XX_FAN_TCPB (NPCM7XX_FAN_TCNT - NPCM7XX_FAN_TIMEOUT)
> +
> +#define NPCM7XX_FAN_POLL_TIMER_200MS 200
> +#define NPCM7XX_FAN_DEFAULT_PULSE_PER_REVOLUTION 2
> +#define NPCM7XX_FAN_TINASEL_FANIN_DEFAULT 0
> +#define NPCM7XX_FAN_CLK_PRESCALE 255
> +
> +#define NPCM7XX_FAN_CMPA 0
> +#define NPCM7XX_FAN_CMPB 1
> +
> +/* Obtain the fan number */
> +#define NPCM7XX_FAN_INPUT(fan, cmp) ((fan << 1) + (cmp))
> +
(fan)
If you want to be intelligent about () in macros, please at least add () where
a parameter is used in an expression.
> +/* fan sample status */
> +#define FAN_DISABLE 0xFF
> +#define FAN_INIT 0x00
> +#define FAN_PREPARE_TO_GET_FIRST_CAPTURE 0x01
> +#define FAN_ENOUGH_SAMPLE 0x02
> +
> +struct Fan_Dev {
> + u8 FanStFlag;
> + u8 FanPlsPerRev;
> + u16 FanCnt;
> + u32 FanCntTemp;
> +};
> +
> +struct npcm7xx_cooling_device {
> + char name[THERMAL_NAME_LENGTH];
> + struct npcm7xx_pwm_fan_data *data;
> + struct thermal_cooling_device *tcdev;
> + int pwm_port;
> + u8 *cooling_levels;
> + u8 max_state;
> + u8 cur_state;
> +};
> +
> +struct npcm7xx_pwm_fan_data {
> + void __iomem *pwm_base, *fan_base;
> + unsigned long pwm_clk_freq, fan_clk_freq;
> + struct clk *pwm_clk, *fan_clk;
> + struct mutex npcm7xx_pwm_lock[NPCM7XX_PWM_MAX_MODULES];
> + spinlock_t npcm7xx_fan_lock[NPCM7XX_FAN_MAX_MODULE];
> + int fan_irq[NPCM7XX_FAN_MAX_MODULE];
> + bool pwm_present[NPCM7XX_PWM_MAX_CHN_NUM],
> + fan_present[NPCM7XX_FAN_MAX_CHN_NUM];
I am sure I asked to split the declarations.
> + u32 InputClkFreq;
> + struct timer_list npcm7xx_fan_timer;
> + struct Fan_Dev npcm7xx_fan[NPCM7XX_FAN_MAX_CHN_NUM];
> + struct npcm7xx_cooling_device *cdev[NPCM7XX_PWM_MAX_CHN_NUM];
> + u8 npcm7xx_fan_select;
> +};
> +
> +static int npcm7xx_pwm_config_set(struct npcm7xx_pwm_fan_data *data,
> + int channel, u16 val)
> +{
> + u32 PWMCh = (channel % NPCM7XX_PWM_MAX_CHN_NUM_IN_A_MODULE);
> + u32 module = (channel / NPCM7XX_PWM_MAX_CHN_NUM_IN_A_MODULE);
> + u32 u32TmpBuf = 0, ctrl_en_bit, env_bit;
> +
I am sure I asked you to drop the u32 from this variable name, and I am sure
I asked not to use capital letters in variable names.
> + /*
> + * Config PWM Comparator register for setting duty cycle
> + */
> + if (val < 0 || val > NPCM7XX_PWM_COMPARATOR_MAX)
> + return -EINVAL;
> +
> + mutex_lock(&data->npcm7xx_pwm_lock[module]);
> +
> + /* write new CMR value */
> + iowrite32(val, NPCM7XX_PWM_REG_CMRx(data->pwm_base, module, PWMCh));
> + u32TmpBuf = ioread32(NPCM7XX_PWM_REG_CR(data->pwm_base, module));
> +
> + switch (PWMCh) {
> + case 0:
> + ctrl_en_bit = NPCM7XX_PWM_CTRL_CH0_EN_BIT;
> + env_bit = NPCM7XX_PWM_CTRL_CH0_INV_BIT;
> + break;
> + case 1:
> + ctrl_en_bit = NPCM7XX_PWM_CTRL_CH1_EN_BIT;
> + env_bit = NPCM7XX_PWM_CTRL_CH1_INV_BIT;
> + break;
> + case 2:
> + ctrl_en_bit = NPCM7XX_PWM_CTRL_CH2_EN_BIT;
> + env_bit = NPCM7XX_PWM_CTRL_CH2_INV_BIT;
> + break;
> + case 3:
> + ctrl_en_bit = NPCM7XX_PWM_CTRL_CH3_EN_BIT;
> + env_bit = NPCM7XX_PWM_CTRL_CH3_INV_BIT;
> + break;
> + default:
> + return -ENODEV;
> + }
> +
> + if (val == 0) {
> + /* Disable PWM */
> + u32TmpBuf &= ~(ctrl_en_bit);
> + u32TmpBuf |= env_bit;
> + } else {
> + /* Enable PWM */
> + u32TmpBuf |= ctrl_en_bit;
> + u32TmpBuf &= ~(env_bit);
> + }
> +
> + iowrite32(u32TmpBuf, NPCM7XX_PWM_REG_CR(data->pwm_base, module));
> + mutex_unlock(&data->npcm7xx_pwm_lock[module]);
> +
> + return 0;
> +}
> +
> +static inline void npcm7xx_fan_start_capture(struct npcm7xx_pwm_fan_data *data,
> + u8 fan, u8 cmp)
> +{
> + u8 fan_id = 0;
> + u8 reg_mode = 0;
> + u8 reg_int = 0;
Those initializations are unnecessary.
> + unsigned long flags;
> +
> + fan_id = NPCM7XX_FAN_INPUT(fan, cmp);
> +
> + /* to check whether any fan tach is enable */
> + if (data->npcm7xx_fan[fan_id].FanStFlag != FAN_DISABLE) {
> + /* reset status */
> + spin_lock_irqsave(&data->npcm7xx_fan_lock[fan], flags);
> +
> + data->npcm7xx_fan[fan_id].FanStFlag = FAN_INIT;
> + reg_int = ioread8(NPCM7XX_FAN_REG_TIEN(data->fan_base, fan));
> +
If the flag bits don't need to be cleared, state that here in a comment.
> + if (cmp == NPCM7XX_FAN_CMPA) {
> + /* enable interrupt */
> + iowrite8((u8) (reg_int | (NPCM7XX_FAN_TIEN_TAIEN |
> + NPCM7XX_FAN_TIEN_TEIEN)),
s/(u8)//
> + NPCM7XX_FAN_REG_TIEN(data->fan_base, fan));
> +
> + reg_mode = NPCM7XX_FAN_TCKC_CLK1_APB
> + | ioread8(NPCM7XX_FAN_REG_TCKC(data->fan_base,
> + fan));
> +
> + /* start to Capture */
> + iowrite8(reg_mode, NPCM7XX_FAN_REG_TCKC(data->fan_base,
> + fan));
> + } else {
> + /* enable interrupt */
> + iowrite8((u8) (reg_int | (NPCM7XX_FAN_TIEN_TBIEN |
> + NPCM7XX_FAN_TIEN_TFIEN)),
> + NPCM7XX_FAN_REG_TIEN(data->fan_base, fan));
> +
> + reg_mode =
> + NPCM7XX_FAN_TCKC_CLK2_APB
> + | ioread8(NPCM7XX_FAN_REG_TCKC(data->fan_base,
> + fan));
> +
> + /* start to Capture */
> + iowrite8(reg_mode,
> + NPCM7XX_FAN_REG_TCKC(data->fan_base, fan));
> + }
> +
> + spin_unlock_irqrestore(&data->npcm7xx_fan_lock[fan], flags);
> + }
> +}
> +
> +/*
> + * Enable a background timer to poll fan tach value, (200ms * 4)
> + * to polling all fan
> + */
> +static void npcm7xx_fan_polling(struct timer_list *t)
> +{
> + struct npcm7xx_pwm_fan_data *data;
> + unsigned long flags;
> + int i;
> +
> + data = from_timer(data, t, npcm7xx_fan_timer);
> +
> + /*
> + * Polling two module per one round,
> + * FAN01 & FAN89 / FAN23 & FAN1011 / FAN45 & FAN1213 / FAN67 & FAN1415
> + */
> + for (i = data->npcm7xx_fan_select; i < NPCM7XX_FAN_MAX_MODULE;
> + i = i+4) {
> + /* clear the flag and reset the counter (TCNT) */
> + spin_lock_irqsave(&data->npcm7xx_fan_lock[i], flags);
> + iowrite8((u8) NPCM7XX_FAN_TICLR_CLEAR_ALL,
> + NPCM7XX_FAN_REG_TICLR(data->fan_base, i));
> + spin_unlock_irqrestore(&data->npcm7xx_fan_lock[i], flags);
> +
> + if (data->fan_present[i*2] == true) {
> + spin_lock_irqsave(&data->npcm7xx_fan_lock[i], flags);
> + iowrite16(NPCM7XX_FAN_TCNT,
> + NPCM7XX_FAN_REG_TCNT1(data->fan_base, i));
> + spin_unlock_irqrestore(&data->npcm7xx_fan_lock[i],
> + flags);
> + npcm7xx_fan_start_capture(data, i, NPCM7XX_FAN_CMPA);
> + }
> + if (data->fan_present[(i*2)+1] == true) {
> + spin_lock_irqsave(&data->npcm7xx_fan_lock[i], flags);
> + iowrite16(NPCM7XX_FAN_TCNT,
> + NPCM7XX_FAN_REG_TCNT2(data->fan_base, i));
> + spin_unlock_irqrestore(&data->npcm7xx_fan_lock[i],
> + flags);
> + npcm7xx_fan_start_capture(data, i, NPCM7XX_FAN_CMPB);
> + }
> + }
> +
> + data->npcm7xx_fan_select++;
> + data->npcm7xx_fan_select &= 0x3;
> +
> + /* reset the timer interval */
> + data->npcm7xx_fan_timer.expires = jiffies +
> + msecs_to_jiffies(NPCM7XX_FAN_POLL_TIMER_200MS);
> + add_timer(&data->npcm7xx_fan_timer);
> +}
> +
> +static inline void npcm7xx_fan_compute(struct npcm7xx_pwm_fan_data *data,
> + u8 fan, u8 cmp, u8 fan_id,
> + u8 flag_int, u8 flag_mode,
> + u8 flag_clear)
> +{
> + u8 reg_int = 0;
> + u8 reg_mode = 0;
> + u16 fan_cap = 0;
> +
> + if (cmp == NPCM7XX_FAN_CMPA)
> + fan_cap = ioread16(NPCM7XX_FAN_REG_TCRA(data->fan_base, fan));
> + else
> + fan_cap = ioread16(NPCM7XX_FAN_REG_TCRB(data->fan_base, fan));
> +
> + /* clear capature flag, H/W will auto reset the NPCM7XX_FAN_TCNTx */
> + iowrite8((u8) flag_clear, NPCM7XX_FAN_REG_TICLR(data->fan_base, fan));
> +
> + if (data->npcm7xx_fan[fan_id].FanStFlag == FAN_INIT) {
> + /* First capture, drop it */
> + data->npcm7xx_fan[fan_id].FanStFlag =
> + FAN_PREPARE_TO_GET_FIRST_CAPTURE;
> +
> + /* reset counter */
> + data->npcm7xx_fan[fan_id].FanCntTemp = 0;
> + } else if (data->npcm7xx_fan[fan_id].FanStFlag <
> + FAN_ENOUGH_SAMPLE) {
> + /*
> + * collect the enough sample,
> + * (ex: 2 pulse fan need to get 2 sample)
> + */
> + data->npcm7xx_fan[fan_id].FanCntTemp +=
> + (NPCM7XX_FAN_TCNT - fan_cap);
> +
> + data->npcm7xx_fan[fan_id].FanStFlag++;
> + } else {
> + /* get enough sample or fan disable */
> + if (data->npcm7xx_fan[fan_id].FanStFlag == FAN_ENOUGH_SAMPLE) {
> + data->npcm7xx_fan[fan_id].FanCntTemp +=
> + (NPCM7XX_FAN_TCNT - fan_cap);
> +
> + /* compute finial average cnt per pulse */
> + data->npcm7xx_fan[fan_id].FanCnt
> + = data->npcm7xx_fan[fan_id].FanCntTemp /
> + FAN_ENOUGH_SAMPLE;
> +
> + data->npcm7xx_fan[fan_id].FanStFlag = FAN_INIT;
> + }
> +
> + reg_int = ioread8(NPCM7XX_FAN_REG_TIEN(data->fan_base, fan));
> +
> + /* disable interrupt */
> + iowrite8((u8) (reg_int & ~flag_int),
> + NPCM7XX_FAN_REG_TIEN(data->fan_base, fan));
> + reg_mode = ioread8(NPCM7XX_FAN_REG_TCKC(data->fan_base, fan));
> +
> + /* stop capturing */
> + iowrite8((u8) (reg_mode & ~flag_mode),
> + NPCM7XX_FAN_REG_TCKC(data->fan_base, fan));
> +
> + }
> +}
> +
> +static inline void npcm7xx_check_cmp(struct npcm7xx_pwm_fan_data *data,
> + u8 fan, u8 cmp, u8 flag)
> +{
> + u8 reg_int = 0;
> + u8 reg_mode = 0;
> + u8 flag_timeout;
> + u8 flag_cap;
> + u8 flag_clear;
> + u8 flag_int;
> + u8 flag_mode;
> + u8 fan_id;
> +
> + fan_id = NPCM7XX_FAN_INPUT(fan, cmp);
> +
> + if (cmp == NPCM7XX_FAN_CMPA) {
> + flag_cap = NPCM7XX_FAN_TICTRL_TAPND;
> + flag_timeout = NPCM7XX_FAN_TICTRL_TEPND;
> + flag_int = NPCM7XX_FAN_TIEN_TAIEN | NPCM7XX_FAN_TIEN_TEIEN;
> + flag_mode = NPCM7XX_FAN_TCKC_CLK1_APB;
> + flag_clear = NPCM7XX_FAN_TICLR_TACLR | NPCM7XX_FAN_TICLR_TECLR;
> + } else {
> + flag_cap = NPCM7XX_FAN_TICTRL_TBPND;
> + flag_timeout = NPCM7XX_FAN_TICTRL_TFPND;
> + flag_int = NPCM7XX_FAN_TIEN_TBIEN | NPCM7XX_FAN_TIEN_TFIEN;
> + flag_mode = NPCM7XX_FAN_TCKC_CLK2_APB;
> + flag_clear = NPCM7XX_FAN_TICLR_TBCLR | NPCM7XX_FAN_TICLR_TFCLR;
> + }
> +
> +
> + if (flag & flag_timeout) {
> +
> + reg_int = ioread8(NPCM7XX_FAN_REG_TIEN(data->fan_base, fan));
> +
> + /* disable interrupt */
> + iowrite8((u8) (reg_int & ~flag_int),
> + NPCM7XX_FAN_REG_TIEN(data->fan_base, fan));
> +
s/(u8)//
> + /* clear interrup flag */
> + iowrite8((u8) flag_clear, NPCM7XX_FAN_REG_TICLR(data->fan_base,
> + fan));
> +
> + reg_mode = ioread8(NPCM7XX_FAN_REG_TCKC(data->fan_base, fan));
> +
> + /* stop capturing */
> + iowrite8((u8) (reg_mode & ~flag_mode),
> + NPCM7XX_FAN_REG_TCKC(data->fan_base, fan));
s/(u8)//
> +
> + /*
> + * If timeout occurs (NPCM7XX_FAN_TIMEOUT), the fan doesn't
> + * connect or speed is lower than 10.6Hz (320RPM/pulse2).
> + * In these situation, the RPM output should be zero.
> + */
> + data->npcm7xx_fan[fan_id].FanCnt = 0;
> + //DEBUG_MSG("%s : it is timeout fan_id %d\n", __func__, fan_id);
No commented out code please.
> + } else {
> + /* input capture is occurred */
> + if (flag & flag_cap)
> + npcm7xx_fan_compute(data, fan, cmp, fan_id, flag_int,
> + flag_mode, flag_clear);
> + }
> +}
> +
> +static irqreturn_t npcm7xx_fan_isr(int irq, void *dev_id)
> +{
> + struct npcm7xx_pwm_fan_data *data = dev_id;
> + u8 flag = 0;
> + int module;
> + unsigned long flags;
> +
> + module = irq - data->fan_irq[0];
> + spin_lock_irqsave(&data->npcm7xx_fan_lock[module], flags);
> +
> + flag = ioread8(NPCM7XX_FAN_REG_TICTRL(data->fan_base, module));
> + if (flag > 0) {
> + npcm7xx_check_cmp(data, module, NPCM7XX_FAN_CMPA, flag);
> + npcm7xx_check_cmp(data, module, NPCM7XX_FAN_CMPB, flag);
> + spin_unlock_irqrestore(&data->npcm7xx_fan_lock[module], flags);
> + return IRQ_HANDLED;
> + }
> +
> + spin_unlock_irqrestore(&data->npcm7xx_fan_lock[module], flags);
> +
> + return IRQ_NONE;
> +}
> +
> +static int npcm7xx_read_pwm(struct device *dev, u32 attr, int channel,
> + long *val)
> +{
> + struct npcm7xx_pwm_fan_data *data = dev_get_drvdata(dev);
> + u32 PWMCh = (channel % NPCM7XX_PWM_MAX_CHN_NUM_IN_A_MODULE);
> + u32 module = (channel / NPCM7XX_PWM_MAX_CHN_NUM_IN_A_MODULE);
> +
> + if (IS_ERR(data))
> + return PTR_ERR(data);
> +
> + switch (attr) {
> + case hwmon_pwm_input:
> + mutex_lock(&data->npcm7xx_pwm_lock[module]);
> + *val = (long)ioread32(
> + NPCM7XX_PWM_REG_CMRx(data->pwm_base, module, PWMCh));
s/(long)//
I don't think you will ever see a negative pwm value. If anything, I would
expect a mask here if the register can have upper bits set.
> + mutex_unlock(&data->npcm7xx_pwm_lock[module]);
> + return 0;
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static int npcm7xx_write_pwm(struct device *dev, u32 attr, int channel,
> + long val)
> +{
> + struct npcm7xx_pwm_fan_data *data = dev_get_drvdata(dev);
> + int err = 0;
> +
> + switch (attr) {
> + case hwmon_pwm_input:
> + err = npcm7xx_pwm_config_set(data, channel, (u16)val);
> + break;
> + default:
> + err = -EOPNOTSUPP;
> + break;
> + }
> +
> + return err;
> +}
> +
> +static umode_t npcm7xx_pwm_is_visible(const void *_data, u32 attr, int channel)
> +{
> + const struct npcm7xx_pwm_fan_data *data = _data;
> +
> + if (!data->pwm_present[channel])
> + return 0;
> +
> + switch (attr) {
> + case hwmon_pwm_input:
> + return 0644;
> + default:
> + return 0;
> + }
> +}
> +
> +static int npcm7xx_read_fan(struct device *dev, u32 attr, int channel,
> + long *val)
> +{
> + struct npcm7xx_pwm_fan_data *data = dev_get_drvdata(dev);
> +
> + switch (attr) {
> + case hwmon_fan_input:
> + *val = 0;
> + if (data->npcm7xx_fan[channel].FanCnt <= 0)
> + return data->npcm7xx_fan[channel].FanCnt;
> +
> + /* Convert the raw reading to RPM */
> + if ((data->npcm7xx_fan[channel].FanCnt > 0) &&
> + data->npcm7xx_fan[channel].FanPlsPerRev > 0)
> + *val = (long)((data->InputClkFreq * 60)/
> + (data->npcm7xx_fan[channel].FanCnt *
> + data->npcm7xx_fan[channel].FanPlsPerRev));
I see you really like typecasts. I don't. Please clean up all of them, and please
drop all commented out code.
> + return 0;
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static umode_t npcm7xx_fan_is_visible(const void *_data, u32 attr, int channel)
> +{
> + const struct npcm7xx_pwm_fan_data *data = _data;
> +
> + if (!data->fan_present[channel])
> + return 0;
> +
> + switch (attr) {
> + case hwmon_fan_input:
> + return 0444;
> + default:
> + return 0;
> + }
> +}
> +
> +static int npcm7xx_read(struct device *dev, enum hwmon_sensor_types type,
> + u32 attr, int channel, long *val)
> +{
> + switch (type) {
> + case hwmon_pwm:
> + return npcm7xx_read_pwm(dev, attr, channel, val);
> + case hwmon_fan:
> + return npcm7xx_read_fan(dev, attr, channel, val);
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static int npcm7xx_write(struct device *dev, enum hwmon_sensor_types type,
> + u32 attr, int channel, long val)
> +{
> + switch (type) {
> + case hwmon_pwm:
> + return npcm7xx_write_pwm(dev, attr, channel, val);
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static umode_t npcm7xx_is_visible(const void *data,
> + enum hwmon_sensor_types type,
> + u32 attr, int channel)
> +{
> + switch (type) {
> + case hwmon_pwm:
> + return npcm7xx_pwm_is_visible(data, attr, channel);
> + case hwmon_fan:
> + return npcm7xx_fan_is_visible(data, attr, channel);
> + default:
> + return 0;
> + }
> +}
> +
> +static const u32 npcm7xx_pwm_config[] = {
> + HWMON_PWM_INPUT,
> + HWMON_PWM_INPUT,
> + HWMON_PWM_INPUT,
> + HWMON_PWM_INPUT,
> + HWMON_PWM_INPUT,
> + HWMON_PWM_INPUT,
> + HWMON_PWM_INPUT,
> + HWMON_PWM_INPUT,
> + 0
> +};
> +
> +static const struct hwmon_channel_info npcm7xx_pwm = {
> + .type = hwmon_pwm,
> + .config = npcm7xx_pwm_config,
> +};
> +
> +static const u32 npcm7xx_fan_config[] = {
> + HWMON_F_INPUT,
> + HWMON_F_INPUT,
> + HWMON_F_INPUT,
> + HWMON_F_INPUT,
> + HWMON_F_INPUT,
> + HWMON_F_INPUT,
> + HWMON_F_INPUT,
> + HWMON_F_INPUT,
> + HWMON_F_INPUT,
> + HWMON_F_INPUT,
> + HWMON_F_INPUT,
> + HWMON_F_INPUT,
> + HWMON_F_INPUT,
> + HWMON_F_INPUT,
> + HWMON_F_INPUT,
> + HWMON_F_INPUT,
> + 0
> +};
> +
> +static const struct hwmon_channel_info npcm7xx_fan = {
> + .type = hwmon_fan,
> + .config = npcm7xx_fan_config,
> +};
> +
> +static const struct hwmon_channel_info *npcm7xx_info[] = {
> + &npcm7xx_pwm,
> + &npcm7xx_fan,
> + NULL
> +};
> +
> +static const struct hwmon_ops npcm7xx_hwmon_ops = {
> + .is_visible = npcm7xx_is_visible,
> + .read = npcm7xx_read,
> + .write = npcm7xx_write,
> +};
> +
> +static const struct hwmon_chip_info npcm7xx_chip_info = {
> + .ops = &npcm7xx_hwmon_ops,
> + .info = npcm7xx_info,
> +};
> +
> +static u32 npcm7xx_pwm_init(struct npcm7xx_pwm_fan_data *data)
> +{
> + int m, ch;
> + u32 Prescale_val, output_freq;
> +
> + data->pwm_clk_freq = clk_get_rate(data->pwm_clk);
> +
> + /* Adjust NPCM7xx PWMs output frequency to ~25Khz */
> + output_freq = data->pwm_clk_freq / PWN_CNT_DEFAULT;
> + Prescale_val = DIV_ROUND_CLOSEST(output_freq, PWM_OUTPUT_FREQ_25KHZ);
> +
> + /* If Prescale_val = 0, then the prescale output clock is stopped */
> + if (Prescale_val < MIN_PRESCALE1)
> + Prescale_val = MIN_PRESCALE1;
> + /*
> + * Prescale_val need to decrement in one because in the PWM Prescale
> + * register the Prescale value increment by one
> + */
> + Prescale_val--;
> +
> + /* Setting PWM Prescale Register value register to both modules */
> + Prescale_val |= (Prescale_val << NPCM7XX_PWM_PRESCALE_SHIFT_CH01);
> +
> + for (m = 0; m < NPCM7XX_PWM_MAX_MODULES ; m++) {
> + iowrite32(Prescale_val, NPCM7XX_PWM_REG_PR(data->pwm_base, m));
> + iowrite32(NPCM7XX_PWM_PRESCALE2_DEFALUT,
> + NPCM7XX_PWM_REG_CSR(data->pwm_base, m));
> + iowrite32(NPCM7XX_PWM_CTRL_MODE_DEFALUT,
> + NPCM7XX_PWM_REG_CR(data->pwm_base, m));
> +
> + for (ch = 0; ch < NPCM7XX_PWM_MAX_CHN_NUM_IN_A_MODULE; ch++) {
> + iowrite32(NPCM7XX_PWM_COUNTER_DEFALUT_NUM,
> + NPCM7XX_PWM_REG_CNRx(data->pwm_base, m, ch));
> + }
> + }
> +
> + return output_freq / ((Prescale_val & 0xf) + 1);
> +}
> +
> +static void npcm7xx_fan_init(struct npcm7xx_pwm_fan_data *data)
> +{
> + int md;
> + int ch;
> + int i;
> + u32 apb_clk_freq;
> +
> + for (md = 0; md < NPCM7XX_FAN_MAX_MODULE; md++) {
> +
> + /* stop FAN0~7 clock */
> + iowrite8((u8)NPCM7XX_FAN_TCKC_CLKX_NONE,
> + NPCM7XX_FAN_REG_TCKC(data->fan_base, md));
> +
> + /* disable all interrupt */
> + iowrite8((u8) 0x00, NPCM7XX_FAN_REG_TIEN(data->fan_base, md));
> +
> + /* clear all interrupt */
> + iowrite8((u8) NPCM7XX_FAN_TICLR_CLEAR_ALL,
> + NPCM7XX_FAN_REG_TICLR(data->fan_base, md));
> +
> + /* set FAN0~7 clock prescaler */
> + iowrite8((u8) NPCM7XX_FAN_CLK_PRESCALE,
> + NPCM7XX_FAN_REG_TPRSC(data->fan_base, md));
> +
> + /* set FAN0~7 mode (high-to-low transition) */
> + iowrite8(
> + (u8) (
> + NPCM7XX_FAN_TMCTRL_MODE_5 |
> + NPCM7XX_FAN_TMCTRL_TBEN |
> + NPCM7XX_FAN_TMCTRL_TAEN
> + ),
> + NPCM7XX_FAN_REG_TMCTRL(data->fan_base, md)
> + );
> +
> + /* set FAN0~7 Initial Count/Cap */
> + iowrite16(NPCM7XX_FAN_TCNT,
> + NPCM7XX_FAN_REG_TCNT1(data->fan_base, md));
> + iowrite16(NPCM7XX_FAN_TCNT,
> + NPCM7XX_FAN_REG_TCNT2(data->fan_base, md));
> +
> + /* set FAN0~7 compare (equal to count) */
> + iowrite8((u8)(NPCM7XX_FAN_TCPCFG_EQAEN |
> + NPCM7XX_FAN_TCPCFG_EQBEN),
> + NPCM7XX_FAN_REG_TCPCFG(data->fan_base, md));
> +
> + /* set FAN0~7 compare value */
> + iowrite16(NPCM7XX_FAN_TCPA,
> + NPCM7XX_FAN_REG_TCPA(data->fan_base, md));
> + iowrite16(NPCM7XX_FAN_TCPB,
> + NPCM7XX_FAN_REG_TCPB(data->fan_base, md));
> +
> + /* set FAN0~7 fan input FANIN 0~15 */
> + iowrite8((u8) NPCM7XX_FAN_TINASEL_FANIN_DEFAULT,
> + NPCM7XX_FAN_REG_TINASEL(data->fan_base, md));
> + iowrite8((u8) NPCM7XX_FAN_TINASEL_FANIN_DEFAULT,
> + NPCM7XX_FAN_REG_TINBSEL(data->fan_base, md));
> +
> + for (i = 0; i < NPCM7XX_FAN_MAX_CHN_NUM_IN_A_MODULE; i++) {
> + ch = md * NPCM7XX_FAN_MAX_CHN_NUM_IN_A_MODULE + i;
> + data->npcm7xx_fan[ch].FanStFlag = FAN_DISABLE;
> + data->npcm7xx_fan[ch].FanPlsPerRev =
> + NPCM7XX_FAN_DEFAULT_PULSE_PER_REVOLUTION;
> + data->npcm7xx_fan[ch].FanCnt = 0;
> + }
> + }
> +
> + apb_clk_freq = clk_get_rate(data->fan_clk);
> +
> + /* Fan tach input clock = APB clock / prescalar, default is 255. */
> + data->InputClkFreq = apb_clk_freq / (NPCM7XX_FAN_CLK_PRESCALE + 1);
> +}
> +
> +static int
> +npcm7xx_pwm_cz_get_max_state(struct thermal_cooling_device *tcdev,
> + unsigned long *state)
> +{
> + struct npcm7xx_cooling_device *cdev = tcdev->devdata;
> +
> + *state = cdev->max_state;
> +
> + return 0;
> +}
> +
> +static int
> +npcm7xx_pwm_cz_get_cur_state(struct thermal_cooling_device *tcdev,
> + unsigned long *state)
> +{
> + struct npcm7xx_cooling_device *cdev = tcdev->devdata;
> +
> + *state = cdev->cur_state;
> +
> + return 0;
> +}
> +
> +static int
> +npcm7xx_pwm_cz_set_cur_state(struct thermal_cooling_device *tcdev,
> + unsigned long state)
> +{
> + struct npcm7xx_cooling_device *cdev = tcdev->devdata;
> + int ret;
> +
> + if (state > cdev->max_state)
> + return -EINVAL;
> +
> + cdev->cur_state = state;
> + ret = npcm7xx_pwm_config_set(cdev->data, cdev->pwm_port,
> + cdev->cooling_levels[cdev->cur_state]);
> +
> + return ret;
> +}
> +
> +static const struct thermal_cooling_device_ops npcm7xx_pwm_cool_ops = {
> + .get_max_state = npcm7xx_pwm_cz_get_max_state,
> + .get_cur_state = npcm7xx_pwm_cz_get_cur_state,
> + .set_cur_state = npcm7xx_pwm_cz_set_cur_state,
> +};
> +
> +static int npcm7xx_create_pwm_cooling(struct device *dev,
> + struct device_node *child,
> + struct npcm7xx_pwm_fan_data *data,
> + u32 pwm_port, u8 num_levels)
> +{
> + int ret;
> + struct npcm7xx_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, THERMAL_NAME_LENGTH, "%s%d", child->name,
> + pwm_port);
> +
> + cdev->tcdev = thermal_of_cooling_device_register(child,
> + cdev->name,
> + cdev,
> + &npcm7xx_pwm_cool_ops);
> + if (IS_ERR(cdev->tcdev))
> + return PTR_ERR(cdev->tcdev);
> +
> + cdev->data = data;
> + cdev->pwm_port = pwm_port;
> +
> + data->cdev[pwm_port] = cdev;
> +
> + return 0;
> +}
> +
> +static int npcm7xx_en_pwm_fan(struct device *dev,
> + struct device_node *child,
> + struct npcm7xx_pwm_fan_data *data)
> +{
> + u8 *fan_ch;
> + u8 pwm_port;
> + int ret, fan_cnt;
> + u8 index, ch;
> +
> + ret = of_property_read_u8(child, "pwm-ch", &pwm_port);
> + if (ret)
> + return ret;
> +
> + data->pwm_present[pwm_port] = true;
> + ret = npcm7xx_pwm_config_set(data, pwm_port,
> + NPCM7XX_PWM_COMPARATOR_DEFALUT_NUM);
> +
> + ret = of_property_count_u8_elems(child, "cooling-levels");
> + if (ret > 0) {
> + ret = npcm7xx_create_pwm_cooling(dev, child, data, pwm_port,
> + ret);
> + if (ret)
> + return ret;
> + }
> +
> + fan_cnt = of_property_count_u8_elems(child, "fan-ch");
> + if (fan_cnt < 1)
> + return -EINVAL;
> +
> + fan_ch = devm_kzalloc(dev, sizeof(*fan_ch) * fan_cnt, GFP_KERNEL);
> + if (!fan_ch)
> + return -ENOMEM;
> +
> + ret = of_property_read_u8_array(child, "fan-ch", fan_ch, fan_cnt);
> + if (ret)
> + return ret;
> +
> + for (ch = 0; ch < fan_cnt; ch++) {
> + index = fan_ch[ch];
> + data->fan_present[index] = true;
> + data->npcm7xx_fan[index].FanStFlag = FAN_INIT;
> + }
> +
> + return 0;
> +}
> +
> +static int npcm7xx_pwm_fan_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct device_node *np, *child;
> + struct npcm7xx_pwm_fan_data *data;
> + struct resource *res;
> + struct device *hwmon;
> + char name[20];
> + int ret, cnt;
> + u32 output_freq;
> + u32 i;
> +
> + np = dev->of_node;
> +
> + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "pwm_base");
> + if (res == NULL) {
> + pr_err("PWM of_address_to_resource fail ret %d\n", ret);
> + return -ENODEV;
> + }
> +
> + data->pwm_base = devm_ioremap_resource(dev, res);
> + pr_debug("pwm base is 0x%08X, res.start 0x%08X , size 0x%08X\n",
> + (u32)data->pwm_base, res->start, resource_size(res));
I am sure I asked to use %pR. I am also sure I asked to replace the pr_ functions
with dev_ functions.
> + if (!data->pwm_base) {
> + pr_err("pwm probe failed: can't read pwm base address\n");
> + return -ENOMEM;
> + }
> +
> + data->pwm_clk = devm_clk_get(dev, "clk_apb3");
> + if (IS_ERR(data->pwm_clk)) {
> + pr_err(" pwm probe failed: can't read clk.\n");
> + return -ENODEV;
> + }
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "fan_base");
> + if (ret) {
> + pr_err("fan of_address_to_resource fail ret %d\n", ret);
> + return -ENODEV;
> + }
> +
> + data->fan_base = devm_ioremap_resource(dev, res);
> + pr_debug("fan base is 0x%08X, res.start 0x%08X , size 0x%08X\n",
> + (u32)data->fan_base, res->start, resource_size(res));
> +
%pR again.
> + if (!data->fan_base) {
> + pr_err("fan probe failed: can't read fan base address.\n");
> + return -ENOMEM;
> + }
> +
> + data->fan_clk = devm_clk_get(dev, "clk_apb4");
> + if (IS_ERR(data->fan_clk)) {
> + pr_err(" FAN probe failed: can't read clk.\n");
> + return -ENODEV;
> + }
> +
> + output_freq = npcm7xx_pwm_init(data);
> + npcm7xx_fan_init(data);
> +
> + for (cnt = 0; cnt < NPCM7XX_PWM_MAX_MODULES ; cnt++)
> + mutex_init(&data->npcm7xx_pwm_lock[cnt]);
> +
> + for (i = 0; i < NPCM7XX_FAN_MAX_MODULE; i++) {
> + spin_lock_init(&data->npcm7xx_fan_lock[i]);
> +
> + data->fan_irq[i] = platform_get_irq(pdev, i);
> + if (!data->fan_irq[i]) {
> + pr_err("%s - failed to map irq %d\n", __func__, i);
> + ret = -EAGAIN;
> + goto err_irq;
> + }
> +
> + sprintf(name, "NPCM7XX-FAN-MD%d", i);
> +
> + if (request_irq(data->fan_irq[i], npcm7xx_fan_isr, 0, name,
> + (void *)data)) {
> + pr_err("NPCM7XX: register irq FAN%d failed\n", i);
> + ret = -EAGAIN;
> + goto err_irq;
> + }
> + }
> +
> + for_each_child_of_node(np, child) {
> + ret = npcm7xx_en_pwm_fan(dev, child, data);
> + if (ret) {
> + pr_err("npcm7xx_en_pwm_fan failed ret %d\n", ret);
> + of_node_put(child);
> + goto err_irq;
> + }
> + }
> +
> + hwmon = devm_hwmon_device_register_with_info(dev, "npcm7xx_pwm_fan",
> + data, &npcm7xx_chip_info,
> + NULL);
> + if (IS_ERR(hwmon)) {
> + pr_err("PWM Driver failed - devm_hwmon_device_register_with_groups failed\n");
> + ret = PTR_ERR(hwmon);
> + goto err_irq;
> + }
> +
> + for (i = 0; i < NPCM7XX_FAN_MAX_CHN_NUM; i++) {
> + if (data->fan_present[i] == true) {
> + /* fan timer initialization */
> + data->npcm7xx_fan_select = 0;
> + data->npcm7xx_fan_timer.expires = jiffies +
> + msecs_to_jiffies(NPCM7XX_FAN_POLL_TIMER_200MS);
> + timer_setup(&data->npcm7xx_fan_timer,
> + npcm7xx_fan_polling, 0);
> + add_timer(&data->npcm7xx_fan_timer);
> + break;
> + }
> + }
> +
> + pr_info("NPCM7XX PWM-FAN Driver probed, output Freq %dHz[PWM], input Freq %dHz[FAN]\n",
> + output_freq, data->InputClkFreq);
> +
> + return 0;
> +
> +err_irq:
> + for (; i > 0; i--)
> + free_irq(data->fan_irq[i-1], (void *)data);
> +
> + return ret;
> +}
> +
> +static const struct of_device_id of_pwm_fan_match_table[] = {
> + { .compatible = "nuvoton,npcm750-pwm-fan", },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, of_pwm_fan_match_table);
> +
> +static struct platform_driver npcm7xx_pwm_fan_driver = {
> + .probe = npcm7xx_pwm_fan_probe,
> + .driver = {
> + .name = "npcm7xx_pwm_fan",
> + .of_match_table = of_pwm_fan_match_table,
> + },
> +};
> +
> +module_platform_driver(npcm7xx_pwm_fan_driver);
> +
> +MODULE_DESCRIPTION("Nuvoton NPCM7XX PWM and Fan Tacho driver");
> +MODULE_AUTHOR("Tomer Maimon <tomer.maimon@nuvoton.com>");
> +MODULE_LICENSE("GPL v2");
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] hwmon: npcm750: add NPCM7xx PWM and Fan driver
2018-06-21 13:17 ` Julia Lawall
@ 2018-06-21 14:48 ` Joe Perches
0 siblings, 0 replies; 11+ messages in thread
From: Joe Perches @ 2018-06-21 14:48 UTC (permalink / raw)
To: Julia Lawall
Cc: Guenter Roeck, Tomer Maimon, cocci, robh+dt, mark.rutland,
jdelvare, avifishman70, yuenn, brendanhiggins, venture, joel,
devicetree, linux-kernel, linux-hwmon, openbmc
On Thu, 2018-06-21 at 15:17 +0200, Julia Lawall wrote:
> On Wed, 20 Jun 2018, Joe Perches wrote:
> > Also, spatch (1.0.4) seems to have a defect for this
> > when the type is used in operations that change a
> > smaller type to int or unsigned int.
> >
> > i.e.: (offset is u16, but offset * 2 is int)
>
> Ah. The rule is that the result type is always the larger one?
Yes, but not quite, no.
The c90 rules are called "integer promotions" and are
detailed in section 6.3 Conversions
Basically, if any type is smaller than int, all operations
are done as int if possible, or unsigned int if necessary.
If any type is larger than int, then the larger type is used.
If you don't have the c90 standard, this one is close enough.
http://c0x.coding-guidelines.com/6.3.html
(use the next button several times to read the whole section)
Also, section 6.5 details "expressions" where the operands
of things like bit operations use integer promotions.
> Unfortunately, Coccinelle doesn't know the size of any type. I could add
> some special cases, but that may be more confusing than helpful.
Maybe, but when I saw the suggested removal, I was surprised.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-06-21 14:48 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-19 10:53 [PATCH v2 0/2] hwmon: Add NPCM7xx PWM and Fan driver support Tomer Maimon
2018-06-19 10:53 ` [PATCH v2 1/2] dt-binding: hwmon: Add NPCM7xx PWM and Fan controller documentation Tomer Maimon
2018-06-19 10:53 ` [PATCH v2 2/2] hwmon: npcm750: add NPCM7xx PWM and Fan driver Tomer Maimon
2018-06-19 14:31 ` kbuild test robot
2018-06-19 19:43 ` kbuild test robot
2018-06-20 16:48 ` Guenter Roeck
2018-06-20 18:25 ` Joe Perches
2018-06-20 19:38 ` Guenter Roeck
2018-06-21 13:17 ` Julia Lawall
2018-06-21 14:48 ` Joe Perches
2018-06-21 13:46 ` Guenter Roeck
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).