Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

arm64: dts: qcom: msm8916-samsung-heatqlte: Add touchkeys #356

Open
wants to merge 1 commit into
base: msm8916/6.7-rc4
Choose a base branch
from

Conversation

celele64
Copy link

Hi! This is my first time contributing to kernel-adjacent projects so please teach me the ways as you find necessary, I'm all up for learning.

This devicetree patch enables the Cypress tm2-touchkey device controlling the menu and back keys in the Samsung heatqlte (SM-G357FZ) and their backlight leds

Since GPIOs 8 and 10 have no hardware I2C interface software emulation is used via i2c-gpio, just like in the downstream kernel

I've set up the pinctrl as well, however I'm yet to learn more about this subsystem. Please point out anything I may have done wrong

Thanks!

pinctrl-names = "default";
};

i2c_touchkey: i2c-touchkey {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
i2c_touchkey: i2c-touchkey {
i2c-touchkey {

Don't think anything would use this label

Comment on lines 65 to 66
interrupt-parent = <&tlmm>;
interrupts = <9 IRQ_TYPE_EDGE_RISING>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think interrupts-extended is preferred nowdays

vcc-supply = <&reg_vcc_touchkey>;
vdd-supply = <&reg_vdd_touchkey_led>;

linux,keycodes = <KEY_APPSELECT KEY_BACK>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd move this last since it's a vendor property (even if vendor is linux in this case)

bias-disable;
};

nfc_touchkey_default: nfc-touchkey-default-state {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nfc?

@celele64
Copy link
Author

Oops, I was glancing at msm8916-samsung-gprime-common.dtsi as a reference for the i2c-gpio usage and that's where I mixed up "nfc" from!

Copy link
Member

@TravMurav TravMurav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Except the dangling nl, he diff looks good but please squash these two commits into one.

linux,keycodes = <KEY_APPSELECT KEY_BACK>;
};
};

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

@celele64 celele64 force-pushed the celele/msm8916/6.7-rc4-wip-touchkeys branch from 72915f9 to 13b110b Compare May 30, 2024 15:45
pinctrl-names = "default";
};

i2c-touchkey {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The nodes are supposed to be sorted in alphabetical order.

};

&tlmm {
vdd_touchkey_default: vdd-touchkey-default-state {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

};
};

&tlmm {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

&tlmm is preferred to be put at the bottom/end of the device tree for msm8916 devices.

@wonderfulShrineMaidenOfParadise

Try branch wip/msm8916/6.9, which has parent dtsi msm8916-samsung-rossa-common.dtsi/msm8916-samsung-fortuna-common.dtsi (former msm8916-samsung-cprime-common.dtsi/msm8916-samsung-gprime-common.dtsi) upstreamed in Torvalds's tree.

Add support for the tm2-touchkey input device providing the menu and
back keys and their backlight leds.

Signed-off-by: Celeste Lucero <celele64@outlook.com>
@celele64 celele64 force-pushed the celele/msm8916/6.7-rc4-wip-touchkeys branch from 13b110b to eba92b3 Compare June 2, 2024 16:55
@celele64
Copy link
Author

celele64 commented Jun 2, 2024

Okay, I'll try this dts on wip/msm8916/6.9
When contributing should I prefer working upon that branch instead of the repo default one?

@wonderfulShrineMaidenOfParadise
Copy link

wonderfulShrineMaidenOfParadise commented Jun 3, 2024

6.7 is deprecated. Development is preferred with latest kernel, which makes it "mainline". Besides 6.7 has been released and there is no point to use 6.7-rc* branches.

https://www.kernel.org/

@celele64
Copy link
Author

celele64 commented Jun 3, 2024

Makes sense, thanks for the heads up

I see no dts for heatqlte at wip/msm8916/6.9 branch, how should I go about contributing to it?
Do I have to add the files by bringing the related commits from the latest branch in repo containing them? (Such that history is not lost) or just add the new files entirely by me? (But then I'd make mine code it's not)

Sorry for the total newbie questions -.-'

TravMurav pushed a commit that referenced this pull request Nov 4, 2024
KASAN reports that the GPU metrics table allocated in
vangogh_tables_init() is not large enough for the memset done in
smu_cmn_init_soft_gpu_metrics(). Condensed report follows:

[   33.861314] BUG: KASAN: slab-out-of-bounds in smu_cmn_init_soft_gpu_metrics+0x73/0x200 [amdgpu]
[   33.861799] Write of size 168 at addr ffff888129f59500 by task mangoapp/1067
...
[   33.861808] CPU: 6 UID: 1000 PID: 1067 Comm: mangoapp Tainted: G        W          6.12.0-rc4 #356 1a56f59a8b5182eeaf67eb7cb8b13594dd23b544
[   33.861816] Tainted: [W]=WARN
[   33.861818] Hardware name: Valve Galileo/Galileo, BIOS F7G0107 12/01/2023
[   33.861822] Call Trace:
[   33.861826]  <TASK>
[   33.861829]  dump_stack_lvl+0x66/0x90
[   33.861838]  print_report+0xce/0x620
[   33.861853]  kasan_report+0xda/0x110
[   33.862794]  kasan_check_range+0xfd/0x1a0
[   33.862799]  __asan_memset+0x23/0x40
[   33.862803]  smu_cmn_init_soft_gpu_metrics+0x73/0x200 [amdgpu 13b1bc364ec578808f676eba412c20eaab792779]
[   33.863306]  vangogh_get_gpu_metrics_v2_4+0x123/0xad0 [amdgpu 13b1bc364ec578808f676eba412c20eaab792779]
[   33.864257]  vangogh_common_get_gpu_metrics+0xb0c/0xbc0 [amdgpu 13b1bc364ec578808f676eba412c20eaab792779]
[   33.865682]  amdgpu_dpm_get_gpu_metrics+0xcc/0x110 [amdgpu 13b1bc364ec578808f676eba412c20eaab792779]
[   33.866160]  amdgpu_get_gpu_metrics+0x154/0x2d0 [amdgpu 13b1bc364ec578808f676eba412c20eaab792779]
[   33.867135]  dev_attr_show+0x43/0xc0
[   33.867147]  sysfs_kf_seq_show+0x1f1/0x3b0
[   33.867155]  seq_read_iter+0x3f8/0x1140
[   33.867173]  vfs_read+0x76c/0xc50
[   33.867198]  ksys_read+0xfb/0x1d0
[   33.867214]  do_syscall_64+0x90/0x160
...
[   33.867353] Allocated by task 378 on cpu 7 at 22.794876s:
[   33.867358]  kasan_save_stack+0x33/0x50
[   33.867364]  kasan_save_track+0x17/0x60
[   33.867367]  __kasan_kmalloc+0x87/0x90
[   33.867371]  vangogh_init_smc_tables+0x3f9/0x840 [amdgpu]
[   33.867835]  smu_sw_init+0xa32/0x1850 [amdgpu]
[   33.868299]  amdgpu_device_init+0x467b/0x8d90 [amdgpu]
[   33.868733]  amdgpu_driver_load_kms+0x19/0xf0 [amdgpu]
[   33.869167]  amdgpu_pci_probe+0x2d6/0xcd0 [amdgpu]
[   33.869608]  local_pci_probe+0xda/0x180
[   33.869614]  pci_device_probe+0x43f/0x6b0

Empirically we can confirm that the former allocates 152 bytes for the
table, while the latter memsets the 168 large block.

Root cause appears that when GPU metrics tables for v2_4 parts were added
it was not considered to enlarge the table to fit.

The fix in this patch is rather "brute force" and perhaps later should be
done in a smarter way, by extracting and consolidating the part version to
size logic to a common helper, instead of brute forcing the largest
possible allocation. Nevertheless, for now this works and fixes the out of
bounds write.

v2:
 * Drop impossible v3_0 case. (Mario)

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Fixes: 41cec40 ("drm/amd/pm: Vangogh: Add new gpu_metrics_v2_4 to acquire gpu_metrics")
Cc: Mario Limonciello <mario.limonciello@amd.com>
Cc: Evan Quan <evan.quan@amd.com>
Cc: Wenyou Yang <WenYou.Yang@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
Link: https://lore.kernel.org/r/20241025145639.19124-1-tursulin@igalia.com
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
(cherry picked from commit 0880f58)
Cc: stable@vger.kernel.org # v6.6+
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants