public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/3] ata: libata-scsi: add multi-LUN support for ATAPI devices
@ 2026-04-09 21:05 Phil Pemberton
  2026-04-09 21:05 ` [PATCH 1/3] scsi: scsi_devinfo: add COMPAQ PD-1 multi-LUN ATAPI device quirk Phil Pemberton
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Phil Pemberton @ 2026-04-09 21:05 UTC (permalink / raw)
  To: dlemoal, cassel, James.Bottomley, martin.petersen
  Cc: linux-ide, linux-scsi, linux-kernel, 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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/3] scsi: scsi_devinfo: add COMPAQ PD-1 multi-LUN ATAPI device quirk
  2026-04-09 21:05 [RFC PATCH 0/3] ata: libata-scsi: add multi-LUN support for ATAPI devices Phil Pemberton
@ 2026-04-09 21:05 ` Phil Pemberton
  2026-04-12  7:25   ` Damien Le Moal
  2026-04-09 21:05 ` [PATCH 2/3] ata: libata-scsi: enable multi-LUN support for ATAPI devices Phil Pemberton
  2026-04-09 21:05 ` [PATCH 3/3] ata: libata-scsi: probe additional LUNs for multi-LUN " Phil Pemberton
  2 siblings, 1 reply; 7+ messages in thread
From: Phil Pemberton @ 2026-04-09 21:05 UTC (permalink / raw)
  To: dlemoal, cassel, James.Bottomley, martin.petersen
  Cc: linux-ide, linux-scsi, linux-kernel, 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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/3] ata: libata-scsi: enable multi-LUN support for ATAPI devices
  2026-04-09 21:05 [RFC PATCH 0/3] ata: libata-scsi: add multi-LUN support for ATAPI devices Phil Pemberton
  2026-04-09 21:05 ` [PATCH 1/3] scsi: scsi_devinfo: add COMPAQ PD-1 multi-LUN ATAPI device quirk Phil Pemberton
@ 2026-04-09 21:05 ` Phil Pemberton
  2026-04-12  7:38   ` Damien Le Moal
  2026-04-09 21:05 ` [PATCH 3/3] ata: libata-scsi: probe additional LUNs for multi-LUN " Phil Pemberton
  2 siblings, 1 reply; 7+ messages in thread
From: Phil Pemberton @ 2026-04-09 21:05 UTC (permalink / raw)
  To: dlemoal, cassel, James.Bottomley, martin.petersen
  Cc: linux-ide, linux-scsi, linux-kernel, 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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/3] ata: libata-scsi: probe additional LUNs for multi-LUN ATAPI devices
  2026-04-09 21:05 [RFC PATCH 0/3] ata: libata-scsi: add multi-LUN support for ATAPI devices Phil Pemberton
  2026-04-09 21:05 ` [PATCH 1/3] scsi: scsi_devinfo: add COMPAQ PD-1 multi-LUN ATAPI device quirk Phil Pemberton
  2026-04-09 21:05 ` [PATCH 2/3] ata: libata-scsi: enable multi-LUN support for ATAPI devices Phil Pemberton
@ 2026-04-09 21:05 ` Phil Pemberton
  2026-04-12  7:41   ` Damien Le Moal
  2 siblings, 1 reply; 7+ messages in thread
From: Phil Pemberton @ 2026-04-09 21:05 UTC (permalink / raw)
  To: dlemoal, cassel, James.Bottomley, martin.petersen
  Cc: linux-ide, linux-scsi, linux-kernel, 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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/3] scsi: scsi_devinfo: add COMPAQ PD-1 multi-LUN ATAPI device quirk
  2026-04-09 21:05 ` [PATCH 1/3] scsi: scsi_devinfo: add COMPAQ PD-1 multi-LUN ATAPI device quirk Phil Pemberton
@ 2026-04-12  7:25   ` Damien Le Moal
  0 siblings, 0 replies; 7+ messages in thread
From: Damien Le Moal @ 2026-04-12  7:25 UTC (permalink / raw)
  To: Phil Pemberton, cassel, James.Bottomley, martin.petersen
  Cc: linux-ide, linux-scsi, linux-kernel

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/3] ata: libata-scsi: enable multi-LUN support for ATAPI devices
  2026-04-09 21:05 ` [PATCH 2/3] ata: libata-scsi: enable multi-LUN support for ATAPI devices Phil Pemberton
@ 2026-04-12  7:38   ` Damien Le Moal
  0 siblings, 0 replies; 7+ messages in thread
From: Damien Le Moal @ 2026-04-12  7:38 UTC (permalink / raw)
  To: Phil Pemberton, cassel, James.Bottomley, martin.petersen
  Cc: linux-ide, linux-scsi, linux-kernel

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/3] ata: libata-scsi: probe additional LUNs for multi-LUN ATAPI devices
  2026-04-09 21:05 ` [PATCH 3/3] ata: libata-scsi: probe additional LUNs for multi-LUN " Phil Pemberton
@ 2026-04-12  7:41   ` Damien Le Moal
  0 siblings, 0 replies; 7+ messages in thread
From: Damien Le Moal @ 2026-04-12  7:41 UTC (permalink / raw)
  To: Phil Pemberton, cassel, James.Bottomley, martin.petersen
  Cc: linux-ide, linux-scsi, linux-kernel

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-04-12  7:41 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-09 21:05 [RFC PATCH 0/3] ata: libata-scsi: add multi-LUN support for ATAPI devices Phil Pemberton
2026-04-09 21:05 ` [PATCH 1/3] scsi: scsi_devinfo: add COMPAQ PD-1 multi-LUN ATAPI device quirk Phil Pemberton
2026-04-12  7:25   ` Damien Le Moal
2026-04-09 21:05 ` [PATCH 2/3] ata: libata-scsi: enable multi-LUN support for ATAPI devices Phil Pemberton
2026-04-12  7:38   ` Damien Le Moal
2026-04-09 21:05 ` [PATCH 3/3] ata: libata-scsi: probe additional LUNs for multi-LUN " Phil Pemberton
2026-04-12  7:41   ` Damien Le Moal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox