On 01/04/2020 10:03, Christian Mauderer wrote:
The rtems_bsd_mmcsd_attach_worker acquired the bus without releasing it.
Fix that by only acquire the bus if necessary (during initialization and
during read / writes).
---
freebsd/sys/dev/mmc/mmcsd.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/freebsd/sys/dev/mmc/mmcsd.c b/freebsd/sys/dev/mmc/mmcsd.c
index 077f8d74..674be6a7 100644
--- a/freebsd/sys/dev/mmc/mmcsd.c
+++ b/freebsd/sys/dev/mmc/mmcsd.c
@@ -268,6 +268,8 @@ rtems_bsd_mmcsd_set_block_size(device_t dev, uint32_t
block_size)
memset(&req, 0, sizeof(req));
memset(&cmd, 0, sizeof(cmd));
+ MMCBUS_ACQUIRE_BUS(device_get_parent(dev), dev);
+
req.cmd = &cmd;
cmd.opcode = MMC_SET_BLOCKLEN;
cmd.flags = MMC_RSP_R1 | MMC_CMD_AC;
@@ -278,6 +280,8 @@ rtems_bsd_mmcsd_set_block_size(device_t dev, uint32_t
block_size)
status_code = RTEMS_IO_ERROR;
}
+ MMCBUS_RELEASE_BUS(device_get_parent(dev), dev);
+
return status_code;
}
@@ -316,6 +320,7 @@ rtems_bsd_mmcsd_disk_read_write(struct mmcsd_part *part, rtems_blkdev_request *b
data_flags = MMC_DATA_READ;
}
+ MMCBUS_ACQUIRE_BUS(device_get_parent(dev), dev);
MMCSD_DISK_LOCK(part);
I don't really know when you have to acquire the bus, but it is an
expensive operation. Did you measure the performance impact of this change?
I guess the use of the disk lock here was an optimization. If we really
want to use the bus lock, then the disk lock should be removed.
for (i = 0; i < buffer_count; ++i) {
@@ -394,6 +399,7 @@ rtems_bsd_mmcsd_disk_read_write(struct mmcsd_part *part,
rtems_blkdev_request *b
error:
MMCSD_DISK_UNLOCK(part);
+ MMCBUS_RELEASE_BUS(device_get_parent(dev), dev);
rtems_blkdev_request_done(blkreq, status_code);
@@ -436,8 +442,6 @@ rtems_bsd_mmcsd_attach_worker(rtems_media_state state, const char *src, char **d
goto error;
}
- MMCBUS_ACQUIRE_BUS(device_get_parent(dev), dev);
I think the bus acquire here is necessary to prevent an
rtems_blkdev_create() while the bus is detached in parallel, e.g. a fast
plug/unplug of an USB stick.
After reviewing the code I think this bus acquire without a release
(which is in the code since the import from libusb in 2015) was a hack
to improve the read/write performance. We didn't support hot plugging of
MMC busses and devices. Do we need this feature now?
-
status_code = rtems_bsd_mmcsd_set_block_size(dev, block_size);
if (status_code != RTEMS_SUCCESSFUL) {
printf("OOPS: set block size failed\n");
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
