On 10/10/19 12:04 PM, Nathan Lynch wrote: > Tyrel Datwyler <tyreld@xxxxxxxxxxxxx> writes: >> The ibm,drc-info property is an array property that contains drc-info >> entries such that each entry is made up of 2 string encoded elements >> followed by 5 int encoded elements. The of_read_drc_info_cell() >> helper contains comments that correctly name the expected elements >> and their encoding. However, the usage of of_prop_next_string() and >> of_prop_next_u32() introduced a subtle skippage of the first u32. >> This is a result of of_prop_next_string() returns a pointer to the >> next property value which is not a string, but actually a (__be32 *). >> As, a result the following call to of_prop_next_u32() passes over the >> current int encoded value and actually stores the next one wrongly. >> >> Simply endian swap the current value in place after reading the first >> two string values. The remaining int encoded values can then be read >> correctly using of_prop_next_u32(). > > Good but I think it would make more sense for a fix for > of_read_drc_info_cell() to precede any patch in the series which > introduces new callers, such as patch #1. > Not sure it matters that much since everything in the series is required to properly enable a working drc-info implementation and the call already exists so it doesn't break bisectability. It ended up second in the series since testing patch 1 exposed the flaw. I'll go ahead and move it first, and add the appropriate fixes tag as well which is currently missing. -Tyrel
