Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220830100700.344594-1-sudeep.holla@arm.com> <20220830100700.344594-2-sudeep.holla@arm.com>
In-Reply-To: <20220830100700.344594-2-sudeep.holla@arm.com>
From:   Jens Wiklander <jens.wiklander@linaro.org>
Date:   Thu, 1 Sep 2022 09:42:55 +0200
Message-ID: <CAHUa44E8q27hy2i+sckuWd5GahGg=QxgLUWLAMQwCrxhUGNrNQ@mail.gmail.com>
Subject: Re: [PATCH 1/9] firmware: arm_ffa: Add pointer to the ffa_dev_ops in
 struct ffa_dev
To:     Sudeep Holla <sudeep.holla@arm.com>
Cc:     linux-kernel@vger.kernel.org, op-tee@lists.trustedfirmware.org,
        Marc Bonnici <marc.bonnici@arm.com>,
        Achin Gupta <achin.gupta@arm.com>,
        Valentin Laurent <valentin.laurent@trustonic.com>,
        Lukas Hanel <lukas.hanel@trustonic.com>,
        Coboy Chen <coboy.chen@mediatek.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, Aug 30, 2022 at 12:07 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> Currently ffa_dev_ops_get() is the way to fetch the ffa_dev_ops pointer.
> It checks if the ffa_dev structure pointer is valid before returning the
> ffa_dev_ops pointer.
>
> Instead, the pointer can be made part of the ffa_dev structure and since
> the core driver is incharge of creating ffa_device for each identified
> partition, there is no need to check for the validity explicitly if the
> pointer is embedded in the structure.
>
> Add the pointer to the ffa_dev_ops in the ffa_dev structure itself and
> initialise the same as part of creation of the device.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  drivers/firmware/arm_ffa/bus.c    | 4 +++-
>  drivers/firmware/arm_ffa/driver.c | 2 +-
>  include/linux/arm_ffa.h           | 7 +++++--
>  3 files changed, 9 insertions(+), 4 deletions(-)

Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>

>
> diff --git a/drivers/firmware/arm_ffa/bus.c b/drivers/firmware/arm_ffa/bus.c
> index 641a91819088..69328041fbc3 100644
> --- a/drivers/firmware/arm_ffa/bus.c
> +++ b/drivers/firmware/arm_ffa/bus.c
> @@ -167,7 +167,8 @@ bool ffa_device_is_valid(struct ffa_device *ffa_dev)
>         return valid;
>  }
>
> -struct ffa_device *ffa_device_register(const uuid_t *uuid, int vm_id)
> +struct ffa_device *ffa_device_register(const uuid_t *uuid, int vm_id,
> +                                      const struct ffa_dev_ops *ops)
>  {
>         int ret;
>         struct device *dev;
> @@ -183,6 +184,7 @@ struct ffa_device *ffa_device_register(const uuid_t *uuid, int vm_id)
>         dev_set_name(&ffa_dev->dev, "arm-ffa-%04x", vm_id);
>
>         ffa_dev->vm_id = vm_id;
> +       ffa_dev->ops = ops;
>         uuid_copy(&ffa_dev->uuid, uuid);
>
>         ret = device_register(&ffa_dev->dev);
> diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
> index ec731e9e942b..213665e5ad0e 100644
> --- a/drivers/firmware/arm_ffa/driver.c
> +++ b/drivers/firmware/arm_ffa/driver.c
> @@ -688,7 +688,7 @@ static void ffa_setup_partitions(void)
>                  * as part of the discovery API, we need to pass the
>                  * discovered UUID here instead.
>                  */
> -               ffa_dev = ffa_device_register(&uuid_null, tpbuf->id);
> +               ffa_dev = ffa_device_register(&uuid_null, tpbuf->id, &ffa_ops);
>                 if (!ffa_dev) {
>                         pr_err("%s: failed to register partition ID 0x%x\n",
>                                __func__, tpbuf->id);
> diff --git a/include/linux/arm_ffa.h b/include/linux/arm_ffa.h
> index e5c76c1ef9ed..91b47e42b73d 100644
> --- a/include/linux/arm_ffa.h
> +++ b/include/linux/arm_ffa.h
> @@ -17,6 +17,7 @@ struct ffa_device {
>         bool mode_32bit;
>         uuid_t uuid;
>         struct device dev;
> +       const struct ffa_dev_ops *ops;
>  };
>
>  #define to_ffa_dev(d) container_of(d, struct ffa_device, dev)
> @@ -47,7 +48,8 @@ static inline void *ffa_dev_get_drvdata(struct ffa_device *fdev)
>  }
>
>  #if IS_REACHABLE(CONFIG_ARM_FFA_TRANSPORT)
> -struct ffa_device *ffa_device_register(const uuid_t *uuid, int vm_id);
> +struct ffa_device *ffa_device_register(const uuid_t *uuid, int vm_id,
> +                                      const struct ffa_dev_ops *ops);
>  void ffa_device_unregister(struct ffa_device *ffa_dev);
>  int ffa_driver_register(struct ffa_driver *driver, struct module *owner,
>                         const char *mod_name);
> @@ -57,7 +59,8 @@ const struct ffa_dev_ops *ffa_dev_ops_get(struct ffa_device *dev);
>
>  #else
>  static inline
> -struct ffa_device *ffa_device_register(const uuid_t *uuid, int vm_id)
> +struct ffa_device *ffa_device_register(const uuid_t *uuid, int vm_id,
> +                                      const struct ffa_dev_ops *ops)
>  {
>         return NULL;
>  }
> --
> 2.37.2
>