Re: [PATCH V4 1/2] watchdog: imx_sc: Add i.MX system controller watchdog support

[Guenter Roeck]

On 2/28/19 7:46 PM, Anson Huang wrote:
> Hi, Guenter
>
> Best Regards!
> Anson Huang
>
> > -----Original Message-----
> > From: Guenter Roeck [mailto:groeck7@xxxxxxxxx] On Behalf Of Guenter
> > Roeck
> > Sent: 2019å2æ28æ 22:58
> > To: Anson Huang <anson.huang@xxxxxxx>; catalin.marinas@xxxxxxx;
> > will.deacon@xxxxxxx; wim@xxxxxxxxxxxxxxxxxx; shawnguo@xxxxxxxxxx;
> > s.hauer@xxxxxxxxxxxxxx; kernel@xxxxxxxxxxxxxx; festevam@xxxxxxxxx;
> > Andy Gross <andy.gross@xxxxxxxxxx>; heiko@xxxxxxxxx;
> > horms+renesas@xxxxxxxxxxxx; arnd@xxxxxxxx; olof@xxxxxxxxx;
> > jagan@xxxxxxxxxxxxxxxxxxxx; bjorn.andersson@xxxxxxxxxx;
> > enric.balletbo@xxxxxxxxxxxxx; marc.w.gonzalez@xxxxxxx; linux-arm-
> > kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-
> > watchdog@xxxxxxxxxxxxxxx
> > Cc: dl-linux-imx <linux-imx@xxxxxxx>
> > Subject: Re: [PATCH V4 1/2] watchdog: imx_sc: Add i.MX system controller
> > watchdog support
> >
> > On 2/27/19 11:59 PM, Anson Huang wrote:
> > > i.MX8QXP is an ARMv8 SoC which has a Cortex-M4 system controller
> > > inside, the system controller is in charge of controlling power, clock
> > > and watchdog etc..
> > >
> > > This patch adds i.MX system controller watchdog driver support,
> > > watchdog operation needs to be done in secure EL3 mode via
> > > ARM-Trusted-Firmware, using SMC call, CPU will trap into
> > > ARM-Trusted-Firmware and then it will request system controller to do
> > > watchdog operation via IPC.
> > >
> > > Signed-off-by: Anson Huang <Anson.Huang@xxxxxxx>
> > > ---
> > > Changes since V3:
> > >           - add ARM64 dependency for this driver;
> > >           - change SPDX license to GPL-2.0 to match module license;
> > >           - register platform device in driver instead of getting from dts;
> > > 	- return linux error code instead of system controller firmware error
> > code for watchdog operation
> > > 	  failed case.
> > > ---
> > >    drivers/watchdog/Kconfig      |  13 +++
> > >    drivers/watchdog/Makefile     |   1 +
> > >    drivers/watchdog/imx_sc_wdt.c | 201
> > ++++++++++++++++++++++++++++++++++++++++++
> > >    3 files changed, 215 insertions(+)
> > >    create mode 100644 drivers/watchdog/imx_sc_wdt.c
> > >
> > > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index
> > > 65c3c42..8c6575e 100644
> > > --- a/drivers/watchdog/Kconfig
> > > +++ b/drivers/watchdog/Kconfig
> > > @@ -625,6 +625,19 @@ config IMX2_WDT
> > >    	  To compile this driver as a module, choose M here: the
> > >    	  module will be called imx2_wdt.
> > >
> > > +config IMX_SC_WDT
> > > +	tristate "IMX SC Watchdog"
> > > +	depends on (ARCH_MXC && ARM64) || COMPILE_TEST
> > > +	select WATCHDOG_CORE
> > > +	help
> > > +	  This is the driver for the system controller watchdog
> > > +	  on the NXP i.MX SoCs with system controller inside.
> > > +	  If you have one of these processors and wish to have
> > > +	  watchdog support enabled, say Y, otherwise say N.
> > > +
> > > +	  To compile this driver as a module, choose M here: the
> > > +	  module will be called imx_sc_wdt.
> > > +
> >
> > With this patch applied, alpha:allmodconfig and almost all other
> > allmodconfig/allyesconfig builds fail with:
> >
> > ERROR: "__arm_smccc_smc" [drivers/watchdog/imx_sc_wdt.ko] undefined!
> > scripts/Makefile.modpost:92: recipe for target '__modpost' failed
> > make[1]: *** [__modpost] Error 1
> > Makefile:1260: recipe for target 'modules' failed
> >
> > as I told you before would happen. For the future, please consider that
> > forcing me to "prove" such failures does take a significant amount of time,
> > which is not always readily available.
>
> Sorry for wasting your time, it is just because I misunderstand your point, NOT
> that I did NOT force you to prove it.
>
> I am a little confuse, since this is new to me about these configs, I looked into
> other drivers which also use arm_smccc_smc, they do NOT add special config
> dependency for the failure build cause as you said, can you be more detail about
> this build issue, I tried below build, but the failure I met is other, so could you please
> advise how to fix such dependency issue, adding dummy function looks like NOT
> a good way since this arm_smccc_smc() API is widely used in kernel, there should be
> some common solution for it, Thanks in advanced and appreciate for your time.

I am quite sure that the other drivers calling arm_smccc_smc don't have
"|| COMPILE_TEST" as dependency, or they have a secondary dependency.

Randomly picking some

RTC_DRV_IMX_SC - no.
ARM_RK3399_DMC_DEVFREQ - no.
PHY_MVEBU_A3700_COMPHY - yes, but it also has "depends on HAVE_ARM_SMCCC" as
separate line, limiting its scope.

You can either drop "|| COMPILE_TEST" or add "depends on HAVE_ARM_SMCCC"
in a separate line. There may be other options, but I did not explore them.

Guenter