[PATCHv2 0/4] Add Freescale FTM PWM driver for Vybrid VF610 TOWER

[PATCHv2 0/4] Add Freescale FTM PWM driver for Vybrid VF610 TOWER

[Xiubo Li]

Hello,

This patch series is the Freescale FTM PWM implementation. And there are 8
channels most supported by the FTM PWM. This implementation is only compatible
with device tree definition.

This patch series is based on linux-next and has been tested on Vybrid VF610TWR
board using device tree.

Changes in v2:
- Remove PWM CPWM/EPWM feature and sysfs.
- Remove some redundant code.
- Revise some code for more readable.
- Remove "fsl,pwm-clk-ps", "fsl,pwm-number", "fsl,pwm-channels",etc.
- Add "fsl,pwm-avaliable-chs", "fsl,pwm-counter-clk", etc.
- Support 8 channels default in dtsi file.
- Add counter clock source selection.
- Rename some function name, macro name, etc.
- Use PWM's and OF's existing function interfaces.
- Split clk_unprepare_enable to clk_unprepare and clk_enable,etc.
- ...

Orignal in v1:
- Add Freescale FTM PWM driver support.
- Add Freescale FTM PWM node for VF610.
- Enable Enables FTM PWM device for Vybrid VF610 TOWER.
- Add device tree bindings for Freescale.

[PATCHv2 1/4] pwm: Add Freescale FTM PWM driver support

[Xiubo Li]

Add Freescale FTM PWM driver support. The FTM PWM device
can be found on Vybrid VF610 and Layerscape LS-1 SoCs.

Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
Signed-off-by: Jingchang Lu <b35083@freescale.com>
---
 drivers/pwm/Kconfig       |  10 +
 drivers/pwm/Makefile      |   1 +
 drivers/pwm/pwm-fsl-ftm.c | 633 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 644 insertions(+)
 create mode 100644 drivers/pwm/pwm-fsl-ftm.c

diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 75840b5..8144fb0 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -62,6 +62,16 @@ config PWM_BFIN
 	  To compile this driver as a module, choose M here: the module
 	  will be called pwm-bfin.
 
+config PWM_FSL_FTM
+	tristate "Freescale FTM PWM support"
+	depends on OF
+	help
+	  Generic FTM PWM framework driver for Freescale VF610 and
+	  Layerscape LS-1 SoCs.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called pwm-fsl-ftm.
+
 config PWM_IMX
 	tristate "i.MX PWM support"
 	depends on ARCH_MXC
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index 77a8c18..f383784 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -3,6 +3,7 @@ obj-$(CONFIG_PWM_SYSFS)		+= sysfs.o
 obj-$(CONFIG_PWM_AB8500)	+= pwm-ab8500.o
 obj-$(CONFIG_PWM_ATMEL_TCB)	+= pwm-atmel-tcb.o
 obj-$(CONFIG_PWM_BFIN)		+= pwm-bfin.o
+obj-$(CONFIG_PWM_FSL_FTM)	+= pwm-fsl-ftm.o
 obj-$(CONFIG_PWM_IMX)		+= pwm-imx.o
 obj-$(CONFIG_PWM_JZ4740)	+= pwm-jz4740.o
 obj-$(CONFIG_PWM_LPC32XX)	+= pwm-lpc32xx.o
diff --git a/drivers/pwm/pwm-fsl-ftm.c b/drivers/pwm/pwm-fsl-ftm.c
new file mode 100644
index 0000000..53770c0
--- /dev/null
+++ b/drivers/pwm/pwm-fsl-ftm.c
@@ -0,0 +1,633 @@
+/*
+ *  Freescale FTM PWM Driver
+ *
+ *  Copyright 2013 Freescale Semiconductor, Inc.
+ *
+ * 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.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/err.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/pwm.h>
+#include <linux/of_address.h>
+#include <linux/pinctrl/consumer.h>
+
+#define FTM_SC              0x00
+#define FTMSC_CLK_MASK      0x03
+#define FTMSC_CLK_OFFSET    0x03
+#define FTMSC_CLKSYS        (0x01 << 3)
+#define FTMSC_CLKFIX        (0x02 << 3)
+#define FTMSC_CLKEXT        (0x03 << 3)
+#define FTMSC_PS_MASK       0x07
+#define FTMSC_PS_OFFSET     0x00
+
+#define FTM_CNT             0x04
+#define FTM_MOD             0x08
+
+#define FTM_CSC_BASE        0x0C
+#define FTM_CSC(_channel) \
+	(FTM_CSC_BASE + ((_channel) * 8))
+#define FTMCnSC_MSB         BIT(5)
+#define FTMCnSC_MSA         BIT(4)
+#define FTMCnSC_ELSB        BIT(3)
+#define FTMCnSC_ELSA        BIT(2)
+
+#define FTM_CV_BASE         0x10
+#define FTM_CV(_channel) \
+	(FTM_CV_BASE + ((_channel) * 8))
+
+#define FTM_CNTIN           0x4C
+#define FTM_STATUS          0x50
+
+#define FTM_MODE            0x54
+#define FTMMODE_FTMEN       BIT(0)
+#define FTMMODE_INIT        BIT(2)
+#define FTMMODE_PWMSYNC     BIT(3)
+
+#define FTM_SYNC            0x58
+#define FTM_OUTINIT         0x5C
+#define FTM_OUTMASK         0x60
+#define FTM_COMBINE         0x64
+#define FTM_DEADTIME        0x68
+#define FTM_EXTTRIG         0x6C
+#define FTM_POL             0x70
+#define FTM_FMS             0x74
+#define FTM_FILTER          0x78
+#define FTM_FLTCTRL         0x7C
+#define FTM_QDCTRL          0x80
+#define FTM_CONF            0x84
+#define FTM_FLTPOL          0x88
+#define FTM_SYNCONF         0x8C
+#define FTM_INVCTRL         0x90
+#define FTM_SWOCTRL         0x94
+#define FTM_PWMLOAD         0x98
+
+#define FTM_CNTIN_VAL       0x00
+#define FTM_MAX_CHANNEL     8
+
+enum {
+	FSL_INVALID = 0,
+	FSL_AVAILABLE,
+};
+
+enum {
+	FSL_COUNTER_CLK_SYS = 0,
+	FSL_COUNTER_CLK_FIX,
+	FSL_COUNTER_CLK_EXT,
+	FSL_COUNTER_CLK_MAX,
+};
+
+struct fsl_pwm_data {
+	unsigned long period_cycles;
+	unsigned long duty_cycles;
+	u32 available;
+};
+
+struct fsl_pwm_chip {
+	struct pwm_chip chip;
+
+	struct clk *sys_clk;
+	struct clk *counter_clk;
+	unsigned int clk_select;
+	unsigned int clk_ps;
+	unsigned int counter_clk_enable;
+
+	void __iomem *base;
+
+	/* pinctrl handle */
+	struct pinctrl *pinctrl;
+};
+
+static inline struct fsl_pwm_chip *to_fsl_chip(struct pwm_chip *chip)
+{
+	return container_of(chip, struct fsl_pwm_chip, chip);
+}
+
+static int fsl_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	int ret;
+	struct fsl_pwm_chip *fpc;
+	struct fsl_pwm_data *pwm_data;
+
+	fpc = to_fsl_chip(chip);
+
+	pwm_data = pwm_get_chip_data(pwm);
+	if (!pwm_data)
+		return -EINVAL;
+
+	if (pwm_data->available != FSL_AVAILABLE)
+		return -EINVAL;
+
+	ret = clk_enable(fpc->sys_clk);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static void fsl_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	struct fsl_pwm_chip *fpc;
+	struct fsl_pwm_data *pwm_data;
+
+	fpc = to_fsl_chip(chip);
+
+	pwm_data = pwm_get_chip_data(pwm);
+	if (!pwm_data)
+		return;
+
+	if (pwm_data->available != FSL_AVAILABLE)
+		return;
+
+	clk_disable(fpc->sys_clk);
+}
+
+static unsigned long fsl_rate_to_cycles(struct fsl_pwm_chip *fpc,
+				       unsigned long time_ns)
+{
+	unsigned long long c;
+	unsigned long ps = 1 << fpc->clk_ps;
+
+	if (fpc->counter_clk)
+		c = clk_get_rate(fpc->counter_clk);
+	else
+		c = clk_get_rate(fpc->sys_clk);
+
+	c = c * time_ns;
+	do_div(c, 1000000000UL);
+	do_div(c, ps);
+
+	return (unsigned long)c;
+}
+
+static int fsl_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
+			  int duty_ns, int period_ns)
+{
+	unsigned long period_cycles, duty_cycles;
+	unsigned long cntin = FTM_CNTIN_VAL;
+	struct fsl_pwm_data *pwm_data;
+	struct fsl_pwm_chip *fpc;
+
+	fpc = to_fsl_chip(chip);
+
+	pwm_data = pwm_get_chip_data(pwm);
+	if (!pwm_data)
+		return -EINVAL;
+
+	if (pwm_data->available != FSL_AVAILABLE)
+		return -EINVAL;
+
+	if (WARN_ON(!test_bit(PWMF_REQUESTED, &pwm->flags)))
+		return -ESHUTDOWN;
+
+	period_cycles = fsl_rate_to_cycles(fpc, period_ns);
+	if (period_cycles > 0xFFFF) {
+		dev_err(chip->dev, "required PWM period cycles(%lu) overflow "
+				"16-bits counter!\n", period_cycles);
+		return -EINVAL;
+	}
+
+	duty_cycles = fsl_rate_to_cycles(fpc, duty_ns);
+	if (duty_cycles >= 0xFFFF) {
+		dev_err(chip->dev, "required PWM duty cycles(%lu) overflow "
+				"16-bits counter!\n", duty_cycles);
+		return -EINVAL;
+	}
+
+	pwm_data->period_cycles = period_cycles;
+	pwm_data->duty_cycles = duty_cycles;
+
+	writel(FTMCnSC_MSB | FTMCnSC_ELSB, fpc->base + FTM_CSC(pwm->hwpwm));
+
+	writel(0xF0, fpc->base + FTM_OUTMASK);
+	writel(0x0F, fpc->base + FTM_OUTINIT);
+	writel(FTM_CNTIN_VAL, fpc->base + FTM_CNTIN);
+
+	writel(period_cycles + cntin - 1, fpc->base + FTM_MOD);
+	writel(duty_cycles + cntin, fpc->base + FTM_CV(pwm->hwpwm));
+
+	return 0;
+}
+
+static int fsl_pwm_set_polarity(struct pwm_chip *chip, struct pwm_device *pwm,
+				enum pwm_polarity polarity)
+{
+	unsigned long reg;
+	struct fsl_pwm_data *pwm_data;
+	struct fsl_pwm_chip *fpc;
+
+	fpc = to_fsl_chip(chip);
+
+	pwm_data = pwm_get_chip_data(pwm);
+	if (!pwm_data)
+		return -EINVAL;
+
+	if (pwm_data->available != FSL_AVAILABLE)
+		return -EINVAL;
+
+	reg = readl(fpc->base + FTM_POL);
+	reg &= ~BIT(pwm->hwpwm);
+	if (polarity == PWM_POLARITY_INVERSED)
+		reg |= BIT(pwm->hwpwm);
+	else
+		reg &= ~BIT(pwm->hwpwm);
+	writel(reg, fpc->base + FTM_POL);
+
+	return 0;
+}
+
+static int fsl_counter_clock_enable(struct fsl_pwm_chip *fpc)
+{
+	int ret;
+	unsigned long reg;
+
+	if (fpc->counter_clk_enable++)
+		return 0;
+
+	ret = clk_enable(fpc->counter_clk);
+	if (ret)
+		return ret;
+
+	reg = readl(fpc->base + FTM_SC);
+	reg &= ~((FTMSC_CLK_MASK << FTMSC_CLK_OFFSET) |
+			(FTMSC_PS_MASK << FTMSC_PS_OFFSET));
+	/* select counter clock source */
+	switch (fpc->clk_select) {
+	case FSL_COUNTER_CLK_SYS:
+		reg |= FTMSC_CLKSYS;
+		break;
+	case FSL_COUNTER_CLK_FIX:
+		reg |= FTMSC_CLKFIX;
+		break;
+	case FSL_COUNTER_CLK_EXT:
+		ret |= FTMSC_CLKEXT;
+		break;
+	default:
+		break;
+	}
+	reg |= fpc->clk_ps;
+	writel(reg, fpc->base + FTM_SC);
+
+	return 0;
+}
+
+static int fsl_counter_clock_disable(struct fsl_pwm_chip *fpc)
+{
+	unsigned long reg;
+
+	if (--fpc->counter_clk_enable)
+		return 0;
+
+	writel(0xFF, fpc->base + FTM_OUTMASK);
+	reg = readl(fpc->base + FTM_SC);
+	reg &= ~(FTMSC_CLK_MASK << FTMSC_CLK_OFFSET);
+	writel(reg, fpc->base + FTM_SC);
+
+	clk_disable(fpc->counter_clk);
+
+	return 0;
+}
+
+static int fsl_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	int ret;
+	struct fsl_pwm_chip *fpc;
+	struct pinctrl_state *pins_state;
+	struct fsl_pwm_data *pwm_data;
+	const char *statename;
+
+	fpc = to_fsl_chip(chip);
+
+	pwm_data = pwm_get_chip_data(pwm);
+	if (!pwm_data)
+		return -EINVAL;
+
+	if (pwm_data->available != FSL_AVAILABLE)
+		return -EINVAL;
+
+	statename = kasprintf(GFP_KERNEL, "ch%d-active", pwm->hwpwm);
+	pins_state = pinctrl_lookup_state(fpc->pinctrl,
+			statename);
+	/* enable pins to be muxed in and configured */
+	if (!IS_ERR(pins_state)) {
+		ret = pinctrl_select_state(fpc->pinctrl, pins_state);
+		if (ret)
+			dev_warn(chip->dev, "could not set default pins\n");
+	} else
+		dev_warn(chip->dev, "could not get default pinstate\n");
+
+	fsl_counter_clock_enable(fpc);
+
+	return 0;
+}
+
+static void fsl_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	int ret;
+	struct fsl_pwm_chip *fpc;
+	struct pinctrl_state    *pins_state;
+	struct fsl_pwm_data *pwm_data;
+	const char *statename;
+
+	fpc = to_fsl_chip(chip);
+
+	pwm_data = pwm_get_chip_data(pwm);
+	if (!pwm_data)
+		return;
+
+	if (pwm_data->available != FSL_AVAILABLE)
+		return;
+
+	statename = kasprintf(GFP_KERNEL, "ch%d-idle", pwm->hwpwm);
+	pins_state = pinctrl_lookup_state(fpc->pinctrl,
+			statename);
+	/* enable pins to be muxed in and configured */
+	if (!IS_ERR(pins_state)) {
+		ret = pinctrl_select_state(fpc->pinctrl, pins_state);
+		if (ret)
+			dev_warn(chip->dev, "could not set default pins\n");
+	} else
+		dev_warn(chip->dev, "could not get default pinstate\n");
+
+	fsl_counter_clock_disable(fpc);
+}
+
+static const struct pwm_ops fsl_pwm_ops = {
+	.request = fsl_pwm_request,
+	.free = fsl_pwm_free,
+	.config = fsl_pwm_config,
+	.set_polarity = fsl_pwm_set_polarity,
+	.enable = fsl_pwm_enable,
+	.disable = fsl_pwm_disable,
+	.owner = THIS_MODULE,
+};
+
+static int fsl_pwm_calculate_ps(struct fsl_pwm_chip *fpc)
+{
+	unsigned long long sys_rate, counter_rate, rate;
+
+	sys_rate = clk_get_rate(fpc->sys_clk);
+	if (!sys_rate)
+		return -EINVAL;
+
+	counter_rate = clk_get_rate(fpc->counter_clk);
+
+	switch (fpc->clk_select) {
+	case FSL_COUNTER_CLK_SYS:
+		counter_rate = 0;
+		break;
+	case FSL_COUNTER_CLK_FIX:
+		if (counter_rate) {
+			rate = 2 * counter_rate - 1;
+			do_div(rate, sys_rate);
+			fpc->clk_ps = rate;
+		}
+		break;
+	case FSL_COUNTER_CLK_EXT:
+		if (counter_rate) {
+			rate = 4 * counter_rate - 1;
+			do_div(rate, sys_rate);
+			fpc->clk_ps = rate;
+		}
+		break;
+	default:
+		break;
+	}
+
+	if (!counter_rate && fpc->clk_select != FSL_COUNTER_CLK_SYS) {
+		dev_warn(fpc->chip.dev,
+				"the counter source clock is a dummy clock, "
+				"so select the system clock as default!\n");
+	}
+
+	if (!counter_rate) {
+		fpc->counter_clk = NULL;
+		fpc->clk_select = FSL_COUNTER_CLK_SYS;
+		fpc->clk_ps = 7;
+	}
+
+	return 0;
+}
+
+static int fsl_pwm_parse_clk_ps(struct fsl_pwm_chip *fpc)
+{
+	const char *cname;
+	int ret, index;
+	struct device_node *np = fpc->chip.dev->of_node;
+
+	ret = of_property_read_string_index(np, "fsl,pwm-counter-clk", 0,
+					    &cname);
+	if (ret < 0) {
+		dev_err(fpc->chip.dev,
+				"failed to get \"fsl,pwm-counter-clk\": %d\n",
+				ret);
+		return ret;
+	}
+
+	index = of_property_match_string(np, "clock-names", cname);
+	if (index < 0)
+		return index;
+	if (index >= FSL_COUNTER_CLK_MAX)
+		return -EINVAL;
+	fpc->clk_select = index;
+
+	fpc->sys_clk = devm_clk_get(fpc->chip.dev, "ftm0");
+	if (IS_ERR(fpc->sys_clk)) {
+		ret = PTR_ERR(fpc->sys_clk);
+		dev_err(fpc->chip.dev,
+				"failed to get \"ftm0\" clock %d\n", ret);
+		return ret;
+	}
+
+	switch (fpc->clk_select) {
+	case FSL_COUNTER_CLK_SYS:
+		fpc->counter_clk = NULL;
+		break;
+	case FSL_COUNTER_CLK_FIX:
+		fpc->counter_clk = devm_clk_get(fpc->chip.dev, "ftm0_fix_sel");
+		if (IS_ERR(fpc->counter_clk)) {
+			ret = PTR_ERR(fpc->counter_clk);
+			dev_err(fpc->chip.dev,
+					"failed to get \"ftm0_fix_sel\" "
+					"clock %d\n", ret);
+			return ret;
+		}
+		break;
+	case FSL_COUNTER_CLK_EXT:
+		fpc->counter_clk = devm_clk_get(fpc->chip.dev, "ftm0_ext_sel");
+		if (IS_ERR(fpc->counter_clk)) {
+			ret = PTR_ERR(fpc->counter_clk);
+			dev_err(fpc->chip.dev,
+					"failed to get \"ftm0_ext_sel\" "
+					"clock %d\n", ret);
+			return ret;
+		}
+		break;
+	default:
+		break;
+	}
+
+	return fsl_pwm_calculate_ps(fpc);
+}
+
+static inline int fsl_set_pwm_state(struct fsl_pwm_data *pwm_data,
+				    unsigned int *chs, int avl, int num)
+{
+	int i;
+
+	if (!pwm_data || !chs)
+		return -EINVAL;
+
+	for (i = 0; i < avl; i++)
+		if (num == chs[i])
+			pwm_data->available = FSL_AVAILABLE;
+
+	return 0;
+}
+
+static int fsl_pwm_set_chip_data(struct fsl_pwm_chip *fpc)
+{
+	int ret, avl, i;
+	unsigned int chs[FTM_MAX_CHANNEL];
+	struct fsl_pwm_data *pwm_data;
+	struct property *prop;
+	struct device_node *np = fpc->chip.dev->of_node;
+
+	prop = of_find_property(np, "fsl,pwm-avaliable-chs", &avl);
+	if (!prop) {
+		dev_err(fpc->chip.dev,
+				"failed to get \"fsl,pwm-avaliable-chs\" property.");
+		return -EINVAL;
+	}
+	avl /= sizeof(u32);
+	ret = of_property_read_u32_array(np, "fsl,pwm-avaliable-chs",
+					 chs, avl);
+	if (ret < 0) {
+		dev_err(fpc->chip.dev,
+				"failed to get the value of"
+				"\"fsl,pwm-avaliable-chs\": %d\n", ret);
+		return ret;
+	}
+
+	for (i = 0; i < FTM_MAX_CHANNEL; i++) {
+		pwm_data = devm_kzalloc(fpc->chip.dev,
+					sizeof(*pwm_data),
+					GFP_KERNEL);
+
+		ret = fsl_set_pwm_state(pwm_data, chs, avl, i);
+		if (ret < 0) {
+			dev_err(fpc->chip.dev,
+					"failed to set PWM channel state: %d\n",
+					ret);
+			return ret;
+		}
+
+		ret = pwm_set_chip_data(&fpc->chip.pwms[i], pwm_data);
+		if (ret < 0) {
+			dev_err(fpc->chip.dev,
+					"failed to set PWM chip data: %d\n",
+					ret);
+			return ret;
+		}
+
+	}
+
+	return 0;
+}
+
+static int fsl_pwm_probe(struct platform_device *pdev)
+{
+	int ret = 0;
+	struct fsl_pwm_chip *fpc;
+	struct resource *res;
+
+	fpc = devm_kzalloc(&pdev->dev, sizeof(*fpc), GFP_KERNEL);
+
+	fpc->chip.dev = &pdev->dev;
+	fpc->counter_clk_enable = 0;
+
+	ret = fsl_pwm_parse_clk_ps(fpc);
+	if (ret < 0)
+		return ret;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	fpc->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(fpc->base)) {
+		ret = PTR_ERR(fpc->base);
+		return ret;
+	}
+
+	ret = clk_prepare(fpc->sys_clk);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare(fpc->counter_clk);
+	if (ret)
+		return ret;
+
+	fpc->pinctrl = devm_pinctrl_get(&pdev->dev);
+	if (IS_ERR(fpc->pinctrl)) {
+		ret = PTR_ERR(fpc->pinctrl);
+		return ret;
+	}
+
+	fpc->chip.ops = &fsl_pwm_ops;
+	fpc->chip.of_xlate = of_pwm_xlate_with_flags;
+	fpc->chip.of_pwm_n_cells = 3;
+	fpc->chip.base = -1;
+	fpc->chip.npwm = FTM_MAX_CHANNEL;
+	ret = pwmchip_add(&fpc->chip);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to add PWM chip %d\n", ret);
+		return ret;
+	}
+
+	ret = fsl_pwm_set_chip_data(fpc);
+	if (ret < 0)
+		return ret;
+
+	platform_set_drvdata(pdev, fpc);
+
+	return 0;
+}
+
+static int fsl_pwm_remove(struct platform_device *pdev)
+{
+	struct fsl_pwm_chip *fpc;
+
+	fpc = platform_get_drvdata(pdev);
+
+	clk_unprepare(fpc->sys_clk);
+	clk_unprepare(fpc->counter_clk);
+
+	return pwmchip_remove(&fpc->chip);
+}
+
+static const struct of_device_id fsl_pwm_dt_ids[] = {
+	{ .compatible = "fsl,vf610-ftm-pwm", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, fsl_pwm_dt_ids);
+
+static struct platform_driver fsl_pwm_driver = {
+	.driver = {
+		.name = "fsl-ftm-pwm",
+		.owner = THIS_MODULE,
+		.of_match_table = of_match_ptr(fsl_pwm_dt_ids),
+	},
+	.probe = fsl_pwm_probe,
+	.remove = fsl_pwm_remove,
+};
+module_platform_driver(fsl_pwm_driver);
+
+MODULE_DESCRIPTION("Freescale FTM PWM Driver");
+MODULE_AUTHOR("Xiubo Li <Li.Xiubo@freescale.com>");
+MODULE_LICENSE("GPL");
-- 
1.8.0

Re: [PATCHv2 1/4] pwm: Add Freescale FTM PWM driver support

[Sascha Hauer]

On Fri, Aug 30, 2013 at 05:48:49PM +0800, Xiubo Li wrote:
> +
> +#define FTM_CSC_BASE        0x0C
> +#define FTM_CSC(_channel) \
> +	(FTM_CSC_BASE + ((_channel) * 8))

Put into a single line.

> +#define FTMCnSC_MSB         BIT(5)
> +#define FTMCnSC_MSA         BIT(4)
> +#define FTMCnSC_ELSB        BIT(3)
> +#define FTMCnSC_ELSA        BIT(2)
> +
> +#define FTM_CV_BASE         0x10
> +#define FTM_CV(_channel) \
> +	(FTM_CV_BASE + ((_channel) * 8))

ditto.

> +static int fsl_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
> +{
> +	int ret;
> +	struct fsl_pwm_chip *fpc;
> +	struct fsl_pwm_data *pwm_data;
> +
> +	fpc = to_fsl_chip(chip);
> +
> +	pwm_data = pwm_get_chip_data(pwm);
> +	if (!pwm_data)
> +		return -EINVAL;
> +
> +	if (pwm_data->available != FSL_AVAILABLE)
> +		return -EINVAL;
> +
> +	ret = clk_enable(fpc->sys_clk);
> +	if (ret)
> +		return ret;

This is non atomic context. You can also prepare the clocks here instead
of doing it in probe().

> +
> +	return 0;
> +}
> +
> +static void fsl_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
> +{
> +	struct fsl_pwm_chip *fpc;
> +	struct fsl_pwm_data *pwm_data;
> +
> +	fpc = to_fsl_chip(chip);
> +
> +	pwm_data = pwm_get_chip_data(pwm);
> +	if (!pwm_data)
> +		return;

THis check seems unnecessary.

> +
> +	if (pwm_data->available != FSL_AVAILABLE)
> +		return;
> +
> +	clk_disable(fpc->sys_clk);
> +}
> +

[...]

> +
> +
> +	pwm_data->period_cycles = period_cycles;
> +	pwm_data->duty_cycles = duty_cycles;

These fields are set but never read. Please drop them.

If you drop the 'available' field also the you can drop chip_data
completely.

> +
> +	writel(FTMCnSC_MSB | FTMCnSC_ELSB, fpc->base + FTM_CSC(pwm->hwpwm));
> +
> +	writel(0xF0, fpc->base + FTM_OUTMASK);
> +	writel(0x0F, fpc->base + FTM_OUTINIT);
> +	writel(FTM_CNTIN_VAL, fpc->base + FTM_CNTIN);
> +
> +	writel(period_cycles + cntin - 1, fpc->base + FTM_MOD);
> +	writel(duty_cycles + cntin, fpc->base + FTM_CV(pwm->hwpwm));
> +
> +	return 0;
> +}
> +
> +static int fsl_pwm_set_polarity(struct pwm_chip *chip, struct pwm_device *pwm,
> +				enum pwm_polarity polarity)
> +{
> +	unsigned long reg;
> +	struct fsl_pwm_data *pwm_data;
> +	struct fsl_pwm_chip *fpc;
> +
> +	fpc = to_fsl_chip(chip);
> +
> +	pwm_data = pwm_get_chip_data(pwm);
> +	if (!pwm_data)
> +		return -EINVAL;
> +
> +	if (pwm_data->available != FSL_AVAILABLE)
> +		return -EINVAL;
> +
> +	reg = readl(fpc->base + FTM_POL);
> +	reg &= ~BIT(pwm->hwpwm);

Either drop this line...

> +	if (polarity == PWM_POLARITY_INVERSED)
> +		reg |= BIT(pwm->hwpwm);
> +	else
> +		reg &= ~BIT(pwm->hwpwm);

...or this one

> +static int fsl_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
> +{
> +	int ret;
> +	struct fsl_pwm_chip *fpc;
> +	struct pinctrl_state *pins_state;
> +	struct fsl_pwm_data *pwm_data;
> +	const char *statename;
> +
> +	fpc = to_fsl_chip(chip);
> +
> +	pwm_data = pwm_get_chip_data(pwm);
> +	if (!pwm_data)
> +		return -EINVAL;
> +
> +	if (pwm_data->available != FSL_AVAILABLE)
> +		return -EINVAL;
> +
> +	statename = kasprintf(GFP_KERNEL, "ch%d-active", pwm->hwpwm);

You loose memory here and in fsl_pwm_disable aswell.

> +	pins_state = pinctrl_lookup_state(fpc->pinctrl,
> +			statename);
> +	/* enable pins to be muxed in and configured */
> +	if (!IS_ERR(pins_state)) {
> +		ret = pinctrl_select_state(fpc->pinctrl, pins_state);
> +		if (ret)
> +			dev_warn(chip->dev, "could not set default pins\n");
> +	} else
> +		dev_warn(chip->dev, "could not get default pinstate\n");

Either it's ok to do without pinctrl or it's not ok, so either return
an error or drop the warnings. Polluting the kernel log with such
messages from a frequently called function is not a good idea.

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 |

RE: [PATCHv2 1/4] pwm: Add Freescale FTM PWM driver support

[Xiubo Li-B47053]

> > +static void fsl_pwm_free(struct pwm_chip *chip, struct pwm_device
> > +*pwm) {
> > +	struct fsl_pwm_chip *fpc;
> > +	struct fsl_pwm_data *pwm_data;
> > +
> > +	fpc = to_fsl_chip(chip);
> > +
> > +	pwm_data = pwm_get_chip_data(pwm);
> > +	if (!pwm_data)
> > +		return;
> 
> THis check seems unnecessary.
> 

But if do not check it here, I must check it in the following code.

> > +
> > +	if (pwm_data->available != FSL_AVAILABLE)
> > +		return;
> > +

So the ' struct fsl_pwm_data' may be removed in the future.

> 
> > +
> > +
> > +	pwm_data->period_cycles = period_cycles;
> > +	pwm_data->duty_cycles = duty_cycles;
> 
> These fields are set but never read. Please drop them.
> 
> If you drop the 'available' field also the you can drop chip_data
> completely.
> 

I think I may move the 'available' field to the PWM driver data struct.

> > +
> > +	writel(FTMCnSC_MSB | FTMCnSC_ELSB, fpc->base + FTM_CSC(pwm->hwpwm));
> > +
> > +	writel(0xF0, fpc->base + FTM_OUTMASK);
> > +	writel(0x0F, fpc->base + FTM_OUTINIT);
> > +	writel(FTM_CNTIN_VAL, fpc->base + FTM_CNTIN);
> > +
> > +	writel(period_cycles + cntin - 1, fpc->base + FTM_MOD);
> > +	writel(duty_cycles + cntin, fpc->base + FTM_CV(pwm->hwpwm));
> > +
> > +	return 0;
> > +}
> > +
> > +static int fsl_pwm_set_polarity(struct pwm_chip *chip, struct
> pwm_device *pwm,
> > +				enum pwm_polarity polarity)
> > +{
> > +	unsigned long reg;
> > +	struct fsl_pwm_data *pwm_data;
> > +	struct fsl_pwm_chip *fpc;
> > +
> > +	fpc = to_fsl_chip(chip);
> > +
> > +	pwm_data = pwm_get_chip_data(pwm);
> > +	if (!pwm_data)
> > +		return -EINVAL;
> > +
> > +	if (pwm_data->available != FSL_AVAILABLE)
> > +		return -EINVAL;
> > +
> > +	reg = readl(fpc->base + FTM_POL);
> > +	reg &= ~BIT(pwm->hwpwm);
> 
> Either drop this line...
> 

This is just for unmasking this bit field.
Here it's not needed, so I will revise it.

> > +	if (polarity == PWM_POLARITY_INVERSED)
> > +		reg |= BIT(pwm->hwpwm);
> > +	else
> > +		reg &= ~BIT(pwm->hwpwm);
> 
> ...or this one
> 

> > +static int fsl_pwm_enable(struct pwm_chip *chip, struct pwm_device
> > +*pwm) {
> > +	int ret;
> > +	struct fsl_pwm_chip *fpc;
> > +	struct pinctrl_state *pins_state;
> > +	struct fsl_pwm_data *pwm_data;
> > +	const char *statename;
> > +
> > +	fpc = to_fsl_chip(chip);
> > +
> > +	pwm_data = pwm_get_chip_data(pwm);
> > +	if (!pwm_data)
> > +		return -EINVAL;
> > +
> > +	if (pwm_data->available != FSL_AVAILABLE)
> > +		return -EINVAL;
> > +
> > +	statename = kasprintf(GFP_KERNEL, "ch%d-active", pwm->hwpwm);
> 
> You loose memory here and in fsl_pwm_disable aswell.
> 

Yes, I will revise it.

> > +	pins_state = pinctrl_lookup_state(fpc->pinctrl,
> > +			statename);
> > +	/* enable pins to be muxed in and configured */
> > +	if (!IS_ERR(pins_state)) {
> > +		ret = pinctrl_select_state(fpc->pinctrl, pins_state);
> > +		if (ret)
> > +			dev_warn(chip->dev, "could not set default pins\n");
> > +	} else
> > +		dev_warn(chip->dev, "could not get default pinstate\n");
> 
> Either it's ok to do without pinctrl or it's not ok, so either return an
> error or drop the warnings. Polluting the kernel log with such messages
> from a frequently called function is not a good idea.
> 

Well, I will just print out some error logs and return the error.

--
Best Regards.
Xiubo

Re: [PATCHv2 1/4] pwm: Add Freescale FTM PWM driver support

[Sascha Hauer]

On Mon, Sep 02, 2013 at 03:33:37AM +0000, Xiubo Li-B47053 wrote:
> 
> > > +static void fsl_pwm_free(struct pwm_chip *chip, struct pwm_device
> > > +*pwm) {
> > > +	struct fsl_pwm_chip *fpc;
> > > +	struct fsl_pwm_data *pwm_data;
> > > +
> > > +	fpc = to_fsl_chip(chip);
> > > +
> > > +	pwm_data = pwm_get_chip_data(pwm);
> > > +	if (!pwm_data)
> > > +		return;
> > 
> > THis check seems unnecessary.
> > 
> 
> But if do not check it here, I must check it in the following code.
> 
> > > +
> > > +	if (pwm_data->available != FSL_AVAILABLE)
> > > +		return;
> > > +
> 
> So the ' struct fsl_pwm_data' may be removed in the future.
> 
> > 
> > > +
> > > +
> > > +	pwm_data->period_cycles = period_cycles;
> > > +	pwm_data->duty_cycles = duty_cycles;
> > 
> > These fields are set but never read. Please drop them.
> > 
> > If you drop the 'available' field also the you can drop chip_data
> > completely.
> > 
> 
> I think I may move the 'available' field to the PWM driver data struct.

You simply don't need the available field. You don't need to track
whether they are available. If a user enables a pwm which is not routed
out of the SoC (disabled in the iomux) simply nothing will happen except
for a slightly increased power consumption.

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 |

RE: [PATCHv2 1/4] pwm: Add Freescale FTM PWM driver support

[Xiubo Li-B47053]

> Subject: Re: [PATCHv2 1/4] pwm: Add Freescale FTM PWM driver support
> 
> On Mon, Sep 02, 2013 at 03:33:37AM +0000, Xiubo Li-B47053 wrote:
> >
> > > > +static void fsl_pwm_free(struct pwm_chip *chip, struct pwm_device
> > > > +*pwm) {
> > > > +	struct fsl_pwm_chip *fpc;
> > > > +	struct fsl_pwm_data *pwm_data;
> > > > +
> > > > +	fpc = to_fsl_chip(chip);
> > > > +
> > > > +	pwm_data = pwm_get_chip_data(pwm);
> > > > +	if (!pwm_data)
> > > > +		return;
> > >
> > > THis check seems unnecessary.
> > >
> >
> > But if do not check it here, I must check it in the following code.
> >
> > > > +
> > > > +	if (pwm_data->available != FSL_AVAILABLE)
> > > > +		return;
> > > > +
> >
> > So the ' struct fsl_pwm_data' may be removed in the future.
> >
> > >
> > > > +
> > > > +
> > > > +	pwm_data->period_cycles = period_cycles;
> > > > +	pwm_data->duty_cycles = duty_cycles;
> > >
> > > These fields are set but never read. Please drop them.
> > >
> > > If you drop the 'available' field also the you can drop chip_data
> > > completely.
> > >
> >
> > I think I may move the 'available' field to the PWM driver data struct.
> 
> You simply don't need the available field. You don't need to track
> whether they are available. If a user enables a pwm which is not routed
> out of the SoC (disabled in the iomux) simply nothing will happen except
> for a slightly increased power consumption.
> 
If the there is not need to explicitly specify the channels are available or not, so there is no doubt that the 'available' field will be dropt.
Why I added this here is because that the 4th and 5th channels' pinctrls are used as UART TX and RX as I have mentioned before, so here if you configure these two pinctrls, the UART TX and RX will be polluted, there maybe some other cases like this.
So, if there is no need to worry about this in PWM driver, the customer should be aware of it and be responsible for the potential risk.
I will think it over and optimize it then.



Thanks very much.
--
Best Regards.
Xiubo

Re: [PATCHv2 1/4] pwm: Add Freescale FTM PWM driver support

[Sascha Hauer]

On Tue, Sep 03, 2013 at 04:17:09AM +0000, Xiubo Li-B47053 wrote:
> > Subject: Re: [PATCHv2 1/4] pwm: Add Freescale FTM PWM driver support
> > 
> > You simply don't need the available field. You don't need to track
> > whether they are available. If a user enables a pwm which is not routed
> > out of the SoC (disabled in the iomux) simply nothing will happen except
> > for a slightly increased power consumption.
> > 
> If the there is not need to explicitly specify the channels are
> available or not, so there is no doubt that the 'available' field will
> be dropt.  Why I added this here is because that the 4th and 5th
> channels' pinctrls are used as UART TX and RX as I have mentioned
> before, so here if you configure these two pinctrls, the UART TX and
> RX will be polluted, there maybe some other cases like this.

If you misconfigure your iomux then usually unexptected things happen.
That is not the problem of the PWM driver, but the problem of the one
writing the devicetree. The kernel will print a message for conflicting
iomux settings. That should be hint enough to fix it.

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 |

RE: [PATCHv2 1/4] pwm: Add Freescale FTM PWM driver support

[Xiubo Li-B47053]

> Subject: Re: [PATCHv2 1/4] pwm: Add Freescale FTM PWM driver support
> 
> On Tue, Sep 03, 2013 at 04:17:09AM +0000, Xiubo Li-B47053 wrote:
> > > Subject: Re: [PATCHv2 1/4] pwm: Add Freescale FTM PWM driver support
> > >
> > > You simply don't need the available field. You don't need to track
> > > whether they are available. If a user enables a pwm which is not
> > > routed out of the SoC (disabled in the iomux) simply nothing will
> > > happen except for a slightly increased power consumption.
> > >
> > If the there is not need to explicitly specify the channels are
> > available or not, so there is no doubt that the 'available' field will
> > be dropt.  Why I added this here is because that the 4th and 5th
> > channels' pinctrls are used as UART TX and RX as I have mentioned
> > before, so here if you configure these two pinctrls, the UART TX and
> > RX will be polluted, there maybe some other cases like this.
> 
> If you misconfigure your iomux then usually unexptected things happen.
> That is not the problem of the PWM driver, but the problem of the one
> writing the devicetree. The kernel will print a message for conflicting
> iomux settings. That should be hint enough to fix it.
> 

That sounds good.
Actully there isn't any conflicting messages will be printed.

I will think it over.


--
Best Regards,
Xiubo 

[PATCHv2 2/4] ARM: dts: Add Freescale FTM PWM node for VF610.

[Xiubo Li]

This adds devicetree for VF610, with necessary support for leds,
backlight motor, etc.
And there are 8 channels supported default.

Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
---
 arch/arm/boot/dts/vf610.dtsi | 103 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 102 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/vf610.dtsi b/arch/arm/boot/dts/vf610.dtsi
index 67d929c..44787b5 100644
--- a/arch/arm/boot/dts/vf610.dtsi
+++ b/arch/arm/boot/dts/vf610.dtsi
@@ -140,6 +140,37 @@
 				clock-names = "pit";
 			};
 
+			pwm0: pwm@40038000 {
+				compatible = "fsl,vf610-ftm-pwm";
+				#pwm-cells = <3>;
+				reg = <0x40038000 0x1000>;
+				clock-names = "ftm0", "ftm0_fix_sel", "ftm0_ext_sel";
+				clocks = <&clks VF610_CLK_FTM0>,
+					<&clks VF610_CLK_FTM0_FIX_SEL>,
+					<&clks VF610_CLK_FTM0_EXT_SEL>;
+				pinctrl-names = "ch0-active", "ch0-idle", "ch1-active", "ch1-idle",
+						"ch2-active", "ch2-idle", "ch3-active", "ch3-idle",
+						"ch4-active", "ch4-idle", "ch5-active", "ch5-idle",
+						"ch6-active", "ch6-idle", "ch7-active", "ch7-idle";
+				pinctrl-0 = <&pinctrl_pwm0_ch0_active>;
+				pinctrl-1 = <&pinctrl_pwm0_ch0_idle>;
+				pinctrl-2 = <&pinctrl_pwm0_ch1_active>;
+				pinctrl-3 = <&pinctrl_pwm0_ch1_idle>;
+				pinctrl-4 = <&pinctrl_pwm0_ch2_active>;
+				pinctrl-5 = <&pinctrl_pwm0_ch2_idle>;
+				pinctrl-6 = <&pinctrl_pwm0_ch3_active>;
+				pinctrl-7 = <&pinctrl_pwm0_ch3_idle>;
+				pinctrl-8 = <&pinctrl_pwm0_ch4_active>;
+				pinctrl-9 = <&pinctrl_pwm0_ch4_idle>;
+				pinctrl-10 = <&pinctrl_pwm0_ch5_active>;
+				pinctrl-11 = <&pinctrl_pwm0_ch5_idle>;
+				pinctrl-12 = <&pinctrl_pwm0_ch6_active>;
+				pinctrl-13 = <&pinctrl_pwm0_ch6_idle>;
+				pinctrl-14 = <&pinctrl_pwm0_ch7_active>;
+				pinctrl-15 = <&pinctrl_pwm0_ch7_idle>;
+				status = "disabled";
+			};
+
 			wdog@4003e000 {
 				compatible = "fsl,vf610-wdt", "fsl,imx21-wdt";
 				reg = <0x4003e000 0x1000>;
@@ -270,16 +301,86 @@
 				};
 
 				pwm0 {
-					pinctrl_pwm0_1: pwm0grp_1 {
+					pinctrl_pwm0_ch0_active: pwm0grp_ch0_active {
 						fsl,pins = <
 						VF610_PAD_PTB0__FTM0_CH0	0x1582
+						>;
+					};
+					pinctrl_pwm0_ch0_idle: pwm0grp_ch0_idle {
+						fsl,pins = <
+						VF610_PAD_PTB0__FTM0_CH0	0x0000
+						>;
+					};
+					pinctrl_pwm0_ch1_active: pwm0grp_ch1_active {
+						fsl,pins = <
 						VF610_PAD_PTB1__FTM0_CH1	0x1582
+						>;
+					};
+					pinctrl_pwm0_ch1_idle: pwm0grp_ch1_idle {
+						fsl,pins = <
+						VF610_PAD_PTB1__FTM0_CH1	0x0000
+						>;
+					};
+					pinctrl_pwm0_ch2_active: pwm0grp_ch2_active {
+						fsl,pins = <
 						VF610_PAD_PTB2__FTM0_CH2	0x1582
+						>;
+					};
+					pinctrl_pwm0_ch2_idle: pwm0grp_ch2_idle {
+						fsl,pins = <
+						VF610_PAD_PTB2__FTM0_CH2	0x0000
+						>;
+					};
+					pinctrl_pwm0_ch3_active: pwm0grp_ch3_active {
+						fsl,pins = <
 						VF610_PAD_PTB3__FTM0_CH3	0x1582
+						>;
+					};
+					pinctrl_pwm0_ch3_idle: pwm0grp_ch3_idle {
+						fsl,pins = <
+						VF610_PAD_PTB3__FTM0_CH3	0x0000
+						>;
+					};
+					pinctrl_pwm0_ch4_active: pwm0grp_ch4_active {
+						fsl,pins = <
+						VF610_PAD_PTB4__FTM0_CH4	0x1582
+						>;
+					};
+					pinctrl_pwm0_ch4_idle: pwm0grp_ch4_idle {
+						fsl,pins = <
+						VF610_PAD_PTB4__FTM0_CH4	0x0000
+						>;
+					};
+					pinctrl_pwm0_ch5_active: pwm0grp_ch5_active {
+						fsl,pins = <
+						VF610_PAD_PTB5__FTM0_CH5	0x1582
+						>;
+					};
+					pinctrl_pwm0_ch5_idle: pwm0grp_ch5_idle {
+						fsl,pins = <
+						VF610_PAD_PTB5__FTM0_CH5	0x0000
+						>;
+					};
+					pinctrl_pwm0_ch6_active: pwm0grp_ch6_active {
+						fsl,pins = <
 						VF610_PAD_PTB6__FTM0_CH6	0x1582
+						>;
+					};
+					pinctrl_pwm0_ch6_idle: pwm0grp_ch6_idle {
+						fsl,pins = <
+						VF610_PAD_PTB6__FTM0_CH6	0x0000
+						>;
+					};
+					pinctrl_pwm0_ch7_active: pwm0grp_ch7_active {
+						fsl,pins = <
 						VF610_PAD_PTB7__FTM0_CH7	0x1582
 						>;
 					};
+					pinctrl_pwm0_ch7_idle: pwm0grp_ch7_idle {
+						fsl,pins = <
+						VF610_PAD_PTB7__FTM0_CH7	0x0000
+						>;
+					};
 				};
 
 				qspi0 {
-- 
1.8.0

[PATCHv2 3/4] ARM: dts: Enables FTM PWM device for Vybrid VF610 TOWER board.

[Xiubo Li]

This adds devicetree for VF610 TOWER board, and there are 6 PWM
channels avaliable.

Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
---
 arch/arm/boot/dts/vf610-twr.dts | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/vf610-twr.dts b/arch/arm/boot/dts/vf610-twr.dts
index 1a58678..82b0a91 100644
--- a/arch/arm/boot/dts/vf610-twr.dts
+++ b/arch/arm/boot/dts/vf610-twr.dts
@@ -57,6 +57,12 @@
 	status = "okay";
 };
 
+&pwm0 {
+	fsl,pwm-counter-clk = "ftm0";
+	fsl,pwm-avaliable-chs = <0 1 2 3 6 7>;
+	status = "okay";
+};
+
 &uart1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_uart1_1>;
-- 
1.8.0

[PATCHv2 4/4] Documentation: Add device tree bindings for Freescale FTM PWM.

[Xiubo Li]

This patch adds the document for Freescale FTM PWM driver under
Documentation/devicetree/bindings/pwm/.

Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
---
 .../devicetree/bindings/pwm/pwm-fsl-ftm.txt        | 40 ++++++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pwm/pwm-fsl-ftm.txt

diff --git a/Documentation/devicetree/bindings/pwm/pwm-fsl-ftm.txt b/Documentation/devicetree/bindings/pwm/pwm-fsl-ftm.txt
new file mode 100644
index 0000000..b2b5214
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/pwm-fsl-ftm.txt
@@ -0,0 +1,40 @@
+Freescale FTM PWM controller
+
+Required properties:
+- compatible: Should be "fsl,vf610-ftm-pwm"
+- reg: Physical base address and length of the controller's registers
+- #pwm-cells: Should be 3. See pwm.txt in this directory for a description of
+  the cells format.
+- clock-names : Includes the following module clock source entries:
+    "ftm0" (system clock),
+    "ftm0_fix_sel" (fixed frequency clock),
+    "ftm0_ext_sel" (external clock)
+- clocks : Must contain an entry list for entries in clock-names.
+- fsl,pwm-counter-clk: The FTM PWM counter clock source, should be one of the
+  entries in clock-names.
+- fsl,pwm-avaliable-chs: The FTM channels ID list of current board which are
+  available as PWM function.
+- For each channel's pinctrl, the "chN-active" and "chN-idle" states should be
+  implemented at same time.
+
+Example:
+
+pwm0: pwm@40038000 {
+		compatible = "fsl,vf610-ftm-pwm";
+		reg = <0x40038000 0x1000>;
+		#pwm-cells = <3>;
+		clock-names = "ftm0", "ftm0_fix_sel", "ftm0_ext_sel";
+		clocks = <&clks VF610_CLK_FTM0>,
+			<&clks VF610_CLK_FTM0_FIX_SEL>,
+			<&clks VF610_CLK_FTM0_EXT_SEL>;
+		pinctrl-names = "ch0-active", "ch0-idle", "ch1-active", "ch1-idle",
+		....;
+		pinctrl-0 = <&pinctrl_pwm0_ch0_active>;
+		pinctrl-1 = <&pinctrl_pwm0_ch0_idle>;
+		pinctrl-2 = <&pinctrl_pwm0_ch1_active>;
+		pinctrl-3 = <&pinctrl_pwm0_ch1_idle>;
+		...
+		fsl,pwm-counter-clk = "ftm0_ext_sel";
+		fsl,pwm-avaliable-chs = <0 3 5 6>;
+		...
+};
-- 
1.8.0

Re: [PATCHv2 4/4] Documentation: Add device tree bindings for Freescale FTM PWM.

[Sascha Hauer]

On Fri, Aug 30, 2013 at 05:48:52PM +0800, Xiubo Li wrote:
> This patch adds the document for Freescale FTM PWM driver under
> Documentation/devicetree/bindings/pwm/.
> 
> Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
> ---
>  .../devicetree/bindings/pwm/pwm-fsl-ftm.txt        | 40 ++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pwm/pwm-fsl-ftm.txt
> 
> diff --git a/Documentation/devicetree/bindings/pwm/pwm-fsl-ftm.txt b/Documentation/devicetree/bindings/pwm/pwm-fsl-ftm.txt
> new file mode 100644
> index 0000000..b2b5214
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pwm/pwm-fsl-ftm.txt
> @@ -0,0 +1,40 @@
> +Freescale FTM PWM controller
> +
> +Required properties:
> +- compatible: Should be "fsl,vf610-ftm-pwm"
> +- reg: Physical base address and length of the controller's registers
> +- #pwm-cells: Should be 3. See pwm.txt in this directory for a description of
> +  the cells format.
> +- clock-names : Includes the following module clock source entries:
> +    "ftm0" (system clock),
> +    "ftm0_fix_sel" (fixed frequency clock),
> +    "ftm0_ext_sel" (external clock)
> +- clocks : Must contain an entry list for entries in clock-names.
> +- fsl,pwm-counter-clk: The FTM PWM counter clock source, should be one of the
> +  entries in clock-names.
> +- fsl,pwm-avaliable-chs: The FTM channels ID list of current board which are
> +  available as PWM function.
> +- For each channel's pinctrl, the "chN-active" and "chN-idle" states should be
> +  implemented at same time.
> +
> +Example:
> +
> +pwm0: pwm@40038000 {
> +		compatible = "fsl,vf610-ftm-pwm";
> +		reg = <0x40038000 0x1000>;
> +		#pwm-cells = <3>;
> +		clock-names = "ftm0", "ftm0_fix_sel", "ftm0_ext_sel";
> +		clocks = <&clks VF610_CLK_FTM0>,
> +			<&clks VF610_CLK_FTM0_FIX_SEL>,
> +			<&clks VF610_CLK_FTM0_EXT_SEL>;
> +		pinctrl-names = "ch0-active", "ch0-idle", "ch1-active", "ch1-idle",
> +		....;
> +		pinctrl-0 = <&pinctrl_pwm0_ch0_active>;
> +		pinctrl-1 = <&pinctrl_pwm0_ch0_idle>;
> +		pinctrl-2 = <&pinctrl_pwm0_ch1_active>;
> +		pinctrl-3 = <&pinctrl_pwm0_ch1_idle>;
> +		...
> +		fsl,pwm-counter-clk = "ftm0_ext_sel";
> +		fsl,pwm-avaliable-chs = <0 3 5 6>;

I don't think this proerty is useful. Just enable all channels. I think
this was mentioned before.

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 |

RE: [PATCHv2 4/4] Documentation: Add device tree bindings for Freescale FTM PWM.

[Xiubo Li-B47053]

> Subject: Re: [PATCHv2 4/4] Documentation: Add device tree bindings for
> Freescale FTM PWM.
> 
...
> > +
> > +pwm0: pwm@40038000 {
> > +		compatible = "fsl,vf610-ftm-pwm";
> > +		reg = <0x40038000 0x1000>;
> > +		#pwm-cells = <3>;
> > +		clock-names = "ftm0", "ftm0_fix_sel", "ftm0_ext_sel";
> > +		clocks = <&clks VF610_CLK_FTM0>,
> > +			<&clks VF610_CLK_FTM0_FIX_SEL>,
> > +			<&clks VF610_CLK_FTM0_EXT_SEL>;
> > +		pinctrl-names = "ch0-active", "ch0-idle", "ch1-active", "ch1-
> idle",
> > +		....;
> > +		pinctrl-0 = <&pinctrl_pwm0_ch0_active>;
> > +		pinctrl-1 = <&pinctrl_pwm0_ch0_idle>;
> > +		pinctrl-2 = <&pinctrl_pwm0_ch1_active>;
> > +		pinctrl-3 = <&pinctrl_pwm0_ch1_idle>;
> > +		...
> > +		fsl,pwm-counter-clk = "ftm0_ext_sel";
> > +		fsl,pwm-avaliable-chs = <0 3 5 6>;
> 
> I don't think this proerty is useful. Just enable all channels. I think
> this was mentioned before.
> 
Yes.
Actully this property is located in board level dts file.
I have added and requested all the channels in SoC level dtsi file, and in board level dts file to tell the customer the limitation, I think is much safter and better.

--
Best Regards.
Xiubo

Re: [PATCHv2 4/4] Documentation: Add device tree bindings for Freescale FTM PWM.

[Sascha Hauer]

On Mon, Sep 02, 2013 at 02:38:53AM +0000, Xiubo Li-B47053 wrote:
> > Subject: Re: [PATCHv2 4/4] Documentation: Add device tree bindings for
> > Freescale FTM PWM.
> > 
> ...
> > > +
> > > +pwm0: pwm@40038000 {
> > > +		compatible = "fsl,vf610-ftm-pwm";
> > > +		reg = <0x40038000 0x1000>;
> > > +		#pwm-cells = <3>;
> > > +		clock-names = "ftm0", "ftm0_fix_sel", "ftm0_ext_sel";
> > > +		clocks = <&clks VF610_CLK_FTM0>,
> > > +			<&clks VF610_CLK_FTM0_FIX_SEL>,
> > > +			<&clks VF610_CLK_FTM0_EXT_SEL>;
> > > +		pinctrl-names = "ch0-active", "ch0-idle", "ch1-active", "ch1-
> > idle",
> > > +		....;
> > > +		pinctrl-0 = <&pinctrl_pwm0_ch0_active>;
> > > +		pinctrl-1 = <&pinctrl_pwm0_ch0_idle>;
> > > +		pinctrl-2 = <&pinctrl_pwm0_ch1_active>;
> > > +		pinctrl-3 = <&pinctrl_pwm0_ch1_idle>;
> > > +		...
> > > +		fsl,pwm-counter-clk = "ftm0_ext_sel";
> > > +		fsl,pwm-avaliable-chs = <0 3 5 6>;
> > 
> > I don't think this proerty is useful. Just enable all channels. I think
> > this was mentioned before.
> > 
> Yes.
> Actully this property is located in board level dts file.
> I have added and requested all the channels in SoC level dtsi file,
> and in board level dts file to tell the customer the limitation, I
> think is much safter and better.

Why should this be in the board file? A pwm that is not available should
simply not be referenced and thus be unused. No need to explicitly
disable it.

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 |