Skip to content

Commit 0561e51

Browse files
author
Damien Le Moal
committed
ata: fix read_id() ata port operation interface
Drivers that need to tweak a device IDENTIFY data implement the read_id() port operation. The IDENTIFY data buffer is passed as an argument to the read_id() operation for drivers to use. However, when this operation is called, the IDENTIFY data is not yet converted to CPU endian and contains le16 words. Change the interface of the read_id operation to pass a __le16 * pointer to the IDENTIFY data buffer to clarify the buffer endianness. Fix the pata_netcell, pata_it821x, ahci_xgene, ahci_ceva and ahci_brcm drivers implementation of this operation and modify the code to corretly deal with identify data words manipulation to avoid sparse warnings such as: 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 Signed-off-by: Damien Le Moal <[email protected]> Reviewed-by: Hannes Reinecke <[email protected]>
1 parent 2bce690 commit 0561e51

File tree

7 files changed

+24
-24
lines changed

7 files changed

+24
-24
lines changed

drivers/ata/ahci_brcm.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ static void brcm_sata_init(struct brcm_ahci_priv *priv)
246246
}
247247

248248
static unsigned int brcm_ahci_read_id(struct ata_device *dev,
249-
struct ata_taskfile *tf, u16 *id)
249+
struct ata_taskfile *tf, __le16 *id)
250250
{
251251
struct ata_port *ap = dev->link->ap;
252252
struct ata_host *host = ap->host;

drivers/ata/ahci_ceva.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,8 @@ struct ceva_ahci_priv {
9292
};
9393

9494
static unsigned int ceva_ahci_read_id(struct ata_device *dev,
95-
struct ata_taskfile *tf, u16 *id)
95+
struct ata_taskfile *tf, __le16 *id)
9696
{
97-
__le16 *__id = (__le16 *)id;
9897
u32 err_mask;
9998

10099
err_mask = ata_do_dev_read_id(dev, tf, id);
@@ -104,7 +103,7 @@ static unsigned int ceva_ahci_read_id(struct ata_device *dev,
104103
* Since CEVA controller does not support device sleep feature, we
105104
* need to clear DEVSLP (bit 8) in word78 of the IDENTIFY DEVICE data.
106105
*/
107-
__id[ATA_ID_FEATURE_SUPP] &= cpu_to_le16(~(1 << 8));
106+
id[ATA_ID_FEATURE_SUPP] &= cpu_to_le16(~(1 << 8));
108107

109108
return 0;
110109
}

drivers/ata/ahci_xgene.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,7 @@ static bool xgene_ahci_is_memram_inited(struct xgene_ahci_context *ctx)
237237
* does not support DEVSLP.
238238
*/
239239
static unsigned int xgene_ahci_read_id(struct ata_device *dev,
240-
struct ata_taskfile *tf, u16 *id)
240+
struct ata_taskfile *tf, __le16 *id)
241241
{
242242
u32 err_mask;
243243

drivers/ata/libata-core.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1722,7 +1722,7 @@ static u32 ata_pio_mask_no_iordy(const struct ata_device *adev)
17221722
* this function is wrapped or replaced by the driver
17231723
*/
17241724
unsigned int ata_do_dev_read_id(struct ata_device *dev,
1725-
struct ata_taskfile *tf, u16 *id)
1725+
struct ata_taskfile *tf, __le16 *id)
17261726
{
17271727
return ata_exec_internal(dev, tf, NULL, DMA_FROM_DEVICE,
17281728
id, sizeof(id[0]) * ATA_ID_WORDS, 0);
@@ -1795,9 +1795,9 @@ int ata_dev_read_id(struct ata_device *dev, unsigned int *p_class,
17951795
tf.flags |= ATA_TFLAG_POLLING;
17961796

17971797
if (ap->ops->read_id)
1798-
err_mask = ap->ops->read_id(dev, &tf, id);
1798+
err_mask = ap->ops->read_id(dev, &tf, (__le16 *)id);
17991799
else
1800-
err_mask = ata_do_dev_read_id(dev, &tf, id);
1800+
err_mask = ata_do_dev_read_id(dev, &tf, (__le16 *)id);
18011801

18021802
if (err_mask) {
18031803
if (err_mask & AC_ERR_NODEV_HINT) {

drivers/ata/pata_it821x.c

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -537,29 +537,28 @@ static void it821x_dev_config(struct ata_device *adev)
537537
*/
538538

539539
static unsigned int it821x_read_id(struct ata_device *adev,
540-
struct ata_taskfile *tf, u16 *id)
540+
struct ata_taskfile *tf, __le16 *id)
541541
{
542542
unsigned int err_mask;
543543
unsigned char model_num[ATA_ID_PROD_LEN + 1];
544544

545545
err_mask = ata_do_dev_read_id(adev, tf, id);
546546
if (err_mask)
547547
return err_mask;
548-
ata_id_c_string(id, model_num, ATA_ID_PROD, sizeof(model_num));
548+
ata_id_c_string((u16 *)id, model_num, ATA_ID_PROD, sizeof(model_num));
549549

550-
id[83] &= ~(1 << 12); /* Cache flush is firmware handled */
551-
id[83] &= ~(1 << 13); /* Ditto for LBA48 flushes */
552-
id[84] &= ~(1 << 6); /* No FUA */
553-
id[85] &= ~(1 << 10); /* No HPA */
554-
id[76] = 0; /* No NCQ/AN etc */
550+
id[83] &= cpu_to_le16(~(1 << 12)); /* Cache flush is firmware handled */
551+
id[84] &= cpu_to_le16(~(1 << 6)); /* No FUA */
552+
id[85] &= cpu_to_le16(~(1 << 10)); /* No HPA */
553+
id[76] = 0; /* No NCQ/AN etc */
555554

556555
if (strstr(model_num, "Integrated Technology Express")) {
557556
/* Set feature bits the firmware neglects */
558-
id[49] |= 0x0300; /* LBA, DMA */
559-
id[83] &= 0x7FFF;
560-
id[83] |= 0x4400; /* Word 83 is valid and LBA48 */
561-
id[86] |= 0x0400; /* LBA48 on */
562-
id[ATA_ID_MAJOR_VER] |= 0x1F;
557+
id[49] |= cpu_to_le16(0x0300); /* LBA, DMA */
558+
id[83] &= cpu_to_le16(0x7FFF);
559+
id[83] |= cpu_to_le16(0x4400); /* Word 83 is valid and LBA48 */
560+
id[86] |= cpu_to_le16(0x0400); /* LBA48 on */
561+
id[ATA_ID_MAJOR_VER] |= cpu_to_le16(0x1F);
563562
/* Clear the serial number because it's different each boot
564563
which breaks validation on resume */
565564
memset(&id[ATA_ID_SERNO], 0x20, ATA_ID_SERNO_LEN);

drivers/ata/pata_netcell.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,13 @@
2121
/* No PIO or DMA methods needed for this device */
2222

2323
static unsigned int netcell_read_id(struct ata_device *adev,
24-
struct ata_taskfile *tf, u16 *id)
24+
struct ata_taskfile *tf, __le16 *id)
2525
{
2626
unsigned int err_mask = ata_do_dev_read_id(adev, tf, id);
27+
2728
/* Firmware forgets to mark words 85-87 valid */
2829
if (err_mask == 0)
29-
id[ATA_ID_CSF_DEFAULT] |= 0x4000;
30+
id[ATA_ID_CSF_DEFAULT] |= cpu_to_le16(0x4000);
3031
return err_mask;
3132
}
3233

include/linux/libata.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -884,7 +884,8 @@ struct ata_port_operations {
884884
void (*set_piomode)(struct ata_port *ap, struct ata_device *dev);
885885
void (*set_dmamode)(struct ata_port *ap, struct ata_device *dev);
886886
int (*set_mode)(struct ata_link *link, struct ata_device **r_failed_dev);
887-
unsigned int (*read_id)(struct ata_device *dev, struct ata_taskfile *tf, u16 *id);
887+
unsigned int (*read_id)(struct ata_device *dev, struct ata_taskfile *tf,
888+
__le16 *id);
888889

889890
void (*dev_config)(struct ata_device *dev);
890891

@@ -1119,7 +1120,7 @@ extern void ata_id_string(const u16 *id, unsigned char *s,
11191120
extern void ata_id_c_string(const u16 *id, unsigned char *s,
11201121
unsigned int ofs, unsigned int len);
11211122
extern unsigned int ata_do_dev_read_id(struct ata_device *dev,
1122-
struct ata_taskfile *tf, u16 *id);
1123+
struct ata_taskfile *tf, __le16 *id);
11231124
extern void ata_qc_complete(struct ata_queued_cmd *qc);
11241125
extern u64 ata_qc_get_active(struct ata_port *ap);
11251126
extern void ata_scsi_simulate(struct ata_device *dev, struct scsi_cmnd *cmd);

0 commit comments

Comments
 (0)