| Message ID | 20190708124227.3422311-1-arnd@arndb.de |
|---|---|
| State | New |
| Headers | show |
| Series | rtc: bd70528: fix link error | expand |
Hello Arnd, Thanks for taking a look on this! On Mon, 2019-07-08 at 14:41 +0200, Arnd Bergmann wrote: > With CONFIG_BD70528_WATCHDOG=m, a built-in rtc driver cannot call > into the low-level functions that are part of the watchdog module: > > drivers/rtc/rtc-bd70528.o: In function `bd70528_set_time': > rtc-bd70528.c:(.text+0x22c): undefined reference to > `bd70528_wdt_lock' > rtc-bd70528.c:(.text+0x2a8): undefined reference to > `bd70528_wdt_unlock' > drivers/rtc/rtc-bd70528.o: In function > `bd70528_set_rtc_based_timers': > rtc-bd70528.c:(.text+0x50c): undefined reference to `bd70528_wdt_set' > > Add a Kconfig dependency on this driver, but still allow compile- > testing > without it. > > Fixes: 32a4a4ebf768 ("rtc: bd70528: Initial support for ROHM bd70528 > RTC") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > drivers/rtc/Kconfig | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig > index 3bfc04a86529..7b071cc74422 100644 > --- a/drivers/rtc/Kconfig > +++ b/drivers/rtc/Kconfig > @@ -498,8 +498,10 @@ config RTC_DRV_M41T80_WDT > help > If you say Y here you will get support for the > watchdog timer in the ST M41T60 and M41T80 RTC chips series. > + > config RTC_DRV_BD70528 > tristate "ROHM BD70528 PMIC RTC" > + depends on BD70528_WATCHDOG || (COMPILE_TEST && > !BD70528_WATCHDOG) I am not fan of this. There may well be use-cases where it is desirable to leave the watchdog out but still compile in the RTC. This is why we have static inline stubs in the header for cases where WDG is not compiled in. (RTC does not need to stop WDG if WDG driver is not included) Adding dependency from RTC to MFD for BD70528 should be done - this will avoid most of the issues (And there has been few patches sent for this already). But that's still not complete solution because configuring RTC and MFD to be built in-kernel and WDG as a module will cause errors again. Is there a way to force WDG in-kernel if RTC is in-kernel? (Or disallow configuring RTC in-kernel if WDG is a module - while still allow RTC to be built without WDG? > help > If you say Y here you will get support for the RTC > on ROHM BD70528 Power Management IC.
On Mon, Jul 8, 2019 at 3:24 PM Vaittinen, Matti <Matti.Vaittinen@fi.rohmeurope.com> wrote: > On Mon, 2019-07-08 at 14:41 +0200, Arnd Bergmann wrote: > > With CONFIG_BD70528_WATCHDOG=m, a built-in rtc driver cannot call > > into the low-level functions that are part of the watchdog module: > > > > drivers/rtc/rtc-bd70528.o: In function `bd70528_set_time': > > rtc-bd70528.c:(.text+0x22c): undefined reference to > > `bd70528_wdt_lock' > > rtc-bd70528.c:(.text+0x2a8): undefined reference to > > `bd70528_wdt_unlock' > > drivers/rtc/rtc-bd70528.o: In function > > `bd70528_set_rtc_based_timers': > > rtc-bd70528.c:(.text+0x50c): undefined reference to `bd70528_wdt_set' > > > > Add a Kconfig dependency on this driver, but still allow compile- > > testing > > without it. > > > > Fixes: 32a4a4ebf768 ("rtc: bd70528: Initial support for ROHM bd70528 > > RTC") > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > --- > > drivers/rtc/Kconfig | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig > > index 3bfc04a86529..7b071cc74422 100644 > > --- a/drivers/rtc/Kconfig > > +++ b/drivers/rtc/Kconfig > > @@ -498,8 +498,10 @@ config RTC_DRV_M41T80_WDT > > help > > If you say Y here you will get support for the > > watchdog timer in the ST M41T60 and M41T80 RTC chips series. > > + > > config RTC_DRV_BD70528 > > tristate "ROHM BD70528 PMIC RTC" > > + depends on BD70528_WATCHDOG || (COMPILE_TEST && > > !BD70528_WATCHDOG) > > I am not fan of this. There may well be use-cases where it is desirable > to leave the watchdog out but still compile in the RTC. This is why we > have static inline stubs in the header for cases where WDG is not > compiled in. (RTC does not need to stop WDG if WDG driver is not > included) > > Adding dependency from RTC to MFD for BD70528 should be done - this > will avoid most of the issues (And there has been few patches sent for > this already). But that's still not complete solution because > configuring RTC and MFD to be built in-kernel and WDG as a module will > cause errors again. > > Is there a way to force WDG in-kernel if RTC is in-kernel? (Or > disallow configuring RTC in-kernel if WDG is a module - while still > allow RTC to be built without WDG? We could make this depends on BD70528_WATCHDOG || !BD70528_WATCHDOG which would allow building with or without watchdog, even when not compile-testing, but still disallow the combination of BD70528_WATCHDOG=m with RTC_DRV_BD70528=y. Arnd
Hello Arnd, (Added Randy who also sent a patch to fix this) On Mon, 2019-07-08 at 18:10 +0200, Arnd Bergmann wrote: > On Mon, Jul 8, 2019 at 3:24 PM Vaittinen, Matti > <Matti.Vaittinen@fi.rohmeurope.com> wrote: > > > On Mon, 2019-07-08 at 14:41 +0200, Arnd Bergmann wrote: > > > With CONFIG_BD70528_WATCHDOG=m, a built-in rtc driver cannot call > > > into the low-level functions that are part of the watchdog > > > module: > > > > > > drivers/rtc/rtc-bd70528.o: In function `bd70528_set_time': > > > rtc-bd70528.c:(.text+0x22c): undefined reference to > > > `bd70528_wdt_lock' > > > rtc-bd70528.c:(.text+0x2a8): undefined reference to > > > `bd70528_wdt_unlock' > > > drivers/rtc/rtc-bd70528.o: In function > > > `bd70528_set_rtc_based_timers': > > > rtc-bd70528.c:(.text+0x50c): undefined reference to > > > `bd70528_wdt_set' > > > > > > Add a Kconfig dependency on this driver, but still allow compile- > > > testing > > > without it. > > > > > > Fixes: 32a4a4ebf768 ("rtc: bd70528: Initial support for ROHM > > > bd70528 > > > RTC") > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > > --- > > > drivers/rtc/Kconfig | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig > > > index 3bfc04a86529..7b071cc74422 100644 > > > --- a/drivers/rtc/Kconfig > > > +++ b/drivers/rtc/Kconfig > > > @@ -498,8 +498,10 @@ config RTC_DRV_M41T80_WDT > > > help > > > If you say Y here you will get support for the > > > watchdog timer in the ST M41T60 and M41T80 RTC chips > > > series. > > > + > > > config RTC_DRV_BD70528 > > > tristate "ROHM BD70528 PMIC RTC" > > > + depends on BD70528_WATCHDOG || (COMPILE_TEST && > > > !BD70528_WATCHDOG) > > > > I am not fan of this. There may well be use-cases where it is > > desirable > > to leave the watchdog out but still compile in the RTC. This is why > > we > > have static inline stubs in the header for cases where WDG is not > > compiled in. (RTC does not need to stop WDG if WDG driver is not > > included) > > > > Adding dependency from RTC to MFD for BD70528 should be done - this > > will avoid most of the issues (And there has been few patches sent > > for > > this already). But that's still not complete solution because > > configuring RTC and MFD to be built in-kernel and WDG as a module > > will > > cause errors again. > > > > Is there a way to force WDG in-kernel if RTC is in-kernel? (Or > > disallow configuring RTC in-kernel if WDG is a module - while still > > allow RTC to be built without WDG? > > We could make this > > depends on BD70528_WATCHDOG || !BD70528_WATCHDOG > > which would allow building with or without watchdog, even when not > compile-testing, but still disallow the combination of > . Thanks for teaching me Arnd! That is clever :) We need something like depends on MFD_ROHM_BD70528 && (BD70528_WATCHDOG || !BD70528_WATCHDOG) (I'm not sure if parenthesis are Ok and respected in Kconfig). I would never have thought of BD70528_WATCHDOG || !BD70528_WATCHDOG - it looks awkward at first sight but indeed - depends on BD70528_WATCHDOG disallows BD70528_WATCHDOG=m with RTC_DRV_BD70528=y while !BD70528_WATCHDOG allows BD70528_WATCHDOG=n. Brilliant and exactly what we need :) Thanks a bunch! > > Arnd
On 2019/7/9 13:25, Vaittinen, Matti wrote: > Hello Arnd, > > (Added Randy who also sent a patch to fix this) > > On Mon, 2019-07-08 at 18:10 +0200, Arnd Bergmann wrote: >> On Mon, Jul 8, 2019 at 3:24 PM Vaittinen, Matti >> <Matti.Vaittinen@fi.rohmeurope.com> wrote: >> >>> On Mon, 2019-07-08 at 14:41 +0200, Arnd Bergmann wrote: >>>> With CONFIG_BD70528_WATCHDOG=m, a built-in rtc driver cannot call >>>> into the low-level functions that are part of the watchdog >>>> module: >>>> >>>> drivers/rtc/rtc-bd70528.o: In function `bd70528_set_time': >>>> rtc-bd70528.c:(.text+0x22c): undefined reference to >>>> `bd70528_wdt_lock' >>>> rtc-bd70528.c:(.text+0x2a8): undefined reference to >>>> `bd70528_wdt_unlock' >>>> drivers/rtc/rtc-bd70528.o: In function >>>> `bd70528_set_rtc_based_timers': >>>> rtc-bd70528.c:(.text+0x50c): undefined reference to >>>> `bd70528_wdt_set' >>>> >>>> Add a Kconfig dependency on this driver, but still allow compile- >>>> testing >>>> without it. >>>> >>>> Fixes: 32a4a4ebf768 ("rtc: bd70528: Initial support for ROHM >>>> bd70528 >>>> RTC") >>>> Signed-off-by: Arnd Bergmann <arnd@arndb.de> >>>> --- >>>> drivers/rtc/Kconfig | 2 ++ >>>> 1 file changed, 2 insertions(+) >>>> >>>> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig >>>> index 3bfc04a86529..7b071cc74422 100644 >>>> --- a/drivers/rtc/Kconfig >>>> +++ b/drivers/rtc/Kconfig >>>> @@ -498,8 +498,10 @@ config RTC_DRV_M41T80_WDT >>>> help >>>> If you say Y here you will get support for the >>>> watchdog timer in the ST M41T60 and M41T80 RTC chips >>>> series. >>>> + >>>> config RTC_DRV_BD70528 >>>> tristate "ROHM BD70528 PMIC RTC" >>>> + depends on BD70528_WATCHDOG || (COMPILE_TEST && >>>> !BD70528_WATCHDOG) >>> >>> I am not fan of this. There may well be use-cases where it is >>> desirable >>> to leave the watchdog out but still compile in the RTC. This is why >>> we >>> have static inline stubs in the header for cases where WDG is not >>> compiled in. (RTC does not need to stop WDG if WDG driver is not >>> included) >>> >>> Adding dependency from RTC to MFD for BD70528 should be done - this >>> will avoid most of the issues (And there has been few patches sent >>> for >>> this already). But that's still not complete solution because >>> configuring RTC and MFD to be built in-kernel and WDG as a module >>> will >>> cause errors again. >>> >>> Is there a way to force WDG in-kernel if RTC is in-kernel? (Or >>> disallow configuring RTC in-kernel if WDG is a module - while still >>> allow RTC to be built without WDG? >> >> We could make this >> >> depends on BD70528_WATCHDOG || !BD70528_WATCHDOG >> >> which would allow building with or without watchdog, even when not >> compile-testing, but still disallow the combination of >> . > > Thanks for teaching me Arnd! That is clever :) We need something like > > depends on MFD_ROHM_BD70528 && (BD70528_WATCHDOG || !BD70528_WATCHDOG) > > (I'm not sure if parenthesis are Ok and respected in Kconfig). I would > never have thought of BD70528_WATCHDOG || !BD70528_WATCHDOG - it looks > awkward at first sight but indeed - depends on BD70528_WATCHDOG > disallows BD70528_WATCHDOG=m with RTC_DRV_BD70528=y while > !BD70528_WATCHDOG allows BD70528_WATCHDOG=n. Brilliant and exactly what > we need :) Thanks a bunch! Hello Vaittinen, the issue still exists in linux-next 20190807, any plan? > >> >> Arnd
Hello All, On Thu, 2019-08-08 at 10:29 +0800, Yuehaibing wrote: > On 2019/7/9 13:25, Vaittinen, Matti wrote: > > Hello Arnd, > > > > (Added Randy who also sent a patch to fix this) > > > > On Mon, 2019-07-08 at 18:10 +0200, Arnd Bergmann wrote: > > > On Mon, Jul 8, 2019 at 3:24 PM Vaittinen, Matti > > > <Matti.Vaittinen@fi.rohmeurope.com> wrote: > > > > > > > On Mon, 2019-07-08 at 14:41 +0200, Arnd Bergmann wrote: > > > > > With CONFIG_BD70528_WATCHDOG=m, a built-in rtc driver cannot > > > > > call > > > > > into the low-level functions that are part of the watchdog > > > > > module: > > > > > > > > > > drivers/rtc/rtc-bd70528.o: In function `bd70528_set_time': > > > > > rtc-bd70528.c:(.text+0x22c): undefined reference to > > > > > `bd70528_wdt_lock' > > > > > rtc-bd70528.c:(.text+0x2a8): undefined reference to > > > > > `bd70528_wdt_unlock' > > > > > drivers/rtc/rtc-bd70528.o: In function > > > > > `bd70528_set_rtc_based_timers': > > > > > rtc-bd70528.c:(.text+0x50c): undefined reference to > > > > > `bd70528_wdt_set' > > > > > > > > > > Add a Kconfig dependency on this driver, but still allow > > > > > compile- > > > > > testing > > > > > without it. > > > > > > > > > > Fixes: 32a4a4ebf768 ("rtc: bd70528: Initial support for ROHM > > > > > bd70528 > > > > > RTC") > > > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > > > > --- > > > > > drivers/rtc/Kconfig | 2 ++ > > > > > 1 file changed, 2 insertions(+) > > > > > > > > > > diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig > > > > > index 3bfc04a86529..7b071cc74422 100644 > > > > > --- a/drivers/rtc/Kconfig > > > > > +++ b/drivers/rtc/Kconfig > > > > > @@ -498,8 +498,10 @@ config RTC_DRV_M41T80_WDT > > > > > help > > > > > If you say Y here you will get support for the > > > > > watchdog timer in the ST M41T60 and M41T80 RTC chips > > > > > series. > > > > > + > > > > > config RTC_DRV_BD70528 > > > > > tristate "ROHM BD70528 PMIC RTC" > > > > > + depends on BD70528_WATCHDOG || (COMPILE_TEST && > > > > > !BD70528_WATCHDOG) > > > > > > > > I am not fan of this. There may well be use-cases where it is > > > > desirable > > > > to leave the watchdog out but still compile in the RTC. This is > > > > why > > > > we > > > > have static inline stubs in the header for cases where WDG is > > > > not > > > > compiled in. (RTC does not need to stop WDG if WDG driver is > > > > not > > > > included) > > > > > > > > Adding dependency from RTC to MFD for BD70528 should be done - > > > > this > > > > will avoid most of the issues (And there has been few patches > > > > sent > > > > for > > > > this already). But that's still not complete solution because > > > > configuring RTC and MFD to be built in-kernel and WDG as a > > > > module > > > > will > > > > cause errors again. > > > > > > > > Is there a way to force WDG in-kernel if RTC is in-kernel? (Or > > > > disallow configuring RTC in-kernel if WDG is a module - while > > > > still > > > > allow RTC to be built without WDG? > > > > > > We could make this > > > > > > depends on BD70528_WATCHDOG || !BD70528_WATCHDOG > > > > > > which would allow building with or without watchdog, even when > > > not > > > compile-testing, but still disallow the combination of > > > . > > > > Thanks for teaching me Arnd! That is clever :) We need something > > like > > > > depends on MFD_ROHM_BD70528 && (BD70528_WATCHDOG || > > !BD70528_WATCHDOG) > > > > (I'm not sure if parenthesis are Ok and respected in Kconfig). I > > would > > never have thought of BD70528_WATCHDOG || !BD70528_WATCHDOG - it > > looks > > awkward at first sight but indeed - depends on BD70528_WATCHDOG > > disallows BD70528_WATCHDOG=m with RTC_DRV_BD70528=y while > > !BD70528_WATCHDOG allows BD70528_WATCHDOG=n. Brilliant and exactly > > what > > we need :) Thanks a bunch! > > Hello Vaittinen, > > the issue still exists in linux-next 20190807, any plan? > > Sorry folks. I thought Arnd would send new patch - I didn't want to > steal his work ;) I will be back to my normal life next week so I > will send a patch at monday if the issue is still open! Br, Matti
On Thu, Aug 8, 2019 at 9:53 PM Vaittinen, Matti <Matti.Vaittinen@fi.rohmeurope.com> wrote: > On Thu, 2019-08-08 at 10:29 +0800, Yuehaibing wrote: > > On 2019/7/9 13:25, Vaittinen, Matti wrote: > > > awkward at first sight but indeed - depends on BD70528_WATCHDOG > > > disallows BD70528_WATCHDOG=m with RTC_DRV_BD70528=y while > > > !BD70528_WATCHDOG allows BD70528_WATCHDOG=n. Brilliant and exactly > > > what > > > we need :) Thanks a bunch! > > > > Hello Vaittinen, > > > > the issue still exists in linux-next 20190807, any plan? > > > > Sorry folks. I thought Arnd would send new patch - I didn't want to > steal his work ;) I will be back to my normal life next week so I > will send a patch at monday if the issue is still open! Sorry I lost track, and thanks for the offer to take care of it. Please add my one Reported-by: Arnd Bergmann <arnd@arndb.de> and/or Suggested-by: Arnd Bergmann <arnd@arndb.de> as appropriate when you create that patch. Arnd
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig index 3bfc04a86529..7b071cc74422 100644 --- a/drivers/rtc/Kconfig +++ b/drivers/rtc/Kconfig @@ -498,8 +498,10 @@ config RTC_DRV_M41T80_WDT help If you say Y here you will get support for the watchdog timer in the ST M41T60 and M41T80 RTC chips series. + config RTC_DRV_BD70528 tristate "ROHM BD70528 PMIC RTC" + depends on BD70528_WATCHDOG || (COMPILE_TEST && !BD70528_WATCHDOG) help If you say Y here you will get support for the RTC on ROHM BD70528 Power Management IC.
With CONFIG_BD70528_WATCHDOG=m, a built-in rtc driver cannot call into the low-level functions that are part of the watchdog module: drivers/rtc/rtc-bd70528.o: In function `bd70528_set_time': rtc-bd70528.c:(.text+0x22c): undefined reference to `bd70528_wdt_lock' rtc-bd70528.c:(.text+0x2a8): undefined reference to `bd70528_wdt_unlock' drivers/rtc/rtc-bd70528.o: In function `bd70528_set_rtc_based_timers': rtc-bd70528.c:(.text+0x50c): undefined reference to `bd70528_wdt_set' Add a Kconfig dependency on this driver, but still allow compile-testing without it. Fixes: 32a4a4ebf768 ("rtc: bd70528: Initial support for ROHM bd70528 RTC") Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- drivers/rtc/Kconfig | 2 ++ 1 file changed, 2 insertions(+) -- 2.20.0