[RFCv2 endianess 0/4] Add Freescale FTM PWM driver.

[RFCv2 endianess 0/4] Add Freescale FTM PWM driver.

[Xiubo Li]

I'm sending the RFCv2 patch series about the FTM IP block registers
read and write endian issue, and there is no buffers or descriptors
involved.

For the FTM IP block, in Vybird VF610 Tower the LE mode is in use,
in LS-1 the BE mode is in use. And the CPU always operates in LE mode.

So now I must take care of all these two cases. In this patch series I
have implemented two functions fsl_pwm_readl() and fsl_pwm_writel() to
replace readl() and writel(). At the same time there should add one
"big-endian" property for dt node.

And this patch is based the V6 series.

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

[Xiubo Li]

The FTM PWM device can be found on Vybrid VF610 Tower and
Layerscape LS-1 SoCs.

Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
Signed-off-by: Alison Wang <b18965@freescale.com>
Signed-off-by: Jingchang Lu <b35083@freescale.com>
Reviewed-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/pwm/Kconfig       |  10 ++
 drivers/pwm/Makefile      |   1 +
 drivers/pwm/pwm-fsl-ftm.c | 422 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 433 insertions(+)
 create mode 100644 drivers/pwm/pwm-fsl-ftm.c

diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index eece329..c77c571 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -71,6 +71,16 @@ config PWM_EP93XX
 	  To compile this driver as a module, choose M here: the module
 	  will be called pwm-ep93xx.
 
+config PWM_FSL_FTM
+	tristate "Freescale FlexTimer Module (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 8b754e4..9029a12 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -5,6 +5,7 @@ obj-$(CONFIG_PWM_ATMEL_TCB)	+= pwm-atmel-tcb.o
 obj-$(CONFIG_PWM_BFIN)		+= pwm-bfin.o
 obj-$(CONFIG_PWM_EP93XX)	+= pwm-ep93xx.o
 obj-$(CONFIG_PWM_IMX)		+= pwm-imx.o
+obj-$(CONFIG_PWM_FSL_FTM)	+= pwm-fsl-ftm.o
 obj-$(CONFIG_PWM_JZ4740)	+= pwm-jz4740.o
 obj-$(CONFIG_PWM_LPC32XX)	+= pwm-lpc32xx.o
 obj-$(CONFIG_PWM_MXS)		+= pwm-mxs.o
diff --git a/drivers/pwm/pwm-fsl-ftm.c b/drivers/pwm/pwm-fsl-ftm.c
new file mode 100644
index 0000000..33df2f9
--- /dev/null
+++ b/drivers/pwm/pwm-fsl-ftm.c
@@ -0,0 +1,422 @@
+/*
+ *  Freescale FlexTimer Module (FTM) PWM Driver
+ *
+ *  Copyright 2012-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/clk.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/pwm.h>
+#include <linux/slab.h>
+
+#define FTM_SC	0x00
+#define FTM_SC_CLK_MASK	0x3
+#define FTM_SC_CLK_SHIFT	3
+#define FTM_SC_CLK_SYS	(0x1 << FTM_SC_CLK_SHIFT)
+#define FTM_SC_CLK_FIX	(0x2 << FTM_SC_CLK_SHIFT)
+#define FTM_SC_CLK_EXT	(0x3 << FTM_SC_CLK_SHIFT)
+#define FTM_SC_PS_MASK	0x7
+#define FTM_SC_PS_SHIFT	0
+
+#define FTM_CNT		0x04
+#define FTM_MOD		0x08
+
+#define FTM_CSC_BASE	0x0C
+#define FTM_CSC_MSB	BIT(5)
+#define FTM_CSC_MSA	BIT(4)
+#define FTM_CSC_ELSB	BIT(3)
+#define FTM_CSC_ELSA	BIT(2)
+#define FTM_CSC(_channel)	(FTM_CSC_BASE + ((_channel) * 8))
+
+#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 FTM_MODE_FTMEN	BIT(0)
+#define FTM_MODE_INIT	BIT(2)
+#define FTM_MODE_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
+
+enum {
+	FSL_PWM_SYS_CLK,
+	FSL_PWM_EXT_CLK,
+	FSL_PWM_FIX_CLK,
+};
+
+struct fsl_pwm_chip {
+	struct pwm_chip chip;
+
+	struct mutex lock;
+
+	struct clk *sys_clk;
+	struct clk *counter_clk;
+	unsigned int counter_clk_select;
+	unsigned int counter_clk_enable;
+	unsigned int clk_ps;
+
+	void __iomem *base;
+
+	int period_ns;
+	int big_endian;
+};
+
+static inline u32 fsl_pwm_readl(struct fsl_pwm_chip *fpc,
+		const void __iomem *addr)
+{
+	if (likely(fpc->big_endian))
+		return ioread32be(addr);
+	else
+		return readl(addr);
+}
+
+static inline void fsl_pwm_writel(struct fsl_pwm_chip *fpc,
+		u32 val, void __iomem *addr)
+{
+	if (likely(fpc->big_endian))
+		iowrite32be(val, addr);
+	else
+		writel(val, addr);
+}
+
+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)
+{
+	struct fsl_pwm_chip *fpc = to_fsl_chip(chip);
+
+	return clk_prepare_enable(fpc->sys_clk);
+}
+
+static void fsl_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	struct fsl_pwm_chip *fpc = to_fsl_chip(chip);
+
+	clk_disable_unprepare(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;
+
+	c = clk_get_rate(fpc->counter_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, duty;
+	struct fsl_pwm_chip *fpc = to_fsl_chip(chip);
+
+	mutex_lock(&fpc->lock);
+	if (fpc->period_ns && fpc->period_ns != period_ns) {
+		dev_err(fpc->chip.dev,
+				"All the PWMs' period value should be "
+				"the same\n");
+		mutex_unlock(&fpc->lock);
+		return -EINVAL;
+	} else {
+		period = fsl_rate_to_cycles(fpc, period_ns);
+		if (period > 0xFFFF) {
+			mutex_unlock(&fpc->lock);
+			return -EINVAL;
+		}
+
+		fsl_pwm_writel(fpc, period - 1, fpc->base + FTM_MOD);
+		fpc->period_ns = period_ns;
+	}
+	mutex_unlock(&fpc->lock);
+
+	duty = fsl_rate_to_cycles(fpc, duty_ns);
+	if (duty >= 0xFFFF)
+		return -EINVAL;
+
+	fsl_pwm_writel(fpc, FTM_CSC_MSB | FTM_CSC_ELSB,
+			fpc->base + FTM_CSC(pwm->hwpwm));
+	fsl_pwm_writel(fpc, 0, fpc->base + FTM_CNTIN);
+	fsl_pwm_writel(fpc, duty, 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)
+{
+	u32 val;
+	struct fsl_pwm_chip *fpc = to_fsl_chip(chip);
+
+	val = fsl_pwm_readl(fpc, fpc->base + FTM_POL);
+	if (polarity == PWM_POLARITY_INVERSED)
+		val |= BIT(pwm->hwpwm);
+	else
+		val &= ~BIT(pwm->hwpwm);
+	fsl_pwm_writel(fpc, val, fpc->base + FTM_POL);
+
+	return 0;
+}
+
+static int fsl_counter_clock_enable(struct fsl_pwm_chip *fpc)
+{
+	int ret;
+	u32 val;
+
+	if (fpc->counter_clk_enable++)
+		return 0;
+
+	ret = clk_prepare_enable(fpc->counter_clk);
+	if (ret) {
+		fpc->counter_clk_enable--;
+		return ret;
+	}
+
+	val = fsl_pwm_readl(fpc, fpc->base + FTM_SC);
+	val &= ~((FTM_SC_CLK_MASK << FTM_SC_CLK_SHIFT) |
+			(FTM_SC_PS_MASK << FTM_SC_PS_SHIFT));
+
+	/* select counter clock source */
+	switch (fpc->counter_clk_select) {
+	case FSL_PWM_SYS_CLK:
+		val |= FTM_SC_CLK_SYS;
+		break;
+	case FSL_PWM_FIX_CLK:
+		val |= FTM_SC_CLK_FIX;
+		break;
+	case FSL_PWM_EXT_CLK:
+		val |= FTM_SC_CLK_EXT;
+		break;
+	default:
+		fpc->counter_clk_enable--;
+		return -EINVAL;
+	}
+
+	val |= fpc->clk_ps;
+	fsl_pwm_writel(fpc, val, fpc->base + FTM_SC);
+
+	return 0;
+}
+
+static int fsl_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	int ret;
+	u32 val;
+	struct fsl_pwm_chip *fpc = to_fsl_chip(chip);
+
+	val = fsl_pwm_readl(fpc, fpc->base + FTM_OUTMASK);
+	val &= ~BIT(pwm->hwpwm);
+	fsl_pwm_writel(fpc, val, fpc->base + FTM_OUTMASK);
+
+	val = fsl_pwm_readl(fpc, fpc->base + FTM_OUTINIT);
+	val &= ~BIT(pwm->hwpwm);
+	fsl_pwm_writel(fpc, val, fpc->base + FTM_OUTINIT);
+
+	mutex_lock(&fpc->lock);
+	ret = fsl_counter_clock_enable(fpc);
+	mutex_unlock(&fpc->lock);
+
+	return ret;
+}
+
+static void fsl_counter_clock_disable(struct fsl_pwm_chip *fpc)
+{
+	u32 val;
+
+	if (--fpc->counter_clk_enable)
+		return;
+
+	val = fsl_pwm_readl(fpc, fpc->base + FTM_SC);
+	val &= ~(FTM_SC_CLK_MASK << FTM_SC_CLK_SHIFT);
+	fsl_pwm_writel(fpc, val, fpc->base + FTM_SC);
+
+	clk_disable_unprepare(fpc->counter_clk);
+}
+
+static void fsl_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	u32 val;
+	struct fsl_pwm_chip *fpc = to_fsl_chip(chip);
+
+	val = fsl_pwm_readl(fpc, fpc->base + FTM_OUTMASK);
+	val |= BIT(pwm->hwpwm);
+	fsl_pwm_writel(fpc, val, fpc->base + FTM_OUTMASK);
+
+	mutex_lock(&fpc->lock);
+	fsl_counter_clock_disable(fpc);
+
+	val = fsl_pwm_readl(fpc, fpc->base + FTM_OUTMASK);
+	if ((val & 0xFF) == 0xFF)
+		fpc->period_ns = 0;
+	mutex_unlock(&fpc->lock);
+
+}
+
+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_parse_clk_ps(struct fsl_pwm_chip *fpc)
+{
+	struct clk *ext_clk, *fix_clk;
+	unsigned long long sys_rate, ext_rate, fix_rate, ratio;
+
+	fpc->sys_clk = devm_clk_get(fpc->chip.dev, "ftm_sys");
+	if (IS_ERR(fpc->sys_clk)) {
+		dev_err(fpc->chip.dev,
+				"failed to get \"ftm_sys\" clock\n");
+		return PTR_ERR(fpc->sys_clk);
+	}
+
+	ext_clk = devm_clk_get(fpc->chip.dev, "ftm_ext");
+	if (IS_ERR(ext_clk))
+		ext_clk = NULL;
+
+	fix_clk = devm_clk_get(fpc->chip.dev, "ftm_fix");
+	if (IS_ERR(fix_clk))
+		fix_clk = NULL;
+
+	sys_rate = clk_get_rate(fpc->sys_clk);
+	if (!sys_rate)
+		return -EINVAL;
+
+	ext_rate = clk_get_rate(ext_clk);
+	fix_rate = clk_get_rate(fix_clk);
+
+	if (sys_rate >= ext_rate && sys_rate >= fix_rate) {
+		fpc->clk_ps = 7;
+		fpc->counter_clk = fpc->sys_clk;
+		fpc->counter_clk_select = FSL_PWM_SYS_CLK;
+	} else if (ext_rate > fix_rate) {
+		ratio = 4 * ext_rate - 1;
+		do_div(ratio, sys_rate);
+		fpc->clk_ps = ratio;
+		fpc->counter_clk = ext_clk;
+		fpc->counter_clk_select = FSL_PWM_FIX_CLK;
+	} else {
+		ratio = 2 * fix_rate - 1;
+		do_div(ratio, sys_rate);
+		fpc->clk_ps = ratio;
+		fpc->counter_clk = fix_clk;
+		fpc->counter_clk_select = FSL_PWM_EXT_CLK;
+	}
+
+	return 0;
+}
+
+static int fsl_pwm_probe(struct platform_device *pdev)
+{
+	int ret;
+	struct fsl_pwm_chip *fpc;
+	struct resource *res;
+	struct device_node *np = pdev->dev.of_node;
+
+	fpc = devm_kzalloc(&pdev->dev, sizeof(*fpc), GFP_KERNEL);
+	if (!fpc)
+		return -ENOMEM;
+
+	mutex_init(&fpc->lock);
+
+	fpc->chip.dev = &pdev->dev;
+
+	if (of_get_property(np, "big-endian", NULL))
+		fpc->big_endian = 1;
+
+	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))
+		return PTR_ERR(fpc->base);
+
+	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 = 8;
+
+	ret = pwmchip_add(&fpc->chip);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to add PWM chip %d\n", ret);
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, fpc);
+
+	return 0;
+}
+
+static int fsl_pwm_remove(struct platform_device *pdev)
+{
+	struct fsl_pwm_chip *fpc = platform_get_drvdata(pdev);
+
+	mutex_destroy(&fpc->lock);
+
+	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",
+		.of_match_table = fsl_pwm_dt_ids,
+	},
+	.probe = fsl_pwm_probe,
+	.remove = fsl_pwm_remove,
+};
+module_platform_driver(fsl_pwm_driver);
+
+MODULE_DESCRIPTION("Freescale FlexTimer Module PWM Driver");
+MODULE_AUTHOR("Xiubo Li <Li.Xiubo@freescale.com>");
+MODULE_ALIAS("platform:fsl-ftm-pwm");
+MODULE_LICENSE("GPL");
-- 
1.8.4

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

[Xiubo Li]

This adds devicetree node for VF610, and there are 8 channels
supported.

Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
Reviewed-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/boot/dts/vf610.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm/boot/dts/vf610.dtsi b/arch/arm/boot/dts/vf610.dtsi
index 67d929c..24aad55 100644
--- a/arch/arm/boot/dts/vf610.dtsi
+++ b/arch/arm/boot/dts/vf610.dtsi
@@ -140,6 +140,17 @@
 				clock-names = "pit";
 			};
 
+			pwm0: pwm@40038000 {
+				compatible = "fsl,vf610-ftm-pwm";
+				#pwm-cells = <3>;
+				reg = <0x40038000 0x1000>;
+				clock-names = "ftm_sys", "ftm_ext", "ftm_fix";
+				clocks = <&clks VF610_CLK_FTM0>,
+					<&clks VF610_CLK_FTM0_EXT_SEL>,
+					<&clks VF610_CLK_FTM0_FIX_SEL>;
+				status = "disabled";
+			};
+
 			wdog@4003e000 {
 				compatible = "fsl,vf610-wdt", "fsl,imx21-wdt";
 				reg = <0x4003e000 0x1000>;
-- 
1.8.4

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

[Xiubo Li]

Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
Reviewed-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 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 82d352f..3130f85 100644
--- a/arch/arm/boot/dts/vf610-twr.dts
+++ b/arch/arm/boot/dts/vf610-twr.dts
@@ -83,6 +83,12 @@
 	status = "okay";
 };
 
+&pwm0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_pwm0_1>;
+	status = "okay";
+};
+
 &uart1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_uart1_1>;
-- 
1.8.4

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

[Xiubo Li]

This adds the binding documentation for Freescale FlexTimer Module
(FTM) PWM driver under Documentation/devicetree/bindings/pwm/.

Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
Reviewed-by: Sascha Hauer <s.hauer@pengutronix.de>
Acked-by: Kumar Gala <galak@codeaurora.org>
---
 .../devicetree/bindings/pwm/pwm-fsl-ftm.txt        | 35 ++++++++++++++++++++++
 1 file changed, 35 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..accc931
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/pwm-fsl-ftm.txt
@@ -0,0 +1,35 @@
+Freescale FlexTimer Module (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 : Should include the following module clock source entries:
+    "ftm_sys" (module clock, also can be used as counter clock),
+    "ftm_ext" (external counter clock),
+    "ftm_fix" (fixed counter clock),
+- clocks : Must contain a clock specifier for each entry in clock-names,
+  See clock/clock-bindings.txt for details of the property values.
+- pinctrl-names: Must contain a "default" entry.
+- pinctrl-NNN: One property must exist for each entry in pinctrl-names.
+  See pinctrl/pinctrl-bindings.txt for details of the property values.
+- big-endian: If this property is absent, the little endian mode will be in
+  use as default, or the big endian mode will be in use for all the device
+  registers.
+
+
+Example:
+
+pwm0: pwm@40038000 {
+		compatible = "fsl,vf610-ftm-pwm";
+		reg = <0x40038000 0x1000>;
+		#pwm-cells = <3>;
+		clock-names = "ftm_sys", "ftm_ext", "ftm_fix";
+		clocks = <&clks VF610_CLK_FTM0>,
+			<&clks VF610_CLK_FTM0_EXT_SEL>,
+			<&clks VF610_CLK_FTM0_FIX_SEL>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_pwm0_1>;
+		big-endian;
+};
-- 
1.8.4

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

[Li.Xiubo]

Hi Mark, Therry,

Is this series of patches ok about endianess fix?

Here I add one boolen big-endian property.

--
Thanks,


> The FTM PWM device can be found on Vybrid VF610 Tower and Layerscape LS-1
> SoCs.
> 
> Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
> Signed-off-by: Alison Wang <b18965@freescale.com>
> Signed-off-by: Jingchang Lu <b35083@freescale.com>
> Reviewed-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/pwm/Kconfig       |  10 ++
>  drivers/pwm/Makefile      |   1 +
>  drivers/pwm/pwm-fsl-ftm.c | 422
> ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 433 insertions(+)
>  create mode 100644 drivers/pwm/pwm-fsl-ftm.c
> 
> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig index
> eece329..c77c571 100644
> --- a/drivers/pwm/Kconfig
> +++ b/drivers/pwm/Kconfig
> @@ -71,6 +71,16 @@ config PWM_EP93XX
>  	  To compile this driver as a module, choose M here: the module
>  	  will be called pwm-ep93xx.
> 
> +config PWM_FSL_FTM
> +	tristate "Freescale FlexTimer Module (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
> 8b754e4..9029a12 100644
> --- a/drivers/pwm/Makefile
> +++ b/drivers/pwm/Makefile
> @@ -5,6 +5,7 @@ obj-$(CONFIG_PWM_ATMEL_TCB)	+= pwm-atmel-tcb.o
>  obj-$(CONFIG_PWM_BFIN)		+= pwm-bfin.o
>  obj-$(CONFIG_PWM_EP93XX)	+= pwm-ep93xx.o
>  obj-$(CONFIG_PWM_IMX)		+= pwm-imx.o
> +obj-$(CONFIG_PWM_FSL_FTM)	+= pwm-fsl-ftm.o
>  obj-$(CONFIG_PWM_JZ4740)	+= pwm-jz4740.o
>  obj-$(CONFIG_PWM_LPC32XX)	+= pwm-lpc32xx.o
>  obj-$(CONFIG_PWM_MXS)		+= pwm-mxs.o
> diff --git a/drivers/pwm/pwm-fsl-ftm.c b/drivers/pwm/pwm-fsl-ftm.c new file
> mode 100644 index 0000000..33df2f9
> --- /dev/null
> +++ b/drivers/pwm/pwm-fsl-ftm.c
> @@ -0,0 +1,422 @@
> +/*
> + *  Freescale FlexTimer Module (FTM) PWM Driver
> + *
> + *  Copyright 2012-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/clk.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of_address.h>
> +#include <linux/platform_device.h>
> +#include <linux/pwm.h>
> +#include <linux/slab.h>
> +
> +#define FTM_SC	0x00
> +#define FTM_SC_CLK_MASK	0x3
> +#define FTM_SC_CLK_SHIFT	3
> +#define FTM_SC_CLK_SYS	(0x1 << FTM_SC_CLK_SHIFT)
> +#define FTM_SC_CLK_FIX	(0x2 << FTM_SC_CLK_SHIFT)
> +#define FTM_SC_CLK_EXT	(0x3 << FTM_SC_CLK_SHIFT)
> +#define FTM_SC_PS_MASK	0x7
> +#define FTM_SC_PS_SHIFT	0
> +
> +#define FTM_CNT		0x04
> +#define FTM_MOD		0x08
> +
> +#define FTM_CSC_BASE	0x0C
> +#define FTM_CSC_MSB	BIT(5)
> +#define FTM_CSC_MSA	BIT(4)
> +#define FTM_CSC_ELSB	BIT(3)
> +#define FTM_CSC_ELSA	BIT(2)
> +#define FTM_CSC(_channel)	(FTM_CSC_BASE + ((_channel) * 8))
> +
> +#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 FTM_MODE_FTMEN	BIT(0)
> +#define FTM_MODE_INIT	BIT(2)
> +#define FTM_MODE_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
> +
> +enum {
> +	FSL_PWM_SYS_CLK,
> +	FSL_PWM_EXT_CLK,
> +	FSL_PWM_FIX_CLK,
> +};
> +
> +struct fsl_pwm_chip {
> +	struct pwm_chip chip;
> +
> +	struct mutex lock;
> +
> +	struct clk *sys_clk;
> +	struct clk *counter_clk;
> +	unsigned int counter_clk_select;
> +	unsigned int counter_clk_enable;
> +	unsigned int clk_ps;
> +
> +	void __iomem *base;
> +
> +	int period_ns;
> +	int big_endian;
> +};
> +
> +static inline u32 fsl_pwm_readl(struct fsl_pwm_chip *fpc,
> +		const void __iomem *addr)
> +{
> +	if (likely(fpc->big_endian))
> +		return ioread32be(addr);
> +	else
> +		return readl(addr);
> +}
> +
> +static inline void fsl_pwm_writel(struct fsl_pwm_chip *fpc,
> +		u32 val, void __iomem *addr)
> +{
> +	if (likely(fpc->big_endian))
> +		iowrite32be(val, addr);
> +	else
> +		writel(val, addr);
> +}
> +
> +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) {
> +	struct fsl_pwm_chip *fpc = to_fsl_chip(chip);
> +
> +	return clk_prepare_enable(fpc->sys_clk); }
> +
> +static void fsl_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
> +{
> +	struct fsl_pwm_chip *fpc = to_fsl_chip(chip);
> +
> +	clk_disable_unprepare(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;
> +
> +	c = clk_get_rate(fpc->counter_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, duty;
> +	struct fsl_pwm_chip *fpc = to_fsl_chip(chip);
> +
> +	mutex_lock(&fpc->lock);
> +	if (fpc->period_ns && fpc->period_ns != period_ns) {
> +		dev_err(fpc->chip.dev,
> +				"All the PWMs' period value should be "
> +				"the same\n");
> +		mutex_unlock(&fpc->lock);
> +		return -EINVAL;
> +	} else {
> +		period = fsl_rate_to_cycles(fpc, period_ns);
> +		if (period > 0xFFFF) {
> +			mutex_unlock(&fpc->lock);
> +			return -EINVAL;
> +		}
> +
> +		fsl_pwm_writel(fpc, period - 1, fpc->base + FTM_MOD);
> +		fpc->period_ns = period_ns;
> +	}
> +	mutex_unlock(&fpc->lock);
> +
> +	duty = fsl_rate_to_cycles(fpc, duty_ns);
> +	if (duty >= 0xFFFF)
> +		return -EINVAL;
> +
> +	fsl_pwm_writel(fpc, FTM_CSC_MSB | FTM_CSC_ELSB,
> +			fpc->base + FTM_CSC(pwm->hwpwm));
> +	fsl_pwm_writel(fpc, 0, fpc->base + FTM_CNTIN);
> +	fsl_pwm_writel(fpc, duty, 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)
> +{
> +	u32 val;
> +	struct fsl_pwm_chip *fpc = to_fsl_chip(chip);
> +
> +	val = fsl_pwm_readl(fpc, fpc->base + FTM_POL);
> +	if (polarity == PWM_POLARITY_INVERSED)
> +		val |= BIT(pwm->hwpwm);
> +	else
> +		val &= ~BIT(pwm->hwpwm);
> +	fsl_pwm_writel(fpc, val, fpc->base + FTM_POL);
> +
> +	return 0;
> +}
> +
> +static int fsl_counter_clock_enable(struct fsl_pwm_chip *fpc) {
> +	int ret;
> +	u32 val;
> +
> +	if (fpc->counter_clk_enable++)
> +		return 0;
> +
> +	ret = clk_prepare_enable(fpc->counter_clk);
> +	if (ret) {
> +		fpc->counter_clk_enable--;
> +		return ret;
> +	}
> +
> +	val = fsl_pwm_readl(fpc, fpc->base + FTM_SC);
> +	val &= ~((FTM_SC_CLK_MASK << FTM_SC_CLK_SHIFT) |
> +			(FTM_SC_PS_MASK << FTM_SC_PS_SHIFT));
> +
> +	/* select counter clock source */
> +	switch (fpc->counter_clk_select) {
> +	case FSL_PWM_SYS_CLK:
> +		val |= FTM_SC_CLK_SYS;
> +		break;
> +	case FSL_PWM_FIX_CLK:
> +		val |= FTM_SC_CLK_FIX;
> +		break;
> +	case FSL_PWM_EXT_CLK:
> +		val |= FTM_SC_CLK_EXT;
> +		break;
> +	default:
> +		fpc->counter_clk_enable--;
> +		return -EINVAL;
> +	}
> +
> +	val |= fpc->clk_ps;
> +	fsl_pwm_writel(fpc, val, fpc->base + FTM_SC);
> +
> +	return 0;
> +}
> +
> +static int fsl_pwm_enable(struct pwm_chip *chip, struct pwm_device
> +*pwm) {
> +	int ret;
> +	u32 val;
> +	struct fsl_pwm_chip *fpc = to_fsl_chip(chip);
> +
> +	val = fsl_pwm_readl(fpc, fpc->base + FTM_OUTMASK);
> +	val &= ~BIT(pwm->hwpwm);
> +	fsl_pwm_writel(fpc, val, fpc->base + FTM_OUTMASK);
> +
> +	val = fsl_pwm_readl(fpc, fpc->base + FTM_OUTINIT);
> +	val &= ~BIT(pwm->hwpwm);
> +	fsl_pwm_writel(fpc, val, fpc->base + FTM_OUTINIT);
> +
> +	mutex_lock(&fpc->lock);
> +	ret = fsl_counter_clock_enable(fpc);
> +	mutex_unlock(&fpc->lock);
> +
> +	return ret;
> +}
> +
> +static void fsl_counter_clock_disable(struct fsl_pwm_chip *fpc) {
> +	u32 val;
> +
> +	if (--fpc->counter_clk_enable)
> +		return;
> +
> +	val = fsl_pwm_readl(fpc, fpc->base + FTM_SC);
> +	val &= ~(FTM_SC_CLK_MASK << FTM_SC_CLK_SHIFT);
> +	fsl_pwm_writel(fpc, val, fpc->base + FTM_SC);
> +
> +	clk_disable_unprepare(fpc->counter_clk);
> +}
> +
> +static void fsl_pwm_disable(struct pwm_chip *chip, struct pwm_device
> +*pwm) {
> +	u32 val;
> +	struct fsl_pwm_chip *fpc = to_fsl_chip(chip);
> +
> +	val = fsl_pwm_readl(fpc, fpc->base + FTM_OUTMASK);
> +	val |= BIT(pwm->hwpwm);
> +	fsl_pwm_writel(fpc, val, fpc->base + FTM_OUTMASK);
> +
> +	mutex_lock(&fpc->lock);
> +	fsl_counter_clock_disable(fpc);
> +
> +	val = fsl_pwm_readl(fpc, fpc->base + FTM_OUTMASK);
> +	if ((val & 0xFF) == 0xFF)
> +		fpc->period_ns = 0;
> +	mutex_unlock(&fpc->lock);
> +
> +}
> +
> +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_parse_clk_ps(struct fsl_pwm_chip *fpc) {
> +	struct clk *ext_clk, *fix_clk;
> +	unsigned long long sys_rate, ext_rate, fix_rate, ratio;
> +
> +	fpc->sys_clk = devm_clk_get(fpc->chip.dev, "ftm_sys");
> +	if (IS_ERR(fpc->sys_clk)) {
> +		dev_err(fpc->chip.dev,
> +				"failed to get \"ftm_sys\" clock\n");
> +		return PTR_ERR(fpc->sys_clk);
> +	}
> +
> +	ext_clk = devm_clk_get(fpc->chip.dev, "ftm_ext");
> +	if (IS_ERR(ext_clk))
> +		ext_clk = NULL;
> +
> +	fix_clk = devm_clk_get(fpc->chip.dev, "ftm_fix");
> +	if (IS_ERR(fix_clk))
> +		fix_clk = NULL;
> +
> +	sys_rate = clk_get_rate(fpc->sys_clk);
> +	if (!sys_rate)
> +		return -EINVAL;
> +
> +	ext_rate = clk_get_rate(ext_clk);
> +	fix_rate = clk_get_rate(fix_clk);
> +
> +	if (sys_rate >= ext_rate && sys_rate >= fix_rate) {
> +		fpc->clk_ps = 7;
> +		fpc->counter_clk = fpc->sys_clk;
> +		fpc->counter_clk_select = FSL_PWM_SYS_CLK;
> +	} else if (ext_rate > fix_rate) {
> +		ratio = 4 * ext_rate - 1;
> +		do_div(ratio, sys_rate);
> +		fpc->clk_ps = ratio;
> +		fpc->counter_clk = ext_clk;
> +		fpc->counter_clk_select = FSL_PWM_FIX_CLK;
> +	} else {
> +		ratio = 2 * fix_rate - 1;
> +		do_div(ratio, sys_rate);
> +		fpc->clk_ps = ratio;
> +		fpc->counter_clk = fix_clk;
> +		fpc->counter_clk_select = FSL_PWM_EXT_CLK;
> +	}
> +
> +	return 0;
> +}
> +
> +static int fsl_pwm_probe(struct platform_device *pdev) {
> +	int ret;
> +	struct fsl_pwm_chip *fpc;
> +	struct resource *res;
> +	struct device_node *np = pdev->dev.of_node;
> +
> +	fpc = devm_kzalloc(&pdev->dev, sizeof(*fpc), GFP_KERNEL);
> +	if (!fpc)
> +		return -ENOMEM;
> +
> +	mutex_init(&fpc->lock);
> +
> +	fpc->chip.dev = &pdev->dev;
> +
> +	if (of_get_property(np, "big-endian", NULL))
> +		fpc->big_endian = 1;
> +
> +	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))
> +		return PTR_ERR(fpc->base);
> +
> +	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 = 8;
> +
> +	ret = pwmchip_add(&fpc->chip);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "failed to add PWM chip %d\n", ret);
> +		return ret;
> +	}
> +
> +	platform_set_drvdata(pdev, fpc);
> +
> +	return 0;
> +}
> +
> +static int fsl_pwm_remove(struct platform_device *pdev) {
> +	struct fsl_pwm_chip *fpc = platform_get_drvdata(pdev);
> +
> +	mutex_destroy(&fpc->lock);
> +
> +	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",
> +		.of_match_table = fsl_pwm_dt_ids,
> +	},
> +	.probe = fsl_pwm_probe,
> +	.remove = fsl_pwm_remove,
> +};
> +module_platform_driver(fsl_pwm_driver);
> +
> +MODULE_DESCRIPTION("Freescale FlexTimer Module PWM Driver");
> +MODULE_AUTHOR("Xiubo Li <Li.Xiubo@freescale.com>");
> +MODULE_ALIAS("platform:fsl-ftm-pwm");
> +MODULE_LICENSE("GPL");
> --
> 1.8.4
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pwm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Mark Rutland]

On Tue, Dec 10, 2013 at 05:10:01AM +0000, Li.Xiubo@freescale.com wrote:
> Hi Mark, Therry,
>
> Is this series of patches ok about endianess fix?
>
> Here I add one boolen big-endian property.

[...]

> > +static inline u32 fsl_pwm_readl(struct fsl_pwm_chip *fpc,
> > +             const void __iomem *addr)
> > +{
> > +     if (likely(fpc->big_endian))
> > +             return ioread32be(addr);
> > +     else
> > +             return readl(addr);
> > +}

It looks a little odd to to have two different accessors here.

Could these not be unified somehow?

> > +static inline void fsl_pwm_writel(struct fsl_pwm_chip *fpc,
> > +             u32 val, void __iomem *addr)
> > +{
> > +     if (likely(fpc->big_endian))
> > +             iowrite32be(val, addr);
> > +     else
> > +             writel(val, addr);
> > +}

Likewise.

[...]

> > +static int fsl_pwm_probe(struct platform_device *pdev) {
> > +     int ret;
> > +     struct fsl_pwm_chip *fpc;
> > +     struct resource *res;
> > +     struct device_node *np = pdev->dev.of_node;
> > +
> > +     fpc = devm_kzalloc(&pdev->dev, sizeof(*fpc), GFP_KERNEL);
> > +     if (!fpc)
> > +             return -ENOMEM;
> > +
> > +     mutex_init(&fpc->lock);
> > +
> > +     fpc->chip.dev = &pdev->dev;
> > +
> > +     if (of_get_property(np, "big-endian", NULL))
> > +             fpc->big_endian = 1;

You can use of_property_read_bool:

fpc->big_endian = of_property_read_bool(np, "big-endian");

Otherwise, the DT stuff looks fine.

Mark.

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

[Li.Xiubo]

> > > +static inline u32 fsl_pwm_readl(struct fsl_pwm_chip *fpc,
> > > +             const void __iomem *addr)
> > > +{
> > > +     if (likely(fpc->big_endian))
> > > +             return ioread32be(addr);
> > > +     else
> > > +             return readl(addr);
> > > +}
> 
> It looks a little odd to to have two different accessors here.
> 
> Could these not be unified somehow?
> 

How about the following :

+static inline u32 fsl_pwm_readl(struct fsl_pwm_chip *fpc,
+			const void __iomem *addr)
+{
+	u32 val;
+
+	if (likely(fpc->big_endian))
+		val = be32_to_cpu(__raw_readl(addr));
+	else
+		val = le32_to_cpu(__raw_readl(addr));
+
+	rmb();
+
+	return val;
+}
+
+static inline void fsl_pwm_writel(struct fsl_pwm_chip *fpc,
+			u32 val, void __iomem *addr)
+{
+	wmb();
+
+	if (likely(fpc->big_endian))
+		__raw_writel(cpu_to_be32(val), addr);
+	else
+		__raw_writel(cpu_to_le32(val), addr);
+}
+


> > > +static inline void fsl_pwm_writel(struct fsl_pwm_chip *fpc,
> > > +             u32 val, void __iomem *addr)
> > > +{
> > > +     if (likely(fpc->big_endian))
> > > +             iowrite32be(val, addr);
> > > +     else
> > > +             writel(val, addr);
> > > +}
> 
> Likewise.
> 
> [...]
> 
> > > +static int fsl_pwm_probe(struct platform_device *pdev) {
> > > +     int ret;
> > > +     struct fsl_pwm_chip *fpc;
> > > +     struct resource *res;
> > > +     struct device_node *np = pdev->dev.of_node;
> > > +
> > > +     fpc = devm_kzalloc(&pdev->dev, sizeof(*fpc), GFP_KERNEL);
> > > +     if (!fpc)
> > > +             return -ENOMEM;
> > > +
> > > +     mutex_init(&fpc->lock);
> > > +
> > > +     fpc->chip.dev = &pdev->dev;
> > > +
> > > +     if (of_get_property(np, "big-endian", NULL))
> > > +             fpc->big_endian = 1;
> 
> You can use of_property_read_bool:
> 
> fpc->big_endian = of_property_read_bool(np, "big-endian");
> 
> Otherwise, the DT stuff looks fine.
> 

That sounds good.


--
Xiubo,

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

[Li.Xiubo]

Hi Mark,


> > > > +static inline u32 fsl_pwm_readl(struct fsl_pwm_chip *fpc,
> > > > +             const void __iomem *addr)
> > > > +{
> > > > +     if (likely(fpc->big_endian))
> > > > +             return ioread32be(addr);
> > > > +     else
> > > > +             return readl(addr);
> > > > +}
> >
> > It looks a little odd to to have two different accessors here.
> >
> > Could these not be unified somehow?
> >
> 
> How about the following :
> 
> +static inline u32 fsl_pwm_readl(struct fsl_pwm_chip *fpc,
> +			const void __iomem *addr)
> +{
> +	u32 val;
> +
> +	if (likely(fpc->big_endian))
> +		val = be32_to_cpu(__raw_readl(addr));
> +	else
> +		val = le32_to_cpu(__raw_readl(addr));
> +
> +	rmb();
> +
> +	return val;
> +}
> +
> +static inline void fsl_pwm_writel(struct fsl_pwm_chip *fpc,
> +			u32 val, void __iomem *addr)
> +{
> +	wmb();
> +
> +	if (likely(fpc->big_endian))
> +		__raw_writel(cpu_to_be32(val), addr);
> +	else
> +		__raw_writel(cpu_to_le32(val), addr);
> +}
> +
> 
> 

Or, will these be much better ?
+++++++++++
+static inline u32 fsl_pwm_readl(struct fsl_pwm_chip *fpc,
+		const void __iomem *addr)
+{
+	u32 val;
+
+	if (likely(fpc->big_endian))
+		val = be32_to_cpu((__force __be32)__raw_readl(addr));
+	else
+		val = le32_to_cpu((__force __le32)__raw_readl(addr));
+
+	rmb();
+
+	return val;
+}
+
+static inline void fsl_pwm_writel(struct fsl_pwm_chip *fpc,
+		u32 val, void __iomem *addr)
+{
+	wmb();
+
+	if (likely(fpc->big_endian))
+		__raw_writel((__force u32)cpu_to_be32(val), addr);
+	else
+		__raw_writel((__force u32)cpu_to_le32(val), addr); }
+
-----------


Thanks,
--
Xiubo

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

[Thierry Reding]

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

On Thu, Dec 12, 2013 at 02:43:14AM +0000, Li.Xiubo@freescale.com wrote:
> Hi Mark,
> 
> 
> > > > > +static inline u32 fsl_pwm_readl(struct fsl_pwm_chip *fpc,
> > > > > +             const void __iomem *addr)
> > > > > +{
> > > > > +     if (likely(fpc->big_endian))
> > > > > +             return ioread32be(addr);
> > > > > +     else
> > > > > +             return readl(addr);
> > > > > +}
> > >
> > > It looks a little odd to to have two different accessors here.
> > >
> > > Could these not be unified somehow?
> > >
> > 
> > How about the following :
> > 
> > +static inline u32 fsl_pwm_readl(struct fsl_pwm_chip *fpc,
> > +			const void __iomem *addr)
> > +{
> > +	u32 val;
> > +
> > +	if (likely(fpc->big_endian))
> > +		val = be32_to_cpu(__raw_readl(addr));
> > +	else
> > +		val = le32_to_cpu(__raw_readl(addr));
> > +
> > +	rmb();
> > +
> > +	return val;
> > +}
> > +
> > +static inline void fsl_pwm_writel(struct fsl_pwm_chip *fpc,
> > +			u32 val, void __iomem *addr)
> > +{
> > +	wmb();
> > +
> > +	if (likely(fpc->big_endian))
> > +		__raw_writel(cpu_to_be32(val), addr);
> > +	else
> > +		__raw_writel(cpu_to_le32(val), addr);
> > +}
> > +
> > 
> > 
> 
> Or, will these be much better ?
> +++++++++++
> +static inline u32 fsl_pwm_readl(struct fsl_pwm_chip *fpc,
> +		const void __iomem *addr)
> +{
> +	u32 val;
> +
> +	if (likely(fpc->big_endian))
> +		val = be32_to_cpu((__force __be32)__raw_readl(addr));
> +	else
> +		val = le32_to_cpu((__force __le32)__raw_readl(addr));
> +
> +	rmb();
> +
> +	return val;
> +}
> +
> +static inline void fsl_pwm_writel(struct fsl_pwm_chip *fpc,
> +		u32 val, void __iomem *addr)
> +{
> +	wmb();
> +
> +	if (likely(fpc->big_endian))
> +		__raw_writel((__force u32)cpu_to_be32(val), addr);
> +	else
> +		__raw_writel((__force u32)cpu_to_le32(val), addr); }
> +
> -----------

I think perhaps what Mark may have meant was something like this:

	static inline u32 fsl_pwm_readl(struct fsl_pwm_chip *fpc,
					const void __iomem *addr)
	{
		u32 value = readl(addr);

		if (likely(fpc->big_endian))
			value = be32_to_cpu(value);
		else
			value = le32_to_cpu(value);

		return value;
	}

	static inline void fsl_pwm_writel(struct fsl_pwm_chip *fpc, u32 value,
					  const void __iomem *addr)
	{
		if (likely(fpc->big_endian))
			value = cpu_to_be32(value);
		else
			value = cpu_to_le32(value);

		writel(value, addr);
	}

That way you call the accessors only once, and do the conversion after
or before that.

Thierry

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

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

[Li.Xiubo]

Hi Thierry,

> I think perhaps what Mark may have meant was something like this:
> 
> 	static inline u32 fsl_pwm_readl(struct fsl_pwm_chip *fpc,
> 					const void __iomem *addr)
> 	{
> 		u32 value = readl(addr);
> 
> 		if (likely(fpc->big_endian))
> 			value = be32_to_cpu(value);
> 		else
> 			value = le32_to_cpu(value);
> 
> 		return value;
> 	}
> 
> 	static inline void fsl_pwm_writel(struct fsl_pwm_chip *fpc, u32 value,
> 					  const void __iomem *addr)
> 	{
> 		if (likely(fpc->big_endian))
> 			value = cpu_to_be32(value);
> 		else
> 			value = cpu_to_le32(value);
> 
> 		writel(value, addr);
> 	}
> 
> That way you call the accessors only once, and do the conversion after
> or before that.
> 

Yes, this looks better.
And I have already sent the v7 patches has addressed about this.

--
Xiubo