Dear Max, Thank you for your instant reply. Am 25.01.24 um 15:36 schrieb Max Tottenham:
On 01/25, Paul Menzel wrote:
Am 25.01.24 um 12:35 schrieb Max Tottenham via SeaBIOS:
[…]
--- src/hw/blockcmd.c | 61 +++++++++++++++++++++++++++++++++++------------ src/hw/blockcmd.h | 27 +++++++++++++++++++++ 2 files changed, 73 insertions(+), 15 deletions(-)
diff --git a/src/hw/blockcmd.c b/src/hw/blockcmd.c index 6b6fea97..9b6a32bf 100644 --- a/src/hw/blockcmd.c +++ b/src/hw/blockcmd.c @@ -66,6 +66,23 @@ cdb_test_unit_ready(struct disk_op_s *op) return process_op(op); }
+static int +cdb_read_capacity16(struct disk_op_s *op, struct cdbres_read_capacity_16 *data) +{ + struct cdb_sai_read_capacity_16 cmd; + memset(&cmd, 0, sizeof(cmd)); + cmd.command = CDB_CMD_SERVICE_ACTION_IN; + cmd.flags = CDB_CMD_SAI_READ_CAPACITY_16; + cmd.len = cpu_to_be32(sizeof(struct cdbres_read_capacity_16)); + op->command = CMD_SCSI; + op->count = 1; + op->buf_fl = data; + op->cdbcmd = &cmd; + op->blocksize = sizeof(*data); + return process_op(op); +} + + // Request capacity static int cdb_read_capacity(struct disk_op_s *op, struct cdbres_read_capacity *data) @@ -111,13 +128,23 @@ scsi_fill_cmd(struct disk_op_s *op, void *cdbcmd, int maxcdb) switch (op->command) { case CMD_READ: case CMD_WRITE: ; - struct cdb_rwdata_10 *cmd = cdbcmd; - memset(cmd, 0, maxcdb); - cmd->command = (op->command == CMD_READ ? CDB_CMD_READ_10 - : CDB_CMD_WRITE_10); - cmd->lba = cpu_to_be32(op->lba); - cmd->count = cpu_to_be16(op->count); - return GET_FLATPTR(op->drive_fl->blksize); + if (op->lba < 0xFFFFFFFFULL) { + struct cdb_rwdata_10 *cmd = cdbcmd; + memset(cmd, 0, maxcdb); + cmd->command = (op->command == CMD_READ ? CDB_CMD_READ_10 + : CDB_CMD_WRITE_10); + cmd->lba = cpu_to_be32(op->lba); + cmd->count = cpu_to_be16(op->count); + return GET_FLATPTR(op->drive_fl->blksize); + } else { + struct cdb_rwdata_16 *cmd = cdbcmd; + memset(cmd, 0, maxcdb); + cmd->command = (op->command == CMD_READ ? CDB_CMD_READ_16 + : CDB_CMD_WRITE_16); + cmd->lba = cpu_to_be64(op->lba); + cmd->count = cpu_to_be32(op->count); + return GET_FLATPTR(op->drive_fl->blksize); + }
It’s bikeshedding, but maybe the code in the branches could be factored into separate functions, and then the return also wouldn’t need to be duplicated.
I missed this comment earlier - Perhaps rather than functions we could just extract the return statement out of each of the branches e.g. do the following?:
if (op->lba < 0xFFFFFFFFULL) { struct cdb_rwdata_10 *cmd = cdbcmd; memset(cmd, 0, maxcdb); cmd->command = (op->command == CMD_READ ? CDB_CMD_READ_10 : CDB_CMD_WRITE_10); cmd->lba = cpu_to_be32(op->lba); cmd->count = cpu_to_be16(op->count); } else { struct cdb_rwdata_16 *cmd = cdbcmd; memset(cmd, 0, maxcdb); cmd->command = (op->command == CMD_READ ? CDB_CMD_READ_16 : CDB_CMD_WRITE_16); cmd->lba = cpu_to_be64(op->lba); cmd->count = cpu_to_be32(op->count); } return GET_FLATPTR(op->drive_fl->blksize);
Seems odd to me to make two separate functions for such a small 'fill this struct with data' ?
Yes, agreed. […] Kind regards, Paul