[PATCH] rtc: bd70528: fix link error

[PATCH] rtc: bd70528: fix link error

[Arnd Bergmann]

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)
 	help
 	  If you say Y here you will get support for the RTC
 	  on ROHM BD70528 Power Management IC.
-- 
2.20.0

Re: [PATCH] rtc: bd70528: fix link error

[Vaittinen, Matti]

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.

Re: [PATCH] rtc: bd70528: fix link error

[Arnd Bergmann]

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

Re: [PATCH] rtc: bd70528: fix link error

[Vaittinen, Matti]

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

Re: [PATCH] rtc: bd70528: fix link error

[Yuehaibing]

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

Re: [PATCH] rtc: bd70528: fix link error

[Vaittinen, Matti]

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

Re: [PATCH] rtc: bd70528: fix link error

[Arnd Bergmann]

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