Bug 88410

It's a question of ordering. Since both the generic MICRO-STAR and the more specific U100/U90/etc patterns match, which ever gets assigned last wins. I guess that this is by design, since the documentation does not say anything about the order. Let's see what Kay thinks.

@Kay: ^^^

(In reply to Zbigniew Jedrzejewski-Szmek from comment #1)
> It's a question of ordering. Since both the generic MICRO-STAR and the more
> specific U100/U90/etc patterns match, which ever gets assigned last wins. I
> guess that this is by design, since the documentation does not say anything
> about the order. Let's see what Kay thinks.
> 
> @Kay: ^^^

Indeed, I tried to remove that lines from generic rule:

 KEYBOARD_KEY_f7=brightnessdown                         # Fn+F4
 KEYBOARD_KEY_f8=brightnessup                           # Fn+F5

And udevadm info /dev/input/event5 shows f7 and f8 as reserved, so *U100* rule works. So asterisk is not a bug. MSI rules rely on order of applying, but it is unspecified.

I think the order in which rules are applied has to be specified. Old 95-keymap.rules were applied from top to bottom in order. New 60-keyboard.hwdb is simply generated from 95-keymap.rules. But the ordering is broken now. Maybe, MSI is not the only case where it is important.

(In reply to Zbigniew Jedrzejewski-Szmek from comment #1)
> It's a question of ordering. Since both the generic MICRO-STAR and the more
> specific U100/U90/etc patterns match, which ever gets assigned last wins. I
> guess that this is by design, since the documentation does not say anything
> about the order. Let's see what Kay thinks.

Right. All matches are applied in order to allow merging of multiple variables from different sections/files. If the specified key names are identical, the later ones overwrite the earlier ones. That is the expected behavior.

> the later ones overwrite the earlier ones
"later" in what sense: ordered lower in the file?

(In reply to Zbigniew Jedrzejewski-Szmek from comment #4)
> > the later ones overwrite the earlier ones
> "later" in what sense: ordered lower in the file?

It's just read as a stream of records. All file names are sorted, the file contents are applied record -by-record from top to bottom.

(In reply to Kay Sievers from comment #5)
> It's just read as a stream of records. All file names are sorted, the file
> contents are applied record -by-record from top to bottom.

This seem to do not work. When two rules are present:

keyboard:dmi:bvn*:bvr*:bd*:svnMICRO-STAR*:pn*
keyboard:dmi:bvn*:bvr*:bd*:svnMICRO-STAR*:pn*U100*:pvr*

the first one wins independently of order in file.

When these two rules are present:

keyboard:dmi:bvn*:bvr*:bd*:svnMICRO-STAR*:pn*
keyboard:dmi:bvn*:bvr*:bd*:svnMICRO-STAR*:pnU90/U100:*

the second one always wins independently of order in file.

It looks like the order in file is ignored, only the names are important.

> All file names are sorted, the file contents are applied record -by-record from top to bottom.
Kay, I think you're thiking about rules files. This is about the hwdb.

I added some debugging statements to figure this out.

fnmatch trie: "*:bvr*:bd*:svnMICRO-STAR*:pn*U100*:pvr*" → yes
hwdb_add_property: KEYBOARD_KEY_f7 → reserved
hwdb_add_property: KEYBOARD_KEY_f8 → reserved
fnmatch trie: "*:bvr*:bd*:svnMICRO-STAR*:pn*" → yes
hwdb_add_property: KEYBOARD_KEY_a0 → mute
hwdb_add_property: KEYBOARD_KEY_ae → volumedown
hwdb_add_property: KEYBOARD_KEY_b0 → volumeup
hwdb_add_property: KEYBOARD_KEY_b2 → www
hwdb_add_property: KEYBOARD_KEY_df → sleep
hwdb_add_property: KEYBOARD_KEY_e2 → bluetooth
hwdb_add_property: KEYBOARD_KEY_e4 → f21
hwdb_add_property: KEYBOARD_KEY_ec → email
hwdb_add_property: KEYBOARD_KEY_ee → camera
hwdb_add_property: KEYBOARD_KEY_f6 → wlan
hwdb_add_property: KEYBOARD_KEY_f7 → brightnessdown
hwdb_add_property: KEYBOARD_KEY_f8 → brightnessup
hwdb_add_property: KEYBOARD_KEY_f9 → search
fnmatch trie: "*:bvr*:bd*:svnMICRO-STAR*:pnU90/U100:*" → yes
hwdb_add_property: KEYBOARD_KEY_e4 → reserved

while hwdb file has:

keyboard:dmi:bvn*:bvr*:bd*:svnMICRO-STAR*:pn*
keyboard:dmi:bvn*:bvr*:bd*:svnMicro-Star*:pn*
 KEYBOARD_KEY_a0=mute                                   # Fn+F9
 KEYBOARD_KEY_ae=volumedown                             # Fn+F7
 KEYBOARD_KEY_b0=volumeup                               # Fn+F8
 KEYBOARD_KEY_b2=www                                    # e button
 KEYBOARD_KEY_df=sleep                                  # Fn+F12
 KEYBOARD_KEY_e2=bluetooth                              # satellite dish2
 KEYBOARD_KEY_e4=f21                                    # Fn+F3 Touchpad disable
 KEYBOARD_KEY_ec=email                                  # envelope button
 KEYBOARD_KEY_ee=camera                                 # Fn+F6 camera disable
 KEYBOARD_KEY_f6=wlan                                   # satellite dish1
 KEYBOARD_KEY_f7=brightnessdown                         # Fn+F4
 KEYBOARD_KEY_f8=brightnessup                           # Fn+F5
 KEYBOARD_KEY_f9=search

# some MSI models generate ACPI/input events on the LNXVIDEO input devices,
# plus some extra synthesized ones on atkbd as an echo of actually changing the
# brightness; so ignore those atkbd ones, to avoid loops
keyboard:dmi:bvn*:bvr*:bd*:svnMICRO-STAR*:pn*U-100*:pvr*
keyboard:dmi:bvn*:bvr*:bd*:svnMICRO-STAR*:pn*U100*:pvr*
keyboard:dmi:bvn*:bvr*:bd*:svnMICRO-STAR*:pn*N033:*
 KEYBOARD_KEY_f7=reserved
 KEYBOARD_KEY_f8=reserved

# MSI Wind U90/U100 generates separate touchpad on/off keycodes so ignore touchpad toggle keycode
keyboard:dmi:bvn*:bvr*:bd*:svnMICRO-STAR*:pnU90/U100:*
 KEYBOARD_KEY_e4=reserved

(In reply to Zbigniew Jedrzejewski-Szmek from comment #7)
> > All file names are sorted, the file contents are applied record -by-record from top to bottom.
> Kay, I think you're thiking about rules files. This is about the hwdb.

No, all fine, this was about hwdb.

> I added some debugging statements to figure this out.
> 
> fnmatch trie: "*:bvr*:bd*:svnMICRO-STAR*:pn*U100*:pvr*" → yes
> hwdb_add_property: KEYBOARD_KEY_f7 → reserved
> hwdb_add_property: KEYBOARD_KEY_f8 → reserved

These are the keys which are specified later in the source hwdb files, but
they are applied first here. These later ones are supposed to overwrite the
earlier ones. The "reserved" should win here.

This is a bug in the hwdb code. Maybe it things changed recently regarding
this logic?

Those entries are put in the trie. How is the ordering supposed to be preserved?

(In reply to Kay Sievers from comment #8)
> (In reply to Zbigniew Jedrzejewski-Szmek from comment #7)
> > > All file names are sorted, the file contents are applied record -by-record from top to bottom.
> > Kay, I think you're thiking about rules files. This is about the hwdb.
> 
> No, all fine, this was about hwdb.
> 
> > I added some debugging statements to figure this out.
> > 
> > fnmatch trie: "*:bvr*:bd*:svnMICRO-STAR*:pn*U100*:pvr*" → yes
> > hwdb_add_property: KEYBOARD_KEY_f7 → reserved
> > hwdb_add_property: KEYBOARD_KEY_f8 → reserved
> 
> These are the keys which are specified later in the source hwdb files, but
> they are applied first here. These later ones are supposed to overwrite the
> earlier ones. The "reserved" should win here.
> 
> This is a bug in the hwdb code. Maybe it things changed recently regarding
> this logic?

Oh, we have multiple records with _different_ match strings which get merged.
I forgot about this detail there.

With different match strings, it should not be about the order in the
file, but about the length of the match string. The data is put in a
patricia trie and the order of individual records should _not_ matter for _different_ match strings.

The intended behavior is that records with longer match strings overwrite
shorter match strings. That means, things which are stored "deeper" in the
tree are more specific/"better" matches than the broader ones stored closer
to the root. And the more specific ones should overwrite the broader ones.

Seem like this logic should be fixed.