This series aims to improve the general readability of the mss reset
sequence on SC7180 SoCs. No functional change intended.
Sibi Sankar (4):
remoteproc: qcom: q6v5-mss: Use regmap_read_poll_timeout
remoteproc: qcom: q6v5-mss: Improve readability across clk handling
remoteproc: qcom: q6v5-mss: Rename boot status timeout
remoteproc: qcom: q6v5-mss: Improve readability of reset_assert
drivers/remoteproc/qcom_q6v5_mss.c | 69 +++++++++++++++++-------------
1 file changed, 40 insertions(+), 29 deletions(-)
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Replace the loop for HALT_ACK detection with regmap_read_poll_timeout.
Signed-off-by: Sibi Sankar <[email protected]>
---
drivers/remoteproc/qcom_q6v5_mss.c | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
index 51f451311f5fc..f20b39c6ff0ed 100644
--- a/drivers/remoteproc/qcom_q6v5_mss.c
+++ b/drivers/remoteproc/qcom_q6v5_mss.c
@@ -73,6 +73,7 @@
#define NAV_AXI_IDLE_BIT BIT(2)
#define HALT_ACK_TIMEOUT_MS 100
+#define NAV_HALT_ACK_TIMEOUT_US 200
/* QDSP6SS_RESET */
#define Q6SS_STOP_CORE BIT(0)
@@ -746,7 +747,6 @@ static void q6v5proc_halt_nav_axi_port(struct q6v5 *qproc,
struct regmap *halt_map,
u32 offset)
{
- unsigned long timeout;
unsigned int val;
int ret;
@@ -760,15 +760,11 @@ static void q6v5proc_halt_nav_axi_port(struct q6v5 *qproc,
NAV_AXI_HALTREQ_BIT);
/* Wait for halt ack*/
- timeout = jiffies + msecs_to_jiffies(HALT_ACK_TIMEOUT_MS);
- for (;;) {
- ret = regmap_read(halt_map, offset, &val);
- if (ret || (val & NAV_AXI_HALTACK_BIT) ||
- time_after(jiffies, timeout))
- break;
-
- udelay(5);
- }
+ ret = regmap_read_poll_timeout(halt_map, offset, val,
+ (val & NAV_AXI_HALTACK_BIT),
+ 5, NAV_HALT_ACK_TIMEOUT_US);
+ if (ret)
+ dev_err(qproc->dev, "nav halt ack timeout\n");
ret = regmap_read(halt_map, offset, &val);
if (ret || !(val & NAV_AXI_IDLE_BIT))
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Rename the FSM timeout on SC7180 to BOOT_STATUS_TIMEOUT_US.
Signed-off-by: Sibi Sankar <[email protected]>
---
drivers/remoteproc/qcom_q6v5_mss.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
index e1784446d3da3..6a98e9029c70b 100644
--- a/drivers/remoteproc/qcom_q6v5_mss.c
+++ b/drivers/remoteproc/qcom_q6v5_mss.c
@@ -114,7 +114,7 @@
#define QDSP6SS_BOOT_CORE_START 0x400
#define QDSP6SS_BOOT_CMD 0x404
#define QDSP6SS_BOOT_STATUS 0x408
-#define SLEEP_CHECK_MAX_LOOPS 200
+#define BOOT_STATUS_TIMEOUT_US 200
#define BOOT_FSM_TIMEOUT 10000
struct reg_info {
@@ -572,7 +572,7 @@ static int q6v5proc_reset(struct q6v5 *qproc)
/* Poll the QDSP6SS_BOOT_STATUS for FSM completion */
ret = readl_poll_timeout(qproc->reg_base + QDSP6SS_BOOT_STATUS,
val, (val & BIT(0)) != 0, 1,
- SLEEP_CHECK_MAX_LOOPS);
+ BOOT_STATUS_TIMEOUT_US);
if (ret) {
dev_err(qproc->dev, "Boot FSM failed to complete.\n");
/* Reset the modem so that boot FSM is in reset state */
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Define CLKEN and CLKOFF for improving readability of Q6SS clock
handling.
Signed-off-by: Sibi Sankar <[email protected]>
---
drivers/remoteproc/qcom_q6v5_mss.c | 32 +++++++++++++++++-------------
1 file changed, 18 insertions(+), 14 deletions(-)
diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
index f20b39c6ff0ed..e1784446d3da3 100644
--- a/drivers/remoteproc/qcom_q6v5_mss.c
+++ b/drivers/remoteproc/qcom_q6v5_mss.c
@@ -80,6 +80,11 @@
#define Q6SS_CORE_ARES BIT(1)
#define Q6SS_BUS_ARES_ENABLE BIT(2)
+/* QDSP6SS CBCR */
+#define Q6SS_CBCR_CLKEN BIT(0)
+#define Q6SS_CBCR_CLKOFF BIT(31)
+#define Q6SS_CBCR_TIMEOUT_US 200
+
/* QDSP6SS_GFMUX_CTL */
#define Q6SS_CLK_ENABLE BIT(1)
@@ -100,7 +105,6 @@
#define QDSP6v56_BHS_ON BIT(24)
#define QDSP6v56_CLAMP_WL BIT(21)
#define QDSP6v56_CLAMP_QMC_MEM BIT(22)
-#define HALT_CHECK_MAX_LOOPS 200
#define QDSP6SS_XO_CBCR 0x0038
#define QDSP6SS_ACC_OVERRIDE_VAL 0x20
@@ -502,12 +506,12 @@ static int q6v5proc_reset(struct q6v5 *qproc)
if (qproc->version == MSS_SDM845) {
val = readl(qproc->reg_base + QDSP6SS_SLEEP);
- val |= 0x1;
+ val |= Q6SS_CBCR_CLKEN;
writel(val, qproc->reg_base + QDSP6SS_SLEEP);
ret = readl_poll_timeout(qproc->reg_base + QDSP6SS_SLEEP,
- val, !(val & BIT(31)), 1,
- SLEEP_CHECK_MAX_LOOPS);
+ val, !(val & Q6SS_CBCR_CLKOFF), 1,
+ Q6SS_CBCR_TIMEOUT_US);
if (ret) {
dev_err(qproc->dev, "QDSP6SS Sleep clock timed out\n");
return -ETIMEDOUT;
@@ -530,12 +534,12 @@ static int q6v5proc_reset(struct q6v5 *qproc)
goto pbl_wait;
} else if (qproc->version == MSS_SC7180) {
val = readl(qproc->reg_base + QDSP6SS_SLEEP);
- val |= 0x1;
+ val |= Q6SS_CBCR_CLKEN;
writel(val, qproc->reg_base + QDSP6SS_SLEEP);
ret = readl_poll_timeout(qproc->reg_base + QDSP6SS_SLEEP,
- val, !(val & BIT(31)), 1,
- SLEEP_CHECK_MAX_LOOPS);
+ val, !(val & Q6SS_CBCR_CLKOFF), 1,
+ Q6SS_CBCR_TIMEOUT_US);
if (ret) {
dev_err(qproc->dev, "QDSP6SS Sleep clock timed out\n");
return -ETIMEDOUT;
@@ -543,12 +547,12 @@ static int q6v5proc_reset(struct q6v5 *qproc)
/* Turn on the XO clock needed for PLL setup */
val = readl(qproc->reg_base + QDSP6SS_XO_CBCR);
- val |= 0x1;
+ val |= Q6SS_CBCR_CLKEN;
writel(val, qproc->reg_base + QDSP6SS_XO_CBCR);
ret = readl_poll_timeout(qproc->reg_base + QDSP6SS_XO_CBCR,
- val, !(val & BIT(31)), 1,
- SLEEP_CHECK_MAX_LOOPS);
+ val, !(val & Q6SS_CBCR_CLKOFF), 1,
+ Q6SS_CBCR_TIMEOUT_US);
if (ret) {
dev_err(qproc->dev, "QDSP6SS XO clock timed out\n");
return -ETIMEDOUT;
@@ -556,7 +560,7 @@ static int q6v5proc_reset(struct q6v5 *qproc)
/* Configure Q6 core CBCR to auto-enable after reset sequence */
val = readl(qproc->reg_base + QDSP6SS_CORE_CBCR);
- val |= 0x1;
+ val |= Q6SS_CBCR_CLKEN;
writel(val, qproc->reg_base + QDSP6SS_CORE_CBCR);
/* De-assert the Q6 stop core signal */
@@ -591,13 +595,13 @@ static int q6v5proc_reset(struct q6v5 *qproc)
/* BHS require xo cbcr to be enabled */
val = readl(qproc->reg_base + QDSP6SS_XO_CBCR);
- val |= 0x1;
+ val |= Q6SS_CBCR_CLKEN;
writel(val, qproc->reg_base + QDSP6SS_XO_CBCR);
/* Read CLKOFF bit to go low indicating CLK is enabled */
ret = readl_poll_timeout(qproc->reg_base + QDSP6SS_XO_CBCR,
- val, !(val & BIT(31)), 1,
- HALT_CHECK_MAX_LOOPS);
+ val, !(val & Q6SS_CBCR_CLKOFF), 1,
+ Q6SS_CBCR_TIMEOUT_US);
if (ret) {
dev_err(qproc->dev,
"xo cbcr enabling timed out (rc:%d)\n", ret);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Define CONN_BOX_SPARE_0_EN and fixup comments to improve readability of
Q6 modem reset_assert sequence on SC7180 SoCs.
Signed-off-by: Sibi Sankar <[email protected]>
---
drivers/remoteproc/qcom_q6v5_mss.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
index 6a98e9029c70b..8c9cfc213d5ff 100644
--- a/drivers/remoteproc/qcom_q6v5_mss.c
+++ b/drivers/remoteproc/qcom_q6v5_mss.c
@@ -71,6 +71,7 @@
#define NAV_AXI_HALTREQ_BIT BIT(0)
#define NAV_AXI_HALTACK_BIT BIT(1)
#define NAV_AXI_IDLE_BIT BIT(2)
+#define CONN_BOX_SPARE_0_EN BIT(0)
#define HALT_ACK_TIMEOUT_MS 100
#define NAV_HALT_ACK_TIMEOUT_US 200
@@ -415,16 +416,26 @@ static int q6v5_reset_assert(struct q6v5 *qproc)
ret = reset_control_reset(qproc->mss_restart);
reset_control_deassert(qproc->pdc_reset);
} else if (qproc->has_halt_nav) {
- /* SWAR using CONN_BOX_SPARE_0 for pipeline glitch issue */
+ /*
+ * SWWA for the pipeline glitch issue seen while
+ * putting the Q6 modem on SC7180 into reset:
+ * 1 - Assert PDC reset
+ * 2 - Set CONN_BOX_SPARE_0_EN
+ * 3 - Withdraw the halt requests
+ * 4 - Assert MSS reset
+ * 5 - Deassert PDC reset
+ * 6 - Clear CONN_BOX_SPARE_0_EN
+ * 7 - Deassert MSS reset
+ */
reset_control_assert(qproc->pdc_reset);
regmap_update_bits(qproc->conn_map, qproc->conn_box,
- BIT(0), BIT(0));
+ CONN_BOX_SPARE_0_EN, 1);
regmap_update_bits(qproc->halt_nav_map, qproc->halt_nav,
NAV_AXI_HALTREQ_BIT, 0);
reset_control_assert(qproc->mss_restart);
reset_control_deassert(qproc->pdc_reset);
regmap_update_bits(qproc->conn_map, qproc->conn_box,
- BIT(0), 0);
+ CONN_BOX_SPARE_0_EN, 0);
ret = reset_control_deassert(qproc->mss_restart);
} else {
ret = reset_control_assert(qproc->mss_restart);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
On Fri, 2020-01-17 at 19:21 +0530, Sibi Sankar wrote:
> Replace the loop for HALT_ACK detection with regmap_read_poll_timeout.
>
> Signed-off-by: Sibi Sankar <[email protected]>
> ---
> drivers/remoteproc/qcom_q6v5_mss.c | 16 ++++++----------
> 1 file changed, 6 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
> index 51f451311f5fc..f20b39c6ff0ed 100644
> --- a/drivers/remoteproc/qcom_q6v5_mss.c
> +++ b/drivers/remoteproc/qcom_q6v5_mss.c
> @@ -73,6 +73,7 @@
> #define NAV_AXI_IDLE_BIT BIT(2)
>
> #define HALT_ACK_TIMEOUT_MS 100
> +#define NAV_HALT_ACK_TIMEOUT_US 200
>
> /* QDSP6SS_RESET */
> #define Q6SS_STOP_CORE BIT(0)
> @@ -746,7 +747,6 @@ static void q6v5proc_halt_nav_axi_port(struct q6v5 *qproc,
> struct regmap *halt_map,
> u32 offset)
> {
> - unsigned long timeout;
> unsigned int val;
> int ret;
>
> @@ -760,15 +760,11 @@ static void q6v5proc_halt_nav_axi_port(struct q6v5 *qproc,
> NAV_AXI_HALTREQ_BIT);
>
> /* Wait for halt ack*/
> - timeout = jiffies + msecs_to_jiffies(HALT_ACK_TIMEOUT_MS);
> - for (;;) {
> - ret = regmap_read(halt_map, offset, &val);
> - if (ret || (val & NAV_AXI_HALTACK_BIT) ||
> - time_after(jiffies, timeout))
> - break;
> -
> - udelay(5);
> - }
> + ret = regmap_read_poll_timeout(halt_map, offset, val,
> + (val & NAV_AXI_HALTACK_BIT),
> + 5, NAV_HALT_ACK_TIMEOUT_US);
> + if (ret)
> + dev_err(qproc->dev, "nav halt ack timeout\n");
>
> ret = regmap_read(halt_map, offset, &val);
> if (ret || !(val & NAV_AXI_IDLE_BIT))
Reviewed-by: Philipp Zabel <[email protected]>
regards
Philipp
On Fri 17 Jan 05:51 PST 2020, Sibi Sankar wrote:
> Replace the loop for HALT_ACK detection with regmap_read_poll_timeout.
>
Nice, but we should be able to do the same in q6v5proc_halt_axi_port()?
> Signed-off-by: Sibi Sankar <[email protected]>
> ---
> drivers/remoteproc/qcom_q6v5_mss.c | 16 ++++++----------
> 1 file changed, 6 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
> index 51f451311f5fc..f20b39c6ff0ed 100644
> --- a/drivers/remoteproc/qcom_q6v5_mss.c
> +++ b/drivers/remoteproc/qcom_q6v5_mss.c
> @@ -73,6 +73,7 @@
> #define NAV_AXI_IDLE_BIT BIT(2)
>
> #define HALT_ACK_TIMEOUT_MS 100
> +#define NAV_HALT_ACK_TIMEOUT_US 200
>
> /* QDSP6SS_RESET */
> #define Q6SS_STOP_CORE BIT(0)
> @@ -746,7 +747,6 @@ static void q6v5proc_halt_nav_axi_port(struct q6v5 *qproc,
> struct regmap *halt_map,
> u32 offset)
> {
> - unsigned long timeout;
> unsigned int val;
> int ret;
>
> @@ -760,15 +760,11 @@ static void q6v5proc_halt_nav_axi_port(struct q6v5 *qproc,
> NAV_AXI_HALTREQ_BIT);
>
> /* Wait for halt ack*/
> - timeout = jiffies + msecs_to_jiffies(HALT_ACK_TIMEOUT_MS);
> - for (;;) {
> - ret = regmap_read(halt_map, offset, &val);
> - if (ret || (val & NAV_AXI_HALTACK_BIT) ||
> - time_after(jiffies, timeout))
> - break;
> -
> - udelay(5);
> - }
> + ret = regmap_read_poll_timeout(halt_map, offset, val,
> + (val & NAV_AXI_HALTACK_BIT),
> + 5, NAV_HALT_ACK_TIMEOUT_US);
> + if (ret)
> + dev_err(qproc->dev, "nav halt ack timeout\n");
Is there a case where this new print adds value beyond the printout we
already have for the case of IDLE_BIT not going high? Can we simply
ignore the return value and skip the print?
Regards,
Bjorn
>
> ret = regmap_read(halt_map, offset, &val);
> if (ret || !(val & NAV_AXI_IDLE_BIT))
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
On Fri 17 Jan 05:51 PST 2020, Sibi Sankar wrote:
> This series aims to improve the general readability of the mss reset
> sequence on SC7180 SoCs. No functional change intended.
>
Thanks Sibi, I picked up patch 2 and 3 for now.
Regards,
Bjorn
> Sibi Sankar (4):
> remoteproc: qcom: q6v5-mss: Use regmap_read_poll_timeout
> remoteproc: qcom: q6v5-mss: Improve readability across clk handling
> remoteproc: qcom: q6v5-mss: Rename boot status timeout
> remoteproc: qcom: q6v5-mss: Improve readability of reset_assert
>
> drivers/remoteproc/qcom_q6v5_mss.c | 69 +++++++++++++++++-------------
> 1 file changed, 40 insertions(+), 29 deletions(-)
>
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
On Fri 17 Jan 05:51 PST 2020, Sibi Sankar wrote:
> Define CONN_BOX_SPARE_0_EN and fixup comments to improve readability of
> Q6 modem reset_assert sequence on SC7180 SoCs.
>
> Signed-off-by: Sibi Sankar <[email protected]>
> ---
> drivers/remoteproc/qcom_q6v5_mss.c | 17 ++++++++++++++---
> 1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
> index 6a98e9029c70b..8c9cfc213d5ff 100644
> --- a/drivers/remoteproc/qcom_q6v5_mss.c
> +++ b/drivers/remoteproc/qcom_q6v5_mss.c
> @@ -71,6 +71,7 @@
> #define NAV_AXI_HALTREQ_BIT BIT(0)
> #define NAV_AXI_HALTACK_BIT BIT(1)
> #define NAV_AXI_IDLE_BIT BIT(2)
> +#define CONN_BOX_SPARE_0_EN BIT(0)
>
> #define HALT_ACK_TIMEOUT_MS 100
> #define NAV_HALT_ACK_TIMEOUT_US 200
> @@ -415,16 +416,26 @@ static int q6v5_reset_assert(struct q6v5 *qproc)
> ret = reset_control_reset(qproc->mss_restart);
> reset_control_deassert(qproc->pdc_reset);
> } else if (qproc->has_halt_nav) {
> - /* SWAR using CONN_BOX_SPARE_0 for pipeline glitch issue */
> + /*
> + * SWWA for the pipeline glitch issue seen while
Is SWWA an abbreviation for SoftWare WorkAround?
> + * putting the Q6 modem on SC7180 into reset:
> + * 1 - Assert PDC reset
> + * 2 - Set CONN_BOX_SPARE_0_EN
> + * 3 - Withdraw the halt requests
> + * 4 - Assert MSS reset
> + * 5 - Deassert PDC reset
> + * 6 - Clear CONN_BOX_SPARE_0_EN
> + * 7 - Deassert MSS reset
This pretty much outlines what's written below. How about making this
something like:
/*
* Work around a pipeline glitch seen when putting the Q6 modem in
* SC7180 into reset by also toggling CONN_BOX_SPARE_0_EN, while holding
* the PDC reset.
*/
Although, it would be even better if it indicated what you mean with
"pipeline glitch"...
Regards,
Bjorn
> + */
> reset_control_assert(qproc->pdc_reset);
> regmap_update_bits(qproc->conn_map, qproc->conn_box,
> - BIT(0), BIT(0));
> + CONN_BOX_SPARE_0_EN, 1);
> regmap_update_bits(qproc->halt_nav_map, qproc->halt_nav,
> NAV_AXI_HALTREQ_BIT, 0);
> reset_control_assert(qproc->mss_restart);
> reset_control_deassert(qproc->pdc_reset);
> regmap_update_bits(qproc->conn_map, qproc->conn_box,
> - BIT(0), 0);
> + CONN_BOX_SPARE_0_EN, 0);
> ret = reset_control_deassert(qproc->mss_restart);
> } else {
> ret = reset_control_assert(qproc->mss_restart);
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
On Fri, Jan 17, 2020 at 5:51 AM Sibi Sankar <[email protected]> wrote:
>
> Define CLKEN and CLKOFF for improving readability of Q6SS clock
> handling.
>
> Signed-off-by: Sibi Sankar <[email protected]>
It took me awhile to wrap my head around how this new define,
Q6SS_CBCR_TIMEOUT_US, sometimes replaces HALT_CHECK_MAX_LOOPS and
sometimes replaces SLEEP_CHECK_MAX_LOOPS. I guess they're conceptually
different but set to the same value for now? And you've fixed up a
place where the wrong one was used? If you thought the distinction was
meaningless I'd also be fine merging these two defines into one.
Either way, assuming the above is intentional, this looks ok to me.
Thanks for renaming that define.
Reviewed-by: Evan Green <[email protected]>
On Fri, Jan 17, 2020 at 5:51 AM Sibi Sankar <[email protected]> wrote:
>
> Rename the FSM timeout on SC7180 to BOOT_STATUS_TIMEOUT_US.
>
> Signed-off-by: Sibi Sankar <[email protected]>
Reviewed-by: Evan Green <[email protected]>
Hey Bjorn,
Thanks for the review!
On 2020-01-21 00:54, Bjorn Andersson wrote:
> On Fri 17 Jan 05:51 PST 2020, Sibi Sankar wrote:
>
>> Replace the loop for HALT_ACK detection with regmap_read_poll_timeout.
sry missed it, will include it
in the next re-spin
>>
>
> Nice, but we should be able to do the same in q6v5proc_halt_axi_port()?
>
>> Signed-off-by: Sibi Sankar <[email protected]>
>> ---
>> drivers/remoteproc/qcom_q6v5_mss.c | 16 ++++++----------
>> 1 file changed, 6 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/remoteproc/qcom_q6v5_mss.c
>> b/drivers/remoteproc/qcom_q6v5_mss.c
>> index 51f451311f5fc..f20b39c6ff0ed 100644
>> --- a/drivers/remoteproc/qcom_q6v5_mss.c
>> +++ b/drivers/remoteproc/qcom_q6v5_mss.c
>> @@ -73,6 +73,7 @@
>> #define NAV_AXI_IDLE_BIT BIT(2)
>>
>> #define HALT_ACK_TIMEOUT_MS 100
>> +#define NAV_HALT_ACK_TIMEOUT_US 200
>>
>> /* QDSP6SS_RESET */
>> #define Q6SS_STOP_CORE BIT(0)
>> @@ -746,7 +747,6 @@ static void q6v5proc_halt_nav_axi_port(struct q6v5
>> *qproc,
>> struct regmap *halt_map,
>> u32 offset)
>> {
>> - unsigned long timeout;
>> unsigned int val;
>> int ret;
>>
>> @@ -760,15 +760,11 @@ static void q6v5proc_halt_nav_axi_port(struct
>> q6v5 *qproc,
>> NAV_AXI_HALTREQ_BIT);
>>
>> /* Wait for halt ack*/
>> - timeout = jiffies + msecs_to_jiffies(HALT_ACK_TIMEOUT_MS);
>> - for (;;) {
>> - ret = regmap_read(halt_map, offset, &val);
>> - if (ret || (val & NAV_AXI_HALTACK_BIT) ||
>> - time_after(jiffies, timeout))
>> - break;
>> -
>> - udelay(5);
>> - }
>> + ret = regmap_read_poll_timeout(halt_map, offset, val,
>> + (val & NAV_AXI_HALTACK_BIT),
>> + 5, NAV_HALT_ACK_TIMEOUT_US);
>> + if (ret)
>> + dev_err(qproc->dev, "nav halt ack timeout\n");
>
> Is there a case where this new print adds value beyond the printout we
> already have for the case of IDLE_BIT not going high? Can we simply
> ignore the return value and skip the print?
yes we can skip the print
>
> Regards,
> Bjorn
>
>>
>> ret = regmap_read(halt_map, offset, &val);
>> if (ret || !(val & NAV_AXI_IDLE_BIT))
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
>> Forum,
>> a Linux Foundation Collaborative Project
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.
Hey Evan,
Thanks for the review!
On 2020-01-22 00:52, Evan Green wrote:
> On Fri, Jan 17, 2020 at 5:51 AM Sibi Sankar <[email protected]>
> wrote:
>>
>> Define CLKEN and CLKOFF for improving readability of Q6SS clock
>> handling.
>>
>> Signed-off-by: Sibi Sankar <[email protected]>
>
> It took me awhile to wrap my head around how this new define,
> Q6SS_CBCR_TIMEOUT_US, sometimes replaces HALT_CHECK_MAX_LOOPS and
> sometimes replaces SLEEP_CHECK_MAX_LOOPS. I guess they're conceptually
> different but set to the same value for now? And you've fixed up a
> place where the wrong one was used? If you thought the distinction was
> meaningless I'd also be fine merging these two defines into one.
They really aren't that different
both are Clks with the same timeout
the previous naming was just plain
bad.
SLEEP_CHECK_MAX_LOOPS was used
probably because it was referring
to QDSP6SS_SLEEP CBCRs timeout.
HALT_CHECK_MAX_LOOOPS seems to
taken directly from CAF code. So
we should be fine with merging
the two defines into one.
> Either way, assuming the above is intentional, this looks ok to me.
> Thanks for renaming that define.
>
> Reviewed-by: Evan Green <[email protected]>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.