[PATCH] firmware: arm_scmi: fix psci dependency

[PATCH] firmware: arm_scmi: fix psci dependency

[Arnd Bergmann]

When CONFIG_ARM_PSCI_FW is disabled but CONFIG_HAVE_ARM_SMCCC is enabled,
arm-scmi runs into a link failure:

arm-linux-gnueabi-ld: drivers/firmware/arm_scmi/smc.o: in function `smc_send_message':
smc.c:(.text+0x200): undefined reference to `arm_smccc_1_1_get_conduit'

Use an inline helper to default to version v1.0 in the absence of psci.

Fixes: 1dc6558062da ("firmware: arm_scmi: Add smc/hvc transport")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 include/linux/arm-smccc.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
index 59494df0f55b..4f7c962856c0 100644
--- a/include/linux/arm-smccc.h
+++ b/include/linux/arm-smccc.h
@@ -95,7 +95,14 @@ enum arm_smccc_conduit {
  *
  * When SMCCCv1.1 is not present, returns SMCCC_CONDUIT_NONE.
  */
+#ifdef CONFIG_ARM_PSCI_FW
 enum arm_smccc_conduit arm_smccc_1_1_get_conduit(void);
+#else
+static inline enum arm_smccc_conduit arm_smccc_1_1_get_conduit(void)
+{
+	return SMCCC_CONDUIT_NONE;
+}
+#endif
 
 /**
  * struct arm_smccc_res - Result from SMC/HVC call
-- 
2.26.0

Re: [PATCH] firmware: arm_scmi: fix psci dependency

[Sudeep Holla]

Hi Arnd,

On Tue, May 05, 2020 at 04:08:08PM +0200, Arnd Bergmann wrote:
> When CONFIG_ARM_PSCI_FW is disabled but CONFIG_HAVE_ARM_SMCCC is enabled,
> arm-scmi runs into a link failure:
>
> arm-linux-gnueabi-ld: drivers/firmware/arm_scmi/smc.o: in function `smc_send_message':
> smc.c:(.text+0x200): undefined reference to `arm_smccc_1_1_get_conduit'
>
> Use an inline helper to default to version v1.0 in the absence of psci.
>

Thanks for fixing this. I was thinking if we can separate PSCI and SMCCC
quickly as a fix for this but I think he needs to be discussed in detail.

I am fine with this fix as is and happy to apply to my tree if no one
objects.

Sorry but taking this patch as opportunity to discuss how to carry the
dependency in future. Just a proposal,

1. Introduce a DT node for SMCCC v1.2+
2. The new SMCCC driver(strictly speaking library/few APIs) can probe 
   independent of PSCI if DT node is present
3. Else we fallback on PSCI and detect the SMCCC version for v1.1 and
   v1.2
4. Assume v1.0 if
	a. PSCI FEATURE returns NOT_SUPPORTED for ARM_SMCCC_VERSION_FUNC_ID
	b. CONFIG_ARM_PSCI{,_FW} is not defined

Mark/Will/Marc,

Any other use-case config missed above ?

--
Regards,
Sudeep

Re: [PATCH] firmware: arm_scmi: fix psci dependency

[Mark Rutland]

On Tue, May 05, 2020 at 04:04:21PM +0100, Sudeep Holla wrote:
> Hi Arnd,
> 
> On Tue, May 05, 2020 at 04:08:08PM +0200, Arnd Bergmann wrote:
> > When CONFIG_ARM_PSCI_FW is disabled but CONFIG_HAVE_ARM_SMCCC is enabled,
> > arm-scmi runs into a link failure:
> >
> > arm-linux-gnueabi-ld: drivers/firmware/arm_scmi/smc.o: in function `smc_send_message':
> > smc.c:(.text+0x200): undefined reference to `arm_smccc_1_1_get_conduit'
> >
> > Use an inline helper to default to version v1.0 in the absence of psci.
> >
> 
> Thanks for fixing this. I was thinking if we can separate PSCI and SMCCC
> quickly as a fix for this but I think he needs to be discussed in detail.
> 
> I am fine with this fix as is and happy to apply to my tree if no one
> objects.
> 
> Sorry but taking this patch as opportunity to discuss how to carry the
> dependency in future. Just a proposal,
> 
> 1. Introduce a DT node for SMCCC v1.2+
> 2. The new SMCCC driver(strictly speaking library/few APIs) can probe 
>    independent of PSCI if DT node is present
> 3. Else we fallback on PSCI and detect the SMCCC version for v1.1 and
>    v1.2
> 4. Assume v1.0 if
> 	a. PSCI FEATURE returns NOT_SUPPORTED for ARM_SMCCC_VERSION_FUNC_ID
> 	b. CONFIG_ARM_PSCI{,_FW} is not defined
> 
> Mark/Will/Marc,
> 
> Any other use-case config missed above ?

Do we need to support SMCCC without PSCI? Is anyone goingto build a
sysyem with SMCCC but no PSCI functionality?

If not, then given we can always probe SMCCC from PSCI (for both ACPI
and DT), I'd prefer to support only support doing things that way
around. i.e. have SMCCC depend on PSCI.

Otherwise I suspect we're inviting more problems than a dependency on
PSCI.

Thanks,
Mark.

Re: [PATCH] firmware: arm_scmi: fix psci dependency

[Will Deacon]

On Tue, May 05, 2020 at 04:04:21PM +0100, Sudeep Holla wrote:
> On Tue, May 05, 2020 at 04:08:08PM +0200, Arnd Bergmann wrote:
> > When CONFIG_ARM_PSCI_FW is disabled but CONFIG_HAVE_ARM_SMCCC is enabled,
> > arm-scmi runs into a link failure:
> >
> > arm-linux-gnueabi-ld: drivers/firmware/arm_scmi/smc.o: in function `smc_send_message':
> > smc.c:(.text+0x200): undefined reference to `arm_smccc_1_1_get_conduit'
> >
> > Use an inline helper to default to version v1.0 in the absence of psci.
> >
> 
> Thanks for fixing this. I was thinking if we can separate PSCI and SMCCC
> quickly as a fix for this but I think he needs to be discussed in detail.
> 
> I am fine with this fix as is and happy to apply to my tree if no one
> objects.
> 
> Sorry but taking this patch as opportunity to discuss how to carry the
> dependency in future. Just a proposal,
> 
> 1. Introduce a DT node for SMCCC v1.2+
> 2. The new SMCCC driver(strictly speaking library/few APIs) can probe 
>    independent of PSCI if DT node is present
> 3. Else we fallback on PSCI and detect the SMCCC version for v1.1 and
>    v1.2
> 4. Assume v1.0 if
> 	a. PSCI FEATURE returns NOT_SUPPORTED for ARM_SMCCC_VERSION_FUNC_ID
> 	b. CONFIG_ARM_PSCI{,_FW} is not defined
> 
> Mark/Will/Marc,
> 
> Any other use-case config missed above ?

Do we really gain much by separating PSCI from SMCCC? In other words, why
do we care about allowing them to be selected independently?

Will

Re: [PATCH] firmware: arm_scmi: fix psci dependency

[Sudeep Holla]

On Tue, May 05, 2020 at 05:21:36PM +0100, Mark Rutland wrote:
> On Tue, May 05, 2020 at 04:04:21PM +0100, Sudeep Holla wrote:
> > Hi Arnd,
> >
> > On Tue, May 05, 2020 at 04:08:08PM +0200, Arnd Bergmann wrote:
> > > When CONFIG_ARM_PSCI_FW is disabled but CONFIG_HAVE_ARM_SMCCC is enabled,
> > > arm-scmi runs into a link failure:
> > >
> > > arm-linux-gnueabi-ld: drivers/firmware/arm_scmi/smc.o: in function `smc_send_message':
> > > smc.c:(.text+0x200): undefined reference to `arm_smccc_1_1_get_conduit'
> > >
> > > Use an inline helper to default to version v1.0 in the absence of psci.
> > >
> >
> > Thanks for fixing this. I was thinking if we can separate PSCI and SMCCC
> > quickly as a fix for this but I think he needs to be discussed in detail.
> >
> > I am fine with this fix as is and happy to apply to my tree if no one
> > objects.
> >
> > Sorry but taking this patch as opportunity to discuss how to carry the
> > dependency in future. Just a proposal,
> >
> > 1. Introduce a DT node for SMCCC v1.2+
> > 2. The new SMCCC driver(strictly speaking library/few APIs) can probe
> >    independent of PSCI if DT node is present
> > 3. Else we fallback on PSCI and detect the SMCCC version for v1.1 and
> >    v1.2
> > 4. Assume v1.0 if
> > 	a. PSCI FEATURE returns NOT_SUPPORTED for ARM_SMCCC_VERSION_FUNC_ID
> > 	b. CONFIG_ARM_PSCI{,_FW} is not defined
> >
> > Mark/Will/Marc,
> >
> > Any other use-case config missed above ?
>
> Do we need to support SMCCC without PSCI? Is anyone goingto build a
> sysyem with SMCCC but no PSCI functionality?
>

May be arm32 using all new fancy specification we may come up to solve
certain areas but continue to use legacy boot/power methods. I may be
wrong. E.g: Today we enable HAVE_ARM_SMCCC for armv7 and above but not
all have PSCI enabled.

> If not, then given we can always probe SMCCC from PSCI (for both ACPI
> and DT), I'd prefer to support only support doing things that way
> around. i.e. have SMCCC depend on PSCI.
>

OK, but we still have above config.

> Otherwise I suspect we're inviting more problems than a dependency on
> PSCI.
>

Agreed and I am happy to keep it as is.

--
Regards,
Sudeep

Re: [PATCH] firmware: arm_scmi: fix psci dependency

[Sudeep Holla]

On Tue, May 05, 2020 at 05:22:42PM +0100, Will Deacon wrote:
> On Tue, May 05, 2020 at 04:04:21PM +0100, Sudeep Holla wrote:
> > On Tue, May 05, 2020 at 04:08:08PM +0200, Arnd Bergmann wrote:
> > > When CONFIG_ARM_PSCI_FW is disabled but CONFIG_HAVE_ARM_SMCCC is enabled,
> > > arm-scmi runs into a link failure:
> > >
> > > arm-linux-gnueabi-ld: drivers/firmware/arm_scmi/smc.o: in function `smc_send_message':
> > > smc.c:(.text+0x200): undefined reference to `arm_smccc_1_1_get_conduit'
> > >
> > > Use an inline helper to default to version v1.0 in the absence of psci.
> > >
> > 
> > Thanks for fixing this. I was thinking if we can separate PSCI and SMCCC
> > quickly as a fix for this but I think he needs to be discussed in detail.
> > 
> > I am fine with this fix as is and happy to apply to my tree if no one
> > objects.
> > 
> > Sorry but taking this patch as opportunity to discuss how to carry the
> > dependency in future. Just a proposal,
> > 
> > 1. Introduce a DT node for SMCCC v1.2+
> > 2. The new SMCCC driver(strictly speaking library/few APIs) can probe 
> >    independent of PSCI if DT node is present
> > 3. Else we fallback on PSCI and detect the SMCCC version for v1.1 and
> >    v1.2
> > 4. Assume v1.0 if
> > 	a. PSCI FEATURE returns NOT_SUPPORTED for ARM_SMCCC_VERSION_FUNC_ID
> > 	b. CONFIG_ARM_PSCI{,_FW} is not defined
> > 
> > Mark/Will/Marc,
> > 
> > Any other use-case config missed above ?
> 
> Do we really gain much by separating PSCI from SMCCC? In other words, why
> do we care about allowing them to be selected independently?
> 

As I mentioned to Mark's response, it is mostly the current arm32
configuration. We enable SMCCC for armv7 and of course we don't mandate
PSCI for them. I assumed depending on PSCI for all uses of SMCCC even
if it is not remotely connected to PSCI was wrong. But if that is fine,
then we don't have any issue 😄

-- 
Regards,
Sudeep

Re: [PATCH] firmware: arm_scmi: fix psci dependency

[Sudeep Holla]

On Tue, May 05, 2020 at 04:04:21PM +0100, Sudeep Holla wrote:
> Hi Arnd,
>
> On Tue, May 05, 2020 at 04:08:08PM +0200, Arnd Bergmann wrote:
> > When CONFIG_ARM_PSCI_FW is disabled but CONFIG_HAVE_ARM_SMCCC is enabled,
> > arm-scmi runs into a link failure:
> >
> > arm-linux-gnueabi-ld: drivers/firmware/arm_scmi/smc.o: in function `smc_send_message':
> > smc.c:(.text+0x200): undefined reference to `arm_smccc_1_1_get_conduit'
> >
> > Use an inline helper to default to version v1.0 in the absence of psci.
> >
>
> Thanks for fixing this. I was thinking if we can separate PSCI and SMCCC
> quickly as a fix for this but I think he needs to be discussed in detail.
>
> I am fine with this fix as is and happy to apply to my tree if no one
> objects.
>
> Sorry but taking this patch as opportunity to discuss how to carry the
> dependency in future. Just a proposal,
>
> 1. Introduce a DT node for SMCCC v1.2+

Sorry for the confusion and the above note might have scared people.
Also I misunderstood some internal discussions we had as part of my
SMCCC SOC_ID series[1]. I realised(rather Mark made me realise) this
SCMI SMC is kind of dependent on PSCI as we reuse the conduit from there.
So for now I will make it depend on PSCI to fix the build temporarily.

Also I will try to work out if we can move HAVE_ARCH_SMCCC to something
like HAVE_ARCH_RAW_SMCCC and use HAVE_ARCH_SMCCC for newer changes that
depend on PSCI for probing and conduit. The legacy SMCCC can then go under
HAVE_ARCH_RAW_SMCCC. This was Mark's suggestion, I will try to work it
out and may be post it as part of my SMCCC SOC_ID series also moving
code under drivers/firmware/smccc

--
Regards,
Sudeep

[1] https://lore.kernel.org/r/20200504092905.10580-1-sudeep.holla@arm.com

[PATCH] firmware: arm_scmi: fix psci dependency

[Sudeep Holla]

When CONFIG_ARM_PSCI_FW is disabled but CONFIG_HAVE_ARM_SMCCC is enabled,
arm-scmi runs into a link failure:

arm-linux-gnueabi-ld: drivers/firmware/arm_scmi/smc.o: in function `smc_send_message':
smc.c:(.text+0x200): undefined reference to `arm_smccc_1_1_get_conduit'

Change from HAVE_ARM_SMCCC to ARM_PSCI_FW config dependency for now.
We rely on PSCI bindings anyways for the conduit and this should be
fine.

Fixes: 1dc6558062da ("firmware: arm_scmi: Add smc/hvc transport")
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/firmware/arm_scmi/Makefile | 2 +-
 drivers/firmware/arm_scmi/driver.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Hi Arnd,

I am planning to push this patch in favour of [1]. Once we have a
separate config[2] for SMCCC v1.1+, I will change to use it instead of
ARM_PSCI_FW.

Regards,
Sudeep

[1] https://lore.kernel.org/r/20200505140820.536615-1-arnd@arndb.de
[2] https://lore.kernel.org/r/20200506164411.3284-1-sudeep.holla@arm.com/


diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile
index 11b238f81923..1cad32b38b29 100644
--- a/drivers/firmware/arm_scmi/Makefile
+++ b/drivers/firmware/arm_scmi/Makefile
@@ -4,6 +4,6 @@ scmi-bus-y = bus.o
 scmi-driver-y = driver.o
 scmi-transport-y = shmem.o
 scmi-transport-$(CONFIG_MAILBOX) += mailbox.o
-scmi-transport-$(CONFIG_HAVE_ARM_SMCCC) += smc.o
+scmi-transport-$(CONFIG_ARM_PSCI_FW) += smc.o
 scmi-protocols-y = base.o clock.o perf.o power.o reset.o sensors.o
 obj-$(CONFIG_ARM_SCMI_POWER_DOMAIN) += scmi_pm_domain.o
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index fec308e47b9d..7483cacf63f9 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -901,7 +901,7 @@ ATTRIBUTE_GROUPS(versions);
 /* Each compatible listed below must have descriptor associated with it */
 static const struct of_device_id scmi_of_match[] = {
 	{ .compatible = "arm,scmi", .data = &scmi_mailbox_desc },
-#ifdef CONFIG_HAVE_ARM_SMCCC
+#ifdef CONFIG_ARM_PSCI_FW
 	{ .compatible = "arm,scmi-smc", .data = &scmi_smc_desc},
 #endif
 	{ /* Sentinel */ },
-- 
2.17.1