From 844c42f079d71e5673dffa2508820c1771ac5bfa Mon Sep 17 00:00:00 2001 From: Adrien Destugues Date: Mon, 4 Jan 2021 21:30:42 +0100 Subject: [PATCH] sdhci_pci: fix xupport for Ricoh controllers There is a quirk needed to switch the controller to standard SD mode (copied from FreeBSD). The response type configuration for R7 response was incorrect, this response type does not have a data phase. This made the Ricoh SDHCI implementation generate an unexpected "transfer complete" interrupt, while apparently the implementation in QEMU didn't. The interrupt generation is a bit different from what I got in QEMU when developping the driver, for some commands, we get only a "transfer complete" and no "command complete" interrupt as I'd expect when the command completes. This is handled in the following way: - The interrupt always releases the semaphore to notify that something has happened (once per event) - When the main thread waits for an event, it always uses the same pattern: while (!condition) acquire_sem(...) This pattern makes it not wait at all if the condition is already satisfied. If the interrupt triggers later or already happened when the code gets to execute this while loop, the semaphore can be left with some tokens in it. These will be emptied the next time the thread waits on something. To make sure ths works properly and everything is synchronizing as expected, some extra checks are added before execution of a command to make sure the hardware is in the expected state. There is also lots of extra tracing, I prefer to leave this enabled initially and wait for some other users to test this new driver on their hardware. When we are confident enough that it is compatible with several implementations, we can reduce the tracing or turn it off. Change-Id: Ib9617dbea62f87124dbaad0027b53a13d949641f Reviewed-on: https://review.haiku-os.org/c/haiku/+/3600 Reviewed-by: Alex von Gluck IV --- src/add-ons/kernel/busses/mmc/sdhci_pci.cpp | 62 ++++++++++++++++----- src/add-ons/kernel/busses/mmc/sdhci_pci.h | 9 ++- 2 files changed, 56 insertions(+), 15 deletions(-) diff --git a/src/add-ons/kernel/busses/mmc/sdhci_pci.cpp b/src/add-ons/kernel/busses/mmc/sdhci_pci.cpp index 6b564dcbc5..59a22f8365 100644 --- a/src/add-ons/kernel/busses/mmc/sdhci_pci.cpp +++ b/src/add-ons/kernel/busses/mmc/sdhci_pci.cpp @@ -163,6 +163,9 @@ SdhciBus::ExecuteCommand(uint8_t command, uint32_t argument, uint32_t* response) { TRACE("ExecuteCommand(%d, %x)\n", command, argument); + // First of all clear the result + fCommandResult = 0; + // Check if it's possible to send a command right now. // It is not possible to send a command as long as the command line is busy. // The spec says we should wait, but we can't do that on kernel side, since @@ -208,7 +211,7 @@ SdhciBus::ExecuteCommand(uint8_t command, uint32_t argument, uint32_t* response) replyType = Command::kR3Type; break; default: - ERROR("Unknown command\n"); + ERROR("Unknown command %x\n", command); return B_BAD_DATA; } @@ -220,25 +223,38 @@ SdhciBus::ExecuteCommand(uint8_t command, uint32_t argument, uint32_t* response) return B_BUSY; } } - - //FIXME : Assign only at this point, if needed : - // -32 bit block count/SDMA system address - // -block size - // -16-bit block count - // -transfer mode + if (fRegisters->present_state.CommandInhibit()) + panic("Command line busy at start of execute command\n"); + + if (replyType == Command::kR1bType) + fRegisters->transfer_mode = 0; + fRegisters->argument = argument; fRegisters->command.SendCommand(command, replyType); // Wait for command response to be available ("command complete" interrupt) TRACE("Wait for command complete..."); - acquire_sem(fSemaphore); - TRACE("command complete OK\n"); + while (fRegisters->present_state.CommandInhibit() + && (fCommandResult == 0)) { + acquire_sem(fSemaphore); + TRACE("command complete sem acquired, status: %x\n", fCommandResult); + TRACE("real status = %x command line busy: %d\n", + fRegisters->interrupt_status, + fRegisters->present_state.CommandInhibit()); + } + + TRACE("Command response available\n"); if (fCommandResult & SDHCI_INT_ERROR) { fRegisters->interrupt_status |= fCommandResult; if (fCommandResult & SDHCI_INT_TIMEOUT) { ERROR("Command execution timed out\n"); + if (fRegisters->present_state.CommandInhibit()) { + TRACE("Command line is still busy, clearing it\n"); + // Clear the stall + fRegisters->software_reset.ResetCommandLine(); + } return B_TIMED_OUT; } if (fCommandResult & SDHCI_INT_CRC) { @@ -277,7 +293,10 @@ SdhciBus::ExecuteCommand(uint8_t command, uint32_t argument, uint32_t* response) if (replyType == Command::kR1bType) { // R1b commands may use the data line so we must wait for the // "transfer complete" interrupt here. - acquire_sem(fSemaphore); + TRACE("Waiting for data...\n"); + while (fRegisters->present_state.DataInhibit()) + acquire_sem(fSemaphore); + TRACE("Got data.\n"); } ERROR("Command execution %d complete\n", command); @@ -643,7 +662,7 @@ SdhciBus::HandleInterrupt() // Check that all interrupts have been cleared (we check all the ones we // enabled, so that should always be the case) - intmask = fRegisters->slot_interrupt_status; + intmask = fRegisters->interrupt_status; if (intmask != 0) { ERROR("Remaining interrupts at end of handler: %x\n", intmask); } @@ -742,7 +761,6 @@ register_device(device_node* parent) static float supports_device(device_node* parent) { - CALLED(); const char* bus; uint16 type, subType; uint16 vendorId, deviceId; @@ -765,7 +783,7 @@ supports_device(device_node* parent) return 0.0f; } - TRACE("Probe device %p (%04x:%04x)\n", parent, vendorId, deviceId); + TRACE("supports_device(vid:%04x pid:%04x)\n", vendorId, deviceId); if (gDeviceManager->get_attr_uint16(parent, B_DEVICE_SUB_TYPE, &subType, false) < B_OK @@ -779,7 +797,8 @@ supports_device(device_node* parent) if (subType != PCI_sd_host) { // Also accept some compliant devices that do not advertise // themselves as such. - if (vendorId != 0x1180 && deviceId != 0xe823) { + if (vendorId != 0x1180 + || (deviceId != 0xe823 && deviceId != 0xe822)) { TRACE("Not the right subclass, and not a Ricoh device\n"); return 0.0f; } @@ -791,6 +810,21 @@ supports_device(device_node* parent) (void**)&device); TRACE("SDHCI Device found! Subtype: 0x%04x, type: 0x%04x\n", subType, type); + + if (vendorId == 0x1180 && deviceId == 0xe823) { + // Switch the device to SD2.0 mode + // This should change its device id to 0xe822 if succesful. + // The device then remains in SD2.0 mode even after reboot. + pci->write_pci_config(device, SDHCI_PCI_RICOH_MODE_KEY, 1, 0xfc); + pci->write_pci_config(device, SDHCI_PCI_RICOH_MODE, 1, + SDHCI_PCI_RICOH_MODE_SD20); + pci->write_pci_config(device, SDHCI_PCI_RICOH_MODE_KEY, 1, 0); + + deviceId = pci->read_pci_config(device, 2, 2); + + TRACE("Device ID after Ricoh quirk: %x\n", deviceId); + } + return 0.8f; } diff --git a/src/add-ons/kernel/busses/mmc/sdhci_pci.h b/src/add-ons/kernel/busses/mmc/sdhci_pci.h index d422bda985..c84bc4daa8 100644 --- a/src/add-ons/kernel/busses/mmc/sdhci_pci.h +++ b/src/add-ons/kernel/busses/mmc/sdhci_pci.h @@ -17,6 +17,13 @@ #define SDHCI_PCI_SLOTS(x) ((( x >> 4) & 7)) #define SDHCI_PCI_SLOT_INFO_FIRST_BASE_INDEX(x) (( x ) & 7) +// Ricoh specific PCI registers +// Ricoh devices start in a vendor-specific mode but can be switched +// to standard sdhci using these PCI registers +#define SDHCI_PCI_RICOH_MODE_KEY 0xf9 +#define SDHCI_PCI_RICOH_MODE 0x150 +#define SDHCI_PCI_RICOH_MODE_SD20 0x10 + #define SDHCI_BUS_TYPE_NAME "bus/sdhci/v1" class TransferMode { @@ -78,7 +85,7 @@ class Command { static const uint8_t kR2Type = kCRCEnable | k128BitResponse; static const uint8_t kR3Type = k32BitResponse; static const uint8_t kR6Type = kCheckIndex | k32BitResponse; - static const uint8_t kR7Type = kDataPresent | kCheckIndex | kCRCEnable + static const uint8_t kR7Type = kCheckIndex | kCRCEnable | k32BitResponse; private: