Add Richtek RT8555 Backlight Support for samsung-gtelwifiue #217
Conversation
| return -ENODEV; | ||
| } | ||
|
|
||
| ret = device_property_read_u32(&client->dev, "max-brightness", &brightness); |
There was a problem hiding this comment.
there should be a commit for dt bindings that would describe those
person4268
force-pushed
the
add-backlight
branch
2 times, most recently
from
ae53db0
to
8678ba7
Compare
|
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. |
| priv->dev = &client->dev; | ||
|
|
||
| priv->enable = | ||
| devm_gpiod_get_optional(&client->dev, NULL, GPIOD_OUT_HIGH); |
There was a problem hiding this comment.
Is it really worth making this GPIO optional? All the error handling required makes me think it's not.
There was a problem hiding this comment.
I personally thought that it's possible that a device -might- not have enable wired to a gpio for whatever reason, and it isn't really required for core functionality. I'll remove it if you want, though.
|
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. |
person4268
force-pushed
the
add-backlight
branch
2 times, most recently
from
5f53c62
to
e485625
Compare
|
Rebased on master since #241 was merged. Is this ready for merging now? |
The Richtek RT8555 is a 6 channel LED driver that supports up to 35mA/LED. This is a backlight driver that drives ILED1. Signed-off-by: Michael Abood <[email protected]>
This adds the RT8555 backlight driver to the device tree. Using i2c-gpio here is necessary, as the backlight ic is not on any i2c pins. Signed-off-by: Michael Abood <[email protected]>
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: 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. |
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
Up to you. You can but you don't have to (not sure if it's worth it for such a small driver).
I would use
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. |
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. |
| if (ret) | ||
| return ret; | ||
|
|
||
| /* "Change Duty" for Mixed Mode */ |
There was a problem hiding this comment.
I'm honestly not sure what to call this. Datasheet calls it "Mixed Mode Change Duty" but I couldn't completely tell what it does exactly. I assume duty implies a duty cycle, so maybe something like "change-duty-cycle"? I left out mixed since I always have mixed mode on, also, not sure if that's the best idea but it seemed like the alternative used the PWM pin which I didn't want to support.
| priv->dev = &client->dev; | ||
|
|
||
| priv->enable = | ||
| devm_gpiod_get_optional(&client->dev, NULL, GPIOD_OUT_HIGH); |
There was a problem hiding this comment.
I personally thought that it's possible that a device -might- not have enable wired to a gpio for whatever reason, and it isn't really required for core functionality. I'll remove it if you want, though.
stephan-gh
changed the base branch from
master
to
backup/5.18/please-rebase
stephan-gh
changed the base branch from
backup/5.18/please-rebase
to
msm8916/5.18
…kprobe_event_gen_test_exit() When trace_get_event_file() failed, gen_kretprobe_test will be assigned as the error code. If module kprobe_event_gen_test is removed now, the null pointer dereference will happen in kprobe_event_gen_test_exit(). Check if gen_kprobe_test or gen_kretprobe_test is error code or NULL before dereference them. BUG: kernel NULL pointer dereference, address: 0000000000000012 PGD 0 P4D 0 Oops: 0000 [#1] SMP PTI CPU: 3 PID: 2210 Comm: modprobe Not tainted 6.1.0-rc1-00171-g2159299a3b74-dirty #217 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.15.0-0-g2dd4b9b3f840-prebuilt.qemu.org 04/01/2014 RIP: 0010:kprobe_event_gen_test_exit+0x1c/0xb5 [kprobe_event_gen_test] Code: Unable to access opcode bytes at 0xffffffff9ffffff2. RSP: 0018:ffffc900015bfeb8 EFLAGS: 00010246 RAX: ffffffffffffffea RBX: ffffffffa0002080 RCX: 0000000000000000 RDX: ffffffffa0001054 RSI: ffffffffa0001064 RDI: ffffffffdfc6349c RBP: ffffffffa0000000 R08: 0000000000000004 R09: 00000000001e95c0 R10: 0000000000000000 R11: 0000000000000001 R12: 0000000000000800 R13: ffffffffa0002420 R14: 0000000000000000 R15: 0000000000000000 FS: 00007f56b75be540(0000) GS:ffff88813bc00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: ffffffff9ffffff2 CR3: 000000010874a006 CR4: 0000000000330ee0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Call Trace: <TASK> __x64_sys_delete_module+0x206/0x380 ? lockdep_hardirqs_on_prepare+0xd8/0x190 ? syscall_enter_from_user_mode+0x1c/0x50 do_syscall_64+0x3f/0x90 entry_SYSCALL_64_after_hwframe+0x63/0xcd Link: https://lore.kernel.org/all/[email protected]/ Fixes: 6483624 ("tracing: Add kprobe event command generation test module") Signed-off-by: Shang XiaoJing <[email protected]> Acked-by: Masami Hiramatsu (Google) <[email protected]> Cc: [email protected] Signed-off-by: Masami Hiramatsu (Google) <[email protected]>
stephan-gh
changed the title
…kprobe_event_gen_test_exit() commit e0d7526 upstream. When trace_get_event_file() failed, gen_kretprobe_test will be assigned as the error code. If module kprobe_event_gen_test is removed now, the null pointer dereference will happen in kprobe_event_gen_test_exit(). Check if gen_kprobe_test or gen_kretprobe_test is error code or NULL before dereference them. BUG: kernel NULL pointer dereference, address: 0000000000000012 PGD 0 P4D 0 Oops: 0000 [msm8953-mainline#1] SMP PTI CPU: 3 PID: 2210 Comm: modprobe Not tainted 6.1.0-rc1-00171-g2159299a3b74-dirty msm8916-mainline#217 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.15.0-0-g2dd4b9b3f840-prebuilt.qemu.org 04/01/2014 RIP: 0010:kprobe_event_gen_test_exit+0x1c/0xb5 [kprobe_event_gen_test] Code: Unable to access opcode bytes at 0xffffffff9ffffff2. RSP: 0018:ffffc900015bfeb8 EFLAGS: 00010246 RAX: ffffffffffffffea RBX: ffffffffa0002080 RCX: 0000000000000000 RDX: ffffffffa0001054 RSI: ffffffffa0001064 RDI: ffffffffdfc6349c RBP: ffffffffa0000000 R08: 0000000000000004 R09: 00000000001e95c0 R10: 0000000000000000 R11: 0000000000000001 R12: 0000000000000800 R13: ffffffffa0002420 R14: 0000000000000000 R15: 0000000000000000 FS: 00007f56b75be540(0000) GS:ffff88813bc00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: ffffffff9ffffff2 CR3: 000000010874a006 CR4: 0000000000330ee0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Call Trace: <TASK> __x64_sys_delete_module+0x206/0x380 ? lockdep_hardirqs_on_prepare+0xd8/0x190 ? syscall_enter_from_user_mode+0x1c/0x50 do_syscall_64+0x3f/0x90 entry_SYSCALL_64_after_hwframe+0x63/0xcd Link: https://lore.kernel.org/all/[email protected]/ Fixes: 6483624 ("tracing: Add kprobe event command generation test module") Signed-off-by: Shang XiaoJing <[email protected]> Acked-by: Masami Hiramatsu (Google) <[email protected]> Cc: [email protected] Signed-off-by: Masami Hiramatsu (Google) <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
If a kernel thread is created by a user thread, it may carry FPU/SIMD thread info flags (TIF_USEDFPU, TIF_USEDSIMD, etc.). Then it will be considered as a fpu owner and kernel try to save its FPU/SIMD context and cause such errors: [ 41.518931] do_fpu invoked from kernel context![#1]: [ 41.523933] CPU: 1 PID: 395 Comm: iou-wrk-394 Not tainted 6.1.0-rc5+ #217 [ 41.530757] Hardware name: Loongson Loongson-3A5000-7A1000-1w-CRB/Loongson-LS3A5000-7A1000-1w-CRB, BIOS vUDK2018-LoongArch-V2.0.pre-beta8 08/18/2022 [ 41.544064] $ 0 : 0000000000000000 90000000011e9468 9000000106c7c000 9000000106c7fcf0 [ 41.552101] $ 4 : 9000000106305d40 9000000106689800 9000000106c7fd08 0000000003995818 [ 41.560138] $ 8 : 0000000000000001 90000000009a72e4 0000000000000020 fffffffffffffffc [ 41.568174] $12 : 0000000000000000 0000000000000000 0000000000000020 00000009aab7e130 [ 41.576211] $16 : 00000000000001ff 0000000000000407 0000000000000001 0000000000000000 [ 41.584247] $20 : 0000000000000000 0000000000000001 9000000106c7fd70 90000001002f0400 [ 41.592284] $24 : 0000000000000000 900000000178f740 90000000011e9834 90000001063057c0 [ 41.600320] $28 : 0000000000000000 0000000000000001 9000000006826b40 9000000106305140 [ 41.608356] era : 9000000000228848 _save_fp+0x0/0xd8 [ 41.613542] ra : 90000000011e9468 __schedule+0x568/0x8d0 [ 41.619160] CSR crmd: 000000b0 [ 41.619163] CSR prmd: 00000000 [ 41.622359] CSR euen: 00000000 [ 41.625558] CSR ecfg: 00071c1c [ 41.628756] CSR estat: 000f0000 [ 41.635239] ExcCode : f (SubCode 0) [ 41.638783] PrId : 0014c010 (Loongson-64bit) [ 41.643191] Modules linked in: acpi_ipmi vfat fat ipmi_si ipmi_devintf cfg80211 ipmi_msghandler rfkill fuse efivarfs [ 41.653734] Process iou-wrk-394 (pid: 395, threadinfo=0000000004ebe913, task=00000000636fa1be) [ 41.662375] Stack : 00000000ffff0875 9000000006800ec0 9000000006800ec0 90000000002d57e0 [ 41.670412] 0000000000000001 0000000000000000 9000000106535880 0000000000000001 [ 41.678450] 9000000105291800 0000000000000000 9000000105291838 900000000178e000 [ 41.686487] 9000000106c7fd90 9000000106305140 0000000000000001 90000000011e9834 [ 41.694523] 00000000ffff0875 90000000011f034c 9000000105291838 9000000105291830 [ 41.702561] 0000000000000000 9000000006801440 00000000ffff0875 90000000002d48c0 [ 41.710597] 9000000128800001 9000000106305140 9000000105291838 9000000105291838 [ 41.718634] 9000000105291830 9000000107811740 9000000105291848 90000000009bf1e0 [ 41.726672] 9000000105291830 9000000107811748 2d6b72772d756f69 0000000000343933 [ 41.734708] 0000000000000000 0000000000000000 0000000000000000 0000000000000000 [ 41.742745] ... [ 41.745252] Call Trace: [ 42.197868] [<9000000000228848>] _save_fp+0x0/0xd8 [ 42.205214] [<90000000011ed468>] __schedule+0x568/0x8d0 [ 42.210485] [<90000000011ed834>] schedule+0x64/0xd4 [ 42.215411] [<90000000011f434c>] schedule_timeout+0x88/0x188 [ 42.221115] [<90000000009c36d0>] io_wqe_worker+0x184/0x350 [ 42.226645] [<9000000000221cf0>] ret_from_kernel_thread+0xc/0x9c This can be easily triggered by ltp testcase syscalls/io_uring02 and it can also be easily fixed by clearing the FPU/SIMD thread info flags for kernel threads in copy_thread(). Cc: [email protected] Reported-by: Qi Hu <[email protected]> Signed-off-by: Huacai Chen <[email protected]>
stephan-gh
changed the title
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.