On Fri, Oct 30, 2020 at 08:28:12PM +0900, Jungrae Kim wrote:

quoted

quoted

On Fri, Oct 30, 2020 at 01:39:16PM +0900, HyungJae Im wrote:

quoted

Hello, This is Hyungjae Im from Samsung Electronics.
Let me answer your questions inline.

quoted

On Thu, Oct 29, 2020 at 10:27:47PM +0900, HyungJae Im wrote:

quoted

From: "hj2.im" <redacted>
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.
 

quoted

quoted

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?

quoted

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/ (local)

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