Re: [PATCH] platform/x86: intel-vbtn: Simplify autorelease logic

[Darren Hart]

On Mon, Dec 11, 2017 at 02:48:23AM +0100, Stefan Brüns wrote:
> On Saturday, December 9, 2017 12:07:08 AM CET Darren Hart (VMware) wrote:
> > The new notify_handler logic determining if autorelease should be used or
> > not is a bit awkward, and can result in more than one call to
> > sparse_keymap_report_event for the same event (scancode). The nesting
> > and long lines also made it difficult to read.
> > 
> > Simplify the logic by eliminating a level of nesting with a goto and
> > always calculate autorelease and val so we can make a single call to
> > sparse_keymap_report_event.
> > 
> > Signed-off-by: Darren Hart (VMware) 
> > Cc: Stefan Brüns 
> > Cc: AceLan Kao 
> > ---
> > Note: This is based on top of Stefan's v2 patch series for intel-vbtn,
> > currently in my review-dvhart branch.
> > 
> >  drivers/platform/x86/intel-vbtn.c | 25 +
> >  1 file changed, 13 insertions(+), 12 deletions(-)
> 
> Reviewed-by: Stefan Brüns 
> Tested-by: Stefan Brüns 

Appreciate you taking the time to review and test, thank you!

-- 
Darren Hart
VMware Open Source Technology Center

Re: [PATCH] platform/x86: intel-vbtn: Simplify autorelease logic

[Darren Hart]

On Mon, Dec 11, 2017 at 02:48:23AM +0100, Stefan Brüns wrote:
> On Saturday, December 9, 2017 12:07:08 AM CET Darren Hart (VMware) wrote:
> > The new notify_handler logic determining if autorelease should be used or
> > not is a bit awkward, and can result in more than one call to
> > sparse_keymap_report_event for the same event (scancode). The nesting
> > and long lines also made it difficult to read.
> > 
> > Simplify the logic by eliminating a level of nesting with a goto and
> > always calculate autorelease and val so we can make a single call to
> > sparse_keymap_report_event.
> > 
> > Signed-off-by: Darren Hart (VMware) 
> > Cc: Stefan Brüns 
> > Cc: AceLan Kao 
> > ---
> > Note: This is based on top of Stefan's v2 patch series for intel-vbtn,
> > currently in my review-dvhart branch.
> > 
> >  drivers/platform/x86/intel-vbtn.c | 25 +
> >  1 file changed, 13 insertions(+), 12 deletions(-)
> 
> Reviewed-by: Stefan Brüns 
> Tested-by: Stefan Brüns 

Appreciate you taking the time to review and test, thank you!

-- 
Darren Hart
VMware Open Source Technology Center

Re: [PATCH] platform/x86: intel-vbtn: Simplify autorelease logic

[Stefan Brüns]

On Saturday, December 9, 2017 12:07:08 AM CET Darren Hart (VMware) wrote:
> The new notify_handler logic determining if autorelease should be used or
> not is a bit awkward, and can result in more than one call to
> sparse_keymap_report_event for the same event (scancode). The nesting
> and long lines also made it difficult to read.
> 
> Simplify the logic by eliminating a level of nesting with a goto and
> always calculate autorelease and val so we can make a single call to
> sparse_keymap_report_event.
> 
> Signed-off-by: Darren Hart (VMware) 
> Cc: Stefan Brüns 
> Cc: AceLan Kao 
> ---
> Note: This is based on top of Stefan's v2 patch series for intel-vbtn,
> currently in my review-dvhart branch.
> 
>  drivers/platform/x86/intel-vbtn.c | 25 +
>  1 file changed, 13 insertions(+), 12 deletions(-)

Reviewed-by: Stefan Brüns 
Tested-by: Stefan Brüns 

Regards,

Stefan

-- 
Stefan Brüns  /  Bergstraße 21  /  52062 Aachen
home: +49 241 53809034 mobile: +49 151 50412019

signature.asc
Description: This is a digitally signed message part.

Re: [PATCH] platform/x86: intel-vbtn: Simplify autorelease logic

[Stefan Brüns]

On Saturday, December 9, 2017 12:07:08 AM CET Darren Hart (VMware) wrote:
> The new notify_handler logic determining if autorelease should be used or
> not is a bit awkward, and can result in more than one call to
> sparse_keymap_report_event for the same event (scancode). The nesting
> and long lines also made it difficult to read.
> 
> Simplify the logic by eliminating a level of nesting with a goto and
> always calculate autorelease and val so we can make a single call to
> sparse_keymap_report_event.
> 
> Signed-off-by: Darren Hart (VMware) 
> Cc: Stefan Brüns 
> Cc: AceLan Kao 
> ---
> Note: This is based on top of Stefan's v2 patch series for intel-vbtn,
> currently in my review-dvhart branch.
> 
>  drivers/platform/x86/intel-vbtn.c | 25 +
>  1 file changed, 13 insertions(+), 12 deletions(-)

Reviewed-by: Stefan Brüns 
Tested-by: Stefan Brüns 

Regards,

Stefan

-- 
Stefan Brüns  /  Bergstraße 21  /  52062 Aachen
home: +49 241 53809034 mobile: +49 151 50412019

signature.asc
Description: This is a digitally signed message part.

Re: [PATCH] platform/x86: intel-vbtn: Simplify autorelease logic

[Darren Hart]

On Sat, Dec 09, 2017 at 12:28:18AM +0100, Stefan Brüns wrote:
> On Saturday, December 9, 2017 12:07:08 AM CET Darren Hart (VMware) wrote:
> > The new notify_handler logic determining if autorelease should be used or
> > not is a bit awkward, and can result in more than one call to
> > sparse_keymap_report_event for the same event (scancode). The nesting
> > and long lines also made it difficult to read.
> > 
> > Simplify the logic by eliminating a level of nesting with a goto and
> > always calculate autorelease and val so we can make a single call to
> > sparse_keymap_report_event.
> > 
> > Signed-off-by: Darren Hart (VMware) 
> > Cc: Stefan Brüns 
> > Cc: AceLan Kao 
> > ---
> > Note: This is based on top of Stefan's v2 patch series for intel-vbtn,
> > currently in my review-dvhart branch.
> 
> Hi Darren,
> 
> is this tree publically available?

Yes, in the tree listed in MAINTAINERS:
git://git.infradead.org/users/dvhart/linux-platform-drivers-x86.git

which is actually an alias I should now update to:

git://git.infradead.org/linux-platform-drivers-x86.git

Note that the review-* branches exist for CI/CD tooling and will rebase often.
testing, fixes, and for-next are the only branches considered to be "published".

-- 
Darren Hart
VMware Open Source Technology Center

Re: [PATCH] platform/x86: intel-vbtn: Simplify autorelease logic

[Darren Hart]

On Sat, Dec 09, 2017 at 12:28:18AM +0100, Stefan Brüns wrote:
> On Saturday, December 9, 2017 12:07:08 AM CET Darren Hart (VMware) wrote:
> > The new notify_handler logic determining if autorelease should be used or
> > not is a bit awkward, and can result in more than one call to
> > sparse_keymap_report_event for the same event (scancode). The nesting
> > and long lines also made it difficult to read.
> > 
> > Simplify the logic by eliminating a level of nesting with a goto and
> > always calculate autorelease and val so we can make a single call to
> > sparse_keymap_report_event.
> > 
> > Signed-off-by: Darren Hart (VMware) 
> > Cc: Stefan Brüns 
> > Cc: AceLan Kao 
> > ---
> > Note: This is based on top of Stefan's v2 patch series for intel-vbtn,
> > currently in my review-dvhart branch.
> 
> Hi Darren,
> 
> is this tree publically available?

Yes, in the tree listed in MAINTAINERS:
git://git.infradead.org/users/dvhart/linux-platform-drivers-x86.git

which is actually an alias I should now update to:

git://git.infradead.org/linux-platform-drivers-x86.git

Note that the review-* branches exist for CI/CD tooling and will rebase often.
testing, fixes, and for-next are the only branches considered to be "published".

-- 
Darren Hart
VMware Open Source Technology Center

Re: [PATCH] platform/x86: intel-vbtn: Simplify autorelease logic

[Stefan Brüns]

On Saturday, December 9, 2017 12:07:08 AM CET Darren Hart (VMware) wrote:
> The new notify_handler logic determining if autorelease should be used or
> not is a bit awkward, and can result in more than one call to
> sparse_keymap_report_event for the same event (scancode). The nesting
> and long lines also made it difficult to read.
> 
> Simplify the logic by eliminating a level of nesting with a goto and
> always calculate autorelease and val so we can make a single call to
> sparse_keymap_report_event.
> 
> Signed-off-by: Darren Hart (VMware) 
> Cc: Stefan Brüns 
> Cc: AceLan Kao 
> ---
> Note: This is based on top of Stefan's v2 patch series for intel-vbtn,
> currently in my review-dvhart branch.

Hi Darren,

is this tree publically available?

Thanks,

Stefan

-- 
Stefan Brüns  /  Bergstraße 21  /  52062 Aachen
home: +49 241 53809034 mobile: +49 151 50412019

signature.asc
Description: This is a digitally signed message part.

Re: [PATCH] platform/x86: intel-vbtn: Simplify autorelease logic

[Stefan Brüns]

On Saturday, December 9, 2017 12:07:08 AM CET Darren Hart (VMware) wrote:
> The new notify_handler logic determining if autorelease should be used or
> not is a bit awkward, and can result in more than one call to
> sparse_keymap_report_event for the same event (scancode). The nesting
> and long lines also made it difficult to read.
> 
> Simplify the logic by eliminating a level of nesting with a goto and
> always calculate autorelease and val so we can make a single call to
> sparse_keymap_report_event.
> 
> Signed-off-by: Darren Hart (VMware) 
> Cc: Stefan Brüns 
> Cc: AceLan Kao 
> ---
> Note: This is based on top of Stefan's v2 patch series for intel-vbtn,
> currently in my review-dvhart branch.

Hi Darren,

is this tree publically available?

Thanks,

Stefan

-- 
Stefan Brüns  /  Bergstraße 21  /  52062 Aachen
home: +49 241 53809034 mobile: +49 151 50412019

signature.asc
Description: This is a digitally signed message part.

[PATCH] platform/x86: intel-vbtn: Simplify autorelease logic

[Darren Hart (VMware)]

The new notify_handler logic determining if autorelease should be used or
not is a bit awkward, and can result in more than one call to
sparse_keymap_report_event for the same event (scancode). The nesting
and long lines also made it difficult to read.

Simplify the logic by eliminating a level of nesting with a goto and
always calculate autorelease and val so we can make a single call to
sparse_keymap_report_event.

Signed-off-by: Darren Hart (VMware) 
Cc: Stefan Brüns 
Cc: AceLan Kao 
---
Note: This is based on top of Stefan's v2 patch series for intel-vbtn, currently
  in my review-dvhart branch.

 drivers/platform/x86/intel-vbtn.c | 25 +
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/platform/x86/intel-vbtn.c 
b/drivers/platform/x86/intel-vbtn.c
index 0861efe..297e1ac 100644
--- a/drivers/platform/x86/intel-vbtn.c
+++ b/drivers/platform/x86/intel-vbtn.c
@@ -80,6 +80,7 @@ static void notify_handler(acpi_handle handle, u32 event, 
void *context)
 {
struct platform_device *device = context;
struct intel_vbtn_priv *priv = dev_get_drvdata(>dev);
+   unsigned int val = !(event & 1); /* Even=press, Odd=release */
const struct key_entry *ke_rel;
bool autorelease;
 
@@ -88,20 +89,20 @@ static void notify_handler(acpi_handle handle, u32 event, 
void *context)
pm_wakeup_hard_event(>dev);
return;
}
-   } else {
-   /* Use the fact press/release come in even/odd pairs */
-   if ((event & 1) && sparse_keymap_report_event(priv->input_dev,
- event, 0, false))
-   return;
+   goto out_unknown;
+   }
 
-   ke_rel = sparse_keymap_entry_from_scancode(priv->input_dev,
-  event | 1);
-   autorelease = !ke_rel || ke_rel->type == KE_IGNORE;
+   /*
+* Event press events are autorelease if there is no corresponding odd
+* release event, or if the odd event is KE_IGNORE.
+*/
+   ke_rel = sparse_keymap_entry_from_scancode(priv->input_dev, event | 1);
+   autorelease = val && (!ke_rel || ke_rel->type == KE_IGNORE);
 
-   if (sparse_keymap_report_event(priv->input_dev, event, 1,
-  autorelease))
-   return;
-   }
+   if (sparse_keymap_report_event(priv->input_dev, event, val, 
autorelease))
+   return;
+
+out_unknown:
dev_dbg(>dev, "unknown event index 0x%x\n", event);
 }
 
-- 
2.9.4


-- 
Darren Hart
VMware Open Source Technology Center

[PATCH] platform/x86: intel-vbtn: Simplify autorelease logic

[Darren Hart (VMware)]

The new notify_handler logic determining if autorelease should be used or
not is a bit awkward, and can result in more than one call to
sparse_keymap_report_event for the same event (scancode). The nesting
and long lines also made it difficult to read.

Simplify the logic by eliminating a level of nesting with a goto and
always calculate autorelease and val so we can make a single call to
sparse_keymap_report_event.

Signed-off-by: Darren Hart (VMware) 
Cc: Stefan Brüns 
Cc: AceLan Kao 
---
Note: This is based on top of Stefan's v2 patch series for intel-vbtn, currently
  in my review-dvhart branch.

 drivers/platform/x86/intel-vbtn.c | 25 +
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/platform/x86/intel-vbtn.c 
b/drivers/platform/x86/intel-vbtn.c
index 0861efe..297e1ac 100644
--- a/drivers/platform/x86/intel-vbtn.c
+++ b/drivers/platform/x86/intel-vbtn.c
@@ -80,6 +80,7 @@ static void notify_handler(acpi_handle handle, u32 event, 
void *context)
 {
struct platform_device *device = context;
struct intel_vbtn_priv *priv = dev_get_drvdata(>dev);
+   unsigned int val = !(event & 1); /* Even=press, Odd=release */
const struct key_entry *ke_rel;
bool autorelease;
 
@@ -88,20 +89,20 @@ static void notify_handler(acpi_handle handle, u32 event, 
void *context)
pm_wakeup_hard_event(>dev);
return;
}
-   } else {
-   /* Use the fact press/release come in even/odd pairs */
-   if ((event & 1) && sparse_keymap_report_event(priv->input_dev,
- event, 0, false))
-   return;
+   goto out_unknown;
+   }
 
-   ke_rel = sparse_keymap_entry_from_scancode(priv->input_dev,
-  event | 1);
-   autorelease = !ke_rel || ke_rel->type == KE_IGNORE;
+   /*
+* Event press events are autorelease if there is no corresponding odd
+* release event, or if the odd event is KE_IGNORE.
+*/
+   ke_rel = sparse_keymap_entry_from_scancode(priv->input_dev, event | 1);
+   autorelease = val && (!ke_rel || ke_rel->type == KE_IGNORE);
 
-   if (sparse_keymap_report_event(priv->input_dev, event, 1,
-  autorelease))
-   return;
-   }
+   if (sparse_keymap_report_event(priv->input_dev, event, val, 
autorelease))
+   return;
+
+out_unknown:
dev_dbg(>dev, "unknown event index 0x%x\n", event);
 }
 
-- 
2.9.4


-- 
Darren Hart
VMware Open Source Technology Center