Assign the unique id used for printing earlier

ata_sas_port_alloc() calls ata_port_alloc() which already assigns ap->lock so there is no need to ata_sas_port_alloc() to assign it again.

Signed-off-by Niklas Cassel <[email protected]>

---
 drivers/ata/libata-sata.c | 1 -
 1 file changed, 1 deletion(-)

drivers/ata/libata-sata.c +0 -1

index 9e047bf912b1..c564eac9d430 100644
@@ -1228,7 +1228,6 @@ struct ata_port *ata_sas_port_alloc(struct ata_host *host,
 		return NULL;
 
 	ap->port_no = 0;
-	ap->lock = &host->lock;
 	ap->pio_mask = port_info->pio_mask;
 	ap->mwdma_mask = port_info->mwdma_mask;
 	ap->udma_mask = port_info->udma_mask;

While the assignment of ap->print_id could have been moved to ata_host_alloc(), let's simply move it to ata_port_alloc().

If you allocate a port, you want to give it a unique name that can be used for printing.

By moving the ap->print_id assignment to ata_port_alloc(), means that we can also remove the ap->print_id assignment from ata_sas_port_alloc().

This will allow a LLD to use the ata_port_*() print functions before ata_host_register() has been called.

Signed-off-by Niklas Cassel <[email protected]>

---
 drivers/ata/libata-core.c | 6 ++----
 drivers/ata/libata-sata.c | 1 -
 2 files changed, 2 insertions(+), 5 deletions(-)

drivers/ata/libata-core.c +1 -1

index 0dc6e18bf492..fb4835c2ba2d 100644
@@ -5463,7 +5463,7 @@ struct ata_port *ata_port_alloc(struct ata_host *host)
 
 	ap->pflags |= ATA_PFLAG_INITIALIZING | ATA_PFLAG_FROZEN;
 	ap->lock = &host->lock;
-	ap->print_id = -1;
+	ap->print_id = atomic_inc_return(&ata_print_id);
 	ap->local_port_no = -1;
 	ap->host = host;
 	ap->dev = host->dev;

drivers/ata/libata-core.c +1 -3

-	for (i = 0; i < host->n_ports; i++) {
-		host->ports[i]->print_id = atomic_inc_return(&ata_print_id);
+	for (i = 0; i < host->n_ports; i++)
 		host->ports[i]->local_port_no = i + 1;
-	}
 
 	/* Create associated sysfs transport objects  */
 	for (i = 0; i < host->n_ports; i++) {

drivers/ata/libata-sata.c +0 -1

index c564eac9d430..def0026188f7 100644
@@ -1234,7 +1234,6 @@ struct ata_port *ata_sas_port_alloc(struct ata_host *host,
 	ap->flags |= port_info->flags;
 	ap->ops = port_info->port_ops;
 	ap->cbl = ATA_CBL_SATA;
-	ap->print_id = atomic_inc_return(&ata_print_id);
 
 	return ap;
 }

While the assignment of ap->print_id could have been moved to ata_host_alloc(), let's simply move it to ata_port_alloc().

If you allocate a port, you want to give it a unique name that can be used for printing.

By moving the ap->print_id assignment to ata_port_alloc(), means that we can also remove the ap->print_id assignment from ata_sas_port_alloc().

This will allow a LLD to use the ata_port_*() print functions before ata_host_register() has been called.

Signed-off-by Niklas Cassel <[email protected]>

---
 drivers/ata/libata-core.c | 6 ++----
 drivers/ata/libata-sata.c | 1 -
 2 files changed, 2 insertions(+), 5 deletions(-)

drivers/ata/libata-core.c +2 -4

index 0dc6e18bf492..fb4835c2ba2d 100644
@@ -5463,7 +5463,7 @@ struct ata_port *ata_port_alloc(struct ata_host *host)
 
 	ap->pflags |= ATA_PFLAG_INITIALIZING | ATA_PFLAG_FROZEN;
 	ap->lock = &host->lock;
-	ap->print_id = -1;
+	ap->print_id = atomic_inc_return(&ata_print_id);
 	ap->local_port_no = -1;
 	ap->host = host;
 	ap->dev = host->dev;

@@ -5903,10 +5903,8 @@ int ata_host_register(struct ata_host *host, const struct scsi_host_template *sh
 		WARN_ON(host->ports[i]);
 
 	/* give ports names and add SCSI hosts */
-	for (i = 0; i < host->n_ports; i++) {
-		host->ports[i]->print_id = atomic_inc_return(&ata_print_id);
+	for (i = 0; i < host->n_ports; i++)
 		host->ports[i]->local_port_no = i + 1;
-	}
 
 	/* Create associated sysfs transport objects  */
 	for (i = 0; i < host->n_ports; i++) {

drivers/ata/libata-sata.c +0 -1

index c564eac9d430..def0026188f7 100644
@@ -1234,7 +1234,6 @@ struct ata_port *ata_sas_port_alloc(struct ata_host *host,
 	ap->flags |= port_info->flags;
 	ap->ops = port_info->port_ops;
 	ap->cbl = ATA_CBL_SATA;
-	ap->print_id = atomic_inc_return(&ata_print_id);
 
 	return ap;
 }

Now when we have removed support for decreasing the number of ports, the most logical thing is to assign local_port_no at the same location(s) where we assign port_no.

Note that we cannot move the local_port_no assignment to ata_port_alloc(), because not all users of ata_port_alloc() assigns local_port_no (i.e. ata_sas_port_alloc()).

I have no idea what local_port_no is actually used for, but at least there should be no functional changes caused by this commit.

Signed-off-by Niklas Cassel <[email protected]>

---
 drivers/ata/libata-core.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

drivers/ata/libata-core.c +1 -4

index fb4835c2ba2d..ee1a75bc0466 100644
@@ -5591,6 +5591,7 @@ struct ata_host *ata_host_alloc(struct device *dev, int n_ports)
 			goto err_out;
 
 		ap->port_no = i;
+		ap->local_port_no = i + 1;
 		host->ports[i] = ap;
 	}
 

@@ -5902,10 +5903,6 @@ int ata_host_register(struct ata_host *host, const struct scsi_host_template *sh
 	for (i = host->n_ports; host->ports[i]; i++)
 		WARN_ON(host->ports[i]);
 
-	/* give ports names and add SCSI hosts */
-	for (i = 0; i < host->n_ports; i++)
-		host->ports[i]->local_port_no = i + 1;
-
 	/* Create associated sysfs transport objects  */
 	for (i = 0; i < host->n_ports; i++) {
 		rc = ata_tport_add(host->dev,host->ports[i]);

So the confusion here is the naming: ap->print_id is an ID increasing for all adapters, ap->port_no is the index of the port in the host (adapter) array of ports and local_port_no is the same + 1...

So I think we can get rid of local_port_no by simply rewriting:

ata_port_simple_attr(local_port_no, port_no, "%u\n", unsigned int);

in libata-transport.c. That will avoid this useless and confusing code.

Add a debug print that tells us if LPM is not getting enabled because the port is external.

Signed-off-by Niklas Cassel <[email protected]>