[PATCH 0/2] pwm: Add support for R-Car PWM Timer

[PATCH 0/2] pwm: Add support for R-Car PWM Timer

[Yoshihiro Shimoda]

R-Car SoCs have a seven-channel pulse width modulation (PWM) timer.
This driver adds support for the PWM Timer as a single PWM chip and
seven PWM devices.

This patch set is based on the linux-pwm.git / for-next branch.
(commit id = efb0de55b6a2ec15fc424e660601f22ae2fa487a)

Yoshihiro Shimoda (2):
  pwm: Add device tree binding document for R-Car PWM Timer
  pwm: Add support for R-Car PWM Timer

 .../devicetree/bindings/pwm/renesas,pwm-rcar.txt   |  21 ++
 drivers/pwm/Kconfig                                |  11 +
 drivers/pwm/Makefile                               |   1 +
 drivers/pwm/pwm-rcar.c                             | 282 +++++++++++++++++++++
 4 files changed, 315 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.txt
 create mode 100644 drivers/pwm/pwm-rcar.c

-- 
1.9.1

[PATCH 0/2] pwm: Add support for R-Car PWM Timer

[Yoshihiro Shimoda]

R-Car SoCs have a seven-channel pulse width modulation (PWM) timer.
This driver adds support for the PWM Timer as a single PWM chip and
seven PWM devices.

This patch set is based on the linux-pwm.git / for-next branch.
(commit id = efb0de55b6a2ec15fc424e660601f22ae2fa487a)

Yoshihiro Shimoda (2):
  pwm: Add device tree binding document for R-Car PWM Timer
  pwm: Add support for R-Car PWM Timer

 .../devicetree/bindings/pwm/renesas,pwm-rcar.txt   |  21 ++
 drivers/pwm/Kconfig                                |  11 +
 drivers/pwm/Makefile                               |   1 +
 drivers/pwm/pwm-rcar.c                             | 282 +++++++++++++++++++++
 4 files changed, 315 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.txt
 create mode 100644 drivers/pwm/pwm-rcar.c

-- 
1.9.1

[PATCH 1/2] pwm: Add device tree binding document for R-Car PWM Timer

[Yoshihiro Shimoda]

Add binding document for Renesas PWM Timer on R-Car SoCs.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 .../devicetree/bindings/pwm/renesas,pwm-rcar.txt    | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.txt

diff --git a/Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.txt b/Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.txt
new file mode 100644
index 0000000..38123f9
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.txt
@@ -0,0 +1,21 @@
+* Renesas R-Car PWM Timer Controller
+
+Required Properties:
+- compatible: must contain "renesas,pwm-rcar"
+- reg: base address and length of the registers block for the PWM
+- #pwm-cells: should be 7. See pwm.txt in this directory for a description of
+  the cells format.
+- clocks: clock phandle and specifier pair.
+- pinctrl-0: phandle, referring to a default pin configuration node.
+- pinctrl-names: Set to "default"
+
+Example: R8A7790 (R-Car H2) PWM Timer node
+
+	pwm: pwm@e6e30000 {
+		compatible = "renesas,pwm-rcar";
+		reg = <0 0xe6e30000 0 0x7000>;
+		#pwm-cells = <7>;
+		clocks = <&mstp5_clks R8A7790_CLK_PWM>;
+		pinctrl-0 = <&pwm3_pins>;
+		pinctrl-names = "default";
+	};
-- 
1.9.1

[PATCH 1/2] pwm: Add device tree binding document for R-Car PWM Timer

[Yoshihiro Shimoda]

Add binding document for Renesas PWM Timer on R-Car SoCs.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 .../devicetree/bindings/pwm/renesas,pwm-rcar.txt    | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.txt

diff --git a/Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.txt b/Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.txt
new file mode 100644
index 0000000..38123f9
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.txt
@@ -0,0 +1,21 @@
+* Renesas R-Car PWM Timer Controller
+
+Required Properties:
+- compatible: must contain "renesas,pwm-rcar"
+- reg: base address and length of the registers block for the PWM
+- #pwm-cells: should be 7. See pwm.txt in this directory for a description of
+  the cells format.
+- clocks: clock phandle and specifier pair.
+- pinctrl-0: phandle, referring to a default pin configuration node.
+- pinctrl-names: Set to "default"
+
+Example: R8A7790 (R-Car H2) PWM Timer node
+
+	pwm: pwm@e6e30000 {
+		compatible = "renesas,pwm-rcar";
+		reg = <0 0xe6e30000 0 0x7000>;
+		#pwm-cells = <7>;
+		clocks = <&mstp5_clks R8A7790_CLK_PWM>;
+		pinctrl-0 = <&pwm3_pins>;
+		pinctrl-names = "default";
+	};
-- 
1.9.1

[PATCH 2/2] pwm: Add support for R-Car PWM Timer

[Yoshihiro Shimoda]

R-Car SoCs have a seven-channel pulse width modulation (PWM) timer.
This driver adds support for the PWM Timer as a single PWM chip and
seven PWM devices.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/pwm/Kconfig    |  11 ++
 drivers/pwm/Makefile   |   1 +
 drivers/pwm/pwm-rcar.c | 276 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 288 insertions(+)
 create mode 100644 drivers/pwm/pwm-rcar.c

diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index b1541f4..7e98175 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -249,6 +249,17 @@ config PWM_PXA
 	  To compile this driver as a module, choose M here: the module
 	  will be called pwm-pxa.
 
+config PWM_RCAR
+	tristate "Renesas R-Car PWM support"
+	depends on ARCH_SHMOBILE || COMPILE_TEST
+	depends on HAS_IOMEM
+	help
+	  This driver exposes the PWM Timer controller found in Renesas
+	  chips through the PWM API.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called pwm-rcar.
+
 config PWM_RENESAS_TPU
 	tristate "Renesas TPU PWM support"
 	depends on ARCH_SHMOBILE || COMPILE_TEST
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index ec50eb5..79d3dc3 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -22,6 +22,7 @@ obj-$(CONFIG_PWM_MXS)		+= pwm-mxs.o
 obj-$(CONFIG_PWM_PCA9685)	+= pwm-pca9685.o
 obj-$(CONFIG_PWM_PUV3)		+= pwm-puv3.o
 obj-$(CONFIG_PWM_PXA)		+= pwm-pxa.o
+obj-$(CONFIG_PWM_RCAR)		+= pwm-rcar.o
 obj-$(CONFIG_PWM_RENESAS_TPU)	+= pwm-renesas-tpu.o
 obj-$(CONFIG_PWM_ROCKCHIP)	+= pwm-rockchip.o
 obj-$(CONFIG_PWM_SAMSUNG)	+= pwm-samsung.o
diff --git a/drivers/pwm/pwm-rcar.c b/drivers/pwm/pwm-rcar.c
new file mode 100644
index 0000000..29011e8
--- /dev/null
+++ b/drivers/pwm/pwm-rcar.c
@@ -0,0 +1,276 @@
+/*
+ * R-Car PWM Timer driver
+ *
+ * Copyright (C) 2015 Renesas Electronics Corporation
+ *
+ * This is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/pwm.h>
+#include <linux/slab.h>
+
+#define NUM_RCAR_PWM_CHANNELS	7
+#define RCAR_PWM_MAX_DIVISION	24
+#define RCAR_PWM_MAX_CYCLE	1023
+
+#define RCAR_PWMCR		0x00
+#define RCAR_PWMCNT		0x04
+#define RCAR_PWM_CH_OFFSET	0x1000
+
+#define RCAR_PWMCR_CC0_MASK	0x000f0000
+#define RCAR_PWMCR_CC0_SHIFT	16
+#define RCAR_PWMCR_CCMD		BIT(15)
+#define RCAR_PWMCR_SYNC		BIT(11)
+#define RCAR_PWMCR_SS0		BIT(4)
+#define RCAR_PWMCR_EN0		BIT(0)
+
+#define RCAR_PWMCNT_CYC0_MASK	0x03ff0000
+#define RCAR_PWMCNT_CYC0_SHIFT	16
+#define RCAR_PWMCNT_PH0_MASK	0x000003ff
+#define RCAR_PWMCNT_PH0_SHIFT	0
+
+struct rcar_pwm_chip {
+	struct platform_device *pdev;
+	struct pwm_chip chip;
+
+	void __iomem *base;
+	struct clk *clk;
+};
+
+#define to_rcar_pwm_chip(chip)	container_of(chip, struct rcar_pwm_chip, chip)
+
+static u32 rcar_pwm_get_reg_offset(unsigned int channel)
+{
+	return channel * RCAR_PWM_CH_OFFSET;
+}
+
+static void rcar_pwm_write(struct rcar_pwm_chip *rp, unsigned int channel,
+			   u32 data, u32 reg)
+{
+	iowrite32(data, rp->base + rcar_pwm_get_reg_offset(channel) + reg);
+}
+
+static u32 rcar_pwm_read(struct rcar_pwm_chip *rp, unsigned int channel,
+			 u32 reg)
+{
+	return ioread32(rp->base + rcar_pwm_get_reg_offset(channel) + reg);
+}
+
+static void rcar_pwm_bit_modify(struct rcar_pwm_chip *rp, unsigned int channel,
+				u32 mask, u32 data, u32 reg)
+{
+	u32 val = rcar_pwm_read(rp, channel, reg);
+
+	val &= ~mask;
+	val |= data & mask;
+	rcar_pwm_write(rp, channel, val, reg);
+}
+
+static int rcar_pwn_get_clock_division(struct rcar_pwm_chip *rp,
+				       int period_ns)
+{
+	int div;
+	unsigned long clk_rate = clk_get_rate(rp->clk);
+	unsigned long long max;	/* max cycle / nanoseconds */
+
+	for (div = 0; div <= RCAR_PWM_MAX_DIVISION; div++) {
+		max = (unsigned long long)NSEC_PER_SEC * RCAR_PWM_MAX_CYCLE;
+		do_div(max, clk_rate / (1 << div));
+		if (period_ns < max)
+			break;
+	}
+
+	return div;
+}
+
+static void rcar_pwm_set_clock_control(struct rcar_pwm_chip *rp,
+				       int channel, int div)
+{
+	u32 val = rcar_pwm_read(rp, channel, RCAR_PWMCR);
+
+	if (div > RCAR_PWM_MAX_DIVISION)
+		return;
+
+	val &= ~(RCAR_PWMCR_CCMD | RCAR_PWMCR_CC0_MASK);
+	if (div & 1)
+		val |= RCAR_PWMCR_CCMD;
+	div >>= 1;
+	val |= div << RCAR_PWMCR_CC0_SHIFT;
+	rcar_pwm_write(rp, channel, val, RCAR_PWMCR);
+}
+
+static void rcar_pwm_set_counter(struct rcar_pwm_chip *rp, int channel,
+				 int div, int duty_ns, int period_ns)
+{
+	unsigned long long one_cycle, tmp;	/* 0.01 nanoseconds */
+	unsigned long clk_rate = clk_get_rate(rp->clk);
+	u32 cyc, ph;
+
+	one_cycle = (unsigned long long)NSEC_PER_SEC * 100;
+	do_div(one_cycle, clk_rate / (1 << div));
+
+	tmp = period_ns * 100;
+	do_div(tmp, one_cycle);
+	cyc = ((u32)tmp << RCAR_PWMCNT_CYC0_SHIFT) & RCAR_PWMCNT_CYC0_MASK;
+
+	tmp = duty_ns * 100;
+	do_div(tmp, one_cycle);
+	ph = (u32)tmp & RCAR_PWMCNT_PH0_MASK;
+
+	/* Avoid prohibited setting */
+	if (cyc && ph)
+		rcar_pwm_write(rp, channel, cyc | ph, RCAR_PWMCNT);
+}
+
+static int rcar_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	struct rcar_pwm_chip *rp = to_rcar_pwm_chip(chip);
+
+	return clk_prepare_enable(rp->clk);
+}
+
+static void rcar_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	struct rcar_pwm_chip *rp = to_rcar_pwm_chip(chip);
+
+	clk_disable_unprepare(rp->clk);
+}
+
+static int rcar_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
+			   int duty_ns, int period_ns)
+{
+	struct rcar_pwm_chip *rp = to_rcar_pwm_chip(chip);
+	int div;
+
+	div = rcar_pwn_get_clock_division(rp, period_ns);
+
+	rcar_pwm_bit_modify(rp, pwm->hwpwm, RCAR_PWMCR_SYNC, RCAR_PWMCR_SYNC,
+			    RCAR_PWMCR);
+	rcar_pwm_set_counter(rp, pwm->hwpwm, div, duty_ns, period_ns);
+	rcar_pwm_set_clock_control(rp, pwm->hwpwm, div);
+	rcar_pwm_bit_modify(rp, pwm->hwpwm, RCAR_PWMCR_SYNC, 0, RCAR_PWMCR);
+
+	return 0;
+}
+
+static int rcar_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	struct rcar_pwm_chip *rp = to_rcar_pwm_chip(chip);
+	u32 pwmcnt;
+
+	/* Don't enable the PWM device if CYC0 or PH0 is 0 */
+	pwmcnt = rcar_pwm_read(rp, pwm->hwpwm, RCAR_PWMCNT);
+	if (!(pwmcnt & RCAR_PWMCNT_CYC0_MASK) ||
+	    !(pwmcnt & RCAR_PWMCNT_PH0_MASK))
+		return -EINVAL;
+
+	rcar_pwm_bit_modify(rp, pwm->hwpwm, RCAR_PWMCR_EN0, RCAR_PWMCR_EN0,
+			    RCAR_PWMCR);
+
+	return 0;
+}
+
+static void rcar_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	struct rcar_pwm_chip *rp = to_rcar_pwm_chip(chip);
+
+	rcar_pwm_bit_modify(rp, pwm->hwpwm, RCAR_PWMCR_EN0, 0, RCAR_PWMCR);
+}
+
+static const struct pwm_ops rcar_pwm_ops = {
+	.request	= rcar_pwm_request,
+	.free		= rcar_pwm_free,
+	.config		= rcar_pwm_config,
+	.enable		= rcar_pwm_enable,
+	.disable	= rcar_pwm_disable,
+	.owner		= THIS_MODULE,
+};
+
+static int rcar_pwm_probe(struct platform_device *pdev)
+{
+	struct rcar_pwm_chip *rcar_pwm;
+	struct resource *res;
+	int ret;
+
+	rcar_pwm = devm_kzalloc(&pdev->dev, sizeof(*rcar_pwm), GFP_KERNEL);
+	if (rcar_pwm = NULL)
+		return -ENOMEM;
+
+	rcar_pwm->pdev = pdev;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	rcar_pwm->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(rcar_pwm->base))
+		return PTR_ERR(rcar_pwm->base);
+
+	rcar_pwm->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(rcar_pwm->clk)) {
+		dev_err(&pdev->dev, "cannot get clock\n");
+		return PTR_ERR(rcar_pwm->clk);
+	}
+
+	platform_set_drvdata(pdev, rcar_pwm);
+
+	rcar_pwm->chip.dev = &pdev->dev;
+	rcar_pwm->chip.ops = &rcar_pwm_ops;
+	rcar_pwm->chip.of_xlate = of_pwm_xlate_with_flags;
+	rcar_pwm->chip.base = -1;
+	rcar_pwm->chip.npwm = NUM_RCAR_PWM_CHANNELS;
+
+	ret = pwmchip_add(&rcar_pwm->chip);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to register PWM chip\n");
+		return ret;
+	}
+
+	dev_info(&pdev->dev, "R-Car PWM Timer registered\n");
+
+	pm_runtime_enable(&pdev->dev);
+
+	return 0;
+}
+
+static int rcar_pwm_remove(struct platform_device *pdev)
+{
+	struct rcar_pwm_chip *rcar_pwm = platform_get_drvdata(pdev);
+	int ret;
+
+	ret = pwmchip_remove(&rcar_pwm->chip);
+	if (ret)
+		return ret;
+
+	pm_runtime_disable(&pdev->dev);
+
+	return 0;
+}
+
+static const struct of_device_id rcar_pwm_of_table[] = {
+	{ .compatible = "renesas,pwm-rcar", },
+	{ },
+};
+
+MODULE_DEVICE_TABLE(of, rcar_pwm_of_table);
+
+static struct platform_driver rcar_pwm_driver = {
+	.probe		= rcar_pwm_probe,
+	.remove		= rcar_pwm_remove,
+	.driver		= {
+		.name	= "pwm-rcar",
+		.of_match_table = of_match_ptr(rcar_pwm_of_table),
+	}
+};
+module_platform_driver(rcar_pwm_driver);
+
+MODULE_AUTHOR("Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>");
+MODULE_DESCRIPTION("Renesas PWM Timer Driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:pwm-rcar");
-- 
1.9.1

[PATCH 2/2] pwm: Add support for R-Car PWM Timer

[Yoshihiro Shimoda]

R-Car SoCs have a seven-channel pulse width modulation (PWM) timer.
This driver adds support for the PWM Timer as a single PWM chip and
seven PWM devices.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 drivers/pwm/Kconfig    |  11 ++
 drivers/pwm/Makefile   |   1 +
 drivers/pwm/pwm-rcar.c | 276 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 288 insertions(+)
 create mode 100644 drivers/pwm/pwm-rcar.c

diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index b1541f4..7e98175 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -249,6 +249,17 @@ config PWM_PXA
 	  To compile this driver as a module, choose M here: the module
 	  will be called pwm-pxa.
 
+config PWM_RCAR
+	tristate "Renesas R-Car PWM support"
+	depends on ARCH_SHMOBILE || COMPILE_TEST
+	depends on HAS_IOMEM
+	help
+	  This driver exposes the PWM Timer controller found in Renesas
+	  chips through the PWM API.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called pwm-rcar.
+
 config PWM_RENESAS_TPU
 	tristate "Renesas TPU PWM support"
 	depends on ARCH_SHMOBILE || COMPILE_TEST
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index ec50eb5..79d3dc3 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -22,6 +22,7 @@ obj-$(CONFIG_PWM_MXS)		+= pwm-mxs.o
 obj-$(CONFIG_PWM_PCA9685)	+= pwm-pca9685.o
 obj-$(CONFIG_PWM_PUV3)		+= pwm-puv3.o
 obj-$(CONFIG_PWM_PXA)		+= pwm-pxa.o
+obj-$(CONFIG_PWM_RCAR)		+= pwm-rcar.o
 obj-$(CONFIG_PWM_RENESAS_TPU)	+= pwm-renesas-tpu.o
 obj-$(CONFIG_PWM_ROCKCHIP)	+= pwm-rockchip.o
 obj-$(CONFIG_PWM_SAMSUNG)	+= pwm-samsung.o
diff --git a/drivers/pwm/pwm-rcar.c b/drivers/pwm/pwm-rcar.c
new file mode 100644
index 0000000..29011e8
--- /dev/null
+++ b/drivers/pwm/pwm-rcar.c
@@ -0,0 +1,276 @@
+/*
+ * R-Car PWM Timer driver
+ *
+ * Copyright (C) 2015 Renesas Electronics Corporation
+ *
+ * This is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/pwm.h>
+#include <linux/slab.h>
+
+#define NUM_RCAR_PWM_CHANNELS	7
+#define RCAR_PWM_MAX_DIVISION	24
+#define RCAR_PWM_MAX_CYCLE	1023
+
+#define RCAR_PWMCR		0x00
+#define RCAR_PWMCNT		0x04
+#define RCAR_PWM_CH_OFFSET	0x1000
+
+#define RCAR_PWMCR_CC0_MASK	0x000f0000
+#define RCAR_PWMCR_CC0_SHIFT	16
+#define RCAR_PWMCR_CCMD		BIT(15)
+#define RCAR_PWMCR_SYNC		BIT(11)
+#define RCAR_PWMCR_SS0		BIT(4)
+#define RCAR_PWMCR_EN0		BIT(0)
+
+#define RCAR_PWMCNT_CYC0_MASK	0x03ff0000
+#define RCAR_PWMCNT_CYC0_SHIFT	16
+#define RCAR_PWMCNT_PH0_MASK	0x000003ff
+#define RCAR_PWMCNT_PH0_SHIFT	0
+
+struct rcar_pwm_chip {
+	struct platform_device *pdev;
+	struct pwm_chip chip;
+
+	void __iomem *base;
+	struct clk *clk;
+};
+
+#define to_rcar_pwm_chip(chip)	container_of(chip, struct rcar_pwm_chip, chip)
+
+static u32 rcar_pwm_get_reg_offset(unsigned int channel)
+{
+	return channel * RCAR_PWM_CH_OFFSET;
+}
+
+static void rcar_pwm_write(struct rcar_pwm_chip *rp, unsigned int channel,
+			   u32 data, u32 reg)
+{
+	iowrite32(data, rp->base + rcar_pwm_get_reg_offset(channel) + reg);
+}
+
+static u32 rcar_pwm_read(struct rcar_pwm_chip *rp, unsigned int channel,
+			 u32 reg)
+{
+	return ioread32(rp->base + rcar_pwm_get_reg_offset(channel) + reg);
+}
+
+static void rcar_pwm_bit_modify(struct rcar_pwm_chip *rp, unsigned int channel,
+				u32 mask, u32 data, u32 reg)
+{
+	u32 val = rcar_pwm_read(rp, channel, reg);
+
+	val &= ~mask;
+	val |= data & mask;
+	rcar_pwm_write(rp, channel, val, reg);
+}
+
+static int rcar_pwn_get_clock_division(struct rcar_pwm_chip *rp,
+				       int period_ns)
+{
+	int div;
+	unsigned long clk_rate = clk_get_rate(rp->clk);
+	unsigned long long max;	/* max cycle / nanoseconds */
+
+	for (div = 0; div <= RCAR_PWM_MAX_DIVISION; div++) {
+		max = (unsigned long long)NSEC_PER_SEC * RCAR_PWM_MAX_CYCLE;
+		do_div(max, clk_rate / (1 << div));
+		if (period_ns < max)
+			break;
+	}
+
+	return div;
+}
+
+static void rcar_pwm_set_clock_control(struct rcar_pwm_chip *rp,
+				       int channel, int div)
+{
+	u32 val = rcar_pwm_read(rp, channel, RCAR_PWMCR);
+
+	if (div > RCAR_PWM_MAX_DIVISION)
+		return;
+
+	val &= ~(RCAR_PWMCR_CCMD | RCAR_PWMCR_CC0_MASK);
+	if (div & 1)
+		val |= RCAR_PWMCR_CCMD;
+	div >>= 1;
+	val |= div << RCAR_PWMCR_CC0_SHIFT;
+	rcar_pwm_write(rp, channel, val, RCAR_PWMCR);
+}
+
+static void rcar_pwm_set_counter(struct rcar_pwm_chip *rp, int channel,
+				 int div, int duty_ns, int period_ns)
+{
+	unsigned long long one_cycle, tmp;	/* 0.01 nanoseconds */
+	unsigned long clk_rate = clk_get_rate(rp->clk);
+	u32 cyc, ph;
+
+	one_cycle = (unsigned long long)NSEC_PER_SEC * 100;
+	do_div(one_cycle, clk_rate / (1 << div));
+
+	tmp = period_ns * 100;
+	do_div(tmp, one_cycle);
+	cyc = ((u32)tmp << RCAR_PWMCNT_CYC0_SHIFT) & RCAR_PWMCNT_CYC0_MASK;
+
+	tmp = duty_ns * 100;
+	do_div(tmp, one_cycle);
+	ph = (u32)tmp & RCAR_PWMCNT_PH0_MASK;
+
+	/* Avoid prohibited setting */
+	if (cyc && ph)
+		rcar_pwm_write(rp, channel, cyc | ph, RCAR_PWMCNT);
+}
+
+static int rcar_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	struct rcar_pwm_chip *rp = to_rcar_pwm_chip(chip);
+
+	return clk_prepare_enable(rp->clk);
+}
+
+static void rcar_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	struct rcar_pwm_chip *rp = to_rcar_pwm_chip(chip);
+
+	clk_disable_unprepare(rp->clk);
+}
+
+static int rcar_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
+			   int duty_ns, int period_ns)
+{
+	struct rcar_pwm_chip *rp = to_rcar_pwm_chip(chip);
+	int div;
+
+	div = rcar_pwn_get_clock_division(rp, period_ns);
+
+	rcar_pwm_bit_modify(rp, pwm->hwpwm, RCAR_PWMCR_SYNC, RCAR_PWMCR_SYNC,
+			    RCAR_PWMCR);
+	rcar_pwm_set_counter(rp, pwm->hwpwm, div, duty_ns, period_ns);
+	rcar_pwm_set_clock_control(rp, pwm->hwpwm, div);
+	rcar_pwm_bit_modify(rp, pwm->hwpwm, RCAR_PWMCR_SYNC, 0, RCAR_PWMCR);
+
+	return 0;
+}
+
+static int rcar_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	struct rcar_pwm_chip *rp = to_rcar_pwm_chip(chip);
+	u32 pwmcnt;
+
+	/* Don't enable the PWM device if CYC0 or PH0 is 0 */
+	pwmcnt = rcar_pwm_read(rp, pwm->hwpwm, RCAR_PWMCNT);
+	if (!(pwmcnt & RCAR_PWMCNT_CYC0_MASK) ||
+	    !(pwmcnt & RCAR_PWMCNT_PH0_MASK))
+		return -EINVAL;
+
+	rcar_pwm_bit_modify(rp, pwm->hwpwm, RCAR_PWMCR_EN0, RCAR_PWMCR_EN0,
+			    RCAR_PWMCR);
+
+	return 0;
+}
+
+static void rcar_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+	struct rcar_pwm_chip *rp = to_rcar_pwm_chip(chip);
+
+	rcar_pwm_bit_modify(rp, pwm->hwpwm, RCAR_PWMCR_EN0, 0, RCAR_PWMCR);
+}
+
+static const struct pwm_ops rcar_pwm_ops = {
+	.request	= rcar_pwm_request,
+	.free		= rcar_pwm_free,
+	.config		= rcar_pwm_config,
+	.enable		= rcar_pwm_enable,
+	.disable	= rcar_pwm_disable,
+	.owner		= THIS_MODULE,
+};
+
+static int rcar_pwm_probe(struct platform_device *pdev)
+{
+	struct rcar_pwm_chip *rcar_pwm;
+	struct resource *res;
+	int ret;
+
+	rcar_pwm = devm_kzalloc(&pdev->dev, sizeof(*rcar_pwm), GFP_KERNEL);
+	if (rcar_pwm == NULL)
+		return -ENOMEM;
+
+	rcar_pwm->pdev = pdev;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	rcar_pwm->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(rcar_pwm->base))
+		return PTR_ERR(rcar_pwm->base);
+
+	rcar_pwm->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(rcar_pwm->clk)) {
+		dev_err(&pdev->dev, "cannot get clock\n");
+		return PTR_ERR(rcar_pwm->clk);
+	}
+
+	platform_set_drvdata(pdev, rcar_pwm);
+
+	rcar_pwm->chip.dev = &pdev->dev;
+	rcar_pwm->chip.ops = &rcar_pwm_ops;
+	rcar_pwm->chip.of_xlate = of_pwm_xlate_with_flags;
+	rcar_pwm->chip.base = -1;
+	rcar_pwm->chip.npwm = NUM_RCAR_PWM_CHANNELS;
+
+	ret = pwmchip_add(&rcar_pwm->chip);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to register PWM chip\n");
+		return ret;
+	}
+
+	dev_info(&pdev->dev, "R-Car PWM Timer registered\n");
+
+	pm_runtime_enable(&pdev->dev);
+
+	return 0;
+}
+
+static int rcar_pwm_remove(struct platform_device *pdev)
+{
+	struct rcar_pwm_chip *rcar_pwm = platform_get_drvdata(pdev);
+	int ret;
+
+	ret = pwmchip_remove(&rcar_pwm->chip);
+	if (ret)
+		return ret;
+
+	pm_runtime_disable(&pdev->dev);
+
+	return 0;
+}
+
+static const struct of_device_id rcar_pwm_of_table[] = {
+	{ .compatible = "renesas,pwm-rcar", },
+	{ },
+};
+
+MODULE_DEVICE_TABLE(of, rcar_pwm_of_table);
+
+static struct platform_driver rcar_pwm_driver = {
+	.probe		= rcar_pwm_probe,
+	.remove		= rcar_pwm_remove,
+	.driver		= {
+		.name	= "pwm-rcar",
+		.of_match_table = of_match_ptr(rcar_pwm_of_table),
+	}
+};
+module_platform_driver(rcar_pwm_driver);
+
+MODULE_AUTHOR("Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>");
+MODULE_DESCRIPTION("Renesas PWM Timer Driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:pwm-rcar");
-- 
1.9.1

Re: [PATCH 1/2] pwm: Add device tree binding document for R-Car PWM Timer

[Geert Uytterhoeven]

Hi Shimoda-san,

On Wed, May 13, 2015 at 11:27 AM, Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> Add binding document for Renesas PWM Timer on R-Car SoCs.

Thanks for your patch!

> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
>  .../devicetree/bindings/pwm/renesas,pwm-rcar.txt    | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.txt
>
> diff --git a/Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.txt b/Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.txt
> new file mode 100644
> index 0000000..38123f9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.txt
> @@ -0,0 +1,21 @@
> +* Renesas R-Car PWM Timer Controller
> +
> +Required Properties:
> +- compatible: must contain "renesas,pwm-rcar"

Can you please add SoC-specific compatible values, too? That way we can handle
 SoC-specific differences if we ever encounter them.
If I'm not mistaken, this PWM IP core is present on both R-Car Gen1 and Gen2
SoCs?

> +- reg: base address and length of the registers block for the PWM
> +- #pwm-cells: should be 7. See pwm.txt in this directory for a description of
> +  the cells format.

Why 7? This is not the number of channels, cfr. pwm.txt you reference above:

    "pwm-specifier typically encodes the chip-relative PWM number and the PWM
     period in nanoseconds.

     Optionally, the pwm-specifier can encode a number of flags (defined in
     <dt-bindings/pwm/pwm.h>) in a third cell:
     - PWM_POLARITY_INVERTED: invert the PWM signal polarity"

So 2 or 3 makes more sense to me.

> +- clocks: clock phandle and specifier pair.
> +- pinctrl-0: phandle, referring to a default pin configuration node.
> +- pinctrl-names: Set to "default"
> +
> +Example: R8A7790 (R-Car H2) PWM Timer node
> +
> +       pwm: pwm@e6e30000 {
> +               compatible = "renesas,pwm-rcar";
> +               reg = <0 0xe6e30000 0 0x7000>;
> +               #pwm-cells = <7>;
> +               clocks = <&mstp5_clks R8A7790_CLK_PWM>;
> +               pinctrl-0 = <&pwm3_pins>;
> +               pinctrl-names = "default";
> +       };

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/2] pwm: Add device tree binding document for R-Car PWM Timer

[Geert Uytterhoeven]

Hi Shimoda-san,

On Wed, May 13, 2015 at 11:27 AM, Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> Add binding document for Renesas PWM Timer on R-Car SoCs.

Thanks for your patch!

> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
>  .../devicetree/bindings/pwm/renesas,pwm-rcar.txt    | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.txt
>
> diff --git a/Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.txt b/Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.txt
> new file mode 100644
> index 0000000..38123f9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.txt
> @@ -0,0 +1,21 @@
> +* Renesas R-Car PWM Timer Controller
> +
> +Required Properties:
> +- compatible: must contain "renesas,pwm-rcar"

Can you please add SoC-specific compatible values, too? That way we can handle
 SoC-specific differences if we ever encounter them.
If I'm not mistaken, this PWM IP core is present on both R-Car Gen1 and Gen2
SoCs?

> +- reg: base address and length of the registers block for the PWM
> +- #pwm-cells: should be 7. See pwm.txt in this directory for a description of
> +  the cells format.

Why 7? This is not the number of channels, cfr. pwm.txt you reference above:

    "pwm-specifier typically encodes the chip-relative PWM number and the PWM
     period in nanoseconds.

     Optionally, the pwm-specifier can encode a number of flags (defined in
     <dt-bindings/pwm/pwm.h>) in a third cell:
     - PWM_POLARITY_INVERTED: invert the PWM signal polarity"

So 2 or 3 makes more sense to me.

> +- clocks: clock phandle and specifier pair.
> +- pinctrl-0: phandle, referring to a default pin configuration node.
> +- pinctrl-names: Set to "default"
> +
> +Example: R8A7790 (R-Car H2) PWM Timer node
> +
> +       pwm: pwm@e6e30000 {
> +               compatible = "renesas,pwm-rcar";
> +               reg = <0 0xe6e30000 0 0x7000>;
> +               #pwm-cells = <7>;
> +               clocks = <&mstp5_clks R8A7790_CLK_PWM>;
> +               pinctrl-0 = <&pwm3_pins>;
> +               pinctrl-names = "default";
> +       };

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/2] pwm: Add support for R-Car PWM Timer

[Geert Uytterhoeven]

Hi Shimoda-san,

On Wed, May 13, 2015 at 11:27 AM, Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> R-Car SoCs have a seven-channel pulse width modulation (PWM) timer.
> This driver adds support for the PWM Timer as a single PWM chip and
> seven PWM devices.

Thanks for your patch!

> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> index b1541f4..7e98175 100644
> --- a/drivers/pwm/Kconfig
> +++ b/drivers/pwm/Kconfig
> @@ -249,6 +249,17 @@ config PWM_PXA
>           To compile this driver as a module, choose M here: the module
>           will be called pwm-pxa.
>
> +config PWM_RCAR
> +       tristate "Renesas R-Car PWM support"
> +       depends on ARCH_SHMOBILE || COMPILE_TEST

Could be instead

depends on ARCH_RCAR_GEN1 ||  ARCH_RCAR_GEN2 || COMPILE_TEST

> +       depends on HAS_IOMEM
> +       help
> +         This driver exposes the PWM Timer controller found in Renesas
> +         chips through the PWM API.

"Renesas R-Car chips" (e.g. R-Mobile uses pwm-renesas-tpu).

> --- /dev/null
> +++ b/drivers/pwm/pwm-rcar.c
> @@ -0,0 +1,276 @@
> +/*
> + * R-Car PWM Timer driver
> + *
> + * Copyright (C) 2015 Renesas Electronics Corporation
> + *
> + * This is free software; you can redistribute it and/or modify
> + * it under the terms of version 2 of the GNU General Public License as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/pwm.h>
> +#include <linux/slab.h>
> +
> +#define NUM_RCAR_PWM_CHANNELS  7

I'm wondering whether this should be a DT property, to prepare for
future expansion.
Alternatively, as these are really 7 individual channels, with 7 individual
register blocks spaced 0x1000 apart, you could have 7 individual device nodes,
each driving one pwm output. That would probably incur more overhead, as
there would be 7 individual pwm controllers. But it does allow for future
expansion, and allows to cope when the individual register blocks are moved
in the SoC address space (I can imagine that on old SoCs e.g. all SCIF
blocks were nicely lay out in the address space, while now they have arbitrary
base addresses).

What do other people think?

> +static int rcar_pwn_get_clock_division(struct rcar_pwm_chip *rp,
> +                                      int period_ns)
> +{
> +       int div;
> +       unsigned long clk_rate = clk_get_rate(rp->clk);
> +       unsigned long long max; /* max cycle / nanoseconds */
> +
> +       for (div = 0; div <= RCAR_PWM_MAX_DIVISION; div++) {
> +               max = (unsigned long long)NSEC_PER_SEC * RCAR_PWM_MAX_CYCLE;
> +               do_div(max, clk_rate / (1 << div));

Dividing clk_rate by "1 << div" reduces accuracy a lot for large values of div.
"NSEC_PER_SEC * RCAR_PWM_MAX_CYCLE * (1 << div)" just fits in 64-bit,
so you can use

        max = (unsigned long long)NSEC_PER_SEC * RCAR_PWM_MAX_CYCLE *
              (1 << div);
        do_div(max, clk_rate);

> +static void rcar_pwm_set_counter(struct rcar_pwm_chip *rp, int channel,
> +                                int div, int duty_ns, int period_ns)
> +{
> +       unsigned long long one_cycle, tmp;      /* 0.01 nanoseconds */
> +       unsigned long clk_rate = clk_get_rate(rp->clk);
> +       u32 cyc, ph;
> +
> +       one_cycle = (unsigned long long)NSEC_PER_SEC * 100;
> +       do_div(one_cycle, clk_rate / (1 << div));

Same here:

        one_cycle = (unsigned long long)NSEC_PER_SEC * 100 * (1 << div)
        do_div(one_cycle, clk_rate);

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/2] pwm: Add support for R-Car PWM Timer

[Geert Uytterhoeven]

Hi Shimoda-san,

On Wed, May 13, 2015 at 11:27 AM, Yoshihiro Shimoda
<yoshihiro.shimoda.uh@renesas.com> wrote:
> R-Car SoCs have a seven-channel pulse width modulation (PWM) timer.
> This driver adds support for the PWM Timer as a single PWM chip and
> seven PWM devices.

Thanks for your patch!

> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> index b1541f4..7e98175 100644
> --- a/drivers/pwm/Kconfig
> +++ b/drivers/pwm/Kconfig
> @@ -249,6 +249,17 @@ config PWM_PXA
>           To compile this driver as a module, choose M here: the module
>           will be called pwm-pxa.
>
> +config PWM_RCAR
> +       tristate "Renesas R-Car PWM support"
> +       depends on ARCH_SHMOBILE || COMPILE_TEST

Could be instead

depends on ARCH_RCAR_GEN1 ||  ARCH_RCAR_GEN2 || COMPILE_TEST

> +       depends on HAS_IOMEM
> +       help
> +         This driver exposes the PWM Timer controller found in Renesas
> +         chips through the PWM API.

"Renesas R-Car chips" (e.g. R-Mobile uses pwm-renesas-tpu).

> --- /dev/null
> +++ b/drivers/pwm/pwm-rcar.c
> @@ -0,0 +1,276 @@
> +/*
> + * R-Car PWM Timer driver
> + *
> + * Copyright (C) 2015 Renesas Electronics Corporation
> + *
> + * This is free software; you can redistribute it and/or modify
> + * it under the terms of version 2 of the GNU General Public License as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/pwm.h>
> +#include <linux/slab.h>
> +
> +#define NUM_RCAR_PWM_CHANNELS  7

I'm wondering whether this should be a DT property, to prepare for
future expansion.
Alternatively, as these are really 7 individual channels, with 7 individual
register blocks spaced 0x1000 apart, you could have 7 individual device nodes,
each driving one pwm output. That would probably incur more overhead, as
there would be 7 individual pwm controllers. But it does allow for future
expansion, and allows to cope when the individual register blocks are moved
in the SoC address space (I can imagine that on old SoCs e.g. all SCIF
blocks were nicely lay out in the address space, while now they have arbitrary
base addresses).

What do other people think?

> +static int rcar_pwn_get_clock_division(struct rcar_pwm_chip *rp,
> +                                      int period_ns)
> +{
> +       int div;
> +       unsigned long clk_rate = clk_get_rate(rp->clk);
> +       unsigned long long max; /* max cycle / nanoseconds */
> +
> +       for (div = 0; div <= RCAR_PWM_MAX_DIVISION; div++) {
> +               max = (unsigned long long)NSEC_PER_SEC * RCAR_PWM_MAX_CYCLE;
> +               do_div(max, clk_rate / (1 << div));

Dividing clk_rate by "1 << div" reduces accuracy a lot for large values of div.
"NSEC_PER_SEC * RCAR_PWM_MAX_CYCLE * (1 << div)" just fits in 64-bit,
so you can use

        max = (unsigned long long)NSEC_PER_SEC * RCAR_PWM_MAX_CYCLE *
              (1 << div);
        do_div(max, clk_rate);

> +static void rcar_pwm_set_counter(struct rcar_pwm_chip *rp, int channel,
> +                                int div, int duty_ns, int period_ns)
> +{
> +       unsigned long long one_cycle, tmp;      /* 0.01 nanoseconds */
> +       unsigned long clk_rate = clk_get_rate(rp->clk);
> +       u32 cyc, ph;
> +
> +       one_cycle = (unsigned long long)NSEC_PER_SEC * 100;
> +       do_div(one_cycle, clk_rate / (1 << div));

Same here:

        one_cycle = (unsigned long long)NSEC_PER_SEC * 100 * (1 << div)
        do_div(one_cycle, clk_rate);

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH 1/2] pwm: Add device tree binding document for R-Car PWM Timer

[Yoshihiro Shimoda]

SGkgR2VlcnQtc2FuLA0KDQpUaGFuayB5b3UgZm9yIHRoZSByZXZpZXchDQoNCj4gU2VudDogRnJp
ZGF5LCBNYXkgMTUsIDIwMTUgMTI6NDggQU0NCj4gDQo+IEhpIFNoaW1vZGEtc2FuLA0KPiANCj4g
T24gV2VkLCBNYXkgMTMsIDIwMTUgYXQgMTE6MjcgQU0sIFlvc2hpaGlybyBTaGltb2RhDQo+IDx5
b3NoaWhpcm8uc2hpbW9kYS51aEByZW5lc2FzLmNvbT4gd3JvdGU6DQo+ID4gQWRkIGJpbmRpbmcg
ZG9jdW1lbnQgZm9yIFJlbmVzYXMgUFdNIFRpbWVyIG9uIFItQ2FyIFNvQ3MuDQo+IA0KPiBUaGFu
a3MgZm9yIHlvdXIgcGF0Y2ghDQo+IA0KPiA+IFNpZ25lZC1vZmYtYnk6IFlvc2hpaGlybyBTaGlt
b2RhIDx5b3NoaWhpcm8uc2hpbW9kYS51aEByZW5lc2FzLmNvbT4NCj4gPiAtLS0NCj4gPiAgLi4u
L2RldmljZXRyZWUvYmluZGluZ3MvcHdtL3JlbmVzYXMscHdtLXJjYXIudHh0ICAgIHwgMjEgKysr
KysrKysrKysrKysrKysrKysrDQo+ID4gIDEgZmlsZSBjaGFuZ2VkLCAyMSBpbnNlcnRpb25zKCsp
DQo+ID4gIGNyZWF0ZSBtb2RlIDEwMDY0NCBEb2N1bWVudGF0aW9uL2RldmljZXRyZWUvYmluZGlu
Z3MvcHdtL3JlbmVzYXMscHdtLXJjYXIudHh0DQo+ID4NCj4gPiBkaWZmIC0tZ2l0IGEvRG9jdW1l
bnRhdGlvbi9kZXZpY2V0cmVlL2JpbmRpbmdzL3B3bS9yZW5lc2FzLHB3bS1yY2FyLnR4dA0KPiBi
L0RvY3VtZW50YXRpb24vZGV2aWNldHJlZS9iaW5kaW5ncy9wd20vcmVuZXNhcyxwd20tcmNhci50
eHQNCj4gPiBuZXcgZmlsZSBtb2RlIDEwMDY0NA0KPiA+IGluZGV4IDAwMDAwMDAuLjM4MTIzZjkN
Cj4gPiAtLS0gL2Rldi9udWxsDQo+ID4gKysrIGIvRG9jdW1lbnRhdGlvbi9kZXZpY2V0cmVlL2Jp
bmRpbmdzL3B3bS9yZW5lc2FzLHB3bS1yY2FyLnR4dA0KPiA+IEBAIC0wLDAgKzEsMjEgQEANCj4g
PiArKiBSZW5lc2FzIFItQ2FyIFBXTSBUaW1lciBDb250cm9sbGVyDQo+ID4gKw0KPiA+ICtSZXF1
aXJlZCBQcm9wZXJ0aWVzOg0KPiA+ICstIGNvbXBhdGlibGU6IG11c3QgY29udGFpbiAicmVuZXNh
cyxwd20tcmNhciINCj4gDQo+IENhbiB5b3UgcGxlYXNlIGFkZCBTb0Mtc3BlY2lmaWMgY29tcGF0
aWJsZSB2YWx1ZXMsIHRvbz8gVGhhdCB3YXkgd2UgY2FuIGhhbmRsZQ0KPiAgU29DLXNwZWNpZmlj
IGRpZmZlcmVuY2VzIGlmIHdlIGV2ZXIgZW5jb3VudGVyIHRoZW0uDQo+IElmIEknbSBub3QgbWlz
dGFrZW4sIHRoaXMgUFdNIElQIGNvcmUgaXMgcHJlc2VudCBvbiBib3RoIFItQ2FyIEdlbjEgYW5k
IEdlbjINCj4gU29Dcz8NCg0KQWNjb3JkaW5nIHRvIHRoZSBtYW51YWxzIG9mIGJvdGggUi1DYXIg
R2VuMSAoSDEsIE0xLCBFMSkgYW5kIEdlbjIsDQp0aGlzIFBXTSBJUCBjb3JlIGlzIHByZXNlbnQg
b24gdGhlbS4gQWxzbyB0aGlzIElQIGlzIHByZXNlbnQgb24gUi1DYXIgbmV4dCBnZW5lcmF0aW9u
Lg0KDQpPZiBjb3Vyc2UsIEkgY2FuIGFkZCBTb0Mtc3BlY2lmaWMgY29tcGF0aWJsZSB2YWx1ZXMs
IGxpa2UgInJlbmVzYXMscHdtLXI3YTc3OTAiIG9yDQpzb21ldGhpbmcuIFNvLCBJIHdpbGwgY2hh
bmdlIHRoZSBjb21wYXRpYmxlIHZhbHVlcyBpbiB2MiBwYXRjaC4NCg0KPiA+ICstIHJlZzogYmFz
ZSBhZGRyZXNzIGFuZCBsZW5ndGggb2YgdGhlIHJlZ2lzdGVycyBibG9jayBmb3IgdGhlIFBXTQ0K
PiA+ICstICNwd20tY2VsbHM6IHNob3VsZCBiZSA3LiBTZWUgcHdtLnR4dCBpbiB0aGlzIGRpcmVj
dG9yeSBmb3IgYSBkZXNjcmlwdGlvbiBvZg0KPiA+ICsgIHRoZSBjZWxscyBmb3JtYXQuDQo+IA0K
PiBXaHkgNz8gVGhpcyBpcyBub3QgdGhlIG51bWJlciBvZiBjaGFubmVscywgY2ZyLiBwd20udHh0
IHlvdSByZWZlcmVuY2UgYWJvdmU6DQo+IA0KPiAgICAgInB3bS1zcGVjaWZpZXIgdHlwaWNhbGx5
IGVuY29kZXMgdGhlIGNoaXAtcmVsYXRpdmUgUFdNIG51bWJlciBhbmQgdGhlIFBXTQ0KPiAgICAg
IHBlcmlvZCBpbiBuYW5vc2Vjb25kcy4NCj4gDQo+ICAgICAgT3B0aW9uYWxseSwgdGhlIHB3bS1z
cGVjaWZpZXIgY2FuIGVuY29kZSBhIG51bWJlciBvZiBmbGFncyAoZGVmaW5lZCBpbg0KPiAgICAg
IDxkdC1iaW5kaW5ncy9wd20vcHdtLmg+KSBpbiBhIHRoaXJkIGNlbGw6DQo+ICAgICAgLSBQV01f
UE9MQVJJVFlfSU5WRVJURUQ6IGludmVydCB0aGUgUFdNIHNpZ25hbCBwb2xhcml0eSINCj4gDQo+
IFNvIDIgb3IgMyBtYWtlcyBtb3JlIHNlbnNlIHRvIG1lLg0KDQpUaGFuayB5b3UgdmVyeSBtdWNo
IGZvciB0aGUgcG9pbnQuIEkgY29tcGxldGVseSBtaXN1bmRlcnN0b29kIHRoaXMgc3BlY2lmaWNh
dGlvbi4NClNpbmNlIHRoaXMgUFdNIElQIGNhbm5vdCBjb250cm9sIHBvbGFyaXR5LCB0aGlzIHZh
bHVlIHNob3VsZCBiZSAyLg0KU28sIEkgd2lsbCBmaXggdGhpcyBpbiB2MiBwYXRjaC4NCg0KQmVz
dCByZWdhcmRzLA0KWW9zaGloaXJvIFNoaW1vZGENCg0K

RE: [PATCH 1/2] pwm: Add device tree binding document for R-Car PWM Timer

[Yoshihiro Shimoda]

Hi Geert-san,

Thank you for the review!

> Sent: Friday, May 15, 2015 12:48 AM
> 
> Hi Shimoda-san,
> 
> On Wed, May 13, 2015 at 11:27 AM, Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com> wrote:
> > Add binding document for Renesas PWM Timer on R-Car SoCs.
> 
> Thanks for your patch!
> 
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > ---
> >  .../devicetree/bindings/pwm/renesas,pwm-rcar.txt    | 21 +++++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.txt
> >
> > diff --git a/Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.txt
> b/Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.txt
> > new file mode 100644
> > index 0000000..38123f9
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.txt
> > @@ -0,0 +1,21 @@
> > +* Renesas R-Car PWM Timer Controller
> > +
> > +Required Properties:
> > +- compatible: must contain "renesas,pwm-rcar"
> 
> Can you please add SoC-specific compatible values, too? That way we can handle
>  SoC-specific differences if we ever encounter them.
> If I'm not mistaken, this PWM IP core is present on both R-Car Gen1 and Gen2
> SoCs?

According to the manuals of both R-Car Gen1 (H1, M1, E1) and Gen2,
this PWM IP core is present on them. Also this IP is present on R-Car next generation.

Of course, I can add SoC-specific compatible values, like "renesas,pwm-r7a7790" or
something. So, I will change the compatible values in v2 patch.

> > +- reg: base address and length of the registers block for the PWM
> > +- #pwm-cells: should be 7. See pwm.txt in this directory for a description of
> > +  the cells format.
> 
> Why 7? This is not the number of channels, cfr. pwm.txt you reference above:
> 
>     "pwm-specifier typically encodes the chip-relative PWM number and the PWM
>      period in nanoseconds.
> 
>      Optionally, the pwm-specifier can encode a number of flags (defined in
>      <dt-bindings/pwm/pwm.h>) in a third cell:
>      - PWM_POLARITY_INVERTED: invert the PWM signal polarity"
> 
> So 2 or 3 makes more sense to me.

Thank you very much for the point. I completely misunderstood this specification.
Since this PWM IP cannot control polarity, this value should be 2.
So, I will fix this in v2 patch.

Best regards,
Yoshihiro Shimoda

RE: [PATCH 2/2] pwm: Add support for R-Car PWM Timer

[Yoshihiro Shimoda]

SGkgR2VlcnQtc2FuLA0KDQpUaGFuayB5b3UgZm9yIHlvdXIgcmV2aWV3IQ0KDQo+IFNlbnQ6IEZy
aWRheSwgTWF5IDE1LCAyMDE1IDE6MDcgQU0NCj4gDQo+IEhpIFNoaW1vZGEtc2FuLA0KPiANCj4g
T24gV2VkLCBNYXkgMTMsIDIwMTUgYXQgMTE6MjcgQU0sIFlvc2hpaGlybyBTaGltb2RhDQo+IDx5
b3NoaWhpcm8uc2hpbW9kYS51aEByZW5lc2FzLmNvbT4gd3JvdGU6DQo+ID4gUi1DYXIgU29DcyBo
YXZlIGEgc2V2ZW4tY2hhbm5lbCBwdWxzZSB3aWR0aCBtb2R1bGF0aW9uIChQV00pIHRpbWVyLg0K
PiA+IFRoaXMgZHJpdmVyIGFkZHMgc3VwcG9ydCBmb3IgdGhlIFBXTSBUaW1lciBhcyBhIHNpbmds
ZSBQV00gY2hpcCBhbmQNCj4gPiBzZXZlbiBQV00gZGV2aWNlcy4NCj4gDQo+IFRoYW5rcyBmb3Ig
eW91ciBwYXRjaCENCj4gDQo+ID4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvcHdtL0tjb25maWcgYi9k
cml2ZXJzL3B3bS9LY29uZmlnDQo+ID4gaW5kZXggYjE1NDFmNC4uN2U5ODE3NSAxMDA2NDQNCj4g
PiAtLS0gYS9kcml2ZXJzL3B3bS9LY29uZmlnDQo+ID4gKysrIGIvZHJpdmVycy9wd20vS2NvbmZp
Zw0KPiA+IEBAIC0yNDksNiArMjQ5LDE3IEBAIGNvbmZpZyBQV01fUFhBDQo+ID4gICAgICAgICAg
IFRvIGNvbXBpbGUgdGhpcyBkcml2ZXIgYXMgYSBtb2R1bGUsIGNob29zZSBNIGhlcmU6IHRoZSBt
b2R1bGUNCj4gPiAgICAgICAgICAgd2lsbCBiZSBjYWxsZWQgcHdtLXB4YS4NCj4gPg0KPiA+ICtj
b25maWcgUFdNX1JDQVINCj4gPiArICAgICAgIHRyaXN0YXRlICJSZW5lc2FzIFItQ2FyIFBXTSBz
dXBwb3J0Ig0KPiA+ICsgICAgICAgZGVwZW5kcyBvbiBBUkNIX1NITU9CSUxFIHx8IENPTVBJTEVf
VEVTVA0KPiANCj4gQ291bGQgYmUgaW5zdGVhZA0KPiANCj4gZGVwZW5kcyBvbiBBUkNIX1JDQVJf
R0VOMSB8fCAgQVJDSF9SQ0FSX0dFTjIgfHwgQ09NUElMRV9URVNUDQoNCkkgd2lsbCBmaXggdGhp
cy4NCg0KPiA+ICsgICAgICAgZGVwZW5kcyBvbiBIQVNfSU9NRU0NCj4gPiArICAgICAgIGhlbHAN
Cj4gPiArICAgICAgICAgVGhpcyBkcml2ZXIgZXhwb3NlcyB0aGUgUFdNIFRpbWVyIGNvbnRyb2xs
ZXIgZm91bmQgaW4gUmVuZXNhcw0KPiA+ICsgICAgICAgICBjaGlwcyB0aHJvdWdoIHRoZSBQV00g
QVBJLg0KPiANCj4gIlJlbmVzYXMgUi1DYXIgY2hpcHMiIChlLmcuIFItTW9iaWxlIHVzZXMgcHdt
LXJlbmVzYXMtdHB1KS4NCg0KSSB3aWxsIGZpeCB0aGlzLg0KDQo+ID4gLS0tIC9kZXYvbnVsbA0K
PiA+ICsrKyBiL2RyaXZlcnMvcHdtL3B3bS1yY2FyLmMNCjwgc25pcCA+DQo+ID4gKyNkZWZpbmUg
TlVNX1JDQVJfUFdNX0NIQU5ORUxTICA3DQo+IA0KPiBJJ20gd29uZGVyaW5nIHdoZXRoZXIgdGhp
cyBzaG91bGQgYmUgYSBEVCBwcm9wZXJ0eSwgdG8gcHJlcGFyZSBmb3INCj4gZnV0dXJlIGV4cGFu
c2lvbi4NCj4gQWx0ZXJuYXRpdmVseSwgYXMgdGhlc2UgYXJlIHJlYWxseSA3IGluZGl2aWR1YWwg
Y2hhbm5lbHMsIHdpdGggNyBpbmRpdmlkdWFsDQo+IHJlZ2lzdGVyIGJsb2NrcyBzcGFjZWQgMHgx
MDAwIGFwYXJ0LCB5b3UgY291bGQgaGF2ZSA3IGluZGl2aWR1YWwgZGV2aWNlIG5vZGVzLA0KPiBl
YWNoIGRyaXZpbmcgb25lIHB3bSBvdXRwdXQuIFRoYXQgd291bGQgcHJvYmFibHkgaW5jdXIgbW9y
ZSBvdmVyaGVhZCwgYXMNCj4gdGhlcmUgd291bGQgYmUgNyBpbmRpdmlkdWFsIHB3bSBjb250cm9s
bGVycy4gQnV0IGl0IGRvZXMgYWxsb3cgZm9yIGZ1dHVyZQ0KPiBleHBhbnNpb24sIGFuZCBhbGxv
d3MgdG8gY29wZSB3aGVuIHRoZSBpbmRpdmlkdWFsIHJlZ2lzdGVyIGJsb2NrcyBhcmUgbW92ZWQN
Cj4gaW4gdGhlIFNvQyBhZGRyZXNzIHNwYWNlIChJIGNhbiBpbWFnaW5lIHRoYXQgb24gb2xkIFNv
Q3MgZS5nLiBhbGwgU0NJRg0KPiBibG9ja3Mgd2VyZSBuaWNlbHkgbGF5IG91dCBpbiB0aGUgYWRk
cmVzcyBzcGFjZSwgd2hpbGUgbm93IHRoZXkgaGF2ZSBhcmJpdHJhcnkNCj4gYmFzZSBhZGRyZXNz
ZXMpLg0KPiANCj4gV2hhdCBkbyBvdGhlciBwZW9wbGUgdGhpbms/DQoNCk15IGluaXRpYWwgZGV2
ZWxvcG1lbnQsIEkgd3JvdGUgNyBpbmRpdmlkdWFsIGRldmljZSBub2Rlcy4gSG93ZXZlciwgYWZ0
ZXIgdGhlIGRyaXZlcg0Kd2FzIHByb3ZlZCwgdGhlIHN5c2ZzIGZvciBwd20gaXM6DQogL3N5cy9j
bGFzcy9wd20vcHdtY2hpcDAvcHdtMA0KIC9zeXMvY2xhc3MvcHdtL3B3bWNoaXAxL3B3bTANCiAu
Li4NCiAvc3lzL2NsYXNzL3B3bS9wd21jaGlwNi9wd20wDQoNCkkgZmVsdCB0aGlzIHdhcyBzdHJh
bmdlIG51bWJlcmluZyBhIGxpdHRsZS4NClNvLCBJIGNoYW5nZWQgdGhlIHN0eWxlIHRvIG9uZSBw
d20gY2hpcCBhbmQgNyBwd20gY2hhbm5lbHM6DQogL3N5cy9jbGFzcy9wd20vcHdtY2hpcDAvcHdt
MA0KIC9zeXMvY2xhc3MvcHdtL3B3bWNoaXAwL3B3bTENCiAuLi4NCiAvc3lzL2NsYXNzL3B3bS9w
d21jaGlwMC9wd202DQoNCkknbSBub3Qgc3VyZSB3aGljaCBzdHlsZSBpcyBnb29kIGZvciB0aGUg
UFdNIElQLg0KDQo+ID4gK3N0YXRpYyBpbnQgcmNhcl9wd25fZ2V0X2Nsb2NrX2RpdmlzaW9uKHN0
cnVjdCByY2FyX3B3bV9jaGlwICpycCwNCj4gPiArICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICBpbnQgcGVyaW9kX25zKQ0KPiA+ICt7DQo+ID4gKyAgICAgICBpbnQgZGl2Ow0K
PiA+ICsgICAgICAgdW5zaWduZWQgbG9uZyBjbGtfcmF0ZSA9IGNsa19nZXRfcmF0ZShycC0+Y2xr
KTsNCj4gPiArICAgICAgIHVuc2lnbmVkIGxvbmcgbG9uZyBtYXg7IC8qIG1heCBjeWNsZSAvIG5h
bm9zZWNvbmRzICovDQo+ID4gKw0KPiA+ICsgICAgICAgZm9yIChkaXYgPSAwOyBkaXYgPD0gUkNB
Ul9QV01fTUFYX0RJVklTSU9OOyBkaXYrKykgew0KPiA+ICsgICAgICAgICAgICAgICBtYXggPSAo
dW5zaWduZWQgbG9uZyBsb25nKU5TRUNfUEVSX1NFQyAqIFJDQVJfUFdNX01BWF9DWUNMRTsNCj4g
PiArICAgICAgICAgICAgICAgZG9fZGl2KG1heCwgY2xrX3JhdGUgLyAoMSA8PCBkaXYpKTsNCj4g
DQo+IERpdmlkaW5nIGNsa19yYXRlIGJ5ICIxIDw8IGRpdiIgcmVkdWNlcyBhY2N1cmFjeSBhIGxv
dCBmb3IgbGFyZ2UgdmFsdWVzIG9mIGRpdi4NCj4gIk5TRUNfUEVSX1NFQyAqIFJDQVJfUFdNX01B
WF9DWUNMRSAqICgxIDw8IGRpdikiIGp1c3QgZml0cyBpbiA2NC1iaXQsDQo+IHNvIHlvdSBjYW4g
dXNlDQo+IA0KPiAgICAgICAgIG1heCA9ICh1bnNpZ25lZCBsb25nIGxvbmcpTlNFQ19QRVJfU0VD
ICogUkNBUl9QV01fTUFYX0NZQ0xFICoNCj4gICAgICAgICAgICAgICAoMSA8PCBkaXYpOw0KPiAg
ICAgICAgIGRvX2RpdihtYXgsIGNsa19yYXRlKTsNCg0KVGhhbmsgeW91IHZlcnkgbXVjaCBmb3Ig
dGhlIHN1Z2dlc3Rpb24uIEkgd2lsbCBmaXggdGhpcyBpbiB2Mi4NCg0KPiA+ICtzdGF0aWMgdm9p
ZCByY2FyX3B3bV9zZXRfY291bnRlcihzdHJ1Y3QgcmNhcl9wd21fY2hpcCAqcnAsIGludCBjaGFu
bmVsLA0KPiA+ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIGludCBkaXYsIGludCBk
dXR5X25zLCBpbnQgcGVyaW9kX25zKQ0KPiA+ICt7DQo+ID4gKyAgICAgICB1bnNpZ25lZCBsb25n
IGxvbmcgb25lX2N5Y2xlLCB0bXA7ICAgICAgLyogMC4wMSBuYW5vc2Vjb25kcyAqLw0KPiA+ICsg
ICAgICAgdW5zaWduZWQgbG9uZyBjbGtfcmF0ZSA9IGNsa19nZXRfcmF0ZShycC0+Y2xrKTsNCj4g
PiArICAgICAgIHUzMiBjeWMsIHBoOw0KPiA+ICsNCj4gPiArICAgICAgIG9uZV9jeWNsZSA9ICh1
bnNpZ25lZCBsb25nIGxvbmcpTlNFQ19QRVJfU0VDICogMTAwOw0KPiA+ICsgICAgICAgZG9fZGl2
KG9uZV9jeWNsZSwgY2xrX3JhdGUgLyAoMSA8PCBkaXYpKTsNCj4gDQo+IFNhbWUgaGVyZToNCj4g
DQo+ICAgICAgICAgb25lX2N5Y2xlID0gKHVuc2lnbmVkIGxvbmcgbG9uZylOU0VDX1BFUl9TRUMg
KiAxMDAgKiAoMSA8PCBkaXYpDQo+ICAgICAgICAgZG9fZGl2KG9uZV9jeWNsZSwgY2xrX3JhdGUp
Ow0KDQpJIHdpbGwgZml4IHRoaXMgaW4gdjIuDQoNCkJlc3QgcmVnYXJkcywNCllvc2hpaGlybyBT
aGltb2RhDQoNCj4gR3J7b2V0amUsZWV0aW5nfXMsDQo+IA0KPiAgICAgICAgICAgICAgICAgICAg
ICAgICBHZWVydA0KPiANCj4gLS0NCj4gR2VlcnQgVXl0dGVyaG9ldmVuIC0tIFRoZXJlJ3MgbG90
cyBvZiBMaW51eCBiZXlvbmQgaWEzMiAtLSBnZWVydEBsaW51eC1tNjhrLm9yZw0KPiANCj4gSW4g
cGVyc29uYWwgY29udmVyc2F0aW9ucyB3aXRoIHRlY2huaWNhbCBwZW9wbGUsIEkgY2FsbCBteXNl
bGYgYSBoYWNrZXIuIEJ1dA0KPiB3aGVuIEknbSB0YWxraW5nIHRvIGpvdXJuYWxpc3RzIEkganVz
dCBzYXkgInByb2dyYW1tZXIiIG9yIHNvbWV0aGluZyBsaWtlIHRoYXQuDQo+ICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgLS0gTGludXMgVG9ydmFsZHMNCg=

RE: [PATCH 2/2] pwm: Add support for R-Car PWM Timer

[Yoshihiro Shimoda]

Hi Geert-san,

Thank you for your review!

> Sent: Friday, May 15, 2015 1:07 AM
> 
> Hi Shimoda-san,
> 
> On Wed, May 13, 2015 at 11:27 AM, Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com> wrote:
> > R-Car SoCs have a seven-channel pulse width modulation (PWM) timer.
> > This driver adds support for the PWM Timer as a single PWM chip and
> > seven PWM devices.
> 
> Thanks for your patch!
> 
> > diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> > index b1541f4..7e98175 100644
> > --- a/drivers/pwm/Kconfig
> > +++ b/drivers/pwm/Kconfig
> > @@ -249,6 +249,17 @@ config PWM_PXA
> >           To compile this driver as a module, choose M here: the module
> >           will be called pwm-pxa.
> >
> > +config PWM_RCAR
> > +       tristate "Renesas R-Car PWM support"
> > +       depends on ARCH_SHMOBILE || COMPILE_TEST
> 
> Could be instead
> 
> depends on ARCH_RCAR_GEN1 ||  ARCH_RCAR_GEN2 || COMPILE_TEST

I will fix this.

> > +       depends on HAS_IOMEM
> > +       help
> > +         This driver exposes the PWM Timer controller found in Renesas
> > +         chips through the PWM API.
> 
> "Renesas R-Car chips" (e.g. R-Mobile uses pwm-renesas-tpu).

I will fix this.

> > --- /dev/null
> > +++ b/drivers/pwm/pwm-rcar.c
< snip >
> > +#define NUM_RCAR_PWM_CHANNELS  7
> 
> I'm wondering whether this should be a DT property, to prepare for
> future expansion.
> Alternatively, as these are really 7 individual channels, with 7 individual
> register blocks spaced 0x1000 apart, you could have 7 individual device nodes,
> each driving one pwm output. That would probably incur more overhead, as
> there would be 7 individual pwm controllers. But it does allow for future
> expansion, and allows to cope when the individual register blocks are moved
> in the SoC address space (I can imagine that on old SoCs e.g. all SCIF
> blocks were nicely lay out in the address space, while now they have arbitrary
> base addresses).
> 
> What do other people think?

My initial development, I wrote 7 individual device nodes. However, after the driver
was proved, the sysfs for pwm is:
 /sys/class/pwm/pwmchip0/pwm0
 /sys/class/pwm/pwmchip1/pwm0
 ...
 /sys/class/pwm/pwmchip6/pwm0

I felt this was strange numbering a little.
So, I changed the style to one pwm chip and 7 pwm channels:
 /sys/class/pwm/pwmchip0/pwm0
 /sys/class/pwm/pwmchip0/pwm1
 ...
 /sys/class/pwm/pwmchip0/pwm6

I'm not sure which style is good for the PWM IP.

> > +static int rcar_pwn_get_clock_division(struct rcar_pwm_chip *rp,
> > +                                      int period_ns)
> > +{
> > +       int div;
> > +       unsigned long clk_rate = clk_get_rate(rp->clk);
> > +       unsigned long long max; /* max cycle / nanoseconds */
> > +
> > +       for (div = 0; div <= RCAR_PWM_MAX_DIVISION; div++) {
> > +               max = (unsigned long long)NSEC_PER_SEC * RCAR_PWM_MAX_CYCLE;
> > +               do_div(max, clk_rate / (1 << div));
> 
> Dividing clk_rate by "1 << div" reduces accuracy a lot for large values of div.
> "NSEC_PER_SEC * RCAR_PWM_MAX_CYCLE * (1 << div)" just fits in 64-bit,
> so you can use
> 
>         max = (unsigned long long)NSEC_PER_SEC * RCAR_PWM_MAX_CYCLE *
>               (1 << div);
>         do_div(max, clk_rate);

Thank you very much for the suggestion. I will fix this in v2.

> > +static void rcar_pwm_set_counter(struct rcar_pwm_chip *rp, int channel,
> > +                                int div, int duty_ns, int period_ns)
> > +{
> > +       unsigned long long one_cycle, tmp;      /* 0.01 nanoseconds */
> > +       unsigned long clk_rate = clk_get_rate(rp->clk);
> > +       u32 cyc, ph;
> > +
> > +       one_cycle = (unsigned long long)NSEC_PER_SEC * 100;
> > +       do_div(one_cycle, clk_rate / (1 << div));
> 
> Same here:
> 
>         one_cycle = (unsigned long long)NSEC_PER_SEC * 100 * (1 << div)
>         do_div(one_cycle, clk_rate);

I will fix this in v2.

Best regards,
Yoshihiro Shimoda

> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds