[PATCH 0/5] libata cleanups

[PATCH 0/5] libata cleanups

[Damien Le Moal]

Here is a set of patches to cleanup a little libata code. Except for
patch 1, there are no functional changes.

Damien Le Moal (5):
  ata: libata-core: improve tag checks in ata_qc_issue()
  ata: libata-sata: simplify ata_sas_queuecmd()
  ata: libata-scsi: simplify ata_scsi_schedule_deferred_qc()
  ata: libata-scsi: make ata_scsi_simulate() static
  ata: libata-scsi: rename and improve ata_qc_done()

 drivers/ata/libata-core.c |   9 +-
 drivers/ata/libata-sata.c |  14 ++-
 drivers/ata/libata-scsi.c | 174 +++++++++++++++++++-------------------
 include/linux/libata.h    |   1 -
 4 files changed, 98 insertions(+), 100 deletions(-)

-- 
2.53.0

[PATCH 1/5] ata: libata-core: improve tag checks in ata_qc_issue()

[Damien Le Moal]

Make sure to check that the tag of a queued command is valid when
ata_qc_issue() is called, and fail the QC if the tag is not valid, or if
there is an on-going non-NCQ command already on the link.

Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
 drivers/ata/libata-core.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index beb6984b379a..0c42fb74eaf1 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -5146,8 +5146,13 @@ void ata_qc_issue(struct ata_queued_cmd *qc)
 	struct ata_link *link = qc->dev->link;
 	u8 prot = qc->tf.protocol;
 
-	/* Make sure only one non-NCQ command is outstanding. */
-	WARN_ON_ONCE(ata_tag_valid(link->active_tag));
+	/*
+	 * Make sure we have a valid tag and that only one non-NCQ command is
+	 * outstanding.
+	 */
+	if (WARN_ON_ONCE(!ata_tag_valid(qc->tag)) ||
+	    WARN_ON_ONCE(ata_tag_valid(link->active_tag)))
+		goto sys_err;
 
 	if (ata_is_ncq(prot)) {
 		WARN_ON_ONCE(link->sactive & (1 << qc->hw_tag));
-- 
2.53.0

Re: [PATCH 1/5] ata: libata-core: improve tag checks in ata_qc_issue()

[Hannes Reinecke]

On 2/19/26 07:23, Damien Le Moal wrote:
> Make sure to check that the tag of a queued command is valid when
> ata_qc_issue() is called, and fail the QC if the tag is not valid, or if
> there is an on-going non-NCQ command already on the link.
> 
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> ---
>   drivers/ata/libata-core.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

[PATCH 2/5] ata: libata-sata: simplify ata_sas_queuecmd()

[Damien Le Moal]

Change ata_sas_queuecmd() to return early the result of
__ata_scsi_queuecmd() and remove the rc local variable.
This simplifies the code without any functional change.

Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
 drivers/ata/libata-sata.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c
index 04e1e774645e..00f7e5443e3d 100644
--- a/drivers/ata/libata-sata.c
+++ b/drivers/ata/libata-sata.c
@@ -1378,15 +1378,13 @@ EXPORT_SYMBOL_GPL(ata_sas_sdev_configure);
 
 int ata_sas_queuecmd(struct scsi_cmnd *cmd, struct ata_port *ap)
 {
-	int rc = 0;
-
 	if (likely(ata_dev_enabled(ap->link.device)))
-		rc = __ata_scsi_queuecmd(cmd, ap->link.device);
-	else {
-		cmd->result = (DID_BAD_TARGET << 16);
-		scsi_done(cmd);
-	}
-	return rc;
+		return __ata_scsi_queuecmd(cmd, ap->link.device);
+
+	cmd->result = (DID_BAD_TARGET << 16);
+	scsi_done(cmd);
+
+	return 0;
 }
 EXPORT_SYMBOL_GPL(ata_sas_queuecmd);
 
-- 
2.53.0

Re: [PATCH 2/5] ata: libata-sata: simplify ata_sas_queuecmd()

[Hannes Reinecke]

On 2/19/26 07:23, Damien Le Moal wrote:
> Change ata_sas_queuecmd() to return early the result of
> __ata_scsi_queuecmd() and remove the rc local variable.
> This simplifies the code without any functional change.
> 
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> ---
>   drivers/ata/libata-sata.c | 14 ++++++--------
>   1 file changed, 6 insertions(+), 8 deletions(-)
> 
Reviewed-by: Hannes Reineke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

[PATCH 3/5] ata: libata-scsi: simplify ata_scsi_schedule_deferred_qc()

[Damien Le Moal]

Use ata_qc_done() instead of calling ata_qc_free() and scsi_done()
directly.

Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
 drivers/ata/libata-scsi.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index c0dd75a0287c..41918e21d0f8 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1685,7 +1685,6 @@ void ata_scsi_deferred_qc_work(struct work_struct *work)
 void ata_scsi_requeue_deferred_qc(struct ata_port *ap)
 {
 	struct ata_queued_cmd *qc = ap->deferred_qc;
-	struct scsi_cmnd *scmd;
 
 	lockdep_assert_held(ap->lock);
 
@@ -1697,11 +1696,9 @@ void ata_scsi_requeue_deferred_qc(struct ata_port *ap)
 	if (!qc)
 		return;
 
-	scmd = qc->scsicmd;
 	ap->deferred_qc = NULL;
-	ata_qc_free(qc);
-	scmd->result = (DID_SOFT_ERROR << 16);
-	scsi_done(scmd);
+	qc->scsicmd->result = (DID_SOFT_ERROR << 16);
+	ata_qc_done(qc);
 }
 
 static void ata_scsi_schedule_deferred_qc(struct ata_port *ap)
-- 
2.53.0

Re: [PATCH 3/5] ata: libata-scsi: simplify ata_scsi_schedule_deferred_qc()

[Sergey Shtylyov]

On 2/19/26 9:23 AM, Damien Le Moal wrote:

> Use ata_qc_done() instead of calling ata_qc_free() and scsi_done()
> directly.
> 
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> ---
>  drivers/ata/libata-scsi.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index c0dd75a0287c..41918e21d0f8 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -1685,7 +1685,6 @@ void ata_scsi_deferred_qc_work(struct work_struct *work)
>  void ata_scsi_requeue_deferred_qc(struct ata_port *ap)

   You're simplifying ata_scsi_requeue_deferred_qc() but the subject has
ata_scsi_schedule_deferred_qc()...

>  {
>  	struct ata_queued_cmd *qc = ap->deferred_qc;
> -	struct scsi_cmnd *scmd;
>  
>  	lockdep_assert_held(ap->lock);
>  
> @@ -1697,11 +1696,9 @@ void ata_scsi_requeue_deferred_qc(struct ata_port *ap)
>  	if (!qc)
>  		return;
>  
> -	scmd = qc->scsicmd;
>  	ap->deferred_qc = NULL;
> -	ata_qc_free(qc);
> -	scmd->result = (DID_SOFT_ERROR << 16);
> -	scsi_done(scmd);
> +	qc->scsicmd->result = (DID_SOFT_ERROR << 16);
> +	ata_qc_done(qc);
>  }
>  
>  static void ata_scsi_schedule_deferred_qc(struct ata_port *ap)

MBR, Sergey

Re: [PATCH 3/5] ata: libata-scsi: simplify ata_scsi_schedule_deferred_qc()

[Hannes Reinecke]

On 2/19/26 07:23, Damien Le Moal wrote:
> Use ata_qc_done() instead of calling ata_qc_free() and scsi_done()
> directly.
> 
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> ---
>   drivers/ata/libata-scsi.c | 7 ++-----
>   1 file changed, 2 insertions(+), 5 deletions(-)
> 
As noted by Sergey, the description refers to the wrong
function.
Other than that:

Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

Re: [PATCH 3/5] ata: libata-scsi: simplify ata_scsi_schedule_deferred_qc()

[Damien Le Moal]

On 2/19/26 17:21, Hannes Reinecke wrote:
> On 2/19/26 07:23, Damien Le Moal wrote:
>> Use ata_qc_done() instead of calling ata_qc_free() and scsi_done()
>> directly.
>>
>> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
>> ---
>>   drivers/ata/libata-scsi.c | 7 ++-----
>>   1 file changed, 2 insertions(+), 5 deletions(-)
>>
> As noted by Sergey, the description refers to the wrong
> function.

Oops. Corrected.

> Other than that:
> 
> Reviewed-by: Hannes Reinecke <hare@suse.de>

Thanks.

> 
> Cheers,
> 
> Hannes


-- 
Damien Le Moal
Western Digital Research

[PATCH 4/5] ata: libata-scsi: make ata_scsi_simulate() static

[Damien Le Moal]

ata_scsi_simulate() is called only from libata-scsi.c. Move this
function definition as a static function before its call in
__ata_scsi_queuecmd() and remove its declaration from
include/linux/libata.h.

No functional changes.

Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
 drivers/ata/libata-scsi.c | 147 +++++++++++++++++++-------------------
 include/linux/libata.h    |   1 -
 2 files changed, 73 insertions(+), 75 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 41918e21d0f8..ad628b398fc3 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -4420,6 +4420,79 @@ static inline ata_xlat_func_t ata_get_xlat_func(struct ata_device *dev, u8 cmd)
 	return NULL;
 }
 
+/**
+ *	ata_scsi_simulate - simulate SCSI command on ATA device
+ *	@dev: the target device
+ *	@cmd: SCSI command being sent to device.
+ *
+ *	Interprets and directly executes a select list of SCSI commands
+ *	that can be handled internally.
+ *
+ *	LOCKING:
+ *	spin_lock_irqsave(host lock)
+ */
+static void ata_scsi_simulate(struct ata_device *dev, struct scsi_cmnd *cmd)
+{
+	const u8 *scsicmd = cmd->cmnd;
+	u8 tmp8;
+
+	switch (scsicmd[0]) {
+	case INQUIRY:
+		ata_scsi_rbuf_fill(dev, cmd, ata_scsiop_inquiry);
+		break;
+
+	case MODE_SENSE:
+	case MODE_SENSE_10:
+		ata_scsi_rbuf_fill(dev, cmd, ata_scsiop_mode_sense);
+		break;
+
+	case READ_CAPACITY:
+	case SERVICE_ACTION_IN_16:
+		ata_scsi_rbuf_fill(dev, cmd, ata_scsiop_read_cap);
+		break;
+
+	case REPORT_LUNS:
+		ata_scsi_rbuf_fill(dev, cmd, ata_scsiop_report_luns);
+		break;
+
+	case REQUEST_SENSE:
+		ata_scsi_set_sense(dev, cmd, 0, 0, 0);
+		break;
+
+	/* if we reach this, then writeback caching is disabled,
+	 * turning this into a no-op.
+	 */
+	case SYNCHRONIZE_CACHE:
+	case SYNCHRONIZE_CACHE_16:
+		fallthrough;
+
+	/* no-op's, complete with success */
+	case REZERO_UNIT:
+	case SEEK_6:
+	case SEEK_10:
+	case TEST_UNIT_READY:
+		break;
+
+	case SEND_DIAGNOSTIC:
+		tmp8 = scsicmd[1] & ~(1 << 3);
+		if (tmp8 != 0x4 || scsicmd[3] || scsicmd[4])
+			ata_scsi_set_invalid_field(dev, cmd, 1, 0xff);
+		break;
+
+	case MAINTENANCE_IN:
+		ata_scsi_rbuf_fill(dev, cmd, ata_scsiop_maint_in);
+		break;
+
+	/* all other commands */
+	default:
+		ata_scsi_set_sense(dev, cmd, ILLEGAL_REQUEST, 0x20, 0x0);
+		/* "Invalid command operation code" */
+		break;
+	}
+
+	scsi_done(cmd);
+}
+
 enum scsi_qc_status __ata_scsi_queuecmd(struct scsi_cmnd *scmd,
 					struct ata_device *dev)
 {
@@ -4522,80 +4595,6 @@ enum scsi_qc_status ata_scsi_queuecmd(struct Scsi_Host *shost,
 }
 EXPORT_SYMBOL_GPL(ata_scsi_queuecmd);
 
-/**
- *	ata_scsi_simulate - simulate SCSI command on ATA device
- *	@dev: the target device
- *	@cmd: SCSI command being sent to device.
- *
- *	Interprets and directly executes a select list of SCSI commands
- *	that can be handled internally.
- *
- *	LOCKING:
- *	spin_lock_irqsave(host lock)
- */
-
-void ata_scsi_simulate(struct ata_device *dev, struct scsi_cmnd *cmd)
-{
-	const u8 *scsicmd = cmd->cmnd;
-	u8 tmp8;
-
-	switch(scsicmd[0]) {
-	case INQUIRY:
-		ata_scsi_rbuf_fill(dev, cmd, ata_scsiop_inquiry);
-		break;
-
-	case MODE_SENSE:
-	case MODE_SENSE_10:
-		ata_scsi_rbuf_fill(dev, cmd, ata_scsiop_mode_sense);
-		break;
-
-	case READ_CAPACITY:
-	case SERVICE_ACTION_IN_16:
-		ata_scsi_rbuf_fill(dev, cmd, ata_scsiop_read_cap);
-		break;
-
-	case REPORT_LUNS:
-		ata_scsi_rbuf_fill(dev, cmd, ata_scsiop_report_luns);
-		break;
-
-	case REQUEST_SENSE:
-		ata_scsi_set_sense(dev, cmd, 0, 0, 0);
-		break;
-
-	/* if we reach this, then writeback caching is disabled,
-	 * turning this into a no-op.
-	 */
-	case SYNCHRONIZE_CACHE:
-	case SYNCHRONIZE_CACHE_16:
-		fallthrough;
-
-	/* no-op's, complete with success */
-	case REZERO_UNIT:
-	case SEEK_6:
-	case SEEK_10:
-	case TEST_UNIT_READY:
-		break;
-
-	case SEND_DIAGNOSTIC:
-		tmp8 = scsicmd[1] & ~(1 << 3);
-		if (tmp8 != 0x4 || scsicmd[3] || scsicmd[4])
-			ata_scsi_set_invalid_field(dev, cmd, 1, 0xff);
-		break;
-
-	case MAINTENANCE_IN:
-		ata_scsi_rbuf_fill(dev, cmd, ata_scsiop_maint_in);
-		break;
-
-	/* all other commands */
-	default:
-		ata_scsi_set_sense(dev, cmd, ILLEGAL_REQUEST, 0x20, 0x0);
-		/* "Invalid command operation code" */
-		break;
-	}
-
-	scsi_done(cmd);
-}
-
 int ata_scsi_add_hosts(struct ata_host *host, const struct scsi_host_template *sht)
 {
 	int i, rc;
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 00346ce3af5e..db87c99e4189 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -1205,7 +1205,6 @@ extern unsigned int ata_do_dev_read_id(struct ata_device *dev,
 				       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);
 extern int ata_std_bios_param(struct scsi_device *sdev,
 			      struct gendisk *unused,
 			      sector_t capacity, int geom[]);
-- 
2.53.0

Re: [PATCH 4/5] ata: libata-scsi: make ata_scsi_simulate() static

[Hannes Reinecke]

On 2/19/26 07:23, Damien Le Moal wrote:
> ata_scsi_simulate() is called only from libata-scsi.c. Move this
> function definition as a static function before its call in
> __ata_scsi_queuecmd() and remove its declaration from
> include/linux/libata.h.
> 
> No functional changes.
> 
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> ---
>   drivers/ata/libata-scsi.c | 147 +++++++++++++++++++-------------------
>   include/linux/libata.h    |   1 -
>   2 files changed, 73 insertions(+), 75 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

[PATCH 5/5] ata: libata-scsi: rename and improve ata_qc_done()

[Damien Le Moal]

Rename ata_qc_done() to ata_scsi_qc_done() and allow to pass a scsi
command result value to set for the completed command to simplify the
caller sites.

No functional changes.

Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
---
 drivers/ata/libata-scsi.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index ad628b398fc3..4225c6d7ff35 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1649,12 +1649,16 @@ static unsigned int ata_scsi_rw_xlat(struct ata_queued_cmd *qc)
 	return 1;
 }
 
-static void ata_qc_done(struct ata_queued_cmd *qc)
+static void ata_scsi_qc_done(struct ata_queued_cmd *qc, bool set_result,
+			     u32 scmd_result)
 {
 	struct scsi_cmnd *cmd = qc->scsicmd;
 	void (*done)(struct scsi_cmnd *) = qc->scsidone;
 
 	ata_qc_free(qc);
+
+	if (set_result)
+		cmd->result = scmd_result;
 	done(cmd);
 }
 
@@ -1693,12 +1697,10 @@ void ata_scsi_requeue_deferred_qc(struct ata_port *ap)
 	 * do not try to be smart about what to do with this deferred command
 	 * and simply retry it by completing it with DID_SOFT_ERROR.
 	 */
-	if (!qc)
-		return;
-
-	ap->deferred_qc = NULL;
-	qc->scsicmd->result = (DID_SOFT_ERROR << 16);
-	ata_qc_done(qc);
+	if (qc) {
+		ap->deferred_qc = NULL;
+		ata_scsi_qc_done(qc, true, DID_SOFT_ERROR << 16);
+	}
 }
 
 static void ata_scsi_schedule_deferred_qc(struct ata_port *ap)
@@ -1754,7 +1756,7 @@ static void ata_scsi_qc_complete(struct ata_queued_cmd *qc)
 		ata_scsi_set_sense_information(qc);
 	}
 
-	ata_qc_done(qc);
+	ata_scsi_qc_done(qc, false, 0);
 
 	ata_scsi_schedule_deferred_qc(ap);
 }
@@ -2913,17 +2915,15 @@ static void atapi_qc_complete(struct ata_queued_cmd *qc)
 		if (qc->cdb[0] == ALLOW_MEDIUM_REMOVAL && qc->dev->sdev)
 			qc->dev->sdev->locked = 0;
 
-		qc->scsicmd->result = SAM_STAT_CHECK_CONDITION;
-		ata_qc_done(qc);
+		ata_scsi_qc_done(qc, true, SAM_STAT_CHECK_CONDITION);
 		return;
 	}
 
 	/* successful completion path */
 	if (cmd->cmnd[0] == INQUIRY && (cmd->cmnd[1] & 0x03) == 0)
 		atapi_fixup_inquiry(cmd);
-	cmd->result = SAM_STAT_GOOD;
 
-	ata_qc_done(qc);
+	ata_scsi_qc_done(qc, true, SAM_STAT_GOOD);
 }
 /**
  *	atapi_xlat - Initialize PACKET taskfile
-- 
2.53.0

Re: [PATCH 5/5] ata: libata-scsi: rename and improve ata_qc_done()

[Hannes Reinecke]

On 2/19/26 07:23, Damien Le Moal wrote:
> Rename ata_qc_done() to ata_scsi_qc_done() and allow to pass a scsi
> command result value to set for the completed command to simplify the
> caller sites.
> 
> No functional changes.
> 
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> ---
>   drivers/ata/libata-scsi.c | 24 ++++++++++++------------
>   1 file changed, 12 insertions(+), 12 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

Re: [PATCH 0/5] libata cleanups

[Damien Le Moal]

On 2/19/26 3:23 PM, Damien Le Moal wrote:
> Here is a set of patches to cleanup a little libata code. Except for
> patch 1, there are no functional changes.
> 
> Damien Le Moal (5):
>   ata: libata-core: improve tag checks in ata_qc_issue()
>   ata: libata-sata: simplify ata_sas_queuecmd()
>   ata: libata-scsi: simplify ata_scsi_schedule_deferred_qc()
>   ata: libata-scsi: make ata_scsi_simulate() static
>   ata: libata-scsi: rename and improve ata_qc_done()
> 
>  drivers/ata/libata-core.c |   9 +-
>  drivers/ata/libata-sata.c |  14 ++-
>  drivers/ata/libata-scsi.c | 174 +++++++++++++++++++-------------------
>  include/linux/libata.h    |   1 -
>  4 files changed, 98 insertions(+), 100 deletions(-)

Applied to for-7.1.


-- 
Damien Le Moal
Western Digital Research