diff mbox series

[1/4] wdt: Add CONFIG_DESIGNWARE_WATCHDOG to Kconfig

Message ID 9b2bb704-e7ea-74ea-c854-60a44472b8ec@gmail.com
State New
Headers show
Series [1/4] wdt: Add CONFIG_DESIGNWARE_WATCHDOG to Kconfig | expand

Commit Message

Sean Anderson Feb. 2, 2020, 5:10 p.m. UTC 
Currently this is set from headers. No board has this set by default, so we
don't need to modify any configs.
---
 drivers/watchdog/Kconfig               | 7 +++++++
 include/configs/socfpga_common.h       | 1 -
 include/configs/socfpga_soc64_common.h | 1 -
 3 files changed, 7 insertions(+), 2 deletions(-)

Comments

Marek Vasut Feb. 2, 2020, 5:15 p.m. UTC | #1 
On 2/2/20 6:10 PM, Sean Anderson wrote:
> Currently this is set from headers. No board has this set by default

Please check where socfpga_common.h and socfpga_soc64_common.h are
included. This should then make it clear that this statement in not true.

, so we
> don't need to modify any configs.

The patch is wrong, see above. Also, it's missing a SoB line.
Sean Anderson Feb. 2, 2020, 5:23 p.m. UTC | #2 
On 2/2/20 12:15 PM, Marek Vasut wrote:
> On 2/2/20 6:10 PM, Sean Anderson wrote:
>> Currently this is set from headers. No board has this set by default
> 
> Please check where socfpga_common.h and socfpga_soc64_common.h are
> included. This should then make it clear that this statement in not true.
> 
> , so we
>> don't need to modify any configs.

CONFIG_DESIGNWARE_WATCHDOG is only defined if CONFIG_HW_WATCHDOG is
defined, and this is never defined in headers (or in the defconfigs).

> The patch is wrong, see above. Also, it's missing a SoB line.

Ah, I just noticed that, thanks for pointing that out.
Sean Anderson Feb. 2, 2020, 5:24 p.m. UTC | #3 
On 2/2/20 12:10 PM, Sean Anderson wrote:
> Currently this is set from headers. No board has this set by default, so we
> don't need to modify any configs.
> ---
>  drivers/watchdog/Kconfig               | 7 +++++++
>  include/configs/socfpga_common.h       | 1 -
>  include/configs/socfpga_soc64_common.h | 1 -
>  3 files changed, 7 insertions(+), 2 deletions(-)

Signed-off-by: Sean Anderson <seanga2 at gmail.com>
Marek Vasut Feb. 2, 2020, 5:40 p.m. UTC | #4 
On 2/2/20 6:23 PM, Sean Anderson wrote:
> On 2/2/20 12:15 PM, Marek Vasut wrote:
>> On 2/2/20 6:10 PM, Sean Anderson wrote:
>>> Currently this is set from headers. No board has this set by default
>>
>> Please check where socfpga_common.h and socfpga_soc64_common.h are
>> included. This should then make it clear that this statement in not true.
>>
>> , so we
>>> don't need to modify any configs.
> 
> CONFIG_DESIGNWARE_WATCHDOG is only defined if CONFIG_HW_WATCHDOG is
> defined, and this is never defined in headers (or in the defconfigs).

This is what I see in socfpga_soc64_common.h on u-boot/master:
153 #ifdef CONFIG_SPL_BUILD
154 #define CONFIG_HW_WATCHDOG

>> The patch is wrong, see above. Also, it's missing a SoB line.
> 
> Ah, I just noticed that, thanks for pointing that out.

Note that I updated u-boot-socfpga/master and sent a PR just now (thanks
for reminding me of that), it contains the DW WDT patches that were
posted to the ML some time ago. You want to rebase the series on top of
that.
Sean Anderson Feb. 2, 2020, 5:48 p.m. UTC | #5 
On 2/2/20 12:40 PM, Marek Vasut wrote:
> On 2/2/20 6:23 PM, Sean Anderson wrote:
>> CONFIG_DESIGNWARE_WATCHDOG is only defined if CONFIG_HW_WATCHDOG is
>> defined, and this is never defined in headers (or in the defconfigs).
> 
> This is what I see in socfpga_soc64_common.h on u-boot/master:
> 153 #ifdef CONFIG_SPL_BUILD
> 154 #define CONFIG_HW_WATCHDOG

Huh, there it is. I guess I expected the usage to be the same as
socfpga_common.h. Would it be best to add DESIGNWARE_WATCHDOG to the
appropriate Kconfigs, defconfigs, or leave it in the header?

> 
>>> The patch is wrong, see above. Also, it's missing a SoB line.
>>
>> Ah, I just noticed that, thanks for pointing that out.
> 
> Note that I updated u-boot-socfpga/master and sent a PR just now (thanks
> for reminding me of that), it contains the DW WDT patches that were
> posted to the ML some time ago. You want to rebase the series on top of
> that.

Ok, I will do that for v2.

--Sean
Marek Vasut Feb. 2, 2020, 5:52 p.m. UTC | #6 
On 2/2/20 6:48 PM, Sean Anderson wrote:
> On 2/2/20 12:40 PM, Marek Vasut wrote:
>> On 2/2/20 6:23 PM, Sean Anderson wrote:
>>> CONFIG_DESIGNWARE_WATCHDOG is only defined if CONFIG_HW_WATCHDOG is
>>> defined, and this is never defined in headers (or in the defconfigs).
>>
>> This is what I see in socfpga_soc64_common.h on u-boot/master:
>> 153 #ifdef CONFIG_SPL_BUILD
>> 154 #define CONFIG_HW_WATCHDOG
> 
> Huh, there it is. I guess I expected the usage to be the same as
> socfpga_common.h. Would it be best to add DESIGNWARE_WATCHDOG to the
> appropriate Kconfigs, defconfigs, or leave it in the header?

I also had to look around, because I knew it was somewhere and the WDT
should've generally been enabled on socfpga (at least the 32bit ones).

Kconfig is indeed the way to go. For bulk enabling config options, it is
better to put them as "select" entry in arch/arm/Kconfig or
arch/arm/mach-foo/Kconfig for the entire platform, so all the boards
pick the option up without any changes to zillion defconfigs.

>>>> The patch is wrong, see above. Also, it's missing a SoB line.
>>>
>>> Ah, I just noticed that, thanks for pointing that out.
>>
>> Note that I updated u-boot-socfpga/master and sent a PR just now (thanks
>> for reminding me of that), it contains the DW WDT patches that were
>> posted to the ML some time ago. You want to rebase the series on top of
>> that.
> 
> Ok, I will do that for v2.

Thanks.

btw what's the motivation for this series, are you hitting some issues
with the WDT on SoCFPGA ?
Sean Anderson Feb. 2, 2020, 6:02 p.m. UTC | #7 
> btw what's the motivation for this series, are you hitting some issues
> with the WDT on SoCFPGA ?

This watchdog appears on the Kendryte K210 CPU, which I am adding
support for. The rest of the board uses devicetree to configure drivers,
so I wanted to add support for this watchdog as well.
Marek Vasut Feb. 3, 2020, 8:13 a.m. UTC | #8 
On 2/2/20 7:02 PM, Sean Anderson wrote:
>> btw what's the motivation for this series, are you hitting some issues
>> with the WDT on SoCFPGA ?
> 
> This watchdog appears on the Kendryte K210 CPU, which I am adding
> support for. The rest of the board uses devicetree to configure drivers,
> so I wanted to add support for this watchdog as well.
> 

You should be able to use what's in u-boot-socfpga/master then. Please
give it a try.
Sean Anderson Feb. 19, 2020, 9:37 p.m. UTC | #9 
On 2/3/20 3:13 AM, Marek Vasut wrote:
> On 2/2/20 7:02 PM, Sean Anderson wrote:
>>> btw what's the motivation for this series, are you hitting some issues
>>> with the WDT on SoCFPGA ?
>>
>> This watchdog appears on the Kendryte K210 CPU, which I am adding
>> support for. The rest of the board uses devicetree to configure drivers,
>> so I wanted to add support for this watchdog as well.
>>
> 
> You should be able to use what's in u-boot-socfpga/master then. Please
> give it a try.
> 

This patch series works except that I still need "[PATCH 2/4] arm: Move
asm/utils.h to log2.h". I will include it with my k210 support series,
since it is specific to that board.

--Sean
Marek Vasut Feb. 20, 2020, 4:39 p.m. UTC | #10 
On 2/19/20 10:37 PM, Sean Anderson wrote:
> On 2/3/20 3:13 AM, Marek Vasut wrote:
>> On 2/2/20 7:02 PM, Sean Anderson wrote:
>>>> btw what's the motivation for this series, are you hitting some issues
>>>> with the WDT on SoCFPGA ?
>>>
>>> This watchdog appears on the Kendryte K210 CPU, which I am adding
>>> support for. The rest of the board uses devicetree to configure drivers,
>>> so I wanted to add support for this watchdog as well.
>>>
>>
>> You should be able to use what's in u-boot-socfpga/master then. Please
>> give it a try.
>>
> 
> This patch series works except that I still need "[PATCH 2/4] arm: Move
> asm/utils.h to log2.h". I will include it with my k210 support series,
> since it is specific to that board.

Great! Rebase on u-boot/master and repost. Thanks.
diff mbox series

Patch

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 8c16d69d33..b717eebe3c 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -176,6 +176,13 @@  config WDT_TANGIER
 	  Intel Tangier SoC. If you're using a board with Intel Tangier
 	  SoC, say Y here.
 
+config DESIGNWARE_WATCHDOG
+	bool "Synopsys Designware watchdog timer support"
+	select HW_WATCHDOG if !WDT
+	help
+	  Enable support for the Synopsys Designware watchdog timer, which can
+	  be found on Altera socfpgas, and on Kendryte CPUs.
+
 config SPL_WDT
 	bool "Enable driver model for watchdog timer drivers in SPL"
 	depends on SPL_DM
diff --git a/include/configs/socfpga_common.h b/include/configs/socfpga_common.h
index 05bfef75c0..5329b19af2 100644
--- a/include/configs/socfpga_common.h
+++ b/include/configs/socfpga_common.h
@@ -105,7 +105,6 @@ 
  * L4 Watchdog
  */
 #ifdef CONFIG_HW_WATCHDOG
-#define CONFIG_DESIGNWARE_WATCHDOG
 #define CONFIG_DW_WDT_BASE		SOCFPGA_L4WD0_ADDRESS
 #define CONFIG_DW_WDT_CLOCK_KHZ		25000
 #endif
diff --git a/include/configs/socfpga_soc64_common.h b/include/configs/socfpga_soc64_common.h
index 4afadafd35..159e60ec6e 100644
--- a/include/configs/socfpga_soc64_common.h
+++ b/include/configs/socfpga_soc64_common.h
@@ -152,7 +152,6 @@  unsigned int cm_get_qspi_controller_clk_hz(void);
  */
 #ifdef CONFIG_SPL_BUILD
 #define CONFIG_HW_WATCHDOG
-#define CONFIG_DESIGNWARE_WATCHDOG
 #define CONFIG_DW_WDT_BASE		SOCFPGA_L4WD0_ADDRESS
 #ifdef CONFIG_TARGET_SOCFPGA_STRATIX10
 #ifndef __ASSEMBLY__