On Thu, Jul 03, 2025 at 04:15:41PM +0900, Damien Le Moal wrote:
> The function ata_set_mode() is defined and used only in libata-eh.c.
> Make this function static to libata-eh.c and rename it
> ata_eh_set_mode() to make it clear where its definition is. This
> renaming allows renaming ata_do_set_mode() to the simpler name
> ata_set_mode().
"This renaming allows renaming" is slightly confusing read.
Perhaps:
... renaming ata_set_mode() to ata_eh_set_mode() allows us to
rename ata_do_set_mode() to ata_set_mode()."
However, I think this commit would be easier to review/less confusing if
it was two separate patches :)
Patch 1 that renames ata_set_mode() to ata_eh_set_mode() and makes it static.
Patch 2 that renames ata_do_set_mode() to ata_set_mode().
>
> No functional changes.
>
> Signed-off-by: Damien Le Moal <dlemoal@xxxxxxxxxx>
> ---
> drivers/ata/libata-core.c | 6 +++---
> drivers/ata/libata-eh.c | 11 ++++++-----
> drivers/ata/libata.h | 1 -
> drivers/ata/pata_optidma.c | 4 +++-
> drivers/ata/pata_pcmcia.c | 4 ++--
> drivers/ata/pata_pdc2027x.c | 2 +-
> drivers/ata/sata_sil.c | 2 +-
> include/linux/libata.h | 2 +-
> 8 files changed, 17 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index fb06aef8e0f6..3bdd65bbff49 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -3508,7 +3508,7 @@ static int ata_dev_set_mode(struct ata_device *dev)
> }
>
> /**
> - * ata_do_set_mode - Program timings and issue SET FEATURES - XFER
> + * ata_set_mode - Program timings and issue SET FEATURES - XFER
> * @link: link on which timings will be programmed
> * @r_failed_dev: out parameter for failed device
> *
> @@ -3524,7 +3524,7 @@ static int ata_dev_set_mode(struct ata_device *dev)
> * 0 on success, negative errno otherwise
> */
>
> -int ata_do_set_mode(struct ata_link *link, struct ata_device **r_failed_dev)
> +int ata_set_mode(struct ata_link *link, struct ata_device **r_failed_dev)
> {
> struct ata_port *ap = link->ap;
> struct ata_device *dev;
> @@ -3605,7 +3605,7 @@ int ata_do_set_mode(struct ata_link *link, struct ata_device **r_failed_dev)
> *r_failed_dev = dev;
> return rc;
> }
> -EXPORT_SYMBOL_GPL(ata_do_set_mode);
> +EXPORT_SYMBOL_GPL(ata_set_mode);
>
> /**
> * ata_wait_ready - wait for link to become ready
> diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
> index 0f9c30f9bc1e..e5fa61fb8a59 100644
> --- a/drivers/ata/libata-eh.c
> +++ b/drivers/ata/libata-eh.c
> @@ -3408,12 +3408,12 @@ static int ata_eh_revalidate_and_attach(struct ata_link *link,
> }
>
> /**
> - * ata_set_mode - Program timings and issue SET FEATURES - XFER
> + * ata_eh_set_mode - Program timings and issue SET FEATURES - XFER
> * @link: link on which timings will be programmed
> * @r_failed_dev: out parameter for failed device
> *
> * Set ATA device disk transfer mode (PIO3, UDMA6, etc.). If
> - * ata_set_mode() fails, pointer to the failing device is
> + * ata_eh_set_mode() fails, pointer to the failing device is
> * returned in @r_failed_dev.
> *
> * LOCKING:
> @@ -3422,7 +3422,8 @@ static int ata_eh_revalidate_and_attach(struct ata_link *link,
> * RETURNS:
> * 0 on success, negative errno otherwise
> */
> -int ata_set_mode(struct ata_link *link, struct ata_device **r_failed_dev)
> +static int ata_eh_set_mode(struct ata_link *link,
> + struct ata_device **r_failed_dev)
> {
> struct ata_port *ap = link->ap;
> struct ata_device *dev;
> @@ -3443,7 +3444,7 @@ int ata_set_mode(struct ata_link *link, struct ata_device **r_failed_dev)
> if (ap->ops->set_mode)
> rc = ap->ops->set_mode(link, r_failed_dev);
> else
> - rc = ata_do_set_mode(link, r_failed_dev);
> + rc = ata_set_mode(link, r_failed_dev);
>
> /* if transfer mode has changed, set DUBIOUS_XFER on device */
> ata_for_each_dev(dev, link, ENABLED) {
> @@ -3926,7 +3927,7 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset,
>
> /* configure transfer mode if necessary */
> if (ehc->i.flags & ATA_EHI_SETMODE) {
> - rc = ata_set_mode(link, &dev);
> + rc = ata_eh_set_mode(link, &dev);
> if (rc)
> goto rest_fail;
> ehc->i.flags &= ~ATA_EHI_SETMODE;
> diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
> index 767a61f8ff89..135e1e5ee44e 100644
> --- a/drivers/ata/libata.h
> +++ b/drivers/ata/libata.h
> @@ -177,7 +177,6 @@ extern void ata_eh_report(struct ata_port *ap);
> extern int ata_eh_reset(struct ata_link *link, int classify,
> ata_prereset_fn_t prereset, ata_reset_fn_t softreset,
> ata_reset_fn_t hardreset, ata_postreset_fn_t postreset);
> -extern int ata_set_mode(struct ata_link *link, struct ata_device **r_failed_dev);
> extern int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset,
> ata_reset_fn_t softreset, ata_reset_fn_t hardreset,
> ata_postreset_fn_t postreset,
> diff --git a/drivers/ata/pata_optidma.c b/drivers/ata/pata_optidma.c
> index dfc36b4ec9c6..cc876dc7a9d8 100644
> --- a/drivers/ata/pata_optidma.c
> +++ b/drivers/ata/pata_optidma.c
> @@ -322,7 +322,9 @@ static int optidma_set_mode(struct ata_link *link, struct ata_device **r_failed)
> u8 r;
> int nybble = 4 * ap->port_no;
> struct pci_dev *pdev = to_pci_dev(ap->host->dev);
> - int rc = ata_do_set_mode(link, r_failed);
> + int rc;
> +
> + rc = ata_set_mode(link, r_failed);
> if (rc == 0) {
> pci_read_config_byte(pdev, 0x43, &r);
>
> diff --git a/drivers/ata/pata_pcmcia.c b/drivers/ata/pata_pcmcia.c
> index 5b602206c522..cf3810933a27 100644
> --- a/drivers/ata/pata_pcmcia.c
> +++ b/drivers/ata/pata_pcmcia.c
> @@ -46,7 +46,7 @@ static int pcmcia_set_mode(struct ata_link *link, struct ata_device **r_failed_d
> struct ata_device *slave = &link->device[1];
>
> if (!ata_dev_enabled(master) || !ata_dev_enabled(slave))
> - return ata_do_set_mode(link, r_failed_dev);
> + return ata_set_mode(link, r_failed_dev);
>
> if (memcmp(master->id + ATA_ID_FW_REV, slave->id + ATA_ID_FW_REV,
> ATA_ID_FW_REV_LEN + ATA_ID_PROD_LEN) == 0) {
> @@ -58,7 +58,7 @@ static int pcmcia_set_mode(struct ata_link *link, struct ata_device **r_failed_d
> ata_dev_disable(slave);
> }
> }
> - return ata_do_set_mode(link, r_failed_dev);
> + return ata_set_mode(link, r_failed_dev);
> }
>
> /**
> diff --git a/drivers/ata/pata_pdc2027x.c b/drivers/ata/pata_pdc2027x.c
> index 6820c5597b14..a4ee3b92c9aa 100644
> --- a/drivers/ata/pata_pdc2027x.c
> +++ b/drivers/ata/pata_pdc2027x.c
> @@ -387,7 +387,7 @@ static int pdc2027x_set_mode(struct ata_link *link, struct ata_device **r_failed
> struct ata_device *dev;
> int rc;
>
> - rc = ata_do_set_mode(link, r_failed);
> + rc = ata_set_mode(link, r_failed);
> if (rc < 0)
> return rc;
>
> diff --git a/drivers/ata/sata_sil.c b/drivers/ata/sata_sil.c
> index 3a99f66198a9..1b6dc950a42a 100644
> --- a/drivers/ata/sata_sil.c
> +++ b/drivers/ata/sata_sil.c
> @@ -351,7 +351,7 @@ static int sil_set_mode(struct ata_link *link, struct ata_device **r_failed)
> u32 tmp, dev_mode[2] = { };
> int rc;
>
> - rc = ata_do_set_mode(link, r_failed);
> + rc = ata_set_mode(link, r_failed);
> if (rc)
> return rc;
>
> diff --git a/include/linux/libata.h b/include/linux/libata.h
> index 78a4addc6659..d092747be588 100644
> --- a/include/linux/libata.h
> +++ b/include/linux/libata.h
> @@ -1218,7 +1218,7 @@ extern int ata_ncq_prio_enabled(struct ata_port *ap, struct scsi_device *sdev,
> extern int ata_ncq_prio_enable(struct ata_port *ap, struct scsi_device *sdev,
> bool enable);
> extern struct ata_device *ata_dev_pair(struct ata_device *adev);
> -extern int ata_do_set_mode(struct ata_link *link, struct ata_device **r_failed_dev);
> +int ata_set_mode(struct ata_link *link, struct ata_device **r_failed_dev);
> extern void ata_scsi_port_error_handler(struct Scsi_Host *host, struct ata_port *ap);
> extern void ata_scsi_cmd_error_handler(struct Scsi_Host *host, struct ata_port *ap, struct list_head *eh_q);
>
> --
> 2.50.0
>
