From c2b0390132edffad1b52e4e84f797343b10f5ef2 Mon Sep 17 00:00:00 2001 From: Yihao Han Date: Thu, 3 Mar 2022 04:44:44 -0800 Subject: [PATCH 01/15] soc: ti: wkup_m3_ipc: fix platform_get_irq.cocci warning Remove dev_err() messages after platform_get_irq*() failures. platform_get_irq() already prints an error. Generated by: scripts/coccinelle/api/platform_get_irq.cocci Signed-off-by: Yihao Han Signed-off-by: Nishanth Menon Link: https://lore.kernel.org/r/20220303124444.3373-1-hanyihao@vivo.com --- drivers/soc/ti/wkup_m3_ipc.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/soc/ti/wkup_m3_ipc.c b/drivers/soc/ti/wkup_m3_ipc.c index 2f03ced0f411..f145e65041fd 100644 --- a/drivers/soc/ti/wkup_m3_ipc.c +++ b/drivers/soc/ti/wkup_m3_ipc.c @@ -450,10 +450,8 @@ static int wkup_m3_ipc_probe(struct platform_device *pdev) return PTR_ERR(m3_ipc->ipc_mem_base); irq = platform_get_irq(pdev, 0); - if (irq < 0) { - dev_err(&pdev->dev, "no irq resource\n"); + if (irq < 0) return irq; - } ret = devm_request_irq(dev, irq, wkup_m3_txev_handler, 0, "wkup_m3_txev", m3_ipc); From d281a982c2693c6a7bffaa1ae1ff3b400d6e6c74 Mon Sep 17 00:00:00 2001 From: Jakob Koschel Date: Thu, 24 Mar 2022 08:25:03 +0100 Subject: [PATCH 02/15] soc: ti: replace usage of found with dedicated list iterator variable To move the list iterator variable into the list_for_each_entry_*() macro in the future it should be avoided to use the list iterator variable after the loop body. To *never* use the list iterator variable after the loop it was concluded to use a separate iterator variable instead of a found boolean [1]. This removes the need to use a found variable and simply checking if the variable was set, can determine if the break/goto was hit. Signed-off-by: Jakob Koschel Signed-off-by: Nishanth Menon Reviewed-by: Vignesh Raghavendra Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/ Link: https://lore.kernel.org/r/20220324072503.63244-1-jakobkoschel@gmail.com --- drivers/soc/ti/knav_dma.c | 26 ++++++++++++-------------- drivers/soc/ti/knav_qmss_queue.c | 16 +++++++--------- 2 files changed, 19 insertions(+), 23 deletions(-) diff --git a/drivers/soc/ti/knav_dma.c b/drivers/soc/ti/knav_dma.c index 700d8eecd8c4..7e126a73e56e 100644 --- a/drivers/soc/ti/knav_dma.c +++ b/drivers/soc/ti/knav_dma.c @@ -415,9 +415,8 @@ static int of_channel_match_helper(struct device_node *np, const char *name, void *knav_dma_open_channel(struct device *dev, const char *name, struct knav_dma_cfg *config) { - struct knav_dma_chan *chan; - struct knav_dma_device *dma; - bool found = false; + struct knav_dma_device *dma = NULL, *iter1; + struct knav_dma_chan *chan = NULL, *iter2; int chan_num = -1; const char *instance; @@ -444,33 +443,32 @@ void *knav_dma_open_channel(struct device *dev, const char *name, } /* Look for correct dma instance */ - list_for_each_entry(dma, &kdev->list, list) { - if (!strcmp(dma->name, instance)) { - found = true; + list_for_each_entry(iter1, &kdev->list, list) { + if (!strcmp(iter1->name, instance)) { + dma = iter1; break; } } - if (!found) { + if (!dma) { dev_err(kdev->dev, "No DMA instance with name %s\n", instance); return (void *)-EINVAL; } /* Look for correct dma channel from dma instance */ - found = false; - list_for_each_entry(chan, &dma->chan_list, list) { + list_for_each_entry(iter2, &dma->chan_list, list) { if (config->direction == DMA_MEM_TO_DEV) { - if (chan->channel == chan_num) { - found = true; + if (iter2->channel == chan_num) { + chan = iter2; break; } } else { - if (chan->flow == chan_num) { - found = true; + if (iter2->flow == chan_num) { + chan = iter2; break; } } } - if (!found) { + if (!chan) { dev_err(kdev->dev, "channel %d is not in DMA %s\n", chan_num, instance); return (void *)-EINVAL; diff --git a/drivers/soc/ti/knav_qmss_queue.c b/drivers/soc/ti/knav_qmss_queue.c index 2ac3856b8d42..4dbaa8c3636c 100644 --- a/drivers/soc/ti/knav_qmss_queue.c +++ b/drivers/soc/ti/knav_qmss_queue.c @@ -758,10 +758,9 @@ void *knav_pool_create(const char *name, int num_desc, int region_id) { struct knav_region *reg_itr, *region = NULL; - struct knav_pool *pool, *pi; + struct knav_pool *pool, *pi = NULL, *iter; struct list_head *node; unsigned last_offset; - bool slot_found; int ret; if (!kdev) @@ -816,18 +815,17 @@ void *knav_pool_create(const char *name, * the request */ last_offset = 0; - slot_found = false; node = ®ion->pools; - list_for_each_entry(pi, ®ion->pools, region_inst) { - if ((pi->region_offset - last_offset) >= num_desc) { - slot_found = true; + list_for_each_entry(iter, ®ion->pools, region_inst) { + if ((iter->region_offset - last_offset) >= num_desc) { + pi = iter; break; } - last_offset = pi->region_offset + pi->num_desc; + last_offset = iter->region_offset + iter->num_desc; } - node = &pi->region_inst; - if (slot_found) { + if (pi) { + node = &pi->region_inst; pool->region = region; pool->num_desc = num_desc; pool->region_offset = last_offset; From f25d2b2b554133b935e72c61deb40d82287a6f75 Mon Sep 17 00:00:00 2001 From: Minghao Chi Date: Fri, 8 Apr 2022 08:08:53 +0000 Subject: [PATCH 03/15] soc: ti: pruss: using pm_runtime_resume_and_get instead of pm_runtime_get_sync Using pm_runtime_resume_and_get is more appropriate for simplifing code Reported-by: Zeal Robot Signed-off-by: Minghao Chi Signed-off-by: Nishanth Menon Reviewed-by: Dave Gerlach Link: https://lore.kernel.org/r/20220408080853.2494292-1-chi.minghao@zte.com.cn --- drivers/soc/ti/pruss.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/soc/ti/pruss.c b/drivers/soc/ti/pruss.c index b36779309e49..0e4ba0f89533 100644 --- a/drivers/soc/ti/pruss.c +++ b/drivers/soc/ti/pruss.c @@ -279,10 +279,9 @@ static int pruss_probe(struct platform_device *pdev) platform_set_drvdata(pdev, pruss); pm_runtime_enable(dev); - ret = pm_runtime_get_sync(dev); + ret = pm_runtime_resume_and_get(dev); if (ret < 0) { dev_err(dev, "couldn't enable module\n"); - pm_runtime_put_noidle(dev); goto rpm_disable; } From cabfa5b46573b4e5ded52b4296a831745c6d32b5 Mon Sep 17 00:00:00 2001 From: "Minghao Chi (CGEL ZTE)" Date: Mon, 7 Mar 2022 03:37:36 +0000 Subject: [PATCH 04/15] soc: ti: omap_prm: Use of_device_get_match_data() Since omap_prm_id_table all have (and expected to have) data entries, use of_device_get_match_data() to simplify the code. Reported-by: Zeal Robot Signed-off-by: Minghao Chi (CGEL ZTE) Signed-off-by: Nishanth Menon Link: https://lore.kernel.org/r/20220307033736.2075221-1-chi.minghao@zte.com.cn --- drivers/soc/ti/omap_prm.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/soc/ti/omap_prm.c b/drivers/soc/ti/omap_prm.c index f32e1cbbe8c5..913b964374a4 100644 --- a/drivers/soc/ti/omap_prm.c +++ b/drivers/soc/ti/omap_prm.c @@ -941,23 +941,20 @@ static int omap_prm_probe(struct platform_device *pdev) struct resource *res; const struct omap_prm_data *data; struct omap_prm *prm; - const struct of_device_id *match; int ret; res = platform_get_resource(pdev, IORESOURCE_MEM, 0); if (!res) return -ENODEV; - match = of_match_device(omap_prm_id_table, &pdev->dev); - if (!match) + data = of_device_get_match_data(&pdev->dev); + if (!data) return -ENOTSUPP; prm = devm_kzalloc(&pdev->dev, sizeof(*prm), GFP_KERNEL); if (!prm) return -ENOMEM; - data = match->data; - while (data->base != res->start) { if (!data->base) return -EINVAL; From ba56291e297d28aa6eb82c5c1964fae2d7594746 Mon Sep 17 00:00:00 2001 From: QintaoShen Date: Thu, 24 Mar 2022 15:44:03 +0800 Subject: [PATCH 05/15] soc: ti: ti_sci_pm_domains: Check for null return of devm_kcalloc The allocation funciton devm_kcalloc may fail and return a null pointer, which would cause a null-pointer dereference later. It might be better to check it and directly return -ENOMEM just like the usage of devm_kcalloc in previous code. Signed-off-by: QintaoShen Signed-off-by: Nishanth Menon Link: https://lore.kernel.org/r/1648107843-29077-1-git-send-email-unSimple1993@163.com --- drivers/soc/ti/ti_sci_pm_domains.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/soc/ti/ti_sci_pm_domains.c b/drivers/soc/ti/ti_sci_pm_domains.c index 8afb3f45d263..a33ec7eaf23d 100644 --- a/drivers/soc/ti/ti_sci_pm_domains.c +++ b/drivers/soc/ti/ti_sci_pm_domains.c @@ -183,6 +183,8 @@ static int ti_sci_pm_domain_probe(struct platform_device *pdev) devm_kcalloc(dev, max_id + 1, sizeof(*pd_provider->data.domains), GFP_KERNEL); + if (!pd_provider->data.domains) + return -ENOMEM; pd_provider->data.num_domains = max_id + 1; pd_provider->data.xlate = ti_sci_pd_xlate; From a6af504184c981efd253f986e6fc54db57b1d39f Mon Sep 17 00:00:00 2001 From: Philipp Zabel Date: Mon, 4 Apr 2022 11:45:00 +0200 Subject: [PATCH 06/15] reset: ti-sci: Allow building under COMPILE_TEST Since commit 043cfff99a18 ("firmware: ti_sci: Fix compilation failure when CONFIG_TI_SCI_PROTOCOL is not defined") it is possible to build reset-ti-sci under CONFIG_COMPILE_TEST. Signed-off-by: Philipp Zabel Signed-off-by: Nishanth Menon Link: https://lore.kernel.org/r/20220404094500.2708816-1-p.zabel@pengutronix.de --- drivers/reset/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig index b496028b6bfa..d6cb52a1dce7 100644 --- a/drivers/reset/Kconfig +++ b/drivers/reset/Kconfig @@ -240,7 +240,7 @@ config RESET_SUNXI config RESET_TI_SCI tristate "TI System Control Interface (TI-SCI) reset driver" - depends on TI_SCI_PROTOCOL + depends on TI_SCI_PROTOCOL || COMPILE_TEST help This enables the reset driver support over TI System Control Interface available on some new TI's SoCs. If you wish to use reset resources From d3e3116f253591a473873fab8363ecb998ddde13 Mon Sep 17 00:00:00 2001 From: Minghao Chi Date: Tue, 12 Apr 2022 08:29:23 +0000 Subject: [PATCH 07/15] soc: ti: knav_dma: Use pm_runtime_resume_and_get instead of pm_runtime_get_sync Using pm_runtime_resume_and_get is more appropriate for simplifying code. Reported-by: Zeal Robot Signed-off-by: Minghao Chi Signed-off-by: Nishanth Menon Reviewed-by: Grygorii Strashko Link: https://lore.kernel.org/r/20220412082923.2532649-1-chi.minghao@zte.com.cn --- drivers/soc/ti/knav_dma.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/soc/ti/knav_dma.c b/drivers/soc/ti/knav_dma.c index 7e126a73e56e..d756591de973 100644 --- a/drivers/soc/ti/knav_dma.c +++ b/drivers/soc/ti/knav_dma.c @@ -745,9 +745,8 @@ static int knav_dma_probe(struct platform_device *pdev) INIT_LIST_HEAD(&kdev->list); pm_runtime_enable(kdev->dev); - ret = pm_runtime_get_sync(kdev->dev); + ret = pm_runtime_resume_and_get(kdev->dev); if (ret < 0) { - pm_runtime_put_noidle(kdev->dev); dev_err(kdev->dev, "unable to enable pktdma, err %d\n", ret); goto err_pm_disable; } From 12eeb74925da70eb39d90abead9de9793be3d4c8 Mon Sep 17 00:00:00 2001 From: Minghao Chi Date: Mon, 18 Apr 2022 06:29:55 +0000 Subject: [PATCH 08/15] soc: ti: knav_qmss_queue: Use pm_runtime_resume_and_get instead of pm_runtime_get_sync Using pm_runtime_resume_and_get is more appropriate for simplifying code. Reported-by: Zeal Robot Signed-off-by: Minghao Chi Signed-off-by: Nishanth Menon Link: https://lore.kernel.org/r/20220418062955.2557949-1-chi.minghao@zte.com.cn --- drivers/soc/ti/knav_qmss_queue.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/soc/ti/knav_qmss_queue.c b/drivers/soc/ti/knav_qmss_queue.c index 4dbaa8c3636c..30612719e2f1 100644 --- a/drivers/soc/ti/knav_qmss_queue.c +++ b/drivers/soc/ti/knav_qmss_queue.c @@ -1783,9 +1783,8 @@ static int knav_queue_probe(struct platform_device *pdev) INIT_LIST_HEAD(&kdev->pdsps); pm_runtime_enable(&pdev->dev); - ret = pm_runtime_get_sync(&pdev->dev); + ret = pm_runtime_resume_and_get(&pdev->dev); if (ret < 0) { - pm_runtime_put_noidle(&pdev->dev); dev_err(dev, "Failed to enable QMSS\n"); return ret; } From f226041424cf87245d39a1b2dfae304308b36b6b Mon Sep 17 00:00:00 2001 From: Dave Gerlach Date: Sat, 9 Apr 2022 14:12:15 -0700 Subject: [PATCH 09/15] soc: ti: wkup_m3_ipc: Add support for toggling VTT regulator Some boards like the AM335x EVM-SK and AM437x GP EVM provide software control via a GPIO pin to toggle the DDR VTT regulator to reduce power consumption in low power states. The VTT regulator should be disabled after enabling self-refresh on suspend, and should be enabled before disabling self-refresh on resume. This is to allow proper self-refresh entry/exit commands to be transmitted to the memory. The "ti,vtt-gpio-pin" device tree property in the wkup_m3_ipc node specifies which GPIO pin to use. This property is communicated to the Wakeup Cortex M3 co-processor where the actual toggling of the GPIO pin happens in CM3 firmware [1]. Please note that the GPIO pin must be on the GPIO0 module as that module is in the wakeup power domain. [1] https://git.ti.com/cgit/processor-firmware/ti-amx3-cm3-pm-firmware/tree/src/pm_services/ddr.c?h=08.02.00.006#n190 Signed-off-by: Dave Gerlach Signed-off-by: Keerthy [dfustini: remove the unnecessary "ti,needs-vtt-toggle" property] Signed-off-by: Drew Fustini Signed-off-by: Nishanth Menon Link: https://lore.kernel.org/r/20220409211215.2529387-3-dfustini@baylibre.com --- drivers/soc/ti/wkup_m3_ipc.c | 26 ++++++++++++++++++++++++-- include/linux/wkup_m3_ipc.h | 1 + 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/drivers/soc/ti/wkup_m3_ipc.c b/drivers/soc/ti/wkup_m3_ipc.c index f145e65041fd..48659758228e 100644 --- a/drivers/soc/ti/wkup_m3_ipc.c +++ b/drivers/soc/ti/wkup_m3_ipc.c @@ -40,6 +40,13 @@ #define M3_FW_VERSION_MASK 0xffff #define M3_WAKE_SRC_MASK 0xff +#define IPC_MEM_TYPE_SHIFT (0x0) +#define IPC_MEM_TYPE_MASK (0x7 << 0) +#define IPC_VTT_STAT_SHIFT (0x3) +#define IPC_VTT_STAT_MASK (0x1 << 3) +#define IPC_VTT_GPIO_PIN_SHIFT (0x4) +#define IPC_VTT_GPIO_PIN_MASK (0x3f << 4) + #define M3_STATE_UNKNOWN 0 #define M3_STATE_RESET 1 #define M3_STATE_INITED 2 @@ -215,6 +222,12 @@ static int wkup_m3_is_available(struct wkup_m3_ipc *m3_ipc) (m3_ipc->state != M3_STATE_UNKNOWN)); } +static void wkup_m3_set_vtt_gpio(struct wkup_m3_ipc *m3_ipc, int gpio) +{ + m3_ipc->vtt_conf = (1 << IPC_VTT_STAT_SHIFT) | + (gpio << IPC_VTT_GPIO_PIN_SHIFT); +} + /* Public functions */ /** * wkup_m3_set_mem_type - Pass wkup_m3 which type of memory is in use @@ -294,7 +307,8 @@ static int wkup_m3_prepare_low_power(struct wkup_m3_ipc *m3_ipc, int state) /* Program each required IPC register then write defaults to others */ wkup_m3_ctrl_ipc_write(m3_ipc, m3_ipc->resume_addr, 0); wkup_m3_ctrl_ipc_write(m3_ipc, m3_power_state, 1); - wkup_m3_ctrl_ipc_write(m3_ipc, m3_ipc->mem_type, 4); + wkup_m3_ctrl_ipc_write(m3_ipc, m3_ipc->mem_type | + m3_ipc->vtt_conf, 4); wkup_m3_ctrl_ipc_write(m3_ipc, DS_IPC_DEFAULT, 2); wkup_m3_ctrl_ipc_write(m3_ipc, DS_IPC_DEFAULT, 3); @@ -433,12 +447,13 @@ static int wkup_m3_rproc_boot_thread(void *arg) static int wkup_m3_ipc_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; - int irq, ret; + int irq, ret, temp; phandle rproc_phandle; struct rproc *m3_rproc; struct resource *res; struct task_struct *task; struct wkup_m3_ipc *m3_ipc; + struct device_node *np = dev->of_node; m3_ipc = devm_kzalloc(dev, sizeof(*m3_ipc), GFP_KERNEL); if (!m3_ipc) @@ -494,6 +509,13 @@ static int wkup_m3_ipc_probe(struct platform_device *pdev) m3_ipc->ops = &ipc_ops; + if (!of_property_read_u32(np, "ti,vtt-gpio-pin", &temp)) { + if (temp >= 0 && temp <= 31) + wkup_m3_set_vtt_gpio(m3_ipc, temp); + else + dev_warn(dev, "Invalid VTT GPIO(%d) pin\n", temp); + } + /* * Wait for firmware loading completion in a thread so we * can boot the wkup_m3 as soon as it's ready without holding diff --git a/include/linux/wkup_m3_ipc.h b/include/linux/wkup_m3_ipc.h index 3f496967b538..2bc52c6381d5 100644 --- a/include/linux/wkup_m3_ipc.h +++ b/include/linux/wkup_m3_ipc.h @@ -33,6 +33,7 @@ struct wkup_m3_ipc { int mem_type; unsigned long resume_addr; + int vtt_conf; int state; struct completion sync_complete; From b9e8a7d950ffed4cdd81e6457cfb8049227da2d1 Mon Sep 17 00:00:00 2001 From: Dave Gerlach Date: Tue, 12 Apr 2022 14:21:38 -0500 Subject: [PATCH 10/15] firmware: ti_sci: Switch transport to polled mode during system suspend During system suspend it is completely valid for devices to invoke TISCI commands during the noirq phase of the suspend path. Specifically this will always be seen for devices that define a power-domains DT property and make use of the ti_sci_pm_domains genpd implementation. The genpd_finish_suspend call will power off devices during the noirq phase, which will invoke TISCI. In order to support this, the ti_sci driver must switch to not use wait_for_completion_timeout during suspend, but instead rely on a manual check for if the completion is not yet done, and proceed only if this is the case. Signed-off-by: Dave Gerlach Signed-off-by: Nishanth Menon Link: https://lore.kernel.org/r/20220412192138.31189-1-d-gerlach@ti.com --- drivers/firmware/ti_sci.c | 61 +++++++++++++++++++++++++++++++++++---- 1 file changed, 55 insertions(+), 6 deletions(-) diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c index 4697edc125b1..ebc32bbd9b83 100644 --- a/drivers/firmware/ti_sci.c +++ b/drivers/firmware/ti_sci.c @@ -2,7 +2,7 @@ /* * Texas Instruments System Control Interface Protocol Driver * - * Copyright (C) 2015-2016 Texas Instruments Incorporated - https://www.ti.com/ + * Copyright (C) 2015-2022 Texas Instruments Incorporated - https://www.ti.com/ * Nishanth Menon */ @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -96,6 +97,7 @@ struct ti_sci_desc { * @node: list head * @host_id: Host ID * @users: Number of users of this instance + * @is_suspending: Flag set to indicate in suspend path. */ struct ti_sci_info { struct device *dev; @@ -114,7 +116,7 @@ struct ti_sci_info { u8 host_id; /* protected by ti_sci_list_mutex */ int users; - + bool is_suspending; }; #define cl_to_ti_sci_info(c) container_of(c, struct ti_sci_info, cl) @@ -349,6 +351,8 @@ static struct ti_sci_xfer *ti_sci_get_one_xfer(struct ti_sci_info *info, hdr = (struct ti_sci_msg_hdr *)xfer->tx_message.buf; xfer->tx_message.len = tx_message_size; + xfer->tx_message.chan_rx = info->chan_rx; + xfer->tx_message.timeout_rx_ms = info->desc->max_rx_timeout_ms; xfer->rx_len = (u8)rx_message_size; reinit_completion(&xfer->done); @@ -406,6 +410,7 @@ static inline int ti_sci_do_xfer(struct ti_sci_info *info, int ret; int timeout; struct device *dev = info->dev; + bool done_state = true; ret = mbox_send_message(info->chan_tx, &xfer->tx_message); if (ret < 0) @@ -413,13 +418,27 @@ static inline int ti_sci_do_xfer(struct ti_sci_info *info, ret = 0; - /* And we wait for the response. */ - timeout = msecs_to_jiffies(info->desc->max_rx_timeout_ms); - if (!wait_for_completion_timeout(&xfer->done, timeout)) { + if (!info->is_suspending) { + /* And we wait for the response. */ + timeout = msecs_to_jiffies(info->desc->max_rx_timeout_ms); + if (!wait_for_completion_timeout(&xfer->done, timeout)) + ret = -ETIMEDOUT; + } else { + /* + * If we are suspending, we cannot use wait_for_completion_timeout + * during noirq phase, so we must manually poll the completion. + */ + ret = read_poll_timeout_atomic(try_wait_for_completion, done_state, + true, 1, + info->desc->max_rx_timeout_ms * 1000, + false, &xfer->done); + } + + if (ret == -ETIMEDOUT || !done_state) { dev_err(dev, "Mbox timedout in resp(caller: %pS)\n", (void *)_RET_IP_); - ret = -ETIMEDOUT; } + /* * NOTE: we might prefer not to need the mailbox ticker to manage the * transfer queueing since the protocol layer queues things by itself. @@ -3264,6 +3283,35 @@ static int tisci_reboot_handler(struct notifier_block *nb, unsigned long mode, return NOTIFY_BAD; } +static void ti_sci_set_is_suspending(struct ti_sci_info *info, bool is_suspending) +{ + info->is_suspending = is_suspending; +} + +static int ti_sci_suspend(struct device *dev) +{ + struct ti_sci_info *info = dev_get_drvdata(dev); + /* + * 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 + * from the noirq phase of suspend. + */ + ti_sci_set_is_suspending(info, true); + + return 0; +} + +static int ti_sci_resume(struct device *dev) +{ + struct ti_sci_info *info = dev_get_drvdata(dev); + + ti_sci_set_is_suspending(info, false); + + return 0; +} + +static DEFINE_SIMPLE_DEV_PM_OPS(ti_sci_pm_ops, ti_sci_suspend, ti_sci_resume); + /* Description for K2G */ static const struct ti_sci_desc ti_sci_pmmc_k2g_desc = { .default_host_id = 2, @@ -3472,6 +3520,7 @@ static struct platform_driver ti_sci_driver = { .driver = { .name = "ti-sci", .of_match_table = of_match_ptr(ti_sci_of_match), + .pm = &ti_sci_pm_ops, }, }; module_platform_driver(ti_sci_driver); From 2b7042500cab7952bdbf4fe4a84de8712b418c36 Mon Sep 17 00:00:00 2001 From: Minghao Chi Date: Mon, 18 Apr 2022 06:30:59 +0000 Subject: [PATCH 11/15] soc: ti: pm33xx: using pm_runtime_resume_and_get instead of pm_runtime_get_sync Using pm_runtime_resume_and_get is more appropriate for simplifing code Reported-by: Zeal Robot Signed-off-by: Minghao Chi Signed-off-by: Nishanth Menon Reviewed-by: Tony Lindgren Link: https://lore.kernel.org/r/20220418063059.2558074-1-chi.minghao@zte.com.cn --- drivers/soc/ti/pm33xx.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/soc/ti/pm33xx.c b/drivers/soc/ti/pm33xx.c index 7bab4bbaf02d..ce09c42eaed2 100644 --- a/drivers/soc/ti/pm33xx.c +++ b/drivers/soc/ti/pm33xx.c @@ -555,11 +555,9 @@ static int am33xx_pm_probe(struct platform_device *pdev) #endif /* CONFIG_SUSPEND */ pm_runtime_enable(dev); - ret = pm_runtime_get_sync(dev); - if (ret < 0) { - pm_runtime_put_noidle(dev); + ret = pm_runtime_resume_and_get(dev); + if (ret < 0) goto err_pm_runtime_disable; - } ret = pm_ops->init(am33xx_do_sram_idle); if (ret) { From d4c41d32cf8af10e4c0a35a6d4995de253b54df6 Mon Sep 17 00:00:00 2001 From: Haowen Bai Date: Sun, 24 Apr 2022 10:05:43 +0800 Subject: [PATCH 12/15] soc: ti: knav_qmss_queue: Use IS_ERR instead of IS_ERR_OR_NULL when checking knav_queue_open() result As the usage of knav_queue_open(): * Returns a handle to the open hardware queue if successful. Use IS_ERR() * to check the returned value for error codes. It will only return error codes, not null. Signed-off-by: Haowen Bai Signed-off-by: Nishanth Menon Link: https://lore.kernel.org/r/1650765944-20170-1-git-send-email-baihaowen@meizu.com --- drivers/soc/ti/knav_qmss_queue.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/soc/ti/knav_qmss_queue.c b/drivers/soc/ti/knav_qmss_queue.c index 30612719e2f1..92af7d1b6f5b 100644 --- a/drivers/soc/ti/knav_qmss_queue.c +++ b/drivers/soc/ti/knav_qmss_queue.c @@ -789,7 +789,7 @@ void *knav_pool_create(const char *name, } pool->queue = knav_queue_open(name, KNAV_QUEUE_GP, 0); - if (IS_ERR_OR_NULL(pool->queue)) { + if (IS_ERR(pool->queue)) { dev_err(kdev->dev, "failed to open queue for pool(%s), error %ld\n", name, PTR_ERR(pool->queue)); From 1dcbae86ee669bdb0338954cd0136863f5c96c0a Mon Sep 17 00:00:00 2001 From: Dave Gerlach Date: Thu, 14 Apr 2022 12:27:24 -0700 Subject: [PATCH 13/15] soc: ti: wkup_m3_ipc: Add support for IO Isolation AM43xx support isolation of the IOs so that control is taken from the peripheral they are connected to and overridden by values present in the CTRL_CONF_* registers for the pad in the control module. The actual toggling happens from the wkup_m3, so use a DT property from the wkup_m3_ipc node to allow the PM code to communicate the necessity for placing the IOs into isolation to the firmware. Signed-off-by: Dave Gerlach Signed-off-by: Keerthy Signed-off-by: Drew Fustini Signed-off-by: Nishanth Menon Link: https://lore.kernel.org/r/20220414192722.2978837-3-dfustini@baylibre.com --- drivers/soc/ti/wkup_m3_ipc.c | 14 ++++++++++++-- include/linux/wkup_m3_ipc.h | 1 + 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/drivers/soc/ti/wkup_m3_ipc.c b/drivers/soc/ti/wkup_m3_ipc.c index 48659758228e..2f2a0ab32aff 100644 --- a/drivers/soc/ti/wkup_m3_ipc.c +++ b/drivers/soc/ti/wkup_m3_ipc.c @@ -46,6 +46,8 @@ #define IPC_VTT_STAT_MASK (0x1 << 3) #define IPC_VTT_GPIO_PIN_SHIFT (0x4) #define IPC_VTT_GPIO_PIN_MASK (0x3f << 4) +#define IPC_IO_ISOLATION_STAT_SHIFT (10) +#define IPC_IO_ISOLATION_STAT_MASK (0x1 << 10) #define M3_STATE_UNKNOWN 0 #define M3_STATE_RESET 1 @@ -228,6 +230,11 @@ static void wkup_m3_set_vtt_gpio(struct wkup_m3_ipc *m3_ipc, int gpio) (gpio << IPC_VTT_GPIO_PIN_SHIFT); } +static void wkup_m3_set_io_isolation(struct wkup_m3_ipc *m3_ipc) +{ + m3_ipc->isolation_conf = (1 << IPC_IO_ISOLATION_STAT_SHIFT); +} + /* Public functions */ /** * wkup_m3_set_mem_type - Pass wkup_m3 which type of memory is in use @@ -308,8 +315,8 @@ static int wkup_m3_prepare_low_power(struct wkup_m3_ipc *m3_ipc, int state) wkup_m3_ctrl_ipc_write(m3_ipc, m3_ipc->resume_addr, 0); wkup_m3_ctrl_ipc_write(m3_ipc, m3_power_state, 1); wkup_m3_ctrl_ipc_write(m3_ipc, m3_ipc->mem_type | - m3_ipc->vtt_conf, 4); - + m3_ipc->vtt_conf | + m3_ipc->isolation_conf, 4); wkup_m3_ctrl_ipc_write(m3_ipc, DS_IPC_DEFAULT, 2); wkup_m3_ctrl_ipc_write(m3_ipc, DS_IPC_DEFAULT, 3); wkup_m3_ctrl_ipc_write(m3_ipc, DS_IPC_DEFAULT, 5); @@ -516,6 +523,9 @@ static int wkup_m3_ipc_probe(struct platform_device *pdev) dev_warn(dev, "Invalid VTT GPIO(%d) pin\n", temp); } + if (of_find_property(np, "ti,set-io-isolation", NULL)) + wkup_m3_set_io_isolation(m3_ipc); + /* * Wait for firmware loading completion in a thread so we * can boot the wkup_m3 as soon as it's ready without holding diff --git a/include/linux/wkup_m3_ipc.h b/include/linux/wkup_m3_ipc.h index 2bc52c6381d5..b706eac58f92 100644 --- a/include/linux/wkup_m3_ipc.h +++ b/include/linux/wkup_m3_ipc.h @@ -34,6 +34,7 @@ struct wkup_m3_ipc { int mem_type; unsigned long resume_addr; int vtt_conf; + int isolation_conf; int state; struct completion sync_complete; From ea082040fe071d2ba1f8f73792743d7ca9fb218e Mon Sep 17 00:00:00 2001 From: Dave Gerlach Date: Tue, 26 Apr 2022 13:07:44 -0700 Subject: [PATCH 14/15] soc: ti: wkup_m3_ipc: Add support for i2c voltage scaling Allow loading of a binary containing i2c scaling sequences to be provided to the wkup_m3 firmware in order to properly scale voltage rails on the PMIC during low power modes like DeepSleep0. Proper binary format is determined by the FW in use. Code expects firmware to have 0x0C57 present as the first two bytes followed by one byte defining offset to sleep sequence followed by one byte defining offset to wake sequence and then lastly both sequences. Each sequence is a series of I2C transfers in the form: u8 length | u8 chip address | u8 byte0/reg address | u8 byte1 | u8 byteN .. The length indicates the number of bytes to transfer, including the register address. The length of each transfer is limited by the I2C buffer size of 32 bytes. Based on previous work by Russ Dill. [dfustini: replace FW_ACTION_HOTPLUG with FW_ACTION_UEVENT] Signed-off-by: Dave Gerlach Signed-off-by: Keerthy [dfustini: add NULL argument to rproc_da_to_va() call] Signed-off-by: Drew Fustini Signed-off-by: Nishanth Menon Link: https://lore.kernel.org/r/20220426200741.712842-3-dfustini@baylibre.com --- drivers/soc/ti/wkup_m3_ipc.c | 93 +++++++++++++++++++++++++++++++++++- include/linux/wkup_m3_ipc.h | 9 ++++ 2 files changed, 101 insertions(+), 1 deletion(-) diff --git a/drivers/soc/ti/wkup_m3_ipc.c b/drivers/soc/ti/wkup_m3_ipc.c index 2f2a0ab32aff..84e9534056f8 100644 --- a/drivers/soc/ti/wkup_m3_ipc.c +++ b/drivers/soc/ti/wkup_m3_ipc.c @@ -8,6 +8,7 @@ */ #include +#include #include #include #include @@ -55,6 +56,12 @@ #define M3_STATE_MSG_FOR_LP 3 #define M3_STATE_MSG_FOR_RESET 4 +#define WKUP_M3_SD_FW_MAGIC 0x570C + +#define WKUP_M3_DMEM_START 0x80000 +#define WKUP_M3_AUXDATA_OFFSET 0x1000 +#define WKUP_M3_AUXDATA_SIZE 0xFF + static struct wkup_m3_ipc *m3_ipc_state; static const struct wkup_m3_wakeup_src wakeups[] = { @@ -75,6 +82,81 @@ static const struct wkup_m3_wakeup_src wakeups[] = { {.irq_nr = 0, .src = "Unknown"}, }; +/** + * wkup_m3_copy_aux_data - Copy auxiliary data to special region of m3 dmem + * @data - pointer to data + * @sz - size of data to copy (limit 256 bytes) + * + * Copies any additional blob of data to the wkup_m3 dmem to be used by the + * firmware + */ +static unsigned long wkup_m3_copy_aux_data(struct wkup_m3_ipc *m3_ipc, + const void *data, int sz) +{ + unsigned long aux_data_dev_addr; + void *aux_data_addr; + + aux_data_dev_addr = WKUP_M3_DMEM_START + WKUP_M3_AUXDATA_OFFSET; + aux_data_addr = rproc_da_to_va(m3_ipc->rproc, + aux_data_dev_addr, + WKUP_M3_AUXDATA_SIZE, + NULL); + memcpy(aux_data_addr, data, sz); + + return WKUP_M3_AUXDATA_OFFSET; +} + +static void wkup_m3_scale_data_fw_cb(const struct firmware *fw, void *context) +{ + unsigned long val, aux_base; + struct wkup_m3_scale_data_header hdr; + struct wkup_m3_ipc *m3_ipc = context; + struct device *dev = m3_ipc->dev; + + if (!fw) { + dev_err(dev, "Voltage scale fw name given but file missing.\n"); + return; + } + + memcpy(&hdr, fw->data, sizeof(hdr)); + + if (hdr.magic != WKUP_M3_SD_FW_MAGIC) { + dev_err(dev, "PM: Voltage Scale Data binary does not appear valid.\n"); + goto release_sd_fw; + } + + aux_base = wkup_m3_copy_aux_data(m3_ipc, fw->data + sizeof(hdr), + fw->size - sizeof(hdr)); + + val = (aux_base + hdr.sleep_offset); + val |= ((aux_base + hdr.wake_offset) << 16); + + m3_ipc->volt_scale_offsets = val; + +release_sd_fw: + release_firmware(fw); +}; + +static int wkup_m3_init_scale_data(struct wkup_m3_ipc *m3_ipc, + struct device *dev) +{ + int ret = 0; + + /* + * If no name is provided, user has already been warned, pm will + * still work so return 0 + */ + + if (!m3_ipc->sd_fw_name) + return ret; + + ret = request_firmware_nowait(THIS_MODULE, FW_ACTION_UEVENT, + m3_ipc->sd_fw_name, dev, GFP_ATOMIC, + m3_ipc, wkup_m3_scale_data_fw_cb); + + return ret; +} + static void am33xx_txev_eoi(struct wkup_m3_ipc *m3_ipc) { writel(AM33XX_M3_TXEV_ACK, @@ -139,6 +221,7 @@ static irqreturn_t wkup_m3_txev_handler(int irq, void *ipc_data) } m3_ipc->state = M3_STATE_INITED; + wkup_m3_init_scale_data(m3_ipc, dev); complete(&m3_ipc->sync_complete); break; case M3_STATE_MSG_FOR_RESET: @@ -300,12 +383,15 @@ static int wkup_m3_prepare_low_power(struct wkup_m3_ipc *m3_ipc, int state) switch (state) { case WKUP_M3_DEEPSLEEP: m3_power_state = IPC_CMD_DS0; + wkup_m3_ctrl_ipc_write(m3_ipc, m3_ipc->volt_scale_offsets, 5); break; case WKUP_M3_STANDBY: m3_power_state = IPC_CMD_STANDBY; + wkup_m3_ctrl_ipc_write(m3_ipc, DS_IPC_DEFAULT, 5); break; case WKUP_M3_IDLE: m3_power_state = IPC_CMD_IDLE; + wkup_m3_ctrl_ipc_write(m3_ipc, DS_IPC_DEFAULT, 5); break; default: return 1; @@ -319,7 +405,6 @@ static int wkup_m3_prepare_low_power(struct wkup_m3_ipc *m3_ipc, int state) m3_ipc->isolation_conf, 4); wkup_m3_ctrl_ipc_write(m3_ipc, DS_IPC_DEFAULT, 2); wkup_m3_ctrl_ipc_write(m3_ipc, DS_IPC_DEFAULT, 3); - wkup_m3_ctrl_ipc_write(m3_ipc, DS_IPC_DEFAULT, 5); wkup_m3_ctrl_ipc_write(m3_ipc, DS_IPC_DEFAULT, 6); wkup_m3_ctrl_ipc_write(m3_ipc, DS_IPC_DEFAULT, 7); @@ -526,6 +611,12 @@ static int wkup_m3_ipc_probe(struct platform_device *pdev) if (of_find_property(np, "ti,set-io-isolation", NULL)) wkup_m3_set_io_isolation(m3_ipc); + ret = of_property_read_string(np, "firmware-name", + &m3_ipc->sd_fw_name); + if (ret) { + dev_dbg(dev, "Voltage scaling data blob not provided from DT.\n"); + }; + /* * Wait for firmware loading completion in a thread so we * can boot the wkup_m3 as soon as it's ready without holding diff --git a/include/linux/wkup_m3_ipc.h b/include/linux/wkup_m3_ipc.h index b706eac58f92..fef0fac60f8c 100644 --- a/include/linux/wkup_m3_ipc.h +++ b/include/linux/wkup_m3_ipc.h @@ -37,6 +37,9 @@ struct wkup_m3_ipc { int isolation_conf; int state; + unsigned long volt_scale_offsets; + const char *sd_fw_name; + struct completion sync_complete; struct mbox_client mbox_client; struct mbox_chan *mbox; @@ -50,6 +53,12 @@ struct wkup_m3_wakeup_src { char src[10]; }; +struct wkup_m3_scale_data_header { + u16 magic; + u8 sleep_offset; + u8 wake_offset; +} __packed; + struct wkup_m3_ipc_ops { void (*set_mem_type)(struct wkup_m3_ipc *m3_ipc, int mem_type); void (*set_resume_address)(struct wkup_m3_ipc *m3_ipc, void *addr); From 2a21f9e6d9a408dbd09a01caf5fff42c2f70fa82 Mon Sep 17 00:00:00 2001 From: Dave Gerlach Date: Sun, 1 May 2022 20:32:12 -0700 Subject: [PATCH 15/15] soc: ti: wkup_m3_ipc: Add debug option to halt m3 in suspend Add a debugfs option to allow configurable halting of the wkup_m3 during suspend at the last possible point before low power mode entry. This condition can only be resolved through JTAG and advancing beyond the while loop in a8_lp_ds0_handler [1]. Although this hangs the system it forces the system to remain active once it has been entirely configured for low power mode entry, allowing for register inspection through JTAG to help in debugging transition errors. Halt mode can be set using the enable_off_mode entry under wkup_m3_ipc in the debugfs. [1] https://git.ti.com/cgit/processor-firmware/ti-amx3-cm3-pm-firmware/tree/src/pm_services/pm_handlers.c?h=08.02.00.006#n141 Suggested-by: Brad Griffis Signed-off-by: Dave Gerlach [dfustini: add link for a8_lp_ds0_handler() in ti-amx3-cm3-pm-firmware] Signed-off-by: Drew Fustini Signed-off-by: Nishanth Menon Link: https://lore.kernel.org/r/20220502033211.1383158-1-dfustini@baylibre.com --- drivers/soc/ti/wkup_m3_ipc.c | 79 +++++++++++++++++++++++++++++++++++- include/linux/wkup_m3_ipc.h | 2 + 2 files changed, 80 insertions(+), 1 deletion(-) diff --git a/drivers/soc/ti/wkup_m3_ipc.c b/drivers/soc/ti/wkup_m3_ipc.c index 84e9534056f8..0076d467ff6b 100644 --- a/drivers/soc/ti/wkup_m3_ipc.c +++ b/drivers/soc/ti/wkup_m3_ipc.c @@ -7,6 +7,7 @@ * Dave Gerlach */ +#include #include #include #include @@ -50,6 +51,9 @@ #define IPC_IO_ISOLATION_STAT_SHIFT (10) #define IPC_IO_ISOLATION_STAT_MASK (0x1 << 10) +#define IPC_DBG_HALT_SHIFT (11) +#define IPC_DBG_HALT_MASK (0x1 << 11) + #define M3_STATE_UNKNOWN 0 #define M3_STATE_RESET 1 #define M3_STATE_INITED 2 @@ -157,6 +161,73 @@ static int wkup_m3_init_scale_data(struct wkup_m3_ipc *m3_ipc, return ret; } +#ifdef CONFIG_DEBUG_FS +static void wkup_m3_set_halt_late(bool enabled) +{ + if (enabled) + m3_ipc_state->halt = (1 << IPC_DBG_HALT_SHIFT); + else + m3_ipc_state->halt = 0; +} + +static int option_get(void *data, u64 *val) +{ + u32 *option = data; + + *val = *option; + + return 0; +} + +static int option_set(void *data, u64 val) +{ + u32 *option = data; + + *option = val; + + if (option == &m3_ipc_state->halt) { + if (val) + wkup_m3_set_halt_late(true); + else + wkup_m3_set_halt_late(false); + } + + return 0; +} + +DEFINE_SIMPLE_ATTRIBUTE(wkup_m3_ipc_option_fops, option_get, option_set, + "%llu\n"); + +static int wkup_m3_ipc_dbg_init(struct wkup_m3_ipc *m3_ipc) +{ + m3_ipc->dbg_path = debugfs_create_dir("wkup_m3_ipc", NULL); + + if (!m3_ipc->dbg_path) + return -EINVAL; + + (void)debugfs_create_file("enable_late_halt", 0644, + m3_ipc->dbg_path, + &m3_ipc->halt, + &wkup_m3_ipc_option_fops); + + return 0; +} + +static inline void wkup_m3_ipc_dbg_destroy(struct wkup_m3_ipc *m3_ipc) +{ + debugfs_remove_recursive(m3_ipc->dbg_path); +} +#else +static inline int wkup_m3_ipc_dbg_init(struct wkup_m3_ipc *m3_ipc) +{ + return 0; +} + +static inline void wkup_m3_ipc_dbg_destroy(struct wkup_m3_ipc *m3_ipc) +{ +} +#endif /* CONFIG_DEBUG_FS */ + static void am33xx_txev_eoi(struct wkup_m3_ipc *m3_ipc) { writel(AM33XX_M3_TXEV_ACK, @@ -402,7 +473,9 @@ static int wkup_m3_prepare_low_power(struct wkup_m3_ipc *m3_ipc, int state) wkup_m3_ctrl_ipc_write(m3_ipc, m3_power_state, 1); wkup_m3_ctrl_ipc_write(m3_ipc, m3_ipc->mem_type | m3_ipc->vtt_conf | - m3_ipc->isolation_conf, 4); + m3_ipc->isolation_conf | + m3_ipc->halt, 4); + wkup_m3_ctrl_ipc_write(m3_ipc, DS_IPC_DEFAULT, 2); wkup_m3_ctrl_ipc_write(m3_ipc, DS_IPC_DEFAULT, 3); wkup_m3_ctrl_ipc_write(m3_ipc, DS_IPC_DEFAULT, 6); @@ -631,6 +704,8 @@ static int wkup_m3_ipc_probe(struct platform_device *pdev) goto err_put_rproc; } + wkup_m3_ipc_dbg_init(m3_ipc); + return 0; err_put_rproc: @@ -642,6 +717,8 @@ err_free_mbox: static int wkup_m3_ipc_remove(struct platform_device *pdev) { + wkup_m3_ipc_dbg_destroy(m3_ipc_state); + mbox_free_channel(m3_ipc_state->mbox); rproc_shutdown(m3_ipc_state->rproc); diff --git a/include/linux/wkup_m3_ipc.h b/include/linux/wkup_m3_ipc.h index fef0fac60f8c..26d1eb058fa3 100644 --- a/include/linux/wkup_m3_ipc.h +++ b/include/linux/wkup_m3_ipc.h @@ -36,6 +36,7 @@ struct wkup_m3_ipc { int vtt_conf; int isolation_conf; int state; + u32 halt; unsigned long volt_scale_offsets; const char *sd_fw_name; @@ -46,6 +47,7 @@ struct wkup_m3_ipc { struct wkup_m3_ipc_ops *ops; int is_rtc_only; + struct dentry *dbg_path; }; struct wkup_m3_wakeup_src {