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


Groups > linux.kernel > #1449716 > unrolled thread

[PATCH 0/2] cros_ec: utilize cros_ec_cmd_xfer_status()

Started byBrian Norris <briannorris@chromium.org>
First post2016-07-25 20:20 +0200
Last post2016-07-25 21:50 +0200
Articles 5 — 4 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH 0/2] cros_ec: utilize cros_ec_cmd_xfer_status() Brian Norris <briannorris@chromium.org> - 2016-07-25 20:20 +0200
    [PATCH 2/2] Input: cros_ec_keyb - Fix usage of cros_ec_cmd_xfer() Brian Norris <briannorris@chromium.org> - 2016-07-25 20:20 +0200
      Re: [PATCH 2/2] Input: cros_ec_keyb - Fix usage of cros_ec_cmd_xfer() Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2016-07-25 20:30 +0200
      Re: [PATCH 2/2] Input: cros_ec_keyb - Fix usage of cros_ec_cmd_xfer() kbuild test robot <lkp@intel.com> - 2016-07-25 20:40 +0200
      Re: [PATCH 2/2] Input: cros_ec_keyb - Fix usage of cros_ec_cmd_xfer() Javier Martinez Canillas <javier@osg.samsung.com> - 2016-07-25 21:50 +0200

#1449716 — [PATCH 0/2] cros_ec: utilize cros_ec_cmd_xfer_status()

FromBrian Norris <briannorris@chromium.org>
Date2016-07-25 20:20 +0200
Subject[PATCH 0/2] cros_ec: utilize cros_ec_cmd_xfer_status()
Message-ID<rYSDU-7Sa-25@gated-at.bofh.it>
Hi all,

I was sorting through some out-of-tree patches to these drivers, and I realized
we should probably start making use of cros_ec_cmd_xfer_status() in these
drivers, now that Thierry has queued them up for v4.8; see:

  git://git.kernel.org/pub/scm/linux/kernel/git/thierry.reding/linux-pwm.git
  refs/heads/for-4.8/mfd

  9798ac6d32c1 mfd: cros_ec: Add cros_ec_cmd_xfer_status() helper

Thierry originally provided the above branch for Lee's sake, but I don't see
why (if these are deemed fixes worthy of v4.8) that can't be pulled by Wolfram
and/or Dmitry for their respective subsystems' patches.

Please review.

Regards,
Brian

Brian Norris (2):
  i2c: cros-ec-tunnel: Fix usage of cros_ec_cmd_xfer()
  Input: cros_ec_keyb - Fix usage of cros_ec_cmd_xfer()

 drivers/i2c/busses/i2c-cros-ec-tunnel.c | 2 +-
 drivers/input/keyboard/cros_ec_keyb.c   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

-- 
2.8.0.rc3.226.g39d4020

[toc] | [next] | [standalone]


#1449719 — [PATCH 2/2] Input: cros_ec_keyb - Fix usage of cros_ec_cmd_xfer()

FromBrian Norris <briannorris@chromium.org>
Date2016-07-25 20:20 +0200
Subject[PATCH 2/2] Input: cros_ec_keyb - Fix usage of cros_ec_cmd_xfer()
Message-ID<rYSDV-7Sa-45@gated-at.bofh.it>
In reply to#1449716
cros_ec_cmd_xfer returns success status if the command transport
completes successfully, but the execution result is incorrectly ignored.
In many cases, the execution result is assumed to be successful, leading
to ignored errors and operating on uninitialized data.

We've recently introduced the cros_ec_cmd_xfer_status() helper to avoid these
problems. Let's use it.

Signed-off-by: Brian Norris <briannorris@chromium.org>
---
 drivers/input/keyboard/cros_ec_keyb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
index b01966dc7eb3..6e48616a3a88 100644
--- a/drivers/input/keyboard/cros_ec_keyb.c
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -160,7 +160,7 @@ static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t *kb_state)
 	msg->insize = ckdev->cols;
 	msg->outsize = 0;
 
-	ret = cros_ec_cmd_xfer(ckdev->ec, msg);
+	ret = cros_ec_cmd_xfer_status(ckdev->ec, msg);
 	if (ret < 0) {
 		dev_err(ckdev->dev, "Error transferring EC message %d\n", ret);
 		goto exit;
-- 
2.8.0.rc3.226.g39d4020

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


#1449730 — Re: [PATCH 2/2] Input: cros_ec_keyb - Fix usage of cros_ec_cmd_xfer()

FromDmitry Torokhov <dmitry.torokhov@gmail.com>
Date2016-07-25 20:30 +0200
SubjectRe: [PATCH 2/2] Input: cros_ec_keyb - Fix usage of cros_ec_cmd_xfer()
Message-ID<rYSNA-7Vk-15@gated-at.bofh.it>
In reply to#1449719
On Mon, Jul 25, 2016 at 11:14:11AM -0700, Brian Norris wrote:
> cros_ec_cmd_xfer returns success status if the command transport
> completes successfully, but the execution result is incorrectly ignored.
> In many cases, the execution result is assumed to be successful, leading
> to ignored errors and operating on uninitialized data.
> 
> We've recently introduced the cros_ec_cmd_xfer_status() helper to avoid these
> problems. Let's use it.
> 
> Signed-off-by: Brian Norris <briannorris@chromium.org>

Instead of me pulling in pwm/mfd branch maybe Thierry can push through
his branch?

Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

> ---
>  drivers/input/keyboard/cros_ec_keyb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
> index b01966dc7eb3..6e48616a3a88 100644
> --- a/drivers/input/keyboard/cros_ec_keyb.c
> +++ b/drivers/input/keyboard/cros_ec_keyb.c
> @@ -160,7 +160,7 @@ static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t *kb_state)
>  	msg->insize = ckdev->cols;
>  	msg->outsize = 0;
>  
> -	ret = cros_ec_cmd_xfer(ckdev->ec, msg);
> +	ret = cros_ec_cmd_xfer_status(ckdev->ec, msg);
>  	if (ret < 0) {
>  		dev_err(ckdev->dev, "Error transferring EC message %d\n", ret);
>  		goto exit;
> -- 
> 2.8.0.rc3.226.g39d4020
> 

-- 
Dmitry

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


#1449738 — Re: [PATCH 2/2] Input: cros_ec_keyb - Fix usage of cros_ec_cmd_xfer()

Fromkbuild test robot <lkp@intel.com>
Date2016-07-25 20:40 +0200
SubjectRe: [PATCH 2/2] Input: cros_ec_keyb - Fix usage of cros_ec_cmd_xfer()
Message-ID<rYSXg-7Yy-27@gated-at.bofh.it>
In reply to#1449719

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

Hi,

[auto build test ERROR on wsa/i2c/for-next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Brian-Norris/cros_ec-utilize-cros_ec_cmd_xfer_status/20160726-021919
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
config: i386-randconfig-x011-201630 (attached as .config)
compiler: gcc-6 (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/input/keyboard/cros_ec_keyb.c: In function 'cros_ec_keyb_get_state':
>> drivers/input/keyboard/cros_ec_keyb.c:163:8: error: implicit declaration of function 'cros_ec_cmd_xfer_status' [-Werror=implicit-function-declaration]
     ret = cros_ec_cmd_xfer_status(ckdev->ec, msg);
           ^~~~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/cros_ec_cmd_xfer_status +163 drivers/input/keyboard/cros_ec_keyb.c

   157	
   158		msg->version = 0;
   159		msg->command = EC_CMD_MKBP_STATE;
   160		msg->insize = ckdev->cols;
   161		msg->outsize = 0;
   162	
 > 163		ret = cros_ec_cmd_xfer_status(ckdev->ec, msg);
   164		if (ret < 0) {
   165			dev_err(ckdev->dev, "Error transferring EC message %d\n", ret);
   166			goto exit;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

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


#1449777 — Re: [PATCH 2/2] Input: cros_ec_keyb - Fix usage of cros_ec_cmd_xfer()

FromJavier Martinez Canillas <javier@osg.samsung.com>
Date2016-07-25 21:50 +0200
SubjectRe: [PATCH 2/2] Input: cros_ec_keyb - Fix usage of cros_ec_cmd_xfer()
Message-ID<rYU30-ak-13@gated-at.bofh.it>
In reply to#1449719
Hello Brian,

On 07/25/2016 02:14 PM, Brian Norris wrote:
> cros_ec_cmd_xfer returns success status if the command transport
> completes successfully, but the execution result is incorrectly ignored.
> In many cases, the execution result is assumed to be successful, leading
> to ignored errors and operating on uninitialized data.
> 
> We've recently introduced the cros_ec_cmd_xfer_status() helper to avoid these
> problems. Let's use it.
> 
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---

Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

Best regards,
-- 
Javier Martinez Canillas
Open Source Group
Samsung Research America

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web