Re: [PATCH v3 2/3] PCI/NPEM: Add Native PCIe Enclosure Management support

[Christoph Hellwig <hch@infradead.org>]

> +#define for_each_indication(ind, inds) \
> +	for (ind = inds; ind->bit; ind++)

I find this more confusing than just seeing the actual for loop in
the two places using it, but that my just be personal preference.

/*
 * Driver has internal list of supported indications. Ideally, driver should not

s/drivers/the driver/ both times?

> +	if (!npem_has_dsm(dev)) {
> +		pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_NPEM);
> +		if (pos == 0)
> +			return;
> +
> +		if (pci_read_config_dword(dev, pos + PCI_NPEM_CAP, &cap) != 0 ||
> +		    (cap & PCI_NPEM_CAP_CAPABLE) == 0)
> +			return;
> +	} else {
> +		/*
> +		 * OS should use the DSM for LED control if it is available
> +		 * PCI Firmware Spec r3.3 sec 4.7.
> +		 */
> +		return;

If you just did a:

	if (npem_has_dsm(dev))
		return;

we'd save a level of identation for the !dsm case an make the
code a bit easier to read.  Also I think "OS" above should be "The OS".

Otherwise looks good:

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