On 1/4/22 20:51, Hannes Reinecke wrote:
> On 1/4/22 11:58 AM, Damien Le Moal wrote:
>> ATA IDENTIFY command returns an array of le16 words. Accessing it as a
>> u16 array triggers the following sparse warning:
>>
>> drivers/ata/ahci_xgene.c:262:33: warning: invalid assignment: &=
>> drivers/ata/ahci_xgene.c:262:33: left side has type unsigned short
>> drivers/ata/ahci_xgene.c:262:33: right side has type restricted __le16
>>
>> Use a local variable to explicitly cast the id array to __le16 to avoid
>> this warning.
>>
>> Signed-off-by: Damien Le Moal <damien.lemoal@xxxxxxxxxxxxxxxxxx>
>> ---
>> drivers/ata/ahci_xgene.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/ata/ahci_xgene.c b/drivers/ata/ahci_xgene.c
>> index 68ec7e9430b2..d5075d0f8cb1 100644
>> --- a/drivers/ata/ahci_xgene.c
>> +++ b/drivers/ata/ahci_xgene.c
>> @@ -239,6 +239,7 @@ static bool xgene_ahci_is_memram_inited(struct xgene_ahci_context *ctx)
>> static unsigned int xgene_ahci_read_id(struct ata_device *dev,
>> struct ata_taskfile *tf, u16 *id)
>> {
>> + __le16 *__id = (__le16 *)id;
>> u32 err_mask;
>>
>> err_mask = ata_do_dev_read_id(dev, tf, id);
>> @@ -259,7 +260,7 @@ static unsigned int xgene_ahci_read_id(struct ata_device *dev,
>> *
>> * Clear reserved bit 8 (DEVSLP bit) as we don't support DEVSLP
>> */
>> - id[ATA_ID_FEATURE_SUPP] &= cpu_to_le16(~(1 << 8));
>> + __id[ATA_ID_FEATURE_SUPP] &= cpu_to_le16(~(1 << 8));
>>
>> return 0;
>> }
>>
> Hmm. I would think that the 'id' argument is wrong; it really should be
> '__le16 *', as it only gets converted later on with the call to
> swap_buf_le16(id, ATA_ID_WORDS) in
> drivers/ata/libata-core.c:ata_dev_read_id(). So when this function is
> called the argument really _is_ __le16, only the declaration doesn't
> tell us.
>
> Maybe one should rather fix this?
Good point. Sending V3 with this patch changed to fix read_id()
operation interface.
>
> Cheers,
>
> Hannes
--
Damien Le Moal
Western Digital Research
