diff mbox series

[V3,1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU case

Message ID 1583714300-19085-1-git-send-email-Anson.Huang@nxp.com (mailing list archive)
State Not Applicable, archived
Headers show
Series [V3,1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU case | expand

Commit Message

Anson Huang March 9, 2020, 12:38 a.m. UTC
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 <Anson.Huang@nxp.com>
---
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(+)

Comments

Alexandre Belloni March 9, 2020, 11:06 a.m. UTC | #1
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 <Anson.Huang@nxp.com>
> ---
> 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.
Anson Huang March 9, 2020, 12:20 p.m. UTC | #2
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 <Anson.Huang@nxp.com>
> > ---
> > 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
Guenter Roeck March 9, 2020, 1:27 p.m. UTC | #3
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 <Anson.Huang@nxp.com>
>> ---
>> 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
Anson Huang March 9, 2020, 1:37 p.m. UTC | #4
> 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 <Anson.Huang@nxp.com>
> >> ---
> >> 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
Peng Fan March 9, 2020, 1:40 p.m. UTC | #5
> 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 <Anson.Huang@nxp.com>
> ---
> 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
Anson Huang March 9, 2020, 2:09 p.m. UTC | #6
> 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 <Anson.Huang@nxp.com>
> > ---
> > 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
Alexandre Belloni March 9, 2020, 4:47 p.m. UTC | #7
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 <Anson.Huang@nxp.com>
> >> ---
> >> 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?
Guenter Roeck March 9, 2020, 5:10 p.m. UTC | #8
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 <Anson.Huang@nxp.com>
> > >> ---
> > >> 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
Alexandre Belloni March 9, 2020, 5:30 p.m. UTC | #9
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 <Anson.Huang@nxp.com>
> > > >> ---
> > > >> 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.
Guenter Roeck March 9, 2020, 6:19 p.m. UTC | #10
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 <Anson.Huang@nxp.com>
> > > > >> ---
> > > > >> 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
Shawn Guo March 16, 2020, 12:52 a.m. UTC | #11
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
Peng Fan March 16, 2020, 2:51 a.m. UTC | #12
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&amp;data=02%7C01%7Cpeng.f
> an%40n
> >
> xp.com%7C995815002e2b490791e008d7c9445133%7C686ea1d3bc2b4c6fa9
> 2cd99c5c
> >
> 301635%7C0%7C0%7C637199167574579419&amp;sdata=RM4Mtwl8LZ3ft9
> 3uL3FQPcHT
> > 9lPHSqBOgugozkcLvag%3D&amp;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
Shawn Guo March 16, 2020, 3:07 a.m. UTC | #13
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&amp;data=02%7C01%7Cpeng.f
> > an%40n
> > >
> > xp.com%7C995815002e2b490791e008d7c9445133%7C686ea1d3bc2b4c6fa9
> > 2cd99c5c
> > >
> > 301635%7C0%7C0%7C637199167574579419&amp;sdata=RM4Mtwl8LZ3ft9
> > 3uL3FQPcHT
> > > 9lPHSqBOgugozkcLvag%3D&amp;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
Peng Fan March 16, 2020, 3:18 a.m. UTC | #14
> 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&amp;data=02%7C01%7Cpeng.f
> > > an%40n
> > > >
> > >
> xp.com%7C995815002e2b490791e008d7c9445133%7C686ea1d3bc2b4c6fa9
> > > 2cd99c5c
> > > >
> > >
> 301635%7C0%7C0%7C637199167574579419&amp;sdata=RM4Mtwl8LZ3ft9
> > > 3uL3FQPcHT
> > > > 9lPHSqBOgugozkcLvag%3D&amp;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
Shawn Guo March 16, 2020, 3:34 a.m. UTC | #15
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
Anson Huang March 16, 2020, 8:04 a.m. UTC | #16
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
Alexandre Belloni March 16, 2020, 8:40 a.m. UTC | #17
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.
Anson Huang March 16, 2020, 8:44 a.m. UTC | #18
> 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
Alexandre Belloni March 16, 2020, 9 a.m. UTC | #19
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.
Anson Huang March 16, 2020, 9:08 a.m. UTC | #20
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
Alexandre Belloni March 16, 2020, 9:15 a.m. UTC | #21
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.
Anson Huang March 16, 2020, 9:40 a.m. UTC | #22
Hi, Alexandre

> -----Original Message-----
> From: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Sent: Monday, March 16, 2020 5:16 PM
> To: Anson Huang <anson.huang@nxp.com>
> Cc: Shawn Guo <shawnguo@kernel.org>; Peng Fan <peng.fan@nxp.com>;
> s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com;
> dmitry.torokhov@gmail.com; a.zummo@towertech.it; rui.zhang@intel.com;
> daniel.lezcano@linaro.org; amit.kucheria@verdurent.com; wim@linux-
> watchdog.org; linux@roeck-us.net; Daniel Baluta <daniel.baluta@nxp.com>;
> gregkh@linuxfoundation.org; linux@rempel-privat.de; tglx@linutronix.de;
> m.felsch@pengutronix.de; andriy.shevchenko@linux.intel.com;
> arnd@arndb.de; ronald@innovation.ch; krzk@kernel.org; robh@kernel.org;
> Leonard Crestez <leonard.crestez@nxp.com>; Aisheng Dong
> <aisheng.dong@nxp.com>; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; linux-input@vger.kernel.org; linux-
> rtc@vger.kernel.org; linux-pm@vger.kernel.org; linux-
> watchdog@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>
> 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
Alexandre Belloni March 16, 2020, 10 a.m. UTC | #23
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.
Anson Huang March 16, 2020, 10:18 a.m. UTC | #24
> 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
Peng Fan March 17, 2020, 2:18 a.m. UTC | #25
> Subject: RE: [PATCH V3 1/7] firmware: imx: Add stubs for !CONFIG_IMX_SCU
> case
> 
> Hi, Alexandre
> 
> > -----Original Message-----
> > From: Alexandre Belloni <alexandre.belloni@bootlin.com>
> > Sent: Monday, March 16, 2020 5:16 PM
> > To: Anson Huang <anson.huang@nxp.com>
> > Cc: Shawn Guo <shawnguo@kernel.org>; Peng Fan <peng.fan@nxp.com>;
> > s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com;
> > dmitry.torokhov@gmail.com; a.zummo@towertech.it;
> rui.zhang@intel.com;
> > daniel.lezcano@linaro.org; amit.kucheria@verdurent.com; wim@linux-
> > watchdog.org; linux@roeck-us.net; Daniel Baluta
> > <daniel.baluta@nxp.com>; gregkh@linuxfoundation.org;
> > linux@rempel-privat.de; tglx@linutronix.de; m.felsch@pengutronix.de;
> > andriy.shevchenko@linux.intel.com;
> > arnd@arndb.de; ronald@innovation.ch; krzk@kernel.org; robh@kernel.org;
> > Leonard Crestez <leonard.crestez@nxp.com>; Aisheng Dong
> > <aisheng.dong@nxp.com>; linux-arm-kernel@lists.infradead.org; linux-
> > kernel@vger.kernel.org; linux-input@vger.kernel.org; linux-
> > rtc@vger.kernel.org; linux-pm@vger.kernel.org; linux-
> > watchdog@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>
> > 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
Anson Huang March 17, 2020, 2:29 a.m. UTC | #26
> 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 <alexandre.belloni@bootlin.com>
> > > Sent: Monday, March 16, 2020 5:16 PM
> > > To: Anson Huang <anson.huang@nxp.com>
> > > Cc: Shawn Guo <shawnguo@kernel.org>; Peng Fan <peng.fan@nxp.com>;
> > > s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com;
> > > dmitry.torokhov@gmail.com; a.zummo@towertech.it;
> > rui.zhang@intel.com;
> > > daniel.lezcano@linaro.org; amit.kucheria@verdurent.com; wim@linux-
> > > watchdog.org; linux@roeck-us.net; Daniel Baluta
> > > <daniel.baluta@nxp.com>; gregkh@linuxfoundation.org;
> > > linux@rempel-privat.de; tglx@linutronix.de; m.felsch@pengutronix.de;
> > > andriy.shevchenko@linux.intel.com;
> > > arnd@arndb.de; ronald@innovation.ch; krzk@kernel.org;
> > > robh@kernel.org; Leonard Crestez <leonard.crestez@nxp.com>; Aisheng
> > > Dong <aisheng.dong@nxp.com>; linux-arm-kernel@lists.infradead.org;
> > > linux- kernel@vger.kernel.org; linux-input@vger.kernel.org; linux-
> > > rtc@vger.kernel.org; linux-pm@vger.kernel.org; linux-
> > > watchdog@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>
> > > 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
Guenter Roeck March 17, 2020, 2:32 a.m. UTC | #27
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 <alexandre.belloni@bootlin.com>
>>> Sent: Monday, March 16, 2020 5:16 PM
>>> To: Anson Huang <anson.huang@nxp.com>
>>> Cc: Shawn Guo <shawnguo@kernel.org>; Peng Fan <peng.fan@nxp.com>;
>>> s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com;
>>> dmitry.torokhov@gmail.com; a.zummo@towertech.it;
>> rui.zhang@intel.com;
>>> daniel.lezcano@linaro.org; amit.kucheria@verdurent.com; wim@linux-
>>> watchdog.org; linux@roeck-us.net; Daniel Baluta
>>> <daniel.baluta@nxp.com>; gregkh@linuxfoundation.org;
>>> linux@rempel-privat.de; tglx@linutronix.de; m.felsch@pengutronix.de;
>>> andriy.shevchenko@linux.intel.com;
>>> arnd@arndb.de; ronald@innovation.ch; krzk@kernel.org; robh@kernel.org;
>>> Leonard Crestez <leonard.crestez@nxp.com>; Aisheng Dong
>>> <aisheng.dong@nxp.com>; linux-arm-kernel@lists.infradead.org; linux-
>>> kernel@vger.kernel.org; linux-input@vger.kernel.org; linux-
>>> rtc@vger.kernel.org; linux-pm@vger.kernel.org; linux-
>>> watchdog@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>
>>> 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
Anson Huang March 17, 2020, 2:35 a.m. UTC | #28
> 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 <alexandre.belloni@bootlin.com>
> >>> Sent: Monday, March 16, 2020 5:16 PM
> >>> To: Anson Huang <anson.huang@nxp.com>
> >>> Cc: Shawn Guo <shawnguo@kernel.org>; Peng Fan
> <peng.fan@nxp.com>;
> >>> s.hauer@pengutronix.de; kernel@pengutronix.de; festevam@gmail.com;
> >>> dmitry.torokhov@gmail.com; a.zummo@towertech.it;
> >> rui.zhang@intel.com;
> >>> daniel.lezcano@linaro.org; amit.kucheria@verdurent.com; wim@linux-
> >>> watchdog.org; linux@roeck-us.net; Daniel Baluta
> >>> <daniel.baluta@nxp.com>; gregkh@linuxfoundation.org;
> >>> linux@rempel-privat.de; tglx@linutronix.de; m.felsch@pengutronix.de;
> >>> andriy.shevchenko@linux.intel.com;
> >>> arnd@arndb.de; ronald@innovation.ch; krzk@kernel.org;
> >>> robh@kernel.org; Leonard Crestez <leonard.crestez@nxp.com>; Aisheng
> >>> Dong <aisheng.dong@nxp.com>; linux-arm-kernel@lists.infradead.org;
> >>> linux- kernel@vger.kernel.org; linux-input@vger.kernel.org; linux-
> >>> rtc@vger.kernel.org; linux-pm@vger.kernel.org; linux-
> >>> watchdog@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>
> >>> 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.

Yup, COMPILE_TEST should ONLY be added to those independent drivers,
those drivers with dependency on "core" driver should NOT have it added.
SCU drivers are similar.

Anson
diff mbox series

Patch

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 */