openbmc.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Guenter Roeck <linux@roeck-us.net>
To: Tomer Maimon <tmaimon77@gmail.com>
Cc: robh+dt@kernel.org, mark.rutland@arm.com, jdelvare@suse.com,
	avifishman70@gmail.com, yuenn@google.com,
	brendanhiggins@google.com, venture@google.com, joel@jms.id.au,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-hwmon@vger.kernel.org, openbmc@lists.ozlabs.org
Subject: Re: [PATCH v6 2/2] hwmon: npcm750: add NPCM7xx PWM and Fan driver
Date: Mon, 2 Jul 2018 11:07:55 -0700	[thread overview]
Message-ID: <20180702180755.GA15406@roeck-us.net> (raw)
In-Reply-To: <20180628114646.343734-3-tmaimon77@gmail.com>

On Thu, Jun 28, 2018 at 02:46:46PM +0300, Tomer Maimon wrote:
> Add Nuvoton BMC NPCM750/730/715/705 Pulse Width Modulation (PWM)
> and Fan tacho driver.
> 
> The Nuvoton BMC NPCM750/730/715/705 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 from 0(off) and 255(full speed)
> 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.
> 
Pretty much down to nitpicks ...

> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> ---
>  Documentation/hwmon/npcm750-pwm-fan |   22 +
>  drivers/hwmon/Kconfig               |    7 +
>  drivers/hwmon/Makefile              |    1 +
>  drivers/hwmon/npcm750-pwm-fan.c     | 1064 +++++++++++++++++++++++++++++++++++
>  4 files changed, 1094 insertions(+)
>  create mode 100644 Documentation/hwmon/npcm750-pwm-fan
>  create mode 100644 drivers/hwmon/npcm750-pwm-fan.c
> 
> diff --git a/Documentation/hwmon/npcm750-pwm-fan b/Documentation/hwmon/npcm750-pwm-fan
> new file mode 100644
> index 000000000000..3cecd4a830fe
> --- /dev/null
> +++ b/Documentation/hwmon/npcm750-pwm-fan
> @@ -0,0 +1,22 @@
> +Kernel driver npcm750-pwm-fan
> +=============================
> +
> +Supported chip:

chips

> +	NUVOTON NPCM750/730/715/705
> +
> +Authors:
> +	<tomer.maimon@nuvoton.com>
> +
> +Description:
> +------------
> +This driver implements support for NUVOTON NPCM7XX PWM and Fan Tacho
> +controller. The PWM controller supports upto 8 PWM outputs. The Fan tacho

up to

> +controller supports up to 16 tachometer inputs.
> +
> +The driver provides the following sensor accesses in sysfs:
> +
> +fanX_input	ro	provide current fan rotation value in RPM as reported
> +			by the fan to the device.
> +
> +pwmX		rw	get or set PWM fan control value. This is an integer
> +			value between 0(off) and 255(full speed).
> diff --git a/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

Can you try "imply THERMAL" here ? That is supposed to work better.

> +	help
> +	  This driver provides support for Nuvoton NPCM7XX PWM and Fan
> +	  controllers.
> +

Please retain alphabetic order.

>  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

Alphabetic order please.

>  
>  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..91b401944ab7
> --- /dev/null
> +++ b/drivers/hwmon/npcm750-pwm-fan.c
> @@ -0,0 +1,1064 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2014-2018 Nuvoton Technology corporation.
> +
> +#include <linux/clk.h>
> +#include <linux/device.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>

Alphabetic order please.

> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/spinlock.h>
> +#include <linux/sysfs.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_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_DEFAULT_NUM		255
> +#define NPCM7XX_PWM_COMPARATOR_DEFAULT_NUM	127
> +
> +#define NPCM7XX_PWM_COMPARATOR_MAX		255
> +

What is the "COMPARATOR" in those defines for ? If it means that the
definitions are used to compare against something, please drop.

> +/* default all PWM channels PRESCALE2 = 1 */
> +#define NPCM7XX_PWM_PRESCALE2_DEFAULT_CH0	0x4
> +#define NPCM7XX_PWM_PRESCALE2_DEFAULT_CH1	0x40
> +#define NPCM7XX_PWM_PRESCALE2_DEFAULT_CH2	0x400
> +#define NPCM7XX_PWM_PRESCALE2_DEFAULT_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_DEFAULT	(NPCM7XX_PWM_PRESCALE2_DEFAULT_CH0 | \
> +					NPCM7XX_PWM_PRESCALE2_DEFAULT_CH1 | \
> +					NPCM7XX_PWM_PRESCALE2_DEFAULT_CH2 | \
> +					NPCM7XX_PWM_PRESCALE2_DEFAULT_CH3)
> +
> +#define NPCM7XX_PWM_CTRL_MODE_DEFAULT	(NPCM7XX_PWM_CTRL_CH0_MODE_BIT | \
> +					NPCM7XX_PWM_CTRL_CH1_MODE_BIT | \
> +					NPCM7XX_PWM_CTRL_CH2_MODE_BIT | \
> +					NPCM7XX_PWM_CTRL_CH3_MODE_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 Definition */
> +/* 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 npcm7xx_fan_dev {
> +	u8 fan_st_flg;
> +	u8 fan_pls_per_rev;
> +	u16 fan_cnt;
> +	u32 fan_cnt_tmp;
> +};
> +
> +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;
> +	void __iomem *fan_base;
> +	unsigned long pwm_clk_freq;
> +	unsigned long fan_clk_freq;
> +	struct clk *pwm_clk;
> +	struct clk *fan_clk;
> +	struct mutex pwm_lock[NPCM7XX_PWM_MAX_MODULES];
> +	spinlock_t fan_lock[NPCM7XX_FAN_MAX_MODULE];
> +	int fan_irq[NPCM7XX_FAN_MAX_MODULE];
> +	bool pwm_present[NPCM7XX_PWM_MAX_CHN_NUM];
> +	bool fan_present[NPCM7XX_FAN_MAX_CHN_NUM];
> +	u32 input_clk_freq;
> +	struct timer_list fan_timer;
> +	struct npcm7xx_fan_dev fan_dev[NPCM7XX_FAN_MAX_CHN_NUM];
> +	struct npcm7xx_cooling_device *cdev[NPCM7XX_PWM_MAX_CHN_NUM];
> +	u8 fan_select;
> +};
> +
> +static int npcm7xx_pwm_config_set(struct npcm7xx_pwm_fan_data *data,
> +				  int channel, u16 val)
> +{
> +	u32 pwm_ch = (channel % NPCM7XX_PWM_MAX_CHN_NUM_IN_A_MODULE);
> +	u32 module = (channel / NPCM7XX_PWM_MAX_CHN_NUM_IN_A_MODULE);
> +	u32 tmp_buf, ctrl_en_bit, env_bit;
> +
> +	/*
> +	 * Config PWM Comparator register for setting duty cycle
> +	 */
> +	mutex_lock(&data->pwm_lock[module]);
> +
> +	/* write new CMR value  */
> +	iowrite32(val, NPCM7XX_PWM_REG_CMRx(data->pwm_base, module, pwm_ch));
> +	tmp_buf = ioread32(NPCM7XX_PWM_REG_CR(data->pwm_base, module));
> +
> +	switch (pwm_ch) {
> +	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:
> +		mutex_unlock(&data->pwm_lock[module]);
> +		return -ENODEV;
> +	}
> +
> +	if (val == 0) {
> +		/* Disable PWM */
> +		tmp_buf &= ~ctrl_en_bit;
> +		tmp_buf |= env_bit;
> +	} else {
> +		/* Enable PWM */
> +		tmp_buf |= ctrl_en_bit;
> +		tmp_buf &= ~env_bit;
> +	}
> +
> +	iowrite32(tmp_buf, NPCM7XX_PWM_REG_CR(data->pwm_base, module));
> +	mutex_unlock(&data->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;
> +	u8 reg_mode;
> +	u8 reg_int;
> +	unsigned long flags;
> +
> +	fan_id = NPCM7XX_FAN_INPUT(fan, cmp);
> +
> +	/* to check whether any fan tach is enable */
> +	if (data->fan_dev[fan_id].fan_st_flg != FAN_DISABLE) {
> +		/* reset status */
> +		spin_lock_irqsave(&data->fan_lock[fan], flags);
> +
> +		data->fan_dev[fan_id].fan_st_flg = FAN_INIT;
> +		reg_int = ioread8(NPCM7XX_FAN_REG_TIEN(data->fan_base, fan));
> +
> +		/*
> +		 * the interrupt enable bits do not need to be cleared before
> +		 * it sets, the interrupt enable bits are cleared only on reset.
> +		 * the clock unit control register is behaving in the same
> +		 * manner that the interrupt enable register behave.
> +		 */
> +		if (cmp == NPCM7XX_FAN_CMPA) {
> +			/* enable interrupt */
> +			iowrite8(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(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->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;
> +	int i;
> +
> +	data = from_timer(data, t, fan_timer);
> +
> +	/*
> +	 * Polling two module per one round,
> +	 * FAN01 & FAN89 / FAN23 & FAN1011 / FAN45 & FAN1213 / FAN67 & FAN1415
> +	 */
> +	for (i = data->fan_select; i < NPCM7XX_FAN_MAX_MODULE;
> +	      i = i + 4) {
> +		/* clear the flag and reset the counter (TCNT) */
> +		iowrite8(NPCM7XX_FAN_TICLR_CLEAR_ALL,
> +			 NPCM7XX_FAN_REG_TICLR(data->fan_base, i));
> +
> +		if (data->fan_present[i * 2]) {
> +			iowrite16(NPCM7XX_FAN_TCNT,
> +				  NPCM7XX_FAN_REG_TCNT1(data->fan_base, i));
> +			npcm7xx_fan_start_capture(data, i, NPCM7XX_FAN_CMPA);
> +		}
> +		if (data->fan_present[(i * 2) + 1]) {
> +			iowrite16(NPCM7XX_FAN_TCNT,
> +				  NPCM7XX_FAN_REG_TCNT2(data->fan_base, i));
> +			npcm7xx_fan_start_capture(data, i, NPCM7XX_FAN_CMPB);
> +		}
> +	}
> +
> +	data->fan_select++;
> +	data->fan_select &= 0x3;
> +
> +	/* reset the timer interval */
> +	data->fan_timer.expires = jiffies +
> +		msecs_to_jiffies(NPCM7XX_FAN_POLL_TIMER_200MS);
> +	add_timer(&data->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;
> +	u8  reg_mode;
> +	u16 fan_cap;
> +
> +	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(flag_clear, NPCM7XX_FAN_REG_TICLR(data->fan_base, fan));
> +
> +	if (data->fan_dev[fan_id].fan_st_flg == FAN_INIT) {
> +		/* First capture, drop it */
> +		data->fan_dev[fan_id].fan_st_flg =
> +			FAN_PREPARE_TO_GET_FIRST_CAPTURE;
> +
> +		/* reset counter */
> +		data->fan_dev[fan_id].fan_cnt_tmp = 0;
> +	} else if (data->fan_dev[fan_id].fan_st_flg < FAN_ENOUGH_SAMPLE) {
> +		/*
> +		 * collect the enough sample,
> +		 * (ex: 2 pulse fan need to get 2 sample)
> +		 */
> +		data->fan_dev[fan_id].fan_cnt_tmp +=
> +			(NPCM7XX_FAN_TCNT - fan_cap);
> +
> +		data->fan_dev[fan_id].fan_st_flg++;
> +	} else {
> +		/* get enough sample or fan disable */
> +		if (data->fan_dev[fan_id].fan_st_flg == FAN_ENOUGH_SAMPLE) {
> +			data->fan_dev[fan_id].fan_cnt_tmp +=
> +				(NPCM7XX_FAN_TCNT - fan_cap);
> +
> +			/* compute finial average cnt per pulse */
> +			data->fan_dev[fan_id].fan_cnt =
> +				data->fan_dev[fan_id].fan_cnt_tmp /
> +				FAN_ENOUGH_SAMPLE;
> +
> +			data->fan_dev[fan_id].fan_st_flg = FAN_INIT;
> +		}
> +
> +		reg_int =  ioread8(NPCM7XX_FAN_REG_TIEN(data->fan_base, fan));
> +
> +		/* disable interrupt */
> +		iowrite8((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((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;
> +	u8 reg_mode;
> +	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((reg_int & ~flag_int),
> +			 NPCM7XX_FAN_REG_TIEN(data->fan_base, fan));
> +
> +		/* clear interrupt flag */
> +		iowrite8(flag_clear,
> +			 NPCM7XX_FAN_REG_TICLR(data->fan_base, fan));
> +
> +		reg_mode =  ioread8(NPCM7XX_FAN_REG_TCKC(data->fan_base, fan));
> +
> +		/* stop capturing */
> +		iowrite8((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->fan_dev[fan_id].fan_cnt = 0;
> +	} 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;
> +	unsigned long flags;
> +	int module;
> +	u8 flag;
> +
> +	module = irq - data->fan_irq[0];
> +	spin_lock_irqsave(&data->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->fan_lock[module], flags);
> +		return IRQ_HANDLED;
> +	}
> +
> +	spin_unlock_irqrestore(&data->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 pmw_ch = (channel % NPCM7XX_PWM_MAX_CHN_NUM_IN_A_MODULE);
> +	u32 module = (channel / NPCM7XX_PWM_MAX_CHN_NUM_IN_A_MODULE);
> +
> +	switch (attr) {
> +	case hwmon_pwm_input:
> +		*val = ioread32
> +			(NPCM7XX_PWM_REG_CMRx(data->pwm_base, module, pmw_ch));
> +		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;
> +
> +	switch (attr) {
> +	case hwmon_pwm_input:
> +		if (val < 0 || val > NPCM7XX_PWM_COMPARATOR_MAX)
> +			return -EINVAL;
> +		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->fan_dev[channel].fan_cnt <= 0)
> +			return data->fan_dev[channel].fan_cnt;
> +
> +		/* Convert the raw reading to RPM */
> +		if (data->fan_dev[channel].fan_cnt > 0 &&
> +		    data->fan_dev[channel].fan_pls_per_rev > 0)
> +			*val = ((data->input_clk_freq * 60) /
> +				(data->fan_dev[channel].fan_cnt *
> +				 data->fan_dev[channel].fan_pls_per_rev));
> +		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_DEFAULT,
> +			  NPCM7XX_PWM_REG_CSR(data->pwm_base, m));
> +		iowrite32(NPCM7XX_PWM_CTRL_MODE_DEFAULT,
> +			  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_DEFAULT_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(NPCM7XX_FAN_TCKC_CLKX_NONE,
> +			 NPCM7XX_FAN_REG_TCKC(data->fan_base, md));
> +
> +		/* disable all interrupt */
> +		iowrite8(0x00, NPCM7XX_FAN_REG_TIEN(data->fan_base, md));
> +
> +		/* clear all interrupt */
> +		iowrite8(NPCM7XX_FAN_TICLR_CLEAR_ALL,
> +			 NPCM7XX_FAN_REG_TICLR(data->fan_base, md));
> +
> +		/* set FAN0~7 clock prescaler */
> +		iowrite8(NPCM7XX_FAN_CLK_PRESCALE,
> +			 NPCM7XX_FAN_REG_TPRSC(data->fan_base, md));
> +
> +		/* set FAN0~7 mode (high-to-low transition) */
> +		iowrite8((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((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(NPCM7XX_FAN_TINASEL_FANIN_DEFAULT,
> +			 NPCM7XX_FAN_REG_TINASEL(data->fan_base, md));
> +		iowrite8(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->fan_dev[ch].fan_st_flg = FAN_DISABLE;
> +			data->fan_dev[ch].fan_pls_per_rev =
> +				NPCM7XX_FAN_DEFAULT_PULSE_PER_REVOLUTION;
> +			data->fan_dev[ch].fan_cnt = 0;
> +		}
> +	}
> +
> +	apb_clk_freq = clk_get_rate(data->fan_clk);
> +
> +	/* Fan tach input clock = APB clock / prescalar, default is 255. */
> +	data->input_clk_freq = 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;
> +	u32 pwm_port;
> +	int ret, fan_cnt;
> +	u8 index, ch;
> +
> +	ret = of_property_read_u32(child, "reg", &pwm_port);
> +	if (ret)
> +		return ret;
> +
> +	data->pwm_present[pwm_port] = true;
> +	ret = npcm7xx_pwm_config_set(data, pwm_port,
> +				     NPCM7XX_PWM_COMPARATOR_DEFAULT_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-tach-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-tach-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->fan_dev[index].fan_st_flg = 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");
> +	if (!res) {
> +		dev_err(dev, "PWM of_address_to_resource fail\n");
> +		return -ENODEV;
> +	}
> +
> +	data->pwm_base = devm_ioremap_resource(dev, res);
> +	dev_dbg(dev, "pwm base resource is %pR\n", res);
> +	if (IS_ERR(data->pwm_base)) {
> +		dev_err(dev, "pwm probe failed: can't read pwm base address\n");
> +		return PTR_ERR(data->pwm_base);
> +	}
> +
> +	data->pwm_clk = devm_clk_get(dev, "clk_apb3");
> +	if (IS_ERR(data->pwm_clk)) {
> +		dev_err(dev, "pwm probe failed: can't read clk.\n");
> +		return PTR_ERR(data->pwm_clk);
> +	}
> +
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "fan");
> +	if (!res) {
> +		dev_err(dev, "fan of_address_to_resource fail\n");
> +		return -ENODEV;
> +	}
> +
> +	data->fan_base = devm_ioremap_resource(dev, res);
> +	dev_dbg(dev, "fan base resource is %pR\n", res);
> +	if (IS_ERR(data->fan_base)) {
> +		dev_err(dev, "fan probe failed: can't read fan base address.\n");
> +		return PTR_ERR(data->fan_base);
> +	}
> +
> +	data->fan_clk = devm_clk_get(dev, "clk_apb4");
> +	if (IS_ERR(data->fan_clk)) {
> +		dev_err(dev, "FAN probe failed: can't read clk.\n");
> +		return PTR_ERR(data->fan_clk);
> +	}
> +
> +	output_freq = npcm7xx_pwm_init(data);
> +	npcm7xx_fan_init(data);
> +
> +	for (cnt = 0; cnt < NPCM7XX_PWM_MAX_MODULES  ; cnt++)
> +		mutex_init(&data->pwm_lock[cnt]);
> +
> +	for (i = 0; i < NPCM7XX_FAN_MAX_MODULE; i++) {
> +		spin_lock_init(&data->fan_lock[i]);
> +
> +		data->fan_irq[i] = platform_get_irq(pdev, i);
> +		if (data->fan_irq[i] < 0) {
> +			dev_err(dev, "%s - failed to map irq %d\n",
> +				__func__, i);

Your prefixes are all over the place.
None, "fan", "fan probe", "FAN", function name. Please be consistent.
the dev_ functions already provide a prefix. Either use no additional
prefix or just "probe" consistently.

> +			ret = data->fan_irq[i];
> +			return ret;
> +		}
> +
> +		sprintf(name, "NPCM7XX-FAN-MD%d", i);
> +		ret = devm_request_irq(dev, data->fan_irq[i], npcm7xx_fan_isr,
> +				       0, name, (void *)data);
> +		if (ret) {
> +			dev_err(dev, "NPCM7XX: register irq FAN%d failed\n", i);

Yet another prefix.

> +			return ret;
> +		}
> +	}
> +
> +	for_each_child_of_node(np, child) {
> +		ret = npcm7xx_en_pwm_fan(dev, child, data);
> +		if (ret) {
> +			dev_err(dev, "npcm7xx_en_pwm_fan failed ret %d\n", ret);

And another one, this time with a function name. That doesn't provide
a value to the user - please drop.

> +			of_node_put(child);
> +			return ret;
> +		}
> +	}
> +
> +	hwmon = devm_hwmon_device_register_with_info(dev, "npcm7xx_pwm_fan",
> +						     data, &npcm7xx_chip_info,
> +						     NULL);
> +	if (IS_ERR(hwmon)) {
> +		dev_err(dev, "PWM Driver failed - devm_hwmon_device_register_with_groups failed\n");

dev_err() already states the failing driver.

> +		return PTR_ERR(hwmon);
> +	}
> +
> +	for (i = 0; i < NPCM7XX_FAN_MAX_CHN_NUM; i++) {
> +		if (data->fan_present[i]) {
> +			/* fan timer initialization */
> +			data->fan_timer.expires = jiffies +
> +				msecs_to_jiffies(NPCM7XX_FAN_POLL_TIMER_200MS);
> +			timer_setup(&data->fan_timer,
> +				    npcm7xx_fan_polling, 0);
> +			add_timer(&data->fan_timer);
> +			break;
> +		}
> +	}
> +
> +	pr_info("NPCM7XX PWM-FAN Driver probed, output Freq %dHz[PWM], input Freq %dHz[FAN]\n",
> +		output_freq, data->input_clk_freq);
> +
> +	return 0;
> +}
> +
> +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
> 

      reply	other threads:[~2018-07-02 18:08 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-28 11:46 [PATCH v6 0/2] hwmon: Add NPCM7xx PWM and Fan driver support Tomer Maimon
2018-06-28 11:46 ` [PATCH v6 1/2] dt-binding: hwmon: Add NPCM7xx PWM and Fan controller documentation Tomer Maimon
2018-06-28 11:46 ` [PATCH v6 2/2] hwmon: npcm750: add NPCM7xx PWM and Fan driver Tomer Maimon
2018-07-02 18:07   ` Guenter Roeck [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180702180755.GA15406@roeck-us.net \
    --to=linux@roeck-us.net \
    --cc=avifishman70@gmail.com \
    --cc=brendanhiggins@google.com \
    --cc=devicetree@vger.kernel.org \
    --cc=jdelvare@suse.com \
    --cc=joel@jms.id.au \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=openbmc@lists.ozlabs.org \
    --cc=robh+dt@kernel.org \
    --cc=tmaimon77@gmail.com \
    --cc=venture@google.com \
    --cc=yuenn@google.com \
    --subject='Re: [PATCH v6 2/2] hwmon: npcm750: add NPCM7xx PWM and Fan driver' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).