Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1341613 > unrolled thread

[PATCH v4 0/5] Process Dell Instant Launch hotkey on Vostro V131 and Inspiron M5110

Started byMichał Kępień <kernel@kempniu.pl>
First post2016-02-24 08:20 +0100
Last post2016-03-01 00:10 +0100
Articles 20 on this page of 27 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v4 0/5] Process Dell Instant Launch hotkey on Vostro V131 and Inspiron M5110 Michał Kępień <kernel@kempniu.pl> - 2016-02-24 08:20 +0100
    [PATCH v4 1/5] dell-laptop: move dell_smi_error() to dell-smbios Michał Kępień <kernel@kempniu.pl> - 2016-02-24 08:20 +0100
      Re: [PATCH v4 1/5] dell-laptop: move dell_smi_error() to dell-smbios Pali Rohár <pali.rohar@gmail.com> - 2016-02-29 14:00 +0100
        Re: [PATCH v4 1/5] dell-laptop: move dell_smi_error() to dell-smbios Michał Kępień <kernel@kempniu.pl> - 2016-02-29 21:30 +0100
          Re: [PATCH v4 1/5] dell-laptop: move dell_smi_error() to dell-smbios Pali Rohár <pali.rohar@gmail.com> - 2016-02-29 21:30 +0100
            Re: [PATCH v4 1/5] dell-laptop: move dell_smi_error() to dell-smbios Michał Kępień <kernel@kempniu.pl> - 2016-02-29 21:50 +0100
              Re: [PATCH v4 1/5] dell-laptop: move dell_smi_error() to dell-smbios Darren Hart <dvhart@infradead.org> - 2016-03-01 00:00 +0100
                Re: [PATCH v4 1/5] dell-laptop: move dell_smi_error() to dell-smbios Michał Kępień <kernel@kempniu.pl> - 2016-03-02 12:50 +0100
                  Re: [PATCH v4 1/5] dell-laptop: move dell_smi_error() to dell-smbios Pali Rohár <pali.rohar@gmail.com> - 2016-03-03 12:40 +0100
    [PATCH v4 5/5] dell-wmi: support Dell Inspiron M5110 Michał Kępień <kernel@kempniu.pl> - 2016-02-24 08:20 +0100
      Re: [PATCH v4 5/5] dell-wmi: support Dell Inspiron M5110 Pali Rohár <pali.rohar@gmail.com> - 2016-02-29 14:10 +0100
    [PATCH v4 3/5] dell-wmi: enable receiving WMI events on Dell Vostro V131 Michał Kępień <kernel@kempniu.pl> - 2016-02-24 08:20 +0100
      Re: [PATCH v4 3/5] dell-wmi: enable receiving WMI events on Dell  Vostro V131 Pali Rohár <pali.rohar@gmail.com> - 2016-02-29 14:00 +0100
    [PATCH v4 2/5] dell-smbios: rename dell_smi_error() to dell_smbios_error() Michał Kępień <kernel@kempniu.pl> - 2016-02-24 08:30 +0100
      Re: [PATCH v4 2/5] dell-smbios: rename dell_smi_error() to  dell_smbios_error() Pali Rohár <pali.rohar@gmail.com> - 2016-02-29 14:00 +0100
    [PATCH v4 4/5] dell-wmi: properly process Dell Instant Launch hotkey Michał Kępień <kernel@kempniu.pl> - 2016-02-24 08:30 +0100
      Re: [PATCH v4 4/5] dell-wmi: properly process Dell Instant Launch  hotkey Pali Rohár <pali.rohar@gmail.com> - 2016-02-29 14:10 +0100
        Re: [PATCH v4 4/5] dell-wmi: properly process Dell Instant Launch  hotkey Michał Kępień <kernel@kempniu.pl> - 2016-02-29 21:40 +0100
          Re: [PATCH v4 4/5] dell-wmi: properly process Dell Instant Launch hotkey Pali Rohár <pali.rohar@gmail.com> - 2016-02-29 21:40 +0100
            Re: [PATCH v4 4/5] dell-wmi: properly process Dell Instant Launch  hotkey Michał Kępień <kernel@kempniu.pl> - 2016-02-29 21:50 +0100
              Re: [PATCH v4 4/5] dell-wmi: properly process Dell Instant Launch hotkey Pali Rohár <pali.rohar@gmail.com> - 2016-02-29 22:00 +0100
                Re: [PATCH v4 4/5] dell-wmi: properly process Dell Instant Launch  hotkey Darren Hart <dvhart@infradead.org> - 2016-03-01 00:10 +0100
                  Re: [PATCH v4 4/5] dell-wmi: properly process Dell Instant Launch  hotkey Michał Kępień <kernel@kempniu.pl> - 2016-03-02 13:40 +0100
                    Re: [PATCH v4 4/5] dell-wmi: properly process Dell Instant Launch  hotkey Darren Hart <dvhart@infradead.org> - 2016-03-03 18:20 +0100
                      Re: [PATCH v4 4/5] dell-wmi: properly process Dell Instant Launch  hotkey Michał Kępień <kernel@kempniu.pl> - 2016-03-03 19:50 +0100
                        Re: [PATCH v4 4/5] dell-wmi: properly process Dell Instant Launch  hotkey Darren Hart <dvhart@infradead.org> - 2016-03-03 21:50 +0100
          Re: [PATCH v4 4/5] dell-wmi: properly process Dell Instant Launch  hotkey Darren Hart <dvhart@infradead.org> - 2016-03-01 00:10 +0100

Page 1 of 2  [1] 2  Next page →

#1341613 — [PATCH v4 0/5] Process Dell Instant Launch hotkey on Vostro V131 and Inspiron M5110

FromMichał Kępień <kernel@kempniu.pl>
Date2016-02-24 08:20 +0100
Subject[PATCH v4 0/5] Process Dell Instant Launch hotkey on Vostro V131 and Inspiron M5110
Message-ID<r5BTP-11s-1@gated-at.bofh.it>
This patch series makes use of the API exported by dell-smbios, so it
should be applied to either testing or dell-smbios.

Changes from v3:

  - Extract code issuing the WMI-controlling SMBIOS request into a
    separate function

  - Document the SMBIOS request used

  - Split keymap section "Shortcut and audio panel keys" into two
    separate sections

Changes from v2:

  - Use a static variable instead of a quirk structure

  - Use API exported by dell-smbios to issue the SMBIOS request required
    for generating WMI events, returning with error from dell_wmi_init()
    if it fails

  - Move dell_smi_error() from dell-laptop to dell-smbios and use it to
    determine error code for returning from dell_wmi_init() when
    enabling WMI fails

  - Support Dell Inspiron M5110

Changes from v1:

  - Use DMI matching instead of a module parameter
  - Change flag name to improve readability

 drivers/platform/x86/Kconfig       |    1 +
 drivers/platform/x86/dell-laptop.c |   30 ++++---------
 drivers/platform/x86/dell-smbios.c |   16 +++++++
 drivers/platform/x86/dell-smbios.h |    2 +
 drivers/platform/x86/dell-wmi.c    |   84 +++++++++++++++++++++++++++++++++++-
 5 files changed, 109 insertions(+), 24 deletions(-)

-- 
1.7.10.4

[toc] | [next] | [standalone]

#1341614 — [PATCH v4 1/5] dell-laptop: move dell_smi_error() to dell-smbios

FromMichał Kępień <kernel@kempniu.pl>
Date2016-02-24 08:20 +0100
Subject[PATCH v4 1/5] dell-laptop: move dell_smi_error() to dell-smbios
Message-ID<r5BTQ-11s-5@gated-at.bofh.it>
In reply to#1341613
The dell_smi_error() method could be used by modules other than
dell-laptop for convenient translation of SMBIOS request errors into
errno values.  Thus, move it to dell-smbios.

Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
 drivers/platform/x86/dell-laptop.c |   14 --------------
 drivers/platform/x86/dell-smbios.c |   16 ++++++++++++++++
 drivers/platform/x86/dell-smbios.h |    2 ++
 3 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index 76064c8..cbafb95 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -273,20 +273,6 @@ static const struct dmi_system_id dell_quirks[] __initconst = {
 	{ }
 };
 
-static inline int dell_smi_error(int value)
-{
-	switch (value) {
-	case 0: /* Completed successfully */
-		return 0;
-	case -1: /* Completed with error */
-		return -EIO;
-	case -2: /* Function not supported */
-		return -ENXIO;
-	default: /* Unknown error */
-		return -EINVAL;
-	}
-}
-
 /*
  * Derived from information in smbios-wireless-ctl:
  *
diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c
index 2a4992a..942572f 100644
--- a/drivers/platform/x86/dell-smbios.c
+++ b/drivers/platform/x86/dell-smbios.c
@@ -16,6 +16,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/dmi.h>
+#include <linux/err.h>
 #include <linux/gfp.h>
 #include <linux/mutex.h>
 #include <linux/slab.h>
@@ -39,6 +40,21 @@ static int da_command_code;
 static int da_num_tokens;
 static struct calling_interface_token *da_tokens;
 
+int dell_smi_error(int value)
+{
+	switch (value) {
+	case 0: /* Completed successfully */
+		return 0;
+	case -1: /* Completed with error */
+		return -EIO;
+	case -2: /* Function not supported */
+		return -ENXIO;
+	default: /* Unknown error */
+		return -EINVAL;
+	}
+}
+EXPORT_SYMBOL_GPL(dell_smi_error);
+
 struct calling_interface_buffer *dell_smbios_get_buffer(void)
 {
 	mutex_lock(&buffer_mutex);
diff --git a/drivers/platform/x86/dell-smbios.h b/drivers/platform/x86/dell-smbios.h
index 4f69b16..52febe6 100644
--- a/drivers/platform/x86/dell-smbios.h
+++ b/drivers/platform/x86/dell-smbios.h
@@ -35,6 +35,8 @@ struct calling_interface_token {
 	};
 };
 
+int dell_smi_error(int value);
+
 struct calling_interface_buffer *dell_smbios_get_buffer(void);
 void dell_smbios_clear_buffer(void);
 void dell_smbios_release_buffer(void);
-- 
1.7.10.4

[toc] | [prev] | [next] | [standalone]

#1345770 — Re: [PATCH v4 1/5] dell-laptop: move dell_smi_error() to dell-smbios

FromPali Rohár <pali.rohar@gmail.com>
Date2016-02-29 14:00 +0100
SubjectRe: [PATCH v4 1/5] dell-laptop: move dell_smi_error() to dell-smbios
Message-ID<r7vAC-38v-17@gated-at.bofh.it>
In reply to#1341614
On Wednesday 24 February 2016 08:20:11 Michał Kępień wrote:
> The dell_smi_error() method could be used by modules other than
> dell-laptop for convenient translation of SMBIOS request errors into
> errno values.  Thus, move it to dell-smbios.
> 
> Signed-off-by: Michał Kępień <kernel@kempniu.pl>
> ---
>  drivers/platform/x86/dell-laptop.c |   14 --------------
>  drivers/platform/x86/dell-smbios.c |   16 ++++++++++++++++
>  drivers/platform/x86/dell-smbios.h |    2 ++
>  3 files changed, 18 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
> index 76064c8..cbafb95 100644
> --- a/drivers/platform/x86/dell-laptop.c
> +++ b/drivers/platform/x86/dell-laptop.c
> @@ -273,20 +273,6 @@ static const struct dmi_system_id dell_quirks[] __initconst = {
>  	{ }
>  };
>  
> -static inline int dell_smi_error(int value)
> -{
> -	switch (value) {
> -	case 0: /* Completed successfully */
> -		return 0;
> -	case -1: /* Completed with error */
> -		return -EIO;
> -	case -2: /* Function not supported */
> -		return -ENXIO;
> -	default: /* Unknown error */
> -		return -EINVAL;
> -	}
> -}
> -
>  /*
>   * Derived from information in smbios-wireless-ctl:
>   *
> diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c
> index 2a4992a..942572f 100644
> --- a/drivers/platform/x86/dell-smbios.c
> +++ b/drivers/platform/x86/dell-smbios.c
> @@ -16,6 +16,7 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/dmi.h>
> +#include <linux/err.h>
>  #include <linux/gfp.h>
>  #include <linux/mutex.h>
>  #include <linux/slab.h>
> @@ -39,6 +40,21 @@ static int da_command_code;
>  static int da_num_tokens;
>  static struct calling_interface_token *da_tokens;
>  
> +int dell_smi_error(int value)
> +{
> +	switch (value) {
> +	case 0: /* Completed successfully */
> +		return 0;
> +	case -1: /* Completed with error */
> +		return -EIO;
> +	case -2: /* Function not supported */
> +		return -ENXIO;
> +	default: /* Unknown error */
> +		return -EINVAL;
> +	}
> +}
> +EXPORT_SYMBOL_GPL(dell_smi_error);
> +
>  struct calling_interface_buffer *dell_smbios_get_buffer(void)
>  {
>  	mutex_lock(&buffer_mutex);
> diff --git a/drivers/platform/x86/dell-smbios.h b/drivers/platform/x86/dell-smbios.h
> index 4f69b16..52febe6 100644
> --- a/drivers/platform/x86/dell-smbios.h
> +++ b/drivers/platform/x86/dell-smbios.h
> @@ -35,6 +35,8 @@ struct calling_interface_token {
>  	};
>  };
>  
> +int dell_smi_error(int value);
> +
>  struct calling_interface_buffer *dell_smbios_get_buffer(void);
>  void dell_smbios_clear_buffer(void);
>  void dell_smbios_release_buffer(void);

And... here what about inline vs EXPORT_SYMBOL function? Just asking...

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]

#1346142 — Re: [PATCH v4 1/5] dell-laptop: move dell_smi_error() to dell-smbios

FromMichał Kępień <kernel@kempniu.pl>
Date2016-02-29 21:30 +0100
SubjectRe: [PATCH v4 1/5] dell-laptop: move dell_smi_error() to dell-smbios
Message-ID<r7CC6-7NL-11@gated-at.bofh.it>
In reply to#1345770
> On Wednesday 24 February 2016 08:20:11 Michał Kępień wrote:
> > The dell_smi_error() method could be used by modules other than
> > dell-laptop for convenient translation of SMBIOS request errors into
> > errno values.  Thus, move it to dell-smbios.
> > 
> > Signed-off-by: Michał Kępień <kernel@kempniu.pl>
> > ---
> >  drivers/platform/x86/dell-laptop.c |   14 --------------
> >  drivers/platform/x86/dell-smbios.c |   16 ++++++++++++++++
> >  drivers/platform/x86/dell-smbios.h |    2 ++
> >  3 files changed, 18 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
> > index 76064c8..cbafb95 100644
> > --- a/drivers/platform/x86/dell-laptop.c
> > +++ b/drivers/platform/x86/dell-laptop.c
> > @@ -273,20 +273,6 @@ static const struct dmi_system_id dell_quirks[] __initconst = {
> >  	{ }
> >  };
> >  
> > -static inline int dell_smi_error(int value)
> > -{
> > -	switch (value) {
> > -	case 0: /* Completed successfully */
> > -		return 0;
> > -	case -1: /* Completed with error */
> > -		return -EIO;
> > -	case -2: /* Function not supported */
> > -		return -ENXIO;
> > -	default: /* Unknown error */
> > -		return -EINVAL;
> > -	}
> > -}
> > -
> >  /*
> >   * Derived from information in smbios-wireless-ctl:
> >   *
> > diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c
> > index 2a4992a..942572f 100644
> > --- a/drivers/platform/x86/dell-smbios.c
> > +++ b/drivers/platform/x86/dell-smbios.c
> > @@ -16,6 +16,7 @@
> >  #include <linux/kernel.h>
> >  #include <linux/module.h>
> >  #include <linux/dmi.h>
> > +#include <linux/err.h>
> >  #include <linux/gfp.h>
> >  #include <linux/mutex.h>
> >  #include <linux/slab.h>
> > @@ -39,6 +40,21 @@ static int da_command_code;
> >  static int da_num_tokens;
> >  static struct calling_interface_token *da_tokens;
> >  
> > +int dell_smi_error(int value)
> > +{
> > +	switch (value) {
> > +	case 0: /* Completed successfully */
> > +		return 0;
> > +	case -1: /* Completed with error */
> > +		return -EIO;
> > +	case -2: /* Function not supported */
> > +		return -ENXIO;
> > +	default: /* Unknown error */
> > +		return -EINVAL;
> > +	}
> > +}
> > +EXPORT_SYMBOL_GPL(dell_smi_error);
> > +
> >  struct calling_interface_buffer *dell_smbios_get_buffer(void)
> >  {
> >  	mutex_lock(&buffer_mutex);
> > diff --git a/drivers/platform/x86/dell-smbios.h b/drivers/platform/x86/dell-smbios.h
> > index 4f69b16..52febe6 100644
> > --- a/drivers/platform/x86/dell-smbios.h
> > +++ b/drivers/platform/x86/dell-smbios.h
> > @@ -35,6 +35,8 @@ struct calling_interface_token {
> >  	};
> >  };
> >  
> > +int dell_smi_error(int value);
> > +
> >  struct calling_interface_buffer *dell_smbios_get_buffer(void);
> >  void dell_smbios_clear_buffer(void);
> >  void dell_smbios_release_buffer(void);
> 
> And... here what about inline vs EXPORT_SYMBOL function? Just asking...

Well, what about it? :)  The commit message is pretty explicit in
describing what happens here, i.e. a previously static function is moved
to another module so that it can be reused.  Thus, keeping the inline
keyword makes no sense.  What exactly is your concern?

-- 
Best regards,
Michał Kępień

[toc] | [prev] | [next] | [standalone]

#1346143 — Re: [PATCH v4 1/5] dell-laptop: move dell_smi_error() to dell-smbios

FromPali Rohár <pali.rohar@gmail.com>
Date2016-02-29 21:30 +0100
SubjectRe: [PATCH v4 1/5] dell-laptop: move dell_smi_error() to dell-smbios
Message-ID<r7CC6-7NL-13@gated-at.bofh.it>
In reply to#1346142

[Multipart message — attachments visible in raw view] — view raw

On Monday 29 February 2016 21:22:54 Michał Kępień wrote:
> > On Wednesday 24 February 2016 08:20:11 Michał Kępień wrote:
> > > The dell_smi_error() method could be used by modules other than
> > > dell-laptop for convenient translation of SMBIOS request errors
> > > into errno values.  Thus, move it to dell-smbios.
> > > 
> > > Signed-off-by: Michał Kępień <kernel@kempniu.pl>
> > > ---
> > > 
> > >  drivers/platform/x86/dell-laptop.c |   14 --------------
> > >  drivers/platform/x86/dell-smbios.c |   16 ++++++++++++++++
> > >  drivers/platform/x86/dell-smbios.h |    2 ++
> > >  3 files changed, 18 insertions(+), 14 deletions(-)
> > > 
> > > diff --git a/drivers/platform/x86/dell-laptop.c
> > > b/drivers/platform/x86/dell-laptop.c index 76064c8..cbafb95
> > > 100644
> > > --- a/drivers/platform/x86/dell-laptop.c
> > > +++ b/drivers/platform/x86/dell-laptop.c
> > > @@ -273,20 +273,6 @@ static const struct dmi_system_id
> > > dell_quirks[] __initconst = {
> > > 
> > >  	{ }
> > >  
> > >  };
> > > 
> > > -static inline int dell_smi_error(int value)
> > > -{
> > > -	switch (value) {
> > > -	case 0: /* Completed successfully */
> > > -		return 0;
> > > -	case -1: /* Completed with error */
> > > -		return -EIO;
> > > -	case -2: /* Function not supported */
> > > -		return -ENXIO;
> > > -	default: /* Unknown error */
> > > -		return -EINVAL;
> > > -	}
> > > -}
> > > -
> > > 
> > >  /*
> > >  
> > >   * Derived from information in smbios-wireless-ctl:
> > >   *
> > > 
> > > diff --git a/drivers/platform/x86/dell-smbios.c
> > > b/drivers/platform/x86/dell-smbios.c index 2a4992a..942572f
> > > 100644
> > > --- a/drivers/platform/x86/dell-smbios.c
> > > +++ b/drivers/platform/x86/dell-smbios.c
> > > @@ -16,6 +16,7 @@
> > > 
> > >  #include <linux/kernel.h>
> > >  #include <linux/module.h>
> > >  #include <linux/dmi.h>
> > > 
> > > +#include <linux/err.h>
> > > 
> > >  #include <linux/gfp.h>
> > >  #include <linux/mutex.h>
> > >  #include <linux/slab.h>
> > > 
> > > @@ -39,6 +40,21 @@ static int da_command_code;
> > > 
> > >  static int da_num_tokens;
> > >  static struct calling_interface_token *da_tokens;
> > > 
> > > +int dell_smi_error(int value)
> > > +{
> > > +	switch (value) {
> > > +	case 0: /* Completed successfully */
> > > +		return 0;
> > > +	case -1: /* Completed with error */
> > > +		return -EIO;
> > > +	case -2: /* Function not supported */
> > > +		return -ENXIO;
> > > +	default: /* Unknown error */
> > > +		return -EINVAL;
> > > +	}
> > > +}
> > > +EXPORT_SYMBOL_GPL(dell_smi_error);
> > > +
> > > 
> > >  struct calling_interface_buffer *dell_smbios_get_buffer(void)
> > >  {
> > >  
> > >  	mutex_lock(&buffer_mutex);
> > > 
> > > diff --git a/drivers/platform/x86/dell-smbios.h
> > > b/drivers/platform/x86/dell-smbios.h index 4f69b16..52febe6
> > > 100644
> > > --- a/drivers/platform/x86/dell-smbios.h
> > > +++ b/drivers/platform/x86/dell-smbios.h
> > > @@ -35,6 +35,8 @@ struct calling_interface_token {
> > > 
> > >  	};
> > >  
> > >  };
> > > 
> > > +int dell_smi_error(int value);
> > > +
> > > 
> > >  struct calling_interface_buffer *dell_smbios_get_buffer(void);
> > >  void dell_smbios_clear_buffer(void);
> > >  void dell_smbios_release_buffer(void);
> > 
> > And... here what about inline vs EXPORT_SYMBOL function? Just
> > asking...
> 
> Well, what about it? :)  The commit message is pretty explicit in
> describing what happens here, i.e. a previously static function is
> moved to another module so that it can be reused.  Thus, keeping the
> inline keyword makes no sense.  What exactly is your concern?

Just asking if this function should be or not be inline (of course in 
header file, not in module .c).

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]

#1346154 — Re: [PATCH v4 1/5] dell-laptop: move dell_smi_error() to dell-smbios

FromMichał Kępień <kernel@kempniu.pl>
Date2016-02-29 21:50 +0100
SubjectRe: [PATCH v4 1/5] dell-laptop: move dell_smi_error() to dell-smbios
Message-ID<r7CVs-7Vn-17@gated-at.bofh.it>
In reply to#1346143
> On Monday 29 February 2016 21:22:54 Michał Kępień wrote:
> > > On Wednesday 24 February 2016 08:20:11 Michał Kępień wrote:
> > > > The dell_smi_error() method could be used by modules other than
> > > > dell-laptop for convenient translation of SMBIOS request errors
> > > > into errno values.  Thus, move it to dell-smbios.
> > > > 
> > > > Signed-off-by: Michał Kępień <kernel@kempniu.pl>
> > > > ---
> > > > 
> > > >  drivers/platform/x86/dell-laptop.c |   14 --------------
> > > >  drivers/platform/x86/dell-smbios.c |   16 ++++++++++++++++
> > > >  drivers/platform/x86/dell-smbios.h |    2 ++
> > > >  3 files changed, 18 insertions(+), 14 deletions(-)
> > > > 
> > > > diff --git a/drivers/platform/x86/dell-laptop.c
> > > > b/drivers/platform/x86/dell-laptop.c index 76064c8..cbafb95
> > > > 100644
> > > > --- a/drivers/platform/x86/dell-laptop.c
> > > > +++ b/drivers/platform/x86/dell-laptop.c
> > > > @@ -273,20 +273,6 @@ static const struct dmi_system_id
> > > > dell_quirks[] __initconst = {
> > > > 
> > > >  	{ }
> > > >  
> > > >  };
> > > > 
> > > > -static inline int dell_smi_error(int value)
> > > > -{
> > > > -	switch (value) {
> > > > -	case 0: /* Completed successfully */
> > > > -		return 0;
> > > > -	case -1: /* Completed with error */
> > > > -		return -EIO;
> > > > -	case -2: /* Function not supported */
> > > > -		return -ENXIO;
> > > > -	default: /* Unknown error */
> > > > -		return -EINVAL;
> > > > -	}
> > > > -}
> > > > -
> > > > 
> > > >  /*
> > > >  
> > > >   * Derived from information in smbios-wireless-ctl:
> > > >   *
> > > > 
> > > > diff --git a/drivers/platform/x86/dell-smbios.c
> > > > b/drivers/platform/x86/dell-smbios.c index 2a4992a..942572f
> > > > 100644
> > > > --- a/drivers/platform/x86/dell-smbios.c
> > > > +++ b/drivers/platform/x86/dell-smbios.c
> > > > @@ -16,6 +16,7 @@
> > > > 
> > > >  #include <linux/kernel.h>
> > > >  #include <linux/module.h>
> > > >  #include <linux/dmi.h>
> > > > 
> > > > +#include <linux/err.h>
> > > > 
> > > >  #include <linux/gfp.h>
> > > >  #include <linux/mutex.h>
> > > >  #include <linux/slab.h>
> > > > 
> > > > @@ -39,6 +40,21 @@ static int da_command_code;
> > > > 
> > > >  static int da_num_tokens;
> > > >  static struct calling_interface_token *da_tokens;
> > > > 
> > > > +int dell_smi_error(int value)
> > > > +{
> > > > +	switch (value) {
> > > > +	case 0: /* Completed successfully */
> > > > +		return 0;
> > > > +	case -1: /* Completed with error */
> > > > +		return -EIO;
> > > > +	case -2: /* Function not supported */
> > > > +		return -ENXIO;
> > > > +	default: /* Unknown error */
> > > > +		return -EINVAL;
> > > > +	}
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(dell_smi_error);
> > > > +
> > > > 
> > > >  struct calling_interface_buffer *dell_smbios_get_buffer(void)
> > > >  {
> > > >  
> > > >  	mutex_lock(&buffer_mutex);
> > > > 
> > > > diff --git a/drivers/platform/x86/dell-smbios.h
> > > > b/drivers/platform/x86/dell-smbios.h index 4f69b16..52febe6
> > > > 100644
> > > > --- a/drivers/platform/x86/dell-smbios.h
> > > > +++ b/drivers/platform/x86/dell-smbios.h
> > > > @@ -35,6 +35,8 @@ struct calling_interface_token {
> > > > 
> > > >  	};
> > > >  
> > > >  };
> > > > 
> > > > +int dell_smi_error(int value);
> > > > +
> > > > 
> > > >  struct calling_interface_buffer *dell_smbios_get_buffer(void);
> > > >  void dell_smbios_clear_buffer(void);
> > > >  void dell_smbios_release_buffer(void);
> > > 
> > > And... here what about inline vs EXPORT_SYMBOL function? Just
> > > asking...
> > 
> > Well, what about it? :)  The commit message is pretty explicit in
> > describing what happens here, i.e. a previously static function is
> > moved to another module so that it can be reused.  Thus, keeping the
> > inline keyword makes no sense.  What exactly is your concern?
> 
> Just asking if this function should be or not be inline (of course in 
> header file, not in module .c).

If you mark a function as inline in the header file, you have to provide
its definition, otherwise you'll get a compilation error.  Given that
this is in no way performance-critical code, I see no point in
clobbering the header file with the body of this function.

-- 
Best regards,
Michał Kępień

[toc] | [prev] | [next] | [standalone]

#1346229 — Re: [PATCH v4 1/5] dell-laptop: move dell_smi_error() to dell-smbios

FromDarren Hart <dvhart@infradead.org>
Date2016-03-01 00:00 +0100
SubjectRe: [PATCH v4 1/5] dell-laptop: move dell_smi_error() to dell-smbios
Message-ID<r7EXf-Ih-5@gated-at.bofh.it>
In reply to#1346154
On Mon, Feb 29, 2016 at 09:41:36PM +0100, Michał Kępień wrote:
> > On Monday 29 February 2016 21:22:54 Michał Kępień wrote:
> > > > On Wednesday 24 February 2016 08:20:11 Michał Kępień wrote:
> > > > > The dell_smi_error() method could be used by modules other than
> > > > > dell-laptop for convenient translation of SMBIOS request errors
> > > > > into errno values.  Thus, move it to dell-smbios.
> > > > > 
> > > > > Signed-off-by: Michał Kępień <kernel@kempniu.pl>
> > > > > ---
> > > > > 
> > > > >  drivers/platform/x86/dell-laptop.c |   14 --------------
> > > > >  drivers/platform/x86/dell-smbios.c |   16 ++++++++++++++++
> > > > >  drivers/platform/x86/dell-smbios.h |    2 ++
> > > > >  3 files changed, 18 insertions(+), 14 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/platform/x86/dell-laptop.c
> > > > > b/drivers/platform/x86/dell-laptop.c index 76064c8..cbafb95
> > > > > 100644
> > > > > --- a/drivers/platform/x86/dell-laptop.c
> > > > > +++ b/drivers/platform/x86/dell-laptop.c
> > > > > @@ -273,20 +273,6 @@ static const struct dmi_system_id
> > > > > dell_quirks[] __initconst = {
> > > > > 
> > > > >  	{ }
> > > > >  
> > > > >  };
> > > > > 
> > > > > -static inline int dell_smi_error(int value)
> > > > > -{
> > > > > -	switch (value) {
> > > > > -	case 0: /* Completed successfully */
> > > > > -		return 0;
> > > > > -	case -1: /* Completed with error */
> > > > > -		return -EIO;
> > > > > -	case -2: /* Function not supported */
> > > > > -		return -ENXIO;
> > > > > -	default: /* Unknown error */
> > > > > -		return -EINVAL;
> > > > > -	}
> > > > > -}
> > > > > -
> > > > > 
> > > > >  /*
> > > > >  
> > > > >   * Derived from information in smbios-wireless-ctl:
> > > > >   *
> > > > > 
> > > > > diff --git a/drivers/platform/x86/dell-smbios.c
> > > > > b/drivers/platform/x86/dell-smbios.c index 2a4992a..942572f
> > > > > 100644
> > > > > --- a/drivers/platform/x86/dell-smbios.c
> > > > > +++ b/drivers/platform/x86/dell-smbios.c
> > > > > @@ -16,6 +16,7 @@
> > > > > 
> > > > >  #include <linux/kernel.h>
> > > > >  #include <linux/module.h>
> > > > >  #include <linux/dmi.h>
> > > > > 
> > > > > +#include <linux/err.h>
> > > > > 
> > > > >  #include <linux/gfp.h>
> > > > >  #include <linux/mutex.h>
> > > > >  #include <linux/slab.h>
> > > > > 
> > > > > @@ -39,6 +40,21 @@ static int da_command_code;
> > > > > 
> > > > >  static int da_num_tokens;
> > > > >  static struct calling_interface_token *da_tokens;
> > > > > 
> > > > > +int dell_smi_error(int value)
> > > > > +{
> > > > > +	switch (value) {
> > > > > +	case 0: /* Completed successfully */
> > > > > +		return 0;
> > > > > +	case -1: /* Completed with error */
> > > > > +		return -EIO;
> > > > > +	case -2: /* Function not supported */
> > > > > +		return -ENXIO;
> > > > > +	default: /* Unknown error */
> > > > > +		return -EINVAL;
> > > > > +	}
> > > > > +}
> > > > > +EXPORT_SYMBOL_GPL(dell_smi_error);
> > > > > +
> > > > > 
> > > > >  struct calling_interface_buffer *dell_smbios_get_buffer(void)
> > > > >  {
> > > > >  
> > > > >  	mutex_lock(&buffer_mutex);
> > > > > 
> > > > > diff --git a/drivers/platform/x86/dell-smbios.h
> > > > > b/drivers/platform/x86/dell-smbios.h index 4f69b16..52febe6
> > > > > 100644
> > > > > --- a/drivers/platform/x86/dell-smbios.h
> > > > > +++ b/drivers/platform/x86/dell-smbios.h
> > > > > @@ -35,6 +35,8 @@ struct calling_interface_token {
> > > > > 
> > > > >  	};
> > > > >  
> > > > >  };
> > > > > 
> > > > > +int dell_smi_error(int value);
> > > > > +
> > > > > 
> > > > >  struct calling_interface_buffer *dell_smbios_get_buffer(void);
> > > > >  void dell_smbios_clear_buffer(void);
> > > > >  void dell_smbios_release_buffer(void);
> > > > 
> > > > And... here what about inline vs EXPORT_SYMBOL function? Just
> > > > asking...
> > > 
> > > Well, what about it? :)  The commit message is pretty explicit in
> > > describing what happens here, i.e. a previously static function is
> > > moved to another module so that it can be reused.  Thus, keeping the
> > > inline keyword makes no sense.  What exactly is your concern?
> > 
> > Just asking if this function should be or not be inline (of course in 
> > header file, not in module .c).
> 
> If you mark a function as inline in the header file, you have to provide
> its definition, otherwise you'll get a compilation error.  Given that
> this is in no way performance-critical code, I see no point in
> clobbering the header file with the body of this function.

Agreed, please leave it as is.

For a discussion on inline, please see:
CodingStyle: Chapter 15: The inline disease

-- 
Darren Hart
Intel Open Source Technology Center

[toc] | [prev] | [next] | [standalone]

#1347977 — Re: [PATCH v4 1/5] dell-laptop: move dell_smi_error() to dell-smbios

FromMichał Kępień <kernel@kempniu.pl>
Date2016-03-02 12:50 +0100
SubjectRe: [PATCH v4 1/5] dell-laptop: move dell_smi_error() to dell-smbios
Message-ID<r8drY-71N-27@gated-at.bofh.it>
In reply to#1346229
> On Mon, Feb 29, 2016 at 09:41:36PM +0100, Michał Kępień wrote:
> > > On Monday 29 February 2016 21:22:54 Michał Kępień wrote:
> > > > > On Wednesday 24 February 2016 08:20:11 Michał Kępień wrote:
> > > > > > The dell_smi_error() method could be used by modules other than
> > > > > > dell-laptop for convenient translation of SMBIOS request errors
> > > > > > into errno values.  Thus, move it to dell-smbios.
> > > > > > 
> > > > > > Signed-off-by: Michał Kępień <kernel@kempniu.pl>
> > > > > > ---
> > > > > > 
> > > > > >  drivers/platform/x86/dell-laptop.c |   14 --------------
> > > > > >  drivers/platform/x86/dell-smbios.c |   16 ++++++++++++++++
> > > > > >  drivers/platform/x86/dell-smbios.h |    2 ++
> > > > > >  3 files changed, 18 insertions(+), 14 deletions(-)
> > > > > > 
> > > > > > diff --git a/drivers/platform/x86/dell-laptop.c
> > > > > > b/drivers/platform/x86/dell-laptop.c index 76064c8..cbafb95
> > > > > > 100644
> > > > > > --- a/drivers/platform/x86/dell-laptop.c
> > > > > > +++ b/drivers/platform/x86/dell-laptop.c
> > > > > > @@ -273,20 +273,6 @@ static const struct dmi_system_id
> > > > > > dell_quirks[] __initconst = {
> > > > > > 
> > > > > >  	{ }
> > > > > >  
> > > > > >  };
> > > > > > 
> > > > > > -static inline int dell_smi_error(int value)
> > > > > > -{
> > > > > > -	switch (value) {
> > > > > > -	case 0: /* Completed successfully */
> > > > > > -		return 0;
> > > > > > -	case -1: /* Completed with error */
> > > > > > -		return -EIO;
> > > > > > -	case -2: /* Function not supported */
> > > > > > -		return -ENXIO;
> > > > > > -	default: /* Unknown error */
> > > > > > -		return -EINVAL;
> > > > > > -	}
> > > > > > -}
> > > > > > -
> > > > > > 
> > > > > >  /*
> > > > > >  
> > > > > >   * Derived from information in smbios-wireless-ctl:
> > > > > >   *
> > > > > > 
> > > > > > diff --git a/drivers/platform/x86/dell-smbios.c
> > > > > > b/drivers/platform/x86/dell-smbios.c index 2a4992a..942572f
> > > > > > 100644
> > > > > > --- a/drivers/platform/x86/dell-smbios.c
> > > > > > +++ b/drivers/platform/x86/dell-smbios.c
> > > > > > @@ -16,6 +16,7 @@
> > > > > > 
> > > > > >  #include <linux/kernel.h>
> > > > > >  #include <linux/module.h>
> > > > > >  #include <linux/dmi.h>
> > > > > > 
> > > > > > +#include <linux/err.h>
> > > > > > 
> > > > > >  #include <linux/gfp.h>
> > > > > >  #include <linux/mutex.h>
> > > > > >  #include <linux/slab.h>
> > > > > > 
> > > > > > @@ -39,6 +40,21 @@ static int da_command_code;
> > > > > > 
> > > > > >  static int da_num_tokens;
> > > > > >  static struct calling_interface_token *da_tokens;
> > > > > > 
> > > > > > +int dell_smi_error(int value)
> > > > > > +{
> > > > > > +	switch (value) {
> > > > > > +	case 0: /* Completed successfully */
> > > > > > +		return 0;
> > > > > > +	case -1: /* Completed with error */
> > > > > > +		return -EIO;
> > > > > > +	case -2: /* Function not supported */
> > > > > > +		return -ENXIO;
> > > > > > +	default: /* Unknown error */
> > > > > > +		return -EINVAL;
> > > > > > +	}
> > > > > > +}
> > > > > > +EXPORT_SYMBOL_GPL(dell_smi_error);
> > > > > > +
> > > > > > 
> > > > > >  struct calling_interface_buffer *dell_smbios_get_buffer(void)
> > > > > >  {
> > > > > >  
> > > > > >  	mutex_lock(&buffer_mutex);
> > > > > > 
> > > > > > diff --git a/drivers/platform/x86/dell-smbios.h
> > > > > > b/drivers/platform/x86/dell-smbios.h index 4f69b16..52febe6
> > > > > > 100644
> > > > > > --- a/drivers/platform/x86/dell-smbios.h
> > > > > > +++ b/drivers/platform/x86/dell-smbios.h
> > > > > > @@ -35,6 +35,8 @@ struct calling_interface_token {
> > > > > > 
> > > > > >  	};
> > > > > >  
> > > > > >  };
> > > > > > 
> > > > > > +int dell_smi_error(int value);
> > > > > > +
> > > > > > 
> > > > > >  struct calling_interface_buffer *dell_smbios_get_buffer(void);
> > > > > >  void dell_smbios_clear_buffer(void);
> > > > > >  void dell_smbios_release_buffer(void);
> > > > > 
> > > > > And... here what about inline vs EXPORT_SYMBOL function? Just
> > > > > asking...
> > > > 
> > > > Well, what about it? :)  The commit message is pretty explicit in
> > > > describing what happens here, i.e. a previously static function is
> > > > moved to another module so that it can be reused.  Thus, keeping the
> > > > inline keyword makes no sense.  What exactly is your concern?
> > > 
> > > Just asking if this function should be or not be inline (of course in 
> > > header file, not in module .c).
> > 
> > If you mark a function as inline in the header file, you have to provide
> > its definition, otherwise you'll get a compilation error.  Given that
> > this is in no way performance-critical code, I see no point in
> > clobbering the header file with the body of this function.
> 
> Agreed, please leave it as is.

Pali,

Given Darren's remark, are you okay with acking this patch?

-- 
Best regards,
Michał Kępień

[toc] | [prev] | [next] | [standalone]

#1349004 — Re: [PATCH v4 1/5] dell-laptop: move dell_smi_error() to dell-smbios

FromPali Rohár <pali.rohar@gmail.com>
Date2016-03-03 12:40 +0100
SubjectRe: [PATCH v4 1/5] dell-laptop: move dell_smi_error() to dell-smbios
Message-ID<r8zLQ-6rX-5@gated-at.bofh.it>
In reply to#1347977
On Wednesday 02 March 2016 12:49:39 Michał Kępień wrote:
> > On Mon, Feb 29, 2016 at 09:41:36PM +0100, Michał Kępień wrote:
> > > > On Monday 29 February 2016 21:22:54 Michał Kępień wrote:
> > > > > > On Wednesday 24 February 2016 08:20:11 Michał Kępień wrote:
> > > > > > > The dell_smi_error() method could be used by modules other than
> > > > > > > dell-laptop for convenient translation of SMBIOS request errors
> > > > > > > into errno values.  Thus, move it to dell-smbios.
> > > > > > > 
> > > > > > > Signed-off-by: Michał Kępień <kernel@kempniu.pl>
> > > > > > > ---
> > > > > > > 
> > > > > > >  drivers/platform/x86/dell-laptop.c |   14 --------------
> > > > > > >  drivers/platform/x86/dell-smbios.c |   16 ++++++++++++++++
> > > > > > >  drivers/platform/x86/dell-smbios.h |    2 ++
> > > > > > >  3 files changed, 18 insertions(+), 14 deletions(-)
> > > > > > > 
> > > > > > > diff --git a/drivers/platform/x86/dell-laptop.c
> > > > > > > b/drivers/platform/x86/dell-laptop.c index 76064c8..cbafb95
> > > > > > > 100644
> > > > > > > --- a/drivers/platform/x86/dell-laptop.c
> > > > > > > +++ b/drivers/platform/x86/dell-laptop.c
> > > > > > > @@ -273,20 +273,6 @@ static const struct dmi_system_id
> > > > > > > dell_quirks[] __initconst = {
> > > > > > > 
> > > > > > >  	{ }
> > > > > > >  
> > > > > > >  };
> > > > > > > 
> > > > > > > -static inline int dell_smi_error(int value)
> > > > > > > -{
> > > > > > > -	switch (value) {
> > > > > > > -	case 0: /* Completed successfully */
> > > > > > > -		return 0;
> > > > > > > -	case -1: /* Completed with error */
> > > > > > > -		return -EIO;
> > > > > > > -	case -2: /* Function not supported */
> > > > > > > -		return -ENXIO;
> > > > > > > -	default: /* Unknown error */
> > > > > > > -		return -EINVAL;
> > > > > > > -	}
> > > > > > > -}
> > > > > > > -
> > > > > > > 
> > > > > > >  /*
> > > > > > >  
> > > > > > >   * Derived from information in smbios-wireless-ctl:
> > > > > > >   *
> > > > > > > 
> > > > > > > diff --git a/drivers/platform/x86/dell-smbios.c
> > > > > > > b/drivers/platform/x86/dell-smbios.c index 2a4992a..942572f
> > > > > > > 100644
> > > > > > > --- a/drivers/platform/x86/dell-smbios.c
> > > > > > > +++ b/drivers/platform/x86/dell-smbios.c
> > > > > > > @@ -16,6 +16,7 @@
> > > > > > > 
> > > > > > >  #include <linux/kernel.h>
> > > > > > >  #include <linux/module.h>
> > > > > > >  #include <linux/dmi.h>
> > > > > > > 
> > > > > > > +#include <linux/err.h>
> > > > > > > 
> > > > > > >  #include <linux/gfp.h>
> > > > > > >  #include <linux/mutex.h>
> > > > > > >  #include <linux/slab.h>
> > > > > > > 
> > > > > > > @@ -39,6 +40,21 @@ static int da_command_code;
> > > > > > > 
> > > > > > >  static int da_num_tokens;
> > > > > > >  static struct calling_interface_token *da_tokens;
> > > > > > > 
> > > > > > > +int dell_smi_error(int value)
> > > > > > > +{
> > > > > > > +	switch (value) {
> > > > > > > +	case 0: /* Completed successfully */
> > > > > > > +		return 0;
> > > > > > > +	case -1: /* Completed with error */
> > > > > > > +		return -EIO;
> > > > > > > +	case -2: /* Function not supported */
> > > > > > > +		return -ENXIO;
> > > > > > > +	default: /* Unknown error */
> > > > > > > +		return -EINVAL;
> > > > > > > +	}
> > > > > > > +}
> > > > > > > +EXPORT_SYMBOL_GPL(dell_smi_error);
> > > > > > > +
> > > > > > > 
> > > > > > >  struct calling_interface_buffer *dell_smbios_get_buffer(void)
> > > > > > >  {
> > > > > > >  
> > > > > > >  	mutex_lock(&buffer_mutex);
> > > > > > > 
> > > > > > > diff --git a/drivers/platform/x86/dell-smbios.h
> > > > > > > b/drivers/platform/x86/dell-smbios.h index 4f69b16..52febe6
> > > > > > > 100644
> > > > > > > --- a/drivers/platform/x86/dell-smbios.h
> > > > > > > +++ b/drivers/platform/x86/dell-smbios.h
> > > > > > > @@ -35,6 +35,8 @@ struct calling_interface_token {
> > > > > > > 
> > > > > > >  	};
> > > > > > >  
> > > > > > >  };
> > > > > > > 
> > > > > > > +int dell_smi_error(int value);
> > > > > > > +
> > > > > > > 
> > > > > > >  struct calling_interface_buffer *dell_smbios_get_buffer(void);
> > > > > > >  void dell_smbios_clear_buffer(void);
> > > > > > >  void dell_smbios_release_buffer(void);
> > > > > > 
> > > > > > And... here what about inline vs EXPORT_SYMBOL function? Just
> > > > > > asking...
> > > > > 
> > > > > Well, what about it? :)  The commit message is pretty explicit in
> > > > > describing what happens here, i.e. a previously static function is
> > > > > moved to another module so that it can be reused.  Thus, keeping the
> > > > > inline keyword makes no sense.  What exactly is your concern?
> > > > 
> > > > Just asking if this function should be or not be inline (of course in 
> > > > header file, not in module .c).
> > > 
> > > If you mark a function as inline in the header file, you have to provide
> > > its definition, otherwise you'll get a compilation error.  Given that
> > > this is in no way performance-critical code, I see no point in
> > > clobbering the header file with the body of this function.
> > 
> > Agreed, please leave it as is.
> 
> Pali,
> 
> Given Darren's remark, are you okay with acking this patch?

I'm just thinking if such function should not be macro as it translate 3
error codes from firmware to standard errno... As external function
exported by EXPORT_SYMBOL looks for me as overkill...

Patch is anyway OK, so add my Reviewed-by.

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]

#1341615 — [PATCH v4 5/5] dell-wmi: support Dell Inspiron M5110

FromMichał Kępień <kernel@kempniu.pl>
Date2016-02-24 08:20 +0100
Subject[PATCH v4 5/5] dell-wmi: support Dell Inspiron M5110
Message-ID<r5BTP-11s-3@gated-at.bofh.it>
In reply to#1341613
Similarly to Dell Vostro V131, Dell Inspiron M5110 also requires an
SMBIOS request to be issued in order for WMI events to be generated and
does not raise an i8042 interrupt when the Dell Instant Launch hotkey is
pressed.  However, the event code for that hotkey on this machine is
0xe029, so add it to the legacy keymap.

Signed-off-by: Michał Kępień <kernel@kempniu.pl>
Tested-by: Darek Stojaczyk <darek.stojaczyk@gmail.com>
---
 drivers/platform/x86/dell-wmi.c |   13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c
index ffc957b5..b1479ab 100644
--- a/drivers/platform/x86/dell-wmi.c
+++ b/drivers/platform/x86/dell-wmi.c
@@ -62,6 +62,14 @@ static int __init dmi_matched(const struct dmi_system_id *dmi)
 static const struct dmi_system_id dell_wmi_smbios_list[] __initconst = {
 	{
 		.callback = dmi_matched,
+		.ident = "Dell Inspiron M5110",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron M5110"),
+		},
+	},
+	{
+		.callback = dmi_matched,
 		.ident = "Dell Vostro V131",
 		.matches = {
 			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
@@ -110,8 +118,11 @@ static const struct key_entry dell_wmi_legacy_keymap[] __initconst = {
 
 	{ KE_IGNORE, 0xe020, { KEY_MUTE } },
 
-	/* Shortcut and audio panel keys */
+	/* Dell Instant Launch key */
 	{ KE_KEY, 0xe025, { KEY_PROG4 } },
+	{ KE_KEY, 0xe029, { KEY_PROG4 } },
+
+	/* Audio panel key */
 	{ KE_IGNORE, 0xe026, { KEY_RESERVED } },
 
 	{ KE_IGNORE, 0xe02e, { KEY_VOLUMEDOWN } },
-- 
1.7.10.4

[toc] | [prev] | [next] | [standalone]

#1345781 — Re: [PATCH v4 5/5] dell-wmi: support Dell Inspiron M5110

FromPali Rohár <pali.rohar@gmail.com>
Date2016-02-29 14:10 +0100
SubjectRe: [PATCH v4 5/5] dell-wmi: support Dell Inspiron M5110
Message-ID<r7vKh-3rD-5@gated-at.bofh.it>
In reply to#1341615
On Wednesday 24 February 2016 08:20:15 Michał Kępień wrote:
> Similarly to Dell Vostro V131, Dell Inspiron M5110 also requires an
> SMBIOS request to be issued in order for WMI events to be generated and
> does not raise an i8042 interrupt when the Dell Instant Launch hotkey is
> pressed.  However, the event code for that hotkey on this machine is
> 0xe029, so add it to the legacy keymap.
> 
> Signed-off-by: Michał Kępień <kernel@kempniu.pl>
> Tested-by: Darek Stojaczyk <darek.stojaczyk@gmail.com>
> ---
>  drivers/platform/x86/dell-wmi.c |   13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c
> index ffc957b5..b1479ab 100644
> --- a/drivers/platform/x86/dell-wmi.c
> +++ b/drivers/platform/x86/dell-wmi.c
> @@ -62,6 +62,14 @@ static int __init dmi_matched(const struct dmi_system_id *dmi)
>  static const struct dmi_system_id dell_wmi_smbios_list[] __initconst = {
>  	{
>  		.callback = dmi_matched,
> +		.ident = "Dell Inspiron M5110",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron M5110"),
> +		},
> +	},
> +	{
> +		.callback = dmi_matched,
>  		.ident = "Dell Vostro V131",
>  		.matches = {
>  			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
> @@ -110,8 +118,11 @@ static const struct key_entry dell_wmi_legacy_keymap[] __initconst = {
>  
>  	{ KE_IGNORE, 0xe020, { KEY_MUTE } },
>  
> -	/* Shortcut and audio panel keys */
> +	/* Dell Instant Launch key */
>  	{ KE_KEY, 0xe025, { KEY_PROG4 } },
> +	{ KE_KEY, 0xe029, { KEY_PROG4 } },
> +
> +	/* Audio panel key */
>  	{ KE_IGNORE, 0xe026, { KEY_RESERVED } },
>  
>  	{ KE_IGNORE, 0xe02e, { KEY_VOLUMEDOWN } },

That's better, add my Reviewed-by.

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]

#1341616 — [PATCH v4 3/5] dell-wmi: enable receiving WMI events on Dell Vostro V131

FromMichał Kępień <kernel@kempniu.pl>
Date2016-02-24 08:20 +0100
Subject[PATCH v4 3/5] dell-wmi: enable receiving WMI events on Dell Vostro V131
Message-ID<r5BTQ-11s-9@gated-at.bofh.it>
In reply to#1341613
On some laptop models (e.g. Dell Vostro V131), WMI events are not
generated until a specific SMBIOS request is issued to register an event
listener [1].  As there seems to be no ACPI method or SMBIOS request to
determine without possible side effects whether a given machine needs to
issue this SMBIOS request in order to receive WMI events, DMI matching
is used to whitelist the models which need it.

[1] https://lists.us.dell.com/pipermail/libsmbios-devel/2015-July/000612.html

Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
 drivers/platform/x86/Kconfig    |    1 +
 drivers/platform/x86/dell-wmi.c |   66 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 659e13b..176c666 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -123,6 +123,7 @@ config DELL_WMI
 	depends on ACPI_WMI
 	depends on INPUT
 	depends on ACPI_VIDEO || ACPI_VIDEO = n
+	depends on DELL_SMBIOS
 	select INPUT_SPARSEKMAP
 	select DMI
 	---help---
diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c
index e38258a..65edd93 100644
--- a/drivers/platform/x86/dell-wmi.c
+++ b/drivers/platform/x86/dell-wmi.c
@@ -37,6 +37,7 @@
 #include <linux/string.h>
 #include <linux/dmi.h>
 #include <acpi/video.h>
+#include "dell-smbios.h"
 
 MODULE_AUTHOR("Matthew Garrett <mjg@redhat.com>");
 MODULE_AUTHOR("Pali Rohár <pali.rohar@gmail.com>");
@@ -47,10 +48,29 @@ MODULE_LICENSE("GPL");
 #define DELL_DESCRIPTOR_GUID "8D9DDCBC-A997-11DA-B012-B622A1EF5492"
 
 static u32 dell_wmi_interface_version;
+static bool wmi_requires_smbios_request;
 
 MODULE_ALIAS("wmi:"DELL_EVENT_GUID);
 MODULE_ALIAS("wmi:"DELL_DESCRIPTOR_GUID);
 
+static int __init dmi_matched(const struct dmi_system_id *dmi)
+{
+	wmi_requires_smbios_request = 1;
+	return 1;
+}
+
+static const struct dmi_system_id dell_wmi_smbios_list[] __initconst = {
+	{
+		.callback = dmi_matched,
+		.ident = "Dell Vostro V131",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Vostro V131"),
+		},
+	},
+	{ }
+};
+
 /*
  * Certain keys are flagged as KE_IGNORE. All of these are either
  * notifications (rather than requests for change) or are also sent
@@ -597,6 +617,38 @@ static int __init dell_wmi_check_descriptor_buffer(void)
 	return 0;
 }
 
+/*
+ * According to Dell SMBIOS documentation:
+ *
+ * 17  3  Application Program Registration
+ *
+ *     cbArg1 Application ID 1 = 0x00010000
+ *     cbArg2 Application ID 2
+ *            QUICKSET/DCP = 0x51534554 "QSET"
+ *            ALS Driver   = 0x416c7353 "AlsS"
+ *            Latitude ON  = 0x4c6f6e52 "LonR"
+ *     cbArg3 Application version or revision number
+ *     cbArg4 0 = Unregister application
+ *            1 = Register application
+ *     cbRes1 Standard return codes (0, -1, -2)
+ */
+
+static int dell_wmi_events_set_enabled(bool enable)
+{
+	struct calling_interface_buffer *buffer;
+	int ret;
+
+	buffer = dell_smbios_get_buffer();
+	buffer->input[0] = 0x10000;
+	buffer->input[1] = 0x51534554;
+	buffer->input[3] = enable;
+	dell_smbios_send_request(17, 3);
+	ret = buffer->output[0];
+	dell_smbios_release_buffer();
+
+	return dell_smbios_error(ret);
+}
+
 static int __init dell_wmi_init(void)
 {
 	int err;
@@ -624,12 +676,26 @@ static int __init dell_wmi_init(void)
 		return -ENODEV;
 	}
 
+	dmi_check_system(dell_wmi_smbios_list);
+
+	if (wmi_requires_smbios_request) {
+		err = dell_wmi_events_set_enabled(true);
+		if (err) {
+			pr_err("Failed to enable WMI events\n");
+			wmi_remove_notify_handler(DELL_EVENT_GUID);
+			dell_wmi_input_destroy();
+			return err;
+		}
+	}
+
 	return 0;
 }
 module_init(dell_wmi_init);
 
 static void __exit dell_wmi_exit(void)
 {
+	if (wmi_requires_smbios_request)
+		dell_wmi_events_set_enabled(false);
 	wmi_remove_notify_handler(DELL_EVENT_GUID);
 	dell_wmi_input_destroy();
 }
-- 
1.7.10.4

[toc] | [prev] | [next] | [standalone]

#1345771 — Re: [PATCH v4 3/5] dell-wmi: enable receiving WMI events on Dell Vostro V131

FromPali Rohár <pali.rohar@gmail.com>
Date2016-02-29 14:00 +0100
SubjectRe: [PATCH v4 3/5] dell-wmi: enable receiving WMI events on Dell Vostro V131
Message-ID<r7vAC-38v-13@gated-at.bofh.it>
In reply to#1341616
On Wednesday 24 February 2016 08:20:13 Michał Kępień wrote:
> On some laptop models (e.g. Dell Vostro V131), WMI events are not
> generated until a specific SMBIOS request is issued to register an event
> listener [1].  As there seems to be no ACPI method or SMBIOS request to
> determine without possible side effects whether a given machine needs to
> issue this SMBIOS request in order to receive WMI events, DMI matching
> is used to whitelist the models which need it.
> 
> [1] https://lists.us.dell.com/pipermail/libsmbios-devel/2015-July/000612.html
> 
> Signed-off-by: Michał Kępień <kernel@kempniu.pl>

Reviewed-by: Pali Rohár <pali.rohar@gmail.com>

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]

#1341619 — [PATCH v4 2/5] dell-smbios: rename dell_smi_error() to dell_smbios_error()

FromMichał Kępień <kernel@kempniu.pl>
Date2016-02-24 08:30 +0100
Subject[PATCH v4 2/5] dell-smbios: rename dell_smi_error() to dell_smbios_error()
Message-ID<r5C3w-16C-13@gated-at.bofh.it>
In reply to#1341613
As dell_smi_error() is exported by dell-smbios, its prefix should be
consistent with other exported symbols, so change function name to
dell_smbios_error().

Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
 drivers/platform/x86/dell-laptop.c |   16 ++++++++--------
 drivers/platform/x86/dell-smbios.c |    4 ++--
 drivers/platform/x86/dell-smbios.h |    2 +-
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index cbafb95..2c2f02b 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -433,7 +433,7 @@ static int dell_rfkill_set(void *data, bool blocked)
 
  out:
 	dell_smbios_release_buffer();
-	return dell_smi_error(ret);
+	return dell_smbios_error(ret);
 }
 
 /* Must be called with the buffer held */
@@ -876,7 +876,7 @@ static int dell_send_intensity(struct backlight_device *bd)
 	else
 		dell_smbios_send_request(1, 1);
 
-	ret = dell_smi_error(buffer->output[0]);
+	ret = dell_smbios_error(buffer->output[0]);
 
 	dell_smbios_release_buffer();
 	return ret;
@@ -901,7 +901,7 @@ static int dell_get_intensity(struct backlight_device *bd)
 		dell_smbios_send_request(0, 1);
 
 	if (buffer->output[0])
-		ret = dell_smi_error(buffer->output[0]);
+		ret = dell_smbios_error(buffer->output[0]);
 	else
 		ret = buffer->output[1];
 
@@ -1160,7 +1160,7 @@ static int kbd_get_info(struct kbd_info *info)
 	ret = buffer->output[0];
 
 	if (ret) {
-		ret = dell_smi_error(ret);
+		ret = dell_smbios_error(ret);
 		goto out;
 	}
 
@@ -1249,7 +1249,7 @@ static int kbd_get_state(struct kbd_state *state)
 	ret = buffer->output[0];
 
 	if (ret) {
-		ret = dell_smi_error(ret);
+		ret = dell_smbios_error(ret);
 		goto out;
 	}
 
@@ -1286,7 +1286,7 @@ static int kbd_set_state(struct kbd_state *state)
 	ret = buffer->output[0];
 	dell_smbios_release_buffer();
 
-	return dell_smi_error(ret);
+	return dell_smbios_error(ret);
 }
 
 static int kbd_set_state_safe(struct kbd_state *state, struct kbd_state *old)
@@ -1329,7 +1329,7 @@ static int kbd_set_token_bit(u8 bit)
 	ret = buffer->output[0];
 	dell_smbios_release_buffer();
 
-	return dell_smi_error(ret);
+	return dell_smbios_error(ret);
 }
 
 static int kbd_get_token_bit(u8 bit)
@@ -1354,7 +1354,7 @@ static int kbd_get_token_bit(u8 bit)
 	dell_smbios_release_buffer();
 
 	if (ret)
-		return dell_smi_error(ret);
+		return dell_smbios_error(ret);
 
 	return (val == token->value);
 }
diff --git a/drivers/platform/x86/dell-smbios.c b/drivers/platform/x86/dell-smbios.c
index 942572f..d2412ab 100644
--- a/drivers/platform/x86/dell-smbios.c
+++ b/drivers/platform/x86/dell-smbios.c
@@ -40,7 +40,7 @@ static int da_command_code;
 static int da_num_tokens;
 static struct calling_interface_token *da_tokens;
 
-int dell_smi_error(int value)
+int dell_smbios_error(int value)
 {
 	switch (value) {
 	case 0: /* Completed successfully */
@@ -53,7 +53,7 @@ int dell_smi_error(int value)
 		return -EINVAL;
 	}
 }
-EXPORT_SYMBOL_GPL(dell_smi_error);
+EXPORT_SYMBOL_GPL(dell_smbios_error);
 
 struct calling_interface_buffer *dell_smbios_get_buffer(void)
 {
diff --git a/drivers/platform/x86/dell-smbios.h b/drivers/platform/x86/dell-smbios.h
index 52febe6..ec7d40a 100644
--- a/drivers/platform/x86/dell-smbios.h
+++ b/drivers/platform/x86/dell-smbios.h
@@ -35,7 +35,7 @@ struct calling_interface_token {
 	};
 };
 
-int dell_smi_error(int value);
+int dell_smbios_error(int value);
 
 struct calling_interface_buffer *dell_smbios_get_buffer(void);
 void dell_smbios_clear_buffer(void);
-- 
1.7.10.4

[toc] | [prev] | [next] | [standalone]

#1345768 — Re: [PATCH v4 2/5] dell-smbios: rename dell_smi_error() to dell_smbios_error()

FromPali Rohár <pali.rohar@gmail.com>
Date2016-02-29 14:00 +0100
SubjectRe: [PATCH v4 2/5] dell-smbios: rename dell_smi_error() to dell_smbios_error()
Message-ID<r7vAB-38v-1@gated-at.bofh.it>
In reply to#1341619
On Wednesday 24 February 2016 08:20:12 Michał Kępień wrote:
> As dell_smi_error() is exported by dell-smbios, its prefix should be
> consistent with other exported symbols, so change function name to
> dell_smbios_error().
> 
> Signed-off-by: Michał Kępień <kernel@kempniu.pl>

Acked-by: Pali Rohár <pali.rohar@gmail.com>

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]

#1341622 — [PATCH v4 4/5] dell-wmi: properly process Dell Instant Launch hotkey

FromMichał Kępień <kernel@kempniu.pl>
Date2016-02-24 08:30 +0100
Subject[PATCH v4 4/5] dell-wmi: properly process Dell Instant Launch hotkey
Message-ID<r5C3x-16C-25@gated-at.bofh.it>
In reply to#1341613
On models on which an SMBIOS request needs to be issued in order for WMI
events to be generated, pressing the Dell Instant Launch hotkey does not
raise an i8042 interrupt - only a WMI event is generated (0xe025 on Dell
Vostro V131).  Thus, the 0xe025 event should only be ignored on machines
which do not require an SMBIOS request for enabling WMI.

Signed-off-by: Michał Kępień <kernel@kempniu.pl>
---
 drivers/platform/x86/dell-wmi.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c
index 65edd93..ffc957b5 100644
--- a/drivers/platform/x86/dell-wmi.c
+++ b/drivers/platform/x86/dell-wmi.c
@@ -111,7 +111,7 @@ static const struct key_entry dell_wmi_legacy_keymap[] __initconst = {
 	{ KE_IGNORE, 0xe020, { KEY_MUTE } },
 
 	/* Shortcut and audio panel keys */
-	{ KE_IGNORE, 0xe025, { KEY_RESERVED } },
+	{ KE_KEY, 0xe025, { KEY_PROG4 } },
 	{ KE_IGNORE, 0xe026, { KEY_RESERVED } },
 
 	{ KE_IGNORE, 0xe02e, { KEY_VOLUMEDOWN } },
@@ -235,6 +235,9 @@ static void dell_wmi_process_key(int reported_key)
 	    acpi_video_handles_brightness_key_presses())
 		return;
 
+	if (key->keycode == KEY_PROG4 && !wmi_requires_smbios_request)
+		return;
+
 	sparse_keymap_report_entry(dell_wmi_input_dev, key, 1, true);
 }
 
-- 
1.7.10.4

[toc] | [prev] | [next] | [standalone]

#1345791 — Re: [PATCH v4 4/5] dell-wmi: properly process Dell Instant Launch hotkey

FromPali Rohár <pali.rohar@gmail.com>
Date2016-02-29 14:10 +0100
SubjectRe: [PATCH v4 4/5] dell-wmi: properly process Dell Instant Launch hotkey
Message-ID<r7vKk-3rD-31@gated-at.bofh.it>
In reply to#1341622
On Wednesday 24 February 2016 08:20:14 Michał Kępień wrote:
> On models on which an SMBIOS request needs to be issued in order for WMI
> events to be generated, pressing the Dell Instant Launch hotkey does not
> raise an i8042 interrupt - only a WMI event is generated (0xe025 on Dell
> Vostro V131).  Thus, the 0xe025 event should only be ignored on machines
> which do not require an SMBIOS request for enabling WMI.
> 
> Signed-off-by: Michał Kępień <kernel@kempniu.pl>
> ---
>  drivers/platform/x86/dell-wmi.c |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c
> index 65edd93..ffc957b5 100644
> --- a/drivers/platform/x86/dell-wmi.c
> +++ b/drivers/platform/x86/dell-wmi.c
> @@ -111,7 +111,7 @@ static const struct key_entry dell_wmi_legacy_keymap[] __initconst = {
>  	{ KE_IGNORE, 0xe020, { KEY_MUTE } },
>  
>  	/* Shortcut and audio panel keys */
> -	{ KE_IGNORE, 0xe025, { KEY_RESERVED } },
> +	{ KE_KEY, 0xe025, { KEY_PROG4 } },
>  	{ KE_IGNORE, 0xe026, { KEY_RESERVED } },
>  
>  	{ KE_IGNORE, 0xe02e, { KEY_VOLUMEDOWN } },
> @@ -235,6 +235,9 @@ static void dell_wmi_process_key(int reported_key)
>  	    acpi_video_handles_brightness_key_presses())
>  		return;
>  
> +	if (key->keycode == KEY_PROG4 && !wmi_requires_smbios_request)
> +		return;
> +

Here I would rather test against reported_key, not keycode. If somebody
in future adds KEY_PROG4 for something else we will have problem...

>  	sparse_keymap_report_entry(dell_wmi_input_dev, key, 1, true);
>  }
>  

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]

#1346146 — Re: [PATCH v4 4/5] dell-wmi: properly process Dell Instant Launch hotkey

FromMichał Kępień <kernel@kempniu.pl>
Date2016-02-29 21:40 +0100
SubjectRe: [PATCH v4 4/5] dell-wmi: properly process Dell Instant Launch hotkey
Message-ID<r7CLM-7QV-5@gated-at.bofh.it>
In reply to#1345791
> > diff --git a/drivers/platform/x86/dell-wmi.c b/drivers/platform/x86/dell-wmi.c
> > index 65edd93..ffc957b5 100644
> > --- a/drivers/platform/x86/dell-wmi.c
> > +++ b/drivers/platform/x86/dell-wmi.c
> > @@ -111,7 +111,7 @@ static const struct key_entry dell_wmi_legacy_keymap[] __initconst = {
> >  	{ KE_IGNORE, 0xe020, { KEY_MUTE } },
> >  
> >  	/* Shortcut and audio panel keys */
> > -	{ KE_IGNORE, 0xe025, { KEY_RESERVED } },
> > +	{ KE_KEY, 0xe025, { KEY_PROG4 } },
> >  	{ KE_IGNORE, 0xe026, { KEY_RESERVED } },
> >  
> >  	{ KE_IGNORE, 0xe02e, { KEY_VOLUMEDOWN } },
> > @@ -235,6 +235,9 @@ static void dell_wmi_process_key(int reported_key)
> >  	    acpi_video_handles_brightness_key_presses())
> >  		return;
> >  
> > +	if (key->keycode == KEY_PROG4 && !wmi_requires_smbios_request)
> > +		return;
> > +
> 
> Here I would rather test against reported_key, not keycode. If somebody
> in future adds KEY_PROG4 for something else we will have problem...

As 0xe025 is currently the only event we know about that should be
ignored on some machines and processed on others, this makes sense, at
least for now.  If I change the first condition to:

    reported_key == 0xe025

will you be okay with adding your Reviewed-by for this patch?  Then, for
Darren's convenience, I could post a v5 of the whole series with the
above change and all your Acked-by and Reviewed-by tags added.

-- 
Best regards,
Michał Kępień

[toc] | [prev] | [next] | [standalone]

#1346148 — Re: [PATCH v4 4/5] dell-wmi: properly process Dell Instant Launch hotkey

FromPali Rohár <pali.rohar@gmail.com>
Date2016-02-29 21:40 +0100
SubjectRe: [PATCH v4 4/5] dell-wmi: properly process Dell Instant Launch hotkey
Message-ID<r7CLM-7QV-9@gated-at.bofh.it>
In reply to#1346146

[Multipart message — attachments visible in raw view] — view raw

On Monday 29 February 2016 21:31:23 Michał Kępień wrote:
> > > diff --git a/drivers/platform/x86/dell-wmi.c
> > > b/drivers/platform/x86/dell-wmi.c index 65edd93..ffc957b5 100644
> > > --- a/drivers/platform/x86/dell-wmi.c
> > > +++ b/drivers/platform/x86/dell-wmi.c
> > > @@ -111,7 +111,7 @@ static const struct key_entry
> > > dell_wmi_legacy_keymap[] __initconst = {
> > > 
> > >  	{ KE_IGNORE, 0xe020, { KEY_MUTE } },
> > >  	
> > >  	/* Shortcut and audio panel keys */
> > > 
> > > -	{ KE_IGNORE, 0xe025, { KEY_RESERVED } },
> > > +	{ KE_KEY, 0xe025, { KEY_PROG4 } },
> > > 
> > >  	{ KE_IGNORE, 0xe026, { KEY_RESERVED } },
> > >  	
> > >  	{ KE_IGNORE, 0xe02e, { KEY_VOLUMEDOWN } },
> > > 
> > > @@ -235,6 +235,9 @@ static void dell_wmi_process_key(int
> > > reported_key)
> > > 
> > >  	    acpi_video_handles_brightness_key_presses())
> > >  		
> > >  		return;
> > > 
> > > +	if (key->keycode == KEY_PROG4 && !wmi_requires_smbios_request)
> > > +		return;
> > > +
> > 
> > Here I would rather test against reported_key, not keycode. If
> > somebody in future adds KEY_PROG4 for something else we will have
> > problem...
> 
> As 0xe025 is currently the only event we know about that should be
> ignored on some machines and processed on others, this makes sense,
> at least for now.  If I change the first condition to:
> 
>     reported_key == 0xe025

There will be need also change for 5/5 patch...

> will you be okay with adding your Reviewed-by for this patch?  Then,
> for Darren's convenience, I could post a v5 of the whole series with
> the above change and all your Acked-by and Reviewed-by tags added.

This is my suggestion as I'm thinking about future changes to this 
driver... But it is also on Darren as maintainer of platform x86 
subsystem.

Basically you do not want to check if pressed key is KEY_PROG4. But you 
want to check if dell wmi sent event 0xe025. This is what I say.

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]

#1346151 — Re: [PATCH v4 4/5] dell-wmi: properly process Dell Instant Launch hotkey

FromMichał Kępień <kernel@kempniu.pl>
Date2016-02-29 21:50 +0100
SubjectRe: [PATCH v4 4/5] dell-wmi: properly process Dell Instant Launch hotkey
Message-ID<r7CVr-7Vn-7@gated-at.bofh.it>
In reply to#1346148
> On Monday 29 February 2016 21:31:23 Michał Kępień wrote:
> > > > diff --git a/drivers/platform/x86/dell-wmi.c
> > > > b/drivers/platform/x86/dell-wmi.c index 65edd93..ffc957b5 100644
> > > > --- a/drivers/platform/x86/dell-wmi.c
> > > > +++ b/drivers/platform/x86/dell-wmi.c
> > > > @@ -111,7 +111,7 @@ static const struct key_entry
> > > > dell_wmi_legacy_keymap[] __initconst = {
> > > > 
> > > >  	{ KE_IGNORE, 0xe020, { KEY_MUTE } },
> > > >  	
> > > >  	/* Shortcut and audio panel keys */
> > > > 
> > > > -	{ KE_IGNORE, 0xe025, { KEY_RESERVED } },
> > > > +	{ KE_KEY, 0xe025, { KEY_PROG4 } },
> > > > 
> > > >  	{ KE_IGNORE, 0xe026, { KEY_RESERVED } },
> > > >  	
> > > >  	{ KE_IGNORE, 0xe02e, { KEY_VOLUMEDOWN } },
> > > > 
> > > > @@ -235,6 +235,9 @@ static void dell_wmi_process_key(int
> > > > reported_key)
> > > > 
> > > >  	    acpi_video_handles_brightness_key_presses())
> > > >  		
> > > >  		return;
> > > > 
> > > > +	if (key->keycode == KEY_PROG4 && !wmi_requires_smbios_request)
> > > > +		return;
> > > > +
> > > 
> > > Here I would rather test against reported_key, not keycode. If
> > > somebody in future adds KEY_PROG4 for something else we will have
> > > problem...
> > 
> > As 0xe025 is currently the only event we know about that should be
> > ignored on some machines and processed on others, this makes sense,
> > at least for now.  If I change the first condition to:
> > 
> >     reported_key == 0xe025
> 
> There will be need also change for 5/5 patch...

Why?  Are you aware of any model which sends a 0xe029 WMI event _and_
generates an i8042 interrupt?  If not, WMI event 0xe029 should always be
turned into a key event, as per the keymap.

> > will you be okay with adding your Reviewed-by for this patch?  Then,
> > for Darren's convenience, I could post a v5 of the whole series with
> > the above change and all your Acked-by and Reviewed-by tags added.
> 
> This is my suggestion as I'm thinking about future changes to this 
> driver... But it is also on Darren as maintainer of platform x86 
> subsystem.
> 
> Basically you do not want to check if pressed key is KEY_PROG4. But you 
> want to check if dell wmi sent event 0xe025. This is what I say.

Well, FWIW, I agree with you and I'll be happy to make that change.  It
would make the code and the commit message more coherent.

-- 
Best regards,
Michał Kępień

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web