On 2022/06/01 19:29, Sergey Shtylyov wrote:
> Hello!
>
> On 5/31/22 8:50 PM, Tyler Erickson wrote:
>
>> From: Tyler Erickson <tyler.j.erickson@xxxxxxxxxxx>
>>
>> The concurrent positioning ranges log is not a fixed size and may depend
>> on how many ranges are supported by the device. This patch uses the size
>> reported in the GPL directory to determine the number of pages supported
>> by the device before attempting to read this log page.
>>
>> Also fixing the page length in the SCSI translation for the concurrent
>> positioning ranges VPD page.
>>
>> This resolves this error from the dmesg output:
>> ata6.00: Read log 0x47 page 0x00 failed, Emask 0x1
>>
>> Signed-off-by: Tyler Erickson <tyler.j.erickson@xxxxxxxxxxx>
>> Reviewed-by: Muhammad Ahmad <muhammad.ahmad@xxxxxxxxxxx>
>> Tested-by: Michael English <michael.english@xxxxxxxxxxx>
>>
>> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
>> index ca64837641be..3d57fa84e2be 100644
>> --- a/drivers/ata/libata-core.c
>> +++ b/drivers/ata/libata-core.c
>> @@ -2003,16 +2003,16 @@ unsigned int ata_read_log_page(struct ata_device *dev, u8 log,
>> return err_mask;
>> }
>>
>> -static bool ata_log_supported(struct ata_device *dev, u8 log)
>> +static int ata_log_supported(struct ata_device *dev, u8 log)
>
> Maybe *unsigned int*? The 'buf_len' variable below is 'size_t' which is *unsigned* type...
int is fine I think. The value is 16-bits so no overflow possible. And having a
signed value, we can change the code to return an error code if ever needed.
>
>> {
>> struct ata_port *ap = dev->link->ap;
>>
>> if (dev->horkage & ATA_HORKAGE_NO_LOG_DIR)
>> - return false;
>> + return 0;
>>
>> if (ata_read_log_page(dev, ATA_LOG_DIRECTORY, 0, ap->sector_buf, 1))
>> - return false;
>> - return get_unaligned_le16(&ap->sector_buf[log * 2]) ? true : false;
>> + return 0;
>> + return get_unaligned_le16(&ap->sector_buf[log * 2]);
>> }
>>
>> static bool ata_identify_page_supported(struct ata_device *dev, u8 page)
>> @@ -2448,15 +2448,20 @@ static void ata_dev_config_cpr(struct ata_device *dev)
>> struct ata_cpr_log *cpr_log = NULL;
>> u8 *desc, *buf = NULL;
>>
>> - if (ata_id_major_version(dev->id) < 11 ||
>> - !ata_log_supported(dev, ATA_LOG_CONCURRENT_POSITIONING_RANGES))
>> + if (ata_id_major_version(dev->id) < 11)
>> + goto out;
>> +
>> + buf_len = ata_log_supported(dev, ATA_LOG_CONCURRENT_POSITIONING_RANGES);
>> + if (buf_len == 0)
>> goto out;
> [...]
>
> MBR, Sergey
--
Damien Le Moal
Western Digital Research
