diff mbox series

[3/4] arm64: defconfig: add S32G RTC module support

Message ID 20240911070028.127659-4-ciprianmarian.costea@oss.nxp.com (mailing list archive)
State New, archived
Headers show
Series add NXP RTC driver support for S32G2/S32G3 SoCs | expand

Commit Message

Ciprian Marian Costea Sept. 11, 2024, 7 a.m. UTC 
From: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>

This patch enables CONFIG_RTC_DRV_S32G as module by default.

Signed-off-by: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

Comments

Krzysztof Kozlowski Sept. 17, 2024, 5:36 p.m. UTC | #1 
On 11/09/2024 09:00, Ciprian Costea wrote:
> From: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
> 
> This patch enables CONFIG_RTC_DRV_S32G as module by default.

We see this from the diff. We do not see why we would like this.

Best regards,
Krzysztof
Ciprian Marian Costea Sept. 18, 2024, 8:02 a.m. UTC | #2 
On 9/17/2024 8:36 PM, Krzysztof Kozlowski wrote:
> On 11/09/2024 09:00, Ciprian Costea wrote:
>> From: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
>>
>> This patch enables CONFIG_RTC_DRV_S32G as module by default.
> 
> We see this from the diff. We do not see why we would like this.
> 
> Best regards,
> Krzysztof
> 

Hello Krzysztof,

Thanks for your feedback.

I can see how not providing any information in the commit message 
regarding why this config is important for S32G2/S32G3 SoCs is not ideal.

Considering this, if I would add such information, would it be 
acceptable to enable this config as module ? Or should I drop this 
commit instead ?

My decision to enable it as module by default was also influenced by 
other similar RTC related configs, which are present in 'defconfig'.

Best Regards,
Ciprian
Krzysztof Kozlowski Sept. 18, 2024, 8:10 a.m. UTC | #3 
On 18/09/2024 10:02, Ciprian Marian Costea wrote:
> On 9/17/2024 8:36 PM, Krzysztof Kozlowski wrote:
>> On 11/09/2024 09:00, Ciprian Costea wrote:
>>> From: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com>
>>>
>>> This patch enables CONFIG_RTC_DRV_S32G as module by default.
>>
>> We see this from the diff. We do not see why we would like this.
>>
>> Best regards,
>> Krzysztof
>>
> 
> Hello Krzysztof,
> 
> Thanks for your feedback.
> 
> I can see how not providing any information in the commit message 
> regarding why this config is important for S32G2/S32G3 SoCs is not ideal.
> 
> Considering this, if I would add such information, would it be 
> acceptable to enable this config as module ? Or should I drop this 
> commit instead ?
> 
> My decision to enable it as module by default was also influenced by 
> other similar RTC related configs, which are present in 'defconfig'.
> 

I don't object the patch contents itself. I want some sort of
explanation in the commit msg. Each commit msg is supposed to answer to
"why?" (unless it is obvious) and the answer is for example that
platform foo bar uses it. See some examples in git history.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 7d32fca64996..5de9ae08294c 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -1198,6 +1198,7 @@  CONFIG_RTC_DRV_DA9063=m
 CONFIG_RTC_DRV_EFI=y
 CONFIG_RTC_DRV_CROS_EC=y
 CONFIG_RTC_DRV_FSL_FTM_ALARM=m
+CONFIG_RTC_DRV_S32G=m
 CONFIG_RTC_DRV_S3C=y
 CONFIG_RTC_DRV_PL031=y
 CONFIG_RTC_DRV_SUN6I=y