Date: Mon, 12 Jan 2015 17:21:24 +0100 From: =?UTF-8?Q?Micha=C5=82_Stanek?= <mst@semihalf.com> To: Konstantin Belousov <kostikbel@gmail.com> Cc: freebsd-current@freebsd.org, freebsd-embedded@freebsd.org Subject: Re: [PATCH] Add support for 64-bit AHCI BAR. Message-ID: <CAMiGqYhiGZNSqLkOP3hJodwZ-T4WbpH3i_BrC2J08rGbQ%2BLBDQ@mail.gmail.com> In-Reply-To: <20150109172715.GU42409@kib.kiev.ua> References: <CAMiGqYhKn49LrdFGS4DeGSHDOuVO1Ab0mwubyqGXzxFLmNGoEQ@mail.gmail.com> <20150108203959.GR42409@kib.kiev.ua> <CAMiGqYhi=SU6hg-utRLUi=HauE5Q1qxdYifAxaGNdbVXReaS%2BA@mail.gmail.com> <20150109172715.GU42409@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
--001a1133fa76c74e71050c76e541 Content-Type: text/plain; charset=UTF-8 2015-01-09 18:27 GMT+01:00 Konstantin Belousov <kostikbel@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@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@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 > > @@ -373,7 +373,6 @@ 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 && > > @@ -392,11 +391,9 @@ 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 > > > > --001a1133fa76c74e71050c76e541 Content-Type: text/x-patch; charset=US-ASCII; name="0001-Add-quirk-for-Cavium-AHCI-BAR-location.patch" Content-Disposition: attachment; filename="0001-Add-quirk-for-Cavium-AHCI-BAR-location.patch" Content-Transfer-Encoding: base64 X-Attachment-Id: f_i4u1x6su0 RnJvbSA1MDA2ZTNkZmY0MzQ2ZTRhNDliNGVkYmJlMzk4Nzk5Yjk0ZGE2NTZmIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBNaWNoYWwgU3RhbmVrIDxtc3RAc2VtaWhhbGYuY29tPgpEYXRl OiBXZWQsIDcgSmFuIDIwMTUgMTg6Mjg6MTggKzAxMDAKU3ViamVjdDogW1BBVENIXSBBZGQgcXVp cmsgZm9yIENhdml1bSBBSENJIEJBUiBsb2NhdGlvbgoKUXVpcmsgYWRkZWQgdG8gb3ZlcnJpZGUg ZGVmYXVsdCBCQVIoNSkgcmlkIGZvciBBSENJLgotLS0KIHN5cy9kZXYvYWhjaS9haGNpLmggICAg IHwgMSArCiBzeXMvZGV2L2FoY2kvYWhjaV9wY2kuYyB8IDkgKysrKysrKy0tCiAyIGZpbGVzIGNo YW5nZWQsIDggaW5zZXJ0aW9ucygrKSwgMiBkZWxldGlvbnMoLSkKCmRpZmYgLS1naXQgYS9zeXMv ZGV2L2FoY2kvYWhjaS5oIGIvc3lzL2Rldi9haGNpL2FoY2kuaAppbmRleCA0NjJmMjA0Li5mZThm Yzk1IDEwMDY0NAotLS0gYS9zeXMvZGV2L2FoY2kvYWhjaS5oCisrKyBiL3N5cy9kZXYvYWhjaS9h aGNpLmgKQEAgLTU3Miw2ICs1NzIsNyBAQCBlbnVtIGFoY2lfZXJyX3R5cGUgewogI2RlZmluZSBB SENJX1FfQVRJX1BNUF9CVUcJMHgyMDAwCiAjZGVmaW5lIEFIQ0lfUV9NQVhJT182NEsJMHg0MDAw CiAjZGVmaW5lIEFIQ0lfUV9TQVRBMV9VTklUMAkweDgwMDAJCS8qIG5lZWQgYmV0dGVyIG1ldGhv ZCBmb3IgdGhpcyAqLworI2RlZmluZSBBSENJX1FfQUJBUjAJCTB4MTAwMDAKIAogI2RlZmluZSBB SENJX1FfQklUX1NUUklORwlcCiAJIlwwMjAiCQkJXApkaWZmIC0tZ2l0IGEvc3lzL2Rldi9haGNp L2FoY2lfcGNpLmMgYi9zeXMvZGV2L2FoY2kvYWhjaV9wY2kuYwppbmRleCBhZjI2OTUxLi44NTc0 ZTQ3IDEwMDY0NAotLS0gYS9zeXMvZGV2L2FoY2kvYWhjaV9wY2kuYworKysgYi9zeXMvZGV2L2Fo Y2kvYWhjaV9wY2kuYwpAQCAtMjg3LDYgKzI4Nyw3IEBAIHN0YXRpYyBjb25zdCBzdHJ1Y3Qgewog CXsweDExODQxMDM5LCAweDAwLCAiU2lTIDk2NiIsCQkwfSwKIAl7MHgxMTg1MTAzOSwgMHgwMCwg IlNpUyA5NjgiLAkJMH0sCiAJezB4MDE4NjEwMzksIDB4MDAsICJTaVMgOTY4IiwJCTB9LAorCXsw eGEwMWMxNzdkLCAweDAwLCAiVGh1bmRlclggU0FUQSIsCUFIQ0lfUV9BQkFSMH0sCiAJezB4MDAw MDAwMDAsIDB4MDAsIE5VTEwsCQkwfQogfTsKIApAQCAtMzg2LDEyICszODcsMTYgQEAgYWhjaV9w Y2lfYXR0YWNoKGRldmljZV90IGRldikKIAkgICAgcGNpX2dldF9zdWJ2ZW5kb3IoZGV2KSA9PSAw eDEwNDMgJiYKIAkgICAgcGNpX2dldF9zdWJkZXZpY2UoZGV2KSA9PSAweDgxZTQpCiAJCWN0bHIt PnF1aXJrcyB8PSBBSENJX1FfU0FUQTFfVU5JVDA7Ci0JLyogaWYgd2UgaGF2ZSBhIG1lbW9yeSBC QVIoNSkgd2UgYXJlIGxpa2VseSBvbiBhbiBBSENJIHBhcnQgKi8KIAljdGxyLT52ZW5kb3JpZCA9 IHBjaV9nZXRfdmVuZG9yKGRldik7CiAJY3Rsci0+ZGV2aWNlaWQgPSBwY2lfZ2V0X2RldmljZShk ZXYpOwogCWN0bHItPnN1YnZlbmRvcmlkID0gcGNpX2dldF9zdWJ2ZW5kb3IoZGV2KTsKIAljdGxy LT5zdWJkZXZpY2VpZCA9IHBjaV9nZXRfc3ViZGV2aWNlKGRldik7Ci0JY3Rsci0+cl9yaWQgPSBQ Q0lSX0JBUig1KTsKKworCS8qIERlZmF1bHQgQUhDSSBCYXNlIEFkZHJlc3MgaXMgQkFSKDUpLCBD YXZpdW0gdXNlcyBCQVIoMCkgKi8KKwlpZiAoY3Rsci0+cXVpcmtzICYgQUhDSV9RX0FCQVIwKQor CQljdGxyLT5yX3JpZCA9IFBDSVJfQkFSKDApOworCWVsc2UKKwkJY3Rsci0+cl9yaWQgPSBQQ0lS X0JBUig1KTsKIAlpZiAoIShjdGxyLT5yX21lbSA9IGJ1c19hbGxvY19yZXNvdXJjZV9hbnkoZGV2 LCBTWVNfUkVTX01FTU9SWSwKIAkgICAgJmN0bHItPnJfcmlkLCBSRl9BQ1RJVkUpKSkKIAkJcmV0 dXJuIEVOWElPOwotLSAKMi4yLjEKCg== --001a1133fa76c74e71050c76e541--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAMiGqYhiGZNSqLkOP3hJodwZ-T4WbpH3i_BrC2J08rGbQ%2BLBDQ>