Re: [PATCH 2/9] remoteproc: qcom: q6v5-mss: Add modem support on MSM8953

Sireesh Kodali
2022-05-12T05:16:31Z

On Wed May 11, 2022 at 10:24 PM IST, Dmitry Baryshkov wrote:
> On 11/05/2022 19:15, Sireesh Kodali wrote:
> > The modem on the MSM8953 platform is similar to the modem on the MSM8996
> > platform in terms of set up. It differs primarily in the way it needs SCM
> > to bless the MPSS firmware region.
> >
> > Signed-off-by: Sireesh Kodali <sireeshkodali1@gmail.com>
> > ---
> > drivers/remoteproc/qcom_q6v5_mss.c | 64 +++++++++++++++++++++++++++---
> > 1 file changed, 58 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
> > index af217de75e4d..a73fdcddeda4 100644
> > --- a/drivers/remoteproc/qcom_q6v5_mss.c
> > +++ b/drivers/remoteproc/qcom_q6v5_mss.c
> > @@ -234,6 +234,7 @@ struct q6v5 {
> >
> > enum {
> > MSS_MSM8916,
> > + MSS_MSM8953,
> > MSS_MSM8974,
> > MSS_MSM8996,
> > MSS_MSM8998,
> > @@ -687,12 +688,14 @@ static int q6v5proc_reset(struct q6v5 *qproc)
> > }
> > goto pbl_wait;
> > } else if (qproc->version == MSS_MSM8996 ||
> > - qproc->version == MSS_MSM8998) {
> > + qproc->version == MSS_MSM8998 ||
> > + qproc->version == MSS_MSM8953) {
> > int mem_pwr_ctl;
> >
> > /* Override the ACC value if required */
> > - writel(QDSP6SS_ACC_OVERRIDE_VAL,
> > - qproc->reg_base + QDSP6SS_STRAP_ACC);
> > + if (qproc->version != MSS_MSM8953)
> > + writel(QDSP6SS_ACC_OVERRIDE_VAL,
> > + qproc->reg_base + QDSP6SS_STRAP_ACC);
> >
> > /* Assert resets, stop core */
> > val = readl(qproc->reg_base + QDSP6SS_RESET_REG);
> > @@ -734,7 +737,8 @@ static int q6v5proc_reset(struct q6v5 *qproc)
> > writel(val, qproc->reg_base + QDSP6SS_PWR_CTL_REG);
> >
> > /* Turn on L1, L2, ETB and JU memories 1 at a time */
> > - if (qproc->version == MSS_MSM8996) {
> > + if (qproc->version == MSS_MSM8996 ||
> > + qproc->version == MSS_MSM8953) {
> > mem_pwr_ctl = QDSP6SS_MEM_PWR_CTL;
> > i = 19;
> > } else {
> > @@ -1314,7 +1318,16 @@ static int q6v5_mpss_load(struct q6v5 *qproc)
> > max_addr = ALIGN(phdr->p_paddr + phdr->p_memsz, SZ_4K);
> > }
> >
> > - /*
> > + if (qproc->version == MSS_MSM8953) {
> > + ret = qcom_scm_pas_mem_setup(5, qproc->mpss_phys, qproc->mpss_size);
> > + if (ret) {
> > + dev_err(qproc->dev,
> > + "setting up mpss memory failed: %d\n", ret);
> > + goto release_firmware;
> > + }
> > + }
> > +
> > + /**
>
> Single star please
>
> > * In case of a modem subsystem restart on secure devices, the modem
> > * memory can be reclaimed only after MBA is loaded.
> > */
> > @@ -1413,7 +1426,6 @@ static int q6v5_mpss_load(struct q6v5 *qproc)
> > writel(RMB_CMD_LOAD_READY, qproc->rmb_base + RMB_MBA_COMMAND_REG);
> > }
> > writel(size, qproc->rmb_base + RMB_PMI_CODE_LENGTH_REG);
> > -
> > ret = readl(qproc->rmb_base + RMB_MBA_STATUS_REG);
> > if (ret < 0) {
> > dev_err(qproc->dev, "MPSS authentication failed: %d\n",
> > @@ -1422,6 +1434,7 @@ static int q6v5_mpss_load(struct q6v5 *qproc)
> > }
> > }
> >
> > +
>
> Unnecessary
>

oops

> > /* Transfer ownership of modem ddr region to q6 */
> > ret = q6v5_xfer_mem_ownership(qproc, &qproc->mpss_perm, false, true,
> > qproc->mpss_phys, qproc->mpss_size);
> > @@ -2198,6 +2211,44 @@ static const struct rproc_hexagon_res msm8996_mss = {
> > .version = MSS_MSM8996,
> > };
> >
> > +static const struct rproc_hexagon_res msm8953_mss = {
> > + .hexagon_mba_image = "mba.mbn",
> > + .proxy_supply = (struct qcom_mss_reg_res[]) {
> > + {
> > + .supply = "pll",
> > + .uA = 100000,
> > + },
> > + {}
> > + },
> > + .proxy_pd_names = (char*[]) {
> > + "cx",
> > + "mx",
> > + NULL
> > + },
> > + .active_supply = (struct qcom_mss_reg_res[]) {
> > + {
> > + .supply = "mss",
> > + .uV = 1050000,
> > + .uA = 100000,
> > + },
> > + {}
> > + },
> > + .proxy_clk_names = (char*[]){
> > + "xo",
> > + NULL
> > + },
> > + .active_clk_names = (char*[]){
> > + "iface",
> > + "bus",
> > + "mem",
> > + NULL
> > + },
> > + .need_mem_protection = false,
> > + .has_alt_reset = false,
> > + .has_spare_reg = false,
>
>
> Please follow the custom and define the rest of fields here.
>

I missed these in the rebase, I'll add them in v2

> > + .version = MSS_MSM8953,
> > +};
> > +
> > static const struct rproc_hexagon_res msm8916_mss = {
> > .hexagon_mba_image = "mba.mbn",
> > .proxy_supply = (struct qcom_mss_reg_res[]) {
> > @@ -2301,6 +2352,7 @@ static const struct of_device_id q6v5_of_match[] = {
> > { .compatible = "qcom,msm8916-mss-pil", .data = &msm8916_mss},
> > { .compatible = "qcom,msm8974-mss-pil", .data = &msm8974_mss},
> > { .compatible = "qcom,msm8996-mss-pil", .data = &msm8996_mss},
> > + { .compatible = "qcom,msm8953-mss-pil", .data = &msm8953_mss},
> > { .compatible = "qcom,msm8998-mss-pil", .data = &msm8998_mss},
> > { .compatible = "qcom,sc7180-mss-pil", .data = &sc7180_mss},
> > { .compatible = "qcom,sc7280-mss-pil", .data = &sc7280_mss},
>
>
> --
> With best wishes
> Dmitry

Thank you for the review, I'll make the changes in v2 of this patch.

Thanks,
Sireesh