* [PATCH v2 0/2] Add Modem support on SC7180 SoCs @ 2019-12-19 5:45 Sibi Sankar 2019-12-19 5:45 ` [PATCH v2 1/2] dt-bindings: remoteproc: qcom: Add Q6V5 Modem PIL binding for SC7180 Sibi Sankar 2019-12-19 5:45 ` [PATCH v2 2/2] remoteproc: mss: q6v5-mss: Add modem support on SC7180 Sibi Sankar 0 siblings, 2 replies; 8+ messages in thread From: Sibi Sankar @ 2019-12-19 5:45 UTC (permalink / raw) To: bjorn.andersson Cc: ohad, mark.rutland, linux-arm-msm, linux-remoteproc, p.zabel, devicetree, linux-kernel, agross, Sibi Sankar This patch series adds support for booting the Modem Q6 DSP found in Qualcomm's SC7180 SoCs. V2: * split the series according to SoC * Duplicate code across q6v5proc_reset * add Rob's 'Reviewed-by' tag Sibi Sankar (2): dt-bindings: remoteproc: qcom: Add Q6V5 Modem PIL binding for SC7180 remoteproc: mss: q6v5-mss: Add modem support on SC7180 .../bindings/remoteproc/qcom,q6v5.txt | 23 +- drivers/remoteproc/qcom_q6v5_mss.c | 199 +++++++++++++++++- 2 files changed, 219 insertions(+), 3 deletions(-) -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 1/2] dt-bindings: remoteproc: qcom: Add Q6V5 Modem PIL binding for SC7180 2019-12-19 5:45 [PATCH v2 0/2] Add Modem support on SC7180 SoCs Sibi Sankar @ 2019-12-19 5:45 ` Sibi Sankar 2019-12-19 5:45 ` [PATCH v2 2/2] remoteproc: mss: q6v5-mss: Add modem support on SC7180 Sibi Sankar 1 sibling, 0 replies; 8+ messages in thread From: Sibi Sankar @ 2019-12-19 5:45 UTC (permalink / raw) To: bjorn.andersson Cc: ohad, mark.rutland, linux-arm-msm, linux-remoteproc, p.zabel, devicetree, linux-kernel, agross, Sibi Sankar, Rob Herring Add a new modem compatible string for Qualcomm SC7180 SoCs and introduce the "qcom,halt-nav-regs" bindings needed by the modem sub-system running on SC7180 SoCs. Reviewed-by: Rob Herring <robh@kernel.org> Signed-off-by: Sibi Sankar <sibis@codeaurora.org> --- .../bindings/remoteproc/qcom,q6v5.txt | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt b/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt index c416746f93cfd..88dfa3fc15f78 100644 --- a/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt +++ b/Documentation/devicetree/bindings/remoteproc/qcom,q6v5.txt @@ -13,6 +13,7 @@ on the Qualcomm Hexagon core. "qcom,msm8974-mss-pil" "qcom,msm8996-mss-pil" "qcom,msm8998-mss-pil" + "qcom,sc7180-mss-pil" "qcom,sdm845-mss-pil" - reg: @@ -43,6 +44,7 @@ on the Qualcomm Hexagon core. must be "wdog", "fatal", "ready", "handover", "stop-ack" qcom,msm8996-mss-pil: qcom,msm8998-mss-pil: + qcom,sc7180-mss-pil: qcom,sdm845-mss-pil: must be "wdog", "fatal", "ready", "handover", "stop-ack", "shutdown-ack" @@ -75,6 +77,9 @@ on the Qualcomm Hexagon core. qcom,msm8998-mss-pil: must be "iface", "bus", "mem", "xo", "gpll0_mss", "snoc_axi", "mnoc_axi", "qdss" + qcom,sc7180-mss-pil: + must be "iface", "bus", "xo", "snoc_axi", "mnoc_axi", + "mss_crypto", "mss_nav", "nav" qcom,sdm845-mss-pil: must be "iface", "bus", "mem", "xo", "gpll0_mss", "snoc_axi", "mnoc_axi", "prng" @@ -86,7 +91,7 @@ on the Qualcomm Hexagon core. reference to the list of 3 reset-controllers for the wcss sub-system reference to the list of 2 reset-controllers for the modem - sub-system on SDM845 SoCs + sub-system on SC7180, SDM845 SoCs - reset-names: Usage: required @@ -95,7 +100,7 @@ on the Qualcomm Hexagon core. must be "wcss_aon_reset", "wcss_reset", "wcss_q6_reset" for the wcss sub-system must be "mss_restart", "pdc_reset" for the modem - sub-system on SDM845 SoCs + sub-system on SC7180, SDM845 SoCs For the compatible strings below the following supplies are required: "qcom,q6v5-pil" @@ -144,6 +149,7 @@ For the compatible string below the following supplies are required: qcom,msm8996-mss-pil: qcom,msm8998-mss-pil: must be "cx", "mx" + qcom,sc7180-mss-pil: qcom,sdm845-mss-pil: must be "cx", "mx", "mss", "load_state" @@ -165,6 +171,19 @@ For the compatible string below the following supplies are required: by the three offsets within syscon for q6, modem and nc halt registers. +For the compatible strings below the following phandle references are required: + "qcom,sc7180-mss-pil" +- qcom,halt-nav-regs: + Usage: required + Value type: <prop-encoded-array> + Definition: reference to a list of 2 phandles with one offset each for + the modem sub-system running on SC7180 SoC. The first + phandle reference is to the mss clock node followed by the + offset within register space for nav halt register. The + second phandle reference is to a syscon representing TCSR + followed by the offset within syscon for conn_box_spare0 + register. + = SUBNODES: The Hexagon node must contain two subnodes, named "mba" and "mpss" representing the memory regions used by the Hexagon firmware. Each sub-node must contain: -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 2/2] remoteproc: mss: q6v5-mss: Add modem support on SC7180 2019-12-19 5:45 [PATCH v2 0/2] Add Modem support on SC7180 SoCs Sibi Sankar 2019-12-19 5:45 ` [PATCH v2 1/2] dt-bindings: remoteproc: qcom: Add Q6V5 Modem PIL binding for SC7180 Sibi Sankar @ 2019-12-19 5:45 ` Sibi Sankar 2020-01-02 13:26 ` Philipp Zabel 2020-01-07 21:21 ` Evan Green 1 sibling, 2 replies; 8+ messages in thread From: Sibi Sankar @ 2019-12-19 5:45 UTC (permalink / raw) To: bjorn.andersson Cc: ohad, mark.rutland, linux-arm-msm, linux-remoteproc, p.zabel, devicetree, linux-kernel, agross, Sibi Sankar Add the out of reset sequence support for modem sub-system on SC7180 SoCs. It requires access to an additional halt nav register to put the modem back into reset. Signed-off-by: Sibi Sankar <sibis@codeaurora.org> --- drivers/remoteproc/qcom_q6v5_mss.c | 199 ++++++++++++++++++++++++++++- 1 file changed, 198 insertions(+), 1 deletion(-) diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c index 164fc2a53ef11..51f451311f5fc 100644 --- a/drivers/remoteproc/qcom_q6v5_mss.c +++ b/drivers/remoteproc/qcom_q6v5_mss.c @@ -68,6 +68,9 @@ #define AXI_HALTREQ_REG 0x0 #define AXI_HALTACK_REG 0x4 #define AXI_IDLE_REG 0x8 +#define NAV_AXI_HALTREQ_BIT BIT(0) +#define NAV_AXI_HALTACK_BIT BIT(1) +#define NAV_AXI_IDLE_BIT BIT(2) #define HALT_ACK_TIMEOUT_MS 100 @@ -101,9 +104,11 @@ #define QDSP6SS_ACC_OVERRIDE_VAL 0x20 /* QDSP6v65 parameters */ +#define QDSP6SS_CORE_CBCR 0x20 #define QDSP6SS_SLEEP 0x3C #define QDSP6SS_BOOT_CORE_START 0x400 #define QDSP6SS_BOOT_CMD 0x404 +#define QDSP6SS_BOOT_STATUS 0x408 #define SLEEP_CHECK_MAX_LOOPS 200 #define BOOT_FSM_TIMEOUT 10000 @@ -131,6 +136,7 @@ struct rproc_hexagon_res { int version; bool need_mem_protection; bool has_alt_reset; + bool has_halt_nav; }; struct q6v5 { @@ -141,9 +147,14 @@ struct q6v5 { void __iomem *rmb_base; struct regmap *halt_map; + struct regmap *halt_nav_map; + struct regmap *conn_map; + u32 halt_q6; u32 halt_modem; u32 halt_nc; + u32 halt_nav; + u32 conn_box; struct reset_control *mss_restart; struct reset_control *pdc_reset; @@ -187,6 +198,7 @@ struct q6v5 { struct qcom_sysmon *sysmon; bool need_mem_protection; bool has_alt_reset; + bool has_halt_nav; int mpss_perm; int mba_perm; const char *hexagon_mdt_image; @@ -198,6 +210,7 @@ enum { MSS_MSM8974, MSS_MSM8996, MSS_MSM8998, + MSS_SC7180, MSS_SDM845, }; @@ -396,6 +409,18 @@ static int q6v5_reset_assert(struct q6v5 *qproc) reset_control_assert(qproc->pdc_reset); 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 */ + reset_control_assert(qproc->pdc_reset); + regmap_update_bits(qproc->conn_map, qproc->conn_box, + BIT(0), BIT(0)); + 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); + ret = reset_control_deassert(qproc->mss_restart); } else { ret = reset_control_assert(qproc->mss_restart); } @@ -413,6 +438,8 @@ static int q6v5_reset_deassert(struct q6v5 *qproc) ret = reset_control_reset(qproc->mss_restart); writel(0, qproc->rmb_base + RMB_MBA_ALT_RESET); reset_control_deassert(qproc->pdc_reset); + } else if (qproc->has_halt_nav) { + ret = reset_control_reset(qproc->mss_restart); } else { ret = reset_control_deassert(qproc->mss_restart); } @@ -499,6 +526,54 @@ static int q6v5proc_reset(struct q6v5 *qproc) return ret; } + goto pbl_wait; + } else if (qproc->version == MSS_SC7180) { + val = readl(qproc->reg_base + QDSP6SS_SLEEP); + val |= 0x1; + 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); + if (ret) { + dev_err(qproc->dev, "QDSP6SS Sleep clock timed out\n"); + return -ETIMEDOUT; + } + + /* Turn on the XO clock needed for PLL setup */ + val = readl(qproc->reg_base + QDSP6SS_XO_CBCR); + val |= 0x1; + 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); + if (ret) { + dev_err(qproc->dev, "QDSP6SS XO clock timed out\n"); + return -ETIMEDOUT; + } + + /* Configure Q6 core CBCR to auto-enable after reset sequence */ + val = readl(qproc->reg_base + QDSP6SS_CORE_CBCR); + val |= 0x1; + writel(val, qproc->reg_base + QDSP6SS_CORE_CBCR); + + /* De-assert the Q6 stop core signal */ + writel(1, qproc->reg_base + QDSP6SS_BOOT_CORE_START); + + /* Trigger the boot FSM to start the Q6 out-of-reset sequence */ + writel(1, qproc->reg_base + QDSP6SS_BOOT_CMD); + + /* 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); + if (ret) { + dev_err(qproc->dev, "Boot FSM failed to complete.\n"); + /* Reset the modem so that boot FSM is in reset state */ + q6v5_reset_deassert(qproc); + return ret; + } goto pbl_wait; } else if (qproc->version == MSS_MSM8996 || qproc->version == MSS_MSM8998) { @@ -667,6 +742,39 @@ static void q6v5proc_halt_axi_port(struct q6v5 *qproc, regmap_write(halt_map, offset + AXI_HALTREQ_REG, 0); } +static void q6v5proc_halt_nav_axi_port(struct q6v5 *qproc, + struct regmap *halt_map, + u32 offset) +{ + unsigned long timeout; + unsigned int val; + int ret; + + /* Check if we're already idle */ + ret = regmap_read(halt_map, offset, &val); + if (!ret && (val & NAV_AXI_IDLE_BIT)) + return; + + /* Assert halt request */ + regmap_update_bits(halt_map, offset, NAV_AXI_HALTREQ_BIT, + 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(halt_map, offset, &val); + if (ret || !(val & NAV_AXI_IDLE_BIT)) + dev_err(qproc->dev, "port failed halt\n"); +} + static int q6v5_mpss_init_image(struct q6v5 *qproc, const struct firmware *fw) { unsigned long dma_attrs = DMA_ATTR_FORCE_CONTIGUOUS; @@ -829,6 +937,9 @@ static int q6v5_mba_load(struct q6v5 *qproc) halt_axi_ports: q6v5proc_halt_axi_port(qproc, qproc->halt_map, qproc->halt_q6); q6v5proc_halt_axi_port(qproc, qproc->halt_map, qproc->halt_modem); + if (qproc->has_halt_nav) + q6v5proc_halt_nav_axi_port(qproc, qproc->halt_nav_map, + qproc->halt_nav); q6v5proc_halt_axi_port(qproc, qproc->halt_map, qproc->halt_nc); reclaim_mba: @@ -876,6 +987,9 @@ static void q6v5_mba_reclaim(struct q6v5 *qproc) q6v5proc_halt_axi_port(qproc, qproc->halt_map, qproc->halt_q6); q6v5proc_halt_axi_port(qproc, qproc->halt_map, qproc->halt_modem); + if (qproc->has_halt_nav) + q6v5proc_halt_nav_axi_port(qproc, qproc->halt_nav_map, + qproc->halt_nav); q6v5proc_halt_axi_port(qproc, qproc->halt_map, qproc->halt_nc); if (qproc->version == MSS_MSM8996) { /* @@ -1253,6 +1367,47 @@ static int q6v5_init_mem(struct q6v5 *qproc, struct platform_device *pdev) qproc->halt_modem = args.args[1]; qproc->halt_nc = args.args[2]; + if (qproc->has_halt_nav) { + struct platform_device *nav_pdev; + + ret = of_parse_phandle_with_fixed_args(pdev->dev.of_node, + "qcom,halt-nav-regs", + 1, 0, &args); + if (ret < 0) { + dev_err(&pdev->dev, "failed to parse halt-nav-regs\n"); + return -EINVAL; + } + + nav_pdev = of_find_device_by_node(args.np); + of_node_put(args.np); + if (!nav_pdev) { + dev_err(&pdev->dev, "failed to get mss clock device\n"); + return -EPROBE_DEFER; + } + + qproc->halt_nav_map = dev_get_regmap(&nav_pdev->dev, NULL); + if (!qproc->halt_nav_map) { + dev_err(&pdev->dev, "failed to get map from device\n"); + return -EINVAL; + } + qproc->halt_nav = args.args[0]; + + ret = of_parse_phandle_with_fixed_args(pdev->dev.of_node, + "qcom,halt-nav-regs", + 1, 1, &args); + if (ret < 0) { + dev_err(&pdev->dev, "failed to parse halt-nav-regs\n"); + return -EINVAL; + } + + qproc->conn_map = syscon_node_to_regmap(args.np); + of_node_put(args.np); + if (IS_ERR(qproc->conn_map)) + return PTR_ERR(qproc->conn_map); + + qproc->conn_box = args.args[0]; + } + return 0; } @@ -1327,7 +1482,7 @@ static int q6v5_init_reset(struct q6v5 *qproc) return PTR_ERR(qproc->mss_restart); } - if (qproc->has_alt_reset) { + if (qproc->has_alt_reset || qproc->has_halt_nav) { qproc->pdc_reset = devm_reset_control_get_exclusive(qproc->dev, "pdc_reset"); if (IS_ERR(qproc->pdc_reset)) { @@ -1426,6 +1581,7 @@ static int q6v5_probe(struct platform_device *pdev) platform_set_drvdata(pdev, qproc); + qproc->has_halt_nav = desc->has_halt_nav; ret = q6v5_init_mem(qproc, pdev); if (ret) goto free_rproc; @@ -1549,6 +1705,41 @@ static int q6v5_remove(struct platform_device *pdev) return 0; } +static const struct rproc_hexagon_res sc7180_mss = { + .hexagon_mba_image = "mba.mbn", + .proxy_clk_names = (char*[]){ + "xo", + NULL + }, + .reset_clk_names = (char*[]){ + "iface", + "bus", + "snoc_axi", + NULL + }, + .active_clk_names = (char*[]){ + "mnoc_axi", + "nav", + "mss_nav", + "mss_crypto", + NULL + }, + .active_pd_names = (char*[]){ + "load_state", + NULL + }, + .proxy_pd_names = (char*[]){ + "cx", + "mx", + "mss", + NULL + }, + .need_mem_protection = true, + .has_alt_reset = false, + .has_halt_nav = true, + .version = MSS_SC7180, +}; + static const struct rproc_hexagon_res sdm845_mss = { .hexagon_mba_image = "mba.mbn", .proxy_clk_names = (char*[]){ @@ -1580,6 +1771,7 @@ static const struct rproc_hexagon_res sdm845_mss = { }, .need_mem_protection = true, .has_alt_reset = true, + .has_halt_nav = false, .version = MSS_SDM845, }; @@ -1606,6 +1798,7 @@ static const struct rproc_hexagon_res msm8998_mss = { }, .need_mem_protection = true, .has_alt_reset = false, + .has_halt_nav = false, .version = MSS_MSM8998, }; @@ -1635,6 +1828,7 @@ static const struct rproc_hexagon_res msm8996_mss = { }, .need_mem_protection = true, .has_alt_reset = false, + .has_halt_nav = false, .version = MSS_MSM8996, }; @@ -1667,6 +1861,7 @@ static const struct rproc_hexagon_res msm8916_mss = { }, .need_mem_protection = false, .has_alt_reset = false, + .has_halt_nav = false, .version = MSS_MSM8916, }; @@ -1707,6 +1902,7 @@ static const struct rproc_hexagon_res msm8974_mss = { }, .need_mem_protection = false, .has_alt_reset = false, + .has_halt_nav = false, .version = MSS_MSM8974, }; @@ -1716,6 +1912,7 @@ static const struct of_device_id q6v5_of_match[] = { { .compatible = "qcom,msm8974-mss-pil", .data = &msm8974_mss}, { .compatible = "qcom,msm8996-mss-pil", .data = &msm8996_mss}, { .compatible = "qcom,msm8998-mss-pil", .data = &msm8998_mss}, + { .compatible = "qcom,sc7180-mss-pil", .data = &sc7180_mss}, { .compatible = "qcom,sdm845-mss-pil", .data = &sdm845_mss}, { }, }; -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] remoteproc: mss: q6v5-mss: Add modem support on SC7180 2019-12-19 5:45 ` [PATCH v2 2/2] remoteproc: mss: q6v5-mss: Add modem support on SC7180 Sibi Sankar @ 2020-01-02 13:26 ` Philipp Zabel 2020-01-03 11:29 ` Sibi Sankar 2020-01-07 21:21 ` Evan Green 1 sibling, 1 reply; 8+ messages in thread From: Philipp Zabel @ 2020-01-02 13:26 UTC (permalink / raw) To: Sibi Sankar, bjorn.andersson Cc: ohad, mark.rutland, linux-arm-msm, linux-remoteproc, devicetree, linux-kernel, agross On Thu, 2019-12-19 at 11:15 +0530, Sibi Sankar wrote: > Add the out of reset sequence support for modem sub-system on SC7180 > SoCs. It requires access to an additional halt nav register to put > the modem back into reset. > > Signed-off-by: Sibi Sankar <sibis@codeaurora.org> > --- > drivers/remoteproc/qcom_q6v5_mss.c | 199 ++++++++++++++++++++++++++++- > 1 file changed, 198 insertions(+), 1 deletion(-) > > diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c > index 164fc2a53ef11..51f451311f5fc 100644 > --- a/drivers/remoteproc/qcom_q6v5_mss.c > +++ b/drivers/remoteproc/qcom_q6v5_mss.c [...] > @@ -396,6 +409,18 @@ static int q6v5_reset_assert(struct q6v5 *qproc) > reset_control_assert(qproc->pdc_reset); > 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 */ > + reset_control_assert(qproc->pdc_reset); > + regmap_update_bits(qproc->conn_map, qproc->conn_box, > + BIT(0), BIT(0)); > + 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); > + ret = reset_control_deassert(qproc->mss_restart); > } else { > ret = reset_control_assert(qproc->mss_restart); > } Without knowing anything about the hardware this does look a bit weird, but I assume there is a good reason for every step. Is the function name still describing its behaviour correctly, or would it make sense to rename q6v5_reset_assert/deassert to something else? [...] > @@ -667,6 +742,39 @@ static void q6v5proc_halt_axi_port(struct q6v5 *qproc, > regmap_write(halt_map, offset + AXI_HALTREQ_REG, 0); > } > > +static void q6v5proc_halt_nav_axi_port(struct q6v5 *qproc, > + struct regmap *halt_map, > + u32 offset) > +{ [...] > + /* 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); > + } This does look like a candidate for using regmap_read_poll_timeout(). regards Philipp ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] remoteproc: mss: q6v5-mss: Add modem support on SC7180 2020-01-02 13:26 ` Philipp Zabel @ 2020-01-03 11:29 ` Sibi Sankar 0 siblings, 0 replies; 8+ messages in thread From: Sibi Sankar @ 2020-01-03 11:29 UTC (permalink / raw) To: Philipp Zabel Cc: bjorn.andersson, ohad, mark.rutland, linux-arm-msm, linux-remoteproc, devicetree, linux-kernel, agross, linux-kernel-owner Hey Philipp, Thanks for taking time to review the patch series. On 2020-01-02 18:56, Philipp Zabel wrote: > On Thu, 2019-12-19 at 11:15 +0530, Sibi Sankar wrote: >> Add the out of reset sequence support for modem sub-system on SC7180 >> SoCs. It requires access to an additional halt nav register to put >> the modem back into reset. >> >> Signed-off-by: Sibi Sankar <sibis@codeaurora.org> >> --- >> drivers/remoteproc/qcom_q6v5_mss.c | 199 >> ++++++++++++++++++++++++++++- >> 1 file changed, 198 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/remoteproc/qcom_q6v5_mss.c >> b/drivers/remoteproc/qcom_q6v5_mss.c >> index 164fc2a53ef11..51f451311f5fc 100644 >> --- a/drivers/remoteproc/qcom_q6v5_mss.c >> +++ b/drivers/remoteproc/qcom_q6v5_mss.c > [...] >> @@ -396,6 +409,18 @@ static int q6v5_reset_assert(struct q6v5 *qproc) >> reset_control_assert(qproc->pdc_reset); >> 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 */ >> + reset_control_assert(qproc->pdc_reset); >> + regmap_update_bits(qproc->conn_map, qproc->conn_box, >> + BIT(0), BIT(0)); >> + 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); >> + ret = reset_control_deassert(qproc->mss_restart); >> } else { >> ret = reset_control_assert(qproc->mss_restart); >> } > > Without knowing anything about the hardware this does look a bit weird, > but I assume there is a good reason for every step. > > Is the function name still describing its behaviour correctly, or would > it make sense to rename q6v5_reset_assert/deassert to something else? In qualcomm terminology, Q6 remote processor requires its reset deasserted while bringing it out of reset and reset asserted while putting it back into reset. However over time deassert/assert has morphed into a sequence which we still collectively refer to reset assert/ deassert. I'm prefer continuing with the same name since it helps abstract the out of reset sequence into a few simple steps (enable supplies->clk->pds->reset_deassert) (disable reset_assert->clk->pds->supplies) I'll let Bjorn decide what to do on this. > > [...] >> @@ -667,6 +742,39 @@ static void q6v5proc_halt_axi_port(struct q6v5 >> *qproc, >> regmap_write(halt_map, offset + AXI_HALTREQ_REG, 0); >> } >> >> +static void q6v5proc_halt_nav_axi_port(struct q6v5 *qproc, >> + struct regmap *halt_map, >> + u32 offset) >> +{ > [...] >> + /* 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); >> + } > > This does look like a candidate for using regmap_read_poll_timeout(). yes it does... will send a patch for it since I think the series is already in lnext. > > regards > Philipp -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] remoteproc: mss: q6v5-mss: Add modem support on SC7180 2019-12-19 5:45 ` [PATCH v2 2/2] remoteproc: mss: q6v5-mss: Add modem support on SC7180 Sibi Sankar 2020-01-02 13:26 ` Philipp Zabel @ 2020-01-07 21:21 ` Evan Green 2020-01-14 7:06 ` Sibi Sankar 1 sibling, 1 reply; 8+ messages in thread From: Evan Green @ 2020-01-07 21:21 UTC (permalink / raw) To: Sibi Sankar Cc: Bjorn Andersson, ohad, Mark Rutland, linux-arm-msm, linux-remoteproc, Philipp Zabel, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML, Andy Gross On Wed, Dec 18, 2019 at 9:45 PM Sibi Sankar <sibis@codeaurora.org> wrote: > > Add the out of reset sequence support for modem sub-system on SC7180 > SoCs. It requires access to an additional halt nav register to put > the modem back into reset. > > Signed-off-by: Sibi Sankar <sibis@codeaurora.org> > --- > drivers/remoteproc/qcom_q6v5_mss.c | 199 ++++++++++++++++++++++++++++- > 1 file changed, 198 insertions(+), 1 deletion(-) > > diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c > index 164fc2a53ef11..51f451311f5fc 100644 > --- a/drivers/remoteproc/qcom_q6v5_mss.c > +++ b/drivers/remoteproc/qcom_q6v5_mss.c > @@ -68,6 +68,9 @@ > #define AXI_HALTREQ_REG 0x0 > #define AXI_HALTACK_REG 0x4 > #define AXI_IDLE_REG 0x8 > +#define NAV_AXI_HALTREQ_BIT BIT(0) > +#define NAV_AXI_HALTACK_BIT BIT(1) > +#define NAV_AXI_IDLE_BIT BIT(2) > > #define HALT_ACK_TIMEOUT_MS 100 > > @@ -101,9 +104,11 @@ > #define QDSP6SS_ACC_OVERRIDE_VAL 0x20 > > /* QDSP6v65 parameters */ > +#define QDSP6SS_CORE_CBCR 0x20 > #define QDSP6SS_SLEEP 0x3C > #define QDSP6SS_BOOT_CORE_START 0x400 > #define QDSP6SS_BOOT_CMD 0x404 > +#define QDSP6SS_BOOT_STATUS 0x408 > #define SLEEP_CHECK_MAX_LOOPS 200 > #define BOOT_FSM_TIMEOUT 10000 > > @@ -131,6 +136,7 @@ struct rproc_hexagon_res { > int version; > bool need_mem_protection; > bool has_alt_reset; > + bool has_halt_nav; > }; > > struct q6v5 { > @@ -141,9 +147,14 @@ struct q6v5 { > void __iomem *rmb_base; > > struct regmap *halt_map; > + struct regmap *halt_nav_map; > + struct regmap *conn_map; > + > u32 halt_q6; > u32 halt_modem; > u32 halt_nc; > + u32 halt_nav; > + u32 conn_box; > > struct reset_control *mss_restart; > struct reset_control *pdc_reset; > @@ -187,6 +198,7 @@ struct q6v5 { > struct qcom_sysmon *sysmon; > bool need_mem_protection; > bool has_alt_reset; > + bool has_halt_nav; > int mpss_perm; > int mba_perm; > const char *hexagon_mdt_image; > @@ -198,6 +210,7 @@ enum { > MSS_MSM8974, > MSS_MSM8996, > MSS_MSM8998, > + MSS_SC7180, > MSS_SDM845, > }; > > @@ -396,6 +409,18 @@ static int q6v5_reset_assert(struct q6v5 *qproc) > reset_control_assert(qproc->pdc_reset); > 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 */ Can you elaborate more in this comment, or remove it? Right now it doesn't help me since I don't know what SWAR is, I don't see a reference to CONN_BOX_SPARE_0 in the code, I don't know what a CONN_BOX is, and I don't know any details about the glitch issue :) > + reset_control_assert(qproc->pdc_reset); > + regmap_update_bits(qproc->conn_map, qproc->conn_box, > + BIT(0), BIT(0)); Make a register name #define for this bit? > + 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); > + ret = reset_control_deassert(qproc->mss_restart); > } else { > ret = reset_control_assert(qproc->mss_restart); > } > @@ -413,6 +438,8 @@ static int q6v5_reset_deassert(struct q6v5 *qproc) > ret = reset_control_reset(qproc->mss_restart); > writel(0, qproc->rmb_base + RMB_MBA_ALT_RESET); > reset_control_deassert(qproc->pdc_reset); > + } else if (qproc->has_halt_nav) { > + ret = reset_control_reset(qproc->mss_restart); > } else { > ret = reset_control_deassert(qproc->mss_restart); > } > @@ -499,6 +526,54 @@ static int q6v5proc_reset(struct q6v5 *qproc) > return ret; > } > > + goto pbl_wait; Ick, this could benefit from a refactor that pulls those sorta-common steps "Remove IO clamp" through "Start core execution" into a helper function and then calls the new function from each case that needs it. > + } else if (qproc->version == MSS_SC7180) { > + val = readl(qproc->reg_base + QDSP6SS_SLEEP); > + val |= 0x1; It'd be nice if there were defines for these magic 0x1 values in this hunk too. > + 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); > + if (ret) { > + dev_err(qproc->dev, "QDSP6SS Sleep clock timed out\n"); > + return -ETIMEDOUT; > + } This toggling of the sleep bit is the same as sdm845, it could also be put into a helper function. > + > + /* Turn on the XO clock needed for PLL setup */ > + val = readl(qproc->reg_base + QDSP6SS_XO_CBCR); > + val |= 0x1; > + 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); Nit: SLEEP_CHECK_MAX_LOOPS isn't a loop count, it should really have been named something like SLEEP_CHECK_TIMEOUT_US. Could be done in a separate change. > + if (ret) { > + dev_err(qproc->dev, "QDSP6SS XO clock timed out\n"); > + return -ETIMEDOUT; > + } > + > + /* Configure Q6 core CBCR to auto-enable after reset sequence */ > + val = readl(qproc->reg_base + QDSP6SS_CORE_CBCR); > + val |= 0x1; > + writel(val, qproc->reg_base + QDSP6SS_CORE_CBCR); > + > + /* De-assert the Q6 stop core signal */ > + writel(1, qproc->reg_base + QDSP6SS_BOOT_CORE_START); > + > + /* Trigger the boot FSM to start the Q6 out-of-reset sequence */ > + writel(1, qproc->reg_base + QDSP6SS_BOOT_CMD); > + > + /* 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); > + if (ret) { > + dev_err(qproc->dev, "Boot FSM failed to complete.\n"); > + /* Reset the modem so that boot FSM is in reset state */ > + q6v5_reset_deassert(qproc); > + return ret; > + } > goto pbl_wait; > } else if (qproc->version == MSS_MSM8996 || > qproc->version == MSS_MSM8998) { > @@ -667,6 +742,39 @@ static void q6v5proc_halt_axi_port(struct q6v5 *qproc, > regmap_write(halt_map, offset + AXI_HALTREQ_REG, 0); > } > > +static void q6v5proc_halt_nav_axi_port(struct q6v5 *qproc, > + struct regmap *halt_map, > + u32 offset) > +{ > + unsigned long timeout; > + unsigned int val; > + int ret; > + > + /* Check if we're already idle */ > + ret = regmap_read(halt_map, offset, &val); > + if (!ret && (val & NAV_AXI_IDLE_BIT)) > + return; > + > + /* Assert halt request */ > + regmap_update_bits(halt_map, offset, NAV_AXI_HALTREQ_BIT, > + 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(halt_map, offset, &val); > + if (ret || !(val & NAV_AXI_IDLE_BIT)) > + dev_err(qproc->dev, "port failed halt\n"); > +} > + > static int q6v5_mpss_init_image(struct q6v5 *qproc, const struct firmware *fw) > { > unsigned long dma_attrs = DMA_ATTR_FORCE_CONTIGUOUS; > @@ -829,6 +937,9 @@ static int q6v5_mba_load(struct q6v5 *qproc) > halt_axi_ports: > q6v5proc_halt_axi_port(qproc, qproc->halt_map, qproc->halt_q6); > q6v5proc_halt_axi_port(qproc, qproc->halt_map, qproc->halt_modem); > + if (qproc->has_halt_nav) > + q6v5proc_halt_nav_axi_port(qproc, qproc->halt_nav_map, > + qproc->halt_nav); > q6v5proc_halt_axi_port(qproc, qproc->halt_map, qproc->halt_nc); > > reclaim_mba: > @@ -876,6 +987,9 @@ static void q6v5_mba_reclaim(struct q6v5 *qproc) > > q6v5proc_halt_axi_port(qproc, qproc->halt_map, qproc->halt_q6); > q6v5proc_halt_axi_port(qproc, qproc->halt_map, qproc->halt_modem); > + if (qproc->has_halt_nav) > + q6v5proc_halt_nav_axi_port(qproc, qproc->halt_nav_map, > + qproc->halt_nav); > q6v5proc_halt_axi_port(qproc, qproc->halt_map, qproc->halt_nc); > if (qproc->version == MSS_MSM8996) { > /* > @@ -1253,6 +1367,47 @@ static int q6v5_init_mem(struct q6v5 *qproc, struct platform_device *pdev) > qproc->halt_modem = args.args[1]; > qproc->halt_nc = args.args[2]; > > + if (qproc->has_halt_nav) { > + struct platform_device *nav_pdev; > + > + ret = of_parse_phandle_with_fixed_args(pdev->dev.of_node, > + "qcom,halt-nav-regs", > + 1, 0, &args); > + if (ret < 0) { > + dev_err(&pdev->dev, "failed to parse halt-nav-regs\n"); > + return -EINVAL; > + } > + > + nav_pdev = of_find_device_by_node(args.np); > + of_node_put(args.np); > + if (!nav_pdev) { > + dev_err(&pdev->dev, "failed to get mss clock device\n"); > + return -EPROBE_DEFER; > + } > + > + qproc->halt_nav_map = dev_get_regmap(&nav_pdev->dev, NULL); > + if (!qproc->halt_nav_map) { > + dev_err(&pdev->dev, "failed to get map from device\n"); > + return -EINVAL; > + } > + qproc->halt_nav = args.args[0]; > + > + ret = of_parse_phandle_with_fixed_args(pdev->dev.of_node, > + "qcom,halt-nav-regs", > + 1, 1, &args); > + if (ret < 0) { > + dev_err(&pdev->dev, "failed to parse halt-nav-regs\n"); > + return -EINVAL; > + } > + > + qproc->conn_map = syscon_node_to_regmap(args.np); > + of_node_put(args.np); > + if (IS_ERR(qproc->conn_map)) > + return PTR_ERR(qproc->conn_map); > + > + qproc->conn_box = args.args[0]; > + } > + > return 0; > } > > @@ -1327,7 +1482,7 @@ static int q6v5_init_reset(struct q6v5 *qproc) > return PTR_ERR(qproc->mss_restart); > } > > - if (qproc->has_alt_reset) { > + if (qproc->has_alt_reset || qproc->has_halt_nav) { > qproc->pdc_reset = devm_reset_control_get_exclusive(qproc->dev, > "pdc_reset"); > if (IS_ERR(qproc->pdc_reset)) { > @@ -1426,6 +1581,7 @@ static int q6v5_probe(struct platform_device *pdev) > > platform_set_drvdata(pdev, qproc); > > + qproc->has_halt_nav = desc->has_halt_nav; > ret = q6v5_init_mem(qproc, pdev); > if (ret) > goto free_rproc; > @@ -1549,6 +1705,41 @@ static int q6v5_remove(struct platform_device *pdev) > return 0; > } > > +static const struct rproc_hexagon_res sc7180_mss = { > + .hexagon_mba_image = "mba.mbn", > + .proxy_clk_names = (char*[]){ > + "xo", > + NULL > + }, > + .reset_clk_names = (char*[]){ > + "iface", > + "bus", > + "snoc_axi", > + NULL > + }, > + .active_clk_names = (char*[]){ > + "mnoc_axi", > + "nav", > + "mss_nav", > + "mss_crypto", > + NULL > + }, > + .active_pd_names = (char*[]){ > + "load_state", > + NULL > + }, > + .proxy_pd_names = (char*[]){ > + "cx", > + "mx", > + "mss", > + NULL > + }, > + .need_mem_protection = true, > + .has_alt_reset = false, > + .has_halt_nav = true, > + .version = MSS_SC7180, > +}; > + > static const struct rproc_hexagon_res sdm845_mss = { > .hexagon_mba_image = "mba.mbn", > .proxy_clk_names = (char*[]){ > @@ -1580,6 +1771,7 @@ static const struct rproc_hexagon_res sdm845_mss = { > }, > .need_mem_protection = true, > .has_alt_reset = true, > + .has_halt_nav = false, > .version = MSS_SDM845, > }; > > @@ -1606,6 +1798,7 @@ static const struct rproc_hexagon_res msm8998_mss = { > }, > .need_mem_protection = true, > .has_alt_reset = false, > + .has_halt_nav = false, > .version = MSS_MSM8998, > }; > > @@ -1635,6 +1828,7 @@ static const struct rproc_hexagon_res msm8996_mss = { > }, > .need_mem_protection = true, > .has_alt_reset = false, > + .has_halt_nav = false, > .version = MSS_MSM8996, > }; > > @@ -1667,6 +1861,7 @@ static const struct rproc_hexagon_res msm8916_mss = { > }, > .need_mem_protection = false, > .has_alt_reset = false, > + .has_halt_nav = false, > .version = MSS_MSM8916, > }; > > @@ -1707,6 +1902,7 @@ static const struct rproc_hexagon_res msm8974_mss = { > }, > .need_mem_protection = false, > .has_alt_reset = false, > + .has_halt_nav = false, > .version = MSS_MSM8974, > }; > > @@ -1716,6 +1912,7 @@ static const struct of_device_id q6v5_of_match[] = { > { .compatible = "qcom,msm8974-mss-pil", .data = &msm8974_mss}, > { .compatible = "qcom,msm8996-mss-pil", .data = &msm8996_mss}, > { .compatible = "qcom,msm8998-mss-pil", .data = &msm8998_mss}, > + { .compatible = "qcom,sc7180-mss-pil", .data = &sc7180_mss}, > { .compatible = "qcom,sdm845-mss-pil", .data = &sdm845_mss}, > { }, > }; > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] remoteproc: mss: q6v5-mss: Add modem support on SC7180 2020-01-07 21:21 ` Evan Green @ 2020-01-14 7:06 ` Sibi Sankar 2020-01-14 17:28 ` Evan Green 0 siblings, 1 reply; 8+ messages in thread From: Sibi Sankar @ 2020-01-14 7:06 UTC (permalink / raw) To: Evan Green Cc: Bjorn Andersson, ohad, Mark Rutland, linux-arm-msm, linux-remoteproc, Philipp Zabel, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML, Andy Gross Hey Evan, Thanks for the review! sry for the delayed response On 2020-01-08 02:51, Evan Green wrote: > On Wed, Dec 18, 2019 at 9:45 PM Sibi Sankar <sibis@codeaurora.org> > wrote: >> >> Add the out of reset sequence support for modem sub-system on SC7180 >> SoCs. It requires access to an additional halt nav register to put >> the modem back into reset. >> >> Signed-off-by: Sibi Sankar <sibis@codeaurora.org> >> --- >> drivers/remoteproc/qcom_q6v5_mss.c | 199 >> ++++++++++++++++++++++++++++- >> 1 file changed, 198 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/remoteproc/qcom_q6v5_mss.c >> b/drivers/remoteproc/qcom_q6v5_mss.c >> index 164fc2a53ef11..51f451311f5fc 100644 >> --- a/drivers/remoteproc/qcom_q6v5_mss.c >> +++ b/drivers/remoteproc/qcom_q6v5_mss.c >> @@ -68,6 +68,9 @@ >> #define AXI_HALTREQ_REG 0x0 >> #define AXI_HALTACK_REG 0x4 >> #define AXI_IDLE_REG 0x8 >> +#define NAV_AXI_HALTREQ_BIT BIT(0) >> +#define NAV_AXI_HALTACK_BIT BIT(1) >> +#define NAV_AXI_IDLE_BIT BIT(2) >> >> #define HALT_ACK_TIMEOUT_MS 100 >> >> @@ -101,9 +104,11 @@ >> #define QDSP6SS_ACC_OVERRIDE_VAL 0x20 >> >> /* QDSP6v65 parameters */ >> +#define QDSP6SS_CORE_CBCR 0x20 >> #define QDSP6SS_SLEEP 0x3C >> #define QDSP6SS_BOOT_CORE_START 0x400 >> #define QDSP6SS_BOOT_CMD 0x404 >> +#define QDSP6SS_BOOT_STATUS 0x408 >> #define SLEEP_CHECK_MAX_LOOPS 200 >> #define BOOT_FSM_TIMEOUT 10000 >> >> @@ -131,6 +136,7 @@ struct rproc_hexagon_res { >> int version; >> bool need_mem_protection; >> bool has_alt_reset; >> + bool has_halt_nav; >> }; >> >> struct q6v5 { >> @@ -141,9 +147,14 @@ struct q6v5 { >> void __iomem *rmb_base; >> >> struct regmap *halt_map; >> + struct regmap *halt_nav_map; >> + struct regmap *conn_map; >> + >> u32 halt_q6; >> u32 halt_modem; >> u32 halt_nc; >> + u32 halt_nav; >> + u32 conn_box; >> >> struct reset_control *mss_restart; >> struct reset_control *pdc_reset; >> @@ -187,6 +198,7 @@ struct q6v5 { >> struct qcom_sysmon *sysmon; >> bool need_mem_protection; >> bool has_alt_reset; >> + bool has_halt_nav; >> int mpss_perm; >> int mba_perm; >> const char *hexagon_mdt_image; >> @@ -198,6 +210,7 @@ enum { >> MSS_MSM8974, >> MSS_MSM8996, >> MSS_MSM8998, >> + MSS_SC7180, >> MSS_SDM845, >> }; >> >> @@ -396,6 +409,18 @@ static int q6v5_reset_assert(struct q6v5 *qproc) >> reset_control_assert(qproc->pdc_reset); >> 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 */ > > Can you elaborate more in this comment, or remove it? Right now it > doesn't help me since I don't know what SWAR is, I don't see a SWAR -> software work around I'll have to stop with the dumb abbreviations > reference to CONN_BOX_SPARE_0 in the code, I don't know what a conn_box_spare_0 is at an offset 0xb3e4 in the conn_map which is described in the bindings. > CONN_BOX is, and I don't know any details about the glitch issue :) lol, yes I get that the comment does not give details on the glitch. It was targeted towards explaining why there is a deviation in the reset_ assert sequence from the other SoCs. If you still feel the comment does not add any value I can get it removed. > >> + reset_control_assert(qproc->pdc_reset); >> + regmap_update_bits(qproc->conn_map, qproc->conn_box, >> + BIT(0), BIT(0)); > > Make a register name #define for this bit? we'll have to make one up for it since conn_box_spare_0 does not have any predefined bits and seems to be used to implement some missing functionality. > >> + 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); >> + ret = reset_control_deassert(qproc->mss_restart); >> } else { >> ret = reset_control_assert(qproc->mss_restart); >> } >> @@ -413,6 +438,8 @@ static int q6v5_reset_deassert(struct q6v5 *qproc) >> ret = reset_control_reset(qproc->mss_restart); >> writel(0, qproc->rmb_base + RMB_MBA_ALT_RESET); >> reset_control_deassert(qproc->pdc_reset); >> + } else if (qproc->has_halt_nav) { >> + ret = reset_control_reset(qproc->mss_restart); >> } else { >> ret = reset_control_deassert(qproc->mss_restart); >> } >> @@ -499,6 +526,54 @@ static int q6v5proc_reset(struct q6v5 *qproc) >> return ret; >> } >> >> + goto pbl_wait; > > Ick, this could benefit from a refactor that pulls those sorta-common > steps "Remove IO clamp" through "Start core execution" into a helper > function and then calls the new function from each case that needs it. > >> + } else if (qproc->version == MSS_SC7180) { >> + val = readl(qproc->reg_base + QDSP6SS_SLEEP); >> + val |= 0x1; > > It'd be nice if there were defines for these magic 0x1 values in this > hunk too. > >> + 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); >> + if (ret) { >> + dev_err(qproc->dev, "QDSP6SS Sleep clock timed >> out\n"); >> + return -ETIMEDOUT; >> + } > > This toggling of the sleep bit is the same as sdm845, it could also be > put into a helper function. https://patchwork.kernel.org/patch/11250301/ https://patchwork.kernel.org/patch/11250317/ I did club ^^ a portion of the out of reset sequence together reducing code duplication initially but that made the entire thing unreadable. The general consensus at that time was to introduce a patch for sc7180 that is most readable and have a cleanup later with a lot of helper functions and less branching like you suggested. > >> + >> + /* Turn on the XO clock needed for PLL setup */ >> + val = readl(qproc->reg_base + QDSP6SS_XO_CBCR); >> + val |= 0x1; >> + 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); > > Nit: SLEEP_CHECK_MAX_LOOPS isn't a loop count, it should really have > been named something like SLEEP_CHECK_TIMEOUT_US. Could be done in a > separate change. I'll skip this for now. I'll make sure that this gets named appropriately during the planned refactor. > >> + if (ret) { >> + dev_err(qproc->dev, "QDSP6SS XO clock timed >> out\n"); >> + return -ETIMEDOUT; >> + } >> + >> + /* Configure Q6 core CBCR to auto-enable after reset >> sequence */ >> + val = readl(qproc->reg_base + QDSP6SS_CORE_CBCR); >> + val |= 0x1; >> + writel(val, qproc->reg_base + QDSP6SS_CORE_CBCR); >> + >> + /* De-assert the Q6 stop core signal */ >> + writel(1, qproc->reg_base + QDSP6SS_BOOT_CORE_START); >> + >> + /* Trigger the boot FSM to start the Q6 out-of-reset >> sequence */ >> + writel(1, qproc->reg_base + QDSP6SS_BOOT_CMD); >> + >> + /* 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); >> + if (ret) { >> + dev_err(qproc->dev, "Boot FSM failed to >> complete.\n"); >> + /* Reset the modem so that boot FSM is in >> reset state */ >> + q6v5_reset_deassert(qproc); >> + return ret; >> + } >> goto pbl_wait; >> } else if (qproc->version == MSS_MSM8996 || >> qproc->version == MSS_MSM8998) { >> @@ -667,6 +742,39 @@ static void q6v5proc_halt_axi_port(struct q6v5 >> *qproc, >> regmap_write(halt_map, offset + AXI_HALTREQ_REG, 0); >> } >> >> +static void q6v5proc_halt_nav_axi_port(struct q6v5 *qproc, >> + struct regmap *halt_map, >> + u32 offset) >> +{ >> + unsigned long timeout; >> + unsigned int val; >> + int ret; >> + >> + /* Check if we're already idle */ >> + ret = regmap_read(halt_map, offset, &val); >> + if (!ret && (val & NAV_AXI_IDLE_BIT)) >> + return; >> + >> + /* Assert halt request */ >> + regmap_update_bits(halt_map, offset, NAV_AXI_HALTREQ_BIT, >> + 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(halt_map, offset, &val); >> + if (ret || !(val & NAV_AXI_IDLE_BIT)) >> + dev_err(qproc->dev, "port failed halt\n"); >> +} >> + >> static int q6v5_mpss_init_image(struct q6v5 *qproc, const struct >> firmware *fw) >> { >> unsigned long dma_attrs = DMA_ATTR_FORCE_CONTIGUOUS; >> @@ -829,6 +937,9 @@ static int q6v5_mba_load(struct q6v5 *qproc) >> halt_axi_ports: >> q6v5proc_halt_axi_port(qproc, qproc->halt_map, >> qproc->halt_q6); >> q6v5proc_halt_axi_port(qproc, qproc->halt_map, >> qproc->halt_modem); >> + if (qproc->has_halt_nav) >> + q6v5proc_halt_nav_axi_port(qproc, qproc->halt_nav_map, >> + qproc->halt_nav); >> q6v5proc_halt_axi_port(qproc, qproc->halt_map, >> qproc->halt_nc); >> >> reclaim_mba: >> @@ -876,6 +987,9 @@ static void q6v5_mba_reclaim(struct q6v5 *qproc) >> >> q6v5proc_halt_axi_port(qproc, qproc->halt_map, >> qproc->halt_q6); >> q6v5proc_halt_axi_port(qproc, qproc->halt_map, >> qproc->halt_modem); >> + if (qproc->has_halt_nav) >> + q6v5proc_halt_nav_axi_port(qproc, qproc->halt_nav_map, >> + qproc->halt_nav); >> q6v5proc_halt_axi_port(qproc, qproc->halt_map, >> qproc->halt_nc); >> if (qproc->version == MSS_MSM8996) { >> /* >> @@ -1253,6 +1367,47 @@ static int q6v5_init_mem(struct q6v5 *qproc, >> struct platform_device *pdev) >> qproc->halt_modem = args.args[1]; >> qproc->halt_nc = args.args[2]; >> >> + if (qproc->has_halt_nav) { >> + struct platform_device *nav_pdev; >> + >> + ret = >> of_parse_phandle_with_fixed_args(pdev->dev.of_node, >> + >> "qcom,halt-nav-regs", >> + 1, 0, &args); >> + if (ret < 0) { >> + dev_err(&pdev->dev, "failed to parse >> halt-nav-regs\n"); >> + return -EINVAL; >> + } >> + >> + nav_pdev = of_find_device_by_node(args.np); >> + of_node_put(args.np); >> + if (!nav_pdev) { >> + dev_err(&pdev->dev, "failed to get mss clock >> device\n"); >> + return -EPROBE_DEFER; >> + } >> + >> + qproc->halt_nav_map = dev_get_regmap(&nav_pdev->dev, >> NULL); >> + if (!qproc->halt_nav_map) { >> + dev_err(&pdev->dev, "failed to get map from >> device\n"); >> + return -EINVAL; >> + } >> + qproc->halt_nav = args.args[0]; >> + >> + ret = >> of_parse_phandle_with_fixed_args(pdev->dev.of_node, >> + >> "qcom,halt-nav-regs", >> + 1, 1, &args); >> + if (ret < 0) { >> + dev_err(&pdev->dev, "failed to parse >> halt-nav-regs\n"); >> + return -EINVAL; >> + } >> + >> + qproc->conn_map = syscon_node_to_regmap(args.np); >> + of_node_put(args.np); >> + if (IS_ERR(qproc->conn_map)) >> + return PTR_ERR(qproc->conn_map); >> + >> + qproc->conn_box = args.args[0]; >> + } >> + >> return 0; >> } >> >> @@ -1327,7 +1482,7 @@ static int q6v5_init_reset(struct q6v5 *qproc) >> return PTR_ERR(qproc->mss_restart); >> } >> >> - if (qproc->has_alt_reset) { >> + if (qproc->has_alt_reset || qproc->has_halt_nav) { >> qproc->pdc_reset = >> devm_reset_control_get_exclusive(qproc->dev, >> >> "pdc_reset"); >> if (IS_ERR(qproc->pdc_reset)) { >> @@ -1426,6 +1581,7 @@ static int q6v5_probe(struct platform_device >> *pdev) >> >> platform_set_drvdata(pdev, qproc); >> >> + qproc->has_halt_nav = desc->has_halt_nav; >> ret = q6v5_init_mem(qproc, pdev); >> if (ret) >> goto free_rproc; >> @@ -1549,6 +1705,41 @@ static int q6v5_remove(struct platform_device >> *pdev) >> return 0; >> } >> >> +static const struct rproc_hexagon_res sc7180_mss = { >> + .hexagon_mba_image = "mba.mbn", >> + .proxy_clk_names = (char*[]){ >> + "xo", >> + NULL >> + }, >> + .reset_clk_names = (char*[]){ >> + "iface", >> + "bus", >> + "snoc_axi", >> + NULL >> + }, >> + .active_clk_names = (char*[]){ >> + "mnoc_axi", >> + "nav", >> + "mss_nav", >> + "mss_crypto", >> + NULL >> + }, >> + .active_pd_names = (char*[]){ >> + "load_state", >> + NULL >> + }, >> + .proxy_pd_names = (char*[]){ >> + "cx", >> + "mx", >> + "mss", >> + NULL >> + }, >> + .need_mem_protection = true, >> + .has_alt_reset = false, >> + .has_halt_nav = true, >> + .version = MSS_SC7180, >> +}; >> + >> static const struct rproc_hexagon_res sdm845_mss = { >> .hexagon_mba_image = "mba.mbn", >> .proxy_clk_names = (char*[]){ >> @@ -1580,6 +1771,7 @@ static const struct rproc_hexagon_res sdm845_mss >> = { >> }, >> .need_mem_protection = true, >> .has_alt_reset = true, >> + .has_halt_nav = false, >> .version = MSS_SDM845, >> }; >> >> @@ -1606,6 +1798,7 @@ static const struct rproc_hexagon_res >> msm8998_mss = { >> }, >> .need_mem_protection = true, >> .has_alt_reset = false, >> + .has_halt_nav = false, >> .version = MSS_MSM8998, >> }; >> >> @@ -1635,6 +1828,7 @@ static const struct rproc_hexagon_res >> msm8996_mss = { >> }, >> .need_mem_protection = true, >> .has_alt_reset = false, >> + .has_halt_nav = false, >> .version = MSS_MSM8996, >> }; >> >> @@ -1667,6 +1861,7 @@ static const struct rproc_hexagon_res >> msm8916_mss = { >> }, >> .need_mem_protection = false, >> .has_alt_reset = false, >> + .has_halt_nav = false, >> .version = MSS_MSM8916, >> }; >> >> @@ -1707,6 +1902,7 @@ static const struct rproc_hexagon_res >> msm8974_mss = { >> }, >> .need_mem_protection = false, >> .has_alt_reset = false, >> + .has_halt_nav = false, >> .version = MSS_MSM8974, >> }; >> >> @@ -1716,6 +1912,7 @@ static const struct of_device_id q6v5_of_match[] >> = { >> { .compatible = "qcom,msm8974-mss-pil", .data = &msm8974_mss}, >> { .compatible = "qcom,msm8996-mss-pil", .data = &msm8996_mss}, >> { .compatible = "qcom,msm8998-mss-pil", .data = &msm8998_mss}, >> + { .compatible = "qcom,sc7180-mss-pil", .data = &sc7180_mss}, >> { .compatible = "qcom,sdm845-mss-pil", .data = &sdm845_mss}, >> { }, >> }; >> -- >> 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] remoteproc: mss: q6v5-mss: Add modem support on SC7180 2020-01-14 7:06 ` Sibi Sankar @ 2020-01-14 17:28 ` Evan Green 0 siblings, 0 replies; 8+ messages in thread From: Evan Green @ 2020-01-14 17:28 UTC (permalink / raw) To: Sibi Sankar Cc: Bjorn Andersson, Ohad Ben Cohen, Mark Rutland, linux-arm-msm, linux-remoteproc, Philipp Zabel, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML, Andy Gross On Mon, Jan 13, 2020 at 11:06 PM Sibi Sankar <sibis@codeaurora.org> wrote: > > Hey Evan, > > Thanks for the review! > sry for the delayed response > > On 2020-01-08 02:51, Evan Green wrote: > > On Wed, Dec 18, 2019 at 9:45 PM Sibi Sankar <sibis@codeaurora.org> > > wrote: > >> > >> Add the out of reset sequence support for modem sub-system on SC7180 > >> SoCs. It requires access to an additional halt nav register to put > >> the modem back into reset. > >> > >> Signed-off-by: Sibi Sankar <sibis@codeaurora.org> > >> --- > >> drivers/remoteproc/qcom_q6v5_mss.c | 199 > >> ++++++++++++++++++++++++++++- > >> 1 file changed, 198 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/remoteproc/qcom_q6v5_mss.c > >> b/drivers/remoteproc/qcom_q6v5_mss.c > >> index 164fc2a53ef11..51f451311f5fc 100644 > >> --- a/drivers/remoteproc/qcom_q6v5_mss.c > >> +++ b/drivers/remoteproc/qcom_q6v5_mss.c > >> @@ -68,6 +68,9 @@ > >> #define AXI_HALTREQ_REG 0x0 > >> #define AXI_HALTACK_REG 0x4 > >> #define AXI_IDLE_REG 0x8 > >> +#define NAV_AXI_HALTREQ_BIT BIT(0) > >> +#define NAV_AXI_HALTACK_BIT BIT(1) > >> +#define NAV_AXI_IDLE_BIT BIT(2) > >> > >> #define HALT_ACK_TIMEOUT_MS 100 > >> > >> @@ -101,9 +104,11 @@ > >> #define QDSP6SS_ACC_OVERRIDE_VAL 0x20 > >> > >> /* QDSP6v65 parameters */ > >> +#define QDSP6SS_CORE_CBCR 0x20 > >> #define QDSP6SS_SLEEP 0x3C > >> #define QDSP6SS_BOOT_CORE_START 0x400 > >> #define QDSP6SS_BOOT_CMD 0x404 > >> +#define QDSP6SS_BOOT_STATUS 0x408 > >> #define SLEEP_CHECK_MAX_LOOPS 200 > >> #define BOOT_FSM_TIMEOUT 10000 > >> > >> @@ -131,6 +136,7 @@ struct rproc_hexagon_res { > >> int version; > >> bool need_mem_protection; > >> bool has_alt_reset; > >> + bool has_halt_nav; > >> }; > >> > >> struct q6v5 { > >> @@ -141,9 +147,14 @@ struct q6v5 { > >> void __iomem *rmb_base; > >> > >> struct regmap *halt_map; > >> + struct regmap *halt_nav_map; > >> + struct regmap *conn_map; > >> + > >> u32 halt_q6; > >> u32 halt_modem; > >> u32 halt_nc; > >> + u32 halt_nav; > >> + u32 conn_box; > >> > >> struct reset_control *mss_restart; > >> struct reset_control *pdc_reset; > >> @@ -187,6 +198,7 @@ struct q6v5 { > >> struct qcom_sysmon *sysmon; > >> bool need_mem_protection; > >> bool has_alt_reset; > >> + bool has_halt_nav; > >> int mpss_perm; > >> int mba_perm; > >> const char *hexagon_mdt_image; > >> @@ -198,6 +210,7 @@ enum { > >> MSS_MSM8974, > >> MSS_MSM8996, > >> MSS_MSM8998, > >> + MSS_SC7180, > >> MSS_SDM845, > >> }; > >> > >> @@ -396,6 +409,18 @@ static int q6v5_reset_assert(struct q6v5 *qproc) > >> reset_control_assert(qproc->pdc_reset); > >> 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 */ > > > > Can you elaborate more in this comment, or remove it? Right now it > > doesn't help me since I don't know what SWAR is, I don't see a > > SWAR -> software work around > > I'll have to stop with the dumb > abbreviations heh I've never heard that one before. SWWA? > > > reference to CONN_BOX_SPARE_0 in the code, I don't know what a > > conn_box_spare_0 is at an offset > 0xb3e4 in the conn_map which is > described in the bindings. > > > CONN_BOX is, and I don't know any details about the glitch issue :) > > lol, yes I get that the comment does > not give details on the glitch. It > was targeted towards explaining why > there is a deviation in the reset_ > assert sequence from the other SoCs. > If you still feel the comment does > not add any value I can get it > removed. Maybe just infusing some of the clarity you described here would be enough. > > > > >> + reset_control_assert(qproc->pdc_reset); > >> + regmap_update_bits(qproc->conn_map, qproc->conn_box, > >> + BIT(0), BIT(0)); > > > > Make a register name #define for this bit? > > we'll have to make one up for it > since conn_box_spare_0 does not > have any predefined bits and seems > to be used to implement some missing > functionality. Hm, so its functionality is controlled by firmware? Or was just defined super late in hardware so as to be termed spare through many earlier spins of the hardware? Making one up sounds fine, it's just nice for readability to get a sense of what exactly this bit is when we toggle it. > > > > >> + 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); > >> + ret = reset_control_deassert(qproc->mss_restart); > >> } else { > >> ret = reset_control_assert(qproc->mss_restart); > >> } > >> @@ -413,6 +438,8 @@ static int q6v5_reset_deassert(struct q6v5 *qproc) > >> ret = reset_control_reset(qproc->mss_restart); > >> writel(0, qproc->rmb_base + RMB_MBA_ALT_RESET); > >> reset_control_deassert(qproc->pdc_reset); > >> + } else if (qproc->has_halt_nav) { > >> + ret = reset_control_reset(qproc->mss_restart); > >> } else { > >> ret = reset_control_deassert(qproc->mss_restart); > >> } > >> @@ -499,6 +526,54 @@ static int q6v5proc_reset(struct q6v5 *qproc) > >> return ret; > >> } > >> > >> + goto pbl_wait; > > > > Ick, this could benefit from a refactor that pulls those sorta-common > > steps "Remove IO clamp" through "Start core execution" into a helper > > function and then calls the new function from each case that needs it. > > > >> + } else if (qproc->version == MSS_SC7180) { > >> + val = readl(qproc->reg_base + QDSP6SS_SLEEP); > >> + val |= 0x1; > > > > It'd be nice if there were defines for these magic 0x1 values in this > > hunk too. > > > >> + 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); > >> + if (ret) { > >> + dev_err(qproc->dev, "QDSP6SS Sleep clock timed > >> out\n"); > >> + return -ETIMEDOUT; > >> + } > > > > This toggling of the sleep bit is the same as sdm845, it could also be > > put into a helper function. > > https://patchwork.kernel.org/patch/11250301/ > https://patchwork.kernel.org/patch/11250317/ > > I did club ^^ a portion of the out of > reset sequence together reducing > code duplication initially but that > made the entire thing unreadable. > > The general consensus at that time > was to introduce a patch for sc7180 > that is most readable and have a > cleanup later with a lot of helper > functions and less branching like > you suggested. Sounds good. > > > > >> + > >> + /* Turn on the XO clock needed for PLL setup */ > >> + val = readl(qproc->reg_base + QDSP6SS_XO_CBCR); > >> + val |= 0x1; > >> + 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); > > > > Nit: SLEEP_CHECK_MAX_LOOPS isn't a loop count, it should really have > > been named something like SLEEP_CHECK_TIMEOUT_US. Could be done in a > > separate change. > > I'll skip this for now. I'll > make sure that this gets named > appropriately during the > planned refactor. Thanks! ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-01-14 17:29 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-12-19 5:45 [PATCH v2 0/2] Add Modem support on SC7180 SoCs Sibi Sankar 2019-12-19 5:45 ` [PATCH v2 1/2] dt-bindings: remoteproc: qcom: Add Q6V5 Modem PIL binding for SC7180 Sibi Sankar 2019-12-19 5:45 ` [PATCH v2 2/2] remoteproc: mss: q6v5-mss: Add modem support on SC7180 Sibi Sankar 2020-01-02 13:26 ` Philipp Zabel 2020-01-03 11:29 ` Sibi Sankar 2020-01-07 21:21 ` Evan Green 2020-01-14 7:06 ` Sibi Sankar 2020-01-14 17:28 ` Evan Green
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).