diff mbox series

[v3] ahci: print the number of implemented ports

Message ID 20240219204744.1365567-1-cassel@kernel.org
State New
Headers show
Series [v3] ahci: print the number of implemented ports | expand

Commit Message

Niklas Cassel Feb. 19, 2024, 8:47 p.m. UTC
We are currently printing the CAP.NP field.
CAP.NP is a 0's based value indicating the maximum number of ports
supported by the HBA silicon. Note that the number of ports indicated
in this field may be more than the number of ports indicated in the
PI (ports implemented) register. (See AHCI 1.3.1, section 3.1.1 -
Offset 00h: CAP – HBA Capabilities.)

PI (ports implemented) register is a field that has a bit set to '1'
if that specific port is implemented. This register is allowed to have
zeroes mixed with ones, i.e. a port in the middle is allowed to be
unimplemented. (See AHCI 1.3.1, section 3.1.4 - Offset 0Ch: PI – Ports
Implemented.)

Since the number of ports implemented might be smaller than the maximum
number of ports supported by the HBA silicon, print the number of
implemented ports as well.

While at it, clarify the properties being printed, and add a separator
(,), as that is currently missing, making it very easy to get confused
if the number before or after <property> belongs to <property>.

before:
ahci 0000:00:03.0: masking port_map 0x3f -> 0x2f
ahci 0000:00:03.0: AHCI 0001.0000 32 slots 6 ports 1.5 Gbps 0x2f impl SATA mode
ahci 0000:00:03.0: flags: 64bit ncq only

after:
ahci 0000:00:03.0: masking port_map 0x3f -> 0x2f
ahci 0000:00:03.0: AHCI vers 0001.0000, 32 command slots, 1.5 Gbps, SATA mode
ahci 0000:00:03.0: 5/6 ports implemented (port mask 0x2f)
ahci 0000:00:03.0: flags: 64bit ncq only

Suggested-by: Damien Le Moal <dlemoal@kernel.org>
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
Changes since V2:
-Print both number of implemented ports and max number of ports.
-Clarify the properties being printed.
-Add a separator (,) between the printed properties.
-Prints two lines instead of one, as it would otherwise be a very
 long line being printed.

 drivers/ata/libahci.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

Comments

Damien Le Moal Feb. 21, 2024, 5:31 a.m. UTC | #1
On 2/20/24 05:47, Niklas Cassel wrote:
> We are currently printing the CAP.NP field.
> CAP.NP is a 0's based value indicating the maximum number of ports
> supported by the HBA silicon. Note that the number of ports indicated
> in this field may be more than the number of ports indicated in the
> PI (ports implemented) register. (See AHCI 1.3.1, section 3.1.1 -
> Offset 00h: CAP – HBA Capabilities.)
> 
> PI (ports implemented) register is a field that has a bit set to '1'
> if that specific port is implemented. This register is allowed to have
> zeroes mixed with ones, i.e. a port in the middle is allowed to be
> unimplemented. (See AHCI 1.3.1, section 3.1.4 - Offset 0Ch: PI – Ports
> Implemented.)
> 
> Since the number of ports implemented might be smaller than the maximum
> number of ports supported by the HBA silicon, print the number of
> implemented ports as well.
> 
> While at it, clarify the properties being printed, and add a separator
> (,), as that is currently missing, making it very easy to get confused
> if the number before or after <property> belongs to <property>.
> 
> before:
> ahci 0000:00:03.0: masking port_map 0x3f -> 0x2f
> ahci 0000:00:03.0: AHCI 0001.0000 32 slots 6 ports 1.5 Gbps 0x2f impl SATA mode
> ahci 0000:00:03.0: flags: 64bit ncq only
> 
> after:
> ahci 0000:00:03.0: masking port_map 0x3f -> 0x2f
> ahci 0000:00:03.0: AHCI vers 0001.0000, 32 command slots, 1.5 Gbps, SATA mode
> ahci 0000:00:03.0: 5/6 ports implemented (port mask 0x2f)
> ahci 0000:00:03.0: flags: 64bit ncq only
> 
> Suggested-by: Damien Le Moal <dlemoal@kernel.org>
> Signed-off-by: Niklas Cassel <cassel@kernel.org>

Nice !

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Niklas Cassel Feb. 21, 2024, 6:16 p.m. UTC | #2
On Mon, Feb 19, 2024 at 09:47:43PM +0100, Niklas Cassel wrote:
> We are currently printing the CAP.NP field.
> CAP.NP is a 0's based value indicating the maximum number of ports
> supported by the HBA silicon. Note that the number of ports indicated
> in this field may be more than the number of ports indicated in the
> PI (ports implemented) register. (See AHCI 1.3.1, section 3.1.1 -
> Offset 00h: CAP – HBA Capabilities.)
> 
> PI (ports implemented) register is a field that has a bit set to '1'
> if that specific port is implemented. This register is allowed to have
> zeroes mixed with ones, i.e. a port in the middle is allowed to be
> unimplemented. (See AHCI 1.3.1, section 3.1.4 - Offset 0Ch: PI – Ports
> Implemented.)
> 
> Since the number of ports implemented might be smaller than the maximum
> number of ports supported by the HBA silicon, print the number of
> implemented ports as well.
> 
> While at it, clarify the properties being printed, and add a separator
> (,), as that is currently missing, making it very easy to get confused
> if the number before or after <property> belongs to <property>.
> 
> before:
> ahci 0000:00:03.0: masking port_map 0x3f -> 0x2f
> ahci 0000:00:03.0: AHCI 0001.0000 32 slots 6 ports 1.5 Gbps 0x2f impl SATA mode
> ahci 0000:00:03.0: flags: 64bit ncq only
> 
> after:
> ahci 0000:00:03.0: masking port_map 0x3f -> 0x2f
> ahci 0000:00:03.0: AHCI vers 0001.0000, 32 command slots, 1.5 Gbps, SATA mode
> ahci 0000:00:03.0: 5/6 ports implemented (port mask 0x2f)
> ahci 0000:00:03.0: flags: 64bit ncq only
> 
> Suggested-by: Damien Le Moal <dlemoal@kernel.org>
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---

Applied:
https://git.kernel.org/pub/scm/linux/kernel/git/libata/linux.git/log/?h=for-6.9
diff mbox series

Patch

diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index fca376f03c9e..83431aae74d8 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -2620,8 +2620,8 @@  void ahci_print_info(struct ata_host *host, const char *scc_s)
 		speed_s = "?";
 
 	dev_info(host->dev,
-		"AHCI %02x%02x.%02x%02x "
-		"%u slots %u ports %s Gbps 0x%x impl %s mode\n"
+		"AHCI vers %02x%02x.%02x%02x, "
+		"%u command slots, %s Gbps, %s mode\n"
 		,
 
 		(vers >> 24) & 0xff,
@@ -2630,11 +2630,17 @@  void ahci_print_info(struct ata_host *host, const char *scc_s)
 		vers & 0xff,
 
 		((cap >> 8) & 0x1f) + 1,
-		(cap & 0x1f) + 1,
 		speed_s,
-		impl,
 		scc_s);
 
+	dev_info(host->dev,
+		"%u/%u ports implemented (port mask 0x%x)\n"
+		,
+
+		hweight32(impl),
+		(cap & 0x1f) + 1,
+		impl);
+
 	dev_info(host->dev,
 		"flags: "
 		"%s%s%s%s%s%s%s"