2023-07-18 15:42:18

by Julien Panis

[permalink] [raw]
Subject: [PATCH v2] ARM: multi_v7_defconfig: Enable OMAP watchdog support

Increase build and test coverage by enabling support for OMAP watchdog,
as used on TI OMAP based boards.

The watchdog timer is an upward counter capable of generating a pulse on
the reset pin and an interrupt to the device system modules following an
overflow condition.

Signed-off-by: Julien Panis <[email protected]>
---
Enable OMAP watchdog support in multi_v7_defconfig for TI OMAP based boards.
---
Changes in v2:
- Add explanations in commit description.
- Link to v1: https://lore.kernel.org/r/[email protected]
---
arch/arm/configs/multi_v7_defconfig | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index f0800f806b5f..7d93e21e0cb9 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -554,6 +554,7 @@ CONFIG_SAMA5D4_WATCHDOG=y
CONFIG_S3C2410_WATCHDOG=m
CONFIG_DW_WATCHDOG=y
CONFIG_DAVINCI_WATCHDOG=m
+CONFIG_OMAP_WATCHDOG=m
CONFIG_ORION_WATCHDOG=y
CONFIG_RN5T618_WATCHDOG=y
CONFIG_SUNXI_WATCHDOG=y

---
base-commit: fdf0eaf11452d72945af31804e2a1048ee1b574c
change-id: 20230718-enable-omap-wd-6a563c280752

Best regards,
--
Julien Panis <[email protected]>



2023-07-19 10:18:02

by Julien Panis

[permalink] [raw]
Subject: Re: [PATCH v2] ARM: multi_v7_defconfig: Enable OMAP watchdog support

On 7/18/23 16:58, Julien Panis wrote:
> Increase build and test coverage by enabling support for OMAP watchdog,
> as used on TI OMAP based boards.
>
> The watchdog timer is an upward counter capable of generating a pulse on
> the reset pin and an interrupt to the device system modules following an
> overflow condition.
>
> Signed-off-by: Julien Panis <[email protected]>

Maybe this patch should not be applied actually. I have 2 questions:

[Q1] Using the following cmd sequence leads to a modified 'multi_v7_defconfig' file:
      'make multi_v7_defconfig'
      'make savedefconfig'
      'mv defconfig arch/arm/configs/multi_v7_defconfig'
...even without modifying CONFIG_OMAP_WATCHDOG flag.
I guess it's due to modifications in various Kconfig files (dependencies for instance).
And perhaps it's also due to previous modifications of 'multi_v7_defconfig' file that
were not done by using 'make savedefconfig' (?)
How should I handle that for this patch ? This v2 has been created by modifying
'multi_v7_defconfig' file manually. Using 'make savedefconfig' would be cleaner,
but as a result many flags would be re-organized whereas the commit intends to
enable 1 flag only.

[Q2] I would like to add another flag in order to enable CONFIG_RTC_DRV_OMAP.
Is it better grouping CONFIG_OMAP_WATCHDOG and CONFIG_RTC_DRV_OMAP in
a single commit ? What's recommended ?

Julien Panis


2023-07-21 07:54:43

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH v2] ARM: multi_v7_defconfig: Enable OMAP watchdog support

* Julien Panis <[email protected]> [230719 10:09]:
> On 7/18/23 16:58, Julien Panis wrote:
> > Increase build and test coverage by enabling support for OMAP watchdog,
> > as used on TI OMAP based boards.
> >
> > The watchdog timer is an upward counter capable of generating a pulse on
> > the reset pin and an interrupt to the device system modules following an
> > overflow condition.
> >
> > Signed-off-by: Julien Panis <[email protected]>

Looks good to me:

Reviewed-by: Tony Lindgren <[email protected]>

> Maybe this patch should not be applied actually. I have 2 questions:
>
> [Q1] Using the following cmd sequence leads to a modified 'multi_v7_defconfig' file:
>       'make multi_v7_defconfig'
>       'make savedefconfig'
>       'mv defconfig arch/arm/configs/multi_v7_defconfig'
> ...even without modifying CONFIG_OMAP_WATCHDOG flag.
> I guess it's due to modifications in various Kconfig files (dependencies for instance).
> And perhaps it's also due to previous modifications of 'multi_v7_defconfig' file that
> were not done by using 'make savedefconfig' (?)
> How should I handle that for this patch ? This v2 has been created by modifying
> 'multi_v7_defconfig' file manually. Using 'make savedefconfig' would be cleaner,
> but as a result many flags would be re-organized whereas the commit intends to
> enable 1 flag only.

After make savedefconfig you can take a look where the new option got placed
and then throw away the changes and add the entry manually :)

> [Q2] I would like to add another flag in order to enable CONFIG_RTC_DRV_OMAP.
> Is it better grouping CONFIG_OMAP_WATCHDOG and CONFIG_RTC_DRV_OMAP in
> a single commit ? What's recommended ?

Adding both is fine for the defconfig change, just try to place them where
they would end up after savedefconfig to avoid it getting more out of sync.

Regards,

Tony

2023-07-21 08:47:48

by Julien Panis

[permalink] [raw]
Subject: Re: [PATCH v2] ARM: multi_v7_defconfig: Enable OMAP watchdog support

On 7/21/23 09:38, Tony Lindgren wrote:
> * Julien Panis <[email protected]> [230719 10:09]:
>> On 7/18/23 16:58, Julien Panis wrote:
>>> Increase build and test coverage by enabling support for OMAP watchdog,
>>> as used on TI OMAP based boards.
>>>
>>> The watchdog timer is an upward counter capable of generating a pulse on
>>> the reset pin and an interrupt to the device system modules following an
>>> overflow condition.
>>>
>>> Signed-off-by: Julien Panis <[email protected]>
> Looks good to me:
>
> Reviewed-by: Tony Lindgren <[email protected]>
>
>> Maybe this patch should not be applied actually. I have 2 questions:
>>
>> [Q1] Using the following cmd sequence leads to a modified 'multi_v7_defconfig' file:
>>       'make multi_v7_defconfig'
>>       'make savedefconfig'
>>       'mv defconfig arch/arm/configs/multi_v7_defconfig'
>> ...even without modifying CONFIG_OMAP_WATCHDOG flag.
>> I guess it's due to modifications in various Kconfig files (dependencies for instance).
>> And perhaps it's also due to previous modifications of 'multi_v7_defconfig' file that
>> were not done by using 'make savedefconfig' (?)
>> How should I handle that for this patch ? This v2 has been created by modifying
>> 'multi_v7_defconfig' file manually. Using 'make savedefconfig' would be cleaner,
>> but as a result many flags would be re-organized whereas the commit intends to
>> enable 1 flag only.
> After make savedefconfig you can take a look where the new option got placed
> and then throw away the changes and add the entry manually :)
>
>> [Q2] I would like to add another flag in order to enable CONFIG_RTC_DRV_OMAP.
>> Is it better grouping CONFIG_OMAP_WATCHDOG and CONFIG_RTC_DRV_OMAP in
>> a single commit ? What's recommended ?
> Adding both is fine for the defconfig change, just try to place them where
> they would end up after savedefconfig to avoid it getting more out of sync.
>
> Regards,
>
> Tony

OK, thank you for these explanations.
There will be other flags to add (much more than 2 actually !). So, it's not worth merging
this patch. I will send another patch with all the flags that must be set.

Julien