From: Lee Jones <lee.jones@linaro.org>
To: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Cc: robh+dt@kernel.org, mark.rutland@arm.com,
alexandre.torgue@st.com, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, thierry.reding@gmail.com,
linux-pwm@vger.kernel.org, jic23@kernel.org, knaack.h@gmx.de,
lars@metafoo.de, pmeerw@pmeerw.net, linux-iio@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, fabrice.gasnier@st.com,
gerald.baeza@st.com, arnaud.pouliquen@st.com,
linus.walleij@linaro.org, linaro-kernel@lists.linaro.org,
Benjamin Gaignard <benjamin.gaignard@st.com>
Subject: Re: [PATCH v3 2/7] MFD: add stm32 general purpose timer driver
Date: Fri, 2 Dec 2016 14:29:31 +0000 [thread overview]
Message-ID: <20161202142931.GP2683@dell> (raw)
In-Reply-To: <1480673842-20804-3-git-send-email-benjamin.gaignard@st.com>
On Fri, 02 Dec 2016, Benjamin Gaignard wrote:
> This hardware block could at used at same time for PWM generation
> and IIO timer for other IPs like DAC, ADC or other timers.
> PWM and IIO timer configuration are mixed in the same registers
> so we need a multi fonction driver to be able to share those registers.
>
> version 2:
> - rename driver "stm32-gptimer" to be align with SoC documentation
> - only keep one compatible
> - use of_platform_populate() instead of devm_mfd_add_devices()
>
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> ---
> drivers/mfd/Kconfig | 10 ++++++
> drivers/mfd/Makefile | 2 ++
> drivers/mfd/stm32-gptimer.c | 73 +++++++++++++++++++++++++++++++++++++++
> include/linux/mfd/stm32-gptimer.h | 62 +++++++++++++++++++++++++++++++++
> 4 files changed, 147 insertions(+)
> create mode 100644 drivers/mfd/stm32-gptimer.c
> create mode 100644 include/linux/mfd/stm32-gptimer.h
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index c6df644..e75abcb 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1607,6 +1607,15 @@ config MFD_STW481X
> in various ST Microelectronics and ST-Ericsson embedded
> Nomadik series.
>
> +config MFD_STM32_GP_TIMER
> + tristate "Support for STM32 General Purpose Timer"
> + select MFD_CORE
> + select REGMAP
> + depends on ARCH_STM32
> + depends on OF
"|| COMPILE_TEST"?
> + help
> + Select this option to enable stm32 general purpose timer
I can see that. Tell us more about the device and what it does.
s/stm32/STM32/
> menu "Multimedia Capabilities Port drivers"
> depends on ARCH_SA1100
>
> @@ -1644,4 +1653,5 @@ config MFD_VEXPRESS_SYSREG
> on the ARM Ltd. Versatile Express board.
>
> endmenu
> +
Please remove this change. It has nothing to do with the set.
> endif
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 9834e66..86353b9 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -211,3 +211,5 @@ obj-$(CONFIG_INTEL_SOC_PMIC) += intel-soc-pmic.o
> obj-$(CONFIG_MFD_MT6397) += mt6397-core.o
>
> obj-$(CONFIG_MFD_ALTERA_A10SR) += altera-a10sr.o
> +
> +obj-$(CONFIG_MFD_STM32_GP_TIMER) += stm32-gptimer.o
> diff --git a/drivers/mfd/stm32-gptimer.c b/drivers/mfd/stm32-gptimer.c
> new file mode 100644
> index 0000000..54fb95c
> --- /dev/null
> +++ b/drivers/mfd/stm32-gptimer.c
> @@ -0,0 +1,73 @@
> +/*
> + * stm32-gptimer.c
Swap this out for a description.
> + * Copyright (C) STMicroelectronics 2016
'\n'
> + * Author: Benjamin Gaignard <benjamin.gaignard@st.com> for STMicroelectronics.
> + * License terms: GNU General Public License (GPL), version 2
> + */
> +
> +#include <linux/module.h>
> +#include <linux/of_platform.h>
> +#include <linux/reset.h>
> +
> +#include <linux/mfd/stm32-gptimer.h>
> +
> +static const struct regmap_config stm32_gptimer_regmap_cfg = {
> + .reg_bits = 32,
> + .val_bits = 32,
> + .reg_stride = sizeof(u32),
> + .max_register = 0x400,
> + .fast_io = true,
> +};
> +
> +static int stm32_gptimer_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct stm32_gptimer_dev *mfd;
s/mfd/ddata/
> + struct resource *res;
> + void __iomem *mmio;
> +
> + mfd = devm_kzalloc(dev, sizeof(*mfd), GFP_KERNEL);
> + if (!mfd)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res)
> + return -ENOMEM;
> +
Remove this 3 lines.
devm_ioremap_resource() does the checking and error printing for you.
> + mmio = devm_ioremap_resource(dev, res);
> + if (IS_ERR(mmio))
> + return PTR_ERR(mmio);
> +
> + mfd->regmap = devm_regmap_init_mmio_clk(dev, "clk_int", mmio,
> + &stm32_gptimer_regmap_cfg);
> + if (IS_ERR(mfd->regmap))
> + return PTR_ERR(mfd->regmap);
> +
> + mfd->clk = devm_clk_get(dev, NULL);
> + if (IS_ERR(mfd->clk))
> + return PTR_ERR(mfd->clk);
> +
> + platform_set_drvdata(pdev, mfd);
> +
> + return of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
> +}
> +
> +static const struct of_device_id stm32_gptimer_of_match[] = {
> + {
> + .compatible = "st,stm32-gptimer",
> + },
One line:
{ .compatible = "st,stm32-gptimer" },
> +};
> +MODULE_DEVICE_TABLE(of, stm32_gptimer_of_match);
> +
> +static struct platform_driver stm32_gptimer_driver = {
> + .probe = stm32_gptimer_probe,
> + .driver = {
> + .name = "stm32-gptimer",
> + .of_match_table = stm32_gptimer_of_match,
> + },
Remove tabs before the '='s.
> +module_platform_driver(stm32_gptimer_driver);
> +
> +MODULE_DESCRIPTION("STMicroelectronics STM32 general purpose timer");
"General Purpose Timer"
> +MODULE_LICENSE("GPL");
"GPL v2"
> diff --git a/include/linux/mfd/stm32-gptimer.h b/include/linux/mfd/stm32-gptimer.h
> new file mode 100644
> index 0000000..f8c92de
> --- /dev/null
> +++ b/include/linux/mfd/stm32-gptimer.h
> @@ -0,0 +1,62 @@
> +/*
> + * stm32-gptimer.h
> + *
> + * Copyright (C) STMicroelectronics 2016
> + * Author: Benjamin Gaignard <benjamin.gaignard@st.com> for STMicroelectronics.
> + * License terms: GNU General Public License (GPL), version 2
> + */
Same comments as before.
> +#ifndef _LINUX_STM32_GPTIMER_H_
> +#define _LINUX_STM32_GPTIMER_H_
> +
> +#include <linux/clk.h>
> +#include <linux/regmap.h>
> +
> +#define TIM_CR1 0x00 /* Control Register 1 */
> +#define TIM_CR2 0x04 /* Control Register 2 */
> +#define TIM_SMCR 0x08 /* Slave mode control reg */
> +#define TIM_DIER 0x0C /* DMA/interrupt register */
> +#define TIM_SR 0x10 /* Status register */
> +#define TIM_EGR 0x14 /* Event Generation Reg */
> +#define TIM_CCMR1 0x18 /* Capt/Comp 1 Mode Reg */
> +#define TIM_CCMR2 0x1C /* Capt/Comp 2 Mode Reg */
> +#define TIM_CCER 0x20 /* Capt/Comp Enable Reg */
> +#define TIM_PSC 0x28 /* Prescaler */
> +#define TIM_ARR 0x2c /* Auto-Reload Register */
> +#define TIM_CCR1 0x34 /* Capt/Comp Register 1 */
> +#define TIM_CCR2 0x38 /* Capt/Comp Register 2 */
> +#define TIM_CCR3 0x3C /* Capt/Comp Register 3 */
> +#define TIM_CCR4 0x40 /* Capt/Comp Register 4 */
> +#define TIM_BDTR 0x44 /* Break and Dead-Time Reg */
> +
> +#define TIM_CR1_CEN BIT(0) /* Counter Enable */
> +#define TIM_CR1_ARPE BIT(7) /* Auto-reload Preload Ena */
> +#define TIM_CR2_MMS (BIT(4) | BIT(5) | BIT(6)) /* Master mode selection */
> +#define TIM_SMCR_SMS (BIT(0) | BIT(1) | BIT(2)) /* Slave mode selection */
> +#define TIM_SMCR_TS (BIT(4) | BIT(5) | BIT(6)) /* Trigger selection */
> +#define TIM_DIER_UIE BIT(0) /* Update interrupt */
> +#define TIM_SR_UIF BIT(0) /* Update interrupt flag */
> +#define TIM_EGR_UG BIT(0) /* Update Generation */
> +#define TIM_CCMR_PE BIT(3) /* Channel Preload Enable */
> +#define TIM_CCMR_M1 (BIT(6) | BIT(5)) /* Channel PWM Mode 1 */
> +#define TIM_CCER_CC1E BIT(0) /* Capt/Comp 1 out Ena */
> +#define TIM_CCER_CC1P BIT(1) /* Capt/Comp 1 Polarity */
> +#define TIM_CCER_CC1NE BIT(2) /* Capt/Comp 1N out Ena */
> +#define TIM_CCER_CC1NP BIT(3) /* Capt/Comp 1N Polarity */
> +#define TIM_CCER_CCXE (BIT(0) | BIT(4) | BIT(8) | BIT(12))
> +#define TIM_BDTR_BKE BIT(12) /* Break input enable */
> +#define TIM_BDTR_BKP BIT(13) /* Break input polarity */
> +#define TIM_BDTR_AOE BIT(14) /* Automatic Output Enable */
> +#define TIM_BDTR_MOE BIT(15) /* Main Output Enable */
> +
> +#define MAX_TIM_PSC 0xFFFF
> +
> +struct stm32_gptimer_dev {
Drop the "_dev" or replace with ddata.
> + /* Device data */
No need for this.
> + struct clk *clk;
> +
> + /* Registers mapping */
No need for this.
> + struct regmap *regmap;
> +};
> +
> +#endif
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
next prev parent reply other threads:[~2016-12-02 14:26 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-12-02 10:17 [PATCH v3 0/7] Add pwm and IIO timer drivers for stm32 Benjamin Gaignard
2016-12-02 10:17 ` [PATCH v3 1/7] MFD: add bindings for stm32 general purpose timer driver Benjamin Gaignard
2016-12-02 10:17 ` [PATCH v3 2/7] MFD: add " Benjamin Gaignard
2016-12-02 14:29 ` Lee Jones [this message]
2016-12-02 10:17 ` [PATCH v3 3/7] PWM: add pwm-stm32 DT bindings Benjamin Gaignard
2016-12-02 14:23 ` Lee Jones
2016-12-05 6:53 ` Thierry Reding
2016-12-05 8:35 ` Lee Jones
2016-12-05 10:50 ` Thierry Reding
2016-12-05 11:08 ` Benjamin Gaignard
2016-12-05 11:23 ` Thierry Reding
2016-12-05 12:12 ` Benjamin Gaignard
2016-12-05 12:21 ` Thierry Reding
2016-12-02 10:17 ` [PATCH v3 4/7] PWM: add pwm driver for stm32 plaftorm Benjamin Gaignard
2016-12-05 7:23 ` Thierry Reding
2016-12-05 8:31 ` Lee Jones
2016-12-05 11:02 ` Benjamin Gaignard
2016-12-05 11:30 ` Thierry Reding
2016-12-02 10:17 ` [PATCH v3 5/7] IIO: add bindings for stm32 timer trigger driver Benjamin Gaignard
2016-12-02 13:59 ` Lee Jones
2016-12-02 14:23 ` Benjamin Gaignard
2016-12-03 9:35 ` Jonathan Cameron
2016-12-02 10:17 ` [PATCH v3 6/7] IIO: add STM32 " Benjamin Gaignard
2016-12-02 13:57 ` Lee Jones
2016-12-02 10:17 ` [PATCH v3 7/7] ARM: dts: stm32: add stm32 general purpose timer driver in DT Benjamin Gaignard
2016-12-02 13:22 ` Lee Jones
2016-12-03 9:42 ` Jonathan Cameron
2016-12-05 8:54 ` Lee Jones
2016-12-05 16:09 ` Alexandre Torgue
2016-12-06 9:48 ` Lee Jones
2016-12-06 9:56 ` Alexandre Torgue
2016-12-06 12:54 ` Lee Jones
2016-12-02 13:41 ` Alexandre Torgue
2016-12-03 9:45 ` Jonathan Cameron
2016-12-03 9:32 ` [PATCH v3 0/7] Add pwm and IIO timer drivers for stm32 Jonathan Cameron
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20161202142931.GP2683@dell \
--to=lee.jones@linaro.org \
--cc=alexandre.torgue@st.com \
--cc=arnaud.pouliquen@st.com \
--cc=benjamin.gaignard@linaro.org \
--cc=benjamin.gaignard@st.com \
--cc=devicetree@vger.kernel.org \
--cc=fabrice.gasnier@st.com \
--cc=gerald.baeza@st.com \
--cc=jic23@kernel.org \
--cc=knaack.h@gmx.de \
--cc=lars@metafoo.de \
--cc=linaro-kernel@lists.linaro.org \
--cc=linus.walleij@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pwm@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=pmeerw@pmeerw.net \
--cc=robh+dt@kernel.org \
--cc=thierry.reding@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).