Message ID | 20220322101406.459e2950@coco.lan (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [GIT,PULL,for,v5.18-rc1] media updates | expand |
From: builder@linuxtv.org
Pull request: https://patchwork.linuxtv.org/project/linux-media/patch/20220322101406.459e2950@coco.lan/
Build log: https://builder.linuxtv.org/job/patchwork/192983/
Build time: 00:00:00
Link: https://lore.kernel.org/linux-media/20220322101406.459e2950@coco.lan
gpg: Signature made Tue 22 Mar 2022 08:51:45 AM UTC
gpg: using RSA key F909AE68FC11DF09C1755C00085F3EBD8EE4E115
gpg: Good signature from "Mauro Carvalho Chehab <mchehab+huawei@kernel.org>" [unknown]
gpg: aka "Mauro Carvalho Chehab <mchehab@kernel.org>" [unknown]
gpg: aka "Mauro Carvalho Chehab <m.chehab@samsung.com>" [unknown]
gpg: aka "Mauro Carvalho Chehab <mchehab@osg.samsung.com>" [unknown]
gpg: aka "Mauro Carvalho Chehab <mchehab@s-opensource.com>" [unknown]
gpg: aka "[jpeg image of size 3594]" [unknown]
gpg: aka "Mauro Carvalho Chehab <mchehab+samsung@kernel.org>" [unknown]
gpg: WARNING: This key is not certified with a trusted signature!
gpg: There is no indication that the signature belongs to the owner.
Primary key fingerprint: F909 AE68 FC11 DF09 C175 5C00 085F 3EBD 8EE4 E115
Build aborted due to a fatal error:
FAILED: patch patch patches/0001-media-staging-media-zoran-move-module-parameter-chec.patch doesn't apply:
Applying patch patches/0001-media-staging-media-zoran-move-module-parameter-chec.patch
patching file drivers/staging/media/zoran/zoran_card.c
Hunk #1 succeeded at 1244 with fuzz 2 (offset 177 lines).
Hunk #2 FAILED at 1318.
1 out of 2 hunks FAILED -- rejects in file drivers/staging/media/zoran/zoran_card.c
Patch patches/0001-media-staging-media-zoran-move-module-parameter-chec.patch does not apply (enforce with -f)
The pull request you sent on Tue, 22 Mar 2022 10:14:06 +0100:
> git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media tags/media/v5.18-1
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/182966e1cd74ec0e326cd376de241803ee79741b
Thank you!
On 22. 03. 22, 10:14, Mauro Carvalho Chehab wrote: > Hi Linus, > > Please pull from: > git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media tags/media/v5.18-1 ... > Sean Young (10): ... > media: lirc: remove unused lirc features Hi, this breaks lirc build: > [ 59s] lircd.cpp:489:49: error: 'LIRC_CAN_SET_REC_FILTER' was not declared in this scope; did you mean 'LIRC_CAN_SET_REC_CARRIER'? > [ 59s] 489 | || (curr_driver->features & LIRC_CAN_SET_REC_FILTER)) { > [ 59s] | ^~~~~~~~~~~~~~~~~~~~~~~ > [ 59s] | LIRC_CAN_SET_REC_CARRIER > [ 59s] lircd.cpp: In function 'void loop()': > [ 59s] lircd.cpp:2069:82: error: 'LIRC_CAN_NOTIFY_DECODE' was not declared in this scope; did you mean 'DRVCTL_NOTIFY_DECODE'? > [ 59s] 2069 | if (curr_driver->drvctl_func && (curr_driver->features & LIRC_CAN_NOTIFY_DECODE)) > [ 59s] | ^~~~~~~~~~~~~~~~~~~~~~ > [ 59s] | DRVCTL_NOTIFY_DECODE So the uapi header defines should be brought back, IMO. thanks,
On 25. 05. 22, 8:42, Jiri Slaby wrote: > On 22. 03. 22, 10:14, Mauro Carvalho Chehab wrote: >> Hi Linus, >> >> Please pull from: >> git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media >> tags/media/v5.18-1 > ... >> Sean Young (10): > ... >> media: lirc: remove unused lirc features > > Hi, > > this breaks lirc build: >> [ 59s] lircd.cpp:489:49: error: 'LIRC_CAN_SET_REC_FILTER' was not >> declared in this scope; did you mean 'LIRC_CAN_SET_REC_CARRIER'? >> [ 59s] 489 | || (curr_driver->features & >> LIRC_CAN_SET_REC_FILTER)) { >> [ 59s] | >> ^~~~~~~~~~~~~~~~~~~~~~~ >> [ 59s] | >> LIRC_CAN_SET_REC_CARRIER >> [ 59s] lircd.cpp: In function 'void loop()': >> [ 59s] lircd.cpp:2069:82: error: 'LIRC_CAN_NOTIFY_DECODE' was not >> declared in this scope; did you mean 'DRVCTL_NOTIFY_DECODE'? >> [ 59s] 2069 | if (curr_driver->drvctl_func >> && (curr_driver->features & LIRC_CAN_NOTIFY_DECODE)) >> [ 59s] >> | >> ^~~~~~~~~~~~~~~~~~~~~~ >> [ 59s] >> | >> DRVCTL_NOTIFY_DECODE > > So the uapi header defines should be brought back, IMO. (And lirc fixed at the same time.) > thanks,-- js
Hi, On Wed, May 25, 2022 at 08:42:26AM +0200, Jiri Slaby wrote: > On 22. 03. 22, 10:14, Mauro Carvalho Chehab wrote: > > Hi Linus, > > > > Please pull from: > > git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media tags/media/v5.18-1 > ... > > Sean Young (10): > ... > > media: lirc: remove unused lirc features > > Hi, > > this breaks lirc build: > > [ 59s] lircd.cpp:489:49: error: 'LIRC_CAN_SET_REC_FILTER' was not declared in this scope; did you mean 'LIRC_CAN_SET_REC_CARRIER'? > > [ 59s] 489 | || (curr_driver->features & LIRC_CAN_SET_REC_FILTER)) { > > [ 59s] | ^~~~~~~~~~~~~~~~~~~~~~~ > > [ 59s] | LIRC_CAN_SET_REC_CARRIER > > [ 59s] lircd.cpp: In function 'void loop()': > > [ 59s] lircd.cpp:2069:82: error: 'LIRC_CAN_NOTIFY_DECODE' was not declared in this scope; did you mean 'DRVCTL_NOTIFY_DECODE'? > > [ 59s] 2069 | if (curr_driver->drvctl_func && (curr_driver->features & LIRC_CAN_NOTIFY_DECODE)) > > [ 59s] | ^~~~~~~~~~~~~~~~~~~~~~ > > [ 59s] | DRVCTL_NOTIFY_DECODE > > So the uapi header defines should be brought back, IMO. The lirc.h uapi defines the lirc chardev uapi. The uapi has not changed in any way, for old or new kernels. So the lirc header used to have feature flags LIRC_CAN_SET_REC_FILTER and LIRC_CAN_NOTIFY_DECODE which were defined the in the lirc.h header, but never implemented by any out of tree or in tree driver. Neither feature was or will be ever implemented in the kernel; LIRC_CAN_NOTIFY_DECODE is handled via the led subsytem, and it is unknown what LIRC_CAN_SET_REC_FILTER is even supposed to mean. Again, I have not found any implementation anywhere. You are trying to build lirc user space daemon which is no longer maintained. The last time the lirc daemon git repo had any commits was in 2019. User space tooling has been replaced with daemon-less ir-ctl and ir-keytable, which uses BPF for IR decoding. The right fix is to simply delete the offending lines in lircd.cpp and all will be well. Sometimes source code needs a little maintainence. These changes in the lirc uapi do not change the uapi in any way, just the ability the build some unmaintained software without trivial changes. Thanks, Sean
On 25. 05. 22, 9:40, Sean Young wrote: > Hi, > > On Wed, May 25, 2022 at 08:42:26AM +0200, Jiri Slaby wrote: >> On 22. 03. 22, 10:14, Mauro Carvalho Chehab wrote: >>> Hi Linus, >>> >>> Please pull from: >>> git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media tags/media/v5.18-1 >> ... >>> Sean Young (10): >> ... >>> media: lirc: remove unused lirc features >> >> Hi, >> >> this breaks lirc build: >>> [ 59s] lircd.cpp:489:49: error: 'LIRC_CAN_SET_REC_FILTER' was not declared in this scope; did you mean 'LIRC_CAN_SET_REC_CARRIER'? >>> [ 59s] 489 | || (curr_driver->features & LIRC_CAN_SET_REC_FILTER)) { >>> [ 59s] | ^~~~~~~~~~~~~~~~~~~~~~~ >>> [ 59s] | LIRC_CAN_SET_REC_CARRIER >>> [ 59s] lircd.cpp: In function 'void loop()': >>> [ 59s] lircd.cpp:2069:82: error: 'LIRC_CAN_NOTIFY_DECODE' was not declared in this scope; did you mean 'DRVCTL_NOTIFY_DECODE'? >>> [ 59s] 2069 | if (curr_driver->drvctl_func && (curr_driver->features & LIRC_CAN_NOTIFY_DECODE)) >>> [ 59s] | ^~~~~~~~~~~~~~~~~~~~~~ >>> [ 59s] | DRVCTL_NOTIFY_DECODE >> >> So the uapi header defines should be brought back, IMO. > > The lirc.h uapi defines the lirc chardev uapi. The uapi has not changed in > any way, for old or new kernels. > > So the lirc header used to have feature flags LIRC_CAN_SET_REC_FILTER and > LIRC_CAN_NOTIFY_DECODE which were defined the in the lirc.h header, but > never implemented by any out of tree or in tree driver. > > Neither feature was or will be ever implemented in the kernel; > LIRC_CAN_NOTIFY_DECODE is handled via the led subsytem, and it is unknown > what LIRC_CAN_SET_REC_FILTER is even supposed to mean. Again, I have not > found any implementation anywhere. > > You are trying to build lirc user space daemon which is no longer maintained. > The last time the lirc daemon git repo had any commits was in 2019. User > space tooling has been replaced with daemon-less ir-ctl and ir-keytable, > which uses BPF for IR decoding. > > The right fix is to simply delete the offending lines in lircd.cpp and all > will be well. Sometimes source code needs a little maintainence. > > These changes in the lirc uapi do not change the uapi in any way, just the > ability the build some unmaintained software without trivial changes. Hi, I don't understand how inability to build software is not an uapi breakage -- care to elaborate? Be it umaintained or not, it's still in distributions (the above is from opensuse build system) and it is broken now. Every single distributor now would have to go and fix this. So either you fix it (e.g. re-add only the entries as I suggested) or I will post a revert of your patch. Sorry, no excuses. thanks,
On Wed, May 25, 2022 at 10:09:38AM +0200, Jiri Slaby wrote: > Be it umaintained or not, it's still in distributions (the above is from > opensuse build system) and it is broken now. Every single distributor now > would have to go and fix this. I am happy to help out with this issue, since lircd upstream does not accept patches. I've attached a patch for your perusal. Sean
On Wed, May 25, 2022 at 08:44:38AM +0200, Jiri Slaby wrote: > On 25. 05. 22, 8:42, Jiri Slaby wrote: > > On 22. 03. 22, 10:14, Mauro Carvalho Chehab wrote: > > > Hi Linus, > > > > > > Please pull from: > > > git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media > > > tags/media/v5.18-1 > > ... > > > Sean Young (10): > > ... > > > media: lirc: remove unused lirc features > > > > Hi, > > > > this breaks lirc build: > > > [ 59s] lircd.cpp:489:49: error: 'LIRC_CAN_SET_REC_FILTER' was not > > > declared in this scope; did you mean 'LIRC_CAN_SET_REC_CARRIER'? > > > [ 59s] 489 | || (curr_driver->features & > > > LIRC_CAN_SET_REC_FILTER)) { > > > [ 59s] | > > > ^~~~~~~~~~~~~~~~~~~~~~~ > > > [ 59s] | > > > LIRC_CAN_SET_REC_CARRIER > > > [ 59s] lircd.cpp: In function 'void loop()': > > > [ 59s] lircd.cpp:2069:82: error: 'LIRC_CAN_NOTIFY_DECODE' was not > > > declared in this scope; did you mean 'DRVCTL_NOTIFY_DECODE'? > > > [ 59s] 2069 | if > > > (curr_driver->drvctl_func && (curr_driver->features & > > > LIRC_CAN_NOTIFY_DECODE)) > > > [ 59s] | > > > ^~~~~~~~~~~~~~~~~~~~~~ > > > [ 59s] | > > > DRVCTL_NOTIFY_DECODE > > > > So the uapi header defines should be brought back, IMO. > > (And lirc fixed at the same time.) What is broken? Sean
On Wed, May 25, 2022 at 10:09:38AM +0200, Jiri Slaby wrote: > I don't understand how inability to build software is not an uapi breakage > -- care to elaborate? So here is a good compromise suggested by Mauro. 1. We add the following to the lirc.h uapi header. #define LIRC_CAN_NOTIFY_DECODE 0 #define LIRC_CAN_SET_REC_FILTER 0 2. Since lirc daemon is unmaintained, I am happy to take on maintainership. This may require forking, depending on what the maintainer says. How does that sound? Sean
On 25. 05. 22, 11:10, Sean Young wrote: > On Wed, May 25, 2022 at 10:09:38AM +0200, Jiri Slaby wrote: >> I don't understand how inability to build software is not an uapi breakage >> -- care to elaborate? > > So here is a good compromise suggested by Mauro. > > 1. We add the following to the lirc.h uapi header. > > #define LIRC_CAN_NOTIFY_DECODE 0 > #define LIRC_CAN_SET_REC_FILTER 0 The code would do "if (x & 0)" or alike, so I'm not sure this won't result in a warning. But as soon as that thing compiles, I don't really care much. If it produces no warning, in fact, the code could be optimized away out thanks to "& 0". Just looked up those defs in the debian code search, only lirc and v4l-utils care about the defines. ANd the latter seems to define their own copies. > 2. Since lirc daemon is unmaintained, I am happy to take on maintainership. > > This may require forking, depending on what the maintainer says. > > How does that sound? Great. thanks,