* [PATCH v6 0/2] pwm: Add support for R-Car PWM Timer
@ 2015-08-19 4:13 ` Yoshihiro Shimoda
0 siblings, 0 replies; 12+ messages in thread
From: Yoshihiro Shimoda @ 2015-08-19 4:13 UTC (permalink / raw)
To: thierry.reding, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak
Cc: linux-pwm, linux-sh, devicetree, Yoshihiro Shimoda
This patch set is based on the latest linux-pwm.git / for-next branch.
(commit id = 6e146f5c41da7e9601fe92fb4d06b45431dbf95b)
Changes from v5:
- Fix coding style and unreadable code in patch 2.
- Add "Reviewed-by: Simon Horman <horms+renesas@verge.net.au>".
Changes from v4:
- Clean up coding style and typo in patch 2.
- Change to_rcar_pwm_chip() macro to static inline function in patch 2.
- Use writel()/readl() instead of iowrite32()/ioread32() in patch 2.
- Add an error handling in rcar_pwm_config() to avoid silent in patch 2.
- Remove success message in rcar_pwm_probe() in patch 2.
- Change rcar_pwm_remove() to always call pm_runtime_disable() in patch 2.
Changes from v3:
- Fix register size in patch 1.
- Add "Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>" in patch 1
- Remove an unnecessary definition in patch 2.
- Use "ULL" to avoid overflow in patch 2.
- Remove unnecessary casts in patch 2.
Changes from v2:
- Add compatible string "renesas,pwm-rcar".
- Remove compatible strings "renesas,pwm-r8a77xx" in rcar_pwm_of_table.
- Fix build error.
Changes from v1:
- Change compatible string to SoC-specific compatible values.
- Fix #pwm-call value to 2 in the device tree document.
- Fix "depends on" value in Kconfig.
- Fix help explanation in Kconfig.
- Remove an unnecessary member in rcar_pwm_chip.
- Remove hardcoded number of channels and change chip.npwm value to 1.
- Fix formulas for clock calculation to improve accuracy.
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 | 27 +++
drivers/pwm/Kconfig | 11 +
drivers/pwm/Makefile | 1 +
drivers/pwm/pwm-rcar.c | 265 +++++++++++++++++++++
4 files changed, 304 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.txt
create mode 100644 drivers/pwm/pwm-rcar.c
--
1.9.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v6 0/2] pwm: Add support for R-Car PWM Timer
@ 2015-08-19 4:13 ` Yoshihiro Shimoda
0 siblings, 0 replies; 12+ messages in thread
From: Yoshihiro Shimoda @ 2015-08-19 4:13 UTC (permalink / raw)
To: thierry.reding, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak
Cc: linux-pwm, linux-sh, devicetree, Yoshihiro Shimoda
This patch set is based on the latest linux-pwm.git / for-next branch.
(commit id = 6e146f5c41da7e9601fe92fb4d06b45431dbf95b)
Changes from v5:
- Fix coding style and unreadable code in patch 2.
- Add "Reviewed-by: Simon Horman <horms+renesas@verge.net.au>".
Changes from v4:
- Clean up coding style and typo in patch 2.
- Change to_rcar_pwm_chip() macro to static inline function in patch 2.
- Use writel()/readl() instead of iowrite32()/ioread32() in patch 2.
- Add an error handling in rcar_pwm_config() to avoid silent in patch 2.
- Remove success message in rcar_pwm_probe() in patch 2.
- Change rcar_pwm_remove() to always call pm_runtime_disable() in patch 2.
Changes from v3:
- Fix register size in patch 1.
- Add "Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>" in patch 1
- Remove an unnecessary definition in patch 2.
- Use "ULL" to avoid overflow in patch 2.
- Remove unnecessary casts in patch 2.
Changes from v2:
- Add compatible string "renesas,pwm-rcar".
- Remove compatible strings "renesas,pwm-r8a77xx" in rcar_pwm_of_table.
- Fix build error.
Changes from v1:
- Change compatible string to SoC-specific compatible values.
- Fix #pwm-call value to 2 in the device tree document.
- Fix "depends on" value in Kconfig.
- Fix help explanation in Kconfig.
- Remove an unnecessary member in rcar_pwm_chip.
- Remove hardcoded number of channels and change chip.npwm value to 1.
- Fix formulas for clock calculation to improve accuracy.
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 | 27 +++
drivers/pwm/Kconfig | 11 +
drivers/pwm/Makefile | 1 +
drivers/pwm/pwm-rcar.c | 265 +++++++++++++++++++++
4 files changed, 304 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.txt
create mode 100644 drivers/pwm/pwm-rcar.c
--
1.9.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v6 1/2] pwm: Add device tree binding document for R-Car PWM Timer
2015-08-19 4:13 ` Yoshihiro Shimoda
@ 2015-08-19 4:13 ` Yoshihiro Shimoda
-1 siblings, 0 replies; 12+ messages in thread
From: Yoshihiro Shimoda @ 2015-08-19 4:13 UTC (permalink / raw)
To: thierry.reding, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak
Cc: linux-pwm, linux-sh, devicetree, Yoshihiro Shimoda
Add binding document for Renesas PWM Timer on R-Car SoCs.
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
---
.../devicetree/bindings/pwm/renesas,pwm-rcar.txt | 27 ++++++++++++++++++++++
1 file changed, 27 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..ea0a27b
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.txt
@@ -0,0 +1,27 @@
+* Renesas R-Car PWM Timer Controller
+
+Required Properties:
+- compatible: should be one of the following.
+ - "renesas,pwm-rcar": for generic R-Car compatible PWM Timer
+ - "renesas,pwm-r8a7778": for R-Car M1A
+ - "renesas,pwm-r8a7779": for R-Car H1
+ - "renesas,pwm-r8a7790": for R-Car H2
+ - "renesas,pwm-r8a7791": for R-Car M2-W
+ - "renesas,pwm-r8a7794": for R-Car E2
+- reg: base address and length of the registers block for the PWM.
+- #pwm-cells: should be 2. 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
+
+ pwm0: pwm@e6e30000 {
+ compatible = "renesas,pwm-r8a7790", "renesas,pwm-rcar";
+ reg = <0 0xe6e30000 0 0x8>;
+ #pwm-cells = <2>;
+ clocks = <&mstp5_clks R8A7790_CLK_PWM>;
+ pinctrl-0 = <&pwm0_pins>;
+ pinctrl-names = "default";
+ };
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v6 1/2] pwm: Add device tree binding document for R-Car PWM Timer
@ 2015-08-19 4:13 ` Yoshihiro Shimoda
0 siblings, 0 replies; 12+ messages in thread
From: Yoshihiro Shimoda @ 2015-08-19 4:13 UTC (permalink / raw)
To: thierry.reding, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak
Cc: linux-pwm, linux-sh, devicetree, Yoshihiro Shimoda
Add binding document for Renesas PWM Timer on R-Car SoCs.
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
---
.../devicetree/bindings/pwm/renesas,pwm-rcar.txt | 27 ++++++++++++++++++++++
1 file changed, 27 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..ea0a27b
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/renesas,pwm-rcar.txt
@@ -0,0 +1,27 @@
+* Renesas R-Car PWM Timer Controller
+
+Required Properties:
+- compatible: should be one of the following.
+ - "renesas,pwm-rcar": for generic R-Car compatible PWM Timer
+ - "renesas,pwm-r8a7778": for R-Car M1A
+ - "renesas,pwm-r8a7779": for R-Car H1
+ - "renesas,pwm-r8a7790": for R-Car H2
+ - "renesas,pwm-r8a7791": for R-Car M2-W
+ - "renesas,pwm-r8a7794": for R-Car E2
+- reg: base address and length of the registers block for the PWM.
+- #pwm-cells: should be 2. 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
+
+ pwm0: pwm@e6e30000 {
+ compatible = "renesas,pwm-r8a7790", "renesas,pwm-rcar";
+ reg = <0 0xe6e30000 0 0x8>;
+ #pwm-cells = <2>;
+ clocks = <&mstp5_clks R8A7790_CLK_PWM>;
+ pinctrl-0 = <&pwm0_pins>;
+ pinctrl-names = "default";
+ };
--
1.9.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v6 2/2] pwm: Add support for R-Car PWM Timer
2015-08-19 4:13 ` Yoshihiro Shimoda
@ 2015-08-19 4:13 ` Yoshihiro Shimoda
-1 siblings, 0 replies; 12+ messages in thread
From: Yoshihiro Shimoda @ 2015-08-19 4:13 UTC (permalink / raw)
To: thierry.reding, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak
Cc: linux-pwm, linux-sh, devicetree, Yoshihiro Shimoda
This patch adds support for R-Car SoCs PWM Timer. The PWM timer of
R-Car H2 has 7 channels. So, we can use the channels if we describe
device tree nodes.
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
---
drivers/pwm/Kconfig | 11 ++
drivers/pwm/Makefile | 1 +
drivers/pwm/pwm-rcar.c | 265 +++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 277 insertions(+)
create mode 100644 drivers/pwm/pwm-rcar.c
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index b1541f4..3e58a68 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_RCAR_GEN1 || ARCH_RCAR_GEN2 || COMPILE_TEST
+ depends on HAS_IOMEM
+ help
+ This driver exposes the PWM Timer controller found in Renesas
+ R-Car 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..a29fd78
--- /dev/null
+++ b/drivers/pwm/pwm-rcar.c
@@ -0,0 +1,265 @@
+/*
+ * 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 RCAR_PWM_MAX_DIVISION 24
+#define RCAR_PWM_MAX_CYCLE 1023
+
+#define RCAR_PWMCR 0x00
+#define RCAR_PWMCNT 0x04
+
+#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 pwm_chip chip;
+ void __iomem *base;
+ struct clk *clk;
+};
+
+static inline struct rcar_pwm_chip *to_rcar_pwm_chip(struct pwm_chip *chip)
+{
+ return container_of(chip, struct rcar_pwm_chip, chip);
+}
+
+static void rcar_pwm_write(struct rcar_pwm_chip *rp, u32 data, u32 reg)
+{
+ writel(data, rp->base + reg);
+}
+
+static u32 rcar_pwm_read(struct rcar_pwm_chip *rp, u32 reg)
+{
+ return readl(rp->base + reg);
+}
+
+static void rcar_pwm_bit_modify(struct rcar_pwm_chip *rp, u32 mask, u32 data,
+ u32 reg)
+{
+ u32 val = rcar_pwm_read(rp, reg);
+
+ val &= ~mask;
+ val |= data & mask;
+ rcar_pwm_write(rp, val, reg);
+}
+
+static int rcar_pwm_get_clock_division(struct rcar_pwm_chip *rp, int period_ns)
+{
+ unsigned int div;
+ unsigned long clk_rate = clk_get_rate(rp->clk);
+ unsigned long long max; /* max cycle / nanoseconds */
+
+ if (clk_rate = 0)
+ return -EINVAL;
+
+ for (div = 0; div <= RCAR_PWM_MAX_DIVISION; div++) {
+ max = (unsigned long long)NSEC_PER_SEC * RCAR_PWM_MAX_CYCLE *
+ (1 << div);
+ do_div(max, clk_rate);
+ if (period_ns < max)
+ break;
+ }
+
+ return (div <= RCAR_PWM_MAX_DIVISION) ? div : -ERANGE;
+}
+
+static void rcar_pwm_set_clock_control(struct rcar_pwm_chip *rp, int div)
+{
+ u32 val = rcar_pwm_read(rp, RCAR_PWMCR);
+
+ 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, val, RCAR_PWMCR);
+}
+
+static int rcar_pwm_set_counter(struct rcar_pwm_chip *rp, 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 * 100ULL * (1 << div);
+ do_div(one_cycle, clk_rate);
+
+ tmp = period_ns * 100ULL;
+ do_div(tmp, one_cycle);
+ cyc = (tmp << RCAR_PWMCNT_CYC0_SHIFT) & RCAR_PWMCNT_CYC0_MASK;
+
+ tmp = duty_ns * 100ULL;
+ do_div(tmp, one_cycle);
+ ph = tmp & RCAR_PWMCNT_PH0_MASK;
+
+ /* Avoid prohibited setting */
+ if (cyc != 0 && ph != 0) {
+ rcar_pwm_write(rp, cyc | ph, RCAR_PWMCNT);
+ return 0;
+ } else {
+ return -EINVAL;
+ }
+}
+
+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 = rcar_pwm_get_clock_division(rp, period_ns);
+ int ret;
+
+ if (div < 0)
+ return div;
+
+ /* Let the core driver set pwm->period if disabled and duty_ns = 0 */
+ if (!test_bit(PWMF_ENABLED, &pwm->flags) && !duty_ns)
+ return 0;
+
+ rcar_pwm_bit_modify(rp, RCAR_PWMCR_SYNC, RCAR_PWMCR_SYNC, RCAR_PWMCR);
+ ret = rcar_pwm_set_counter(rp, div, duty_ns, period_ns);
+ rcar_pwm_set_clock_control(rp, div);
+ rcar_pwm_bit_modify(rp, RCAR_PWMCR_SYNC, 0, RCAR_PWMCR);
+
+ return ret;
+}
+
+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, RCAR_PWMCNT);
+ if ((pwmcnt & RCAR_PWMCNT_CYC0_MASK) = 0 ||
+ (pwmcnt & RCAR_PWMCNT_PH0_MASK) = 0)
+ return -EINVAL;
+
+ rcar_pwm_bit_modify(rp, 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, 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;
+
+ 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 = 1;
+
+ ret = pwmchip_add(&rcar_pwm->chip);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "failed to register PWM chip\n");
+ return ret;
+ }
+
+ 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);
+
+ pm_runtime_disable(&pdev->dev);
+
+ return pwmchip_remove(&rcar_pwm->chip);
+}
+
+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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v6 2/2] pwm: Add support for R-Car PWM Timer
@ 2015-08-19 4:13 ` Yoshihiro Shimoda
0 siblings, 0 replies; 12+ messages in thread
From: Yoshihiro Shimoda @ 2015-08-19 4:13 UTC (permalink / raw)
To: thierry.reding, robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak
Cc: linux-pwm, linux-sh, devicetree, Yoshihiro Shimoda
This patch adds support for R-Car SoCs PWM Timer. The PWM timer of
R-Car H2 has 7 channels. So, we can use the channels if we describe
device tree nodes.
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
---
drivers/pwm/Kconfig | 11 ++
drivers/pwm/Makefile | 1 +
drivers/pwm/pwm-rcar.c | 265 +++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 277 insertions(+)
create mode 100644 drivers/pwm/pwm-rcar.c
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index b1541f4..3e58a68 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_RCAR_GEN1 || ARCH_RCAR_GEN2 || COMPILE_TEST
+ depends on HAS_IOMEM
+ help
+ This driver exposes the PWM Timer controller found in Renesas
+ R-Car 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..a29fd78
--- /dev/null
+++ b/drivers/pwm/pwm-rcar.c
@@ -0,0 +1,265 @@
+/*
+ * 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 RCAR_PWM_MAX_DIVISION 24
+#define RCAR_PWM_MAX_CYCLE 1023
+
+#define RCAR_PWMCR 0x00
+#define RCAR_PWMCNT 0x04
+
+#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 pwm_chip chip;
+ void __iomem *base;
+ struct clk *clk;
+};
+
+static inline struct rcar_pwm_chip *to_rcar_pwm_chip(struct pwm_chip *chip)
+{
+ return container_of(chip, struct rcar_pwm_chip, chip);
+}
+
+static void rcar_pwm_write(struct rcar_pwm_chip *rp, u32 data, u32 reg)
+{
+ writel(data, rp->base + reg);
+}
+
+static u32 rcar_pwm_read(struct rcar_pwm_chip *rp, u32 reg)
+{
+ return readl(rp->base + reg);
+}
+
+static void rcar_pwm_bit_modify(struct rcar_pwm_chip *rp, u32 mask, u32 data,
+ u32 reg)
+{
+ u32 val = rcar_pwm_read(rp, reg);
+
+ val &= ~mask;
+ val |= data & mask;
+ rcar_pwm_write(rp, val, reg);
+}
+
+static int rcar_pwm_get_clock_division(struct rcar_pwm_chip *rp, int period_ns)
+{
+ unsigned int div;
+ unsigned long clk_rate = clk_get_rate(rp->clk);
+ unsigned long long max; /* max cycle / nanoseconds */
+
+ if (clk_rate == 0)
+ return -EINVAL;
+
+ for (div = 0; div <= RCAR_PWM_MAX_DIVISION; div++) {
+ max = (unsigned long long)NSEC_PER_SEC * RCAR_PWM_MAX_CYCLE *
+ (1 << div);
+ do_div(max, clk_rate);
+ if (period_ns < max)
+ break;
+ }
+
+ return (div <= RCAR_PWM_MAX_DIVISION) ? div : -ERANGE;
+}
+
+static void rcar_pwm_set_clock_control(struct rcar_pwm_chip *rp, int div)
+{
+ u32 val = rcar_pwm_read(rp, RCAR_PWMCR);
+
+ 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, val, RCAR_PWMCR);
+}
+
+static int rcar_pwm_set_counter(struct rcar_pwm_chip *rp, 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 * 100ULL * (1 << div);
+ do_div(one_cycle, clk_rate);
+
+ tmp = period_ns * 100ULL;
+ do_div(tmp, one_cycle);
+ cyc = (tmp << RCAR_PWMCNT_CYC0_SHIFT) & RCAR_PWMCNT_CYC0_MASK;
+
+ tmp = duty_ns * 100ULL;
+ do_div(tmp, one_cycle);
+ ph = tmp & RCAR_PWMCNT_PH0_MASK;
+
+ /* Avoid prohibited setting */
+ if (cyc != 0 && ph != 0) {
+ rcar_pwm_write(rp, cyc | ph, RCAR_PWMCNT);
+ return 0;
+ } else {
+ return -EINVAL;
+ }
+}
+
+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 = rcar_pwm_get_clock_division(rp, period_ns);
+ int ret;
+
+ if (div < 0)
+ return div;
+
+ /* Let the core driver set pwm->period if disabled and duty_ns == 0 */
+ if (!test_bit(PWMF_ENABLED, &pwm->flags) && !duty_ns)
+ return 0;
+
+ rcar_pwm_bit_modify(rp, RCAR_PWMCR_SYNC, RCAR_PWMCR_SYNC, RCAR_PWMCR);
+ ret = rcar_pwm_set_counter(rp, div, duty_ns, period_ns);
+ rcar_pwm_set_clock_control(rp, div);
+ rcar_pwm_bit_modify(rp, RCAR_PWMCR_SYNC, 0, RCAR_PWMCR);
+
+ return ret;
+}
+
+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, RCAR_PWMCNT);
+ if ((pwmcnt & RCAR_PWMCNT_CYC0_MASK) == 0 ||
+ (pwmcnt & RCAR_PWMCNT_PH0_MASK) == 0)
+ return -EINVAL;
+
+ rcar_pwm_bit_modify(rp, 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, 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;
+
+ 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 = 1;
+
+ ret = pwmchip_add(&rcar_pwm->chip);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "failed to register PWM chip\n");
+ return ret;
+ }
+
+ 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);
+
+ pm_runtime_disable(&pdev->dev);
+
+ return pwmchip_remove(&rcar_pwm->chip);
+}
+
+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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v6 2/2] pwm: Add support for R-Car PWM Timer
2015-08-19 4:13 ` Yoshihiro Shimoda
@ 2015-08-19 10:07 ` Thierry Reding
-1 siblings, 0 replies; 12+ messages in thread
From: Thierry Reding @ 2015-08-19 10:07 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
linux-pwm, linux-sh, devicetree
[-- Attachment #1: Type: text/plain, Size: 3624 bytes --]
On Wed, Aug 19, 2015 at 01:13:59PM +0900, Yoshihiro Shimoda wrote:
> This patch adds support for R-Car SoCs PWM Timer. The PWM timer of
> R-Car H2 has 7 channels. So, we can use the channels if we describe
> device tree nodes.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
> ---
> drivers/pwm/Kconfig | 11 ++
> drivers/pwm/Makefile | 1 +
> drivers/pwm/pwm-rcar.c | 265 +++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 277 insertions(+)
> create mode 100644 drivers/pwm/pwm-rcar.c
Found a couple more things. No need to respin for any of these, I can
make the changes when applying, but I'd like confirmation on a couple
of things below.
[...]
> diff --git a/drivers/pwm/pwm-rcar.c b/drivers/pwm/pwm-rcar.c
[...]
> +static int rcar_pwm_set_counter(struct rcar_pwm_chip *rp, int div, int duty_ns,
> + int period_ns)
> +{
> + unsigned long long one_cycle, tmp; /* 0.01 nanoseconds */
I'm not quite sure why you need the extra multiplication and division by
100 here. Is this for extra accuracy?
> + unsigned long clk_rate = clk_get_rate(rp->clk);
> + u32 cyc, ph;
> +
> + one_cycle = (unsigned long long)NSEC_PER_SEC * 100ULL * (1 << div);
> + do_div(one_cycle, clk_rate);
> +
> + tmp = period_ns * 100ULL;
> + do_div(tmp, one_cycle);
> + cyc = (tmp << RCAR_PWMCNT_CYC0_SHIFT) & RCAR_PWMCNT_CYC0_MASK;
> +
> + tmp = duty_ns * 100ULL;
> + do_div(tmp, one_cycle);
> + ph = tmp & RCAR_PWMCNT_PH0_MASK;
> +
> + /* Avoid prohibited setting */
> + if (cyc != 0 && ph != 0) {
> + rcar_pwm_write(rp, cyc | ph, RCAR_PWMCNT);
> + return 0;
> + } else {
> + return -EINVAL;
> + }
I think the ordering here is unintuitive, better would be:
if (cyc == 0 || ph == 0)
return -EINVAL;
rcar_pwm_write(rp, cyc | ph, RCAR_PWMCNT);
return 0;
> +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 = rcar_pwm_get_clock_division(rp, period_ns);
> + int ret;
> +
> + if (div < 0)
> + return div;
> +
> + /* Let the core driver set pwm->period if disabled and duty_ns == 0 */
> + if (!test_bit(PWMF_ENABLED, &pwm->flags) && !duty_ns)
> + return 0;
> +
> + rcar_pwm_bit_modify(rp, RCAR_PWMCR_SYNC, RCAR_PWMCR_SYNC, RCAR_PWMCR);
> + ret = rcar_pwm_set_counter(rp, div, duty_ns, period_ns);
> + rcar_pwm_set_clock_control(rp, div);
> + rcar_pwm_bit_modify(rp, RCAR_PWMCR_SYNC, 0, RCAR_PWMCR);
Just making sure: is it correct to execute the above two lines even if
ret < 0?
> +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;
> +
> + 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;
This seems to be missing a:
rcar_pwm->chip.of_pwm_n_cells = 3;
?
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v6 2/2] pwm: Add support for R-Car PWM Timer
@ 2015-08-19 10:07 ` Thierry Reding
0 siblings, 0 replies; 12+ messages in thread
From: Thierry Reding @ 2015-08-19 10:07 UTC (permalink / raw)
To: Yoshihiro Shimoda
Cc: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
linux-pwm, linux-sh, devicetree
[-- Attachment #1: Type: text/plain, Size: 3624 bytes --]
On Wed, Aug 19, 2015 at 01:13:59PM +0900, Yoshihiro Shimoda wrote:
> This patch adds support for R-Car SoCs PWM Timer. The PWM timer of
> R-Car H2 has 7 channels. So, we can use the channels if we describe
> device tree nodes.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
> ---
> drivers/pwm/Kconfig | 11 ++
> drivers/pwm/Makefile | 1 +
> drivers/pwm/pwm-rcar.c | 265 +++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 277 insertions(+)
> create mode 100644 drivers/pwm/pwm-rcar.c
Found a couple more things. No need to respin for any of these, I can
make the changes when applying, but I'd like confirmation on a couple
of things below.
[...]
> diff --git a/drivers/pwm/pwm-rcar.c b/drivers/pwm/pwm-rcar.c
[...]
> +static int rcar_pwm_set_counter(struct rcar_pwm_chip *rp, int div, int duty_ns,
> + int period_ns)
> +{
> + unsigned long long one_cycle, tmp; /* 0.01 nanoseconds */
I'm not quite sure why you need the extra multiplication and division by
100 here. Is this for extra accuracy?
> + unsigned long clk_rate = clk_get_rate(rp->clk);
> + u32 cyc, ph;
> +
> + one_cycle = (unsigned long long)NSEC_PER_SEC * 100ULL * (1 << div);
> + do_div(one_cycle, clk_rate);
> +
> + tmp = period_ns * 100ULL;
> + do_div(tmp, one_cycle);
> + cyc = (tmp << RCAR_PWMCNT_CYC0_SHIFT) & RCAR_PWMCNT_CYC0_MASK;
> +
> + tmp = duty_ns * 100ULL;
> + do_div(tmp, one_cycle);
> + ph = tmp & RCAR_PWMCNT_PH0_MASK;
> +
> + /* Avoid prohibited setting */
> + if (cyc != 0 && ph != 0) {
> + rcar_pwm_write(rp, cyc | ph, RCAR_PWMCNT);
> + return 0;
> + } else {
> + return -EINVAL;
> + }
I think the ordering here is unintuitive, better would be:
if (cyc == 0 || ph == 0)
return -EINVAL;
rcar_pwm_write(rp, cyc | ph, RCAR_PWMCNT);
return 0;
> +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 = rcar_pwm_get_clock_division(rp, period_ns);
> + int ret;
> +
> + if (div < 0)
> + return div;
> +
> + /* Let the core driver set pwm->period if disabled and duty_ns == 0 */
> + if (!test_bit(PWMF_ENABLED, &pwm->flags) && !duty_ns)
> + return 0;
> +
> + rcar_pwm_bit_modify(rp, RCAR_PWMCR_SYNC, RCAR_PWMCR_SYNC, RCAR_PWMCR);
> + ret = rcar_pwm_set_counter(rp, div, duty_ns, period_ns);
> + rcar_pwm_set_clock_control(rp, div);
> + rcar_pwm_bit_modify(rp, RCAR_PWMCR_SYNC, 0, RCAR_PWMCR);
Just making sure: is it correct to execute the above two lines even if
ret < 0?
> +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;
> +
> + 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;
This seems to be missing a:
rcar_pwm->chip.of_pwm_n_cells = 3;
?
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH v6 2/2] pwm: Add support for R-Car PWM Timer
2015-08-19 10:07 ` Thierry Reding
@ 2015-08-19 11:00 ` Yoshihiro Shimoda
-1 siblings, 0 replies; 12+ messages in thread
From: Yoshihiro Shimoda @ 2015-08-19 11:00 UTC (permalink / raw)
To: Thierry Reding
Cc: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
linux-pwm, linux-sh, devicetree
Hi Thierry,
> Sent: Wednesday, August 19, 2015 7:08 PM
>
> On Wed, Aug 19, 2015 at 01:13:59PM +0900, Yoshihiro Shimoda wrote:
> > This patch adds support for R-Car SoCs PWM Timer. The PWM timer of
> > R-Car H2 has 7 channels. So, we can use the channels if we describe
> > device tree nodes.
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
> > ---
> > drivers/pwm/Kconfig | 11 ++
> > drivers/pwm/Makefile | 1 +
> > drivers/pwm/pwm-rcar.c | 265 +++++++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 277 insertions(+)
> > create mode 100644 drivers/pwm/pwm-rcar.c
>
> Found a couple more things. No need to respin for any of these, I can
> make the changes when applying, but I'd like confirmation on a couple
> of things below.
Thank you!
> > diff --git a/drivers/pwm/pwm-rcar.c b/drivers/pwm/pwm-rcar.c
> [...]
> > +static int rcar_pwm_set_counter(struct rcar_pwm_chip *rp, int div, int duty_ns,
> > + int period_ns)
> > +{
> > + unsigned long long one_cycle, tmp; /* 0.01 nanoseconds */
>
> I'm not quite sure why you need the extra multiplication and division by
> 100 here. Is this for extra accuracy?
Yes, this is for extra accuracy.
> > + unsigned long clk_rate = clk_get_rate(rp->clk);
> > + u32 cyc, ph;
> > +
> > + one_cycle = (unsigned long long)NSEC_PER_SEC * 100ULL * (1 << div);
> > + do_div(one_cycle, clk_rate);
> > +
> > + tmp = period_ns * 100ULL;
> > + do_div(tmp, one_cycle);
> > + cyc = (tmp << RCAR_PWMCNT_CYC0_SHIFT) & RCAR_PWMCNT_CYC0_MASK;
> > +
> > + tmp = duty_ns * 100ULL;
> > + do_div(tmp, one_cycle);
> > + ph = tmp & RCAR_PWMCNT_PH0_MASK;
> > +
> > + /* Avoid prohibited setting */
> > + if (cyc != 0 && ph != 0) {
> > + rcar_pwm_write(rp, cyc | ph, RCAR_PWMCNT);
> > + return 0;
> > + } else {
> > + return -EINVAL;
> > + }
>
> I think the ordering here is unintuitive, better would be:
>
> if (cyc = 0 || ph = 0)
> return -EINVAL;
>
> rcar_pwm_write(rp, cyc | ph, RCAR_PWMCNT);
>
> return 0;
Thank you! I think so.
> > +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 = rcar_pwm_get_clock_division(rp, period_ns);
> > + int ret;
> > +
> > + if (div < 0)
> > + return div;
> > +
> > + /* Let the core driver set pwm->period if disabled and duty_ns = 0 */
> > + if (!test_bit(PWMF_ENABLED, &pwm->flags) && !duty_ns)
> > + return 0;
> > +
> > + rcar_pwm_bit_modify(rp, RCAR_PWMCR_SYNC, RCAR_PWMCR_SYNC, RCAR_PWMCR);
> > + ret = rcar_pwm_set_counter(rp, div, duty_ns, period_ns);
> > + rcar_pwm_set_clock_control(rp, div);
> > + rcar_pwm_bit_modify(rp, RCAR_PWMCR_SYNC, 0, RCAR_PWMCR);
>
> Just making sure: is it correct to execute the above two lines even if
> ret < 0?
Oops, this driver should not call the rcar_pwm_set_clock_control()
because divider ratio might be changed.
However, this driver should execute rcar_pwm_bit_modify(rp, RCAR_PWMCR_SYNC, 0, RCAR_PWMCR)
to clear the "SYNC" bit.
> > +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;
> > +
> > + 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;
>
> This seems to be missing a:
>
> rcar_pwm->chip.of_pwm_n_cells = 3;
>
> ?
Thank you for the point.
I just tested this driver only sysfs interface.
This driver should use of_pwm_simple_xlate and the value is set to 2
because this PWM timer doesn't support polarity.
Best regards,
Yoshihiro Shimoda
> Thierry
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH v6 2/2] pwm: Add support for R-Car PWM Timer
@ 2015-08-19 11:00 ` Yoshihiro Shimoda
0 siblings, 0 replies; 12+ messages in thread
From: Yoshihiro Shimoda @ 2015-08-19 11:00 UTC (permalink / raw)
To: Thierry Reding
Cc: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
linux-pwm, linux-sh, devicetree
Hi Thierry,
> Sent: Wednesday, August 19, 2015 7:08 PM
>
> On Wed, Aug 19, 2015 at 01:13:59PM +0900, Yoshihiro Shimoda wrote:
> > This patch adds support for R-Car SoCs PWM Timer. The PWM timer of
> > R-Car H2 has 7 channels. So, we can use the channels if we describe
> > device tree nodes.
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
> > ---
> > drivers/pwm/Kconfig | 11 ++
> > drivers/pwm/Makefile | 1 +
> > drivers/pwm/pwm-rcar.c | 265 +++++++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 277 insertions(+)
> > create mode 100644 drivers/pwm/pwm-rcar.c
>
> Found a couple more things. No need to respin for any of these, I can
> make the changes when applying, but I'd like confirmation on a couple
> of things below.
Thank you!
> > diff --git a/drivers/pwm/pwm-rcar.c b/drivers/pwm/pwm-rcar.c
> [...]
> > +static int rcar_pwm_set_counter(struct rcar_pwm_chip *rp, int div, int duty_ns,
> > + int period_ns)
> > +{
> > + unsigned long long one_cycle, tmp; /* 0.01 nanoseconds */
>
> I'm not quite sure why you need the extra multiplication and division by
> 100 here. Is this for extra accuracy?
Yes, this is for extra accuracy.
> > + unsigned long clk_rate = clk_get_rate(rp->clk);
> > + u32 cyc, ph;
> > +
> > + one_cycle = (unsigned long long)NSEC_PER_SEC * 100ULL * (1 << div);
> > + do_div(one_cycle, clk_rate);
> > +
> > + tmp = period_ns * 100ULL;
> > + do_div(tmp, one_cycle);
> > + cyc = (tmp << RCAR_PWMCNT_CYC0_SHIFT) & RCAR_PWMCNT_CYC0_MASK;
> > +
> > + tmp = duty_ns * 100ULL;
> > + do_div(tmp, one_cycle);
> > + ph = tmp & RCAR_PWMCNT_PH0_MASK;
> > +
> > + /* Avoid prohibited setting */
> > + if (cyc != 0 && ph != 0) {
> > + rcar_pwm_write(rp, cyc | ph, RCAR_PWMCNT);
> > + return 0;
> > + } else {
> > + return -EINVAL;
> > + }
>
> I think the ordering here is unintuitive, better would be:
>
> if (cyc == 0 || ph == 0)
> return -EINVAL;
>
> rcar_pwm_write(rp, cyc | ph, RCAR_PWMCNT);
>
> return 0;
Thank you! I think so.
> > +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 = rcar_pwm_get_clock_division(rp, period_ns);
> > + int ret;
> > +
> > + if (div < 0)
> > + return div;
> > +
> > + /* Let the core driver set pwm->period if disabled and duty_ns == 0 */
> > + if (!test_bit(PWMF_ENABLED, &pwm->flags) && !duty_ns)
> > + return 0;
> > +
> > + rcar_pwm_bit_modify(rp, RCAR_PWMCR_SYNC, RCAR_PWMCR_SYNC, RCAR_PWMCR);
> > + ret = rcar_pwm_set_counter(rp, div, duty_ns, period_ns);
> > + rcar_pwm_set_clock_control(rp, div);
> > + rcar_pwm_bit_modify(rp, RCAR_PWMCR_SYNC, 0, RCAR_PWMCR);
>
> Just making sure: is it correct to execute the above two lines even if
> ret < 0?
Oops, this driver should not call the rcar_pwm_set_clock_control()
because divider ratio might be changed.
However, this driver should execute rcar_pwm_bit_modify(rp, RCAR_PWMCR_SYNC, 0, RCAR_PWMCR)
to clear the "SYNC" bit.
> > +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;
> > +
> > + 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;
>
> This seems to be missing a:
>
> rcar_pwm->chip.of_pwm_n_cells = 3;
>
> ?
Thank you for the point.
I just tested this driver only sysfs interface.
This driver should use of_pwm_simple_xlate and the value is set to 2
because this PWM timer doesn't support polarity.
Best regards,
Yoshihiro Shimoda
> Thierry
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH v6 2/2] pwm: Add support for R-Car PWM Timer
2015-08-19 10:07 ` Thierry Reding
(?)
(?)
@ 2015-09-15 10:00 ` Yoshihiro Shimoda
-1 siblings, 0 replies; 12+ messages in thread
From: Yoshihiro Shimoda @ 2015-09-15 10:00 UTC (permalink / raw)
To: Thierry Reding
Cc: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
linux-pwm, linux-sh, devicetree
Hi Thierry,
> Sent: Wednesday, August 19, 2015 8:01 PM
>
> Hi Thierry,
>
> > Sent: Wednesday, August 19, 2015 7:08 PM
> >
> > On Wed, Aug 19, 2015 at 01:13:59PM +0900, Yoshihiro Shimoda wrote:
> > > This patch adds support for R-Car SoCs PWM Timer. The PWM timer of
> > > R-Car H2 has 7 channels. So, we can use the channels if we describe
> > > device tree nodes.
> > >
> > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > > Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
> > > ---
> > > drivers/pwm/Kconfig | 11 ++
> > > drivers/pwm/Makefile | 1 +
> > > drivers/pwm/pwm-rcar.c | 265 +++++++++++++++++++++++++++++++++++++++++++++++++
> > > 3 files changed, 277 insertions(+)
> > > create mode 100644 drivers/pwm/pwm-rcar.c
> >
> > Found a couple more things. No need to respin for any of these, I can
> > make the changes when applying, but I'd like confirmation on a couple
> > of things below.
>
> Thank you!
I could not find this driver in the latest your linux-pwm.git / for-next branch.
Should I send v7 patch?
Best regards,
Yoshihiro Shimoda
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH v6 2/2] pwm: Add support for R-Car PWM Timer
2015-08-19 10:07 ` Thierry Reding
` (2 preceding siblings ...)
(?)
@ 2015-09-30 8:49 ` Yoshihiro Shimoda
-1 siblings, 0 replies; 12+ messages in thread
From: Yoshihiro Shimoda @ 2015-09-30 8:49 UTC (permalink / raw)
To: Thierry Reding
Cc: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak,
linux-pwm, linux-sh, devicetree
Hi Thierry,
> Sent: Tuesday, September 15, 2015 7:01 PM
>
> Hi Thierry,
>
> > Sent: Wednesday, August 19, 2015 8:01 PM
> >
> > Hi Thierry,
> >
> > > Sent: Wednesday, August 19, 2015 7:08 PM
> > >
> > > On Wed, Aug 19, 2015 at 01:13:59PM +0900, Yoshihiro Shimoda wrote:
> > > > This patch adds support for R-Car SoCs PWM Timer. The PWM timer of
> > > > R-Car H2 has 7 channels. So, we can use the channels if we describe
> > > > device tree nodes.
> > > >
> > > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > > > Reviewed-by: Simon Horman <horms+renesas@verge.net.au>
> > > > ---
> > > > drivers/pwm/Kconfig | 11 ++
> > > > drivers/pwm/Makefile | 1 +
> > > > drivers/pwm/pwm-rcar.c | 265 +++++++++++++++++++++++++++++++++++++++++++++++++
> > > > 3 files changed, 277 insertions(+)
> > > > create mode 100644 drivers/pwm/pwm-rcar.c
> > >
> > > Found a couple more things. No need to respin for any of these, I can
> > > make the changes when applying, but I'd like confirmation on a couple
> > > of things below.
> >
> > Thank you!
>
> I could not find this driver in the latest your linux-pwm.git / for-next branch.
> Should I send v7 patch?
I submitted v7 patch now. So, would you review it?
Best regards,
Yoshihiro Shimoda
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2015-09-30 8:49 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-19 4:13 [PATCH v6 0/2] pwm: Add support for R-Car PWM Timer Yoshihiro Shimoda
2015-08-19 4:13 ` Yoshihiro Shimoda
2015-08-19 4:13 ` [PATCH v6 1/2] pwm: Add device tree binding document " Yoshihiro Shimoda
2015-08-19 4:13 ` Yoshihiro Shimoda
2015-08-19 4:13 ` [PATCH v6 2/2] pwm: Add support " Yoshihiro Shimoda
2015-08-19 4:13 ` Yoshihiro Shimoda
2015-08-19 10:07 ` Thierry Reding
2015-08-19 10:07 ` Thierry Reding
2015-08-19 11:00 ` Yoshihiro Shimoda
2015-08-19 11:00 ` Yoshihiro Shimoda
2015-09-15 10:00 ` Yoshihiro Shimoda
2015-09-30 8:49 ` Yoshihiro Shimoda
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.