[haiku-commits] Change in haiku[master]: sdhci_pci: fix quirk and interrupt handling for Ricoh device

  • From: Gerrit <review@xxxxxxxxxxxxxxxxxxx>
  • To: waddlesplash <waddlesplash@xxxxxxxxx>, haiku-commits@xxxxxxxxxxxxx
  • Date: Thu, 7 Jan 2021 20:06:43 +0000

From Adrien Destugues <pulkomandy@xxxxxxxxx>:

Adrien Destugues has uploaded this change for review. ( 
https://review.haiku-os.org/c/haiku/+/3609 ;)


Change subject: sdhci_pci: fix quirk and interrupt handling for Ricoh device
......................................................................

sdhci_pci: fix quirk and interrupt handling for Ricoh device

- The quirk was not properly applied due to misuse of the device API
- The interrupts could run out of sync, leading to not properly waiting
  for commands to terminate before reading their result. Add panic to
  check for that (at the start of the next command) and fix the code.
---
M src/add-ons/kernel/busses/mmc/sdhci_pci.cpp
1 file changed, 22 insertions(+), 17 deletions(-)



  git pull ssh://git.haiku-os.org:22/haiku refs/changes/09/3609/1

diff --git a/src/add-ons/kernel/busses/mmc/sdhci_pci.cpp 
b/src/add-ons/kernel/busses/mmc/sdhci_pci.cpp
index 9f69788..78e391c 100644
--- a/src/add-ons/kernel/busses/mmc/sdhci_pci.cpp
+++ b/src/add-ons/kernel/busses/mmc/sdhci_pci.cpp
@@ -182,7 +182,11 @@
        // only during command execution, and we don't leave this function with 
ac
        // command running.
        if (fRegisters->present_state.CommandInhibit()) {
-               ERROR("Execution aborted, command inhibit\n");
+               panic("Command execution impossible, command inhibit\n");
+               return B_BUSY;
+       }
+       if (fRegisters->present_state.DataInhibit()) {
+               panic("Command execution unwise, data inhibit\n");
                return B_BUSY;
        }

@@ -297,13 +301,15 @@
                        break;
        }

-       if (replyType == Command::kR1bType) {
+       if (replyType == Command::kR1bType &&
+                       (fCommandResult & SDHCI_INT_TRANS_CMP == 0)) {
                // R1b commands may use the data line so we must wait for the
                // "transfer complete" interrupt here.
-               TRACE("Waiting for data...\n");
-               while (fRegisters->present_state.DataInhibit())
+               TRACE("Waiting for data line...\n");
+               do {
                        acquire_sem(fSemaphore);
-               TRACE("Got data.\n");
+               } while (fRegisters->present_state.DataInhibit());
+               TRACE("Dataline is released.\n");
        }
 
        ERROR("Command execution %d complete\n", command);
@@ -555,6 +561,7 @@
        }

        if (intmask & SDHCI_INT_TRANS_CMP) {
+               fCommandResult = intmask;
                fRegisters->interrupt_status |= SDHCI_INT_TRANS_CMP;
                release_sem_etc(fSemaphore, 1, B_DO_NOT_RESCHEDULE);
                TRACE("Transfer complete interrupt handled\n");
@@ -766,11 +773,11 @@
        context->fNode = node;
        *device_cookie = context;

-       if (gDeviceManager->get_attr_uint16(node, B_DEVICE_VENDOR_ID, &vendorId,
-                       false) != B_OK
+       if (gDeviceManager->get_attr_uint16(node, B_DEVICE_VENDOR_ID,
+                       &vendorId, true) != B_OK
                || gDeviceManager->get_attr_uint16(node, B_DEVICE_ID, &deviceId,
-                       false) != B_OK) {
-               TRACE("No vendor or device id attribute\n");
+                       true) != B_OK) {
+               panic("No vendor or device id attribute\n");
                return B_OK; // Let's hope it didn't need the quirk?
        }

@@ -805,23 +812,21 @@
        device_node* pciParent = 
gDeviceManager->get_parent_node(context->fNode);
        gDeviceManager->get_driver(pciParent, (driver_module_info**)&pci,
                (void**)&device);
-       gDeviceManager->put_node(pciParent);

        if (gDeviceManager->get_attr_uint16(context->fNode, B_DEVICE_VENDOR_ID,
-                       &vendorId, false) != B_OK
+                       &vendorId, true) != B_OK
                || gDeviceManager->get_attr_uint16(context->fNode, B_DEVICE_ID,
                        &deviceId, false) != B_OK) {
-               TRACE("No vendor or device id attribute\n");
-               delete context;
-               return;
-       }
-
-       if (vendorId == 0x1180 && deviceId == 0xe823) {
+               ERROR("No vendor or device id attribute\n");
+       } else if (vendorId == 0x1180 && deviceId == 0xe823) {
                pci->write_pci_config(device, SDHCI_PCI_RICOH_MODE_KEY, 1, 
0xfc);
                pci->write_pci_config(device, SDHCI_PCI_RICOH_MODE, 1,
                        context->fRicohOriginalMode);
                pci->write_pci_config(device, SDHCI_PCI_RICOH_MODE_KEY, 1, 0);
        }
+
+       gDeviceManager->put_node(pciParent);
+
        delete context;
 }


--
To view, visit https://review.haiku-os.org/c/haiku/+/3609
To unsubscribe, or for help writing mail filters, visit 
https://review.haiku-os.org/settings

Gerrit-Project: haiku
Gerrit-Branch: master
Gerrit-Change-Id: If83b7bbb3c3446d374115ed22c09b91020028578
Gerrit-Change-Number: 3609
Gerrit-PatchSet: 1
Gerrit-Owner: Adrien Destugues <pulkomandy@xxxxxxxxx>
Gerrit-MessageType: newchange

Other related posts:

  • » [haiku-commits] Change in haiku[master]: sdhci_pci: fix quirk and interrupt handling for Ricoh device - Gerrit