2021-04-22 15:16:42

by Arnd Bergmann

[permalink] [raw]
Subject: [PATCH] [v2] rtc: bd70528: fix BD71815 watchdog dependency

From: Arnd Bergmann <[email protected]>

The added Kconfig dependency is slightly incorrect, which can
lead to a link failure when the watchdog is a loadable module:

arm-linux-gnueabi-ld: drivers/rtc/rtc-bd70528.o: in function `bd70528_set_rtc_based_timers':
rtc-bd70528.c:(.text+0x6cc): undefined reference to `bd70528_wdt_set'
arm-linux-gnueabi-ld: drivers/rtc/rtc-bd70528.o: in function `bd70528_set_time':
rtc-bd70528.c:(.text+0xaa0): undefined reference to `bd70528_wdt_lock'
arm-linux-gnueabi-ld: rtc-bd70528.c:(.text+0xab8): undefined reference to `bd70528_wdt_unlock'
arm-linux-gnueabi-ld: drivers/rtc/rtc-bd70528.o: in function `bd70528_alm_enable':
rtc-bd70528.c:(.text+0xfc0): undefined reference to `bd70528_wdt_lock'
arm-linux-gnueabi-ld: rtc-bd70528.c:(.text+0x1030): undefined reference to `bd70528_wdt_unlock'

The problem is that it allows to be built-in if MFD_ROHM_BD71828
is built-in, even when the watchdog is a loadable module.

Rework this so that having the watchdog as a loadable module always
forces the rtc to be a module as well instead of built-in,
regardless of bd71828.

Fixes: c56dc069f268 ("rtc: bd70528: Support RTC on ROHM BD71815")
Signed-off-by: Arnd Bergmann <[email protected]>
---
v2: Fix as suggested by Guenter Roeck, reword description
---
drivers/rtc/Kconfig | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index d8c13fded164..914497abeef9 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -502,7 +502,8 @@ config RTC_DRV_M41T80_WDT

config RTC_DRV_BD70528
tristate "ROHM BD70528, BD71815 and BD71828 PMIC RTC"
- depends on MFD_ROHM_BD71828 || MFD_ROHM_BD70528 && (BD70528_WATCHDOG || !BD70528_WATCHDOG)
+ depends on MFD_ROHM_BD71828 || MFD_ROHM_BD70528
+ depends on BD70528_WATCHDOG || !BD70528_WATCHDOG
help
If you say Y here you will get support for the RTC
block on ROHM BD70528, BD71815 and BD71828 Power Management IC.
--
2.29.2


2021-04-22 16:11:10

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH] [v2] rtc: bd70528: fix BD71815 watchdog dependency

On Thu, Apr 22, 2021 at 05:15:21PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> The added Kconfig dependency is slightly incorrect, which can
> lead to a link failure when the watchdog is a loadable module:
>
> arm-linux-gnueabi-ld: drivers/rtc/rtc-bd70528.o: in function `bd70528_set_rtc_based_timers':
> rtc-bd70528.c:(.text+0x6cc): undefined reference to `bd70528_wdt_set'
> arm-linux-gnueabi-ld: drivers/rtc/rtc-bd70528.o: in function `bd70528_set_time':
> rtc-bd70528.c:(.text+0xaa0): undefined reference to `bd70528_wdt_lock'
> arm-linux-gnueabi-ld: rtc-bd70528.c:(.text+0xab8): undefined reference to `bd70528_wdt_unlock'
> arm-linux-gnueabi-ld: drivers/rtc/rtc-bd70528.o: in function `bd70528_alm_enable':
> rtc-bd70528.c:(.text+0xfc0): undefined reference to `bd70528_wdt_lock'
> arm-linux-gnueabi-ld: rtc-bd70528.c:(.text+0x1030): undefined reference to `bd70528_wdt_unlock'
>
> The problem is that it allows to be built-in if MFD_ROHM_BD71828
> is built-in, even when the watchdog is a loadable module.
>
> Rework this so that having the watchdog as a loadable module always
> forces the rtc to be a module as well instead of built-in,
> regardless of bd71828.
>
> Fixes: c56dc069f268 ("rtc: bd70528: Support RTC on ROHM BD71815")
> Signed-off-by: Arnd Bergmann <[email protected]>

Reviewed-by: Guenter Roeck <[email protected]>

> ---
> v2: Fix as suggested by Guenter Roeck, reword description
> ---
> drivers/rtc/Kconfig | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index d8c13fded164..914497abeef9 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -502,7 +502,8 @@ config RTC_DRV_M41T80_WDT
>
> config RTC_DRV_BD70528
> tristate "ROHM BD70528, BD71815 and BD71828 PMIC RTC"
> - depends on MFD_ROHM_BD71828 || MFD_ROHM_BD70528 && (BD70528_WATCHDOG || !BD70528_WATCHDOG)
> + depends on MFD_ROHM_BD71828 || MFD_ROHM_BD70528
> + depends on BD70528_WATCHDOG || !BD70528_WATCHDOG
> help
> If you say Y here you will get support for the RTC
> block on ROHM BD70528, BD71815 and BD71828 Power Management IC.
> --
> 2.29.2
>

2021-04-23 03:59:36

by Matti Vaittinen

[permalink] [raw]
Subject: Re: [PATCH] [v2] rtc: bd70528: fix BD71815 watchdog dependency

Hi Arnd & Guenter,

On Thu, 2021-04-22 at 09:08 -0700, Guenter Roeck wrote:
> On Thu, Apr 22, 2021 at 05:15:21PM +0200, Arnd Bergmann wrote:
> > From: Arnd Bergmann <[email protected]>
> >
> > The added Kconfig dependency is slightly incorrect, which can
> > lead to a link failure when the watchdog is a loadable module:
> >
> > arm-linux-gnueabi-ld: drivers/rtc/rtc-bd70528.o: in function
> > `bd70528_set_rtc_based_timers':
> > rtc-bd70528.c:(.text+0x6cc): undefined reference to
> > `bd70528_wdt_set'
> > arm-linux-gnueabi-ld: drivers/rtc/rtc-bd70528.o: in function
> > `bd70528_set_time':
> > rtc-bd70528.c:(.text+0xaa0): undefined reference to
> > `bd70528_wdt_lock'
> > arm-linux-gnueabi-ld: rtc-bd70528.c:(.text+0xab8): undefined
> > reference to `bd70528_wdt_unlock'
> > arm-linux-gnueabi-ld: drivers/rtc/rtc-bd70528.o: in function
> > `bd70528_alm_enable':
> > rtc-bd70528.c:(.text+0xfc0): undefined reference to
> > `bd70528_wdt_lock'
> > arm-linux-gnueabi-ld: rtc-bd70528.c:(.text+0x1030): undefined
> > reference to `bd70528_wdt_unlock'
> >
> > The problem is that it allows to be built-in if MFD_ROHM_BD71828
> > is built-in, even when the watchdog is a loadable module.

Right. I overlooked this as the WDG is not present in BD71828 HW. But
you are correct, it does not mean it can't be configured in on system
with BD71828. (I think Guenter pointed this to me also earlier when I
thought the WDG should not be allowed to be configured in on system
with BD71828). Sorry for the trouble.

> >
> > Rework this so that having the watchdog as a loadable module always
> > forces the rtc to be a module as well instead of built-in,
> > regardless of bd71828.

This sounds like the right thing to do. Thanks a lot for both of you!

> >
> > Fixes: c56dc069f268 ("rtc: bd70528: Support RTC on ROHM BD71815")
> > Signed-off-by: Arnd Bergmann <[email protected]>
>
> Reviewed-by: Guenter Roeck <[email protected]>

Reviewed-by: Matti Vaittinen <[email protected]>

>
> > ---
> > v2: Fix as suggested by Guenter Roeck, reword description
> > ---
> > drivers/rtc/Kconfig | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> > index d8c13fded164..914497abeef9 100644
> > --- a/drivers/rtc/Kconfig
> > +++ b/drivers/rtc/Kconfig
> > @@ -502,7 +502,8 @@ config RTC_DRV_M41T80_WDT
> >
> > config RTC_DRV_BD70528
> > tristate "ROHM BD70528, BD71815 and BD71828 PMIC RTC"
> > - depends on MFD_ROHM_BD71828 || MFD_ROHM_BD70528 &&
> > (BD70528_WATCHDOG || !BD70528_WATCHDOG)
> > + depends on MFD_ROHM_BD71828 || MFD_ROHM_BD70528
> > + depends on BD70528_WATCHDOG || !BD70528_WATCHDOG
> > help
> > If you say Y here you will get support for the RTC
> > block on ROHM BD70528, BD71815 and BD71828 Power Management
> > IC.
> > --
> > 2.29.2
> >


2021-04-28 01:41:11

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH] [v2] rtc: bd70528: fix BD71815 watchdog dependency

On 4/22/21 8:15 AM, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> The added Kconfig dependency is slightly incorrect, which can
> lead to a link failure when the watchdog is a loadable module:
>
> arm-linux-gnueabi-ld: drivers/rtc/rtc-bd70528.o: in function `bd70528_set_rtc_based_timers':
> rtc-bd70528.c:(.text+0x6cc): undefined reference to `bd70528_wdt_set'
> arm-linux-gnueabi-ld: drivers/rtc/rtc-bd70528.o: in function `bd70528_set_time':
> rtc-bd70528.c:(.text+0xaa0): undefined reference to `bd70528_wdt_lock'
> arm-linux-gnueabi-ld: rtc-bd70528.c:(.text+0xab8): undefined reference to `bd70528_wdt_unlock'
> arm-linux-gnueabi-ld: drivers/rtc/rtc-bd70528.o: in function `bd70528_alm_enable':
> rtc-bd70528.c:(.text+0xfc0): undefined reference to `bd70528_wdt_lock'
> arm-linux-gnueabi-ld: rtc-bd70528.c:(.text+0x1030): undefined reference to `bd70528_wdt_unlock'
>
> The problem is that it allows to be built-in if MFD_ROHM_BD71828
> is built-in, even when the watchdog is a loadable module.
>
> Rework this so that having the watchdog as a loadable module always
> forces the rtc to be a module as well instead of built-in,
> regardless of bd71828.
>
> Fixes: c56dc069f268 ("rtc: bd70528: Support RTC on ROHM BD71815")
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> v2: Fix as suggested by Guenter Roeck, reword description
> ---
> drivers/rtc/Kconfig | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index d8c13fded164..914497abeef9 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -502,7 +502,8 @@ config RTC_DRV_M41T80_WDT
>
> config RTC_DRV_BD70528
> tristate "ROHM BD70528, BD71815 and BD71828 PMIC RTC"
> - depends on MFD_ROHM_BD71828 || MFD_ROHM_BD70528 && (BD70528_WATCHDOG || !BD70528_WATCHDOG)
> + depends on MFD_ROHM_BD71828 || MFD_ROHM_BD70528
> + depends on BD70528_WATCHDOG || !BD70528_WATCHDOG
> help
> If you say Y here you will get support for the RTC
> block on ROHM BD70528, BD71815 and BD71828 Power Management IC.
>

Acked-by: Randy Dunlap <[email protected]> # build-tested

thanks.

--
~Randy

2021-05-24 22:27:05

by Alexandre Belloni

[permalink] [raw]
Subject: Re: [PATCH] [v2] rtc: bd70528: fix BD71815 watchdog dependency

On Thu, 22 Apr 2021 17:15:21 +0200, Arnd Bergmann wrote:
> The added Kconfig dependency is slightly incorrect, which can
> lead to a link failure when the watchdog is a loadable module:
>
> arm-linux-gnueabi-ld: drivers/rtc/rtc-bd70528.o: in function `bd70528_set_rtc_based_timers':
> rtc-bd70528.c:(.text+0x6cc): undefined reference to `bd70528_wdt_set'
> arm-linux-gnueabi-ld: drivers/rtc/rtc-bd70528.o: in function `bd70528_set_time':
> rtc-bd70528.c:(.text+0xaa0): undefined reference to `bd70528_wdt_lock'
> arm-linux-gnueabi-ld: rtc-bd70528.c:(.text+0xab8): undefined reference to `bd70528_wdt_unlock'
> arm-linux-gnueabi-ld: drivers/rtc/rtc-bd70528.o: in function `bd70528_alm_enable':
> rtc-bd70528.c:(.text+0xfc0): undefined reference to `bd70528_wdt_lock'
> arm-linux-gnueabi-ld: rtc-bd70528.c:(.text+0x1030): undefined reference to `bd70528_wdt_unlock'
>
> [...]

Applied, thanks!

[1/1] rtc: bd70528: fix BD71815 watchdog dependency
commit: b0ddc5b170058a9ed3c9f031501d735a4eb8ee89

Best regards,
--
Alexandre Belloni <[email protected]>