search

msm8226-mainline / linux

Mainline Kernel fork for MSM8x26 devices
Other
2 stars 5 forks source link

Add support for samsung-milletwifi #3

Closed Susurrus closed 9 months ago

Susurrus commented 9 months ago

Only the following hardware is confirmed working at this point:

Susurrus commented 9 months ago

I think 2cb7428 isn't right and breaks some other things. That needs to be cleaned up before this can be merged I think.

Susurrus commented 9 months ago

Removed the panel code (incl 2cb7428) as it's not working yet and the commits need some cleanup. Instead going to limit this to other components and simple framebuffer support. I think this can be merged as an initial starting point.

Susurrus commented 9 months ago

Okay, I restricted this to everything that's at least partially working + the hall effect sensor. Not certain why that doesn't work, but it's the same config as matisse-wifi, so I'd like to keep it in here so I can keep playing with it.

Not certain what to do about the pinctrl feedback as well.

Susurrus commented 9 months ago

I'm actively working on this now, think I may have figured out the lid sensor and doing some other minor cleanup work suggested by the MSM8916 mainlining article.

Susurrus commented 9 months ago

Okay, the cover sensor now works. Only thing I know that doesn't work correctly is the backlight:

Looking at the downstream DTB I haven't been able to decipher if there's a pin I'm missing. I'm happy to drop this commit if it moves this along, otherwise some help figuring this out would be appreciated!

Susurrus commented 9 months ago

I think the first 5 commits here are pretty uncontroversial and could be merged. I'm still working on the backlight and touchscreen right now. Would it make sense to merge the first 5 now and I'll submit a separate PR for these when they're more solid?

Susurrus commented 9 months ago

The last commit is pending me getting the firmware files sorted, but the touchscreen is confirmed working now. Known issues:

Other systems need to be enables (USB-OTG, battery, charging, wifi/bt, audio, GPS, and irda), but what's already done so far seems solid.

Susurrus commented 9 months ago

I removed the change with GPS, audio, and wifi/bt as that'll require firmware and additional changes I haven't tested yet.

z3ntu commented 9 months ago

Looks pretty good!

Now last thing for now (I hope) is getting the commit messages right.

The commit adding dts file (and the Makefile entry) should be the last commit in the series and titled something like ARM: dts: qcom: Add support for Samsung Galaxy Tab 4 8.0 Wi-Fi with a decent commit message what you're doing.

For the format of commit messages you can always check git log <path-to-file> (or if you're adding a new file, check that directory for examples).

Every commit needs to have Signed-off-by: Your Name <your@email.com>: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin

And your patchset should be two patches in total: one adding the string to Documentation/devicetree/bindings/arm/qcom.yaml (also check the commit messages example there), the other one adding the dts (+makefile change). No need to have them separate for initial bringup.

All the patches that come later should be one per feature for example though.

Susurrus commented 9 months ago

And your patchset should be two patches in total: one adding the string to Documentation/devicetree/bindings/arm/qcom.yaml (also check the commit messages example there), the other one adding the dts (+makefile change). No need to have them separate for initial bringup.

All the patches that come later should be one per feature for example though.

Does "initial bringup" mean everything I've included in this PR? My interpretation here is that there should be 2 commits as you mentioned. Then, in subsequent PRs I'll file, I should do commits per-feature (much like I did already in this initial PR).

Is there a reason you want me to squash all the DTS changes into 1 commit rather than leave them as single commits per-feature as I've already done? I'd think that'd be easiest to review and clearest in the git history. And since you mention it's what I should do for subsequent feature work, seems most consistent.

z3ntu commented 9 months ago

Is there a reason you want me to squash all the DTS changes into 1 commit rather than leave them as single commits per-feature as I've already done? I'd think that'd be easiest to review and clearest in the git history. And since you mention it's what I should do for subsequent feature work, seems most consistent.

Upstream doesn't want a thousand commits for initial dts inclusion :)

But yes I mean all the dts commit here you squash into one which adds the features here at once.

Susurrus commented 9 months ago

Okay, should be ready! I'm compiling it now to push it to the device to make sure I didn't mess anything up.

Susurrus commented 9 months ago

Alright, tested as working on my device. I think we're gtg here! :-)

z3ntu commented 9 months ago

@Susurrus you wanna add the melfas,mms252 change still? https://github.com/msm8226-mainline/linux/pull/3#discussion_r1362594376

Otherwise I'll add it before merging :)

Susurrus commented 9 months ago

Feel free to add it. Wasn't certain what you meant and won't be able to do it tonight at this point.

On October 17, 2023 9:17:35 PM GMT+02:00, Luca Weiss @.***> wrote:

@Susurrus you wanna add the melfas,mms252 change still? https://github.com/msm8226-mainline/linux/pull/3#discussion_r1362594376

Otherwise I'll add it before merging :)

-- Reply to this email directly or view it on GitHub: https://github.com/msm8226-mainline/linux/pull/3#issuecomment-1767018440 You are receiving this because you were mentioned.

Message ID: @.***>

z3ntu commented 9 months ago

okay, modified https://github.com/msm8226-mainline/linux/pull/3#discussion_r1362594376 and https://github.com/msm8226-mainline/linux/pull/3#discussion_r1362753329

LGTM now!!