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

2015-01-09 18:27 GMT+01:00 Konstantin Belousov <kostikbel_at_gmail.com>:

> 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.
>

OK, I added AHCI_Q_ABAR0 as a new quirk and test the bit in
ahci_pci_attach. It works the same. New patch is in the attachment.



> >
> > >
> > > > 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
> >
>
>