Messages in this thread | | | Date | Tue, 5 Oct 2021 12:12:30 +0200 | From | Marijn Suijten <> | Subject | Re: [PATCH 09/10] backlight: qcom-wled: Consistently use enabled-strings in set_brightness |
| |
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.
| |