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

Groups > linux.kernel > #1449742 > unrolled thread

[PATCH] Input: cros_ec_keyb - cleanup use of dev

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

Back to article view | Back to linux.kernel

Contents

  [PATCH] Input: cros_ec_keyb - cleanup use of dev Brian Norris <briannorris@chromium.org> - 2016-07-25 20:50 +0200
    Re: [PATCH] Input: cros_ec_keyb - cleanup use of dev Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2016-07-25 22:40 +0200
      Re: [PATCH] Input: cros_ec_keyb - cleanup use of dev Brian Norris <briannorris@chromium.org> - 2016-07-25 22:50 +0200
        Re: [PATCH] Input: cros_ec_keyb - cleanup use of dev Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2016-07-25 23:00 +0200

#1449742 — [PATCH] Input: cros_ec_keyb - cleanup use of dev

FromBrian Norris <briannorris@chromium.org>
Date2016-07-25 20:50 +0200
Subject[PATCH] Input: cros_ec_keyb - cleanup use of dev
Message-ID<rYT6V-81R-7@gated-at.bofh.it>
From: Douglas Anderson <dianders@chromium.org>

In cros_ec_keyb we stored "dev" in "struct cros_ec_keyb", but this was
the EC's dev pointer and not the keyboard's.  Let's clean this up to
make it the keyboard's dev pointer.  This could be useful in future
patches but also has the nice effect of changing a few printouts to
include the name of the keyboard device instead of the EC device, which
seems more technically correct.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
 drivers/input/keyboard/cros_ec_keyb.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
index 6e48616a3a88..ab1b50152b86 100644
--- a/drivers/input/keyboard/cros_ec_keyb.c
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -186,7 +186,7 @@ static irqreturn_t cros_ec_keyb_irq(int irq, void *data)
 	if (ret >= 0)
 		cros_ec_keyb_process(ckdev, kb_state, ret);
 	else
-		dev_err(ec->dev, "failed to get keyboard state: %d\n", ret);
+		dev_err(ckdev->dev, "failed to get keyboard state: %d\n", ret);
 
 	return IRQ_HANDLED;
 }
@@ -236,7 +236,7 @@ static void cros_ec_keyb_compute_valid_keys(struct cros_ec_keyb *ckdev)
 static int cros_ec_keyb_probe(struct platform_device *pdev)
 {
 	struct cros_ec_device *ec = dev_get_drvdata(pdev->dev.parent);
-	struct device *dev = ec->dev;
+	struct device *dev = &pdev->dev;
 	struct cros_ec_keyb *ckdev;
 	struct input_dev *idev;
 	struct device_node *np;
@@ -246,23 +246,22 @@ static int cros_ec_keyb_probe(struct platform_device *pdev)
 	if (!np)
 		return -ENODEV;
 
-	ckdev = devm_kzalloc(&pdev->dev, sizeof(*ckdev), GFP_KERNEL);
+	ckdev = devm_kzalloc(dev, sizeof(*ckdev), GFP_KERNEL);
 	if (!ckdev)
 		return -ENOMEM;
-	err = matrix_keypad_parse_of_params(&pdev->dev, &ckdev->rows,
-					    &ckdev->cols);
+	err = matrix_keypad_parse_of_params(dev, &ckdev->rows, &ckdev->cols);
 	if (err)
 		return err;
 
-	ckdev->valid_keys = devm_kzalloc(&pdev->dev, ckdev->cols, GFP_KERNEL);
+	ckdev->valid_keys = devm_kzalloc(dev, ckdev->cols, GFP_KERNEL);
 	if (!ckdev->valid_keys)
 		return -ENOMEM;
 
-	ckdev->old_kb_state = devm_kzalloc(&pdev->dev, ckdev->cols, GFP_KERNEL);
+	ckdev->old_kb_state = devm_kzalloc(dev, ckdev->cols, GFP_KERNEL);
 	if (!ckdev->old_kb_state)
 		return -ENOMEM;
 
-	idev = devm_input_allocate_device(&pdev->dev);
+	idev = devm_input_allocate_device(dev);
 	if (!idev)
 		return -ENOMEM;
 
@@ -273,7 +272,7 @@ static int cros_ec_keyb_probe(struct platform_device *pdev)
 
 	ckdev->ec = ec;
 	ckdev->dev = dev;
-	dev_set_drvdata(&pdev->dev, ckdev);
+	dev_set_drvdata(dev, ckdev);
 
 	idev->name = CROS_EC_DEV_NAME;
 	idev->phys = ec->phys_name;
@@ -282,7 +281,7 @@ static int cros_ec_keyb_probe(struct platform_device *pdev)
 	idev->id.bustype = BUS_VIRTUAL;
 	idev->id.version = 1;
 	idev->id.product = 0;
-	idev->dev.parent = &pdev->dev;
+	idev->dev.parent = dev;
 	idev->open = cros_ec_keyb_open;
 	idev->close = cros_ec_keyb_close;
 
-- 
2.8.0.rc3.226.g39d4020

[toc] | [next] | [standalone]

#1449788

FromDmitry Torokhov <dmitry.torokhov@gmail.com>
Date2016-07-25 22:40 +0200
Message-ID<rYUPn-HU-29@gated-at.bofh.it>
In reply to#1449742
On Mon, Jul 25, 2016 at 11:43:48AM -0700, Brian Norris wrote:
> From: Douglas Anderson <dianders@chromium.org>
> 
> In cros_ec_keyb we stored "dev" in "struct cros_ec_keyb", but this was
> the EC's dev pointer and not the keyboard's.  Let's clean this up to
> make it the keyboard's dev pointer.  This could be useful in future
> patches but also has the nice effect of changing a few printouts to
> include the name of the keyboard device instead of the EC device, which
> seems more technically correct.

What is the difference in the output? Could you show old vs new example?

> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
>  drivers/input/keyboard/cros_ec_keyb.c | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
> index 6e48616a3a88..ab1b50152b86 100644
> --- a/drivers/input/keyboard/cros_ec_keyb.c
> +++ b/drivers/input/keyboard/cros_ec_keyb.c
> @@ -186,7 +186,7 @@ static irqreturn_t cros_ec_keyb_irq(int irq, void *data)
>  	if (ret >= 0)
>  		cros_ec_keyb_process(ckdev, kb_state, ret);
>  	else
> -		dev_err(ec->dev, "failed to get keyboard state: %d\n", ret);
> +		dev_err(ckdev->dev, "failed to get keyboard state: %d\n", ret);
>  
>  	return IRQ_HANDLED;
>  }
> @@ -236,7 +236,7 @@ static void cros_ec_keyb_compute_valid_keys(struct cros_ec_keyb *ckdev)
>  static int cros_ec_keyb_probe(struct platform_device *pdev)
>  {
>  	struct cros_ec_device *ec = dev_get_drvdata(pdev->dev.parent);
> -	struct device *dev = ec->dev;
> +	struct device *dev = &pdev->dev;
>  	struct cros_ec_keyb *ckdev;
>  	struct input_dev *idev;
>  	struct device_node *np;
> @@ -246,23 +246,22 @@ static int cros_ec_keyb_probe(struct platform_device *pdev)
>  	if (!np)
>  		return -ENODEV;
>  
> -	ckdev = devm_kzalloc(&pdev->dev, sizeof(*ckdev), GFP_KERNEL);
> +	ckdev = devm_kzalloc(dev, sizeof(*ckdev), GFP_KERNEL);
>  	if (!ckdev)
>  		return -ENOMEM;
> -	err = matrix_keypad_parse_of_params(&pdev->dev, &ckdev->rows,
> -					    &ckdev->cols);
> +	err = matrix_keypad_parse_of_params(dev, &ckdev->rows, &ckdev->cols);
>  	if (err)
>  		return err;
>  
> -	ckdev->valid_keys = devm_kzalloc(&pdev->dev, ckdev->cols, GFP_KERNEL);
> +	ckdev->valid_keys = devm_kzalloc(dev, ckdev->cols, GFP_KERNEL);
>  	if (!ckdev->valid_keys)
>  		return -ENOMEM;
>  
> -	ckdev->old_kb_state = devm_kzalloc(&pdev->dev, ckdev->cols, GFP_KERNEL);
> +	ckdev->old_kb_state = devm_kzalloc(dev, ckdev->cols, GFP_KERNEL);
>  	if (!ckdev->old_kb_state)
>  		return -ENOMEM;
>  
> -	idev = devm_input_allocate_device(&pdev->dev);
> +	idev = devm_input_allocate_device(dev);
>  	if (!idev)
>  		return -ENOMEM;
>  
> @@ -273,7 +272,7 @@ static int cros_ec_keyb_probe(struct platform_device *pdev)
>  
>  	ckdev->ec = ec;
>  	ckdev->dev = dev;
> -	dev_set_drvdata(&pdev->dev, ckdev);
> +	dev_set_drvdata(dev, ckdev);
>  
>  	idev->name = CROS_EC_DEV_NAME;
>  	idev->phys = ec->phys_name;
> @@ -282,7 +281,7 @@ static int cros_ec_keyb_probe(struct platform_device *pdev)
>  	idev->id.bustype = BUS_VIRTUAL;
>  	idev->id.version = 1;
>  	idev->id.product = 0;
> -	idev->dev.parent = &pdev->dev;
> +	idev->dev.parent = dev;
>  	idev->open = cros_ec_keyb_open;
>  	idev->close = cros_ec_keyb_close;
>  
> -- 
> 2.8.0.rc3.226.g39d4020
> 

-- 
Dmitry

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

#1449791

FromBrian Norris <briannorris@chromium.org>
Date2016-07-25 22:50 +0200
Message-ID<rYUZ3-Lt-1@gated-at.bofh.it>
In reply to#1449788
On Mon, Jul 25, 2016 at 01:31:51PM -0700, Dmitry Torokhov wrote:
> On Mon, Jul 25, 2016 at 11:43:48AM -0700, Brian Norris wrote:
> > From: Douglas Anderson <dianders@chromium.org>
> > 
> > In cros_ec_keyb we stored "dev" in "struct cros_ec_keyb", but this was
> > the EC's dev pointer and not the keyboard's.  Let's clean this up to
> > make it the keyboard's dev pointer.  This could be useful in future
> > patches but also has the nice effect of changing a few printouts to
> > include the name of the keyboard device instead of the EC device, which
> > seems more technically correct.
> 
> What is the difference in the output? Could you show old vs new example?

For instance, on a veyron Chromebook, I see this on bootup with v4.7:

# dmesg | grep -e cros-ec -e keyb
[    0.539153] cros-ec-spi spi0.0: Chrome EC device registered
[    1.224505] cros-ec-spi spi0.0: valid_keys[00] = 0x14
[    1.224509] cros-ec-spi spi0.0: valid_keys[01] = 0xff
[    1.224514] cros-ec-spi spi0.0: valid_keys[02] = 0xff
[    1.224518] cros-ec-spi spi0.0: valid_keys[03] = 0xff
[    1.224521] cros-ec-spi spi0.0: valid_keys[04] = 0xff
[    1.224525] cros-ec-spi spi0.0: valid_keys[05] = 0xf5
[    1.224529] cros-ec-spi spi0.0: valid_keys[06] = 0xff
[    1.224533] cros-ec-spi spi0.0: valid_keys[07] = 0xa4
[    1.224536] cros-ec-spi spi0.0: valid_keys[08] = 0xff
[    1.224540] cros-ec-spi spi0.0: valid_keys[09] = 0xfe
[    1.224544] cros-ec-spi spi0.0: valid_keys[10] = 0x55
[    1.224548] cros-ec-spi spi0.0: valid_keys[11] = 0xfa
[    1.224551] cros-ec-spi spi0.0: valid_keys[12] = 0xca
[    1.224632] input: cros_ec as /devices/platform/ff110000.spi/spi_master/spi0/spi0.0/ff110000.spi:ec@0:keyboard-controller/input/input0

and this, with this patch applied:

# dmesg | grep -e cros-ec -e keyb
[    0.539437] cros-ec-spi spi0.0: Chrome EC device registered
[    1.224648] cros-ec-keyb ff110000.spi:ec@0:keyboard-controller: valid_keys[00] = 0x14
[    1.224653] cros-ec-keyb ff110000.spi:ec@0:keyboard-controller: valid_keys[01] = 0xff
[    1.224657] cros-ec-keyb ff110000.spi:ec@0:keyboard-controller: valid_keys[02] = 0xff
[    1.224662] cros-ec-keyb ff110000.spi:ec@0:keyboard-controller: valid_keys[03] = 0xff
[    1.224666] cros-ec-keyb ff110000.spi:ec@0:keyboard-controller: valid_keys[04] = 0xff
[    1.224670] cros-ec-keyb ff110000.spi:ec@0:keyboard-controller: valid_keys[05] = 0xf5
[    1.224674] cros-ec-keyb ff110000.spi:ec@0:keyboard-controller: valid_keys[06] = 0xff
[    1.224678] cros-ec-keyb ff110000.spi:ec@0:keyboard-controller: valid_keys[07] = 0xa4
[    1.224682] cros-ec-keyb ff110000.spi:ec@0:keyboard-controller: valid_keys[08] = 0xff
[    1.224686] cros-ec-keyb ff110000.spi:ec@0:keyboard-controller: valid_keys[09] = 0xfe
[    1.224690] cros-ec-keyb ff110000.spi:ec@0:keyboard-controller: valid_keys[10] = 0x55
[    1.224694] cros-ec-keyb ff110000.spi:ec@0:keyboard-controller: valid_keys[11] = 0xfa
[    1.224698] cros-ec-keyb ff110000.spi:ec@0:keyboard-controller: valid_keys[12] = 0xca
[    1.224779] input: cros_ec as /devices/platform/ff110000.spi/spi_master/spi0/spi0.0/ff110000.spi:ec@0:keyboard-controller/input/input

(both are done with an added '#define DEBUG', since that's easier than fiddling
with dynamic debug prints)

I think it's important to see the "cros-ec-keyb" driver name, and the
specific device name is a nice bonus.

Do you want to paste something like this into the commit description, or
should I resend?

Rebards,
Brian

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

#1449819

FromDmitry Torokhov <dmitry.torokhov@gmail.com>
Date2016-07-25 23:00 +0200
Message-ID<rYV8L-Pn-45@gated-at.bofh.it>
In reply to#1449791
On Mon, Jul 25, 2016 at 01:46:05PM -0700, Brian Norris wrote:
> On Mon, Jul 25, 2016 at 01:31:51PM -0700, Dmitry Torokhov wrote:
> > On Mon, Jul 25, 2016 at 11:43:48AM -0700, Brian Norris wrote:
> > > From: Douglas Anderson <dianders@chromium.org>
> > > 
> > > In cros_ec_keyb we stored "dev" in "struct cros_ec_keyb", but this was
> > > the EC's dev pointer and not the keyboard's.  Let's clean this up to
> > > make it the keyboard's dev pointer.  This could be useful in future
> > > patches but also has the nice effect of changing a few printouts to
> > > include the name of the keyboard device instead of the EC device, which
> > > seems more technically correct.
> > 
> > What is the difference in the output? Could you show old vs new example?
> 
> For instance, on a veyron Chromebook, I see this on bootup with v4.7:
> 
> # dmesg | grep -e cros-ec -e keyb
> [    0.539153] cros-ec-spi spi0.0: Chrome EC device registered
> [    1.224505] cros-ec-spi spi0.0: valid_keys[00] = 0x14
> [    1.224509] cros-ec-spi spi0.0: valid_keys[01] = 0xff
> [    1.224514] cros-ec-spi spi0.0: valid_keys[02] = 0xff
> [    1.224518] cros-ec-spi spi0.0: valid_keys[03] = 0xff
> [    1.224521] cros-ec-spi spi0.0: valid_keys[04] = 0xff
> [    1.224525] cros-ec-spi spi0.0: valid_keys[05] = 0xf5
> [    1.224529] cros-ec-spi spi0.0: valid_keys[06] = 0xff
> [    1.224533] cros-ec-spi spi0.0: valid_keys[07] = 0xa4
> [    1.224536] cros-ec-spi spi0.0: valid_keys[08] = 0xff
> [    1.224540] cros-ec-spi spi0.0: valid_keys[09] = 0xfe
> [    1.224544] cros-ec-spi spi0.0: valid_keys[10] = 0x55
> [    1.224548] cros-ec-spi spi0.0: valid_keys[11] = 0xfa
> [    1.224551] cros-ec-spi spi0.0: valid_keys[12] = 0xca
> [    1.224632] input: cros_ec as /devices/platform/ff110000.spi/spi_master/spi0/spi0.0/ff110000.spi:ec@0:keyboard-controller/input/input0
> 
> and this, with this patch applied:
> 
> # dmesg | grep -e cros-ec -e keyb
> [    0.539437] cros-ec-spi spi0.0: Chrome EC device registered
> [    1.224648] cros-ec-keyb ff110000.spi:ec@0:keyboard-controller: valid_keys[00] = 0x14
> [    1.224653] cros-ec-keyb ff110000.spi:ec@0:keyboard-controller: valid_keys[01] = 0xff
> [    1.224657] cros-ec-keyb ff110000.spi:ec@0:keyboard-controller: valid_keys[02] = 0xff
> [    1.224662] cros-ec-keyb ff110000.spi:ec@0:keyboard-controller: valid_keys[03] = 0xff
> [    1.224666] cros-ec-keyb ff110000.spi:ec@0:keyboard-controller: valid_keys[04] = 0xff
> [    1.224670] cros-ec-keyb ff110000.spi:ec@0:keyboard-controller: valid_keys[05] = 0xf5
> [    1.224674] cros-ec-keyb ff110000.spi:ec@0:keyboard-controller: valid_keys[06] = 0xff
> [    1.224678] cros-ec-keyb ff110000.spi:ec@0:keyboard-controller: valid_keys[07] = 0xa4
> [    1.224682] cros-ec-keyb ff110000.spi:ec@0:keyboard-controller: valid_keys[08] = 0xff
> [    1.224686] cros-ec-keyb ff110000.spi:ec@0:keyboard-controller: valid_keys[09] = 0xfe
> [    1.224690] cros-ec-keyb ff110000.spi:ec@0:keyboard-controller: valid_keys[10] = 0x55
> [    1.224694] cros-ec-keyb ff110000.spi:ec@0:keyboard-controller: valid_keys[11] = 0xfa
> [    1.224698] cros-ec-keyb ff110000.spi:ec@0:keyboard-controller: valid_keys[12] = 0xca
> [    1.224779] input: cros_ec as /devices/platform/ff110000.spi/spi_master/spi0/spi0.0/ff110000.spi:ec@0:keyboard-controller/input/input
> 
> (both are done with an added '#define DEBUG', since that's easier than fiddling
> with dynamic debug prints)
> 
> I think it's important to see the "cros-ec-keyb" driver name, and the
> specific device name is a nice bonus.
> 
> Do you want to paste something like this into the commit description, or
> should I resend?

Yes, I munged the commit message a tiny bit, thank you.

-- 
Dmitry

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web