[PATCH V2 0/9] Implementation of Tegra Tachometer driver

[PATCH V2 0/9] Implementation of Tegra Tachometer driver

[Rajkumar Rampelli]

The following patches adds support for PWM based Tegra Tachometer driver
which implements PWM capture interface to analyze the PWM signal of a
electronic fan and reports it in periods and duty cycles.

Added fan device attribute fan1_input in pwm-fan driver to monitor the
speed of fan in rotations per minute using PWM interface.
RPM of Fan will be exposed to user interface through HWMON sysfs interface
avialable at location: /sys/class/hwmon/hwmon0/fan1_input

Steps to validate Tachometer on Tegra186 SoC:
A. push modules pwm-tegra.ko, pwm-tegra-tachometer.ko and pwm-fan.ko to
   linux device using scp command.
   scp build/tegra186/drivers/pwm/pwm-tegra.ko ubuntu@10.19.65.176:/tmp/
   scp build/tegra186/drivers/pwm/pwm-tegra-tachometer.ko ubuntu@10.19.65.176:/tmp/
   scp build/tegra186/drivers/hwmon/pwm-fan.ko ubuntu@10.19.65.176:/tmp/
B. On Linux device console, insert these modules using insmod command.
   insmod /tmp/pwm-tegra.ko
   insmod /tmp/pwm-tegra-tachometer.ko
   insmod /tmp/pwm-fan.ko
C. Read RPM value at below sysfs node
   cat /sys/calss/hwmon/hwmon0/fan1_input
D. Change the FAN speed using PWM sysfs interface. Follow below steps for the same:
   a. cd /sys/class/pwm/pwmchip0
   b. ls -la (make sure pwm controller is c340000.pwm)
      Output should be: device -> ../../../c340000.pwm
   c. echo 0 > export
   d. cd pwmchip0:0
   e. echo 8000 > period
   f. echo 1 > enable
   g. echo 8000 > duty_cycle # change duty_cycles from 0 to 8000 for FAN speed variation
   h. cat /sys/calss/hwmon/hwmon0/fan1_input
   i. echo 4000 > duty_cycle
   h. cat /sys/calss/hwmon/hwmon0/fan1_input
   i. echo 2000 > duty_cycle
   h. cat /sys/calss/hwmon/hwmon0/fan1_input
   i. echo 0 > duty_cycle
   h. cat /sys/calss/hwmon/hwmon0/fan1_input

Rajkumar Rampelli (9):
  pwm: core: Add support for PWM HW driver with pwm capture only
  arm64: tegra: Add PWM controller on Tegra186 soc
  dt-bindings: Tegra186 tachometer device tree bindings
  arm64: tegra: Add Tachometer Controller on Tegra186
  pwm: tegra: Add PWM based Tachometer driver
  arm64: tegra: Add pwm based fan support on Tegra186
  hwmon: pwm-fan: add sysfs node to read rpm of fan
  arm64: defconfig: enable Nvidia Tegra Tachometer as a module
  arm64: defconfig: enable pwm-fan as a loadable module

 .../bindings/pwm/pwm-tegra-tachometer.txt          |  31 +++
 arch/arm64/boot/dts/nvidia/tegra186-p2771-0000.dts |   5 +
 arch/arm64/boot/dts/nvidia/tegra186.dtsi           |  28 ++
 arch/arm64/configs/defconfig                       |   2 +
 drivers/hwmon/pwm-fan.c                            |  23 ++
 drivers/pwm/Kconfig                                |  10 +
 drivers/pwm/Makefile                               |   1 +
 drivers/pwm/core.c                                 |  21 +-
 drivers/pwm/pwm-tegra-tachometer.c                 | 303 +++++++++++++++++++++
 9 files changed, 418 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pwm/pwm-tegra-tachometer.txt
 create mode 100644 drivers/pwm/pwm-tegra-tachometer.c

-- 
2.1.4

[PATCH V2 1/9] pwm: core: Add support for PWM HW driver with pwm capture only

[Rajkumar Rampelli]

Add support for pwm HW driver which has only capture functionality.
This helps to implement the PWM based Tachometer driver which reads
the PWM output signals from electronic fans.

PWM Tachometer captures the period and duty cycle of the PWM signal

Add conditional checks for callabacks enable(), disable(), config()
to check if they are supported by the client driver or not. Skip these
callbacks if they are not supported.

Signed-off-by: Rajkumar Rampelli <rrajk@nvidia.com>
---

V2: Added if conditional checks for pwm callbacks since drivers may
    implements only pwm capture functionality.

 drivers/pwm/core.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
index 1581f6a..f70fe68 100644
--- a/drivers/pwm/core.c
+++ b/drivers/pwm/core.c
@@ -246,6 +246,10 @@ static bool pwm_ops_check(const struct pwm_ops *ops)
 	if (ops->apply)
 		return true;
 
+	/* driver supports capture operation */
+	if (ops->capture)
+		return true;
+
 	return false;
 }
 
@@ -495,7 +499,8 @@ int pwm_apply_state(struct pwm_device *pwm, struct pwm_state *state)
 			 * ->apply().
 			 */
 			if (pwm->state.enabled) {
-				pwm->chip->ops->disable(pwm->chip, pwm);
+				if (pwm->chip->ops->disable)
+					pwm->chip->ops->disable(pwm->chip, pwm);
 				pwm->state.enabled = false;
 			}
 
@@ -509,22 +514,26 @@ int pwm_apply_state(struct pwm_device *pwm, struct pwm_state *state)
 
 		if (state->period != pwm->state.period ||
 		    state->duty_cycle != pwm->state.duty_cycle) {
-			err = pwm->chip->ops->config(pwm->chip, pwm,
+			if (pwm->chip->ops->config) {
+				err = pwm->chip->ops->config(pwm->chip, pwm,
 						     state->duty_cycle,
 						     state->period);
-			if (err)
-				return err;
+				if (err)
+					return err;
+			}
 
 			pwm->state.duty_cycle = state->duty_cycle;
 			pwm->state.period = state->period;
 		}
 
 		if (state->enabled != pwm->state.enabled) {
-			if (state->enabled) {
+			if (state->enabled && pwm->chip->ops->enable) {
 				err = pwm->chip->ops->enable(pwm->chip, pwm);
 				if (err)
 					return err;
-			} else {
+			}
+
+			if (!state->enabled && pwm->chip->ops->disable) {
 				pwm->chip->ops->disable(pwm->chip, pwm);
 			}
 
-- 
2.1.4

[PATCH V2 2/9] arm64: tegra: Add PWM controller on Tegra186 soc

[Rajkumar Rampelli]

The NVIDIA Tegra186 SoC has a PWM controller which is
used in FAN control use case.

Signed-off-by: Rajkumar Rampelli <rrajk@nvidia.com>
---

V2: no changes in this patch

 arch/arm64/boot/dts/nvidia/tegra186.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/boot/dts/nvidia/tegra186.dtsi b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
index b762227..731cd01 100644
--- a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
@@ -1031,4 +1031,15 @@
 				(GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_LOW)>;
 		interrupt-parent = <&gic>;
 	};
+
+	pwm@c340000 {
+		compatible = "nvidia,tegra186-pwm";
+		reg = <0x0 0xc340000 0x0 0x10000>;
+		clocks = <&bpmp TEGRA186_CLK_PWM4>;
+		clock-names = "pwm";
+		#pwm-cells = <2>;
+		resets = <&bpmp TEGRA186_RESET_PWM4>;
+		reset-names = "pwm";
+		status = "disabled";
+	};
 };
-- 
2.1.4

[PATCH V2 3/9] dt-bindings: Tegra186 tachometer device tree bindings

[Rajkumar Rampelli]

Supply Device tree binding documentation for the NVIDIA
Tegra186 SoC's Tachometer Controller

Signed-off-by: Rajkumar Rampelli <rrajk@nvidia.com>
---

V2: Renamed compatible string to "nvidia,tegra186-pwm-tachometer"
    Renamed dt property values of clock-names and reset-names to "tachometer"
    from "tach"

 .../bindings/pwm/pwm-tegra-tachometer.txt          | 31 ++++++++++++++++++++++
 1 file changed, 31 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pwm/pwm-tegra-tachometer.txt

diff --git a/Documentation/devicetree/bindings/pwm/pwm-tegra-tachometer.txt b/Documentation/devicetree/bindings/pwm/pwm-tegra-tachometer.txt
new file mode 100644
index 0000000..4a7ead4
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/pwm-tegra-tachometer.txt
@@ -0,0 +1,31 @@
+Bindings for a PWM based Tachometer driver
+
+Required properties:
+- compatible: Must be "nvidia,tegra186-pwm-tachometer"
+- reg: physical base addresses of the controller and length of
+  memory mapped region.
+- #pwm-cells: should be 2. See pwm.txt in this directory for a
+  description of the cells format.
+- clocks: phandle list of tachometer clocks
+- clock-names: should be "tachometer". See clock-bindings.txt in documentations
+- resets: phandle to the reset controller for the Tachometer IP
+- reset-names: should be "tachometer". See reset.txt in documentations
+- nvidia,pulse-per-rev: Integer, pulses per revolution of a Fan. This value
+  obtained from Fan specification document.
+- nvidia,capture-window-len: Integer, window of the Fan Tach monitor, it indicates
+  that how many period of the input fan tach signal will the FAN TACH logic
+  monitor. Valid values are 1, 2, 4 and 8 only.
+
+Example:
+	tegra_tachometer: tachometer@39c0000 {
+		compatible = "nvidia,tegra186-pwm-tachometer";
+		reg = <0x0 0x039c0000 0x0 0x10>;
+		#pwm-cells = <2>;
+		clocks = <&tegra_car TEGRA186_CLK_TACH>;
+		clock-names = "tachometer";
+		resets = <&tegra_car TEGRA186_RESET_TACH>;
+		reset-names = "tachometer";
+		nvidia,pulse-per-rev = <2>;
+		nvidia,capture-window-len = <2>;
+		status = "disabled";
+	};
-- 
2.1.4

[PATCH V2 4/9] arm64: tegra: Add Tachometer Controller on Tegra186

[Rajkumar Rampelli]

The NVIDIA Tegra186 SoC has a Tachometer Controller that analyzes the
PWM signal of a Fan and reports the period value through pwm interface.

Signed-off-by: Rajkumar Rampelli <rrajk@nvidia.com>
---

V2: Renamed clock-names/reset-names dt properties values to "tachometer"
    Renamed compatible property value to "nvidia-tegra186-pwm-tachometer"

 arch/arm64/boot/dts/nvidia/tegra186-p2771-0000.dts |  5 +++++
 arch/arm64/boot/dts/nvidia/tegra186.dtsi           | 11 +++++++++++
 2 files changed, 16 insertions(+)

diff --git a/arch/arm64/boot/dts/nvidia/tegra186-p2771-0000.dts b/arch/arm64/boot/dts/nvidia/tegra186-p2771-0000.dts
index bd5305a..13c3e59 100644
--- a/arch/arm64/boot/dts/nvidia/tegra186-p2771-0000.dts
+++ b/arch/arm64/boot/dts/nvidia/tegra186-p2771-0000.dts
@@ -172,4 +172,9 @@
 			vin-supply = <&vdd_5v0_sys>;
 		};
 	};
+
+	tachometer@39c0000 {
+		nvidia,pulse-per-rev = <2>;
+		nvidia,capture-window-len = <2>;
+	};
 };
diff --git a/arch/arm64/boot/dts/nvidia/tegra186.dtsi b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
index 731cd01..19e1afc 100644
--- a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
@@ -1042,4 +1042,15 @@
 		reset-names = "pwm";
 		status = "disabled";
 	};
+
+	tegra_tachometer: tachometer@39c0000 {
+		compatible = "nvidia,tegra186-pwm-tachometer";
+		reg = <0x0 0x039c0000 0x0 0x10>;
+		#pwm-cells = <2>;
+		clocks = <&bpmp TEGRA186_CLK_TACH>;
+		clock-names = "tachometer";
+		resets = <&bpmp TEGRA186_RESET_TACH>;
+		reset-names = "tachometer";
+		status = "disabled";
+	};
 };
-- 
2.1.4

[PATCH V2 5/9] pwm: tegra: Add PWM based Tachometer driver

[Rajkumar Rampelli]

PWM Tachometer driver capture the PWM signal which is output of FAN
in general and provide the period of PWM signal which is converted to
RPM by SW.

Add Tegra Tachometer driver which implements the pwm-capture to
measure period.

Signed-off-by: Rajkumar Rampelli <rrajk@nvidia.com>
Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---

V2: Renamed compatible string to "nvidia-tegra186-pwm-tachometer"
    Renamed arguments of reset and clk apis to "tachometer" from "tach"
 
 drivers/pwm/Kconfig                |  10 ++
 drivers/pwm/Makefile               |   1 +
 drivers/pwm/pwm-tegra-tachometer.c | 303 +++++++++++++++++++++++++++++++++++++
 3 files changed, 314 insertions(+)
 create mode 100644 drivers/pwm/pwm-tegra-tachometer.c

diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 763ee50..29aeeeb 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -454,6 +454,16 @@ config PWM_TEGRA
 	  To compile this driver as a module, choose M here: the module
 	  will be called pwm-tegra.
 
+config PWM_TEGRA_TACHOMETER
+	tristate "NVIDIA Tegra Tachometer PWM driver"
+	depends on ARCH_TEGRA
+	help
+	  NVIDIA Tegra Tachometer reads the PWM signal and reports the PWM
+	  signal periods. This helps in measuring the fan speed where Fan
+	  output for speed is PWM signal.
+
+	  This driver support the Tachometer driver in PWM framework.
+
 config  PWM_TIECAP
 	tristate "ECAP PWM support"
 	depends on ARCH_OMAP2PLUS || ARCH_DAVINCI_DA8XX || ARCH_KEYSTONE
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index 0258a74..14c183e 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -45,6 +45,7 @@ obj-$(CONFIG_PWM_STM32_LP)	+= pwm-stm32-lp.o
 obj-$(CONFIG_PWM_STMPE)		+= pwm-stmpe.o
 obj-$(CONFIG_PWM_SUN4I)		+= pwm-sun4i.o
 obj-$(CONFIG_PWM_TEGRA)		+= pwm-tegra.o
+obj-$(CONFIG_PWM_TEGRA_TACHOMETER)	+= pwm-tegra-tachometer.o
 obj-$(CONFIG_PWM_TIECAP)	+= pwm-tiecap.o
 obj-$(CONFIG_PWM_TIEHRPWM)	+= pwm-tiehrpwm.o
 obj-$(CONFIG_PWM_TIPWMSS)	+= pwm-tipwmss.o
diff --git a/drivers/pwm/pwm-tegra-tachometer.c b/drivers/pwm/pwm-tegra-tachometer.c
new file mode 100644
index 0000000..bcc44ce
--- /dev/null
+++ b/drivers/pwm/pwm-tegra-tachometer.c
@@ -0,0 +1,303 @@
+/*
+ * Tegra Tachometer Pulse-Width-Modulation driver
+ *
+ * Copyright (c) 2017-2018, NVIDIA CORPORATION.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ */
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/pwm.h>
+#include <linux/reset.h>
+#include <linux/slab.h>
+
+/* Since oscillator clock (38.4MHz) serves as a clock source for
+ * the tach input controller, 1.0105263MHz (i.e. 38.4/38) has to be
+ * used as a clock value in the RPM calculations
+ */
+#define TACH_COUNTER_CLK			1010526
+
+#define TACH_FAN_TACH0				0x0
+#define TACH_FAN_TACH0_PERIOD_MASK		0x7FFFF
+#define TACH_FAN_TACH0_PERIOD_MAX		0x7FFFF
+#define TACH_FAN_TACH0_PERIOD_MIN		0x0
+#define TACH_FAN_TACH0_WIN_LENGTH_SHIFT		25
+#define TACH_FAN_TACH0_WIN_LENGTH_MASK		0x3
+#define TACH_FAN_TACH0_OVERFLOW_MASK		BIT(24)
+
+#define TACH_FAN_TACH1				0x4
+#define TACH_FAN_TACH1_HI_MASK			0x7FFFF
+/*
+ * struct pwm_tegra_tach - Tegra tachometer object
+ * @dev: device providing the Tachometer
+ * @pulse_per_rev: Pulses per revolution of a Fan
+ * @capture_window_len: Defines the window of the FAN TACH monitor
+ * @regs: physical base addresses of the controller
+ * @clk: phandle list of tachometer clocks
+ * @rst: phandle to reset the controller
+ * @chip: PWM chip providing this PWM device
+ */
+struct pwm_tegra_tach {
+	struct device		*dev;
+	void __iomem		*regs;
+	struct clk		*clk;
+	struct reset_control	*rst;
+	u32			pulse_per_rev;
+	u32			capture_window_len;
+	struct pwm_chip		chip;
+};
+
+static struct pwm_tegra_tach *to_tegra_pwm_chip(struct pwm_chip *chip)
+{
+	return container_of(chip, struct pwm_tegra_tach, chip);
+}
+
+static u32 tachometer_readl(struct pwm_tegra_tach *ptt, unsigned long reg)
+{
+	return readl(ptt->regs + reg);
+}
+
+static inline void tachometer_writel(struct pwm_tegra_tach *ptt, u32 val,
+				     unsigned long reg)
+{
+	writel(val, ptt->regs + reg);
+}
+
+static int pwm_tegra_tach_set_wlen(struct pwm_tegra_tach *ptt,
+				   u32 window_length)
+{
+	u32 tach0, wlen;
+
+	/*
+	 * As per FAN Spec, the window length value should be greater than or
+	 * equal to Pulses Per Revolution value to measure the time period
+	 * values accurately.
+	 */
+	if (ptt->pulse_per_rev > ptt->capture_window_len) {
+		dev_err(ptt->dev,
+			"Window length value < pulses per revolution value\n");
+		return -EINVAL;
+	}
+
+	if (hweight8(window_length) != 1) {
+		dev_err(ptt->dev,
+			"Valid value of window length is {1, 2, 4 or 8}\n");
+		return -EINVAL;
+	}
+
+	wlen = ffs(window_length) - 1;
+	tach0 = tachometer_readl(ptt, TACH_FAN_TACH0);
+	tach0 &= ~(TACH_FAN_TACH0_WIN_LENGTH_MASK <<
+			TACH_FAN_TACH0_WIN_LENGTH_SHIFT);
+	tach0 |= wlen << TACH_FAN_TACH0_WIN_LENGTH_SHIFT;
+	tachometer_writel(ptt, tach0, TACH_FAN_TACH0);
+
+	return 0;
+}
+
+static int pwm_tegra_tach_capture(struct pwm_chip *chip,
+				  struct pwm_device *pwm,
+				  struct pwm_capture *result,
+				  unsigned long timeout)
+{
+	struct pwm_tegra_tach *ptt = to_tegra_pwm_chip(chip);
+	unsigned long period;
+	u32 tach;
+
+	tach = tachometer_readl(ptt, TACH_FAN_TACH1);
+	result->duty_cycle = tach & TACH_FAN_TACH1_HI_MASK;
+
+	tach = tachometer_readl(ptt, TACH_FAN_TACH0);
+	if (tach & TACH_FAN_TACH0_OVERFLOW_MASK) {
+		/* Fan is stalled, clear overflow state by writing 1 */
+		dev_dbg(ptt->dev, "Tachometer Overflow is detected\n");
+		tachometer_writel(ptt, tach, TACH_FAN_TACH0);
+	}
+
+	period = tach & TACH_FAN_TACH0_PERIOD_MASK;
+	if ((period == TACH_FAN_TACH0_PERIOD_MIN) ||
+	    (period == TACH_FAN_TACH0_PERIOD_MAX)) {
+		dev_dbg(ptt->dev, "Period set to min/max 0x%lx, Invalid RPM\n",
+			period);
+		result->period = 0;
+		result->duty_cycle = 0;
+		return 0;
+	}
+
+	period = period + 1;
+
+	period = DIV_ROUND_CLOSEST_ULL(period * ptt->pulse_per_rev * 1000000ULL,
+				       ptt->capture_window_len *
+				       TACH_COUNTER_CLK);
+
+	/*
+	 * period & duty cycle values are in units of micro seconds.
+	 * Hence, convert them into nano seconds and store.
+	 */
+	result->period = period * 1000;
+	result->duty_cycle = result->duty_cycle * 1000;
+
+	return 0;
+}
+
+static const struct pwm_ops pwm_tegra_tach_ops = {
+	.capture = pwm_tegra_tach_capture,
+	.owner = THIS_MODULE,
+};
+
+static int pwm_tegra_tach_read_platform_data(struct pwm_tegra_tach *ptt)
+{
+	struct device_node *np = ptt->dev->of_node;
+	u32 pval;
+	int err = 0;
+
+	err = of_property_read_u32(np, "nvidia,pulse-per-rev", &pval);
+	if (err < 0) {
+		dev_err(ptt->dev,
+			"\"nvidia,pulse-per-rev\" property is missing\n");
+		return err;
+	}
+	ptt->pulse_per_rev = pval;
+
+	err = of_property_read_u32(np, "nvidia,capture-window-len", &pval);
+	if (err < 0) {
+		dev_err(ptt->dev,
+			"\"nvidia,capture-window-len\" property is missing\n");
+		return err;
+	}
+	ptt->capture_window_len = pval;
+
+	return err;
+}
+
+static int pwm_tegra_tach_probe(struct platform_device *pdev)
+{
+	struct pwm_tegra_tach *ptt;
+	struct resource *res;
+	int err = 0;
+
+	ptt = devm_kzalloc(&pdev->dev, sizeof(*ptt), GFP_KERNEL);
+	if (!ptt)
+		return -ENOMEM;
+
+	ptt->dev = &pdev->dev;
+
+	err = pwm_tegra_tach_read_platform_data(ptt);
+	if (err < 0)
+		return err;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	ptt->regs = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(ptt->regs)) {
+		dev_err(&pdev->dev, "Failed to remap I/O memory\n");
+		return PTR_ERR(ptt->regs);
+	}
+
+	platform_set_drvdata(pdev, ptt);
+
+	ptt->clk = devm_clk_get(&pdev->dev, "tachometer");
+	if (IS_ERR(ptt->clk)) {
+		err = PTR_ERR(ptt->clk);
+		dev_err(&pdev->dev, "Failed to get Tachometer clk: %d\n", err);
+		return err;
+	}
+
+	ptt->rst = devm_reset_control_get(&pdev->dev, "tachometer");
+	if (IS_ERR(ptt->rst)) {
+		err = PTR_ERR(ptt->rst);
+		dev_err(&pdev->dev, "Failed to get reset handle: %d\n", err);
+		return err;
+	}
+
+	err = clk_prepare_enable(ptt->clk);
+	if (err < 0) {
+		dev_err(&pdev->dev, "Failed to prepare clock: %d\n", err);
+		return err;
+	}
+
+	err = clk_set_rate(ptt->clk, TACH_COUNTER_CLK);
+	if (err < 0) {
+		dev_err(&pdev->dev, "Failed to set clock rate %d: %d\n",
+			TACH_COUNTER_CLK, err);
+		goto clk_unprep;
+	}
+
+	reset_control_reset(ptt->rst);
+
+	ptt->chip.dev = &pdev->dev;
+	ptt->chip.ops = &pwm_tegra_tach_ops;
+	ptt->chip.base = -1;
+	ptt->chip.npwm = 1;
+
+	err = pwmchip_add(&ptt->chip);
+	if (err < 0) {
+		dev_err(&pdev->dev, "Failed to add tachometer PWM: %d\n", err);
+		goto reset_assert;
+	}
+
+	err = pwm_tegra_tach_set_wlen(ptt, ptt->capture_window_len);
+	if (err < 0) {
+		dev_err(ptt->dev, "Failed to set window length: %d\n", err);
+		goto pwm_remove;
+	}
+
+	return 0;
+
+pwm_remove:
+	pwmchip_remove(&ptt->chip);
+
+reset_assert:
+	reset_control_assert(ptt->rst);
+
+clk_unprep:
+	clk_disable_unprepare(ptt->clk);
+
+	return err;
+}
+
+static int pwm_tegra_tach_remove(struct platform_device *pdev)
+{
+	struct pwm_tegra_tach *ptt = platform_get_drvdata(pdev);
+
+	reset_control_assert(ptt->rst);
+
+	clk_disable_unprepare(ptt->clk);
+
+	return pwmchip_remove(&ptt->chip);
+}
+
+static const struct of_device_id pwm_tegra_tach_of_match[] = {
+	{ .compatible = "nvidia,tegra186-pwm-tachometer" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, pwm_tegra_tach_of_match);
+
+static struct platform_driver tegra_tach_driver = {
+	.driver = {
+		.name = "pwm-tegra-tachometer",
+		.of_match_table = pwm_tegra_tach_of_match,
+	},
+	.probe = pwm_tegra_tach_probe,
+	.remove = pwm_tegra_tach_remove,
+};
+
+module_platform_driver(tegra_tach_driver);
+
+MODULE_DESCRIPTION("PWM based NVIDIA Tegra Tachometer driver");
+MODULE_AUTHOR("Rajkumar Rampelli <rrajk@nvidia.com>");
+MODULE_AUTHOR("Laxman Dewangan <ldewangan@nvidia.com>");
+MODULE_LICENSE("GPL v2");
-- 
2.1.4

[PATCH V2 6/9] arm64: tegra: Add pwm based fan support on Tegra186

[Rajkumar Rampelli]

Add pwm fan driver support on Tegra186 SoC.

Signed-off-by: Rajkumar Rampelli <rrajk@nvidia.com>
---

V2: Removed generic-pwm-tachometer driver dt node and using pwm-fan driver
    for reading fan speed.

 arch/arm64/boot/dts/nvidia/tegra186.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/boot/dts/nvidia/tegra186.dtsi b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
index 19e1afc..27ae73e 100644
--- a/arch/arm64/boot/dts/nvidia/tegra186.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra186.dtsi
@@ -1053,4 +1053,10 @@
 		reset-names = "tachometer";
 		status = "disabled";
 	};
+
+	pwm_fan {
+		compatible = "pwm-fan";
+		pwms = <&tegra_tachometer 0 1000000>;
+		status = "disabled";
+	};
 };
-- 
2.1.4

[PATCH V2 7/9] hwmon: pwm-fan: add sysfs node to read rpm of fan

[Rajkumar Rampelli]

Add fan device attribute fan1_input in pwm-fan driver
to read speed of fan in rotations per minute.

Signed-off-by: Rajkumar Rampelli <rrajk@nvidia.com>
---

V2: Removed generic-pwm-tachometer driver and using pwm-fan driver as per suggestions
    to read fan speed.
    Added fan device attribute to report speed of fan in rpms through hwmon sysfs.

 drivers/hwmon/pwm-fan.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
index 70cc0d1..8dda209 100644
--- a/drivers/hwmon/pwm-fan.c
+++ b/drivers/hwmon/pwm-fan.c
@@ -98,11 +98,34 @@ static ssize_t show_pwm(struct device *dev,
 	return sprintf(buf, "%u\n", ctx->pwm_value);
 }
 
+static ssize_t show_rpm(struct device *dev, struct device_attribute *attr,
+			char *buf)
+{
+	struct pwm_fan_ctx *ptt = dev_get_drvdata(dev);
+	struct pwm_device *pwm = ptt->pwm;
+	struct pwm_capture result;
+	unsigned int rpm = 0;
+	int ret;
+
+	ret = pwm_capture(pwm, &result, 0);
+	if (ret < 0) {
+		pr_err("Failed to capture PWM: %d\n", ret);
+		return ret;
+	}
+
+	if (result.period)
+		rpm = DIV_ROUND_CLOSEST_ULL(60ULL * NSEC_PER_SEC,
+					    result.period);
+
+	return sprintf(buf, "%u\n", rpm);
+}
 
 static SENSOR_DEVICE_ATTR(pwm1, S_IRUGO | S_IWUSR, show_pwm, set_pwm, 0);
+static SENSOR_DEVICE_ATTR(fan1_input, 0444, show_rpm, NULL, 0);
 
 static struct attribute *pwm_fan_attrs[] = {
 	&sensor_dev_attr_pwm1.dev_attr.attr,
+	&sensor_dev_attr_fan1_input.dev_attr.attr,
 	NULL,
 };
 
-- 
2.1.4

[PATCH V2 8/9] arm64: defconfig: enable Nvidia Tegra Tachometer as a module

[Rajkumar Rampelli]

Tegra Tachometer driver implements PWM capture to measure
period. Enable this driver as a module in the ARM64 defconfig.

Signed-off-by: Rajkumar Rampelli <rrajk@nvidia.com>
---

V2: No changes in this patch

 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 634b373..8b2bda7 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -550,6 +550,7 @@ CONFIG_PWM_MESON=m
 CONFIG_PWM_ROCKCHIP=y
 CONFIG_PWM_SAMSUNG=y
 CONFIG_PWM_TEGRA=m
+CONFIG_PWM_TEGRA_TACHOMETER=m
 CONFIG_PHY_RCAR_GEN3_USB2=y
 CONFIG_PHY_HI6220_USB=y
 CONFIG_PHY_QCOM_USB_HS=y
-- 
2.1.4

[PATCH V2 9/9] arm64: defconfig: enable pwm-fan as a loadable module

[Rajkumar Rampelli]

Enable pwm-fan driver to make use of a PWM interface to
read speed of a fan in rotations per minute.

Signed-off-by: Rajkumar Rampelli <rrajk@nvidia.com>
---

V2: Added pwm-fan driver support as a loadable module.
    Removed generic-pwm-tachometer driver support which was added as part of v1

 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 8b2bda7..50aa3bce 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -320,6 +320,7 @@ CONFIG_SYSCON_REBOOT_MODE=y
 CONFIG_BATTERY_BQ27XXX=y
 CONFIG_SENSORS_ARM_SCPI=y
 CONFIG_SENSORS_LM90=m
+CONFIG_SENSORS_PWM_FAN=m
 CONFIG_SENSORS_INA2XX=m
 CONFIG_THERMAL_GOV_POWER_ALLOCATOR=y
 CONFIG_CPU_THERMAL=y
-- 
2.1.4

Re: [PATCH V2 7/9] hwmon: pwm-fan: add sysfs node to read rpm of fan

[Guenter Roeck]

On 03/20/2018 09:40 PM, Rajkumar Rampelli wrote:
> Add fan device attribute fan1_input in pwm-fan driver
> to read speed of fan in rotations per minute.
> 
> Signed-off-by: Rajkumar Rampelli <rrajk@nvidia.com>
> ---
> 
> V2: Removed generic-pwm-tachometer driver and using pwm-fan driver as per suggestions
>      to read fan speed.
>      Added fan device attribute to report speed of fan in rpms through hwmon sysfs.
> 
>   drivers/hwmon/pwm-fan.c | 23 +++++++++++++++++++++++
>   1 file changed, 23 insertions(+)
> 
> diff --git a/drivers/hwmon/pwm-fan.c b/drivers/hwmon/pwm-fan.c
> index 70cc0d1..8dda209 100644
> --- a/drivers/hwmon/pwm-fan.c
> +++ b/drivers/hwmon/pwm-fan.c
> @@ -98,11 +98,34 @@ static ssize_t show_pwm(struct device *dev,
>   	return sprintf(buf, "%u\n", ctx->pwm_value);
>   }
>   
> +static ssize_t show_rpm(struct device *dev, struct device_attribute *attr,
> +			char *buf)
> +{
> +	struct pwm_fan_ctx *ptt = dev_get_drvdata(dev);
> +	struct pwm_device *pwm = ptt->pwm;
> +	struct pwm_capture result;
> +	unsigned int rpm = 0;
> +	int ret;
> +
> +	ret = pwm_capture(pwm, &result, 0);
> +	if (ret < 0) {
> +		pr_err("Failed to capture PWM: %d\n", ret);
> +		return ret;
> +	}
> +
> +	if (result.period)
> +		rpm = DIV_ROUND_CLOSEST_ULL(60ULL * NSEC_PER_SEC,
> +					    result.period);
> +
> +	return sprintf(buf, "%u\n", rpm);
> +}
>   
>   static SENSOR_DEVICE_ATTR(pwm1, S_IRUGO | S_IWUSR, show_pwm, set_pwm, 0);
> +static SENSOR_DEVICE_ATTR(fan1_input, 0444, show_rpm, NULL, 0);
>   
>   static struct attribute *pwm_fan_attrs[] = {
>   	&sensor_dev_attr_pwm1.dev_attr.attr,
> +	&sensor_dev_attr_fan1_input.dev_attr.attr,

This doesn't make sense. The same pwm can not both control the fan speed
and report it.

Guenter

>   	NULL,
>   };
>   
> 

Re: [PATCH V2 3/9] dt-bindings: Tegra186 tachometer device tree bindings

[Rob Herring]

On Wed, Mar 21, 2018 at 10:10:38AM +0530, Rajkumar Rampelli wrote:
> Supply Device tree binding documentation for the NVIDIA
> Tegra186 SoC's Tachometer Controller
> 
> Signed-off-by: Rajkumar Rampelli <rrajk@nvidia.com>
> ---
> 
> V2: Renamed compatible string to "nvidia,tegra186-pwm-tachometer"
>     Renamed dt property values of clock-names and reset-names to "tachometer"
>     from "tach"

Read my prior comments on v1.

Rob

Re: [PATCH V2 3/9] dt-bindings: Tegra186 tachometer device tree bindings

[Rob Herring]

On Tue, Mar 27, 2018 at 09:52:49AM -0500, Rob Herring wrote:
> On Wed, Mar 21, 2018 at 10:10:38AM +0530, Rajkumar Rampelli wrote:
> > Supply Device tree binding documentation for the NVIDIA
> > Tegra186 SoC's Tachometer Controller
> > 
> > Signed-off-by: Rajkumar Rampelli <rrajk@nvidia.com>
> > ---
> > 
> > V2: Renamed compatible string to "nvidia,tegra186-pwm-tachometer"
> >     Renamed dt property values of clock-names and reset-names to "tachometer"
> >     from "tach"
> 
> Read my prior comments on v1.

Also, I'm trying to make sense of who you Cc'ed on this. There's a ton 
of folks I know that I'm pretty sure don't care about this series. Start 
with get_maintainers.pl and add people you know need to see this series.

Rob

Re: [PATCH V2 3/9] dt-bindings: Tegra186 tachometer device tree bindings

[Mikko Perttunen]

Rob,

this binding is for a specific IP block (for measuring/aggregating input 
pulses) on the Tegra186 SoC, so I don't think it fits into any generic 
binding.

Thanks,
Mikko

On 03/27/2018 05:52 PM, Rob Herring wrote:
> On Wed, Mar 21, 2018 at 10:10:38AM +0530, Rajkumar Rampelli wrote:
>> Supply Device tree binding documentation for the NVIDIA
>> Tegra186 SoC's Tachometer Controller
>>
>> Signed-off-by: Rajkumar Rampelli <rrajk@nvidia.com>
>> ---
>>
>> V2: Renamed compatible string to "nvidia,tegra186-pwm-tachometer"
>>      Renamed dt property values of clock-names and reset-names to "tachometer"
>>      from "tach"
> 
> Read my prior comments on v1.
> 
> Rob
> --
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH V2 3/9] dt-bindings: Tegra186 tachometer device tree bindings

[Rob Herring]

On Mon, Apr 9, 2018 at 12:38 AM, Mikko Perttunen <cyndis@kapsi.fi> wrote:
> Rob,

Please don't top post to lists.

> this binding is for a specific IP block (for measuring/aggregating input
> pulses) on the Tegra186 SoC, so I don't think it fits into any generic
> binding.

What is it hooked up to to measure? You only mention "fan" five times
in the doc.

You have #pwm-cells too, so this block has PWM output as well? If not,
then where's the PWM for the fan control because there is no point in
having fan tach without some control mechanism.

There's only so many ways to control fans and types of fans, so yes,
the interface of control and feedback lines between a fan and its
controller should absolutely be generic.

Rob

>
> Thanks,
> Mikko
>
>
> On 03/27/2018 05:52 PM, Rob Herring wrote:
>>
>> On Wed, Mar 21, 2018 at 10:10:38AM +0530, Rajkumar Rampelli wrote:
>>>
>>> Supply Device tree binding documentation for the NVIDIA
>>> Tegra186 SoC's Tachometer Controller
>>>
>>> Signed-off-by: Rajkumar Rampelli <rrajk@nvidia.com>
>>> ---
>>>
>>> V2: Renamed compatible string to "nvidia,tegra186-pwm-tachometer"
>>>      Renamed dt property values of clock-names and reset-names to
>>> "tachometer"
>>>      from "tach"
>>
>>
>> Read my prior comments on v1.
>>
>> Rob
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 3/9] dt-bindings: Tegra186 tachometer device tree bindings

[Mikko Perttunen]

On 04/09/2018 04:21 PM, Rob Herring wrote:
> On Mon, Apr 9, 2018 at 12:38 AM, Mikko Perttunen <cyndis@kapsi.fi> wrote:
>> Rob,
> 
> Please don't top post to lists.
> 
>> this binding is for a specific IP block (for measuring/aggregating input
>> pulses) on the Tegra186 SoC, so I don't think it fits into any generic
>> binding.
> 
> What is it hooked up to to measure? You only mention "fan" five times
> in the doc.

In practice, fans.

> 
> You have #pwm-cells too, so this block has PWM output as well? If not,
> then where's the PWM for the fan control because there is no point in
> having fan tach without some control mechanism.

It doesn't provide a PWM output. The (Linux) PWM framework provides 
functionality in both directions - control and capture. But if the 
device tree #pwm-cells/pwms properties are only for control, we may need 
to introduce a new #capture-pwm-cells/capture-pwms or similar.

The idea is that the generic fan node can then specify two pwms, one for 
control and one for capture, to enable e.g. closed-loop control (I'm not 
personally familiar with the usecase for this but I could imagine 
something like that). The control PWM can be something completely 
different, maybe not a PWM in the first place (e.g. some fixed voltage).

> 
> There's only so many ways to control fans and types of fans, so yes,
> the interface of control and feedback lines between a fan and its
> controller should absolutely be generic.

I'm not quite getting what you mean by this. Clearly we need a custom 
compatibility string for the tachometer as it's a different hardware 
block with different programming than others. Or are you complaining 
about the nvidia,pulse-per-rev/capture-window-len properties?

Thanks,
Mikko

> 
> Rob
> 
>>
>> Thanks,
>> Mikko
>>
>>
>> On 03/27/2018 05:52 PM, Rob Herring wrote:
>>>
>>> On Wed, Mar 21, 2018 at 10:10:38AM +0530, Rajkumar Rampelli wrote:
>>>>
>>>> Supply Device tree binding documentation for the NVIDIA
>>>> Tegra186 SoC's Tachometer Controller
>>>>
>>>> Signed-off-by: Rajkumar Rampelli <rrajk@nvidia.com>
>>>> ---
>>>>
>>>> V2: Renamed compatible string to "nvidia,tegra186-pwm-tachometer"
>>>>       Renamed dt property values of clock-names and reset-names to
>>>> "tachometer"
>>>>       from "tach"
>>>
>>>
>>> Read my prior comments on v1.
>>>
>>> Rob
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH V2 3/9] dt-bindings: Tegra186 tachometer device tree bindings

[Rob Herring]

On Mon, Apr 9, 2018 at 9:37 AM, Mikko Perttunen <cyndis@kapsi.fi> wrote:
>
>
> On 04/09/2018 04:21 PM, Rob Herring wrote:
>>
>> On Mon, Apr 9, 2018 at 12:38 AM, Mikko Perttunen <cyndis@kapsi.fi> wrote:
>>>
>>> Rob,
>>
>>
>> Please don't top post to lists.
>>
>>> this binding is for a specific IP block (for measuring/aggregating input
>>> pulses) on the Tegra186 SoC, so I don't think it fits into any generic
>>> binding.
>>
>>
>> What is it hooked up to to measure? You only mention "fan" five times
>> in the doc.
>
>
> In practice, fans.
>
>>
>> You have #pwm-cells too, so this block has PWM output as well? If not,
>> then where's the PWM for the fan control because there is no point in
>> having fan tach without some control mechanism.
>
>
> It doesn't provide a PWM output. The (Linux) PWM framework provides
> functionality in both directions - control and capture. But if the device
> tree #pwm-cells/pwms properties are only for control, we may need to
> introduce a new #capture-pwm-cells/capture-pwms or similar.

Yes, perhaps. But there is no point in having
#capture-pwm-cells/capture-pwms if you aren't describing the
connection between the fan and the fan controller.

> The idea is that the generic fan node can then specify two pwms, one for
> control and one for capture, to enable e.g. closed-loop control (I'm not
> personally familiar with the usecase for this but I could imagine something
> like that). The control PWM can be something completely different, maybe not
> a PWM in the first place (e.g. some fixed voltage).

Yes. As you can have different types of fans (3-wire, 4-wire, etc.)
they would have different compatibles and differing properties
associated with them.

>> There's only so many ways to control fans and types of fans, so yes,
>> the interface of control and feedback lines between a fan and its
>> controller should absolutely be generic.
>
>
> I'm not quite getting what you mean by this. Clearly we need a custom
> compatibility string for the tachometer as it's a different hardware block
> with different programming than others.

Yes, of course. It's the interface between fan controllers and fans
that I'm concerned about.

> Or are you complaining about the
> nvidia,pulse-per-rev/capture-window-len properties?

Well, those sound like properties of a fan (at least the first one),
so they belong in a fan node.

The aspeed fan controller is probably the closest thing we have to a
fan binding. Look at that if you haven't already.

Rob

Re: [PATCH V2 3/9] dt-bindings: Tegra186 tachometer device tree bindings

[Guenter Roeck]

On 04/10/2018 06:30 AM, Rob Herring wrote:
> On Mon, Apr 9, 2018 at 9:37 AM, Mikko Perttunen <cyndis@kapsi.fi> wrote:
>>
>>
>> On 04/09/2018 04:21 PM, Rob Herring wrote:
>>>
>>> On Mon, Apr 9, 2018 at 12:38 AM, Mikko Perttunen <cyndis@kapsi.fi> wrote:
>>>>
>>>> Rob,
>>>
>>>
>>> Please don't top post to lists.
>>>
>>>> this binding is for a specific IP block (for measuring/aggregating input
>>>> pulses) on the Tegra186 SoC, so I don't think it fits into any generic
>>>> binding.
>>>
>>>
>>> What is it hooked up to to measure? You only mention "fan" five times
>>> in the doc.
>>
>>
>> In practice, fans.
>>
>>>
>>> You have #pwm-cells too, so this block has PWM output as well? If not,
>>> then where's the PWM for the fan control because there is no point in
>>> having fan tach without some control mechanism.
>>
>>
>> It doesn't provide a PWM output. The (Linux) PWM framework provides
>> functionality in both directions - control and capture. But if the device
>> tree #pwm-cells/pwms properties are only for control, we may need to
>> introduce a new #capture-pwm-cells/capture-pwms or similar.
> 
> Yes, perhaps. But there is no point in having
> #capture-pwm-cells/capture-pwms if you aren't describing the
> connection between the fan and the fan controller.
> 
>> The idea is that the generic fan node can then specify two pwms, one for
>> control and one for capture, to enable e.g. closed-loop control (I'm not
>> personally familiar with the usecase for this but I could imagine something
>> like that). The control PWM can be something completely different, maybe not
>> a PWM in the first place (e.g. some fixed voltage).
> 
> Yes. As you can have different types of fans (3-wire, 4-wire, etc.)
> they would have different compatibles and differing properties
> associated with them.
> 
>>> There's only so many ways to control fans and types of fans, so yes,
>>> the interface of control and feedback lines between a fan and its
>>> controller should absolutely be generic.
>>
>>
>> I'm not quite getting what you mean by this. Clearly we need a custom
>> compatibility string for the tachometer as it's a different hardware block
>> with different programming than others.
> 
> Yes, of course. It's the interface between fan controllers and fans
> that I'm concerned about.
> 
>> Or are you complaining about the
>> nvidia,pulse-per-rev/capture-window-len properties?
> 
> Well, those sound like properties of a fan (at least the first one),
> so they belong in a fan node.
> 
> The aspeed fan controller is probably the closest thing we have to a
> fan binding. Look at that if you haven't already.
> 

FWIW, this is a fan speed (tachometer) counter which is modeled as pwm input.
This, in my opinion, and as stated before, is conceptually wrong. The pwm
subsystem should not (need to) know anything about fans, much less about
specifics such as the number of pulses per revolution.

Guenter

Re: [PATCH V2 1/9] pwm: core: Add support for PWM HW driver with pwm capture only

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1848 bytes --]

On Wed, Mar 21, 2018 at 10:10:36AM +0530, Rajkumar Rampelli wrote:
> Add support for pwm HW driver which has only capture functionality.
> This helps to implement the PWM based Tachometer driver which reads
> the PWM output signals from electronic fans.
> 
> PWM Tachometer captures the period and duty cycle of the PWM signal
> 
> Add conditional checks for callabacks enable(), disable(), config()
> to check if they are supported by the client driver or not. Skip these
> callbacks if they are not supported.
> 
> Signed-off-by: Rajkumar Rampelli <rrajk@nvidia.com>
> ---
> 
> V2: Added if conditional checks for pwm callbacks since drivers may
>     implements only pwm capture functionality.
> 
>  drivers/pwm/core.c | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
> index 1581f6a..f70fe68 100644
> --- a/drivers/pwm/core.c
> +++ b/drivers/pwm/core.c
> @@ -246,6 +246,10 @@ static bool pwm_ops_check(const struct pwm_ops *ops)
>  	if (ops->apply)
>  		return true;
>  
> +	/* driver supports capture operation */
> +	if (ops->capture)
> +		return true;
> +
>  	return false;
>  }
>  
> @@ -495,7 +499,8 @@ int pwm_apply_state(struct pwm_device *pwm, struct pwm_state *state)
>  			 * ->apply().
>  			 */
>  			if (pwm->state.enabled) {
> -				pwm->chip->ops->disable(pwm->chip, pwm);
> +				if (pwm->chip->ops->disable)
> +					pwm->chip->ops->disable(pwm->chip, pwm);

This is not a good idea. It means that you'll be able to successfully
configure a capture-only PWM channel for output. I think all of the
output configuration functions should return an error (-ENOSYS?) for
capture-only devices, much like we return -ENOSYS for pwm_capture() if
the driver doesn't implement capture support.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]