msm8953-mainline / linux

Linux mainline kernel with WIP patches for msm8953 devices
Other
111 stars 59 forks source link

arm64: dts: qcom: xiaomi-vince: add magnetometer and light sensor #88

Closed M0Rf30 closed 1 year ago

M0Rf30 commented 1 year ago

This commit enables Asahi Kasei AK09918 and Liteon ltrf216a for xiaomi-vince. It also add a missing property for battery: constant-charge-current-max-microamp

M0Rf30 commented 1 year ago

@alikates @z3ntu @fekz115 Let me know what do you think

z3ntu commented 1 year ago

Looks okay. But maybe check indent of the 2 lines after mount-matrix, can't tell on github if they align or not.

M0Rf30 commented 1 year ago

Looks okay. But maybe check indent of the 2 lines after mount-matrix, can't tell on github if they align or not.

checked, two tabs in the lines next to mount-matrix

alikates commented 1 year ago

Can you please separate the changes? Removing the DTS symlinks and adding an audio codec don't have much to do with the commit message. It wil make it easier for maintaining the changes when there's some squash/fixup or changes are accepted.

Btw,regarding the magnetometer I tried a couple changes while reviewing the datasheet but I couldn't make it work.

M0Rf30 commented 1 year ago

Can you please separate the changes? Removing the DTS symlinks and adding an audio codec don't have much to do with the commit message. It wil make it easier for maintaining the changes when there's some squash/fixup or changes are accepted.

Btw,regarding the magnetometer I tried a couple changes while reviewing the datasheet but I couldn't make it work.

Done

fekz115 commented 1 year ago

I think it is a good idea to add also

&panel {
    compatible = "xiaomi,vince-panel";
};

because it is absent for now.

M0Rf30 commented 1 year ago

I think it is a good idea to add also

&panel {
  compatible = "xiaomi,vince-panel";
};

because it is absent for now.

It's already in with latest force-pushed commit (just before your comment) https://github.com/msm8953-mainline/linux/compare/0d714f785a76d6e5f2f7eab2671ed1a662dbab9e..b894674db42651895e28dd6619f05f8bbde59d23

M0Rf30 commented 1 year ago

@alikates @z3ntu shall we merge?

alikates commented 1 year ago

Yes, but please next time split changes in multiple commits if they are not related (currently the commit also adds audio and touchscreen nodes). This time it's just one file so it's not too problematic.

M0Rf30 commented 1 year ago

Yes, but please next time split changes in multiple commits if they are not related (currently the commit also adds audio and touchscreen nodes). This time it's just one file so it's not too problematic.

Ok will take care next time