Re: [PATCH][RfC] Some buttons on the Fujitsu u7[25]7 laptops are not working

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> > For me, adding support for the new systems in an elegant way would mean:
> > 
> >   - detecting models without dedicated hotkeys and forcing an immediate
> >     release event when a "press" is detected on such models; we can take
> >     a shot at bit 29 being the indicator, the worst that is going to
> >     happen is that we will get double release events on some machines,
> 
> The effect of a double release depends on the way the key events go on to be
> handled.  If they trigger on key-down there will be no issue.  If they go on
> key-up then each key will get pressed twice for each physical action.  I
> guess time will tell.
> 
> >   - keeping GIRB and S000 keymaps separate as these two hotkey handling
> >     methods are completely different: the former utilizes a ring buffer,
> >     the latter relies on setting bits in the return value of a function
> >     and clearing them after they have been "retrieved"; the two keymaps
> >     would then have to be merged into a single sparse keymap upon device
> >     instantiation.
> > 
> > However, it is Jonathan who is the module maintainer and Jan-Marek's
> > patch does not break stuff.  I am thus happy to present my vision after
> > this patch gets merged.
> 
> In the interests of making things work for owners of the newer systems in
> the short term I am inclined to say we merge the patch from Jan-Marek, on
> the understanding that the more extensive infrastructure changes will be
> needed in order to handle this in a sane way into the future.  I don't think
> the proposed patch will make it any harder to effect the changes you have in
> mind, will it?

No worries, it will not.  There are, however, a couple of "procedural"
issues with Jan-Marek's patch which I listed below.

Jan-Marek, first, please ensure "checkpatch.pl --strict" run on your
patch does not report any issues (e.g. there is a typo in your SOB).

Once that is dealt with, please ensure that you:

  - Send your patch as a separate email, not as part of another message.
    Obviously this is okay to do for draft patches, like the ones you
    sent earlier in this thread, but not really for stuff you would
    actually like to see merged.  "git send-email" is your friend.

  - Make sure your subject line includes the "<subsystem>: <driver>:"
    prefix, i.e. "platform/x86: fujitsu-laptop:".

  - Rename "funcret" to "ret".  I know fujitsu-laptop is not yet
    consistent in naming such variables, but it soon will be, so please
    change it.

  - Not a technical issue, but rather one style-related (just my two
    cents): I would rephrase the subject and the commit log message to:

        platform/x86: fujitsu-laptop: Support Lifebook U7x7 hotkeys

	Generate input events for hotkeys present in Fujitsu Lifebook
	U727 and U757 laptops: Fn+F1 (KEY_MICMUTE) and Fn+F5
	(KEY_RFKILL).

    This is basically what the patch does.  The rest of what you wrote
    in the log message of your draft patch seems redundant to me.

> I'm all for elegant solutions, so I would welcome patches working towards
> the broader ideas you outlined above.  Keeping both handling methods
> separate does seem to be a good idea.

Okay, I will try to prepare something in this regard.

-- 
Best regards,
Michał Kępień



[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux