| Message ID | 20220222163158.1666-5-pali@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Lorenzo Pieralisi |
| Headers | show |
| Series | PCI: mvebu: Slot support | expand |
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 >
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 > >
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
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 --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) { }