PSCI version misdetection

PSCI version misdetection

[Fabio Estevam]

Hi,

On a  imx7d-sdb board running 4.14-rc6:

[    0.000000] psci: probing for conduit method from DT.
[    0.000000] psci: PSCIv65535.65535 detected in firmware.
[    0.000000] psci: Using standard PSCI v0.2 function IDs
[    0.000000] psci: MIGRATE_INFO_TYPE not supported.

The PSCI version seems to be misdetected.

CONFIG_ARMV7_PSCI_1_0 is passed in U-Boot, so I would expect to see:

psci: PSCIv1.0 detected in firmware.

Any ideas?

Thanks

PSCI version misdetection

[Fabio Estevam]

On Sat, Mar 10, 2018 at 1:46 PM, Fabio Estevam <festevam@gmail.com> wrote:
> Hi,
>
> On a  imx7d-sdb board running 4.14-rc6:

I meant 4.14-rc4 :-)

>
> [    0.000000] psci: probing for conduit method from DT.
> [    0.000000] psci: PSCIv65535.65535 detected in firmware.
> [    0.000000] psci: Using standard PSCI v0.2 function IDs
> [    0.000000] psci: MIGRATE_INFO_TYPE not supported.
>
> The PSCI version seems to be misdetected.
>
> CONFIG_ARMV7_PSCI_1_0 is passed in U-Boot, so I would expect to see:
>
> psci: PSCIv1.0 detected in firmware.
>
> Any ideas?
>
> Thanks

PSCI version misdetection

[Fabio Estevam]

On Sun, Mar 11, 2018 at 3:37 PM, Fabio Estevam <festevam@gmail.com> wrote:
> On Sat, Mar 10, 2018 at 1:46 PM, Fabio Estevam <festevam@gmail.com> wrote:
>> Hi,
>>
>> On a  imx7d-sdb board running 4.14-rc6:
>
> I meant 4.14-rc4 :-)

Ok, let me type it right: 4.16-rc4

PSCI version misdetection

[Mark Rutland]

On Sat, Mar 10, 2018 at 01:46:30PM -0300, Fabio Estevam wrote:
> Hi,
> 
> On a  imx7d-sdb board running 4.14-rc6:
> 
> [    0.000000] psci: probing for conduit method from DT.
> [    0.000000] psci: PSCIv65535.65535 detected in firmware.
> [    0.000000] psci: Using standard PSCI v0.2 function IDs
> [    0.000000] psci: MIGRATE_INFO_TYPE not supported.
> 
> The PSCI version seems to be misdetected.
> 
> CONFIG_ARMV7_PSCI_1_0 is passed in U-Boot, so I would expect to see:
> 
> psci: PSCIv1.0 detected in firmware.
> 
> Any ideas?

It sounds like U-Boot hasn't implemented the PSCI_VERSION call, and is
returning NOT_IMPLEMENTED (-1). If so, this is not a conforming PSCI 1.0
implementation, as PSCI_VERSION is mandatory.

AFAICT, The default implementation in arch/arm/cpu/armv7/psci.S returns
NOT_SUPPORTED.

Where does the implementation for imx7d-sdb live? Is this upstream?

Thanks,
Mark.

PSCI version misdetection

[Fabio Estevam]

Hi Mark,

On Mon, Mar 12, 2018 at 7:02 AM, Mark Rutland <mark.rutland@arm.com> wrote:

> It sounds like U-Boot hasn't implemented the PSCI_VERSION call, and is
> returning NOT_IMPLEMENTED (-1). If so, this is not a conforming PSCI 1.0
> implementation, as PSCI_VERSION is mandatory.
>
> AFAICT, The default implementation in arch/arm/cpu/armv7/psci.S returns
> NOT_SUPPORTED.
>
> Where does the implementation for imx7d-sdb live? Is this upstream?

Yes, it is in mainline U-Boot.

As per this commit it would use PSCIv1.0:

http://git.denx.de/?p=u-boot.git;a=commitdiff;h=0ec3d98f76923619acfd91f93f8221e609e1e5bc;hp=04c28a7810970e404550f3510f8d9dfd90d6dc81

Thanks

PSCI version misdetection

[Mark Rutland]

On Mon, Mar 12, 2018 at 08:12:02AM -0300, Fabio Estevam wrote:
> Hi Mark,
> 
> On Mon, Mar 12, 2018 at 7:02 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> 
> > It sounds like U-Boot hasn't implemented the PSCI_VERSION call, and is
> > returning NOT_IMPLEMENTED (-1). If so, this is not a conforming PSCI 1.0
> > implementation, as PSCI_VERSION is mandatory.
> >
> > AFAICT, The default implementation in arch/arm/cpu/armv7/psci.S returns
> > NOT_SUPPORTED.
> >
> > Where does the implementation for imx7d-sdb live? Is this upstream?
> 
> Yes, it is in mainline U-Boot.
> 
> As per this commit it would use PSCIv1.0:
> 
> http://git.denx.de/?p=u-boot.git;a=commitdiff;h=0ec3d98f76923619acfd91f93f8221e609e1e5bc;hp=04c28a7810970e404550f3510f8d9dfd90d6dc81

Ok. AFAICT, you need to implement psci_version, and any other mandatory
PSCI features such as affinity info in your board code, as otherwise the
default weak implementation is used.

Defining CONFIG_ARMV7_PSCI_1_0 alone is not sufficient, and I would
strongly recommend reverting the above commit until those functions are
implemented. There is no benefit to an incomplete and non-conforming
PSCI 1.0, and this will cause problems.

I believe that also applies to the uniphier platform code, judging by
include/configs/uniphier.h.

Thanks,
Mark.

PSCI version misdetection

[Fabio Estevam]

Hi Mark,

On Mon, Mar 12, 2018 at 8:36 AM, Mark Rutland <mark.rutland@arm.com> wrote:

> Ok. AFAICT, you need to implement psci_version, and any other mandatory
> PSCI features such as affinity info in your board code, as otherwise the
> default weak implementation is used.

Thanks for your clarification.

I did as suggested and implemeted psci_version (in the same way as ls1021xa):
https://pastebin.com/ZfJkN2HC

and now I get:

[    0.000000] psci: probing for conduit method from DT.
[    0.000000] psci: PSCIv1.0 detected in firmware.
[    0.000000] psci: Using standard PSCI v0.2 function IDs
[    0.000000] psci: MIGRATE_INFO_TYPE not supported.

Thanks

PSCI version misdetection

[Mark Rutland]

On Mon, Mar 12, 2018 at 08:56:41AM -0300, Fabio Estevam wrote:
> Hi Mark,
> 
> On Mon, Mar 12, 2018 at 8:36 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> 
> > Ok. AFAICT, you need to implement psci_version, and any other mandatory
> > PSCI features such as affinity info in your board code, as otherwise the
> > default weak implementation is used.
> 
> Thanks for your clarification.
> 
> I did as suggested and implemeted psci_version (in the same way as ls1021xa):
> https://pastebin.com/ZfJkN2HC
> 
> and now I get:
> 
> [    0.000000] psci: probing for conduit method from DT.
> [    0.000000] psci: PSCIv1.0 detected in firmware.
> [    0.000000] psci: Using standard PSCI v0.2 function IDs
> [    0.000000] psci: MIGRATE_INFO_TYPE not supported.

Cool!

As I mentioned, you must also implement the other mandatory PSCI 1.0
features:

* AFFINITY_INFO -- this is necessary for correct synchronization of
  CPU_OFF calls.

* CPU_SUSPEND -- this can simply return INVALID_PARAMETERS for all
  power_state arguments.

Thanks,
Mark.