[Skiboot] [PATCH 02/16] platforms/astbmc: ibm, slot-label not depend on ibm, slot-location-code

[Andrew Donnellan]

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
> "ibm,slot-location-code".
>
> 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)
>  		return;
>
> +	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