Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <d8b0f332-b8e3-59ac-741c-edc7f2f3341f@baylibre.com>
Date:   Fri, 21 Jul 2023 10:24:11 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.13.0
Subject: Re: [PATCH v2] ARM: multi_v7_defconfig: Enable OMAP watchdog support
Content-Language: en-US
To:     Tony Lindgren <tony@atomide.com>
Cc:     Russell King <linux@armlinux.org.uk>,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        vigneshr@ti.com, afd@ti.com
References: <20230718-enable-omap-wd-v2-1-921f829bc0a5@baylibre.com>
 <4db1d958-7b11-40d9-3f68-3e9390539cbe@baylibre.com>
 <20230721073836.GS5194@atomide.com>
From:   Julien Panis <jpanis@baylibre.com>
In-Reply-To: <20230721073836.GS5194@atomide.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 7/21/23 09:38, Tony Lindgren wrote:
> * Julien Panis <jpanis@baylibre.com> [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 <jpanis@baylibre.com>
> Looks good to me:
>
> Reviewed-by: Tony Lindgren <tony@atomide.com>
>
>> 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