Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Message-ID: <5dcf376f.1c69fb81.6fa51.4fa7@mx.google.com>
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
In-Reply-To: <1573593774-12539-7-git-send-email-eberman@codeaurora.org>
References: <1573593774-12539-1-git-send-email-eberman@codeaurora.org> <1573593774-12539-7-git-send-email-eberman@codeaurora.org>
Cc:     Elliot Berman <eberman@codeaurora.org>, tsoni@codeaurora.org,
        sidgup@codeaurora.org, psodagud@codeaurora.org,
        linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org
To:     Elliot Berman <eberman@codeaurora.org>, agross@kernel.org,
        bjorn.andersson@linaro.org, saiprakash.ranjan@codeaurora.org
Subject: Re: [PATCH v2 06/18] firmware: qcom_scm-64: Move svc/cmd/owner into qcom_scm_desc
From:   Stephen Boyd <swboyd@chromium.org>
User-Agent: alot/0.8.1
Date:   Fri, 15 Nov 2019 15:40:30 -0800
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Quoting Elliot Berman (2019-11-12 13:22:42)
> diff --git a/drivers/firmware/qcom_scm-64.c b/drivers/firmware/qcom_scm-6=
4.c
> index 7de6022..e81fb6e 100644
> --- a/drivers/firmware/qcom_scm-64.c
> +++ b/drivers/firmware/qcom_scm-64.c
> @@ -46,8 +46,11 @@ enum qcom_scm_arg_types {
>   * @res:       The values returned by the secure syscall
>   */
>  struct qcom_scm_desc {
> +       u32 svc;
> +       u32 cmd;
>         u32 arginfo;
>         u64 args[MAX_QCOM_SCM_ARGS];
> +       u32 owner;

Owner is always the same. Why move it to the qcom_scm_desc structure? I
can understand svc and cmd but there's no justification for this struct
member.

>  };
> =20
>  static u64 qcom_smccc_convention =3D -1;
[...]
> @@ -542,26 +584,32 @@ int __qcom_scm_io_readl(struct device *dev, phys_ad=
dr_t addr,
> =20
>  int __qcom_scm_io_writel(struct device *dev, phys_addr_t addr, unsigned =
int val)
>  {
> -       struct qcom_scm_desc desc =3D {0};
> +       struct qcom_scm_desc desc =3D {
> +               .svc =3D QCOM_SCM_SVC_IO,
> +               .cmd =3D QCOM_SCM_IO_WRITE,
> +               .owner =3D ARM_SMCCC_OWNER_SIP,
> +       };
>         struct arm_smccc_res res;
> =20
>         desc.args[0] =3D addr;
>         desc.args[1] =3D val;
>         desc.arginfo =3D QCOM_SCM_ARGS(2);

The style is weird here. Half of the struct is initialized during
declaration and the arguments are set here with individual statements.
Can we have one way or the other and not both?

> =20
> -       return qcom_scm_call(dev, QCOM_SCM_SVC_IO, QCOM_SCM_IO_WRITE,
> -                            &desc, &res);
> +       return qcom_scm_call(dev, &desc, &res);
>  }
> =20