diff mbox series

[4/6] PCI: Add function for parsing 'slot-power-limit-milliwatt' DT property

Message ID 20220222163158.1666-5-pali@kernel.org (mailing list archive)
State New, archived
Headers show
Series PCI: mvebu: Slot support | expand

Commit Message

Pali Rohár Feb. 22, 2022, 4:31 p.m. UTC
Add function of_pci_get_slot_power_limit(), which parses the
'slot-power-limit-milliwatt' DT property, returning the value in
milliwatts and in format ready for the PCIe Slot Capabilities Register.

Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Marek Behún <kabel@kernel.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/of.c  | 64 +++++++++++++++++++++++++++++++++++++++++++++++
 drivers/pci/pci.h | 15 +++++++++++
 2 files changed, 79 insertions(+)

Comments

Bjorn Helgaas Feb. 24, 2022, 8:47 p.m. UTC | #1
On Tue, Feb 22, 2022 at 05:31:56PM +0100, Pali Rohár wrote:
> Add function of_pci_get_slot_power_limit(), which parses the
> 'slot-power-limit-milliwatt' DT property, returning the value in
> milliwatts and in format ready for the PCIe Slot Capabilities Register.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Signed-off-by: Marek Behún <kabel@kernel.org>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
>  drivers/pci/of.c  | 64 +++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/pci/pci.h | 15 +++++++++++
>  2 files changed, 79 insertions(+)
> 
> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index cb2e8351c2cc..2b0c0a3641a8 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -633,3 +633,67 @@ int of_pci_get_max_link_speed(struct device_node *node)
>  	return max_link_speed;
>  }
>  EXPORT_SYMBOL_GPL(of_pci_get_max_link_speed);
> +
> +/**
> + * of_pci_get_slot_power_limit - Parses the "slot-power-limit-milliwatt"
> + *				 property.
> + *
> + * @node: device tree node with the slot power limit information
> + * @slot_power_limit_value: pointer where the value should be stored in PCIe
> + *			    Slot Capabilities Register format
> + * @slot_power_limit_scale: pointer where the scale should be stored in PCIe
> + *			    Slot Capabilities Register format
> + *
> + * Returns the slot power limit in milliwatts and if @slot_power_limit_value
> + * and @slot_power_limit_scale pointers are non-NULL, fills in the value and
> + * scale in format used by PCIe Slot Capabilities Register.
> + *
> + * If the property is not found or is invalid, returns 0.
> + */
> +u32 of_pci_get_slot_power_limit(struct device_node *node,
> +				u8 *slot_power_limit_value,
> +				u8 *slot_power_limit_scale)
> +{
> +	u32 slot_power_limit;

Including "mw" or similar reference to the units would give a hint of
how to relate the code to the spec.

> +	u8 value, scale;
> +
> +	if (of_property_read_u32(node, "slot-power-limit-milliwatt",
> +				 &slot_power_limit))
> +		slot_power_limit = 0;
> +
> +	/* Calculate Slot Power Limit Value and Slot Power Limit Scale */

Add a spec reference to PCIe r6.0, sec 7.5.3.9.  IIUC, this supports
up to 300W, which was what r5.0 defined, but r6.0 added values up to
0xfe (600W).

> +	if (slot_power_limit == 0) {
> +		value = 0x00;
> +		scale = 0;
> +	} else if (slot_power_limit <= 255) {
> +		value = slot_power_limit;
> +		scale = 3;
> +	} else if (slot_power_limit <= 255*10) {
> +		value = slot_power_limit / 10;
> +		scale = 2;
> +	} else if (slot_power_limit <= 255*100) {
> +		value = slot_power_limit / 100;
> +		scale = 1;
> +	} else if (slot_power_limit <= 239*1000) {
> +		value = slot_power_limit / 1000;
> +		scale = 0;
> +	} else if (slot_power_limit <= 250*1000) {
> +		value = 0xF0;
> +		scale = 0;
> +	} else if (slot_power_limit <= 275*1000) {
> +		value = 0xF1;
> +		scale = 0;
> +	} else {
> +		value = 0xF2;
> +		scale = 0;
> +	}
> +
> +	if (slot_power_limit_value)
> +		*slot_power_limit_value = value;
> +
> +	if (slot_power_limit_scale)
> +		*slot_power_limit_scale = scale;
> +
> +	return slot_power_limit;

If "slot-power-limit-milliwatt" contains a value larger than can be
represented in "value" and "scale", the return value will not agree
with value/scale, will it?

Currently you only use the return value for a log message, so no real
harm yet, other than the fact that we might print "Slot power limit
1000.0W" when the hardware will only advertise 600W available.

Also, if "slot-power-limit-milliwatt" contains something like
260000 mW (260 W), we'll return 0xF1/0, so the hardware will
advertise 275 W available.

> +}
> +EXPORT_SYMBOL_GPL(of_pci_get_slot_power_limit);
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 3d60cabde1a1..e10cdec6c56e 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -627,6 +627,9 @@ struct device_node;
>  int of_pci_parse_bus_range(struct device_node *node, struct resource *res);
>  int of_get_pci_domain_nr(struct device_node *node);
>  int of_pci_get_max_link_speed(struct device_node *node);
> +u32 of_pci_get_slot_power_limit(struct device_node *node,
> +				u8 *slot_power_limit_value,
> +				u8 *slot_power_limit_scale);
>  void pci_set_of_node(struct pci_dev *dev);
>  void pci_release_of_node(struct pci_dev *dev);
>  void pci_set_bus_of_node(struct pci_bus *bus);
> @@ -653,6 +656,18 @@ of_pci_get_max_link_speed(struct device_node *node)
>  	return -EINVAL;
>  }
>  
> +static inline u32
> +of_pci_get_slot_power_limit(struct device_node *node,
> +			    u8 *slot_power_limit_value,
> +			    u8 *slot_power_limit_scale)
> +{
> +	if (slot_power_limit_value)
> +		*slot_power_limit_value = 0;
> +	if (slot_power_limit_scale)
> +		*slot_power_limit_scale = 0;
> +	return 0;
> +}
> +
>  static inline void pci_set_of_node(struct pci_dev *dev) { }
>  static inline void pci_release_of_node(struct pci_dev *dev) { }
>  static inline void pci_set_bus_of_node(struct pci_bus *bus) { }
> -- 
> 2.20.1
>
Pali Rohár Feb. 25, 2022, 12:30 p.m. UTC | #2
On Thursday 24 February 2022 14:47:15 Bjorn Helgaas wrote:
> On Tue, Feb 22, 2022 at 05:31:56PM +0100, Pali Rohár wrote:
> > Add function of_pci_get_slot_power_limit(), which parses the
> > 'slot-power-limit-milliwatt' DT property, returning the value in
> > milliwatts and in format ready for the PCIe Slot Capabilities Register.
> > 
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > Signed-off-by: Marek Behún <kabel@kernel.org>
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> > ---
> >  drivers/pci/of.c  | 64 +++++++++++++++++++++++++++++++++++++++++++++++
> >  drivers/pci/pci.h | 15 +++++++++++
> >  2 files changed, 79 insertions(+)
> > 
> > diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> > index cb2e8351c2cc..2b0c0a3641a8 100644
> > --- a/drivers/pci/of.c
> > +++ b/drivers/pci/of.c
> > @@ -633,3 +633,67 @@ int of_pci_get_max_link_speed(struct device_node *node)
> >  	return max_link_speed;
> >  }
> >  EXPORT_SYMBOL_GPL(of_pci_get_max_link_speed);
> > +
> > +/**
> > + * of_pci_get_slot_power_limit - Parses the "slot-power-limit-milliwatt"
> > + *				 property.
> > + *
> > + * @node: device tree node with the slot power limit information
> > + * @slot_power_limit_value: pointer where the value should be stored in PCIe
> > + *			    Slot Capabilities Register format
> > + * @slot_power_limit_scale: pointer where the scale should be stored in PCIe
> > + *			    Slot Capabilities Register format
> > + *
> > + * Returns the slot power limit in milliwatts and if @slot_power_limit_value
> > + * and @slot_power_limit_scale pointers are non-NULL, fills in the value and
> > + * scale in format used by PCIe Slot Capabilities Register.
> > + *
> > + * If the property is not found or is invalid, returns 0.
> > + */
> > +u32 of_pci_get_slot_power_limit(struct device_node *node,
> > +				u8 *slot_power_limit_value,
> > +				u8 *slot_power_limit_scale)
> > +{
> > +	u32 slot_power_limit;
> 
> Including "mw" or similar reference to the units would give a hint of
> how to relate the code to the spec.
> 
> > +	u8 value, scale;
> > +
> > +	if (of_property_read_u32(node, "slot-power-limit-milliwatt",
> > +				 &slot_power_limit))
> > +		slot_power_limit = 0;
> > +
> > +	/* Calculate Slot Power Limit Value and Slot Power Limit Scale */
> 
> Add a spec reference to PCIe r6.0, sec 7.5.3.9.  IIUC, this supports
> up to 300W, which was what r5.0 defined, but r6.0 added values up to
> 0xfe (600W).

I did not know about it and I have not seen/read r6.0.

It would be nice if somebody with access to r6.0 send a patch to lspci
utility, so we could write support for 600W based on lspci parser.

> > +	if (slot_power_limit == 0) {
> > +		value = 0x00;
> > +		scale = 0;
> > +	} else if (slot_power_limit <= 255) {
> > +		value = slot_power_limit;
> > +		scale = 3;
> > +	} else if (slot_power_limit <= 255*10) {
> > +		value = slot_power_limit / 10;
> > +		scale = 2;
> > +	} else if (slot_power_limit <= 255*100) {
> > +		value = slot_power_limit / 100;
> > +		scale = 1;
> > +	} else if (slot_power_limit <= 239*1000) {
> > +		value = slot_power_limit / 1000;
> > +		scale = 0;
> > +	} else if (slot_power_limit <= 250*1000) {
> > +		value = 0xF0;
> > +		scale = 0;
> > +	} else if (slot_power_limit <= 275*1000) {
> > +		value = 0xF1;
> > +		scale = 0;
> > +	} else {
> > +		value = 0xF2;
> > +		scale = 0;
> > +	}
> > +
> > +	if (slot_power_limit_value)
> > +		*slot_power_limit_value = value;
> > +
> > +	if (slot_power_limit_scale)
> > +		*slot_power_limit_scale = scale;
> > +
> > +	return slot_power_limit;
> 
> If "slot-power-limit-milliwatt" contains a value larger than can be
> represented in "value" and "scale", the return value will not agree
> with value/scale, will it?

In previous version 0xF2 was reserved for values above 275 W. So for me
it looked like a correct solution.

> Currently you only use the return value for a log message, so no real
> harm yet, other than the fact that we might print "Slot power limit
> 1000.0W" when the hardware will only advertise 600W available.
> 
> Also, if "slot-power-limit-milliwatt" contains something like
> 260000 mW (260 W), we'll return 0xF1/0, so the hardware will
> advertise 275 W available.

There is no way how to encode 260 W. It is possible only 250 W or 275 W,
and nothing between. I chose to round value to upper limit. What do you
prefer in these cases? Upper or lower limit?

> > +}
> > +EXPORT_SYMBOL_GPL(of_pci_get_slot_power_limit);
> > diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> > index 3d60cabde1a1..e10cdec6c56e 100644
> > --- a/drivers/pci/pci.h
> > +++ b/drivers/pci/pci.h
> > @@ -627,6 +627,9 @@ struct device_node;
> >  int of_pci_parse_bus_range(struct device_node *node, struct resource *res);
> >  int of_get_pci_domain_nr(struct device_node *node);
> >  int of_pci_get_max_link_speed(struct device_node *node);
> > +u32 of_pci_get_slot_power_limit(struct device_node *node,
> > +				u8 *slot_power_limit_value,
> > +				u8 *slot_power_limit_scale);
> >  void pci_set_of_node(struct pci_dev *dev);
> >  void pci_release_of_node(struct pci_dev *dev);
> >  void pci_set_bus_of_node(struct pci_bus *bus);
> > @@ -653,6 +656,18 @@ of_pci_get_max_link_speed(struct device_node *node)
> >  	return -EINVAL;
> >  }
> >  
> > +static inline u32
> > +of_pci_get_slot_power_limit(struct device_node *node,
> > +			    u8 *slot_power_limit_value,
> > +			    u8 *slot_power_limit_scale)
> > +{
> > +	if (slot_power_limit_value)
> > +		*slot_power_limit_value = 0;
> > +	if (slot_power_limit_scale)
> > +		*slot_power_limit_scale = 0;
> > +	return 0;
> > +}
> > +
> >  static inline void pci_set_of_node(struct pci_dev *dev) { }
> >  static inline void pci_release_of_node(struct pci_dev *dev) { }
> >  static inline void pci_set_bus_of_node(struct pci_bus *bus) { }
> > -- 
> > 2.20.1
> >
Bjorn Helgaas Feb. 25, 2022, 3:51 p.m. UTC | #3
On Fri, Feb 25, 2022 at 01:30:51PM +0100, Pali Rohár wrote:
> On Thursday 24 February 2022 14:47:15 Bjorn Helgaas wrote:
> > On Tue, Feb 22, 2022 at 05:31:56PM +0100, Pali Rohár wrote:
> > > Add function of_pci_get_slot_power_limit(), which parses the
> > > 'slot-power-limit-milliwatt' DT property, returning the value in
> > > milliwatts and in format ready for the PCIe Slot Capabilities Register.
> > > 
> > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > Signed-off-by: Marek Behún <kabel@kernel.org>
> > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> > > ---
> > >  drivers/pci/of.c  | 64 +++++++++++++++++++++++++++++++++++++++++++++++
> > >  drivers/pci/pci.h | 15 +++++++++++
> > >  2 files changed, 79 insertions(+)
> > > 
> > > diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> > > index cb2e8351c2cc..2b0c0a3641a8 100644
> > > --- a/drivers/pci/of.c
> > > +++ b/drivers/pci/of.c
> > > @@ -633,3 +633,67 @@ int of_pci_get_max_link_speed(struct device_node *node)
> > >  	return max_link_speed;
> > >  }
> > >  EXPORT_SYMBOL_GPL(of_pci_get_max_link_speed);
> > > +
> > > +/**
> > > + * of_pci_get_slot_power_limit - Parses the "slot-power-limit-milliwatt"
> > > + *				 property.
> > > + *
> > > + * @node: device tree node with the slot power limit information
> > > + * @slot_power_limit_value: pointer where the value should be stored in PCIe
> > > + *			    Slot Capabilities Register format
> > > + * @slot_power_limit_scale: pointer where the scale should be stored in PCIe
> > > + *			    Slot Capabilities Register format
> > > + *
> > > + * Returns the slot power limit in milliwatts and if @slot_power_limit_value
> > > + * and @slot_power_limit_scale pointers are non-NULL, fills in the value and
> > > + * scale in format used by PCIe Slot Capabilities Register.
> > > + *
> > > + * If the property is not found or is invalid, returns 0.
> > > + */
> > > +u32 of_pci_get_slot_power_limit(struct device_node *node,
> > > +				u8 *slot_power_limit_value,
> > > +				u8 *slot_power_limit_scale)
> > > +{
> > > +	u32 slot_power_limit;
> > 
> > Including "mw" or similar reference to the units would give a hint of
> > how to relate the code to the spec.
> > 
> > > +	u8 value, scale;
> > > +
> > > +	if (of_property_read_u32(node, "slot-power-limit-milliwatt",
> > > +				 &slot_power_limit))
> > > +		slot_power_limit = 0;
> > > +
> > > +	/* Calculate Slot Power Limit Value and Slot Power Limit Scale */
> > 
> > Add a spec reference to PCIe r6.0, sec 7.5.3.9.  IIUC, this supports
> > up to 300W, which was what r5.0 defined, but r6.0 added values up to
> > 0xfe (600W).
> 
> I did not know about it and I have not seen/read r6.0.
> 
> It would be nice if somebody with access to r6.0 send a patch to lspci
> utility, so we could write support for 600W based on lspci parser.

Of course, sorry!  Obviously you would have implemented them all if
you had the spec!

Here's the info from r6.0, sec 7.5.3.9:

  Slot Power Limit Value - In combination with the Slot Power Limit
  Scale value, specifies the upper limit on power supplied by the slot
  (see § Section 6.9) or by other means to the adapter.

  Power limit (in Watts) is calculated by multiplying the value in
  this field by the value in the Slot Power Limit Scale field except
  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   > 239 W and ≤ 250 W Slot Power Limit
    F1h   > 250 W and ≤ 275 W Slot Power Limit
    F2h   > 275 W and ≤ 300 W Slot Power Limit
    F3h   > 300 W and ≤ 325 W Slot Power Limit
    F4h   > 325 W and ≤ 350 W Slot Power Limit
    F5h   > 350 W and ≤ 375 W Slot Power Limit
    F6h   > 375 W and ≤ 400 W Slot Power Limit
    F7h   > 400 W and ≤ 425 W Slot Power Limit
    F8h   > 425 W and ≤ 450 W Slot Power Limit
    F9h   > 450 W and ≤ 475 W Slot Power Limit
    FAh   > 475 W and ≤ 500 W Slot Power Limit
    FBh   > 500 W and ≤ 525 W Slot Power Limit
    FCh   > 525 W and ≤ 550 W Slot Power Limit
    FDh   > 550 W and ≤ 575 W Slot Power Limit
    FEh   > 575 W and ≤ 600 W Slot Power Limit
    FFh   Reserved for Slot Power Limit Values above 600 W

  This register must be implemented if the Slot Implemented bit is Set.

  Writes to this register also cause the Port to send the
  Set_Slot_Power_Limit Message.

> > > +	if (slot_power_limit == 0) {
> > > +		value = 0x00;
> > > +		scale = 0;
> > > +	} else if (slot_power_limit <= 255) {
> > > +		value = slot_power_limit;
> > > +		scale = 3;
> > > +	} else if (slot_power_limit <= 255*10) {
> > > +		value = slot_power_limit / 10;
> > > +		scale = 2;
> > > +	} else if (slot_power_limit <= 255*100) {
> > > +		value = slot_power_limit / 100;
> > > +		scale = 1;
> > > +	} else if (slot_power_limit <= 239*1000) {
> > > +		value = slot_power_limit / 1000;
> > > +		scale = 0;
> > > +	} else if (slot_power_limit <= 250*1000) {
> > > +		value = 0xF0;
> > > +		scale = 0;
> > > +	} else if (slot_power_limit <= 275*1000) {
> > > +		value = 0xF1;
> > > +		scale = 0;
> > > +	} else {
> > > +		value = 0xF2;
> > > +		scale = 0;
> > > +	}
> > > +
> > > +	if (slot_power_limit_value)
> > > +		*slot_power_limit_value = value;
> > > +
> > > +	if (slot_power_limit_scale)
> > > +		*slot_power_limit_scale = scale;
> > > +
> > > +	return slot_power_limit;
> > 
> > If "slot-power-limit-milliwatt" contains a value larger than can be
> > represented in "value" and "scale", the return value will not agree
> > with value/scale, will it?
> 
> In previous version 0xF2 was reserved for values above 275 W. So for me
> it looked like a correct solution.
> 
> > Currently you only use the return value for a log message, so no real
> > harm yet, other than the fact that we might print "Slot power limit
> > 1000.0W" when the hardware will only advertise 600W available.
> > 
> > Also, if "slot-power-limit-milliwatt" contains something like
> > 260000 mW (260 W), we'll return 0xF1/0, so the hardware will
> > advertise 275 W available.
> 
> There is no way how to encode 260 W. It is possible only 250 W or 275 W,
> and nothing between. I chose to round value to upper limit. What do you
> prefer in these cases? Upper or lower limit?

I think rounding down is better.  If we round up, the slot will
advertise more power than it can deliver, and if the device tries to
consume the amount of power advertised, it may not work reliably.

So I think we should return encoded values that are no higher than
what the slot can actually deliver, and the return value should match
what Slot Capabilities advertises.

Bjorn
Pali Rohár Feb. 25, 2022, 5:58 p.m. UTC | #4
On Friday 25 February 2022 09:51:56 Bjorn Helgaas wrote:
> On Fri, Feb 25, 2022 at 01:30:51PM +0100, Pali Rohár wrote:
> > On Thursday 24 February 2022 14:47:15 Bjorn Helgaas wrote:
> > > On Tue, Feb 22, 2022 at 05:31:56PM +0100, Pali Rohár wrote:
> > > > Add function of_pci_get_slot_power_limit(), which parses the
> > > > 'slot-power-limit-milliwatt' DT property, returning the value in
> > > > milliwatts and in format ready for the PCIe Slot Capabilities Register.
> > > > 
> > > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > > Signed-off-by: Marek Behún <kabel@kernel.org>
> > > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > > Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> > > > ---
> > > >  drivers/pci/of.c  | 64 +++++++++++++++++++++++++++++++++++++++++++++++
> > > >  drivers/pci/pci.h | 15 +++++++++++
> > > >  2 files changed, 79 insertions(+)
> > > > 
> > > > diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> > > > index cb2e8351c2cc..2b0c0a3641a8 100644
> > > > --- a/drivers/pci/of.c
> > > > +++ b/drivers/pci/of.c
> > > > @@ -633,3 +633,67 @@ int of_pci_get_max_link_speed(struct device_node *node)
> > > >  	return max_link_speed;
> > > >  }
> > > >  EXPORT_SYMBOL_GPL(of_pci_get_max_link_speed);
> > > > +
> > > > +/**
> > > > + * of_pci_get_slot_power_limit - Parses the "slot-power-limit-milliwatt"
> > > > + *				 property.
> > > > + *
> > > > + * @node: device tree node with the slot power limit information
> > > > + * @slot_power_limit_value: pointer where the value should be stored in PCIe
> > > > + *			    Slot Capabilities Register format
> > > > + * @slot_power_limit_scale: pointer where the scale should be stored in PCIe
> > > > + *			    Slot Capabilities Register format
> > > > + *
> > > > + * Returns the slot power limit in milliwatts and if @slot_power_limit_value
> > > > + * and @slot_power_limit_scale pointers are non-NULL, fills in the value and
> > > > + * scale in format used by PCIe Slot Capabilities Register.
> > > > + *
> > > > + * If the property is not found or is invalid, returns 0.
> > > > + */
> > > > +u32 of_pci_get_slot_power_limit(struct device_node *node,
> > > > +				u8 *slot_power_limit_value,
> > > > +				u8 *slot_power_limit_scale)
> > > > +{
> > > > +	u32 slot_power_limit;
> > > 
> > > Including "mw" or similar reference to the units would give a hint of
> > > how to relate the code to the spec.
> > > 
> > > > +	u8 value, scale;
> > > > +
> > > > +	if (of_property_read_u32(node, "slot-power-limit-milliwatt",
> > > > +				 &slot_power_limit))
> > > > +		slot_power_limit = 0;
> > > > +
> > > > +	/* Calculate Slot Power Limit Value and Slot Power Limit Scale */
> > > 
> > > Add a spec reference to PCIe r6.0, sec 7.5.3.9.  IIUC, this supports
> > > up to 300W, which was what r5.0 defined, but r6.0 added values up to
> > > 0xfe (600W).
> > 
> > I did not know about it and I have not seen/read r6.0.
> > 
> > It would be nice if somebody with access to r6.0 send a patch to lspci
> > utility, so we could write support for 600W based on lspci parser.
> 
> Of course, sorry!  Obviously you would have implemented them all if
> you had the spec!
> 
> Here's the info from r6.0, sec 7.5.3.9:
> 
>   Slot Power Limit Value - In combination with the Slot Power Limit
>   Scale value, specifies the upper limit on power supplied by the slot
>   (see § Section 6.9) or by other means to the adapter.
> 
>   Power limit (in Watts) is calculated by multiplying the value in
>   this field by the value in the Slot Power Limit Scale field except
>   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   > 239 W and ≤ 250 W Slot Power Limit
>     F1h   > 250 W and ≤ 275 W Slot Power Limit
>     F2h   > 275 W and ≤ 300 W Slot Power Limit
>     F3h   > 300 W and ≤ 325 W Slot Power Limit
>     F4h   > 325 W and ≤ 350 W Slot Power Limit
>     F5h   > 350 W and ≤ 375 W Slot Power Limit
>     F6h   > 375 W and ≤ 400 W Slot Power Limit
>     F7h   > 400 W and ≤ 425 W Slot Power Limit
>     F8h   > 425 W and ≤ 450 W Slot Power Limit
>     F9h   > 450 W and ≤ 475 W Slot Power Limit
>     FAh   > 475 W and ≤ 500 W Slot Power Limit
>     FBh   > 500 W and ≤ 525 W Slot Power Limit
>     FCh   > 525 W and ≤ 550 W Slot Power Limit
>     FDh   > 550 W and ≤ 575 W Slot Power Limit
>     FEh   > 575 W and ≤ 600 W Slot Power Limit
>     FFh   Reserved for Slot Power Limit Values above 600 W
> 
>   This register must be implemented if the Slot Implemented bit is Set.
> 
>   Writes to this register also cause the Port to send the
>   Set_Slot_Power_Limit Message.

Ok, thank you!

I will send also update for lspci.

> > > > +	if (slot_power_limit == 0) {
> > > > +		value = 0x00;
> > > > +		scale = 0;
> > > > +	} else if (slot_power_limit <= 255) {
> > > > +		value = slot_power_limit;
> > > > +		scale = 3;
> > > > +	} else if (slot_power_limit <= 255*10) {
> > > > +		value = slot_power_limit / 10;
> > > > +		scale = 2;
> > > > +	} else if (slot_power_limit <= 255*100) {
> > > > +		value = slot_power_limit / 100;
> > > > +		scale = 1;
> > > > +	} else if (slot_power_limit <= 239*1000) {
> > > > +		value = slot_power_limit / 1000;
> > > > +		scale = 0;
> > > > +	} else if (slot_power_limit <= 250*1000) {
> > > > +		value = 0xF0;
> > > > +		scale = 0;
> > > > +	} else if (slot_power_limit <= 275*1000) {
> > > > +		value = 0xF1;
> > > > +		scale = 0;
> > > > +	} else {
> > > > +		value = 0xF2;
> > > > +		scale = 0;
> > > > +	}
> > > > +
> > > > +	if (slot_power_limit_value)
> > > > +		*slot_power_limit_value = value;
> > > > +
> > > > +	if (slot_power_limit_scale)
> > > > +		*slot_power_limit_scale = scale;
> > > > +
> > > > +	return slot_power_limit;
> > > 
> > > If "slot-power-limit-milliwatt" contains a value larger than can be
> > > represented in "value" and "scale", the return value will not agree
> > > with value/scale, will it?
> > 
> > In previous version 0xF2 was reserved for values above 275 W. So for me
> > it looked like a correct solution.
> > 
> > > Currently you only use the return value for a log message, so no real
> > > harm yet, other than the fact that we might print "Slot power limit
> > > 1000.0W" when the hardware will only advertise 600W available.
> > > 
> > > Also, if "slot-power-limit-milliwatt" contains something like
> > > 260000 mW (260 W), we'll return 0xF1/0, so the hardware will
> > > advertise 275 W available.
> > 
> > There is no way how to encode 260 W. It is possible only 250 W or 275 W,
> > and nothing between. I chose to round value to upper limit. What do you
> > prefer in these cases? Upper or lower limit?
> 
> I think rounding down is better.  If we round up, the slot will
> advertise more power than it can deliver, and if the device tries to
> consume the amount of power advertised, it may not work reliably.
> 
> So I think we should return encoded values that are no higher than
> what the slot can actually deliver, and the return value should match
> what Slot Capabilities advertises.
> 
> Bjorn

It makes sense.
diff mbox series

Patch

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index cb2e8351c2cc..2b0c0a3641a8 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -633,3 +633,67 @@  int of_pci_get_max_link_speed(struct device_node *node)
 	return max_link_speed;
 }
 EXPORT_SYMBOL_GPL(of_pci_get_max_link_speed);
+
+/**
+ * of_pci_get_slot_power_limit - Parses the "slot-power-limit-milliwatt"
+ *				 property.
+ *
+ * @node: device tree node with the slot power limit information
+ * @slot_power_limit_value: pointer where the value should be stored in PCIe
+ *			    Slot Capabilities Register format
+ * @slot_power_limit_scale: pointer where the scale should be stored in PCIe
+ *			    Slot Capabilities Register format
+ *
+ * Returns the slot power limit in milliwatts and if @slot_power_limit_value
+ * and @slot_power_limit_scale pointers are non-NULL, fills in the value and
+ * scale in format used by PCIe Slot Capabilities Register.
+ *
+ * If the property is not found or is invalid, returns 0.
+ */
+u32 of_pci_get_slot_power_limit(struct device_node *node,
+				u8 *slot_power_limit_value,
+				u8 *slot_power_limit_scale)
+{
+	u32 slot_power_limit;
+	u8 value, scale;
+
+	if (of_property_read_u32(node, "slot-power-limit-milliwatt",
+				 &slot_power_limit))
+		slot_power_limit = 0;
+
+	/* Calculate Slot Power Limit Value and Slot Power Limit Scale */
+	if (slot_power_limit == 0) {
+		value = 0x00;
+		scale = 0;
+	} else if (slot_power_limit <= 255) {
+		value = slot_power_limit;
+		scale = 3;
+	} else if (slot_power_limit <= 255*10) {
+		value = slot_power_limit / 10;
+		scale = 2;
+	} else if (slot_power_limit <= 255*100) {
+		value = slot_power_limit / 100;
+		scale = 1;
+	} else if (slot_power_limit <= 239*1000) {
+		value = slot_power_limit / 1000;
+		scale = 0;
+	} else if (slot_power_limit <= 250*1000) {
+		value = 0xF0;
+		scale = 0;
+	} else if (slot_power_limit <= 275*1000) {
+		value = 0xF1;
+		scale = 0;
+	} else {
+		value = 0xF2;
+		scale = 0;
+	}
+
+	if (slot_power_limit_value)
+		*slot_power_limit_value = value;
+
+	if (slot_power_limit_scale)
+		*slot_power_limit_scale = scale;
+
+	return slot_power_limit;
+}
+EXPORT_SYMBOL_GPL(of_pci_get_slot_power_limit);
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 3d60cabde1a1..e10cdec6c56e 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -627,6 +627,9 @@  struct device_node;
 int of_pci_parse_bus_range(struct device_node *node, struct resource *res);
 int of_get_pci_domain_nr(struct device_node *node);
 int of_pci_get_max_link_speed(struct device_node *node);
+u32 of_pci_get_slot_power_limit(struct device_node *node,
+				u8 *slot_power_limit_value,
+				u8 *slot_power_limit_scale);
 void pci_set_of_node(struct pci_dev *dev);
 void pci_release_of_node(struct pci_dev *dev);
 void pci_set_bus_of_node(struct pci_bus *bus);
@@ -653,6 +656,18 @@  of_pci_get_max_link_speed(struct device_node *node)
 	return -EINVAL;
 }
 
+static inline u32
+of_pci_get_slot_power_limit(struct device_node *node,
+			    u8 *slot_power_limit_value,
+			    u8 *slot_power_limit_scale)
+{
+	if (slot_power_limit_value)
+		*slot_power_limit_value = 0;
+	if (slot_power_limit_scale)
+		*slot_power_limit_scale = 0;
+	return 0;
+}
+
 static inline void pci_set_of_node(struct pci_dev *dev) { }
 static inline void pci_release_of_node(struct pci_dev *dev) { }
 static inline void pci_set_bus_of_node(struct pci_bus *bus) { }