Re: [PATCH V2 1/1] nvme: add support for dynamic quirk configuration via module parameter

[Christoph Hellwig <hch@lst.de>]

> +static int quirks_param_set(const char *value, const struct kernel_param *kp)
> +{
> +	int qcount, err, i;

qcount makes me thing of the queue count.  Given that this is the only
count, maybe just drop the q prefix?

> +	struct quirk_entry *qlist= NULL;

Missing space before the "=".

> +	char *field, *val = NULL;
> +	char *sep_ptr;
> +
> +	err = param_set_copystring(value, kp);
> +	if (err)
> +		goto exit;

Just return directly here.  Then you also don't need to initialize
val.

> +
> +	val = kstrdup(value, GFP_KERNEL);
> +	if (!val) {
> +		err = -ENOMEM;
> +		goto exit;
> +	}

Also here.

> +
> +	if (!*val)
> +		goto exit;

and then use separate out_free_val and out_free_qlist labels
to avoid the need to initialize qlist.

> +		if (nvme_parse_quirk_entry(field, &qlist[i])) {
> +			pr_err("nvme: failed to parse quirk string %s\n",
> +								value);
> +			goto exit;

The indentation is a bit odd, usualy this would be a two-tab alignment
or after the opening brace.

> +
> +	nvme_pci_quirk_count = qcount;
> +	nvme_pci_quirk_list  = qlist;
> +	kfree(val);

And once you have the out_free_val label, it can be shared with
the regular return.

Otherwise this looks good, and the parsing is much more readable
being split up a bit like this.