[RFC PATCH 0/3] ata: libata-scsi: add multi-LUN support for ATAPI devices

[RFC PATCH 0/3] ata: libata-scsi: add multi-LUN support for ATAPI devices

[Phil Pemberton]

Hi all,

This series gives libata support for ATAPI devices with multiple LUNs,
such as the Panasonic PD-1 PD/CD combo drive. This exposes both the
CD-ROM and rewritable PD optical interfaces: CD-ROM as LUN 0 and PD
as LUN 1.

libata has never supported multi-LUN ATAPI. This series adds support
by fixing the following limitations:

  1. shost->max_lun is hardcoded to 1 in ata_scsi_add_hosts(), preventing
     the SCSI layer from probing any LUN beyond 0.

  2. __ata_scsi_find_dev() rejects all commands where scsidev->lun != 0,
     returning NULL and resulting in DID_BAD_TARGET.

  3. The SCSI-2 CDB LUN field (byte 1, bits 7:5) is never populated.
     ATAPI tunnels SCSI commands over the ATA PACKET interface, and the
     transport-layer LUN addressing used by SPC-3+ is not available.
     Older multi-LUN ATAPI devices rely on this CDB field to route
     commands to the correct LUN.

  4. ata_scsi_scan_host() only calls __scsi_add_device() for LUN 0,
     never probing additional LUNs even when the SCSI device info table
     would indicate the device supports them.

  5. ata_scsi_dev_config() assigned dev->sdev for every LUN, and
     ata_scsi_sdev_destroy() tore down the entire ATA device whenever
     any sdev was destroyed -- so removing a spurious LUN result during
     scanning would kill the whole port.

The series is split as:

  1/3: scsi_devinfo: add the COMPAQ-branded variant of the PD-1 to the
       device info table.  An entry already exists for the Panasonic
       OEM-branded "MATSHITA PD-1" and the NEC "NEC PD-1 ODX654P".

  2/3: libata-scsi: raise max_lun, route non-zero LUNs to the same
       ata_device for ATAPI, encode the LUN in CDB byte 1, and fix the
       slave_configure/slave_destroy callbacks to track LUN 0 only.

  3/3: libata-scsi: probe additional LUNs in ata_scsi_scan_host() for
       ATAPI devices flagged BLIST_FORCELUN, stopping at PDT 0x1f.

Tested on a Panasonic LF-1195C PD/CD (Compaq branded) attached to an
ata_piix host on i686. The CD-ROM enumerates as an 'sr' device and
the PD side enumerates as an 'sd' block device. Both LUNs work for I/O:
the CD reads correctly, and the PD drive can be partitioned, formatted
and mounted. An iHAS124 DVD writer on the same machine (single-LUN, no
BLIST_FORCELUN entry) is unaffected: it does not enter the multi-LUN
probing path.

If the iHAS124 is scanned regardless, it seems to ignore the LUN
parameter, and enumerates as eight drives. I expect most standard ATAPI
devices would behave this way, hence the BLIST_FORCELUN gate.

The "PDT 0x1f" check is based on the behaviour of the Panasonic PD/CD
drive: when an unrecognised LUN is probed, it responds with SCSI
peripheral device type 0x1F (no device).

All testing was done on kernel 7.0.0-rc7+.

This is marked RFC because:

  - The CDB-byte-1 LUN encoding is a SCSI-2-era convention that may
    affect well-behaved modern devices in unexpected ways.  It is
    currently applied to all non-zero LUN commands on ATAPI devices;
    a more conservative approach would gate it on a quirk flag.

  - The blast radius of changing __ata_scsi_find_dev() and the slave
    callbacks deserves wider review, even though all changes are
    conditional on dev->class == ATA_DEV_ATAPI or sdev->lun == 0.

  - I do not have other multi-LUN ATAPI hardware to test against.

Comments and suggestions welcome.

Phil Pemberton (3):
  scsi: scsi_devinfo: add COMPAQ PD-1 multi-LUN ATAPI device quirk
  ata: libata-scsi: enable multi-LUN support for ATAPI devices
  ata: libata-scsi: probe additional LUNs for multi-LUN ATAPI devices

 drivers/ata/libata-scsi.c   | 67 ++++++++++++++++++++++++++++++++++---
 drivers/scsi/scsi_devinfo.c |  1 +
 2 files changed, 64 insertions(+), 4 deletions(-)

--
2.39.5

[PATCH 1/3] scsi: scsi_devinfo: add COMPAQ PD-1 multi-LUN ATAPI device quirk

[Phil Pemberton]

The Compaq-branded variant of the Panasonic/Matsushita LF-1195C PD/CD
combo drive is a multi-LUN ATAPI device that exposes the CD-ROM on
LUN 0 and a PD optical drive on LUN 1.

An entry already exists for the "MATSHITA PD-1" variant with OEM
firmware.

Add the Compaq-branded variant with the same BLIST_FORCELUN |
BLIST_SINGLELUN flags so that additional LUNs are scanned and commands
are serialised, matching the existing entries.

Assisted-by: Claude:claude-opus-4-6
Signed-off-by: Phil Pemberton <philpem@philpem.me.uk>
---
 drivers/scsi/scsi_devinfo.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c
index 68a992494b12..06b211b93567 100644
--- a/drivers/scsi/scsi_devinfo.c
+++ b/drivers/scsi/scsi_devinfo.c
@@ -150,6 +150,7 @@ static struct {
 	{"COMPAQ", "MSA1000", NULL, BLIST_SPARSELUN | BLIST_NOSTARTONADD},
 	{"COMPAQ", "MSA1000 VOLUME", NULL, BLIST_SPARSELUN | BLIST_NOSTARTONADD},
 	{"COMPAQ", "HSV110", NULL, BLIST_REPORTLUN2 | BLIST_NOSTARTONADD},
+	{"COMPAQ", "PD-1", NULL, BLIST_FORCELUN | BLIST_SINGLELUN},
 	{"DDN", "SAN DataDirector", "*", BLIST_SPARSELUN},
 	{"DEC", "HSG80", NULL, BLIST_REPORTLUN2 | BLIST_NOSTARTONADD},
 	{"DELL", "PV660F", NULL, BLIST_SPARSELUN},
-- 
2.39.5

[PATCH 2/3] ata: libata-scsi: enable multi-LUN support for ATAPI devices

[Phil Pemberton]

libata has never supported multi-LUN ATAPI devices like the Panasonic
and NEC PD/CD combo drives due to three limitations:

  - shost->max_lun is hardcoded to 1 in ata_scsi_add_hosts(), which
    stops the SCSI layer from probing any LUN other than 0.

  - __ata_scsi_find_dev() rejects all commands where scsidev->lun != 0,
    returning NULL which causes DID_BAD_TARGET.

  - The SCSI-2 CDB LUN field (byte 1, bits 7:5) is never set. Older
    multi-LUN ATAPI devices rely on this field to route commands to the
    correct LUN, as transport-layer LUN addressing (per SPC-3+) is not
    available over the ATA PACKET interface.

To fix all three, this change:

  - Raises max_lun from 1 to 8 (matching the SCSI host default).
    Sequential LUN scanning stops at the first non-responding LUN, so
    single-LUN devices are unaffected.

  - In __ata_scsi_find_dev(), allow non-zero LUNs for ATAPI devices by
    routing them to the same ata_device as LUN 0.

  - In atapi_xlat(), encode the target LUN into CDB byte 1 bits 7:5
    before passing the command packet to the device.

These changes are prerequisites for probing additional LUNs during
host scanning, which is done in a subsequent patch.

Additionally, fix two related issues exposed by multi-LUN scanning:

  - ata_scsi_dev_config() previously assigned dev->sdev = sdev for every
    LUN configured.  With multiple LUNs sharing one ata_device, this
    caused dev->sdev to be overwritten by each non-LUN-0 sdev.  Restrict
    the assignment to LUN 0 so that dev->sdev always tracks the
    canonical scsi_device for the underlying ATA device.

  - ata_scsi_sdev_destroy() detached the entire ATA device whenever
    dev->sdev was non-NULL.  When a spurious multi-LUN scan result was
    removed, this incorrectly tore down the underlying device.  Detach
    only when the canonical (LUN 0) sdev is being destroyed.

Assisted-by: Claude:claude-opus-4-6
Signed-off-by: Phil Pemberton <philpem@philpem.me.uk>
---
 drivers/ata/libata-scsi.c | 38 ++++++++++++++++++++++++++++++++++----
 1 file changed, 34 insertions(+), 4 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 3b65df914ebb..dc6829e60fb3 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -25,6 +25,7 @@
 #include <scsi/scsi_eh.h>
 #include <scsi/scsi_device.h>
 #include <scsi/scsi_tcq.h>
+#include <scsi/scsi_devinfo.h>
 #include <scsi/scsi_transport.h>
 #include <linux/libata.h>
 #include <linux/hdreg.h>
@@ -1131,7 +1132,14 @@ int ata_scsi_dev_config(struct scsi_device *sdev, struct queue_limits *lim,
 	if (dev->flags & ATA_DFLAG_TRUSTED)
 		sdev->security_supported = 1;
 
-	dev->sdev = sdev;
+	/*
+	 * Only LUN 0 is treated as the canonical scsi_device for the ATA
+	 * device.  Multi-LUN ATAPI devices share a single ata_device, so
+	 * dev->sdev must continue to track LUN 0 even when additional LUNs
+	 * are added or removed.
+	 */
+	if (sdev->lun == 0)
+		dev->sdev = sdev;
 	return 0;
 }
 
@@ -1220,7 +1228,12 @@ void ata_scsi_sdev_destroy(struct scsi_device *sdev)
 
 	spin_lock_irqsave(ap->lock, flags);
 	dev = __ata_scsi_find_dev(ap, sdev);
-	if (dev && dev->sdev) {
+	/*
+	 * Only detach when the canonical (LUN 0) scsi_device is going away.
+	 * Removing a non-LUN-0 sdev (e.g. a spurious multi-LUN scan result)
+	 * must not tear down the underlying ATA device.
+	 */
+	if (dev && dev->sdev == sdev) {
 		/* SCSI device already in CANCEL state, no need to offline it */
 		dev->sdev = NULL;
 		dev->flags |= ATA_DFLAG_DETACH;
@@ -2950,6 +2963,15 @@ static unsigned int atapi_xlat(struct ata_queued_cmd *qc)
 	memset(qc->cdb, 0, dev->cdb_len);
 	memcpy(qc->cdb, scmd->cmnd, scmd->cmd_len);
 
+	/*
+	 * Encode LUN in CDB byte 1 bits 7:5 for multi-LUN ATAPI devices
+	 * that use the SCSI-2 CDB LUN convention (e.g. Panasonic PD/CD
+	 * combo drives).
+	 */
+	if (scmd->device->lun)
+		qc->cdb[1] = (qc->cdb[1] & 0x1f) |
+			      ((scmd->device->lun & 0x7) << 5);
+
 	qc->complete_fn = atapi_qc_complete;
 
 	qc->tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
@@ -3062,9 +3084,17 @@ static struct ata_device *__ata_scsi_find_dev(struct ata_port *ap,
 
 	/* skip commands not addressed to targets we simulate */
 	if (!sata_pmp_attached(ap)) {
-		if (unlikely(scsidev->channel || scsidev->lun))
+		if (unlikely(scsidev->channel))
 			return NULL;
 		devno = scsidev->id;
+		/* Allow non-zero LUNs for ATAPI devices (e.g. PD/CD combos) */
+		if (unlikely(scsidev->lun)) {
+			struct ata_device *dev = ata_find_dev(ap, devno);
+
+			if (!dev || dev->class != ATA_DEV_ATAPI)
+				return NULL;
+			return dev;
+		}
 	} else {
 		if (unlikely(scsidev->id || scsidev->lun))
 			return NULL;
@@ -4620,7 +4650,7 @@ int ata_scsi_add_hosts(struct ata_host *host, const struct scsi_host_template *s
 		shost->transportt = ata_scsi_transport_template;
 		shost->unique_id = ap->print_id;
 		shost->max_id = 16;
-		shost->max_lun = 1;
+		shost->max_lun = 8;
 		shost->max_channel = 1;
 		shost->max_cmd_len = 32;
 
-- 
2.39.5

[PATCH 3/3] ata: libata-scsi: probe additional LUNs for multi-LUN ATAPI devices

[Phil Pemberton]

Some ATAPI devices (e.g. the Panasonic PD/CD combo drives) implement
multiple logical units. For instance the aforementioned PD/CD has a CD
drive on LUN 0 and the rewritable Phase-change Dual (PD) block device on
LUN 1.

ata_scsi_scan_host() previously only scanned LUN 0 via
__scsi_add_device(). With the multi-LUN work now in place, extend this
scan to probe for additional LUNs on devices which have BLIST_FORCELUN
set in the SCSI device info table.

Scanning stops when __scsi_add_device() fails, or the device reports
device type 0x1f (unknown or no device type). The PD drive returns this
for unimplemented LUNs.

The aforementioned BLIST_FORCELUN guard prevents non-multilun devices
from being affected.

Tested with a Panasonic LF-1195C PD/CD with Compaq firmware, which now
correctly enumerates as a CD drive (sr) and PD optical drive (sd).

Also tested with a LITE-ON iHAS124 DVD drive, which has a single LUN and
ignores the LUN parameter in the CDB. As a result, without the
BLIST_FORCELUN guard, this drive would pop up as eight separate devices.

Assisted-by: Claude:claude-opus-4-6
Signed-off-by: Phil Pemberton <philpem@philpem.me.uk>
---
 drivers/ata/libata-scsi.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index dc6829e60fb3..0a7ce44118fe 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -4732,6 +4732,35 @@ void ata_scsi_scan_host(struct ata_port *ap, int sync)
 			if (!IS_ERR(sdev)) {
 				dev->sdev = sdev;
 				ata_scsi_assign_ofnode(dev, ap);
+				/*
+				 * Multi-LUN ATAPI devices (e.g. PD/CD combo
+				 * drives) are flagged BLIST_FORCELUN in
+				 * scsi_devinfo.  Probe additional LUNs when
+				 * the flag is set.
+				 */
+				if (dev->class == ATA_DEV_ATAPI &&
+				    (sdev->sdev_bflags & BLIST_FORCELUN)) {
+					u64 lun;
+
+					for (lun = 1; lun < ap->scsi_host->max_lun;
+					     lun++) {
+						struct scsi_device *extra;
+
+						extra = __scsi_add_device(
+							ap->scsi_host,
+							channel, id, lun,
+							NULL);
+						if (IS_ERR(extra))
+							break;
+						/* PDT 0x1f: no device type */
+						if (extra->type == 0x1f) {
+							scsi_remove_device(extra);
+							scsi_device_put(extra);
+							break;
+						}
+						scsi_device_put(extra);
+					}
+				}
 				scsi_device_put(sdev);
 			} else {
 				dev->sdev = NULL;
-- 
2.39.5

Re: [PATCH 1/3] scsi: scsi_devinfo: add COMPAQ PD-1 multi-LUN ATAPI device quirk

[Damien Le Moal]

On 4/9/26 23:05, Phil Pemberton wrote:
> The Compaq-branded variant of the Panasonic/Matsushita LF-1195C PD/CD
> combo drive is a multi-LUN ATAPI device that exposes the CD-ROM on
> LUN 0 and a PD optical drive on LUN 1.
> 
> An entry already exists for the "MATSHITA PD-1" variant with OEM
> firmware.
> 
> Add the Compaq-branded variant with the same BLIST_FORCELUN |
> BLIST_SINGLELUN flags so that additional LUNs are scanned and commands
> are serialised, matching the existing entries.

Hmmm, since the following patches actually enable multi-LUN scanning, it feels
like this patch should come last in the series.

> 
> Assisted-by: Claude:claude-opus-4-6

Let's stay neutral vis-a-vis commercial software/tools and drop this please.

> Signed-off-by: Phil Pemberton <philpem@philpem.me.uk>
> ---
>  drivers/scsi/scsi_devinfo.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c
> index 68a992494b12..06b211b93567 100644
> --- a/drivers/scsi/scsi_devinfo.c
> +++ b/drivers/scsi/scsi_devinfo.c
> @@ -150,6 +150,7 @@ static struct {
>  	{"COMPAQ", "MSA1000", NULL, BLIST_SPARSELUN | BLIST_NOSTARTONADD},
>  	{"COMPAQ", "MSA1000 VOLUME", NULL, BLIST_SPARSELUN | BLIST_NOSTARTONADD},
>  	{"COMPAQ", "HSV110", NULL, BLIST_REPORTLUN2 | BLIST_NOSTARTONADD},
> +	{"COMPAQ", "PD-1", NULL, BLIST_FORCELUN | BLIST_SINGLELUN},
>  	{"DDN", "SAN DataDirector", "*", BLIST_SPARSELUN},
>  	{"DEC", "HSG80", NULL, BLIST_REPORTLUN2 | BLIST_NOSTARTONADD},
>  	{"DELL", "PV660F", NULL, BLIST_SPARSELUN},


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 2/3] ata: libata-scsi: enable multi-LUN support for ATAPI devices

[Damien Le Moal]

On 4/9/26 23:05, Phil Pemberton wrote:
> libata has never supported multi-LUN ATAPI devices like the Panasonic
> and NEC PD/CD combo drives due to three limitations:
> 
>   - shost->max_lun is hardcoded to 1 in ata_scsi_add_hosts(), which
>     stops the SCSI layer from probing any LUN other than 0.
> 
>   - __ata_scsi_find_dev() rejects all commands where scsidev->lun != 0,
>     returning NULL which causes DID_BAD_TARGET.
> 
>   - The SCSI-2 CDB LUN field (byte 1, bits 7:5) is never set. Older
>     multi-LUN ATAPI devices rely on this field to route commands to the
>     correct LUN, as transport-layer LUN addressing (per SPC-3+) is not
>     available over the ATA PACKET interface.
> 
> To fix all three, this change:
> 
>   - Raises max_lun from 1 to 8 (matching the SCSI host default).
>     Sequential LUN scanning stops at the first non-responding LUN, so
>     single-LUN devices are unaffected.

If the only case that we can encounter with libata are these special ATAPI
devices with 2 LUNs, I would limit the maximum to 2.

>   - In __ata_scsi_find_dev(), allow non-zero LUNs for ATAPI devices by
>     routing them to the same ata_device as LUN 0.
> 
>   - In atapi_xlat(), encode the target LUN into CDB byte 1 bits 7:5
>     before passing the command packet to the device.
> 
> These changes are prerequisites for probing additional LUNs during
> host scanning, which is done in a subsequent patch.
> 
> Additionally, fix two related issues exposed by multi-LUN scanning:
> 
>   - ata_scsi_dev_config() previously assigned dev->sdev = sdev for every
>     LUN configured.  With multiple LUNs sharing one ata_device, this
>     caused dev->sdev to be overwritten by each non-LUN-0 sdev.  Restrict
>     the assignment to LUN 0 so that dev->sdev always tracks the
>     canonical scsi_device for the underlying ATA device.

Special casing this does not seem nice at all. Why not simply increasing the
sdev reference count when it is assigned to a LUN that is not LUN 0 ? And drop
that reference when the LUN is torn down ? That will remove any dependency on
the order in which LUNs are torn down too.

>   - ata_scsi_sdev_destroy() detached the entire ATA device whenever
>     dev->sdev was non-NULL.  When a spurious multi-LUN scan result was
>     removed, this incorrectly tore down the underlying device.  Detach
>     only when the canonical (LUN 0) sdev is being destroyed.

This should not happen with the reference count change suggested above.

This is a lot of changes for one patch. So I also suggest splitting this patch.

> 
> Assisted-by: Claude:claude-opus-4-6
> Signed-off-by: Phil Pemberton <philpem@philpem.me.uk>
> ---
>  drivers/ata/libata-scsi.c | 38 ++++++++++++++++++++++++++++++++++----
>  1 file changed, 34 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 3b65df914ebb..dc6829e60fb3 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -25,6 +25,7 @@
>  #include <scsi/scsi_eh.h>
>  #include <scsi/scsi_device.h>
>  #include <scsi/scsi_tcq.h>
> +#include <scsi/scsi_devinfo.h>
>  #include <scsi/scsi_transport.h>
>  #include <linux/libata.h>
>  #include <linux/hdreg.h>
> @@ -1131,7 +1132,14 @@ int ata_scsi_dev_config(struct scsi_device *sdev, struct queue_limits *lim,
>  	if (dev->flags & ATA_DFLAG_TRUSTED)
>  		sdev->security_supported = 1;
>  
> -	dev->sdev = sdev;
> +	/*
> +	 * Only LUN 0 is treated as the canonical scsi_device for the ATA
> +	 * device.  Multi-LUN ATAPI devices share a single ata_device, so
> +	 * dev->sdev must continue to track LUN 0 even when additional LUNs
> +	 * are added or removed.
> +	 */
> +	if (sdev->lun == 0)
> +		dev->sdev = sdev;
>  	return 0;
>  }
>  
> @@ -1220,7 +1228,12 @@ void ata_scsi_sdev_destroy(struct scsi_device *sdev)
>  
>  	spin_lock_irqsave(ap->lock, flags);
>  	dev = __ata_scsi_find_dev(ap, sdev);
> -	if (dev && dev->sdev) {
> +	/*
> +	 * Only detach when the canonical (LUN 0) scsi_device is going away.
> +	 * Removing a non-LUN-0 sdev (e.g. a spurious multi-LUN scan result)
> +	 * must not tear down the underlying ATA device.
> +	 */
> +	if (dev && dev->sdev == sdev) {
>  		/* SCSI device already in CANCEL state, no need to offline it */
>  		dev->sdev = NULL;
>  		dev->flags |= ATA_DFLAG_DETACH;
> @@ -2950,6 +2963,15 @@ static unsigned int atapi_xlat(struct ata_queued_cmd *qc)
>  	memset(qc->cdb, 0, dev->cdb_len);
>  	memcpy(qc->cdb, scmd->cmnd, scmd->cmd_len);
>  
> +	/*
> +	 * Encode LUN in CDB byte 1 bits 7:5 for multi-LUN ATAPI devices
> +	 * that use the SCSI-2 CDB LUN convention (e.g. Panasonic PD/CD
> +	 * combo drives).
> +	 */
> +	if (scmd->device->lun)
> +		qc->cdb[1] = (qc->cdb[1] & 0x1f) |
> +			      ((scmd->device->lun & 0x7) << 5);

Splitting the line after the "=" should look nicer.

> +
>  	qc->complete_fn = atapi_qc_complete;
>  
>  	qc->tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
> @@ -3062,9 +3084,17 @@ static struct ata_device *__ata_scsi_find_dev(struct ata_port *ap,
>  
>  	/* skip commands not addressed to targets we simulate */
>  	if (!sata_pmp_attached(ap)) {
> -		if (unlikely(scsidev->channel || scsidev->lun))
> +		if (unlikely(scsidev->channel))
>  			return NULL;
>  		devno = scsidev->id;
> +		/* Allow non-zero LUNs for ATAPI devices (e.g. PD/CD combos) */
> +		if (unlikely(scsidev->lun)) {
> +			struct ata_device *dev = ata_find_dev(ap, devno);
> +
> +			if (!dev || dev->class != ATA_DEV_ATAPI)
> +				return NULL;
> +			return dev;
> +		}

Hmmm... What if the multi-LUN ATAPI device is attached to a port multiplier ?

>  	} else {
>  		if (unlikely(scsidev->id || scsidev->lun))
>  			return NULL;
> @@ -4620,7 +4650,7 @@ int ata_scsi_add_hosts(struct ata_host *host, const struct scsi_host_template *s
>  		shost->transportt = ata_scsi_transport_template;
>  		shost->unique_id = ap->print_id;
>  		shost->max_id = 16;
> -		shost->max_lun = 1;
> +		shost->max_lun = 8;
>  		shost->max_channel = 1;
>  		shost->max_cmd_len = 32;
>  


-- 
Damien Le Moal
Western Digital Research

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

[Damien Le Moal]

On 4/9/26 23:05, Phil Pemberton wrote:
> Some ATAPI devices (e.g. the Panasonic PD/CD combo drives) implement
> multiple logical units. For instance the aforementioned PD/CD has a CD
> drive on LUN 0 and the rewritable Phase-change Dual (PD) block device on
> LUN 1.
> 
> ata_scsi_scan_host() previously only scanned LUN 0 via
> __scsi_add_device(). With the multi-LUN work now in place, extend this
> scan to probe for additional LUNs on devices which have BLIST_FORCELUN
> set in the SCSI device info table.
> 
> Scanning stops when __scsi_add_device() fails, or the device reports
> device type 0x1f (unknown or no device type). The PD drive returns this
> for unimplemented LUNs.
> 
> The aforementioned BLIST_FORCELUN guard prevents non-multilun devices
> from being affected.
> 
> Tested with a Panasonic LF-1195C PD/CD with Compaq firmware, which now
> correctly enumerates as a CD drive (sr) and PD optical drive (sd).
> 
> Also tested with a LITE-ON iHAS124 DVD drive, which has a single LUN and
> ignores the LUN parameter in the CDB. As a result, without the
> BLIST_FORCELUN guard, this drive would pop up as eight separate devices.
> 
> Assisted-by: Claude:claude-opus-4-6
> Signed-off-by: Phil Pemberton <philpem@philpem.me.uk>
> ---
>  drivers/ata/libata-scsi.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index dc6829e60fb3..0a7ce44118fe 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -4732,6 +4732,35 @@ void ata_scsi_scan_host(struct ata_port *ap, int sync)
>  			if (!IS_ERR(sdev)) {
>  				dev->sdev = sdev;
>  				ata_scsi_assign_ofnode(dev, ap);
> +				/*
> +				 * Multi-LUN ATAPI devices (e.g. PD/CD combo
> +				 * drives) are flagged BLIST_FORCELUN in
> +				 * scsi_devinfo.  Probe additional LUNs when
> +				 * the flag is set.
> +				 */
> +				if (dev->class == ATA_DEV_ATAPI &&
> +				    (sdev->sdev_bflags & BLIST_FORCELUN)) {
> +					u64 lun;
> +
> +					for (lun = 1; lun < ap->scsi_host->max_lun;
> +					     lun++) {
> +						struct scsi_device *extra;
> +
> +						extra = __scsi_add_device(
> +							ap->scsi_host,
> +							channel, id, lun,
> +							NULL);
> +						if (IS_ERR(extra))
> +							break;
> +						/* PDT 0x1f: no device type */
> +						if (extra->type == 0x1f) {
> +							scsi_remove_device(extra);
> +							scsi_device_put(extra);
> +							break;
> +						}
> +						scsi_device_put(extra);
> +					}
> +				}

It would be a lot nicer and more readable to have this hunk as a helper
function, e.g.: ata_scsi_scan_atapi_luns().

>  				scsi_device_put(sdev);
>  			} else {
>  				dev->sdev = NULL;


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 2/3] ata: libata-scsi: enable multi-LUN support for ATAPI devices

[Phil Pemberton]

On 12/04/2026 08:38, Damien Le Moal wrote:
> On 4/9/26 23:05, Phil Pemberton wrote:
>>    - Raises max_lun from 1 to 8 (matching the SCSI host default).
>>      Sequential LUN scanning stops at the first non-responding LUN, so
>>      single-LUN devices are unaffected.
> 
> If the only case that we can encounter with libata are these special ATAPI
> devices with 2 LUNs, I would limit the maximum to 2.

I'm inclined to agree, but there are devices with higher LUN counts: the 
Nakamichi CD changers. The MJ-4.4 and MJ-5.16 were available in an ATAPI 
variant which exposed a LUN for each disc in the changer stack. There's 
a Cathode Ray Dude video demonstrating the latter.

I like the idea of the lower LUN cap for compatibility, but I think I'd 
hedge bets by also having a module parameter (e.g. libata.atapi_max_lun) 
to override it. Default 2 seems like a good compromise.

In any case, the BLIST_FORCELUN gate should limit things to one LUN for 
any device which isn't on the device list.


>>    - ata_scsi_dev_config() previously assigned dev->sdev = sdev for every
>>      LUN configured.  With multiple LUNs sharing one ata_device, this
>>      caused dev->sdev to be overwritten by each non-LUN-0 sdev.  Restrict
>>      the assignment to LUN 0 so that dev->sdev always tracks the
>>      canonical scsi_device for the underlying ATA device.
> 
> Special casing this does not seem nice at all. Why not simply increasing the
> sdev reference count when it is assigned to a LUN that is not LUN 0 ? And drop
> that reference when the LUN is torn down ? That will remove any dependency on
> the order in which LUNs are torn down too.

The if (sdev->lun == 0) guard isn't about teardown ordering; it's about 
which device dev->sdev points at.

dev->sdev is a single pointer, but with multi-LUN ATAPI there are now 
multiple sdevs sharing one ata_device. Without the guard, each call to 
ata_scsi_dev_config() overwrites the pointer, so it ends up tracking the 
last-configured LUN (likely the highest-numbered one).

This is really made clear by ata_scsi_sdev_destroy(). It uses
   dev->sdev == sdev
to decide when to schedule ATA-level detach. If the pointer has been 
overwritten, destroying the higher LUN will tear down the whole device, 
and destroying LUN 0 won't trigger a detach.

Refcounting keeps whichever sdev is stored there alive, but it doesn't 
decide which one should be stored in the first place. Picking LUN 0 
keeps the existing invariant intact for single-LUN devices, and the 
other users of dev->sdev (scsi_remove_device() in ata_port_detach(), 
ACPI uevents, zpodd) continue to operate on a stable, well-defined sdev.

Happy to add scsi_device_get() on the LUN-0 sdev when a higher LUN is 
configured, and the matching put in sdev_destroy, so LUN 0 can't be 
freed while higher LUNs still exist. That gives you the ordering 
guarantee on top of the pointer-stability guarantee.

>>    - ata_scsi_sdev_destroy() detached the entire ATA device whenever
>>      dev->sdev was non-NULL.  When a spurious multi-LUN scan result was
>>      removed, this incorrectly tore down the underlying device.  Detach
>>      only when the canonical (LUN 0) sdev is being destroyed.
> 
> This should not happen with the reference count change suggested above.
> 
> This is a lot of changes for one patch. So I also suggest splitting this patch.

Will do.

All other comments acked.


-- 
Phil.
philpem@philpem.me.uk
https://www.philpem.me.uk/

Re: [PATCH 2/3] ata: libata-scsi: enable multi-LUN support for ATAPI devices

[Damien Le Moal]

On 2026/04/12 21:40, Phil Pemberton wrote:
> On 12/04/2026 08:38, Damien Le Moal wrote:
>> On 4/9/26 23:05, Phil Pemberton wrote:
>>>    - Raises max_lun from 1 to 8 (matching the SCSI host default).
>>>      Sequential LUN scanning stops at the first non-responding LUN, so
>>>      single-LUN devices are unaffected.
>>
>> If the only case that we can encounter with libata are these special ATAPI
>> devices with 2 LUNs, I would limit the maximum to 2.
> 
> I'm inclined to agree, but there are devices with higher LUN counts: the 
> Nakamichi CD changers. The MJ-4.4 and MJ-5.16 were available in an ATAPI 
> variant which exposed a LUN for each disc in the changer stack. There's 
> a Cathode Ray Dude video demonstrating the latter.
> 
> I like the idea of the lower LUN cap for compatibility, but I think I'd 
> hedge bets by also having a module parameter (e.g. libata.atapi_max_lun) 
> to override it. Default 2 seems like a good compromise.
> 
> In any case, the BLIST_FORCELUN gate should limit things to one LUN for 
> any device which isn't on the device list.

Ideally, we want to keep the default 1 LUN value and change it to a higher count
only if we probe that we are dealing with an ATAPI device (device class is
ATAPI). Not sure if that is possible. Need to look again at that code.

>>>    - ata_scsi_dev_config() previously assigned dev->sdev = sdev for every
>>>      LUN configured.  With multiple LUNs sharing one ata_device, this
>>>      caused dev->sdev to be overwritten by each non-LUN-0 sdev.  Restrict
>>>      the assignment to LUN 0 so that dev->sdev always tracks the
>>>      canonical scsi_device for the underlying ATA device.
>>
>> Special casing this does not seem nice at all. Why not simply increasing the
>> sdev reference count when it is assigned to a LUN that is not LUN 0 ? And drop
>> that reference when the LUN is torn down ? That will remove any dependency on
>> the order in which LUNs are torn down too.
> 
> The if (sdev->lun == 0) guard isn't about teardown ordering; it's about 
> which device dev->sdev points at.
> 
> dev->sdev is a single pointer, but with multi-LUN ATAPI there are now 
> multiple sdevs sharing one ata_device. Without the guard, each call to 
> ata_scsi_dev_config() overwrites the pointer, so it ends up tracking the 
> last-configured LUN (likely the highest-numbered one).
> 
> This is really made clear by ata_scsi_sdev_destroy(). It uses
>    dev->sdev == sdev
> to decide when to schedule ATA-level detach. If the pointer has been 
> overwritten, destroying the higher LUN will tear down the whole device, 
> and destroying LUN 0 won't trigger a detach.
> 
> Refcounting keeps whichever sdev is stored there alive, but it doesn't 
> decide which one should be stored in the first place. Picking LUN 0 
> keeps the existing invariant intact for single-LUN devices, and the 
> other users of dev->sdev (scsi_remove_device() in ata_port_detach(), 
> ACPI uevents, zpodd) continue to operate on a stable, well-defined sdev.
> 
> Happy to add scsi_device_get() on the LUN-0 sdev when a higher LUN is 
> configured, and the matching put in sdev_destroy, so LUN 0 can't be 
> freed while higher LUNs still exist. That gives you the ordering 
> guarantee on top of the pointer-stability guarantee.

OK. I misunderstood your change. I really need to look again at that code, which
I have not done in a while.

I think your change may be generally OK, but I am worried that things like
suspend/resume may have issues. Have you tested that ?

Anyway, please give us some time to look into this (sorry, but I am super busy
these days, so it may take a couple of weeks).

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 2/3] ata: libata-scsi: enable multi-LUN support for ATAPI devices

[Hannes Reinecke]

On 4/12/26 21:40, Phil Pemberton wrote:
> On 12/04/2026 08:38, Damien Le Moal wrote:
>> On 4/9/26 23:05, Phil Pemberton wrote:
>>>    - Raises max_lun from 1 to 8 (matching the SCSI host default).
>>>      Sequential LUN scanning stops at the first non-responding LUN, so
>>>      single-LUN devices are unaffected.
>>
>> If the only case that we can encounter with libata are these special 
>> ATAPI
>> devices with 2 LUNs, I would limit the maximum to 2.
> 
> I'm inclined to agree, but there are devices with higher LUN counts: the 
> Nakamichi CD changers. The MJ-4.4 and MJ-5.16 were available in an ATAPI 
> variant which exposed a LUN for each disc in the changer stack. There's 
> a Cathode Ray Dude video demonstrating the latter.
> 
Don't get too excited. This is ancient technology, with an extremely 
narrow user-base :-)
'Were available' is not identical to 'in current use', and I'm somewhat
disinclined to add support for technology with no users.

> I like the idea of the lower LUN cap for compatibility, but I think I'd 
> hedge bets by also having a module parameter (e.g. libata.atapi_max_lun) 
> to override it. Default 2 seems like a good compromise.
> 
Hmm. I really would like to restrict max lun to 1 (as it somewhat mimics 
the master/slave thingie on IDE). But higher than that really is 
arbitrary; the next sensible limit would be '7', as this is what SCSI-2
could handle. I completely fail to see the motivation to have a limit
somewhere in between.

> In any case, the BLIST_FORCELUN gate should limit things to one LUN for 
> any device which isn't on the device list.
> 
> 
>>>    - ata_scsi_dev_config() previously assigned dev->sdev = sdev for 
>>> every
>>>      LUN configured.  With multiple LUNs sharing one ata_device, this
>>>      caused dev->sdev to be overwritten by each non-LUN-0 sdev.  
>>> Restrict
>>>      the assignment to LUN 0 so that dev->sdev always tracks the
>>>      canonical scsi_device for the underlying ATA device.
>>
>> Special casing this does not seem nice at all. Why not simply 
>> increasing the
>> sdev reference count when it is assigned to a LUN that is not LUN 0 ? 
>> And drop
>> that reference when the LUN is torn down ? That will remove any 
>> dependency on
>> the order in which LUNs are torn down too.
> 
> The if (sdev->lun == 0) guard isn't about teardown ordering; it's about 
> which device dev->sdev points at.
> 
> dev->sdev is a single pointer, but with multi-LUN ATAPI there are now 
> multiple sdevs sharing one ata_device. Without the guard, each call to 
> ata_scsi_dev_config() overwrites the pointer, so it ends up tracking the 
> last-configured LUN (likely the highest-numbered one).
> 
And how would you address the 'sdev' of LUN 1?
(and how of LUN 2, if we decide to have one?)

Please, don't. The correct way would be to convert 'dev->sdev' into
a list.

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