Re: [PATCH v2 08/11] PCI/IDE: Add IDE establishment helpers

[Dan Williams <dan.j.williams@intel.com>]

Alexey Kardashevskiy wrote:
[..]
> Doing it in one go as you suggest works with one of my devices but not 
> the other.
> 
> And to make things "clear", the spec also says:
> ===
> It is strongly recommended to complete key programming for a Stream 
> before Setting the Enable bit in the IDE Extended Capability entry for 
> that Stream.
> ◦ It is permitted, but strongly not recommended, to Set the Enable bit 
> in the IDE Extended Capability entry for a Stream prior to the 
> completion of key programming for that Stream
> ====
> 
> So are we going to do "permitted" or not "not recommended" (== 
> recommended)? Thanks,

So the spec is only talking about the key programming not the control
register, but if a device wants the control register to have everything
but "enable" set, that seems reasonable to me. Does this work for that
device?

diff --git a/drivers/pci/ide.c b/drivers/pci/ide.c
index 2b5295615a3a..4657138d3a7a 100644
--- a/drivers/pci/ide.c
+++ b/drivers/pci/ide.c
@@ -344,6 +344,18 @@ static struct pci_ide_partner *to_settings(struct pci_dev *pdev, struct pci_ide
 	}
 }
 
+static void set_ide_sel_ctl(struct pci_dev *pdev, struct pci_ide *ide, int pos,
+			    bool enable)
+{
+	u32 val = FIELD_PREP(PCI_IDE_SEL_CTL_ID_MASK, ide->stream_id) |
+		  FIELD_PREP(PCI_IDE_SEL_CTL_DEFAULT, 1) |
+		  FIELD_PREP(PCI_IDE_SEL_CTL_CFG_EN, pdev->ide_cfg) |
+		  FIELD_PREP(PCI_IDE_SEL_CTL_TEE_LIMITED, pdev->ide_tee_limit) |
+		  FIELD_PREP(PCI_IDE_SEL_CTL_EN, enable);
+
+	pci_write_config_dword(pdev, pos + PCI_IDE_SEL_CTL, val);
+}
+
 /**
  * pci_ide_stream_setup() - program settings to Selective IDE Stream registers
  * @pdev: PCIe device object for either a Root Port or Endpoint Partner Port
@@ -371,6 +383,12 @@ void pci_ide_stream_setup(struct pci_dev *pdev, struct pci_ide *ide)
 
 	val = PREP_PCI_IDE_SEL_RID_2(settings->rid_start, ide_domain(pdev));
 	pci_write_config_dword(pdev, pos + PCI_IDE_SEL_RID_2, val);
+
+	/*
+	 * Setup control register early for devices that expect
+	 * stream_id is set during key programming.
+	 */
+	set_ide_sel_ctl(pdev, ide, pos, false);
 }
 EXPORT_SYMBOL_GPL(pci_ide_stream_setup);
 
@@ -411,7 +429,6 @@ void pci_ide_stream_enable(struct pci_dev *pdev, struct pci_ide *ide)
 {
 	struct pci_ide_partner *settings = to_settings(pdev, ide);
 	int pos;
-	u32 val;
 
 	if (!settings)
 		return;
@@ -419,12 +436,7 @@ void pci_ide_stream_enable(struct pci_dev *pdev, struct pci_ide *ide)
 	pos = sel_ide_offset(pdev->nr_link_ide, settings->stream_index,
 			     pdev->nr_ide_mem);
 
-	val = FIELD_PREP(PCI_IDE_SEL_CTL_ID_MASK, ide->stream_id) |
-	      FIELD_PREP(PCI_IDE_SEL_CTL_DEFAULT, 1) |
-	      FIELD_PREP(PCI_IDE_SEL_CTL_CFG_EN, pdev->ide_cfg) |
-	      FIELD_PREP(PCI_IDE_SEL_CTL_TEE_LIMITED, pdev->ide_tee_limit) |
-	      FIELD_PREP(PCI_IDE_SEL_CTL_EN, 1);
-	pci_write_config_dword(pdev, pos + PCI_IDE_SEL_CTL, val);
+	set_ide_sel_ctl(pdev, ide, pos, true);
 }
 EXPORT_SYMBOL_GPL(pci_ide_stream_enable);