On Mon, 2017-12-11 at 20:10 -0500, Douglas Gilbert wrote:
> -static const struct opcode_info_t vl_iarr[1] = { /* VARIABLE LENGTH */
> +static const struct opcode_info_t vl_iarr[2] = { /* VARIABLE LENGTH */
Please leave out the array size and let the compiler determine the array size.
> {0, 0x7f, 0xb, F_SA_HIGH | F_D_OUT | FF_DIRECT_IO, resp_write_dt0,
> - NULL, {32, 0xc7, 0, 0, 0, 0, 0x1f, 0x18, 0x0, 0xb, 0xfa,
> - 0, 0xff, 0xff, 0xff, 0xff} }, /* WRITE(32) */
> + NULL, {32, 0xc7, 0, 0, 0, 0, 0x3f, 0x18, 0x0, 0xb, 0xfa,
> + 0, 0xff, 0xff, 0xff, 0xff} }, /* WRITE(32) */
Shouldn't this change have been included in the patch that fixes the group
number mask?
> static const struct opcode_info_t maint_in_iarr[2] = {
> @@ -518,9 +523,10 @@ static const struct opcode_info_t opcode_info_arr[SDEB_I_LAST_ELEMENT + 1] = {
> {6, 0x1, 0, 0xf, 0xf7, 0xc7, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0} },
> {1, 0x9e, 0x10, F_SA_LOW | F_D_IN, resp_readcap16, sa_in_iarr,
> {16, 0x10, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
> - 0xff, 0xff, 0xff, 0x1, 0xc7} }, /* READ CAPACITY(16) */
> - {0, 0, 0, F_INV_OP | FF_RESPOND, NULL, NULL, /* SA OUT */
> - {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0} },
> + 0xff, 0xff, 0xff, 0x1, 0xc7} },/* SA_IN(16), READ CAPACITY(16) */
Shouldn't the above change be folded into one of the other patches?
> @@ -529,9 +535,9 @@ static const struct opcode_info_t opcode_info_arr[SDEB_I_LAST_ELEMENT + 1] = {
> {0, 0x2f, 0, F_D_OUT_MAYBE | FF_DIRECT_IO, NULL, NULL, /* VERIFY(10) */
> {10, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xc7,
> 0, 0, 0, 0, 0, 0} },
> - {1, 0x7f, 0x9, F_SA_HIGH | F_D_IN | FF_DIRECT_IO, resp_read_dt0,
> - vl_iarr, {32, 0xc7, 0, 0, 0, 0, 0x1f, 0x18, 0x0, 0x9, 0xfe, 0,
> - 0xff, 0xff, 0xff, 0xff} },/* VARIABLE LENGTH, READ(32) */
> + {2, 0x7f, 0x9, F_SA_HIGH | F_D_IN | FF_DIRECT_IO, resp_read_dt0,
> + vl_iarr, {32, 0xc7, 0, 0, 0, 0, 0x3f, 0x18, 0x0, 0x9, 0xfe, 0,
> + 0xff, 0xff, 0xff, 0xff} }, /* VARIABLE LENGTH, READ(32) */
Have you considered to use ARRAY_SIZE(vl_iarr) instead of hard-coding the array
size?
> + if (cmd[0] == VARIABLE_LENGTH_CMD) {
> + is_16 = false;
> + wrprotect = (cmd[10] >> 5) & 0x7;
> + lbdof = get_unaligned_be16(cmd + 12);
> + num_lrd = get_unaligned_be16(cmd + 16);
> + bt_len = get_unaligned_be32(cmd + 28);
> + check_prot = false;
> + } else { /* that leaves WRITE SCATTERED(16) */
> + is_16 = true;
> + wrprotect = (cmd[2] >> 5) & 0x7;
> + lbdof = get_unaligned_be16(cmd + 4);
> + num_lrd = get_unaligned_be16(cmd + 8);
> + bt_len = get_unaligned_be32(cmd + 10);
> + check_prot = true;
> + }
It's not clear to me why check_prot is set to false for WRITE SCATTERED(32)
and set to true for WRITE SCATTERED(16)?
Bart.
