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

[Hannes Reinecke]

On 4/20/26 14:23, 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/5.  Devices without BLIST_FORCELUN (the vast
> 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.
>
> To suppress spurious "No Device" log entries from non-responding LUNs
> (e.g. LUN 2+ on a two-LUN PD/CD drive), set pdt_1f_for_no_lun on the
> scsi_target during LUN 0 configuration.  The Panasonic PD-1 returns
> PQ=0/PDT=0x1f for unpopulated LUNs rather than the standard PQ=3;
> with this flag, scsi_probe_and_add_lun() silently skips them.
>
> If BLIST_FORCELUN is set but atapi_max_lun is still at its default of
> 1, an informational message points the user at the module parameter so
> the knob is discoverable from dmesg.
>
> Signed-off-by: Phil Pemberton <philpem@xxxxxxxxxxxxx>
> ---
>   drivers/ata/libata-scsi.c | 53 ++++++++++++++++++++++++++++++++-------
>   1 file changed, 44 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 4e88ae7d94c3..9d18ef2835a5 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>
> @@ -1132,6 +1133,22 @@ int ata_scsi_dev_config(struct scsi_device *sdev, struct queue_limits *lim,
>   		sdev->security_supported = 1;
>   
>   	dev->sdev[sdev->lun] = sdev;
> +
> +	/*
> +	 * Tell the SCSI scan layer that PDT 0x1f with PQ 0 means "no LUN
> +	 * present" for this target.  The Panasonic PD-1 (and likely other
> +	 * multi-LUN ATAPI devices) returns PQ=0/PDT=0x1f for unpopulated
> +	 * LUNs instead of the standard PQ=3.  Setting this flag lets the
> +	 * sequential LUN scan skip those LUNs cleanly.
> +	 */

But this would hard-code device-specifics in the generic code.

> +	if (dev->class == ATA_DEV_ATAPI && sdev->lun == 0) {
> +		sdev->sdev_target->pdt_1f_for_no_lun = 1;

Don't. That should be done with blacklist flags for specific devices.

> +
> +		if ((sdev->sdev_bflags & BLIST_FORCELUN) && atapi_max_lun < 2)
> +			ata_dev_info(dev,
> +				"device has additional LUNs; set libata.atapi_max_lun=2 or higher to access them\n");

Huh? How do you know? The device _might_ have additional luns, but
here we just probed for LUN 0, giving no indication whether other
LUNs are present.

And you already know which devices are affected (as the blacklist flags
should make that clear).
So I don't think we need that warning.

> +	}
> +
>   	return 0;
>   }
>   
> @@ -4700,7 +4717,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 +4727,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;
> +
> +				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);
>   		}
>   	}
>   

Cheers,

Hannes
--
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@xxxxxxx                                +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