* [PATCH V2 0/3] add iwdg2 support for stm32mp157c
@ 2018-06-20 7:53 Ludovic Barre
2018-06-20 7:53 ` [PATCH V2 1/3] watchdog: stm32: add pclk feature for stm32mp1 Ludovic Barre
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Ludovic Barre @ 2018-06-20 7:53 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck, Rob Herring
Cc: devicetree, Alexandre Torgue, linux-kernel, Ludovic Barre,
Maxime Coquelin, linux-arm-kernel, linux-watchdog
From: Ludovic Barre <ludovic.barre@st.com>
This patch series updates stm32_iwdg driver to manage config
by compatible. stm32mp1 config requires a pclk clock.
Ludovic Barre (3):
watchdog: stm32: add pclk feature for stm32mp1
ARM: dts: stm32: add iwdg2 support for stm32mp157c
ARM: dts: stm32: add iwdg2 support for stm32mp157c-ed1
.../devicetree/bindings/watchdog/st,stm32-iwdg.txt | 21 +++-
arch/arm/boot/dts/stm32mp157c-ed1.dts | 5 +
arch/arm/boot/dts/stm32mp157c.dtsi | 8 ++
drivers/watchdog/stm32_iwdg.c | 132 ++++++++++++++-------
4 files changed, 117 insertions(+), 49 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH V2 1/3] watchdog: stm32: add pclk feature for stm32mp1 2018-06-20 7:53 [PATCH V2 0/3] add iwdg2 support for stm32mp157c Ludovic Barre @ 2018-06-20 7:53 ` Ludovic Barre 2018-06-20 9:19 ` Guenter Roeck 2018-06-20 7:53 ` [PATCH V2 2/3] ARM: dts: stm32: add iwdg2 support for stm32mp157c Ludovic Barre ` (2 subsequent siblings) 3 siblings, 1 reply; 9+ messages in thread From: Ludovic Barre @ 2018-06-20 7:53 UTC (permalink / raw) To: Wim Van Sebroeck, Guenter Roeck, Rob Herring Cc: devicetree, Alexandre Torgue, linux-kernel, Ludovic Barre, Maxime Coquelin, linux-arm-kernel, linux-watchdog From: Ludovic Barre <ludovic.barre@st.com> This patch adds config data to manage specific properties by compatible. Adds stm32mp1 config which requires pclk clock. Signed-off-by: Ludovic Barre <ludovic.barre@st.com> --- .../devicetree/bindings/watchdog/st,stm32-iwdg.txt | 21 +++- drivers/watchdog/stm32_iwdg.c | 132 ++++++++++++++------- 2 files changed, 104 insertions(+), 49 deletions(-) diff --git a/Documentation/devicetree/bindings/watchdog/st,stm32-iwdg.txt b/Documentation/devicetree/bindings/watchdog/st,stm32-iwdg.txt index cc13b10a..f07f6d89 100644 --- a/Documentation/devicetree/bindings/watchdog/st,stm32-iwdg.txt +++ b/Documentation/devicetree/bindings/watchdog/st,stm32-iwdg.txt @@ -2,18 +2,31 @@ STM32 Independent WatchDoG (IWDG) --------------------------------- Required properties: -- compatible: "st,stm32-iwdg" -- reg: physical base address and length of the registers set for the device -- clocks: must contain a single entry describing the clock input +- compatible: Should be either "st,stm32-iwdg" or "st,stm32mp1-iwdg" +- reg: Physical base address and length of the registers set for the device +- clocks: Reference to the clock entry lsi. Additional pclk clock entry + is required only for st,stm32mp1-iwdg. +- clock-names: Name of the clocks used. + "lsi" for st,stm32-iwdg + "pclk", "lsi" for st,stm32mp1-iwdg Optional Properties: - timeout-sec: Watchdog timeout value in seconds. -Example: +Examples: iwdg: watchdog@40003000 { compatible = "st,stm32-iwdg"; reg = <0x40003000 0x400>; clocks = <&clk_lsi>; + clock-names = "lsi"; + timeout-sec = <32>; +}; + +iwdg: iwdg@5a002000 { + compatible = "st,stm32mp1-iwdg"; + reg = <0x5a002000 0x400>; + clocks = <&rcc IWDG2>, <&clk_lsi>; + clock-names = "pclk", "lsi"; timeout-sec = <32>; }; diff --git a/drivers/watchdog/stm32_iwdg.c b/drivers/watchdog/stm32_iwdg.c index c97ad56..fc96670 100644 --- a/drivers/watchdog/stm32_iwdg.c +++ b/drivers/watchdog/stm32_iwdg.c @@ -11,12 +11,13 @@ #include <linux/clk.h> #include <linux/delay.h> -#include <linux/kernel.h> -#include <linux/module.h> #include <linux/interrupt.h> #include <linux/io.h> #include <linux/iopoll.h> +#include <linux/kernel.h> +#include <linux/module.h> #include <linux/of.h> +#include <linux/of_device.h> #include <linux/platform_device.h> #include <linux/watchdog.h> @@ -54,11 +55,17 @@ #define TIMEOUT_US 100000 #define SLEEP_US 1000 +struct stm32_iwdg_config { + bool has_pclk; +}; + struct stm32_iwdg { - struct watchdog_device wdd; - void __iomem *regs; - struct clk *clk; - unsigned int rate; + struct watchdog_device wdd; + void __iomem *regs; + struct stm32_iwdg_config *config; + struct clk *clk_lsi; + struct clk *clk_pclk; + unsigned int rate; }; static inline u32 reg_read(void __iomem *base, u32 reg) @@ -133,6 +140,44 @@ static int stm32_iwdg_set_timeout(struct watchdog_device *wdd, return 0; } +static int stm32_iwdg_clk_init(struct platform_device *pdev, + struct stm32_iwdg *wdt) +{ + u32 ret; + + wdt->clk_lsi = devm_clk_get(&pdev->dev, "lsi"); + if (IS_ERR(wdt->clk_lsi)) { + dev_err(&pdev->dev, "Unable to get lsi clock\n"); + return PTR_ERR(wdt->clk_lsi); + } + + /* optional peripheral clock */ + if (wdt->config->has_pclk) { + wdt->clk_pclk = devm_clk_get(&pdev->dev, "pclk"); + if (IS_ERR(wdt->clk_pclk)) { + dev_err(&pdev->dev, "Unable to get pclk clock\n"); + return PTR_ERR(wdt->clk_pclk); + } + + ret = clk_prepare_enable(wdt->clk_pclk); + if (ret) { + dev_err(&pdev->dev, "Unable to prepare pclk clock\n"); + return ret; + } + } + + ret = clk_prepare_enable(wdt->clk_lsi); + if (ret) { + dev_err(&pdev->dev, "Unable to prepare lsi clock\n"); + clk_disable_unprepare(wdt->clk_pclk); + return ret; + } + + wdt->rate = clk_get_rate(wdt->clk_lsi); + + return 0; +} + static const struct watchdog_info stm32_iwdg_info = { .options = WDIOF_SETTIMEOUT | WDIOF_MAGICCLOSE | @@ -147,49 +192,50 @@ static const struct watchdog_ops stm32_iwdg_ops = { .set_timeout = stm32_iwdg_set_timeout, }; +static const struct stm32_iwdg_config stm32_iwdg_cfg = { + .has_pclk = false, +}; + +static const struct stm32_iwdg_config stm32mp1_iwdg_cfg = { + .has_pclk = true, +}; + +static const struct of_device_id stm32_iwdg_of_match[] = { + { .compatible = "st,stm32-iwdg", .data = &stm32_iwdg_cfg }, + { .compatible = "st,stm32mp1-iwdg", .data = &stm32mp1_iwdg_cfg }, + { /* end node */ } +}; +MODULE_DEVICE_TABLE(of, stm32_iwdg_of_match); + static int stm32_iwdg_probe(struct platform_device *pdev) { struct watchdog_device *wdd; + const struct of_device_id *match; struct stm32_iwdg *wdt; struct resource *res; - void __iomem *regs; - struct clk *clk; int ret; + match = of_match_device(stm32_iwdg_of_match, &pdev->dev); + if (!match || !match->data) + return -ENODEV; + + wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL); + if (!wdt) + return -ENOMEM; + + wdt->config = (struct stm32_iwdg_config *)match->data; + /* This is the timer base. */ res = platform_get_resource(pdev, IORESOURCE_MEM, 0); - regs = devm_ioremap_resource(&pdev->dev, res); - if (IS_ERR(regs)) { + wdt->regs = devm_ioremap_resource(&pdev->dev, res); + if (IS_ERR(wdt->regs)) { dev_err(&pdev->dev, "Could not get resource\n"); - return PTR_ERR(regs); + return PTR_ERR(wdt->regs); } - clk = devm_clk_get(&pdev->dev, NULL); - if (IS_ERR(clk)) { - dev_err(&pdev->dev, "Unable to get clock\n"); - return PTR_ERR(clk); - } - - ret = clk_prepare_enable(clk); - if (ret) { - dev_err(&pdev->dev, "Unable to prepare clock %p\n", clk); + ret = stm32_iwdg_clk_init(pdev, wdt); + if (ret) return ret; - } - - /* - * Allocate our watchdog driver data, which has the - * struct watchdog_device nested within it. - */ - wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL); - if (!wdt) { - ret = -ENOMEM; - goto err; - } - - /* Initialize struct stm32_iwdg. */ - wdt->regs = regs; - wdt->clk = clk; - wdt->rate = clk_get_rate(clk); /* Initialize struct watchdog_device. */ wdd = &wdt->wdd; @@ -217,7 +263,8 @@ static int stm32_iwdg_probe(struct platform_device *pdev) return 0; err: - clk_disable_unprepare(clk); + clk_disable_unprepare(wdt->clk_lsi); + clk_disable_unprepare(wdt->clk_pclk); return ret; } @@ -227,23 +274,18 @@ static int stm32_iwdg_remove(struct platform_device *pdev) struct stm32_iwdg *wdt = platform_get_drvdata(pdev); watchdog_unregister_device(&wdt->wdd); - clk_disable_unprepare(wdt->clk); + clk_disable_unprepare(wdt->clk_lsi); + clk_disable_unprepare(wdt->clk_pclk); return 0; } -static const struct of_device_id stm32_iwdg_of_match[] = { - { .compatible = "st,stm32-iwdg" }, - { /* end node */ } -}; -MODULE_DEVICE_TABLE(of, stm32_iwdg_of_match); - static struct platform_driver stm32_iwdg_driver = { .probe = stm32_iwdg_probe, .remove = stm32_iwdg_remove, .driver = { .name = "iwdg", - .of_match_table = stm32_iwdg_of_match, + .of_match_table = of_match_ptr(stm32_iwdg_of_match), }, }; module_platform_driver(stm32_iwdg_driver); -- 2.7.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH V2 1/3] watchdog: stm32: add pclk feature for stm32mp1 2018-06-20 7:53 ` [PATCH V2 1/3] watchdog: stm32: add pclk feature for stm32mp1 Ludovic Barre @ 2018-06-20 9:19 ` Guenter Roeck 2018-06-20 13:24 ` Ludovic BARRE 0 siblings, 1 reply; 9+ messages in thread From: Guenter Roeck @ 2018-06-20 9:19 UTC (permalink / raw) To: Ludovic Barre, Wim Van Sebroeck, Rob Herring Cc: Maxime Coquelin, Alexandre Torgue, linux-watchdog, linux-arm-kernel, linux-kernel, devicetree On 06/20/2018 12:53 AM, Ludovic Barre wrote: > From: Ludovic Barre <ludovic.barre@st.com> > > This patch adds config data to manage specific properties by > compatible. Adds stm32mp1 config which requires pclk clock. > > Signed-off-by: Ludovic Barre <ludovic.barre@st.com> > --- > .../devicetree/bindings/watchdog/st,stm32-iwdg.txt | 21 +++- > drivers/watchdog/stm32_iwdg.c | 132 ++++++++++++++------- > 2 files changed, 104 insertions(+), 49 deletions(-) > > diff --git a/Documentation/devicetree/bindings/watchdog/st,stm32-iwdg.txt b/Documentation/devicetree/bindings/watchdog/st,stm32-iwdg.txt > index cc13b10a..f07f6d89 100644 > --- a/Documentation/devicetree/bindings/watchdog/st,stm32-iwdg.txt > +++ b/Documentation/devicetree/bindings/watchdog/st,stm32-iwdg.txt > @@ -2,18 +2,31 @@ STM32 Independent WatchDoG (IWDG) > --------------------------------- > > Required properties: > -- compatible: "st,stm32-iwdg" > -- reg: physical base address and length of the registers set for the device > -- clocks: must contain a single entry describing the clock input > +- compatible: Should be either "st,stm32-iwdg" or "st,stm32mp1-iwdg" > +- reg: Physical base address and length of the registers set for the device > +- clocks: Reference to the clock entry lsi. Additional pclk clock entry > + is required only for st,stm32mp1-iwdg. > +- clock-names: Name of the clocks used. > + "lsi" for st,stm32-iwdg > + "pclk", "lsi" for st,stm32mp1-iwdg > > Optional Properties: > - timeout-sec: Watchdog timeout value in seconds. > > -Example: > +Examples: > > iwdg: watchdog@40003000 { > compatible = "st,stm32-iwdg"; > reg = <0x40003000 0x400>; > clocks = <&clk_lsi>; > + clock-names = "lsi"; > + timeout-sec = <32>; > +}; > + > +iwdg: iwdg@5a002000 { > + compatible = "st,stm32mp1-iwdg"; > + reg = <0x5a002000 0x400>; > + clocks = <&rcc IWDG2>, <&clk_lsi>; > + clock-names = "pclk", "lsi"; > timeout-sec = <32>; > }; > diff --git a/drivers/watchdog/stm32_iwdg.c b/drivers/watchdog/stm32_iwdg.c > index c97ad56..fc96670 100644 > --- a/drivers/watchdog/stm32_iwdg.c > +++ b/drivers/watchdog/stm32_iwdg.c > @@ -11,12 +11,13 @@ > > #include <linux/clk.h> > #include <linux/delay.h> > -#include <linux/kernel.h> > -#include <linux/module.h> > #include <linux/interrupt.h> > #include <linux/io.h> > #include <linux/iopoll.h> > +#include <linux/kernel.h> > +#include <linux/module.h> > #include <linux/of.h> > +#include <linux/of_device.h> > #include <linux/platform_device.h> > #include <linux/watchdog.h> > > @@ -54,11 +55,17 @@ > #define TIMEOUT_US 100000 > #define SLEEP_US 1000 > > +struct stm32_iwdg_config { > + bool has_pclk; > +}; > + This data structure is unnecessary. Just assign the boolean directly to .data and ... > struct stm32_iwdg { > - struct watchdog_device wdd; > - void __iomem *regs; > - struct clk *clk; > - unsigned int rate; > + struct watchdog_device wdd; > + void __iomem *regs; > + struct stm32_iwdg_config *config; declare bool has_pclk here. > + struct clk *clk_lsi; > + struct clk *clk_pclk; > + unsigned int rate; > }; > > static inline u32 reg_read(void __iomem *base, u32 reg) > @@ -133,6 +140,44 @@ static int stm32_iwdg_set_timeout(struct watchdog_device *wdd, > return 0; > } > > +static int stm32_iwdg_clk_init(struct platform_device *pdev, > + struct stm32_iwdg *wdt) > +{ > + u32 ret; > + > + wdt->clk_lsi = devm_clk_get(&pdev->dev, "lsi"); > + if (IS_ERR(wdt->clk_lsi)) { > + dev_err(&pdev->dev, "Unable to get lsi clock\n"); > + return PTR_ERR(wdt->clk_lsi); > + } > + > + /* optional peripheral clock */ > + if (wdt->config->has_pclk) { > + wdt->clk_pclk = devm_clk_get(&pdev->dev, "pclk"); > + if (IS_ERR(wdt->clk_pclk)) { > + dev_err(&pdev->dev, "Unable to get pclk clock\n"); > + return PTR_ERR(wdt->clk_pclk); > + } > + > + ret = clk_prepare_enable(wdt->clk_pclk); > + if (ret) { > + dev_err(&pdev->dev, "Unable to prepare pclk clock\n"); > + return ret; > + } > + } > + > + ret = clk_prepare_enable(wdt->clk_lsi); > + if (ret) { > + dev_err(&pdev->dev, "Unable to prepare lsi clock\n"); > + clk_disable_unprepare(wdt->clk_pclk); > + return ret; > + } > + > + wdt->rate = clk_get_rate(wdt->clk_lsi); > + > + return 0; > +} > + > static const struct watchdog_info stm32_iwdg_info = { > .options = WDIOF_SETTIMEOUT | > WDIOF_MAGICCLOSE | > @@ -147,49 +192,50 @@ static const struct watchdog_ops stm32_iwdg_ops = { > .set_timeout = stm32_iwdg_set_timeout, > }; > > +static const struct stm32_iwdg_config stm32_iwdg_cfg = { > + .has_pclk = false, > +}; > + > +static const struct stm32_iwdg_config stm32mp1_iwdg_cfg = { > + .has_pclk = true, > +}; > + > +static const struct of_device_id stm32_iwdg_of_match[] = { > + { .compatible = "st,stm32-iwdg", .data = &stm32_iwdg_cfg }, > + { .compatible = "st,stm32mp1-iwdg", .data = &stm32mp1_iwdg_cfg }, > + { /* end node */ } > +}; > +MODULE_DEVICE_TABLE(of, stm32_iwdg_of_match); > + > static int stm32_iwdg_probe(struct platform_device *pdev) > { > struct watchdog_device *wdd; > + const struct of_device_id *match; > struct stm32_iwdg *wdt; > struct resource *res; > - void __iomem *regs; > - struct clk *clk; > int ret; > > + match = of_match_device(stm32_iwdg_of_match, &pdev->dev); > + if (!match || !match->data) > + return -ENODEV; > + > + wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL); > + if (!wdt) > + return -ENOMEM; > + > + wdt->config = (struct stm32_iwdg_config *)match->data; > + > /* This is the timer base. */ > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > - regs = devm_ioremap_resource(&pdev->dev, res); > - if (IS_ERR(regs)) { > + wdt->regs = devm_ioremap_resource(&pdev->dev, res); > + if (IS_ERR(wdt->regs)) { > dev_err(&pdev->dev, "Could not get resource\n"); > - return PTR_ERR(regs); > + return PTR_ERR(wdt->regs); > } > > - clk = devm_clk_get(&pdev->dev, NULL); > - if (IS_ERR(clk)) { > - dev_err(&pdev->dev, "Unable to get clock\n"); > - return PTR_ERR(clk); > - } > - > - ret = clk_prepare_enable(clk); > - if (ret) { > - dev_err(&pdev->dev, "Unable to prepare clock %p\n", clk); > + ret = stm32_iwdg_clk_init(pdev, wdt); > + if (ret) > return ret; > - } > - > - /* > - * Allocate our watchdog driver data, which has the > - * struct watchdog_device nested within it. > - */ > - wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL); > - if (!wdt) { > - ret = -ENOMEM; > - goto err; > - } > - > - /* Initialize struct stm32_iwdg. */ > - wdt->regs = regs; > - wdt->clk = clk; > - wdt->rate = clk_get_rate(clk); > > /* Initialize struct watchdog_device. */ > wdd = &wdt->wdd; > @@ -217,7 +263,8 @@ static int stm32_iwdg_probe(struct platform_device *pdev) > > return 0; > err: > - clk_disable_unprepare(clk); > + clk_disable_unprepare(wdt->clk_lsi); > + clk_disable_unprepare(wdt->clk_pclk); > > return ret; > } > @@ -227,23 +274,18 @@ static int stm32_iwdg_remove(struct platform_device *pdev) > struct stm32_iwdg *wdt = platform_get_drvdata(pdev); > > watchdog_unregister_device(&wdt->wdd); > - clk_disable_unprepare(wdt->clk); > + clk_disable_unprepare(wdt->clk_lsi); > + clk_disable_unprepare(wdt->clk_pclk); > > return 0; > } > > -static const struct of_device_id stm32_iwdg_of_match[] = { > - { .compatible = "st,stm32-iwdg" }, > - { /* end node */ } > -}; > -MODULE_DEVICE_TABLE(of, stm32_iwdg_of_match); > - > static struct platform_driver stm32_iwdg_driver = { > .probe = stm32_iwdg_probe, > .remove = stm32_iwdg_remove, > .driver = { > .name = "iwdg", > - .of_match_table = stm32_iwdg_of_match, > + .of_match_table = of_match_ptr(stm32_iwdg_of_match), > }, > }; > module_platform_driver(stm32_iwdg_driver); > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH V2 1/3] watchdog: stm32: add pclk feature for stm32mp1 2018-06-20 9:19 ` Guenter Roeck @ 2018-06-20 13:24 ` Ludovic BARRE 2018-06-20 13:29 ` Guenter Roeck 0 siblings, 1 reply; 9+ messages in thread From: Ludovic BARRE @ 2018-06-20 13:24 UTC (permalink / raw) To: Guenter Roeck, Wim Van Sebroeck, Rob Herring Cc: devicetree, Alexandre Torgue, linux-kernel, Maxime Coquelin, linux-arm-kernel, linux-watchdog On 06/20/2018 11:19 AM, Guenter Roeck wrote: > On 06/20/2018 12:53 AM, Ludovic Barre wrote: >> From: Ludovic Barre <ludovic.barre@st.com> >> >> This patch adds config data to manage specific properties by >> compatible. Adds stm32mp1 config which requires pclk clock. >> >> Signed-off-by: Ludovic Barre <ludovic.barre@st.com> >> --- >> .../devicetree/bindings/watchdog/st,stm32-iwdg.txt | 21 +++- >> drivers/watchdog/stm32_iwdg.c | 132 >> ++++++++++++++------- >> 2 files changed, 104 insertions(+), 49 deletions(-) >> >> diff --git >> a/Documentation/devicetree/bindings/watchdog/st,stm32-iwdg.txt >> b/Documentation/devicetree/bindings/watchdog/st,stm32-iwdg.txt >> index cc13b10a..f07f6d89 100644 >> --- a/Documentation/devicetree/bindings/watchdog/st,stm32-iwdg.txt >> +++ b/Documentation/devicetree/bindings/watchdog/st,stm32-iwdg.txt >> @@ -2,18 +2,31 @@ STM32 Independent WatchDoG (IWDG) >> --------------------------------- >> Required properties: >> -- compatible: "st,stm32-iwdg" >> -- reg: physical base address and length of the registers set for the >> device >> -- clocks: must contain a single entry describing the clock input >> +- compatible: Should be either "st,stm32-iwdg" or "st,stm32mp1-iwdg" >> +- reg: Physical base address and length of the registers set for the >> device >> +- clocks: Reference to the clock entry lsi. Additional pclk clock entry >> + is required only for st,stm32mp1-iwdg. >> +- clock-names: Name of the clocks used. >> + "lsi" for st,stm32-iwdg >> + "pclk", "lsi" for st,stm32mp1-iwdg >> Optional Properties: >> - timeout-sec: Watchdog timeout value in seconds. >> -Example: >> +Examples: >> iwdg: watchdog@40003000 { >> compatible = "st,stm32-iwdg"; >> reg = <0x40003000 0x400>; >> clocks = <&clk_lsi>; >> + clock-names = "lsi"; >> + timeout-sec = <32>; >> +}; >> + >> +iwdg: iwdg@5a002000 { >> + compatible = "st,stm32mp1-iwdg"; >> + reg = <0x5a002000 0x400>; >> + clocks = <&rcc IWDG2>, <&clk_lsi>; >> + clock-names = "pclk", "lsi"; >> timeout-sec = <32>; >> }; >> diff --git a/drivers/watchdog/stm32_iwdg.c >> b/drivers/watchdog/stm32_iwdg.c >> index c97ad56..fc96670 100644 >> --- a/drivers/watchdog/stm32_iwdg.c >> +++ b/drivers/watchdog/stm32_iwdg.c >> @@ -11,12 +11,13 @@ >> #include <linux/clk.h> >> #include <linux/delay.h> >> -#include <linux/kernel.h> >> -#include <linux/module.h> >> #include <linux/interrupt.h> >> #include <linux/io.h> >> #include <linux/iopoll.h> >> +#include <linux/kernel.h> >> +#include <linux/module.h> >> #include <linux/of.h> >> +#include <linux/of_device.h> >> #include <linux/platform_device.h> >> #include <linux/watchdog.h> >> @@ -54,11 +55,17 @@ >> #define TIMEOUT_US 100000 >> #define SLEEP_US 1000 >> +struct stm32_iwdg_config { >> + bool has_pclk; >> +}; >> + > > This data structure is unnecessary. Just assign the boolean directly to > .data > and ... > Ok, I send a v3, with boolean directly to .data like: #define NO_PCLK false #define HAS_PCLK true ... static const struct of_device_id stm32_iwdg_of_match[] = { { .compatible = "st,stm32-iwdg", .data = (void *) NO_PCLK }, { .compatible = "st,stm32mp1-iwdg", .data = (void *) HAS_PCLK }, { /* end node */ } }; Note: V3, because I sent my original version with V2 (it's mistake) >> struct stm32_iwdg { >> - struct watchdog_device wdd; >> - void __iomem *regs; >> - struct clk *clk; >> - unsigned int rate; >> + struct watchdog_device wdd; >> + void __iomem *regs; >> + struct stm32_iwdg_config *config; > > declare bool has_pclk here. > >> + struct clk *clk_lsi; >> + struct clk *clk_pclk; >> + unsigned int rate; >> }; >> static inline u32 reg_read(void __iomem *base, u32 reg) >> @@ -133,6 +140,44 @@ static int stm32_iwdg_set_timeout(struct >> watchdog_device *wdd, >> return 0; >> } >> +static int stm32_iwdg_clk_init(struct platform_device *pdev, >> + struct stm32_iwdg *wdt) >> +{ >> + u32 ret; >> + >> + wdt->clk_lsi = devm_clk_get(&pdev->dev, "lsi"); >> + if (IS_ERR(wdt->clk_lsi)) { >> + dev_err(&pdev->dev, "Unable to get lsi clock\n"); >> + return PTR_ERR(wdt->clk_lsi); >> + } >> + >> + /* optional peripheral clock */ >> + if (wdt->config->has_pclk) { >> + wdt->clk_pclk = devm_clk_get(&pdev->dev, "pclk"); >> + if (IS_ERR(wdt->clk_pclk)) { >> + dev_err(&pdev->dev, "Unable to get pclk clock\n"); >> + return PTR_ERR(wdt->clk_pclk); >> + } >> + >> + ret = clk_prepare_enable(wdt->clk_pclk); >> + if (ret) { >> + dev_err(&pdev->dev, "Unable to prepare pclk clock\n"); >> + return ret; >> + } >> + } >> + >> + ret = clk_prepare_enable(wdt->clk_lsi); >> + if (ret) { >> + dev_err(&pdev->dev, "Unable to prepare lsi clock\n"); >> + clk_disable_unprepare(wdt->clk_pclk); >> + return ret; >> + } >> + >> + wdt->rate = clk_get_rate(wdt->clk_lsi); >> + >> + return 0; >> +} >> + >> static const struct watchdog_info stm32_iwdg_info = { >> .options = WDIOF_SETTIMEOUT | >> WDIOF_MAGICCLOSE | >> @@ -147,49 +192,50 @@ static const struct watchdog_ops stm32_iwdg_ops = { >> .set_timeout = stm32_iwdg_set_timeout, >> }; >> +static const struct stm32_iwdg_config stm32_iwdg_cfg = { >> + .has_pclk = false, >> +}; >> + >> +static const struct stm32_iwdg_config stm32mp1_iwdg_cfg = { >> + .has_pclk = true, >> +}; >> + >> +static const struct of_device_id stm32_iwdg_of_match[] = { >> + { .compatible = "st,stm32-iwdg", .data = &stm32_iwdg_cfg }, >> + { .compatible = "st,stm32mp1-iwdg", .data = &stm32mp1_iwdg_cfg }, >> + { /* end node */ } >> +}; >> +MODULE_DEVICE_TABLE(of, stm32_iwdg_of_match); >> + >> static int stm32_iwdg_probe(struct platform_device *pdev) >> { >> struct watchdog_device *wdd; >> + const struct of_device_id *match; >> struct stm32_iwdg *wdt; >> struct resource *res; >> - void __iomem *regs; >> - struct clk *clk; >> int ret; >> + match = of_match_device(stm32_iwdg_of_match, &pdev->dev); >> + if (!match || !match->data) >> + return -ENODEV; >> + >> + wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL); >> + if (!wdt) >> + return -ENOMEM; >> + >> + wdt->config = (struct stm32_iwdg_config *)match->data; >> + >> /* This is the timer base. */ >> res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> - regs = devm_ioremap_resource(&pdev->dev, res); >> - if (IS_ERR(regs)) { >> + wdt->regs = devm_ioremap_resource(&pdev->dev, res); >> + if (IS_ERR(wdt->regs)) { >> dev_err(&pdev->dev, "Could not get resource\n"); >> - return PTR_ERR(regs); >> + return PTR_ERR(wdt->regs); >> } >> - clk = devm_clk_get(&pdev->dev, NULL); >> - if (IS_ERR(clk)) { >> - dev_err(&pdev->dev, "Unable to get clock\n"); >> - return PTR_ERR(clk); >> - } >> - >> - ret = clk_prepare_enable(clk); >> - if (ret) { >> - dev_err(&pdev->dev, "Unable to prepare clock %p\n", clk); >> + ret = stm32_iwdg_clk_init(pdev, wdt); >> + if (ret) >> return ret; >> - } >> - >> - /* >> - * Allocate our watchdog driver data, which has the >> - * struct watchdog_device nested within it. >> - */ >> - wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL); >> - if (!wdt) { >> - ret = -ENOMEM; >> - goto err; >> - } >> - >> - /* Initialize struct stm32_iwdg. */ >> - wdt->regs = regs; >> - wdt->clk = clk; >> - wdt->rate = clk_get_rate(clk); >> /* Initialize struct watchdog_device. */ >> wdd = &wdt->wdd; >> @@ -217,7 +263,8 @@ static int stm32_iwdg_probe(struct platform_device >> *pdev) >> return 0; >> err: >> - clk_disable_unprepare(clk); >> + clk_disable_unprepare(wdt->clk_lsi); >> + clk_disable_unprepare(wdt->clk_pclk); >> return ret; >> } >> @@ -227,23 +274,18 @@ static int stm32_iwdg_remove(struct >> platform_device *pdev) >> struct stm32_iwdg *wdt = platform_get_drvdata(pdev); >> watchdog_unregister_device(&wdt->wdd); >> - clk_disable_unprepare(wdt->clk); >> + clk_disable_unprepare(wdt->clk_lsi); >> + clk_disable_unprepare(wdt->clk_pclk); >> return 0; >> } >> -static const struct of_device_id stm32_iwdg_of_match[] = { >> - { .compatible = "st,stm32-iwdg" }, >> - { /* end node */ } >> -}; >> -MODULE_DEVICE_TABLE(of, stm32_iwdg_of_match); >> - >> static struct platform_driver stm32_iwdg_driver = { >> .probe = stm32_iwdg_probe, >> .remove = stm32_iwdg_remove, >> .driver = { >> .name = "iwdg", >> - .of_match_table = stm32_iwdg_of_match, >> + .of_match_table = of_match_ptr(stm32_iwdg_of_match), >> }, >> }; >> module_platform_driver(stm32_iwdg_driver); >> > _______________________________________________ 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] 9+ messages in thread
* Re: [PATCH V2 1/3] watchdog: stm32: add pclk feature for stm32mp1 2018-06-20 13:24 ` Ludovic BARRE @ 2018-06-20 13:29 ` Guenter Roeck 2018-06-20 13:35 ` Ludovic BARRE 0 siblings, 1 reply; 9+ messages in thread From: Guenter Roeck @ 2018-06-20 13:29 UTC (permalink / raw) To: Ludovic BARRE, Wim Van Sebroeck, Rob Herring Cc: Maxime Coquelin, Alexandre Torgue, linux-watchdog, linux-arm-kernel, linux-kernel, devicetree On 06/20/2018 06:24 AM, Ludovic BARRE wrote: > > > On 06/20/2018 11:19 AM, Guenter Roeck wrote: >> On 06/20/2018 12:53 AM, Ludovic Barre wrote: >>> From: Ludovic Barre <ludovic.barre@st.com> >>> >>> This patch adds config data to manage specific properties by >>> compatible. Adds stm32mp1 config which requires pclk clock. >>> >>> Signed-off-by: Ludovic Barre <ludovic.barre@st.com> >>> --- >>> .../devicetree/bindings/watchdog/st,stm32-iwdg.txt | 21 +++- >>> drivers/watchdog/stm32_iwdg.c | 132 ++++++++++++++------- >>> 2 files changed, 104 insertions(+), 49 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/watchdog/st,stm32-iwdg.txt b/Documentation/devicetree/bindings/watchdog/st,stm32-iwdg.txt >>> index cc13b10a..f07f6d89 100644 >>> --- a/Documentation/devicetree/bindings/watchdog/st,stm32-iwdg.txt >>> +++ b/Documentation/devicetree/bindings/watchdog/st,stm32-iwdg.txt >>> @@ -2,18 +2,31 @@ STM32 Independent WatchDoG (IWDG) >>> --------------------------------- >>> Required properties: >>> -- compatible: "st,stm32-iwdg" >>> -- reg: physical base address and length of the registers set for the device >>> -- clocks: must contain a single entry describing the clock input >>> +- compatible: Should be either "st,stm32-iwdg" or "st,stm32mp1-iwdg" >>> +- reg: Physical base address and length of the registers set for the device >>> +- clocks: Reference to the clock entry lsi. Additional pclk clock entry >>> + is required only for st,stm32mp1-iwdg. >>> +- clock-names: Name of the clocks used. >>> + "lsi" for st,stm32-iwdg >>> + "pclk", "lsi" for st,stm32mp1-iwdg >>> Optional Properties: >>> - timeout-sec: Watchdog timeout value in seconds. >>> -Example: >>> +Examples: >>> iwdg: watchdog@40003000 { >>> compatible = "st,stm32-iwdg"; >>> reg = <0x40003000 0x400>; >>> clocks = <&clk_lsi>; >>> + clock-names = "lsi"; >>> + timeout-sec = <32>; >>> +}; >>> + >>> +iwdg: iwdg@5a002000 { >>> + compatible = "st,stm32mp1-iwdg"; >>> + reg = <0x5a002000 0x400>; >>> + clocks = <&rcc IWDG2>, <&clk_lsi>; >>> + clock-names = "pclk", "lsi"; >>> timeout-sec = <32>; >>> }; >>> diff --git a/drivers/watchdog/stm32_iwdg.c b/drivers/watchdog/stm32_iwdg.c >>> index c97ad56..fc96670 100644 >>> --- a/drivers/watchdog/stm32_iwdg.c >>> +++ b/drivers/watchdog/stm32_iwdg.c >>> @@ -11,12 +11,13 @@ >>> #include <linux/clk.h> >>> #include <linux/delay.h> >>> -#include <linux/kernel.h> >>> -#include <linux/module.h> >>> #include <linux/interrupt.h> >>> #include <linux/io.h> >>> #include <linux/iopoll.h> >>> +#include <linux/kernel.h> >>> +#include <linux/module.h> >>> #include <linux/of.h> >>> +#include <linux/of_device.h> >>> #include <linux/platform_device.h> >>> #include <linux/watchdog.h> >>> @@ -54,11 +55,17 @@ >>> #define TIMEOUT_US 100000 >>> #define SLEEP_US 1000 >>> +struct stm32_iwdg_config { >>> + bool has_pclk; >>> +}; >>> + >> >> This data structure is unnecessary. Just assign the boolean directly to .data >> and ... >> > > Ok, I send a v3, with boolean directly to .data > like: > > #define NO_PCLK false > #define HAS_PCLK true > ... Just use true/false directly. There is no need for those defines. If you want the reader to understand what is defined, I would be ok with #define HAS_PCLK true static const struct of_device_id stm32_iwdg_of_match[] = { { .compatible = "st,stm32-iwdg", .data = (void *) !HAS_PCLK }, { .compatible = "st,stm32mp1-iwdg", .data = (void *) HAS_PCLK }, { /* end node */ } Guenter > static const struct of_device_id stm32_iwdg_of_match[] = { > { .compatible = "st,stm32-iwdg", .data = (void *) NO_PCLK }, > { .compatible = "st,stm32mp1-iwdg", .data = (void *) HAS_PCLK }, > { /* end node */ } > }; > > Note: > V3, because I sent my original version with V2 > (it's mistake) > >>> struct stm32_iwdg { >>> - struct watchdog_device wdd; >>> - void __iomem *regs; >>> - struct clk *clk; >>> - unsigned int rate; >>> + struct watchdog_device wdd; >>> + void __iomem *regs; >>> + struct stm32_iwdg_config *config; >> >> declare bool has_pclk here. >> >>> + struct clk *clk_lsi; >>> + struct clk *clk_pclk; >>> + unsigned int rate; >>> }; >>> static inline u32 reg_read(void __iomem *base, u32 reg) >>> @@ -133,6 +140,44 @@ static int stm32_iwdg_set_timeout(struct watchdog_device *wdd, >>> return 0; >>> } >>> +static int stm32_iwdg_clk_init(struct platform_device *pdev, >>> + struct stm32_iwdg *wdt) >>> +{ >>> + u32 ret; >>> + >>> + wdt->clk_lsi = devm_clk_get(&pdev->dev, "lsi"); >>> + if (IS_ERR(wdt->clk_lsi)) { >>> + dev_err(&pdev->dev, "Unable to get lsi clock\n"); >>> + return PTR_ERR(wdt->clk_lsi); >>> + } >>> + >>> + /* optional peripheral clock */ >>> + if (wdt->config->has_pclk) { >>> + wdt->clk_pclk = devm_clk_get(&pdev->dev, "pclk"); >>> + if (IS_ERR(wdt->clk_pclk)) { >>> + dev_err(&pdev->dev, "Unable to get pclk clock\n"); >>> + return PTR_ERR(wdt->clk_pclk); >>> + } >>> + >>> + ret = clk_prepare_enable(wdt->clk_pclk); >>> + if (ret) { >>> + dev_err(&pdev->dev, "Unable to prepare pclk clock\n"); >>> + return ret; >>> + } >>> + } >>> + >>> + ret = clk_prepare_enable(wdt->clk_lsi); >>> + if (ret) { >>> + dev_err(&pdev->dev, "Unable to prepare lsi clock\n"); >>> + clk_disable_unprepare(wdt->clk_pclk); >>> + return ret; >>> + } >>> + >>> + wdt->rate = clk_get_rate(wdt->clk_lsi); >>> + >>> + return 0; >>> +} >>> + >>> static const struct watchdog_info stm32_iwdg_info = { >>> .options = WDIOF_SETTIMEOUT | >>> WDIOF_MAGICCLOSE | >>> @@ -147,49 +192,50 @@ static const struct watchdog_ops stm32_iwdg_ops = { >>> .set_timeout = stm32_iwdg_set_timeout, >>> }; >>> +static const struct stm32_iwdg_config stm32_iwdg_cfg = { >>> + .has_pclk = false, >>> +}; >>> + >>> +static const struct stm32_iwdg_config stm32mp1_iwdg_cfg = { >>> + .has_pclk = true, >>> +}; >>> + >>> +static const struct of_device_id stm32_iwdg_of_match[] = { >>> + { .compatible = "st,stm32-iwdg", .data = &stm32_iwdg_cfg }, >>> + { .compatible = "st,stm32mp1-iwdg", .data = &stm32mp1_iwdg_cfg }, >>> + { /* end node */ } >>> +}; >>> +MODULE_DEVICE_TABLE(of, stm32_iwdg_of_match); >>> + >>> static int stm32_iwdg_probe(struct platform_device *pdev) >>> { >>> struct watchdog_device *wdd; >>> + const struct of_device_id *match; >>> struct stm32_iwdg *wdt; >>> struct resource *res; >>> - void __iomem *regs; >>> - struct clk *clk; >>> int ret; >>> + match = of_match_device(stm32_iwdg_of_match, &pdev->dev); >>> + if (!match || !match->data) >>> + return -ENODEV; >>> + >>> + wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL); >>> + if (!wdt) >>> + return -ENOMEM; >>> + >>> + wdt->config = (struct stm32_iwdg_config *)match->data; >>> + >>> /* This is the timer base. */ >>> res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >>> - regs = devm_ioremap_resource(&pdev->dev, res); >>> - if (IS_ERR(regs)) { >>> + wdt->regs = devm_ioremap_resource(&pdev->dev, res); >>> + if (IS_ERR(wdt->regs)) { >>> dev_err(&pdev->dev, "Could not get resource\n"); >>> - return PTR_ERR(regs); >>> + return PTR_ERR(wdt->regs); >>> } >>> - clk = devm_clk_get(&pdev->dev, NULL); >>> - if (IS_ERR(clk)) { >>> - dev_err(&pdev->dev, "Unable to get clock\n"); >>> - return PTR_ERR(clk); >>> - } >>> - >>> - ret = clk_prepare_enable(clk); >>> - if (ret) { >>> - dev_err(&pdev->dev, "Unable to prepare clock %p\n", clk); >>> + ret = stm32_iwdg_clk_init(pdev, wdt); >>> + if (ret) >>> return ret; >>> - } >>> - >>> - /* >>> - * Allocate our watchdog driver data, which has the >>> - * struct watchdog_device nested within it. >>> - */ >>> - wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL); >>> - if (!wdt) { >>> - ret = -ENOMEM; >>> - goto err; >>> - } >>> - >>> - /* Initialize struct stm32_iwdg. */ >>> - wdt->regs = regs; >>> - wdt->clk = clk; >>> - wdt->rate = clk_get_rate(clk); >>> /* Initialize struct watchdog_device. */ >>> wdd = &wdt->wdd; >>> @@ -217,7 +263,8 @@ static int stm32_iwdg_probe(struct platform_device *pdev) >>> return 0; >>> err: >>> - clk_disable_unprepare(clk); >>> + clk_disable_unprepare(wdt->clk_lsi); >>> + clk_disable_unprepare(wdt->clk_pclk); >>> return ret; >>> } >>> @@ -227,23 +274,18 @@ static int stm32_iwdg_remove(struct platform_device *pdev) >>> struct stm32_iwdg *wdt = platform_get_drvdata(pdev); >>> watchdog_unregister_device(&wdt->wdd); >>> - clk_disable_unprepare(wdt->clk); >>> + clk_disable_unprepare(wdt->clk_lsi); >>> + clk_disable_unprepare(wdt->clk_pclk); >>> return 0; >>> } >>> -static const struct of_device_id stm32_iwdg_of_match[] = { >>> - { .compatible = "st,stm32-iwdg" }, >>> - { /* end node */ } >>> -}; >>> -MODULE_DEVICE_TABLE(of, stm32_iwdg_of_match); >>> - >>> static struct platform_driver stm32_iwdg_driver = { >>> .probe = stm32_iwdg_probe, >>> .remove = stm32_iwdg_remove, >>> .driver = { >>> .name = "iwdg", >>> - .of_match_table = stm32_iwdg_of_match, >>> + .of_match_table = of_match_ptr(stm32_iwdg_of_match), >>> }, >>> }; >>> module_platform_driver(stm32_iwdg_driver); >>> >> > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH V2 1/3] watchdog: stm32: add pclk feature for stm32mp1 2018-06-20 13:29 ` Guenter Roeck @ 2018-06-20 13:35 ` Ludovic BARRE 0 siblings, 0 replies; 9+ messages in thread From: Ludovic BARRE @ 2018-06-20 13:35 UTC (permalink / raw) To: Guenter Roeck, Wim Van Sebroeck, Rob Herring Cc: devicetree, Alexandre Torgue, linux-kernel, Maxime Coquelin, linux-arm-kernel, linux-watchdog On 06/20/2018 03:29 PM, Guenter Roeck wrote: > On 06/20/2018 06:24 AM, Ludovic BARRE wrote: >> >> >> On 06/20/2018 11:19 AM, Guenter Roeck wrote: >>> On 06/20/2018 12:53 AM, Ludovic Barre wrote: >>>> From: Ludovic Barre <ludovic.barre@st.com> >>>> >>>> This patch adds config data to manage specific properties by >>>> compatible. Adds stm32mp1 config which requires pclk clock. >>>> >>>> Signed-off-by: Ludovic Barre <ludovic.barre@st.com> >>>> --- >>>> .../devicetree/bindings/watchdog/st,stm32-iwdg.txt | 21 +++- >>>> drivers/watchdog/stm32_iwdg.c | 132 >>>> ++++++++++++++------- >>>> 2 files changed, 104 insertions(+), 49 deletions(-) >>>> >>>> diff --git >>>> a/Documentation/devicetree/bindings/watchdog/st,stm32-iwdg.txt >>>> b/Documentation/devicetree/bindings/watchdog/st,stm32-iwdg.txt >>>> index cc13b10a..f07f6d89 100644 >>>> --- a/Documentation/devicetree/bindings/watchdog/st,stm32-iwdg.txt >>>> +++ b/Documentation/devicetree/bindings/watchdog/st,stm32-iwdg.txt >>>> @@ -2,18 +2,31 @@ STM32 Independent WatchDoG (IWDG) >>>> --------------------------------- >>>> Required properties: >>>> -- compatible: "st,stm32-iwdg" >>>> -- reg: physical base address and length of the registers set for >>>> the device >>>> -- clocks: must contain a single entry describing the clock input >>>> +- compatible: Should be either "st,stm32-iwdg" or "st,stm32mp1-iwdg" >>>> +- reg: Physical base address and length of the registers set for >>>> the device >>>> +- clocks: Reference to the clock entry lsi. Additional pclk clock >>>> entry >>>> + is required only for st,stm32mp1-iwdg. >>>> +- clock-names: Name of the clocks used. >>>> + "lsi" for st,stm32-iwdg >>>> + "pclk", "lsi" for st,stm32mp1-iwdg >>>> Optional Properties: >>>> - timeout-sec: Watchdog timeout value in seconds. >>>> -Example: >>>> +Examples: >>>> iwdg: watchdog@40003000 { >>>> compatible = "st,stm32-iwdg"; >>>> reg = <0x40003000 0x400>; >>>> clocks = <&clk_lsi>; >>>> + clock-names = "lsi"; >>>> + timeout-sec = <32>; >>>> +}; >>>> + >>>> +iwdg: iwdg@5a002000 { >>>> + compatible = "st,stm32mp1-iwdg"; >>>> + reg = <0x5a002000 0x400>; >>>> + clocks = <&rcc IWDG2>, <&clk_lsi>; >>>> + clock-names = "pclk", "lsi"; >>>> timeout-sec = <32>; >>>> }; >>>> diff --git a/drivers/watchdog/stm32_iwdg.c >>>> b/drivers/watchdog/stm32_iwdg.c >>>> index c97ad56..fc96670 100644 >>>> --- a/drivers/watchdog/stm32_iwdg.c >>>> +++ b/drivers/watchdog/stm32_iwdg.c >>>> @@ -11,12 +11,13 @@ >>>> #include <linux/clk.h> >>>> #include <linux/delay.h> >>>> -#include <linux/kernel.h> >>>> -#include <linux/module.h> >>>> #include <linux/interrupt.h> >>>> #include <linux/io.h> >>>> #include <linux/iopoll.h> >>>> +#include <linux/kernel.h> >>>> +#include <linux/module.h> >>>> #include <linux/of.h> >>>> +#include <linux/of_device.h> >>>> #include <linux/platform_device.h> >>>> #include <linux/watchdog.h> >>>> @@ -54,11 +55,17 @@ >>>> #define TIMEOUT_US 100000 >>>> #define SLEEP_US 1000 >>>> +struct stm32_iwdg_config { >>>> + bool has_pclk; >>>> +}; >>>> + >>> >>> This data structure is unnecessary. Just assign the boolean directly >>> to .data >>> and ... >>> >> >> Ok, I send a v3, with boolean directly to .data >> like: >> >> #define NO_PCLK false >> #define HAS_PCLK true >> ... > > Just use true/false directly. There is no need for those defines. > > If you want the reader to understand what is defined, I would be ok with > > #define HAS_PCLK true > > static const struct of_device_id stm32_iwdg_of_match[] = { > { .compatible = "st,stm32-iwdg", .data = (void *) !HAS_PCLK }, > { .compatible = "st,stm32mp1-iwdg", .data = (void *) HAS_PCLK }, > { /* end node */ } > > Guenter Ok, thanks > >> static const struct of_device_id stm32_iwdg_of_match[] = { >> { .compatible = "st,stm32-iwdg", .data = (void *) NO_PCLK }, >> { .compatible = "st,stm32mp1-iwdg", .data = (void *) HAS_PCLK }, >> { /* end node */ } >> }; >> >> Note: >> V3, because I sent my original version with V2 >> (it's mistake) >> >>>> struct stm32_iwdg { >>>> - struct watchdog_device wdd; >>>> - void __iomem *regs; >>>> - struct clk *clk; >>>> - unsigned int rate; >>>> + struct watchdog_device wdd; >>>> + void __iomem *regs; >>>> + struct stm32_iwdg_config *config; >>> >>> declare bool has_pclk here. >>> >>>> + struct clk *clk_lsi; >>>> + struct clk *clk_pclk; >>>> + unsigned int rate; >>>> }; >>>> static inline u32 reg_read(void __iomem *base, u32 reg) >>>> @@ -133,6 +140,44 @@ static int stm32_iwdg_set_timeout(struct >>>> watchdog_device *wdd, >>>> return 0; >>>> } >>>> +static int stm32_iwdg_clk_init(struct platform_device *pdev, >>>> + struct stm32_iwdg *wdt) >>>> +{ >>>> + u32 ret; >>>> + >>>> + wdt->clk_lsi = devm_clk_get(&pdev->dev, "lsi"); >>>> + if (IS_ERR(wdt->clk_lsi)) { >>>> + dev_err(&pdev->dev, "Unable to get lsi clock\n"); >>>> + return PTR_ERR(wdt->clk_lsi); >>>> + } >>>> + >>>> + /* optional peripheral clock */ >>>> + if (wdt->config->has_pclk) { >>>> + wdt->clk_pclk = devm_clk_get(&pdev->dev, "pclk"); >>>> + if (IS_ERR(wdt->clk_pclk)) { >>>> + dev_err(&pdev->dev, "Unable to get pclk clock\n"); >>>> + return PTR_ERR(wdt->clk_pclk); >>>> + } >>>> + >>>> + ret = clk_prepare_enable(wdt->clk_pclk); >>>> + if (ret) { >>>> + dev_err(&pdev->dev, "Unable to prepare pclk clock\n"); >>>> + return ret; >>>> + } >>>> + } >>>> + >>>> + ret = clk_prepare_enable(wdt->clk_lsi); >>>> + if (ret) { >>>> + dev_err(&pdev->dev, "Unable to prepare lsi clock\n"); >>>> + clk_disable_unprepare(wdt->clk_pclk); >>>> + return ret; >>>> + } >>>> + >>>> + wdt->rate = clk_get_rate(wdt->clk_lsi); >>>> + >>>> + return 0; >>>> +} >>>> + >>>> static const struct watchdog_info stm32_iwdg_info = { >>>> .options = WDIOF_SETTIMEOUT | >>>> WDIOF_MAGICCLOSE | >>>> @@ -147,49 +192,50 @@ static const struct watchdog_ops >>>> stm32_iwdg_ops = { >>>> .set_timeout = stm32_iwdg_set_timeout, >>>> }; >>>> +static const struct stm32_iwdg_config stm32_iwdg_cfg = { >>>> + .has_pclk = false, >>>> +}; >>>> + >>>> +static const struct stm32_iwdg_config stm32mp1_iwdg_cfg = { >>>> + .has_pclk = true, >>>> +}; >>>> + >>>> +static const struct of_device_id stm32_iwdg_of_match[] = { >>>> + { .compatible = "st,stm32-iwdg", .data = &stm32_iwdg_cfg }, >>>> + { .compatible = "st,stm32mp1-iwdg", .data = &stm32mp1_iwdg_cfg }, >>>> + { /* end node */ } >>>> +}; >>>> +MODULE_DEVICE_TABLE(of, stm32_iwdg_of_match); >>>> + >>>> static int stm32_iwdg_probe(struct platform_device *pdev) >>>> { >>>> struct watchdog_device *wdd; >>>> + const struct of_device_id *match; >>>> struct stm32_iwdg *wdt; >>>> struct resource *res; >>>> - void __iomem *regs; >>>> - struct clk *clk; >>>> int ret; >>>> + match = of_match_device(stm32_iwdg_of_match, &pdev->dev); >>>> + if (!match || !match->data) >>>> + return -ENODEV; >>>> + >>>> + wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL); >>>> + if (!wdt) >>>> + return -ENOMEM; >>>> + >>>> + wdt->config = (struct stm32_iwdg_config *)match->data; >>>> + >>>> /* This is the timer base. */ >>>> res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >>>> - regs = devm_ioremap_resource(&pdev->dev, res); >>>> - if (IS_ERR(regs)) { >>>> + wdt->regs = devm_ioremap_resource(&pdev->dev, res); >>>> + if (IS_ERR(wdt->regs)) { >>>> dev_err(&pdev->dev, "Could not get resource\n"); >>>> - return PTR_ERR(regs); >>>> + return PTR_ERR(wdt->regs); >>>> } >>>> - clk = devm_clk_get(&pdev->dev, NULL); >>>> - if (IS_ERR(clk)) { >>>> - dev_err(&pdev->dev, "Unable to get clock\n"); >>>> - return PTR_ERR(clk); >>>> - } >>>> - >>>> - ret = clk_prepare_enable(clk); >>>> - if (ret) { >>>> - dev_err(&pdev->dev, "Unable to prepare clock %p\n", clk); >>>> + ret = stm32_iwdg_clk_init(pdev, wdt); >>>> + if (ret) >>>> return ret; >>>> - } >>>> - >>>> - /* >>>> - * Allocate our watchdog driver data, which has the >>>> - * struct watchdog_device nested within it. >>>> - */ >>>> - wdt = devm_kzalloc(&pdev->dev, sizeof(*wdt), GFP_KERNEL); >>>> - if (!wdt) { >>>> - ret = -ENOMEM; >>>> - goto err; >>>> - } >>>> - >>>> - /* Initialize struct stm32_iwdg. */ >>>> - wdt->regs = regs; >>>> - wdt->clk = clk; >>>> - wdt->rate = clk_get_rate(clk); >>>> /* Initialize struct watchdog_device. */ >>>> wdd = &wdt->wdd; >>>> @@ -217,7 +263,8 @@ static int stm32_iwdg_probe(struct >>>> platform_device *pdev) >>>> return 0; >>>> err: >>>> - clk_disable_unprepare(clk); >>>> + clk_disable_unprepare(wdt->clk_lsi); >>>> + clk_disable_unprepare(wdt->clk_pclk); >>>> return ret; >>>> } >>>> @@ -227,23 +274,18 @@ static int stm32_iwdg_remove(struct >>>> platform_device *pdev) >>>> struct stm32_iwdg *wdt = platform_get_drvdata(pdev); >>>> watchdog_unregister_device(&wdt->wdd); >>>> - clk_disable_unprepare(wdt->clk); >>>> + clk_disable_unprepare(wdt->clk_lsi); >>>> + clk_disable_unprepare(wdt->clk_pclk); >>>> return 0; >>>> } >>>> -static const struct of_device_id stm32_iwdg_of_match[] = { >>>> - { .compatible = "st,stm32-iwdg" }, >>>> - { /* end node */ } >>>> -}; >>>> -MODULE_DEVICE_TABLE(of, stm32_iwdg_of_match); >>>> - >>>> static struct platform_driver stm32_iwdg_driver = { >>>> .probe = stm32_iwdg_probe, >>>> .remove = stm32_iwdg_remove, >>>> .driver = { >>>> .name = "iwdg", >>>> - .of_match_table = stm32_iwdg_of_match, >>>> + .of_match_table = of_match_ptr(stm32_iwdg_of_match), >>>> }, >>>> }; >>>> module_platform_driver(stm32_iwdg_driver); >>>> >>> >> > _______________________________________________ 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] 9+ messages in thread
* [PATCH V2 2/3] ARM: dts: stm32: add iwdg2 support for stm32mp157c 2018-06-20 7:53 [PATCH V2 0/3] add iwdg2 support for stm32mp157c Ludovic Barre 2018-06-20 7:53 ` [PATCH V2 1/3] watchdog: stm32: add pclk feature for stm32mp1 Ludovic Barre @ 2018-06-20 7:53 ` Ludovic Barre 2018-06-20 7:53 ` [PATCH V2 3/3] ARM: dts: stm32: add iwdg2 support for stm32mp157c-ed1 Ludovic Barre 2018-06-20 10:03 ` [PATCH V2 0/3] add iwdg2 support for stm32mp157c Alexandre Torgue 3 siblings, 0 replies; 9+ messages in thread From: Ludovic Barre @ 2018-06-20 7:53 UTC (permalink / raw) To: Wim Van Sebroeck, Guenter Roeck, Rob Herring Cc: devicetree, Alexandre Torgue, linux-kernel, Ludovic Barre, Maxime Coquelin, linux-arm-kernel, linux-watchdog From: Ludovic Barre <ludovic.barre@st.com> This patch adds independent watchdog support for stm32mp157c. Signed-off-by: Ludovic Barre <ludovic.barre@st.com> --- arch/arm/boot/dts/stm32mp157c.dtsi | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/arch/arm/boot/dts/stm32mp157c.dtsi b/arch/arm/boot/dts/stm32mp157c.dtsi index 7d17538..95cc166 100644 --- a/arch/arm/boot/dts/stm32mp157c.dtsi +++ b/arch/arm/boot/dts/stm32mp157c.dtsi @@ -784,6 +784,14 @@ status = "disabled"; }; + iwdg2: watchdog@5a002000 { + compatible = "st,stm32mp1-iwdg"; + reg = <0x5a002000 0x400>; + clocks = <&rcc IWDG2>, <&rcc CK_LSI>; + clock-names = "pclk", "lsi"; + status = "disabled"; + }; + usbphyc: usbphyc@5a006000 { #address-cells = <1>; #size-cells = <0>; -- 2.7.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH V2 3/3] ARM: dts: stm32: add iwdg2 support for stm32mp157c-ed1 2018-06-20 7:53 [PATCH V2 0/3] add iwdg2 support for stm32mp157c Ludovic Barre 2018-06-20 7:53 ` [PATCH V2 1/3] watchdog: stm32: add pclk feature for stm32mp1 Ludovic Barre 2018-06-20 7:53 ` [PATCH V2 2/3] ARM: dts: stm32: add iwdg2 support for stm32mp157c Ludovic Barre @ 2018-06-20 7:53 ` Ludovic Barre 2018-06-20 10:03 ` [PATCH V2 0/3] add iwdg2 support for stm32mp157c Alexandre Torgue 3 siblings, 0 replies; 9+ messages in thread From: Ludovic Barre @ 2018-06-20 7:53 UTC (permalink / raw) To: Wim Van Sebroeck, Guenter Roeck, Rob Herring Cc: devicetree, Alexandre Torgue, linux-kernel, Ludovic Barre, Maxime Coquelin, linux-arm-kernel, linux-watchdog From: Ludovic Barre <ludovic.barre@st.com> This patch activates independent watchdog support for stm32mp157c board. Signed-off-by: Ludovic Barre <ludovic.barre@st.com> --- arch/arm/boot/dts/stm32mp157c-ed1.dts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/arch/arm/boot/dts/stm32mp157c-ed1.dts b/arch/arm/boot/dts/stm32mp157c-ed1.dts index ae33653..8af263a 100644 --- a/arch/arm/boot/dts/stm32mp157c-ed1.dts +++ b/arch/arm/boot/dts/stm32mp157c-ed1.dts @@ -68,6 +68,11 @@ status = "okay"; }; +&iwdg2 { + timeout-sec = <32>; + status = "okay"; +}; + &uart4 { pinctrl-names = "default"; pinctrl-0 = <&uart4_pins_a>; -- 2.7.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH V2 0/3] add iwdg2 support for stm32mp157c 2018-06-20 7:53 [PATCH V2 0/3] add iwdg2 support for stm32mp157c Ludovic Barre ` (2 preceding siblings ...) 2018-06-20 7:53 ` [PATCH V2 3/3] ARM: dts: stm32: add iwdg2 support for stm32mp157c-ed1 Ludovic Barre @ 2018-06-20 10:03 ` Alexandre Torgue 3 siblings, 0 replies; 9+ messages in thread From: Alexandre Torgue @ 2018-06-20 10:03 UTC (permalink / raw) To: Ludovic Barre, Wim Van Sebroeck, Guenter Roeck, Rob Herring Cc: linux-arm-kernel, devicetree, linux-watchdog, Maxime Coquelin, linux-kernel Hi Ludovic On 06/20/2018 09:53 AM, Ludovic Barre wrote: > From: Ludovic Barre <ludovic.barre@st.com> > > This patch series updates stm32_iwdg driver to manage config > by compatible. stm32mp1 config requires a pclk clock. > > Ludovic Barre (3): > watchdog: stm32: add pclk feature for stm32mp1 > ARM: dts: stm32: add iwdg2 support for stm32mp157c > ARM: dts: stm32: add iwdg2 support for stm32mp157c-ed1 > > .../devicetree/bindings/watchdog/st,stm32-iwdg.txt | 21 +++- > arch/arm/boot/dts/stm32mp157c-ed1.dts | 5 + > arch/arm/boot/dts/stm32mp157c.dtsi | 8 ++ > drivers/watchdog/stm32_iwdg.c | 132 ++++++++++++++------- > 4 files changed, 117 insertions(+), 49 deletions(-) > STM32 DT patches looks good to me. I will take them as soon as driver part is acked. Regards Alex ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-06-20 13:35 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-06-20 7:53 [PATCH V2 0/3] add iwdg2 support for stm32mp157c Ludovic Barre 2018-06-20 7:53 ` [PATCH V2 1/3] watchdog: stm32: add pclk feature for stm32mp1 Ludovic Barre 2018-06-20 9:19 ` Guenter Roeck 2018-06-20 13:24 ` Ludovic BARRE 2018-06-20 13:29 ` Guenter Roeck 2018-06-20 13:35 ` Ludovic BARRE 2018-06-20 7:53 ` [PATCH V2 2/3] ARM: dts: stm32: add iwdg2 support for stm32mp157c Ludovic Barre 2018-06-20 7:53 ` [PATCH V2 3/3] ARM: dts: stm32: add iwdg2 support for stm32mp157c-ed1 Ludovic Barre 2018-06-20 10:03 ` [PATCH V2 0/3] add iwdg2 support for stm32mp157c Alexandre Torgue
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).