On Fri, May 19, 2017 at 01:28:36PM +0300, Mika Westerberg wrote: > On Fri, May 19, 2017 at 12:07:10PM +0200, Lukas Wunner wrote: > > Apple uses 0x30 to store a > > serial number. Is this attribute number assigned by Intel to Apple > > or is it reserved for vendor use or did they arbitrarily choose it? > > It is part of the DROM specification. The 0x30 - 0x3e are vendor > specific entries.
Ah, so I have to qualify the vendor number with Apple's ID before I know that it's a serial number. Thanks. > > If there can be many attributes, should they be stored in a list > > rather than adding a char* pointer for each one to struct tb_switch? > > The latter doesn't scale. > > I don't think we need other attributes (well, at least right now). The > device/vendor name is useful because that's what we expose to the > userspace for device identification along with the device/vendor ID. Okay. It might be worth to log additional attributes with info level. > > > +static void tb_drom_parse_generic_entry(struct tb_switch *sw, > > > + struct tb_drom_entry_generic *entry) > > > +{ > > > + if (entry->header.index == 1) > > > + sw->vendor_name = kstrdup((char *)entry->data, GFP_KERNEL); > > > + else if (entry->header.index == 2) > > > + sw->device_name = kstrdup((char *)entry->data, GFP_KERNEL); > > > +} > > > > This assumes that these are properly null-terminated strings, but the DROM > > may contain complete garbage. The existing drom parser is very careful > > to validate and sanitize everything. > > The DROM specification says they must be null-terminated but I yes, it > is possible that some of the devices have it wrong. The generic entry > includes length field so I suppose we can use that + kmemdup() instead > here? Yes, as long as you check that the last character is null. Thanks, Lukas