Re: [PATCH] Add support for 64-bit AHCI BAR.

On Fri, Jan 09, 2015 at 06:07:39PM +0100, Micha?? Stanek wrote:
> 2015-01-08 21:40 GMT+01:00 Konstantin Belousov <kostikbel_at_gmail.com>:
> 
> > > However, if
> > > AHCI uses 64-bit base addresses, then this register consists of two
> > dwords
> > > starting at offset 0x20 - BAR4 and BAR5. This is the case on our arm64
> > > target and possibly other platforms using 64-bit BARs for AHCI.
> > Is it specified anywhere, or just a quirk of the specific implementation ?
> > If it is a quirk, would it make sense to also check the vendor or device
> > id before applying the logic ?
> >
> >
> Yes, indeed it is a quirk as I just found out that our platform vendor
> actually uses BAR(0) as AHCI ABAR, while BAR(4) is used for something else.
> I found it implemented as a quirk in Linux AHCI code.
> The BAR is still 64-bit but in a different position than AHCI spec stated.
> I changed it as you suggested, the new patch is in the attachment. Please
> take a look.
This is probably technically correct (I am not AHCI code author), but note
that we have more structured quirks mechanism than directly checking vendor
and device id.  Look at the ahci_ids table and the quirks member.

Add a bit declaring the need of the quirk and test the bit, instead of
the vendor/devid.

> 
> >
> > > The following patch adds a check for the extended BAR in
> > ahci_pci_attach()
> > > and sets the 'rid' in bus_alloc_resource_any accordingly. It fixes the
> > > allocation error on our platform.
> > >
> > > Please review and test this patch on other platforms. If there are no
> > > issues then it will be committed in a week.
> > >
> >

> From b6220884d9e71d7c4fc1c2a22ade374fc023c831 Mon Sep 17 00:00:00 2001
> From: Michal Stanek <mst_at_semihalf.com>
> Date: Fri, 9 Jan 2015 17:20:38 +0100
> Subject: [PATCH] Add quirk for Cavium AHCI BAR location
> 
> ---
>  sys/dev/ahci/ahci_pci.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/sys/dev/ahci/ahci_pci.c b/sys/dev/ahci/ahci_pci.c
> index 43723a6..dce4acb 100644
> --- a/sys/dev/ahci/ahci_pci.c
> +++ b/sys/dev/ahci/ahci_pci.c
> _at__at_ -373,7 +373,6 _at__at_ ahci_pci_attach(device_t dev)
>  	int	error, i;
>  	uint32_t devid = pci_get_devid(dev);
>  	uint8_t revid = pci_get_revid(dev);
> -	struct pci_map *map;
>  
>  	i = 0;
>  	while (ahci_ids[i].id != 0 &&
> _at__at_ -392,11 +391,9 _at__at_ ahci_pci_attach(device_t dev)
>  	ctlr->subvendorid = pci_get_subvendor(dev);
>  	ctlr->subdeviceid = pci_get_subdevice(dev);
>  
> -	/* AHCI Base Address is BAR(5) by default, unless BARs are 64-bit */
> -	map = pci_find_bar(dev, PCIR_BAR(4));
> -	if (map != NULL &&
> -	    ((map->pm_value & PCIM_BAR_MEM_TYPE) == PCIM_BAR_MEM_64))
> -		ctlr->r_rid = PCIR_BAR(4);
> +	/* Default AHCI Base Address is BAR(5), Cavium uses BAR(0) */
> +	if (ctlr->vendorid == 0x177d && ctlr->deviceid == 0xa01c)
> +		ctlr->r_rid = PCIR_BAR(0);
>  	else
>  		ctlr->r_rid = PCIR_BAR(5);
>  	if (!(ctlr->r_mem = bus_alloc_resource_any(dev, SYS_RES_MEMORY,
> -- 
> 2.2.1
>