Re: [PATCH] nvme: copy firmware_rev on each init

[Chao Leng <lengchao@huawei.com>]

On 2022/9/20 3:45, Keith Busch wrote:
> From: Keith Busch <kbusch@kernel.org>
> 
> The firmware revision can change on after a reset so copy the most
> recent info each time instead of just the first time, otherwise the
> sysfs firmware_rev entry may contain stale data.
> 
> Reported-by: Jeff Lien <jeff.lien@wdc.com>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
>   drivers/nvme/host/core.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index ea6694fd550f..e56ecc7fda2d 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -2893,7 +2893,6 @@ static int nvme_init_subsystem(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id)
>   	nvme_init_subnqn(subsys, ctrl, id);
>   	memcpy(subsys->serial, id->sn, sizeof(subsys->serial));
>   	memcpy(subsys->model, id->mn, sizeof(subsys->model));
> -	memcpy(subsys->firmware_rev, id->fr, sizeof(subsys->firmware_rev));
>   	subsys->vendor_id = le16_to_cpu(id->vid);
>   	subsys->cmic = id->cmic;
>   
> @@ -3112,6 +3111,8 @@ static int nvme_init_identify(struct nvme_ctrl *ctrl)
>   				ctrl->quirks |= core_quirks[i].quirks;
>   		}
>   	}
> +	memcpy(ctrl->subsys->firmware_rev, id->fr,
> +	       sizeof(ctrl->subsys->firmware_rev));
Perhaps it would be better to place it near the call to nvme_init_subsystem.
>   
>   	if (force_apst && (ctrl->quirks & NVME_QUIRK_NO_DEEPEST_PS)) {
>   		dev_warn(ctrl->device, "forcibly allowing all power states due to nvme_core.force_apst -- use at your own risk\n");
> 
Anyway, looks good to me.

Reviewed-by: Chao Leng <lengchao@huawei.com>