[PATCH v5 1/1] nvme: Add support for FW activation without reset

Christoph Hellwig hch at lst.de
Wed Jul 12 00:34:21 PDT 2017


>  }
>  
> +
> +static bool nvme_ctrl_pp_status(struct nvme_ctrl *ctrl)

This adds a duplicate empty line.

> +{
> +
> +	u32 csts;
> +
> +	if (ctrl->ops->reg_read32(ctrl, NVME_REG_CSTS, &csts))
> +		return false;
> +
> +	if (csts == ~0)
> +		return false;
> +
> +	return ((ctrl->ctrl_config & NVME_CC_ENABLE)
> +			&& (csts & NVME_CSTS_PP));

These should be on one line, and if if they weren't the "&&" placement
is wrong.

> +}

> +static __le32 nvme_get_log_dw10(u8 lid, int size)
> +{
> +	return cpu_to_le32((((size / 4) - 1) << 16) | lid);
> +}

Can you move this up the file a bit so that it's not hidden inside
the FW activation bits.  Also size should be of type size_t.

> +	c.common.nsid = cpu_to_le32(0xFFFFFFFF);

Except for one weird spot we use lower-cases hex numbers.  But in
the longer run we really should have an NVME_NSID_ALL define for this
constant..

Otherwise this looks fine to me:

Reviewed-by: Christoph Hellwig <hch at lst.de>



More information about the Linux-nvme mailing list