On 05-06-18, 11:12, Sricharan R wrote: > +config QCOM_Q6V5_WCSS > + tristate "Qualcomm Hexagon based WCSS Peripheral Image Loader" > + depends on OF && ARCH_QCOM > + depends on QCOM_SMEM > + depends on RPMSG_QCOM_SMD || (COMPILE_TEST && RPMSG_QCOM_SMD=n) > + depends on RPMSG_QCOM_GLINK_SMEM || RPMSG_QCOM_GLINK_SMEM=n
Is there a reason why it depends on RPMSG_QCOM_GLINK_SMEM=n? What would happen if distro wants both this and RPMSG_QCOM_GLINK_SMEM > +/* QDSP6SS Register Offsets */ > +#define QDSP6SS_RESET_REG 0x014 > +#define QDSP6SS_GFMUX_CTL_REG 0x020 > +#define QDSP6SS_PWR_CTL_REG 0x030 > +#define QDSP6SS_MEM_PWR_CTL 0x0B0 > + > +/* AXI Halt Register Offsets */ > +#define AXI_HALTREQ_REG 0x0 > +#define AXI_HALTACK_REG 0x4 > +#define AXI_IDLE_REG 0x8 > + > +#define HALT_ACK_TIMEOUT_MS 100 > + > +/* QDSP6SS_RESET */ > +#define Q6SS_STOP_CORE BIT(0) > +#define Q6SS_CORE_ARES BIT(1) > +#define Q6SS_BUS_ARES_ENABLE BIT(2) Wouldn't it be nice if the defines are all consistent, some of them are QDSP6SS_xxx, some Q6SS_ some are not Please do pick one and make it consistent :) > +/* QDSP6v56 parameters */ > +#define QDSP6v56_LDO_BYP BIT(25) > +#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 GENMASK() perhaps? > +static int q6v5_wcss_reset(struct q6v5_wcss *wcss) > +{ > + int ret; > + u32 val; > + int i; int ret, i; > +static int q6v5_wcss_start(struct rproc *rproc) > +{ > + struct q6v5_wcss *wcss = rproc->priv; > + int ret; > + > + qcom_q6v5_prepare(&wcss->q6v5); > + > + /* Release Q6 and WCSS reset */ > + ret = reset_control_deassert(wcss->wcss_reset); > + if (ret) { > + dev_err(wcss->dev, "wcss_reset failed\n"); > + return ret; > + } > + > + ret = reset_control_deassert(wcss->wcss_q6_reset); > + if (ret) { > + dev_err(wcss->dev, "wcss_q6_reset failed\n"); > + goto wcss_reset; > + } > + > + /* Lithium configuration - clock gating and bus arbitration */ > + ret = regmap_update_bits(wcss->halt_map, > + wcss->halt_nc + TCSR_GLOBAL_CFG0, > + 0x1F, 0x14); magic numbers?? > +static int q6v5_wcss_powerdown(struct q6v5_wcss *wcss) > +{ > + int ret; > + u32 val; > + > + /* 1 - Assert WCSS/Q6 HALTREQ */ > + q6v5_wcss_halt_axi_port(wcss, wcss->halt_map, wcss->halt_wcss); > + > + /* 2 - Enable WCSSAON_CONFIG */ > + val = readl(wcss->rmb_base + SSCAON_CONFIG); > + val |= SSCAON_ENABLE; > + writel(val, wcss->rmb_base + SSCAON_CONFIG); > + > + /* 3 - Set SSCAON_CONFIG */ > + val |= BIT(15); > + val &= ~BIT(16); > + val &= ~BIT(17); > + val &= ~BIT(18); wouldn't it be nice to define these bits? > +static int q6v5_q6_powerdown(struct q6v5_wcss *wcss) > +{ > + int ret; > + u32 val; > + int i; int ret, i; -- ~Vinod