During suspend, some devices free their irqs and request them at resume.
But there are some devices which don't do it, and assume that it's done
by the resource allocator.
On j7200, during suspend to ram the SoC is powered-off, so the allocated
irqs are lost.
With this series, ti_sci has an internal list of allocated irqs. It will
restore these irqs during its resume_noirq.
Moreover, ti_sci checks that there is at least a reserved memory region for
lpm. These regions are used by TF-A and R5 SPL during suspend and resume.
We need to reserve some memory from the linux point of view to avoid any
memory corruption.
A new compatible (ti,j7200-sci) was created for this specific behavior.
Signed-off-by: Thomas Richard <[email protected]>
---
Thomas Richard (5):
dt-bindings: arm: keystone: add ti,j7200-sci compatible
firmware: ti_sci: add suspend/resume support for irqs
arm64: dts: ti: k3-j7200: use ti,j7200-sci compatible
firmware: ti-sci: for j7200 before suspend check the reserved memory for lpm
arm64: dts: ti: k3-j7200: add reserved memory regions for lpm
.../devicetree/bindings/arm/keystone/ti,sci.yaml | 2 +
arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi | 2 +-
arch/arm64/boot/dts/ti/k3-j7200-som-p0.dtsi | 10 ++
drivers/firmware/ti_sci.c | 191 ++++++++++++++++++++-
4 files changed, 196 insertions(+), 9 deletions(-)
---
base-commit: 2041413d851e6dccd5c91321498e1d159ea432f2
change-id: 20231129-j7200-tisci-s2r-69c219c33456
Best regards,
--
Thomas Richard <[email protected]>
On j7200, during suspend to ram the soc is powered-off.
At resume requested irqs shall be restored which is a different behavior
from other platforms.
Signed-off-by: Thomas Richard <[email protected]>
---
Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
index c24ad0968f3e..53d9c58dcd70 100644
--- a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
+++ b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
@@ -40,6 +40,8 @@ properties:
- description: System controller on TI AM654 SoC
items:
- const: ti,am654-sci
+ - description: System controller on TI J7200 SOC
+ - const: ti,j7200-sci
reg-names:
description: |
--
2.39.2
On j7200, suspend to ram poweroff the SOC.
This compatible restores irqs at resume.
Signed-off-by: Thomas Richard <[email protected]>
---
arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
index 467970fab3a0..d8dc1421e75e 100644
--- a/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
@@ -7,7 +7,7 @@
&cbass_mcu_wakeup {
dmsc: system-controller@44083000 {
- compatible = "ti,k2g-sci";
+ compatible = "ti,j7200-sci", "ti,k2g-sci";
ti,host-id = <12>;
mbox-names = "rx", "tx";
--
2.39.2
On j7200, during suspend to ram, the SoC is powered-off.
The irqs configuration is lost, so it shall be restored at resume.
The ti-sci has an internal list of irqs updated at each set/free.
All irqs in this list are restored at resume.
Signed-off-by: Thomas Richard <[email protected]>
---
drivers/firmware/ti_sci.c | 155 +++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 147 insertions(+), 8 deletions(-)
diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index 7041befc756a..c26ec86a5ff2 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -25,6 +25,7 @@
#include <linux/soc/ti/ti-msgmgr.h>
#include <linux/soc/ti/ti_sci_protocol.h>
#include <linux/reboot.h>
+#include <linux/suspend.h>
#include "ti_sci.h"
@@ -75,12 +76,24 @@ struct ti_sci_xfers_info {
* @max_msgs: Maximum number of messages that can be pending
* simultaneously in the system
* @max_msg_size: Maximum size of data per message that can be handled.
+ * @restore_irqs: Set to true if allocated irqs shall be restored at resume
*/
struct ti_sci_desc {
u8 default_host_id;
int max_rx_timeout_ms;
int max_msgs;
int max_msg_size;
+ bool restore_irqs;
+};
+
+/**
+ * struct ti_sci_irq - Description of allocated irqs
+ * @list: List head
+ * @desc: Description of the irq
+ */
+struct ti_sci_irq {
+ struct list_head list;
+ struct ti_sci_msg_req_manage_irq desc;
};
/**
@@ -97,6 +110,7 @@ struct ti_sci_desc {
* @chan_tx: Transmit mailbox channel
* @chan_rx: Receive mailbox channel
* @minfo: Message info
+ * @irqs: List of allocated irqs
* @node: list head
* @host_id: Host ID
* @users: Number of users of this instance
@@ -114,6 +128,7 @@ struct ti_sci_info {
struct mbox_chan *chan_tx;
struct mbox_chan *chan_rx;
struct ti_sci_xfers_info minfo;
+ struct ti_sci_irq irqs;
struct list_head node;
u8 host_id;
/* protected by ti_sci_list_mutex */
@@ -1890,6 +1905,29 @@ static int ti_sci_manage_irq(const struct ti_sci_handle *handle,
return ret;
}
+/**
+ * ti_sci_irqs_equal() - Helper API to compare two irqs (generic headers are not
+ * compared)
+ * @irq_a: IRQ A to compare
+ * @irq_b: IRQ B to compare
+ *
+ * Return: true if the two irqs are equal, else false.
+ */
+static bool ti_sci_irqs_equal(struct ti_sci_msg_req_manage_irq irq_a,
+ struct ti_sci_msg_req_manage_irq irq_b)
+{
+ return irq_a.valid_params == irq_b.valid_params &&
+ irq_a.src_id == irq_b.src_id &&
+ irq_a.src_index == irq_b.src_index &&
+ irq_a.dst_id == irq_b.dst_id &&
+ irq_a.dst_host_irq == irq_b.dst_host_irq &&
+ irq_a.ia_id == irq_b.ia_id &&
+ irq_a.vint == irq_b.vint &&
+ irq_a.global_event == irq_b.global_event &&
+ irq_a.vint_status_bit == irq_b.vint_status_bit &&
+ irq_a.secondary_host == irq_b.secondary_host;
+}
+
/**
* ti_sci_set_irq() - Helper api to configure the irq route between the
* requested source and destination
@@ -1913,15 +1951,39 @@ static int ti_sci_set_irq(const struct ti_sci_handle *handle, u32 valid_params,
u16 dst_host_irq, u16 ia_id, u16 vint,
u16 global_event, u8 vint_status_bit, u8 s_host)
{
+ struct ti_sci_info *info = handle_to_ti_sci_info(handle);
+ struct ti_sci_msg_req_manage_irq *desc;
+ struct ti_sci_irq *irq;
+ int ret;
+
pr_debug("%s: IRQ set with valid_params = 0x%x from src = %d, index = %d, to dst = %d, irq = %d,via ia_id = %d, vint = %d, global event = %d,status_bit = %d\n",
__func__, valid_params, src_id, src_index,
dst_id, dst_host_irq, ia_id, vint, global_event,
vint_status_bit);
- return ti_sci_manage_irq(handle, valid_params, src_id, src_index,
- dst_id, dst_host_irq, ia_id, vint,
- global_event, vint_status_bit, s_host,
- TI_SCI_MSG_SET_IRQ);
+ ret = ti_sci_manage_irq(handle, valid_params, src_id, src_index,
+ dst_id, dst_host_irq, ia_id, vint,
+ global_event, vint_status_bit, s_host,
+ TI_SCI_MSG_SET_IRQ);
+
+ if (ret)
+ return ret;
+
+ irq = kmalloc(sizeof(*irq), GFP_KERNEL);
+ desc = &irq->desc;
+ desc->valid_params = valid_params;
+ desc->src_id = src_id;
+ desc->src_index = src_index;
+ desc->dst_id = dst_id;
+ desc->dst_host_irq = dst_host_irq;
+ desc->ia_id = ia_id;
+ desc->vint = vint;
+ desc->global_event = global_event;
+ desc->vint_status_bit = vint_status_bit;
+ desc->secondary_host = s_host;
+ list_add(&irq->list, &info->irqs.list);
+
+ return ret;
}
/**
@@ -1947,15 +2009,46 @@ static int ti_sci_free_irq(const struct ti_sci_handle *handle, u32 valid_params,
u16 dst_host_irq, u16 ia_id, u16 vint,
u16 global_event, u8 vint_status_bit, u8 s_host)
{
+ struct ti_sci_info *info = handle_to_ti_sci_info(handle);
+ struct ti_sci_msg_req_manage_irq irq_desc;
+ struct ti_sci_irq *this_irq;
+ struct list_head *this;
+ int ret;
+
pr_debug("%s: IRQ release with valid_params = 0x%x from src = %d, index = %d, to dst = %d, irq = %d,via ia_id = %d, vint = %d, global event = %d,status_bit = %d\n",
__func__, valid_params, src_id, src_index,
dst_id, dst_host_irq, ia_id, vint, global_event,
vint_status_bit);
- return ti_sci_manage_irq(handle, valid_params, src_id, src_index,
- dst_id, dst_host_irq, ia_id, vint,
- global_event, vint_status_bit, s_host,
- TI_SCI_MSG_FREE_IRQ);
+ ret = ti_sci_manage_irq(handle, valid_params, src_id, src_index,
+ dst_id, dst_host_irq, ia_id, vint,
+ global_event, vint_status_bit, s_host,
+ TI_SCI_MSG_FREE_IRQ);
+
+ if (ret)
+ return ret;
+
+ irq_desc.valid_params = valid_params;
+ irq_desc.src_id = src_id;
+ irq_desc.src_index = src_index;
+ irq_desc.dst_id = dst_id;
+ irq_desc.dst_host_irq = dst_host_irq;
+ irq_desc.ia_id = ia_id;
+ irq_desc.vint = vint;
+ irq_desc.global_event = global_event;
+ irq_desc.vint_status_bit = vint_status_bit;
+ irq_desc.secondary_host = s_host;
+
+ list_for_each(this, &info->irqs.list) {
+ this_irq = list_entry(this, struct ti_sci_irq, list);
+ if (ti_sci_irqs_equal(irq_desc, this_irq->desc)) {
+ list_del(&this_irq->list);
+ kfree(this_irq);
+ break;
+ }
+ }
+
+ return ret;
}
/**
@@ -3266,6 +3359,37 @@ static int tisci_reboot_handler(struct notifier_block *nb, unsigned long mode,
return NOTIFY_BAD;
}
+static int __maybe_unused ti_sci_resume_noirq(struct device *dev)
+{
+ const struct ti_sci_desc *desc = device_get_match_data(dev);
+ struct ti_sci_info *info = dev_get_drvdata(dev);
+ struct ti_sci_msg_req_manage_irq *irq_desc;
+ struct ti_sci_irq *irq;
+ struct list_head *this;
+ int ret = 0;
+
+ if (!desc->restore_irqs || pm_suspend_target_state != PM_SUSPEND_MEM)
+ return ret;
+
+ /* restore irqs */
+ list_for_each(this, &info->irqs.list) {
+ irq = list_entry(this, struct ti_sci_irq, list);
+ irq_desc = &irq->desc;
+ ret |= ti_sci_manage_irq(&info->handle, irq_desc->valid_params,
+ irq_desc->src_id, irq_desc->src_index,
+ irq_desc->dst_id, irq_desc->dst_host_irq,
+ irq_desc->ia_id, irq_desc->vint,
+ irq_desc->global_event, irq_desc->vint_status_bit,
+ irq_desc->secondary_host, TI_SCI_MSG_SET_IRQ);
+ }
+
+ return ret;
+}
+
+static const struct dev_pm_ops ti_sci_pm_ops = {
+ SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(NULL, ti_sci_resume_noirq)
+};
+
/* Description for K2G */
static const struct ti_sci_desc ti_sci_pmmc_k2g_desc = {
.default_host_id = 2,
@@ -3286,9 +3410,21 @@ static const struct ti_sci_desc ti_sci_pmmc_am654_desc = {
.max_msg_size = 60,
};
+/* Description for J7200 */
+static const struct ti_sci_desc ti_sci_pmmc_j7200_desc = {
+ .default_host_id = 2,
+ /* Conservative duration */
+ .max_rx_timeout_ms = 1000,
+ /* Limited by MBOX_TX_QUEUE_LEN. K2G can handle up to 128 messages! */
+ .max_msgs = 20,
+ .max_msg_size = 64,
+ .restore_irqs = true,
+};
+
static const struct of_device_id ti_sci_of_match[] = {
{.compatible = "ti,k2g-sci", .data = &ti_sci_pmmc_k2g_desc},
{.compatible = "ti,am654-sci", .data = &ti_sci_pmmc_am654_desc},
+ {.compatible = "ti,j7200-sci", .data = &ti_sci_pmmc_j7200_desc},
{ /* Sentinel */ },
};
MODULE_DEVICE_TABLE(of, ti_sci_of_match);
@@ -3415,6 +3551,8 @@ static int ti_sci_probe(struct platform_device *pdev)
info->handle.version.firmware_revision,
info->handle.version.firmware_description);
+ INIT_LIST_HEAD(&info->irqs.list);
+
mutex_lock(&ti_sci_list_mutex);
list_add_tail(&info->node, &ti_sci_list);
mutex_unlock(&ti_sci_list_mutex);
@@ -3435,6 +3573,7 @@ static struct platform_driver ti_sci_driver = {
.name = "ti-sci",
.of_match_table = of_match_ptr(ti_sci_of_match),
.suppress_bind_attrs = true,
+ .pm = &ti_sci_pm_ops,
},
};
module_platform_driver(ti_sci_driver);
--
2.39.2
On j7200, a reserved memory region is needed for TF-A to save its context
during suspend to ram.
In ti-sci suspend, check if the region was defined, if not return an
error.
Signed-off-by: Thomas Richard <[email protected]>
---
drivers/firmware/ti_sci.c | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
index c26ec86a5ff2..deaca17322df 100644
--- a/drivers/firmware/ti_sci.c
+++ b/drivers/firmware/ti_sci.c
@@ -77,6 +77,8 @@ struct ti_sci_xfers_info {
* simultaneously in the system
* @max_msg_size: Maximum size of data per message that can be handled.
* @restore_irqs: Set to true if allocated irqs shall be restored at resume
+ * @lpm_region: Set to true if a reserved memory region is needed for suspend to
+ * ram
*/
struct ti_sci_desc {
u8 default_host_id;
@@ -84,6 +86,7 @@ struct ti_sci_desc {
int max_msgs;
int max_msg_size;
bool restore_irqs;
+ bool lpm_region;
};
/**
@@ -3359,6 +3362,32 @@ static int tisci_reboot_handler(struct notifier_block *nb, unsigned long mode,
return NOTIFY_BAD;
}
+static bool ti_sci_check_lpm_region(void)
+{
+ struct device_node *parent, *node;
+
+ parent = of_find_node_by_path("/reserved-memory");
+ for_each_child_of_node(parent, node) {
+ if (of_node_name_eq(node, "lpm-memory"))
+ return true;
+ }
+
+ return false;
+}
+
+static int __maybe_unused ti_sci_suspend(struct device *dev)
+{
+ const struct ti_sci_desc *desc = device_get_match_data(dev);
+
+ if (pm_suspend_target_state == PM_SUSPEND_MEM &&
+ desc->lpm_region && !ti_sci_check_lpm_region()) {
+ dev_err(dev, "lpm region is required for suspend to ram but is not provided\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static int __maybe_unused ti_sci_resume_noirq(struct device *dev)
{
const struct ti_sci_desc *desc = device_get_match_data(dev);
@@ -3387,6 +3416,7 @@ static int __maybe_unused ti_sci_resume_noirq(struct device *dev)
}
static const struct dev_pm_ops ti_sci_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(ti_sci_suspend, NULL)
SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(NULL, ti_sci_resume_noirq)
};
@@ -3419,6 +3449,7 @@ static const struct ti_sci_desc ti_sci_pmmc_j7200_desc = {
.max_msgs = 20,
.max_msg_size = 64,
.restore_irqs = true,
+ .lpm_region = true,
};
static const struct of_device_id ti_sci_of_match[] = {
@@ -3546,6 +3577,11 @@ static int ti_sci_probe(struct platform_device *pdev)
}
}
+#ifdef CONFIG_PM_SLEEP
+ if (desc->lpm_region && !ti_sci_check_lpm_region())
+ dev_warn(dev, "lpm region is required for suspend to ram but is not provided\n");
+#endif
+
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.39.2
Two memory regions are reserved for suspend to ram on j7200.
One is dedicated to TF-A, it uses this region to save its context during
suspend.
The second region is for R5 SPL, which uses it for its stacks and to store
some firmware images.
These regions are reserved from the linux point of view to avoid any
memory corruption.
Signed-off-by: Thomas Richard <[email protected]>
---
arch/arm64/boot/dts/ti/k3-j7200-som-p0.dtsi | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/arm64/boot/dts/ti/k3-j7200-som-p0.dtsi b/arch/arm64/boot/dts/ti/k3-j7200-som-p0.dtsi
index 5a300d4c8ba0..dc8c9c3e8443 100644
--- a/arch/arm64/boot/dts/ti/k3-j7200-som-p0.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j7200-som-p0.dtsi
@@ -79,6 +79,16 @@ rtos_ipc_memory_region: ipc-memories@a4000000 {
alignment = <0x1000>;
no-map;
};
+
+ lpm_r5_spl_ctx_ddr: lpm-memory@a5000000 {
+ reg = <0x00 0xa5000000 0x00 0x1000000>;
+ no-map;
+ };
+
+ lpm_bl31_ctx_ddr: lpm-memory@a6000000 {
+ reg = <0x00 0xa6000000 0x00 0x20000>;
+ no-map;
+ };
};
};
--
2.39.2
On Wed, Nov 29, 2023 at 04:31:17PM +0100, Thomas Richard wrote:
> On j7200, during suspend to ram the soc is powered-off.
> At resume requested irqs shall be restored which is a different behavior
> from other platforms.
>
> Signed-off-by: Thomas Richard <[email protected]>
Acked-by: Conor Dooley <[email protected]>
Cheers,
Conor.
On Wed, Nov 29, 2023 at 04:31:19PM +0100, Thomas Richard wrote:
> On j7200, suspend to ram poweroff the SOC.
> This compatible restores irqs at resume.
>
> Signed-off-by: Thomas Richard <[email protected]>
> ---
> arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
> index 467970fab3a0..d8dc1421e75e 100644
> --- a/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
> @@ -7,7 +7,7 @@
>
> &cbass_mcu_wakeup {
> dmsc: system-controller@44083000 {
> - compatible = "ti,k2g-sci";
> + compatible = "ti,j7200-sci", "ti,k2g-sci";
This is not what your dt-binding change allows. Did you test this with
dtbs_check?
On Wed, Nov 29, 2023 at 03:34:20PM +0000, Conor Dooley wrote:
> On Wed, Nov 29, 2023 at 04:31:17PM +0100, Thomas Richard wrote:
> > On j7200, during suspend to ram the soc is powered-off.
> > At resume requested irqs shall be restored which is a different behavior
> > from other platforms.
> >
> > Signed-off-by: Thomas Richard <[email protected]>
>
> Acked-by: Conor Dooley <[email protected]>
Un-Acked. Your dts patch contradicts this one.
Is the programming model compatible with the existing devices? To be
compatible, the existing device only need to support a compatible subset
of behaviours.
If so, this patch is wrong. If not, then the dts one is.
Thanks,
Conor.
On 11/29/23 9:31 AM, Thomas Richard wrote:
> On j7200, during suspend to ram the soc is powered-off.
> At resume requested irqs shall be restored which is a different behavior
> from other platforms.
Why is J7200 different? All K3 can/will support off mode suspend
to RAM. The only difference is you are adding support for it to this
one SoC first. You are describing a software behavior, not hardware.
Using a compatible to describe if a SW feature is enabled is not a
correct use of DT.
Andrew
>
> Signed-off-by: Thomas Richard <[email protected]>
> ---
> Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> index c24ad0968f3e..53d9c58dcd70 100644
> --- a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> +++ b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> @@ -40,6 +40,8 @@ properties:
> - description: System controller on TI AM654 SoC
> items:
> - const: ti,am654-sci
> + - description: System controller on TI J7200 SOC
> + - const: ti,j7200-sci
>
> reg-names:
> description: |
>
On Wed, Nov 29, 2023 at 03:38:04PM +0000, Conor Dooley wrote:
> On Wed, Nov 29, 2023 at 03:34:20PM +0000, Conor Dooley wrote:
> > On Wed, Nov 29, 2023 at 04:31:17PM +0100, Thomas Richard wrote:
> > > On j7200, during suspend to ram the soc is powered-off.
> > > At resume requested irqs shall be restored which is a different behavior
> > > from other platforms.
> > >
> > > Signed-off-by: Thomas Richard <[email protected]>
> >
> > Acked-by: Conor Dooley <[email protected]>
>
> Un-Acked. Your dts patch contradicts this one.
>
> Is the programming model compatible with the existing devices? To be
> compatible, the existing device only need to support a compatible subset
> of behaviours.
> If so, this patch is wrong. If not, then the dts one is.
Given Andrew's response, it looks like the dts patch is the correct one
of the two, and this patch should document the k2g as a fallback for the
jh7200.
Cheers,
Conor.
On 16:31-20231129, Thomas Richard wrote:
> On j7200, during suspend to ram the soc is powered-off.
> At resume requested irqs shall be restored which is a different behavior
> from other platforms.
>
> Signed-off-by: Thomas Richard <[email protected]>
> ---
> Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> index c24ad0968f3e..53d9c58dcd70 100644
> --- a/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> +++ b/Documentation/devicetree/bindings/arm/keystone/ti,sci.yaml
> @@ -40,6 +40,8 @@ properties:
> - description: System controller on TI AM654 SoC
> items:
> - const: ti,am654-sci
> + - description: System controller on TI J7200 SOC
> + - const: ti,j7200-sci
>
> reg-names:
> description: |
>
> --
> 2.39.2
>
Sorry, but I don't see why this is any different for all K3 devices.
they all follow the same pattern of usage.
Also, constraints you are speaking off is also present even for
am654-sci. just handle this in the driver.
--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D
On 16:31-20231129, Thomas Richard wrote:
> On j7200, during suspend to ram, the SoC is powered-off.
A53 and main domain is powered off in the case of J7200 - there are
different nuanced suspend-to-ram scenarios involved which the system
control firmware manages for us (Linux).
> The irqs configuration is lost, so it shall be restored at resume.
> The ti-sci has an internal list of irqs updated at each set/free.
> All irqs in this list are restored at resume.
This I don't disagree - but apply to all K3 SoCs. But what I do disagree
is that this is sufficient enough to have suspend-to-ram actually
functional. there are additional handshakes that are needed and the
driver just has a single suspend-resume handler..
git log --oneline drivers/firmware/ti_sci.c tells us that we have
already had a bunch of introduction, revert of suspend-resume handler
(too much for my comfort). I wish I didn't have to say this, but I
guess having seen TFA[1], U-boot[2] series (not that the kernel patch
discussion is independent of TFA/U-boot discussions), and having known
that the AM62x team is also working towards a suspend-resume scheme
(CCying Kevin and AM62x folks) - Since we all share a single driver,
I suggest we get together and draw through the arch of how this flow
will look like and not create additional confusion in the public list
on this till we have a settled down view (also the reason I haven't
picked up am62x patches so far either).
For now, I am going to punt this series until I see clarity overall
and I am assured that we have a relatively final path for all users of
TISCI driver/lib/infra.
[1] https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/23992/7
[2] https://lore.kernel.org/u-boot/20231109140738.GF6601@bill-the-cat/
--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D