* [PATCH v2 0/5] Add PWM for MStar SoCs @ 2022-09-07 13:12 Romain Perier 2022-09-07 13:12 ` [PATCH v2 1/5] dt-bindings: pwm: Add Mstar MSC313e PWM devicetree bindings documentation Romain Perier ` (5 more replies) 0 siblings, 6 replies; 15+ messages in thread From: Romain Perier @ 2022-09-07 13:12 UTC (permalink / raw) To: Thierry Reding, Lee Jones, Daniel Palmer, Romain Perier, Rob Herring Cc: linux-arm-kernel, devicetree, linux-pwm, linux-kernel 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. Changes since v1: - Fixed commit message for the dt-bindings doc - Removed "OneOf" from the dt-bindings doc - Re-ordered alphabetically in Kconfig and remove unseless empty lines - Explain and adds comment in _writecounter() (hw constrainst) - Reworked the msc313e_pwm_config() function - Fixed clk handling - Removed extra callbacks, only keep .apply and .get_state - Implement .get_state completly, this fixes the driver with PWM_DEBUG (the whole driver has been tested with PWM_DEBUG). - Dropped useless lines in _probe - I have kept regmap_field() because it is more clean and helpful, it avoids to do too much of offset and mask and shift all over the place. 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 | 46 +++ 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 | 269 ++++++++++++++++++ 8 files changed, 355 insertions(+) create mode 100644 Documentation/devicetree/bindings/pwm/mstar,msc313e-pwm.yaml create mode 100644 drivers/pwm/pwm-msc313e.c -- 2.35.1 _______________________________________________ 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] 15+ messages in thread
* [PATCH v2 1/5] dt-bindings: pwm: Add Mstar MSC313e PWM devicetree bindings documentation 2022-09-07 13:12 [PATCH v2 0/5] Add PWM for MStar SoCs Romain Perier @ 2022-09-07 13:12 ` Romain Perier 2022-09-12 20:33 ` Rob Herring 2022-10-28 23:33 ` Krzysztof Kozlowski 2022-09-07 13:12 ` [PATCH v2 2/5] pwm: Add support for the MSTAR MSC313 PWM Romain Perier ` (4 subsequent siblings) 5 siblings, 2 replies; 15+ messages in thread From: Romain Perier @ 2022-09-07 13:12 UTC (permalink / raw) To: Thierry Reding, Lee Jones, Daniel Palmer, Romain Perier, Rob Herring Cc: linux-arm-kernel, devicetree, linux-pwm, linux-kernel This adds the documentation for the devicetree bindings of the Mstar MSC313e PWM driver, it includes MSC313e SoCs and SSD20xd. Signed-off-by: Romain Perier <romain.perier@gmail.com> --- .../bindings/pwm/mstar,msc313e-pwm.yaml | 46 +++++++++++++++++++ 1 file changed, 46 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..07f3f576f21b --- /dev/null +++ b/Documentation/devicetree/bindings/pwm/mstar,msc313e-pwm.yaml @@ -0,0 +1,46 @@ +# 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 <daniel@0x0f.com> + - Romain Perier <romain.perier@gmail.com> + +properties: + compatible: + 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 _______________________________________________ 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] 15+ messages in thread
* Re: [PATCH v2 1/5] dt-bindings: pwm: Add Mstar MSC313e PWM devicetree bindings documentation 2022-09-07 13:12 ` [PATCH v2 1/5] dt-bindings: pwm: Add Mstar MSC313e PWM devicetree bindings documentation Romain Perier @ 2022-09-12 20:33 ` Rob Herring 2022-10-28 23:33 ` Krzysztof Kozlowski 1 sibling, 0 replies; 15+ messages in thread From: Rob Herring @ 2022-09-12 20:33 UTC (permalink / raw) To: Romain Perier Cc: Thierry Reding, Lee Jones, devicetree, Daniel Palmer, Rob Herring, linux-arm-kernel, linux-pwm, linux-kernel On Wed, 07 Sep 2022 15:12:37 +0200, Romain Perier wrote: > This adds the documentation for the devicetree bindings of the Mstar > MSC313e PWM driver, it includes MSC313e SoCs and SSD20xd. > > Signed-off-by: Romain Perier <romain.perier@gmail.com> > --- > .../bindings/pwm/mstar,msc313e-pwm.yaml | 46 +++++++++++++++++++ > 1 file changed, 46 insertions(+) > create mode 100644 Documentation/devicetree/bindings/pwm/mstar,msc313e-pwm.yaml > Reviewed-by: Rob Herring <robh@kernel.org> _______________________________________________ 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] 15+ messages in thread
* Re: [PATCH v2 1/5] dt-bindings: pwm: Add Mstar MSC313e PWM devicetree bindings documentation 2022-09-07 13:12 ` [PATCH v2 1/5] dt-bindings: pwm: Add Mstar MSC313e PWM devicetree bindings documentation Romain Perier 2022-09-12 20:33 ` Rob Herring @ 2022-10-28 23:33 ` Krzysztof Kozlowski 1 sibling, 0 replies; 15+ messages in thread From: Krzysztof Kozlowski @ 2022-10-28 23:33 UTC (permalink / raw) To: Romain Perier, Thierry Reding, Lee Jones, Daniel Palmer, Rob Herring Cc: linux-arm-kernel, devicetree, linux-pwm, linux-kernel On 07/09/2022 09:12, Romain Perier wrote: > This adds the documentation for the devicetree bindings of the Mstar > MSC313e PWM driver, it includes MSC313e SoCs and SSD20xd. > > Signed-off-by: Romain Perier <romain.perier@gmail.com> > --- > .../bindings/pwm/mstar,msc313e-pwm.yaml | 46 +++++++++++++++++++ > 1 file changed, 46 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..07f3f576f21b > --- /dev/null > +++ b/Documentation/devicetree/bindings/pwm/mstar,msc313e-pwm.yaml > @@ -0,0 +1,46 @@ > +# 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 <daniel@0x0f.com> > + - Romain Perier <romain.perier@gmail.com> > + > +properties: > + compatible: > + items: Drop items, you have only one item. Would be nice if you used Linux tools for getting list of people to Cc... Best regards, Krzysztof _______________________________________________ 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] 15+ messages in thread
* [PATCH v2 2/5] pwm: Add support for the MSTAR MSC313 PWM 2022-09-07 13:12 [PATCH v2 0/5] Add PWM for MStar SoCs Romain Perier 2022-09-07 13:12 ` [PATCH v2 1/5] dt-bindings: pwm: Add Mstar MSC313e PWM devicetree bindings documentation Romain Perier @ 2022-09-07 13:12 ` Romain Perier 2022-09-27 16:33 ` Uwe Kleine-König 2022-09-07 13:12 ` [PATCH v2 3/5] ARM: dts: mstar: Add pwm device node to infinity Romain Perier ` (3 subsequent siblings) 5 siblings, 1 reply; 15+ messages in thread From: Romain Perier @ 2022-09-07 13:12 UTC (permalink / raw) To: Thierry Reding, Lee Jones, Daniel Palmer, Romain Perier, Rob Herring Cc: linux-arm-kernel, devicetree, linux-pwm, linux-kernel From: Daniel Palmer <daniel@0x0f.com> This adds support for the PWM block on the Mstar MSC313e SoCs and newer. Signed-off-by: Daniel Palmer <daniel@0x0f.com> Co-developed-by: Romain Perier <romain.perier@gmail.com> Signed-off-by: Romain Perier <romain.perier@gmail.com> --- MAINTAINERS | 1 + drivers/pwm/Kconfig | 9 ++ drivers/pwm/Makefile | 1 + drivers/pwm/pwm-msc313e.c | 269 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 280 insertions(+) create mode 100644 drivers/pwm/pwm-msc313e.c diff --git a/MAINTAINERS b/MAINTAINERS index 9d7f64dc0efe..c3b39b09097c 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2439,6 +2439,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 60d13a949bc5..8049fd03a821 100644 --- a/drivers/pwm/Kconfig +++ b/drivers/pwm/Kconfig @@ -372,6 +372,15 @@ config PWM_MESON To compile this driver as a module, choose M here: the module will be called pwm-meson. +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_MTK_DISP tristate "MediaTek display PWM driver" depends on ARCH_MEDIATEK || COMPILE_TEST diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile index 7bf1a29f02b8..bc285c054f2a 100644 --- a/drivers/pwm/Makefile +++ b/drivers/pwm/Makefile @@ -62,4 +62,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..a71f39ba66c3 --- /dev/null +++ b/drivers/pwm/pwm-msc313e.c @@ -0,0 +1,269 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2021 Daniel Palmer <daniel@thingy.jp> + * Copyright (C) 2022 Romain Perier <romain.perier@gmail.com> + */ + +#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) +{ + /* The bus that connects the CPU to the peripheral registers splits 32 bit registers into + * two 16bit registers placed 4 bytes apart. It's the hardware design they used. The counter + * we are about to write has this contrainst. + */ + regmap_field_write(low, value & 0xffff); + regmap_field_write(high, value >> 16); +} + +static void msc313e_pwm_readcounter(struct regmap_field *low, struct regmap_field *high, u32 *value) +{ + unsigned int val = 0; + + regmap_field_read(low, &val); + *value = val; + regmap_field_read(high, &val); + *value = (val << 16) | *value; +} + +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); + unsigned long long nspertick = DIV_ROUND_DOWN_ULL(NSEC_PER_SEC, clk_get_rate(pwm->clk)); + struct msc313e_pwm_channel *channel = &pwm->channels[device->hwpwm]; + unsigned long long div = 1; + + /* Fit the period into the period register by prescaling the clk */ + while (DIV_ROUND_DOWN_ULL(period_ns, nspertick) > 0x3ffff) { + div++; + if (div > (0xffff + 1)) { + /* Force clk div to the maximum allowed value */ + div = 0xffff; + break; + } + nspertick = DIV_ROUND_DOWN_ULL(nspertick, div); + } + + regmap_field_write(channel->clkdiv, div - 1); + msc313e_pwm_writecounter(channel->dutyl, channel->dutyh, + DIV_ROUND_DOWN_ULL(duty_ns, nspertick)); + msc313e_pwm_writecounter(channel->periodl, channel->periodh, + DIV_ROUND_DOWN_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; + return regmap_field_write(channel->swrst, 0); +} + +static int 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]; + int ret; + + ret = regmap_field_write(channel->swrst, 1); + clk_disable_unprepare(pwm->clk); + return ret; +} + +static int msc313e_apply(struct pwm_chip *chip, struct pwm_device *pwm, + const struct pwm_state *state) +{ + int ret; + + if (state->enabled) { + if (!pwm->state.enabled) { + ret = msc313e_pwm_enable(chip, pwm); + if (ret) + return ret; + } + msc313e_pwm_set_polarity(chip, pwm, state->polarity); + msc313e_pwm_config(chip, pwm, state->duty_cycle, state->period); + } else if (pwm->state.enabled) { + ret = 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 long long nspertick = DIV_ROUND_DOWN_ULL(NSEC_PER_SEC, clk_get_rate(pwm->clk)); + unsigned int val = 0; + + regmap_field_read(channel->polarity, &val); + state->polarity = val ? PWM_POLARITY_INVERSED : PWM_POLARITY_NORMAL; + + regmap_field_read(channel->swrst, &val); + state->enabled = val == 0 ? true : false; + + msc313e_pwm_readcounter(channel->dutyl, channel->dutyh, &val); + state->duty_cycle = val * nspertick; + + msc313e_pwm_readcounter(channel->periodl, channel->periodh, &val); + state->period = val * nspertick; +} + +static const struct pwm_ops msc313e_pwm_ops = { + .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); + + /* Channels are enabled on boot, disable it until the pwm subsystem re-enable it + * explicitly + */ + regmap_field_write(pwm->channels[i].swrst, 1); + } + + pwm->pwmchip.dev = dev; + pwm->pwmchip.ops = &msc313e_pwm_ops; + pwm->pwmchip.npwm = match_data->channels; + + 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 <daniel@thingy.jp>"); -- 2.35.1 _______________________________________________ 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] 15+ messages in thread
* Re: [PATCH v2 2/5] pwm: Add support for the MSTAR MSC313 PWM 2022-09-07 13:12 ` [PATCH v2 2/5] pwm: Add support for the MSTAR MSC313 PWM Romain Perier @ 2022-09-27 16:33 ` Uwe Kleine-König 2022-09-28 12:47 ` Daniel Palmer 2022-10-27 8:36 ` Romain Perier 0 siblings, 2 replies; 15+ messages in thread From: Uwe Kleine-König @ 2022-09-27 16:33 UTC (permalink / raw) To: Romain Perier Cc: Thierry Reding, Lee Jones, Daniel Palmer, Rob Herring, Mark Brown, linux-arm-kernel, devicetree, linux-pwm, linux-kernel [-- Attachment #1.1: Type: text/plain, Size: 13853 bytes --] Hello Romain, hello Daniel, adding Mark Brown to Cc: for the regmap stuff. On Wed, Sep 07, 2022 at 03:12:38PM +0200, Romain Perier wrote: > From: Daniel Palmer <daniel@0x0f.com> > > This adds support for the PWM block on the Mstar MSC313e SoCs and newer. > > Signed-off-by: Daniel Palmer <daniel@0x0f.com> > Co-developed-by: Romain Perier <romain.perier@gmail.com> > Signed-off-by: Romain Perier <romain.perier@gmail.com> > --- > MAINTAINERS | 1 + > drivers/pwm/Kconfig | 9 ++ > drivers/pwm/Makefile | 1 + > drivers/pwm/pwm-msc313e.c | 269 ++++++++++++++++++++++++++++++++++++++ > 4 files changed, 280 insertions(+) > create mode 100644 drivers/pwm/pwm-msc313e.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 9d7f64dc0efe..c3b39b09097c 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -2439,6 +2439,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 60d13a949bc5..8049fd03a821 100644 > --- a/drivers/pwm/Kconfig > +++ b/drivers/pwm/Kconfig > @@ -372,6 +372,15 @@ config PWM_MESON > To compile this driver as a module, choose M here: the module > will be called pwm-meson. > > +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_MTK_DISP > tristate "MediaTek display PWM driver" > depends on ARCH_MEDIATEK || COMPILE_TEST > diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile > index 7bf1a29f02b8..bc285c054f2a 100644 > --- a/drivers/pwm/Makefile > +++ b/drivers/pwm/Makefile > @@ -62,4 +62,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..a71f39ba66c3 > --- /dev/null > +++ b/drivers/pwm/pwm-msc313e.c > @@ -0,0 +1,269 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2021 Daniel Palmer <daniel@thingy.jp> > + * Copyright (C) 2022 Romain Perier <romain.perier@gmail.com> > + */ > + > +#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) > +{ > + /* The bus that connects the CPU to the peripheral registers splits 32 bit registers into Please fix the comment style to use /* on a line for itself. Also for comments staying below 80 chars per line is appreciated. > + * two 16bit registers placed 4 bytes apart. It's the hardware design they used. The counter > + * we are about to write has this contrainst. s/contrainst/contraint/ I wonder if that could be abstracted by regmap?! > + */ > + regmap_field_write(low, value & 0xffff); > + regmap_field_write(high, value >> 16); > +} > + > +static void msc313e_pwm_readcounter(struct regmap_field *low, struct regmap_field *high, u32 *value) > +{ > + unsigned int val = 0; > + > + regmap_field_read(low, &val); > + *value = val; > + regmap_field_read(high, &val); > + *value = (val << 16) | *value; > +} > + > +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); > + unsigned long long nspertick = DIV_ROUND_DOWN_ULL(NSEC_PER_SEC, clk_get_rate(pwm->clk)); > + struct msc313e_pwm_channel *channel = &pwm->channels[device->hwpwm]; > + unsigned long long div = 1; > + > + /* Fit the period into the period register by prescaling the clk */ > + while (DIV_ROUND_DOWN_ULL(period_ns, nspertick) > 0x3ffff) { dividing by the result of a division looses precision. Also rounding down both divisions looks wrong. > + div++; > + if (div > (0xffff + 1)) { > + /* Force clk div to the maximum allowed value */ > + div = 0xffff; > + break; > + } > + nspertick = DIV_ROUND_DOWN_ULL(nspertick, div); > + } > + > + regmap_field_write(channel->clkdiv, div - 1); > + msc313e_pwm_writecounter(channel->dutyl, channel->dutyh, > + DIV_ROUND_DOWN_ULL(duty_ns, nspertick)); > + msc313e_pwm_writecounter(channel->periodl, channel->periodh, > + DIV_ROUND_DOWN_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; > + return regmap_field_write(channel->swrst, 0); > +} > + > +static int 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]; > + int ret; > + > + ret = regmap_field_write(channel->swrst, 1); > + clk_disable_unprepare(pwm->clk); > + return ret; > +} > + > +static int msc313e_apply(struct pwm_chip *chip, struct pwm_device *pwm, > + const struct pwm_state *state) > +{ > + int ret; > + > + if (state->enabled) { > + if (!pwm->state.enabled) { > + ret = msc313e_pwm_enable(chip, pwm); > + if (ret) > + return ret; > + } > + msc313e_pwm_set_polarity(chip, pwm, state->polarity); > + msc313e_pwm_config(chip, pwm, state->duty_cycle, state->period); Does the output emit a previous setting after msc313e_pwm_enable()? If so, please do the enable at the end. Does calling msc313e_pwm_set_polarity() and msc313e_pwm_config() have an immediate effect? Or is the currently running period completed? > + } else if (pwm->state.enabled) { > + ret = msc313e_pwm_disable(chip, pwm); How does the output behave on disable? Typical options are: - freezes where ever it currently is - goes to the inactive level Does it complete the currently running period? Please answer these in a "Limitations:" section similar to e.g. pwm-sl28cpld.c and pwm-iqs620a. > + } > + return 0; > +} > + > +static void msc313e_get_state(struct pwm_chip *chip, struct pwm_device *device, Please s/device/pwm/ to match the naming used in other drivers. > + struct pwm_state *state) > +{ > + struct msc313e_pwm *pwm = to_msc313e_pwm(chip); > + struct msc313e_pwm_channel *channel = &pwm->channels[device->hwpwm]; > + unsigned long long nspertick = DIV_ROUND_DOWN_ULL(NSEC_PER_SEC, clk_get_rate(pwm->clk)); > + unsigned int val = 0; > + > + regmap_field_read(channel->polarity, &val); > + state->polarity = val ? PWM_POLARITY_INVERSED : PWM_POLARITY_NORMAL; > + > + regmap_field_read(channel->swrst, &val); > + state->enabled = val == 0 ? true : false; > + > + msc313e_pwm_readcounter(channel->dutyl, channel->dutyh, &val); > + state->duty_cycle = val * nspertick; Please do the division at the end. Consider val = 333335 and clk_get_rate() returning 666666667. Then the exact duty cycle would be 500002.49974999874 but you would calculate nspertick = 1 and so duty_cycle = 333335. Also you need to round up. Did you test your driver with PWM_DEBUG enabled? The idea is that .get_state should return 500003 here to have apply ∘ get_state idempotent. As apply is supposed to round down to the next available setting, get_state has to round up. > + msc313e_pwm_readcounter(channel->periodl, channel->periodh, &val); > + state->period = val * nspertick; > +} > + > +static const struct pwm_ops msc313e_pwm_ops = { > + .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); Please don't use "pwm" as a name for your driver data variable. That name is reserved for struct pwm_device pointers. Something like "ddata" would be fine, but there is a wide variety in pwm driver land to choose from. > + 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); > + > + /* Channels are enabled on boot, disable it until the pwm subsystem re-enable it > + * explicitly You're supposed to keep the PWM running. (Consider a bootloader setting up a splash screen. You're probably killing the backlight here.) Note you need improved clk handling to do that right. > + */ > + regmap_field_write(pwm->channels[i].swrst, 1); > + } > + > + pwm->pwmchip.dev = dev; > + pwm->pwmchip.ops = &msc313e_pwm_ops; > + pwm->pwmchip.npwm = match_data->channels; > + > + platform_set_drvdata(pdev, pwm); This is unused, please 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 <daniel@thingy.jp>"); That address doesn't match the author of this patch. Is this on purpose? 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] 15+ messages in thread
* Re: [PATCH v2 2/5] pwm: Add support for the MSTAR MSC313 PWM 2022-09-27 16:33 ` Uwe Kleine-König @ 2022-09-28 12:47 ` Daniel Palmer 2022-10-27 8:36 ` Romain Perier 1 sibling, 0 replies; 15+ messages in thread From: Daniel Palmer @ 2022-09-28 12:47 UTC (permalink / raw) To: Uwe Kleine-König Cc: Romain Perier, Thierry Reding, Lee Jones, Rob Herring, Mark Brown, linux-arm-kernel, devicetree, linux-pwm, linux-kernel Hi Uwe, On Wed, 28 Sept 2022 at 01:33, Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > > Hello Romain, hello Daniel, Romain is doing the fixes to this so I'll let him answer the other bits but on all of the "what does the hardware do?" questions I'll have to test that on the hardware and follow up. The only documentation we have is very rough listings of registers and bits and not much else. > > > +MODULE_LICENSE("GPL v2"); > > +MODULE_DESCRIPTION("Mstar MSC313e PWM driver"); > > +MODULE_AUTHOR("Daniel Palmer <daniel@thingy.jp>"); > > That address doesn't match the author of this patch. Is this on purpose? My intention is to put all of the stuff for this MStar (Now Sigmastar) platform under this domain including the kernel, u-boot and PCB files so that at some point I can handover the whole thing to someone else or maybe a community that want to take over by giving them the domain. I should really set my email address in git for these commits to match but keep forgetting. Cheers, Daniel _______________________________________________ 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] 15+ messages in thread
* Re: [PATCH v2 2/5] pwm: Add support for the MSTAR MSC313 PWM 2022-09-27 16:33 ` Uwe Kleine-König 2022-09-28 12:47 ` Daniel Palmer @ 2022-10-27 8:36 ` Romain Perier 2022-10-27 9:22 ` Uwe Kleine-König 1 sibling, 1 reply; 15+ messages in thread From: Romain Perier @ 2022-10-27 8:36 UTC (permalink / raw) To: Uwe Kleine-König Cc: Thierry Reding, Lee Jones, Daniel Palmer, Rob Herring, Mark Brown, linux-arm-kernel, devicetree, linux-pwm, linux-kernel Hi, Le mar. 27 sept. 2022 à 18:33, Uwe Kleine-König <u.kleine-koenig@pengutronix.de> a écrit : > > Hello Romain, hello Daniel, > > adding Mark Brown to Cc: for the regmap stuff. > > On Wed, Sep 07, 2022 at 03:12:38PM +0200, Romain Perier wrote: > > From: Daniel Palmer <daniel@0x0f.com> > > > > This adds support for the PWM block on the Mstar MSC313e SoCs and newer. > > > > Signed-off-by: Daniel Palmer <daniel@0x0f.com> > > Co-developed-by: Romain Perier <romain.perier@gmail.com> > > Signed-off-by: Romain Perier <romain.perier@gmail.com> > > --- > > MAINTAINERS | 1 + > > drivers/pwm/Kconfig | 9 ++ > > drivers/pwm/Makefile | 1 + > > drivers/pwm/pwm-msc313e.c | 269 ++++++++++++++++++++++++++++++++++++++ > > 4 files changed, 280 insertions(+) > > create mode 100644 drivers/pwm/pwm-msc313e.c > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 9d7f64dc0efe..c3b39b09097c 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -2439,6 +2439,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 60d13a949bc5..8049fd03a821 100644 > > --- a/drivers/pwm/Kconfig > > +++ b/drivers/pwm/Kconfig > > @@ -372,6 +372,15 @@ config PWM_MESON > > To compile this driver as a module, choose M here: the module > > will be called pwm-meson. > > > > +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_MTK_DISP > > tristate "MediaTek display PWM driver" > > depends on ARCH_MEDIATEK || COMPILE_TEST > > diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile > > index 7bf1a29f02b8..bc285c054f2a 100644 > > --- a/drivers/pwm/Makefile > > +++ b/drivers/pwm/Makefile > > @@ -62,4 +62,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..a71f39ba66c3 > > --- /dev/null > > +++ b/drivers/pwm/pwm-msc313e.c > > @@ -0,0 +1,269 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Copyright (C) 2021 Daniel Palmer <daniel@thingy.jp> > > + * Copyright (C) 2022 Romain Perier <romain.perier@gmail.com> > > + */ > > + > > +#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) > > +{ > > + /* The bus that connects the CPU to the peripheral registers splits 32 bit registers into > > Please fix the comment style to use /* on a line for itself. Also for > comments staying below 80 chars per line is appreciated. even if check-patch.pl --strict passed ? ^^ > > > + * two 16bit registers placed 4 bytes apart. It's the hardware design they used. The counter > > + * we are about to write has this contrainst. > > s/contrainst/contraint/ > > I wonder if that could be abstracted by regmap?! I had the same thought, not from what I have read/found, but perhaps the regmap maintainer has an opinion. > > > + */ > > + regmap_field_write(low, value & 0xffff); > > + regmap_field_write(high, value >> 16); > > +} > > + > > +static void msc313e_pwm_readcounter(struct regmap_field *low, struct regmap_field *high, u32 *value) > > +{ > > + unsigned int val = 0; > > + > > + regmap_field_read(low, &val); > > + *value = val; > > + regmap_field_read(high, &val); > > + *value = (val << 16) | *value; > > +} > > + > > +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); > > + unsigned long long nspertick = DIV_ROUND_DOWN_ULL(NSEC_PER_SEC, clk_get_rate(pwm->clk)); > > + struct msc313e_pwm_channel *channel = &pwm->channels[device->hwpwm]; > > + unsigned long long div = 1; > > + > > + /* Fit the period into the period register by prescaling the clk */ > > + while (DIV_ROUND_DOWN_ULL(period_ns, nspertick) > 0x3ffff) { > > dividing by the result of a division looses precision. Also rounding > down both divisions looks wrong. Such cases are not supposed to be covered by PWM_DEBUG ? (because everything passed with PWM_DEBUG) > > > + div++; > > + if (div > (0xffff + 1)) { > > + /* Force clk div to the maximum allowed value */ > > + div = 0xffff; > > + break; > > + } > > + nspertick = DIV_ROUND_DOWN_ULL(nspertick, div); > > + } > > + > > + regmap_field_write(channel->clkdiv, div - 1); > > + msc313e_pwm_writecounter(channel->dutyl, channel->dutyh, > > + DIV_ROUND_DOWN_ULL(duty_ns, nspertick)); > > + msc313e_pwm_writecounter(channel->periodl, channel->periodh, > > + DIV_ROUND_DOWN_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; > > + return regmap_field_write(channel->swrst, 0); > > +} > > + > > +static int 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]; > > + int ret; > > + > > + ret = regmap_field_write(channel->swrst, 1); > > + clk_disable_unprepare(pwm->clk); > > + return ret; > > +} > > + > > +static int msc313e_apply(struct pwm_chip *chip, struct pwm_device *pwm, > > + const struct pwm_state *state) > > +{ > > + int ret; > > + > > + if (state->enabled) { > > + if (!pwm->state.enabled) { > > + ret = msc313e_pwm_enable(chip, pwm); > > + if (ret) > > + return ret; > > + } > > + msc313e_pwm_set_polarity(chip, pwm, state->polarity); > > + msc313e_pwm_config(chip, pwm, state->duty_cycle, state->period); > > Does the output emit a previous setting after msc313e_pwm_enable()? If > so, please do the enable at the end. > > Does calling msc313e_pwm_set_polarity() and msc313e_pwm_config() have an > immediate effect? Or is the currently running period completed? > > > + } else if (pwm->state.enabled) { > > + ret = msc313e_pwm_disable(chip, pwm); > > How does the output behave on disable? Typical options are: > > - freezes where ever it currently is > - goes to the inactive level I let Daniel answer these questions. > > Does it complete the currently running period? Please answer these in a > "Limitations:" section similar to e.g. pwm-sl28cpld.c and pwm-iqs620a. > > > + } > > + return 0; > > +} > > + > > +static void msc313e_get_state(struct pwm_chip *chip, struct pwm_device *device, > > Please s/device/pwm/ to match the naming used in other drivers. > > > + struct pwm_state *state) > > +{ > > + struct msc313e_pwm *pwm = to_msc313e_pwm(chip); > > + struct msc313e_pwm_channel *channel = &pwm->channels[device->hwpwm]; > > + unsigned long long nspertick = DIV_ROUND_DOWN_ULL(NSEC_PER_SEC, clk_get_rate(pwm->clk)); > > + unsigned int val = 0; > > + > > + regmap_field_read(channel->polarity, &val); > > + state->polarity = val ? PWM_POLARITY_INVERSED : PWM_POLARITY_NORMAL; > > + > > + regmap_field_read(channel->swrst, &val); > > + state->enabled = val == 0 ? true : false; > > + > > + msc313e_pwm_readcounter(channel->dutyl, channel->dutyh, &val); > > + state->duty_cycle = val * nspertick; > > Please do the division at the end. Consider val = 333335 and > clk_get_rate() returning 666666667. > > Then the exact duty cycle would be 500002.49974999874 but you would > calculate nspertick = 1 and so duty_cycle = 333335. > > Also you need to round up. Did you test your driver with PWM_DEBUG > enabled? Take a look at my cover letter, yeah I have tested everything with PWM_DEBUG, everything passed ^^ The idea is that .get_state should return 500003 here to have > apply ∘ get_state idempotent. As apply is supposed to round down to the > next available setting, get_state has to round up. > > > + msc313e_pwm_readcounter(channel->periodl, channel->periodh, &val); > > + state->period = val * nspertick; > > +} > > + > > +static const struct pwm_ops msc313e_pwm_ops = { > > + .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); > > Please don't use "pwm" as a name for your driver data variable. That > name is reserved for struct pwm_device pointers. Something like "ddata" > would be fine, but there is a wide variety in pwm driver land to choose > from. > > > + 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); > > + > > + /* Channels are enabled on boot, disable it until the pwm subsystem re-enable it > > + * explicitly > > You're supposed to keep the PWM running. (Consider a bootloader setting > up a splash screen. You're probably killing the backlight here.) Note > you need improved clk handling to do that right. > > > + */ > > + regmap_field_write(pwm->channels[i].swrst, 1); > > + } > > + > > + pwm->pwmchip.dev = dev; > > + pwm->pwmchip.ops = &msc313e_pwm_ops; > > + pwm->pwmchip.npwm = match_data->channels; > > + > > + platform_set_drvdata(pdev, pwm); > > This is unused, please 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 <daniel@thingy.jp>"); > > That address doesn't match the author of this patch. Is this on purpose? > > Best regards > Uwe Regards, Romain _______________________________________________ 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] 15+ messages in thread
* Re: [PATCH v2 2/5] pwm: Add support for the MSTAR MSC313 PWM 2022-10-27 8:36 ` Romain Perier @ 2022-10-27 9:22 ` Uwe Kleine-König 0 siblings, 0 replies; 15+ messages in thread From: Uwe Kleine-König @ 2022-10-27 9:22 UTC (permalink / raw) To: Romain Perier Cc: Thierry Reding, Lee Jones, Daniel Palmer, Rob Herring, Mark Brown, linux-arm-kernel, devicetree, linux-pwm, linux-kernel [-- Attachment #1.1: Type: text/plain, Size: 7467 bytes --] Hello Romain, On Thu, Oct 27, 2022 at 10:36:10AM +0200, Romain Perier wrote: > Le mar. 27 sept. 2022 à 18:33, Uwe Kleine-König > <u.kleine-koenig@pengutronix.de> a écrit : > > > > Hello Romain, hello Daniel, > > > > adding Mark Brown to Cc: for the regmap stuff. > > > > On Wed, Sep 07, 2022 at 03:12:38PM +0200, Romain Perier wrote: > > > From: Daniel Palmer <daniel@0x0f.com> > > > > > > This adds support for the PWM block on the Mstar MSC313e SoCs and newer. > > > > > > Signed-off-by: Daniel Palmer <daniel@0x0f.com> > > > Co-developed-by: Romain Perier <romain.perier@gmail.com> > > > Signed-off-by: Romain Perier <romain.perier@gmail.com> > > > --- > > > MAINTAINERS | 1 + > > > drivers/pwm/Kconfig | 9 ++ > > > drivers/pwm/Makefile | 1 + > > > drivers/pwm/pwm-msc313e.c | 269 ++++++++++++++++++++++++++++++++++++++ > > > 4 files changed, 280 insertions(+) > > > create mode 100644 drivers/pwm/pwm-msc313e.c > > > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > > index 9d7f64dc0efe..c3b39b09097c 100644 > > > --- a/MAINTAINERS > > > +++ b/MAINTAINERS > > > @@ -2439,6 +2439,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 60d13a949bc5..8049fd03a821 100644 > > > --- a/drivers/pwm/Kconfig > > > +++ b/drivers/pwm/Kconfig > > > @@ -372,6 +372,15 @@ config PWM_MESON > > > To compile this driver as a module, choose M here: the module > > > will be called pwm-meson. > > > > > > +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_MTK_DISP > > > tristate "MediaTek display PWM driver" > > > depends on ARCH_MEDIATEK || COMPILE_TEST > > > diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile > > > index 7bf1a29f02b8..bc285c054f2a 100644 > > > --- a/drivers/pwm/Makefile > > > +++ b/drivers/pwm/Makefile > > > @@ -62,4 +62,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..a71f39ba66c3 > > > --- /dev/null > > > +++ b/drivers/pwm/pwm-msc313e.c > > > @@ -0,0 +1,269 @@ > > > +// SPDX-License-Identifier: GPL-2.0 > > > +/* > > > + * Copyright (C) 2021 Daniel Palmer <daniel@thingy.jp> > > > + * Copyright (C) 2022 Romain Perier <romain.perier@gmail.com> > > > + */ > > > + > > > +#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) > > > +{ > > > + /* The bus that connects the CPU to the peripheral registers splits 32 bit registers into > > > > Please fix the comment style to use /* on a line for itself. Also for > > comments staying below 80 chars per line is appreciated. > > even if check-patch.pl --strict passed ? ^^ I also already wondered about check-patch not demanding this. *shrug* > > > + * two 16bit registers placed 4 bytes apart. It's the hardware design they used. The counter > > > + * we are about to write has this contrainst. > > > > s/contrainst/contraint/ > > > > I wonder if that could be abstracted by regmap?! > > I had the same thought, not from what I have read/found, but perhaps > the regmap maintainer has an opinion. > > > > > > + */ > > > + regmap_field_write(low, value & 0xffff); > > > + regmap_field_write(high, value >> 16); > > > +} > > > + > > > +static void msc313e_pwm_readcounter(struct regmap_field *low, struct regmap_field *high, u32 *value) > > > +{ > > > + unsigned int val = 0; > > > + > > > + regmap_field_read(low, &val); > > > + *value = val; > > > + regmap_field_read(high, &val); > > > + *value = (val << 16) | *value; > > > +} > > > + > > > +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); > > > + unsigned long long nspertick = DIV_ROUND_DOWN_ULL(NSEC_PER_SEC, clk_get_rate(pwm->clk)); > > > + struct msc313e_pwm_channel *channel = &pwm->channels[device->hwpwm]; > > > + unsigned long long div = 1; > > > + > > > + /* Fit the period into the period register by prescaling the clk */ > > > + while (DIV_ROUND_DOWN_ULL(period_ns, nspertick) > 0x3ffff) { > > > > dividing by the result of a division looses precision. Also rounding > > down both divisions looks wrong. > > Such cases are not supposed to be covered by PWM_DEBUG ? (because > everything passed with PWM_DEBUG) Note that PWM_DEBUG being silent isn't an indicator that everything is fine. It cannot catch everything and so doesn't replace human review. If you tell me what clk_get_rate() returns for you, I might be able to tell you a procedure that makes PWM_DEBUG unhappy. 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] 15+ messages in thread
* [PATCH v2 3/5] ARM: dts: mstar: Add pwm device node to infinity 2022-09-07 13:12 [PATCH v2 0/5] Add PWM for MStar SoCs Romain Perier 2022-09-07 13:12 ` [PATCH v2 1/5] dt-bindings: pwm: Add Mstar MSC313e PWM devicetree bindings documentation Romain Perier 2022-09-07 13:12 ` [PATCH v2 2/5] pwm: Add support for the MSTAR MSC313 PWM Romain Perier @ 2022-09-07 13:12 ` Romain Perier 2022-09-07 13:12 ` [PATCH v2 4/5] ARM: dts: mstar: Add pwm device node to infinity3 Romain Perier ` (2 subsequent siblings) 5 siblings, 0 replies; 15+ messages in thread From: Romain Perier @ 2022-09-07 13:12 UTC (permalink / raw) To: Thierry Reding, Lee Jones, Daniel Palmer, Romain Perier, Rob Herring Cc: linux-arm-kernel, devicetree, linux-pwm, linux-kernel 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 <romain.perier@gmail.com> --- 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 _______________________________________________ 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] 15+ messages in thread
* [PATCH v2 4/5] ARM: dts: mstar: Add pwm device node to infinity3 2022-09-07 13:12 [PATCH v2 0/5] Add PWM for MStar SoCs Romain Perier ` (2 preceding siblings ...) 2022-09-07 13:12 ` [PATCH v2 3/5] ARM: dts: mstar: Add pwm device node to infinity Romain Perier @ 2022-09-07 13:12 ` Romain Perier 2022-09-07 13:12 ` [PATCH v2 5/5] ARM: dts: mstar: Add pwm device node to infinity2m Romain Perier 2022-09-27 6:41 ` [PATCH v2 0/5] Add PWM for MStar SoCs Romain Perier 5 siblings, 0 replies; 15+ messages in thread From: Romain Perier @ 2022-09-07 13:12 UTC (permalink / raw) To: Thierry Reding, Lee Jones, Daniel Palmer, Romain Perier, Rob Herring Cc: linux-arm-kernel, devicetree, linux-pwm, linux-kernel 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 <romain.perier@gmail.com> --- 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 _______________________________________________ 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] 15+ messages in thread
* [PATCH v2 5/5] ARM: dts: mstar: Add pwm device node to infinity2m 2022-09-07 13:12 [PATCH v2 0/5] Add PWM for MStar SoCs Romain Perier ` (3 preceding siblings ...) 2022-09-07 13:12 ` [PATCH v2 4/5] ARM: dts: mstar: Add pwm device node to infinity3 Romain Perier @ 2022-09-07 13:12 ` Romain Perier 2022-09-27 6:41 ` [PATCH v2 0/5] Add PWM for MStar SoCs Romain Perier 5 siblings, 0 replies; 15+ messages in thread From: Romain Perier @ 2022-09-07 13:12 UTC (permalink / raw) To: Thierry Reding, Lee Jones, Daniel Palmer, Romain Perier, Rob Herring Cc: linux-arm-kernel, devicetree, linux-pwm, linux-kernel 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 <romain.perier@gmail.com> --- 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 _______________________________________________ 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] 15+ messages in thread
* Re: [PATCH v2 0/5] Add PWM for MStar SoCs 2022-09-07 13:12 [PATCH v2 0/5] Add PWM for MStar SoCs Romain Perier ` (4 preceding siblings ...) 2022-09-07 13:12 ` [PATCH v2 5/5] ARM: dts: mstar: Add pwm device node to infinity2m Romain Perier @ 2022-09-27 6:41 ` Romain Perier 2022-10-28 23:32 ` Krzysztof Kozlowski 5 siblings, 1 reply; 15+ messages in thread From: Romain Perier @ 2022-09-27 6:41 UTC (permalink / raw) To: Thierry Reding, Lee Jones, Daniel Palmer, Romain Perier, Uwe Kleine-König Cc: linux-arm-kernel, devicetree, linux-pwm, linux-kernel ping ;) Regards, Le mer. 7 sept. 2022 à 15:12, Romain Perier <romain.perier@gmail.com> a écrit : > > 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. > > Changes since v1: > - Fixed commit message for the dt-bindings doc > - Removed "OneOf" from the dt-bindings doc > - Re-ordered alphabetically in Kconfig and remove > unseless empty lines > - Explain and adds comment in _writecounter() (hw > constrainst) > - Reworked the msc313e_pwm_config() function > - Fixed clk handling > - Removed extra callbacks, only keep .apply and .get_state > - Implement .get_state completly, this fixes the driver with PWM_DEBUG > (the whole driver has been tested with PWM_DEBUG). > - Dropped useless lines in _probe > - I have kept regmap_field() because it is more clean and helpful, it > avoids to do too much of offset and mask and shift all over the place. > > 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 | 46 +++ > 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 | 269 ++++++++++++++++++ > 8 files changed, 355 insertions(+) > create mode 100644 Documentation/devicetree/bindings/pwm/mstar,msc313e-pwm.yaml > create mode 100644 drivers/pwm/pwm-msc313e.c > > -- > 2.35.1 > _______________________________________________ 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] 15+ messages in thread
* Re: [PATCH v2 0/5] Add PWM for MStar SoCs 2022-09-27 6:41 ` [PATCH v2 0/5] Add PWM for MStar SoCs Romain Perier @ 2022-10-28 23:32 ` Krzysztof Kozlowski [not found] ` <CABgxDo+2OXLy7-xEetQ4zzaPQbB4tbQ=WtdcU494Uo5xWpPkVg@mail.gmail.com> 0 siblings, 1 reply; 15+ messages in thread From: Krzysztof Kozlowski @ 2022-10-28 23:32 UTC (permalink / raw) To: Romain Perier, Thierry Reding, Lee Jones, Daniel Palmer, Uwe Kleine-König Cc: linux-arm-kernel, devicetree, linux-pwm, linux-kernel On 27/09/2022 02:41, Romain Perier wrote: > ping ;) > Please use scripts/get_maintainers.pl to get a list of necessary people and lists to CC. It might happen, that command when run on an older kernel, gives you outdated entries. Therefore please be sure you base your patches on recent Linux kernel. Best regards, Krzysztof _______________________________________________ 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] 15+ messages in thread
[parent not found: <CABgxDo+2OXLy7-xEetQ4zzaPQbB4tbQ=WtdcU494Uo5xWpPkVg@mail.gmail.com>]
* Re: [PATCH v2 0/5] Add PWM for MStar SoCs [not found] ` <CABgxDo+2OXLy7-xEetQ4zzaPQbB4tbQ=WtdcU494Uo5xWpPkVg@mail.gmail.com> @ 2022-11-02 14:06 ` Krzysztof Kozlowski 0 siblings, 0 replies; 15+ messages in thread From: Krzysztof Kozlowski @ 2022-11-02 14:06 UTC (permalink / raw) To: Romain Perier Cc: Thierry Reding, Lee Jones, Daniel Palmer, Uwe Kleine-König, linux-arm-kernel, devicetree, linux-pwm, Linux Kernel Mailing List On 29/10/2022 03:48, Romain Perier wrote: > Hi > > This is what I already do and what I do for every patch series, usually ^^ > And I send To: the maintainers and everyone else in Cc: , as the process or > other maintainers suggest. > > The command was run on 6.0 for the first v1 I think . > > I have forgot something ? You forgot to Cc at least me. I did not check other missing entries. Best regards, Krzysztof _______________________________________________ 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] 15+ messages in thread
end of thread, other threads:[~2022-11-02 14:07 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-09-07 13:12 [PATCH v2 0/5] Add PWM for MStar SoCs Romain Perier 2022-09-07 13:12 ` [PATCH v2 1/5] dt-bindings: pwm: Add Mstar MSC313e PWM devicetree bindings documentation Romain Perier 2022-09-12 20:33 ` Rob Herring 2022-10-28 23:33 ` Krzysztof Kozlowski 2022-09-07 13:12 ` [PATCH v2 2/5] pwm: Add support for the MSTAR MSC313 PWM Romain Perier 2022-09-27 16:33 ` Uwe Kleine-König 2022-09-28 12:47 ` Daniel Palmer 2022-10-27 8:36 ` Romain Perier 2022-10-27 9:22 ` Uwe Kleine-König 2022-09-07 13:12 ` [PATCH v2 3/5] ARM: dts: mstar: Add pwm device node to infinity Romain Perier 2022-09-07 13:12 ` [PATCH v2 4/5] ARM: dts: mstar: Add pwm device node to infinity3 Romain Perier 2022-09-07 13:12 ` [PATCH v2 5/5] ARM: dts: mstar: Add pwm device node to infinity2m Romain Perier 2022-09-27 6:41 ` [PATCH v2 0/5] Add PWM for MStar SoCs Romain Perier 2022-10-28 23:32 ` Krzysztof Kozlowski [not found] ` <CABgxDo+2OXLy7-xEetQ4zzaPQbB4tbQ=WtdcU494Uo5xWpPkVg@mail.gmail.com> 2022-11-02 14:06 ` Krzysztof Kozlowski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).