Re: [PATCH v2] input: add 2 kind of switch
Hi HyungJae, Thank you for the patch! Yet something to improve: [auto build test ERROR on input/next] [also build test ERROR on linus/master linux/master v5.10-rc2 next-20201102] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/HyungJae-Im/input-add-2-kind-of-switch/20201029-213108 base: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next config: riscv-randconfig-r036-20201030 (attached as .config) compiler: riscv64-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/bf61e9f978d895850b5a30fe6a7308acb2f245d7 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review HyungJae-Im/input-add-2-kind-of-switch/20201029-213108 git checkout bf61e9f978d895850b5a30fe6a7308acb2f245d7 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=riscv If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>): >> drivers/input/ext_pen_detect.c:23:10: fatal error: linux/wakelock.h: No such >> file or directory 23 | #include | ^~ compilation terminated. vim +23 drivers/input/ext_pen_detect.c > 23 #include 24 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip
Re: Re: (3) [PATCH v2] input: add 2 kind of switch
On Fri, Oct 30, 2020 at 08:59:18PM +0900, Jungrae Kim wrote: > > On Fri, Oct 30, 2020 at 08:28:12PM +0900, Jungrae Kim wrote: > > > > On Fri, Oct 30, 2020 at 01:39:16PM +0900, HyungJae Im wrote: > > > > > Hello, This is Hyungjae Im from Samsung Electronics. > > > > > Let me answer your questions inline. > > > > > > > > > > >On Thu, Oct 29, 2020 at 10:27:47PM +0900, HyungJae Im wrote: > > > > > >> From: "hj2.im" > > > > > >> Date: Thu, 29 Oct 2020 22:11:24 +0900 > > > > > >> Subject: [PATCH v2] input: add 2 kind of switch > > > > > > > > > > > >Why is this in the body of that patch? > > > > > > > > > > I read "how to send your first kernel patch", but still making so > > > > > many mistakes. > > > > > I will be cautious with this. > > > > > > > > > > >> > > > > > >> We need support to various accessories on the device, > > > > > >> some switch does not exist in switch list. > > > > > >> So added switch for the following purpose. > > > > > >> > > > > > >> SW_COVER_ATTACHED is for the checking the cover > > > > > >> attached or not on the device. SW_EXT_PEN_ATTACHED is for the > > > > > >> checking the external pen attached or not on the device > > > > > > > > > > > >You didn't answer the previous question as to why the existing > > > > > >values do > > > > > >not work for you instead of having to create new ones? > > > > > > > > > > I think I should clarify this part the most for this review. > > > > > As you know, new added events both has similar existing events, > > > > > but it has to operate separately. > > > > > > > > > > First, SW_COVER_ATTACHED is similar with SW_MACHINE_COVER. > > > > > We need two events for our cover interaction. > > > > > One is to detect if flip cover is open/closed(covers screen or not), > > > > > and one is for detecting if cover is attached(detect if device is > > > > >put into cover). > > > > > With the second event, we send event for attachment and start > > > > >authentication > > > > > distinguishing if it was Samsung made cover. > > > > > > > > > > Second, SW_EXT_PEN_ATTACHED detects if pen is attached externally on > > > > >tablet models. > > > > > It is different with SW_PEN_INSERTED since this is detecting pens > > > > >like our NOTE series. > > > > > SW_EXT_PEN_ATTACHED has an unique role to set wacom tuning table > > > > >differently > > > > > while pen is attached/detached. > > > > > > > > All of that needs to go in the changelog text for the individual patches > > > > when you submit them. > > > > > > > > But as Dmitry pointed out, it doesn't look like either of these drivers > > > > are needed at all, just use the gpio-keys driver instead. > > > > > > > > thanks, > > > > > > > > greg k-h > > > > > > Can you accept V1 patch? or need to add a change of device tree? > > > > What is "v1" patch? Do you have a pointer to it on lore.kernel.org? > > > > > Please let me know what do I do now. > > > > What is wrong with just using a device tree entry for the gpio-keys > > driver instead? > > > > thanks, > > > > greg k-h > > V1 Patch : > https://lore.kernel.org/lkml/20201021031216epcms1p556d8d7d5d763ec47f67cd8cbe3972935@epcms1p5/ As I said there, that patch is not acceptable for style reasons alone, nothing we can do with that unless it is fixed, right? > I think do not need modify gpio_keys. And I`m not sure device tree need to > added to patch. No, you don't need to modify it, just use it. So what exactly is the issue anymore? Just use the gpio-keys driver and all should be fine, right? thanks, greg k-h
RE: Re: (3) [PATCH v2] input: add 2 kind of switch
> On Fri, Oct 30, 2020 at 08:28:12PM +0900, Jungrae Kim wrote: > > > On Fri, Oct 30, 2020 at 01:39:16PM +0900, HyungJae Im wrote: > > > > Hello, This is Hyungjae Im from Samsung Electronics. > > > > Let me answer your questions inline. > > > > > > > > >On Thu, Oct 29, 2020 at 10:27:47PM +0900, HyungJae Im wrote: > > > > >> From: "hj2.im" > > > > >> Date: Thu, 29 Oct 2020 22:11:24 +0900 > > > > >> Subject: [PATCH v2] input: add 2 kind of switch > > > > > > > > > >Why is this in the body of that patch? > > > > > > > > I read "how to send your first kernel patch", but still making so many > > > > mistakes. > > > > I will be cautious with this. > > > > > > > > >> > > > > >> We need support to various accessories on the device, > > > > >> some switch does not exist in switch list. > > > > >> So added switch for the following purpose. > > > > >> > > > > >> SW_COVER_ATTACHED is for the checking the cover > > > > >> attached or not on the device. SW_EXT_PEN_ATTACHED is for the > > > > >> checking the external pen attached or not on the device > > > > > > > > > >You didn't answer the previous question as to why the existing values > > > > >do > > > > >not work for you instead of having to create new ones? > > > > > > > > I think I should clarify this part the most for this review. > > > > As you know, new added events both has similar existing events, > > > > but it has to operate separately. > > > > > > > > First, SW_COVER_ATTACHED is similar with SW_MACHINE_COVER. > > > > We need two events for our cover interaction. > > > > One is to detect if flip cover is open/closed(covers screen or not), > > > > and one is for detecting if cover is attached(detect if device is put > > > >into cover). > > > > With the second event, we send event for attachment and start > > > >authentication > > > > distinguishing if it was Samsung made cover. > > > > > > > > Second, SW_EXT_PEN_ATTACHED detects if pen is attached externally on > > > >tablet models. > > > > It is different with SW_PEN_INSERTED since this is detecting pens like > > > >our NOTE series. > > > > SW_EXT_PEN_ATTACHED has an unique role to set wacom tuning table > > > >differently > > > > while pen is attached/detached. > > > > > > All of that needs to go in the changelog text for the individual patches > > > when you submit them. > > > > > > But as Dmitry pointed out, it doesn't look like either of these drivers > > > are needed at all, just use the gpio-keys driver instead. > > > > > > thanks, > > > > > > greg k-h > > > > Can you accept V1 patch? or need to add a change of device tree? > > What is "v1" patch? Do you have a pointer to it on lore.kernel.org? > > > Please let me know what do I do now. > > What is wrong with just using a device tree entry for the gpio-keys > driver instead? > > thanks, > > greg k-h V1 Patch : https://lore.kernel.org/lkml/20201021031216epcms1p556d8d7d5d763ec47f67cd8cbe3972935@epcms1p5/ I think do not need modify gpio_keys. And I`m not sure device tree need to added to patch. Please let me know if you think need more fix than Patch v1. Thanks Jungrae Kim
Re: (3) [PATCH v2] input: add 2 kind of switch
On Fri, Oct 30, 2020 at 08:28:12PM +0900, Jungrae Kim wrote: > > On Fri, Oct 30, 2020 at 01:39:16PM +0900, HyungJae Im wrote: > > > Hello, This is Hyungjae Im from Samsung Electronics. > > > Let me answer your questions inline. > > > > > > >On Thu, Oct 29, 2020 at 10:27:47PM +0900, HyungJae Im wrote: > > > >> From: "hj2.im" > > > >> Date: Thu, 29 Oct 2020 22:11:24 +0900 > > > >> Subject: [PATCH v2] input: add 2 kind of switch > > > > > > > >Why is this in the body of that patch? > > > > > > I read "how to send your first kernel patch", but still making so many > > > mistakes. > > > I will be cautious with this. > > > > > > >> > > > >> We need support to various accessories on the device, > > > >> some switch does not exist in switch list. > > > >> So added switch for the following purpose. > > > >> > > > >> SW_COVER_ATTACHED is for the checking the cover > > > >> attached or not on the device. SW_EXT_PEN_ATTACHED is for the > > > >> checking the external pen attached or not on the device > > > > > > > >You didn't answer the previous question as to why the existing values do > > > >not work for you instead of having to create new ones? > > > > > > I think I should clarify this part the most for this review. > > > As you know, new added events both has similar existing events, > > > but it has to operate separately. > > > > > > First, SW_COVER_ATTACHED is similar with SW_MACHINE_COVER. > > > We need two events for our cover interaction. > > > One is to detect if flip cover is open/closed(covers screen or not), > > > and one is for detecting if cover is attached(detect if device is put > > >into cover). > > > With the second event, we send event for attachment and start > > >authentication > > > distinguishing if it was Samsung made cover. > > > > > > Second, SW_EXT_PEN_ATTACHED detects if pen is attached externally on > > >tablet models. > > > It is different with SW_PEN_INSERTED since this is detecting pens like > > >our NOTE series. > > > SW_EXT_PEN_ATTACHED has an unique role to set wacom tuning table > > >differently > > > while pen is attached/detached. > > > > All of that needs to go in the changelog text for the individual patches > > when you submit them. > > > > But as Dmitry pointed out, it doesn't look like either of these drivers > > are needed at all, just use the gpio-keys driver instead. > > > > thanks, > > > > greg k-h > > Can you accept V1 patch? or need to add a change of device tree? What is "v1" patch? Do you have a pointer to it on lore.kernel.org? > Please let me know what do I do now. What is wrong with just using a device tree entry for the gpio-keys driver instead? thanks, greg k-h
Re: (3) [PATCH v2] input: add 2 kind of switch
> On Fri, Oct 30, 2020 at 01:39:16PM +0900, HyungJae Im wrote: > > Hello, This is Hyungjae Im from Samsung Electronics. > > Let me answer your questions inline. > > > > >On Thu, Oct 29, 2020 at 10:27:47PM +0900, HyungJae Im wrote: > > >> From: "hj2.im" > > >> Date: Thu, 29 Oct 2020 22:11:24 +0900 > > >> Subject: [PATCH v2] input: add 2 kind of switch > > > > > >Why is this in the body of that patch? > > > > I read "how to send your first kernel patch", but still making so many > > mistakes. > > I will be cautious with this. > > > > >> > > >> We need support to various accessories on the device, > > >> some switch does not exist in switch list. > > >> So added switch for the following purpose. > > >> > > >> SW_COVER_ATTACHED is for the checking the cover > > >> attached or not on the device. SW_EXT_PEN_ATTACHED is for the > > >> checking the external pen attached or not on the device > > > > > >You didn't answer the previous question as to why the existing values do > > >not work for you instead of having to create new ones? > > > > I think I should clarify this part the most for this review. > > As you know, new added events both has similar existing events, > > but it has to operate separately. > > > > First, SW_COVER_ATTACHED is similar with SW_MACHINE_COVER. > > We need two events for our cover interaction. > > One is to detect if flip cover is open/closed(covers screen or not), > > and one is for detecting if cover is attached(detect if device is put into > >cover). > > With the second event, we send event for attachment and start > >authentication > > distinguishing if it was Samsung made cover. > > > > Second, SW_EXT_PEN_ATTACHED detects if pen is attached externally on > >tablet models. > > It is different with SW_PEN_INSERTED since this is detecting pens like our > >NOTE series. > > SW_EXT_PEN_ATTACHED has an unique role to set wacom tuning table > >differently > > while pen is attached/detached. > > All of that needs to go in the changelog text for the individual patches > when you submit them. > > But as Dmitry pointed out, it doesn't look like either of these drivers > are needed at all, just use the gpio-keys driver instead. > > thanks, > > greg k-h Can you accept V1 patch? or need to add a change of device tree? Please let me know what do I do now.
Re: (2) [PATCH v2] input: add 2 kind of switch
On Fri, Oct 30, 2020 at 01:39:16PM +0900, HyungJae Im wrote: > Hello, This is Hyungjae Im from Samsung Electronics. > Let me answer your questions inline. > > >On Thu, Oct 29, 2020 at 10:27:47PM +0900, HyungJae Im wrote: > >> From: "hj2.im" > >> Date: Thu, 29 Oct 2020 22:11:24 +0900 > >> Subject: [PATCH v2] input: add 2 kind of switch > > > >Why is this in the body of that patch? > > I read "how to send your first kernel patch", but still making so many > mistakes. > I will be cautious with this. > > >> > >> We need support to various accessories on the device, > >> some switch does not exist in switch list. > >> So added switch for the following purpose. > >> > >> SW_COVER_ATTACHED is for the checking the cover > >> attached or not on the device. SW_EXT_PEN_ATTACHED is for the > >> checking the external pen attached or not on the device > > > >You didn't answer the previous question as to why the existing values do > >not work for you instead of having to create new ones? > > I think I should clarify this part the most for this review. > As you know, new added events both has similar existing events, > but it has to operate separately. > > First, SW_COVER_ATTACHED is similar with SW_MACHINE_COVER. > We need two events for our cover interaction. > One is to detect if flip cover is open/closed(covers screen or not), > and one is for detecting if cover is attached(detect if device is put into > cover). > With the second event, we send event for attachment and start authentication > distinguishing if it was Samsung made cover. > > Second, SW_EXT_PEN_ATTACHED detects if pen is attached externally on tablet > models. > It is different with SW_PEN_INSERTED since this is detecting pens like our > NOTE series. > SW_EXT_PEN_ATTACHED has an unique role to set wacom tuning table differently > while pen is attached/detached. All of that needs to go in the changelog text for the individual patches when you submit them. But as Dmitry pointed out, it doesn't look like either of these drivers are needed at all, just use the gpio-keys driver instead. thanks, greg k-h
RE:(2) [PATCH v2] input: add 2 kind of switch
Hello, This is Hyungjae Im from Samsung Electronics. Let me answer your questions inline. >On Thu, Oct 29, 2020 at 10:27:47PM +0900, HyungJae Im wrote: >> From: "hj2.im" >> Date: Thu, 29 Oct 2020 22:11:24 +0900 >> Subject: [PATCH v2] input: add 2 kind of switch > >Why is this in the body of that patch? I read "how to send your first kernel patch", but still making so many mistakes. I will be cautious with this. >> >> We need support to various accessories on the device, >> some switch does not exist in switch list. >> So added switch for the following purpose. >> >> SW_COVER_ATTACHED is for the checking the cover >> attached or not on the device. SW_EXT_PEN_ATTACHED is for the >> checking the external pen attached or not on the device > >You didn't answer the previous question as to why the existing values do >not work for you instead of having to create new ones? I think I should clarify this part the most for this review. As you know, new added events both has similar existing events, but it has to operate separately. First, SW_COVER_ATTACHED is similar with SW_MACHINE_COVER. We need two events for our cover interaction. One is to detect if flip cover is open/closed(covers screen or not), and one is for detecting if cover is attached(detect if device is put into cover). With the second event, we send event for attachment and start authentication distinguishing if it was Samsung made cover. Second, SW_EXT_PEN_ATTACHED detects if pen is attached externally on tablet models. It is different with SW_PEN_INSERTED since this is detecting pens like our NOTE series. SW_EXT_PEN_ATTACHED has an unique role to set wacom tuning table differently while pen is attached/detached. >> >> Signed-off-by: Hyungjae Im >> --- >> drivers/input/Kconfig | 20 ++ >> drivers/input/Makefile | 3 + >> drivers/input/cover_detect.c | 242 >> drivers/input/ext_pen_detect.c | 243 + >> include/linux/mod_devicetable.h| 2 +- >> include/uapi/linux/input-event-codes.h | 4 +- >> 6 files changed, 512 insertions(+), 2 deletions(-) >> create mode 100644 drivers/input/cover_detect.c >> create mode 100644 drivers/input/ext_pen_detect.c >If this is v2, what changed from v1? > >And this is 2 different drivers, it should be 2 different patches at the >least, right?> V2 has additional submit for drivers using SW events. I will separate these two drivers and submit it separately. >> >> diff --git a/drivers/input/Kconfig b/drivers/input/Kconfig >> index 1efd3154b68d..df902f4a549e 100644 >> --- a/drivers/input/Kconfig >> +++ b/drivers/input/Kconfig >> @@ -185,6 +185,26 @@ config INPUT_APMPOWER >>To compile this driver as a module, choose M here: the >>module will be called apm-power. >> >> +config COVER_DETECT >INPUT_COVER_DETECT? Will change feature name to INPUT_COVER_DETECT. >> +tristate "Enable cover attach detection" >> +default n >"default n" is always the default, no need for this here. Thanks for comment. I will erase it. >> +help >> + Say Y here to enable cover attach detection >> + and send a event when cover is attached/detached. >> + Active gpio state is low and active event value is 0. >> + >> + If unsure, say N. >What is the module name? I will add additional explanation for module name >> + >> +config EXT_PEN_DETECT >INPUT_EXT_PEN_DETECT? > +tristate "Enable external pen attach detection" > +default n >No default n. >> +help >> + Say Y here to enable external pen attach detection >> + and send a event when external pen is attached/detached. >> + Active gpio state is low and active event value is 0. >> + >> + If unsure, say N. >What is the module name? >> + >> comment "Input Device Drivers" >> >> source "drivers/input/keyboard/Kconfig" >> diff --git a/drivers/input/Makefile b/drivers/input/Makefile >> index e35650930371..31ee1f2d2e21 100644 >> --- a/drivers/input/Makefile >> +++ b/drivers/input/Makefile >> @@ -29,3 +29,6 @@ obj-$(CONFIG_INPUT_MISC)+= misc/ >> obj-$(CONFIG_INPUT_APMPOWER)+= apm-power.o >> >> obj-$(CONFIG_RMI4_CORE)+= rmi4/ >> + >> +obj-$(CONFIG_COVER_DETECT)+= cover_detect.o >> +obj-$(CONFIG_EXT_PEN_DETECT)+= ext_pen_detect.o >
Re: [PATCH v2] input: add 2 kind of switch
On Thu, Oct 29, 2020 at 02:57:15PM +0100, gre...@linuxfoundation.org wrote: > On Thu, Oct 29, 2020 at 10:27:47PM +0900, HyungJae Im wrote: > > From: "hj2.im" > > Date: Thu, 29 Oct 2020 22:11:24 +0900 > > Subject: [PATCH v2] input: add 2 kind of switch > > Why is this in the body of that patch? > > > > > We need support to various accessories on the device, > > some switch does not exist in switch list. > > So added switch for the following purpose. > > > > SW_COVER_ATTACHED is for the checking the cover > > attached or not on the device. SW_EXT_PEN_ATTACHED is for the > > checking the external pen attached or not on the device > > You didn't answer the previous question as to why the existing values do > not work for you instead of having to create new ones? > > > > > > Signed-off-by: Hyungjae Im > > --- > > drivers/input/Kconfig | 20 ++ > > drivers/input/Makefile | 3 + > > drivers/input/cover_detect.c | 242 > > drivers/input/ext_pen_detect.c | 243 + > > include/linux/mod_devicetable.h| 2 +- > > include/uapi/linux/input-event-codes.h | 4 +- > > 6 files changed, 512 insertions(+), 2 deletions(-) > > create mode 100644 drivers/input/cover_detect.c > > create mode 100644 drivers/input/ext_pen_detect.c > > If this is v2, what changed from v1? > > And this is 2 different drivers, it should be 2 different patches at the > least, right?> Actually the should simply use gpio-keys.c for this and dispense with the custom drivers. Thanks. -- Dmitry
Re: [PATCH v2] input: add 2 kind of switch
Hi
> [...]
> > > diff --git a/include/linux/mod_devicetable.h
> > > b/include/linux/mod_devicetable.h
> > > index 5b08a473cdba..897f5a3e7721 100644
> > > --- a/include/linux/mod_devicetable.h
> > > +++ b/include/linux/mod_devicetable.h
> > > @@ -320,7 +320,7 @@ struct pcmcia_device_id {
> > > #define INPUT_DEVICE_ID_LED_MAX 0x0f
> > > #define INPUT_DEVICE_ID_SND_MAX 0x07
> > > #define INPUT_DEVICE_ID_FF_MAX 0x7f
> > > -#define INPUT_DEVICE_ID_SW_MAX 0x10
> > > +#define INPUT_DEVICE_ID_SW_MAX 0x12
> > > #define INPUT_DEVICE_ID_PROP_MAX 0x1f
> > >
> > > #define INPUT_DEVICE_ID_MATCH_BUS1
> > > diff --git a/include/uapi/linux/input-event-codes.h
> > > b/include/uapi/linux/input-event-codes.h
> > > index 0c2e27d28e0a..8ca2acee1f92 100644
> > > --- a/include/uapi/linux/input-event-codes.h
> > > +++ b/include/uapi/linux/input-event-codes.h
> > > @@ -889,7 +889,9 @@
> > > #define SW_MUTE_DEVICE 0x0e /* set = device disabled */
> > > #define SW_PEN_INSERTED 0x0f /* set = pen inserted */
> > > #define SW_MACHINE_COVER 0x10 /* set = cover closed */
> > > -#define SW_MAX 0x10
> > > +#define SW_COVER_ATTACHED0x11 /* set = cover attached */
> > > +#define SW_EXT_PEN_ATTACHED 0x12 /* set = external pen attached */
> > > +#define SW_MAX 0x12
> > > #define SW_CNT (SW_MAX+1)
> > > [...]
> >
> > This part of the patch conflicts with another one:
> > https://lore.kernel.org/linux-input/20201026144512.621479-1-markpear...@lenovo.com/
>
> Is that merged? If not, it's fine as-is until then, and someone has to
> pick to go first :)
It is not, to my knowledge. Nonetheless I figured the information may be
relevant.
Regards,
Barnabás Pőcze
Re: [PATCH v2] input: add 2 kind of switch
On Thu, Oct 29, 2020 at 10:27:47PM +0900, HyungJae Im wrote: > From: "hj2.im" > Date: Thu, 29 Oct 2020 22:11:24 +0900 > Subject: [PATCH v2] input: add 2 kind of switch Why is this in the body of that patch? > > We need support to various accessories on the device, > some switch does not exist in switch list. > So added switch for the following purpose. > > SW_COVER_ATTACHED is for the checking the cover > attached or not on the device. SW_EXT_PEN_ATTACHED is for the > checking the external pen attached or not on the device You didn't answer the previous question as to why the existing values do not work for you instead of having to create new ones? > > Signed-off-by: Hyungjae Im > --- > drivers/input/Kconfig | 20 ++ > drivers/input/Makefile | 3 + > drivers/input/cover_detect.c | 242 > drivers/input/ext_pen_detect.c | 243 + > include/linux/mod_devicetable.h| 2 +- > include/uapi/linux/input-event-codes.h | 4 +- > 6 files changed, 512 insertions(+), 2 deletions(-) > create mode 100644 drivers/input/cover_detect.c > create mode 100644 drivers/input/ext_pen_detect.c If this is v2, what changed from v1? And this is 2 different drivers, it should be 2 different patches at the least, right?> > > diff --git a/drivers/input/Kconfig b/drivers/input/Kconfig > index 1efd3154b68d..df902f4a549e 100644 > --- a/drivers/input/Kconfig > +++ b/drivers/input/Kconfig > @@ -185,6 +185,26 @@ config INPUT_APMPOWER > To compile this driver as a module, choose M here: the > module will be called apm-power. > > +config COVER_DETECT INPUT_COVER_DETECT? > + tristate "Enable cover attach detection" > + default n "default n" is always the default, no need for this here. > + help > + Say Y here to enable cover attach detection > + and send a event when cover is attached/detached. > + Active gpio state is low and active event value is 0. > + > + If unsure, say N. What is the module name? > + > +config EXT_PEN_DETECT INPUT_EXT_PEN_DETECT? > + tristate "Enable external pen attach detection" > + default n No default n. > + help > + Say Y here to enable external pen attach detection > + and send a event when external pen is attached/detached. > + Active gpio state is low and active event value is 0. > + > + If unsure, say N. What is the module name? > + > comment "Input Device Drivers" > > source "drivers/input/keyboard/Kconfig" > diff --git a/drivers/input/Makefile b/drivers/input/Makefile > index e35650930371..31ee1f2d2e21 100644 > --- a/drivers/input/Makefile > +++ b/drivers/input/Makefile > @@ -29,3 +29,6 @@ obj-$(CONFIG_INPUT_MISC)+= misc/ > obj-$(CONFIG_INPUT_APMPOWER) += apm-power.o > > obj-$(CONFIG_RMI4_CORE) += rmi4/ > + > +obj-$(CONFIG_COVER_DETECT) += cover_detect.o > +obj-$(CONFIG_EXT_PEN_DETECT) += ext_pen_detect.o > diff --git a/drivers/input/cover_detect.c b/drivers/input/cover_detect.c > new file mode 100644 > index ..4d3d68c616ec > --- /dev/null > +++ b/drivers/input/cover_detect.c > @@ -0,0 +1,242 @@ > +/* > + * Copyright (C) 2015 Samsung Electronics Co. Ltd. All Rights Reserved. Please use a SPDX line, and no need for this: > + * > + * This software is licensed under the terms of the GNU General Public > + * License version 2, as published by the Free Software Foundation, and > + * may be copied, distributed, and modified under those terms. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. That can be removed. Also your copyright line is wrong, unless you really have not touched this file in 5 years. Same comments on the other file. thanks, greg k-h
Re: [PATCH v2] input: add 2 kind of switch
On Thu, Oct 29, 2020 at 01:41:57PM +, Barnabás Pőcze wrote:
> Hi
>
> > [...]
> > diff --git a/include/linux/mod_devicetable.h
> > b/include/linux/mod_devicetable.h
> > index 5b08a473cdba..897f5a3e7721 100644
> > --- a/include/linux/mod_devicetable.h
> > +++ b/include/linux/mod_devicetable.h
> > @@ -320,7 +320,7 @@ struct pcmcia_device_id {
> > #define INPUT_DEVICE_ID_LED_MAX0x0f
> > #define INPUT_DEVICE_ID_SND_MAX0x07
> > #define INPUT_DEVICE_ID_FF_MAX 0x7f
> > -#define INPUT_DEVICE_ID_SW_MAX 0x10
> > +#define INPUT_DEVICE_ID_SW_MAX 0x12
> > #define INPUT_DEVICE_ID_PROP_MAX 0x1f
> >
> > #define INPUT_DEVICE_ID_MATCH_BUS 1
> > diff --git a/include/uapi/linux/input-event-codes.h
> > b/include/uapi/linux/input-event-codes.h
> > index 0c2e27d28e0a..8ca2acee1f92 100644
> > --- a/include/uapi/linux/input-event-codes.h
> > +++ b/include/uapi/linux/input-event-codes.h
> > @@ -889,7 +889,9 @@
> > #define SW_MUTE_DEVICE 0x0e /* set = device disabled */
> > #define SW_PEN_INSERTED0x0f /* set = pen inserted */
> > #define SW_MACHINE_COVER 0x10 /* set = cover closed */
> > -#define SW_MAX 0x10
> > +#define SW_COVER_ATTACHED 0x11 /* set = cover attached */
> > +#define SW_EXT_PEN_ATTACHED0x12 /* set = external pen attached */
> > +#define SW_MAX 0x12
> > #define SW_CNT (SW_MAX+1)
> > [...]
>
> This part of the patch conflicts with another one:
> https://lore.kernel.org/linux-input/20201026144512.621479-1-markpear...@lenovo.com/
Is that merged? If not, it's fine as-is until then, and someone has to
pick to go first :)
But, most importantly, the questions asked last time about this patch
have not been addressed at all :(
thanks,
greg k-h
Re: [PATCH v2] input: add 2 kind of switch
Hi
> [...]
> diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
> index 5b08a473cdba..897f5a3e7721 100644
> --- a/include/linux/mod_devicetable.h
> +++ b/include/linux/mod_devicetable.h
> @@ -320,7 +320,7 @@ struct pcmcia_device_id {
> #define INPUT_DEVICE_ID_LED_MAX 0x0f
> #define INPUT_DEVICE_ID_SND_MAX 0x07
> #define INPUT_DEVICE_ID_FF_MAX 0x7f
> -#define INPUT_DEVICE_ID_SW_MAX 0x10
> +#define INPUT_DEVICE_ID_SW_MAX 0x12
> #define INPUT_DEVICE_ID_PROP_MAX 0x1f
>
> #define INPUT_DEVICE_ID_MATCH_BUS1
> diff --git a/include/uapi/linux/input-event-codes.h
> b/include/uapi/linux/input-event-codes.h
> index 0c2e27d28e0a..8ca2acee1f92 100644
> --- a/include/uapi/linux/input-event-codes.h
> +++ b/include/uapi/linux/input-event-codes.h
> @@ -889,7 +889,9 @@
> #define SW_MUTE_DEVICE 0x0e /* set = device disabled */
> #define SW_PEN_INSERTED 0x0f /* set = pen inserted */
> #define SW_MACHINE_COVER 0x10 /* set = cover closed */
> -#define SW_MAX 0x10
> +#define SW_COVER_ATTACHED0x11 /* set = cover attached */
> +#define SW_EXT_PEN_ATTACHED 0x12 /* set = external pen attached */
> +#define SW_MAX 0x12
> #define SW_CNT (SW_MAX+1)
> [...]
This part of the patch conflicts with another one:
https://lore.kernel.org/linux-input/20201026144512.621479-1-markpear...@lenovo.com/
Regards,
Barnabás Pőcze
[PATCH v2] input: add 2 kind of switch
From: "hj2.im"
Date: Thu, 29 Oct 2020 22:11:24 +0900
Subject: [PATCH v2] input: add 2 kind of switch
We need support to various accessories on the device,
some switch does not exist in switch list.
So added switch for the following purpose.
SW_COVER_ATTACHED is for the checking the cover
attached or not on the device. SW_EXT_PEN_ATTACHED is for the
checking the external pen attached or not on the device
Signed-off-by: Hyungjae Im
---
drivers/input/Kconfig | 20 ++
drivers/input/Makefile | 3 +
drivers/input/cover_detect.c | 242
drivers/input/ext_pen_detect.c | 243 +
include/linux/mod_devicetable.h| 2 +-
include/uapi/linux/input-event-codes.h | 4 +-
6 files changed, 512 insertions(+), 2 deletions(-)
create mode 100644 drivers/input/cover_detect.c
create mode 100644 drivers/input/ext_pen_detect.c
diff --git a/drivers/input/Kconfig b/drivers/input/Kconfig
index 1efd3154b68d..df902f4a549e 100644
--- a/drivers/input/Kconfig
+++ b/drivers/input/Kconfig
@@ -185,6 +185,26 @@ config INPUT_APMPOWER
To compile this driver as a module, choose M here: the
module will be called apm-power.
+config COVER_DETECT
+ tristate "Enable cover attach detection"
+ default n
+ help
+ Say Y here to enable cover attach detection
+ and send a event when cover is attached/detached.
+ Active gpio state is low and active event value is 0.
+
+ If unsure, say N.
+
+config EXT_PEN_DETECT
+ tristate "Enable external pen attach detection"
+ default n
+ help
+ Say Y here to enable external pen attach detection
+ and send a event when external pen is attached/detached.
+ Active gpio state is low and active event value is 0.
+
+ If unsure, say N.
+
comment "Input Device Drivers"
source "drivers/input/keyboard/Kconfig"
diff --git a/drivers/input/Makefile b/drivers/input/Makefile
index e35650930371..31ee1f2d2e21 100644
--- a/drivers/input/Makefile
+++ b/drivers/input/Makefile
@@ -29,3 +29,6 @@ obj-$(CONFIG_INPUT_MISC) += misc/
obj-$(CONFIG_INPUT_APMPOWER) += apm-power.o
obj-$(CONFIG_RMI4_CORE)+= rmi4/
+
+obj-$(CONFIG_COVER_DETECT) += cover_detect.o
+obj-$(CONFIG_EXT_PEN_DETECT) += ext_pen_detect.o
diff --git a/drivers/input/cover_detect.c b/drivers/input/cover_detect.c
new file mode 100644
index ..4d3d68c616ec
--- /dev/null
+++ b/drivers/input/cover_detect.c
@@ -0,0 +1,242 @@
+/*
+ * Copyright (C) 2015 Samsung Electronics Co. Ltd. All Rights Reserved.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+struct cover_detect_drvdata {
+ struct input_dev *input;
+ struct delayed_work cover_detect_dwork;
+ struct wakeup_source *ws;
+ int gpio_cover_detect;
+ int irq_cover_detect;
+};
+
+static void cover_detect_work(struct work_struct *work)
+{
+ struct cover_detect_drvdata *ddata =
+ container_of(work, struct cover_detect_drvdata,
+ cover_detect_dwork.work);
+ bool cover_status;
+
+ cover_status = gpio_get_value(ddata->gpio_cover_detect);
+
+ input_report_switch(ddata->input,
+ SW_COVER_ATTACHED, cover_status);
+ input_sync(ddata->input);
+}
+
+static void __cover_detect(struct cover_detect_drvdata *ddata,
+bool cover_status)
+{
+ cancel_delayed_work_sync(>cover_detect_dwork);
+ if (cover_status) {
+ __pm_wakeup_event(ddata->ws, jiffies_to_msecs(HZ / 20));
+ schedule_delayed_work(>cover_detect_dwork, HZ * 1 / 100);
+ } else {
+ __pm_relax(ddata->ws);
+ schedule_delayed_work(>cover_detect_dwork, 0);
+ }
+}
+
+static irqreturn_t cover_detect_irq(int irq, void *dev_id)
+{
+ bool cover_status;
+ struct cover_detect_drvdata *ddata = dev_id;
+
+ cover_status = gpio_get_value(ddata->gpio_cover_detect);
+
+ __cover_detect(ddata, cover_status);
+
+ return IRQ_HANDLED;
+}
+
+static int cover_detect_open(struct input_dev *input)
+{
+ struct cover_detect_drvdata *ddata = input_get_drvdata(input);
+ /* update the current status */
+ schedule_delayed_work(>cover_detect_dwork,
