Aren/haiku
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

sd/mmc: Cleanup and improve reliability

Browse Source 
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@gmail.com>
This commit is contained in:
Adrien Destugues 2021-01-16 22:50:22 +01:00 committed by Adrien Destugues
parent 34552f8e66
commit 5ec64c5cdd
7 changed files with 65 additions and 69 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
headers/private/drivers/mmc.h
View File

@ -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;

7
src/add-ons/kernel/bus_managers/mmc/mmc_bus.cpp
View File

@ -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");

39
src/add-ons/kernel/bus_managers/mmc/mmc_module.cpp
View File

@ -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();

40
src/add-ons/kernel/busses/mmc/sdhci_pci.cpp
View File

@ -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");
release_sem_etc(fScanSemaphore, 1, B_DO_NOT_RESCHEDULE);
fRegisters->interrupt_status |= SDHCI_INT_CARD_REM;
TRACE("Card removal interrupt handled\n");
}
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;

11
src/add-ons/kernel/busses/mmc/sdhci_pci.h
View File

@ -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));

30
src/add-ons/kernel/drivers/disk/mmc/mmc_disk.cpp
View File

@ -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);

1
src/add-ons/kernel/drivers/disk/mmc/mmc_disk.h
View File

@ -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;