Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Sat, 10 Aug 2019 00:43:17 -0700
From:   Christoph Hellwig <hch@infradead.org>
To:     Stephen Douthit <stephend@silicom-usa.com>
Cc:     Jens Axboe <axboe@kernel.dk>,
        "linux-ide@vger.kernel.org" <linux-ide@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Dan Williams <dan.j.williams@intel.com>
Subject: Re: [PATCH] ata: ahci: Lookup PCS register offset based on PCI
 device ID
Message-ID: <20190810074317.GA18582@infradead.org>
References: <20190808202415.25166-1-stephend@silicom-usa.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20190808202415.25166-1-stephend@silicom-usa.com>
User-Agent: Mutt/1.11.4 (2019-03-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, Aug 08, 2019 at 08:24:31PM +0000, Stephen Douthit wrote:
> Intel moved the PCS register from 0x92 to 0x94 on Denverton for some
> reason, so now we get to check the device ID before poking it on reset.

And now you just match on the new IDs, which means we'll perpetually
catch up on any new device.  Dan, can you reach out inside Intel to
figure out if there is a way to find out the PCS register location
without the PCI ID check?


>  static int ahci_pci_reset_controller(struct ata_host *host)
>  {
>  	struct pci_dev *pdev = to_pci_dev(host->dev);
> @@ -634,13 +669,14 @@ static int ahci_pci_reset_controller(struct ata_host *host)
>  
>  	if (pdev->vendor == PCI_VENDOR_ID_INTEL) {
>  		struct ahci_host_priv *hpriv = host->private_data;
> +		int pcs = ahci_pcs_offset(host);
>  		u16 tmp16;
>  
>  		/* configure PCS */
> -		pci_read_config_word(pdev, 0x92, &tmp16);
> +		pci_read_config_word(pdev, pcs, &tmp16);
>  		if ((tmp16 & hpriv->port_map) != hpriv->port_map) {
> -			tmp16 |= hpriv->port_map;
> -			pci_write_config_word(pdev, 0x92, tmp16);
> +			tmp16 |= hpriv->port_map & 0xff;
> +			pci_write_config_word(pdev, pcs, tmp16);
>  		}
>  	}

And Stephen, while you are at it, can you split this Intel-specific
quirk into a separate helper?