On 29 June 2016 at 15:34, Christopher Covington cov@codeaurora.org wrote:
Hi Tomasz,
On 06/29/2016 06:48 AM, Tomasz Nowicki wrote:
On 28.06.2016 18:12, Duc Dang wrote:
On Tue, Jun 28, 2016 at 6:04 AM, Christopher Covington cov@codeaurora.org wrote:
Hi Tomasz,
Ard's comments on v3 included:
"... exact OEM table/rev id matches ..." "... substring match ... out of the question ..."
Digging through the archives I see Jon Master commented earlier to "be careful with substring match".
I think having OEM Table ID as "PLAT " and then "PLAT2 " (the the next version of the SoC) is common. So yes, matching full string is better as we can use "PLAT2 " in MCFG table and not worry about the "PLAT" sub-string match causes the quirk to be applied unintentionally.
Note that platforms already shipped where OEM string has no padding will
I'm confused by this statement. OEMID is defined as 6 bytes long and OEM Table ID as 8 bytes long in the ACPI specification. As far as I can tell, if your string isn't exactly that long, padding up to that length is required.
have change the firmware or add 0 padding to our quirk array IDs.
The fixed 6 or 8 character string compare, as used v2 of this patchset, will be compatible with existing firmware as best I can tell. Adding padding to the quirk array IDs is exactly what I'm suggesting, although all the strings I've seen are space padded rather than null padded.
I don't think any interpretation of the 6 or 8 byte wide OEM fields is necessary to be able to match it against a list of known values as used by the quirky platforms. We need an exact match against whatever we know is in the table of an affected system, and whether a space qualifies as padding or as a character is irrelevant.
Matches: {"APM ", "XGENE ", 1} {"CAVIUM", "THUNDERX", 1} {"HISI ", "HISI-D02", 1} {"HISI ", "HISI-D03", 1} {"QCOM ", "QDF2432 ", 1}
I would not mind listing these as
{ { 'A','P','M',' ',' ',' ',' '}, {'X','G','E','N','E',' ',' ',' '}, 1} ...
just to stress that we are not dealing with C strings (and to avoid having to deal with the implicit NUL terminator). That also means memcmp() with a fixed length is the most appropriate to perform the comparison
Given the above tuples, won't accidentally match: (guessing at possible future ids) {"APM ", "XGENEi ", 1} {"CAVIUM", "THUNDERX", i} i != 1 {"CAVIUM", "THUNDERi", 1} {"CAVIUM", "THUNDRXi", 1} {"HISI ", "HISI-D0i", 1} i != 2 && i != 3 {"QCOM ", "QDF24ij ", 1} i != 3 && j != 2
References for APM, HiSilicon IDs: https://lists.linaro.org/pipermail/linaro-acpi/2016-June/007108.html https://lists.linaro.org/pipermail/linaro-acpi/2016-June/007043.html
Thanks, Cov
-- Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project