Re: [PATCH v5 0/1] platform/x86/tuxedo: Add virtual LampArray for TUXEDO NB04 devices

[Pavel Machek <pavel@ucw.cz>]

[-- Attachment #1: Type: text/plain, Size: 1892 bytes --]

Hi!

> > I now got my feet a little wet with hid-bpf regarding something else, and
> > with that knowledge I would leave the long arrays in the beginning in the
> > kernel code for the time being:
> > 
> > sirius_16_ansii_kbl_mapping and sirius_16_iso_kbl_mapping are required
> > during initialization so they have to exist in the kernel code anyway.
> > 
> > report_descriptor will most likly not change even for future models and
> > afaik having report_descriptors in kernel drivers is not unheard of.
> > 
> > So the only things that could be meaningfully moved to a hid-bpf program
> > are the sirius_16_*_kbl_mapping_pos_* arrays. But for these is have to give
> > out some fallback value anyway for the case where a hid-bpf file is missing
> > or fails to load. So why not use real world values from my test device for
> > these values?
> > 
> > As soon as there is a future device that can use the same driver with just
> > these pos arrays different, then I would implement that change via a bpf
> > program instead of a change to the kernel driver.
> > 
> > Let me know if you too think this is a sensefull approach?
> > 
> > 
> > Another question: Would this patch need to wait for a userspace
> > implementation of lamp array before it can get accepted?
> 
> It would be nice if you could test the LampArray implementation. But other than that
> userspace can catch up later.
> 
> Still, i am interested in the opinion of the LED maintainers
> regarding the fake HID interface.

Comments from previous review were not addressed.

Most importantly, this is not a way to do kernel interface. We want
reasonable interface that can be documented and modified as needed. We
want to pass /dev/input to userspace, not raw HID. This is not ok.

Best regards,
								Pavel
-- 
People of Russia, stop Putin before his war on Ukraine escalates.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]