LinuxLists.cc - [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX

[permalink] [raw]

Subject: [PATCH V3 5/7] input: keyboard: imx_sc_key: Fix build warning for !CONFIG_IMX_SCU case

Fix below build warning when COMPILE_TEST is enabled while IMX_SCU is not:

drivers/input/keyboard/imx_sc_key.c: In function ‘imx_sc_check_for_events’:
drivers/input/keyboard/imx_sc_key.c:87:27: warning: ‘msg.state’ is used
uninitialized in this function [-Wuninitialized]
state = (bool)(msg.state & 0xff);
^
AR drivers/input/keyboard/built-in.a

Signed-off-by: Anson Huang <[email protected]>
---
No change.
---
drivers/input/keyboard/imx_sc_key.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/input/keyboard/imx_sc_key.c b/drivers/input/keyboard/imx_sc_key.c
index 2672fd4..1b55348 100644
--- a/drivers/input/keyboard/imx_sc_key.c
+++ b/drivers/input/keyboard/imx_sc_key.c
@@ -69,6 +69,7 @@ static void imx_sc_check_for_events(struct work_struct *work)
hdr->func = IMX_SC_MISC_FUNC_GET_BUTTON_STATUS;
hdr->size = 1;

+ msg.state = 0;
error = imx_scu_call_rpc(priv->key_ipc_handle, &msg, true);
if (error) {
dev_err(&input->dev, "read imx sc key failed, error %d\n", error);
--
2.7.4

2020-03-09 00:47:28

[permalink] [raw]

Subject: [PATCH V3 6/7] watchdog: add COMPILE_TEST support for IMX_SC_WDT

Add COMPILE_TEST support to i.MX SC watchdog driver for better compile
testing coverage.

Signed-off-by: Anson Huang <[email protected]>
---
No change.
---
drivers/watchdog/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index cec868f..c5cc567 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -711,7 +711,7 @@ config IMX2_WDT
config IMX_SC_WDT
tristate "IMX SC Watchdog"
depends on HAVE_ARM_SMCCC
- depends on IMX_SCU
+ depends on IMX_SCU || COMPILE_TEST
select WATCHDOG_CORE
help
This is the driver for the system controller watchdog
--
2.7.4

2020-03-09 11:06:57

[permalink] [raw]

Subject: Re: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU case

On 09/03/2020 08:38:14+0800, Anson Huang wrote:
> Add stubs for those i.MX SCU APIs to make those modules depending
> on IMX_SCU can pass build when COMPILE_TEST is enabled.
>
> Signed-off-by: Anson Huang <[email protected]>
> ---
> Changes since V2:
> - return error for stubs.

I'm not sure why you are sending v3 with the stubs as we determined that
2/7 is enough to compile all the drivers with COMPILE_TEST.

--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

2020-03-09 12:22:02

[permalink] [raw]

Subject: RE: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU case

Hi, Alexandre

> Subject: Re: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU
> case
>
> On 09/03/2020 08:38:14+0800, Anson Huang wrote:
> > Add stubs for those i.MX SCU APIs to make those modules depending on
> > IMX_SCU can pass build when COMPILE_TEST is enabled.
> >
> > Signed-off-by: Anson Huang <[email protected]>
> > ---
> > Changes since V2:
> > - return error for stubs.
>
> I'm not sure why you are sending v3 with the stubs as we determined that
> 2/7 is enough to compile all the drivers with COMPILE_TEST.

It is just because I am NOT sure which approach maintainer prefer, the V3 is to
address the comment of V2. If everyone agree that 2/7 is enough, then I think IMX_SCU
maintainer can just pick up V1 patch series, sorry for the confusion.

Thanks,
Anson

2020-03-09 13:28:23

[permalink] [raw]

Subject: Re: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU case

On 3/9/20 4:06 AM, Alexandre Belloni wrote:
> On 09/03/2020 08:38:14+0800, Anson Huang wrote:
>> Add stubs for those i.MX SCU APIs to make those modules depending
>> on IMX_SCU can pass build when COMPILE_TEST is enabled.
>>
>> Signed-off-by: Anson Huang <[email protected]>
>> ---
>> Changes since V2:
>> - return error for stubs.
>
> I'm not sure why you are sending v3 with the stubs as we determined that
> 2/7 is enough to compile all the drivers with COMPILE_TEST.
>
>
2/7 alone is not sufficient. With only 2/7, one can explicitly configure
IMX_SCU=n, COMPILE_TEST=y, and get lots of compile failures. Granted,
one should not do that, but 0day does (I don't know if that is the result
of RANDCONFIG), and I am not looking forward having to deal with the
fallout.

Guenter

2020-03-09 13:38:58

[permalink] [raw]

Subject: RE: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU case

> Subject: Re: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU
> case
>
> On 3/9/20 4:06 AM, Alexandre Belloni wrote:
> > On 09/03/2020 08:38:14+0800, Anson Huang wrote:
> >> Add stubs for those i.MX SCU APIs to make those modules depending on
> >> IMX_SCU can pass build when COMPILE_TEST is enabled.
> >>
> >> Signed-off-by: Anson Huang <[email protected]>
> >> ---
> >> Changes since V2:
> >> - return error for stubs.
> >
> > I'm not sure why you are sending v3 with the stubs as we determined
> > that
> > 2/7 is enough to compile all the drivers with COMPILE_TEST.
> >
> >
> 2/7 alone is not sufficient. With only 2/7, one can explicitly configure
> IMX_SCU=n, COMPILE_TEST=y, and get lots of compile failures. Granted, one
> should not do that, but 0day does (I don't know if that is the result of
> RANDCONFIG), and I am not looking forward having to deal with the fallout.

So the V3 patch series looks better, adding stubs can cover various corner cases.

Anson

2020-03-09 13:42:02

[permalink] [raw]

Subject: RE: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU case

> Subject: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU case

I have one patch pending reviewing.
https://patchwork.kernel.org/patch/11395247/

Thanks,
Peng.

>
> Add stubs for those i.MX SCU APIs to make those modules depending on
> IMX_SCU can pass build when COMPILE_TEST is enabled.
>
> Signed-off-by: Anson Huang <[email protected]>
> ---
> Changes since V2:
> - return error for stubs.
> ---
> include/linux/firmware/imx/ipc.h | 11 +++++++++++
> include/linux/firmware/imx/sci.h | 19 +++++++++++++++++++
> 2 files changed, 30 insertions(+)
>
> diff --git a/include/linux/firmware/imx/ipc.h
> b/include/linux/firmware/imx/ipc.h
> index 8910574..9e3d808 100644
> --- a/include/linux/firmware/imx/ipc.h
> +++ b/include/linux/firmware/imx/ipc.h
> @@ -34,6 +34,7 @@ struct imx_sc_rpc_msg {
> uint8_t func;
> };
>
> +#ifdef CONFIG_IMX_SCU
> /*
> * This is an function to send an RPC message over an IPC channel.
> * It is called by client-side SCFW API function shims.
> @@ -55,4 +56,14 @@ int imx_scu_call_rpc(struct imx_sc_ipc *ipc, void *msg,
> bool have_resp);
> * @return Returns an error code (0 = success, failed if < 0)
> */
> int imx_scu_get_handle(struct imx_sc_ipc **ipc);
> +#else
> +static inline int imx_scu_call_rpc(struct imx_sc_ipc *ipc, void *msg,
> +bool have_resp) {
> + return -ENOENT;
> +}
> +static inline int imx_scu_get_handle(struct imx_sc_ipc **ipc) {
> + return -ENOENT;
> +}
> +#endif
> #endif /* _SC_IPC_H */
> diff --git a/include/linux/firmware/imx/sci.h
> b/include/linux/firmware/imx/sci.h
> index 17ba4e4..022129b 100644
> --- a/include/linux/firmware/imx/sci.h
> +++ b/include/linux/firmware/imx/sci.h
> @@ -16,8 +16,27 @@
> #include <linux/firmware/imx/svc/misc.h> #include
> <linux/firmware/imx/svc/pm.h>
>
> +#ifdef CONFIG_IMX_SCU
> int imx_scu_enable_general_irq_channel(struct device *dev); int
> imx_scu_irq_register_notifier(struct notifier_block *nb); int
> imx_scu_irq_unregister_notifier(struct notifier_block *nb); int
> imx_scu_irq_group_enable(u8 group, u32 mask, u8 enable);
> +#else
> +static inline int imx_scu_enable_general_irq_channel(struct device
> +*dev) {
> + return -ENOENT;
> +}
> +static inline int imx_scu_irq_register_notifier(struct notifier_block
> +*nb) {
> + return -ENOENT;
> +}
> +static inline int imx_scu_irq_unregister_notifier(struct notifier_block
> +*nb) {
> + return -ENOENT;
> +}
> +static inline int imx_scu_irq_group_enable(u8 group, u32 mask, u8
> +enable) {
> + return -ENOENT;
> +}
> +#endif
> #endif /* _SC_SCI_H */
> --
> 2.7.4

2020-03-09 14:10:42

[permalink] [raw]

Subject: RE: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU case

> Subject: RE: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU
> case
>
> > Subject: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU
> > case
>
> I have one patch pending reviewing.
> https://patchwork.kernel.org/patch/11395247/

OK, if your patch is picked up, then 1/7 is unnecessary for this patch series, but
the rest are still needed.

Anson

>
> Thanks,
> Peng.
>
> >
> > Add stubs for those i.MX SCU APIs to make those modules depending on
> > IMX_SCU can pass build when COMPILE_TEST is enabled.
> >
> > Signed-off-by: Anson Huang <[email protected]>
> > ---
> > Changes since V2:
> > - return error for stubs.
> > ---
> > include/linux/firmware/imx/ipc.h | 11 +++++++++++
> > include/linux/firmware/imx/sci.h | 19 +++++++++++++++++++
> > 2 files changed, 30 insertions(+)
> >
> > diff --git a/include/linux/firmware/imx/ipc.h
> > b/include/linux/firmware/imx/ipc.h
> > index 8910574..9e3d808 100644
> > --- a/include/linux/firmware/imx/ipc.h
> > +++ b/include/linux/firmware/imx/ipc.h
> > @@ -34,6 +34,7 @@ struct imx_sc_rpc_msg {
> > uint8_t func;
> > };
> >
> > +#ifdef CONFIG_IMX_SCU
> > /*
> > * This is an function to send an RPC message over an IPC channel.
> > * It is called by client-side SCFW API function shims.
> > @@ -55,4 +56,14 @@ int imx_scu_call_rpc(struct imx_sc_ipc *ipc, void
> > *msg, bool have_resp);
> > * @return Returns an error code (0 = success, failed if < 0)
> > */
> > int imx_scu_get_handle(struct imx_sc_ipc **ipc);
> > +#else
> > +static inline int imx_scu_call_rpc(struct imx_sc_ipc *ipc, void *msg,
> > +bool have_resp) {
> > + return -ENOENT;
> > +}
> > +static inline int imx_scu_get_handle(struct imx_sc_ipc **ipc) {
> > + return -ENOENT;
> > +}
> > +#endif
> > #endif /* _SC_IPC_H */
> > diff --git a/include/linux/firmware/imx/sci.h
> > b/include/linux/firmware/imx/sci.h
> > index 17ba4e4..022129b 100644
> > --- a/include/linux/firmware/imx/sci.h
> > +++ b/include/linux/firmware/imx/sci.h
> > @@ -16,8 +16,27 @@
> > #include <linux/firmware/imx/svc/misc.h> #include
> > <linux/firmware/imx/svc/pm.h>
> >
> > +#ifdef CONFIG_IMX_SCU
> > int imx_scu_enable_general_irq_channel(struct device *dev); int
> > imx_scu_irq_register_notifier(struct notifier_block *nb); int
> > imx_scu_irq_unregister_notifier(struct notifier_block *nb); int
> > imx_scu_irq_group_enable(u8 group, u32 mask, u8 enable);
> > +#else
> > +static inline int imx_scu_enable_general_irq_channel(struct device
> > +*dev) {
> > + return -ENOENT;
> > +}
> > +static inline int imx_scu_irq_register_notifier(struct notifier_block
> > +*nb) {
> > + return -ENOENT;
> > +}
> > +static inline int imx_scu_irq_unregister_notifier(struct
> > +notifier_block
> > +*nb) {
> > + return -ENOENT;
> > +}
> > +static inline int imx_scu_irq_group_enable(u8 group, u32 mask, u8
> > +enable) {
> > + return -ENOENT;
> > +}
> > +#endif
> > #endif /* _SC_SCI_H */
> > --
> > 2.7.4

2020-03-09 16:48:03

[permalink] [raw]

Subject: Re: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU case

On 09/03/2020 06:27:06-0700, Guenter Roeck wrote:
> On 3/9/20 4:06 AM, Alexandre Belloni wrote:
> > On 09/03/2020 08:38:14+0800, Anson Huang wrote:
> >> Add stubs for those i.MX SCU APIs to make those modules depending
> >> on IMX_SCU can pass build when COMPILE_TEST is enabled.
> >>
> >> Signed-off-by: Anson Huang <[email protected]>
> >> ---
> >> Changes since V2:
> >> - return error for stubs.
> >
> > I'm not sure why you are sending v3 with the stubs as we determined that
> > 2/7 is enough to compile all the drivers with COMPILE_TEST.
> >
> >
> 2/7 alone is not sufficient. With only 2/7, one can explicitly configure
> IMX_SCU=n, COMPILE_TEST=y, and get lots of compile failures. Granted,
> one should not do that, but 0day does (I don't know if that is the result
> of RANDCONFIG), and I am not looking forward having to deal with the
> fallout.
>

How would that be possible if the drivers all depend on IMX_SCU?

--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

2020-03-09 17:12:49

[permalink] [raw]

Subject: Re: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU case

On Mon, Mar 09, 2020 at 05:47:05PM +0100, Alexandre Belloni wrote:
> On 09/03/2020 06:27:06-0700, Guenter Roeck wrote:
> > On 3/9/20 4:06 AM, Alexandre Belloni wrote:
> > > On 09/03/2020 08:38:14+0800, Anson Huang wrote:
> > >> Add stubs for those i.MX SCU APIs to make those modules depending
> > >> on IMX_SCU can pass build when COMPILE_TEST is enabled.
> > >>
> > >> Signed-off-by: Anson Huang <[email protected]>
> > >> ---
> > >> Changes since V2:
> > >> - return error for stubs.
> > >
> > > I'm not sure why you are sending v3 with the stubs as we determined that
> > > 2/7 is enough to compile all the drivers with COMPILE_TEST.
> > >
> > >
> > 2/7 alone is not sufficient. With only 2/7, one can explicitly configure
> > IMX_SCU=n, COMPILE_TEST=y, and get lots of compile failures. Granted,
> > one should not do that, but 0day does (I don't know if that is the result
> > of RANDCONFIG), and I am not looking forward having to deal with the
> > fallout.
> >
>
> How would that be possible if the drivers all depend on IMX_SCU?
>
That dependency is being changed to IMX_SCU || COMPILE_TEST
as part of the series.

Guenter

2020-03-09 17:31:30

[permalink] [raw]

Subject: Re: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU case

On 09/03/2020 10:10:12-0700, Guenter Roeck wrote:
> On Mon, Mar 09, 2020 at 05:47:05PM +0100, Alexandre Belloni wrote:
> > On 09/03/2020 06:27:06-0700, Guenter Roeck wrote:
> > > On 3/9/20 4:06 AM, Alexandre Belloni wrote:
> > > > On 09/03/2020 08:38:14+0800, Anson Huang wrote:
> > > >> Add stubs for those i.MX SCU APIs to make those modules depending
> > > >> on IMX_SCU can pass build when COMPILE_TEST is enabled.
> > > >>
> > > >> Signed-off-by: Anson Huang <[email protected]>
> > > >> ---
> > > >> Changes since V2:
> > > >> - return error for stubs.
> > > >
> > > > I'm not sure why you are sending v3 with the stubs as we determined that
> > > > 2/7 is enough to compile all the drivers with COMPILE_TEST.
> > > >
> > > >
> > > 2/7 alone is not sufficient. With only 2/7, one can explicitly configure
> > > IMX_SCU=n, COMPILE_TEST=y, and get lots of compile failures. Granted,
> > > one should not do that, but 0day does (I don't know if that is the result
> > > of RANDCONFIG), and I am not looking forward having to deal with the
> > > fallout.
> > >
> >
> > How would that be possible if the drivers all depend on IMX_SCU?
> >
> That dependency is being changed to IMX_SCU || COMPILE_TEST
> as part of the series.
>

Yes, my point is that those patches should not be applied at all, only
2/7.

--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

2020-03-09 18:20:26

[permalink] [raw]

Subject: Re: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU case

On Mon, Mar 09, 2020 at 06:30:54PM +0100, Alexandre Belloni wrote:
> On 09/03/2020 10:10:12-0700, Guenter Roeck wrote:
> > On Mon, Mar 09, 2020 at 05:47:05PM +0100, Alexandre Belloni wrote:
> > > On 09/03/2020 06:27:06-0700, Guenter Roeck wrote:
> > > > On 3/9/20 4:06 AM, Alexandre Belloni wrote:
> > > > > On 09/03/2020 08:38:14+0800, Anson Huang wrote:
> > > > >> Add stubs for those i.MX SCU APIs to make those modules depending
> > > > >> on IMX_SCU can pass build when COMPILE_TEST is enabled.
> > > > >>
> > > > >> Signed-off-by: Anson Huang <[email protected]>
> > > > >> ---
> > > > >> Changes since V2:
> > > > >> - return error for stubs.
> > > > >
> > > > > I'm not sure why you are sending v3 with the stubs as we determined that
> > > > > 2/7 is enough to compile all the drivers with COMPILE_TEST.
> > > > >
> > > > >
> > > > 2/7 alone is not sufficient. With only 2/7, one can explicitly configure
> > > > IMX_SCU=n, COMPILE_TEST=y, and get lots of compile failures. Granted,
> > > > one should not do that, but 0day does (I don't know if that is the result
> > > > of RANDCONFIG), and I am not looking forward having to deal with the
> > > > fallout.
> > > >
> > >
> > > How would that be possible if the drivers all depend on IMX_SCU?
> > >
> > That dependency is being changed to IMX_SCU || COMPILE_TEST
> > as part of the series.
> >
>
> Yes, my point is that those patches should not be applied at all, only
> 2/7.

Ah, now I get it. Sorry, I missed that part. You are correct, that would
be sufficient, and I would very much prefer that approach.

Thanks,
Guenter

2020-03-16 00:53:04

by Shawn Guo

[permalink] [raw]

Subject: Re: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU case

On Mon, Mar 09, 2020 at 01:40:18PM +0000, Peng Fan wrote:
> > Subject: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU case
>
> I have one patch pending reviewing.
> https://patchwork.kernel.org/patch/11395247/

I dropped that patch from my queue and picked patch #2 from this series
as the favor.

Shawn

2020-03-16 03:19:56

[permalink] [raw]

Subject: RE: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU case

Hi Shawn,

> Subject: Re: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU
> case
>
> On Mon, Mar 09, 2020 at 01:40:18PM +0000, Peng Fan wrote:
> > > Subject: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU
> > > case
> >
> > I have one patch pending reviewing.
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc
> >
> hwork.kernel.org%2Fpatch%2F11395247%2F&data=02%7C01%7Cpeng.f
> an%40n
> >
> xp.com%7C995815002e2b490791e008d7c9445133%7C686ea1d3bc2b4c6fa9
> 2cd99c5c
> >
> 301635%7C0%7C0%7C637199167574579419&sdata=RM4Mtwl8LZ3ft9
> 3uL3FQPcHT
> > 9lPHSqBOgugozkcLvag%3D&reserved=0
>
> I dropped that patch from my queue and picked patch #2 from this series as
> the favor.

I think dropping that patch might cause Linux-next build fail as previously showed,
because IMX_SCU_SOC depends on COMPILE_TEST. If you drop that patch,
also need to drop COMPILE_TEST from IMX_SCU_SOC.

ld: drivers/soc/imx/soc-imx-scu.o: in function `.imx_scu_soc_probe':
soc-imx-scu.c:(.text.imx_scu_soc_probe+0x44): undefined reference to
`.imx_scu_get_handle'
ld: soc-imx-scu.c:(.text.imx_scu_soc_probe+0x134): undefined reference
to `.imx_scu_call_rpc'
ld: soc-imx-scu.c:(.text.imx_scu_soc_probe+0x20c): undefined reference
to `.imx_scu_call_rpc'

Caused by commit

68c189e3a93c ("soc: imx: increase build coverage for imx8m soc
driver")

What do you prefer? I personally think dummy functions would be good.

Thanks,
Peng.
>
> Shawn

2020-03-16 03:37:15

[permalink] [raw]

Subject: RE: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU case

> Subject: Re: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU
> case
>
> On Mon, Mar 16, 2020 at 02:51:47AM +0000, Peng Fan wrote:
> > Hi Shawn,
> >
> > > Subject: Re: [PATCH V3 1/7] firmware: imx: Add stubs for
> > > !CONFIG_IMX_SCU case
> > >
> > > On Mon, Mar 09, 2020 at 01:40:18PM +0000, Peng Fan wrote:
> > > > > Subject: [PATCH V3 1/7] firmware: imx: Add stubs for
> > > > > !CONFIG_IMX_SCU case
> > > >
> > > > I have one patch pending reviewing.
> > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2F
> > > > patc
> > > >
> > >
> hwork.kernel.org%2Fpatch%2F11395247%2F&data=02%7C01%7Cpeng.f
> > > an%40n
> > > >
> > >
> xp.com%7C995815002e2b490791e008d7c9445133%7C686ea1d3bc2b4c6fa9
> > > 2cd99c5c
> > > >
> > >
> 301635%7C0%7C0%7C637199167574579419&sdata=RM4Mtwl8LZ3ft9
> > > 3uL3FQPcHT
> > > > 9lPHSqBOgugozkcLvag%3D&reserved=0
> > >
> > > I dropped that patch from my queue and picked patch #2 from this
> > > series as the favor.
> >
> > I think dropping that patch might cause Linux-next build fail as
> > previously showed, because IMX_SCU_SOC depends on COMPILE_TEST. If
> you
> > drop that patch, also need to drop COMPILE_TEST from IMX_SCU_SOC.
> >
> > ld: drivers/soc/imx/soc-imx-scu.o: in function `.imx_scu_soc_probe':
> > soc-imx-scu.c:(.text.imx_scu_soc_probe+0x44): undefined reference to
> > `.imx_scu_get_handle'
> > ld: soc-imx-scu.c:(.text.imx_scu_soc_probe+0x134): undefined
> > reference to `.imx_scu_call_rpc'
> > ld: soc-imx-scu.c:(.text.imx_scu_soc_probe+0x20c): undefined
> > reference to `.imx_scu_call_rpc'
> >
> > Caused by commit
> >
> > 68c189e3a93c ("soc: imx: increase build coverage for imx8m soc
> > driver")
> >
> > What do you prefer? I personally think dummy functions would be good.
>
> I would rather drop COMPILE_TEST from IMX_SCU_SOC. Could you send a
> patch for that shortly?

Just sent out. One more thing, I think all drivers depends on IMX_SCU should not
have COMPILE_TEST if we plan not to add dummy functions. I see you picked up
Anson's patch in imx/drivers branch, please check more.

Thanks,
Peng.

>
> Shawn

2020-03-16 03:38:23

by Shawn Guo

[permalink] [raw]

Subject: Re: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU case

On Mon, Mar 16, 2020 at 03:18:43AM +0000, Peng Fan wrote:
> Just sent out. One more thing, I think all drivers depends on IMX_SCU should not
> have COMPILE_TEST if we plan not to add dummy functions. I see you picked up
> Anson's patch in imx/drivers branch, please check more.

Ha, yes. COMPILE_TEST should be dropped for IMX_SCU_PD in Anson's patch.
Thanks for reminding.

Shawn

2020-03-16 03:43:22

by Shawn Guo

[permalink] [raw]

Subject: Re: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU case

On Mon, Mar 16, 2020 at 02:51:47AM +0000, Peng Fan wrote:
> Hi Shawn,
>
> > Subject: Re: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU
> > case
> >
> > On Mon, Mar 09, 2020 at 01:40:18PM +0000, Peng Fan wrote:
> > > > Subject: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU
> > > > case
> > >
> > > I have one patch pending reviewing.
> > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc
> > >
> > hwork.kernel.org%2Fpatch%2F11395247%2F&data=02%7C01%7Cpeng.f
> > an%40n
> > >
> > xp.com%7C995815002e2b490791e008d7c9445133%7C686ea1d3bc2b4c6fa9
> > 2cd99c5c
> > >
> > 301635%7C0%7C0%7C637199167574579419&sdata=RM4Mtwl8LZ3ft9
> > 3uL3FQPcHT
> > > 9lPHSqBOgugozkcLvag%3D&reserved=0
> >
> > I dropped that patch from my queue and picked patch #2 from this series as
> > the favor.
>
> I think dropping that patch might cause Linux-next build fail as previously showed,
> because IMX_SCU_SOC depends on COMPILE_TEST. If you drop that patch,
> also need to drop COMPILE_TEST from IMX_SCU_SOC.
>
> ld: drivers/soc/imx/soc-imx-scu.o: in function `.imx_scu_soc_probe':
> soc-imx-scu.c:(.text.imx_scu_soc_probe+0x44): undefined reference to
> `.imx_scu_get_handle'
> ld: soc-imx-scu.c:(.text.imx_scu_soc_probe+0x134): undefined reference
> to `.imx_scu_call_rpc'
> ld: soc-imx-scu.c:(.text.imx_scu_soc_probe+0x20c): undefined reference
> to `.imx_scu_call_rpc'
>
> Caused by commit
>
> 68c189e3a93c ("soc: imx: increase build coverage for imx8m soc
> driver")
>
> What do you prefer? I personally think dummy functions would be good.

I would rather drop COMPILE_TEST from IMX_SCU_SOC. Could you send a
patch for that shortly?

Shawn

2020-03-16 08:05:01

[permalink] [raw]

Subject: RE: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU case

Hi, Shawn

> Subject: Re: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU
> case
>
> On Mon, Mar 16, 2020 at 03:18:43AM +0000, Peng Fan wrote:
> > Just sent out. One more thing, I think all drivers depends on IMX_SCU
> > should not have COMPILE_TEST if we plan not to add dummy functions. I
> > see you picked up Anson's patch in imx/drivers branch, please check more.
>
> Ha, yes. COMPILE_TEST should be dropped for IMX_SCU_PD in Anson's patch.
> Thanks for reminding.

I still NOT quite understand why we won't support COMPILE_TEST for SCU drivers,
with whose stubs, the build should be OK, if there is any build error, we should try
to fix it, NOT just remove the COMPILE_TEST support, any special reason?

Thanks,
Anson

2020-03-16 08:42:58

[permalink] [raw]

Subject: Re: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU case

On 16/03/2020 08:04:17+0000, Anson Huang wrote:
> Hi, Shawn
>
> > Subject: Re: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU
> > case
> >
> > On Mon, Mar 16, 2020 at 03:18:43AM +0000, Peng Fan wrote:
> > > Just sent out. One more thing, I think all drivers depends on IMX_SCU
> > > should not have COMPILE_TEST if we plan not to add dummy functions. I
> > > see you picked up Anson's patch in imx/drivers branch, please check more.
> >
> > Ha, yes. COMPILE_TEST should be dropped for IMX_SCU_PD in Anson's patch.
> > Thanks for reminding.
>
> I still NOT quite understand why we won't support COMPILE_TEST for SCU drivers,
> with whose stubs, the build should be OK, if there is any build error, we should try
> to fix it, NOT just remove the COMPILE_TEST support, any special reason?
>

COMPILE_TEST is supported as long as IMX_SCU is selected like is it for
any driver depending on any bus.

--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

2020-03-16 08:44:57

[permalink] [raw]

Subject: RE: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU case

> Subject: Re: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU
> case
>
> On 16/03/2020 08:04:17+0000, Anson Huang wrote:
> > Hi, Shawn
> >
> > > Subject: Re: [PATCH V3 1/7] firmware: imx: Add stubs for
> > > !CONFIG_IMX_SCU case
> > >
> > > On Mon, Mar 16, 2020 at 03:18:43AM +0000, Peng Fan wrote:
> > > > Just sent out. One more thing, I think all drivers depends on
> > > > IMX_SCU should not have COMPILE_TEST if we plan not to add dummy
> > > > functions. I see you picked up Anson's patch in imx/drivers branch,
> please check more.
> > >
> > > Ha, yes. COMPILE_TEST should be dropped for IMX_SCU_PD in Anson's
> patch.
> > > Thanks for reminding.
> >
> > I still NOT quite understand why we won't support COMPILE_TEST for SCU
> > drivers, with whose stubs, the build should be OK, if there is any
> > build error, we should try to fix it, NOT just remove the COMPILE_TEST
> support, any special reason?
> >
>
> COMPILE_TEST is supported as long as IMX_SCU is selected like is it for any
> driver depending on any bus.

But without having " || COMPILE_TEST " in kconfig, COMPILE_TEST will NOT be supported,
I think as long as we have stubs for those SCU APIs, all drivers depending on IMX_SCU can
support COMPILE_TEST independently.

Anson

2020-03-16 09:01:46

[permalink] [raw]

Subject: Re: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU case

On 16/03/2020 08:44:10+0000, Anson Huang wrote:
>
>
> > Subject: Re: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU
> > case
> >
> > On 16/03/2020 08:04:17+0000, Anson Huang wrote:
> > > Hi, Shawn
> > >
> > > > Subject: Re: [PATCH V3 1/7] firmware: imx: Add stubs for
> > > > !CONFIG_IMX_SCU case
> > > >
> > > > On Mon, Mar 16, 2020 at 03:18:43AM +0000, Peng Fan wrote:
> > > > > Just sent out. One more thing, I think all drivers depends on
> > > > > IMX_SCU should not have COMPILE_TEST if we plan not to add dummy
> > > > > functions. I see you picked up Anson's patch in imx/drivers branch,
> > please check more.
> > > >
> > > > Ha, yes. COMPILE_TEST should be dropped for IMX_SCU_PD in Anson's
> > patch.
> > > > Thanks for reminding.
> > >
> > > I still NOT quite understand why we won't support COMPILE_TEST for SCU
> > > drivers, with whose stubs, the build should be OK, if there is any
> > > build error, we should try to fix it, NOT just remove the COMPILE_TEST
> > support, any special reason?
> > >
> >
> > COMPILE_TEST is supported as long as IMX_SCU is selected like is it for any
> > driver depending on any bus.
>
> But without having " || COMPILE_TEST " in kconfig, COMPILE_TEST will NOT be supported,
> I think as long as we have stubs for those SCU APIs, all drivers depending on IMX_SCU can
> support COMPILE_TEST independently.
>

Why do you absolutely need to compile them independently? From a code
coverage point of view, having:

COMPILE_TEST=y
CONFIG_IMX_SCU=y

is enough to select and compile the remaining drivers.

--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

2020-03-16 09:10:51

[permalink] [raw]

Subject: RE: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU case

Hi, Alexandre

> Subject: Re: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU
> case
>
> On 16/03/2020 08:44:10+0000, Anson Huang wrote:
> >
> >
> > > Subject: Re: [PATCH V3 1/7] firmware: imx: Add stubs for
> > > !CONFIG_IMX_SCU case
> > >
> > > On 16/03/2020 08:04:17+0000, Anson Huang wrote:
> > > > Hi, Shawn
> > > >
> > > > > Subject: Re: [PATCH V3 1/7] firmware: imx: Add stubs for
> > > > > !CONFIG_IMX_SCU case
> > > > >
> > > > > On Mon, Mar 16, 2020 at 03:18:43AM +0000, Peng Fan wrote:
> > > > > > Just sent out. One more thing, I think all drivers depends on
> > > > > > IMX_SCU should not have COMPILE_TEST if we plan not to add
> > > > > > dummy functions. I see you picked up Anson's patch in
> > > > > > imx/drivers branch,
> > > please check more.
> > > > >
> > > > > Ha, yes. COMPILE_TEST should be dropped for IMX_SCU_PD in
> > > > > Anson's
> > > patch.
> > > > > Thanks for reminding.
> > > >
> > > > I still NOT quite understand why we won't support COMPILE_TEST for
> > > > SCU drivers, with whose stubs, the build should be OK, if there is
> > > > any build error, we should try to fix it, NOT just remove the
> > > > COMPILE_TEST
> > > support, any special reason?
> > > >
> > >
> > > COMPILE_TEST is supported as long as IMX_SCU is selected like is it
> > > for any driver depending on any bus.
> >
> > But without having " || COMPILE_TEST " in kconfig, COMPILE_TEST will
> > NOT be supported, I think as long as we have stubs for those SCU APIs,
> > all drivers depending on IMX_SCU can support COMPILE_TEST
> independently.
> >
>
>
> Why do you absolutely need to compile them independently? From a code
> coverage point of view, having:
>
> COMPILE_TEST=y
> CONFIG_IMX_SCU=y
>
> is enough to select and compile the remaining drivers.

What I meant is for below case, like using other arch config which does NOT have
CONFIG_IMX_SCU selected, ONLY with COMPILE_TEST selected, adding stubs for
IMX_SCU APIs can fix such scenario.

COMPILE_TEST=y
CONFIG_IMX_SCU=n

Anson

2020-03-16 09:23:07

[permalink] [raw]

Subject: Re: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU case

On 16/03/2020 09:08:53+0000, Anson Huang wrote:
> > Why do you absolutely need to compile them independently? From a code
> > coverage point of view, having:
> >
> > COMPILE_TEST=y
> > CONFIG_IMX_SCU=y
> >
> > is enough to select and compile the remaining drivers.
>
> What I meant is for below case, like using other arch config which does NOT have
> CONFIG_IMX_SCU selected, ONLY with COMPILE_TEST selected, adding stubs for
> IMX_SCU APIs can fix such scenario.
>
> COMPILE_TEST=y
> CONFIG_IMX_SCU=n
>

Why is that an issue? If they don't have IMX_SCU selected, then the
other SCU driver are not selected either, having stubs doesn't change
that you will have to select at least one option. Please explain what is
the issue that is not solved here.

--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

2020-03-16 09:41:49

[permalink] [raw]

Subject: RE: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU case

Hi, Alexandre

> -----Original Message-----
> From: Alexandre Belloni <[email protected]>
> Sent: Monday, March 16, 2020 5:16 PM
> To: Anson Huang <[email protected]>
> Cc: Shawn Guo <[email protected]>; Peng Fan <[email protected]>;
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; wim@linux-
> watchdog.org; [email protected]; Daniel Baluta <[email protected]>;
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]; [email protected];
> Leonard Crestez <[email protected]>; Aisheng Dong
> <[email protected]>; [email protected]; linux-
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]; linux-
> [email protected]; dl-linux-imx <[email protected]>
> Subject: Re: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU
> case
>
> On 16/03/2020 09:08:53+0000, Anson Huang wrote:
> > > Why do you absolutely need to compile them independently? From a
> > > code coverage point of view, having:
> > >
> > > COMPILE_TEST=y
> > > CONFIG_IMX_SCU=y
> > >
> > > is enough to select and compile the remaining drivers.
> >
> > What I meant is for below case, like using other arch config which
> > does NOT have CONFIG_IMX_SCU selected, ONLY with COMPILE_TEST
> > selected, adding stubs for IMX_SCU APIs can fix such scenario.
> >
> > COMPILE_TEST=y
> > CONFIG_IMX_SCU=n
> >
>
> Why is that an issue? If they don't have IMX_SCU selected, then the other
> SCU driver are not selected either, having stubs doesn't change that you will
> have to select at least one option. Please explain what is the issue that is not
> solved here.

OK, what I thought is even without IMX_SCU selected, other SCU drivers still can be
selected for build test after adding "COMPILE_TEST" to the kconfig, like below, if
without IMX_SCU API stubs, the "COMPILE_TEST" can NOT be added to SCU drivers
to enable build test, so I think the IMX_SCU API stubs should be added?

config KEYBOARD_IMX_SC_KEY
tristate "IMX SCU Key Driver"
depends on IMX_SCU || COMPILE_TEST

thanks,
Anson

2020-03-16 10:06:34

[permalink] [raw]

Subject: Re: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU case

On 16/03/2020 09:40:52+0000, Anson Huang wrote:
> > Why is that an issue? If they don't have IMX_SCU selected, then the other
> > SCU driver are not selected either, having stubs doesn't change that you will
> > have to select at least one option. Please explain what is the issue that is not
> > solved here.
>
> OK, what I thought is even without IMX_SCU selected, other SCU drivers still can be
> selected for build test after adding "COMPILE_TEST" to the kconfig, like below, if
> without IMX_SCU API stubs, the "COMPILE_TEST" can NOT be added to SCU drivers
> to enable build test, so I think the IMX_SCU API stubs should be added?
>

No they shouldn't because there is not point adding COMPILE_TEST to the
SCU drivers. We don't add COMPILE_TEST to all the drivers and add stubs
to all the subsystems. E.g there is no point trying to compile an I2C
driver if the I2C core is not enabled.

--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

2020-03-16 10:19:11

[permalink] [raw]

Subject: RE: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU case

> Subject: Re: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU
> case
>
> On 16/03/2020 09:40:52+0000, Anson Huang wrote:
> > > Why is that an issue? If they don't have IMX_SCU selected, then the
> > > other SCU driver are not selected either, having stubs doesn't
> > > change that you will have to select at least one option. Please
> > > explain what is the issue that is not solved here.
> >
> > OK, what I thought is even without IMX_SCU selected, other SCU drivers
> > still can be selected for build test after adding "COMPILE_TEST" to
> > the kconfig, like below, if without IMX_SCU API stubs, the
> > "COMPILE_TEST" can NOT be added to SCU drivers to enable build test, so I
> think the IMX_SCU API stubs should be added?
> >
>
> No they shouldn't because there is not point adding COMPILE_TEST to the
> SCU drivers. We don't add COMPILE_TEST to all the drivers and add stubs to
> all the subsystems. E.g there is no point trying to compile an I2C driver if the
> I2C core is not enabled.

OK, make sense.

Thanks,
Anson

2020-03-17 03:26:12

[permalink] [raw]

Subject: RE: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU case

> Subject: RE: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU
> case
>
> Hi, Alexandre
>
> > -----Original Message-----
> > From: Alexandre Belloni <[email protected]>
> > Sent: Monday, March 16, 2020 5:16 PM
> > To: Anson Huang <[email protected]>
> > Cc: Shawn Guo <[email protected]>; Peng Fan <[email protected]>;
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected];
> [email protected];
> > [email protected]; [email protected]; wim@linux-
> > watchdog.org; [email protected]; Daniel Baluta
> > <[email protected]>; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected];
> > [email protected]; [email protected]; [email protected]; [email protected];
> > Leonard Crestez <[email protected]>; Aisheng Dong
> > <[email protected]>; [email protected]; linux-
> > [email protected]; [email protected]; linux-
> > [email protected]; [email protected]; linux-
> > [email protected]; dl-linux-imx <[email protected]>
> > Subject: Re: [PATCH V3 1/7] firmware: imx: Add stubs for
> > !CONFIG_IMX_SCU case
> >
> > On 16/03/2020 09:08:53+0000, Anson Huang wrote:
> > > > Why do you absolutely need to compile them independently? From a
> > > > code coverage point of view, having:
> > > >
> > > > COMPILE_TEST=y
> > > > CONFIG_IMX_SCU=y
> > > >
> > > > is enough to select and compile the remaining drivers.
> > >
> > > What I meant is for below case, like using other arch config which
> > > does NOT have CONFIG_IMX_SCU selected, ONLY with COMPILE_TEST
> > > selected, adding stubs for IMX_SCU APIs can fix such scenario.
> > >
> > > COMPILE_TEST=y
> > > CONFIG_IMX_SCU=n
> > >
> >
> > Why is that an issue? If they don't have IMX_SCU selected, then the
> > other SCU driver are not selected either, having stubs doesn't change
> > that you will have to select at least one option. Please explain what
> > is the issue that is not solved here.
>
> OK, what I thought is even without IMX_SCU selected, other SCU drivers still
> can be selected for build test after adding "COMPILE_TEST" to the kconfig,
> like below, if without IMX_SCU API stubs, the "COMPILE_TEST" can NOT be
> added to SCU drivers to enable build test, so I think the IMX_SCU API stubs
> should be added?

Forgot to mention, without stub api, for drivers with
" #include <linux/firmware/imx/sci.h> " will met compile error without
+#ifdef CONFIG_IMX_SCU
+#endif

So we have to use ifdef CONFIG_IMX_SCU to guard the include.

Regards,
Peng.

>
> config KEYBOARD_IMX_SC_KEY
> tristate "IMX SCU Key Driver"
> depends on IMX_SCU || COMPILE_TEST
>
> thanks,
> Anson

2020-03-17 03:27:58

[permalink] [raw]

Subject: RE: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU case

> Subject: RE: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU
> case
>
> > Subject: RE: [PATCH V3 1/7] firmware: imx: Add stubs for
> > !CONFIG_IMX_SCU case
> >
> > Hi, Alexandre
> >
> > > -----Original Message-----
> > > From: Alexandre Belloni <[email protected]>
> > > Sent: Monday, March 16, 2020 5:16 PM
> > > To: Anson Huang <[email protected]>
> > > Cc: Shawn Guo <[email protected]>; Peng Fan <[email protected]>;
> > > [email protected]; [email protected]; [email protected];
> > > [email protected]; [email protected];
> > [email protected];
> > > [email protected]; [email protected]; wim@linux-
> > > watchdog.org; [email protected]; Daniel Baluta
> > > <[email protected]>; [email protected];
> > > [email protected]; [email protected]; [email protected];
> > > [email protected];
> > > [email protected]; [email protected]; [email protected];
> > > [email protected]; Leonard Crestez <[email protected]>; Aisheng
> > > Dong <[email protected]>; [email protected];
> > > linux- [email protected]; [email protected]; linux-
> > > [email protected]; [email protected]; linux-
> > > [email protected]; dl-linux-imx <[email protected]>
> > > Subject: Re: [PATCH V3 1/7] firmware: imx: Add stubs for
> > > !CONFIG_IMX_SCU case
> > >
> > > On 16/03/2020 09:08:53+0000, Anson Huang wrote:
> > > > > Why do you absolutely need to compile them independently? From a
> > > > > code coverage point of view, having:
> > > > >
> > > > > COMPILE_TEST=y
> > > > > CONFIG_IMX_SCU=y
> > > > >
> > > > > is enough to select and compile the remaining drivers.
> > > >
> > > > What I meant is for below case, like using other arch config which
> > > > does NOT have CONFIG_IMX_SCU selected, ONLY with COMPILE_TEST
> > > > selected, adding stubs for IMX_SCU APIs can fix such scenario.
> > > >
> > > > COMPILE_TEST=y
> > > > CONFIG_IMX_SCU=n
> > > >
> > >
> > > Why is that an issue? If they don't have IMX_SCU selected, then the
> > > other SCU driver are not selected either, having stubs doesn't
> > > change that you will have to select at least one option. Please
> > > explain what is the issue that is not solved here.
> >
> > OK, what I thought is even without IMX_SCU selected, other SCU drivers
> > still can be selected for build test after adding "COMPILE_TEST" to
> > the kconfig, like below, if without IMX_SCU API stubs, the
> > "COMPILE_TEST" can NOT be added to SCU drivers to enable build test,
> > so I think the IMX_SCU API stubs should be added?
>
> Forgot to mention, without stub api, for drivers with " #include
> <linux/firmware/imx/sci.h> " will met compile error without
> +#ifdef CONFIG_IMX_SCU
> +#endif
>
> So we have to use ifdef CONFIG_IMX_SCU to guard the include.

The idea here is that all modules depending on IMX_SCU should NOT be enabled
without IMX_SCU enabled. So it should be fine.

Anson

2020-03-17 03:29:29

[permalink] [raw]

Subject: Re: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU case

On 3/16/20 7:18 PM, Peng Fan wrote:
>> Subject: RE: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU
>> case
>>
>> Hi, Alexandre
>>
>>> -----Original Message-----
>>> From: Alexandre Belloni <[email protected]>
>>> Sent: Monday, March 16, 2020 5:16 PM
>>> To: Anson Huang <[email protected]>
>>> Cc: Shawn Guo <[email protected]>; Peng Fan <[email protected]>;
>>> [email protected]; [email protected]; [email protected];
>>> [email protected]; [email protected];
>> [email protected];
>>> [email protected]; [email protected]; wim@linux-
>>> watchdog.org; [email protected]; Daniel Baluta
>>> <[email protected]>; [email protected];
>>> [email protected]; [email protected]; [email protected];
>>> [email protected];
>>> [email protected]; [email protected]; [email protected]; [email protected];
>>> Leonard Crestez <[email protected]>; Aisheng Dong
>>> <[email protected]>; [email protected]; linux-
>>> [email protected]; [email protected]; linux-
>>> [email protected]; [email protected]; linux-
>>> [email protected]; dl-linux-imx <[email protected]>
>>> Subject: Re: [PATCH V3 1/7] firmware: imx: Add stubs for
>>> !CONFIG_IMX_SCU case
>>>
>>> On 16/03/2020 09:08:53+0000, Anson Huang wrote:
>>>>> Why do you absolutely need to compile them independently? From a
>>>>> code coverage point of view, having:
>>>>>
>>>>> COMPILE_TEST=y
>>>>> CONFIG_IMX_SCU=y
>>>>>
>>>>> is enough to select and compile the remaining drivers.
>>>>
>>>> What I meant is for below case, like using other arch config which
>>>> does NOT have CONFIG_IMX_SCU selected, ONLY with COMPILE_TEST
>>>> selected, adding stubs for IMX_SCU APIs can fix such scenario.
>>>>
>>>> COMPILE_TEST=y
>>>> CONFIG_IMX_SCU=n
>>>>
>>>
>>> Why is that an issue? If they don't have IMX_SCU selected, then the
>>> other SCU driver are not selected either, having stubs doesn't change
>>> that you will have to select at least one option. Please explain what
>>> is the issue that is not solved here.
>>
>> OK, what I thought is even without IMX_SCU selected, other SCU drivers still
>> can be selected for build test after adding "COMPILE_TEST" to the kconfig,
>> like below, if without IMX_SCU API stubs, the "COMPILE_TEST" can NOT be
>> added to SCU drivers to enable build test, so I think the IMX_SCU API stubs
>> should be added?
>
> Forgot to mention, without stub api, for drivers with
> " #include <linux/firmware/imx/sci.h> " will met compile error without
> +#ifdef CONFIG_IMX_SCU
> +#endif
>
> So we have to use ifdef CONFIG_IMX_SCU to guard the include.
>
Add "depends on IMX_SCU" to the Kconfig entry for those drivers,
and/or drop "COMPILE_TEST" from their Kconfig entry.

Really, COMPILE_TEST is abused here. I start to understand those who
advocate that it should be removed. This is an excellent case in point.

Guenter

2020-03-17 03:29:30