* [PATCH] Input: cros_ec_keyb - cleanup use of dev
@ 2016-07-25 18:43 Brian Norris
2016-07-25 20:31 ` Dmitry Torokhov
0 siblings, 1 reply; 4+ messages in thread
From: Brian Norris @ 2016-07-25 18:43 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Brian Norris, linux-input, linux-kernel, Douglas Anderson,
Brian Norris
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
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] Input: cros_ec_keyb - cleanup use of dev
2016-07-25 18:43 [PATCH] Input: cros_ec_keyb - cleanup use of dev Brian Norris
@ 2016-07-25 20:31 ` Dmitry Torokhov
2016-07-25 20:46 ` Brian Norris
0 siblings, 1 reply; 4+ messages in thread
From: Dmitry Torokhov @ 2016-07-25 20:31 UTC (permalink / raw)
To: Brian Norris; +Cc: Brian Norris, linux-input, linux-kernel, Douglas Anderson
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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] Input: cros_ec_keyb - cleanup use of dev
2016-07-25 20:31 ` Dmitry Torokhov
@ 2016-07-25 20:46 ` Brian Norris
2016-07-25 20:50 ` Dmitry Torokhov
0 siblings, 1 reply; 4+ messages in thread
From: Brian Norris @ 2016-07-25 20:46 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Brian Norris, linux-input, linux-kernel, Douglas Anderson
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Input: cros_ec_keyb - cleanup use of dev
2016-07-25 20:46 ` Brian Norris
@ 2016-07-25 20:50 ` Dmitry Torokhov
0 siblings, 0 replies; 4+ messages in thread
From: Dmitry Torokhov @ 2016-07-25 20:50 UTC (permalink / raw)
To: Brian Norris; +Cc: Brian Norris, linux-input, linux-kernel, Douglas Anderson
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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-07-25 20:50 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-07-25 18:43 [PATCH] Input: cros_ec_keyb - cleanup use of dev Brian Norris
2016-07-25 20:31 ` Dmitry Torokhov
2016-07-25 20:46 ` Brian Norris
2016-07-25 20:50 ` Dmitry Torokhov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).