diff mbox

[v3,06/10] pwm: Add NVIDIA Tegra SoC support

Message ID 1329923841-32017-7-git-send-email-thierry.reding@avionic-design.de
State Superseded, archived
Headers show

Commit Message

Thierry Reding Feb. 22, 2012, 3:17 p.m. UTC
This commit adds a generic PWM framework driver for the PWFM controller
found on NVIDIA Tegra SoCs. The driver is based on code from the
Chromium kernel tree and was originally written by Gary King (NVIDIA)
and later modified by Simon Que (Chromium).

Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
---
Changes in v3:
  - use pwm_device.hwpwm instead of recomputing it
  - update pwm_ops for changes in patch 2

Changes in v2:
  - set pwm_chip.dev field
  - rename clk_enb to enable
  - introduce NUM_PWM macro instead of hard-coding the number of PWM
    devices
  - update comment in tegra_pwm_config
  - fix coding-style for multi-line comments
  - use module_platform_driver
  - change license to GPL

 drivers/pwm/Kconfig     |   10 ++
 drivers/pwm/Makefile    |    1 +
 drivers/pwm/pwm-tegra.c |  263 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 274 insertions(+), 0 deletions(-)
 create mode 100644 drivers/pwm/pwm-tegra.c

Comments

Ryan Mallon Feb. 23, 2012, 1:47 a.m. UTC | #1
On 23/02/12 02:17, Thierry Reding wrote:

> This commit adds a generic PWM framework driver for the PWFM controller
> found on NVIDIA Tegra SoCs. The driver is based on code from the
> Chromium kernel tree and was originally written by Gary King (NVIDIA)
> and later modified by Simon Que (Chromium).
> 
> Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
> ---

<snip>

> +
> +	pwm->chip.dev = &pdev->dev;
> +	pwm->chip.ops = &tegra_pwm_ops;
> +	pwm->chip.base = -1;
> +	pwm->chip.npwm = NUM_PWM;

> +
> +	ret = pwmchip_add(&pwm->chip);


If a driver fails to initialise the pwm_chip structure correctly it can
cause problems in the pwm core. For example, if the dev field doesn't
get set, then you will get an oops if you try to cat the pwm debugfs file.

pwmchip_add should probably verify that the initialisation of the
pwm_chip structure is sane to avoid problems like this.

~Ryan
--
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
Thierry Reding Feb. 23, 2012, 8:14 a.m. UTC | #2
* Ryan Mallon wrote:
> On 23/02/12 02:17, Thierry Reding wrote:
> 
> > This commit adds a generic PWM framework driver for the PWFM controller
> > found on NVIDIA Tegra SoCs. The driver is based on code from the
> > Chromium kernel tree and was originally written by Gary King (NVIDIA)
> > and later modified by Simon Que (Chromium).
> > 
> > Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
> > ---
> 
> <snip>
> 
> > +
> > +	pwm->chip.dev = &pdev->dev;
> > +	pwm->chip.ops = &tegra_pwm_ops;
> > +	pwm->chip.base = -1;
> > +	pwm->chip.npwm = NUM_PWM;
> 
> > +
> > +	ret = pwmchip_add(&pwm->chip);
> 
> 
> If a driver fails to initialise the pwm_chip structure correctly it can
> cause problems in the pwm core. For example, if the dev field doesn't
> get set, then you will get an oops if you try to cat the pwm debugfs file.
> 
> pwmchip_add should probably verify that the initialisation of the
> pwm_chip structure is sane to avoid problems like this.

Absolutely. What would be the best response to an invalid struct pwm_chip? I
suppose at least returning -EINVAL, perhaps complemented with WARN_ON?

Thierry
Ryan Mallon Feb. 23, 2012, 9:25 a.m. UTC | #3
On 23/02/12 19:14, Thierry Reding wrote:

> * Ryan Mallon wrote:
>> On 23/02/12 02:17, Thierry Reding wrote:
>>
>>> This commit adds a generic PWM framework driver for the PWFM controller
>>> found on NVIDIA Tegra SoCs. The driver is based on code from the
>>> Chromium kernel tree and was originally written by Gary King (NVIDIA)
>>> and later modified by Simon Que (Chromium).
>>>
>>> Signed-off-by: Thierry Reding <thierry.reding@avionic-design.de>
>>> ---
>>
>> <snip>
>>
>>> +
>>> +	pwm->chip.dev = &pdev->dev;
>>> +	pwm->chip.ops = &tegra_pwm_ops;
>>> +	pwm->chip.base = -1;
>>> +	pwm->chip.npwm = NUM_PWM;
>>
>>> +
>>> +	ret = pwmchip_add(&pwm->chip);
>>
>>
>> If a driver fails to initialise the pwm_chip structure correctly it can
>> cause problems in the pwm core. For example, if the dev field doesn't
>> get set, then you will get an oops if you try to cat the pwm debugfs file.
>>
>> pwmchip_add should probably verify that the initialisation of the
>> pwm_chip structure is sane to avoid problems like this.
> 
> Absolutely. What would be the best response to an invalid struct pwm_chip? I
> suppose at least returning -EINVAL, perhaps complemented with WARN_ON?


Just returning -EINVAL should be okay. I don't think you need a WARN_ON,
since failing to register the hardware should be enough of a reason for
a user to report a problem.

~Ryan
--
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
Thierry Reding Feb. 24, 2012, 6:48 a.m. UTC | #4
* Ryan Mallon wrote:
> On 23/02/12 19:14, Thierry Reding wrote:
> > * Ryan Mallon wrote:
[...]
> >> pwmchip_add should probably verify that the initialisation of the
> >> pwm_chip structure is sane to avoid problems like this.
> > 
> > Absolutely. What would be the best response to an invalid struct pwm_chip? I
> > suppose at least returning -EINVAL, perhaps complemented with WARN_ON?
> 
> Just returning -EINVAL should be okay. I don't think you need a WARN_ON,
> since failing to register the hardware should be enough of a reason for
> a user to report a problem.

Okay, will do.

Thierry
Stephen Warren Feb. 28, 2012, 9:14 p.m. UTC | #5
Thierry Reding wrote at Wednesday, February 22, 2012 8:17 AM:
> This commit adds a generic PWM framework driver for the PWFM controller
> found on NVIDIA Tegra SoCs. The driver is based on code from the
> Chromium kernel tree and was originally written by Gary King (NVIDIA)
> and later modified by Simon Que (Chromium).

> diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c

> +static int tegra_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,

> +	/*
> +	 * Convert from duty_ns / period_ns to a fixed number of duty
> +	 * ticks per (1 << PWM_DUTY_WIDTH) cycles.
> +	 */
> +	c = duty_ns * ((1 << PWM_DUTY_WIDTH) - 1);

The driver in ChromeOS has the following extra line here:

    c += (period_ns / 2);

and the following extra line in the comment above:

    Also, make sure to round to the nearest integer during division.

I don't know the HW well enough to know if that line should be present?

Otherwise, this looks similar enough to the ChromeOS driver, so:

Acked-by: Stephen Warren <swarren@nvidia.com>
Thierry Reding March 3, 2012, 10:42 p.m. UTC | #6
* Stephen Warren wrote:
> Thierry Reding wrote at Wednesday, February 22, 2012 8:17 AM:
> > This commit adds a generic PWM framework driver for the PWFM controller
> > found on NVIDIA Tegra SoCs. The driver is based on code from the
> > Chromium kernel tree and was originally written by Gary King (NVIDIA)
> > and later modified by Simon Que (Chromium).
> 
> > diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
> 
> > +static int tegra_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> 
> > +	/*
> > +	 * Convert from duty_ns / period_ns to a fixed number of duty
> > +	 * ticks per (1 << PWM_DUTY_WIDTH) cycles.
> > +	 */
> > +	c = duty_ns * ((1 << PWM_DUTY_WIDTH) - 1);
> 
> The driver in ChromeOS has the following extra line here:
> 
>     c += (period_ns / 2);
> 
> and the following extra line in the comment above:
> 
>     Also, make sure to round to the nearest integer during division.
> 
> I don't know the HW well enough to know if that line should be present?
> 
> Otherwise, this looks similar enough to the ChromeOS driver, so:
> 
> Acked-by: Stephen Warren <swarren@nvidia.com>

Those probably weren't there yet when I prepared the first series. I'll have
another look will sync up with it after testing that it still works.

Thierry
Olof Johansson March 5, 2012, 3:39 a.m. UTC | #7
On Sat, Mar 03, 2012 at 11:42:12PM +0100, Thierry Reding wrote:

> Those probably weren't there yet when I prepared the first series. I'll have
> another look will sync up with it after testing that it still works.


Yep, the patch in question is from Simon, and it's at:

http://git.chromium.org/gitweb/?p=chromiumos/third_party/kernel.git;a=commitdiff;h=7778c724bf3211c534f9bcef7b26afc6cb067a82

Essentially, the driver accidentally rounded down a backlight setting of
'1' which is how it was found.


-Olof
--
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
Thierry Reding March 5, 2012, 7 a.m. UTC | #8
* Olof Johansson wrote:
> On Sat, Mar 03, 2012 at 11:42:12PM +0100, Thierry Reding wrote:
> 
> > Those probably weren't there yet when I prepared the first series. I'll have
> > another look will sync up with it after testing that it still works.
> 
> 
> Yep, the patch in question is from Simon, and it's at:
> 
> http://git.chromium.org/gitweb/?p=chromiumos/third_party/kernel.git;a=commitdiff;h=7778c724bf3211c534f9bcef7b26afc6cb067a82
> 
> Essentially, the driver accidentally rounded down a backlight setting of
> '1' which is how it was found.

Thanks for the link, I'll squash it into this patch in the next version.

Thierry
diff mbox

Patch

diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 93c1052..bda6f23 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -9,4 +9,14 @@  menuconfig PWM
 
 if PWM
 
+config PWM_TEGRA
+	tristate "NVIDIA Tegra PWM support"
+	depends on ARCH_TEGRA
+	help
+	  Generic PWM framework driver for the PWFM controller found on NVIDIA
+	  Tegra SoCs.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called pwm-tegra.
+
 endif
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index 3469c3d..12300f5 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -1 +1,2 @@ 
 obj-$(CONFIG_PWM)		+= core.o
+obj-$(CONFIG_PWM_TEGRA)		+= pwm-tegra.o
diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
new file mode 100644
index 0000000..1c95e08
--- /dev/null
+++ b/drivers/pwm/pwm-tegra.c
@@ -0,0 +1,263 @@ 
+/*
+ * drivers/pwm/pwm-tegra.c
+ *
+ * Tegra pulse-width-modulation controller driver
+ *
+ * Copyright (c) 2010, NVIDIA Corporation.
+ * Based on arch/arm/plat-mxc/pwm.c by Sascha Hauer <s.hauer@pengutronix.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that 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.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/pwm.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+#define PWM_ENABLE	(1 << 31)
+#define PWM_DUTY_WIDTH	8
+#define PWM_DUTY_SHIFT	16
+#define PWM_SCALE_WIDTH	13
+#define PWM_SCALE_SHIFT	0
+
+#define NUM_PWM 4
+
+struct tegra_pwm_chip {
+	struct pwm_chip		chip;
+	struct device		*dev;
+
+	struct clk		*clk;
+
+	int			enable[NUM_PWM];
+	void __iomem		*mmio_base;
+};
+
+static inline struct tegra_pwm_chip *to_tegra_pwm_chip(struct pwm_chip *chip)
+{
+	return container_of(chip, struct tegra_pwm_chip, chip);
+}
+
+static inline int pwm_writel(struct tegra_pwm_chip *chip, unsigned int num,
+		unsigned long val)
+{
+	unsigned long offset = num << 4;
+	int rc;
+
+	rc = clk_enable(chip->clk);
+	if (WARN_ON(rc))
+		return rc;
+
+	writel(val, chip->mmio_base + offset);
+	clk_disable(chip->clk);
+
+	return 0;
+}
+
+static int tegra_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
+		int duty_ns, int period_ns)
+{
+	struct tegra_pwm_chip *pc = to_tegra_pwm_chip(chip);
+	unsigned long long c;
+	unsigned long rate, hz;
+	u32 val = 0;
+
+	/*
+	 * Convert from duty_ns / period_ns to a fixed number of duty
+	 * ticks per (1 << PWM_DUTY_WIDTH) cycles.
+	 */
+	c = duty_ns * ((1 << PWM_DUTY_WIDTH) - 1);
+	do_div(c, period_ns);
+
+	val = (u32)c << PWM_DUTY_SHIFT;
+
+	/*
+	 * Compute the prescaler value for which (1 << PWM_DUTY_WIDTH)
+	 * cycles at the PWM clock rate will take period_ns nanoseconds.
+	 */
+	rate = clk_get_rate(pc->clk) >> PWM_DUTY_WIDTH;
+	hz = 1000000000ul / period_ns;
+
+	rate = (rate + (hz / 2)) / hz;
+
+	/*
+	 * Since the actual PWM divider is the register's frequency divider
+	 * field minus 1, we need to decrement to get the correct value to
+	 * write to the register.
+	 */
+	if (rate > 0)
+		rate--;
+
+	/*
+	 * Make sure that the rate will fit in the register's frequency
+	 * divider field.
+	 */
+	if (rate >> PWM_SCALE_WIDTH)
+		return -EINVAL;
+
+	val |= (rate << PWM_SCALE_SHIFT);
+
+	/* If the PWM channel is enabled, keep it enabled */
+	if (pc->enable[pwm->hwpwm])
+		val |= PWM_ENABLE;
+
+	return pwm_writel(pc, pwm->hwpwm, val);
+}
+
+static int tegra_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	struct tegra_pwm_chip *pc = to_tegra_pwm_chip(chip);
+	int rc = 0;
+
+	if (!pc->enable[pwm->hwpwm]) {
+		rc = clk_enable(pc->clk);
+		if (!rc) {
+			unsigned long offset = pwm->hwpwm << 4;
+			u32 val;
+
+			val = readl(pc->mmio_base + offset);
+			val |= PWM_ENABLE;
+			writel(val, pc->mmio_base + offset);
+
+			pc->enable[pwm->hwpwm] = 1;
+		}
+	}
+
+	return 0;
+}
+
+static void tegra_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	struct tegra_pwm_chip *pc = to_tegra_pwm_chip(chip);
+
+	if (pc->enable[pwm->hwpwm]) {
+		unsigned long offset = pwm->hwpwm << 4;
+		u32 val;
+
+		val = readl(pc->mmio_base + offset);
+		val &= ~PWM_ENABLE;
+		writel(val, pc->mmio_base + offset);
+
+		clk_disable(pc->clk);
+		pc->enable[pwm->hwpwm] = 0;
+	}
+}
+
+static struct pwm_ops tegra_pwm_ops = {
+	.config = tegra_pwm_config,
+	.enable = tegra_pwm_enable,
+	.disable = tegra_pwm_disable,
+	.owner = THIS_MODULE,
+};
+
+static int tegra_pwm_probe(struct platform_device *pdev)
+{
+	struct tegra_pwm_chip *pwm;
+	struct resource *r;
+	int ret;
+
+	pwm = devm_kzalloc(&pdev->dev, sizeof(*pwm), GFP_KERNEL);
+	if (!pwm) {
+		dev_err(&pdev->dev, "failed to allocate memory\n");
+		return -ENOMEM;
+	}
+
+	pwm->dev = &pdev->dev;
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!r) {
+		dev_err(&pdev->dev, "no memory resources defined\n");
+		return -ENODEV;
+	}
+
+	r = devm_request_mem_region(&pdev->dev, r->start, resource_size(r),
+			pdev->name);
+	if (!r) {
+		dev_err(&pdev->dev, "failed to request memory\n");
+		return -EBUSY;
+	}
+
+	pwm->mmio_base = devm_ioremap(&pdev->dev, r->start, resource_size(r));
+	if (!pwm->mmio_base) {
+		dev_err(&pdev->dev, "failed to ioremap() region\n");
+		return -ENODEV;
+	}
+
+	platform_set_drvdata(pdev, pwm);
+
+	pwm->clk = clk_get(&pdev->dev, NULL);
+	if (IS_ERR(pwm->clk))
+		return PTR_ERR(pwm->clk);
+
+	pwm->chip.dev = &pdev->dev;
+	pwm->chip.ops = &tegra_pwm_ops;
+	pwm->chip.base = -1;
+	pwm->chip.npwm = NUM_PWM;
+
+	ret = pwmchip_add(&pwm->chip);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "pwmchip_add() failed: %d\n", ret);
+		clk_put(pwm->clk);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int __devexit tegra_pwm_remove(struct platform_device *pdev)
+{
+	struct tegra_pwm_chip *pwm = platform_get_drvdata(pdev);
+	int i;
+
+	if (WARN_ON(!pwm))
+		return -ENODEV;
+
+	pwmchip_remove(&pwm->chip);
+
+	for (i = 0; i < NUM_PWM; i++) {
+		pwm_writel(pwm, i, 0);
+
+		if (pwm->enable[i])
+			clk_disable(pwm->clk);
+	}
+
+	clk_put(pwm->clk);
+
+	return 0;
+}
+
+#ifdef CONFIG_OF
+static struct of_device_id tegra_pwm_of_match[] = {
+	{ .compatible = "nvidia,tegra20-pwm" },
+	{ }
+};
+#endif
+
+static struct platform_driver tegra_pwm_driver = {
+	.driver		= {
+		.name		= "tegra-pwm",
+		.of_match_table	= of_match_ptr(tegra_pwm_of_match),
+	},
+	.probe		= tegra_pwm_probe,
+	.remove		= __devexit_p(tegra_pwm_remove),
+};
+
+module_platform_driver(tegra_pwm_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("NVIDIA Corporation");