On 2021-10-05 10:33:31, Daniel Thompson wrote: > On Mon, Oct 04, 2021 at 09:27:40PM +0200, Marijn Suijten wrote: > > The hardware is capable of controlling any non-contiguous sequence of > > LEDs specified in the DT using qcom,enabled-strings as u32 > > array, and this also follows from the DT-bindings documentation. The > > numbers specified in this array represent indices of the LED strings > > that are to be enabled and disabled. > > > > Its value is appropriately used to setup and enable string modules, but > > completely disregarded in the set_brightness paths which only iterate > > over the number of strings linearly. > > Take an example where only string 2 is enabled with > > qcom,enabled_strings=<2>: this string is appropriately enabled but > > subsequent brightness changes would have only touched the zero'th > > brightness register because num_strings is 1 here. This is simply > > addressed by looking up the string for this index in the enabled_strings > > array just like the other codepaths that iterate over num_strings. > > This isn't true until patch 10 is applied! Patch 9 and 10 were split up at a last resort to prevent a clash in the title, apologies for that. > Given both patches fix the same issue in different functions I'd prefer > these to be squashed together (and doubly so because the autodetect code > uses set_brightness() as a helper function). That's a fair reason, and solution I agree on. I'll figure out how to generify the title and re-spin this patchset except if there are other reviewers/maintainers I should wait for. - Marijn > Daniel.