[PATCH 0/2] drivers: firmware: psci: Extract psci_is_available() helper

[PATCH 0/2] drivers: firmware: psci: Extract psci_is_available() helper

[Geert Uytterhoeven]

	Hi Mark, Lorenzo,

PSCI support may be disabled at build time (by configuration) or at
run-time (PSCI firmware not present).  While CONFIG_ARM_PSCI_FW can be
used to check for build time enablement, there is currently no simple
way to check if PSCI is actually available and used.

The first patch adds a helper function to check if PSCI is available.
This is useful for e.g. drivers that are used on platforms with and
without PSCI.  Such drivers may need to take provisions for proper
operation when PSCI is used, and/or to implement functionality that is
usually provided by PSCI.

The second patch makes the PSCI checker use the new helper.

Thanks for your comments!

Geert Uytterhoeven (2):
  drivers: firmware: psci: Add psci_is_available()
  drivers: psci: checker: Use psci_is_available()

 drivers/firmware/psci.c         | 5 +++++
 drivers/firmware/psci_checker.c | 2 +-
 include/linux/psci.h            | 2 ++
 3 files changed, 8 insertions(+), 1 deletion(-)

-- 
2.7.4

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

[PATCH 0/2] drivers: firmware: psci: Extract psci_is_available() helper

[Geert Uytterhoeven]

	Hi Mark, Lorenzo,

PSCI support may be disabled at build time (by configuration) or at
run-time (PSCI firmware not present).  While CONFIG_ARM_PSCI_FW can be
used to check for build time enablement, there is currently no simple
way to check if PSCI is actually available and used.

The first patch adds a helper function to check if PSCI is available.
This is useful for e.g. drivers that are used on platforms with and
without PSCI.  Such drivers may need to take provisions for proper
operation when PSCI is used, and/or to implement functionality that is
usually provided by PSCI.

The second patch makes the PSCI checker use the new helper.

Thanks for your comments!

Geert Uytterhoeven (2):
  drivers: firmware: psci: Add psci_is_available()
  drivers: psci: checker: Use psci_is_available()

 drivers/firmware/psci.c         | 5 +++++
 drivers/firmware/psci_checker.c | 2 +-
 include/linux/psci.h            | 2 ++
 3 files changed, 8 insertions(+), 1 deletion(-)

-- 
2.7.4

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

[PATCH 1/2] drivers: firmware: psci: Add psci_is_available()

[Geert Uytterhoeven]

PSCI support may be disabled at build time (by configuration) or at
run-time (PSCI firmware not present).  While CONFIG_ARM_PSCI_FW can be
used to check for build time enablement, there is currently no simple
way to check if PSCI is actually available and used.

Hence add a helper function to check if PSCI is available.

This is useful for e.g. drivers that are used on platforms with and
without PSCI.  Such drivers may need to take provisions for proper
operation when PSCI is used, and/or to implement functionality that is
usually provided by PSCI.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/firmware/psci.c | 5 +++++
 include/linux/psci.h    | 2 ++
 2 files changed, 7 insertions(+)

diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
index da469c972b503f83..a3a11e2d8ffffaaa 100644
--- a/drivers/firmware/psci.c
+++ b/drivers/firmware/psci.c
@@ -670,6 +670,11 @@ int __init psci_dt_init(void)
 	return init_fn(np);
 }
 
+bool psci_is_available(void)
+{
+	return psci_ops.cpu_off && psci_ops.cpu_on && psci_ops.cpu_suspend;
+}
+
 #ifdef CONFIG_ACPI
 /*
  * We use PSCI 0.2+ when ACPI is deployed on ARM64 and it's
diff --git a/include/linux/psci.h b/include/linux/psci.h
index bdea1cb5e1db142b..2bdee325aeb80cf6 100644
--- a/include/linux/psci.h
+++ b/include/linux/psci.h
@@ -39,8 +39,10 @@ extern struct psci_operations psci_ops;
 
 #if defined(CONFIG_ARM_PSCI_FW)
 int __init psci_dt_init(void);
+bool psci_is_available(void);
 #else
 static inline int psci_dt_init(void) { return 0; }
+static inline bool psci_is_available(void) { return false; }
 #endif
 
 #if defined(CONFIG_ARM_PSCI_FW) && defined(CONFIG_ACPI)
-- 
2.7.4

[PATCH 1/2] drivers: firmware: psci: Add psci_is_available()

[Geert Uytterhoeven]

PSCI support may be disabled at build time (by configuration) or at
run-time (PSCI firmware not present).  While CONFIG_ARM_PSCI_FW can be
used to check for build time enablement, there is currently no simple
way to check if PSCI is actually available and used.

Hence add a helper function to check if PSCI is available.

This is useful for e.g. drivers that are used on platforms with and
without PSCI.  Such drivers may need to take provisions for proper
operation when PSCI is used, and/or to implement functionality that is
usually provided by PSCI.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/firmware/psci.c | 5 +++++
 include/linux/psci.h    | 2 ++
 2 files changed, 7 insertions(+)

diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
index da469c972b503f83..a3a11e2d8ffffaaa 100644
--- a/drivers/firmware/psci.c
+++ b/drivers/firmware/psci.c
@@ -670,6 +670,11 @@ int __init psci_dt_init(void)
 	return init_fn(np);
 }
 
+bool psci_is_available(void)
+{
+	return psci_ops.cpu_off && psci_ops.cpu_on && psci_ops.cpu_suspend;
+}
+
 #ifdef CONFIG_ACPI
 /*
  * We use PSCI 0.2+ when ACPI is deployed on ARM64 and it's
diff --git a/include/linux/psci.h b/include/linux/psci.h
index bdea1cb5e1db142b..2bdee325aeb80cf6 100644
--- a/include/linux/psci.h
+++ b/include/linux/psci.h
@@ -39,8 +39,10 @@ extern struct psci_operations psci_ops;
 
 #if defined(CONFIG_ARM_PSCI_FW)
 int __init psci_dt_init(void);
+bool psci_is_available(void);
 #else
 static inline int psci_dt_init(void) { return 0; }
+static inline bool psci_is_available(void) { return false; }
 #endif
 
 #if defined(CONFIG_ARM_PSCI_FW) && defined(CONFIG_ACPI)
-- 
2.7.4

Re: [PATCH 1/2] drivers: firmware: psci: Add psci_is_available()

[Jisheng Zhang]

On Wed, 11 Oct 2017 10:03:01 +0200 Geert Uytterhoeven wrote:

> PSCI support may be disabled at build time (by configuration) or at
> run-time (PSCI firmware not present).  While CONFIG_ARM_PSCI_FW can be
> used to check for build time enablement, there is currently no simple
> way to check if PSCI is actually available and used.
> 
> Hence add a helper function to check if PSCI is available.
> 
> This is useful for e.g. drivers that are used on platforms with and
> without PSCI.  Such drivers may need to take provisions for proper
> operation when PSCI is used, and/or to implement functionality that is
> usually provided by PSCI.

the psci_ops is a global var, why not check psci_ops.mmnn members
individually in each drivers? check IS_ENABLED(CONFIG_ARM_PSCI_FW) for
building time ennoblement.

> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  drivers/firmware/psci.c | 5 +++++
>  include/linux/psci.h    | 2 ++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
> index da469c972b503f83..a3a11e2d8ffffaaa 100644
> --- a/drivers/firmware/psci.c
> +++ b/drivers/firmware/psci.c
> @@ -670,6 +670,11 @@ int __init psci_dt_init(void)
>  	return init_fn(np);
>  }
>  
> +bool psci_is_available(void)
> +{
> +	return psci_ops.cpu_off && psci_ops.cpu_on && psci_ops.cpu_suspend;

IIRC, for PSCI 0.1, cpu_suspend is optional, we can't say the PSCI isn't
available if cpu_suspend is missing.

Thanks

[PATCH 1/2] drivers: firmware: psci: Add psci_is_available()

[Jisheng Zhang]

On Wed, 11 Oct 2017 10:03:01 +0200 Geert Uytterhoeven wrote:

> PSCI support may be disabled at build time (by configuration) or at
> run-time (PSCI firmware not present).  While CONFIG_ARM_PSCI_FW can be
> used to check for build time enablement, there is currently no simple
> way to check if PSCI is actually available and used.
> 
> Hence add a helper function to check if PSCI is available.
> 
> This is useful for e.g. drivers that are used on platforms with and
> without PSCI.  Such drivers may need to take provisions for proper
> operation when PSCI is used, and/or to implement functionality that is
> usually provided by PSCI.

the psci_ops is a global var, why not check psci_ops.mmnn members
individually in each drivers? check IS_ENABLED(CONFIG_ARM_PSCI_FW) for
building time ennoblement.

> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  drivers/firmware/psci.c | 5 +++++
>  include/linux/psci.h    | 2 ++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
> index da469c972b503f83..a3a11e2d8ffffaaa 100644
> --- a/drivers/firmware/psci.c
> +++ b/drivers/firmware/psci.c
> @@ -670,6 +670,11 @@ int __init psci_dt_init(void)
>  	return init_fn(np);
>  }
>  
> +bool psci_is_available(void)
> +{
> +	return psci_ops.cpu_off && psci_ops.cpu_on && psci_ops.cpu_suspend;

IIRC, for PSCI 0.1, cpu_suspend is optional, we can't say the PSCI isn't
available if cpu_suspend is missing.

Thanks

Re: [PATCH 1/2] drivers: firmware: psci: Add psci_is_available()

[Geert Uytterhoeven]

Hi Jisheng,

On Thu, Oct 12, 2017 at 4:58 AM, Jisheng Zhang
<Jisheng.Zhang@synaptics.com> wrote:
> On Wed, 11 Oct 2017 10:03:01 +0200 Geert Uytterhoeven wrote:
>
>> PSCI support may be disabled at build time (by configuration) or at
>> run-time (PSCI firmware not present).  While CONFIG_ARM_PSCI_FW can be
>> used to check for build time enablement, there is currently no simple
>> way to check if PSCI is actually available and used.
>>
>> Hence add a helper function to check if PSCI is available.
>>
>> This is useful for e.g. drivers that are used on platforms with and
>> without PSCI.  Such drivers may need to take provisions for proper
>> operation when PSCI is used, and/or to implement functionality that is
>> usually provided by PSCI.
>
> the psci_ops is a global var, why not check psci_ops.mmnn members
> individually in each drivers? check IS_ENABLED(CONFIG_ARM_PSCI_FW) for
> building time ennoblement.

While psci_ops is a global var, it only exists if CONFIG_ARM_PSCI_FW=y,
so typical code like

    if (IS_ENABLED(CONFIG_ARM_PSCI_FW) && psci_ops.cpu_suspend) {
            ...
    }

won't link. So having a helper is useful.
The alternative is #ifdef.

>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> ---
>>  drivers/firmware/psci.c | 5 +++++
>>  include/linux/psci.h    | 2 ++
>>  2 files changed, 7 insertions(+)
>>
>> diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
>> index da469c972b503f83..a3a11e2d8ffffaaa 100644
>> --- a/drivers/firmware/psci.c
>> +++ b/drivers/firmware/psci.c
>> @@ -670,6 +670,11 @@ int __init psci_dt_init(void)
>>       return init_fn(np);
>>  }
>>
>> +bool psci_is_available(void)
>> +{
>> +     return psci_ops.cpu_off && psci_ops.cpu_on && psci_ops.cpu_suspend;
>
> IIRC, for PSCI 0.1, cpu_suspend is optional, we can't say the PSCI isn't
> available if cpu_suspend is missing.

Hmm...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH 1/2] drivers: firmware: psci: Add psci_is_available()

[Geert Uytterhoeven]

Hi Jisheng,

On Thu, Oct 12, 2017 at 4:58 AM, Jisheng Zhang
<Jisheng.Zhang@synaptics.com> wrote:
> On Wed, 11 Oct 2017 10:03:01 +0200 Geert Uytterhoeven wrote:
>
>> PSCI support may be disabled at build time (by configuration) or at
>> run-time (PSCI firmware not present).  While CONFIG_ARM_PSCI_FW can be
>> used to check for build time enablement, there is currently no simple
>> way to check if PSCI is actually available and used.
>>
>> Hence add a helper function to check if PSCI is available.
>>
>> This is useful for e.g. drivers that are used on platforms with and
>> without PSCI.  Such drivers may need to take provisions for proper
>> operation when PSCI is used, and/or to implement functionality that is
>> usually provided by PSCI.
>
> the psci_ops is a global var, why not check psci_ops.mmnn members
> individually in each drivers? check IS_ENABLED(CONFIG_ARM_PSCI_FW) for
> building time ennoblement.

While psci_ops is a global var, it only exists if CONFIG_ARM_PSCI_FW=y,
so typical code like

    if (IS_ENABLED(CONFIG_ARM_PSCI_FW) && psci_ops.cpu_suspend) {
            ...
    }

won't link. So having a helper is useful.
The alternative is #ifdef.

>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> ---
>>  drivers/firmware/psci.c | 5 +++++
>>  include/linux/psci.h    | 2 ++
>>  2 files changed, 7 insertions(+)
>>
>> diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
>> index da469c972b503f83..a3a11e2d8ffffaaa 100644
>> --- a/drivers/firmware/psci.c
>> +++ b/drivers/firmware/psci.c
>> @@ -670,6 +670,11 @@ int __init psci_dt_init(void)
>>       return init_fn(np);
>>  }
>>
>> +bool psci_is_available(void)
>> +{
>> +     return psci_ops.cpu_off && psci_ops.cpu_on && psci_ops.cpu_suspend;
>
> IIRC, for PSCI 0.1, cpu_suspend is optional, we can't say the PSCI isn't
> available if cpu_suspend is missing.

Hmm...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/2] drivers: firmware: psci: Add psci_is_available()

[Geert Uytterhoeven]

Hi Jisheng,

On Thu, Oct 12, 2017 at 4:58 AM, Jisheng Zhang
<Jisheng.Zhang@synaptics.com> wrote:
> On Wed, 11 Oct 2017 10:03:01 +0200 Geert Uytterhoeven wrote:
>
>> PSCI support may be disabled at build time (by configuration) or at
>> run-time (PSCI firmware not present).  While CONFIG_ARM_PSCI_FW can be
>> used to check for build time enablement, there is currently no simple
>> way to check if PSCI is actually available and used.
>>
>> Hence add a helper function to check if PSCI is available.
>>
>> This is useful for e.g. drivers that are used on platforms with and
>> without PSCI.  Such drivers may need to take provisions for proper
>> operation when PSCI is used, and/or to implement functionality that is
>> usually provided by PSCI.
>
> the psci_ops is a global var, why not check psci_ops.mmnn members
> individually in each drivers? check IS_ENABLED(CONFIG_ARM_PSCI_FW) for
> building time ennoblement.

While psci_ops is a global var, it only exists if CONFIG_ARM_PSCI_FW=y,
so typical code like

    if (IS_ENABLED(CONFIG_ARM_PSCI_FW) && psci_ops.cpu_suspend) {
            ...
    }

won't link. So having a helper is useful.
The alternative is #ifdef.

>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> ---
>>  drivers/firmware/psci.c | 5 +++++
>>  include/linux/psci.h    | 2 ++
>>  2 files changed, 7 insertions(+)
>>
>> diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
>> index da469c972b503f83..a3a11e2d8ffffaaa 100644
>> --- a/drivers/firmware/psci.c
>> +++ b/drivers/firmware/psci.c
>> @@ -670,6 +670,11 @@ int __init psci_dt_init(void)
>>       return init_fn(np);
>>  }
>>
>> +bool psci_is_available(void)
>> +{
>> +     return psci_ops.cpu_off && psci_ops.cpu_on && psci_ops.cpu_suspend;
>
> IIRC, for PSCI 0.1, cpu_suspend is optional, we can't say the PSCI isn't
> available if cpu_suspend is missing.

Hmm...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [1/2] drivers: firmware: psci: Add psci_is_available()

[Lorenzo Pieralisi]

On Wed, Oct 11, 2017 at 10:03:01AM +0200, Geert Uytterhoeven wrote:
> PSCI support may be disabled at build time (by configuration) or at
> run-time (PSCI firmware not present).  While CONFIG_ARM_PSCI_FW can be
> used to check for build time enablement, there is currently no simple
> way to check if PSCI is actually available and used.
> 
> Hence add a helper function to check if PSCI is available.

Hi Geert,

excuse us the delay in responding. I think it would be better if the
check just carries out a DT/ACPI matching check rather than being based
on PSCI ops initialization but before doing that I would like first to
understand what this function can be actually used for (ie I do not
think the usage in the PSCI checker is relevant to this discussion).

Thanks,
Lorenzo

> This is useful for e.g. drivers that are used on platforms with and
> without PSCI.  Such drivers may need to take provisions for proper
> operation when PSCI is used, and/or to implement functionality that is
> usually provided by PSCI.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  drivers/firmware/psci.c | 5 +++++
>  include/linux/psci.h    | 2 ++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
> index da469c972b503f83..a3a11e2d8ffffaaa 100644
> --- a/drivers/firmware/psci.c
> +++ b/drivers/firmware/psci.c
> @@ -670,6 +670,11 @@ int __init psci_dt_init(void)
>  	return init_fn(np);
>  }
>  
> +bool psci_is_available(void)
> +{
> +	return psci_ops.cpu_off && psci_ops.cpu_on && psci_ops.cpu_suspend;
> +}
> +
>  #ifdef CONFIG_ACPI
>  /*
>   * We use PSCI 0.2+ when ACPI is deployed on ARM64 and it's
> diff --git a/include/linux/psci.h b/include/linux/psci.h
> index bdea1cb5e1db142b..2bdee325aeb80cf6 100644
> --- a/include/linux/psci.h
> +++ b/include/linux/psci.h
> @@ -39,8 +39,10 @@ extern struct psci_operations psci_ops;
>  
>  #if defined(CONFIG_ARM_PSCI_FW)
>  int __init psci_dt_init(void);
> +bool psci_is_available(void);
>  #else
>  static inline int psci_dt_init(void) { return 0; }
> +static inline bool psci_is_available(void) { return false; }
>  #endif
>  
>  #if defined(CONFIG_ARM_PSCI_FW) && defined(CONFIG_ACPI)

[1/2] drivers: firmware: psci: Add psci_is_available()

[Lorenzo Pieralisi]

On Wed, Oct 11, 2017 at 10:03:01AM +0200, Geert Uytterhoeven wrote:
> PSCI support may be disabled at build time (by configuration) or at
> run-time (PSCI firmware not present).  While CONFIG_ARM_PSCI_FW can be
> used to check for build time enablement, there is currently no simple
> way to check if PSCI is actually available and used.
> 
> Hence add a helper function to check if PSCI is available.

Hi Geert,

excuse us the delay in responding. I think it would be better if the
check just carries out a DT/ACPI matching check rather than being based
on PSCI ops initialization but before doing that I would like first to
understand what this function can be actually used for (ie I do not
think the usage in the PSCI checker is relevant to this discussion).

Thanks,
Lorenzo

> This is useful for e.g. drivers that are used on platforms with and
> without PSCI.  Such drivers may need to take provisions for proper
> operation when PSCI is used, and/or to implement functionality that is
> usually provided by PSCI.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  drivers/firmware/psci.c | 5 +++++
>  include/linux/psci.h    | 2 ++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
> index da469c972b503f83..a3a11e2d8ffffaaa 100644
> --- a/drivers/firmware/psci.c
> +++ b/drivers/firmware/psci.c
> @@ -670,6 +670,11 @@ int __init psci_dt_init(void)
>  	return init_fn(np);
>  }
>  
> +bool psci_is_available(void)
> +{
> +	return psci_ops.cpu_off && psci_ops.cpu_on && psci_ops.cpu_suspend;
> +}
> +
>  #ifdef CONFIG_ACPI
>  /*
>   * We use PSCI 0.2+ when ACPI is deployed on ARM64 and it's
> diff --git a/include/linux/psci.h b/include/linux/psci.h
> index bdea1cb5e1db142b..2bdee325aeb80cf6 100644
> --- a/include/linux/psci.h
> +++ b/include/linux/psci.h
> @@ -39,8 +39,10 @@ extern struct psci_operations psci_ops;
>  
>  #if defined(CONFIG_ARM_PSCI_FW)
>  int __init psci_dt_init(void);
> +bool psci_is_available(void);
>  #else
>  static inline int psci_dt_init(void) { return 0; }
> +static inline bool psci_is_available(void) { return false; }
>  #endif
>  
>  #if defined(CONFIG_ARM_PSCI_FW) && defined(CONFIG_ACPI)

[PATCH 2/2] drivers: firmware: psci: checker: Use psci_is_available()

[Geert Uytterhoeven]

Use the psci_is_available() helper instead of direct accesses to
psci_ops internals.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/firmware/psci_checker.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/psci_checker.c b/drivers/firmware/psci_checker.c
index 6523ce9628659755..72b40ed2dcce7cf4 100644
--- a/drivers/firmware/psci_checker.c
+++ b/drivers/firmware/psci_checker.c
@@ -54,7 +54,7 @@ static int psci_ops_check(void)
 	int migrate_type = -1;
 	int cpu;
 
-	if (!(psci_ops.cpu_off && psci_ops.cpu_on && psci_ops.cpu_suspend)) {
+	if (!psci_is_available()) {
 		pr_warn("Missing PSCI operations, aborting tests\n");
 		return -EOPNOTSUPP;
 	}
-- 
2.7.4