Discussion:
Kernel Patches
(too old to reply)
Shawn W Dunn
2014-07-29 13:39:59 UTC
Permalink
Raw Message
So, what is the generally accepted procedure for kernel patches?

I have one to enable type cover keyboard support for the MS Surface Pro
devices, and coolo suggested I ask here, instead of pushing it directly in OBS
to the factory kernel
Andrew Wafaa
2014-07-29 14:14:35 UTC
Permalink
Raw Message
Post by Shawn W Dunn
So, what is the generally accepted procedure for kernel patches?
Submit your patch for review to this mailing list. It would be
advisable to also say which branches you tested against -
13.1/FactorySLE/etc. Other than that there's not much else really :)
Post by Shawn W Dunn
I have one to enable type cover keyboard support for the MS Surface Pro
devices, and coolo suggested I ask here, instead of pushing it directly in OBS
to the factory kernel
Sounds like a useful patch, do you happen to know if it has been
submitted upstream?

Regards,
Andy
Jeff Mahoney
2014-07-29 14:18:24 UTC
Permalink
Raw Message
Post by Shawn W Dunn
So, what is the generally accepted procedure for kernel patches?
I have one to enable type cover keyboard support for the MS Surface
Pro devices, and coolo suggested I ask here, instead of pushing it
directly in OBS to the factory kernel
Hi Shawn -

I did see your patch sent to me. Apologies, I was away over the
weekend and hadn't gotten to it yet. Quick review is that the content
is ok, but you need to add more documentation to it in order for it to
be accepted. I don't mean in the form of comments in the code -- but
documentation as to what it fixes and your Signed-off-by.

The generally accepted practice is that a patch must be accepted into
the upstream kernel or a 'maintainer' branch that is known to flow
directly into the upstream kernel on a regular basis. To do otherwise
would make the openSUSE kernel divergent from mainline rather than
just a branch with backports[1].

Once your patch is upstream, we can include it into the openSUSE kernel.

A good place to start would be to look at the SubmittingPatches[2]
file in the upstream kernel source and the README[3] file in our own
kernel-source repository. The former documents what a patch should
generally look like for inclusion in the upstream kernel. The latter
documents the specific rules that must be followed for us to accept it
into the openSUSE kernel. Once you have everything set for the
upstream kernel, all you'll need to do is open a bug report at
bugzilla.novell.com, add a reference to it in your patch using the
"References" tag, and add the "Patch-mainline" and "Git-commit" tags
that reference where the commit landed upstream. There are examples of
acceptable patch headers in the README file. Really, though, picking
nearly any other patch already in the repository would be a good
source to compare against.

As for asking here instead of pushing directly to OBS, that's the
right thing to do. The kernel-source package is maintained in a
separate git repository[4] and patches against the OBS package will
either be (correctly) declined or, if someone violated the rules, will
be lost with the next sync from the git repository.

I know this is cumbersome, but the documentation rules are mostly from
the upstream kernel. They exist for long-term maintainability of the
kernel. I know you just want to get your tablet's keyboard working,
but everyone wants to just get /something/ working and we'd end up
with a mess if we accepted patches outside of the rules.

Unfortunately, it seems that the HID subsystem doesn't support the
new_id mechanism that PCI and general USB devices do (HID is an
abstraction layer above the USB bus). Otherwise, it would be possible
to tell the driver about the new IDs at runtime until your kernel
patch is accepted.

I've added Jiri Kosina and Jiri Slaby, who are both active maintainers
in the input/HID area, and may be able to help you once you get your
patch in shape.

- -Jeff

[1] The SUSE "supported" patches and Xen are notable counterexamples
here. The former will necessarily always be out-of-tree. There are
several SUSE engineers working full-time on getting Xen reworked and
merged upstream.
[2]
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches
[3] http://kernel.opensuse.org/cgit/kernel-source/tree/README
[4] http://kernel.opensuse.org/cgit/kernel-source/log/

- --
Jeff Mahoney
SUSE Labs
Jiri Slaby
2014-07-29 14:26:21 UTC
Permalink
Raw Message
Post by Jeff Mahoney
I know this is cumbersome, but the documentation rules are mostly from
the upstream kernel. They exist for long-term maintainability of the
kernel. I know you just want to get your tablet's keyboard working,
but everyone wants to just get /something/ working and we'd end up
with a mess if we accepted patches outside of the rules.
What a nice write-up, Jeff!
Post by Jeff Mahoney
Unfortunately, it seems that the HID subsystem doesn't support the
new_id mechanism that PCI and general USB devices do (HID is an
abstraction layer above the USB bus). Otherwise, it would be possible
to tell the driver about the new IDs at runtime until your kernel
patch is accepted.
new_id support was added some time later after the conversion to bus. So
Shawn, you actually can echo something to that file to make your device
working. But since I see no device information here, I cannot tell you
what exactly to write to which driver in /sys/bus/hid/drivers/*/new_id

thanks,
--
js
suse labs
Jeff Mahoney
2014-07-29 14:30:23 UTC
Permalink
Raw Message
Post by Jiri Slaby
Post by Jeff Mahoney
I know this is cumbersome, but the documentation rules are mostly
from the upstream kernel. They exist for long-term
maintainability of the kernel. I know you just want to get your
tablet's keyboard working, but everyone wants to just get
/something/ working and we'd end up with a mess if we accepted
patches outside of the rules.
What a nice write-up, Jeff!
Post by Jeff Mahoney
Unfortunately, it seems that the HID subsystem doesn't support
the new_id mechanism that PCI and general USB devices do (HID is
an abstraction layer above the USB bus). Otherwise, it would be
possible to tell the driver about the new IDs at runtime until
your kernel patch is accepted.
new_id support was added some time later after the conversion to
bus. So Shawn, you actually can echo something to that file to make
your device working. But since I see no device information here, I
cannot tell you what exactly to write to which driver in
/sys/bus/hid/drivers/*/new_id
Ah, yeah, I tested it on 3.12. I didn't test it with a newer version.
If that's the case, then it makes things nicer in the short term. It
should be as simple as:

echo -n "0x045e 0x07dc" > /sys/bus/hid/drivers/hid_microsoft/new_id

It could be hid_microsoft, or hid-microsoft. I don't have a 3.15
kernel booted to check it with.

- -Jeff


- --
Jeff Mahoney
SUSE Labs
Jiri Kosina
2014-07-29 14:35:37 UTC
Permalink
Raw Message
Post by Jeff Mahoney
Ah, yeah, I tested it on 3.12. I didn't test it with a newer version.
If that's the case, then it makes things nicer in the short term. It
echo -n "0x045e 0x07dc" > /sys/bus/hid/drivers/hid_microsoft/new_id
It could be hid_microsoft, or hid-microsoft. I don't have a 3.15
kernel booted to check it with.
You also very likely would need to unbind hid-generic from the keyboard
(if it has been claimed by the generic driver) and bind hid-microsoft to
it.

It can be done also through sysfs.

Being the upstream maintainer of this code, I'd be happy to accept the
patch to the upstream kernel once it's sent my way.
--
Jiri Kosina
SUSE Labs
Shawn W Dunn
2014-07-29 14:30:42 UTC
Permalink
Raw Message
Post by Jeff Mahoney
I know this is cumbersome, but the documentation rules are mostly from
the upstream kernel. They exist for long-term maintainability of the
kernel. I know you just want to get your tablet's keyboard working,
but everyone wants to just get /something/ working and we'd end up
with a mess if we accepted patches outside of the rules.
Oh, I've got a kernel built that works for *me*, it would just be nice to get
it out there so everybody can use it. Especially considering nobody is
shipping a linux distro with full OOB support for the Surface Pro tablets.

(and we still might not be able to, there is a firmware update for the
wifi/bluetooth in it, but I'm still trying to verify if it is licensed for
redistribution or not)
Loading...