msm8916-mainline
Add Richtek RT8555 Backlight Support for samsung-gtelwifiue
The Richtek RT8555 is used as a backlight controller in samsung-gtelwifiue, and this adds support for it. It has support for both 8 bit and 10 bits of brightness levels, and this patch uses the 10 bit brightness mode.
I've just finished rewriting this with actually proper IC initialization support. It contains commits from #241 because it'll probably get merged first and it creates a merge conflict, so I'd rather just rebase this PR on master once it gets merged.
There's probably some code quality issues, but it does work properly, and it also adds the backlight to the panel in the dtb, so brightness control properly works in userspace.
I'd also like to ask whether it'd be worth (or even a good idea to do) implementing a simple brightness -> perceived brightness conversion inside the driver, since currently the relation between the set brightness and the perceived brightness doesn't seem even remotely linear (the last half of the brightness values have much less of a difference in brightness than the first half). Other mainline drivers don't appear to do it, but the downstream does (though a massive lookup table). There's various functions that can estimate this (though I haven't tried), such as `pow(max_brightness, brightness/max_brightness), and the inverse of this one could be easily calculated.
There is something in pwm_bl for this, see the cie1931 function. I don't know if this is worth it or not or if userspace should be responsible for this.
I don't really think there'd be a clean way to implement this in userspace, personally. But then again, I'd also probably have to create the inverse of the function for the get_brightness function (or create a table like they do), which seems like a lot of effort. I'd personally rather leave such functionality out for now.
Is this ready for merging now?
The comment with the dt-bindings and the one potentially making the GPIO optional is still open.
Also, as per the contribution guidelines I would appreciate if you could send this driver to the upstream mailing lists. We do not have the capacity to keep additional drivers up to date. Feel free to ask any questions about the upstreaming process. :)
Personally, I'm against the making the enable GPIO non-optional, simply because other drivers leave it optional and it's not integral to the functionality of the backlight, and doesn't make the logic that much more complex, and @stephan-gh hasn't replied, so I think we can consider that closed.
Do I need to add myself to the MAINTAINERS file also? It only has some individual drivers, so I'm not sure.
For sending patches to the mailing list, who would I end up addressing them to, and which fields would I use? scripts/get_maintainer.pl gives me 7 email addresses. Here's the output:
% scripts/get_maintainer.pl 0001-video-backlight-Add-support-for-Richtek-RT8555-Backl.patch
Lee Jones <[email protected]> (maintainer:BACKLIGHT CLASS/SUBSYSTEM)
Daniel Thompson <[email protected]> (maintainer:BACKLIGHT CLASS/SUBSYSTEM)
Jingoo Han <[email protected]> (maintainer:BACKLIGHT CLASS/SUBSYSTEM)
Helge Deller <[email protected]> (maintainer:FRAMEBUFFER LAYER)
[email protected] (open list)
[email protected] (open list:BACKLIGHT CLASS/SUBSYSTEM)
[email protected] (open list:FRAMEBUFFER LAYER)
I'd assume I'd want to mail to most of the mailing lists, except for maybe fbdev, and then CC all the maintainers? Then I imagine I would have to email mainly just the devicetree mailing list for the currently nonexistent dt-bindings commit.
Anyways, I'll start work on the dt-bindings now.
Personally, I'm against the making the enable GPIO non-optional, simply because other drivers leave it optional and it's not integral to the functionality of the backlight, and doesn't make the logic that much more complex, and @stephan-gh hasn't replied, so I think we can consider that closed.
My comment is not marked as "resolved" like the others and you have not replied, so it was your turn, not mine. :)
If you want to keep the GPIO optional you need to fix the error handling. The IS_ERR check for the GPIO must be in the probe function to detect errors. Then it can only be NULL or valid later. Functions like gpiod_set_value() have internal NULL checks so you can likely drop most of the if statements.
Do I need to add myself to the MAINTAINERS file also? It only has some individual drivers, so I'm not sure.
Up to you. You can but you don't have to (not sure if it's worth it for such a small driver).
For sending patches to the mailing list, who would I end up addressing them to, and which fields would I use? scripts/get_maintainer.pl gives me 7 email addresses. Here's the output:
I would use --to for the BACKLIGHT CLASS/SUBSYSTEM maintainers (in other words: the recipients that you would expect to apply the patch), and --cc everyone else, including the mailing lists. However, there is no strict rule for that, AFAIK.
I'd assume I'd want to mail to most of the mailing lists, except for maybe fbdev, and then CC all the maintainers? Then I imagine I would have to email mainly just the devicetree mailing list for the currently nonexistent dt-bindings commit.
Please send the entire series (the two patches) to the same recipients. So you'd get the DT mailing lists and the two DT maintainers as additional Cc recipients.
My comment is not marked as "resolved" like the others and you have not replied, so it was your turn, not mine. :) I did reply, though?
It's probably the pending tag I guess? I replied to a lot of your other comments and they had the tag, so I'm guessing you can't see those either? Will note the rest of your suggestions/advice, though, and simplify the error checking.
is anyone going to upstream this? since the backlight driver (unlike the broken panel driver) is working perfectly fine under mainline
Honestly, it's been in a perpetual state of "I want to upstream this" but I've never gotten around to actually doing it, half due to me being unsure on some properties when writing the documentation yaml (writing a description is hard when the datasheet's unclear as to what some fields mean), and half due to how completely unfamiliar I am with the LKML submission process.
I have some uncommitted WIP files I could dig out, plus I think an updated version of the driver that builds on newer kernels that I can put up in this PR. I also wouldn't be completely opposed to someone else continuing the PR and submitting it to the LKML in my place (though I will likely be looking into it oncemore in the coming week or so).
Also, the panel driver's broken? That's news to me. I compiled this patch against latest the pmOS kernel about 5 months ago and it was fine minus some graphical glitchiness in Plasma Mobile that wasn't there before.