All of lore.kernel.org
 help / color / mirror / Atom feed
From: Damien Le Moal <damien.lemoal@opensource.wdc.com>
To: linux-ide@vger.kernel.org
Cc: Hannes Reinecke <hare@suse.de>
Subject: [PATCH v3 22/22] ata: fix read_id() ata port operation interface
Date: Wed,  5 Jan 2022 14:17:35 +0900	[thread overview]
Message-ID: <20220105051735.1871177-23-damien.lemoal@opensource.wdc.com> (raw)
In-Reply-To: <20220105051735.1871177-1-damien.lemoal@opensource.wdc.com>

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 <damien.lemoal@opensource.wdc.com>
---
 drivers/ata/ahci_brcm.c    |  2 +-
 drivers/ata/ahci_ceva.c    |  5 ++---
 drivers/ata/ahci_xgene.c   |  2 +-
 drivers/ata/libata-core.c  |  6 +++---
 drivers/ata/pata_it821x.c  | 23 +++++++++++------------
 drivers/ata/pata_netcell.c |  5 +++--
 include/linux/libata.h     |  5 +++--
 7 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/drivers/ata/ahci_brcm.c b/drivers/ata/ahci_brcm.c
index 6e9c5ade4c2e..ba695338927a 100644
--- a/drivers/ata/ahci_brcm.c
+++ b/drivers/ata/ahci_brcm.c
@@ -246,7 +246,7 @@ static void brcm_sata_init(struct brcm_ahci_priv *priv)
 }
 
 static unsigned int brcm_ahci_read_id(struct ata_device *dev,
-				      struct ata_taskfile *tf, u16 *id)
+				      struct ata_taskfile *tf, __le16 *id)
 {
 	struct ata_port *ap = dev->link->ap;
 	struct ata_host *host = ap->host;
diff --git a/drivers/ata/ahci_ceva.c b/drivers/ata/ahci_ceva.c
index e9c7c07fd84c..acf59f51b356 100644
--- a/drivers/ata/ahci_ceva.c
+++ b/drivers/ata/ahci_ceva.c
@@ -92,9 +92,8 @@ struct ceva_ahci_priv {
 };
 
 static unsigned int ceva_ahci_read_id(struct ata_device *dev,
-					struct ata_taskfile *tf, u16 *id)
+				      struct ata_taskfile *tf, __le16 *id)
 {
-	__le16 *__id = (__le16 *)id;
 	u32 err_mask;
 
 	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,
 	 * Since CEVA controller does not support device sleep feature, we
 	 * need to clear DEVSLP (bit 8) in word78 of the IDENTIFY DEVICE data.
 	 */
-	__id[ATA_ID_FEATURE_SUPP] &= cpu_to_le16(~(1 << 8));
+	id[ATA_ID_FEATURE_SUPP] &= cpu_to_le16(~(1 << 8));
 
 	return 0;
 }
diff --git a/drivers/ata/ahci_xgene.c b/drivers/ata/ahci_xgene.c
index 68ec7e9430b2..8e206379d699 100644
--- a/drivers/ata/ahci_xgene.c
+++ b/drivers/ata/ahci_xgene.c
@@ -237,7 +237,7 @@ static bool xgene_ahci_is_memram_inited(struct xgene_ahci_context *ctx)
  * does not support DEVSLP.
  */
 static unsigned int xgene_ahci_read_id(struct ata_device *dev,
-				       struct ata_taskfile *tf, u16 *id)
+				       struct ata_taskfile *tf, __le16 *id)
 {
 	u32 err_mask;
 
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 9c2947905d1e..67f88027680a 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -1722,7 +1722,7 @@ static u32 ata_pio_mask_no_iordy(const struct ata_device *adev)
  *	this function is wrapped or replaced by the driver
  */
 unsigned int ata_do_dev_read_id(struct ata_device *dev,
-					struct ata_taskfile *tf, u16 *id)
+				struct ata_taskfile *tf, __le16 *id)
 {
 	return ata_exec_internal(dev, tf, NULL, DMA_FROM_DEVICE,
 				     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,
 	tf.flags |= ATA_TFLAG_POLLING;
 
 	if (ap->ops->read_id)
-		err_mask = ap->ops->read_id(dev, &tf, id);
+		err_mask = ap->ops->read_id(dev, &tf, (__le16 *)id);
 	else
-		err_mask = ata_do_dev_read_id(dev, &tf, id);
+		err_mask = ata_do_dev_read_id(dev, &tf, (__le16 *)id);
 
 	if (err_mask) {
 		if (err_mask & AC_ERR_NODEV_HINT) {
diff --git a/drivers/ata/pata_it821x.c b/drivers/ata/pata_it821x.c
index b77ef0046dbe..8a5b4e0079ab 100644
--- a/drivers/ata/pata_it821x.c
+++ b/drivers/ata/pata_it821x.c
@@ -537,7 +537,7 @@ static void it821x_dev_config(struct ata_device *adev)
  */
 
 static unsigned int it821x_read_id(struct ata_device *adev,
-					struct ata_taskfile *tf, u16 *id)
+				   struct ata_taskfile *tf, __le16 *id)
 {
 	unsigned int err_mask;
 	unsigned char model_num[ATA_ID_PROD_LEN + 1];
@@ -545,21 +545,20 @@ static unsigned int it821x_read_id(struct ata_device *adev,
 	err_mask = ata_do_dev_read_id(adev, tf, id);
 	if (err_mask)
 		return err_mask;
-	ata_id_c_string(id, model_num, ATA_ID_PROD, sizeof(model_num));
+	ata_id_c_string((u16 *)id, model_num, ATA_ID_PROD, sizeof(model_num));
 
-	id[83] &= ~(1 << 12);	/* Cache flush is firmware handled */
-	id[83] &= ~(1 << 13);	/* Ditto for LBA48 flushes */
-	id[84] &= ~(1 << 6);	/* No FUA */
-	id[85] &= ~(1 << 10);	/* No HPA */
-	id[76] = 0;		/* No NCQ/AN etc */
+	id[83] &= cpu_to_le16(~(1 << 12)); /* Cache flush is firmware handled */
+	id[84] &= cpu_to_le16(~(1 << 6));  /* No FUA */
+	id[85] &= cpu_to_le16(~(1 << 10)); /* No HPA */
+	id[76] = 0;			   /* No NCQ/AN etc */
 
 	if (strstr(model_num, "Integrated Technology Express")) {
 		/* Set feature bits the firmware neglects */
-		id[49] |= 0x0300;	/* LBA, DMA */
-		id[83] &= 0x7FFF;
-		id[83] |= 0x4400;	/* Word 83 is valid and LBA48 */
-		id[86] |= 0x0400;	/* LBA48 on */
-		id[ATA_ID_MAJOR_VER] |= 0x1F;
+		id[49] |= cpu_to_le16(0x0300);	/* LBA, DMA */
+		id[83] &= cpu_to_le16(0x7FFF);
+		id[83] |= cpu_to_le16(0x4400);	/* Word 83 is valid and LBA48 */
+		id[86] |= cpu_to_le16(0x0400);	/* LBA48 on */
+		id[ATA_ID_MAJOR_VER] |= cpu_to_le16(0x1F);
 		/* Clear the serial number because it's different each boot
 		   which breaks validation on resume */
 		memset(&id[ATA_ID_SERNO], 0x20, ATA_ID_SERNO_LEN);
diff --git a/drivers/ata/pata_netcell.c b/drivers/ata/pata_netcell.c
index a7ecc1a204b5..06929e77c491 100644
--- a/drivers/ata/pata_netcell.c
+++ b/drivers/ata/pata_netcell.c
@@ -21,12 +21,13 @@
 /* No PIO or DMA methods needed for this device */
 
 static unsigned int netcell_read_id(struct ata_device *adev,
-					struct ata_taskfile *tf, u16 *id)
+				    struct ata_taskfile *tf, __le16 *id)
 {
 	unsigned int err_mask = ata_do_dev_read_id(adev, tf, id);
+
 	/* Firmware forgets to mark words 85-87 valid */
 	if (err_mask == 0)
-		id[ATA_ID_CSF_DEFAULT] |= 0x4000;
+		id[ATA_ID_CSF_DEFAULT] |= cpu_to_le16(0x4000);
 	return err_mask;
 }
 
diff --git a/include/linux/libata.h b/include/linux/libata.h
index cafe360ab3cd..27b693d87e4b 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -884,7 +884,8 @@ struct ata_port_operations {
 	void (*set_piomode)(struct ata_port *ap, struct ata_device *dev);
 	void (*set_dmamode)(struct ata_port *ap, struct ata_device *dev);
 	int  (*set_mode)(struct ata_link *link, struct ata_device **r_failed_dev);
-	unsigned int (*read_id)(struct ata_device *dev, struct ata_taskfile *tf, u16 *id);
+	unsigned int (*read_id)(struct ata_device *dev, struct ata_taskfile *tf,
+				__le16 *id);
 
 	void (*dev_config)(struct ata_device *dev);
 
@@ -1119,7 +1120,7 @@ extern void ata_id_string(const u16 *id, unsigned char *s,
 extern void ata_id_c_string(const u16 *id, unsigned char *s,
 			    unsigned int ofs, unsigned int len);
 extern unsigned int ata_do_dev_read_id(struct ata_device *dev,
-					struct ata_taskfile *tf, u16 *id);
+				       struct ata_taskfile *tf, __le16 *id);
 extern void ata_qc_complete(struct ata_queued_cmd *qc);
 extern u64 ata_qc_get_active(struct ata_port *ap);
 extern void ata_scsi_simulate(struct ata_device *dev, struct scsi_cmnd *cmd);
-- 
2.31.1


  parent reply	other threads:[~2022-01-05  5:18 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-05  5:17 [PATCH v3 00/22] Improve compile test coverage Damien Le Moal
2022-01-05  5:17 ` [PATCH v3 01/22] ata: sata_fsl: add compile test support Damien Le Moal
2022-01-05  5:17 ` [PATCH v3 02/22] ata: ahci_brcm: " Damien Le Moal
2022-01-05  5:17 ` [PATCH v3 03/22] ata: ahci_da850: " Damien Le Moal
2022-01-05  5:17 ` [PATCH v3 04/22] ata: ahci_dm816: " Damien Le Moal
2022-01-05  5:17 ` [PATCH v3 05/22] ata: ahci_st: " Damien Le Moal
2022-01-05  5:17 ` [PATCH v3 06/22] ata: ahci_mtk: " Damien Le Moal
2022-01-05  5:17 ` [PATCH v3 07/22] ata: ahci_mvebu: " Damien Le Moal
2022-01-05  5:17 ` [PATCH v3 08/22] ata: ahci_sunxi: " Damien Le Moal
2022-01-05  5:17 ` [PATCH v3 09/22] ata: ahci_tegra: " Damien Le Moal
2022-01-05  5:17 ` [PATCH v3 10/22] ata: ahci_xgene: " Damien Le Moal
2022-01-05  5:17 ` [PATCH v3 11/22] ata: ahci_seattle: " Damien Le Moal
2022-01-05  5:17 ` [PATCH v3 12/22] ata: pata_bk3710: " Damien Le Moal
2022-01-05  5:17 ` [PATCH v3 13/22] ata: pata_cs5535: " Damien Le Moal
2022-01-05  5:17 ` [PATCH v3 14/22] ata: pata_ftide010: " Damien Le Moal
2022-01-05  5:17 ` [PATCH v3 15/22] ata: pata_imx: " Damien Le Moal
2022-01-05  5:17 ` [PATCH v3 16/22] ata: pata_pxa: " Damien Le Moal
2022-01-05  5:17 ` [PATCH v3 17/22] ata: pata_legacy: " Damien Le Moal
2022-01-05  5:17 ` [PATCH v3 18/22] ata: pata_samsung_cf: " Damien Le Moal
2022-01-05  5:17 ` [PATCH v3 19/22] ata: sata_fsl: fix scsi host initialization Damien Le Moal
2022-01-05  5:17 ` [PATCH v3 20/22] ata: sata_fsl: fix cmdhdr_tbl_entry and prde struct definitions Damien Le Moal
2022-01-05  5:17 ` [PATCH v3 21/22] ata: ahci_xgene: use correct type for port mmio address Damien Le Moal
2022-01-05  5:17 ` Damien Le Moal [this message]
2022-01-05  7:31   ` [PATCH v3 22/22] ata: fix read_id() ata port operation interface Hannes Reinecke

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220105051735.1871177-23-damien.lemoal@opensource.wdc.com \
    --to=damien.lemoal@opensource.wdc.com \
    --cc=hare@suse.de \
    --cc=linux-ide@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.