[PATCH 0/6] firmware: ti_sci: Introduce system suspend support

[PATCH 0/6] firmware: ti_sci: Introduce system suspend support

[Dave Gerlach]

Hi,

This series introduces necessary ti_sci driver functionality in
preparation of supporting DeepSleep mode for suspend to mem on TI K3
AM62 [1]. This series applies on top of [2], which is also required for
proper system suspend operation.

Deep Sleep mode is described in section "5.2.4.4 DeepSleep" of the AM62
Technical Reference Manual: https://www.ti.com/lit/pdf/spruiv7

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, which is
  enabled by the pending patches here [3]. 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. (Patch 3)

* A memory carveout address must be provided to the firmware using
  the above message, which is passed through device tree to the
  driver. (Patches 1 and 4)

* System must load firmware to a specific location before Deep Sleep is
  entered, and this is accomplished using an lpm memory region in device
  tree to indicate where this firmware should be loaded, and also a
  "ti,lpm-firmware-name" property to indicate the name of the firmware
  to load. The ti_sci driver checks in its suspend handler to see if
  the firmware has been loaded and if not, loads it. (Patches 2 and 5)

* 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 6)

This is tested on am625-sk using a ramdisk with all devices disabled
apart from cpu0, main_uart0, dmsc, and secure_proxy_main.

Regards,
Dave

[1] https://lore.kernel.org/lkml/20220225120239.1303821-1-vigneshr@ti.com/
[2] https://lore.kernel.org/lkml/20220412192138.31189-1-d-gerlach@ti.com/
[3] https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/14714/3

Dave Gerlach (6):
  dt-bindings: ti,sci: Add ti,ctx-memory-region property
  dt-bindings: ti,sci: Add lpm region and ti,lpm-firmware-name
  firmware: ti_sci: Introduce Power Management Ops
  firmware: ti_sci: Introduce ti,ctx-memory-region for reserved LPM
    memory
  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         |  30 ++-
 drivers/firmware/ti_sci.c                     | 207 +++++++++++++++++-
 drivers/firmware/ti_sci.h                     |  32 ++-
 include/linux/soc/ti/ti_sci_protocol.h        |   6 +
 4 files changed, 269 insertions(+), 6 deletions(-)

-- 
2.35.0

[PATCH 0/6] firmware: ti_sci: Introduce system suspend support

[Dave Gerlach]

Hi,

This series introduces necessary ti_sci driver functionality in
preparation of supporting DeepSleep mode for suspend to mem on TI K3
AM62 [1]. This series applies on top of [2], which is also required for
proper system suspend operation.

Deep Sleep mode is described in section "5.2.4.4 DeepSleep" of the AM62
Technical Reference Manual: https://www.ti.com/lit/pdf/spruiv7

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, which is
  enabled by the pending patches here [3]. 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. (Patch 3)

* A memory carveout address must be provided to the firmware using
  the above message, which is passed through device tree to the
  driver. (Patches 1 and 4)

* System must load firmware to a specific location before Deep Sleep is
  entered, and this is accomplished using an lpm memory region in device
  tree to indicate where this firmware should be loaded, and also a
  "ti,lpm-firmware-name" property to indicate the name of the firmware
  to load. The ti_sci driver checks in its suspend handler to see if
  the firmware has been loaded and if not, loads it. (Patches 2 and 5)

* 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 6)

This is tested on am625-sk using a ramdisk with all devices disabled
apart from cpu0, main_uart0, dmsc, and secure_proxy_main.

Regards,
Dave

[1] https://lore.kernel.org/lkml/20220225120239.1303821-1-vigneshr@ti.com/
[2] https://lore.kernel.org/lkml/20220412192138.31189-1-d-gerlach@ti.com/
[3] https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/14714/3

Dave Gerlach (6):
  dt-bindings: ti,sci: Add ti,ctx-memory-region property
  dt-bindings: ti,sci: Add lpm region and ti,lpm-firmware-name
  firmware: ti_sci: Introduce Power Management Ops
  firmware: ti_sci: Introduce ti,ctx-memory-region for reserved LPM
    memory
  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         |  30 ++-
 drivers/firmware/ti_sci.c                     | 207 +++++++++++++++++-
 drivers/firmware/ti_sci.h                     |  32 ++-
 include/linux/soc/ti/ti_sci_protocol.h        |   6 +
 4 files changed, 269 insertions(+), 6 deletions(-)

-- 
2.35.0


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

[PATCH 1/6] dt-bindings: ti,sci: Add ti,ctx-memory-region property

[Dave Gerlach]

Add documentation for the ti,ctx-memory-region property which is a
phandle to a reserved-memory carveout to be used by the ti_sci driver
storage of low power mode memory context. This is optional for normal
system operation but required to enabled suspend-to-mem usage of Deep
Sleep state.

Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
---
 .../devicetree/bindings/arm/keystone/ti,sci.yaml         | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
index 34f5f877d444..ec88aa88a2a0 100644
--- a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
+++ b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
@@ -61,6 +61,15 @@ properties:
   mboxes:
     minItems: 2
 
+  ti,ctx-memory-region:
+    description:
+      Phandle to the reserved memory node to be associated with the
+      ti-sci device, to be used for saving low power context. The
+      reserved memory node should be a carveout node, and should
+      be defined as per the bindings in
+      Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
+    $ref: /schemas/types.yaml#/definitions/string
+
   ti,system-reboot-controller:
     description: Determines If system reboot can be triggered by SoC reboot
     type: boolean
-- 
2.35.0

[PATCH 1/6] dt-bindings: ti,sci: Add ti,ctx-memory-region property

[Dave Gerlach]

Add documentation for the ti,ctx-memory-region property which is a
phandle to a reserved-memory carveout to be used by the ti_sci driver
storage of low power mode memory context. This is optional for normal
system operation but required to enabled suspend-to-mem usage of Deep
Sleep state.

Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
---
 .../devicetree/bindings/arm/keystone/ti,sci.yaml         | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
index 34f5f877d444..ec88aa88a2a0 100644
--- a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
+++ b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
@@ -61,6 +61,15 @@ properties:
   mboxes:
     minItems: 2
 
+  ti,ctx-memory-region:
+    description:
+      Phandle to the reserved memory node to be associated with the
+      ti-sci device, to be used for saving low power context. The
+      reserved memory node should be a carveout node, and should
+      be defined as per the bindings in
+      Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
+    $ref: /schemas/types.yaml#/definitions/string
+
   ti,system-reboot-controller:
     description: Determines If system reboot can be triggered by SoC reboot
     type: boolean
-- 
2.35.0


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

Re: [PATCH 1/6] dt-bindings: ti,sci: Add ti,ctx-memory-region property

[Andrew Davis]

On 4/21/22 3:36 PM, Dave Gerlach wrote:
> Add documentation for the ti,ctx-memory-region property which is a
> phandle to a reserved-memory carveout to be used by the ti_sci driver
> storage of low power mode memory context. This is optional for normal
> system operation but required to enabled suspend-to-mem usage of Deep
> Sleep state.
> 
> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
> ---
>   .../devicetree/bindings/arm/keystone/ti,sci.yaml         | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> index 34f5f877d444..ec88aa88a2a0 100644
> --- a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> +++ b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> @@ -61,6 +61,15 @@ properties:
>     mboxes:
>       minItems: 2
>   
> +  ti,ctx-memory-region:
> +    description:
> +      Phandle to the reserved memory node to be associated with the
> +      ti-sci device, to be used for saving low power context. The
> +      reserved memory node should be a carveout node, and should
> +      be defined as per the bindings in
> +      Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
> +    $ref: /schemas/types.yaml#/definitions/string
> +


Why does this have to be yet another reserved carveout region,
should be dynamically allocated.

Andrew


>     ti,system-reboot-controller:
>       description: Determines If system reboot can be triggered by SoC reboot
>       type: boolean

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

Re: [PATCH 1/6] dt-bindings: ti,sci: Add ti,ctx-memory-region property

[Andrew Davis]

On 4/21/22 3:36 PM, Dave Gerlach wrote:
> Add documentation for the ti,ctx-memory-region property which is a
> phandle to a reserved-memory carveout to be used by the ti_sci driver
> storage of low power mode memory context. This is optional for normal
> system operation but required to enabled suspend-to-mem usage of Deep
> Sleep state.
> 
> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
> ---
>   .../devicetree/bindings/arm/keystone/ti,sci.yaml         | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> index 34f5f877d444..ec88aa88a2a0 100644
> --- a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> +++ b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> @@ -61,6 +61,15 @@ properties:
>     mboxes:
>       minItems: 2
>   
> +  ti,ctx-memory-region:
> +    description:
> +      Phandle to the reserved memory node to be associated with the
> +      ti-sci device, to be used for saving low power context. The
> +      reserved memory node should be a carveout node, and should
> +      be defined as per the bindings in
> +      Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
> +    $ref: /schemas/types.yaml#/definitions/string
> +


Why does this have to be yet another reserved carveout region,
should be dynamically allocated.

Andrew


>     ti,system-reboot-controller:
>       description: Determines If system reboot can be triggered by SoC reboot
>       type: boolean

Re: [PATCH 1/6] dt-bindings: ti,sci: Add ti,ctx-memory-region property

[Andrew Davis]

Re: [PATCH 1/6] dt-bindings: ti,sci: Add ti,ctx-memory-region property

[Dave Gerlach]

Hi,

On 4/22/22 14:02, Andrew Davis wrote:
> On 4/21/22 3:36 PM, Dave Gerlach wrote:
>> Add documentation for the ti,ctx-memory-region property which is a
>> phandle to a reserved-memory carveout to be used by the ti_sci driver
>> storage of low power mode memory context. This is optional for normal
>> system operation but required to enabled suspend-to-mem usage of Deep
>> Sleep state.
>>
>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
>> ---
>>   .../devicetree/bindings/arm/keystone/ti,sci.yaml         | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
>> index 34f5f877d444..ec88aa88a2a0 100644
>> --- a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
>> +++ b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
>> @@ -61,6 +61,15 @@ properties:
>>     mboxes:
>>       minItems: 2
>>   
>> +  ti,ctx-memory-region:
>> +    description:
>> +      Phandle to the reserved memory node to be associated with the
>> +      ti-sci device, to be used for saving low power context. The
>> +      reserved memory node should be a carveout node, and should
>> +      be defined as per the bindings in
>> +      Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
>> +    $ref: /schemas/types.yaml#/definitions/string
>> +
> 
> 
> Why does this have to be yet another reserved carveout region,
> should be dynamically allocated.
> 

This must be a fixed address in order to support other low power modes
which have not yet been introduced.

Regards,
Dave

> Andrew
> 
> 
>>     ti,system-reboot-controller:
>>       description: Determines If system reboot can be triggered by SoC reboot
>>       type: boolean

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

Re: [PATCH 1/6] dt-bindings: ti,sci: Add ti,ctx-memory-region property

[Dave Gerlach]

Hi,

On 4/22/22 14:02, Andrew Davis wrote:
> On 4/21/22 3:36 PM, Dave Gerlach wrote:
>> Add documentation for the ti,ctx-memory-region property which is a
>> phandle to a reserved-memory carveout to be used by the ti_sci driver
>> storage of low power mode memory context. This is optional for normal
>> system operation but required to enabled suspend-to-mem usage of Deep
>> Sleep state.
>>
>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
>> ---
>>   .../devicetree/bindings/arm/keystone/ti,sci.yaml         | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
>> index 34f5f877d444..ec88aa88a2a0 100644
>> --- a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
>> +++ b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
>> @@ -61,6 +61,15 @@ properties:
>>     mboxes:
>>       minItems: 2
>>   
>> +  ti,ctx-memory-region:
>> +    description:
>> +      Phandle to the reserved memory node to be associated with the
>> +      ti-sci device, to be used for saving low power context. The
>> +      reserved memory node should be a carveout node, and should
>> +      be defined as per the bindings in
>> +      Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
>> +    $ref: /schemas/types.yaml#/definitions/string
>> +
> 
> 
> Why does this have to be yet another reserved carveout region,
> should be dynamically allocated.
> 

This must be a fixed address in order to support other low power modes
which have not yet been introduced.

Regards,
Dave

> Andrew
> 
> 
>>     ti,system-reboot-controller:
>>       description: Determines If system reboot can be triggered by SoC reboot
>>       type: boolean

Re: [PATCH 1/6] dt-bindings: ti,sci: Add ti,ctx-memory-region property

[Nishanth Menon]

On 14:10-20220422, Dave Gerlach wrote:
> Hi,
> 
> On 4/22/22 14:02, Andrew Davis wrote:
> > On 4/21/22 3:36 PM, Dave Gerlach wrote:
> >> Add documentation for the ti,ctx-memory-region property which is a
> >> phandle to a reserved-memory carveout to be used by the ti_sci driver
> >> storage of low power mode memory context. This is optional for normal
> >> system operation but required to enabled suspend-to-mem usage of Deep
> >> Sleep state.
> >>
> >> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
> >> ---
> >>   .../devicetree/bindings/arm/keystone/ti,sci.yaml         | 9 +++++++++
> >>   1 file changed, 9 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> >> index 34f5f877d444..ec88aa88a2a0 100644
> >> --- a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> >> +++ b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> >> @@ -61,6 +61,15 @@ properties:
> >>     mboxes:
> >>       minItems: 2
> >>   
> >> +  ti,ctx-memory-region:
> >> +    description:
> >> +      Phandle to the reserved memory node to be associated with the
> >> +      ti-sci device, to be used for saving low power context. The
> >> +      reserved memory node should be a carveout node, and should
> >> +      be defined as per the bindings in
> >> +      Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
> >> +    $ref: /schemas/types.yaml#/definitions/string
> >> +
> > 
> > 
> > Why does this have to be yet another reserved carveout region,
> > should be dynamically allocated.
> > 
> 
> This must be a fixed address in order to support other low power modes
> which have not yet been introduced.

Please elaborate the need - Many of our devices, esp the AM62 class ones
are memory constrained devices - LPM states are controlled entry states, why
should we loose a chunk of DDR in operational state while waiting for
the suspend or idle state to be invoked?
OR, is the argument is as follows:
- need a guarenteed memory for me to enter low power and not be
  dependent on availability on attempt.
- Latency overhead of allocation during a "hot path" such as cpu idle,
  this is completely unacceptable?

  or something of that form.. please elaborate?
-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Re: [PATCH 1/6] dt-bindings: ti,sci: Add ti,ctx-memory-region property

[Nishanth Menon]

On 14:10-20220422, Dave Gerlach wrote:
> Hi,
> 
> On 4/22/22 14:02, Andrew Davis wrote:
> > On 4/21/22 3:36 PM, Dave Gerlach wrote:
> >> Add documentation for the ti,ctx-memory-region property which is a
> >> phandle to a reserved-memory carveout to be used by the ti_sci driver
> >> storage of low power mode memory context. This is optional for normal
> >> system operation but required to enabled suspend-to-mem usage of Deep
> >> Sleep state.
> >>
> >> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
> >> ---
> >>   .../devicetree/bindings/arm/keystone/ti,sci.yaml         | 9 +++++++++
> >>   1 file changed, 9 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> >> index 34f5f877d444..ec88aa88a2a0 100644
> >> --- a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> >> +++ b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> >> @@ -61,6 +61,15 @@ properties:
> >>     mboxes:
> >>       minItems: 2
> >>   
> >> +  ti,ctx-memory-region:
> >> +    description:
> >> +      Phandle to the reserved memory node to be associated with the
> >> +      ti-sci device, to be used for saving low power context. The
> >> +      reserved memory node should be a carveout node, and should
> >> +      be defined as per the bindings in
> >> +      Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
> >> +    $ref: /schemas/types.yaml#/definitions/string
> >> +
> > 
> > 
> > Why does this have to be yet another reserved carveout region,
> > should be dynamically allocated.
> > 
> 
> This must be a fixed address in order to support other low power modes
> which have not yet been introduced.

Please elaborate the need - Many of our devices, esp the AM62 class ones
are memory constrained devices - LPM states are controlled entry states, why
should we loose a chunk of DDR in operational state while waiting for
the suspend or idle state to be invoked?
OR, is the argument is as follows:
- need a guarenteed memory for me to enter low power and not be
  dependent on availability on attempt.
- Latency overhead of allocation during a "hot path" such as cpu idle,
  this is completely unacceptable?

  or something of that form.. please elaborate?
-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

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

Re: [PATCH 1/6] dt-bindings: ti,sci: Add ti,ctx-memory-region property

[Dave Gerlach]

Hi,

On 4/23/22 08:36, Nishanth Menon wrote:
> On 14:10-20220422, Dave Gerlach wrote:
>> Hi,
>>
>> On 4/22/22 14:02, Andrew Davis wrote:
>>> On 4/21/22 3:36 PM, Dave Gerlach wrote:
>>>> Add documentation for the ti,ctx-memory-region property which is a
>>>> phandle to a reserved-memory carveout to be used by the ti_sci driver
>>>> storage of low power mode memory context. This is optional for normal
>>>> system operation but required to enabled suspend-to-mem usage of Deep
>>>> Sleep state.
>>>>
>>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
>>>> ---
>>>>   .../devicetree/bindings/arm/keystone/ti,sci.yaml         | 9 +++++++++
>>>>   1 file changed, 9 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
>>>> index 34f5f877d444..ec88aa88a2a0 100644
>>>> --- a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
>>>> +++ b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
>>>> @@ -61,6 +61,15 @@ properties:
>>>>     mboxes:
>>>>       minItems: 2
>>>>   
>>>> +  ti,ctx-memory-region:
>>>> +    description:
>>>> +      Phandle to the reserved memory node to be associated with the
>>>> +      ti-sci device, to be used for saving low power context. The
>>>> +      reserved memory node should be a carveout node, and should
>>>> +      be defined as per the bindings in
>>>> +      Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
>>>> +    $ref: /schemas/types.yaml#/definitions/string
>>>> +
>>>
>>>
>>> Why does this have to be yet another reserved carveout region,
>>> should be dynamically allocated.
>>>
>>
>> This must be a fixed address in order to support other low power modes
>> which have not yet been introduced.
> 
> Please elaborate the need - Many of our devices, esp the AM62 class ones
> are memory constrained devices - LPM states are controlled entry states, why
> should we loose a chunk of DDR in operational state while waiting for
> the suspend or idle state to be invoked?
> OR, is the argument is as follows:
> - need a guarenteed memory for me to enter low power and not be
>   dependent on availability on attempt.
> - Latency overhead of allocation during a "hot path" such as cpu idle,
>   this is completely unacceptable?
> 
>   or something of that form.. please elaborate?

Yes, in the case of some future low power modes, it is possible for the
SoC to completely lose context. For the mode being introduced here, I
agree that we could dynamically allocate this memory and then share that
address around, but keeping it fixed of course works here, and *also*
works for the complete context loss case, as a different mechanism would
be restoring this context and can easily get the fixed address straight
from the DT. Otherwise we would have two completely divergent paths
because there is no way to share some dynamic address across the transition.

To me it makes sense to keep it the same for all modes when possible.

Regards,
Dave

Re: [PATCH 1/6] dt-bindings: ti,sci: Add ti,ctx-memory-region property

[Dave Gerlach]

Hi,

On 4/23/22 08:36, Nishanth Menon wrote:
> On 14:10-20220422, Dave Gerlach wrote:
>> Hi,
>>
>> On 4/22/22 14:02, Andrew Davis wrote:
>>> On 4/21/22 3:36 PM, Dave Gerlach wrote:
>>>> Add documentation for the ti,ctx-memory-region property which is a
>>>> phandle to a reserved-memory carveout to be used by the ti_sci driver
>>>> storage of low power mode memory context. This is optional for normal
>>>> system operation but required to enabled suspend-to-mem usage of Deep
>>>> Sleep state.
>>>>
>>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
>>>> ---
>>>>   .../devicetree/bindings/arm/keystone/ti,sci.yaml         | 9 +++++++++
>>>>   1 file changed, 9 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
>>>> index 34f5f877d444..ec88aa88a2a0 100644
>>>> --- a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
>>>> +++ b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
>>>> @@ -61,6 +61,15 @@ properties:
>>>>     mboxes:
>>>>       minItems: 2
>>>>   
>>>> +  ti,ctx-memory-region:
>>>> +    description:
>>>> +      Phandle to the reserved memory node to be associated with the
>>>> +      ti-sci device, to be used for saving low power context. The
>>>> +      reserved memory node should be a carveout node, and should
>>>> +      be defined as per the bindings in
>>>> +      Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
>>>> +    $ref: /schemas/types.yaml#/definitions/string
>>>> +
>>>
>>>
>>> Why does this have to be yet another reserved carveout region,
>>> should be dynamically allocated.
>>>
>>
>> This must be a fixed address in order to support other low power modes
>> which have not yet been introduced.
> 
> Please elaborate the need - Many of our devices, esp the AM62 class ones
> are memory constrained devices - LPM states are controlled entry states, why
> should we loose a chunk of DDR in operational state while waiting for
> the suspend or idle state to be invoked?
> OR, is the argument is as follows:
> - need a guarenteed memory for me to enter low power and not be
>   dependent on availability on attempt.
> - Latency overhead of allocation during a "hot path" such as cpu idle,
>   this is completely unacceptable?
> 
>   or something of that form.. please elaborate?

Yes, in the case of some future low power modes, it is possible for the
SoC to completely lose context. For the mode being introduced here, I
agree that we could dynamically allocate this memory and then share that
address around, but keeping it fixed of course works here, and *also*
works for the complete context loss case, as a different mechanism would
be restoring this context and can easily get the fixed address straight
from the DT. Otherwise we would have two completely divergent paths
because there is no way to share some dynamic address across the transition.

To me it makes sense to keep it the same for all modes when possible.

Regards,
Dave

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

Re: [PATCH 1/6] dt-bindings: ti,sci: Add ti,ctx-memory-region property

[Nishanth Menon]

On 15:24-20220425, Dave Gerlach wrote:
> Hi,
> 
> On 4/23/22 08:36, Nishanth Menon wrote:
> > On 14:10-20220422, Dave Gerlach wrote:
> >> Hi,
> >>
> >> On 4/22/22 14:02, Andrew Davis wrote:
> >>> On 4/21/22 3:36 PM, Dave Gerlach wrote:
> >>>> Add documentation for the ti,ctx-memory-region property which is a
> >>>> phandle to a reserved-memory carveout to be used by the ti_sci driver
> >>>> storage of low power mode memory context. This is optional for normal
> >>>> system operation but required to enabled suspend-to-mem usage of Deep
> >>>> Sleep state.
> >>>>
> >>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
> >>>> ---
> >>>>   .../devicetree/bindings/arm/keystone/ti,sci.yaml         | 9 +++++++++
> >>>>   1 file changed, 9 insertions(+)
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> >>>> index 34f5f877d444..ec88aa88a2a0 100644
> >>>> --- a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> >>>> +++ b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> >>>> @@ -61,6 +61,15 @@ properties:
> >>>>     mboxes:
> >>>>       minItems: 2
> >>>>   
> >>>> +  ti,ctx-memory-region:

Just memory-region?

> >>>> +    description:
> >>>> +      Phandle to the reserved memory node to be associated with the
> >>>> +      ti-sci device, to be used for saving low power context. The
> >>>> +      reserved memory node should be a carveout node, and should
> >>>> +      be defined as per the bindings in
> >>>> +      Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
> >>>> +    $ref: /schemas/types.yaml#/definitions/string

$ref: /schemas/types.yaml#/definitions/phandle ?
maxItems: 1

> >>>> +
> >>>
> >>>
> >>> Why does this have to be yet another reserved carveout region,
> >>> should be dynamically allocated.
> >>>
> >>
> >> This must be a fixed address in order to support other low power modes
> >> which have not yet been introduced.
> > 
> > Please elaborate the need - Many of our devices, esp the AM62 class ones
> > are memory constrained devices - LPM states are controlled entry states, why
> > should we loose a chunk of DDR in operational state while waiting for
> > the suspend or idle state to be invoked?
> > OR, is the argument is as follows:
> > - need a guarenteed memory for me to enter low power and not be
> >   dependent on availability on attempt.
> > - Latency overhead of allocation during a "hot path" such as cpu idle,
> >   this is completely unacceptable?
> > 
> >   or something of that form.. please elaborate?
> 
> Yes, in the case of some future low power modes, it is possible for the
> SoC to completely lose context. For the mode being introduced here, I
> agree that we could dynamically allocate this memory and then share that
> address around, but keeping it fixed of course works here, and *also*
> works for the complete context loss case, as a different mechanism would
> be restoring this context and can easily get the fixed address straight
> from the DT. Otherwise we would have two completely divergent paths
> because there is no way to share some dynamic address across the transition.
> 
> To me it makes sense to keep it the same for all modes when possible.

a) Clearly this does'nt apply to all SoCs supporting ti,sci. Can we make it
   more stringent?
b) This is a hardware description of a memory region that is carvedout
   for context information and maybe used as part of various LPM modes
   where restoration needs to occur prior to OS startup (and dynamic
   handshake can occur with the entity that manages power controls).

I think this should be clearly articulated in commit message and
description to help understand the rationale.

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Re: [PATCH 1/6] dt-bindings: ti,sci: Add ti,ctx-memory-region property

[Nishanth Menon]

On 15:24-20220425, Dave Gerlach wrote:
> Hi,
> 
> On 4/23/22 08:36, Nishanth Menon wrote:
> > On 14:10-20220422, Dave Gerlach wrote:
> >> Hi,
> >>
> >> On 4/22/22 14:02, Andrew Davis wrote:
> >>> On 4/21/22 3:36 PM, Dave Gerlach wrote:
> >>>> Add documentation for the ti,ctx-memory-region property which is a
> >>>> phandle to a reserved-memory carveout to be used by the ti_sci driver
> >>>> storage of low power mode memory context. This is optional for normal
> >>>> system operation but required to enabled suspend-to-mem usage of Deep
> >>>> Sleep state.
> >>>>
> >>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
> >>>> ---
> >>>>   .../devicetree/bindings/arm/keystone/ti,sci.yaml         | 9 +++++++++
> >>>>   1 file changed, 9 insertions(+)
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> >>>> index 34f5f877d444..ec88aa88a2a0 100644
> >>>> --- a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> >>>> +++ b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> >>>> @@ -61,6 +61,15 @@ properties:
> >>>>     mboxes:
> >>>>       minItems: 2
> >>>>   
> >>>> +  ti,ctx-memory-region:

Just memory-region?

> >>>> +    description:
> >>>> +      Phandle to the reserved memory node to be associated with the
> >>>> +      ti-sci device, to be used for saving low power context. The
> >>>> +      reserved memory node should be a carveout node, and should
> >>>> +      be defined as per the bindings in
> >>>> +      Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
> >>>> +    $ref: /schemas/types.yaml#/definitions/string

$ref: /schemas/types.yaml#/definitions/phandle ?
maxItems: 1

> >>>> +
> >>>
> >>>
> >>> Why does this have to be yet another reserved carveout region,
> >>> should be dynamically allocated.
> >>>
> >>
> >> This must be a fixed address in order to support other low power modes
> >> which have not yet been introduced.
> > 
> > Please elaborate the need - Many of our devices, esp the AM62 class ones
> > are memory constrained devices - LPM states are controlled entry states, why
> > should we loose a chunk of DDR in operational state while waiting for
> > the suspend or idle state to be invoked?
> > OR, is the argument is as follows:
> > - need a guarenteed memory for me to enter low power and not be
> >   dependent on availability on attempt.
> > - Latency overhead of allocation during a "hot path" such as cpu idle,
> >   this is completely unacceptable?
> > 
> >   or something of that form.. please elaborate?
> 
> Yes, in the case of some future low power modes, it is possible for the
> SoC to completely lose context. For the mode being introduced here, I
> agree that we could dynamically allocate this memory and then share that
> address around, but keeping it fixed of course works here, and *also*
> works for the complete context loss case, as a different mechanism would
> be restoring this context and can easily get the fixed address straight
> from the DT. Otherwise we would have two completely divergent paths
> because there is no way to share some dynamic address across the transition.
> 
> To me it makes sense to keep it the same for all modes when possible.

a) Clearly this does'nt apply to all SoCs supporting ti,sci. Can we make it
   more stringent?
b) This is a hardware description of a memory region that is carvedout
   for context information and maybe used as part of various LPM modes
   where restoration needs to occur prior to OS startup (and dynamic
   handshake can occur with the entity that manages power controls).

I think this should be clearly articulated in commit message and
description to help understand the rationale.

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

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

Re: [PATCH 1/6] dt-bindings: ti,sci: Add ti,ctx-memory-region property

[Rob Herring]

On Thu, Apr 21, 2022 at 03:36:54PM -0500, Dave Gerlach wrote:
> Add documentation for the ti,ctx-memory-region property which is a
> phandle to a reserved-memory carveout to be used by the ti_sci driver
> storage of low power mode memory context. This is optional for normal
> system operation but required to enabled suspend-to-mem usage of Deep
> Sleep state.
> 
> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
> ---
>  .../devicetree/bindings/arm/keystone/ti,sci.yaml         | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> index 34f5f877d444..ec88aa88a2a0 100644
> --- a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> +++ b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> @@ -61,6 +61,15 @@ properties:
>    mboxes:
>      minItems: 2
>  
> +  ti,ctx-memory-region:

What's wrong with 'memory-region'?

> +    description:
> +      Phandle to the reserved memory node to be associated with the
> +      ti-sci device, to be used for saving low power context. The
> +      reserved memory node should be a carveout node, and should
> +      be defined as per the bindings in
> +      Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
> +    $ref: /schemas/types.yaml#/definitions/string
> +
>    ti,system-reboot-controller:
>      description: Determines If system reboot can be triggered by SoC reboot
>      type: boolean
> -- 
> 2.35.0
> 
> 

Re: [PATCH 1/6] dt-bindings: ti,sci: Add ti,ctx-memory-region property

[Rob Herring]

On Thu, Apr 21, 2022 at 03:36:54PM -0500, Dave Gerlach wrote:
> Add documentation for the ti,ctx-memory-region property which is a
> phandle to a reserved-memory carveout to be used by the ti_sci driver
> storage of low power mode memory context. This is optional for normal
> system operation but required to enabled suspend-to-mem usage of Deep
> Sleep state.
> 
> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
> ---
>  .../devicetree/bindings/arm/keystone/ti,sci.yaml         | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> index 34f5f877d444..ec88aa88a2a0 100644
> --- a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> +++ b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> @@ -61,6 +61,15 @@ properties:
>    mboxes:
>      minItems: 2
>  
> +  ti,ctx-memory-region:

What's wrong with 'memory-region'?

> +    description:
> +      Phandle to the reserved memory node to be associated with the
> +      ti-sci device, to be used for saving low power context. The
> +      reserved memory node should be a carveout node, and should
> +      be defined as per the bindings in
> +      Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml
> +    $ref: /schemas/types.yaml#/definitions/string
> +
>    ti,system-reboot-controller:
>      description: Determines If system reboot can be triggered by SoC reboot
>      type: boolean
> -- 
> 2.35.0
> 
> 

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

[PATCH 2/6] dt-bindings: ti,sci: Add lpm region and ti,lpm-firmware-name

[Dave Gerlach]

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
ti,lpm-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.

Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
---
 .../bindings/arm/keystone/ti,sci.yaml         | 21 +++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
index ec88aa88a2a0..31c783507cd0 100644
--- a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
+++ b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
@@ -42,12 +42,19 @@ properties:
           - const: ti,am654-sci
 
   reg-names:
-    description: |
-      Specifies the debug messages memory mapped region that is optionally
-      made available from TI-SCI controller.
-    const: debug_messages
+    items:
+      - const: debug_messages
+      - const: lpm
+    minItems: 1
 
   reg:
+    items:
+      - description: |
+          Specifies the debug messages memory mapped region that is optionally
+          made available from TI-SCI controller.
+      - description: |
+          Specifies the lpm memory mapped region where the FS Stub low power
+          firmware is to be loaded.
     minItems: 1
 
   mbox-names:
@@ -92,6 +99,12 @@ properties:
     type: object
     $ref: /schemas/reset/ti,sci-reset.yaml#
 
+  ti,lpm-firmware-name:
+    description: |
+      Name of binary of FS Stub low power firmware located on the
+      firmware search path.
+    $ref: /schemas/types.yaml#/definitions/string
+
 required:
   - compatible
   - mbox-names
-- 
2.35.0

[PATCH 2/6] dt-bindings: ti, sci: Add lpm region and ti, lpm-firmware-name

[Dave Gerlach]

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
ti,lpm-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.

Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
---
 .../bindings/arm/keystone/ti,sci.yaml         | 21 +++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
index ec88aa88a2a0..31c783507cd0 100644
--- a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
+++ b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
@@ -42,12 +42,19 @@ properties:
           - const: ti,am654-sci
 
   reg-names:
-    description: |
-      Specifies the debug messages memory mapped region that is optionally
-      made available from TI-SCI controller.
-    const: debug_messages
+    items:
+      - const: debug_messages
+      - const: lpm
+    minItems: 1
 
   reg:
+    items:
+      - description: |
+          Specifies the debug messages memory mapped region that is optionally
+          made available from TI-SCI controller.
+      - description: |
+          Specifies the lpm memory mapped region where the FS Stub low power
+          firmware is to be loaded.
     minItems: 1
 
   mbox-names:
@@ -92,6 +99,12 @@ properties:
     type: object
     $ref: /schemas/reset/ti,sci-reset.yaml#
 
+  ti,lpm-firmware-name:
+    description: |
+      Name of binary of FS Stub low power firmware located on the
+      firmware search path.
+    $ref: /schemas/types.yaml#/definitions/string
+
 required:
   - compatible
   - mbox-names
-- 
2.35.0


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

Re: [PATCH 2/6] dt-bindings: ti,sci: Add lpm region and ti,lpm-firmware-name

[Rob Herring]

On Thu, Apr 21, 2022 at 03:36:55PM -0500, Dave Gerlach wrote:
> 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
> ti,lpm-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.
> 
> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
> ---
>  .../bindings/arm/keystone/ti,sci.yaml         | 21 +++++++++++++++----
>  1 file changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> index ec88aa88a2a0..31c783507cd0 100644
> --- a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> +++ b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> @@ -42,12 +42,19 @@ properties:
>            - const: ti,am654-sci
>  
>    reg-names:
> -    description: |
> -      Specifies the debug messages memory mapped region that is optionally
> -      made available from TI-SCI controller.
> -    const: debug_messages
> +    items:
> +      - const: debug_messages
> +      - const: lpm
> +    minItems: 1
>  
>    reg:
> +    items:
> +      - description: |
> +          Specifies the debug messages memory mapped region that is optionally
> +          made available from TI-SCI controller.
> +      - description: |
> +          Specifies the lpm memory mapped region where the FS Stub low power
> +          firmware is to be loaded.
>      minItems: 1
>  
>    mbox-names:
> @@ -92,6 +99,12 @@ properties:
>      type: object
>      $ref: /schemas/reset/ti,sci-reset.yaml#
>  
> +  ti,lpm-firmware-name:

firmware-name

> +    description: |
> +      Name of binary of FS Stub low power firmware located on the
> +      firmware search path.
> +    $ref: /schemas/types.yaml#/definitions/string
> +
>  required:
>    - compatible
>    - mbox-names
> -- 
> 2.35.0
> 
> 

Re: [PATCH 2/6] dt-bindings: ti,sci: Add lpm region and ti,lpm-firmware-name

[Rob Herring]

On Thu, Apr 21, 2022 at 03:36:55PM -0500, Dave Gerlach wrote:
> 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
> ti,lpm-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.
> 
> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
> ---
>  .../bindings/arm/keystone/ti,sci.yaml         | 21 +++++++++++++++----
>  1 file changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> index ec88aa88a2a0..31c783507cd0 100644
> --- a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> +++ b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> @@ -42,12 +42,19 @@ properties:
>            - const: ti,am654-sci
>  
>    reg-names:
> -    description: |
> -      Specifies the debug messages memory mapped region that is optionally
> -      made available from TI-SCI controller.
> -    const: debug_messages
> +    items:
> +      - const: debug_messages
> +      - const: lpm
> +    minItems: 1
>  
>    reg:
> +    items:
> +      - description: |
> +          Specifies the debug messages memory mapped region that is optionally
> +          made available from TI-SCI controller.
> +      - description: |
> +          Specifies the lpm memory mapped region where the FS Stub low power
> +          firmware is to be loaded.
>      minItems: 1
>  
>    mbox-names:
> @@ -92,6 +99,12 @@ properties:
>      type: object
>      $ref: /schemas/reset/ti,sci-reset.yaml#
>  
> +  ti,lpm-firmware-name:

firmware-name

> +    description: |
> +      Name of binary of FS Stub low power firmware located on the
> +      firmware search path.
> +    $ref: /schemas/types.yaml#/definitions/string
> +
>  required:
>    - compatible
>    - mbox-names
> -- 
> 2.35.0
> 
> 

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

[PATCH 3/6] firmware: ti_sci: Introduce Power Management Ops

[Dave Gerlach]

Introduce power management ops to the ti_sci driver and add a single
prepare_sleep op that is used by the kernel suspend layer to provide
details to firmware about the state being entered.

Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
---
 drivers/firmware/ti_sci.c              | 62 ++++++++++++++++++++++++++
 drivers/firmware/ti_sci.h              | 32 ++++++++++++-
 include/linux/soc/ti/ti_sci_protocol.h |  6 +++
 3 files changed, 99 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index ebc32bbd9b83..4b0f747251c8 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -1667,6 +1667,65 @@ static int ti_sci_cmd_clk_get_freq(const struct ti_sci_handle *handle,
 	return ret;
 }
 
+/**
+ * ti_sci_cmd_prepare_sleep() - Prepare system for system suspend
+ * @handle:		pointer to TI SCI handle
+ * @mode:		Device identifier
+ * @ctx_lo:		Low part of address for context save
+ * @ctx_hi:		High part of address for context save
+ * @debug_flags:	Debug flags to pass to firmware
+ *
+ * Return: 0 if all went well, else returns appropriate error value.
+ */
+static int ti_sci_cmd_prepare_sleep(const struct ti_sci_handle *handle, u8 mode,
+				    u32 ctx_lo, u32 ctx_hi, u32 debug_flags)
+{
+	struct ti_sci_info *info;
+	struct ti_sci_msg_req_prepare_sleep *req;
+	struct ti_sci_msg_hdr *resp;
+	struct ti_sci_xfer *xfer;
+	struct device *dev;
+	int ret = 0;
+
+	if (IS_ERR(handle))
+		return PTR_ERR(handle);
+	if (!handle)
+		return -EINVAL;
+
+	info = handle_to_ti_sci_info(handle);
+	dev = info->dev;
+
+	xfer = ti_sci_get_one_xfer(info, TI_SCI_MSG_PREPARE_SLEEP,
+				   TI_SCI_FLAG_REQ_ACK_ON_PROCESSED,
+				   sizeof(*req), sizeof(*resp));
+	if (IS_ERR(xfer)) {
+		ret = PTR_ERR(xfer);
+		dev_err(dev, "Message alloc failed(%d)\n", ret);
+		return ret;
+	}
+
+	req = (struct ti_sci_msg_req_prepare_sleep *)xfer->xfer_buf;
+	req->mode = mode;
+	req->ctx_lo = ctx_lo;
+	req->ctx_hi = ctx_hi;
+	req->debug_flags = debug_flags;
+
+	ret = ti_sci_do_xfer(info, xfer);
+	if (ret) {
+		dev_err(dev, "Mbox send fail %d\n", ret);
+		goto fail;
+	}
+
+	resp = (struct ti_sci_msg_hdr *)xfer->xfer_buf;
+
+	ret = ti_sci_is_response_ack(resp) ? 0 : -ENODEV;
+
+fail:
+	ti_sci_put_one_xfer(&info->minfo, xfer);
+
+	return ret;
+}
+
 static int ti_sci_cmd_core_reboot(const struct ti_sci_handle *handle)
 {
 	struct ti_sci_info *info;
@@ -2808,6 +2867,7 @@ static void ti_sci_setup_ops(struct ti_sci_info *info)
 	struct ti_sci_core_ops *core_ops = &ops->core_ops;
 	struct ti_sci_dev_ops *dops = &ops->dev_ops;
 	struct ti_sci_clk_ops *cops = &ops->clk_ops;
+	struct ti_sci_pm_ops *pmops = &ops->pm_ops;
 	struct ti_sci_rm_core_ops *rm_core_ops = &ops->rm_core_ops;
 	struct ti_sci_rm_irq_ops *iops = &ops->rm_irq_ops;
 	struct ti_sci_rm_ringacc_ops *rops = &ops->rm_ring_ops;
@@ -2847,6 +2907,8 @@ static void ti_sci_setup_ops(struct ti_sci_info *info)
 	cops->set_freq = ti_sci_cmd_clk_set_freq;
 	cops->get_freq = ti_sci_cmd_clk_get_freq;
 
+	pmops->prepare_sleep = ti_sci_cmd_prepare_sleep;
+
 	rm_core_ops->get_range = ti_sci_cmd_get_resource_range;
 	rm_core_ops->get_range_from_shost =
 				ti_sci_cmd_get_resource_range_from_shost;
diff --git a/drivers/firmware/ti_sci.h b/drivers/firmware/ti_sci.h
index ef3a8214d002..439e15ec54db 100644
--- a/drivers/firmware/ti_sci.h
+++ b/drivers/firmware/ti_sci.h
@@ -6,7 +6,7 @@
  * The system works in a message response protocol
  * See: http://processors.wiki.ti.com/index.php/TISCI for details
  *
- * Copyright (C)  2015-2016 Texas Instruments Incorporated - https://www.ti.com/
+ * Copyright (C)  2015-2022 Texas Instruments Incorporated - https://www.ti.com/
  */
 
 #ifndef __TI_SCI_H
@@ -35,6 +35,9 @@
 #define TI_SCI_MSG_QUERY_CLOCK_FREQ	0x010d
 #define TI_SCI_MSG_GET_CLOCK_FREQ	0x010e
 
+/* Low Power Mode Requests */
+#define TI_SCI_MSG_PREPARE_SLEEP       0x0300
+
 /* Resource Management Requests */
 #define TI_SCI_MSG_GET_RESOURCE_RANGE	0x1500
 
@@ -545,6 +548,33 @@ struct ti_sci_msg_resp_get_clock_freq {
 	u64 freq_hz;
 } __packed;
 
+#define TISCI_MSG_VALUE_SLEEP_MODE_DEEP_SLEEP				0x0
+#define TISCI_MSG_VALUE_SLEEP_MODE_MCU_ONLY				0x1
+#define TISCI_MSG_VALUE_SLEEP_MODE_STANDBY				0x2
+
+/**
+ * struct tisci_msg_prepare_sleep_req - Request for TISCI_MSG_PREPARE_SLEEP.
+ *
+ * @hdr				TISCI header to provide ACK/NAK flags to the host.
+ * @mode			Low power mode to enter.
+ * @ctx_lo			Low 32-bits of physical pointer to address to use for context save.
+ * @ctx_hi			High 32-bits of physical pointer to address to use for context save.
+ * @debug_flags			Flags that can be set to halt the sequence during suspend or
+ *				resume to allow JTAG connection and debug.
+ *
+ * This message is used as the first step of entering a low power mode. It
+ * allows configurable information, including which state to enter to be
+ * easily shared from the application, as this is a non-secure message and
+ * therefore can be sent by anyone.
+ */
+struct ti_sci_msg_req_prepare_sleep {
+	struct ti_sci_msg_hdr	hdr;
+	u8			mode;
+	u32			ctx_lo;
+	u32			ctx_hi;
+	u32			debug_flags;
+} __packed;
+
 #define TI_SCI_IRQ_SECONDARY_HOST_INVALID	0xff
 
 /**
diff --git a/include/linux/soc/ti/ti_sci_protocol.h b/include/linux/soc/ti/ti_sci_protocol.h
index bd0d11af76c5..8adcb45d4b5c 100644
--- a/include/linux/soc/ti/ti_sci_protocol.h
+++ b/include/linux/soc/ti/ti_sci_protocol.h
@@ -195,6 +195,11 @@ struct ti_sci_clk_ops {
 			u64 *current_freq);
 };
 
+struct ti_sci_pm_ops {
+	int (*prepare_sleep)(const struct ti_sci_handle *handle, u8 mode,
+			     u32 ctx_lo, u32 ctx_hi, u32 flags);
+};
+
 /**
  * struct ti_sci_resource_desc - Description of TI SCI resource instance range.
  * @start:	Start index of the first resource range.
@@ -539,6 +544,7 @@ struct ti_sci_ops {
 	struct ti_sci_core_ops core_ops;
 	struct ti_sci_dev_ops dev_ops;
 	struct ti_sci_clk_ops clk_ops;
+	struct ti_sci_pm_ops pm_ops;
 	struct ti_sci_rm_core_ops rm_core_ops;
 	struct ti_sci_rm_irq_ops rm_irq_ops;
 	struct ti_sci_rm_ringacc_ops rm_ring_ops;
-- 
2.35.0

[PATCH 3/6] firmware: ti_sci: Introduce Power Management Ops

[Dave Gerlach]

Introduce power management ops to the ti_sci driver and add a single
prepare_sleep op that is used by the kernel suspend layer to provide
details to firmware about the state being entered.

Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
---
 drivers/firmware/ti_sci.c              | 62 ++++++++++++++++++++++++++
 drivers/firmware/ti_sci.h              | 32 ++++++++++++-
 include/linux/soc/ti/ti_sci_protocol.h |  6 +++
 3 files changed, 99 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index ebc32bbd9b83..4b0f747251c8 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -1667,6 +1667,65 @@ static int ti_sci_cmd_clk_get_freq(const struct ti_sci_handle *handle,
 	return ret;
 }
 
+/**
+ * ti_sci_cmd_prepare_sleep() - Prepare system for system suspend
+ * @handle:		pointer to TI SCI handle
+ * @mode:		Device identifier
+ * @ctx_lo:		Low part of address for context save
+ * @ctx_hi:		High part of address for context save
+ * @debug_flags:	Debug flags to pass to firmware
+ *
+ * Return: 0 if all went well, else returns appropriate error value.
+ */
+static int ti_sci_cmd_prepare_sleep(const struct ti_sci_handle *handle, u8 mode,
+				    u32 ctx_lo, u32 ctx_hi, u32 debug_flags)
+{
+	struct ti_sci_info *info;
+	struct ti_sci_msg_req_prepare_sleep *req;
+	struct ti_sci_msg_hdr *resp;
+	struct ti_sci_xfer *xfer;
+	struct device *dev;
+	int ret = 0;
+
+	if (IS_ERR(handle))
+		return PTR_ERR(handle);
+	if (!handle)
+		return -EINVAL;
+
+	info = handle_to_ti_sci_info(handle);
+	dev = info->dev;
+
+	xfer = ti_sci_get_one_xfer(info, TI_SCI_MSG_PREPARE_SLEEP,
+				   TI_SCI_FLAG_REQ_ACK_ON_PROCESSED,
+				   sizeof(*req), sizeof(*resp));
+	if (IS_ERR(xfer)) {
+		ret = PTR_ERR(xfer);
+		dev_err(dev, "Message alloc failed(%d)\n", ret);
+		return ret;
+	}
+
+	req = (struct ti_sci_msg_req_prepare_sleep *)xfer->xfer_buf;
+	req->mode = mode;
+	req->ctx_lo = ctx_lo;
+	req->ctx_hi = ctx_hi;
+	req->debug_flags = debug_flags;
+
+	ret = ti_sci_do_xfer(info, xfer);
+	if (ret) {
+		dev_err(dev, "Mbox send fail %d\n", ret);
+		goto fail;
+	}
+
+	resp = (struct ti_sci_msg_hdr *)xfer->xfer_buf;
+
+	ret = ti_sci_is_response_ack(resp) ? 0 : -ENODEV;
+
+fail:
+	ti_sci_put_one_xfer(&info->minfo, xfer);
+
+	return ret;
+}
+
 static int ti_sci_cmd_core_reboot(const struct ti_sci_handle *handle)
 {
 	struct ti_sci_info *info;
@@ -2808,6 +2867,7 @@ static void ti_sci_setup_ops(struct ti_sci_info *info)
 	struct ti_sci_core_ops *core_ops = &ops->core_ops;
 	struct ti_sci_dev_ops *dops = &ops->dev_ops;
 	struct ti_sci_clk_ops *cops = &ops->clk_ops;
+	struct ti_sci_pm_ops *pmops = &ops->pm_ops;
 	struct ti_sci_rm_core_ops *rm_core_ops = &ops->rm_core_ops;
 	struct ti_sci_rm_irq_ops *iops = &ops->rm_irq_ops;
 	struct ti_sci_rm_ringacc_ops *rops = &ops->rm_ring_ops;
@@ -2847,6 +2907,8 @@ static void ti_sci_setup_ops(struct ti_sci_info *info)
 	cops->set_freq = ti_sci_cmd_clk_set_freq;
 	cops->get_freq = ti_sci_cmd_clk_get_freq;
 
+	pmops->prepare_sleep = ti_sci_cmd_prepare_sleep;
+
 	rm_core_ops->get_range = ti_sci_cmd_get_resource_range;
 	rm_core_ops->get_range_from_shost =
 				ti_sci_cmd_get_resource_range_from_shost;
diff --git a/drivers/firmware/ti_sci.h b/drivers/firmware/ti_sci.h
index ef3a8214d002..439e15ec54db 100644
--- a/drivers/firmware/ti_sci.h
+++ b/drivers/firmware/ti_sci.h
@@ -6,7 +6,7 @@
  * The system works in a message response protocol
  * See: http://processors.wiki.ti.com/index.php/TISCI for details
  *
- * Copyright (C)  2015-2016 Texas Instruments Incorporated - https://www.ti.com/
+ * Copyright (C)  2015-2022 Texas Instruments Incorporated - https://www.ti.com/
  */
 
 #ifndef __TI_SCI_H
@@ -35,6 +35,9 @@
 #define TI_SCI_MSG_QUERY_CLOCK_FREQ	0x010d
 #define TI_SCI_MSG_GET_CLOCK_FREQ	0x010e
 
+/* Low Power Mode Requests */
+#define TI_SCI_MSG_PREPARE_SLEEP       0x0300
+
 /* Resource Management Requests */
 #define TI_SCI_MSG_GET_RESOURCE_RANGE	0x1500
 
@@ -545,6 +548,33 @@ struct ti_sci_msg_resp_get_clock_freq {
 	u64 freq_hz;
 } __packed;
 
+#define TISCI_MSG_VALUE_SLEEP_MODE_DEEP_SLEEP				0x0
+#define TISCI_MSG_VALUE_SLEEP_MODE_MCU_ONLY				0x1
+#define TISCI_MSG_VALUE_SLEEP_MODE_STANDBY				0x2
+
+/**
+ * struct tisci_msg_prepare_sleep_req - Request for TISCI_MSG_PREPARE_SLEEP.
+ *
+ * @hdr				TISCI header to provide ACK/NAK flags to the host.
+ * @mode			Low power mode to enter.
+ * @ctx_lo			Low 32-bits of physical pointer to address to use for context save.
+ * @ctx_hi			High 32-bits of physical pointer to address to use for context save.
+ * @debug_flags			Flags that can be set to halt the sequence during suspend or
+ *				resume to allow JTAG connection and debug.
+ *
+ * This message is used as the first step of entering a low power mode. It
+ * allows configurable information, including which state to enter to be
+ * easily shared from the application, as this is a non-secure message and
+ * therefore can be sent by anyone.
+ */
+struct ti_sci_msg_req_prepare_sleep {
+	struct ti_sci_msg_hdr	hdr;
+	u8			mode;
+	u32			ctx_lo;
+	u32			ctx_hi;
+	u32			debug_flags;
+} __packed;
+
 #define TI_SCI_IRQ_SECONDARY_HOST_INVALID	0xff
 
 /**
diff --git a/include/linux/soc/ti/ti_sci_protocol.h b/include/linux/soc/ti/ti_sci_protocol.h
index bd0d11af76c5..8adcb45d4b5c 100644
--- a/include/linux/soc/ti/ti_sci_protocol.h
+++ b/include/linux/soc/ti/ti_sci_protocol.h
@@ -195,6 +195,11 @@ struct ti_sci_clk_ops {
 			u64 *current_freq);
 };
 
+struct ti_sci_pm_ops {
+	int (*prepare_sleep)(const struct ti_sci_handle *handle, u8 mode,
+			     u32 ctx_lo, u32 ctx_hi, u32 flags);
+};
+
 /**
  * struct ti_sci_resource_desc - Description of TI SCI resource instance range.
  * @start:	Start index of the first resource range.
@@ -539,6 +544,7 @@ struct ti_sci_ops {
 	struct ti_sci_core_ops core_ops;
 	struct ti_sci_dev_ops dev_ops;
 	struct ti_sci_clk_ops clk_ops;
+	struct ti_sci_pm_ops pm_ops;
 	struct ti_sci_rm_core_ops rm_core_ops;
 	struct ti_sci_rm_irq_ops rm_irq_ops;
 	struct ti_sci_rm_ringacc_ops rm_ring_ops;
-- 
2.35.0


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

[PATCH 4/6] firmware: ti_sci: Introduce ti,ctx-memory-region for reserved LPM memory

[Dave Gerlach]

A reserved memory region in DDR must be used during low power modes for
saving of some system context when using the ti_sci firmware, so
introduce a property to allow providing this in the device tree so that
it can be read and shared with the firmware.

Also send a TISCI_MSG_PREPARE_SUSPEND message to the firmware during
probe to determine if system suspend is supported and if
ti_sci_init_suspend should be called based on the response received.

Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
---
 drivers/firmware/ti_sci.c | 45 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index 4b0f747251c8..1c2000b40e8f 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -17,6 +17,7 @@
 #include <linux/mailbox_client.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
+#include <linux/of_reserved_mem.h>
 #include <linux/semaphore.h>
 #include <linux/slab.h>
 #include <linux/soc/ti/ti-msgmgr.h>
@@ -96,6 +97,8 @@ struct ti_sci_desc {
  * @minfo:	Message info
  * @node:	list head
  * @host_id:	Host ID
+ * @mem_ctx_lo: Low word of address used for low power context memory
+ * @mem_ctx_hi: High word of address used for low power context memory
  * @users:	Number of users of this instance
  * @is_suspending: Flag set to indicate in suspend path.
  */
@@ -114,6 +117,8 @@ struct ti_sci_info {
 	struct ti_sci_xfers_info minfo;
 	struct list_head node;
 	u8 host_id;
+	u32 mem_ctx_lo;
+	u32 mem_ctx_hi;
 	/* protected by ti_sci_list_mutex */
 	int users;
 	bool is_suspending;
@@ -3374,6 +3379,29 @@ static int ti_sci_resume(struct device *dev)
 
 static DEFINE_SIMPLE_DEV_PM_OPS(ti_sci_pm_ops, ti_sci_suspend, ti_sci_resume);
 
+static int ti_sci_init_suspend(struct platform_device *pdev, struct ti_sci_info *info)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *rmem_np;
+	struct reserved_mem *rmem;
+
+	rmem_np = of_parse_phandle(dev->of_node, "ti,ctx-memory-region", 0);
+	if (!rmem_np) {
+		dev_warn(dev, "ti,ctx-memory-region is required for suspend but not provided.\n");
+		return -EINVAL;
+	}
+
+	rmem = of_reserved_mem_lookup(rmem_np);
+	of_node_put(rmem_np);
+	if (!rmem)
+		return -EINVAL;
+
+	info->mem_ctx_lo = (rmem->base & 0xFFFFFFFF);
+	info->mem_ctx_hi = (rmem->base >> 32);
+
+	return 0;
+}
+
 /* Description for K2G */
 static const struct ti_sci_desc ti_sci_pmmc_k2g_desc = {
 	.default_host_id = 2,
@@ -3526,6 +3554,23 @@ static int ti_sci_probe(struct platform_device *pdev)
 		}
 	}
 
+	/*
+	 * 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, info->mem_ctx_lo,
+				       info->mem_ctx_hi, 0);
+	if (!ret) {
+		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;
+
 	dev_info(dev, "ABI: %d.%d (firmware rev 0x%04x '%s')\n",
 		 info->handle.version.abi_major, info->handle.version.abi_minor,
 		 info->handle.version.firmware_revision,
-- 
2.35.0

[PATCH 4/6] firmware: ti_sci: Introduce ti, ctx-memory-region for reserved LPM memory

[Dave Gerlach]

A reserved memory region in DDR must be used during low power modes for
saving of some system context when using the ti_sci firmware, so
introduce a property to allow providing this in the device tree so that
it can be read and shared with the firmware.

Also send a TISCI_MSG_PREPARE_SUSPEND message to the firmware during
probe to determine if system suspend is supported and if
ti_sci_init_suspend should be called based on the response received.

Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
---
 drivers/firmware/ti_sci.c | 45 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index 4b0f747251c8..1c2000b40e8f 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -17,6 +17,7 @@
 #include <linux/mailbox_client.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
+#include <linux/of_reserved_mem.h>
 #include <linux/semaphore.h>
 #include <linux/slab.h>
 #include <linux/soc/ti/ti-msgmgr.h>
@@ -96,6 +97,8 @@ struct ti_sci_desc {
  * @minfo:	Message info
  * @node:	list head
  * @host_id:	Host ID
+ * @mem_ctx_lo: Low word of address used for low power context memory
+ * @mem_ctx_hi: High word of address used for low power context memory
  * @users:	Number of users of this instance
  * @is_suspending: Flag set to indicate in suspend path.
  */
@@ -114,6 +117,8 @@ struct ti_sci_info {
 	struct ti_sci_xfers_info minfo;
 	struct list_head node;
 	u8 host_id;
+	u32 mem_ctx_lo;
+	u32 mem_ctx_hi;
 	/* protected by ti_sci_list_mutex */
 	int users;
 	bool is_suspending;
@@ -3374,6 +3379,29 @@ static int ti_sci_resume(struct device *dev)
 
 static DEFINE_SIMPLE_DEV_PM_OPS(ti_sci_pm_ops, ti_sci_suspend, ti_sci_resume);
 
+static int ti_sci_init_suspend(struct platform_device *pdev, struct ti_sci_info *info)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *rmem_np;
+	struct reserved_mem *rmem;
+
+	rmem_np = of_parse_phandle(dev->of_node, "ti,ctx-memory-region", 0);
+	if (!rmem_np) {
+		dev_warn(dev, "ti,ctx-memory-region is required for suspend but not provided.\n");
+		return -EINVAL;
+	}
+
+	rmem = of_reserved_mem_lookup(rmem_np);
+	of_node_put(rmem_np);
+	if (!rmem)
+		return -EINVAL;
+
+	info->mem_ctx_lo = (rmem->base & 0xFFFFFFFF);
+	info->mem_ctx_hi = (rmem->base >> 32);
+
+	return 0;
+}
+
 /* Description for K2G */
 static const struct ti_sci_desc ti_sci_pmmc_k2g_desc = {
 	.default_host_id = 2,
@@ -3526,6 +3554,23 @@ static int ti_sci_probe(struct platform_device *pdev)
 		}
 	}
 
+	/*
+	 * 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, info->mem_ctx_lo,
+				       info->mem_ctx_hi, 0);
+	if (!ret) {
+		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;
+
 	dev_info(dev, "ABI: %d.%d (firmware rev 0x%04x '%s')\n",
 		 info->handle.version.abi_major, info->handle.version.abi_minor,
 		 info->handle.version.firmware_revision,
-- 
2.35.0


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

[PATCH 5/6] firmware: ti_sci: Use dt provided fw name and address to load at suspend time

[Dave Gerlach]

Use request_firmware_direct to load the fs stub LPM firmware to a
provided lpm memory region. The filename for the firmware is provided in
the device tree as "ti,lpm-firmware-name"

Also, add support for "lazy loading" of this firmware during the suspend
callback for the driver so that it is loaded at first suspend. It is
possible in the future for this firmware to require reload, so add a
check to indicate that the firmware is currently loaded so it is only
loaded once at this time.

Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
---
 drivers/firmware/ti_sci.c | 69 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 68 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index 1c2000b40e8f..772643cb940c 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -11,6 +11,7 @@
 #include <linux/bitmap.h>
 #include <linux/debugfs.h>
 #include <linux/export.h>
+#include <linux/firmware.h>
 #include <linux/io.h>
 #include <linux/iopoll.h>
 #include <linux/kernel.h>
@@ -90,6 +91,8 @@ struct ti_sci_desc {
  * @debug_region: Memory region where the debug message are available
  * @debug_region_size: Debug region size
  * @debug_buffer: Buffer allocated to copy debug messages.
+ * @lpm_region: Memory region where the FS Stub LPM Firmware will be stored
+ * @lpm_region_size: LPM region size
  * @handle:	Instance of TI SCI handle to send to clients.
  * @cl:		Mailbox Client
  * @chan_tx:	Transmit mailbox channel
@@ -101,6 +104,8 @@ struct ti_sci_desc {
  * @mem_ctx_hi: High word of address used for low power context memory
  * @users:	Number of users of this instance
  * @is_suspending: Flag set to indicate in suspend path.
+ * @lpm_firmware_loaded: Flag to indicate if LPM firmware has been loaded
+ * @lpm_firmware_name: Name of firmware binary to load from fw search path
  */
 struct ti_sci_info {
 	struct device *dev;
@@ -110,6 +115,8 @@ struct ti_sci_info {
 	void __iomem *debug_region;
 	char *debug_buffer;
 	size_t debug_region_size;
+	void __iomem *lpm_region;
+	size_t lpm_region_size;
 	struct ti_sci_handle handle;
 	struct mbox_client cl;
 	struct mbox_chan *chan_tx;
@@ -122,6 +129,8 @@ struct ti_sci_info {
 	/* protected by ti_sci_list_mutex */
 	int users;
 	bool is_suspending;
+	bool lpm_firmware_loaded;
+	const char *lpm_firmware_name;
 };
 
 #define cl_to_ti_sci_info(c)	container_of(c, struct ti_sci_info, cl)
@@ -3350,6 +3359,30 @@ static int tisci_reboot_handler(struct notifier_block *nb, unsigned long mode,
 	return NOTIFY_BAD;
 }
 
+static int ti_sci_load_lpm_firmware(struct device *dev, struct ti_sci_info *info)
+{
+	const struct firmware *firmware;
+	int ret = 0;
+
+	/* If no firmware name is set, do not attempt to load. */
+	if (!info->lpm_firmware_name)
+		return 0;
+
+	if (request_firmware_direct(&firmware, info->lpm_firmware_name, dev)) {
+		dev_warn(dev, "Cannot load %s\n", info->lpm_firmware_name);
+		return -ENODEV;
+	}
+
+	if (firmware->size > info->lpm_region_size)
+		return -ENOMEM;
+
+	memcpy_toio(info->lpm_region, firmware->data, firmware->size);
+
+	release_firmware(firmware);
+
+	return ret;
+}
+
 static void ti_sci_set_is_suspending(struct ti_sci_info *info, bool is_suspending)
 {
 	info->is_suspending = is_suspending;
@@ -3358,6 +3391,8 @@ static void ti_sci_set_is_suspending(struct ti_sci_info *info, bool is_suspendin
 static int ti_sci_suspend(struct device *dev)
 {
 	struct ti_sci_info *info = dev_get_drvdata(dev);
+	int ret = 0;
+
 	/*
 	 * We must switch operation to polled mode now as drivers and the genpd
 	 * layer may make late TI SCI calls to change clock and device states
@@ -3365,7 +3400,19 @@ static int ti_sci_suspend(struct device *dev)
 	 */
 	ti_sci_set_is_suspending(info, true);
 
-	return 0;
+	if (!info->lpm_firmware_loaded) {
+		ret = ti_sci_load_lpm_firmware(dev, info);
+		if (ret) {
+			dev_err(dev,
+				"Failed to load low power mode firmware, suspend is non functional (%d)\n",
+				ret);
+			ret = -ENODEV;
+		} else {
+			info->lpm_firmware_loaded = true;
+		}
+	}
+
+	return ret;
 }
 
 static int ti_sci_resume(struct device *dev)
@@ -3384,6 +3431,7 @@ static int ti_sci_init_suspend(struct platform_device *pdev, struct ti_sci_info
 	struct device *dev = &pdev->dev;
 	struct device_node *rmem_np;
 	struct reserved_mem *rmem;
+	struct resource *res;
 
 	rmem_np = of_parse_phandle(dev->of_node, "ti,ctx-memory-region", 0);
 	if (!rmem_np) {
@@ -3399,6 +3447,25 @@ static int ti_sci_init_suspend(struct platform_device *pdev, struct ti_sci_info
 	info->mem_ctx_lo = (rmem->base & 0xFFFFFFFF);
 	info->mem_ctx_hi = (rmem->base >> 32);
 
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "lpm");
+	if (!res) {
+		dev_warn(dev,
+			 "lpm region is required for suspend but not provided.\n");
+		return -EINVAL;
+	}
+
+	info->lpm_region = devm_ioremap_resource(dev, res);
+	if (IS_ERR(info->lpm_region))
+		return PTR_ERR(info->lpm_region);
+	info->lpm_region_size = resource_size(res);
+
+	if (of_property_read_string(dev->of_node, "ti,lpm-firmware-name",
+				    &info->lpm_firmware_name)) {
+		dev_warn(dev,
+			 "ti,lpm-firmware-name is required for suspend but not provided.\n");
+		return -EINVAL;
+	}
+
 	return 0;
 }
 
-- 
2.35.0

[PATCH 5/6] firmware: ti_sci: Use dt provided fw name and address to load at suspend time

[Dave Gerlach]

Use request_firmware_direct to load the fs stub LPM firmware to a
provided lpm memory region. The filename for the firmware is provided in
the device tree as "ti,lpm-firmware-name"

Also, add support for "lazy loading" of this firmware during the suspend
callback for the driver so that it is loaded at first suspend. It is
possible in the future for this firmware to require reload, so add a
check to indicate that the firmware is currently loaded so it is only
loaded once at this time.

Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
---
 drivers/firmware/ti_sci.c | 69 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 68 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index 1c2000b40e8f..772643cb940c 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -11,6 +11,7 @@
 #include <linux/bitmap.h>
 #include <linux/debugfs.h>
 #include <linux/export.h>
+#include <linux/firmware.h>
 #include <linux/io.h>
 #include <linux/iopoll.h>
 #include <linux/kernel.h>
@@ -90,6 +91,8 @@ struct ti_sci_desc {
  * @debug_region: Memory region where the debug message are available
  * @debug_region_size: Debug region size
  * @debug_buffer: Buffer allocated to copy debug messages.
+ * @lpm_region: Memory region where the FS Stub LPM Firmware will be stored
+ * @lpm_region_size: LPM region size
  * @handle:	Instance of TI SCI handle to send to clients.
  * @cl:		Mailbox Client
  * @chan_tx:	Transmit mailbox channel
@@ -101,6 +104,8 @@ struct ti_sci_desc {
  * @mem_ctx_hi: High word of address used for low power context memory
  * @users:	Number of users of this instance
  * @is_suspending: Flag set to indicate in suspend path.
+ * @lpm_firmware_loaded: Flag to indicate if LPM firmware has been loaded
+ * @lpm_firmware_name: Name of firmware binary to load from fw search path
  */
 struct ti_sci_info {
 	struct device *dev;
@@ -110,6 +115,8 @@ struct ti_sci_info {
 	void __iomem *debug_region;
 	char *debug_buffer;
 	size_t debug_region_size;
+	void __iomem *lpm_region;
+	size_t lpm_region_size;
 	struct ti_sci_handle handle;
 	struct mbox_client cl;
 	struct mbox_chan *chan_tx;
@@ -122,6 +129,8 @@ struct ti_sci_info {
 	/* protected by ti_sci_list_mutex */
 	int users;
 	bool is_suspending;
+	bool lpm_firmware_loaded;
+	const char *lpm_firmware_name;
 };
 
 #define cl_to_ti_sci_info(c)	container_of(c, struct ti_sci_info, cl)
@@ -3350,6 +3359,30 @@ static int tisci_reboot_handler(struct notifier_block *nb, unsigned long mode,
 	return NOTIFY_BAD;
 }
 
+static int ti_sci_load_lpm_firmware(struct device *dev, struct ti_sci_info *info)
+{
+	const struct firmware *firmware;
+	int ret = 0;
+
+	/* If no firmware name is set, do not attempt to load. */
+	if (!info->lpm_firmware_name)
+		return 0;
+
+	if (request_firmware_direct(&firmware, info->lpm_firmware_name, dev)) {
+		dev_warn(dev, "Cannot load %s\n", info->lpm_firmware_name);
+		return -ENODEV;
+	}
+
+	if (firmware->size > info->lpm_region_size)
+		return -ENOMEM;
+
+	memcpy_toio(info->lpm_region, firmware->data, firmware->size);
+
+	release_firmware(firmware);
+
+	return ret;
+}
+
 static void ti_sci_set_is_suspending(struct ti_sci_info *info, bool is_suspending)
 {
 	info->is_suspending = is_suspending;
@@ -3358,6 +3391,8 @@ static void ti_sci_set_is_suspending(struct ti_sci_info *info, bool is_suspendin
 static int ti_sci_suspend(struct device *dev)
 {
 	struct ti_sci_info *info = dev_get_drvdata(dev);
+	int ret = 0;
+
 	/*
 	 * We must switch operation to polled mode now as drivers and the genpd
 	 * layer may make late TI SCI calls to change clock and device states
@@ -3365,7 +3400,19 @@ static int ti_sci_suspend(struct device *dev)
 	 */
 	ti_sci_set_is_suspending(info, true);
 
-	return 0;
+	if (!info->lpm_firmware_loaded) {
+		ret = ti_sci_load_lpm_firmware(dev, info);
+		if (ret) {
+			dev_err(dev,
+				"Failed to load low power mode firmware, suspend is non functional (%d)\n",
+				ret);
+			ret = -ENODEV;
+		} else {
+			info->lpm_firmware_loaded = true;
+		}
+	}
+
+	return ret;
 }
 
 static int ti_sci_resume(struct device *dev)
@@ -3384,6 +3431,7 @@ static int ti_sci_init_suspend(struct platform_device *pdev, struct ti_sci_info
 	struct device *dev = &pdev->dev;
 	struct device_node *rmem_np;
 	struct reserved_mem *rmem;
+	struct resource *res;
 
 	rmem_np = of_parse_phandle(dev->of_node, "ti,ctx-memory-region", 0);
 	if (!rmem_np) {
@@ -3399,6 +3447,25 @@ static int ti_sci_init_suspend(struct platform_device *pdev, struct ti_sci_info
 	info->mem_ctx_lo = (rmem->base & 0xFFFFFFFF);
 	info->mem_ctx_hi = (rmem->base >> 32);
 
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "lpm");
+	if (!res) {
+		dev_warn(dev,
+			 "lpm region is required for suspend but not provided.\n");
+		return -EINVAL;
+	}
+
+	info->lpm_region = devm_ioremap_resource(dev, res);
+	if (IS_ERR(info->lpm_region))
+		return PTR_ERR(info->lpm_region);
+	info->lpm_region_size = resource_size(res);
+
+	if (of_property_read_string(dev->of_node, "ti,lpm-firmware-name",
+				    &info->lpm_firmware_name)) {
+		dev_warn(dev,
+			 "ti,lpm-firmware-name is required for suspend but not provided.\n");
+		return -EINVAL;
+	}
+
 	return 0;
 }
 
-- 
2.35.0


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

[PATCH 6/6] firmware: ti_sci: Introduce prepare system suspend call

[Dave Gerlach]

Introduce a ti_sci_prepare_system_suspend call to be used in the driver
suspend handler to allow the system to identify the low power mode being
entered and if nevessary, send TI_SCI_MSG_BEGIN_SLEEP with information
about the mode is being entered and the address for DDR memory carveout.

Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
---
 drivers/firmware/ti_sci.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index 772643cb940c..07b6fa98bb45 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -23,6 +23,7 @@
 #include <linux/slab.h>
 #include <linux/soc/ti/ti-msgmgr.h>
 #include <linux/soc/ti/ti_sci_protocol.h>
+#include <linux/suspend.h>
 #include <linux/reboot.h>
 
 #include "ti_sci.h"
@@ -3388,11 +3389,41 @@ static void ti_sci_set_is_suspending(struct ti_sci_info *info, bool is_suspendin
 	info->is_suspending = is_suspending;
 }
 
+static int ti_sci_prepare_system_suspend(struct ti_sci_info *info)
+{
+	int ret = 0;
+	int mode;
+
+	switch (pm_suspend_target_state) {
+	case PM_SUSPEND_MEM:
+		mode = TISCI_MSG_VALUE_SLEEP_MODE_DEEP_SLEEP;
+		break;
+	default:
+		ret = -EINVAL;
+	}
+
+	/*
+	 * Do not fail if we don't have action to take for a
+	 * specific suspend mode.
+	 */
+	if (ret)
+		return 0;
+
+	ret = ti_sci_cmd_prepare_sleep(&info->handle, mode, info->mem_ctx_lo,
+				       info->mem_ctx_hi, 0);
+
+	return ret;
+}
+
 static int ti_sci_suspend(struct device *dev)
 {
 	struct ti_sci_info *info = dev_get_drvdata(dev);
 	int ret = 0;
 
+	ret = ti_sci_prepare_system_suspend(info);
+	if (ret)
+		return ret;
+
 	/*
 	 * We must switch operation to polled mode now as drivers and the genpd
 	 * layer may make late TI SCI calls to change clock and device states
-- 
2.35.0

[PATCH 6/6] firmware: ti_sci: Introduce prepare system suspend call

[Dave Gerlach]

Introduce a ti_sci_prepare_system_suspend call to be used in the driver
suspend handler to allow the system to identify the low power mode being
entered and if nevessary, send TI_SCI_MSG_BEGIN_SLEEP with information
about the mode is being entered and the address for DDR memory carveout.

Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
---
 drivers/firmware/ti_sci.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index 772643cb940c..07b6fa98bb45 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -23,6 +23,7 @@
 #include <linux/slab.h>
 #include <linux/soc/ti/ti-msgmgr.h>
 #include <linux/soc/ti/ti_sci_protocol.h>
+#include <linux/suspend.h>
 #include <linux/reboot.h>
 
 #include "ti_sci.h"
@@ -3388,11 +3389,41 @@ static void ti_sci_set_is_suspending(struct ti_sci_info *info, bool is_suspendin
 	info->is_suspending = is_suspending;
 }
 
+static int ti_sci_prepare_system_suspend(struct ti_sci_info *info)
+{
+	int ret = 0;
+	int mode;
+
+	switch (pm_suspend_target_state) {
+	case PM_SUSPEND_MEM:
+		mode = TISCI_MSG_VALUE_SLEEP_MODE_DEEP_SLEEP;
+		break;
+	default:
+		ret = -EINVAL;
+	}
+
+	/*
+	 * Do not fail if we don't have action to take for a
+	 * specific suspend mode.
+	 */
+	if (ret)
+		return 0;
+
+	ret = ti_sci_cmd_prepare_sleep(&info->handle, mode, info->mem_ctx_lo,
+				       info->mem_ctx_hi, 0);
+
+	return ret;
+}
+
 static int ti_sci_suspend(struct device *dev)
 {
 	struct ti_sci_info *info = dev_get_drvdata(dev);
 	int ret = 0;
 
+	ret = ti_sci_prepare_system_suspend(info);
+	if (ret)
+		return ret;
+
 	/*
 	 * We must switch operation to polled mode now as drivers and the genpd
 	 * layer may make late TI SCI calls to change clock and device states
-- 
2.35.0


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