| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAE-0n5371rNqs6+_ZRtPDqOb7WCrzXUHbxGMjPAdVeLsGgX8_w@mail.gmail.com>
Date: Fri, 19 Nov 2021 01:54:49 +0100
From: Stephen Boyd <swboyd@...omium.org>
To: Rakesh Pillai <pillair@...eaurora.org>, agross@...nel.org,
bjorn.andersson@...aro.org, mathieu.poirier@...aro.org,
ohad@...ery.com, p.zabel@...gutronix.de, robh+dt@...nel.org
Cc: linux-arm-msm@...r.kernel.org, linux-remoteproc@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
sibis@...eaurora.org, mpubbise@...eaurora.org, kuabhs@...omium.org
Subject: Re: [PATCH v9 3/3] remoteproc: qcom: q6v5_wpss: Add support for
sc7280 WPSS
Quoting Rakesh Pillai (2021-11-18 07:50:20)
> Add support for PIL loading of WPSS processor for SC7280
> - WPSS boot will be requested by the wifi driver and hence
> disable auto-boot for WPSS.
> - Add a separate shutdown sequence handler for WPSS.
> - Add multiple power-domain voting support
> - Parse firmware-name from dtsi entry
>
> Signed-off-by: Rakesh Pillai <pillair@...eaurora.org>
> ---
Just a couple nitpicks. Otherwise looks good to me.
> drivers/remoteproc/qcom_q6v5_adsp.c | 222 +++++++++++++++++++++++++++++++++---
> 1 file changed, 206 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c b/drivers/remoteproc/qcom_q6v5_adsp.c
> index 098362e6..34a6b73 100644
> --- a/drivers/remoteproc/qcom_q6v5_adsp.c
> +++ b/drivers/remoteproc/qcom_q6v5_adsp.c
> @@ -32,6 +32,7 @@
>
> /* time out value */
> #define ACK_TIMEOUT 1000
> +#define ACK_TIMEOUT_US 1000000
> #define BOOT_FSM_TIMEOUT 10000
> /* mask values */
> #define EVB_MASK GENMASK(27, 4)
> @@ -51,6 +52,8 @@
> #define QDSP6SS_CORE_CBCR 0x20
> #define QDSP6SS_SLEEP_CBCR 0x3c
>
> +#define QCOM_Q6V5_RPROC_PROXY_PD_MAX 3
> +
> struct adsp_pil_data {
> int crash_reason_smem;
> const char *firmware_name;
> @@ -58,9 +61,13 @@ struct adsp_pil_data {
> const char *ssr_name;
> const char *sysmon_name;
> int ssctl_id;
> + bool is_wpss;
> + bool auto_boot;
>
> const char **clk_ids;
> int num_clks;
> + const char **proxy_pd_names;
> + const char *load_state;
> };
>
> struct qcom_adsp {
> @@ -93,11 +100,146 @@ struct qcom_adsp {
> void *mem_region;
> size_t mem_size;
>
> + struct device *proxy_pds[QCOM_Q6V5_RPROC_PROXY_PD_MAX];
> + size_t proxy_pd_count;
> +
> struct qcom_rproc_glink glink_subdev;
> struct qcom_rproc_ssr ssr_subdev;
> struct qcom_sysmon *sysmon;
> +
> + int (*shutdown)(struct qcom_adsp *adsp);
> };
>
> +static int qcom_rproc_pds_attach(struct device *dev, struct device **devs,
Can 'devs' be replaced by 'struct qcom_adsp'? And then we can compare
the size against ARRAY_SIZE(adsp->proxy_pds) instead of the #define.
> + const char **pd_names)
> +{
> + size_t num_pds = 0;
> + int ret;
> + int i;
> +
> + if (!pd_names)
> + return 0;
> +
> + /* Handle single power domain */
> + if (dev->pm_domain) {
> + devs[0] = dev;
> + pm_runtime_enable(dev);
> + return 1;
> + }
> +
> + while (pd_names[num_pds])
> + num_pds++;
> +
> + if (num_pds > QCOM_Q6V5_RPROC_PROXY_PD_MAX)
> + return -E2BIG;
> +
> + for (i = 0; i < num_pds; i++) {
> + devs[i] = dev_pm_domain_attach_by_name(dev, pd_names[i]);
> + if (IS_ERR_OR_NULL(devs[i])) {
> + ret = PTR_ERR(devs[i]) ? : -ENODATA;
> + goto unroll_attach;
> + }
> + }
> +
> + return num_pds;
> +
> +unroll_attach:
> + for (i--; i >= 0; i--)
> + dev_pm_domain_detach(devs[i], false);
> +
> + return ret;
> +}
> +
> +static void qcom_rproc_pds_detach(struct qcom_adsp *adsp, struct device **pds,
> + size_t pd_count)
> +{
> + struct device *dev = adsp->dev;
> + int i;
> +
> + /* Handle single power domain */
> + if (dev->pm_domain && pd_count) {
> + pm_runtime_disable(dev);
> + return;
> + }
> +
> + for (i = 0; i < pd_count; i++)
> + dev_pm_domain_detach(pds[i], false);
> +}
> +
> +static int qcom_rproc_pds_enable(struct qcom_adsp *adsp, struct device **pds,
> + size_t pd_count)
> +{
> + int ret;
> + int i;
> +
> + for (i = 0; i < pd_count; i++) {
> + dev_pm_genpd_set_performance_state(pds[i], INT_MAX);
> + ret = pm_runtime_get_sync(pds[i]);
> + if (ret < 0) {
> + pm_runtime_put_noidle(pds[i]);
> + dev_pm_genpd_set_performance_state(pds[i], 0);
> + goto unroll_pd_votes;
> + }
> + }
> +
> + return 0;
> +
> +unroll_pd_votes:
> + for (i--; i >= 0; i--) {
> + dev_pm_genpd_set_performance_state(pds[i], 0);
> + pm_runtime_put(pds[i]);
> + }
> +
> + return ret;
> +}
> +
> +static void qcom_rproc_pds_disable(struct qcom_adsp *adsp, struct device **pds,
> + size_t pd_count)
> +{
> + int i;
> +
> + for (i = 0; i < pd_count; i++) {
> + dev_pm_genpd_set_performance_state(pds[i], 0);
> + pm_runtime_put(pds[i]);
> + }
> +}
> +
> +static int qcom_wpss_shutdown(struct qcom_adsp *adsp)
> +{
> + unsigned int val;
> +
> + regmap_write(adsp->halt_map, adsp->halt_lpass + LPASS_HALTREQ_REG, 1);
> +
> + /* Wait for halt ACK from QDSP6 */
> + regmap_read_poll_timeout(adsp->halt_map,
> + adsp->halt_lpass + LPASS_HALTACK_REG, val,
> + val, 1000, ACK_TIMEOUT_US);
> +
> + /* Assert the WPSS PDC Reset */
> + reset_control_assert(adsp->pdc_sync_reset);
> + /* Place the WPSS processor into reset */
> + reset_control_assert(adsp->restart);
> + /* wait after asserting subsystem restart from AOSS */
> + usleep_range(200, 205);
> + /* Remove the WPSS reset */
> + reset_control_deassert(adsp->restart);
> + /* De-assert the WPSS PDC Reset */
> + reset_control_deassert(adsp->pdc_sync_reset);
Please add newlines between comments and previous code. The above chunk
is really hard to read.
> +
> + usleep_range(100, 105);
> +
> + clk_bulk_disable_unprepare(adsp->num_clks, adsp->clks);
> +
> + regmap_write(adsp->halt_map, adsp->halt_lpass + LPASS_HALTREQ_REG, 0);
> +
> + /* Wait for halt ACK from QDSP6 */
> + regmap_read_poll_timeout(adsp->halt_map,
> + adsp->halt_lpass + LPASS_HALTACK_REG, val,
> + !val, 1000, ACK_TIMEOUT_US);
> +
> + return 0;
> +}
> +
> static int qcom_adsp_shutdown(struct qcom_adsp *adsp)
> {
> unsigned long timeout;
Powered by blists - more mailing lists