Re: [PATCH v2 8/8] ata: remove ata_bus_probe()

[John Garry <john.g.garry@oracle.com>]

On 20/07/2023 01:42, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@wdc.com>
> 
> Remove ata_bus_probe() as it is unused.

Is there still a reference to this in Documentation? There seems to be.

> 
> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> ---
>   drivers/ata/libata-core.c | 138 --------------------------------------
>   drivers/ata/libata.h      |   1 -
>   2 files changed, 139 deletions(-)
> 
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index dedae669c9da..0af88ef231d1 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -3057,144 +3057,6 @@ int ata_cable_sata(struct ata_port *ap)
>   }
>   EXPORT_SYMBOL_GPL(ata_cable_sata);
>   
> -/**
> - *	ata_bus_probe - Reset and probe ATA bus
> - *	@ap: Bus to probe
> - *
> - *	Master ATA bus probing function.  Initiates a hardware-dependent
> - *	bus reset, then attempts to identify any devices found on
> - *	the bus.
> - *
> - *	LOCKING:
> - *	PCI/etc. bus probe sem.
> - *
> - *	RETURNS:
> - *	Zero on success, negative errno otherwise.
> - */
> -
> -int ata_bus_probe(struct ata_port *ap)
> -{
> -	unsigned int classes[ATA_MAX_DEVICES];
> -	int tries[ATA_MAX_DEVICES];
> -	int rc;
> -	struct ata_device *dev;
> -
> -	ata_for_each_dev(dev, &ap->link, ALL)
> -		tries[dev->devno] = ATA_PROBE_MAX_TRIES;

ATA_PROBE_MAX_TRIES is no longer referenced, AFAICS

> -
> - retry:
> -	ata_for_each_dev(dev, &ap->link, ALL) {
> -		/* If we issue an SRST then an ATA drive (not ATAPI)
> -		 * may change configuration and be in PIO0 timing. If
> -		 * we do a hard reset (or are coming from power on)
> -		 * this is true for ATA or ATAPI. Until we've set a
> -		 * suitable controller mode we should not touch the
> -		 * bus as we may be talking too fast.
> -		 */
> -		dev->pio_mode = XFER_PIO_0;
> -		dev->dma_mode = 0xff;
> -
> -		/* If the controller has a pio mode setup function
> -		 * then use it to set the chipset to rights. Don't
> -		 * touch the DMA setup as that will be dealt with when
> -		 * configuring devices.
> -		 */
> -		if (ap->ops->set_piomode)
> -			ap->ops->set_piomode(ap, dev);
> -	}
> -
> -	/* reset and determine device classes */
> -	ap->ops->phy_reset(ap);

Does .phy_reset still have a user? If not, maybe all referenced symbols 
removed here can be checked for remaining references.

> -
> -	ata_for_each_dev(dev, &ap->link, ALL) {
> -		if (dev->class != ATA_DEV_UNKNOWN)
> -			classes[dev->devno] = dev->class;
> -		else
> -			classes[dev->devno] = ATA_DEV_NONE;
> -
> -		dev->class = ATA_DEV_UNKNOWN;
> -	}
> -
> -	/* read IDENTIFY page and configure devices. We have to do the identify
> -	   specific sequence bass-ackwards so that PDIAG- is released by
> -	   the slave device */
> -
> -	ata_for_each_dev(dev, &ap->link, ALL_REVERSE) {
> -		if (tries[dev->devno])
> -			dev->class = classes[dev->devno];
> -
> -		if (!ata_dev_enabled(dev))
> -			continue;
> -
> -		rc = ata_dev_read_id(dev, &dev->class, ATA_READID_POSTRESET,
> -				     dev->id);
> -		if (rc)
> -			goto fail;
> -	}
> -
> -	/* Now ask for the cable type as PDIAG- should have been released */
> -	if (ap->ops->cable_detect)
> -		ap->cbl = ap->ops->cable_detect(ap);
> -
> -	/* We may have SATA bridge glue hiding here irrespective of
> -	 * the reported cable types and sensed types.  When SATA
> -	 * drives indicate we have a bridge, we don't know which end
> -	 * of the link the bridge is which is a problem.
> -	 */
> -	ata_for_each_dev(dev, &ap->link, ENABLED)
> -		if (ata_id_is_sata(dev->id))
> -			ap->cbl = ATA_CBL_SATA;
> -
> -	/* After the identify sequence we can now set up the devices. We do
> -	   this in the normal order so that the user doesn't get confused */
> -
> -	ata_for_each_dev(dev, &ap->link, ENABLED) {
> -		ap->link.eh_context.i.flags |= ATA_EHI_PRINTINFO;
> -		rc = ata_dev_configure(dev);
> -		ap->link.eh_context.i.flags &= ~ATA_EHI_PRINTINFO;
> -		if (rc)
> -			goto fail;
> -	}
> -
> -	/* configure transfer mode */
> -	rc = ata_set_mode(&ap->link, &dev);
> -	if (rc)
> -		goto fail;
> -
> -	ata_for_each_dev(dev, &ap->link, ENABLED)
> -		return 0;
> -
> -	return -ENODEV;
> -
> - fail:
> -	tries[dev->devno]--;
> -
> -	switch (rc) {
> -	case -EINVAL:
> -		/* eeek, something went very wrong, give up */
> -		tries[dev->devno] = 0;
> -		break;
> -
> -	case -ENODEV:
> -		/* give it just one more chance */
> -		tries[dev->devno] = min(tries[dev->devno], 1);
> -		fallthrough;
> -	case -EIO:
> -		if (tries[dev->devno] == 1) {
> -			/* This is the last chance, better to slow
> -			 * down than lose it.
> -			 */
> -			sata_down_spd_limit(&ap->link, 0);
> -			ata_down_xfermask_limit(dev, ATA_DNXFER_PIO);
> -		}
> -	}
> -
> -	if (!tries[dev->devno])
> -		ata_dev_disable(dev);
> -
> -	goto retry;
> -}
> -
>   /**
>    *	sata_print_link_status - Print SATA link status
>    *	@link: SATA link to printk link status about
> diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
> index 1ec9b4427b84..6e7d352803bd 100644
> --- a/drivers/ata/libata.h
> +++ b/drivers/ata/libata.h
> @@ -122,7 +122,6 @@ extern void ata_scsi_media_change_notify(struct ata_device *dev);
>   extern void ata_scsi_hotplug(struct work_struct *work);
>   extern void ata_schedule_scsi_eh(struct Scsi_Host *shost);
>   extern void ata_scsi_dev_rescan(struct work_struct *work);
> -extern int ata_bus_probe(struct ata_port *ap);
>   extern int ata_scsi_user_scan(struct Scsi_Host *shost, unsigned int channel,
>   			      unsigned int id, u64 lun);
>   void ata_scsi_sdev_config(struct scsi_device *sdev);