[haiku-commits] haiku: hrev54902 - src/add-ons/kernel/busses/mmc src/add-ons/kernel/bus_managers/mmc src/add-ons/kernel/drivers/disk/mmc headers/private/drivers

  • From: Adrien Destugues <pulkomandy@xxxxxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Wed, 20 Jan 2021 19:02:43 +0000 (UTC)

hrev54902 adds 2 changesets to branch 'master'
old head: d57b8f90e2ba38f2b57afb2742726ba39c45183e
new head: 5ec64c5cdd43187f30726a3a5dece4c0db863689
overview: 
https://git.haiku-os.org/haiku/log/?qt=range&q=5ec64c5cdd43+%5Ed57b8f90e2ba

----------------------------------------------------------------------------

34552f8e66a9: sd/mmc: enable 4-bit data transfers
  
  It works, but performance is still unexpectedly low (getting about
  50kB/s write speed) with almost no CPU load.
  
  Change-Id: I7da3ee70c8b379c4e6c2250d67f880c78635874f
  Reviewed-on: https://review.haiku-os.org/c/haiku/+/3630
  Reviewed-by: Jérôme Duval <jerome.duval@xxxxxxxxx>

5ec64c5cdd43: sd/mmc: Cleanup and improve reliability
  
  Store the bus cookie in the mmc_disk driver and pass it to the bus
  manager when executing commands. This avoids calling into the device
  manager at each read and write operation. The code to get the cookie
  from mmc_disk isn't so nice since it needs to access the grandparent
  device (the mmc bus root), it would be simpler if this cookie would be
  available directly from mmc bus devices.
  
  We can get card removal and card insertion interrupt at the same time
  due to insufficient hardware debouncing (the SDHCI spec says we
  shouldn't, but it happens on Ricoh controllers. Can't blame them, they
  don't advertise themselves as compliant with the spec). So, check the
  card status from the interrupt handler and ignore the incorrect
  interrupts.
  
  Fix unreliable card initialization: power must be turned on before
  starting up the SD clock. Remove a now unneeded delay that was added in
  an attempt to avoid initial instability.
  
  Change-Id: Ibd8d051da1a1d859f3924ee535f4a05d9b6398d4
  Reviewed-on: https://review.haiku-os.org/c/haiku/+/3639
  Reviewed-by: Jérôme Duval <jerome.duval@xxxxxxxxx>

                             [ Adrien Destugues <pulkomandy@xxxxxxxxxxxxx> ]

----------------------------------------------------------------------------

8 files changed, 138 insertions(+), 56 deletions(-)
headers/private/drivers/mmc.h                    |  8 +-
src/add-ons/kernel/bus_managers/mmc/mmc_bus.cpp  | 16 +++-
src/add-ons/kernel/bus_managers/mmc/mmc_bus.h    |  1 +
.../kernel/bus_managers/mmc/mmc_module.cpp       | 42 +++++------
src/add-ons/kernel/busses/mmc/sdhci_pci.cpp      | 78 ++++++++++++++++----
src/add-ons/kernel/busses/mmc/sdhci_pci.h        | 15 ++++
src/add-ons/kernel/drivers/disk/mmc/mmc_disk.cpp | 33 ++++++---
src/add-ons/kernel/drivers/disk/mmc/mmc_disk.h   |  1 +

############################################################################

Commit:      34552f8e66a94ddf82b9dfa684c8be558cf6d659
URL:         https://git.haiku-os.org/haiku/commit/?id=34552f8e66a9
Author:      Adrien Destugues <pulkomandy@xxxxxxxxxxxxx>
Date:        Thu Jan 14 17:35:05 2021 UTC
Committer:   Adrien Destugues <pulkomandy@xxxxxxxxx>
Commit-Date: Wed Jan 20 19:02:38 2021 UTC

sd/mmc: enable 4-bit data transfers

It works, but performance is still unexpectedly low (getting about
50kB/s write speed) with almost no CPU load.

Change-Id: I7da3ee70c8b379c4e6c2250d67f880c78635874f
Reviewed-on: https://review.haiku-os.org/c/haiku/+/3630
Reviewed-by: Jérôme Duval <jerome.duval@xxxxxxxxx>

----------------------------------------------------------------------------

diff --git a/headers/private/drivers/mmc.h b/headers/private/drivers/mmc.h
index 98fd6ab83c..0bf832091f 100644
--- a/headers/private/drivers/mmc.h
+++ b/headers/private/drivers/mmc.h
@@ -81,6 +81,8 @@ typedef struct mmc_bus_interface {
                // Execute a command that involves a data transfer.
        void (*set_scan_semaphore)(void* controller, sem_id sem);
                // Pass the semaphore used for device rescan to the bus 
controller
+       void (*set_bus_width)(void* controller, int width);
+               // Set the data bus width to 1, 4 or 8 bit mode.
 } mmc_bus_interface;
 
 
@@ -97,6 +99,8 @@ typedef struct mmc_device_interface {
        status_t (*do_io)(device_node* controller, uint16_t rca,
                uint8_t command, IOOperation* operation, bool offsetAsSectors);
                // Execute a command that involves a data transfer.
+       void (*set_bus_width)(device_node* controller, int width);
+               // Set the data bus width to 1, 4 or 8 bit mode.
 } mmc_device_interface;
 
 
diff --git a/src/add-ons/kernel/bus_managers/mmc/mmc_bus.cpp 
b/src/add-ons/kernel/bus_managers/mmc/mmc_bus.cpp
index 448304a155..45124d2893 100644
--- a/src/add-ons/kernel/bus_managers/mmc/mmc_bus.cpp
+++ b/src/add-ons/kernel/bus_managers/mmc/mmc_bus.cpp
@@ -106,6 +106,13 @@ MMCBus::SetClock(int frequency)
 }
 
 
+void
+MMCBus::SetBusWidth(int width)
+{
+       fController->set_bus_width(fCookie, width);
+}
+
+
 status_t
 MMCBus::_ActivateDevice(uint16_t rca)
 {
@@ -151,6 +158,12 @@ MMCBus::_WorkerThread(void* cookie)
        status_t result;
        do {
                bus->_AcquireScanSemaphore();
+
+               // In case we just got a "card inserted", wait for things to 
settle
+               // a bit before continuing (there can be glitches while the 
card is
+               // being inserted)
+               snooze(30000);
+
                TRACE("Reset the bus...\n");
                result = bus->ExecuteCommand(0, SD_GO_IDLE_STATE, 0, NULL);
                TRACE("CMD0 result: %s\n", strerror(result));
@@ -158,14 +171,16 @@ MMCBus::_WorkerThread(void* cookie)
 
        // Need to wait at least 8 clock cycles after CMD0 before sending the 
next
        // command. With the default 400kHz clock that would be 20 microseconds,
-       // but apparently we need more.
+       // but we need to wait at least 20ms here, otherwise the next command 
times
+       // out
        snooze(20000);
 
        while (bus->fStatus != B_SHUTTING_DOWN) {
                TRACE("Scanning the bus\n");
 
-               // Use the low speed clock for scanning
+               // Use the low speed clock and 1bit bus width for scanning
                bus->SetClock(400);
+               bus->SetBusWidth(1);
 
                // Probe the voltage range
                enum {
diff --git a/src/add-ons/kernel/bus_managers/mmc/mmc_bus.h 
b/src/add-ons/kernel/bus_managers/mmc/mmc_bus.h
index fe42c06b75..0adc3b9c49 100644
--- a/src/add-ons/kernel/bus_managers/mmc/mmc_bus.h
+++ b/src/add-ons/kernel/bus_managers/mmc/mmc_bus.h
@@ -48,6 +48,7 @@ public:
                                                                        bool 
offsetAsSectors);
 
                                void                    SetClock(int frequency);
+                               void                    SetBusWidth(int width);
 
                                void                    AcquireBus() { 
acquire_sem(fLockSemaphore); }
                                void                    ReleaseBus() { 
release_sem(fLockSemaphore); }
diff --git a/src/add-ons/kernel/bus_managers/mmc/mmc_module.cpp 
b/src/add-ons/kernel/bus_managers/mmc/mmc_module.cpp
index e09d4ccebd..a1bac8d808 100644
--- a/src/add-ons/kernel/bus_managers/mmc/mmc_module.cpp
+++ b/src/add-ons/kernel/bus_managers/mmc/mmc_module.cpp
@@ -124,6 +124,26 @@ mmc_bus_do_io(device_node* node, uint16_t rca, uint8_t 
command,
 }
 
 
+static void
+mmc_bus_set_width(device_node* node, int width)
+{
+       driver_module_info* mmc;
+       void* cookie;
+
+       // FIXME store the parent cookie in the bus cookie or something instead 
of
+       // getting/putting the parent each time.
+       device_node* parent = gDeviceManager->get_parent_node(node);
+       gDeviceManager->get_driver(parent, &mmc, &cookie);
+       gDeviceManager->put_node(parent);
+
+       MMCBus* bus = (MMCBus*)cookie;
+
+       bus->AcquireBus();
+       bus->SetBusWidth(width);
+       bus->ReleaseBus();
+}
+
+
 static status_t
 std_ops(int32 op, ...)
 {
@@ -174,7 +194,8 @@ mmc_device_interface mmc_bus_controller_module = {
                NULL
        },
        mmc_bus_execute_command,
-       mmc_bus_do_io
+       mmc_bus_do_io,
+       mmc_bus_set_width
 };
 
 
diff --git a/src/add-ons/kernel/busses/mmc/sdhci_pci.cpp 
b/src/add-ons/kernel/busses/mmc/sdhci_pci.cpp
index 92d13329b4..18cb8ffe81 100644
--- a/src/add-ons/kernel/busses/mmc/sdhci_pci.cpp
+++ b/src/add-ons/kernel/busses/mmc/sdhci_pci.cpp
@@ -56,6 +56,7 @@ class SdhciBus {
                status_t                        DoIO(uint8_t command, 
IOOperation* operation,
                                                                bool 
offsetAsSectors);
                void                            SetScanSemaphore(sem_id sem);
+               void                            SetBusWidth(int width);
 
        private:
                bool                            PowerOn();
@@ -389,7 +390,7 @@ SdhciBus::DoIO(uint8_t command, IOOperation* operation, 
bool offsetAsSectors)
        off_t offset = operation->Offset();
        generic_size_t length = operation->Length();
 
-       TRACE("%s %" B_PRIuSIZE " bytes at %" B_PRIdOFF "\n",
+       TRACE("%s %"B_PRIu64 " bytes at %" B_PRIdOFF "\n",
                isWrite ? "Write" : "Read", length, offset);
 
        // Check that the IO scheduler did its job in following our DMA 
restrictions
@@ -489,6 +490,28 @@ SdhciBus::SetScanSemaphore(sem_id sem)
 }
 
 
+void
+SdhciBus::SetBusWidth(int width)
+{
+       uint8_t widthBits;
+       switch(width) {
+               case 1:
+                       widthBits = HostControl::kDataTransfer1Bit;
+                       break;
+               case 4:
+                       widthBits = HostControl::kDataTransfer4Bit;
+                       break;
+               case 8:
+                       widthBits = HostControl::kDataTransfer8Bit;
+                       break;
+               default:
+                       panic("Incorrect bitwidth value");
+                       return;
+       }
+       fRegisters->host_control.SetDataTransferWidth(widthBits);
+}
+
+
 bool
 SdhciBus::PowerOn()
 {
@@ -962,6 +985,16 @@ set_scan_semaphore(void* controller, sem_id sem)
 }
 
 
+static void
+set_bus_width(void* controller, int width)
+{
+       CALLED();
+
+       SdhciBus* bus = (SdhciBus*)controller;
+       return bus->SetBusWidth(width);
+}
+
+
 module_dependency module_dependencies[] = {
        { MMC_BUS_MODULE_NAME, (module_info**)&gMMCBusController},
        { B_DEVICE_MANAGER_MODULE_NAME, (module_info**)&gDeviceManager },
@@ -990,7 +1023,8 @@ static mmc_bus_interface gSDHCIPCIDeviceModule = {
        set_clock,
        execute_command,
        do_io,
-       set_scan_semaphore
+       set_scan_semaphore,
+       set_bus_width
 };
 
 
diff --git a/src/add-ons/kernel/busses/mmc/sdhci_pci.h 
b/src/add-ons/kernel/busses/mmc/sdhci_pci.h
index ec5d0ad0b2..381e503384 100644
--- a/src/add-ons/kernel/busses/mmc/sdhci_pci.h
+++ b/src/add-ons/kernel/busses/mmc/sdhci_pci.h
@@ -224,10 +224,20 @@ class HostControl {
                        value = (value & ~kDmaMask) | dmaMode;
                }
 
+               void SetDataTransferWidth(uint8_t width)
+               {
+                       value = (value & ~kDataTransferWidthMask) | width;
+               }
+
                static const uint8_t kDmaMask = 3 << 3;
                static const uint8_t kSdma = 0 << 3;
                static const uint8_t kAdma32 = 2 << 3;
                static const uint8_t kAdma64 = 3 << 3;
+
+               static const uint8_t kDataTransferWidthMask = 0x22;
+               static const uint8_t kDataTransfer1Bit = 0;
+               static const uint8_t kDataTransfer4Bit = 2;
+               static const uint8_t kDataTransfer8Bit = 0x20;
        private:
                volatile uint8_t value;
 } __attribute__((packed));
diff --git a/src/add-ons/kernel/drivers/disk/mmc/mmc_disk.cpp 
b/src/add-ons/kernel/drivers/disk/mmc/mmc_disk.cpp
index c80d7fc444..9d2c8d25c6 100644
--- a/src/add-ons/kernel/drivers/disk/mmc/mmc_disk.cpp
+++ b/src/add-ons/kernel/drivers/disk/mmc/mmc_disk.cpp
@@ -170,7 +170,6 @@ mmc_block_get_geometry(mmc_disk_driver_info* info, 
device_geometry* geometry)
        geometry->read_only = false; // TODO check write protect switch?
        geometry->write_once = false;
 
-#if 0
        // This function will be called before all data transfers, so we use 
this
        // opportunity to switch the card to 4-bit data transfers (instead of 
the
        // default 1 bit mode)
@@ -180,7 +179,9 @@ mmc_block_get_geometry(mmc_disk_driver_info* info, 
device_geometry* geometry)
                info->rca << 16, &cardStatus);
        info->mmc->execute_command(info->parent, info->rca, SD_SET_BUS_WIDTH,
                k4BitMode, &cardStatus);
-#endif
+
+       // From now on we use 4 bit mode
+       info->mmc->set_bus_width(info->parent, 4);
 
        return B_OK;
 }

############################################################################

Revision:    hrev54902
Commit:      5ec64c5cdd43187f30726a3a5dece4c0db863689
URL:         https://git.haiku-os.org/haiku/commit/?id=5ec64c5cdd43
Author:      Adrien Destugues <pulkomandy@xxxxxxxxxxxxx>
Date:        Sat Jan 16 21:50:22 2021 UTC
Committer:   Adrien Destugues <pulkomandy@xxxxxxxxx>
Commit-Date: Wed Jan 20 19:02:38 2021 UTC

sd/mmc: Cleanup and improve reliability

Store the bus cookie in the mmc_disk driver and pass it to the bus
manager when executing commands. This avoids calling into the device
manager at each read and write operation. The code to get the cookie
from mmc_disk isn't so nice since it needs to access the grandparent
device (the mmc bus root), it would be simpler if this cookie would be
available directly from mmc bus devices.

We can get card removal and card insertion interrupt at the same time
due to insufficient hardware debouncing (the SDHCI spec says we
shouldn't, but it happens on Ricoh controllers. Can't blame them, they
don't advertise themselves as compliant with the spec). So, check the
card status from the interrupt handler and ignore the incorrect
interrupts.

Fix unreliable card initialization: power must be turned on before
starting up the SD clock. Remove a now unneeded delay that was added in
an attempt to avoid initial instability.

Change-Id: Ibd8d051da1a1d859f3924ee535f4a05d9b6398d4
Reviewed-on: https://review.haiku-os.org/c/haiku/+/3639
Reviewed-by: Jérôme Duval <jerome.duval@xxxxxxxxx>

----------------------------------------------------------------------------

diff --git a/headers/private/drivers/mmc.h b/headers/private/drivers/mmc.h
index 0bf832091f..8d3780e122 100644
--- a/headers/private/drivers/mmc.h
+++ b/headers/private/drivers/mmc.h
@@ -93,13 +93,13 @@ typedef struct mmc_bus_interface {
 // type of card.
 typedef struct mmc_device_interface {
        driver_module_info info;
-       status_t (*execute_command)(device_node* node, uint16_t rca,
+       status_t (*execute_command)(device_node* node, void* cookie, uint16_t 
rca,
                uint8_t command, uint32_t argument, uint32_t* result);
                // Execute a command with no I/O phase
-       status_t (*do_io)(device_node* controller, uint16_t rca,
+       status_t (*do_io)(device_node* controller, void* cookie, uint16_t rca,
                uint8_t command, IOOperation* operation, bool offsetAsSectors);
                // Execute a command that involves a data transfer.
-       void (*set_bus_width)(device_node* controller, int width);
+       void (*set_bus_width)(device_node* controller, void* cookie, int width);
                // Set the data bus width to 1, 4 or 8 bit mode.
 } mmc_device_interface;
 
diff --git a/src/add-ons/kernel/bus_managers/mmc/mmc_bus.cpp 
b/src/add-ons/kernel/bus_managers/mmc/mmc_bus.cpp
index 45124d2893..e8587eab00 100644
--- a/src/add-ons/kernel/bus_managers/mmc/mmc_bus.cpp
+++ b/src/add-ons/kernel/bus_managers/mmc/mmc_bus.cpp
@@ -159,11 +159,6 @@ MMCBus::_WorkerThread(void* cookie)
        do {
                bus->_AcquireScanSemaphore();
 
-               // In case we just got a "card inserted", wait for things to 
settle
-               // a bit before continuing (there can be glitches while the 
card is
-               // being inserted)
-               snooze(30000);
-
                TRACE("Reset the bus...\n");
                result = bus->ExecuteCommand(0, SD_GO_IDLE_STATE, 0, NULL);
                TRACE("CMD0 result: %s\n", strerror(result));
@@ -173,7 +168,7 @@ MMCBus::_WorkerThread(void* cookie)
        // command. With the default 400kHz clock that would be 20 microseconds,
        // but we need to wait at least 20ms here, otherwise the next command 
times
        // out
-       snooze(20000);
+       snooze(30000);
 
        while (bus->fStatus != B_SHUTTING_DOWN) {
                TRACE("Scanning the bus\n");
diff --git a/src/add-ons/kernel/bus_managers/mmc/mmc_module.cpp 
b/src/add-ons/kernel/bus_managers/mmc/mmc_module.cpp
index a1bac8d808..c74bafa9b6 100644
--- a/src/add-ons/kernel/bus_managers/mmc/mmc_module.cpp
+++ b/src/add-ons/kernel/bus_managers/mmc/mmc_module.cpp
@@ -78,64 +78,39 @@ mmc_bus_added_device(device_node* parent)
 
 
 static status_t
-mmc_bus_execute_command(device_node* node, uint16_t rca, uint8_t command,
-       uint32_t argument, uint32_t* result)
+mmc_bus_execute_command(device_node* node, void* cookie, uint16_t rca,
+       uint8_t command, uint32_t argument, uint32_t* result)
 {
-       // FIXME store the parent cookie in the bus cookie or something instead 
of
-       // getting/putting the parent each time.
-       driver_module_info* mmc;
-       void* cookie;
-
        TRACE("In mmc_bus_execute_command\n");
-       device_node* parent = gDeviceManager->get_parent_node(node);
-       gDeviceManager->get_driver(parent, &mmc, &cookie);
-       gDeviceManager->put_node(parent);
 
        MMCBus* bus = (MMCBus*)cookie;
 
        bus->AcquireBus();
        status_t error = bus->ExecuteCommand(rca, command, argument, result);
        bus->ReleaseBus();
+
        return error;
 }
 
 
 static status_t
-mmc_bus_do_io(device_node* node, uint16_t rca, uint8_t command,
+mmc_bus_do_io(device_node* node, void* cookie, uint16_t rca, uint8_t command,
        IOOperation* operation, bool offsetAsSectors)
 {
-       driver_module_info* mmc;
-       void* cookie;
-
-       // FIXME store the parent cookie in the bus cookie or something instead 
of
-       // getting/putting the parent each time.
-       device_node* parent = gDeviceManager->get_parent_node(node);
-       gDeviceManager->get_driver(parent, &mmc, &cookie);
-       gDeviceManager->put_node(parent);
-
        MMCBus* bus = (MMCBus*)cookie;
-       bus->AcquireBus();
        status_t result = B_OK;
 
+       bus->AcquireBus();
        result = bus->DoIO(rca, command, operation, offsetAsSectors);
-
        bus->ReleaseBus();
+
        return result;
 }
 
 
 static void
-mmc_bus_set_width(device_node* node, int width)
+mmc_bus_set_width(device_node* node, void* cookie, int width)
 {
-       driver_module_info* mmc;
-       void* cookie;
-
-       // FIXME store the parent cookie in the bus cookie or something instead 
of
-       // getting/putting the parent each time.
-       device_node* parent = gDeviceManager->get_parent_node(node);
-       gDeviceManager->get_driver(parent, &mmc, &cookie);
-       gDeviceManager->put_node(parent);
-
        MMCBus* bus = (MMCBus*)cookie;
 
        bus->AcquireBus();
diff --git a/src/add-ons/kernel/busses/mmc/sdhci_pci.cpp 
b/src/add-ons/kernel/busses/mmc/sdhci_pci.cpp
index 18cb8ffe81..ae9831496f 100644
--- a/src/add-ons/kernel/busses/mmc/sdhci_pci.cpp
+++ b/src/add-ons/kernel/busses/mmc/sdhci_pci.cpp
@@ -118,11 +118,12 @@ SdhciBus::SdhciBus(struct registers* registers, uint8_t 
irq)
        // way is to reset everything.
        Reset();
 
-       // Then we configure the clock to the frequency needed for 
initialization
-       SetClock(400);
-
        // Turn on the power supply to the card, if there is a card inserted
-       PowerOn();
+       if (PowerOn()) {
+               // Then we configure the clock to the frequency needed for
+               // initialization
+               SetClock(400);
+       }
 
        // Finally, configure some useful interrupts
        EnableInterrupts(SDHCI_INT_CMD_CMP | SDHCI_INT_CARD_REM
@@ -575,22 +576,33 @@ SdhciBus::HandleInterrupt()
 
        TRACE("interrupt function called %x\n", intmask);
 
-       // handling card presence interrupt
-       if ((intmask & SDHCI_INT_CARD_INS) != 0) {
-               PowerOn();
+       // handling card presence interrupts
+       if ((intmask & SDHCI_INT_CARD_REM) != 0) {
+               // We can get spurious interrupts as the card is inserted or 
removed,
+               // so check the actual state before acting
+               if (!fRegisters->present_state.IsCardInserted())
+                       fRegisters->power_control.PowerOff();
+               else
+                       TRACE("Card removed interrupt, but card is inserted\n");
+
+               fRegisters->interrupt_status |= SDHCI_INT_CARD_REM;
+               TRACE("Card removal interrupt handled\n");
+       }
 
-               release_sem_etc(fScanSemaphore, 1, B_DO_NOT_RESCHEDULE);
+       if ((intmask & SDHCI_INT_CARD_INS) != 0) {
+               // We can get spurious interrupts as the card is inserted or 
removed,
+               // so check the actual state before acting
+               if (fRegisters->present_state.IsCardInserted()) {
+                       if (PowerOn())
+                               SetClock(400);
+                       release_sem_etc(fScanSemaphore, 1, B_DO_NOT_RESCHEDULE);
+               } else
+                       TRACE("Card insertion interrupt, but card is 
removed\n");
 
                fRegisters->interrupt_status |= SDHCI_INT_CARD_INS;
                TRACE("Card presence interrupt handled\n");
        }
 
-       if ((intmask & SDHCI_INT_CARD_REM) != 0) {
-               fRegisters->power_control.PowerOff();
-               fRegisters->interrupt_status |= SDHCI_INT_CARD_REM;
-               TRACE("Card removal interrupt handled\n");
-       }
-
        // handling command interrupt
        if (intmask & SDHCI_INT_CMD_MASK) {
                fCommandResult = intmask;
diff --git a/src/add-ons/kernel/busses/mmc/sdhci_pci.h 
b/src/add-ons/kernel/busses/mmc/sdhci_pci.h
index 381e503384..418f2ed931 100644
--- a/src/add-ons/kernel/busses/mmc/sdhci_pci.h
+++ b/src/add-ons/kernel/busses/mmc/sdhci_pci.h
@@ -234,10 +234,15 @@ class HostControl {
                static const uint8_t kAdma32 = 2 << 3;
                static const uint8_t kAdma64 = 3 << 3;
 
-               static const uint8_t kDataTransferWidthMask = 0x22;
+               // It's convenient to think of this as a single "bit width" 
setting,
+               // but the bits for 4-bit and 8-bit modes were introduced at 
different
+               // times and are not next to each other in the register.
                static const uint8_t kDataTransfer1Bit = 0;
-               static const uint8_t kDataTransfer4Bit = 2;
-               static const uint8_t kDataTransfer8Bit = 0x20;
+               static const uint8_t kDataTransfer4Bit = 1 << 1;
+               static const uint8_t kDataTransfer8Bit = 1 << 5;
+
+               static const uint8_t kDataTransferWidthMask
+                       = kDataTransfer4Bit | kDataTransfer8Bit;
        private:
                volatile uint8_t value;
 } __attribute__((packed));
diff --git a/src/add-ons/kernel/drivers/disk/mmc/mmc_disk.cpp 
b/src/add-ons/kernel/drivers/disk/mmc/mmc_disk.cpp
index 9d2c8d25c6..2fec95e5aa 100644
--- a/src/add-ons/kernel/drivers/disk/mmc/mmc_disk.cpp
+++ b/src/add-ons/kernel/drivers/disk/mmc/mmc_disk.cpp
@@ -129,8 +129,8 @@ mmc_disk_execute_iorequest(void* data, IOOperation* 
operation)
                command = SD_WRITE_MULTIPLE_BLOCKS;
        else
                command = SD_READ_MULTIPLE_BLOCKS;
-       error = info->mmc->do_io(info->parent, info->rca, command, operation,
-               (info->flags & kIoCommandOffsetAsSectors) != 0);
+       error = info->mmc->do_io(info->parent, info->parentCookie, info->rca,
+               command, operation, (info->flags & kIoCommandOffsetAsSectors) 
!= 0);
 
        if (error != B_OK) {
                info->scheduler->OperationCompleted(operation, error, 0);
@@ -147,8 +147,8 @@ mmc_block_get_geometry(mmc_disk_driver_info* info, 
device_geometry* geometry)
 {
        struct mmc_disk_csd csd;
        TRACE("Get geometry\n");
-       status_t error = info->mmc->execute_command(info->parent, 0, 
SD_SEND_CSD,
-               info->rca << 16, (uint32_t*)&csd);
+       status_t error = info->mmc->execute_command(info->parent,
+               info->parentCookie, 0, SD_SEND_CSD, info->rca << 16, 
(uint32_t*)&csd);
        if (error != B_OK) {
                TRACE("Could not get CSD! %s\n", strerror(error));
                return error;
@@ -175,13 +175,13 @@ mmc_block_get_geometry(mmc_disk_driver_info* info, 
device_geometry* geometry)
        // default 1 bit mode)
        uint32_t cardStatus;
        const uint32 k4BitMode = 2;
-       info->mmc->execute_command(info->parent, info->rca, SD_APP_CMD,
-               info->rca << 16, &cardStatus);
-       info->mmc->execute_command(info->parent, info->rca, SD_SET_BUS_WIDTH,
-               k4BitMode, &cardStatus);
+       info->mmc->execute_command(info->parent, info->parentCookie, info->rca,
+               SD_APP_CMD, info->rca << 16, &cardStatus);
+       info->mmc->execute_command(info->parent, info->parentCookie, info->rca,
+               SD_SET_BUS_WIDTH, k4BitMode, &cardStatus);
 
        // From now on we use 4 bit mode
-       info->mmc->set_bus_width(info->parent, 4);
+       info->mmc->set_bus_width(info->parent, info->parentCookie, 4);
 
        return B_OK;
 }
@@ -199,11 +199,19 @@ mmc_disk_init_driver(device_node* node, void** cookie)
 
        memset(info, 0, sizeof(*info));
 
-       void* unused;
+       void* unused2;
        info->node = node;
        info->parent = sDeviceManager->get_parent_node(info->node);
        sDeviceManager->get_driver(info->parent, (driver_module_info 
**)&info->mmc,
-               &unused);
+               &unused2);
+
+       // We need to grab the bus cookie as well
+       // FIXME it would be easier if that was available from the get_driver 
call
+       // above directly, but currently it isn't.
+       device_node* busNode = sDeviceManager->get_parent_node(info->parent);
+       driver_module_info* unused;
+       sDeviceManager->get_driver(busNode, &unused, &info->parentCookie);
+       sDeviceManager->put_node(busNode);
 
        TRACE("MMC bus handle: %p %s\n", info->mmc, info->mmc->info.info.name);
 
diff --git a/src/add-ons/kernel/drivers/disk/mmc/mmc_disk.h 
b/src/add-ons/kernel/drivers/disk/mmc/mmc_disk.h
index 8f82daba5f..c17e052c4a 100644
--- a/src/add-ons/kernel/drivers/disk/mmc/mmc_disk.h
+++ b/src/add-ons/kernel/drivers/disk/mmc/mmc_disk.h
@@ -30,6 +30,7 @@ enum MMCDiskFlags {
 typedef struct {
        device_node* node;
        device_node* parent;
+       void* parentCookie;
        mmc_device_interface* mmc;
        uint16_t rca;
        uint32_t flags;


Other related posts:

  • » [haiku-commits] haiku: hrev54902 - src/add-ons/kernel/busses/mmc src/add-ons/kernel/bus_managers/mmc src/add-ons/kernel/drivers/disk/mmc headers/private/drivers - Adrien Destugues