On 19.11.2016 14:48, Simon Glass wrote:
> Hi Michal,
>
> On 18 November 2016 at 08:44, Michal Simek <michal.si...@xilinx.com> wrote:
>> The patch enables running detection algorithm on block device
>> description structure.
>>
>> Signed-off-by: Michal Simek <michal.si...@xilinx.com>
>> ---
>>
>> common/scsi.c | 134
>> ++++++++++++++++++++++++++++++++--------------------------
>> 1 file changed, 73 insertions(+), 61 deletions(-)
>
> Reviewed-by: Simon Glass <s...@chromium.org>
>
> Please see below.
>
>>
>> diff --git a/common/scsi.c b/common/scsi.c
>> index 0bce91dfa099..a02c7a505639 100644
>> --- a/common/scsi.c
>> +++ b/common/scsi.c
>> @@ -480,15 +480,79 @@ static void scsi_init_dev_desc(struct blk_desc
>> *dev_desc, int devnum)
>> #endif
>> }
>>
>> +static int scsi_detect_dev(ccb *pccb, struct blk_desc *dev_desc, int lun)
>
> Can you comment this function?
will do.
>
>> +{
>> + unsigned char perq, modi;
>> + lbaint_t capacity;
>> + unsigned long blksz;
>> +
>> + pccb->lun = lun;
>> + pccb->pdata = (unsigned char *)&tempbuff;
>> + pccb->datalen = 512;
>> + scsi_setup_inquiry(pccb);
>> + if (scsi_exec(pccb) != true) {
>> + if (pccb->contr_stat == SCSI_SEL_TIME_OUT) {
>> + /*
>> + * selection timeout => assuming no
>> + * device present
>> + */
>> + debug("Selection timeout ID %d\n",
>> + pccb->target);
>> + return -ETIMEDOUT;
>> + }
>> + scsi_print_error(pccb);
>> + return -ENODEV;
>> + }
>> + perq = tempbuff[0];
>> + modi = tempbuff[1];
>> + if ((perq & 0x1f) == 0x1f)
>> + return -ENODEV; /* skip unknown devices */
>> + if ((modi & 0x80) == 0x80) /* drive is removable */
>> + dev_desc->removable = true;
>> + /* get info for this device */
>> + scsi_ident_cpy((unsigned char *)&dev_desc
>> + [0].vendor[0],
>> + &tempbuff[8], 8);
>> + scsi_ident_cpy((unsigned char *)&dev_desc
>> + [0].product[0],
>
> Can you use more of the line here? You have 80 columns!
will fix.
>
>> + &tempbuff[16], 16);
>> + scsi_ident_cpy((unsigned char *)&dev_desc
>> + [0].revision[0],
>> + &tempbuff[32], 4);
>> + dev_desc->target = pccb->target;
>> + dev_desc->lun = pccb->lun;
>> +
>> + pccb->datalen = 0;
>> + scsi_setup_test_unit_ready(pccb);
>> + if (scsi_exec(pccb) != true) {
>> + if (dev_desc->removable) {
>> + dev_desc->type = perq;
>> + goto removable;
>> + }
>> + scsi_print_error(pccb);
>> + return -EINVAL;
>> + }
>> + if (scsi_read_capacity(pccb, &capacity, &blksz)) {
>> + scsi_print_error(pccb);
>> + return -EINVAL;
>
> Should you not return the error from scsi_read_capacity()?
>
scsi_read_capacity returns 1 or 0 error value which is just useless.
Thanks,
Michal
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
