* Re: [PATCH v2 01/10] tpm/tpm_i2c_infineon: switch to i2c_lock_bus(..., I2C_LOCK_SEGMENT)
From: Jarkko Sakkinen @ 2018-06-25 10:24 UTC (permalink / raw)
To: Peter Rosin
Cc: linux-kernel, Peter Huewe, Jason Gunthorpe, Arnd Bergmann,
Greg Kroah-Hartman, Brian Norris, Gregory Fong, Florian Fainelli,
bcm-kernel-feedback-list, Sekhar Nori, Kevin Hilman,
Haavard Skinnemoen, Kukjin Kim, Krzysztof Kozlowski, Orson Zhai,
Baolin Wang, Chunyan Zhang, Wolfram Sang, Guenter Roeck, Crt Mori,
Jonathan Cameron
In-Reply-To: <20180620051803.12206-2-peda@axentia.se>
On Wed, Jun 20, 2018 at 07:17:54AM +0200, Peter Rosin wrote:
> Locking the root adapter for __i2c_transfer will deadlock if the
> device sits behind a mux-locked I2C mux. Switch to the finer-grained
> i2c_lock_bus with the I2C_LOCK_SEGMENT flag. If the device does not
> sit behind a mux-locked mux, the two locking variants are equivalent.
>
> Signed-off-by: Peter Rosin <peda@axentia.se>
Studied enough so that I can give
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Do not have hardware to test this, however.
/Jarkko
^ permalink raw reply
* Re: [PATCH v2 5/8] Input: cros_ec_keyb - Switch to SPDX identifier.
From: Greg KH @ 2018-06-25 6:42 UTC (permalink / raw)
To: Enric Balletbo i Serra
Cc: Fabio Estevam, linux-kernel, Gwendal Grignou, kernel,
Dmitry Torokhov, linux-input
In-Reply-To: <5926a3f2-3646-f9e1-619a-255e619f41f0@collabora.com>
On Mon, Jun 25, 2018 at 08:23:38AM +0200, Enric Balletbo i Serra wrote:
> Hi Grek,
>
> On 25/06/18 03:57, Greg KH wrote:
> > On Tue, Jun 05, 2018 at 08:14:24PM +0200, Enric Balletbo i Serra wrote:
> >> Hi Fabio,
> >>
> >> On 05/06/18 20:04, Fabio Estevam wrote:
> >>> Hi Enric,
> >>>
> >>> On Tue, Jun 5, 2018 at 2:54 PM, Enric Balletbo i Serra
> >>> <enric.balletbo@collabora.com> wrote:
> >>>> Adopt the SPDX license identifier headers to ease license compliance
> >>>> management.
> >>>>
> >>>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> >>>> ---
> >>>>
> >>>> Changes in v2:
> >>>> - [6/9] Do not remove last paragraph.
> >>>>
> >>>> drivers/input/keyboard/cros_ec_keyb.c | 34 ++++++++++-----------------
> >>>> 1 file changed, 12 insertions(+), 22 deletions(-)
> >>>>
> >>>> diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
> >>>> index 79eb29550c34..91b2839c12df 100644
> >>>> --- a/drivers/input/keyboard/cros_ec_keyb.c
> >>>> +++ b/drivers/input/keyboard/cros_ec_keyb.c
> >>>> @@ -1,25 +1,15 @@
> >>>> -/*
> >>>> - * ChromeOS EC keyboard driver
> >>>> - *
> >>>> - * Copyright (C) 2012 Google, Inc
> >>>> - *
> >>>> - * This software is licensed under the terms of the GNU General Public
> >>>> - * License version 2, as published by the Free Software Foundation, and
> >>>
> >>> Original text says GPL-2.0...
> >>>
> >>>> - * may be copied, distributed, and modified under those terms.
> >>>> - *
> >>>> - * This program is distributed in the hope that it will be useful,
> >>>> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >>>> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> >>>> - * GNU General Public License for more details.
> >>>> - *
> >>>> - * This driver uses the Chrome OS EC byte-level message-based protocol for
> >>>> - * communicating the keyboard state (which keys are pressed) from a keyboard EC
> >>>> - * to the AP over some bus (such as i2c, lpc, spi). The EC does debouncing,
> >>>> - * but everything else (including deghosting) is done here. The main
> >>>> - * motivation for this is to keep the EC firmware as simple as possible, since
> >>>> - * it cannot be easily upgraded and EC flash/IRAM space is relatively
> >>>> - * expensive.
> >>>> - */
> >>>> +// SPDX-License-Identifier: GPL-2.0+
> >>>
> >>> but here you add a GPL-2.0+ type.
> >>>
> >>
> >> Right, but the module license is set to GPL which means GNU Public License v2 or
> >> later, see [1]. So, there is a mismatch. In such cases I assumed GPL-2.0+ as the
> >> default. These mismatches are common so I think that should be fine for the
> >> authors, if someone is disagree just let me know and I will change.
> >
> > NO!
> >
> > The license text is what you need to follow as almost everyone gets the
> > MODULE_LICENSE() stuff wrong. You can not follow the MODULE_LICENSE
> > marking for determining GPLv2 vs. GPLv2+ at this point in time. After
> > we get the SPDX stuff all cleaned up then we can go and fix up the
> > MODULE_LICENSE mismatch.
> >
> > So again, always defer to the license text in the file, NOT the
> > MODULE_LICENSE() marking.
> >
>
> Many thanks for your feedback, Dmitry already told me that I was doing wrong
> this and I sent another version following the license text, so this patchset was
> superseded for v3 [1].
>
> [1] https://patchwork.ozlabs.org/cover/925906/
Ah, good, sorry for the noise then, I didn't see that series.
greg k-h
^ permalink raw reply
* Re: [PATCH v2 5/8] Input: cros_ec_keyb - Switch to SPDX identifier.
From: Enric Balletbo i Serra @ 2018-06-25 6:23 UTC (permalink / raw)
To: Greg KH
Cc: Fabio Estevam, linux-kernel, Gwendal Grignou, kernel,
Dmitry Torokhov, linux-input
In-Reply-To: <20180625015717.GA30408@kroah.com>
Hi Grek,
On 25/06/18 03:57, Greg KH wrote:
> On Tue, Jun 05, 2018 at 08:14:24PM +0200, Enric Balletbo i Serra wrote:
>> Hi Fabio,
>>
>> On 05/06/18 20:04, Fabio Estevam wrote:
>>> Hi Enric,
>>>
>>> On Tue, Jun 5, 2018 at 2:54 PM, Enric Balletbo i Serra
>>> <enric.balletbo@collabora.com> wrote:
>>>> Adopt the SPDX license identifier headers to ease license compliance
>>>> management.
>>>>
>>>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>>>> ---
>>>>
>>>> Changes in v2:
>>>> - [6/9] Do not remove last paragraph.
>>>>
>>>> drivers/input/keyboard/cros_ec_keyb.c | 34 ++++++++++-----------------
>>>> 1 file changed, 12 insertions(+), 22 deletions(-)
>>>>
>>>> diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
>>>> index 79eb29550c34..91b2839c12df 100644
>>>> --- a/drivers/input/keyboard/cros_ec_keyb.c
>>>> +++ b/drivers/input/keyboard/cros_ec_keyb.c
>>>> @@ -1,25 +1,15 @@
>>>> -/*
>>>> - * ChromeOS EC keyboard driver
>>>> - *
>>>> - * Copyright (C) 2012 Google, Inc
>>>> - *
>>>> - * This software is licensed under the terms of the GNU General Public
>>>> - * License version 2, as published by the Free Software Foundation, and
>>>
>>> Original text says GPL-2.0...
>>>
>>>> - * may be copied, distributed, and modified under those terms.
>>>> - *
>>>> - * This program is distributed in the hope that it will be useful,
>>>> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>>> - * GNU General Public License for more details.
>>>> - *
>>>> - * This driver uses the Chrome OS EC byte-level message-based protocol for
>>>> - * communicating the keyboard state (which keys are pressed) from a keyboard EC
>>>> - * to the AP over some bus (such as i2c, lpc, spi). The EC does debouncing,
>>>> - * but everything else (including deghosting) is done here. The main
>>>> - * motivation for this is to keep the EC firmware as simple as possible, since
>>>> - * it cannot be easily upgraded and EC flash/IRAM space is relatively
>>>> - * expensive.
>>>> - */
>>>> +// SPDX-License-Identifier: GPL-2.0+
>>>
>>> but here you add a GPL-2.0+ type.
>>>
>>
>> Right, but the module license is set to GPL which means GNU Public License v2 or
>> later, see [1]. So, there is a mismatch. In such cases I assumed GPL-2.0+ as the
>> default. These mismatches are common so I think that should be fine for the
>> authors, if someone is disagree just let me know and I will change.
>
> NO!
>
> The license text is what you need to follow as almost everyone gets the
> MODULE_LICENSE() stuff wrong. You can not follow the MODULE_LICENSE
> marking for determining GPLv2 vs. GPLv2+ at this point in time. After
> we get the SPDX stuff all cleaned up then we can go and fix up the
> MODULE_LICENSE mismatch.
>
> So again, always defer to the license text in the file, NOT the
> MODULE_LICENSE() marking.
>
Many thanks for your feedback, Dmitry already told me that I was doing wrong
this and I sent another version following the license text, so this patchset was
superseded for v3 [1].
[1] https://patchwork.ozlabs.org/cover/925906/
Thanks,
Enric
> thanks,
>
> greg k-h
>
^ permalink raw reply
* Re: [PATCH v2 5/8] Input: cros_ec_keyb - Switch to SPDX identifier.
From: Greg KH @ 2018-06-25 1:57 UTC (permalink / raw)
To: Enric Balletbo i Serra
Cc: Fabio Estevam, linux-kernel, Gwendal Grignou, kernel,
Dmitry Torokhov, linux-input
In-Reply-To: <7085d0bb-557c-0796-d3ae-756e000a7950@collabora.com>
On Tue, Jun 05, 2018 at 08:14:24PM +0200, Enric Balletbo i Serra wrote:
> Hi Fabio,
>
> On 05/06/18 20:04, Fabio Estevam wrote:
> > Hi Enric,
> >
> > On Tue, Jun 5, 2018 at 2:54 PM, Enric Balletbo i Serra
> > <enric.balletbo@collabora.com> wrote:
> >> Adopt the SPDX license identifier headers to ease license compliance
> >> management.
> >>
> >> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> >> ---
> >>
> >> Changes in v2:
> >> - [6/9] Do not remove last paragraph.
> >>
> >> drivers/input/keyboard/cros_ec_keyb.c | 34 ++++++++++-----------------
> >> 1 file changed, 12 insertions(+), 22 deletions(-)
> >>
> >> diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
> >> index 79eb29550c34..91b2839c12df 100644
> >> --- a/drivers/input/keyboard/cros_ec_keyb.c
> >> +++ b/drivers/input/keyboard/cros_ec_keyb.c
> >> @@ -1,25 +1,15 @@
> >> -/*
> >> - * ChromeOS EC keyboard driver
> >> - *
> >> - * Copyright (C) 2012 Google, Inc
> >> - *
> >> - * This software is licensed under the terms of the GNU General Public
> >> - * License version 2, as published by the Free Software Foundation, and
> >
> > Original text says GPL-2.0...
> >
> >> - * may be copied, distributed, and modified under those terms.
> >> - *
> >> - * This program is distributed in the hope that it will be useful,
> >> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> >> - * GNU General Public License for more details.
> >> - *
> >> - * This driver uses the Chrome OS EC byte-level message-based protocol for
> >> - * communicating the keyboard state (which keys are pressed) from a keyboard EC
> >> - * to the AP over some bus (such as i2c, lpc, spi). The EC does debouncing,
> >> - * but everything else (including deghosting) is done here. The main
> >> - * motivation for this is to keep the EC firmware as simple as possible, since
> >> - * it cannot be easily upgraded and EC flash/IRAM space is relatively
> >> - * expensive.
> >> - */
> >> +// SPDX-License-Identifier: GPL-2.0+
> >
> > but here you add a GPL-2.0+ type.
> >
>
> Right, but the module license is set to GPL which means GNU Public License v2 or
> later, see [1]. So, there is a mismatch. In such cases I assumed GPL-2.0+ as the
> default. These mismatches are common so I think that should be fine for the
> authors, if someone is disagree just let me know and I will change.
NO!
The license text is what you need to follow as almost everyone gets the
MODULE_LICENSE() stuff wrong. You can not follow the MODULE_LICENSE
marking for determining GPLv2 vs. GPLv2+ at this point in time. After
we get the SPDX stuff all cleaned up then we can go and fix up the
MODULE_LICENSE mismatch.
So again, always defer to the license text in the file, NOT the
MODULE_LICENSE() marking.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] synaptics: Enable RMI4 for Clevo P870DM laptops
From: Teika Kazura @ 2018-06-24 6:04 UTC (permalink / raw)
To: Rosen Penev
Cc: linux-input, linux-kernel, Benjamin Tissoires, Dmitry Torokhov,
jan.steffens, Pablo Cholaky
In-Reply-To: <20180617014216.4043-1-rosenp@gmail.com>
Hi, Rosen. Thank you for your patch. (@Jan Steffens and @Pablo Cholaky: Thanks for your reports.) But I'm afraid some more work is necessary. Let me propose some points.
I'm not a kernel developer, but have been working on this issue to help them. (See https://www.spinics.net/lists/linux-input/msg55950.html )
0.
Could you resend it, cc-ing to the relevant developers. (Use the one of my reply here. You can use <kernel src tree>/scripts/get_maintainer.pl next time.) Otherwise, it is unlikely to be noticed by them.
1.
You added your device to forcepad_pnp_ids[], but I guess it should be smbus_pnp_ids[].
When making the patch again, please use the kernel 4.17 or newer, because smbus_pnp_ids was changed recently.
If it works by using smbus_pnp_ids, please test it for a week or so, especially making sure that suspend/resume works without any problem.
2.
Though it's my personal impression, the change description looked somewhat personal. At the same time, it's important, guaranteeing the stability of the patch.- I know you originally reported it last August [1]. Furthermore, there have been two other users [2] who reported SMBus worked for the same device, namely SYN1219. (They two are cc-ed to in this reply.)
You can use "$ git format-patch --cover-letter"; it will generate a "cover-letter", the introductory part of the patch, explaining the acceptability of the patch to kernel developers. (For an example, see https://www.spinics.net/lists/linux-input/msg57041.html)
So the full description can be given in the cover letter, and the real patch description can be something like "This enables SMBus for xxx", being almost identical to the email subject.
3. This is not essential, but the style of the email subject that the device maintainer prefers might be:
------------------------------------------------------------------------
[PATCH v2] Input: synaptics - Enable RMI4 for Clevo P870DM laptops
------------------------------------------------------------------------
after looking at this tree [3]. ("v2" meaning version 2, due to the above change.) You can feed "--subject-prefix='PATCH v2'" to "$ git format-patch".
4. If you want to give the credit for my reply, you can add
------------------------------------------------------------------------
Suggested-by: Teika Kazura <teika@gmx.com>
------------------------------------------------------------------------
after Signed-off-by. It's completely ok without it. :-)
[1] https://marc.info/?l=linux-input&m=150284057602358&w=2
[2] https://marc.info/?l=linux-input&m=150049625613055&w=2
https://marc.info/?l=linux-input&m=150094561111026&w=2
[3] https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git/log/
Best regards,
Teika (Teika kazura)
From: Rosen Penev <rosenp@gmail.com>
Subject: [PATCH] synaptics: Enable RMI4 for Clevo P870DM laptops
Date: Sat, 16 Jun 2018 18:42:16 -0700
> I have been testing this for half a year with no issues to report.
>
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
> drivers/input/mouse/synaptics.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> index 55d33500d55e..23f5bb2cf9da 100644
> --- a/drivers/input/mouse/synaptics.c
> +++ b/drivers/input/mouse/synaptics.c
> @@ -183,6 +183,7 @@ static const char * const smbus_pnp_ids[] = {
> };
>
> static const char * const forcepad_pnp_ids[] = {
> + "SYN1219",
> "SYN300D",
> "SYN3014",
> NULL
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH v3 1/2] HID: i2c-hid: Use devm to allocate i2c_hid struct
From: Dmitry Torokhov @ 2018-06-22 22:39 UTC (permalink / raw)
To: Hans de Goede
Cc: Benjamin Tissoires, swboyd, Jiri Kosina, lkml,
open list:HID CORE LAYER, andriy.shevchenko, Doug Anderson
In-Reply-To: <095738e5-eb31-905b-b496-0d78489e253d@redhat.com>
On Fri, Jun 22, 2018 at 2:13 AM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 22-06-18 09:16, Benjamin Tissoires wrote:
> > On Fri, Jun 22, 2018 at 4:27 AM, Stephen Boyd <swboyd@chromium.org> wrote:
> >> Use devm here to save some lines and prepare for bulk regulator usage in
> >> this driver. Otherwise, when we devm bulk get regulators we'll free the
> >> containing i2c_hid structure and try to put regulator pointers from
> >> freed memory.
> >>
> >> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> >> Cc: Hans de Goede <hdegoede@redhat.com>
> >> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> >> Cc: Dmitry Torokhov <dtor@chromium.org>
> >> Cc: Doug Anderson <dianders@chromium.org>
> >> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> >> ---
> >> drivers/hid/i2c-hid/i2c-hid.c | 9 +++------
> >> 1 file changed, 3 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
> >> index c1652bb7bd15..c7d6738dc524 100644
> >> --- a/drivers/hid/i2c-hid/i2c-hid.c
> >> +++ b/drivers/hid/i2c-hid/i2c-hid.c
> >> @@ -1002,18 +1002,18 @@ static int i2c_hid_probe(struct i2c_client *client,
> >> return client->irq;
> >> }
> >>
> >> - ihid = kzalloc(sizeof(struct i2c_hid), GFP_KERNEL);
> >> + ihid = devm_kzalloc(&client->dev, sizeof(*ihid), GFP_KERNEL);
> >
> > IIRC, I never made the switch towards devm for i2c_hid because at the
> > time there was a "all or nothing" rule regarding devm.
>
> I'm not aware of any such rule. Sure ideally everything should use
> devm, because it makes life just so much easier. But I've seen mixed
> use in plenty of cases.
When mixing managed and unmanaged resources you need to be extremely
careful to make sure they are released in proper order. There were
many naive conversions that would convert just part of resources to
devm and end up, let's say, powering down rails of a device or turning
off clocks while interrupts are still registered, which causes errors
if interrupt fires, and so on. I usually request people to do either
full conversion or leave the driver alone.
That said, using devm to allocate the "main" driver structure is usually safe.
>
> With that said converting fully to devm is not necessarily a bad
> idea.
>
> Regards,
>
> Hans
>
>
> > But given that the regulator already has a devm inside, I think we are
> > screwed here and we should probably try to devm-ize the module.
> >
> > I seem to remember that someone posted a devm version of
> > hid_allocate_device/hid-add_device, but I don't think this ever went
> > upstream (because no use).
There is always devm_add_action_or_reset() to inject custom actions
into devm stack to work around lacking devm APIs.
> >
> > Otherwise, for the series:
> > Acked-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> >
> > Cheers,
> > Benjamin
> >
> >
> >
> >> if (!ihid)
> >> return -ENOMEM;
> >>
> >> if (client->dev.of_node) {
> >> ret = i2c_hid_of_probe(client, &ihid->pdata);
> >> if (ret)
> >> - goto err;
> >> + return ret;
> >> } else if (!platform_data) {
> >> ret = i2c_hid_acpi_pdata(client, &ihid->pdata);
> >> if (ret)
> >> - goto err;
> >> + return ret;
> >> } else {
> >> ihid->pdata = *platform_data;
> >> }
> >> @@ -1126,7 +1126,6 @@ static int i2c_hid_probe(struct i2c_client *client,
> >>
> >> err:
> >> i2c_hid_free_buffers(ihid);
> >> - kfree(ihid);
> >> return ret;
> >> }
> >>
> >> @@ -1150,8 +1149,6 @@ static int i2c_hid_remove(struct i2c_client *client)
> >>
> >> regulator_disable(ihid->pdata.supply);
> >>
> >> - kfree(ihid);
> >> -
> >> return 0;
> >> }
> >>
> >> --
> >> Sent by a computer through tubes
> >>
^ permalink raw reply
* Re: [PATCH v3 3/5] bitmap: Add bitmap_alloc(), bitmap_zalloc() and bitmap_free()
From: Dmitry Torokhov @ 2018-06-22 18:46 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Andrew Morton, Andy Shevchenko, agk, Mike Snitzer,
device-mapper development, shli, linux-raid,
linux-input@vger.kernel.org, Yury Norov, lkml, Mika Westerberg,
Joe Perches
In-Reply-To: <CAHp75Vei2_V7h4oKAU+uoCG2gUJH6uXtg9zVqQ_vnyr5KEmNYw@mail.gmail.com>
On Thu, Jun 21, 2018 at 05:13:39AM +0300, Andy Shevchenko wrote:
> On Tue, Jun 19, 2018 at 2:10 AM, Andrew Morton
> <akpm@linux-foundation.org> wrote:
> > On Mon, 18 Jun 2018 15:01:43 -0700 Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
>
> >> We can't as we end up including bitmap.h (by the way of cpumask.h)
> >> form slab.h, so we gen circular dependency.
> >
>
> It's not just so easy. See below.
>
> > That info should have been in the changelog, and probably a code
> > comment.
> >
> >> Maybe if we removed memcg
> >> stuff from slab.h so we do not need to include workqueue.h...
> >
> > Or move the basic slab API stuff out of slab.h into a new header. Or
> > create a new, standalone work_struct.h - that looks pretty simple.
>
> I tried to move out work_struct, it didn't help. There are actually
> several circular dependencies that ends in bitmap.h either way or
> another.
>
> First one is
>
> slab.h -> gfp.h -> mmzone.h -> nodemask.h -> bitmap.h
>
> And so on...
>
> Splitting out kXalloc stuff to a separate header won't help, I think,
> because of the above.
> Splitting out struct work_struct is just a tip of an iceberg.
> Splitting out memcg stuff won't help in the similar way.
>
> I'm all ears for (a better) solution.
I think ultimately we'd want to untangle this, but allocating bitmaps is
not in any hot paths so having them as non-inlined functions should not
hurt us that much for time being.
Just my 2 cents...
--
Dmitry
^ permalink raw reply
* Re: [PATCH] input: mtk-pmic-keys: Fix probe when no DT node present
From: Dmitry Torokhov @ 2018-06-22 18:31 UTC (permalink / raw)
To: Matthias Brugger
Cc: chen.zhong, lee.jones, linux-input, linux-arm-kernel,
linux-mediatek, linux-kernel, Matthias Brugger
In-Reply-To: <20180622114228.8619-1-matthias.bgg@gmail.com>
On Fri, Jun 22, 2018 at 01:42:28PM +0200, Matthias Brugger wrote:
> The drivers gets probed from a mfd devices. So the driver runs
> probe although no DT node exists. This leads to a NULL pointer
> dereference in the probe function. Check if a node exists and
> error out in case none is present.
Hmm, if a cell specifies of_compatible and no such node is present in DT
maybe we should bail out in mfd_add_device() instead of pushing the
check into individual drivers? Something like:
diff --git a/drivers/mfd/mfd-core.c b/drivers/mfd/mfd-core.c
index 94e3f32ce935..36a5fd98000d 100644
--- a/drivers/mfd/mfd-core.c
+++ b/drivers/mfd/mfd-core.c
@@ -137,6 +137,18 @@ static inline void mfd_acpi_add_device(const struct mfd_cell *cell,
}
#endif
+static struct device_node *mfd_get_node_for_cell(struct device *parent,
+ const struct mfd_cell *cell)
+{
+ struct device_node *np;
+
+ for_each_child_of_node(parent->of_node, np)
+ if (of_device_is_compatible(np, cell->of_compatible))
+ return np;
+
+ return NULL;
+}
+
static int mfd_add_device(struct device *parent, int id,
const struct mfd_cell *cell, atomic_t *usage_count,
struct resource *mem_base,
@@ -144,7 +156,6 @@ static int mfd_add_device(struct device *parent, int id,
{
struct resource *res;
struct platform_device *pdev;
- struct device_node *np = NULL;
int ret = -ENOMEM;
int platform_id;
int r;
@@ -176,11 +187,11 @@ static int mfd_add_device(struct device *parent, int id,
goto fail_res;
if (parent->of_node && cell->of_compatible) {
- for_each_child_of_node(parent->of_node, np) {
- if (of_device_is_compatible(np, cell->of_compatible)) {
- pdev->dev.of_node = np;
- break;
- }
+ pdev->dev.of_node = mfd_get_node_for_cell(parent, cell);
+ if (!pdev->dev.of_node) {
+ dev_err(parent, "unable to locate node for %s (%s)\n",
+ cell->name, cell->of_compatible);
+ goto fail_res;
}
}
Lee?
>
> Fixes: 3e9f0b3e2b27 ("input: Add MediaTek PMIC keys support")
> Signed-off-by: Matthias Brugger <mbrugger@suse.com>
> ---
> drivers/input/keyboard/mtk-pmic-keys.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/input/keyboard/mtk-pmic-keys.c b/drivers/input/keyboard/mtk-pmic-keys.c
> index 02c67a1749fc..388043e1939c 100644
> --- a/drivers/input/keyboard/mtk-pmic-keys.c
> +++ b/drivers/input/keyboard/mtk-pmic-keys.c
> @@ -257,6 +257,9 @@ static int mtk_pmic_keys_probe(struct platform_device *pdev)
> const struct of_device_id *of_id =
> of_match_device(of_mtk_pmic_keys_match_tbl, &pdev->dev);
>
> + if (of_id == NULL)
> + return -ENODEV;
> +
> keys = devm_kzalloc(&pdev->dev, sizeof(*keys), GFP_KERNEL);
> if (!keys)
> return -ENOMEM;
> --
> 2.17.1
>
Thanks.
--
Dmitry
^ permalink raw reply related
* [RESEND PATCH v2] MAINTAINERS: add PhoenixRC Flight Controller Adapter
From: Marcus Folkesson @ 2018-06-22 17:06 UTC (permalink / raw)
To: David S . Miller, Mauro Carvalho Chehab, Greg Kroah-Hartman,
Andrew Morton, Randy Dunlap
Cc: linux-kernel, linux-input, Marcus Folkesson
Add MAINTAINERS entry for PhoenixRC Flight Controller Adapter
Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---
Notes:
v2:
Add proper mailing list
MAINTAINERS | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 9ce5062dc028..d8b5d9f810e4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11103,6 +11103,13 @@ S: Maintained
F: include/linux/personality.h
F: include/uapi/linux/personality.h
+PHOENIX RC FLIGHT CONTROLLER ADAPTER
+M: Marcus Folkesson <marcus.folkesson@gmail.com>
+L: linux-input@vger.kernel.org
+S: Maintained
+F: Documentation/input/devices/pxrc.rst
+F: drivers/input/joystick/pxrc.c
+
PHONET PROTOCOL
M: Remi Denis-Courmont <courmisch@gmail.com>
S: Supported
--
2.16.2
^ permalink raw reply related
* [PATCH] i2c-hid: Fix "incomplete report" noise
From: Jason Andryuk @ 2018-06-22 16:25 UTC (permalink / raw)
To: linux-kernel
Cc: john-s-84, Jason Andryuk, stable, Jiri Kosina, Benjamin Tissoires,
linux-input
Commit ac75a041048b ("HID: i2c-hid: fix size check and type usage")
started writing messages when the ret_size is <= 2 from i2c_master_recv.
However, my device i2c-DLL07D1 returns 2 for a short period of time
(~0.5s) after I stop moving the pointing stick or touchpad. It varies,
but you get ~50 messages each time which spams the log hard.
[ 95.925055] i2c_hid i2c-DLL07D1:01: i2c_hid_get_input: incomplete report (83/2)
This has also been observed with a i2c-ALP0017.
[ 1781.266353] i2c_hid i2c-ALP0017:00: i2c_hid_get_input: incomplete report (30/2)
Only print the message when ret_size is totally invalid and less than 2
to cut down on the log spam.
Reported-by: John Smith <john-s-84@gmx.net>
Cc: stable@vger.kernel.org
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
John Smith originally reported this, but his post did not include a git
formatted patch nor a Signed-off-by.
https://www.spinics.net/lists/linux-input/msg56171.html
https://patchwork.kernel.org/patch/10374383/
When ret_size is 2, hid_input_report is passed 0 and returns early. ret_size
== 2 seems to be a header size saying there is no content. Should
i2c_hid_get_input just return early in that case?
Also, should this condition be noted to stop an interrupt from firing to
avoid the ~50 bogus messages?
drivers/hid/i2c-hid/i2c-hid.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index c1652bb7bd15..eae0cb3ddec6 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -484,7 +484,7 @@ static void i2c_hid_get_input(struct i2c_hid *ihid)
return;
}
- if ((ret_size > size) || (ret_size <= 2)) {
+ if ((ret_size > size) || (ret_size < 2)) {
dev_err(&ihid->client->dev, "%s: incomplete report (%d/%d)\n",
__func__, size, ret_size);
return;
--
2.17.1
^ permalink raw reply related
* [PATCH] input: mtk-pmic-keys: Fix probe when no DT node present
From: Matthias Brugger @ 2018-06-22 11:42 UTC (permalink / raw)
To: dmitry.torokhov, chen.zhong, lee.jones
Cc: matthias.bgg, linux-input, linux-arm-kernel, linux-mediatek,
linux-kernel, Matthias Brugger
The drivers gets probed from a mfd devices. So the driver runs
probe although no DT node exists. This leads to a NULL pointer
dereference in the probe function. Check if a node exists and
error out in case none is present.
Fixes: 3e9f0b3e2b27 ("input: Add MediaTek PMIC keys support")
Signed-off-by: Matthias Brugger <mbrugger@suse.com>
---
drivers/input/keyboard/mtk-pmic-keys.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/input/keyboard/mtk-pmic-keys.c b/drivers/input/keyboard/mtk-pmic-keys.c
index 02c67a1749fc..388043e1939c 100644
--- a/drivers/input/keyboard/mtk-pmic-keys.c
+++ b/drivers/input/keyboard/mtk-pmic-keys.c
@@ -257,6 +257,9 @@ static int mtk_pmic_keys_probe(struct platform_device *pdev)
const struct of_device_id *of_id =
of_match_device(of_mtk_pmic_keys_match_tbl, &pdev->dev);
+ if (of_id == NULL)
+ return -ENODEV;
+
keys = devm_kzalloc(&pdev->dev, sizeof(*keys), GFP_KERNEL);
if (!keys)
return -ENOMEM;
--
2.17.1
^ permalink raw reply related
* [PATCH v4] MAINTAINERS: Add file patterns for serio device tree bindings
From: Geert Uytterhoeven @ 2018-06-22 10:08 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Rob Herring, Mark Rutland, linux-input, devicetree, linux-kernel,
Geert Uytterhoeven
Submitters of device tree binding documentation may forget to CC
the subsystem maintainer if this is missing.
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
v3:
- Update for next-20180622,
v2:
- Add Acked-by.
Impact on "scripts/get_maintainer.pl -f Documentation/devicetree/bindings/serio/":
-Rob Herring <robh+dt@kernel.org> (maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,commit_signer:3/3=100%,authored:1/3=33%)
-Mark Rutland <mark.rutland@arm.com> (maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,commit_signer:1/3=33%)
-Dmitry Torokhov <dmitry.torokhov@gmail.com> (commit_signer:1/3=33%)
-Danilo Krummrich <danilokrummrich@dk-develop.de> (commit_signer:1/3=33%,authored:1/3=33%)
-Linus Walleij <linus.walleij@linaro.org> (commit_signer:1/3=33%)
-Mathieu Malaterre <malat@debian.org> (authored:1/3=33%)
+Dmitry Torokhov <dmitry.torokhov@gmail.com> (maintainer:INPUT (KEYBOARD, MOUSE, JOYSTICK, TOUCHSCREEN)...)
+Rob Herring <robh+dt@kernel.org> (maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS)
+Mark Rutland <mark.rutland@arm.com> (maintainer:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS)
+linux-input@vger.kernel.org (open list:INPUT (KEYBOARD, MOUSE, JOYSTICK, TOUCHSCREEN)...)
devicetree@vger.kernel.org (open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS)
linux-kernel@vger.kernel.org (open list)
---
MAINTAINERS | 1 +
1 file changed, 1 insertion(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 464d91879df89fce..df246daab5a27af2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7100,6 +7100,7 @@ F: include/uapi/linux/input.h
F: include/uapi/linux/input-event-codes.h
F: include/linux/input/
F: Documentation/devicetree/bindings/input/
+F: Documentation/devicetree/bindings/serio/
F: Documentation/input/
INPUT MULTITOUCH (MT) PROTOCOL
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v3 1/2] HID: i2c-hid: Use devm to allocate i2c_hid struct
From: Hans de Goede @ 2018-06-22 9:13 UTC (permalink / raw)
To: Benjamin Tissoires, Stephen Boyd
Cc: Jiri Kosina, lkml, open list:HID CORE LAYER, Andy Shevchenko,
Dmitry Torokhov, Doug Anderson
In-Reply-To: <CAO-hwJJxSHqwEcypc-PVSDUoOSM4OHcOP18QJpoxq6OqKcbq-w@mail.gmail.com>
Hi,
On 22-06-18 09:16, Benjamin Tissoires wrote:
> On Fri, Jun 22, 2018 at 4:27 AM, Stephen Boyd <swboyd@chromium.org> wrote:
>> Use devm here to save some lines and prepare for bulk regulator usage in
>> this driver. Otherwise, when we devm bulk get regulators we'll free the
>> containing i2c_hid structure and try to put regulator pointers from
>> freed memory.
>>
>> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>> Cc: Hans de Goede <hdegoede@redhat.com>
>> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> Cc: Dmitry Torokhov <dtor@chromium.org>
>> Cc: Doug Anderson <dianders@chromium.org>
>> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
>> ---
>> drivers/hid/i2c-hid/i2c-hid.c | 9 +++------
>> 1 file changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
>> index c1652bb7bd15..c7d6738dc524 100644
>> --- a/drivers/hid/i2c-hid/i2c-hid.c
>> +++ b/drivers/hid/i2c-hid/i2c-hid.c
>> @@ -1002,18 +1002,18 @@ static int i2c_hid_probe(struct i2c_client *client,
>> return client->irq;
>> }
>>
>> - ihid = kzalloc(sizeof(struct i2c_hid), GFP_KERNEL);
>> + ihid = devm_kzalloc(&client->dev, sizeof(*ihid), GFP_KERNEL);
>
> IIRC, I never made the switch towards devm for i2c_hid because at the
> time there was a "all or nothing" rule regarding devm.
I'm not aware of any such rule. Sure ideally everything should use
devm, because it makes life just so much easier. But I've seen mixed
use in plenty of cases.
With that said converting fully to devm is not necessarily a bad
idea.
Regards,
Hans
> But given that the regulator already has a devm inside, I think we are
> screwed here and we should probably try to devm-ize the module.
>
> I seem to remember that someone posted a devm version of
> hid_allocate_device/hid-add_device, but I don't think this ever went
> upstream (because no use).
>
> Otherwise, for the series:
> Acked-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>
> Cheers,
> Benjamin
>
>
>
>> if (!ihid)
>> return -ENOMEM;
>>
>> if (client->dev.of_node) {
>> ret = i2c_hid_of_probe(client, &ihid->pdata);
>> if (ret)
>> - goto err;
>> + return ret;
>> } else if (!platform_data) {
>> ret = i2c_hid_acpi_pdata(client, &ihid->pdata);
>> if (ret)
>> - goto err;
>> + return ret;
>> } else {
>> ihid->pdata = *platform_data;
>> }
>> @@ -1126,7 +1126,6 @@ static int i2c_hid_probe(struct i2c_client *client,
>>
>> err:
>> i2c_hid_free_buffers(ihid);
>> - kfree(ihid);
>> return ret;
>> }
>>
>> @@ -1150,8 +1149,6 @@ static int i2c_hid_remove(struct i2c_client *client)
>>
>> regulator_disable(ihid->pdata.supply);
>>
>> - kfree(ihid);
>> -
>> return 0;
>> }
>>
>> --
>> Sent by a computer through tubes
>>
^ permalink raw reply
* Re: [PATCH v3 1/2] HID: i2c-hid: Use devm to allocate i2c_hid struct
From: Benjamin Tissoires @ 2018-06-22 7:16 UTC (permalink / raw)
To: Stephen Boyd
Cc: Jiri Kosina, lkml, open list:HID CORE LAYER, Hans de Goede,
Andy Shevchenko, Dmitry Torokhov, Doug Anderson
In-Reply-To: <20180622022717.134300-2-swboyd@chromium.org>
On Fri, Jun 22, 2018 at 4:27 AM, Stephen Boyd <swboyd@chromium.org> wrote:
> Use devm here to save some lines and prepare for bulk regulator usage in
> this driver. Otherwise, when we devm bulk get regulators we'll free the
> containing i2c_hid structure and try to put regulator pointers from
> freed memory.
>
> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Dmitry Torokhov <dtor@chromium.org>
> Cc: Doug Anderson <dianders@chromium.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
> drivers/hid/i2c-hid/i2c-hid.c | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
> index c1652bb7bd15..c7d6738dc524 100644
> --- a/drivers/hid/i2c-hid/i2c-hid.c
> +++ b/drivers/hid/i2c-hid/i2c-hid.c
> @@ -1002,18 +1002,18 @@ static int i2c_hid_probe(struct i2c_client *client,
> return client->irq;
> }
>
> - ihid = kzalloc(sizeof(struct i2c_hid), GFP_KERNEL);
> + ihid = devm_kzalloc(&client->dev, sizeof(*ihid), GFP_KERNEL);
IIRC, I never made the switch towards devm for i2c_hid because at the
time there was a "all or nothing" rule regarding devm.
But given that the regulator already has a devm inside, I think we are
screwed here and we should probably try to devm-ize the module.
I seem to remember that someone posted a devm version of
hid_allocate_device/hid-add_device, but I don't think this ever went
upstream (because no use).
Otherwise, for the series:
Acked-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cheers,
Benjamin
> if (!ihid)
> return -ENOMEM;
>
> if (client->dev.of_node) {
> ret = i2c_hid_of_probe(client, &ihid->pdata);
> if (ret)
> - goto err;
> + return ret;
> } else if (!platform_data) {
> ret = i2c_hid_acpi_pdata(client, &ihid->pdata);
> if (ret)
> - goto err;
> + return ret;
> } else {
> ihid->pdata = *platform_data;
> }
> @@ -1126,7 +1126,6 @@ static int i2c_hid_probe(struct i2c_client *client,
>
> err:
> i2c_hid_free_buffers(ihid);
> - kfree(ihid);
> return ret;
> }
>
> @@ -1150,8 +1149,6 @@ static int i2c_hid_remove(struct i2c_client *client)
>
> regulator_disable(ihid->pdata.supply);
>
> - kfree(ihid);
> -
> return 0;
> }
>
> --
> Sent by a computer through tubes
>
^ permalink raw reply
* Re: [RESEND PATCH] dt-bindings: input: Add common keyboard document bindings
From: Chen Zhong @ 2018-06-22 6:39 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Rob Herring, Mark Rutland, Matthias Brugger, Lee Jones,
linux-input, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek
In-Reply-To: <1529648198-14231-1-git-send-email-chen.zhong@mediatek.com>
Hi Dmitry,
The other patches in the series to add mediatek pmic keys support are
now available in 4.18-rc1, but this patch seems not being merged.
git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git
ib-mfd-input-rtc-v4.18
I have resend the patch, and rebase to 4.18-rc1.
Could you help to see if it is good enough to be merged into your tree?
Thank you.
On Fri, 2018-06-22 at 14:16 +0800, Chen Zhong wrote:
> This patch adds the device tree binding documentation for common
> keyboard.
>
> Acked-by: Rob Herring <robh@kernel.org>
> Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Signed-off-by: Chen Zhong <chen.zhong@mediatek.com>
> ---
> Documentation/devicetree/bindings/input/keys.txt | 8 ++++++++
> 1 file changed, 8 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/keys.txt
>
> diff --git a/Documentation/devicetree/bindings/input/keys.txt b/Documentation/devicetree/bindings/input/keys.txt
> new file mode 100644
> index 0000000..a0ea7eb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/keys.txt
> @@ -0,0 +1,8 @@
> +General Keys Properties:
> +
> +Optional properties for Keys:
> +- power-off-time-sec: Duration in seconds which the key should be kept
> + pressed for device to power off automatically. Device with key pressed
> + shutdown feature can specify this property.
> +- linux,keycodes: Specifies the numeric keycode values to be used for
> + reporting key presses.
> \ No newline at end of file
^ permalink raw reply
* [RESEND PATCH] dt-bindings: input: Add common keyboard document bindings
From: Chen Zhong @ 2018-06-22 6:16 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Rob Herring, Mark Rutland, Matthias Brugger, Lee Jones,
linux-input, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, Chen Zhong
This patch adds the device tree binding documentation for common
keyboard.
Acked-by: Rob Herring <robh@kernel.org>
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Chen Zhong <chen.zhong@mediatek.com>
---
Documentation/devicetree/bindings/input/keys.txt | 8 ++++++++
1 file changed, 8 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/keys.txt
diff --git a/Documentation/devicetree/bindings/input/keys.txt b/Documentation/devicetree/bindings/input/keys.txt
new file mode 100644
index 0000000..a0ea7eb
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/keys.txt
@@ -0,0 +1,8 @@
+General Keys Properties:
+
+Optional properties for Keys:
+- power-off-time-sec: Duration in seconds which the key should be kept
+ pressed for device to power off automatically. Device with key pressed
+ shutdown feature can specify this property.
+- linux,keycodes: Specifies the numeric keycode values to be used for
+ reporting key presses.
\ No newline at end of file
--
1.8.1.1.dirty
^ permalink raw reply related
* [PATCH v3 2/2] HID: i2c-hid: Add vddl regulator control
From: Stephen Boyd @ 2018-06-22 2:27 UTC (permalink / raw)
To: Jiri Kosina
Cc: linux-kernel, linux-input, Benjamin Tissoires, Hans de Goede,
Andy Shevchenko, Dmitry Torokhov, Doug Anderson, devicetree
In-Reply-To: <20180622022717.134300-1-swboyd@chromium.org>
Some wacom w9013 devices have a vddl supply for "low valtage"
requirements. Add support in this driver to turn on this low voltage
supply. We can also drop a handful of error messages because the
regulator core is already printing an error when bulk regulators fail to
enable or disable.
Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Dmitry Torokhov <dtor@chromium.org>
Cc: Doug Anderson <dianders@chromium.org>
Acked-by: Rob Herring <robh@kernel.org>
Cc: <devicetree@vger.kernel.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
.../bindings/input/hid-over-i2c.txt | 3 +-
drivers/hid/i2c-hid/i2c-hid.c | 48 +++++++++----------
include/linux/platform_data/i2c-hid.h | 7 ++-
3 files changed, 29 insertions(+), 29 deletions(-)
diff --git a/Documentation/devicetree/bindings/input/hid-over-i2c.txt b/Documentation/devicetree/bindings/input/hid-over-i2c.txt
index 4d3da9d91de4..89e6ab89ba38 100644
--- a/Documentation/devicetree/bindings/input/hid-over-i2c.txt
+++ b/Documentation/devicetree/bindings/input/hid-over-i2c.txt
@@ -26,7 +26,8 @@ device-specific compatible properties, which should be used in addition to the
- compatible:
* "wacom,w9013" (Wacom W9013 digitizer). Supports:
- - vdd-supply
+ - vdd-supply (3.3V)
+ - vddl-supply (1.8V)
- post-power-on-delay-ms
- vdd-supply: phandle of the regulator that provides the supply voltage.
diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index c7d6738dc524..4f532d9238fb 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -1021,21 +1021,20 @@ static int i2c_hid_probe(struct i2c_client *client,
/* Parse platform agnostic common properties from ACPI / device tree */
i2c_hid_fwnode_probe(client, &ihid->pdata);
- ihid->pdata.supply = devm_regulator_get(&client->dev, "vdd");
- if (IS_ERR(ihid->pdata.supply)) {
- ret = PTR_ERR(ihid->pdata.supply);
- if (ret != -EPROBE_DEFER)
- dev_err(&client->dev, "Failed to get regulator: %d\n",
- ret);
- goto err;
- }
+ ihid->pdata.supplies[0].supply = "vdd";
+ ihid->pdata.supplies[1].supply = "vddl";
+
+ ret = devm_regulator_bulk_get(&client->dev,
+ ARRAY_SIZE(ihid->pdata.supplies),
+ ihid->pdata.supplies);
+ if (ret)
+ return ret;
+
+ ret = regulator_bulk_enable(ARRAY_SIZE(ihid->pdata.supplies),
+ ihid->pdata.supplies);
+ if (ret < 0)
+ return ret;
- ret = regulator_enable(ihid->pdata.supply);
- if (ret < 0) {
- dev_err(&client->dev, "Failed to enable regulator: %d\n",
- ret);
- goto err;
- }
if (ihid->pdata.post_power_delay_ms)
msleep(ihid->pdata.post_power_delay_ms);
@@ -1122,9 +1121,8 @@ static int i2c_hid_probe(struct i2c_client *client,
pm_runtime_disable(&client->dev);
err_regulator:
- regulator_disable(ihid->pdata.supply);
-
-err:
+ regulator_bulk_disable(ARRAY_SIZE(ihid->pdata.supplies),
+ ihid->pdata.supplies);
i2c_hid_free_buffers(ihid);
return ret;
}
@@ -1147,7 +1145,8 @@ static int i2c_hid_remove(struct i2c_client *client)
if (ihid->bufsize)
i2c_hid_free_buffers(ihid);
- regulator_disable(ihid->pdata.supply);
+ regulator_bulk_disable(ARRAY_SIZE(ihid->pdata.supplies),
+ ihid->pdata.supplies);
return 0;
}
@@ -1198,9 +1197,8 @@ static int i2c_hid_suspend(struct device *dev)
hid_warn(hid, "Failed to enable irq wake: %d\n",
wake_status);
} else {
- ret = regulator_disable(ihid->pdata.supply);
- if (ret < 0)
- hid_warn(hid, "Failed to disable supply: %d\n", ret);
+ regulator_bulk_disable(ARRAY_SIZE(ihid->pdata.supplies),
+ ihid->pdata.supplies);
}
return 0;
@@ -1215,9 +1213,11 @@ static int i2c_hid_resume(struct device *dev)
int wake_status;
if (!device_may_wakeup(&client->dev)) {
- ret = regulator_enable(ihid->pdata.supply);
- if (ret < 0)
- hid_warn(hid, "Failed to enable supply: %d\n", ret);
+ ret = regulator_bulk_enable(ARRAY_SIZE(ihid->pdata.supplies),
+ ihid->pdata.supplies);
+ if (ret)
+ hid_warn(hid, "Failed to enable supplies: %d\n", ret);
+
if (ihid->pdata.post_power_delay_ms)
msleep(ihid->pdata.post_power_delay_ms);
} else if (ihid->irq_wake_enabled) {
diff --git a/include/linux/platform_data/i2c-hid.h b/include/linux/platform_data/i2c-hid.h
index 1fb088239d12..c628bb5e1061 100644
--- a/include/linux/platform_data/i2c-hid.h
+++ b/include/linux/platform_data/i2c-hid.h
@@ -12,14 +12,13 @@
#ifndef __LINUX_I2C_HID_H
#define __LINUX_I2C_HID_H
+#include <linux/regulator/consumer.h>
#include <linux/types.h>
-struct regulator;
-
/**
* struct i2chid_platform_data - used by hid over i2c implementation.
* @hid_descriptor_address: i2c register where the HID descriptor is stored.
- * @supply: regulator for powering on the device.
+ * @supplies: regulators for powering on the device.
* @post_power_delay_ms: delay after powering on before device is usable.
*
* Note that it is the responsibility of the platform driver (or the acpi 5.0
@@ -35,7 +34,7 @@ struct regulator;
*/
struct i2c_hid_platform_data {
u16 hid_descriptor_address;
- struct regulator *supply;
+ struct regulator_bulk_data supplies[2];
int post_power_delay_ms;
};
--
Sent by a computer through tubes
^ permalink raw reply related
* [PATCH v3 1/2] HID: i2c-hid: Use devm to allocate i2c_hid struct
From: Stephen Boyd @ 2018-06-22 2:27 UTC (permalink / raw)
To: Jiri Kosina
Cc: linux-kernel, linux-input, Benjamin Tissoires, Hans de Goede,
Andy Shevchenko, Dmitry Torokhov, Doug Anderson
In-Reply-To: <20180622022717.134300-1-swboyd@chromium.org>
Use devm here to save some lines and prepare for bulk regulator usage in
this driver. Otherwise, when we devm bulk get regulators we'll free the
containing i2c_hid structure and try to put regulator pointers from
freed memory.
Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Dmitry Torokhov <dtor@chromium.org>
Cc: Doug Anderson <dianders@chromium.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
drivers/hid/i2c-hid/i2c-hid.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index c1652bb7bd15..c7d6738dc524 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -1002,18 +1002,18 @@ static int i2c_hid_probe(struct i2c_client *client,
return client->irq;
}
- ihid = kzalloc(sizeof(struct i2c_hid), GFP_KERNEL);
+ ihid = devm_kzalloc(&client->dev, sizeof(*ihid), GFP_KERNEL);
if (!ihid)
return -ENOMEM;
if (client->dev.of_node) {
ret = i2c_hid_of_probe(client, &ihid->pdata);
if (ret)
- goto err;
+ return ret;
} else if (!platform_data) {
ret = i2c_hid_acpi_pdata(client, &ihid->pdata);
if (ret)
- goto err;
+ return ret;
} else {
ihid->pdata = *platform_data;
}
@@ -1126,7 +1126,6 @@ static int i2c_hid_probe(struct i2c_client *client,
err:
i2c_hid_free_buffers(ihid);
- kfree(ihid);
return ret;
}
@@ -1150,8 +1149,6 @@ static int i2c_hid_remove(struct i2c_client *client)
regulator_disable(ihid->pdata.supply);
- kfree(ihid);
-
return 0;
}
--
Sent by a computer through tubes
^ permalink raw reply related
* [PATCH v3 0/2] Add another regulator to i2c-hid
From: Stephen Boyd @ 2018-06-22 2:27 UTC (permalink / raw)
To: Jiri Kosina
Cc: linux-kernel, linux-input, Benjamin Tissoires, Hans de Goede,
Andy Shevchenko, Dmitry Torokhov, Doug Anderson
The wacom w9013 device has two supplies that need to be turned
on and in some cases these come from two different power supplies.
This patch series supports that case.
Changes from v1:
* Use the bulk regulator APIs instead of open coding it (Dmitry)
Changes from v2:
* Split out devm for allocation into another patch (Dmitry)
* Drop errant error print that snuck in across port
Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Dmitry Torokhov <dtor@chromium.org>
Cc: Doug Anderson <dianders@chromium.org>
Stephen Boyd (2):
HID: i2c-hid: Use devm to allocate i2c_hid struct
HID: i2c-hid: Add vddl regulator control
.../bindings/input/hid-over-i2c.txt | 3 +-
drivers/hid/i2c-hid/i2c-hid.c | 57 +++++++++----------
include/linux/platform_data/i2c-hid.h | 7 +--
3 files changed, 32 insertions(+), 35 deletions(-)
--
Sent by a computer through tubes
^ permalink raw reply
* Re: [PATCH v2] HID: i2c-hid: Add vddl regulator control
From: Stephen Boyd @ 2018-06-22 1:33 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Jiri Kosina, lkml, open list:HID CORE LAYER, Benjamin Tissoires,
Hans de Goede, andriy.shevchenko, Doug Anderson, devicetree
In-Reply-To: <CAE_wzQ8YKUF1U4MKBfi9GQpFz=971j57w-1n-ggZHidMA4HEzQ@mail.gmail.com>
Quoting Dmitry Torokhov (2018-06-21 16:58:31)
> On Thu, Jun 21, 2018 at 4:41 PM Stephen Boyd <swboyd@chromium.org> wrote:
> >
> > Some wacom w9013 devices have a vddl supply for "low valtage"
> > requirements. Add support in this driver to turn on this low voltage
> > supply. We can also drop a handful of error messages because the
> > regulator core is already printing an error when bulk regulators fail to
> > enable or disable.
> >
> > Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > Cc: Hans de Goede <hdegoede@redhat.com>
> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Cc: Dmitry Torokhov <dtor@chromium.org>
> > Cc: Doug Anderson <dianders@chromium.org>
> > Acked-by: Rob Herring <robh@kernel.org>
> > Cc: <devicetree@vger.kernel.org>
> > Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> > ---
> >
> > Changes from v1:
> > * Use the bulk regulator APIs instead of open coding it (Dmitry)
> >
> > .../bindings/input/hid-over-i2c.txt | 3 +-
> > drivers/hid/i2c-hid/i2c-hid.c | 62 +++++++++----------
> > include/linux/platform_data/i2c-hid.h | 7 +--
> > 3 files changed, 36 insertions(+), 36 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/input/hid-over-i2c.txt b/Documentation/devicetree/bindings/input/hid-over-i2c.txt
> > index 4d3da9d91de4..89e6ab89ba38 100644
> > --- a/Documentation/devicetree/bindings/input/hid-over-i2c.txt
> > +++ b/Documentation/devicetree/bindings/input/hid-over-i2c.txt
> > @@ -26,7 +26,8 @@ device-specific compatible properties, which should be used in addition to the
> >
> > - compatible:
> > * "wacom,w9013" (Wacom W9013 digitizer). Supports:
> > - - vdd-supply
> > + - vdd-supply (3.3V)
> > + - vddl-supply (1.8V)
> > - post-power-on-delay-ms
> >
> > - vdd-supply: phandle of the regulator that provides the supply voltage.
> > diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
> > index c1652bb7bd15..1d83531454c5 100644
> > --- a/drivers/hid/i2c-hid/i2c-hid.c
> > +++ b/drivers/hid/i2c-hid/i2c-hid.c
> > @@ -1002,18 +1002,21 @@ static int i2c_hid_probe(struct i2c_client *client,
> > return client->irq;
> > }
> >
> > - ihid = kzalloc(sizeof(struct i2c_hid), GFP_KERNEL);
> > + ihid = devm_kzalloc(&client->dev, sizeof(*ihid), GFP_KERNEL);
>
> The patch looks good, but I think this chunk needs to be split out.
>
Ok. It needs to be done otherwise the regulator put with devm blows up
when the containing structure is freed underneath it.
^ permalink raw reply
* Re: [PATCH] Input: elantech - Fix V4 report decoding for module with middle key
From: 'Dmitry Torokhov' @ 2018-06-22 0:16 UTC (permalink / raw)
To: 廖崇榮
Cc: linux-kernel, linux-input, ulrik.debie-os, phoenix, aaron.ma,
josh.chen
In-Reply-To: <000301d3f7e4$5c9e51c0$15daf540$@emc.com.tw>
On Wed, May 30, 2018 at 03:03:50PM +0800, 廖崇榮 wrote:
> Hi Dmitry,
>
> -----Original Message-----
> From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
> Sent: Wednesday, May 30, 2018 2:05 AM
> To: KT Liao
> Cc: linux-kernel@vger.kernel.org; linux-input@vger.kernel.org;
> ulrik.debie-os@e2big.org; phoenix@emc.com.tw; aaron.ma@canonical.com;
> josh.chen@emc.com.tw
> Subject: Re: [PATCH] Input: elantech - Fix V4 report decoding for module
> with middle key
>
> Hi KT,
>
> On Mon, May 28, 2018 at 07:33:02PM +0800, KT Liao wrote:
> > Some touchpad has middle key and it will be indicated in bit 2 of
> packet[0].
> > We need to fix V4 formation's byte mask to prevent error decoding.
>
> Could you please let me know what devices this patch fixes? Are they
> released or new hardware?
>
>
> The primary target is Lenovo thinkpad P52 and it will be released in 6/M.
>
> force_crc_enabled will fix the issue too because less bit-check in the
> specific byte.
> I guess Fujitsu H730/H760 in elantech_dmi_force_crc_enabled may have the
> same issue.
> I leave them in DMI table because I am not sure of it.
OK, thank you for the info. Applied.
>
> Thanks
> KT
> >
> > Signed-off-by: KT Liao <kt.liao@emc.com.tw>
> > ---
> > drivers/input/mouse/elantech.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/input/mouse/elantech.c
> > b/drivers/input/mouse/elantech.c index fb4d902..f39dc66 100644
> > --- a/drivers/input/mouse/elantech.c
> > +++ b/drivers/input/mouse/elantech.c
> > @@ -799,7 +799,7 @@ static int elantech_packet_check_v4(struct psmouse
> *psmouse)
> > else if (ic_version == 7 && etd->info.samples[1] == 0x2A)
> > sanity_check = ((packet[3] & 0x1c) == 0x10);
> > else
> > - sanity_check = ((packet[0] & 0x0c) == 0x04 &&
> > + sanity_check = ((packet[0] & 0x08) == 0x00 &&
> > (packet[3] & 0x1c) == 0x10);
> >
> > if (!sanity_check)
> > --
> > 2.7.4
> >
>
> Thanks.
>
> --
> Dmitry
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2 10/10] Input: ams_delta_serio: Get FIQ buffer from platform_data
From: Dmitry Torokhov @ 2018-06-22 0:11 UTC (permalink / raw)
To: Janusz Krzysztofik
Cc: Tony Lindgren, Aaro Koskinen, David S . Miller ,
Mauro Carvalho Chehab, Greg Kroah-Hartman, Andrew Morton,
Randy Dunlap, Linus Walleij, Mark Brown, Liam Girdwood,
linux-arm-kernel, linux-omap, linux-input, linux-gpio,
linux-kernel
In-Reply-To: <20180621224128.17623-10-jmkrzyszt@gmail.com>
On Fri, Jun 22, 2018 at 12:41:28AM +0200, Janusz Krzysztofik wrote:
> Instead of exporting the FIQ buffer symbol to be used in
> ams-delta-serio driver, pass it to the driver as platform_data.
>
> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
Input bits look good.
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> Changelog:
> v2: rebased on v4.18-rc1, no conflicts
>
> arch/arm/mach-omap1/ams-delta-fiq.c | 6 +++---
> arch/arm/mach-omap1/board-ams-delta.c | 8 ++++++++
> drivers/input/serio/ams_delta_serio.c | 20 +++++++++++++-------
> include/linux/platform_data/ams-delta-fiq.h | 4 ----
> 4 files changed, 24 insertions(+), 14 deletions(-)
>
> diff --git a/arch/arm/mach-omap1/ams-delta-fiq.c b/arch/arm/mach-omap1/ams-delta-fiq.c
> index 82ca4246a5e4..b0dc7ddf5877 100644
> --- a/arch/arm/mach-omap1/ams-delta-fiq.c
> +++ b/arch/arm/mach-omap1/ams-delta-fiq.c
> @@ -40,8 +40,7 @@ static struct fiq_handler fh = {
> * keystrokes received from the qwerty keyboard. See
> * <linux/platform_data/ams-delta-fiq.h> for details of offsets.
> */
> -unsigned int fiq_buffer[1024];
> -EXPORT_SYMBOL(fiq_buffer);
> +static unsigned int fiq_buffer[1024];
>
> static struct irq_chip *irq_chip;
> static struct irq_data *irq_data[16];
> @@ -203,9 +202,10 @@ void __init ams_delta_init_fiq(struct gpio_chip *chip,
> val = omap_readl(OMAP_IH1_BASE + offset) | 1;
> omap_writel(val, OMAP_IH1_BASE + offset);
>
> - /* Initialize serio device IRQ resource */
> + /* Initialize serio device IRQ resource and platform_data */
> serio->resource[0].start = gpiod_to_irq(clk);
> serio->resource[0].end = serio->resource[0].start;
> + serio->dev.platform_data = fiq_buffer;
>
> /*
> * Since FIQ handler performs handling of GPIO registers for
> diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
> index 84177ba3e39a..772892487827 100644
> --- a/arch/arm/mach-omap1/board-ams-delta.c
> +++ b/arch/arm/mach-omap1/board-ams-delta.c
> @@ -520,6 +520,14 @@ static struct resource ams_delta_serio_resources[] = {
> static struct platform_device ams_delta_serio_device = {
> .name = "ams-delta-serio",
> .id = PLATFORM_DEVID_NONE,
> + .dev = {
> + /*
> + * Initialize .platform_data explicitly with NULL to
> + * indicate it is going to be used. It will be replaced
> + * with FIQ buffer address as soon as FIQ is initialized.
> + */
> + .platform_data = NULL,
> + },
> .num_resources = ARRAY_SIZE(ams_delta_serio_resources),
> .resource = ams_delta_serio_resources,
> };
> diff --git a/drivers/input/serio/ams_delta_serio.c b/drivers/input/serio/ams_delta_serio.c
> index c1f8226f172e..f8663d7891f2 100644
> --- a/drivers/input/serio/ams_delta_serio.c
> +++ b/drivers/input/serio/ams_delta_serio.c
> @@ -37,6 +37,7 @@ MODULE_LICENSE("GPL");
> struct ams_delta_serio {
> struct serio *serio;
> struct regulator *vcc;
> + unsigned int *fiq_buffer;
> };
>
> static int check_data(struct serio *serio, int data)
> @@ -66,22 +67,23 @@ static int check_data(struct serio *serio, int data)
> static irqreturn_t ams_delta_serio_interrupt(int irq, void *dev_id)
> {
> struct ams_delta_serio *priv = dev_id;
> - int *circ_buff = &fiq_buffer[FIQ_CIRC_BUFF];
> + int *circ_buff = &priv->fiq_buffer[FIQ_CIRC_BUFF];
> int data, dfl;
> u8 scancode;
>
> - fiq_buffer[FIQ_IRQ_PEND] = 0;
> + priv->fiq_buffer[FIQ_IRQ_PEND] = 0;
>
> /*
> * Read data from the circular buffer, check it
> * and then pass it on the serio
> */
> - while (fiq_buffer[FIQ_KEYS_CNT] > 0) {
> + while (priv->fiq_buffer[FIQ_KEYS_CNT] > 0) {
>
> - data = circ_buff[fiq_buffer[FIQ_HEAD_OFFSET]++];
> - fiq_buffer[FIQ_KEYS_CNT]--;
> - if (fiq_buffer[FIQ_HEAD_OFFSET] == fiq_buffer[FIQ_BUF_LEN])
> - fiq_buffer[FIQ_HEAD_OFFSET] = 0;
> + data = circ_buff[priv->fiq_buffer[FIQ_HEAD_OFFSET]++];
> + priv->fiq_buffer[FIQ_KEYS_CNT]--;
> + if (priv->fiq_buffer[FIQ_HEAD_OFFSET] ==
> + priv->fiq_buffer[FIQ_BUF_LEN])
> + priv->fiq_buffer[FIQ_HEAD_OFFSET] = 0;
>
> dfl = check_data(priv->serio, data);
> scancode = (u8) (data >> 1) & 0xFF;
> @@ -116,6 +118,10 @@ static int ams_delta_serio_init(struct platform_device *pdev)
> if (!priv)
> return -ENOMEM;
>
> + priv->fiq_buffer = pdev->dev.platform_data;
> + if (!priv->fiq_buffer)
> + return -EINVAL;
> +
> priv->vcc = devm_regulator_get(&pdev->dev, "vcc");
> if (IS_ERR(priv->vcc)) {
> err = PTR_ERR(priv->vcc);
> diff --git a/include/linux/platform_data/ams-delta-fiq.h b/include/linux/platform_data/ams-delta-fiq.h
> index dc0f835ea918..cf4589ccb720 100644
> --- a/include/linux/platform_data/ams-delta-fiq.h
> +++ b/include/linux/platform_data/ams-delta-fiq.h
> @@ -55,8 +55,4 @@
>
> #define FIQ_CIRC_BUFF 30 /*Start of circular buffer */
>
> -#ifndef __ASSEMBLER__
> -extern unsigned int fiq_buffer[];
> -#endif
> -
> #endif
> --
> 2.16.4
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2 09/10] Input: ams_delta_serio: use IRQ resource
From: Dmitry Torokhov @ 2018-06-22 0:11 UTC (permalink / raw)
To: Janusz Krzysztofik
Cc: Tony Lindgren, Aaro Koskinen, David S . Miller ,
Mauro Carvalho Chehab, Greg Kroah-Hartman, Andrew Morton,
Randy Dunlap, Linus Walleij, Mark Brown, Liam Girdwood,
linux-arm-kernel, linux-omap, linux-input, linux-gpio,
linux-kernel
In-Reply-To: <20180621224128.17623-9-jmkrzyszt@gmail.com>
On Fri, Jun 22, 2018 at 12:41:27AM +0200, Janusz Krzysztofik wrote:
> The driver still obtains IRQ number from a hardcoded GPIO. Use IRQ
> resource instead.
>
> For this to work on Amstrad Delta, add the IRQ resource to
> ams-delta-serio platform device structure. Obtain the IRQ number
> assigned to "keybrd_clk" GPIO pin from FIQ initialization routine.
>
> As a benefit, the driver no longer needs to include
> <mach/board-ams-delta.h>.
>
> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
Input bits look good.
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> Changelog:
> v2: - obsolete assignment of handle_simple_irq() removed from the driver
> and replaced with a comment in the FIQ init file,
> - rebased on v4.18-rc1, no conflicts.
>
> arch/arm/mach-omap1/ams-delta-fiq.c | 20 +++++++++++++++++++-
> arch/arm/mach-omap1/ams-delta-fiq.h | 3 ++-
> arch/arm/mach-omap1/board-ams-delta.c | 17 ++++++++++++++++-
> drivers/input/serio/ams_delta_serio.c | 32 +++++++++-----------------------
> 4 files changed, 46 insertions(+), 26 deletions(-)
>
> diff --git a/arch/arm/mach-omap1/ams-delta-fiq.c b/arch/arm/mach-omap1/ams-delta-fiq.c
> index e72935034d42..82ca4246a5e4 100644
> --- a/arch/arm/mach-omap1/ams-delta-fiq.c
> +++ b/arch/arm/mach-omap1/ams-delta-fiq.c
> @@ -20,6 +20,7 @@
> #include <linux/module.h>
> #include <linux/io.h>
> #include <linux/platform_data/ams-delta-fiq.h>
> +#include <linux/platform_device.h>
>
> #include <mach/board-ams-delta.h>
>
> @@ -84,7 +85,8 @@ static irqreturn_t deferred_fiq(int irq, void *dev_id)
> return IRQ_HANDLED;
> }
>
> -void __init ams_delta_init_fiq(struct gpio_chip *chip)
> +void __init ams_delta_init_fiq(struct gpio_chip *chip,
> + struct platform_device *serio)
> {
> struct gpio_desc *gpiod, *data = NULL, *clk = NULL;
> void *fiqhandler_start;
> @@ -201,6 +203,22 @@ void __init ams_delta_init_fiq(struct gpio_chip *chip)
> val = omap_readl(OMAP_IH1_BASE + offset) | 1;
> omap_writel(val, OMAP_IH1_BASE + offset);
>
> + /* Initialize serio device IRQ resource */
> + serio->resource[0].start = gpiod_to_irq(clk);
> + serio->resource[0].end = serio->resource[0].start;
> +
> + /*
> + * Since FIQ handler performs handling of GPIO registers for
> + * "keybrd_clk" IRQ pin, ams_delta_serio driver used to set
> + * handle_simple_irq() as active IRQ handler for that pin to avoid
> + * bad interaction with gpio-omap driver. This is no longer needed
> + * as handle_simple_irq() is now the default handler for OMAP GPIO
> + * edge interrupts.
> + * This comment replaces the obsolete code which has been removed
> + * from the ams_delta_serio driver and stands here only as a reminder
> + * of that dependency on gpio-omap driver behavior.
> + */
> +
> return;
>
> out_gpio:
> diff --git a/arch/arm/mach-omap1/ams-delta-fiq.h b/arch/arm/mach-omap1/ams-delta-fiq.h
> index 3f691d68aa62..fd76df3cce37 100644
> --- a/arch/arm/mach-omap1/ams-delta-fiq.h
> +++ b/arch/arm/mach-omap1/ams-delta-fiq.h
> @@ -35,7 +35,8 @@
> #ifndef __ASSEMBLER__
> extern unsigned char qwerty_fiqin_start, qwerty_fiqin_end;
>
> -extern void __init ams_delta_init_fiq(struct gpio_chip *chip);
> +extern void __init ams_delta_init_fiq(struct gpio_chip *chip,
> + struct platform_device *pdev);
> #endif
>
> #endif
> diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
> index fe9a3e7cbfeb..84177ba3e39a 100644
> --- a/arch/arm/mach-omap1/board-ams-delta.c
> +++ b/arch/arm/mach-omap1/board-ams-delta.c
> @@ -504,9 +504,24 @@ static struct platform_device cx20442_codec_device = {
> .id = -1,
> };
>
> +static struct resource ams_delta_serio_resources[] = {
> + {
> + .flags = IORESOURCE_IRQ,
> + /*
> + * Initialize IRQ resource with invalid IRQ number.
> + * It will be replaced with dynamically allocated GPIO IRQ
> + * obtained from GPIO chip as soon as the chip is available.
> + */
> + .start = -EINVAL,
> + .end = -EINVAL,
> + },
> +};
> +
> static struct platform_device ams_delta_serio_device = {
> .name = "ams-delta-serio",
> .id = PLATFORM_DEVID_NONE,
> + .num_resources = ARRAY_SIZE(ams_delta_serio_resources),
> + .resource = ams_delta_serio_resources,
> };
>
> static struct regulator_consumer_supply keybrd_pwr_consumers[] = {
> @@ -615,7 +630,7 @@ static void __init omap_gpio_deps_init(void)
> return;
> }
>
> - ams_delta_init_fiq(chip);
> + ams_delta_init_fiq(chip, &ams_delta_serio_device);
> }
>
> static void __init ams_delta_init(void)
> diff --git a/drivers/input/serio/ams_delta_serio.c b/drivers/input/serio/ams_delta_serio.c
> index 2602f7cff5ae..c1f8226f172e 100644
> --- a/drivers/input/serio/ams_delta_serio.c
> +++ b/drivers/input/serio/ams_delta_serio.c
> @@ -20,7 +20,6 @@
> * However, when used with the E3 mailboard that producecs non-standard
> * scancodes, a custom key table must be prepared and loaded from userspace.
> */
> -#include <linux/gpio.h>
> #include <linux/irq.h>
> #include <linux/platform_data/ams-delta-fiq.h>
> #include <linux/platform_device.h>
> @@ -29,8 +28,6 @@
> #include <linux/slab.h>
> #include <linux/module.h>
>
> -#include <mach/board-ams-delta.h>
> -
> #define DRIVER_NAME "ams-delta-serio"
>
> MODULE_AUTHOR("Matt Callow");
> @@ -113,7 +110,7 @@ static int ams_delta_serio_init(struct platform_device *pdev)
> {
> struct ams_delta_serio *priv;
> struct serio *serio;
> - int err;
> + int irq, err;
>
> priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> if (!priv)
> @@ -139,26 +136,20 @@ static int ams_delta_serio_init(struct platform_device *pdev)
> return err;
> }
>
> - err = request_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK),
> - ams_delta_serio_interrupt, IRQ_TYPE_EDGE_RISING,
> - DRIVER_NAME, priv);
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0)
> + return -ENXIO;
> +
> + err = devm_request_irq(&pdev->dev, irq, ams_delta_serio_interrupt,
> + IRQ_TYPE_EDGE_RISING, DRIVER_NAME, priv);
> if (err < 0) {
> dev_err(&pdev->dev, "IRQ request failed (%d)\n", err);
> return err;
> }
> - /*
> - * Since GPIO register handling for keyboard clock pin is performed
> - * at FIQ level, switch back from edge to simple interrupt handler
> - * to avoid bad interaction.
> - */
> - irq_set_handler(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK),
> - handle_simple_irq);
>
> serio = kzalloc(sizeof(*serio), GFP_KERNEL);
> - if (!serio) {
> - err = -ENOMEM;
> - goto irq;
> - }
> + if (!serio)
> + return -ENOMEM;
>
> priv->serio = serio;
>
> @@ -177,10 +168,6 @@ static int ams_delta_serio_init(struct platform_device *pdev)
> dev_info(&serio->dev, "%s\n", serio->name);
>
> return 0;
> -
> -irq:
> - free_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK), priv);
> - return err;
> }
>
> static int ams_delta_serio_exit(struct platform_device *pdev)
> @@ -188,7 +175,6 @@ static int ams_delta_serio_exit(struct platform_device *pdev)
> struct ams_delta_serio *priv = platform_get_drvdata(pdev);
>
> serio_unregister_port(priv->serio);
> - free_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK), 0);
>
> return 0;
> }
> --
> 2.16.4
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2 08/10] ARM: OMAP1: Get rid of <mach/ams-delta-fiq.h>
From: Dmitry Torokhov @ 2018-06-22 0:10 UTC (permalink / raw)
To: Janusz Krzysztofik
Cc: Tony Lindgren, Aaro Koskinen, David S . Miller ,
Mauro Carvalho Chehab, Greg Kroah-Hartman, Andrew Morton,
Randy Dunlap, Linus Walleij, Mark Brown, Liam Girdwood,
linux-arm-kernel, linux-omap, linux-input, linux-gpio,
linux-kernel
In-Reply-To: <20180621224128.17623-8-jmkrzyszt@gmail.com>
On Fri, Jun 22, 2018 at 12:41:26AM +0200, Janusz Krzysztofik wrote:
> Split the header file into two parts and move them to directories where
> they belong.
>
> Information on internal structure of FIQ buffer is moved to
> <linux/platform_data/ams-delta-fiq.h> for ams-delta-serio driver use.
>
> Other information used by ams-delta board init file and FIQ code is
> made local to mach-omap1 root directory.
>
> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
Input bits look good.
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> Changelog:
> v2: rebased on v4.18-rc1, no conflicts
>
> MAINTAINERS | 1 +
> arch/arm/mach-omap1/ams-delta-fiq-handler.S | 5 +--
> arch/arm/mach-omap1/ams-delta-fiq.c | 7 ++--
> arch/arm/mach-omap1/ams-delta-fiq.h | 41 ++++++++++++++++++++++
> arch/arm/mach-omap1/board-ams-delta.c | 2 +-
> drivers/input/serio/ams_delta_serio.c | 3 +-
> .../linux/platform_data}/ams-delta-fiq.h | 27 +++-----------
> 7 files changed, 56 insertions(+), 30 deletions(-)
> create mode 100644 arch/arm/mach-omap1/ams-delta-fiq.h
> rename {arch/arm/mach-omap1/include/mach => include/linux/platform_data}/ams-delta-fiq.h (71%)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 9d5eeff51b5f..c78b7dc42a81 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10391,6 +10391,7 @@ F: arch/arm/plat-omap/
> F: arch/arm/configs/omap1_defconfig
> F: drivers/i2c/busses/i2c-omap.c
> F: include/linux/platform_data/i2c-omap.h
> +F: include/linux/platform_data/ams-delta-fiq.h
>
> OMAP2+ SUPPORT
> M: Tony Lindgren <tony@atomide.com>
> diff --git a/arch/arm/mach-omap1/ams-delta-fiq-handler.S b/arch/arm/mach-omap1/ams-delta-fiq-handler.S
> index bf608441b357..ddc27638ba2a 100644
> --- a/arch/arm/mach-omap1/ams-delta-fiq-handler.S
> +++ b/arch/arm/mach-omap1/ams-delta-fiq-handler.S
> @@ -14,11 +14,12 @@
> */
>
> #include <linux/linkage.h>
> -#include <asm/assembler.h>
> +#include <linux/platform_data/ams-delta-fiq.h>
>
> +#include <asm/assembler.h>
> #include <mach/board-ams-delta.h>
> -#include <mach/ams-delta-fiq.h>
>
> +#include "ams-delta-fiq.h"
> #include "iomap.h"
> #include "soc.h"
>
> diff --git a/arch/arm/mach-omap1/ams-delta-fiq.c b/arch/arm/mach-omap1/ams-delta-fiq.c
> index 5a6c59ac9b5f..e72935034d42 100644
> --- a/arch/arm/mach-omap1/ams-delta-fiq.c
> +++ b/arch/arm/mach-omap1/ams-delta-fiq.c
> @@ -19,12 +19,13 @@
> #include <linux/irq.h>
> #include <linux/module.h>
> #include <linux/io.h>
> +#include <linux/platform_data/ams-delta-fiq.h>
>
> #include <mach/board-ams-delta.h>
>
> #include <asm/fiq.h>
>
> -#include <mach/ams-delta-fiq.h>
> +#include "ams-delta-fiq.h"
>
> static struct fiq_handler fh = {
> .name = "ams-delta-fiq"
> @@ -35,8 +36,8 @@ static struct fiq_handler fh = {
> * The FIQ and IRQ isrs can both read and write it.
> * It is structured as a header section several 32bit slots,
> * followed by the circular buffer where the FIQ isr stores
> - * keystrokes received from the qwerty keyboard.
> - * See ams-delta-fiq.h for details of offsets.
> + * keystrokes received from the qwerty keyboard. See
> + * <linux/platform_data/ams-delta-fiq.h> for details of offsets.
> */
> unsigned int fiq_buffer[1024];
> EXPORT_SYMBOL(fiq_buffer);
> diff --git a/arch/arm/mach-omap1/ams-delta-fiq.h b/arch/arm/mach-omap1/ams-delta-fiq.h
> new file mode 100644
> index 000000000000..3f691d68aa62
> --- /dev/null
> +++ b/arch/arm/mach-omap1/ams-delta-fiq.h
> @@ -0,0 +1,41 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +/*
> + * arch/arm/mach-omap1/ams-delta-fiq.h
> + *
> + * Taken from the original Amstrad modifications to fiq.h
> + *
> + * Copyright (c) 2004 Amstrad Plc
> + * Copyright (c) 2006 Matt Callow
> + * Copyright (c) 2010 Janusz Krzysztofik
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#ifndef __AMS_DELTA_FIQ_H
> +#define __AMS_DELTA_FIQ_H
> +
> +#include <mach/irqs.h>
> +
> +/*
> + * Interrupt number used for passing control from FIQ to IRQ.
> + * IRQ12, described as reserved, has been selected.
> + */
> +#define INT_DEFERRED_FIQ INT_1510_RES12
> +/*
> + * Base address of an interrupt handler that the INT_DEFERRED_FIQ belongs to.
> + */
> +#if (INT_DEFERRED_FIQ < IH2_BASE)
> +#define DEFERRED_FIQ_IH_BASE OMAP_IH1_BASE
> +#else
> +#define DEFERRED_FIQ_IH_BASE OMAP_IH2_BASE
> +#endif
> +
> +#ifndef __ASSEMBLER__
> +extern unsigned char qwerty_fiqin_start, qwerty_fiqin_end;
> +
> +extern void __init ams_delta_init_fiq(struct gpio_chip *chip);
> +#endif
> +
> +#endif
> diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
> index f15c0793c34b..fe9a3e7cbfeb 100644
> --- a/arch/arm/mach-omap1/board-ams-delta.c
> +++ b/arch/arm/mach-omap1/board-ams-delta.c
> @@ -41,10 +41,10 @@
> #include <mach/mux.h>
>
> #include <mach/hardware.h>
> -#include <mach/ams-delta-fiq.h>
> #include "camera.h"
> #include <mach/usb.h>
>
> +#include "ams-delta-fiq.h"
> #include "iomap.h"
> #include "common.h"
>
> diff --git a/drivers/input/serio/ams_delta_serio.c b/drivers/input/serio/ams_delta_serio.c
> index 7952a29f9540..2602f7cff5ae 100644
> --- a/drivers/input/serio/ams_delta_serio.c
> +++ b/drivers/input/serio/ams_delta_serio.c
> @@ -22,6 +22,7 @@
> */
> #include <linux/gpio.h>
> #include <linux/irq.h>
> +#include <linux/platform_data/ams-delta-fiq.h>
> #include <linux/platform_device.h>
> #include <linux/regulator/consumer.h>
> #include <linux/serio.h>
> @@ -30,8 +31,6 @@
>
> #include <mach/board-ams-delta.h>
>
> -#include <mach/ams-delta-fiq.h>
> -
> #define DRIVER_NAME "ams-delta-serio"
>
> MODULE_AUTHOR("Matt Callow");
> diff --git a/arch/arm/mach-omap1/include/mach/ams-delta-fiq.h b/include/linux/platform_data/ams-delta-fiq.h
> similarity index 71%
> rename from arch/arm/mach-omap1/include/mach/ams-delta-fiq.h
> rename to include/linux/platform_data/ams-delta-fiq.h
> index a9769ff396bc..dc0f835ea918 100644
> --- a/arch/arm/mach-omap1/include/mach/ams-delta-fiq.h
> +++ b/include/linux/platform_data/ams-delta-fiq.h
> @@ -1,5 +1,7 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> /*
> - * arch/arm/mach-omap1/include/ams-delta-fiq.h
> + * include/linux/platform_data/ams-delta-fiq.h
> *
> * Taken from the original Amstrad modifications to fiq.h
> *
> @@ -11,24 +13,8 @@
> * it under the terms of the GNU General Public License version 2 as
> * published by the Free Software Foundation.
> */
> -#ifndef __AMS_DELTA_FIQ_H
> -#define __AMS_DELTA_FIQ_H
> -
> -#include <mach/irqs.h>
> -
> -/*
> - * Interrupt number used for passing control from FIQ to IRQ.
> - * IRQ12, described as reserved, has been selected.
> - */
> -#define INT_DEFERRED_FIQ INT_1510_RES12
> -/*
> - * Base address of an interrupt handler that the INT_DEFERRED_FIQ belongs to.
> - */
> -#if (INT_DEFERRED_FIQ < IH2_BASE)
> -#define DEFERRED_FIQ_IH_BASE OMAP_IH1_BASE
> -#else
> -#define DEFERRED_FIQ_IH_BASE OMAP_IH2_BASE
> -#endif
> +#ifndef __LINUX_PLATFORM_DATA_AMS_DELTA_FIQ_H
> +#define __LINUX_PLATFORM_DATA_AMS_DELTA_FIQ_H
>
> /*
> * These are the offsets from the beginning of the fiq_buffer. They are put here
> @@ -71,9 +57,6 @@
>
> #ifndef __ASSEMBLER__
> extern unsigned int fiq_buffer[];
> -extern unsigned char qwerty_fiqin_start, qwerty_fiqin_end;
> -
> -extern void __init ams_delta_init_fiq(struct gpio_chip *chip);
> #endif
>
> #endif
> --
> 2.16.4
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2 07/10] ARM: OMAP1: ams-delta FIQ: Keep serio input GPIOs requested
From: Dmitry Torokhov @ 2018-06-22 0:10 UTC (permalink / raw)
To: Janusz Krzysztofik
Cc: Tony Lindgren, Aaro Koskinen, David S . Miller ,
Mauro Carvalho Chehab, Greg Kroah-Hartman, Andrew Morton,
Randy Dunlap, Linus Walleij, Mark Brown, Liam Girdwood,
linux-arm-kernel, linux-omap, linux-input, linux-gpio,
linux-kernel
In-Reply-To: <20180621224128.17623-7-jmkrzyszt@gmail.com>
On Fri, Jun 22, 2018 at 12:41:25AM +0200, Janusz Krzysztofik wrote:
> From the very beginning, input GPIO pins of ams-delta serio port have
> been used by FIQ handler, not serio driver.
>
> Don't request those pins from the ams-delta-serio driver any longer,
> instead keep them requested and initialized by the FIQ initialization
> routine which already requests them and releases while identifying GPIO
> IRQs.
>
> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
Input bits look good.
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> Changelog:
> v2: rebased on v4.18-rc1, no conflicts
>
> arch/arm/mach-omap1/ams-delta-fiq.c | 42 ++++++++++++++++++++++++++++++-----
> drivers/input/serio/ams_delta_serio.c | 30 ++-----------------------
> 2 files changed, 39 insertions(+), 33 deletions(-)
>
> diff --git a/arch/arm/mach-omap1/ams-delta-fiq.c b/arch/arm/mach-omap1/ams-delta-fiq.c
> index 1d54a6177f14..5a6c59ac9b5f 100644
> --- a/arch/arm/mach-omap1/ams-delta-fiq.c
> +++ b/arch/arm/mach-omap1/ams-delta-fiq.c
> @@ -45,6 +45,11 @@ static struct irq_chip *irq_chip;
> static struct irq_data *irq_data[16];
> static unsigned int irq_counter[16];
>
> +static const char *pin_name[16] __initconst = {
> + [AMS_DELTA_GPIO_PIN_KEYBRD_DATA] = "keybrd_data",
> + [AMS_DELTA_GPIO_PIN_KEYBRD_CLK] = "keybrd_clk",
> +};
> +
> static irqreturn_t deferred_fiq(int irq, void *dev_id)
> {
> struct irq_data *d;
> @@ -80,7 +85,7 @@ static irqreturn_t deferred_fiq(int irq, void *dev_id)
>
> void __init ams_delta_init_fiq(struct gpio_chip *chip)
> {
> - struct gpio_desc *gpiod;
> + struct gpio_desc *gpiod, *data = NULL, *clk = NULL;
> void *fiqhandler_start;
> unsigned int fiqhandler_length;
> struct pt_regs FIQ_regs;
> @@ -96,7 +101,7 @@ void __init ams_delta_init_fiq(struct gpio_chip *chip)
> }
>
> for (i = 0; i < ARRAY_SIZE(irq_data); i++) {
> - gpiod = gpiochip_request_own_desc(chip, i, NULL);
> + gpiod = gpiochip_request_own_desc(chip, i, pin_name[i]);
> if (IS_ERR(gpiod)) {
> pr_err("%s: failed to get GPIO pin %d (%ld)\n",
> __func__, i, PTR_ERR(gpiod));
> @@ -105,8 +110,27 @@ void __init ams_delta_init_fiq(struct gpio_chip *chip)
> /* Store irq_data location for IRQ handler use */
> irq_data[i] = irq_get_irq_data(gpiod_to_irq(gpiod));
>
> - gpiochip_free_own_desc(gpiod);
> + /*
> + * FIQ handler takes full control over serio data and clk GPIO
> + * pins. Initiaize them and keep requested so nobody can
> + * interfere. Fail if any of those two couldn't be requested.
> + */
> + switch (i) {
> + case AMS_DELTA_GPIO_PIN_KEYBRD_DATA:
> + data = gpiod;
> + gpiod_direction_input(data);
> + break;
> + case AMS_DELTA_GPIO_PIN_KEYBRD_CLK:
> + clk = gpiod;
> + gpiod_direction_input(clk);
> + break;
> + default:
> + gpiochip_free_own_desc(gpiod);
> + break;
> + }
> }
> + if (!data || !clk)
> + goto out_gpio;
>
> fiqhandler_start = &qwerty_fiqin_start;
> fiqhandler_length = &qwerty_fiqin_end - &qwerty_fiqin_start;
> @@ -117,7 +141,7 @@ void __init ams_delta_init_fiq(struct gpio_chip *chip)
> if (retval) {
> pr_err("ams_delta_init_fiq(): couldn't claim FIQ, ret=%d\n",
> retval);
> - return;
> + goto out_gpio;
> }
>
> retval = request_irq(INT_DEFERRED_FIQ, deferred_fiq,
> @@ -125,7 +149,7 @@ void __init ams_delta_init_fiq(struct gpio_chip *chip)
> if (retval < 0) {
> pr_err("Failed to get deferred_fiq IRQ, ret=%d\n", retval);
> release_fiq(&fh);
> - return;
> + goto out_gpio;
> }
> /*
> * Since no set_type() method is provided by OMAP irq chip,
> @@ -175,4 +199,12 @@ void __init ams_delta_init_fiq(struct gpio_chip *chip)
> offset = IRQ_ILR0_REG_OFFSET + (INT_GPIO_BANK1 - NR_IRQS_LEGACY) * 0x4;
> val = omap_readl(OMAP_IH1_BASE + offset) | 1;
> omap_writel(val, OMAP_IH1_BASE + offset);
> +
> + return;
> +
> +out_gpio:
> + if (data)
> + gpiochip_free_own_desc(data);
> + if (clk)
> + gpiochip_free_own_desc(clk);
> }
> diff --git a/drivers/input/serio/ams_delta_serio.c b/drivers/input/serio/ams_delta_serio.c
> index b955c6a72e99..7952a29f9540 100644
> --- a/drivers/input/serio/ams_delta_serio.c
> +++ b/drivers/input/serio/ams_delta_serio.c
> @@ -110,19 +110,6 @@ static void ams_delta_serio_close(struct serio *serio)
> regulator_disable(priv->vcc);
> }
>
> -static const struct gpio ams_delta_gpios[] __initconst_or_module = {
> - {
> - .gpio = AMS_DELTA_GPIO_PIN_KEYBRD_DATA,
> - .flags = GPIOF_DIR_IN,
> - .label = "serio-data",
> - },
> - {
> - .gpio = AMS_DELTA_GPIO_PIN_KEYBRD_CLK,
> - .flags = GPIOF_DIR_IN,
> - .label = "serio-clock",
> - },
> -};
> -
> static int ams_delta_serio_init(struct platform_device *pdev)
> {
> struct ams_delta_serio *priv;
> @@ -133,13 +120,6 @@ static int ams_delta_serio_init(struct platform_device *pdev)
> if (!priv)
> return -ENOMEM;
>
> - err = gpio_request_array(ams_delta_gpios,
> - ARRAY_SIZE(ams_delta_gpios));
> - if (err) {
> - dev_err(&pdev->dev, "Couldn't request gpio pins\n");
> - goto serio;
> - }
> -
> priv->vcc = devm_regulator_get(&pdev->dev, "vcc");
> if (IS_ERR(priv->vcc)) {
> err = PTR_ERR(priv->vcc);
> @@ -157,7 +137,7 @@ static int ams_delta_serio_init(struct platform_device *pdev)
> */
> if (err == -ENODEV)
> err = -EPROBE_DEFER;
> - goto gpio;
> + return err;
> }
>
> err = request_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK),
> @@ -165,7 +145,7 @@ static int ams_delta_serio_init(struct platform_device *pdev)
> DRIVER_NAME, priv);
> if (err < 0) {
> dev_err(&pdev->dev, "IRQ request failed (%d)\n", err);
> - goto gpio;
> + return err;
> }
> /*
> * Since GPIO register handling for keyboard clock pin is performed
> @@ -201,10 +181,6 @@ static int ams_delta_serio_init(struct platform_device *pdev)
>
> irq:
> free_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK), priv);
> -gpio:
> - gpio_free_array(ams_delta_gpios,
> - ARRAY_SIZE(ams_delta_gpios));
> -serio:
> return err;
> }
>
> @@ -214,8 +190,6 @@ static int ams_delta_serio_exit(struct platform_device *pdev)
>
> serio_unregister_port(priv->serio);
> free_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK), 0);
> - gpio_free_array(ams_delta_gpios,
> - ARRAY_SIZE(ams_delta_gpios));
>
> return 0;
> }
> --
> 2.16.4
>
--
Dmitry
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox