new-lg4ff icon indicating copy to clipboard operation
new-lg4ff copied to clipboard

Upstreaming the driver

Open RyuzakiKK opened this issue 4 years ago • 14 comments

Is there any plan for upstreaming this module driver to expand/replace the current in-tree hid-logitech?

RyuzakiKK avatar Dec 08 '20 13:12 RyuzakiKK

That's what I'd like to do. I've been planning it in my head but I think there's some work to do and I'm not sure how well will it be received. I just need some motivation and a bit of time.

I think I should break the changes into several patches for each feature, there would be a big one for the effects implementation and other smaller patches for the secondary features. And for every patch I work on, send it upstream and try to get it merged.

Any advice and guiding from someone knowledgeable would help at this point.

berarma avatar Dec 08 '20 17:12 berarma

That's what I'd like to do. I've been planning it in my head but I think there's some work to do and I'm not sure how well will it be received. I just need some motivation and a bit of time.

I think I should break the changes into several patches for each feature, there would be a big one for the effects implementation and other smaller patches for the secondary features. And for every patch I work on, send it upstream and try to get it merged.

Any advice and guiding from someone knowledgeable would help at this point.

Hi :)

I have contributed to Linux before, although only a minor patch (adding a device ID to a serial device, many moons ago). But! It isn't too difficult and don't get too intimidated.

It looks like the HID device maintainer list is at [email protected], as found in the MAINTAINERS file:

HID CORE LAYER
M:	Jiri Kosina <[email protected]>
M:	Benjamin Tissoires <[email protected]>
L:	[email protected]
S:	Maintained
T:	git git://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git
F:	drivers/hid/
F:	include/linux/hid*
F:	include/uapi/linux/hid*

It's best to touch base with them first and get feedback on your changes and how they want to go about this.

Make sure all your code follows the kernel coding style. I haven't looked too deeply at the code, so I'm not sure if it all conforms, but I saw no obvious issues.

Elizafox avatar Mar 29 '21 19:03 Elizafox

Thanks for sharing!

I was just starting to take it more seriously. I wanted to simplify the changes before starting commiting patches but I'm hesitating about talking to them first.

When you say touching base do you mean explaining them the changes without sending any code?

berarma avatar Mar 29 '21 20:03 berarma

I think #49 should be fixed before trying to upstream the driver.

berarma avatar Aug 19 '21 13:08 berarma

#49 is possibly fixed but needs someone with a G29 and Euro Truck Sim 1.41 to verify, You have to use the master tree as it was after version 0.4.0 where the issue might have been fixed as it fixed my issue with auto centering on the DFGT.

motolav avatar Jan 31 '23 04:01 motolav

#49 is possibly fixed but needs someone with a G29 and Euro Truck Sim 1.41 to verify, You have to use the master tree as it was after version 0.4.0 where the issue might have been fixed as it fixed my issue with auto centering on the DFGT.

Nothing has been fixed in the driver, it's that ETS2 has changed the way it uses FFB.

I haven't tried to upstream the driver because, due to my limited time, it'll most probably be a lot of work for me to create the patches they'll want. And if they're picky, I have already received hints that they will, they could block the important parts because they're not a perfect implementation. The truth is that the driver implements the features games use but in corner cases it wouldn't do what it's supposed to do.

Given the high probability that I could do the work just to be rejected, I'll wait until someone close to the kernel people gives me an indication that they're really interested and tell me which parts should be improved before approaching upstream.

berarma avatar Jan 31 '23 09:01 berarma

Nothing has been fixed in the driver, it's that ETS2 has changed the way it uses FFB.

I was testing with the OLD FFB version of ETS2 that's why i specified 1.41 as its the old FFB model, versions 1.42 and after use the new FFB model. There absolutely was an unintentional fix in the driver with the changes after 0.4.0.

Given the high probability that I could do the work just to be rejected, I'll wait until someone close to the kernel people gives me an indication that they're really interested and tell me which parts should be improved before approaching upstream.

Submitting your code or someone else on your behalf to the mailing list is the only real way to actually get help on what needs to be changed

motolav avatar Jan 31 '23 20:01 motolav

Thats's right, even if you are uninterested in doing all this work, it may attract actual kernel dev, having this hardware and taking over this process in the end. Logitech wheels are most popular BTW,

On 1/31/23, motolav @.***> wrote:

Nothing has been fixed in the driver, it's that ETS2 has changed the way it uses FFB.

I was testing with the OLD FFB version of ETS2 that's why i specified 1.41 as its the old FFB model, versions 1.42 and after use the new FFB model. There absolutely was an unintentional fix in the driver with the changes after 0.4.0.

Given the high probability that I could do the work just to be rejected, I'll wait until someone close to the kernel people gives me an indication that they're really interested and tell me which parts should be improved before approaching upstream.

Submitting your code or someone else on your behalf to the mailing list is the only real way to actually get help on what needs to be changed

-- Reply to this email directly or view it on GitHub: https://github.com/berarma/new-lg4ff/issues/35#issuecomment-1411034233 You are receiving this because you are subscribed to this thread.

Message ID: @.***>

isopix avatar Jan 31 '23 20:01 isopix

I'll try to get in touch with the kernel maintainers and ask them what would I need to do to upstream it. Maybe it's not that hard.

berarma avatar Jan 31 '23 21:01 berarma

Nothing has been fixed in the driver, it's that ETS2 has changed the way it uses FFB.

I was testing with the OLD FFB version of ETS2 that's why i specified 1.41 as its the old FFB model, versions 1.42 and after use the new FFB model. There absolutely was an unintentional fix in the driver with the changes after 0.4.0.

That's very weird. I'll have to try it.

berarma avatar Jan 31 '23 21:01 berarma

I'll try to get in touch with the kernel maintainers and ask them what would I need to do to upstream it. Maybe it's not that hard.

Setting up an email client is the hardest part if you want people to be able to compile and test your code from an email. You will likely have to make a new repo/branch to create either a single large commit of all your changes or a few smaller commits of all your major changes.

motolav avatar Jan 31 '23 21:01 motolav

I pasted your driver into the kernel code see the diff and it seems a bit too much to submit as one commit, 1544 insertions(+), 531 deletions(-). (I manually updated hid-ids.h and hid-lg.c as there was a few changes after your fork)

motolav avatar Feb 03 '23 22:02 motolav

Don't worry about your code being bug-free before submitting, as long as it isn't going to break anything that works today. You are adding functionality that is lacking. There are totally bugs, when they are known, they can be fixed with patches. Break your patches up as you said above, into logical units of work. Make sure each one compiles cleanly. Run scripts/checkpatch.pl against each one to ensure you have correct formatting. Set up git to send email and use "git send-email" to submit them. Cc the necessary lists (always lkml), and send it to the maintainer of the subsystem and anyone else that has contributed a lot to the code you are changing (use ./scripts/get_maintainer.pl to find out who) Base your code on either Linus' branch or the subsystems git reop and tell the maintainer in the cover letter (git format-patch - --cover-letter, edit 000-cover-letter.patch and add details on the changes)

howlett avatar Jun 14 '23 00:06 howlett

Maybe it's not that hard.

Indeet, it's not. As a total beginner in kernel development, I contributed some minor HID patches and an insignificant gamepad device driver. The HID maintainers were friendly folks and very patient, contributing was a pleasant experience. Your driver is very helpful and would be a very good addition to the mainline kernel. Please be encouraged to go on upstreaming it. Thanks!

hzulla avatar Nov 05 '23 18:11 hzulla