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

["Pali Rohár" <pali@kernel.org>]

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]);
> }
>