Re: [PATCH v3 5/7] ata: libata-scsi: probe additional LUNs for multi-LUN ATAPI devices

[Damien Le Moal <dlemoal@kernel.org>]

On 4/27/26 4:09 AM, Phil Pemberton wrote:
> After LUN 0 is added for an ATAPI device, check its BLIST_FORCELUN
> flag.  If set, call scsi_scan_target() with SCAN_WILD_CARD to trigger
> the SCSI layer's built-in sequential LUN scan for that target only.
> This probes LUNs 1..shost->max_lun, driven by the atapi_max_lun module
> parameter from patch 1/6.  Devices without BLIST_FORCELUN (the vast

Please remove the mention of patch 1/6. Once the patches are applied, that will
not mean anything.

> majority of ATAPI devices) are left with only LUN 0 -- no sequential
> scan is triggered, so single-LUN devices like the iHAS124 DVD writer
> are completely unaffected.
> 
> Non-responding LUNs (PQ=0/PDT=0x1f) are silently skipped by
> scsi_probe_and_add_lun() when BLIST_NO_LUN_1F is set on the device
> via scsi_devinfo (see patch 4/6).

Same thing here.

> 
> Signed-off-by: Phil Pemberton <philpem@philpem.me.uk>
> ---
>  drivers/ata/libata-scsi.c | 37 ++++++++++++++++++++++++++++---------
>  1 file changed, 28 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 48c7d323d6f9..fc719e3d7d60 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -26,6 +26,7 @@
>  #include <scsi/scsi_device.h>
>  #include <scsi/scsi_tcq.h>
>  #include <scsi/scsi_transport.h>
> +#include <scsi/scsi_devinfo.h>
>  #include <linux/libata.h>
>  #include <linux/hdreg.h>
>  #include <linux/uaccess.h>
> @@ -4700,7 +4701,6 @@ void ata_scsi_scan_host(struct ata_port *ap, int sync)
>   repeat:
>  	ata_for_each_link(link, ap, EDGE) {
>  		ata_for_each_dev(dev, link, ENABLED) {
> -			struct scsi_device *sdev;
>  			int channel = 0, id = 0;
>  
>  			if (dev->sdev[0])
> @@ -4711,15 +4711,34 @@ void ata_scsi_scan_host(struct ata_port *ap, int sync)
>  			else
>  				channel = link->pmp;
>  
> -			sdev = __scsi_add_device(ap->scsi_host, channel, id, 0,
> -						 NULL);
> -			if (!IS_ERR(sdev)) {
> -				dev->sdev[0] = sdev;
> -				ata_scsi_assign_ofnode(dev, ap);
> -				scsi_device_put(sdev);
> -			} else {
> -				dev->sdev[0] = NULL;
> +			{
> +				struct scsi_device *sdev;

This is not pretty... Please leave the declaration of sdev where it was and
remove this curly bracket. That will save one level of identation and make the
code cleaner.

> +
> +				sdev = __scsi_add_device(ap->scsi_host,
> +							 channel, id, 0, NULL);
> +				if (!IS_ERR(sdev)) {
> +					/*
> +					 * For multi-LUN ATAPI (BLIST_FORCELUN),
> +					 * trigger the sequential LUN scan.
> +					 * pdt_1f_for_no_lun (set during LUN 0
> +					 * configure) ensures non-responding LUNs
> +					 * are silently skipped.  dev->sdev[] is
> +					 * populated by ata_scsi_dev_config()
> +					 * during the scan callbacks.
> +					 */
> +					if (dev->class == ATA_DEV_ATAPI &&
> +					    sdev->sdev_bflags & BLIST_FORCELUN)
> +						scsi_scan_target(
> +							&ap->scsi_host->shost_gendev,
> +							channel, id,
> +							SCAN_WILD_CARD,
> +							SCSI_SCAN_RESCAN);
> +					scsi_device_put(sdev);
> +				}
>  			}
> +
> +			if (dev->sdev[0])
> +				ata_scsi_assign_ofnode(dev, ap);
>  		}
>  	}
>  


-- 
Damien Le Moal
Western Digital Research