All of lore.kernel.org
 help / color / mirror / Atom feed
From: Simon Glass <sjg@chromium.org>
To: u-boot@lists.denx.de
Subject: [PATCH v2 1/3] board: ns3: add optee based bnxt fw load driver
Date: Sun, 24 May 2020 20:15:20 -0600	[thread overview]
Message-ID: <CAPnjgZ3JkOVOOAXvyVY2J-c1hxb-YU8xQF5OHmfV3H-CZRLhHg@mail.gmail.com> (raw)
In-Reply-To: <20200524102419.5568-2-rayagonda.kokatanur@broadcom.com>

Hi Rayagonda,

On Sun, 24 May 2020 at 04:24, Rayagonda Kokatanur
<rayagonda.kokatanur@broadcom.com> wrote:
>
> From: Vikas Gupta <vikas.gupta@broadcom.com>
>
> Add optee based bnxt fw load driver.
> bnxt is Broadcom NetXtreme controller Ethernet card.
> This driver is used to load bnxt firmware binary using OpTEE.
>
> Signed-off-by: Vikas Gupta <vikas.gupta@broadcom.com>
> Signed-off-by: Rayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
> ---
> Changes from v1:
>  -Address review comments from Thomas Fitzsimmons,
>   Expand the bnxt full form.
>
>  -Address review comments from Simon Glass,
>   Move c file from board/broadcom/bcmns3/chimp_optee.c to
>   drivers/tee/broadcom,
>   Move header file from include/brcm/chimp.h to include/broadcom/chimp.h
>
>  drivers/tee/Kconfig                |   1 +
>  drivers/tee/Makefile               |   1 +
>  drivers/tee/broadcom/Kconfig       |   7 ++
>  drivers/tee/broadcom/Makefile      |   3 +
>  drivers/tee/broadcom/chimp_optee.c | 154 +++++++++++++++++++++++++++++
>  include/broadcom/chimp.h           |  40 ++++++++
>  6 files changed, 206 insertions(+)
>  create mode 100644 drivers/tee/broadcom/Kconfig
>  create mode 100644 drivers/tee/broadcom/Makefile
>  create mode 100644 drivers/tee/broadcom/chimp_optee.c
>  create mode 100644 include/broadcom/chimp.h
>
> diff --git a/drivers/tee/Kconfig b/drivers/tee/Kconfig
> index 5c0c89043f..5ca5a0836c 100644
> --- a/drivers/tee/Kconfig
> +++ b/drivers/tee/Kconfig
> @@ -29,6 +29,7 @@ config SANDBOX_TEE
>           "avb" commands.
>
>  source "drivers/tee/optee/Kconfig"
> +source "drivers/tee/broadcom/Kconfig"
>
>  endmenu
>
> diff --git a/drivers/tee/Makefile b/drivers/tee/Makefile
> index f72c68c09f..5c8ffdbce8 100644
> --- a/drivers/tee/Makefile
> +++ b/drivers/tee/Makefile
> @@ -3,3 +3,4 @@
>  obj-y += tee-uclass.o
>  obj-$(CONFIG_SANDBOX) += sandbox.o
>  obj-$(CONFIG_OPTEE) += optee/
> +obj-y += broadcom/
> diff --git a/drivers/tee/broadcom/Kconfig b/drivers/tee/broadcom/Kconfig
> new file mode 100644
> index 0000000000..ce95072d4e
> --- /dev/null
> +++ b/drivers/tee/broadcom/Kconfig
> @@ -0,0 +1,7 @@
> +config CHIMP_OPTEE
> +       bool "Enable secure ChiMP firmware loading"
> +       depends on OPTEE
> +       default y
> +       help
> +         This driver is used to load bnxt firmware binary using OpTEE.
> +         bnxt is Broadcom NetXtreme controller Ethernet card.
> diff --git a/drivers/tee/broadcom/Makefile b/drivers/tee/broadcom/Makefile
> new file mode 100644
> index 0000000000..5e1c3943de
> --- /dev/null
> +++ b/drivers/tee/broadcom/Makefile
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +
> +obj-$(CONFIG_CHIMP_OPTEE) += chimp_optee.o
> diff --git a/drivers/tee/broadcom/chimp_optee.c b/drivers/tee/broadcom/chimp_optee.c
> new file mode 100644
> index 0000000000..f432625ffe
> --- /dev/null
> +++ b/drivers/tee/broadcom/chimp_optee.c
> @@ -0,0 +1,154 @@
> +// SPDX-License-Identifier: BSD-2-Clause
> +/*
> + * Copyright 2020 Broadcom.
> + */
> +
> +#include <common.h>
> +#include <tee.h>
> +#include <broadcom/chimp.h>
> +
> +#define CHMIP_BOOT_UUID { 0x6272636D, 0x2019, 0x0716, \
> +                  { 0x42, 0x43, 0x4D, 0x5F, 0x53, 0x43, 0x48, 0x49 } }
> +
> +enum {
> +       TEE_CHIMP_FASTBOOT = 0,
> +       TEE_CHIMP_HEALTH_STATUS,
> +       TEE_CHIMP_HANDSHAKE_STATUS,
> +} tee_chmip_cmd;
> +
> +struct bcm_chimp_data {
> +       struct udevice *tee;
> +       u32 session;
> +} chimp_data;
> +
> +static int get_open_session(struct bcm_chimp_data *b_data)
> +{
> +       struct udevice *tee = NULL;
> +
> +       while (!b_data->tee) {
> +               const struct tee_optee_ta_uuid uuid = CHMIP_BOOT_UUID;
> +               struct tee_open_session_arg arg;
> +               int rc;
> +
> +               tee = tee_find_device(tee, NULL, NULL, NULL);
> +               if (!tee)
> +                       return -ENODEV;
> +
> +               memset(&arg, 0, sizeof(arg));
> +               tee_optee_ta_uuid_to_octets(arg.uuid, &uuid);
> +               rc = tee_open_session(tee, &arg, 0, NULL);
> +               if (!rc) {
> +                       b_data->tee = tee;
> +                       b_data->session = arg.session;
> +               }
> +       }
> +
> +       return 0;
> +}
> +
> +int chimp_handshake_status_optee(u32 timeout, u32 *hs)
> +{
> +       struct tee_invoke_arg arg;
> +       struct tee_param param[1];
> +       int ret;
> +
> +       if (get_open_session(&chimp_data))
> +               return BCM_CHIMP_FAILURE;
> +
> +       memset(&arg, 0, sizeof(arg));
> +       arg.func = TEE_CHIMP_HANDSHAKE_STATUS;
> +       arg.session = chimp_data.session;
> +
> +       param[0].attr = TEE_PARAM_ATTR_TYPE_VALUE_INOUT;
> +       param[0].u.value.a = timeout;
> +
> +       if (tee_invoke_func(chimp_data.tee, &arg,
> +                           ARRAY_SIZE(param), param)) {
> +               printf("Handshake status command failed\n");
> +               ret = BCM_CHIMP_FAILURE;
> +               goto out;
> +       }
> +       switch (arg.ret) {
> +       case TEE_SUCCESS:
> +               *hs = param[0].u.value.a;
> +               ret =  BCM_CHIMP_SUCCESS;
> +               break;
> +       default:
> +               ret = BCM_CHIMP_FAILURE;
> +               break;
> +       }
> +out:
> +       tee_close_session(chimp_data.tee, chimp_data.session);
> +       chimp_data.tee = NULL;
> +
> +       return ret;
> +}
> +
> +int chimp_health_status_optee(u32 *health)
> +{
> +       struct tee_invoke_arg arg;
> +       struct tee_param param[1];
> +       int ret;
> +
> +       if (get_open_session(&chimp_data))
> +               return BCM_CHIMP_FAILURE;

Why are you using your own error codes here? Could you just use 0 for
success, and -EINVAL, or whatever get_open_session() returns, instead?

> +
> +       memset(&arg, 0, sizeof(arg));
> +       arg.func = TEE_CHIMP_HEALTH_STATUS;
> +       arg.session = chimp_data.session;
> +
> +       param[0].attr = TEE_PARAM_ATTR_TYPE_VALUE_OUTPUT;
> +
> +       if (tee_invoke_func(chimp_data.tee, &arg,
> +                           ARRAY_SIZE(param), param)) {
> +               printf("Helath status command failed\n");
> +               ret =  BCM_CHIMP_FAILURE;
> +               goto out;
> +       }
> +       switch (arg.ret) {
> +       case TEE_SUCCESS:
> +               *health = param[0].u.value.a;
> +               ret =  BCM_CHIMP_SUCCESS;
> +               break;
> +       default:
> +               ret = BCM_CHIMP_FAILURE;
> +               break;
> +       }
> +out:
> +       tee_close_session(chimp_data.tee, chimp_data.session);
> +       chimp_data.tee = NULL;
> +
> +       return ret;
> +}
> +
> +int chimp_fastboot_optee(void)
> +{
> +       struct tee_invoke_arg arg;
> +       int ret;
> +
> +       if (get_open_session(&chimp_data))
> +               return BCM_CHIMP_FAILURE;
> +
> +       memset(&arg, 0, sizeof(arg));
> +       arg.func = TEE_CHIMP_FASTBOOT;
> +       arg.session = chimp_data.session;

It seems like the above lines are common throughout. How about putting
them in a static function?

> +
> +       if (tee_invoke_func(chimp_data.tee, &arg, 0, NULL)) {
> +               printf("Chimp boot_fail\n");
> +               ret =  BCM_CHIMP_FAILURE;
> +               goto out;
> +       }
> +       switch (arg.ret) {
> +       case TEE_SUCCESS:
> +               ret = BCM_CHIMP_SUCCESS;
> +               break;
> +       default:
> +               ret = BCM_CHIMP_FAILURE;
> +               break;
> +       }
> +out:
> +       tee_close_session(chimp_data.tee, chimp_data.session);
> +       chimp_data.tee = NULL;
> +
> +       return ret;
> +}
> diff --git a/include/broadcom/chimp.h b/include/broadcom/chimp.h
> new file mode 100644
> index 0000000000..c3d4594c4b
> --- /dev/null
> +++ b/include/broadcom/chimp.h
> @@ -0,0 +1,40 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright 2020 Broadcom.
> + *
> + */
> +
> +#ifndef __CHIMP_H__
> +#define __CHIMP_H__
> +
> +#include <common.h>

Please drop that and just include what you need. Maybe linux/types.h?

> +#include <linux/compiler.h>
> +
> +#define BCM_CHIMP_SUCCESS      0
> +#define BCM_CHIMP_FAILURE      (!BCM_CHIMP_SUCCESS)
> +
> +#ifdef CONFIG_CHIMP_OPTEE
> +int chimp_fastboot_optee(void);
> +int chimp_health_status_optee(u32 *status);
> +int chimp_handshake_status_optee(u32 timeout, u32 *hstatus);
> +#else
> +static inline int chimp_handshake_status_optee(u32 timeout, u32 *status)
> +{
> +       printf("ChiMP handshake status fail (OPTEE not enabled)\n");
> +       return BCM_CHIMP_FAILURE;

Code should go in the C file, particularly when it has printf().

Why does this have to be inline at all?

> +}
> +
> +static inline int chimp_health_status_optee(u32 *status)
> +{
> +       printf("ChiMP health status fail (OPTEE not enabled)\n");
> +       return BCM_CHIMP_FAILURE;
> +}
> +
> +static inline int chimp_fastboot_optee(void)
> +{
> +       printf("ChiMP secure boot fail (OPTEE not enabled)\n");
> +       return BCM_CHIMP_FAILURE;
> +}
> +#endif
> +
> +#endif
> --
> 2.17.1
>

Regards,
Simon

  reply	other threads:[~2020-05-25  2:15 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-24 10:24 [PATCH v2 0/3] add optee support for broadcom NS3 soc Rayagonda Kokatanur
2020-05-24 10:24 ` [PATCH v2 1/3] board: ns3: add optee based bnxt fw load driver Rayagonda Kokatanur
2020-05-25  2:15   ` Simon Glass [this message]
2020-05-24 10:24 ` [PATCH v2 2/3] configs: ns3: enable tee and optee driver Rayagonda Kokatanur
2020-05-25  2:15   ` Simon Glass
2020-05-24 10:24 ` [PATCH v2 3/3] arm: dts: ns3: add optee node Rayagonda Kokatanur
2020-05-25  2:15   ` Simon Glass

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAPnjgZ3JkOVOOAXvyVY2J-c1hxb-YU8xQF5OHmfV3H-CZRLhHg@mail.gmail.com \
    --to=sjg@chromium.org \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.