This patches series adds a new driver for the PWM found in the Mstar
MSC313e SoCs and newer. It adds a basic pwm driver, the corresponding
devicetree bindings and its documentation.
Daniel Palmer (1):
pwm: Add support for the MSTAR MSC313 PWM
Romain Perier (4):
dt-bindings: pwm: Add Mstar MSC313e PWM devicetree bindings
documentation
ARM: dts: mstar: Add pwm device node to infinity
ARM: dts: mstar: Add pwm device node to infinity3
ARM: dts: mstar: Add pwm device node to infinity2m
.../bindings/pwm/mstar,msc313e-pwm.yaml | 47 ++++
MAINTAINERS | 1 +
arch/arm/boot/dts/mstar-infinity.dtsi | 10 +
arch/arm/boot/dts/mstar-infinity2m.dtsi | 8 +
arch/arm/boot/dts/mstar-infinity3.dtsi | 10 +
drivers/pwm/Kconfig | 10 +
drivers/pwm/Makefile | 1 +
drivers/pwm/pwm-msc313e.c | 242 ++++++++++++++++++
8 files changed, 329 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pwm/mstar,msc313e-pwm.yaml
create mode 100644 drivers/pwm/pwm-msc313e.c
--
2.35.1
This adds definition of the pwm device node, infinity2m has its own
hardware variant, so use the one for ssd20xd.
Signed-off-by: Romain Perier <[email protected]>
---
arch/arm/boot/dts/mstar-infinity2m.dtsi | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/arm/boot/dts/mstar-infinity2m.dtsi b/arch/arm/boot/dts/mstar-infinity2m.dtsi
index 1b485efd7156..70561e512483 100644
--- a/arch/arm/boot/dts/mstar-infinity2m.dtsi
+++ b/arch/arm/boot/dts/mstar-infinity2m.dtsi
@@ -32,6 +32,14 @@ cpu1: cpu@1 {
};
&riu {
+ pwm: pwm@3400 {
+ compatible = "mstar,ssd20xd-pwm";
+ reg = <0x3400 0x400>;
+ #pwm-cells = <2>;
+ clocks = <&xtal_div2>;
+ status = "disabled";
+ };
+
smpctrl: smpctrl@204000 {
reg = <0x204000 0x200>;
status = "disabled";
--
2.35.1
This adds the definition of the pwm device node. The PWM being able to
work with the oscillator at 12Mhz for now, it shares the same xtal than
other devices (rtc or watchdog for instance).
Signed-off-by: Romain Perier <[email protected]>
---
arch/arm/boot/dts/mstar-infinity.dtsi | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/arm/boot/dts/mstar-infinity.dtsi b/arch/arm/boot/dts/mstar-infinity.dtsi
index 441a917b88ba..752f4c26b31c 100644
--- a/arch/arm/boot/dts/mstar-infinity.dtsi
+++ b/arch/arm/boot/dts/mstar-infinity.dtsi
@@ -38,6 +38,16 @@ opp-800000000 {
};
};
+&soc {
+ pm_pwm: pwm@1f001da0 {
+ compatible = "mstar,msc313-pwm";
+ reg = <0x1f001da0 0xc>;
+ #pwm-cells = <2>;
+ clocks = <&xtal_div2>;
+ status = "disabled";
+ };
+};
+
&cpu0 {
operating-points-v2 = <&cpu0_opp_table>;
};
--
2.35.1
From: Daniel Palmer <[email protected]>
This adds support for the PWM block on the Mstar MSC313e SoCs and newer.
Signed-off-by: Daniel Palmer <[email protected]>
Co-developed-by: Romain Perier <[email protected]>
Signed-off-by: Romain Perier <[email protected]>
---
MAINTAINERS | 1 +
drivers/pwm/Kconfig | 10 ++
drivers/pwm/Makefile | 1 +
drivers/pwm/pwm-msc313e.c | 242 ++++++++++++++++++++++++++++++++++++++
4 files changed, 254 insertions(+)
create mode 100644 drivers/pwm/pwm-msc313e.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 2316278d9db9..45d001643b93 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2389,6 +2389,7 @@ F: arch/arm/mach-mstar/
F: drivers/clk/mstar/
F: drivers/clocksource/timer-msc313e.c
F: drivers/gpio/gpio-msc313.c
+F: drivers/pwm/pwm-msc313e.c
F: drivers/rtc/rtc-msc313.c
F: drivers/watchdog/msc313e_wdt.c
F: include/dt-bindings/clock/mstar-*
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 904de8d61828..802573122b25 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -651,6 +651,16 @@ config PWM_VT8500
To compile this driver as a module, choose M here: the module
will be called pwm-vt8500.
+config PWM_MSC313E
+ tristate "MStar MSC313e PWM support"
+ depends on ARCH_MSTARV7 || COMPILE_TEST
+ help
+ Generic PWM framework driver for MSTAR MSC313e.
+
+ To compile this driver as a module, choose M here: the module
+ will be called pwm-msc313e.
+
+
config PWM_XILINX
tristate "Xilinx AXI Timer PWM support"
depends on OF_ADDRESS
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index 5c08bdb817b4..e24a48c78335 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -61,4 +61,5 @@ 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
+obj-$(CONFIG_PWM_MSC313E) += pwm-msc313e.o
obj-$(CONFIG_PWM_XILINX) += pwm-xilinx.o
diff --git a/drivers/pwm/pwm-msc313e.c b/drivers/pwm/pwm-msc313e.c
new file mode 100644
index 000000000000..f20419c6b9be
--- /dev/null
+++ b/drivers/pwm/pwm-msc313e.c
@@ -0,0 +1,242 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2021 Daniel Palmer <[email protected]>
+ * Copyright (C) 2022 Romain Perier <[email protected]>
+ */
+
+#include <linux/clk.h>
+#include <linux/of_device.h>
+#include <linux/pwm.h>
+#include <linux/regmap.h>
+
+#define DRIVER_NAME "msc313e-pwm"
+
+#define CHANNEL_OFFSET 0x80
+#define REG_DUTY 0x8
+#define REG_PERIOD 0x10
+#define REG_DIV 0x18
+#define REG_CTRL 0x1c
+#define REG_SWRST 0x1fc
+
+struct msc313e_pwm_channel {
+ struct regmap_field *clkdiv;
+ struct regmap_field *polarity;
+ struct regmap_field *dutyl;
+ struct regmap_field *dutyh;
+ struct regmap_field *periodl;
+ struct regmap_field *periodh;
+ struct regmap_field *swrst;
+};
+
+struct msc313e_pwm {
+ struct regmap *regmap;
+ struct pwm_chip pwmchip;
+ struct clk *clk;
+ struct msc313e_pwm_channel channels[];
+};
+
+struct msc313e_pwm_info {
+ unsigned int channels;
+};
+
+#define to_msc313e_pwm(ptr) container_of(ptr, struct msc313e_pwm, pwmchip)
+
+static const struct regmap_config msc313e_pwm_regmap_config = {
+ .reg_bits = 16,
+ .val_bits = 16,
+ .reg_stride = 4,
+};
+
+static const struct msc313e_pwm_info msc313e_data = {
+ .channels = 8,
+};
+
+static const struct msc313e_pwm_info ssd20xd_data = {
+ .channels = 4,
+};
+
+static void msc313e_pwm_writecounter(struct regmap_field *low, struct regmap_field *high, u32 value)
+{
+ regmap_field_write(low, value);
+ regmap_field_write(high, value >> 16);
+}
+
+static int msc313e_pwm_config(struct pwm_chip *chip, struct pwm_device *device,
+ int duty_ns, int period_ns)
+{
+ struct msc313e_pwm *pwm = to_msc313e_pwm(chip);
+ struct msc313e_pwm_channel *channel = &pwm->channels[device->hwpwm];
+ unsigned long long nspertick = DIV_ROUND_CLOSEST_ULL(NSEC_PER_SEC, clk_get_rate(pwm->clk));
+ unsigned long long div = 1;
+
+ /* fit the period into the period register by prescaling the clk */
+ while (DIV_ROUND_CLOSEST_ULL(period_ns, (nspertick = DIV_ROUND_CLOSEST_ULL(nspertick, div)))
+ > 0x3ffff){
+ div++;
+ if (div > (0xffff + 1)) {
+ dev_err(chip->dev, "Can't fit period into period register\n");
+ return -EINVAL;
+ }
+ }
+
+ regmap_field_write(channel->clkdiv, div - 1);
+ msc313e_pwm_writecounter(channel->dutyl, channel->dutyh,
+ DIV_ROUND_CLOSEST_ULL(duty_ns, nspertick));
+ msc313e_pwm_writecounter(channel->periodl, channel->periodh,
+ DIV_ROUND_CLOSEST_ULL(period_ns, nspertick));
+
+ return 0;
+};
+
+static int msc313e_pwm_set_polarity(struct pwm_chip *chip, struct pwm_device *device,
+ enum pwm_polarity polarity)
+{
+ struct msc313e_pwm *pwm = to_msc313e_pwm(chip);
+ struct msc313e_pwm_channel *channel = &pwm->channels[device->hwpwm];
+ unsigned int pol = 0;
+
+ if (polarity == PWM_POLARITY_INVERSED)
+ pol = 1;
+ regmap_field_update_bits(channel->polarity, 1, pol);
+
+ return 0;
+}
+
+static int msc313e_pwm_enable(struct pwm_chip *chip, struct pwm_device *device)
+{
+ struct msc313e_pwm *pwm = to_msc313e_pwm(chip);
+ struct msc313e_pwm_channel *channel = &pwm->channels[device->hwpwm];
+ int ret;
+
+ ret = clk_prepare_enable(pwm->clk);
+ if (ret)
+ return ret;
+
+ regmap_field_write(channel->swrst, 0);
+
+ return 0;
+}
+
+static void msc313e_pwm_disable(struct pwm_chip *chip, struct pwm_device *device)
+{
+ struct msc313e_pwm *pwm = to_msc313e_pwm(chip);
+ struct msc313e_pwm_channel *channel = &pwm->channels[device->hwpwm];
+
+ regmap_field_write(channel->swrst, 1);
+ clk_disable(pwm->clk);
+}
+
+static int msc313e_apply(struct pwm_chip *chip, struct pwm_device *pwm,
+ const struct pwm_state *state)
+{
+ if (state->enabled) {
+ msc313e_pwm_enable(chip, pwm);
+ msc313e_pwm_set_polarity(chip, pwm, state->polarity);
+ msc313e_pwm_config(chip, pwm, state->duty_cycle, state->period);
+ } else {
+ msc313e_pwm_disable(chip, pwm);
+ }
+ return 0;
+}
+
+static void msc313e_get_state(struct pwm_chip *chip, struct pwm_device *device,
+ struct pwm_state *state)
+{
+ struct msc313e_pwm *pwm = to_msc313e_pwm(chip);
+ struct msc313e_pwm_channel *channel = &pwm->channels[device->hwpwm];
+ unsigned int pol = 0;
+
+ regmap_field_read(channel->polarity, &pol);
+ state->polarity = pol;
+}
+
+static const struct pwm_ops msc313e_pwm_ops = {
+ .config = msc313e_pwm_config,
+ .set_polarity = msc313e_pwm_set_polarity,
+ .enable = msc313e_pwm_enable,
+ .disable = msc313e_pwm_disable,
+ .apply = msc313e_apply,
+ .get_state = msc313e_get_state,
+ .owner = THIS_MODULE
+};
+
+static int msc313e_pwm_probe(struct platform_device *pdev)
+{
+ const struct msc313e_pwm_info *match_data;
+ struct device *dev = &pdev->dev;
+ struct msc313e_pwm *pwm;
+ __iomem void *base;
+ int i;
+
+ match_data = of_device_get_match_data(dev);
+ if (!match_data)
+ return -EINVAL;
+
+ base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(base))
+ return PTR_ERR(base);
+
+ pwm = devm_kzalloc(dev, struct_size(pwm, channels, match_data->channels), GFP_KERNEL);
+ if (!pwm)
+ return -ENOMEM;
+
+ pwm->clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(pwm->clk))
+ return dev_err_probe(dev, PTR_ERR(pwm->clk), "Cannot get clk\n");
+
+ pwm->regmap = devm_regmap_init_mmio(dev, base, &msc313e_pwm_regmap_config);
+ if (IS_ERR(pwm->regmap))
+ return dev_err_probe(dev, PTR_ERR(pwm->regmap), "Cannot get regmap\n");
+
+ for (i = 0; i < match_data->channels; i++) {
+ unsigned int offset = CHANNEL_OFFSET * i;
+ struct reg_field div_clkdiv_field = REG_FIELD(offset + REG_DIV, 0, 7);
+ struct reg_field ctrl_polarity_field = REG_FIELD(offset + REG_CTRL, 4, 4);
+ struct reg_field dutyl_field = REG_FIELD(offset + REG_DUTY, 0, 15);
+ struct reg_field dutyh_field = REG_FIELD(offset + REG_DUTY + 4, 0, 2);
+ struct reg_field periodl_field = REG_FIELD(offset + REG_PERIOD, 0, 15);
+ struct reg_field periodh_field = REG_FIELD(offset + REG_PERIOD + 4, 0, 2);
+ struct reg_field swrst_field = REG_FIELD(REG_SWRST, i, i);
+
+ pwm->channels[i].clkdiv = devm_regmap_field_alloc(dev, pwm->regmap,
+ div_clkdiv_field);
+ pwm->channels[i].polarity = devm_regmap_field_alloc(dev, pwm->regmap,
+ ctrl_polarity_field);
+ pwm->channels[i].dutyl = devm_regmap_field_alloc(dev, pwm->regmap, dutyl_field);
+ pwm->channels[i].dutyh = devm_regmap_field_alloc(dev, pwm->regmap, dutyh_field);
+ pwm->channels[i].periodl = devm_regmap_field_alloc(dev, pwm->regmap, periodl_field);
+ pwm->channels[i].periodh = devm_regmap_field_alloc(dev, pwm->regmap, periodh_field);
+ pwm->channels[i].swrst = devm_regmap_field_alloc(dev, pwm->regmap, swrst_field);
+ }
+
+ pwm->pwmchip.dev = dev;
+ pwm->pwmchip.ops = &msc313e_pwm_ops;
+ pwm->pwmchip.base = -1;
+ pwm->pwmchip.npwm = match_data->channels;
+ pwm->pwmchip.of_xlate = of_pwm_xlate_with_flags;
+ pwm->pwmchip.of_pwm_n_cells = 3;
+
+ platform_set_drvdata(pdev, pwm);
+
+ return devm_pwmchip_add(dev, &pwm->pwmchip);
+}
+
+static const struct of_device_id msc313e_pwm_dt_ids[] = {
+ { .compatible = "mstar,msc313e-pwm", .data = &msc313e_data },
+ { .compatible = "mstar,ssd20xd-pwm", .data = &ssd20xd_data },
+ {},
+};
+MODULE_DEVICE_TABLE(of, msc313e_pwm_dt_ids);
+
+static struct platform_driver msc313e_pwm_driver = {
+ .probe = msc313e_pwm_probe,
+ .driver = {
+ .name = DRIVER_NAME,
+ .of_match_table = msc313e_pwm_dt_ids,
+ },
+};
+module_platform_driver(msc313e_pwm_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Mstar MSC313e PWM driver");
+MODULE_AUTHOR("Daniel Palmer <[email protected]>");
--
2.35.1
This adds the definition of the pwm device node. The PWM being able to
work with the oscillator at 12Mhz for now, it shares the same xtal than
other devices (rtc or watchdog for instance).
Signed-off-by: Romain Perier <[email protected]>
---
arch/arm/boot/dts/mstar-infinity3.dtsi | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/arm/boot/dts/mstar-infinity3.dtsi b/arch/arm/boot/dts/mstar-infinity3.dtsi
index a56cf29e5d82..aa26f25392d0 100644
--- a/arch/arm/boot/dts/mstar-infinity3.dtsi
+++ b/arch/arm/boot/dts/mstar-infinity3.dtsi
@@ -67,3 +67,13 @@ opp-1512000000 {
&imi {
reg = <0xa0000000 0x20000>;
};
+
+&riu {
+ pwm: pwm@3400 {
+ compatible = "mstar,msc313e-pwm";
+ reg = <0x3400 0x400>;
+ #pwm-cells = <2>;
+ clocks = <&xtal_div2>;
+ status = "disabled";
+ };
+};
--
2.35.1
This adds the documentation for the devicetree bindings of the Mstar
MSC313e RTC driver, it includes MSC313e SoCs and SSD20xd.
Signed-off-by: Romain Perier <[email protected]>
---
.../bindings/pwm/mstar,msc313e-pwm.yaml | 47 +++++++++++++++++++
1 file changed, 47 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pwm/mstar,msc313e-pwm.yaml
diff --git a/Documentation/devicetree/bindings/pwm/mstar,msc313e-pwm.yaml b/Documentation/devicetree/bindings/pwm/mstar,msc313e-pwm.yaml
new file mode 100644
index 000000000000..82f2357db085
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/mstar,msc313e-pwm.yaml
@@ -0,0 +1,47 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pwm/mstar,msc313e-pwm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Mstar MSC313e PWM controller
+
+allOf:
+ - $ref: "pwm.yaml#"
+
+maintainers:
+ - Daniel Palmer <[email protected]>
+ - Romain Perier <[email protected]>
+
+properties:
+ compatible:
+ oneOf:
+ - items:
+ - enum:
+ - mstar,msc313e-pwm
+ - mstar,ssd20xd-pwm
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ "#pwm-cells":
+ const: 2
+
+required:
+ - compatible
+ - reg
+ - clocks
+
+additionalProperties: false
+
+examples:
+ - |
+ pwm: pwm@3400 {
+ compatible = "mstar,msc313e-pwm";
+ reg = <0x3400 0x400>;
+ #pwm-cells = <2>;
+ clocks = <&xtal_div2>;
+ };
--
2.35.1
On Wed, 15 Jun 2022 09:08:09 +0200, Romain Perier wrote:
> This adds the documentation for the devicetree bindings of the Mstar
> MSC313e RTC driver, it includes MSC313e SoCs and SSD20xd.
>
> Signed-off-by: Romain Perier <[email protected]>
> ---
> .../bindings/pwm/mstar,msc313e-pwm.yaml | 47 +++++++++++++++++++
> 1 file changed, 47 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pwm/mstar,msc313e-pwm.yaml
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
./Documentation/devicetree/bindings/pwm/mstar,msc313e-pwm.yaml:20:9: [warning] wrong indentation: expected 10 but found 8 (indentation)
./Documentation/devicetree/bindings/pwm/mstar,msc313e-pwm.yaml:21:11: [warning] wrong indentation: expected 12 but found 10 (indentation)
dtschema/dtc warnings/errors:
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/patch/
This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit.
Hi,
Le mer. 15 juin 2022 à 15:16, Rob Herring <[email protected]> a écrit :
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
>
> pip3 install dtschema --upgrade
>
> Please check and re-submit.
>
Mhhh, not sure to have yamllint, I will check it and fix it.
Regards,
Romain
On Wed, Jun 15, 2022 at 09:08:09AM +0200, Romain Perier wrote:
> This adds the documentation for the devicetree bindings of the Mstar
> MSC313e RTC driver, it includes MSC313e SoCs and SSD20xd.
>
> Signed-off-by: Romain Perier <[email protected]>
> ---
> .../bindings/pwm/mstar,msc313e-pwm.yaml | 47 +++++++++++++++++++
> 1 file changed, 47 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pwm/mstar,msc313e-pwm.yaml
>
> diff --git a/Documentation/devicetree/bindings/pwm/mstar,msc313e-pwm.yaml b/Documentation/devicetree/bindings/pwm/mstar,msc313e-pwm.yaml
> new file mode 100644
> index 000000000000..82f2357db085
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pwm/mstar,msc313e-pwm.yaml
> @@ -0,0 +1,47 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pwm/mstar,msc313e-pwm.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Mstar MSC313e PWM controller
> +
> +allOf:
> + - $ref: "pwm.yaml#"
> +
> +maintainers:
> + - Daniel Palmer <[email protected]>
> + - Romain Perier <[email protected]>
> +
> +properties:
> + compatible:
> + oneOf:
Don't need oneOf with only 1 entry.
> + - items:
> + - enum:
> + - mstar,msc313e-pwm
> + - mstar,ssd20xd-pwm
> +
> + reg:
> + maxItems: 1
> +
> + clocks:
> + maxItems: 1
> +
> + "#pwm-cells":
> + const: 2
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + pwm: pwm@3400 {
> + compatible = "mstar,msc313e-pwm";
> + reg = <0x3400 0x400>;
> + #pwm-cells = <2>;
> + clocks = <&xtal_div2>;
> + };
> --
> 2.35.1
>
>
Hello,
On Wed, Jun 15, 2022 at 09:08:10AM +0200, Romain Perier wrote:
> From: Daniel Palmer <[email protected]>
>
> This adds support for the PWM block on the Mstar MSC313e SoCs and newer.
>
> Signed-off-by: Daniel Palmer <[email protected]>
> Co-developed-by: Romain Perier <[email protected]>
> Signed-off-by: Romain Perier <[email protected]>
> ---
> MAINTAINERS | 1 +
> drivers/pwm/Kconfig | 10 ++
> drivers/pwm/Makefile | 1 +
> drivers/pwm/pwm-msc313e.c | 242 ++++++++++++++++++++++++++++++++++++++
> 4 files changed, 254 insertions(+)
> create mode 100644 drivers/pwm/pwm-msc313e.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2316278d9db9..45d001643b93 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2389,6 +2389,7 @@ F: arch/arm/mach-mstar/
> F: drivers/clk/mstar/
> F: drivers/clocksource/timer-msc313e.c
> F: drivers/gpio/gpio-msc313.c
> +F: drivers/pwm/pwm-msc313e.c
> F: drivers/rtc/rtc-msc313.c
> F: drivers/watchdog/msc313e_wdt.c
> F: include/dt-bindings/clock/mstar-*
> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> index 904de8d61828..802573122b25 100644
> --- a/drivers/pwm/Kconfig
> +++ b/drivers/pwm/Kconfig
> @@ -651,6 +651,16 @@ config PWM_VT8500
> To compile this driver as a module, choose M here: the module
> will be called pwm-vt8500.
>
> +config PWM_MSC313E
> + tristate "MStar MSC313e PWM support"
> + depends on ARCH_MSTARV7 || COMPILE_TEST
> + help
> + Generic PWM framework driver for MSTAR MSC313e.
> +
> + To compile this driver as a module, choose M here: the module
> + will be called pwm-msc313e.
> +
> +
only one empty line between entries, and also please stick to alphabetic
ordering.
> config PWM_XILINX
> tristate "Xilinx AXI Timer PWM support"
> depends on OF_ADDRESS
> diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
> index 5c08bdb817b4..e24a48c78335 100644
> --- a/drivers/pwm/Makefile
> +++ b/drivers/pwm/Makefile
> @@ -61,4 +61,5 @@ 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
> +obj-$(CONFIG_PWM_MSC313E) += pwm-msc313e.o
> obj-$(CONFIG_PWM_XILINX) += pwm-xilinx.o
> diff --git a/drivers/pwm/pwm-msc313e.c b/drivers/pwm/pwm-msc313e.c
> new file mode 100644
> index 000000000000..f20419c6b9be
> --- /dev/null
> +++ b/drivers/pwm/pwm-msc313e.c
> @@ -0,0 +1,242 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2021 Daniel Palmer <[email protected]>
> + * Copyright (C) 2022 Romain Perier <[email protected]>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/of_device.h>
> +#include <linux/pwm.h>
> +#include <linux/regmap.h>
> +
> +#define DRIVER_NAME "msc313e-pwm"
> +
> +#define CHANNEL_OFFSET 0x80
> +#define REG_DUTY 0x8
> +#define REG_PERIOD 0x10
> +#define REG_DIV 0x18
> +#define REG_CTRL 0x1c
> +#define REG_SWRST 0x1fc
> +
> +struct msc313e_pwm_channel {
> + struct regmap_field *clkdiv;
> + struct regmap_field *polarity;
> + struct regmap_field *dutyl;
> + struct regmap_field *dutyh;
> + struct regmap_field *periodl;
> + struct regmap_field *periodh;
> + struct regmap_field *swrst;
> +};
> +
> +struct msc313e_pwm {
> + struct regmap *regmap;
> + struct pwm_chip pwmchip;
> + struct clk *clk;
> + struct msc313e_pwm_channel channels[];
> +};
> +
> +struct msc313e_pwm_info {
> + unsigned int channels;
> +};
> +
> +#define to_msc313e_pwm(ptr) container_of(ptr, struct msc313e_pwm, pwmchip)
> +
> +static const struct regmap_config msc313e_pwm_regmap_config = {
> + .reg_bits = 16,
> + .val_bits = 16,
> + .reg_stride = 4,
> +};
> +
> +static const struct msc313e_pwm_info msc313e_data = {
> + .channels = 8,
> +};
> +
> +static const struct msc313e_pwm_info ssd20xd_data = {
> + .channels = 4,
> +};
> +
> +static void msc313e_pwm_writecounter(struct regmap_field *low, struct regmap_field *high, u32 value)
> +{
> + regmap_field_write(low, value);
> + regmap_field_write(high, value >> 16);
Is this racy? E.g. if the hw is running and the low register overflows
before the high register is updated?
> +}
> +
> +static int msc313e_pwm_config(struct pwm_chip *chip, struct pwm_device *device,
> + int duty_ns, int period_ns)
> +{
> + struct msc313e_pwm *pwm = to_msc313e_pwm(chip);
> + struct msc313e_pwm_channel *channel = &pwm->channels[device->hwpwm];
> + unsigned long long nspertick = DIV_ROUND_CLOSEST_ULL(NSEC_PER_SEC, clk_get_rate(pwm->clk));
> + unsigned long long div = 1;
> +
> + /* fit the period into the period register by prescaling the clk */
> + while (DIV_ROUND_CLOSEST_ULL(period_ns, (nspertick = DIV_ROUND_CLOSEST_ULL(nspertick, div)))
> + > 0x3ffff){
Strange line breaking.
Dividing by a division is inexact, also please round down, not
round-closest.
Please test your driver with PWM_DEBUG enabled, and use something like I
proposed in
https://lore.kernel.org/linux-pwm/[email protected] .
> + div++;
> + if (div > (0xffff + 1)) {
> + dev_err(chip->dev, "Can't fit period into period register\n");
> + return -EINVAL;
> + }
If the requested period is too big, please configure the biggest
possible period.
Also .apply() shouldn't emit error messages as this might flood the
kernel log.
> + }
> +
> + regmap_field_write(channel->clkdiv, div - 1);
> + msc313e_pwm_writecounter(channel->dutyl, channel->dutyh,
> + DIV_ROUND_CLOSEST_ULL(duty_ns, nspertick));
> + msc313e_pwm_writecounter(channel->periodl, channel->periodh,
> + DIV_ROUND_CLOSEST_ULL(period_ns, nspertick));
> +
> + return 0;
> +};
> +
> +static int msc313e_pwm_set_polarity(struct pwm_chip *chip, struct pwm_device *device,
> + enum pwm_polarity polarity)
> +{
> + struct msc313e_pwm *pwm = to_msc313e_pwm(chip);
> + struct msc313e_pwm_channel *channel = &pwm->channels[device->hwpwm];
> + unsigned int pol = 0;
> +
> + if (polarity == PWM_POLARITY_INVERSED)
> + pol = 1;
> + regmap_field_update_bits(channel->polarity, 1, pol);
> +
> + return 0;
> +}
> +
> +static int msc313e_pwm_enable(struct pwm_chip *chip, struct pwm_device *device)
> +{
> + struct msc313e_pwm *pwm = to_msc313e_pwm(chip);
> + struct msc313e_pwm_channel *channel = &pwm->channels[device->hwpwm];
> + int ret;
> +
> + ret = clk_prepare_enable(pwm->clk);
> + if (ret)
> + return ret;
> +
> + regmap_field_write(channel->swrst, 0);
> +
> + return 0;
> +}
> +
> +static void msc313e_pwm_disable(struct pwm_chip *chip, struct pwm_device *device)
> +{
> + struct msc313e_pwm *pwm = to_msc313e_pwm(chip);
> + struct msc313e_pwm_channel *channel = &pwm->channels[device->hwpwm];
> +
> + regmap_field_write(channel->swrst, 1);
> + clk_disable(pwm->clk);
how does the hardware behave on disable? Does it emit the inactive
level? Or 0? Or does it freeze? Or high-Z? Please document that like
it's done e.g. in drivers/pwm/pwm-sl28cpld.c. Stick to the format used
there. (i.e. "Limitations:" + a list of hardware properties in the
toplevel comment.)
Does setting swrst ("software reset"?) reset the other registers?
> +}
> +
> +static int msc313e_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> + const struct pwm_state *state)
> +{
> + if (state->enabled) {
> + msc313e_pwm_enable(chip, pwm);
> + msc313e_pwm_set_polarity(chip, pwm, state->polarity);
> + msc313e_pwm_config(chip, pwm, state->duty_cycle, state->period);
If you enable at the end, you might prevent a glitch. I assume the
glitch isn't preventable in general?
Is the currently running period completed when a new configuration is
written to the registers?
As msc313e_pwm_enable calls clk_prepare_enable() unconditionally, and
it's valid to call pwm_apply several times in a row with state->enabled
= true, the clk calls are not balanced.
> + } else {
> + msc313e_pwm_disable(chip, pwm);
> + }
> + return 0;
> +}
> +
> +static void msc313e_get_state(struct pwm_chip *chip, struct pwm_device *device,
> + struct pwm_state *state)
> +{
> + struct msc313e_pwm *pwm = to_msc313e_pwm(chip);
> + struct msc313e_pwm_channel *channel = &pwm->channels[device->hwpwm];
> + unsigned int pol = 0;
> +
> + regmap_field_read(channel->polarity, &pol);
> + state->polarity = pol;
I'd prefer something like:
state->polarity = pol ? PWM_POLARITY_INVERSED : PWM_POLARITY_NORMAL;
to not hardcode the values of the PWM constants in the driver.
Also this is incomplete, you need to handle .duty_cycle, .period and
.enabled, too.
> +}
> +
> +static const struct pwm_ops msc313e_pwm_ops = {
> + .config = msc313e_pwm_config,
> + .set_polarity = msc313e_pwm_set_polarity,
> + .enable = msc313e_pwm_enable,
> + .disable = msc313e_pwm_disable,
Please drop these. If there is an apply functions, these are all unused.
> + .apply = msc313e_apply,
> + .get_state = msc313e_get_state,
> + .owner = THIS_MODULE
> +};
> +
> +static int msc313e_pwm_probe(struct platform_device *pdev)
> +{
> + const struct msc313e_pwm_info *match_data;
> + struct device *dev = &pdev->dev;
> + struct msc313e_pwm *pwm;
> + __iomem void *base;
> + int i;
> +
> + match_data = of_device_get_match_data(dev);
> + if (!match_data)
> + return -EINVAL;
> +
> + base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> + pwm = devm_kzalloc(dev, struct_size(pwm, channels, match_data->channels), GFP_KERNEL);
> + if (!pwm)
> + return -ENOMEM;
> +
> + pwm->clk = devm_clk_get(dev, NULL);
> + if (IS_ERR(pwm->clk))
> + return dev_err_probe(dev, PTR_ERR(pwm->clk), "Cannot get clk\n");
> +
> + pwm->regmap = devm_regmap_init_mmio(dev, base, &msc313e_pwm_regmap_config);
> + if (IS_ERR(pwm->regmap))
> + return dev_err_probe(dev, PTR_ERR(pwm->regmap), "Cannot get regmap\n");
> +
> + for (i = 0; i < match_data->channels; i++) {
> + unsigned int offset = CHANNEL_OFFSET * i;
> + struct reg_field div_clkdiv_field = REG_FIELD(offset + REG_DIV, 0, 7);
> + struct reg_field ctrl_polarity_field = REG_FIELD(offset + REG_CTRL, 4, 4);
> + struct reg_field dutyl_field = REG_FIELD(offset + REG_DUTY, 0, 15);
> + struct reg_field dutyh_field = REG_FIELD(offset + REG_DUTY + 4, 0, 2);
> + struct reg_field periodl_field = REG_FIELD(offset + REG_PERIOD, 0, 15);
> + struct reg_field periodh_field = REG_FIELD(offset + REG_PERIOD + 4, 0, 2);
> + struct reg_field swrst_field = REG_FIELD(REG_SWRST, i, i);
> +
> + pwm->channels[i].clkdiv = devm_regmap_field_alloc(dev, pwm->regmap,
> + div_clkdiv_field);
> + pwm->channels[i].polarity = devm_regmap_field_alloc(dev, pwm->regmap,
> + ctrl_polarity_field);
> + pwm->channels[i].dutyl = devm_regmap_field_alloc(dev, pwm->regmap, dutyl_field);
> + pwm->channels[i].dutyh = devm_regmap_field_alloc(dev, pwm->regmap, dutyh_field);
> + pwm->channels[i].periodl = devm_regmap_field_alloc(dev, pwm->regmap, periodl_field);
> + pwm->channels[i].periodh = devm_regmap_field_alloc(dev, pwm->regmap, periodh_field);
> + pwm->channels[i].swrst = devm_regmap_field_alloc(dev, pwm->regmap, swrst_field);
Huh, never saw something like that. Is that really easier than using
regmap_write()?
> + }
> +
> + pwm->pwmchip.dev = dev;
> + pwm->pwmchip.ops = &msc313e_pwm_ops;
> + pwm->pwmchip.base = -1;
Please drop this line
> + pwm->pwmchip.npwm = match_data->channels;
> + pwm->pwmchip.of_xlate = of_pwm_xlate_with_flags;
You can drop this, this is assigned by default in the pwmchip_add
function.
> + pwm->pwmchip.of_pwm_n_cells = 3;
I didn't double check, but if the dtb has #pwm-cells = <3> this isn't
needed.
> +
> + platform_set_drvdata(pdev, pwm);
This is unused -> drop.
> + return devm_pwmchip_add(dev, &pwm->pwmchip);
> +}
> +
> +static const struct of_device_id msc313e_pwm_dt_ids[] = {
> + { .compatible = "mstar,msc313e-pwm", .data = &msc313e_data },
> + { .compatible = "mstar,ssd20xd-pwm", .data = &ssd20xd_data },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, msc313e_pwm_dt_ids);
> +
> +static struct platform_driver msc313e_pwm_driver = {
> + .probe = msc313e_pwm_probe,
> + .driver = {
> + .name = DRIVER_NAME,
> + .of_match_table = msc313e_pwm_dt_ids,
> + },
> +};
> +module_platform_driver(msc313e_pwm_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Mstar MSC313e PWM driver");
> +MODULE_AUTHOR("Daniel Palmer <[email protected]>");
> --
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Hi,
Le dim. 19 juin 2022 à 23:35, Uwe Kleine-König
<[email protected]> a écrit :
> > diff --git a/drivers/pwm/pwm-msc313e.c b/drivers/pwm/pwm-msc313e.c
> > new file mode 100644
> > index 000000000000..f20419c6b9be
> > --- /dev/null
> > +++ b/drivers/pwm/pwm-msc313e.c
> > @@ -0,0 +1,242 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2021 Daniel Palmer <[email protected]>
> > + * Copyright (C) 2022 Romain Perier <[email protected]>
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/of_device.h>
> > +#include <linux/pwm.h>
> > +#include <linux/regmap.h>
> > +
> > +#define DRIVER_NAME "msc313e-pwm"
> > +
> > +#define CHANNEL_OFFSET 0x80
> > +#define REG_DUTY 0x8
> > +#define REG_PERIOD 0x10
> > +#define REG_DIV 0x18
> > +#define REG_CTRL 0x1c
> > +#define REG_SWRST 0x1fc
> > +
> > +struct msc313e_pwm_channel {
> > + struct regmap_field *clkdiv;
> > + struct regmap_field *polarity;
> > + struct regmap_field *dutyl;
> > + struct regmap_field *dutyh;
> > + struct regmap_field *periodl;
> > + struct regmap_field *periodh;
> > + struct regmap_field *swrst;
> > +};
> > +
> > +struct msc313e_pwm {
> > + struct regmap *regmap;
> > + struct pwm_chip pwmchip;
> > + struct clk *clk;
> > + struct msc313e_pwm_channel channels[];
> > +};
> > +
> > +struct msc313e_pwm_info {
> > + unsigned int channels;
> > +};
> > +
> > +#define to_msc313e_pwm(ptr) container_of(ptr, struct msc313e_pwm, pwmchip)
> > +
> > +static const struct regmap_config msc313e_pwm_regmap_config = {
> > + .reg_bits = 16,
> > + .val_bits = 16,
> > + .reg_stride = 4,
> > +};
> > +
> > +static const struct msc313e_pwm_info msc313e_data = {
> > + .channels = 8,
> > +};
> > +
> > +static const struct msc313e_pwm_info ssd20xd_data = {
> > + .channels = 4,
> > +};
> > +
> > +static void msc313e_pwm_writecounter(struct regmap_field *low, struct regmap_field *high, u32 value)
> > +{
> > + regmap_field_write(low, value);
> > + regmap_field_write(high, value >> 16);
>
> Is this racy? E.g. if the hw is running and the low register overflows
> before the high register is updated?
>
Ack, I am re-working most of the stuff you noticed. The problem with
this IP blocks (and others...) is we have close registers
but we only need to write or read 16 bits in each of these (it is
mainly reverse engineered from vendor source or runtime most of the
time) . You cannot really do a single read (except by doing an obscur
thing via readq ? ...)
. We had exactly the same issue with the rtc driver see [1]
1. https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/rtc/rtc-msc313.c#n50
.
Regards,
Romain
Hello Romain,
On Fri, Aug 19, 2022 at 03:12:56PM +0200, Romain Perier wrote:
> Le dim. 19 juin 2022 ? 23:35, Uwe Kleine-K?nig
> <[email protected]> a ?crit :
> > > +static void msc313e_pwm_writecounter(struct regmap_field *low, struct regmap_field *high, u32 value)
> > > +{
> > > + regmap_field_write(low, value);
> > > + regmap_field_write(high, value >> 16);
> >
> > Is this racy? E.g. if the hw is running and the low register overflows
> > before the high register is updated?
> >
>
> Ack, I am re-working most of the stuff you noticed. The problem with
> this IP blocks (and others...) is we have close registers
> but we only need to write or read 16 bits in each of these (it is
> mainly reverse engineered from vendor source or runtime most of the
> time) . You cannot really do a single read (except by doing an obscur
> thing via readq ? ...)
This is fine, but pleast document that in a comment.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |