| Message ID | 20211101144740.14256-1-pali@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] lspci: Show Slot Power Limit values above EFh | expand |
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]); }
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]); > } >
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 --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));