All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Rutland <mark.rutland@arm.com>
To: Florian Fainelli <f.fainelli@gmail.com>
Cc: linux-arm-kernel@lists.infradead.org,
	"maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE" 
	<bcm-kernel-feedback-list@broadcom.com>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 0/4] Broadcom STB PM PSCI extensions
Date: Thu, 3 Feb 2022 10:47:13 +0000	[thread overview]
Message-ID: <YfuysSBnZbnMF4OC@FVFF77S0Q05N> (raw)
In-Reply-To: <20220122035421.4086618-1-f.fainelli@gmail.com>

On Fri, Jan 21, 2022 at 07:54:17PM -0800, Florian Fainelli wrote:
> Hi all,

Hi Florian,

> This patch series contains the Broadcom STB PSCI extensions which adds
> some additional functions on top of the existing standard PSCI interface
> which is the reason for having the driver implement a custom
> suspend_ops.

I *really* don't like the idea of having non-standard PSCI extensions, because
it somewhat defeats the point of PSCI being a standard, and opens the door for
the zoo of mechanisms we had on 32-bit.

I think this needs a fair amount more explanation and justification.

> These platforms have traditionally supported a mode that is akin to
> ACPI's S2 with the CPU in WFI and all of the chip being clock gated
> which is entered with "echo standby > /sys/power/state". Additional a
> true suspend to DRAM as defined in ACPI by S3 is implemented with "echo
> mem > /sys/power/state".

Why isn't a combination of CPU_SUSPEND and SYSTEM_SUSPEND sufficient here?

What specifically *can't* you do with standard PSCI calls?

> These platforms also may have an external Broadcom PMIC chip which can
> cause the SoC to be powercycled assuming that we communicate that intent
> via a vendor specific PSCI SYSTEM_RESET2.
> 
> Since it is desirable to get any new functionality added to the kernel
> to be loadable as a module as part of shipping said products in a Google
> Kernel Image (GKI) environment, we need to export a couple of symbols from
> drivers/firmware/psci/psci.c.

I really don't want to export the guts of psci.c.

Thanks,
Mark.

> Thanks for your feedback.
> 
> Florian Fainelli (4):
>   firmware: psci: Export a couple of suspend symbols
>   soc: bcm: brcmstb: Make legacy PM code depend on !ARM_PSCI_FW
>   soc: bcm: brcmstb: Added support for PSCI system suspend operations
>   Documentation: ABI: Document Broadcom STB PSCI firmware files
> 
>  .../ABI/testing/sysfs-firmware-brcmstb        |  16 +
>  drivers/firmware/psci/psci.c                  |   9 +-
>  drivers/soc/bcm/brcmstb/Kconfig               |   4 +-
>  drivers/soc/bcm/brcmstb/pm/Makefile           |   3 +
>  drivers/soc/bcm/brcmstb/pm/pm-psci.c          | 315 ++++++++++++++++++
>  include/linux/psci.h                          |   2 +
>  include/linux/soc/brcmstb/brcmstb-smccc.h     |  84 +++++
>  7 files changed, 430 insertions(+), 3 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-firmware-brcmstb
>  create mode 100644 drivers/soc/bcm/brcmstb/pm/pm-psci.c
>  create mode 100644 include/linux/soc/brcmstb/brcmstb-smccc.h
> 
> -- 
> 2.25.1
> 

WARNING: multiple messages have this Message-ID (diff)
From: Mark Rutland <mark.rutland@arm.com>
To: Florian Fainelli <f.fainelli@gmail.com>
Cc: linux-arm-kernel@lists.infradead.org,
	"maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE"
	<bcm-kernel-feedback-list@broadcom.com>,
	 Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 0/4] Broadcom STB PM PSCI extensions
Date: Thu, 3 Feb 2022 10:47:13 +0000	[thread overview]
Message-ID: <YfuysSBnZbnMF4OC@FVFF77S0Q05N> (raw)
In-Reply-To: <20220122035421.4086618-1-f.fainelli@gmail.com>

On Fri, Jan 21, 2022 at 07:54:17PM -0800, Florian Fainelli wrote:
> Hi all,

Hi Florian,

> This patch series contains the Broadcom STB PSCI extensions which adds
> some additional functions on top of the existing standard PSCI interface
> which is the reason for having the driver implement a custom
> suspend_ops.

I *really* don't like the idea of having non-standard PSCI extensions, because
it somewhat defeats the point of PSCI being a standard, and opens the door for
the zoo of mechanisms we had on 32-bit.

I think this needs a fair amount more explanation and justification.

> These platforms have traditionally supported a mode that is akin to
> ACPI's S2 with the CPU in WFI and all of the chip being clock gated
> which is entered with "echo standby > /sys/power/state". Additional a
> true suspend to DRAM as defined in ACPI by S3 is implemented with "echo
> mem > /sys/power/state".

Why isn't a combination of CPU_SUSPEND and SYSTEM_SUSPEND sufficient here?

What specifically *can't* you do with standard PSCI calls?

> These platforms also may have an external Broadcom PMIC chip which can
> cause the SoC to be powercycled assuming that we communicate that intent
> via a vendor specific PSCI SYSTEM_RESET2.
> 
> Since it is desirable to get any new functionality added to the kernel
> to be loadable as a module as part of shipping said products in a Google
> Kernel Image (GKI) environment, we need to export a couple of symbols from
> drivers/firmware/psci/psci.c.

I really don't want to export the guts of psci.c.

Thanks,
Mark.

> Thanks for your feedback.
> 
> Florian Fainelli (4):
>   firmware: psci: Export a couple of suspend symbols
>   soc: bcm: brcmstb: Make legacy PM code depend on !ARM_PSCI_FW
>   soc: bcm: brcmstb: Added support for PSCI system suspend operations
>   Documentation: ABI: Document Broadcom STB PSCI firmware files
> 
>  .../ABI/testing/sysfs-firmware-brcmstb        |  16 +
>  drivers/firmware/psci/psci.c                  |   9 +-
>  drivers/soc/bcm/brcmstb/Kconfig               |   4 +-
>  drivers/soc/bcm/brcmstb/pm/Makefile           |   3 +
>  drivers/soc/bcm/brcmstb/pm/pm-psci.c          | 315 ++++++++++++++++++
>  include/linux/psci.h                          |   2 +
>  include/linux/soc/brcmstb/brcmstb-smccc.h     |  84 +++++
>  7 files changed, 430 insertions(+), 3 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-firmware-brcmstb
>  create mode 100644 drivers/soc/bcm/brcmstb/pm/pm-psci.c
>  create mode 100644 include/linux/soc/brcmstb/brcmstb-smccc.h
> 
> -- 
> 2.25.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  parent reply	other threads:[~2022-02-03 10:47 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-22  3:54 [PATCH 0/4] Broadcom STB PM PSCI extensions Florian Fainelli
2022-01-22  3:54 ` Florian Fainelli
2022-01-22  3:54 ` [PATCH 1/4] firmware: psci: Export a couple of suspend symbols Florian Fainelli
2022-01-22  3:54   ` Florian Fainelli
2022-01-22 12:22   ` kernel test robot
2022-01-22 12:22     ` kernel test robot
2022-01-22  3:54 ` [PATCH 2/4] soc: bcm: brcmstb: Make legacy PM code depend on !ARM_PSCI_FW Florian Fainelli
2022-01-22  3:54   ` Florian Fainelli
2022-01-22  3:54 ` [PATCH 3/4] soc: bcm: brcmstb: Added support for PSCI system suspend operations Florian Fainelli
2022-01-22  3:54   ` Florian Fainelli
2022-01-22  6:09   ` kernel test robot
2022-01-22  6:09     ` kernel test robot
2022-01-22  7:10   ` kernel test robot
2022-01-22  7:10     ` kernel test robot
2022-02-03 12:09   ` Mark Rutland
2022-02-03 12:09     ` Mark Rutland
2022-02-03 18:45     ` Florian Fainelli
2022-02-03 18:45       ` Florian Fainelli
2022-01-22  3:54 ` [PATCH 4/4] Documentation: ABI: Document Broadcom STB PSCI firmware files Florian Fainelli
2022-01-22  3:54   ` Florian Fainelli
2022-01-27  3:55 ` [PATCH 0/4] Broadcom STB PM PSCI extensions Florian Fainelli
2022-01-27  3:55   ` Florian Fainelli
2022-02-03 10:47 ` Mark Rutland [this message]
2022-02-03 10:47   ` Mark Rutland
2022-02-03 18:32   ` Florian Fainelli
2022-02-03 18:32     ` Florian Fainelli
2022-02-03 11:14 ` Sudeep Holla
2022-02-03 11:14   ` Sudeep Holla
2022-02-03 17:36   ` Florian Fainelli
2022-02-03 17:36     ` Florian Fainelli
2022-02-03 18:52     ` Sudeep Holla
2022-02-03 18:52       ` Sudeep Holla
2022-02-03 19:33       ` Florian Fainelli
2022-02-03 19:33         ` Florian Fainelli
2022-02-07 16:27         ` Sudeep Holla
2022-02-07 16:27           ` Sudeep Holla
2022-02-14 18:12           ` Florian Fainelli
2022-02-14 18:12             ` Florian Fainelli

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YfuysSBnZbnMF4OC@FVFF77S0Q05N \
    --to=mark.rutland@arm.com \
    --cc=bcm-kernel-feedback-list@broadcom.com \
    --cc=f.fainelli@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lorenzo.pieralisi@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.