Re: [PATCH v2] platform/mellanox: Add bootctl driver for Mellanox BlueField Soc

[Andy Shevchenko]

On Thu, Jan 31, 2019 at 6:53 PM Liming Sun <lsun@xxxxxxxxxxxx> wrote:
>
> This commit adds the bootctl platform driver for Mellanox BlueField
> Soc, which controls the eMMC boot partition swapping and sends SMC
> calls to ATF running at exception level EL3 to program some system
> register. This register is only accessible in secure code and is
> used to enable the watchdog after reboot.
>
> Below are the sequences of a typical use case.
>
>   1. User-space tool upgrades one eMMC boot partition and requests
>      the boot partition swapping;
>
>   2. The bootctl driver handles such request and sends SMC call
>      to ATF. ATF programs register BREADCRUMB0 which has value
>      preserved during software reset. It also programs eMMC to
>      swap the boot partition;
>
>   3. After software reset (rebooting), ATF BL1 (BootRom) checks
>      register BREADCRUMB0 to enable watchdog if configured;
>
>   4. If booting fails, the watchdog timer will trigger rebooting.
>      In such case, ATF BootRom will switch the boot partition
>      to the previous one.

Thanks for an update. My comments below.


> Reviewed-by: David Woods <dwoods@xxxxxxxxxxxx>

I'm not sure I see this guy to review v2. Of course if you consider
all changes minor, you may leave this tag.

> Signed-off-by: Liming Sun <lsun@xxxxxxxxxxxx>
> ---

Here should be a changelog what had been done in new version.
> +/* UUID used to probe ATF service. */

> +static const char * const mlxbf_bootctl_svc_uuid_str =
> +       "89c036b4-e7d7-11e6-8797-001aca00bfc4";

static const char *xxx = ...;

> +/*
> + * UUID structure used to match the returned value from ATF in
> + * four 32-bit words. No need to do endian conversion here.
> + */
> +union mlxbf_bootctl_uuid {
> +       guid_t guid;
> +       u32 words[4];
> +};

I'm not sure it's the best you can do. instead of using union, better
to use conversion from and to corresponding uuid type.

The rest I will comment on v3.

-- 
With Best Regards,
Andy Shevchenko