| Message ID | 1474002323-31380-3-git-send-email-gwshan@linux.vnet.ibm.com |
|---|---|
| State | Superseded |
| Headers | show |
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@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@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?
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) @@ -111,7 +113,6 @@ static void add_slot_properties(struct pci_slot *slot, strcat(loc_code, ent->name); dt_add_property(np, "ibm,slot-location-code", loc_code, strlen(loc_code) + 1); - dt_add_property_string(np, "ibm,slot-label", ent->name); } void slot_table_get_slot_info(struct phb *phb, struct pci_device *pd)
"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" 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@linux.vnet.ibm.com> --- platforms/astbmc/slots.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)