Re: [PATCH v2 04/11] PCI/IDE: Enumerate Selective Stream IDE capabilities

[Aneesh Kumar K.V <aneesh.kumar@kernel.org>]

Dan Williams <dan.j.williams@intel.com> writes:

.....

> +void pci_ide_init(struct pci_dev *pdev)
> +{
> +	u8 nr_link_ide, nr_ide_mem, nr_streams;
> +	u16 ide_cap;
> +	u32 val;
> +
> +	if (!pci_is_pcie(pdev))
> +		return;
> +
> +	ide_cap = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_IDE);
> +	if (!ide_cap)
> +		return;
> +
> +	pci_read_config_dword(pdev, ide_cap + PCI_IDE_CAP, &val);
> +	if ((val & PCI_IDE_CAP_SELECTIVE) == 0)
> +		return;
> +
> +	/*
> +	 * Require endpoint IDE capability to be paired with IDE Root
> +	 * Port IDE capability.
> +	 */
> +	if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ENDPOINT) {
> +		struct pci_dev *rp = pcie_find_root_port(pdev);
> +
> +		if (!rp->ide_cap)
> +			return;
> +	}
> +
> +	if (val & PCI_IDE_CAP_SEL_CFG)
> +		pdev->ide_cfg = 1;
> +
> +	if (val & PCI_IDE_CAP_TEE_LIMITED)
> +		pdev->ide_tee_limit = 1;
> +
> +	if (val & PCI_IDE_CAP_LINK)
> +		nr_link_ide = 1 + FIELD_GET(PCI_IDE_CAP_LINK_TC_NUM_MASK, val);
> +
> +	nr_ide_mem = 0;
> +	nr_streams = min(1 + FIELD_GET(PCI_IDE_CAP_SEL_NUM_MASK, val),
> +			 CONFIG_PCI_IDE_STREAM_MAX);
> +	for (int i = 0; i < nr_streams; i++) {
> +		int offset = sel_ide_offset(nr_link_ide, i, nr_ide_mem);
> +		int nr_assoc;
> +		u32 val;
> +
> +		pci_read_config_dword(pdev, ide_cap + offset, &val);
> +
> +		/*
> +		 * Let's not entertain devices that do not have a
> +		 * constant number of address association blocks
> +		 */
> +		nr_assoc = FIELD_GET(PCI_IDE_SEL_CAP_ASSOC_NUM_MASK, val);
> +		if (i && (nr_assoc != nr_ide_mem)) {
> +			pci_info(pdev, "Unsupported Selective Stream %d capability\n", i);
> +			return;
> +		}
> +
> +		nr_ide_mem = nr_assoc;
>

What is the purpose of the loop? Should it use the minimum value to
ensure we select the lowest supported value for all selective IDE
stream? I assume that, in practice, the number of address association
register blocks will be the same for all selective streams?

                 nr_ide_mem = min(nr_ide_mem, nr_assoc);

> +	}
> +
> +	pdev->ide_cap = ide_cap;
> +	pdev->nr_link_ide = nr_link_ide;
> +	pdev->nr_ide_mem = nr_ide_mem;
> +}


-aneesh