> + case UPIU_TRANSACTION_QUERY_REQ:
> + qr = &bsg_request->upiu_req.qr;
> + if (qr->opcode == UPIU_QUERY_OPCODE_READ_DESC)
> + goto not_supported;
> +
> + if (ufs_bsg_get_query_desc_size(hba, qr, &desc_len))
> + goto null_desc_buff;
> +
> + if (qr->opcode == UPIU_QUERY_OPCODE_WRITE_DESC) {
> + rw = WRITE;
> + desc_buff = (uint8_t *)(bsg_request + 1);
> + }
> +
> +null_desc_buff:
> + /* fall through */
> + case UPIU_TRANSACTION_NOP_OUT:
> + case UPIU_TRANSACTION_TASK_REQ:
> + /* Now that we know if its a read or write, verify again */
> + if (rw != UFS_BSG_NOP || desc_len) {
> + ret = ufs_bsg_verify_query_size(request_len, reply_len,
> + rw, desc_len);
> + if (ret) {
> + dev_err(job->dev,
> + "not enough space assigned\n");
> + goto out;
> + }
> + }
I think this check should be moved into the above switch case
as it can only be hit for UPIU_TRANSACTION_QUERY_REQ requests. In
fact I think the code would be a lot cleaner if you could factor
the UPIU_TRANSACTION_QUERY_REQ case into a little helper function
(ufs_bsg_get_query_desc_size should probably be merged into that
helper also).
> + case UPIU_TRANSACTION_UIC_CMD:
> + /* later */
> + case UPIU_TRANSACTION_COMMAND:
> + case UPIU_TRANSACTION_DATA_OUT:
> +not_supported:
> + /* for the time being, we do not support data transfer upiu */
> + ret = -ENOTSUPP;
> + dev_err(job->dev, "unsupported msgcode 0x%x\n", msgcode);
> +
> + break;
> + default:
> + ret = -EINVAL;
> +
> + break;
> + }
This difference between known but not supported and not recognized is
a bit odd. I think there should be just the generic not supported
case, and you can decide if you want to log it or not.
Also please try to avoid goto labels jumping into switch statements,
code is generlaly a lot more readable if you keep the goto targets
outside the switch statement.
