arm64: Kconfig: select PM for ARCH_VEXPRESS

The Linux AMBA bus framework probes the peripheral IDs when adding the AMBA devices very early on the boot. Generally they are on APB bus and just require APB clocks to be on even when most of the core logic of the IP is powered down.

However on Juno, the entire debugsys domain needs to be ON to access even the coresight components' CID/PID registers and hence broken by design. Accessing those while debugsys power domain is off will lead to the bridge stalling the transactions instead of returning the slave error.

Further, the AMBA framework can't deal with !CONFIG_PM case: it ignores the error and proceeds to access the device region. It was suggested to always enable CONFIG_PM in order to handle this scenario.

Suggested-by Ulf Hansson <[email protected]>

Signed-off-by Sudeep Holla <[email protected]>

---
 arch/arm64/Kconfig.platforms | 1 +
 1 file changed, 1 insertion(+)

Hi ARM-SoC team,

The discussion on this happened on linux-pm list[1]. This is need on Juno once we introduce coresight components in the DT. With !CONFIG_PM, the board stalls on boot and hence this patch is needed. This shouldn't change any thing in the defconfig as couple of other platforms already do the same. It's needed in case all other ARCH_* configs are disabled.

Without this, we need a dirty trick in the DT[2] to handle !CONFIG_PM.

Can you please pick this for v4.8 ?

Quoted 28 lines · CEST Sudeep Holla

The Linux AMBA bus framework probes the peripheral IDs when adding the AMBA devices very early on the boot. Generally they are on APB bus and just require APB clocks to be on even when most of the core logic of the IP is powered down.

However on Juno, the entire debugsys domain needs to be ON to access even the coresight components' CID/PID registers and hence broken by design. Accessing those while debugsys power domain is off will lead to the bridge stalling the transactions instead of returning the slave error.

Further, the AMBA framework can't deal with !CONFIG_PM case: it ignores the error and proceeds to access the device region. It was suggested to always enable CONFIG_PM in order to handle this scenario.

Suggested-by Ulf Hansson <[email protected]>

Signed-off-by Sudeep Holla <[email protected]>

---
  arch/arm64/Kconfig.platforms | 1 +
  1 file changed, 1 insertion(+)

Hi ARM-SoC team,

The discussion on this happened on linux-pm list[1]. This is need on Juno once we introduce coresight components in the DT. With !CONFIG_PM, the board stalls on boot and hence this patch is needed. This shouldn't change any thing in the defconfig as couple of other platforms already do the same. It's needed in case all other ARCH_* configs are disabled.

Quoted 3 lines

Without this, we need a dirty trick in the DT[2] to handle !CONFIG_PM.

Can you please pick this for v4.8 ?

Question: if AMBA cannot deal with CONFIG_PM disabled, should the 'select' be done from CONFIG_ARM_AMBA instead?

I also feel a little uncomfortable with just adding 'select PM', given that this option is normally just implied by CONFIG_SUSPEND || CONFIG_HIBERNATE_CALLBACKS,

but not selected individually,

so we might run into regressionswhen we get configurations that were invalid before.

Then again ARCH_RENESAS seems to already do this.

Quoted 8 lines · Arnd Bergmann

Quoted 8 lines

The discussion on this happened on linux-pm list[1]. This is need on Juno once we introduce coresight components in the DT. With !CONFIG_PM, the board stalls on boot and hence this patch is needed. This shouldn't change any thing in the defconfig as couple of other platforms already do the same. It's needed in case all other ARCH_* configs are disabled.

Without this, we need a dirty trick in the DT[2] to handle !CONFIG_PM.

Quoted 5 lines

Quoted 1 line

Can you please pick this for v4.8 ?

Question: if AMBA cannot deal with CONFIG_PM disabled, should the 'select' be done from CONFIG_ARM_AMBA instead?

But few platforms using AMBA may not want PM at all.

Quoted 3 lines

I also feel a little uncomfortable with just adding 'select PM', given that this option is normally just implied by CONFIG_SUSPEND || CONFIG_HIBERNATE_CALLBACKS,

On that analogy, for the current requirement I would say PM_GENERIC_DOMAINS should select but it just depends on PM.

Quoted 2 lines

so we might run into regressionswhen we get configurations that were invalid before.

Sorry my build/config knowledge is limited, I don't understand what you mean by the above. I am asking just to improve my knowledge.

Quoted 1 line

Then again ARCH_RENESAS seems to already do this.

Yes, it was hard to find get !CONFIG_PM as many platforms select it directly or indirectly.

Quoted 13 lines · CEST Sudeep Holla

Quoted 12 lines · Arnd Bergmann

Quoted 11 lines

The discussion on this happened on linux-pm list[1]. This is need on Juno once we introduce coresight components in the DT. With !CONFIG_PM, the board stalls on boot and hence this patch is needed. This shouldn't change any thing in the defconfig as couple of other platforms already do the same. It's needed in case all other ARCH_* configs are disabled.

Without this, we need a dirty trick in the DT[2] to handle !CONFIG_PM.

Can you please pick this for v4.8 ?

Quoted 5 lines

Quoted 3 lines

Question: if AMBA cannot deal with CONFIG_PM disabled, should the 'select' be done from CONFIG_ARM_AMBA instead?

But few platforms using AMBA may not want PM at all.

Ok, then I just misunderstood what you meant with "the AMBA framework can't deal with !CONFIG_PM case".

Quoted 4 lines

Quoted 3 lines

I also feel a little uncomfortable with just adding 'select PM', given that this option is normally just implied by CONFIG_SUSPEND || CONFIG_HIBERNATE_CALLBACKS,

Quoted 2 lines

On that analogy, for the current requirement I would say PM_GENERIC_DOMAINS should select but it just depends on PM.

Possibly, it's not entirely clear what the dependency is used for here, or what CONFIG_PM actually means. I'm also still unsure what aspect of CONFIG_PM you actually rely on here. Is this about the amba_pm_runtime_suspend/amba_pm_runtime_resume operations or something else?

Quoted 5 lines

Quoted 2 lines

so we might run into regressions when we get configurations that were invalid before.

Sorry my build/config knowledge is limited, I don't understand what you mean by the above. I am asking just to improve my knowledge.

If today, CONFIG_PM is only set when CONFIG_PM_SLEEP is set and we introduce a case where we have PM without PM_SLEEP, a lot of drivers that incorrectly make the assumption that PM_SLEEP is mean with CONFIG_PM may behave in new unexpected ways.

Quoted 4 lines

Quoted 1 line

Then again ARCH_RENESAS seems to already do this.

Yes, it was hard to find get !CONFIG_PM as many platforms select it directly or indirectly.

Starting from an allmodconfig build, I just needed to disable ARCH_RENESAS, ARCH_OMAP2PLUS_TYPICAL, SUSPEND and HIBERNATE.

Quoted 14 lines · Arnd Bergmann

Quoted 13 lines · CEST Sudeep Holla

Quoted 12 lines · Arnd Bergmann

Quoted 11 lines

The discussion on this happened on linux-pm list[1]. This is need on Juno once we introduce coresight components in the DT. With !CONFIG_PM, the board stalls on boot and hence this patch is needed. This shouldn't change any thing in the defconfig as couple of other platforms already do the same. It's needed in case all other ARCH_* configs are disabled.

Without this, we need a dirty trick in the DT[2] to handle !CONFIG_PM.

Can you please pick this for v4.8 ?

Quoted 9 lines

Quoted 5 lines

Quoted 3 lines

Question: if AMBA cannot deal with CONFIG_PM disabled, should the 'select' be done from CONFIG_ARM_AMBA instead?

But few platforms using AMBA may not want PM at all.

Ok, then I just misunderstood what you meant with "the AMBA framework can't deal with !CONFIG_PM case".

To be more precise genpd/runtime code return same error -ENODEV when PM_GENERIC_DOMAINS=n and if the no power domain is found with PM_GENERIC_DOMAINS=y . The AMBA bus code just checks for -EPROBE_DEFER to stop adding devices, while proceeds to access the device otherwise.

However on Juno, the coresight needs to powered up before accessing it. Well all is fine if you specify power domains in DT and have PM_GENERIC_DOMAINS=y. However if PM_GENERIC_DOMAINS=n the AMBA ignores the error(-ENODEV) from runtime PM and access the device resulting in boot hang when adding amba devices.

I thought we need is to have code to check for power domains in DT for the device even when PM_GENERIC_DOMAINS=n and return an error that can be used to skip registering the device and clearly identifies with other allowed errors. It may need a bit of surgery and closer look IMO. But when asked for opinion, I was suggested to do this. I admit that I didn't propose my thoughts then :)

Quoted 8 lines

Quoted 4 lines

Quoted 1 line

Then again ARCH_RENESAS seems to already do this.

Yes, it was hard to find get !CONFIG_PM as many platforms select it directly or indirectly.

Starting from an allmodconfig build, I just needed to disable ARCH_RENESAS, ARCH_OMAP2PLUS_TYPICAL, SUSPEND and HIBERNATE.

I assume this is in ARM32, I am looking at ARM64, but allmodconfig might help.

On Friday, June 17, 2016 3:59:34 PM CEST Sudeep Holla wrote:

Quoted 11 lines

To be more precise genpd/runtime code return same error -ENODEV when PM_GENERIC_DOMAINS=n and if the no power domain is found with PM_GENERIC_DOMAINS=y . The AMBA bus code just checks for -EPROBE_DEFER to stop adding devices, while proceeds to access the device otherwise.

However on Juno, the coresight needs to powered up before accessing it. Well all is fine if you specify power domains in DT and have PM_GENERIC_DOMAINS=y. However if PM_GENERIC_DOMAINS=n the AMBA ignores the error(-ENODEV) from runtime PM and access the device resulting in boot hang when adding amba devices.

Quoted 6 lines

I thought we need is to have code to check for power domains in DT for the device even when PM_GENERIC_DOMAINS=n and return an error that can be used to skip registering the device and clearly identifies with other allowed errors. It may need a bit of surgery and closer look IMO. But when asked for opinion, I was suggested to do this. I admit that I didn't propose my thoughts then :)

Ok, so we also need PM_GENERIC_DOMAINS, not just PM, right?

It also sounds like there is no easy way to just turn on the clocks in the amba bus handler when PM_GENERIC_DOMAINS is turned off, other than reimplementing PM_GENERIC_DOMAINS?

I'm just guessing here, I haven't looked into the code in enough depth.

Quoted 1 line · Arnd Bergmann

On Friday, June 17, 2016 3:59:34 PM CEST Sudeep Holla wrote:

[...]

Quoted 18 lines

Quoted 17 lines

To be more precise genpd/runtime code return same error -ENODEV when PM_GENERIC_DOMAINS=n and if the no power domain is found with PM_GENERIC_DOMAINS=y . The AMBA bus code just checks for -EPROBE_DEFER to stop adding devices, while proceeds to access the device otherwise.

However on Juno, the coresight needs to powered up before accessing it. Well all is fine if you specify power domains in DT and have PM_GENERIC_DOMAINS=y. However if PM_GENERIC_DOMAINS=n the AMBA ignores the error(-ENODEV) from runtime PM and access the device resulting in boot hang when adding amba devices.

I thought we need is to have code to check for power domains in DT for the device even when PM_GENERIC_DOMAINS=n and return an error that can be used to skip registering the device and clearly identifies with other allowed errors. It may need a bit of surgery and closer look IMO. But when asked for opinion, I was suggested to do this. I admit that I didn't propose my thoughts then :)

Quoted 2 lines

Ok, so we also need PM_GENERIC_DOMAINS, not just PM, right?

Yes, but latter won't select former. Generally I have seen domain controller code selecting PM_GENERIC_DOMAINS if PM=y

Quoted 4 lines

It also sounds like there is no easy way to just turn on the clocks in the amba bus handler when PM_GENERIC_DOMAINS is turned off, other than reimplementing PM_GENERIC_DOMAINS?

Turn on power domain, not just clocks(just to be precise), yes. It needs some thoughts as there are some assumptions on which existing platform work. Adding the support to handle the scenario I mentioned earlier is not trivial. Also the driver to handle the device may not be compiled in when PM_GENERIC_DOMAINS=n or when PM=n. So I was thinking of not accessing the device and hence skip adding it.

Quoted 24 lines · CEST Sudeep Holla

Quoted 1 line · Arnd Bergmann

On Friday, June 17, 2016 3:59:34 PM CEST Sudeep Holla wrote:

[...]

Quoted 18 lines

Quoted 17 lines

To be more precise genpd/runtime code return same error -ENODEV when PM_GENERIC_DOMAINS=n and if the no power domain is found with PM_GENERIC_DOMAINS=y . The AMBA bus code just checks for -EPROBE_DEFER to stop adding devices, while proceeds to access the device otherwise.

However on Juno, the coresight needs to powered up before accessing it. Well all is fine if you specify power domains in DT and have PM_GENERIC_DOMAINS=y. However if PM_GENERIC_DOMAINS=n the AMBA ignores the error(-ENODEV) from runtime PM and access the device resulting in boot hang when adding amba devices.

I thought we need is to have code to check for power domains in DT for the device even when PM_GENERIC_DOMAINS=n and return an error that can be used to skip registering the device and clearly identifies with other allowed errors. It may need a bit of surgery and closer look IMO. But when asked for opinion, I was suggested to do this. I admit that I didn't propose my thoughts then :)

Quoted 4 lines

Quoted 2 lines

Ok, so we also need PM_GENERIC_DOMAINS, not just PM, right?

Yes, but latter won't select former. Generally I have seen domain controller code selecting PM_GENERIC_DOMAINS if PM=y

Then at least we need the vexpress code to select both as well, otherwise you could end up with PM_GENERIC_DOMAINS disabled in configurations that have none of the other code.

Quoted 11 lines

Quoted 4 lines

It also sounds like there is no easy way to just turn on the clocks in the amba bus handler when PM_GENERIC_DOMAINS is turned off, other than reimplementing PM_GENERIC_DOMAINS?

Turn on power domain, not just clocks(just to be precise), yes. It needs some thoughts as there are some assumptions on which existing platform work. Adding the support to handle the scenario I mentioned earlier is not trivial. Also the driver to handle the device may not be compiled in when PM_GENERIC_DOMAINS=n or when PM=n. So I was thinking of not accessing the device and hence skip adding it.

I see. Another idea I had is flawed, too: we can't easily make ARM_AMBA depend on PM_GENERIC_DOMAINS, because the main relevant use case I can see for !PM builds is a kernel that is only used in virtual machines, but we still want to use the PL01x serial port driver in that configuration.

I'd say let's select both PM_GENERIC_DOMAINS and PM on VEXPRESS for now, (almost) as you have in your patch.