linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] Insert ATA transport objects in SCSI syfs topology.
       [not found] <20120920020145.GA23210@mint-spring.sh.intel.com>
@ 2012-09-27 19:04 ` Gwendal Grignou
  2012-09-28  6:27   ` Aaron Lu
  2012-09-27 19:04 ` [PATCH 1/3] Revert "ata: make ata port as parent device of scsi host" Gwendal Grignou
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 15+ messages in thread
From: Gwendal Grignou @ 2012-09-27 19:04 UTC (permalink / raw)
  To: aaron.lu, minggr, tj, jgarzik, james.bottomley
  Cc: Gwendal Grignou, linux-scsi, linux-ide

This set of patches improve ATA transport classes integration with SCSI
objects.

Before [2.6.x]

Ata and scsi transport class where separated:
`--0000:09:00.0
|  `--ata1
|  |  `--port_port
|  |  `--link1
|  |  |  `--dev1.0
|  |  |  `--dev1.1
|  `--ata2
|  ...
|  `--host0
|  |  `--scsi_host
|  |  |  `--host0
|  |  `--target0:0:0
|  |  |  `--0:0:0:0
|  |  |  |  `--block
|  |  |  |  |  `--sda
|  |  |  |  |  |  `--sda1

In 3.2, Lin - in commit 9a6d6a2ddabbd32c07f6a38b659e5f3db319fa5a - addressed
the issue of linking the ata port with the scsi host object by placing the
scsi_host object under ata port objects.

However to be more consistent with other transport, this patch does the opposite:

For instance, with SAS transport, We have
`--0000:0b:00.0
|  `--host6
|  |  `--phy-6:0
|  |  `--phy-6:1
...
|  |  `--port-6:0
|  |  |  `--end_device-6:0
|  |  |  |  `--sas_device
|  |  |  |  |  `--end_device-6:0
|  |  |  |  `--sas_end_device
|  |  |  |  |  `--end_device-6:0
|  |  |  |  `--target6:0:0
|  |  |  |  |  `--6:0:0:0
|  |  |  |  |  |  `--block
|  |  |  |  |  |  |  `--sdb
...
|  |  `--port-6:1
|  |  |  `--end_device-6:1
...
phy and port have to be separated, sas_port are created dynamically.

For ata, all objects are created at initialization time, so the layout is:
`--0000:09:00.0
|  `--host0
|  |  `--port1
|  |  |  `--link1
|  |  |  |  `--dev1.0
|  |  |  |  |  `--target0:0:0
|  |  |  |  |  |  `--0:0:0:0
|  |  |  |  |  |  |  `--block
|  |  |  |  |  |  |  |  `--sda

If we have a port multiplier, more links are created.
`--0000:09:00.0
...
|  `--host4
|  |  `--port5
|  |  |  `--link5
|  |  |  |  `--dev5.0
[device for the port multiplier]
|  |  |  `--link5.0
|  |  |  |  `--dev5.0.0
|  |  |  |  |  `--target4:0:0
|  |  |  |  |  |  `--4:0:0:0
|  |  |  |  |  |  |  `--block
|  |  |  |  |  |  |  |  `--sdc
[disk in port 0 of the port multiplier]
...
|  |  |  `--link5.2
|  |  |  |  `--dev5.2.0
|  |  |  |  |  `--target4:2:0
|  |  |  |  |  |  `--4:2:0:0
|  |  |  |  |  |  |  `--block
|  |  |  |  |  |  |  |  `--sde
[disk in port 2 of the port multiplier]

In consequence, the path of a scsi device becomes:
.../0000:00:1f.2/host0/ata1/link1/dev1.0/target0:0:0/0:0:0:0
dev1.0 indicates the master device [0] in ata port 1.
ata1 being under host0, we know the reliationships between the scsi_host id and
ata port id.

or when a port multiplier is present: for instance the device in port 4 of the
port multiplier:
.../0000:00:06.0/0000:09:00.0/host5/ata6/link6.4/dev6.4.0/target5:4:0/5:4:0:0

Gwendal Grignou (3):
  Revert "ata: make ata port as parent device of scsi host"
  scsi: Allow devices to have arbitrary parent
  libata: Change transport topology layout

 drivers/ata/libata-core.c      |   13 ++++++-------
 drivers/ata/libata-scsi.c      |    4 ++--
 drivers/ata/libata-transport.c |    2 +-
 drivers/firewire/sbp2.c        |    3 ++-
 drivers/message/i2o/i2o_scsi.c |    4 ++--
 drivers/scsi/scsi_scan.c       |    9 +++++----
 include/scsi/scsi_device.h     |    2 +-
 7 files changed, 19 insertions(+), 18 deletions(-)

-- 
1.7.7.3


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

* [PATCH 1/3] Revert "ata: make ata port as parent device of scsi host"
       [not found] <20120920020145.GA23210@mint-spring.sh.intel.com>
  2012-09-27 19:04 ` [PATCH 0/3] Insert ATA transport objects in SCSI syfs topology Gwendal Grignou
@ 2012-09-27 19:04 ` Gwendal Grignou
  2012-09-29 17:08   ` Sergei Shtylyov
  2012-09-27 19:04 ` [PATCH 2/3] scsi: Allow devices to have arbitrary parent Gwendal Grignou
  2012-09-27 19:04 ` [PATCH 3/3] libata: Change transport topology layout Gwendal Grignou
  3 siblings, 1 reply; 15+ messages in thread
From: Gwendal Grignou @ 2012-09-27 19:04 UTC (permalink / raw)
  To: aaron.lu, minggr, tj, jgarzik, james.bottomley
  Cc: Gwendal Grignou, linux-scsi, linux-ide

This reverts commit 9a6d6a2ddabbd32c07f6a38b659e5f3db319fa5a.

Instead, melt libata transport sysfs object in scsi objects.

Change-Id: I8c709f63ddf7ba97b9e6f449d5c0b8b85e44e818

Signed-off-by: Gwendal Grignou <gwendal@google.com>
---
 drivers/ata/libata-scsi.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index e3bda07..be38930 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -3609,7 +3609,7 @@ int ata_scsi_add_hosts(struct ata_host *host, struct scsi_host_template *sht)
 		shost->max_host_blocked = 1;
 
 		rc = scsi_add_host_with_dma(ap->scsi_host,
-						&ap->tdev, ap->host->dev);
+					    host->dev, host->dev);
 		if (rc)
 			goto err_add;
 	}
-- 
1.7.7.3


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

* [PATCH 2/3] scsi: Allow devices to have arbitrary parent
       [not found] <20120920020145.GA23210@mint-spring.sh.intel.com>
  2012-09-27 19:04 ` [PATCH 0/3] Insert ATA transport objects in SCSI syfs topology Gwendal Grignou
  2012-09-27 19:04 ` [PATCH 1/3] Revert "ata: make ata port as parent device of scsi host" Gwendal Grignou
@ 2012-09-27 19:04 ` Gwendal Grignou
  2012-09-27 19:04 ` [PATCH 3/3] libata: Change transport topology layout Gwendal Grignou
  3 siblings, 0 replies; 15+ messages in thread
From: Gwendal Grignou @ 2012-09-27 19:04 UTC (permalink / raw)
  To: aaron.lu, minggr, tj, jgarzik, james.bottomley
  Cc: Gwendal Grignou, linux-scsi, linux-ide

Allow driver who calls __scsi_add_device directly to create the scsi
device on any parent, not just scsi_host directly.
This is alreay done for transport with their own class [SAS, iSCSI, FC, ...]

Change-Id: Ibcf132a8959fbf732dcd0b34a7f4a570d7cf394d

Signed-off-by: Gwendal Grignou <gwendal@google.com>
---
 drivers/ata/libata-scsi.c      |    4 ++--
 drivers/firewire/sbp2.c        |    3 ++-
 drivers/message/i2o/i2o_scsi.c |    4 ++--
 drivers/scsi/scsi_scan.c       |    9 +++++----
 include/scsi/scsi_device.h     |    2 +-
 5 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index be38930..bfda61f 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -3649,8 +3649,8 @@ 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);
+			sdev = __scsi_add_device(&ap->scsi_host->shost_gendev,
+						 channel, id, 0, NULL);
 			if (!IS_ERR(sdev)) {
 				dev->sdev = sdev;
 				scsi_device_put(sdev);
diff --git a/drivers/firewire/sbp2.c b/drivers/firewire/sbp2.c
index 1162d6b..839afa5 100644
--- a/drivers/firewire/sbp2.c
+++ b/drivers/firewire/sbp2.c
@@ -879,7 +879,8 @@ static void sbp2_login(struct work_struct *work)
 		ssleep(SBP2_INQUIRY_DELAY);
 
 	shost = container_of((void *)tgt, struct Scsi_Host, hostdata[0]);
-	sdev = __scsi_add_device(shost, 0, 0, sbp2_lun2int(lu->lun), lu);
+	sdev = __scsi_add_device(&shost->shost_gendev, 0, 0,
+				 sbp2_lun2int(lu->lun), lu);
 	/*
 	 * FIXME:  We are unable to perform reconnects while in sbp2_login().
 	 * Therefore __scsi_add_device() will get into trouble if a bus reset
diff --git a/drivers/message/i2o/i2o_scsi.c b/drivers/message/i2o/i2o_scsi.c
index 1d31d72..ee1353c 100644
--- a/drivers/message/i2o/i2o_scsi.c
+++ b/drivers/message/i2o/i2o_scsi.c
@@ -294,8 +294,8 @@ static int i2o_scsi_probe(struct device *dev)
 	}
 
 	scsi_dev =
-	    __scsi_add_device(i2o_shost->scsi_host, channel, le32_to_cpu(id),
-			      le64_to_cpu(lun), i2o_dev);
+	    __scsi_add_device(&i2o_shost->scsi_host->shost_gendev, channel,
+			      le32_to_cpu(id), le64_to_cpu(lun), i2o_dev);
 
 	if (IS_ERR(scsi_dev)) {
 		osm_warn("can not add SCSI device %03x\n",
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 56a9379..105123c 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -1489,11 +1489,11 @@ static int scsi_report_lun_scan(struct scsi_target *starget, int bflags,
 	return ret;
 }
 
-struct scsi_device *__scsi_add_device(struct Scsi_Host *shost, uint channel,
+struct scsi_device *__scsi_add_device(struct device *parent, uint channel,
 				      uint id, uint lun, void *hostdata)
 {
+	struct Scsi_Host *shost = dev_to_shost(parent);
 	struct scsi_device *sdev = ERR_PTR(-ENODEV);
-	struct device *parent = &shost->shost_gendev;
 	struct scsi_target *starget;
 
 	if (strncmp(scsi_scan_type, "none", 4) == 0)
@@ -1524,8 +1524,9 @@ EXPORT_SYMBOL(__scsi_add_device);
 int scsi_add_device(struct Scsi_Host *host, uint channel,
 		    uint target, uint lun)
 {
-	struct scsi_device *sdev = 
-		__scsi_add_device(host, channel, target, lun, NULL);
+	struct scsi_device *sdev =
+		__scsi_add_device(&host->shost_gendev, channel, target,
+				  lun, NULL);
 	if (IS_ERR(sdev))
 		return PTR_ERR(sdev);
 
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 9895f69..9646a1d 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -285,7 +285,7 @@ static inline struct scsi_target *scsi_target(struct scsi_device *sdev)
 #define starget_printk(prefix, starget, fmt, a...)	\
 	dev_printk(prefix, &(starget)->dev, fmt, ##a)
 
-extern struct scsi_device *__scsi_add_device(struct Scsi_Host *,
+extern struct scsi_device *__scsi_add_device(struct device *,
 		uint, uint, uint, void *hostdata);
 extern int scsi_add_device(struct Scsi_Host *host, uint channel,
 			   uint target, uint lun);
-- 
1.7.7.3


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

* [PATCH 3/3] libata: Change transport topology layout
       [not found] <20120920020145.GA23210@mint-spring.sh.intel.com>
                   ` (2 preceding siblings ...)
  2012-09-27 19:04 ` [PATCH 2/3] scsi: Allow devices to have arbitrary parent Gwendal Grignou
@ 2012-09-27 19:04 ` Gwendal Grignou
  2012-09-28  6:38   ` Aaron Lu
  3 siblings, 1 reply; 15+ messages in thread
From: Gwendal Grignou @ 2012-09-27 19:04 UTC (permalink / raw)
  To: aaron.lu, minggr, tj, jgarzik, james.bottomley
  Cc: Gwendal Grignou, linux-scsi, linux-ide

Integrate ata objects [port, link, device] with scsi objects.


The path of a scsi device is:
.../0000:00:1f.2/host0/ata1/link1/dev1.0/target0:0:0/0:0:0:0

or when a port multiplier is present: for instance the device in port 4 of the
port multiplier:
.../0000:00:06.0/0000:09:00.0/host5/ata6/link6.4/dev6.4.0/target5:4:0/5:4:0:0


Change-Id: I202e089208e8746ccdaf2053d43da831a0c0976d

Signed-off-by: Gwendal Grignou <gwendal@google.com>
---
 drivers/ata/libata-core.c      |   13 ++++++-------
 drivers/ata/libata-scsi.c      |    4 ++--
 drivers/ata/libata-transport.c |    2 +-
 3 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 611050d..c83590b 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -6063,19 +6063,18 @@ int ata_host_register(struct ata_host *host, struct scsi_host_template *sht)
 	for (i = 0; i < host->n_ports; i++)
 		host->ports[i]->print_id = atomic_inc_return(&ata_print_id);
 
+	rc = ata_scsi_add_hosts(host, sht);
+	if (rc)
+		return rc;
 
 	/* Create associated sysfs transport objects  */
 	for (i = 0; i < host->n_ports; i++) {
-		rc = ata_tport_add(host->dev,host->ports[i]);
-		if (rc) {
+		struct ata_port *ap = host->ports[i];
+		rc = ata_tport_add(&ap->scsi_host->shost_gendev, ap);
+		if (rc)
 			goto err_tadd;
-		}
 	}
 
-	rc = ata_scsi_add_hosts(host, sht);
-	if (rc)
-		goto err_tadd;
-
 	/* set cable, sata_spd_limit and report */
 	for (i = 0; i < host->n_ports; i++) {
 		struct ata_port *ap = host->ports[i];
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index bfda61f..9023bb1 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -3649,8 +3649,8 @@ void ata_scsi_scan_host(struct ata_port *ap, int sync)
 			else
 				channel = link->pmp;
 
-			sdev = __scsi_add_device(&ap->scsi_host->shost_gendev,
-						 channel, id, 0, NULL);
+			sdev = __scsi_add_device(&dev->tdev, channel, id, 0,
+						 NULL);
 			if (!IS_ERR(sdev)) {
 				dev->sdev = sdev;
 				scsi_device_put(sdev);
diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c
index c04d393..6829be6 100644
--- a/drivers/ata/libata-transport.c
+++ b/drivers/ata/libata-transport.c
@@ -284,7 +284,7 @@ int ata_tport_add(struct device *parent,
 
 	dev->parent = get_device(parent);
 	dev->release = ata_tport_release;
-	dev_set_name(dev, "ata%d", ap->print_id);
+	dev_set_name(dev, "port%d", ap->print_id);
 	transport_setup_device(dev);
 	error = device_add(dev);
 	if (error) {
-- 
1.7.7.3


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

* Re: [PATCH 0/3] Insert ATA transport objects in SCSI syfs topology.
  2012-09-27 19:04 ` [PATCH 0/3] Insert ATA transport objects in SCSI syfs topology Gwendal Grignou
@ 2012-09-28  6:27   ` Aaron Lu
  2012-10-01 18:22     ` Gwendal Grignou
  0 siblings, 1 reply; 15+ messages in thread
From: Aaron Lu @ 2012-09-28  6:27 UTC (permalink / raw)
  To: Gwendal Grignou
  Cc: minggr, tj, jgarzik, james.bottomley, linux-scsi, linux-ide

On Thu, Sep 27, 2012 at 12:04:01PM -0700, Gwendal Grignou wrote:
> This set of patches improve ATA transport classes integration with SCSI
> objects.
> 
> Before [2.6.x]
> 
> Ata and scsi transport class where separated:
> `--0000:09:00.0
> |  `--ata1
> |  |  `--port_port
> |  |  `--link1
> |  |  |  `--dev1.0
> |  |  |  `--dev1.1
> |  `--ata2
> |  ...
> |  `--host0
> |  |  `--scsi_host
> |  |  |  `--host0
> |  |  `--target0:0:0
> |  |  |  `--0:0:0:0
> |  |  |  |  `--block
> |  |  |  |  |  `--sda
> |  |  |  |  |  |  `--sda1
> 
> In 3.2, Lin - in commit 9a6d6a2ddabbd32c07f6a38b659e5f3db319fa5a - addressed
> the issue of linking the ata port with the scsi host object by placing the
> scsi_host object under ata port objects.
> 
> However to be more consistent with other transport, this patch does the opposite:
> 
> For instance, with SAS transport, We have
> `--0000:0b:00.0
> |  `--host6
> |  |  `--phy-6:0
> |  |  `--phy-6:1
> ...
> |  |  `--port-6:0
> |  |  |  `--end_device-6:0
> |  |  |  |  `--sas_device
> |  |  |  |  |  `--end_device-6:0
> |  |  |  |  `--sas_end_device
> |  |  |  |  |  `--end_device-6:0
> |  |  |  |  `--target6:0:0
> |  |  |  |  |  `--6:0:0:0
> |  |  |  |  |  |  `--block
> |  |  |  |  |  |  |  `--sdb
> ...
> |  |  `--port-6:1
> |  |  |  `--end_device-6:1
> ...
> phy and port have to be separated, sas_port are created dynamically.
> 
> For ata, all objects are created at initialization time, so the layout is:
> `--0000:09:00.0
> |  `--host0
> |  |  `--port1
> |  |  |  `--link1
> |  |  |  |  `--dev1.0
> |  |  |  |  |  `--target0:0:0
> |  |  |  |  |  |  `--0:0:0:0
> |  |  |  |  |  |  |  `--block
> |  |  |  |  |  |  |  |  `--sda
> 
> If we have a port multiplier, more links are created.
> `--0000:09:00.0
> ...
> |  `--host4
> |  |  `--port5
> |  |  |  `--link5
> |  |  |  |  `--dev5.0
> [device for the port multiplier]
> |  |  |  `--link5.0
> |  |  |  |  `--dev5.0.0
> |  |  |  |  |  `--target4:0:0
> |  |  |  |  |  |  `--4:0:0:0
> |  |  |  |  |  |  |  `--block
> |  |  |  |  |  |  |  |  `--sdc
> [disk in port 0 of the port multiplier]
> ...
> |  |  |  `--link5.2
> |  |  |  |  `--dev5.2.0
> |  |  |  |  |  `--target4:2:0
> |  |  |  |  |  |  `--4:2:0:0
> |  |  |  |  |  |  |  `--block
> |  |  |  |  |  |  |  |  `--sde
> [disk in port 2 of the port multiplier]
> 
> In consequence, the path of a scsi device becomes:
> .../0000:00:1f.2/host0/ata1/link1/dev1.0/target0:0:0/0:0:0:0
                         ~~~~
Should be port1 :-)

> dev1.0 indicates the master device [0] in ata port 1.
> ata1 being under host0, we know the reliationships between the scsi_host id and
> ata port id.
> 
> or when a port multiplier is present: for instance the device in port 4 of the
> port multiplier:
> .../0000:00:06.0/0000:09:00.0/host5/ata6/link6.4/dev6.4.0/target5:4:0/5:4:0:0

Same here.

Thanks,
Aaron

> 
> Gwendal Grignou (3):
>   Revert "ata: make ata port as parent device of scsi host"
>   scsi: Allow devices to have arbitrary parent
>   libata: Change transport topology layout
> 
>  drivers/ata/libata-core.c      |   13 ++++++-------
>  drivers/ata/libata-scsi.c      |    4 ++--
>  drivers/ata/libata-transport.c |    2 +-
>  drivers/firewire/sbp2.c        |    3 ++-
>  drivers/message/i2o/i2o_scsi.c |    4 ++--
>  drivers/scsi/scsi_scan.c       |    9 +++++----
>  include/scsi/scsi_device.h     |    2 +-
>  7 files changed, 19 insertions(+), 18 deletions(-)
> 
> -- 
> 1.7.7.3
> 

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

* Re: [PATCH 3/3] libata: Change transport topology layout
  2012-09-27 19:04 ` [PATCH 3/3] libata: Change transport topology layout Gwendal Grignou
@ 2012-09-28  6:38   ` Aaron Lu
  2012-10-04  0:49     ` Gwendal Grignou
  0 siblings, 1 reply; 15+ messages in thread
From: Aaron Lu @ 2012-09-28  6:38 UTC (permalink / raw)
  To: Gwendal Grignou
  Cc: minggr, tj, jgarzik, james.bottomley, linux-scsi, linux-ide

On Thu, Sep 27, 2012 at 12:04:04PM -0700, Gwendal Grignou wrote:
> Integrate ata objects [port, link, device] with scsi objects.
> 
> 
> The path of a scsi device is:
> .../0000:00:1f.2/host0/ata1/link1/dev1.0/target0:0:0/0:0:0:0

After test, I noticed that this will break the current ata acpi binding
code, but can be fixed with the following changes:

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index 30eb7177..459c5b4 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -978,7 +978,7 @@ void ata_acpi_on_disable(struct ata_device *dev)
 
 static int compat_pci_ata(struct ata_port *ap)
 {
-	struct device *dev = ap->tdev.parent;
+	struct device *dev = ap->host->dev;
 	struct pci_dev *pdev;
 
 	if (!is_pci_dev(dev))
@@ -998,7 +998,7 @@ static int ata_acpi_bind_host(struct ata_port *ap, acpi_handle *handle)
 	if (ap->flags & ATA_FLAG_ACPI_SATA)
 		return -ENODEV;
 
-	*handle = acpi_get_child(DEVICE_ACPI_HANDLE(ap->tdev.parent),
+	*handle = acpi_get_child(DEVICE_ACPI_HANDLE(ap->host->dev),
 			ap->port_no);
 
 	if (!*handle)
@@ -1061,7 +1061,12 @@ static struct ata_port *dev_to_ata_port(struct device *dev)
 
 static int ata_acpi_find_device(struct device *dev, acpi_handle *handle)
 {
-	struct ata_port *ap = dev_to_ata_port(dev);
+	struct ata_port *ap;
+
+	if (scsi_is_host_device(dev))
+		ap = ata_shost_to_port(dev_to_shost(dev));
+	else
+		ap = dev_to_ata_port(dev);
 
 	if (!ap)
 		return -ENODEV;


And to make zero power ODD function, the following changes to enable
runtime pm with no callbacks for the ata_link/ata_device transport
devices are necessary.

diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c
index c04d393..ce91bd2 100644
--- a/drivers/ata/libata-transport.c
+++ b/drivers/ata/libata-transport.c
@@ -421,6 +421,10 @@ int ata_tlink_add(struct ata_link *link)
 		goto tlink_err;
 	}
 
+	pm_runtime_no_callbacks(dev);
+	pm_runtime_set_active(dev);
+	pm_runtime_enable(dev);
+
 	transport_add_device(dev);
 	transport_configure_device(dev);
 
@@ -649,6 +653,10 @@ static int ata_tdev_add(struct ata_device *ata_dev)
 		return error;
 	}
 
+	pm_runtime_no_callbacks(dev);
+	pm_runtime_set_active(dev);
+	pm_runtime_enable(dev);
+
 	transport_add_device(dev);
 	transport_configure_device(dev);
 	return 0;

There is no other problems I can see.
Should I prepare a patch to addess the 2 issues?

Thanks,
Aaron

> 
> or when a port multiplier is present: for instance the device in port 4 of the
> port multiplier:
> .../0000:00:06.0/0000:09:00.0/host5/ata6/link6.4/dev6.4.0/target5:4:0/5:4:0:0
> 
> 
> Change-Id: I202e089208e8746ccdaf2053d43da831a0c0976d
> 
> Signed-off-by: Gwendal Grignou <gwendal@google.com>
> ---
>  drivers/ata/libata-core.c      |   13 ++++++-------
>  drivers/ata/libata-scsi.c      |    4 ++--
>  drivers/ata/libata-transport.c |    2 +-
>  3 files changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 611050d..c83590b 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -6063,19 +6063,18 @@ int ata_host_register(struct ata_host *host, struct scsi_host_template *sht)
>  	for (i = 0; i < host->n_ports; i++)
>  		host->ports[i]->print_id = atomic_inc_return(&ata_print_id);
>  
> +	rc = ata_scsi_add_hosts(host, sht);
> +	if (rc)
> +		return rc;
>  
>  	/* Create associated sysfs transport objects  */
>  	for (i = 0; i < host->n_ports; i++) {
> -		rc = ata_tport_add(host->dev,host->ports[i]);
> -		if (rc) {
> +		struct ata_port *ap = host->ports[i];
> +		rc = ata_tport_add(&ap->scsi_host->shost_gendev, ap);
> +		if (rc)
>  			goto err_tadd;
> -		}
>  	}
>  
> -	rc = ata_scsi_add_hosts(host, sht);
> -	if (rc)
> -		goto err_tadd;
> -
>  	/* set cable, sata_spd_limit and report */
>  	for (i = 0; i < host->n_ports; i++) {
>  		struct ata_port *ap = host->ports[i];
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index bfda61f..9023bb1 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -3649,8 +3649,8 @@ void ata_scsi_scan_host(struct ata_port *ap, int sync)
>  			else
>  				channel = link->pmp;
>  
> -			sdev = __scsi_add_device(&ap->scsi_host->shost_gendev,
> -						 channel, id, 0, NULL);
> +			sdev = __scsi_add_device(&dev->tdev, channel, id, 0,
> +						 NULL);
>  			if (!IS_ERR(sdev)) {
>  				dev->sdev = sdev;
>  				scsi_device_put(sdev);
> diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c
> index c04d393..6829be6 100644
> --- a/drivers/ata/libata-transport.c
> +++ b/drivers/ata/libata-transport.c
> @@ -284,7 +284,7 @@ int ata_tport_add(struct device *parent,
>  
>  	dev->parent = get_device(parent);
>  	dev->release = ata_tport_release;
> -	dev_set_name(dev, "ata%d", ap->print_id);
> +	dev_set_name(dev, "port%d", ap->print_id);
>  	transport_setup_device(dev);
>  	error = device_add(dev);
>  	if (error) {
> -- 
> 1.7.7.3
> 

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

* Re: [PATCH 1/3] Revert "ata: make ata port as parent device of scsi host"
  2012-09-27 19:04 ` [PATCH 1/3] Revert "ata: make ata port as parent device of scsi host" Gwendal Grignou
@ 2012-09-29 17:08   ` Sergei Shtylyov
  2012-10-01 18:22     ` Gwendal Grignou
                       ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Sergei Shtylyov @ 2012-09-29 17:08 UTC (permalink / raw)
  To: Gwendal Grignou
  Cc: aaron.lu, minggr, tj, jgarzik, james.bottomley, linux-scsi,
	linux-ide

Hello.

On 27-09-2012 21:04, Gwendal Grignou wrote:

> This reverts commit 9a6d6a2ddabbd32c07f6a38b659e5f3db319fa5a.
>
> Instead, melt libata transport sysfs object in scsi objects.
>
> Change-Id: I8c709f63ddf7ba97b9e6f449d5c0b8b85e44e818

    Remove this line please, it has no place in the upstream commit.

> Signed-off-by: Gwendal Grignou<gwendal@google.com>
>

MBR, Sergei


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

* [PATCH 0/3] Insert ATA transport objects in SCSI syfs topology.
  2012-09-28  6:27   ` Aaron Lu
@ 2012-10-01 18:22     ` Gwendal Grignou
  2012-10-01 19:14       ` Dan Williams
  0 siblings, 1 reply; 15+ messages in thread
From: Gwendal Grignou @ 2012-10-01 18:22 UTC (permalink / raw)
  To: aaron.lu, minggr, tj, jgarzik, james.bottomley
  Cc: Gwendal Grignou, linux-scsi, linux-ide

This set of patches improve ATA transport classes integration with SCSI
objects.

Before [2.6.x]

Ata and scsi transport class where separated:
`--0000:09:00.0
|  `--ata1
|  |  `--port_port
|  |  `--link1
|  |  |  `--dev1.0
|  |  |  `--dev1.1
|  `--ata2
|  ...
|  `--host0
|  |  `--scsi_host
|  |  |  `--host0
|  |  `--target0:0:0
|  |  |  `--0:0:0:0
|  |  |  |  `--block
|  |  |  |  |  `--sda
|  |  |  |  |  |  `--sda1

In 3.2, Lin - in commit 9a6d6a2ddabbd32c07f6a38b659e5f3db319fa5a - addressed
the issue of linking the ata port with the scsi host object by placing the
scsi_host object under ata port objects.

However to be more consistent with other transport, this patch does the opposite:

For instance, with SAS transport, We have
`--0000:0b:00.0
|  `--host6
|  |  `--phy-6:0
|  |  `--phy-6:1
...
|  |  `--port-6:0
|  |  |  `--end_device-6:0
|  |  |  |  `--sas_device
|  |  |  |  |  `--end_device-6:0
|  |  |  |  `--sas_end_device
|  |  |  |  |  `--end_device-6:0
|  |  |  |  `--target6:0:0
|  |  |  |  |  `--6:0:0:0
|  |  |  |  |  |  `--block
|  |  |  |  |  |  |  `--sdb
...
|  |  `--port-6:1
|  |  |  `--end_device-6:1
...
phy and port have to be separated, sas_port are created dynamically.

For ata, all objects are created at initialization time, so the layout is:
`--0000:09:00.0
|  `--host0
|  |  `--port1
|  |  |  `--link1
|  |  |  |  `--dev1.0
|  |  |  |  |  `--target0:0:0
|  |  |  |  |  |  `--0:0:0:0
|  |  |  |  |  |  |  `--block
|  |  |  |  |  |  |  |  `--sda

If we have a port multiplier, more links are created.
`--0000:09:00.0
...
|  `--host4
|  |  `--port5
|  |  |  `--link5
|  |  |  |  `--dev5.0
[device for the port multiplier]
|  |  |  `--link5.0
|  |  |  |  `--dev5.0.0
|  |  |  |  |  `--target4:0:0
|  |  |  |  |  |  `--4:0:0:0
|  |  |  |  |  |  |  `--block
|  |  |  |  |  |  |  |  `--sdc
[disk in port 0 of the port multiplier]
...
|  |  |  `--link5.2
|  |  |  |  `--dev5.2.0
|  |  |  |  |  `--target4:2:0
|  |  |  |  |  |  `--4:2:0:0
|  |  |  |  |  |  |  `--block
|  |  |  |  |  |  |  |  `--sde
[disk in port 2 of the port multiplier]

In consequence, the path of a scsi device becomes:
.../0000:00:1f.2/host0/port1/link1/dev1.0/target0:0:0/0:0:0:0
dev1.0 indicates the master device [0] in ata port 1.
ata1 being under host0, we know the reliationships between the scsi_host id and
ata port id.

or when a port multiplier is present: for instance the device in port 4 of the
port multiplier:
.../0000:00:06.0/0000:09:00.0/host5/port6/link6.4/dev6.4.0/target5:4:0/5:4:0:0

Gwendal Grignou (3):
  Revert "ata: make ata port as parent device of scsi host"
  scsi: Allow devices to have arbitrary parent
  libata: Change transport topology layout

 drivers/ata/libata-core.c      |   13 ++++++-------
 drivers/ata/libata-scsi.c      |    4 ++--
 drivers/ata/libata-transport.c |    2 +-
 drivers/firewire/sbp2.c        |    3 ++-
 drivers/message/i2o/i2o_scsi.c |    4 ++--
 drivers/scsi/scsi_scan.c       |    9 +++++----
 include/scsi/scsi_device.h     |    2 +-
 7 files changed, 19 insertions(+), 18 deletions(-)

-- 
1.7.7.3


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

* [PATCH 1/3] Revert "ata: make ata port as parent device of scsi host"
  2012-09-29 17:08   ` Sergei Shtylyov
@ 2012-10-01 18:22     ` Gwendal Grignou
  2012-10-01 18:22     ` [PATCH 2/3] scsi: Allow devices to have arbitrary parent Gwendal Grignou
  2012-10-01 18:22     ` [PATCH 3/3] libata: Change transport topology layout Gwendal Grignou
  2 siblings, 0 replies; 15+ messages in thread
From: Gwendal Grignou @ 2012-10-01 18:22 UTC (permalink / raw)
  To: aaron.lu, minggr, tj, jgarzik, james.bottomley
  Cc: Gwendal Grignou, linux-scsi, linux-ide

This reverts commit 9a6d6a2ddabbd32c07f6a38b659e5f3db319fa5a.

Instead, melt libata transport sysfs object in scsi objects.

Signed-off-by: Gwendal Grignou <gwendal@google.com>
---
 drivers/ata/libata-scsi.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index e3bda07..be38930 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -3609,7 +3609,7 @@ int ata_scsi_add_hosts(struct ata_host *host, struct scsi_host_template *sht)
 		shost->max_host_blocked = 1;
 
 		rc = scsi_add_host_with_dma(ap->scsi_host,
-						&ap->tdev, ap->host->dev);
+					    host->dev, host->dev);
 		if (rc)
 			goto err_add;
 	}
-- 
1.7.7.3


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

* [PATCH 2/3] scsi: Allow devices to have arbitrary parent
  2012-09-29 17:08   ` Sergei Shtylyov
  2012-10-01 18:22     ` Gwendal Grignou
@ 2012-10-01 18:22     ` Gwendal Grignou
  2012-10-01 18:22     ` [PATCH 3/3] libata: Change transport topology layout Gwendal Grignou
  2 siblings, 0 replies; 15+ messages in thread
From: Gwendal Grignou @ 2012-10-01 18:22 UTC (permalink / raw)
  To: aaron.lu, minggr, tj, jgarzik, james.bottomley
  Cc: Gwendal Grignou, linux-scsi, linux-ide

Allow driver who calls __scsi_add_device directly to create the scsi
device on any parent, not just scsi_host directly.
This is alreay done for transport with their own class [SAS, iSCSI, FC, ...]

Signed-off-by: Gwendal Grignou <gwendal@google.com>
---
 drivers/ata/libata-scsi.c      |    4 ++--
 drivers/firewire/sbp2.c        |    3 ++-
 drivers/message/i2o/i2o_scsi.c |    4 ++--
 drivers/scsi/scsi_scan.c       |    9 +++++----
 include/scsi/scsi_device.h     |    2 +-
 5 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index be38930..bfda61f 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -3649,8 +3649,8 @@ 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);
+			sdev = __scsi_add_device(&ap->scsi_host->shost_gendev,
+						 channel, id, 0, NULL);
 			if (!IS_ERR(sdev)) {
 				dev->sdev = sdev;
 				scsi_device_put(sdev);
diff --git a/drivers/firewire/sbp2.c b/drivers/firewire/sbp2.c
index 1162d6b..839afa5 100644
--- a/drivers/firewire/sbp2.c
+++ b/drivers/firewire/sbp2.c
@@ -879,7 +879,8 @@ static void sbp2_login(struct work_struct *work)
 		ssleep(SBP2_INQUIRY_DELAY);
 
 	shost = container_of((void *)tgt, struct Scsi_Host, hostdata[0]);
-	sdev = __scsi_add_device(shost, 0, 0, sbp2_lun2int(lu->lun), lu);
+	sdev = __scsi_add_device(&shost->shost_gendev, 0, 0,
+				 sbp2_lun2int(lu->lun), lu);
 	/*
 	 * FIXME:  We are unable to perform reconnects while in sbp2_login().
 	 * Therefore __scsi_add_device() will get into trouble if a bus reset
diff --git a/drivers/message/i2o/i2o_scsi.c b/drivers/message/i2o/i2o_scsi.c
index 1d31d72..ee1353c 100644
--- a/drivers/message/i2o/i2o_scsi.c
+++ b/drivers/message/i2o/i2o_scsi.c
@@ -294,8 +294,8 @@ static int i2o_scsi_probe(struct device *dev)
 	}
 
 	scsi_dev =
-	    __scsi_add_device(i2o_shost->scsi_host, channel, le32_to_cpu(id),
-			      le64_to_cpu(lun), i2o_dev);
+	    __scsi_add_device(&i2o_shost->scsi_host->shost_gendev, channel,
+			      le32_to_cpu(id), le64_to_cpu(lun), i2o_dev);
 
 	if (IS_ERR(scsi_dev)) {
 		osm_warn("can not add SCSI device %03x\n",
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 56a9379..105123c 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -1489,11 +1489,11 @@ static int scsi_report_lun_scan(struct scsi_target *starget, int bflags,
 	return ret;
 }
 
-struct scsi_device *__scsi_add_device(struct Scsi_Host *shost, uint channel,
+struct scsi_device *__scsi_add_device(struct device *parent, uint channel,
 				      uint id, uint lun, void *hostdata)
 {
+	struct Scsi_Host *shost = dev_to_shost(parent);
 	struct scsi_device *sdev = ERR_PTR(-ENODEV);
-	struct device *parent = &shost->shost_gendev;
 	struct scsi_target *starget;
 
 	if (strncmp(scsi_scan_type, "none", 4) == 0)
@@ -1524,8 +1524,9 @@ EXPORT_SYMBOL(__scsi_add_device);
 int scsi_add_device(struct Scsi_Host *host, uint channel,
 		    uint target, uint lun)
 {
-	struct scsi_device *sdev = 
-		__scsi_add_device(host, channel, target, lun, NULL);
+	struct scsi_device *sdev =
+		__scsi_add_device(&host->shost_gendev, channel, target,
+				  lun, NULL);
 	if (IS_ERR(sdev))
 		return PTR_ERR(sdev);
 
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 9895f69..9646a1d 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -285,7 +285,7 @@ static inline struct scsi_target *scsi_target(struct scsi_device *sdev)
 #define starget_printk(prefix, starget, fmt, a...)	\
 	dev_printk(prefix, &(starget)->dev, fmt, ##a)
 
-extern struct scsi_device *__scsi_add_device(struct Scsi_Host *,
+extern struct scsi_device *__scsi_add_device(struct device *,
 		uint, uint, uint, void *hostdata);
 extern int scsi_add_device(struct Scsi_Host *host, uint channel,
 			   uint target, uint lun);
-- 
1.7.7.3


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

* [PATCH 3/3] libata: Change transport topology layout
  2012-09-29 17:08   ` Sergei Shtylyov
  2012-10-01 18:22     ` Gwendal Grignou
  2012-10-01 18:22     ` [PATCH 2/3] scsi: Allow devices to have arbitrary parent Gwendal Grignou
@ 2012-10-01 18:22     ` Gwendal Grignou
  2 siblings, 0 replies; 15+ messages in thread
From: Gwendal Grignou @ 2012-10-01 18:22 UTC (permalink / raw)
  To: aaron.lu, minggr, tj, jgarzik, james.bottomley
  Cc: Gwendal Grignou, linux-scsi, linux-ide

Integrate ata objects [port, link, device] with scsi objects.


The path of a scsi device is:
.../0000:00:1f.2/host0/port1/link1/dev1.0/target0:0:0/0:0:0:0

or when a port multiplier is present: for instance the device in port 4 of the
port multiplier:
.../0000:00:06.0/0000:09:00.0/host5/port6/link6.4/dev6.4.0/target5:4:0/5:4:0:0

Signed-off-by: Gwendal Grignou <gwendal@google.com>
---
 drivers/ata/libata-core.c      |   13 ++++++-------
 drivers/ata/libata-scsi.c      |    4 ++--
 drivers/ata/libata-transport.c |    2 +-
 3 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 611050d..c83590b 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -6063,19 +6063,18 @@ int ata_host_register(struct ata_host *host, struct scsi_host_template *sht)
 	for (i = 0; i < host->n_ports; i++)
 		host->ports[i]->print_id = atomic_inc_return(&ata_print_id);
 
+	rc = ata_scsi_add_hosts(host, sht);
+	if (rc)
+		return rc;
 
 	/* Create associated sysfs transport objects  */
 	for (i = 0; i < host->n_ports; i++) {
-		rc = ata_tport_add(host->dev,host->ports[i]);
-		if (rc) {
+		struct ata_port *ap = host->ports[i];
+		rc = ata_tport_add(&ap->scsi_host->shost_gendev, ap);
+		if (rc)
 			goto err_tadd;
-		}
 	}
 
-	rc = ata_scsi_add_hosts(host, sht);
-	if (rc)
-		goto err_tadd;
-
 	/* set cable, sata_spd_limit and report */
 	for (i = 0; i < host->n_ports; i++) {
 		struct ata_port *ap = host->ports[i];
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index bfda61f..9023bb1 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -3649,8 +3649,8 @@ void ata_scsi_scan_host(struct ata_port *ap, int sync)
 			else
 				channel = link->pmp;
 
-			sdev = __scsi_add_device(&ap->scsi_host->shost_gendev,
-						 channel, id, 0, NULL);
+			sdev = __scsi_add_device(&dev->tdev, channel, id, 0,
+						 NULL);
 			if (!IS_ERR(sdev)) {
 				dev->sdev = sdev;
 				scsi_device_put(sdev);
diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c
index c04d393..6829be6 100644
--- a/drivers/ata/libata-transport.c
+++ b/drivers/ata/libata-transport.c
@@ -284,7 +284,7 @@ int ata_tport_add(struct device *parent,
 
 	dev->parent = get_device(parent);
 	dev->release = ata_tport_release;
-	dev_set_name(dev, "ata%d", ap->print_id);
+	dev_set_name(dev, "port%d", ap->print_id);
 	transport_setup_device(dev);
 	error = device_add(dev);
 	if (error) {
-- 
1.7.7.3


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

* Re: [PATCH 0/3] Insert ATA transport objects in SCSI syfs topology.
  2012-10-01 18:22     ` Gwendal Grignou
@ 2012-10-01 19:14       ` Dan Williams
  2012-10-04 16:56         ` Gwendal Grignou
  0 siblings, 1 reply; 15+ messages in thread
From: Dan Williams @ 2012-10-01 19:14 UTC (permalink / raw)
  To: Gwendal Grignou
  Cc: aaron.lu, minggr, tj, jgarzik, james.bottomley, linux-scsi,
	linux-ide

On Mon, Oct 1, 2012 at 11:22 AM, Gwendal Grignou <gwendal@google.com> wrote:
> This set of patches improve ATA transport classes integration with SCSI
> objects.
>
> Before [2.6.x]
>
> Ata and scsi transport class where separated:
> `--0000:09:00.0
> |  `--ata1
> |  |  `--port_port
> |  |  `--link1
> |  |  |  `--dev1.0
> |  |  |  `--dev1.1
> |  `--ata2
> |  ...
> |  `--host0
> |  |  `--scsi_host
> |  |  |  `--host0
> |  |  `--target0:0:0
> |  |  |  `--0:0:0:0
> |  |  |  |  `--block
> |  |  |  |  |  `--sda
> |  |  |  |  |  |  `--sda1
>
> In 3.2, Lin - in commit 9a6d6a2ddabbd32c07f6a38b659e5f3db319fa5a - addressed
> the issue of linking the ata port with the scsi host object by placing the
> scsi_host object under ata port objects.
>
> However to be more consistent with other transport, this patch does the opposite:
>
> For instance, with SAS transport, We have
> `--0000:0b:00.0
> |  `--host6
> |  |  `--phy-6:0
> |  |  `--phy-6:1
> ...
> |  |  `--port-6:0
> |  |  |  `--end_device-6:0
> |  |  |  |  `--sas_device
> |  |  |  |  |  `--end_device-6:0
> |  |  |  |  `--sas_end_device
> |  |  |  |  |  `--end_device-6:0
> |  |  |  |  `--target6:0:0
> |  |  |  |  |  `--6:0:0:0
> |  |  |  |  |  |  `--block
> |  |  |  |  |  |  |  `--sdb
> ...
> |  |  `--port-6:1
> |  |  |  `--end_device-6:1
> ...
> phy and port have to be separated, sas_port are created dynamically.
>
> For ata, all objects are created at initialization time, so the layout is:
> `--0000:09:00.0
> |  `--host0
> |  |  `--port1
> |  |  |  `--link1
> |  |  |  |  `--dev1.0
> |  |  |  |  |  `--target0:0:0
> |  |  |  |  |  |  `--0:0:0:0
> |  |  |  |  |  |  |  `--block
> |  |  |  |  |  |  |  |  `--sda
>
> If we have a port multiplier, more links are created.
> `--0000:09:00.0
> ...
> |  `--host4
> |  |  `--port5
> |  |  |  `--link5
> |  |  |  |  `--dev5.0
> [device for the port multiplier]
> |  |  |  `--link5.0
> |  |  |  |  `--dev5.0.0
> |  |  |  |  |  `--target4:0:0
> |  |  |  |  |  |  `--4:0:0:0
> |  |  |  |  |  |  |  `--block
> |  |  |  |  |  |  |  |  `--sdc
> [disk in port 0 of the port multiplier]
> ...
> |  |  |  `--link5.2
> |  |  |  |  `--dev5.2.0
> |  |  |  |  |  `--target4:2:0
> |  |  |  |  |  |  `--4:2:0:0
> |  |  |  |  |  |  |  `--block
> |  |  |  |  |  |  |  |  `--sde
> [disk in port 2 of the port multiplier]
>
> In consequence, the path of a scsi device becomes:
> .../0000:00:1f.2/host0/port1/link1/dev1.0/target0:0:0/0:0:0:0
> dev1.0 indicates the master device [0] in ata port 1.
> ata1 being under host0, we know the reliationships between the scsi_host id and
> ata port id.
>
> or when a port multiplier is present: for instance the device in port 4 of the
> port multiplier:
> .../0000:00:06.0/0000:09:00.0/host5/port6/link6.4/dev6.4.0/target5:4:0/5:4:0:0

What's the benefit of this?  From a PM perspective now it seems we'll
have the parent hardware port suspended before all the scsi_hosts on
that port, which defeats the original purpose of putting the ata_port
in the PM hierarchy.

--
Dan

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

* [PATCH 3/3] libata: Change transport topology layout
  2012-09-28  6:38   ` Aaron Lu
@ 2012-10-04  0:49     ` Gwendal Grignou
  0 siblings, 0 replies; 15+ messages in thread
From: Gwendal Grignou @ 2012-10-04  0:49 UTC (permalink / raw)
  To: aaron.lu, tj, jgarzik, james.bottomley
  Cc: Gwendal Grignou, linux-scsi, linux-ide

Integrate ata objects [port, link, device] with scsi objects.

Before [2.6.x]

The path of a scsi device is:
.../0000:00:1f.2/host0/port1/link1/dev1.0/target0:0:0/0:0:0:0

or when a port multiplier is present: for instance the device in port 4 of the
port multiplier:
.../0000:00:06.0/0000:09:00.0/host5/port6/link6.4/dev6.4.0/target5:4:0/5:4:0:0

Fix ACPI code that relied on previous topology. 


Signed-off-by: Gwendal Grignou <gwendal@google.com>
---
 drivers/ata/libata-acpi.c      |   11 ++++++++---
 drivers/ata/libata-core.c      |   13 ++++++-------
 drivers/ata/libata-scsi.c      |    4 ++--
 drivers/ata/libata-transport.c |   10 +++++++++-
 4 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index fd9ecf7..8e7f451 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -1065,7 +1065,7 @@ void ata_acpi_unbind(struct ata_device *dev)
 
 static int compat_pci_ata(struct ata_port *ap)
 {
-	struct device *dev = ap->tdev.parent;
+	struct device *dev = ap->host->dev;
 	struct pci_dev *pdev;
 
 	if (!is_pci_dev(dev))
@@ -1085,7 +1085,7 @@ static int ata_acpi_bind_host(struct ata_port *ap, acpi_handle *handle)
 	if (ap->flags & ATA_FLAG_ACPI_SATA)
 		return -ENODEV;
 
-	*handle = acpi_get_child(DEVICE_ACPI_HANDLE(ap->tdev.parent),
+	*handle = acpi_get_child(DEVICE_ACPI_HANDLE(ap->host->dev),
 			ap->port_no);
 
 	if (!*handle)
@@ -1150,7 +1150,12 @@ static struct ata_port *dev_to_ata_port(struct device *dev)
 
 static int ata_acpi_find_device(struct device *dev, acpi_handle *handle)
 {
-	struct ata_port *ap = dev_to_ata_port(dev);
+	struct ata_port *ap;
+
+	if (scsi_is_host_device(dev))
+		ap = ata_shost_to_port(dev_to_shost(dev));
+	else
+		ap = dev_to_ata_port(dev);
 
 	if (!ap)
 		return -ENODEV;
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 611050d..c83590b 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -6063,19 +6063,18 @@ int ata_host_register(struct ata_host *host, struct scsi_host_template *sht)
 	for (i = 0; i < host->n_ports; i++)
 		host->ports[i]->print_id = atomic_inc_return(&ata_print_id);
 
+	rc = ata_scsi_add_hosts(host, sht);
+	if (rc)
+		return rc;
 
 	/* Create associated sysfs transport objects  */
 	for (i = 0; i < host->n_ports; i++) {
-		rc = ata_tport_add(host->dev,host->ports[i]);
-		if (rc) {
+		struct ata_port *ap = host->ports[i];
+		rc = ata_tport_add(&ap->scsi_host->shost_gendev, ap);
+		if (rc)
 			goto err_tadd;
-		}
 	}
 
-	rc = ata_scsi_add_hosts(host, sht);
-	if (rc)
-		goto err_tadd;
-
 	/* set cable, sata_spd_limit and report */
 	for (i = 0; i < host->n_ports; i++) {
 		struct ata_port *ap = host->ports[i];
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index bfda61f..9023bb1 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -3649,8 +3649,8 @@ void ata_scsi_scan_host(struct ata_port *ap, int sync)
 			else
 				channel = link->pmp;
 
-			sdev = __scsi_add_device(&ap->scsi_host->shost_gendev,
-						 channel, id, 0, NULL);
+			sdev = __scsi_add_device(&dev->tdev, channel, id, 0,
+						 NULL);
 			if (!IS_ERR(sdev)) {
 				dev->sdev = sdev;
 				scsi_device_put(sdev);
diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c
index c04d393..ce91bd2 100644
--- a/drivers/ata/libata-transport.c
+++ b/drivers/ata/libata-transport.c
@@ -284,7 +284,7 @@ int ata_tport_add(struct device *parent,
 
 	dev->parent = get_device(parent);
 	dev->release = ata_tport_release;
-	dev_set_name(dev, "ata%d", ap->print_id);
+	dev_set_name(dev, "port%d", ap->print_id);
 	transport_setup_device(dev);
 	error = device_add(dev);
 	if (error) {
@@ -421,6 +421,10 @@ int ata_tlink_add(struct ata_link *link)
 		goto tlink_err;
 	}
 
+	pm_runtime_no_callbacks(dev);
+	pm_runtime_set_active(dev);
+	pm_runtime_enable(dev);
+
 	transport_add_device(dev);
 	transport_configure_device(dev);
 
@@ -649,6 +653,10 @@ static int ata_tdev_add(struct ata_device *ata_dev)
 		return error;
 	}
 
+	pm_runtime_no_callbacks(dev);
+	pm_runtime_set_active(dev);
+	pm_runtime_enable(dev);
+
 	transport_add_device(dev);
 	transport_configure_device(dev);
 	return 0;
-- 
1.7.7.3


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

* Re: [PATCH 0/3] Insert ATA transport objects in SCSI syfs topology.
  2012-10-01 19:14       ` Dan Williams
@ 2012-10-04 16:56         ` Gwendal Grignou
  2012-10-07 23:13           ` Dan Williams
  0 siblings, 1 reply; 15+ messages in thread
From: Gwendal Grignou @ 2012-10-04 16:56 UTC (permalink / raw)
  To: Dan Williams
  Cc: aaron.lu, minggr, tj, jgarzik, james.bottomley, linux-scsi,
	linux-ide

On Mon, Oct 1, 2012 at 12:14 PM, Dan Williams <djbw@fb.com> wrote:
> On Mon, Oct 1, 2012 at 11:22 AM, Gwendal Grignou <gwendal@google.com> wrote:
>> This set of patches improve ATA transport classes integration with SCSI
>> objects.
>>
>> Before [2.6.x]
>>
>> Ata and scsi transport class where separated:
>> `--0000:09:00.0
>> |  `--ata1
>> |  |  `--port_port
>> |  |  `--link1
>> |  |  |  `--dev1.0
>> |  |  |  `--dev1.1
>> |  `--ata2
>> |  ...
>> |  `--host0
>> |  |  `--scsi_host
>> |  |  |  `--host0
>> |  |  `--target0:0:0
>> |  |  |  `--0:0:0:0
>> |  |  |  |  `--block
>> |  |  |  |  |  `--sda
>> |  |  |  |  |  |  `--sda1
>>
>> In 3.2, Lin - in commit 9a6d6a2ddabbd32c07f6a38b659e5f3db319fa5a - addressed
>> the issue of linking the ata port with the scsi host object by placing the
>> scsi_host object under ata port objects.
>>
>> However to be more consistent with other transport, this patch does the opposite:
>>
>> For instance, with SAS transport, We have
>> `--0000:0b:00.0
>> |  `--host6
>> |  |  `--phy-6:0
>> |  |  `--phy-6:1
>> ...
>> |  |  `--port-6:0
>> |  |  |  `--end_device-6:0
>> |  |  |  |  `--sas_device
>> |  |  |  |  |  `--end_device-6:0
>> |  |  |  |  `--sas_end_device
>> |  |  |  |  |  `--end_device-6:0
>> |  |  |  |  `--target6:0:0
>> |  |  |  |  |  `--6:0:0:0
>> |  |  |  |  |  |  `--block
>> |  |  |  |  |  |  |  `--sdb
>> ...
>> |  |  `--port-6:1
>> |  |  |  `--end_device-6:1
>> ...
>> phy and port have to be separated, sas_port are created dynamically.
>>
>> For ata, all objects are created at initialization time, so the layout is:
>> `--0000:09:00.0
>> |  `--host0
>> |  |  `--port1
>> |  |  |  `--link1
>> |  |  |  |  `--dev1.0
>> |  |  |  |  |  `--target0:0:0
>> |  |  |  |  |  |  `--0:0:0:0
>> |  |  |  |  |  |  |  `--block
>> |  |  |  |  |  |  |  |  `--sda
>>
>> If we have a port multiplier, more links are created.
>> `--0000:09:00.0
>> ...
>> |  `--host4
>> |  |  `--port5
>> |  |  |  `--link5
>> |  |  |  |  `--dev5.0
>> [device for the port multiplier]
>> |  |  |  `--link5.0
>> |  |  |  |  `--dev5.0.0
>> |  |  |  |  |  `--target4:0:0
>> |  |  |  |  |  |  `--4:0:0:0
>> |  |  |  |  |  |  |  `--block
>> |  |  |  |  |  |  |  |  `--sdc
>> [disk in port 0 of the port multiplier]
>> ...
>> |  |  |  `--link5.2
>> |  |  |  |  `--dev5.2.0
>> |  |  |  |  |  `--target4:2:0
>> |  |  |  |  |  |  `--4:2:0:0
>> |  |  |  |  |  |  |  `--block
>> |  |  |  |  |  |  |  |  `--sde
>> [disk in port 2 of the port multiplier]
>>
>> In consequence, the path of a scsi device becomes:
>> .../0000:00:1f.2/host0/port1/link1/dev1.0/target0:0:0/0:0:0:0
>> dev1.0 indicates the master device [0] in ata port 1.
>> ata1 being under host0, we know the reliationships between the scsi_host id and
>> ata port id.
>>
>> or when a port multiplier is present: for instance the device in port 4 of the
>> port multiplier:
>> .../0000:00:06.0/0000:09:00.0/host5/port6/link6.4/dev6.4.0/target5:4:0/5:4:0:0
>
> What's the benefit of this?
+ To unify ata transport sysfs topology with other scsi transport.
+ To easily map a ata_port with its associated scsi_host structure.

> From a PM perspective now it seems we'll
> have the parent hardware port suspended before all the scsi_hosts on
> that port,
There is a one to one mapping between ata port and scsi-host, so it works.
All ata ports must be suspended before the parent hardware is.

Gwendal.
> which defeats the original purpose of putting the ata_port
> in the PM hierarchy.
>
> --
> Dan

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

* Re: [PATCH 0/3] Insert ATA transport objects in SCSI syfs topology.
  2012-10-04 16:56         ` Gwendal Grignou
@ 2012-10-07 23:13           ` Dan Williams
  0 siblings, 0 replies; 15+ messages in thread
From: Dan Williams @ 2012-10-07 23:13 UTC (permalink / raw)
  To: Gwendal Grignou
  Cc: aaron.lu, minggr, tj, jgarzik, james.bottomley, linux-scsi,
	linux-ide

On Thu, Oct 4, 2012 at 9:56 AM, Gwendal Grignou <gwendal@google.com> wrote:
>> What's the benefit of this?
> + To unify ata transport sysfs topology with other scsi transport.

My concern is the thrash and breakage to switch the ordering around
given the (minor) growing pains injecting an ata_port into the device
path caused.  Although, it seems like Aaron has caught where this
reversal broke things at the cost of some additional special-casing (4
files changed, 25 insertions(+), 13 deletions(-)).  Patch 1 also
creates a problem for bisections as the code that assumes
<dev>/port/host will break.

I don't know... I'm all for consistency, but if the only justification
is to make the transports look the "same" we'll still have a glaring
transport difference between ata and sas.  In the sas case one
hba/host spanning all possible sas domains vs the ata case of a
guaranteed ata_port per "ata domain" relationship with at least one
host per port.  The "port" does live higher in the topology in the ata
case.

> + To easily map a ata_port with its associated scsi_host structure.

Not sure this is getting any easier.  There would now be three options
based on kernel version: look for the ata_port as a host attribute,
look at the host's parent, or look for the host's child.

--
Dan

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

end of thread, other threads:[~2012-10-07 23:13 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20120920020145.GA23210@mint-spring.sh.intel.com>
2012-09-27 19:04 ` [PATCH 0/3] Insert ATA transport objects in SCSI syfs topology Gwendal Grignou
2012-09-28  6:27   ` Aaron Lu
2012-10-01 18:22     ` Gwendal Grignou
2012-10-01 19:14       ` Dan Williams
2012-10-04 16:56         ` Gwendal Grignou
2012-10-07 23:13           ` Dan Williams
2012-09-27 19:04 ` [PATCH 1/3] Revert "ata: make ata port as parent device of scsi host" Gwendal Grignou
2012-09-29 17:08   ` Sergei Shtylyov
2012-10-01 18:22     ` Gwendal Grignou
2012-10-01 18:22     ` [PATCH 2/3] scsi: Allow devices to have arbitrary parent Gwendal Grignou
2012-10-01 18:22     ` [PATCH 3/3] libata: Change transport topology layout Gwendal Grignou
2012-09-27 19:04 ` [PATCH 2/3] scsi: Allow devices to have arbitrary parent Gwendal Grignou
2012-09-27 19:04 ` [PATCH 3/3] libata: Change transport topology layout Gwendal Grignou
2012-09-28  6:38   ` Aaron Lu
2012-10-04  0:49     ` Gwendal Grignou

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).