[haiku-commits] Change in haiku[master]: sdhci_pci: fix xupport for Ricoh controllers

  • From: Gerrit <review@xxxxxxxxxxxxxxxxxxx>
  • To: waddlesplash <waddlesplash@xxxxxxxxx>, haiku-commits@xxxxxxxxxxxxx
  • Date: Mon, 4 Jan 2021 20:52:07 +0000

From Adrien Destugues <pulkomandy@xxxxxxxxx>:

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


Change subject: sdhci_pci: fix xupport for Ricoh controllers
......................................................................

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.
---
M src/add-ons/kernel/busses/mmc/sdhci_pci.cpp
M src/add-ons/kernel/busses/mmc/sdhci_pci.h
2 files changed, 55 insertions(+), 15 deletions(-)



  git pull ssh://git.haiku-os.org:22/haiku refs/changes/00/3600/1

diff --git a/src/add-ons/kernel/busses/mmc/sdhci_pci.cpp 
b/src/add-ons/kernel/busses/mmc/sdhci_pci.cpp
index a4fce5f..c36aff0 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 @@
 {
        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 @@
                        replyType = Command::kR3Type;
                        break;
                default:
-                       ERROR("Unknown command\n");
+                       ERROR("Unknown command %x\n", command);
                        return B_BAD_DATA;
        }

@@ -220,25 +223,38 @@
                        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 @@
        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 @@

        // 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 @@
 static float
 supports_device(device_node* parent)
 {
-       CALLED();
        const char* bus;
        uint16 type, subType;
        uint16 vendorId, deviceId;
@@ -765,7 +783,7 @@
                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 @@
                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,20 @@
                        (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_MODE_KEY, 1, 
0xfc);
+                       pci->write_pci_config(device, SDHCI_PCI_MODE, 1, 
SDHCI_PCI_MODE_SD20);
+                       pci->write_pci_config(device, SDHCI_PCI_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 d422bda..660ca9b 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_MODE_KEY                                                     
        0xf9
+#define SDHCI_PCI_MODE                                                         
        0x150
+#define SDHCI_PCI_MODE_SD20                                                    
        0x10
+
 #define SDHCI_BUS_TYPE_NAME                                                    
"bus/sdhci/v1"

 class TransferMode {
@@ -78,7 +85,7 @@
                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:

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

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

Other related posts:

  • » [haiku-commits] Change in haiku[master]: sdhci_pci: fix xupport for Ricoh controllers - Gerrit