* [PATCH v6 0/2] pwm: visconti: Add Toshiba Visconti SoC PWM support
@ 2021-04-19 0:00 ` Nobuhiro Iwamatsu
0 siblings, 0 replies; 12+ messages in thread
From: Nobuhiro Iwamatsu @ 2021-04-19 0:00 UTC (permalink / raw)
To: Rob Herring, Thierry Reding, Uwe Kleine-König, Lee Jones
Cc: devicetree, linux-pwm, punit1.agrawal, yuji2.ishikawa,
linux-arm-kernel, linux-kernel, Nobuhiro Iwamatsu
Hi,
This series is the PWM driver for Toshiba's ARM SoC, Visconti[0].
This provides DT binding documentation and device driver.
[0]: https://toshiba.semicon-storage.com/ap-en/semiconductor/product/image-recognition-processors-visconti.html
Updates:
dt-bindings: pwm: Add bindings for Toshiba Visconti PWM Controller
v5 -> v6:
- No update.
v4 -> v5:
- No update.
v3 -> v4:
- No update.
v2 -> v3:
- Change compatible to toshiba,visconti-pwm
- Change filename to toshiba,visconti-pwm.yaml.
- Add Reviewed-by tag from Rob.
v1 -> v2:
- Change SPDX-License-Identifier to GPL-2.0-only OR BSD-2-Clause.
- Set compatible toshiba,pwm-visconti only.
- Drop unnecessary comments.
pwm: visconti: Add Toshiba Visconti SoC PWM support
v5 -> v6:
- Update year in copyright.
- Update limitations.
- Fix coding style, used braces for both branches.
v4 -> v5:
- Droped checking PIPGM_PCSR from visconti_pwm_get_state.
- Changed from to_visconti_chip to visconti_pwm_from_chip.
- Removed pwmchip_remove return value management.
- Add limitations of this device.
- Add 'state->enabled = true' to visconti_pwm_get_state().
v3 -> v4:
- Sorted alphabetically include files.
- Changed container_of to using static inline functions.
- Dropped unnecessary dev_dbg().
- Drop Initialization of chip.base.
- Drop commnet "period too small".
- Rebased for-next.
v2 -> v3:
- Change compatible to toshiba,visconti-pwm.
- Fix MODULE_ALIAS to platform:pwm-visconti, again.
- Align continuation line to the opening parenthesis.
- Rewrite the contents of visconti_pwm_apply() based on the contents suggested by Uwe.
v1 -> v2:
- Change SPDX-License-Identifier to GPL-2.0-only.
- Add prefix for the register defines.
- Drop struct device from struct visconti_pwm_chip.
- Use '>>' instead of '/'.
- Drop error message by devm_platform_ioremap_resource().
- Use dev_err_probe instead of dev_err.
- Change dev_info to dev_dbg.
- Remove some empty lines.
- Fix MODULE_ALIAS to platform:pwm-visconti.
- Add .get_state() function.
- Use the author name and email address to MODULE_AUTHOR.
- Add more comment to function of the hardware.
- Support .get_status() function.
- Use NSEC_PER_USEC instead of 1000.
- Alphabetically sorted for Makefile and Kconfig.
- Added check for set value in visconti_pwm_apply().
Nobuhiro Iwamatsu (2):
dt-bindings: pwm: Add bindings for Toshiba Visconti PWM Controller
pwm: visconti: Add Toshiba Visconti SoC PWM support
.../bindings/pwm/toshiba,pwm-visconti.yaml | 43 ++++
drivers/pwm/Kconfig | 9 +
drivers/pwm/Makefile | 1 +
drivers/pwm/pwm-visconti.c | 189 ++++++++++++++++++
4 files changed, 242 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pwm/toshiba,pwm-visconti.yaml
create mode 100644 drivers/pwm/pwm-visconti.c
--
2.30.0.rc2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH v6 1/2] dt-bindings: pwm: Add bindings for Toshiba Visconti PWM Controller
2021-04-19 0:00 ` Nobuhiro Iwamatsu
@ 2021-04-19 0:00 ` Nobuhiro Iwamatsu
-1 siblings, 0 replies; 12+ messages in thread
From: Nobuhiro Iwamatsu @ 2021-04-19 0:00 UTC (permalink / raw)
To: Rob Herring, Thierry Reding, Uwe Kleine-König, Lee Jones
Cc: devicetree, linux-pwm, punit1.agrawal, yuji2.ishikawa,
linux-arm-kernel, linux-kernel, Nobuhiro Iwamatsu, Rob Herring
Add bindings for the Toshiba Visconti PWM Controller.
Signed-off-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
Reviewed-by: Rob Herring <robh@kernel.org>
---
.../bindings/pwm/toshiba,pwm-visconti.yaml | 43 +++++++++++++++++++
1 file changed, 43 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pwm/toshiba,pwm-visconti.yaml
diff --git a/Documentation/devicetree/bindings/pwm/toshiba,pwm-visconti.yaml b/Documentation/devicetree/bindings/pwm/toshiba,pwm-visconti.yaml
new file mode 100644
index 000000000000..d350f5edfb67
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/toshiba,pwm-visconti.yaml
@@ -0,0 +1,43 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pwm/toshiba,pwm-visconti.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Toshiba Visconti PWM Controller
+
+maintainers:
+ - Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
+
+properties:
+ compatible:
+ items:
+ - const: toshiba,visconti-pwm
+
+ reg:
+ maxItems: 1
+
+ '#pwm-cells':
+ const: 2
+
+required:
+ - compatible
+ - reg
+ - '#pwm-cells'
+
+additionalProperties: false
+
+examples:
+ - |
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ pwm: pwm@241c0000 {
+ compatible = "toshiba,visconti-pwm";
+ reg = <0 0x241c0000 0 0x1000>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pwm_mux>;
+ #pwm-cells = <2>;
+ };
+ };
--
2.30.0.rc2
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v6 1/2] dt-bindings: pwm: Add bindings for Toshiba Visconti PWM Controller
@ 2021-04-19 0:00 ` Nobuhiro Iwamatsu
0 siblings, 0 replies; 12+ messages in thread
From: Nobuhiro Iwamatsu @ 2021-04-19 0:00 UTC (permalink / raw)
To: Rob Herring, Thierry Reding, Uwe Kleine-König, Lee Jones
Cc: devicetree, linux-pwm, punit1.agrawal, yuji2.ishikawa,
linux-arm-kernel, linux-kernel, Nobuhiro Iwamatsu, Rob Herring
Add bindings for the Toshiba Visconti PWM Controller.
Signed-off-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
Reviewed-by: Rob Herring <robh@kernel.org>
---
.../bindings/pwm/toshiba,pwm-visconti.yaml | 43 +++++++++++++++++++
1 file changed, 43 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pwm/toshiba,pwm-visconti.yaml
diff --git a/Documentation/devicetree/bindings/pwm/toshiba,pwm-visconti.yaml b/Documentation/devicetree/bindings/pwm/toshiba,pwm-visconti.yaml
new file mode 100644
index 000000000000..d350f5edfb67
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/toshiba,pwm-visconti.yaml
@@ -0,0 +1,43 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pwm/toshiba,pwm-visconti.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Toshiba Visconti PWM Controller
+
+maintainers:
+ - Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
+
+properties:
+ compatible:
+ items:
+ - const: toshiba,visconti-pwm
+
+ reg:
+ maxItems: 1
+
+ '#pwm-cells':
+ const: 2
+
+required:
+ - compatible
+ - reg
+ - '#pwm-cells'
+
+additionalProperties: false
+
+examples:
+ - |
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ pwm: pwm@241c0000 {
+ compatible = "toshiba,visconti-pwm";
+ reg = <0 0x241c0000 0 0x1000>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pwm_mux>;
+ #pwm-cells = <2>;
+ };
+ };
--
2.30.0.rc2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v6 2/2] pwm: visconti: Add Toshiba Visconti SoC PWM support
2021-04-19 0:00 ` Nobuhiro Iwamatsu
@ 2021-04-19 0:00 ` Nobuhiro Iwamatsu
-1 siblings, 0 replies; 12+ messages in thread
From: Nobuhiro Iwamatsu @ 2021-04-19 0:00 UTC (permalink / raw)
To: Rob Herring, Thierry Reding, Uwe Kleine-König, Lee Jones
Cc: devicetree, linux-pwm, punit1.agrawal, yuji2.ishikawa,
linux-arm-kernel, linux-kernel, Nobuhiro Iwamatsu
Add driver for the PWM controller on Toshiba Visconti ARM SoC.
Signed-off-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
---
drivers/pwm/Kconfig | 9 ++
drivers/pwm/Makefile | 1 +
drivers/pwm/pwm-visconti.c | 189 +++++++++++++++++++++++++++++++++++++
3 files changed, 199 insertions(+)
create mode 100644 drivers/pwm/pwm-visconti.c
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 9a4f66ae8070..8ae68d6203fb 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -601,6 +601,15 @@ config PWM_TWL_LED
To compile this driver as a module, choose M here: the module
will be called pwm-twl-led.
+config PWM_VISCONTI
+ tristate "Toshiba Visconti PWM support"
+ depends on ARCH_VISCONTI || COMPILE_TEST
+ help
+ PWM Subsystem driver support for Toshiba Visconti SoCs.
+
+ To compile this driver as a module, choose M here: the module
+ will be called pwm-visconti.
+
config PWM_VT8500
tristate "vt8500 PWM support"
depends on ARCH_VT8500 || COMPILE_TEST
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index 6374d3b1d6f3..d43b1e17e8e1 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -56,4 +56,5 @@ obj-$(CONFIG_PWM_TIECAP) += pwm-tiecap.o
obj-$(CONFIG_PWM_TIEHRPWM) += pwm-tiehrpwm.o
obj-$(CONFIG_PWM_TWL) += pwm-twl.o
obj-$(CONFIG_PWM_TWL_LED) += pwm-twl-led.o
+obj-$(CONFIG_PWM_VISCONTI) += pwm-visconti.o
obj-$(CONFIG_PWM_VT8500) += pwm-vt8500.o
diff --git a/drivers/pwm/pwm-visconti.c b/drivers/pwm/pwm-visconti.c
new file mode 100644
index 000000000000..beecadda566a
--- /dev/null
+++ b/drivers/pwm/pwm-visconti.c
@@ -0,0 +1,189 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Toshiba Visconti pulse-width-modulation controller driver
+ *
+ * Copyright (c) 2020 - 2021 TOSHIBA CORPORATION
+ * Copyright (c) 2020 - 2021 Toshiba Electronic Devices & Storage Corporation
+ *
+ * Authors: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
+ *
+ * Limitations:
+ * - The fixed input clock is running at 1 MHz and is divided by either 1,
+ * 2, 4 or 8.
+ * - When the settings of the PWM are modified, the new values are shadowed
+ * in hardware until the PIPGM_PCSR register is written and the currently
+ * running period is completed. This way the hardware switches atomically
+ * from the old setting to the new.
+ * - Disabling the hardware completes the currently running period and keeps
+ * the output at low level at all times.
+ */
+
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/pwm.h>
+
+#define PIPGM_PCSR(ch) (0x400 + 4 * (ch))
+#define PIPGM_PDUT(ch) (0x420 + 4 * (ch))
+#define PIPGM_PWMC(ch) (0x440 + 4 * (ch))
+
+#define PIPGM_PWMC_PWMACT BIT(5)
+#define PIPGM_PWMC_CLK_MASK GENMASK(1, 0)
+#define PIPGM_PWMC_POLARITY_MASK GENMASK(5, 5)
+
+struct visconti_pwm_chip {
+ struct pwm_chip chip;
+ void __iomem *base;
+};
+
+static inline struct visconti_pwm_chip *visconti_pwm_from_chip(struct pwm_chip *chip)
+{
+ return container_of(chip, struct visconti_pwm_chip, chip);
+}
+
+static int visconti_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
+ const struct pwm_state *state)
+{
+ struct visconti_pwm_chip *priv = visconti_pwm_from_chip(chip);
+ u32 period, duty_cycle, pwmc0;
+
+ if (!state->enabled) {
+ writel(0, priv->base + PIPGM_PCSR(pwm->hwpwm));
+ return 0;
+ }
+
+ /*
+ * The biggest period the hardware can provide is
+ * (0xffff << 3) * 1000 ns
+ * This value fits easily in an u32, so simplify the maths by
+ * capping the values to 32 bit integers.
+ */
+ if (state->period > (0xffff << 3) * 1000)
+ period = (0xffff << 3) * 1000;
+ else
+ period = state->period;
+
+ if (state->duty_cycle > period)
+ duty_cycle = period;
+ else
+ duty_cycle = state->duty_cycle;
+
+ /*
+ * The input clock runs fixed at 1 MHz, so we have only
+ * microsecond resolution and so can divide by
+ * NSEC_PER_SEC / CLKFREQ = 1000 without loosing precision.
+ */
+ period /= 1000;
+ duty_cycle /= 1000;
+
+ if (!period)
+ return -ERANGE;
+
+ /*
+ * PWMC controls a divider that divides the input clk by a
+ * power of two between 1 and 8. As a smaller divider yields
+ * higher precision, pick the smallest possible one.
+ */
+ if (period > 0xffff) {
+ pwmc0 = ilog2(period >> 16);
+ BUG_ON(pwmc0 > 3);
+ } else {
+ pwmc0 = 0;
+ }
+
+ period >>= pwmc0;
+ duty_cycle >>= pwmc0;
+
+ if (state->polarity == PWM_POLARITY_INVERSED)
+ pwmc0 |= PIPGM_PWMC_PWMACT;
+ writel(pwmc0, priv->base + PIPGM_PWMC(pwm->hwpwm));
+ writel(duty_cycle, priv->base + PIPGM_PDUT(pwm->hwpwm));
+ writel(period, priv->base + PIPGM_PCSR(pwm->hwpwm));
+
+ return 0;
+}
+
+static void visconti_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
+ struct pwm_state *state)
+{
+ struct visconti_pwm_chip *priv = visconti_pwm_from_chip(chip);
+ u32 period, duty, pwmc0, pwmc0_clk;
+
+ period = readl(priv->base + PIPGM_PCSR(pwm->hwpwm));
+ duty = readl(priv->base + PIPGM_PDUT(pwm->hwpwm));
+ pwmc0 = readl(priv->base + PIPGM_PWMC(pwm->hwpwm));
+ pwmc0_clk = pwmc0 & PIPGM_PWMC_CLK_MASK;
+
+ state->period = (period << pwmc0_clk) * NSEC_PER_USEC;
+ state->duty_cycle = (duty << pwmc0_clk) * NSEC_PER_USEC;
+ if (pwmc0 & PIPGM_PWMC_POLARITY_MASK)
+ state->polarity = PWM_POLARITY_INVERSED;
+ else
+ state->polarity = PWM_POLARITY_NORMAL;
+
+ state->enabled = true;
+}
+
+static const struct pwm_ops visconti_pwm_ops = {
+ .apply = visconti_pwm_apply,
+ .get_state = visconti_pwm_get_state,
+ .owner = THIS_MODULE,
+};
+
+static int visconti_pwm_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct visconti_pwm_chip *priv;
+ int ret;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(priv->base))
+ return PTR_ERR(priv->base);
+
+ platform_set_drvdata(pdev, priv);
+
+ priv->chip.dev = dev;
+ priv->chip.ops = &visconti_pwm_ops;
+ priv->chip.npwm = 4;
+
+ ret = pwmchip_add(&priv->chip);
+ if (ret < 0)
+ return dev_err_probe(&pdev->dev, ret, "Cannot register visconti PWM\n");
+
+ return 0;
+}
+
+static int visconti_pwm_remove(struct platform_device *pdev)
+{
+ struct visconti_pwm_chip *priv = platform_get_drvdata(pdev);
+
+ pwmchip_remove(&priv->chip);
+
+ return 0;
+}
+
+static const struct of_device_id visconti_pwm_of_match[] = {
+ { .compatible = "toshiba,visconti-pwm", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, visconti_pwm_of_match);
+
+static struct platform_driver visconti_pwm_driver = {
+ .driver = {
+ .name = "pwm-visconti",
+ .of_match_table = visconti_pwm_of_match,
+ },
+ .probe = visconti_pwm_probe,
+ .remove = visconti_pwm_remove,
+};
+module_platform_driver(visconti_pwm_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>");
+MODULE_ALIAS("platform:pwm-visconti");
--
2.30.0.rc2
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v6 2/2] pwm: visconti: Add Toshiba Visconti SoC PWM support
@ 2021-04-19 0:00 ` Nobuhiro Iwamatsu
0 siblings, 0 replies; 12+ messages in thread
From: Nobuhiro Iwamatsu @ 2021-04-19 0:00 UTC (permalink / raw)
To: Rob Herring, Thierry Reding, Uwe Kleine-König, Lee Jones
Cc: devicetree, linux-pwm, punit1.agrawal, yuji2.ishikawa,
linux-arm-kernel, linux-kernel, Nobuhiro Iwamatsu
Add driver for the PWM controller on Toshiba Visconti ARM SoC.
Signed-off-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
---
drivers/pwm/Kconfig | 9 ++
drivers/pwm/Makefile | 1 +
drivers/pwm/pwm-visconti.c | 189 +++++++++++++++++++++++++++++++++++++
3 files changed, 199 insertions(+)
create mode 100644 drivers/pwm/pwm-visconti.c
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 9a4f66ae8070..8ae68d6203fb 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -601,6 +601,15 @@ config PWM_TWL_LED
To compile this driver as a module, choose M here: the module
will be called pwm-twl-led.
+config PWM_VISCONTI
+ tristate "Toshiba Visconti PWM support"
+ depends on ARCH_VISCONTI || COMPILE_TEST
+ help
+ PWM Subsystem driver support for Toshiba Visconti SoCs.
+
+ To compile this driver as a module, choose M here: the module
+ will be called pwm-visconti.
+
config PWM_VT8500
tristate "vt8500 PWM support"
depends on ARCH_VT8500 || COMPILE_TEST
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index 6374d3b1d6f3..d43b1e17e8e1 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -56,4 +56,5 @@ obj-$(CONFIG_PWM_TIECAP) += pwm-tiecap.o
obj-$(CONFIG_PWM_TIEHRPWM) += pwm-tiehrpwm.o
obj-$(CONFIG_PWM_TWL) += pwm-twl.o
obj-$(CONFIG_PWM_TWL_LED) += pwm-twl-led.o
+obj-$(CONFIG_PWM_VISCONTI) += pwm-visconti.o
obj-$(CONFIG_PWM_VT8500) += pwm-vt8500.o
diff --git a/drivers/pwm/pwm-visconti.c b/drivers/pwm/pwm-visconti.c
new file mode 100644
index 000000000000..beecadda566a
--- /dev/null
+++ b/drivers/pwm/pwm-visconti.c
@@ -0,0 +1,189 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Toshiba Visconti pulse-width-modulation controller driver
+ *
+ * Copyright (c) 2020 - 2021 TOSHIBA CORPORATION
+ * Copyright (c) 2020 - 2021 Toshiba Electronic Devices & Storage Corporation
+ *
+ * Authors: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
+ *
+ * Limitations:
+ * - The fixed input clock is running at 1 MHz and is divided by either 1,
+ * 2, 4 or 8.
+ * - When the settings of the PWM are modified, the new values are shadowed
+ * in hardware until the PIPGM_PCSR register is written and the currently
+ * running period is completed. This way the hardware switches atomically
+ * from the old setting to the new.
+ * - Disabling the hardware completes the currently running period and keeps
+ * the output at low level at all times.
+ */
+
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/pwm.h>
+
+#define PIPGM_PCSR(ch) (0x400 + 4 * (ch))
+#define PIPGM_PDUT(ch) (0x420 + 4 * (ch))
+#define PIPGM_PWMC(ch) (0x440 + 4 * (ch))
+
+#define PIPGM_PWMC_PWMACT BIT(5)
+#define PIPGM_PWMC_CLK_MASK GENMASK(1, 0)
+#define PIPGM_PWMC_POLARITY_MASK GENMASK(5, 5)
+
+struct visconti_pwm_chip {
+ struct pwm_chip chip;
+ void __iomem *base;
+};
+
+static inline struct visconti_pwm_chip *visconti_pwm_from_chip(struct pwm_chip *chip)
+{
+ return container_of(chip, struct visconti_pwm_chip, chip);
+}
+
+static int visconti_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
+ const struct pwm_state *state)
+{
+ struct visconti_pwm_chip *priv = visconti_pwm_from_chip(chip);
+ u32 period, duty_cycle, pwmc0;
+
+ if (!state->enabled) {
+ writel(0, priv->base + PIPGM_PCSR(pwm->hwpwm));
+ return 0;
+ }
+
+ /*
+ * The biggest period the hardware can provide is
+ * (0xffff << 3) * 1000 ns
+ * This value fits easily in an u32, so simplify the maths by
+ * capping the values to 32 bit integers.
+ */
+ if (state->period > (0xffff << 3) * 1000)
+ period = (0xffff << 3) * 1000;
+ else
+ period = state->period;
+
+ if (state->duty_cycle > period)
+ duty_cycle = period;
+ else
+ duty_cycle = state->duty_cycle;
+
+ /*
+ * The input clock runs fixed at 1 MHz, so we have only
+ * microsecond resolution and so can divide by
+ * NSEC_PER_SEC / CLKFREQ = 1000 without loosing precision.
+ */
+ period /= 1000;
+ duty_cycle /= 1000;
+
+ if (!period)
+ return -ERANGE;
+
+ /*
+ * PWMC controls a divider that divides the input clk by a
+ * power of two between 1 and 8. As a smaller divider yields
+ * higher precision, pick the smallest possible one.
+ */
+ if (period > 0xffff) {
+ pwmc0 = ilog2(period >> 16);
+ BUG_ON(pwmc0 > 3);
+ } else {
+ pwmc0 = 0;
+ }
+
+ period >>= pwmc0;
+ duty_cycle >>= pwmc0;
+
+ if (state->polarity == PWM_POLARITY_INVERSED)
+ pwmc0 |= PIPGM_PWMC_PWMACT;
+ writel(pwmc0, priv->base + PIPGM_PWMC(pwm->hwpwm));
+ writel(duty_cycle, priv->base + PIPGM_PDUT(pwm->hwpwm));
+ writel(period, priv->base + PIPGM_PCSR(pwm->hwpwm));
+
+ return 0;
+}
+
+static void visconti_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
+ struct pwm_state *state)
+{
+ struct visconti_pwm_chip *priv = visconti_pwm_from_chip(chip);
+ u32 period, duty, pwmc0, pwmc0_clk;
+
+ period = readl(priv->base + PIPGM_PCSR(pwm->hwpwm));
+ duty = readl(priv->base + PIPGM_PDUT(pwm->hwpwm));
+ pwmc0 = readl(priv->base + PIPGM_PWMC(pwm->hwpwm));
+ pwmc0_clk = pwmc0 & PIPGM_PWMC_CLK_MASK;
+
+ state->period = (period << pwmc0_clk) * NSEC_PER_USEC;
+ state->duty_cycle = (duty << pwmc0_clk) * NSEC_PER_USEC;
+ if (pwmc0 & PIPGM_PWMC_POLARITY_MASK)
+ state->polarity = PWM_POLARITY_INVERSED;
+ else
+ state->polarity = PWM_POLARITY_NORMAL;
+
+ state->enabled = true;
+}
+
+static const struct pwm_ops visconti_pwm_ops = {
+ .apply = visconti_pwm_apply,
+ .get_state = visconti_pwm_get_state,
+ .owner = THIS_MODULE,
+};
+
+static int visconti_pwm_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct visconti_pwm_chip *priv;
+ int ret;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(priv->base))
+ return PTR_ERR(priv->base);
+
+ platform_set_drvdata(pdev, priv);
+
+ priv->chip.dev = dev;
+ priv->chip.ops = &visconti_pwm_ops;
+ priv->chip.npwm = 4;
+
+ ret = pwmchip_add(&priv->chip);
+ if (ret < 0)
+ return dev_err_probe(&pdev->dev, ret, "Cannot register visconti PWM\n");
+
+ return 0;
+}
+
+static int visconti_pwm_remove(struct platform_device *pdev)
+{
+ struct visconti_pwm_chip *priv = platform_get_drvdata(pdev);
+
+ pwmchip_remove(&priv->chip);
+
+ return 0;
+}
+
+static const struct of_device_id visconti_pwm_of_match[] = {
+ { .compatible = "toshiba,visconti-pwm", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, visconti_pwm_of_match);
+
+static struct platform_driver visconti_pwm_driver = {
+ .driver = {
+ .name = "pwm-visconti",
+ .of_match_table = visconti_pwm_of_match,
+ },
+ .probe = visconti_pwm_probe,
+ .remove = visconti_pwm_remove,
+};
+module_platform_driver(visconti_pwm_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>");
+MODULE_ALIAS("platform:pwm-visconti");
--
2.30.0.rc2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v6 2/2] pwm: visconti: Add Toshiba Visconti SoC PWM support
2021-04-19 0:00 ` Nobuhiro Iwamatsu
@ 2021-04-19 6:23 ` Uwe Kleine-König
-1 siblings, 0 replies; 12+ messages in thread
From: Uwe Kleine-König @ 2021-04-19 6:23 UTC (permalink / raw)
To: Nobuhiro Iwamatsu
Cc: Rob Herring, Thierry Reding, Lee Jones, devicetree, linux-pwm,
punit1.agrawal, yuji2.ishikawa, linux-arm-kernel, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 490 bytes --]
On Mon, Apr 19, 2021 at 09:00:07AM +0900, Nobuhiro Iwamatsu wrote:
> Add driver for the PWM controller on Toshiba Visconti ARM SoC.
>
> Signed-off-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Thanks for your endurance to improve the driver
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v6 2/2] pwm: visconti: Add Toshiba Visconti SoC PWM support
@ 2021-04-19 6:23 ` Uwe Kleine-König
0 siblings, 0 replies; 12+ messages in thread
From: Uwe Kleine-König @ 2021-04-19 6:23 UTC (permalink / raw)
To: Nobuhiro Iwamatsu
Cc: Rob Herring, Thierry Reding, Lee Jones, devicetree, linux-pwm,
punit1.agrawal, yuji2.ishikawa, linux-arm-kernel, linux-kernel
[-- Attachment #1.1: Type: text/plain, Size: 490 bytes --]
On Mon, Apr 19, 2021 at 09:00:07AM +0900, Nobuhiro Iwamatsu wrote:
> Add driver for the PWM controller on Toshiba Visconti ARM SoC.
>
> Signed-off-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Thanks for your endurance to improve the driver
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v6 0/2] pwm: visconti: Add Toshiba Visconti SoC PWM support
2021-04-19 0:00 ` Nobuhiro Iwamatsu
@ 2021-04-23 17:05 ` Thierry Reding
-1 siblings, 0 replies; 12+ messages in thread
From: Thierry Reding @ 2021-04-23 17:05 UTC (permalink / raw)
To: Nobuhiro Iwamatsu
Cc: Rob Herring, Uwe Kleine-König, Lee Jones, devicetree,
linux-pwm, punit1.agrawal, yuji2.ishikawa, linux-arm-kernel,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 4657 bytes --]
On Mon, Apr 19, 2021 at 09:00:05AM +0900, Nobuhiro Iwamatsu wrote:
> Hi,
>
> This series is the PWM driver for Toshiba's ARM SoC, Visconti[0].
> This provides DT binding documentation and device driver.
>
> [0]: https://toshiba.semicon-storage.com/ap-en/semiconductor/product/image-recognition-processors-visconti.html
>
> Updates:
>
> dt-bindings: pwm: Add bindings for Toshiba Visconti PWM Controller
> v5 -> v6:
> - No update.
> v4 -> v5:
> - No update.
> v3 -> v4:
> - No update.
> v2 -> v3:
> - Change compatible to toshiba,visconti-pwm
> - Change filename to toshiba,visconti-pwm.yaml.
> - Add Reviewed-by tag from Rob.
> v1 -> v2:
> - Change SPDX-License-Identifier to GPL-2.0-only OR BSD-2-Clause.
> - Set compatible toshiba,pwm-visconti only.
> - Drop unnecessary comments.
>
> pwm: visconti: Add Toshiba Visconti SoC PWM support
> v5 -> v6:
> - Update year in copyright.
> - Update limitations.
> - Fix coding style, used braces for both branches.
> v4 -> v5:
> - Droped checking PIPGM_PCSR from visconti_pwm_get_state.
> - Changed from to_visconti_chip to visconti_pwm_from_chip.
> - Removed pwmchip_remove return value management.
> - Add limitations of this device.
> - Add 'state->enabled = true' to visconti_pwm_get_state().
> v3 -> v4:
> - Sorted alphabetically include files.
> - Changed container_of to using static inline functions.
> - Dropped unnecessary dev_dbg().
> - Drop Initialization of chip.base.
> - Drop commnet "period too small".
> - Rebased for-next.
> v2 -> v3:
> - Change compatible to toshiba,visconti-pwm.
> - Fix MODULE_ALIAS to platform:pwm-visconti, again.
> - Align continuation line to the opening parenthesis.
> - Rewrite the contents of visconti_pwm_apply() based on the contents suggested by Uwe.
> v1 -> v2:
> - Change SPDX-License-Identifier to GPL-2.0-only.
> - Add prefix for the register defines.
> - Drop struct device from struct visconti_pwm_chip.
> - Use '>>' instead of '/'.
> - Drop error message by devm_platform_ioremap_resource().
> - Use dev_err_probe instead of dev_err.
> - Change dev_info to dev_dbg.
> - Remove some empty lines.
> - Fix MODULE_ALIAS to platform:pwm-visconti.
> - Add .get_state() function.
> - Use the author name and email address to MODULE_AUTHOR.
> - Add more comment to function of the hardware.
> - Support .get_status() function.
> - Use NSEC_PER_USEC instead of 1000.
> - Alphabetically sorted for Makefile and Kconfig.
> - Added check for set value in visconti_pwm_apply().
>
> Nobuhiro Iwamatsu (2):
> dt-bindings: pwm: Add bindings for Toshiba Visconti PWM Controller
> pwm: visconti: Add Toshiba Visconti SoC PWM support
>
> .../bindings/pwm/toshiba,pwm-visconti.yaml | 43 ++++
> drivers/pwm/Kconfig | 9 +
> drivers/pwm/Makefile | 1 +
> drivers/pwm/pwm-visconti.c | 189 ++++++++++++++++++
> 4 files changed, 242 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pwm/toshiba,pwm-visconti.yaml
> create mode 100644 drivers/pwm/pwm-visconti.c
Both patches applied, thanks.
checkpatch did complain when I applied:
> WARNING: please write a paragraph that describes the config symbol fully
> #9: FILE: drivers/pwm/Kconfig:604:
> +config PWM_VISCONTI
That seems a bit excessive. The paragraph is perhaps not a poster child
for Kconfig, but there are others that aren't better, so I think that's
fine.
> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> #32:
> new file mode 100644
Fine, too.
> WARNING: 'loosing' may be misspelled - perhaps 'losing'?
> #112: FILE: drivers/pwm/pwm-visconti.c:76:
> + * NSEC_PER_SEC / CLKFREQ = 1000 without loosing precision.
> ^^^^^^^
I've fixed that up while applying.
> WARNING: Avoid crashing the kernel - try using WARN_ON & recovery code rather than BUG() or BUG_ON()
> #127: FILE: drivers/pwm/pwm-visconti.c:91:
> + BUG_ON(pwmc0 > 3);
I think that one is legit. I've turned that into:
if (WARN_ON(pwmc0 > 3))
return -EINVAL;
so that requests for too big period will be rejected rather than crash
the system. Next time, please run checkpatch before submitting and
eliminate at least the big warnings.
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v6 0/2] pwm: visconti: Add Toshiba Visconti SoC PWM support
@ 2021-04-23 17:05 ` Thierry Reding
0 siblings, 0 replies; 12+ messages in thread
From: Thierry Reding @ 2021-04-23 17:05 UTC (permalink / raw)
To: Nobuhiro Iwamatsu
Cc: Rob Herring, Uwe Kleine-König, Lee Jones, devicetree,
linux-pwm, punit1.agrawal, yuji2.ishikawa, linux-arm-kernel,
linux-kernel
[-- Attachment #1.1: Type: text/plain, Size: 4657 bytes --]
On Mon, Apr 19, 2021 at 09:00:05AM +0900, Nobuhiro Iwamatsu wrote:
> Hi,
>
> This series is the PWM driver for Toshiba's ARM SoC, Visconti[0].
> This provides DT binding documentation and device driver.
>
> [0]: https://toshiba.semicon-storage.com/ap-en/semiconductor/product/image-recognition-processors-visconti.html
>
> Updates:
>
> dt-bindings: pwm: Add bindings for Toshiba Visconti PWM Controller
> v5 -> v6:
> - No update.
> v4 -> v5:
> - No update.
> v3 -> v4:
> - No update.
> v2 -> v3:
> - Change compatible to toshiba,visconti-pwm
> - Change filename to toshiba,visconti-pwm.yaml.
> - Add Reviewed-by tag from Rob.
> v1 -> v2:
> - Change SPDX-License-Identifier to GPL-2.0-only OR BSD-2-Clause.
> - Set compatible toshiba,pwm-visconti only.
> - Drop unnecessary comments.
>
> pwm: visconti: Add Toshiba Visconti SoC PWM support
> v5 -> v6:
> - Update year in copyright.
> - Update limitations.
> - Fix coding style, used braces for both branches.
> v4 -> v5:
> - Droped checking PIPGM_PCSR from visconti_pwm_get_state.
> - Changed from to_visconti_chip to visconti_pwm_from_chip.
> - Removed pwmchip_remove return value management.
> - Add limitations of this device.
> - Add 'state->enabled = true' to visconti_pwm_get_state().
> v3 -> v4:
> - Sorted alphabetically include files.
> - Changed container_of to using static inline functions.
> - Dropped unnecessary dev_dbg().
> - Drop Initialization of chip.base.
> - Drop commnet "period too small".
> - Rebased for-next.
> v2 -> v3:
> - Change compatible to toshiba,visconti-pwm.
> - Fix MODULE_ALIAS to platform:pwm-visconti, again.
> - Align continuation line to the opening parenthesis.
> - Rewrite the contents of visconti_pwm_apply() based on the contents suggested by Uwe.
> v1 -> v2:
> - Change SPDX-License-Identifier to GPL-2.0-only.
> - Add prefix for the register defines.
> - Drop struct device from struct visconti_pwm_chip.
> - Use '>>' instead of '/'.
> - Drop error message by devm_platform_ioremap_resource().
> - Use dev_err_probe instead of dev_err.
> - Change dev_info to dev_dbg.
> - Remove some empty lines.
> - Fix MODULE_ALIAS to platform:pwm-visconti.
> - Add .get_state() function.
> - Use the author name and email address to MODULE_AUTHOR.
> - Add more comment to function of the hardware.
> - Support .get_status() function.
> - Use NSEC_PER_USEC instead of 1000.
> - Alphabetically sorted for Makefile and Kconfig.
> - Added check for set value in visconti_pwm_apply().
>
> Nobuhiro Iwamatsu (2):
> dt-bindings: pwm: Add bindings for Toshiba Visconti PWM Controller
> pwm: visconti: Add Toshiba Visconti SoC PWM support
>
> .../bindings/pwm/toshiba,pwm-visconti.yaml | 43 ++++
> drivers/pwm/Kconfig | 9 +
> drivers/pwm/Makefile | 1 +
> drivers/pwm/pwm-visconti.c | 189 ++++++++++++++++++
> 4 files changed, 242 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pwm/toshiba,pwm-visconti.yaml
> create mode 100644 drivers/pwm/pwm-visconti.c
Both patches applied, thanks.
checkpatch did complain when I applied:
> WARNING: please write a paragraph that describes the config symbol fully
> #9: FILE: drivers/pwm/Kconfig:604:
> +config PWM_VISCONTI
That seems a bit excessive. The paragraph is perhaps not a poster child
for Kconfig, but there are others that aren't better, so I think that's
fine.
> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> #32:
> new file mode 100644
Fine, too.
> WARNING: 'loosing' may be misspelled - perhaps 'losing'?
> #112: FILE: drivers/pwm/pwm-visconti.c:76:
> + * NSEC_PER_SEC / CLKFREQ = 1000 without loosing precision.
> ^^^^^^^
I've fixed that up while applying.
> WARNING: Avoid crashing the kernel - try using WARN_ON & recovery code rather than BUG() or BUG_ON()
> #127: FILE: drivers/pwm/pwm-visconti.c:91:
> + BUG_ON(pwmc0 > 3);
I think that one is legit. I've turned that into:
if (WARN_ON(pwmc0 > 3))
return -EINVAL;
so that requests for too big period will be rejected rather than crash
the system. Next time, please run checkpatch before submitting and
eliminate at least the big warnings.
Thierry
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v6 0/2] pwm: visconti: Add Toshiba Visconti SoC PWM support
2021-04-23 17:05 ` Thierry Reding
@ 2021-04-23 17:20 ` Uwe Kleine-König
-1 siblings, 0 replies; 12+ messages in thread
From: Uwe Kleine-König @ 2021-04-23 17:20 UTC (permalink / raw)
To: Thierry Reding
Cc: Nobuhiro Iwamatsu, Rob Herring, Lee Jones, devicetree, linux-pwm,
punit1.agrawal, yuji2.ishikawa, linux-arm-kernel, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 5625 bytes --]
On Fri, Apr 23, 2021 at 07:05:35PM +0200, Thierry Reding wrote:
> On Mon, Apr 19, 2021 at 09:00:05AM +0900, Nobuhiro Iwamatsu wrote:
> > Hi,
> >
> > This series is the PWM driver for Toshiba's ARM SoC, Visconti[0].
> > This provides DT binding documentation and device driver.
> >
> > [0]: https://toshiba.semicon-storage.com/ap-en/semiconductor/product/image-recognition-processors-visconti.html
> >
> > Updates:
> >
> > dt-bindings: pwm: Add bindings for Toshiba Visconti PWM Controller
> > v5 -> v6:
> > - No update.
> > v4 -> v5:
> > - No update.
> > v3 -> v4:
> > - No update.
> > v2 -> v3:
> > - Change compatible to toshiba,visconti-pwm
> > - Change filename to toshiba,visconti-pwm.yaml.
> > - Add Reviewed-by tag from Rob.
> > v1 -> v2:
> > - Change SPDX-License-Identifier to GPL-2.0-only OR BSD-2-Clause.
> > - Set compatible toshiba,pwm-visconti only.
> > - Drop unnecessary comments.
> >
> > pwm: visconti: Add Toshiba Visconti SoC PWM support
> > v5 -> v6:
> > - Update year in copyright.
> > - Update limitations.
> > - Fix coding style, used braces for both branches.
> > v4 -> v5:
> > - Droped checking PIPGM_PCSR from visconti_pwm_get_state.
> > - Changed from to_visconti_chip to visconti_pwm_from_chip.
> > - Removed pwmchip_remove return value management.
> > - Add limitations of this device.
> > - Add 'state->enabled = true' to visconti_pwm_get_state().
> > v3 -> v4:
> > - Sorted alphabetically include files.
> > - Changed container_of to using static inline functions.
> > - Dropped unnecessary dev_dbg().
> > - Drop Initialization of chip.base.
> > - Drop commnet "period too small".
> > - Rebased for-next.
> > v2 -> v3:
> > - Change compatible to toshiba,visconti-pwm.
> > - Fix MODULE_ALIAS to platform:pwm-visconti, again.
> > - Align continuation line to the opening parenthesis.
> > - Rewrite the contents of visconti_pwm_apply() based on the contents suggested by Uwe.
> > v1 -> v2:
> > - Change SPDX-License-Identifier to GPL-2.0-only.
> > - Add prefix for the register defines.
> > - Drop struct device from struct visconti_pwm_chip.
> > - Use '>>' instead of '/'.
> > - Drop error message by devm_platform_ioremap_resource().
> > - Use dev_err_probe instead of dev_err.
> > - Change dev_info to dev_dbg.
> > - Remove some empty lines.
> > - Fix MODULE_ALIAS to platform:pwm-visconti.
> > - Add .get_state() function.
> > - Use the author name and email address to MODULE_AUTHOR.
> > - Add more comment to function of the hardware.
> > - Support .get_status() function.
> > - Use NSEC_PER_USEC instead of 1000.
> > - Alphabetically sorted for Makefile and Kconfig.
> > - Added check for set value in visconti_pwm_apply().
> >
> > Nobuhiro Iwamatsu (2):
> > dt-bindings: pwm: Add bindings for Toshiba Visconti PWM Controller
> > pwm: visconti: Add Toshiba Visconti SoC PWM support
> >
> > .../bindings/pwm/toshiba,pwm-visconti.yaml | 43 ++++
> > drivers/pwm/Kconfig | 9 +
> > drivers/pwm/Makefile | 1 +
> > drivers/pwm/pwm-visconti.c | 189 ++++++++++++++++++
> > 4 files changed, 242 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/pwm/toshiba,pwm-visconti.yaml
> > create mode 100644 drivers/pwm/pwm-visconti.c
>
> Both patches applied, thanks.
>
> checkpatch did complain when I applied:
>
> > WARNING: please write a paragraph that describes the config symbol fully
> > #9: FILE: drivers/pwm/Kconfig:604:
> > +config PWM_VISCONTI
>
> That seems a bit excessive. The paragraph is perhaps not a poster child
> for Kconfig, but there are others that aren't better, so I think that's
> fine.
>
> > WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> > #32:
> > new file mode 100644
>
> Fine, too.
>
> > WARNING: 'loosing' may be misspelled - perhaps 'losing'?
> > #112: FILE: drivers/pwm/pwm-visconti.c:76:
> > + * NSEC_PER_SEC / CLKFREQ = 1000 without loosing precision.
> > ^^^^^^^
>
> I've fixed that up while applying.
>
> > WARNING: Avoid crashing the kernel - try using WARN_ON & recovery code rather than BUG() or BUG_ON()
> > #127: FILE: drivers/pwm/pwm-visconti.c:91:
> > + BUG_ON(pwmc0 > 3);
>
> I think that one is legit. I've turned that into:
>
> if (WARN_ON(pwmc0 > 3))
> return -EINVAL;
>
> so that requests for too big period will be rejected rather than crash
> the system.
If this BUG_ON (or your if) triggers we have a compiler or memory
problem. The relevant parts of the code are:
if (state->period > (0xffff << 3) * 1000)
period = (0xffff << 3) * 1000;
else
period = state->period;
period /= 1000;
if (period > 0xffff) {
pwmc0 = ilog2(period >> 16);
BUG_ON(pwmc0 > 3);
Given that period is never bigger than 0xffff << 3 when it is used to
calculate the argument to ilog2, pwmc0 <= ilog2(7) = 2.
Hmm, I wonder if the formula is wrong given that pwmc0 never becomes 3?!
Should this better be
pwmc0 = fls(period >> 16);
?
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v6 0/2] pwm: visconti: Add Toshiba Visconti SoC PWM support
@ 2021-04-23 17:20 ` Uwe Kleine-König
0 siblings, 0 replies; 12+ messages in thread
From: Uwe Kleine-König @ 2021-04-23 17:20 UTC (permalink / raw)
To: Thierry Reding
Cc: Nobuhiro Iwamatsu, Rob Herring, Lee Jones, devicetree, linux-pwm,
punit1.agrawal, yuji2.ishikawa, linux-arm-kernel, linux-kernel
[-- Attachment #1.1: Type: text/plain, Size: 5625 bytes --]
On Fri, Apr 23, 2021 at 07:05:35PM +0200, Thierry Reding wrote:
> On Mon, Apr 19, 2021 at 09:00:05AM +0900, Nobuhiro Iwamatsu wrote:
> > Hi,
> >
> > This series is the PWM driver for Toshiba's ARM SoC, Visconti[0].
> > This provides DT binding documentation and device driver.
> >
> > [0]: https://toshiba.semicon-storage.com/ap-en/semiconductor/product/image-recognition-processors-visconti.html
> >
> > Updates:
> >
> > dt-bindings: pwm: Add bindings for Toshiba Visconti PWM Controller
> > v5 -> v6:
> > - No update.
> > v4 -> v5:
> > - No update.
> > v3 -> v4:
> > - No update.
> > v2 -> v3:
> > - Change compatible to toshiba,visconti-pwm
> > - Change filename to toshiba,visconti-pwm.yaml.
> > - Add Reviewed-by tag from Rob.
> > v1 -> v2:
> > - Change SPDX-License-Identifier to GPL-2.0-only OR BSD-2-Clause.
> > - Set compatible toshiba,pwm-visconti only.
> > - Drop unnecessary comments.
> >
> > pwm: visconti: Add Toshiba Visconti SoC PWM support
> > v5 -> v6:
> > - Update year in copyright.
> > - Update limitations.
> > - Fix coding style, used braces for both branches.
> > v4 -> v5:
> > - Droped checking PIPGM_PCSR from visconti_pwm_get_state.
> > - Changed from to_visconti_chip to visconti_pwm_from_chip.
> > - Removed pwmchip_remove return value management.
> > - Add limitations of this device.
> > - Add 'state->enabled = true' to visconti_pwm_get_state().
> > v3 -> v4:
> > - Sorted alphabetically include files.
> > - Changed container_of to using static inline functions.
> > - Dropped unnecessary dev_dbg().
> > - Drop Initialization of chip.base.
> > - Drop commnet "period too small".
> > - Rebased for-next.
> > v2 -> v3:
> > - Change compatible to toshiba,visconti-pwm.
> > - Fix MODULE_ALIAS to platform:pwm-visconti, again.
> > - Align continuation line to the opening parenthesis.
> > - Rewrite the contents of visconti_pwm_apply() based on the contents suggested by Uwe.
> > v1 -> v2:
> > - Change SPDX-License-Identifier to GPL-2.0-only.
> > - Add prefix for the register defines.
> > - Drop struct device from struct visconti_pwm_chip.
> > - Use '>>' instead of '/'.
> > - Drop error message by devm_platform_ioremap_resource().
> > - Use dev_err_probe instead of dev_err.
> > - Change dev_info to dev_dbg.
> > - Remove some empty lines.
> > - Fix MODULE_ALIAS to platform:pwm-visconti.
> > - Add .get_state() function.
> > - Use the author name and email address to MODULE_AUTHOR.
> > - Add more comment to function of the hardware.
> > - Support .get_status() function.
> > - Use NSEC_PER_USEC instead of 1000.
> > - Alphabetically sorted for Makefile and Kconfig.
> > - Added check for set value in visconti_pwm_apply().
> >
> > Nobuhiro Iwamatsu (2):
> > dt-bindings: pwm: Add bindings for Toshiba Visconti PWM Controller
> > pwm: visconti: Add Toshiba Visconti SoC PWM support
> >
> > .../bindings/pwm/toshiba,pwm-visconti.yaml | 43 ++++
> > drivers/pwm/Kconfig | 9 +
> > drivers/pwm/Makefile | 1 +
> > drivers/pwm/pwm-visconti.c | 189 ++++++++++++++++++
> > 4 files changed, 242 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/pwm/toshiba,pwm-visconti.yaml
> > create mode 100644 drivers/pwm/pwm-visconti.c
>
> Both patches applied, thanks.
>
> checkpatch did complain when I applied:
>
> > WARNING: please write a paragraph that describes the config symbol fully
> > #9: FILE: drivers/pwm/Kconfig:604:
> > +config PWM_VISCONTI
>
> That seems a bit excessive. The paragraph is perhaps not a poster child
> for Kconfig, but there are others that aren't better, so I think that's
> fine.
>
> > WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> > #32:
> > new file mode 100644
>
> Fine, too.
>
> > WARNING: 'loosing' may be misspelled - perhaps 'losing'?
> > #112: FILE: drivers/pwm/pwm-visconti.c:76:
> > + * NSEC_PER_SEC / CLKFREQ = 1000 without loosing precision.
> > ^^^^^^^
>
> I've fixed that up while applying.
>
> > WARNING: Avoid crashing the kernel - try using WARN_ON & recovery code rather than BUG() or BUG_ON()
> > #127: FILE: drivers/pwm/pwm-visconti.c:91:
> > + BUG_ON(pwmc0 > 3);
>
> I think that one is legit. I've turned that into:
>
> if (WARN_ON(pwmc0 > 3))
> return -EINVAL;
>
> so that requests for too big period will be rejected rather than crash
> the system.
If this BUG_ON (or your if) triggers we have a compiler or memory
problem. The relevant parts of the code are:
if (state->period > (0xffff << 3) * 1000)
period = (0xffff << 3) * 1000;
else
period = state->period;
period /= 1000;
if (period > 0xffff) {
pwmc0 = ilog2(period >> 16);
BUG_ON(pwmc0 > 3);
Given that period is never bigger than 0xffff << 3 when it is used to
calculate the argument to ilog2, pwmc0 <= ilog2(7) = 2.
Hmm, I wonder if the formula is wrong given that pwmc0 never becomes 3?!
Should this better be
pwmc0 = fls(period >> 16);
?
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 12+ messages in thread