All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] pwm: add support for Intel Low Power Subsystem PWM
@ 2014-02-28 14:50 Chew Chiau Ee
  2014-03-12  6:42 ` Mika Westerberg
  2014-03-18 15:28 ` Thierry Reding
  0 siblings, 2 replies; 4+ messages in thread
From: Chew Chiau Ee @ 2014-02-28 14:50 UTC (permalink / raw)
  To: Thierry Reding, Mika Westerberg, Chew Kean Ho, Chang Rebecca Swee Fun
  Cc: linux-pwm, linux-kernel

From: Mika Westerberg <mika.westerberg@linux.intel.com>

Add support for Intel Low Power I/O subsystem PWM controllers found on
Intel BayTrail SoC.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Chew, Kean Ho <kean.ho.chew@intel.com>
Signed-off-by: Chang, Rebecca Swee Fun <rebecca.swee.fun.chang@intel.com>
Signed-off-by: Chew, Chiau Ee <chiau.ee.chew@intel.com>
---
 drivers/pwm/Kconfig    |   10 +++
 drivers/pwm/Makefile   |    1 +
 drivers/pwm/pwm-lpss.c |  179 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 190 insertions(+), 0 deletions(-)
 create mode 100644 drivers/pwm/pwm-lpss.c

diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 0b7a3c9..aaa91cc 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -119,6 +119,16 @@ config PWM_LPC32XX
 	  To compile this driver as a module, choose M here: the module
 	  will be called pwm-lpc32xx.
 
+config PWM_LPSS
+	tristate "Intel LPSS PWM support"
+	depends on ACPI
+	help
+	  Generic PWM framework driver for Intel Low Power Subsystem PWM
+	  controller.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called pwm-lpss.
+
 config PWM_MXS
 	tristate "Freescale MXS PWM support"
 	depends on ARCH_MXS && OF
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index d8906ec..61bf073 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_PWM_IMX)		+= pwm-imx.o
 obj-$(CONFIG_PWM_JZ4740)	+= pwm-jz4740.o
 obj-$(CONFIG_PWM_LP3943)	+= pwm-lp3943.o
 obj-$(CONFIG_PWM_LPC32XX)	+= pwm-lpc32xx.o
+obj-$(CONFIG_PWM_LPSS)		+= pwm-lpss.o
 obj-$(CONFIG_PWM_MXS)		+= pwm-mxs.o
 obj-$(CONFIG_PWM_PCA9685)	+= pwm-pca9685.o
 obj-$(CONFIG_PWM_PUV3)		+= pwm-puv3.o
diff --git a/drivers/pwm/pwm-lpss.c b/drivers/pwm/pwm-lpss.c
new file mode 100644
index 0000000..2ecd945
--- /dev/null
+++ b/drivers/pwm/pwm-lpss.c
@@ -0,0 +1,179 @@
+/*
+ * Intel Low Power Subsystem PWM controller driver
+ *
+ * Copyright (C) 2014, Intel Corporation
+ * Author: Mika Westerberg <mika.westerberg@linux.intel.com>
+ * Author: Chew Kean Ho <kean.ho.chew@intel.com>
+ * Author: Chang Rebecca Swee Fun <rebecca.swee.fun.chang@intel.com>
+ * Author: Chew Chiau Ee <chiau.ee.chew@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/acpi.h>
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/pwm.h>
+#include <linux/platform_device.h>
+
+#define PWM				0x00000000
+#define PWM_ENABLE			BIT(31)
+#define PWM_SW_UPDATE			BIT(30)
+#define PWM_BASE_UNIT_SHIFT		8
+#define PWM_BASE_UNIT_MASK		0x00ffff00
+#define PWM_ON_TIME_DIV_MASK		0x000000ff
+#define PWM_DIVISION_CORRECTION		0x2
+#define PWM_LIMIT			(0x8000 + PWM_DIVISION_CORRECTION)
+#define NSECS_PER_SEC			1000000000UL
+
+struct pwm_lpss_chip {
+	struct pwm_chip chip;
+	void __iomem *regs;
+	struct clk *clk;
+};
+
+static inline struct pwm_lpss_chip *to_lpwm(struct pwm_chip *chip)
+{
+	return container_of(chip, struct pwm_lpss_chip, chip);
+}
+
+static int pwm_lpss_config(struct pwm_chip *chip, struct pwm_device *pwm,
+			   int duty_ns, int period_ns)
+{
+	struct pwm_lpss_chip *lpwm = to_lpwm(chip);
+	u8 on_time_div;
+	unsigned long c = clk_get_rate(lpwm->clk);
+	unsigned long long base_unit, freq = NSECS_PER_SEC;
+	u32 ctrl;
+
+	do_div(freq, period_ns);
+
+	/* The equation is: base_unit = ((freq / c) * 65536) + correction */
+	base_unit = freq * 65536;
+	do_div(base_unit, c);
+	base_unit += PWM_DIVISION_CORRECTION;
+	if (base_unit > PWM_LIMIT)
+		return -EINVAL;
+
+	if (duty_ns <= 0)
+		duty_ns = 1;
+	on_time_div = 255 - (255 * duty_ns / period_ns);
+
+	ctrl = readl(lpwm->regs + PWM);
+	ctrl &= ~(PWM_BASE_UNIT_MASK | PWM_ON_TIME_DIV_MASK);
+	ctrl |= (u16) base_unit << PWM_BASE_UNIT_SHIFT;
+	ctrl |= on_time_div;
+	/* request PWM to update on next cycle */
+	ctrl |= PWM_SW_UPDATE;
+	writel(ctrl, lpwm->regs + PWM);
+
+	return 0;
+}
+
+static int pwm_lpss_enable(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	struct pwm_lpss_chip *lpwm = to_lpwm(chip);
+	u32 ctrl;
+	int ret;
+
+	ret = clk_prepare_enable(lpwm->clk);
+	if (ret)
+		return ret;
+
+	ctrl = readl(lpwm->regs + PWM);
+	writel(ctrl | PWM_ENABLE, lpwm->regs + PWM);
+
+	return 0;
+}
+
+static void pwm_lpss_disable(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	struct pwm_lpss_chip *lpwm = to_lpwm(chip);
+	u32 ctrl;
+
+	ctrl = readl(lpwm->regs + PWM);
+	writel(ctrl & ~PWM_ENABLE, lpwm->regs + PWM);
+
+	clk_disable_unprepare(lpwm->clk);
+}
+
+static const struct pwm_ops pwm_lpss_ops = {
+	.config = pwm_lpss_config,
+	.enable = pwm_lpss_enable,
+	.disable = pwm_lpss_disable,
+	.owner = THIS_MODULE,
+};
+
+static const struct acpi_device_id pwm_lpss_acpi_match[] = {
+	{ "80860F08", 0 },
+	{ "80860F09", 0 },
+	{ },
+};
+MODULE_DEVICE_TABLE(acpi, pwm_lpss_acpi_match);
+
+static int pwm_lpss_probe(struct platform_device *pdev)
+{
+	struct pwm_lpss_chip *lpwm;
+	struct resource *r;
+	int ret;
+
+	lpwm = devm_kzalloc(&pdev->dev, sizeof(*lpwm), GFP_KERNEL);
+	if (!lpwm)
+		return -ENOMEM;
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+	lpwm->regs = devm_ioremap_resource(&pdev->dev, r);
+	if (IS_ERR(lpwm->regs))
+		return PTR_ERR(lpwm->regs);
+
+	lpwm->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(lpwm->clk)) {
+		dev_err(&pdev->dev, "failed to get PWM clock\n");
+		return PTR_ERR(lpwm->clk);
+	}
+
+	lpwm->chip.dev = &pdev->dev;
+	lpwm->chip.ops = &pwm_lpss_ops;
+	lpwm->chip.base = -1;
+	lpwm->chip.npwm = 1;
+
+	ret = pwmchip_add(&lpwm->chip);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to add PWM chip: %d\n", ret);
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, lpwm);
+	return 0;
+}
+
+static int pwm_lpss_remove(struct platform_device *pdev)
+{
+	struct pwm_lpss_chip *lpwm = platform_get_drvdata(pdev);
+	u32 ctrl;
+
+	ctrl = readl(lpwm->regs + PWM);
+	writel(ctrl & ~PWM_ENABLE, lpwm->regs + PWM);
+
+	return pwmchip_remove(&lpwm->chip);
+}
+
+static struct platform_driver pwm_lpss_driver = {
+	.driver = {
+		.name = "pwm-lpss",
+		.acpi_match_table = pwm_lpss_acpi_match,
+	},
+	.probe = pwm_lpss_probe,
+	.remove = pwm_lpss_remove,
+};
+module_platform_driver(pwm_lpss_driver);
+
+MODULE_DESCRIPTION("PWM driver for Intel LPSS");
+MODULE_AUTHOR("Mika Westerberg <mika.westerberg@linux.intel.com>");
+MODULE_LICENSE("GPLv2");
+MODULE_ALIAS("platform:pwm-lpss");
-- 
1.7.4.4


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] pwm: add support for Intel Low Power Subsystem PWM
  2014-02-28 14:50 [PATCH v2] pwm: add support for Intel Low Power Subsystem PWM Chew Chiau Ee
@ 2014-03-12  6:42 ` Mika Westerberg
  2014-03-18 15:28 ` Thierry Reding
  1 sibling, 0 replies; 4+ messages in thread
From: Mika Westerberg @ 2014-03-12  6:42 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Chew Chiau Ee, Chew Kean Ho, Chang Rebecca Swee Fun, linux-pwm,
	linux-kernel

On Fri, Feb 28, 2014 at 10:50:57PM +0800, Chew Chiau Ee wrote:
> From: Mika Westerberg <mika.westerberg@linux.intel.com>
> 
> Add support for Intel Low Power I/O subsystem PWM controllers found on
> Intel BayTrail SoC.
> 
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Signed-off-by: Chew, Kean Ho <kean.ho.chew@intel.com>
> Signed-off-by: Chang, Rebecca Swee Fun <rebecca.swee.fun.chang@intel.com>
> Signed-off-by: Chew, Chiau Ee <chiau.ee.chew@intel.com>

Hi Thierry,

Any comments to this patch? Could you consider merging it?

Thanks.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] pwm: add support for Intel Low Power Subsystem PWM
  2014-02-28 14:50 [PATCH v2] pwm: add support for Intel Low Power Subsystem PWM Chew Chiau Ee
  2014-03-12  6:42 ` Mika Westerberg
@ 2014-03-18 15:28 ` Thierry Reding
  2014-03-19  1:25   ` Chew, Chiau Ee
  1 sibling, 1 reply; 4+ messages in thread
From: Thierry Reding @ 2014-03-18 15:28 UTC (permalink / raw)
  To: Chew Chiau Ee
  Cc: Mika Westerberg, Chew Kean Ho, Chang Rebecca Swee Fun, linux-pwm,
	linux-kernel

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

On Fri, Feb 28, 2014 at 10:50:57PM +0800, Chew Chiau Ee wrote:
> From: Mika Westerberg <mika.westerberg@linux.intel.com>
> 
> Add support for Intel Low Power I/O subsystem PWM controllers found on
> Intel BayTrail SoC.
> 
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Signed-off-by: Chew, Kean Ho <kean.ho.chew@intel.com>
> Signed-off-by: Chang, Rebecca Swee Fun <rebecca.swee.fun.chang@intel.com>
> Signed-off-by: Chew, Chiau Ee <chiau.ee.chew@intel.com>
> ---
>  drivers/pwm/Kconfig    |   10 +++
>  drivers/pwm/Makefile   |    1 +
>  drivers/pwm/pwm-lpss.c |  179 ++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 190 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/pwm/pwm-lpss.c

Sorry for taking so long to get back to you on this. Things have been
somewhat crazy lately.

This generally looks very good, but I found two issues that escaped last
time. See below.

> diff --git a/drivers/pwm/pwm-lpss.c b/drivers/pwm/pwm-lpss.c
[...]
> +static int pwm_lpss_config(struct pwm_chip *chip, struct pwm_device *pwm,
> +			   int duty_ns, int period_ns)
> +{
> +	struct pwm_lpss_chip *lpwm = to_lpwm(chip);
> +	u8 on_time_div;
> +	unsigned long c = clk_get_rate(lpwm->clk);
> +	unsigned long long base_unit, freq = NSECS_PER_SEC;
> +	u32 ctrl;
> +
> +	do_div(freq, period_ns);
> +
> +	/* The equation is: base_unit = ((freq / c) * 65536) + correction */
> +	base_unit = freq * 65536;
> +	do_div(base_unit, c);

clk_get_rate() can return 0, so perhaps it would be safer to check for
the validity of c before dividing by it. This will probably never
happen for your driver or platform, but people may look at your driver
for inspiration at some point, so it should still be handling this
correctly.

> +MODULE_DESCRIPTION("PWM driver for Intel LPSS");
> +MODULE_AUTHOR("Mika Westerberg <mika.westerberg@linux.intel.com>");
> +MODULE_LICENSE("GPLv2");

I think this needs to be "GPL v2".

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: [PATCH v2] pwm: add support for Intel Low Power Subsystem PWM
  2014-03-18 15:28 ` Thierry Reding
@ 2014-03-19  1:25   ` Chew, Chiau Ee
  0 siblings, 0 replies; 4+ messages in thread
From: Chew, Chiau Ee @ 2014-03-19  1:25 UTC (permalink / raw)
  To: Thierry Reding
  Cc: Mika Westerberg, Chew, Kean Ho, Chang, Rebecca Swee Fun,
	linux-pwm, linux-kernel

> On Fri, Feb 28, 2014 at 10:50:57PM +0800, Chew Chiau Ee wrote:
> > From: Mika Westerberg <mika.westerberg@linux.intel.com>
> >
> > Add support for Intel Low Power I/O subsystem PWM controllers found on
> > Intel BayTrail SoC.
> >
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > Signed-off-by: Chew, Kean Ho <kean.ho.chew@intel.com>
> > Signed-off-by: Chang, Rebecca Swee Fun
> > <rebecca.swee.fun.chang@intel.com>
> > Signed-off-by: Chew, Chiau Ee <chiau.ee.chew@intel.com>
> > ---
> >  drivers/pwm/Kconfig    |   10 +++
> >  drivers/pwm/Makefile   |    1 +
> >  drivers/pwm/pwm-lpss.c |  179
> > ++++++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 190 insertions(+), 0 deletions(-)  create mode
> > 100644 drivers/pwm/pwm-lpss.c
> 
> Sorry for taking so long to get back to you on this. Things have been somewhat
> crazy lately.
> 
> This generally looks very good, but I found two issues that escaped last time.
> See below.
>
> > diff --git a/drivers/pwm/pwm-lpss.c b/drivers/pwm/pwm-lpss.c
> [...]
> > +static int pwm_lpss_config(struct pwm_chip *chip, struct pwm_device *pwm,
> > +			   int duty_ns, int period_ns)
> > +{
> > +	struct pwm_lpss_chip *lpwm = to_lpwm(chip);
> > +	u8 on_time_div;
> > +	unsigned long c = clk_get_rate(lpwm->clk);
> > +	unsigned long long base_unit, freq = NSECS_PER_SEC;
> > +	u32 ctrl;
> > +
> > +	do_div(freq, period_ns);
> > +
> > +	/* The equation is: base_unit = ((freq / c) * 65536) + correction */
> > +	base_unit = freq * 65536;
> > +	do_div(base_unit, c);
> 
> clk_get_rate() can return 0, so perhaps it would be safer to check for the
> validity of c before dividing by it. This will probably never happen for your
> driver or platform, but people may look at your driver for inspiration at some
> point, so it should still be handling this correctly.

Agreed. Will update the code accordingly.

> > +MODULE_DESCRIPTION("PWM driver for Intel LPSS");
> MODULE_AUTHOR("Mika
> > +Westerberg <mika.westerberg@linux.intel.com>");
> > +MODULE_LICENSE("GPLv2");
> 
> I think this needs to be "GPL v2".

Ok. Will update this as well.

Thanks for your review!

> 
> Thierry

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2014-03-19  1:27 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-28 14:50 [PATCH v2] pwm: add support for Intel Low Power Subsystem PWM Chew Chiau Ee
2014-03-12  6:42 ` Mika Westerberg
2014-03-18 15:28 ` Thierry Reding
2014-03-19  1:25   ` Chew, Chiau Ee

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.