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

Kernel support for recent Dell notebook Alps trackpads #2999

Merged
merged 1 commit into from Oct 25, 2018

Conversation

@shiznix
Copy link
Contributor

commented Sep 24, 2018

No description provided.

@lrusak

This comment has been minimized.

Copy link
Member

commented Sep 24, 2018

Thanks for the PR! Did you build test this at all or do you need a test build that includes these changes?

@MilhouseVH

This comment has been minimized.

Copy link
Contributor

commented Sep 24, 2018

This change is incomplete - the CONFIG_MFD_INTEL_LPSS_ACPI and CONFIG_MFD_INTEL_LPSS_PCI options introduce a number of additional config options that are not included in this PR:

+CONFIG_MFD_INTEL_LPSS=y
...
+# CONFIG_SND_SOC_INTEL_SKL_RT286_MACH is not set
+# CONFIG_SND_SOC_INTEL_SKL_NAU88L25_SSM4567_MACH is not set
+# CONFIG_SND_SOC_INTEL_SKL_NAU88L25_MAX98357A_MACH is not set
+# CONFIG_SND_SOC_INTEL_BXT_DA7219_MAX98357A_MACH is not set
+# CONFIG_SND_SOC_INTEL_BXT_RT298_MACH is not set
+# CONFIG_SND_SOC_INTEL_KBL_RT5663_MAX98927_MACH is not set
+# CONFIG_SND_SOC_INTEL_KBL_DA7219_MAX98357A_MACH is not set

Are the MFD_INTEL_LPSS options required for the ALPS device? If so please include the missing options. Otherwise, remove the MFD_INTEL_LPSS_* options.

@shiznix

This comment has been minimized.

Copy link
Contributor Author

commented Sep 24, 2018

Hi, no problems and happy to contribute.
Apologies, I'll re-submit the PR as I can see where my error was made.

I cloned master, built the entire project, made the kernel changes using 'make menuconfig', re-built and tested - trackpad now works yay!

However to share the love via a PR I had to delete that clone, fork master, clone my fork and make the changes again - but made the changes by editing the conf file directly instead of using 'make menuconfig' to re-create the conf file.

Yes, MFD_INTEL_LPSS options are required as that enables support for it's Serial IO I2C Controller that the trackpad hardware sits on.

@MilhouseVH

This comment has been minimized.

Copy link
Contributor

commented Sep 24, 2018

Personally I find make oldconfig a little easier. :)

Yes, MFD_INTEL_LPSS options are required as that enables support for it's Serial IO I2C Controller that the trackpad hardware sits on.

OK, in that case this is the change you need: http://ix.io/1npv

You shouldn't need a new PR, just update this one if you can (add and squash a new commit then force push).

@shiznix

This comment has been minimized.

Copy link
Contributor Author

commented Sep 24, 2018

Actually, I can't re-submit because 'master' projects/Generic/linux/linux.x86_64.conf version is 4.18.1, but the kernel used in 'master' that would write the new .config is 4.18.3.

Unsure how to proceed from here...

EDIT: Just read your message, will update with your change :)

@chewitt

This comment has been minimized.

Copy link
Member

commented Sep 24, 2018

The main mistake is working in the master branch instead of a topic branch, as this prevents you from rebasing your changes against our upstream master branch after we make changes to the same file that you've changed. This would clean things up:

git checkout master
git fetch upstream master
git reset --hard upstream/master
git push -f origin master
git checkout -b alps
<make changes to projects/Generic/linux/linux.x86_64.conf>
git add projects/Generic/linux/linux.x86_64.conf
git commit -m "linux: add support for alps trackpads"
git push origin alps

You can now change the source branch of your PR from master to alps via the edit button on the top-right of the GitHub page and the PR will show diff between our master and your alps branches.

Alternatively (now that you pushed commits to fix your master branch while I was writing) you just need to squash the commits (back to a single commit) and force push the change to your master branch to update the current PR, e.g.

git rebase -i HEAD~3
<in edit mode .. 'pick' the original commit
then change the next two commits from 'pick' to 'fixup' and they will be squashed into the original,
or 'squash' if you want to compare the commit names and choose which one to use .. fixup is 
usually quicker .. bonus credit for changing the first commit to 'reword' and changing the message
to follow house style: "linux: add support for alps trackpads">
git push -f origin master

Lesson learned: always work in a topic branch! :)

@shiznix

This comment has been minimized.

Copy link
Contributor Author

commented Sep 24, 2018

Thanks chewitt for the good info :)

@MilhouseVH

This comment has been minimized.

Copy link
Contributor

commented Sep 24, 2018

@shiznix Thanks - please squash into a single commit:

  1. git rebase -i HEAD~3
  2. Drop the first two commits by changing pick to drop on commits bde4749 and 3979847
  3. CTRL-X to exit
  4. Force push: git push -f

I'll include this in tonight's #0924 test build - please confirm once your testing is successful (or not). Thanks.

@shiznix shiznix force-pushed the shiznix:master branch from 06a22bd to a361129 Sep 25, 2018

@MilhouseVH

This comment has been minimized.

Copy link
Contributor

commented Sep 26, 2018

@shiznix thanks for squashing, are you able to confirm that your trackpad is now working in test builds #0924 or #0925 (see testing thread).

@shiznix

This comment has been minimized.

Copy link
Contributor Author

commented Sep 26, 2018

@MilhouseVH I won't have access to that specific hardware for another 3 weeks now.
I can test then but I'm guessing you'll be ahead by more test build versions by that stage...

@chewitt

This comment has been minimized.

Copy link
Member

commented Sep 27, 2018

@shiznix What Dell laptop model do you have? (there's a few older Dell things in cupboards here, so might have something suitable).

@shiznix

This comment has been minimized.

Copy link
Contributor Author

commented Sep 28, 2018

@chewitt It's a Dell Latitude 7380, but the same hardware is present in a lot of (all?) recent Latitude models from 7000 and 5000 series (16 different models in total).
It may also be present in 3000 series models, though not sure on that.

@MilhouseVH

This comment has been minimized.

Copy link
Contributor

commented Oct 15, 2018

Any progress?

@MilhouseVH MilhouseVH self-requested a review Oct 25, 2018

@MilhouseVH
Copy link
Contributor

left a comment

Needs confirmation that the changes work as expected.

@shiznix

This comment has been minimized.

Copy link
Contributor Author

commented Oct 25, 2018

Hi,
Now have access to hardware again and can confirm that trackpad still does not work with latest release.

Am using Kodi 18.0-Beta5-Leia and have updated to the latest test build (#1024 at present).

I'll rebuild the kernel with changes already discussed and drop it into the latest test build to confirm it works.

Is there any particular branch I need to be pulling from to do the kernel, or will 'master' be OK?

@MilhouseVH

This comment has been minimized.

Copy link
Contributor

commented Oct 25, 2018

Am using Kodi 18.0-Beta5-Leia and have updated to the latest test build (#1024 at present).

This change isn't present in #1024 as my test builds are now based on 4.19.0 and this PR doesn't apply to 4.19.0 without modification, and since it wasn't looking good for this PR to be merged I dropped the customised version of this PR from 4.19.0 starting with #1024.

You should test #1023 as that included a custom version of this PR suitable for 4.19.0. And of course #1022 which is the last 4.18.y build that included this PR.

If you confirm that both #1022 and #1023 are working then we can merge this PR and I'll rebase the 4.19 PR #2937 to take this change into account. However if #2937 is merged (in a day or so) before this PR then you'll need to rebase this PR on top of 4.19.0, so time is of the essence...

@shiznix

This comment has been minimized.

Copy link
Contributor Author

commented Oct 25, 2018

Too easy!

Have just tested builds #1022 and #1023 and they both work beautifully!

Trackpad functions well, even two finger scrolling works out the box :)

Thanks for sticking with this and getting it to work, much appreciated.

@MilhouseVH

This comment has been minimized.

Copy link
Contributor

commented Oct 25, 2018

Thanks.

@MilhouseVH MilhouseVH merged commit 868a61b into LibreELEC:master Oct 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.