| Message ID | 20221116181307.198209-1-g-vlaev@ti.com |
|---|---|
| Headers | show |
| Series | firmware: ti_sci: Introduce system suspend support | expand |
On 16/11/2022 19:13, Georgi Vlaev wrote: > From: Dave Gerlach <d-gerlach@ti.com> > > Add documentation for the lpm region which tells the ti-sci driver where > to load the FS Stub low power mode firmware and also the firmware-name > which tells the driver which binary to load. Both of these are optional > for normal system operation but required to enabled suspend-to-mem usage > of Deep Sleep state. > I think you got here Rob's tag after sending v4. Reviewed-by: Rob Herring <robh@kernel.org> Best regards, Krzysztof
On 20:13-20221116, Georgi Vlaev wrote: [...] > +static int ti_sci_init_suspend(struct platform_device *pdev, > + struct ti_sci_info *info) > +{ > + struct device *dev = &pdev->dev; > + int ret; > + > + dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)); > + info->ctx_mem_buf = dma_alloc_coherent(info->dev, LPM_CTX_MEM_SIZE, > + &info->ctx_mem_addr, > + GFP_KERNEL); > + if (!info->ctx_mem_buf) { > + dev_err(info->dev, "Failed to allocate LPM context memory\n"); > + return -ENOMEM; > + } > + > + /* > + * Attempt to call prepare_sleep, this will be NAK'd if suspend is not > + * supported by firmware in use, in which case we will not attempt to > + * init suspend. > + */ > + ret = ti_sci_cmd_prepare_sleep(&info->handle, 0, > + (u32)(info->ctx_mem_addr & 0xffffffff), > + (u32)((u64)info->ctx_mem_addr >> 32), 0); > + > + if (ret) > + goto err; > + > + return 0; > +err: > + dma_free_coherent(info->dev, LPM_CTX_MEM_SIZE, > + info->ctx_mem_buf, > + info->ctx_mem_addr); > + return ret; > +} > + > /* Description for K2G */ > static const struct ti_sci_desc ti_sci_pmmc_k2g_desc = { > .default_host_id = 2, > @@ -3639,6 +3682,14 @@ static int ti_sci_probe(struct platform_device *pdev) > } > } > > + ret = ti_sci_init_suspend(pdev, info); > + if (ret) > + dev_warn(dev, > + "ti_sci_init_suspend failed, mem suspend will be non-functional.\n"); > + > + /* Suspend is an optional feature, reset return value and continue. */ > + ret = 0; We end up getting this warning on all platforms with TISCI - even if LPM sequence is capable or not - what does the message mean? firmware is not capable of supporting sleep or it is a firmware capable of supporting, but failed to allocate LPM context memory? If it is optional (since it is probing to see if it has functionality), then do we need a dev_warn - maybe a softer of form? [...]
On 20:13-20221116, Georgi Vlaev wrote: > + /* > + * Attempt to call prepare_sleep, this will be NAK'd if suspend is not > + * supported by firmware in use, in which case we will not attempt to > + * init suspend. > + */ > + ret = ti_sci_cmd_prepare_sleep(&info->handle, 0, > + (u32)(info->ctx_mem_addr & 0xffffffff), > + (u32)((u64)info->ctx_mem_addr >> 32), 0); > + https://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/pm/lpm.html#tisci-msg-prepare-sleep "Prepare the SOC for entering into a low power mode." But we are in the init process here. From the documentation, firmware does'nt seem to guarantee it would do something unexpected (like setup io daisy chain or something like that which normal LP entry state would have to do) - How is it safe to use it as a discovery of capability API?
Hi Nishanth, On 11/21/22 20:44, Nishanth Menon wrote: > On 20:13-20221116, Georgi Vlaev wrote: > [...] > >> +static int ti_sci_init_suspend(struct platform_device *pdev, >> + struct ti_sci_info *info) >> +{ >> + struct device *dev = &pdev->dev; >> + int ret; >> + >> + dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)); >> + info->ctx_mem_buf = dma_alloc_coherent(info->dev, LPM_CTX_MEM_SIZE, >> + &info->ctx_mem_addr, >> + GFP_KERNEL); >> + if (!info->ctx_mem_buf) { >> + dev_err(info->dev, "Failed to allocate LPM context memory\n"); >> + return -ENOMEM; >> + } >> + >> + /* >> + * Attempt to call prepare_sleep, this will be NAK'd if suspend is not >> + * supported by firmware in use, in which case we will not attempt to >> + * init suspend. >> + */ >> + ret = ti_sci_cmd_prepare_sleep(&info->handle, 0, >> + (u32)(info->ctx_mem_addr & 0xffffffff), >> + (u32)((u64)info->ctx_mem_addr >> 32), 0); >> + >> + if (ret) >> + goto err; >> + >> + return 0; >> +err: >> + dma_free_coherent(info->dev, LPM_CTX_MEM_SIZE, >> + info->ctx_mem_buf, >> + info->ctx_mem_addr); >> + return ret; >> +} >> + >> /* Description for K2G */ >> static const struct ti_sci_desc ti_sci_pmmc_k2g_desc = { >> .default_host_id = 2, >> @@ -3639,6 +3682,14 @@ static int ti_sci_probe(struct platform_device *pdev) >> } >> } >> >> + ret = ti_sci_init_suspend(pdev, info); >> + if (ret) >> + dev_warn(dev, >> + "ti_sci_init_suspend failed, mem suspend will be non-functional.\n"); >> + >> + /* Suspend is an optional feature, reset return value and continue. */ >> + ret = 0; > > We end up getting this warning on all platforms with TISCI - even if > LPM sequence is capable or not - what does the message mean? firmware is > not capable of supporting sleep or it is a firmware capable of > supporting, but failed to allocate LPM context memory? > > If it is optional (since it is probing to see if it has functionality), > then do we need a dev_warn - maybe a softer of form? > Yeah, I agree, the message looks confusing. In both cases we can't enter suspend-to-ram, but we consider that an optional feature, so a softer message will be more appropriate. > [...] >
Hi Nishanth, On 11/21/22 20:56, Nishanth Menon wrote: > On 20:13-20221116, Georgi Vlaev wrote: >> + /* >> + * Attempt to call prepare_sleep, this will be NAK'd if suspend is not >> + * supported by firmware in use, in which case we will not attempt to >> + * init suspend. >> + */ >> + ret = ti_sci_cmd_prepare_sleep(&info->handle, 0, >> + (u32)(info->ctx_mem_addr & 0xffffffff), >> + (u32)((u64)info->ctx_mem_addr >> 32), 0); >> + > > https://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/pm/lpm.html#tisci-msg-prepare-sleep > "Prepare the SOC for entering into a low power mode." > > But we are in the init process here. From the documentation, firmware > does'nt seem to guarantee it would do something unexpected (like setup > io daisy chain or something like that which normal LP entry state > would have to do) - How is it safe to use it as a discovery of > capability API? > > Well, you're correct, there's no guarantee. It is safe to call it now on AM62x in both places we actually use it. However, this may change in the future and it's not a good idea to misuse that API. We'll switch the detection part to a more appropriate message, that's better suited for this purpose on all K3 platforms.
Hi, On 11/18/22 14:59, Krzysztof Kozlowski wrote: > On 16/11/2022 19:13, Georgi Vlaev wrote: >> From: Dave Gerlach <d-gerlach@ti.com> >> >> Add documentation for the lpm region which tells the ti-sci driver where >> to load the FS Stub low power mode firmware and also the firmware-name >> which tells the driver which binary to load. Both of these are optional >> for normal system operation but required to enabled suspend-to-mem usage >> of Deep Sleep state. >> > > I think you got here Rob's tag after sending v4. > > Reviewed-by: Rob Herring <robh@kernel.org> > I will pick it up in v5. Thanks. > Best regards, > Krzysztof >
This series introduces necessary ti_sci driver functionality in preparation of supporting DeepSleep mode for suspend to mem on TI K3 AM62x. This version is a fixup and rebase of the patch series by Dave Gerlach [1]. It applies on top of v6.1-rc5. Deep Sleep mode is described in section "5.2.4.4 DeepSleep" of the AM62x Technical Reference Manual [2]. The kernel triggers entry to Deep Sleep mode through the mem suspend transition with the following: * Use a TF-A binary that supports PSCI_SYSTEM_SUSPEND call. This causes system to use PSCI system suspend as last step of mem sleep. * The firmware requires that the OS sends a TISCI_MSG_PREPARE_SLEEP message in order to provide details about suspend, so we must add the ability to send this message. We also add TISCI_MSG_LPM_WAKE_REASON and TISCI_MSG_SET_IO_ISOLATION messages as part of a new PM ops. These messages are part of the TISCI PM Low Power Mode API [3]. (Patch 2) * A memory address must be provided to the firmware using the above message, which is allocated and managed by dma_alloc_coherent() and friends. (Patch 3) * System must load firmware to a specific location before Deep Sleep is entered, and this is accomplished using a memory region in device tree to indicate where this firmware should be loaded, and also a "firmware-name" property to indicate the name of the firmware to load. The ti_sci driver checks in its pm handler to see if the firmware has been loaded and if not, loads it. (Patch 4) * Finally, the ti_sci driver must actually send TISCI_MSG_PREPARE_SLEEP message to firmware with the above information included, which it does during the driver suspend handler when PM_MEM_SUSPEND is the determined state being entered. (Patch 5) This is tested on am625-sk using a limited dts with all devices disabled apart from cpu0, main_uart0, i2c, rtc, mmc/sd, dmsc, and secure_proxy_main. Testing this sequence requires K3 sdhci suspend/resume support [4], enable the wkup_rtc in the am625-sk.dts, disable devices that don't support system suspend/resume like OSPI and CPSW3G. In can be tested on the following branch: https://github.com/gvlaev/linux/tree/upstream-v6.2/lpm-ti-sci-v2 Changelog: v4: - Fix checkpacth warnings in patches 2 and 3. - Drop the links with anchors in patch 2. v3: - Fix the compile warnings on 32-bit platforms reported by the kernel test robot in patches (3,5). - Pick up Roger's "Tested-by" tags. v2: - Addressed comments received for v1 series [1]. - Updated v1 patch 5 to use pm notifier to avoid firmware loading issues. - Dropped the reserved region requirement and allocate DMA memory instead. The reserved region binding patch is also removed. - Introduce two more TISCI LPM messages that are supported in SysFW. - Fixes in error handling. [1] https://lore.kernel.org/lkml/20220421203659.27853-1-d-gerlach@ti.com [2] https://www.ti.com/lit/pdf/spruiv7 [3] https://software-dl.ti.com/tisci/esd/latest/2_tisci_msgs/pm/lpm.html [4] https://lore.kernel.org/lkml/20220408124338.27090-1-a-govindraju@ti.com Georgi Vlaev (5): dt-bindings: ti, sci: Add lpm region and firmware-name firmware: ti_sci: Introduce Power Management Ops firmware: ti_sci: Allocate memory for the LPM modes firmware: ti_sci: Use dt provided fw name and address to load at suspend time firmware: ti_sci: Introduce prepare system suspend call .../bindings/arm/keystone/ti,sci.yaml | 21 +- drivers/firmware/ti_sci.c | 357 ++++++++++++++++++ drivers/firmware/ti_sci.h | 64 +++- include/linux/soc/ti/ti_sci_protocol.h | 44 +++ 4 files changed, 481 insertions(+), 5 deletions(-) base-commit: 094226ad94f471a9f19e8f8e7140a09c2625abaa