[PATCH 0/3] SPEAr320: Add pwm support

[PATCH 0/3] SPEAr320: Add pwm support

[Viresh Kumar]

This patchset adds driver for ST Microelectronics PWM and machine support for
SPEAr320 Machines.

It was earlier reviewed as part of following patchset:
"[PATCH V6 00/17] Adding devices support for all spear machines"

Changes since earlier patchset:
 - Moved driver to drivers/misc/ directory.

Viresh Kumar (3):
  misc/st_pwm: Add support for ST's Pulse Width Modulator
  SPEAr3xx: Add machine support for ST's PWM IP
  SPEAr3xx: Update defconfig for ST's PWM

 MAINTAINERS                                   |    5 +
 arch/arm/configs/spear3xx_defconfig           |    2 +
 arch/arm/mach-spear3xx/include/mach/generic.h |    1 +
 arch/arm/mach-spear3xx/spear320.c             |   16 +
 arch/arm/mach-spear3xx/spear320_evb.c         |    1 +
 drivers/misc/Kconfig                          |    7 +
 drivers/misc/Makefile                         |    1 +
 drivers/misc/st_pwm.c                         |  486 +++++++++++++++++++++++++
 8 files changed, 519 insertions(+), 0 deletions(-)
 create mode 100644 drivers/misc/st_pwm.c

-- 
1.7.2.2

[PATCH 1/3] misc/st_pwm: Add support for ST's Pulse Width Modulator

[Viresh Kumar]

This patch adds support for ST Microelectronics Pulse Width Modulator. This is
currently used by ST's SPEAr platform and tested on the same.

Reviewed-by: Stanley Miao <stanley.miao@windriver.com>
Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
---
 MAINTAINERS           |    5 +
 drivers/misc/Kconfig  |    7 +
 drivers/misc/Makefile |    1 +
 drivers/misc/st_pwm.c |  486 +++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 499 insertions(+), 0 deletions(-)
 create mode 100644 drivers/misc/st_pwm.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 48b0a4f..7c74b99 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6031,6 +6031,11 @@ M:	Jan-Benedict Glaw <jbglaw@lug-owl.de>
 S:	Maintained
 F:	arch/alpha/kernel/srm_env.c
 
+ST Microelectronics Pulse Width Modulator Support
+M:	Viresh Kumar <viresh.kumar@st.com>
+S:	Maintained
+F:	drivers/misc/st_pwm.c
+
 STABLE BRANCH
 M:	Greg Kroah-Hartman <greg@kroah.com>
 L:	stable at kernel.org
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 4e349cd..ae5f250 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -425,6 +425,13 @@ config SPEAR13XX_PCIE_GADGET
 	 entry will be created for that controller. User can use these
 	 sysfs node to configure PCIe EP as per his requirements.
 
+config ST_PWM
+	tristate "ST Microelectronics Pulse Width Modulator"
+	default n
+	help
+	  Support for ST Microelectronics Pulse Width Modulator. Currently it is
+	  present and tested on SPEAr Platform only.
+
 config TI_DAC7512
 	tristate "Texas Instruments DAC7512"
 	depends on SPI && SYSFS
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 5f03172..3361411 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -39,6 +39,7 @@ obj-$(CONFIG_HMC6352)		+= hmc6352.o
 obj-y				+= eeprom/
 obj-y				+= cb710/
 obj-$(CONFIG_SPEAR13XX_PCIE_GADGET)	+= spear13xx_pcie_gadget.o
+obj-$(CONFIG_ST_PWM)		+= st_pwm.o
 obj-$(CONFIG_VMWARE_BALLOON)	+= vmw_balloon.o
 obj-$(CONFIG_ARM_CHARLCD)	+= arm-charlcd.o
 obj-$(CONFIG_PCH_PHUB)		+= pch_phub.o
diff --git a/drivers/misc/st_pwm.c b/drivers/misc/st_pwm.c
new file mode 100644
index 0000000..9556dd2
--- /dev/null
+++ b/drivers/misc/st_pwm.c
@@ -0,0 +1,486 @@
+/*
+ * drivers/misc/st_pwm.c
+ *
+ * ST Microelectronics Pulse Width Modulator driver
+ *
+ * Copyright (C) 2010-2011 ST Microelectronics
+ * Viresh Kumar<viresh.kumar@st.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+/* Tested on ST's SPEAr Platform */
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/ioport.h>
+#include <linux/kernel.h>
+#include <linux/math64.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pwm.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+
+/* PWM registers and bits definitions */
+#define PWMCR			0x00
+#define PWMDCR			0x04
+#define PWMPCR			0x08
+
+#define PWM_EN_MASK		0x1
+#define MIN_PRESCALE		0x00
+#define MAX_PRESCALE		0x3FFF
+#define PRESCALE_SHIFT		2
+#define MIN_DUTY		0x0001
+#define MAX_DUTY		0xFFFF
+#define MAX_PERIOD		0xFFFF
+#define MIN_PERIOD		0x0001
+
+#define PWM_DEVICE_PER_IP	4
+#define PWM_DEVICE_OFFSET	0x10
+
+/* lock for pwm_list */
+static DEFINE_SPINLOCK(list_lock);
+/* list of all pwm ips available in system */
+static LIST_HEAD(pwm_list);
+
+/**
+ * struct pwm_device: struct representing pwm device/channel
+ *
+ * pwmd_id: id of pwm device
+ * pwm: pointer to parent pwm ip
+ * label: used for storing label passed in pwm_request
+ * offset: base address offset from parent pwm mmio_base
+ * busy: represents usage status of a pwm device
+ * lock: lock specific to a pwm device
+ * node: node for adding device to parent pwm's devices list
+ *
+ * Each pwm IP contains four independent pwm device/channels. Some or all of
+ * which may be present in our configuration.
+ */
+struct pwm_device {
+	unsigned pwmd_id;
+	struct pwm *pwm;
+	const char *label;
+	unsigned offset;
+	unsigned busy;
+	spinlock_t lock;
+	struct list_head node;
+};
+
+/**
+ * struct pwm: struct representing pwm ip
+ *
+ * id: id of pwm ip
+ * mmio_base: base address of pwm
+ * clk: pointer to clk structure of pwm ip
+ * clk_enabled: clock enable status
+ * pdev: pointer to pdev structure of pwm
+ * lock: lock specific to current pwm ip
+ * devices: list of devices/childrens of pwm ip
+ * node: node for adding pwm to global list of all pwm ips
+ */
+struct pwm {
+	unsigned id;
+	void __iomem *mmio_base;
+	struct clk *clk;
+	int clk_enabled;
+	struct platform_device *pdev;
+	spinlock_t lock;
+	struct list_head devices;
+	struct list_head node;
+};
+
+/*
+ * period_ns = 10^9 * (PRESCALE + 1) * PV / PWM_CLK_RATE
+ * duty_ns = 10^9 * (PRESCALE + 1) * DC / PWM_CLK_RATE
+ *
+ * PV = (PWM_CLK_RATE * period_ns)/ (10^9 * (PRESCALE + 1))
+ * DC = (PWM_CLK_RATE * duty_ns)/ (10^9 * (PRESCALE + 1))
+ */
+int pwm_config(struct pwm_device *pwmd, int duty_ns, int period_ns)
+{
+	u64 val, div, clk_rate;
+	unsigned long prescale = MIN_PRESCALE, pv, dc;
+	int ret = 0;
+
+	if (!pwmd) {
+		pr_err("pwm: config - NULL pwm device pointer\n");
+		return -EFAULT;
+	}
+
+	if (period_ns == 0 || duty_ns > period_ns) {
+		ret = -EINVAL;
+		goto err;
+	}
+
+	/* TODO: Need to optimize this loop */
+	while (1) {
+		div = 1000000000;
+		div *= 1 + prescale;
+		clk_rate = clk_get_rate(pwmd->pwm->clk);
+		val = clk_rate * period_ns;
+		pv = div64_u64(val, div);
+		val = clk_rate * duty_ns;
+		dc = div64_u64(val, div);
+
+		if ((pv == 0) || (dc == 0)) {
+			ret = -EINVAL;
+			goto err;
+		}
+		if ((pv > MAX_PERIOD) || (dc > MAX_DUTY)) {
+			prescale++;
+			if (prescale > MAX_PRESCALE) {
+				ret = -EINVAL;
+				goto err;
+			}
+			continue;
+		}
+		if ((pv < MIN_PERIOD) || (dc < MIN_DUTY)) {
+			ret = -EINVAL;
+			goto err;
+		}
+		break;
+	}
+
+	/*
+	 * NOTE: the clock to PWM has to be enabled first
+	 * before writing to the registers
+	 */
+	spin_lock(&pwmd->pwm->lock);
+	ret = clk_enable(pwmd->pwm->clk);
+	if (ret) {
+		spin_unlock(&pwmd->pwm->lock);
+		goto err;
+	}
+
+	spin_lock(&pwmd->lock);
+	writel(prescale << PRESCALE_SHIFT, pwmd->pwm->mmio_base +
+			pwmd->offset + PWMCR);
+	writel(dc, pwmd->pwm->mmio_base + pwmd->offset + PWMDCR);
+	writel(pv, pwmd->pwm->mmio_base + pwmd->offset + PWMPCR);
+	spin_unlock(&pwmd->lock);
+	clk_disable(pwmd->pwm->clk);
+	spin_unlock(&pwmd->pwm->lock);
+
+	return 0;
+err:
+	dev_err(&pwmd->pwm->pdev->dev, "pwm config fail\n");
+	return ret;
+}
+EXPORT_SYMBOL(pwm_config);
+
+int pwm_enable(struct pwm_device *pwmd)
+{
+	int ret = 0;
+	u32 val = 0;
+
+	if (!pwmd) {
+		pr_err("pwm: enable - NULL pwm device pointer\n");
+		return -EFAULT;
+	}
+
+	spin_lock(&pwmd->pwm->lock);
+	ret = clk_enable(pwmd->pwm->clk);
+	if (!ret)
+		pwmd->pwm->clk_enabled++;
+	else {
+		spin_unlock(&pwmd->pwm->lock);
+		goto err;
+	}
+
+	spin_lock(&pwmd->lock);
+	val = readl(pwmd->pwm->mmio_base + pwmd->offset + PWMCR);
+	writel(val | PWM_EN_MASK, pwmd->pwm->mmio_base + pwmd->offset + PWMCR);
+	spin_unlock(&pwmd->lock);
+	spin_unlock(&pwmd->pwm->lock);
+	return 0;
+err:
+	dev_err(&pwmd->pwm->pdev->dev, "pwm enable fail\n");
+	return ret;
+}
+EXPORT_SYMBOL(pwm_enable);
+
+void pwm_disable(struct pwm_device *pwmd)
+{
+	if (!pwmd) {
+		pr_err("pwm: disable - NULL pwm device pointer\n");
+		return;
+	}
+
+	spin_lock(&pwmd->pwm->lock);
+	spin_lock(&pwmd->lock);
+	writel(0, pwmd->pwm->mmio_base + pwmd->offset + PWMCR);
+	if (pwmd->pwm->clk_enabled) {
+		clk_disable(pwmd->pwm->clk);
+		pwmd->pwm->clk_enabled--;
+	}
+	spin_unlock(&pwmd->lock);
+	spin_unlock(&pwmd->pwm->lock);
+}
+EXPORT_SYMBOL(pwm_disable);
+
+struct pwm_device *pwm_request(int pwmd_id, const char *label)
+{
+	int found = 0;
+	struct pwm *pwm;
+	struct pwm_device *pwmd = NULL;
+
+	spin_lock(&list_lock);
+	list_for_each_entry(pwm, &pwm_list, node) {
+		spin_lock(&pwm->lock);
+		list_for_each_entry(pwmd, &pwm->devices, node) {
+			if (pwmd->pwmd_id == pwmd_id) {
+				found = 1;
+				break;
+			}
+		}
+		spin_unlock(&pwm->lock);
+		if (found)
+			break;
+	}
+	spin_unlock(&list_lock);
+
+	if (found) {
+		spin_lock(&pwmd->lock);
+		if (pwmd->busy == 0) {
+			pwmd->busy++;
+			pwmd->label = label;
+		} else
+			pwmd = ERR_PTR(-EBUSY);
+		spin_unlock(&pwmd->lock);
+	} else
+		pwmd = ERR_PTR(-ENOENT);
+
+	if (IS_ERR(pwmd))
+		pr_err("pwm: request fail\n");
+
+	return pwmd;
+}
+EXPORT_SYMBOL(pwm_request);
+
+void pwm_free(struct pwm_device *pwmd)
+{
+	if (!pwmd) {
+		pr_err("pwm: disable - NULL pwm device pointer\n");
+		return;
+	}
+
+	spin_lock(&pwmd->lock);
+	if (pwmd->busy) {
+		pwmd->busy--;
+		pwmd->label = NULL;
+	} else {
+		spin_unlock(&pwmd->lock);
+		dev_warn(&pwmd->pwm->pdev->dev, "pwm device already freed\n");
+		return;
+	}
+
+	spin_unlock(&pwmd->lock);
+}
+EXPORT_SYMBOL(pwm_free);
+
+/* creates and add pwmd device to parent pwm's devices list */
+static int add_pwm_device(unsigned int pwmd_id, struct pwm *pwm)
+{
+	struct pwm_device *pwmd;
+
+	pwmd = kzalloc(sizeof(*pwmd), GFP_KERNEL);
+	if (!pwmd)
+		return -ENOMEM;
+
+	pwmd->pwm = pwm;
+	pwmd->busy = 0;
+	pwmd->pwmd_id = pwmd_id + pwm->id * PWM_DEVICE_PER_IP;
+	pwmd->offset = pwmd_id * PWM_DEVICE_OFFSET;
+	spin_lock_init(&pwmd->lock);
+
+	spin_lock(&pwm->lock);
+	list_add_tail(&pwmd->node, &pwm->devices);
+	spin_unlock(&pwm->lock);
+
+	return 0;
+}
+
+/* removes all pwmd devices from parent pwm's devices list */
+static void remove_pwm_devices(struct pwm *pwm)
+{
+	struct pwm_device *pwmd;
+
+	spin_lock(&pwm->lock);
+	list_for_each_entry(pwmd, &pwm->devices, node) {
+		list_del(&pwmd->node);
+		kfree(pwmd);
+	}
+	spin_unlock(&pwm->lock);
+}
+
+static int __devinit st_pwm_probe(struct platform_device *pdev)
+{
+	struct pwm *pwm = NULL;
+	struct resource *res;
+	int ret = 0, pwmd_id = 0;
+
+	pwm = kzalloc(sizeof(*pwm), GFP_KERNEL);
+	if (!pwm) {
+		ret = -ENOMEM;
+		dev_dbg(&pdev->dev, "failed to allocate memory\n");
+		goto err;
+	}
+
+	pwm->clk = clk_get(&pdev->dev, NULL);
+	if (IS_ERR(pwm->clk)) {
+		ret = PTR_ERR(pwm->clk);
+		dev_dbg(&pdev->dev, "Error getting clock\n");
+		goto err_free;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		ret = -ENODEV;
+		dev_dbg(&pdev->dev, "no memory resource defined\n");
+		goto err_free_clk;
+	}
+
+	if (!request_mem_region(res->start, resource_size(res), pdev->name)) {
+		ret = -EBUSY;
+		dev_dbg(&pdev->dev, "failed to request memory resource\n");
+		goto err_free_clk;
+	}
+
+	pwm->mmio_base = ioremap(res->start, resource_size(res));
+	if (!pwm->mmio_base) {
+		ret = -ENOMEM;
+		dev_dbg(&pdev->dev, "failed to ioremap\n");
+		goto err_free_mem;
+	}
+
+	/* initialize pwm structure */
+	pwm->clk_enabled = 0;
+	pwm->pdev = pdev;
+	/* if pdev->id is -1, only one pwm ip is present */
+	if (pdev->id == -1)
+		pwm->id = 0;
+	else
+		pwm->id = pdev->id;
+
+	spin_lock_init(&pwm->lock);
+	INIT_LIST_HEAD(&pwm->devices);
+	platform_set_drvdata(pdev, pwm);
+
+	/* add pwm to pwm list */
+	spin_lock(&list_lock);
+	list_add_tail(&pwm->node, &pwm_list);
+	spin_unlock(&list_lock);
+
+	/* add pwm devices */
+	for (pwmd_id = 0; pwmd_id < PWM_DEVICE_PER_IP; pwmd_id++) {
+		ret = add_pwm_device(pwmd_id, pwm);
+		if (!ret)
+			continue;
+		dev_err(&pdev->dev, "Add device fail for pwm device id: %d\n",
+				pwmd_id);
+	}
+
+	if (list_empty(&pwm->node))
+		goto err_remove_pwm;
+
+	dev_info(&pdev->dev, "Initialization successful\n");
+	return 0;
+
+err_remove_pwm:
+	spin_lock(&list_lock);
+	list_del(&pwm->node);
+	spin_unlock(&list_lock);
+
+	platform_set_drvdata(pdev, NULL);
+	iounmap(pwm->mmio_base);
+err_free_mem:
+	release_mem_region(res->start, resource_size(res));
+err_free_clk:
+	clk_put(pwm->clk);
+err_free:
+	kfree(pwm);
+err:
+	dev_err(&pdev->dev, "Initialization Fail. Error: %d\n", ret);
+
+	return ret;
+}
+
+static int __devexit st_pwm_remove(struct platform_device *pdev)
+{
+	struct pwm *pwm;
+	struct resource *res;
+	int ret = 0;
+
+	pwm = platform_get_drvdata(pdev);
+	if (pwm == NULL) {
+		ret = -ENODEV;
+		dev_dbg(&pdev->dev, "Remove: get_drvdata fail\n");
+		goto err;
+	}
+	platform_set_drvdata(pdev, NULL);
+
+	/* remove pwm devices */
+	remove_pwm_devices(pwm);
+
+	/* remove pwm from pwm_list */
+	spin_lock(&list_lock);
+	list_del(&pwm->node);
+	spin_unlock(&list_lock);
+
+	iounmap(pwm->mmio_base);
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		ret = -ENODEV;
+		dev_dbg(&pdev->dev, "Remove: get_resource fail\n");
+		goto err;
+	}
+	release_mem_region(res->start, resource_size(res));
+
+	if (pwm->clk_enabled)
+		clk_disable(pwm->clk);
+	clk_put(pwm->clk);
+
+	kfree(pwm);
+	return 0;
+
+err:
+	dev_err(&pdev->dev, "Remove: Fail - %d\n", ret);
+	return ret;
+}
+
+static struct platform_driver st_pwm_driver = {
+	.driver = {
+		.name = "st_pwm",
+		.bus = &platform_bus_type,
+		.owner = THIS_MODULE,
+	},
+	.probe = st_pwm_probe,
+	.remove = __devexit_p(st_pwm_remove)
+};
+
+static int __init st_pwm_init(void)
+{
+	int ret = 0;
+
+	ret = platform_driver_register(&st_pwm_driver);
+	if (ret)
+		pr_err("failed to register st_pwm_driver\n");
+
+	return ret;
+}
+module_init(st_pwm_init);
+
+static void __exit st_pwm_exit(void)
+{
+	platform_driver_unregister(&st_pwm_driver);
+}
+module_exit(st_pwm_exit);
+
+MODULE_AUTHOR("Viresh Kumar <viresh.kumar@st.com>");
+MODULE_DESCRIPTION("ST PWM Driver");
+MODULE_LICENSE("GPL");
-- 
1.7.2.2

[PATCH 2/3] SPEAr3xx: Add machine support for ST's PWM IP

[Viresh Kumar]

Reviewed-by: Stanley Miao <stanley.miao@windriver.com>
Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
---
 arch/arm/mach-spear3xx/include/mach/generic.h |    1 +
 arch/arm/mach-spear3xx/spear320.c             |   16 ++++++++++++++++
 arch/arm/mach-spear3xx/spear320_evb.c         |    1 +
 3 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-spear3xx/include/mach/generic.h b/arch/arm/mach-spear3xx/include/mach/generic.h
index 8cc3a73..a88e359 100644
--- a/arch/arm/mach-spear3xx/include/mach/generic.h
+++ b/arch/arm/mach-spear3xx/include/mach/generic.h
@@ -162,6 +162,7 @@ void __init spear310_init(struct pmx_mode *pmx_mode, struct pmx_dev **pmx_devs,
 #ifdef CONFIG_MACH_SPEAR320
 /* Add spear320 machine device structure declarations here */
 extern struct platform_device spear320_plgpio_device;
+extern struct platform_device spear320_pwm_device;
 
 /* pad mux modes */
 extern struct pmx_mode spear320_auto_net_smii_mode;
diff --git a/arch/arm/mach-spear3xx/spear320.c b/arch/arm/mach-spear3xx/spear320.c
index 575493a..d84e986 100644
--- a/arch/arm/mach-spear3xx/spear320.c
+++ b/arch/arm/mach-spear3xx/spear320.c
@@ -548,6 +548,22 @@ struct platform_device spear320_plgpio_device = {
 	.resource = plgpio_resources,
 };
 
+/* pwm device registeration */
+static struct resource pwm_resources[] = {
+	{
+		.start = SPEAR320_PWM_BASE,
+		.end = SPEAR320_PWM_BASE + SZ_4K - 1,
+		.flags = IORESOURCE_MEM,
+	},
+};
+
+struct platform_device spear320_pwm_device = {
+	.name = "st_pwm",
+	.id = -1,
+	.num_resources = ARRAY_SIZE(pwm_resources),
+	.resource = pwm_resources,
+};
+
 /* spear320 routines */
 void __init spear320_init(struct pmx_mode *pmx_mode, struct pmx_dev **pmx_devs,
 		u8 pmx_dev_count)
diff --git a/arch/arm/mach-spear3xx/spear320_evb.c b/arch/arm/mach-spear3xx/spear320_evb.c
index 4d747db..6190660 100644
--- a/arch/arm/mach-spear3xx/spear320_evb.c
+++ b/arch/arm/mach-spear3xx/spear320_evb.c
@@ -50,6 +50,7 @@ static struct platform_device *plat_devs[] __initdata = {
 
 	/* spear320 specific devices */
 	&spear320_plgpio_device,
+	&spear320_pwm_device,
 };
 
 static void __init spear320_evb_init(void)
-- 
1.7.2.2

[PATCH 3/3] SPEAr3xx: Update defconfig for ST's PWM

[Viresh Kumar]

Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
---
 arch/arm/configs/spear3xx_defconfig |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/arm/configs/spear3xx_defconfig b/arch/arm/configs/spear3xx_defconfig
index 07e9613..34c0ac2 100644
--- a/arch/arm/configs/spear3xx_defconfig
+++ b/arch/arm/configs/spear3xx_defconfig
@@ -14,6 +14,8 @@ CONFIG_BINFMT_MISC=y
 CONFIG_UEVENT_HELPER_PATH="/sbin/hotplug"
 CONFIG_BLK_DEV_RAM=y
 CONFIG_BLK_DEV_RAM_SIZE=16384
+CONFIG_MISC_DEVICES=y
+CONFIG_ST_PWM=y
 CONFIG_INPUT_FF_MEMLESS=y
 # CONFIG_INPUT_MOUSEDEV_PSAUX is not set
 # CONFIG_INPUT_KEYBOARD is not set
-- 
1.7.2.2

[PATCH 0/3] SPEAr320: Add pwm support

[Arnd Bergmann]

On Monday 30 May 2011 09:21:02 Viresh Kumar wrote:
>  MAINTAINERS                                   |    5 +
>  arch/arm/configs/spear3xx_defconfig           |    2 +
>  arch/arm/mach-spear3xx/include/mach/generic.h |    1 +
>  arch/arm/mach-spear3xx/spear320.c             |   16 +
>  arch/arm/mach-spear3xx/spear320_evb.c         |    1 +
>  drivers/misc/Kconfig                          |    7 +
>  drivers/misc/Makefile                         |    1 +
>  drivers/misc/st_pwm.c                         |  486 +++++++++++++++++++++++++
>  8 files changed, 519 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/misc/st_pwm.c

No objection to your patch, but I think we need to have
a long-term plan for pwm:

arnd at wuerfel:~/linux-2.6$ git ls-files | grep pwm.c
arch/arm/mach-exynos4/include/mach/pwm-clock.h
arch/arm/mach-mxs/devices/platform-mxs-pwm.c
arch/arm/mach-s3c64xx/include/mach/pwm-clock.h
arch/arm/mach-s5p64x0/include/mach/pwm-clock.h
arch/arm/mach-s5pc100/include/mach/pwm-clock.h
arch/arm/mach-s5pv210/include/mach/pwm-clock.h
arch/arm/mach-vt8500/pwm.c
arch/arm/plat-mxc/devices/platform-mxc_pwm.c
arch/arm/plat-mxc/pwm.c
arch/arm/plat-pxa/pwm.c
arch/arm/plat-s3c24xx/include/mach/pwm-clock.h
arch/arm/plat-samsung/dev-pwm.c
arch/arm/plat-samsung/pwm-clock.c
arch/arm/plat-samsung/pwm.c
arch/mips/jz4740/pwm.c
arch/unicore32/kernel/pwm.c
drivers/leds/leds-atmel-pwm.c
drivers/leds/leds-pwm.c
drivers/mfd/twl6030-pwm.c
drivers/misc/ab8500-pwm.c
drivers/misc/atmel_pwm.c
drivers/misc/ep93xx_pwm.c

My rough feeling is that what this should look like is a new
subsystem that exports a set of operations and has specific
drivers, like:

drivers/pwm/Kconfig
drivers/pwm/pwm_core.c
drivers/pwm/pwm_pxa.c
drivers/pwm/pwm_unicore.c
drivers/pwm/pwm_st.c
drivers/pwm/pwm_...

This is in the spirit of turning code that is organized by platform
into code that is organized by subsystem, and factoring out the
common bits.

A real problem we are going to hit without this is the fact that each
of these drivers exports the same symbols, which prevents you from
building a multi-platform kernel. It will need to export the common
set of symbols and pass through operations to the individual drivers
then:

arnd at wuerfel:~/linux-2.6$ git ls-files |  grep pwm.c | xargs grep EXPORT_SYMBOL -h | sort | uniq -c
      1 EXPORT_SYMBOL(pwm_channel_alloc);
      1 EXPORT_SYMBOL(pwm_channel_free);
      1 EXPORT_SYMBOL(pwm_channel_handler);
      1 EXPORT_SYMBOL(__pwm_channel_onoff);
      1 EXPORT_SYMBOL(pwm_clk_alloc);
      1 EXPORT_SYMBOL(pwm_clk_free);
      7 EXPORT_SYMBOL(pwm_config);
      7 EXPORT_SYMBOL(pwm_disable);
      7 EXPORT_SYMBOL(pwm_enable);
      7 EXPORT_SYMBOL(pwm_free);
      7 EXPORT_SYMBOL(pwm_request);

As you can see here, the one driver is the odd one out, it provides a completely
different API. This is drivers/misc/atmel_pwm.c, and it should eventually get
converted over.

	Arnd

[PATCH 0/3] SPEAr320: Add pwm support

[viresh kumar]

On 05/30/2011 01:15 PM, Arnd Bergmann wrote:
> No objection to your patch, but I think we need to have
> a long-term plan for pwm:
> 
> arnd at wuerfel:~/linux-2.6$ git ls-files | grep pwm.c
> arch/arm/mach-exynos4/include/mach/pwm-clock.h
> arch/arm/mach-mxs/devices/platform-mxs-pwm.c
> arch/arm/mach-s3c64xx/include/mach/pwm-clock.h
> arch/arm/mach-s5p64x0/include/mach/pwm-clock.h
> arch/arm/mach-s5pc100/include/mach/pwm-clock.h
> arch/arm/mach-s5pv210/include/mach/pwm-clock.h
> arch/arm/mach-vt8500/pwm.c
> arch/arm/plat-mxc/devices/platform-mxc_pwm.c
> arch/arm/plat-mxc/pwm.c
> arch/arm/plat-pxa/pwm.c
> arch/arm/plat-s3c24xx/include/mach/pwm-clock.h
> arch/arm/plat-samsung/dev-pwm.c
> arch/arm/plat-samsung/pwm-clock.c
> arch/arm/plat-samsung/pwm.c
> arch/mips/jz4740/pwm.c
> arch/unicore32/kernel/pwm.c
> drivers/leds/leds-atmel-pwm.c
> drivers/leds/leds-pwm.c
> drivers/mfd/twl6030-pwm.c
> drivers/misc/ab8500-pwm.c
> drivers/misc/atmel_pwm.c
> drivers/misc/ep93xx_pwm.c
> 
> My rough feeling is that what this should look like is a new
> subsystem that exports a set of operations and has specific
> drivers, like:
> 
> drivers/pwm/Kconfig
> drivers/pwm/pwm_core.c
> drivers/pwm/pwm_pxa.c
> drivers/pwm/pwm_unicore.c
> drivers/pwm/pwm_st.c
> drivers/pwm/pwm_...
> 

Sascha sent a patchset for something similar earlier.
But don't know what happened to it.

-- 
viresh

[PATCH 0/3] SPEAr320: Add pwm support

[Sascha Hauer]

On Mon, May 30, 2011 at 01:53:20PM +0530, viresh kumar wrote:
> On 05/30/2011 01:15 PM, Arnd Bergmann wrote:
> > No objection to your patch, but I think we need to have
> > a long-term plan for pwm:
> > 
> > arnd at wuerfel:~/linux-2.6$ git ls-files | grep pwm.c
> > arch/arm/mach-exynos4/include/mach/pwm-clock.h
> > arch/arm/mach-mxs/devices/platform-mxs-pwm.c
> > arch/arm/mach-s3c64xx/include/mach/pwm-clock.h
> > arch/arm/mach-s5p64x0/include/mach/pwm-clock.h
> > arch/arm/mach-s5pc100/include/mach/pwm-clock.h
> > arch/arm/mach-s5pv210/include/mach/pwm-clock.h
> > arch/arm/mach-vt8500/pwm.c
> > arch/arm/plat-mxc/devices/platform-mxc_pwm.c
> > arch/arm/plat-mxc/pwm.c
> > arch/arm/plat-pxa/pwm.c
> > arch/arm/plat-s3c24xx/include/mach/pwm-clock.h
> > arch/arm/plat-samsung/dev-pwm.c
> > arch/arm/plat-samsung/pwm-clock.c
> > arch/arm/plat-samsung/pwm.c
> > arch/mips/jz4740/pwm.c
> > arch/unicore32/kernel/pwm.c
> > drivers/leds/leds-atmel-pwm.c
> > drivers/leds/leds-pwm.c
> > drivers/mfd/twl6030-pwm.c
> > drivers/misc/ab8500-pwm.c
> > drivers/misc/atmel_pwm.c
> > drivers/misc/ep93xx_pwm.c
> > 
> > My rough feeling is that what this should look like is a new
> > subsystem that exports a set of operations and has specific
> > drivers, like:
> > 
> > drivers/pwm/Kconfig
> > drivers/pwm/pwm_core.c
> > drivers/pwm/pwm_pxa.c
> > drivers/pwm/pwm_unicore.c
> > drivers/pwm/pwm_st.c
> > drivers/pwm/pwm_...
> > 
> 
> Sascha sent a patchset for something similar earlier.
> But don't know what happened to it.

Nothing yet, unfortunately. It was a straight forward implementation
which basically implements the existing pwm functions and adds
register/unregister functions.
Bill Gatliff posts a pwm framework since 2008. I wasn't aware of his
patches by the time I wrote my pwm layer. Bills patches target more the
motor controller use case and are (in my opinion) too complicated. I
can't agree on his approach, but didn't want to go with competing
patches either.

As Bills patches still didn't make it into the kernel Maybe it's time
to respin my version.

For reference, find my patches here:

https://lkml.org/lkml/2011/1/28/92

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 0/3] SPEAr320: Add pwm support

[Arnd Bergmann]

On Monday 30 May 2011, Sascha Hauer wrote:
> > Sascha sent a patchset for something similar earlier.
> > But don't know what happened to it.
> 
> Nothing yet, unfortunately. It was a straight forward implementation
> which basically implements the existing pwm functions and adds
> register/unregister functions.
> Bill Gatliff posts a pwm framework since 2008. I wasn't aware of his
> patches by the time I wrote my pwm layer. Bills patches target more the
> motor controller use case and are (in my opinion) too complicated. I
> can't agree on his approach, but didn't want to go with competing
> patches either.
> 
> As Bills patches still didn't make it into the kernel Maybe it's time
> to respin my version.

Yes, I think that would be good, then we can add the new SPEAr320 pwm
driver to the new subsystem as a first example, and move the other
drivers over after that.

	Arnd

[PATCH 0/3] SPEAr320: Add pwm support

[viresh kumar]

On 05/30/2011 05:22 PM, Arnd Bergmann wrote:
> On Monday 30 May 2011, Sascha Hauer wrote:
>>> Sascha sent a patchset for something similar earlier.
>>> But don't know what happened to it.
>>
>> Nothing yet, unfortunately. It was a straight forward implementation
>> which basically implements the existing pwm functions and adds
>> register/unregister functions.
>> Bill Gatliff posts a pwm framework since 2008. I wasn't aware of his
>> patches by the time I wrote my pwm layer. Bills patches target more the
>> motor controller use case and are (in my opinion) too complicated. I
>> can't agree on his approach, but didn't want to go with competing
>> patches either.
>>
>> As Bills patches still didn't make it into the kernel Maybe it's time
>> to respin my version.
> 
> Yes, I think that would be good, then we can add the new SPEAr320 pwm
> driver to the new subsystem as a first example, and move the other
> drivers over after that.
> 

Obviously this can be done, but as it has already taken too long for that
framework to get added in mainline, and still we don't know how much more
time it will take. I would suggest to push this patchset now. And i
will update this accordingly once that framework comes up.

-- 
viresh

[PATCH 0/3] SPEAr320: Add pwm support

[Arnd Bergmann]

On Monday 30 May 2011, viresh kumar wrote:
> >> As Bills patches still didn't make it into the kernel Maybe it's time
> >> to respin my version.
> > 
> > Yes, I think that would be good, then we can add the new SPEAr320 pwm
> > driver to the new subsystem as a first example, and move the other
> > drivers over after that.
> > 
> 
> Obviously this can be done, but as it has already taken too long for that
> framework to get added in mainline, and still we don't know how much more
> time it will take. I would suggest to push this patchset now. And i
> will update this accordingly once that framework comes up.

Ok, makes sense.

How about starting a drivers/pwm directory though? I think we have
broad support for the idea of moving drivers into their subsystems,
so even if you don't change the code, I think it would be a better
place than drivers/misc.

	Arnd

[PATCH 0/3] SPEAr320: Add pwm support

[viresh kumar]

On 05/30/2011 06:23 PM, Arnd Bergmann wrote:
>> > Obviously this can be done, but as it has already taken too long for that
>> > framework to get added in mainline, and still we don't know how much more
>> > time it will take. I would suggest to push this patchset now. And i
>> > will update this accordingly once that framework comes up.
> Ok, makes sense.
> 
> How about starting a drivers/pwm directory though? I think we have
> broad support for the idea of moving drivers into their subsystems,
> so even if you don't change the code, I think it would be a better
> place than drivers/misc.

Ya, i can easily do it. I didn't do it earlier, because i thought people
might ask me to provide framework with it too. New folder with only my driver
as part of it might not be accepted. If nobody have any issues with it
i can send patch with required changes.

One more thing: Who will push drivers/pwm now?
@Andrew: Will that be you??

-- 
viresh

[PATCH 0/3] SPEAr320: Add pwm support

[Arnd Bergmann]

On Tuesday 31 May 2011, viresh kumar wrote:
> On 05/30/2011 06:23 PM, Arnd Bergmann wrote:
> >> > Obviously this can be done, but as it has already taken too long for that
> >> > framework to get added in mainline, and still we don't know how much more
> >> > time it will take. I would suggest to push this patchset now. And i
> >> > will update this accordingly once that framework comes up.
> > Ok, makes sense.
> > 
> > How about starting a drivers/pwm directory though? I think we have
> > broad support for the idea of moving drivers into their subsystems,
> > so even if you don't change the code, I think it would be a better
> > place than drivers/misc.
> 
> Ya, i can easily do it. I didn't do it earlier, because i thought people
> might ask me to provide framework with it too. New folder with only my driver
> as part of it might not be accepted. If nobody have any issues with it
> i can send patch with required changes.
> 
> One more thing: Who will push drivers/pwm now?
> @Andrew: Will that be you??

I can start a branch once we have a new tree for the subarchitectures,
then I'll use the same branch to collect patches that move the other
pwm drivers from arch/arm and from drivers/misc, as well as the
new framework from Sascha.

Note that this doesn't make me maintainer of pwm, that position is
still open ;-)

	Arnd

SPEAr320: Add pwm support

[Arnd Bergmann]

On Monday 30 May 2011, Kurt Van Dijck wrote:
> I was not aware of such. Me too did a generic PWM implementation (not published yet)
> but with 2 differences (apart from code quality I guess):
> 
> * do pwm input too (pwm measurement) since we have some kind of
>   dedicated PWM in a remote CPU over i2c, that can measure them too.
> * /dev interface
> 
> Is this something usefull to try adding?

I'd leave that for later. In particular, user interface design is hard, so
let's consolidate the drivers first and then look at possible extensions.

	Arnd