[Skiboot] [PATCH 02/16] platforms/astbmc: ibm, slot-label not depend on ibm, slot-location-code
andrew.donnellan at au1.ibm.com
Tue Sep 20 14:07:22 AEST 2016
On 16/09/16 15:05, Gavin Shan wrote:
> "ibm,slot-label" should not depend on "ibm,slot-location-code". We
> might fail to populate the later one because of oversized "ibm,slot-label"
later -> latter
> or PHB's base location code.
> This populates "ibm,slot-label" unconditionally. It won't depend on
> Signed-off-by: Gavin Shan <gwshan at linux.vnet.ibm.com>
It took me a while to figure out what this commit message meant. Could
you reword it to be slightly clearer?
I'd prefer something along the lines of:
platforms/astbmc: populate ibm,slot-label even if
ibm,slot-location-code is too long
When populating slot properties, if the length of the PHB's
base location code plus the length of the slot label is too
large, we return immediately without populating either
ibm,slot-location-code or ibm,slot-label.
ibm,slot-label shouldn't depend on populating ibm,slot-
location-code, so populate ibm,slot-label before this check.
Otherwise, looks good.
Reviewed-by: Andrew Donnellan <andrew.donnellan at au1.ibm.com>
> platforms/astbmc/slots.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
> diff --git a/platforms/astbmc/slots.c b/platforms/astbmc/slots.c
> index 6ffec7d..e71eb38 100644
> --- a/platforms/astbmc/slots.c
> +++ b/platforms/astbmc/slots.c
> @@ -95,6 +95,8 @@ static void add_slot_properties(struct pci_slot *slot,
> if (!np || !ent)
> + dt_add_property_string(np, "ibm,slot-label", ent->name);
> base_loc_code_len = phb->base_loc_code ? strlen(phb->base_loc_code) : 0;
> slot_label_len = strlen(ent->name);
> if ((base_loc_code_len + slot_label_len + 1) >= LOC_CODE_SIZE)
Is there a good reason not to just truncate ibm,slot-location-code to 80
characters? And is this something we run into on a regular basis?
Andrew Donnellan OzLabs, ADL Canberra
andrew.donnellan at au1.ibm.com IBM Australia Limited
More information about the Skiboot