diff mbox series

[v2] lspci: Show Slot Power Limit values above EFh

Message ID 20211101144740.14256-1-pali@kernel.org
State New
Headers show
Series [v2] lspci: Show Slot Power Limit values above EFh | expand

Commit Message

Pali Rohár Nov. 1, 2021, 2:47 p.m. UTC 
PCI Express Base Specification rev. 3.0 has the following definition for
the Slot Power Limit Value:

=======================================================================
When the Slot Power Limit Scale field equals 00b (1.0x) and Slot Power
Limit Value exceeds EFh, the following alternative encodings are used:
  F0h = 250 W Slot Power Limit
  F1h = 275 W Slot Power Limit
  F2h = 300 W Slot Power Limit
  F3h to FFh = Reserved for Slot Power Limit values above 300 W
=======================================================================

Replace function power_limit() by show_power_limit() which also prints
power limit value. Show reserved value as string ">300W" and omit usage of
floating point variables as it is not needed.
---
 ls-caps.c | 38 ++++++++++++++++++++++++++++----------
 1 file changed, 28 insertions(+), 10 deletions(-)

Comments

Matthew Wilcox Nov. 1, 2021, 3:03 p.m. UTC | #1 
On Mon, Nov 01, 2021 at 03:47:40PM +0100, Pali Rohár wrote:
> PCI Express Base Specification rev. 3.0 has the following definition for
> the Slot Power Limit Value:
> 
> =======================================================================
> When the Slot Power Limit Scale field equals 00b (1.0x) and Slot Power
> Limit Value exceeds EFh, the following alternative encodings are used:
>   F0h = 250 W Slot Power Limit
>   F1h = 275 W Slot Power Limit
>   F2h = 300 W Slot Power Limit
>   F3h to FFh = Reserved for Slot Power Limit values above 300 W
> =======================================================================
> 
> Replace function power_limit() by show_power_limit() which also prints
> power limit value. Show reserved value as string ">300W" and omit usage of
> floating point variables as it is not needed.

I don't understand why you want to avoid the use of floating point here?

> +++ b/ls-caps.c
> @@ -656,10 +656,27 @@ static int exp_downstream_port(int type)
>  	 type == PCI_EXP_TYPE_PCIE_BRIDGE;	/* PCI/PCI-X to PCIe Bridge */
>  }
>  
> -static float power_limit(int value, int scale)
> +static void show_power_limit(int value, int scale)
>  {
> -  static const float scales[4] = { 1.0, 0.1, 0.01, 0.001 };
> -  return value * scales[scale];
> +  static const int scales[4] = { 1000, 100, 10, 1 };
> +  static const int scale0_values[3] = { 250, 275, 300 };
> +  if (scale == 0 && value >= 0xF0) {
> +    /* F3h to FFh = Reserved for Slot Power Limit values above 300 W */
> +    if (value >= 0xF3) {
> +      printf(">300W");
> +      return;
> +    }
> +    value = scale0_values[value - 0xF0];
> +  }
> +  value *= scales[scale];
> +  printf("%d", value / 1000);
> +  if (value % 10)
> +    printf(".%03d", value % 1000);
> +  else if (value % 100)
> +    printf(".%02d", (value / 10) % 100);
> +  else if (value % 1000)
> +    printf(".%d", (value / 100) % 10);
> +  printf("W");

Wouldn't this be clearer if written as:

static void show_power_limit(int value, int scale)
{
  static const float scales[4] = { 1.0, 0.1, 0.01, 0.001 };
  static const int scale0_values[3] = { 250, 275, 300 };

  if (scale == 0 && value >= 0xF0) {
    /* F3h to FFh = Reserved for Slot Power Limit values above 300 W */
    if (value >= 0xF3) {
      printf(">300W");
      return;
    }
    value = scale0_values[value - 0xF0];
  }
  printf("%.3fW", value * scales[scale]);
}
Pali Rohár Nov. 24, 2021, 12:46 p.m. UTC | #2 
On Monday 01 November 2021 15:03:31 Matthew Wilcox wrote:
> On Mon, Nov 01, 2021 at 03:47:40PM +0100, Pali Rohár wrote:
> > PCI Express Base Specification rev. 3.0 has the following definition for
> > the Slot Power Limit Value:
> > 
> > =======================================================================
> > When the Slot Power Limit Scale field equals 00b (1.0x) and Slot Power
> > Limit Value exceeds EFh, the following alternative encodings are used:
> >   F0h = 250 W Slot Power Limit
> >   F1h = 275 W Slot Power Limit
> >   F2h = 300 W Slot Power Limit
> >   F3h to FFh = Reserved for Slot Power Limit values above 300 W
> > =======================================================================
> > 
> > Replace function power_limit() by show_power_limit() which also prints
> > power limit value. Show reserved value as string ">300W" and omit usage of
> > floating point variables as it is not needed.
> 
> I don't understand why you want to avoid the use of floating point here?

Because library does not use floating point. So I thought that it is a
good idea to not use it neither for printing power limit.

I can change it, just I wanted to hear project / library preference.

> > +++ b/ls-caps.c
> > @@ -656,10 +656,27 @@ static int exp_downstream_port(int type)
> >  	 type == PCI_EXP_TYPE_PCIE_BRIDGE;	/* PCI/PCI-X to PCIe Bridge */
> >  }
> >  
> > -static float power_limit(int value, int scale)
> > +static void show_power_limit(int value, int scale)
> >  {
> > -  static const float scales[4] = { 1.0, 0.1, 0.01, 0.001 };
> > -  return value * scales[scale];
> > +  static const int scales[4] = { 1000, 100, 10, 1 };
> > +  static const int scale0_values[3] = { 250, 275, 300 };
> > +  if (scale == 0 && value >= 0xF0) {
> > +    /* F3h to FFh = Reserved for Slot Power Limit values above 300 W */
> > +    if (value >= 0xF3) {
> > +      printf(">300W");
> > +      return;
> > +    }
> > +    value = scale0_values[value - 0xF0];
> > +  }
> > +  value *= scales[scale];
> > +  printf("%d", value / 1000);
> > +  if (value % 10)
> > +    printf(".%03d", value % 1000);
> > +  else if (value % 100)
> > +    printf(".%02d", (value / 10) % 100);
> > +  else if (value % 1000)
> > +    printf(".%d", (value / 100) % 10);
> > +  printf("W");
> 
> Wouldn't this be clearer if written as:
> 
> static void show_power_limit(int value, int scale)
> {
>   static const float scales[4] = { 1.0, 0.1, 0.01, 0.001 };
>   static const int scale0_values[3] = { 250, 275, 300 };
> 
>   if (scale == 0 && value >= 0xF0) {
>     /* F3h to FFh = Reserved for Slot Power Limit values above 300 W */
>     if (value >= 0xF3) {
>       printf(">300W");
>       return;
>     }
>     value = scale0_values[value - 0xF0];
>   }
>   printf("%.3fW", value * scales[scale]);
> }
>
Martin Mareš Dec. 26, 2021, 10:07 p.m. UTC | #3 
Hi!

> Because library does not use floating point. So I thought that it is a
> good idea to not use it neither for printing power limit.
> 
> I can change it, just I wanted to hear project / library preference.

Floating point in lspci is perfectly fine and it's the preferred solution.

I would hesitate for a moment before using it in the library, but lspci
is definitely OK.

					Martin
diff mbox series

Patch

diff --git a/ls-caps.c b/ls-caps.c
index db56556971cb..7fa6c1da45bd 100644
--- a/ls-caps.c
+++ b/ls-caps.c
@@ -656,10 +656,27 @@  static int exp_downstream_port(int type)
 	 type == PCI_EXP_TYPE_PCIE_BRIDGE;	/* PCI/PCI-X to PCIe Bridge */
 }
 
-static float power_limit(int value, int scale)
+static void show_power_limit(int value, int scale)
 {
-  static const float scales[4] = { 1.0, 0.1, 0.01, 0.001 };
-  return value * scales[scale];
+  static const int scales[4] = { 1000, 100, 10, 1 };
+  static const int scale0_values[3] = { 250, 275, 300 };
+  if (scale == 0 && value >= 0xF0) {
+    /* F3h to FFh = Reserved for Slot Power Limit values above 300 W */
+    if (value >= 0xF3) {
+      printf(">300W");
+      return;
+    }
+    value = scale0_values[value - 0xF0];
+  }
+  value *= scales[scale];
+  printf("%d", value / 1000);
+  if (value % 10)
+    printf(".%03d", value % 1000);
+  else if (value % 100)
+    printf(".%02d", (value / 10) % 100);
+  else if (value % 1000)
+    printf(".%d", (value / 100) % 10);
+  printf("W");
 }
 
 static const char *latency_l0s(int value)
@@ -700,10 +717,10 @@  static void cap_express_dev(struct device *d, int where, int type)
     printf(" FLReset%c",
 	FLAG(t, PCI_EXP_DEVCAP_FLRESET));
   if ((type == PCI_EXP_TYPE_ENDPOINT) || (type == PCI_EXP_TYPE_UPSTREAM) ||
-      (type == PCI_EXP_TYPE_PCI_BRIDGE))
-    printf(" SlotPowerLimit %.3fW",
-	power_limit((t & PCI_EXP_DEVCAP_PWR_VAL) >> 18,
-		    (t & PCI_EXP_DEVCAP_PWR_SCL) >> 26));
+      (type == PCI_EXP_TYPE_PCI_BRIDGE)) {
+    printf(" SlotPowerLimit ");
+    show_power_limit((t & PCI_EXP_DEVCAP_PWR_VAL) >> 18, (t & PCI_EXP_DEVCAP_PWR_SCL) >> 26);
+  }
   printf("\n");
 
   w = get_conf_word(d, where + PCI_EXP_DEVCTL);
@@ -871,9 +888,10 @@  static void cap_express_slot(struct device *d, int where)
 	FLAG(t, PCI_EXP_SLTCAP_PWRI),
 	FLAG(t, PCI_EXP_SLTCAP_HPC),
 	FLAG(t, PCI_EXP_SLTCAP_HPS));
-  printf("\t\t\tSlot #%d, PowerLimit %.3fW; Interlock%c NoCompl%c\n",
-	(t & PCI_EXP_SLTCAP_PSN) >> 19,
-	power_limit((t & PCI_EXP_SLTCAP_PWR_VAL) >> 7, (t & PCI_EXP_SLTCAP_PWR_SCL) >> 15),
+  printf("\t\t\tSlot #%d, PowerLimit ",
+	(t & PCI_EXP_SLTCAP_PSN) >> 19);
+  show_power_limit((t & PCI_EXP_SLTCAP_PWR_VAL) >> 7, (t & PCI_EXP_SLTCAP_PWR_SCL) >> 15);
+  printf("; Interlock%c NoCompl%c\n",
 	FLAG(t, PCI_EXP_SLTCAP_INTERLOCK),
 	FLAG(t, PCI_EXP_SLTCAP_NOCMDCOMP));