* Re: [PATCH v4 1/2] xen/kbdif: Sync up with the canonical definition in Xen
From: Juergen Gross @ 2018-05-18 11:17 UTC (permalink / raw)
To: Oleksandr Andrushchenko, xen-devel, linux-input, linux-kernel,
dmitry.torokhov, lyan, boris.ostrovsky
Cc: konrad.wilk, andrii_chepurnyi, Oleksandr Andrushchenko
In-Reply-To: <20180518095935.26740-1-andr2000@gmail.com>
On 18/05/18 11:59, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>
> This is the sync up with the canonical definition of the keyboard
> protocol in Xen:
> 1. Add missing string constants for {feature|request}-raw-pointer
> to align with the rest of the interface file.
>
> 2. Add new XenStore feature fields, so it is possible to individually
> control set of exposed virtual devices for each guest OS:
> - set feature-disable-keyboard to 1 if no keyboard device needs
> to be created
> - set feature-disable-pointer to 1 if no pointer device needs
> to be created
>
> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Juergen
^ permalink raw reply
* Re: [BUG] i2c-hid: ELAN Touchpad does not work on ASUS X580GD
From: Hans de Goede @ 2018-05-18 13:09 UTC (permalink / raw)
To: Jarkko Nikula, Andy Shevchenko, Chris Chiu
Cc: Daniel Drake, Jian-Hong Pan, Jiri Kosina, Benjamin Tissoires,
Jani Nikula, Dmitry Torokhov, Adrian Salido, Jason Gerecke,
linux-input, Mika Westerberg, Wolfram Sang, linux-i2c,
Linux Kernel, Linux Upstreaming Team
In-Reply-To: <9fd4cd99-5a4f-2d9e-88ba-18575e951490@linux.intel.com>
Hi,
On 18-05-18 12:58, Jarkko Nikula wrote:
> On 05/18/2018 01:20 PM, Andy Shevchenko wrote:
>> On Fri, 2018-05-18 at 11:37 +0300, Andy Shevchenko wrote:
>>> On Fri, 2018-05-18 at 09:48 +0200, Hans de Goede wrote:
>>>
>>>
>>>> Could it be the i2c input clock definition in drivers/mfd/intel-
>>>> lpss-
>>>> pci.c
>>>> is also wrong for Apollo Lake (N3450) ? There are lots of people
>>>> having
>>>> various issues with i2c attached touchpads on Apollo Lake devices,
>>>> this bug:
>>>> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1728244
>>>>
>>>> Is sort of a collection bug for these. Various models laptops, lots
>>>> of
>>>> reporters. Note not sure thie is an i2c-designware issue, but it
>>>> would
>>>> be good to double check the input clock on Apollo Lake.
>>>>
>>>> I've checked the datasheet and the datasheet mentions 133MHz as
>>>> "serial input clk" in the lpio_bxt_regs Registers Summary, which is
>>>> also part of the LPSS, no clk is mentioned in the "Summary of
>>>> DW_apb_i2c_mem_map_DW_apb_i2c_addr_block1 Registers".
>>>
>>> The internal datasheet we have access to mentioned in this case for
>>> Broxton and Cannonlake together. So, your assumption might be quite
>>> close to the truth and the issue is inherited from Broxton.
>>>
>>
> Nope. The specification I have mention the I2C input clock in Broxton is fixed 133 MHz but in Cannon Lake it is derived through non-SW visible divider.
>
>> Hans, can your reporters try the following patch? Depending on the
>> result I may send it out ASAP.
>>
>> --- a/drivers/mfd/intel-lpss-pci.c
>> +++ b/drivers/mfd/intel-lpss-pci.c
>> @@ -120,7 +120,7 @@ static struct property_entry apl_i2c_properties[] =
>> {
>> };
>> static const struct intel_lpss_platform_info apl_i2c_info = {
>> - .clk_rate = 133000000,
>> + .clk_rate = 216000000,
>> .properties = apl_i2c_properties,
>> };
>>
> Nack. The Apollo Lake HW here shows expected I2C bus clock on oscilloscope so it is indeed clocked at 133 MHz.
Ok, thank you for checking this.
Regards,
Hans
^ permalink raw reply
* Re: [BUG] i2c-hid: ELAN Touchpad does not work on ASUS X580GD
From: Jarkko Nikula @ 2018-05-18 13:15 UTC (permalink / raw)
To: Hans de Goede, Chris Chiu
Cc: Daniel Drake, Jian-Hong Pan, Jiri Kosina, Benjamin Tissoires,
Jani Nikula, Dmitry Torokhov, Adrian Salido, Jason Gerecke,
linux-input, Andy Shevchenko, Mika Westerberg, Wolfram Sang,
linux-i2c, Linux Kernel, Linux Upstreaming Team
In-Reply-To: <ba99be8e-8e8b-87c1-8607-24ace8a01c64@redhat.com>
On 05/18/2018 10:48 AM, Hans de Goede wrote:
> Could it be the i2c input clock definition in drivers/mfd/intel-lpss-pci.c
> is also wrong for Apollo Lake (N3450) ? There are lots of people having
> various issues with i2c attached touchpads on Apollo Lake devices, this
> bug:
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1728244
>
> Is sort of a collection bug for these. Various models laptops, lots of
> reporters. Note not sure thie is an i2c-designware issue, but it would
> be good to double check the input clock on Apollo Lake.
>
Does i2c_designware_core.dyndbg=+p and i2c-hid.debug=1 command line
arguments give any useful debug information from those machines?
--
Jarkko
^ permalink raw reply
* Re: [BUG] i2c-hid: ELAN Touchpad does not work on ASUS X580GD
From: Hans de Goede @ 2018-05-18 13:26 UTC (permalink / raw)
To: Jarkko Nikula, Chris Chiu
Cc: Daniel Drake, Jian-Hong Pan, Jiri Kosina, Benjamin Tissoires,
Jani Nikula, Dmitry Torokhov, Adrian Salido, Jason Gerecke,
linux-input, Andy Shevchenko, Mika Westerberg, Wolfram Sang,
linux-i2c, Linux Kernel, Linux Upstreaming Team
In-Reply-To: <91f87c43-0ae1-00ec-6b3e-819b639aa1b2@linux.intel.com>
Hi,
On 18-05-18 15:15, Jarkko Nikula wrote:
> On 05/18/2018 10:48 AM, Hans de Goede wrote:
>> Could it be the i2c input clock definition in drivers/mfd/intel-lpss-pci.c
>> is also wrong for Apollo Lake (N3450) ? There are lots of people having
>> various issues with i2c attached touchpads on Apollo Lake devices, this bug:
>> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1728244
>>
>> Is sort of a collection bug for these. Various models laptops, lots of
>> reporters. Note not sure thie is an i2c-designware issue, but it would
>> be good to double check the input clock on Apollo Lake.
>>
> Does i2c_designware_core.dyndbg=+p and i2c-hid.debug=1 command line arguments give any useful debug information from those machines?
I've asked people in the bug to collect debug output with this
added.
Regards,
Hans
^ permalink raw reply
* Re: [PATCH v5 4/9] dt-bindings: input: touchscreen: resistive-adc-touch: create bindings
From: Rob Herring @ 2018-05-18 14:20 UTC (permalink / raw)
To: Eugen Hristev
Cc: jic23, ludovic.desroches, alexandre.belloni, linux-arm-kernel,
devicetree, linux-kernel, linux-iio, linux-input, nicolas.ferre,
dmitry.torokhov
In-Reply-To: <1525949114-29263-5-git-send-email-eugen.hristev@microchip.com>
On Thu, May 10, 2018 at 01:45:09PM +0300, Eugen Hristev wrote:
> Added bindings for generic resistive touchscreen ADC.
>
> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
> ---
> Changes in v5:
> - changed property name touchscreen-threshold-pressure to
> touchscreen-min-pressure
>
> Changes in v3:
> - renamed file and compatible to exclude "generic" keyword
> - removed the pressure threshold property, added it as a common
> touchscreen property in the touchscreen common bindings in a separate
> commit.
>
> Changes in v2:
> - modified bindings to have a generic resistive touchscreen adc driver
> instead of specific architecture one.
>
>
> .../input/touchscreen/resistive-adc-touch.txt | 30 ++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/touchscreen/resistive-adc-touch.txt
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v4 2/2] Input: xen-kbdfront - allow better run-time configuration
From: Oleksandr Andrushchenko @ 2018-05-18 14:33 UTC (permalink / raw)
To: xen-devel, linux-input, linux-kernel, dmitry.torokhov, jgross,
lyan, boris.ostrovsky
Cc: konrad.wilk, andrii_chepurnyi, Oleksandr Andrushchenko
In-Reply-To: <20180518095935.26740-2-andr2000@gmail.com>
Sorry for top posting.
Due to a bug in the test used to verify this patch
I need to make two more changes, please see inline.
Please ignore this patch for now, I will send v5 soon.
Sorry for inconvenience,
Oleksandr
On 05/18/2018 12:59 PM, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>
> It is now only possible to control if multi-touch virtual device
> is created or not (via the corresponding XenStore entries),
> but keyboard and pointer devices are always created.
> In some cases this is not desirable. For example, if virtual
> keyboard device is exposed to Android then the latter won't
> automatically show on-screen keyboard as it expects that a
> physical keyboard device can be used for typing.
>
> Utilize keyboard and pointer device XenStore feature fields to
> configure which virtual devices are created:
> - set "feature-disable-keyboard" to 1 if no keyboard device
> needs to be created
> - set "feature-disable-pointer" to 1 if no pointer device
> needs to be created
> Keep old behavior by default.
>
> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> Suggested-by: Andrii Chepurnyi <andrii_chepurnyi@epam.com>
> Tested-by: Andrii Chepurnyi <andrii_chepurnyi@epam.com>
> ---
> Changes since v3:
> - do not connect backend if all virtual devices are disabled (Dmitry)
>
> Changes since v2:
> - based on XenStore kbdif features to control which devices are
> exposed instead of module parameters.
>
> drivers/input/misc/xen-kbdfront.c | 177 ++++++++++++++++++------------
> 1 file changed, 106 insertions(+), 71 deletions(-)
>
> diff --git a/drivers/input/misc/xen-kbdfront.c b/drivers/input/misc/xen-kbdfront.c
> index d91f3b1c5375..0e8926c75559 100644
> --- a/drivers/input/misc/xen-kbdfront.c
> +++ b/drivers/input/misc/xen-kbdfront.c
> @@ -63,6 +63,9 @@ static void xenkbd_disconnect_backend(struct xenkbd_info *);
> static void xenkbd_handle_motion_event(struct xenkbd_info *info,
> struct xenkbd_motion *motion)
> {
> + if (unlikely(!info->ptr))
> + return;
> +
> input_report_rel(info->ptr, REL_X, motion->rel_x);
> input_report_rel(info->ptr, REL_Y, motion->rel_y);
> if (motion->rel_z)
> @@ -73,6 +76,9 @@ static void xenkbd_handle_motion_event(struct xenkbd_info *info,
> static void xenkbd_handle_position_event(struct xenkbd_info *info,
> struct xenkbd_position *pos)
> {
> + if (unlikely(!info->ptr))
> + return;
> +
> input_report_abs(info->ptr, ABS_X, pos->abs_x);
> input_report_abs(info->ptr, ABS_Y, pos->abs_y);
> if (pos->rel_z)
> @@ -97,6 +103,9 @@ static void xenkbd_handle_key_event(struct xenkbd_info *info,
> return;
> }
>
> + if (unlikely(!dev))
> + return;
> +
> input_event(dev, EV_KEY, key->keycode, value);
> input_sync(dev);
> }
> @@ -192,7 +201,7 @@ static int xenkbd_probe(struct xenbus_device *dev,
> const struct xenbus_device_id *id)
> {
> int ret, i;
> - unsigned int abs, touch;
> + bool with_mtouch, with_kbd, with_ptr;
> struct xenkbd_info *info;
> struct input_dev *kbd, *ptr, *mtouch;
>
> @@ -211,93 +220,114 @@ static int xenkbd_probe(struct xenbus_device *dev,
> if (!info->page)
> goto error_nomem;
>
> - /* Set input abs params to match backend screen res */
> - abs = xenbus_read_unsigned(dev->otherend,
> - XENKBD_FIELD_FEAT_ABS_POINTER, 0);
> - ptr_size[KPARAM_X] = xenbus_read_unsigned(dev->otherend,
> - XENKBD_FIELD_WIDTH,
> - ptr_size[KPARAM_X]);
> - ptr_size[KPARAM_Y] = xenbus_read_unsigned(dev->otherend,
> - XENKBD_FIELD_HEIGHT,
> - ptr_size[KPARAM_Y]);
> - if (abs) {
> - ret = xenbus_write(XBT_NIL, dev->nodename,
> - XENKBD_FIELD_REQ_ABS_POINTER, "1");
> - if (ret) {
> - pr_warn("xenkbd: can't request abs-pointer\n");
> - abs = 0;
> - }
> - }
> + /*
> + * The below are reverse logic, e.g. if the feature is set, then
> + * do not expose the corresponding virtual device.
> + */
> + with_kbd = !xenbus_read_unsigned(dev->nodename,
According to the kbdif protocol this must be "dev->otherend"
> + XENKBD_FIELD_FEAT_DSBL_KEYBRD, 0);
> +
> + with_ptr = !xenbus_read_unsigned(dev->nodename,
According to the kbdif protocol this must be "dev->otherend"
> + XENKBD_FIELD_FEAT_DSBL_POINTER, 0);
>
> - touch = xenbus_read_unsigned(dev->nodename,
> - XENKBD_FIELD_FEAT_MTOUCH, 0);
> - if (touch) {
> + /* Direct logic: if set, then create multi-touch device. */
> + with_mtouch = xenbus_read_unsigned(dev->nodename,
According to the kbdif protocol this must be "dev->otherend"
> + XENKBD_FIELD_FEAT_MTOUCH, 0);
> + if (with_mtouch) {
> ret = xenbus_write(XBT_NIL, dev->nodename,
> XENKBD_FIELD_REQ_MTOUCH, "1");
> if (ret) {
> pr_warn("xenkbd: can't request multi-touch");
> - touch = 0;
> + with_mtouch = 0;
> }
> }
>
> /* keyboard */
> - kbd = input_allocate_device();
> - if (!kbd)
> - goto error_nomem;
> - kbd->name = "Xen Virtual Keyboard";
> - kbd->phys = info->phys;
> - kbd->id.bustype = BUS_PCI;
> - kbd->id.vendor = 0x5853;
> - kbd->id.product = 0xffff;
> -
> - __set_bit(EV_KEY, kbd->evbit);
> - for (i = KEY_ESC; i < KEY_UNKNOWN; i++)
> - __set_bit(i, kbd->keybit);
> - for (i = KEY_OK; i < KEY_MAX; i++)
> - __set_bit(i, kbd->keybit);
> -
> - ret = input_register_device(kbd);
> - if (ret) {
> - input_free_device(kbd);
> - xenbus_dev_fatal(dev, ret, "input_register_device(kbd)");
> - goto error;
> + if (with_kbd) {
> + kbd = input_allocate_device();
> + if (!kbd)
> + goto error_nomem;
> + kbd->name = "Xen Virtual Keyboard";
> + kbd->phys = info->phys;
> + kbd->id.bustype = BUS_PCI;
> + kbd->id.vendor = 0x5853;
> + kbd->id.product = 0xffff;
> +
> + __set_bit(EV_KEY, kbd->evbit);
> + for (i = KEY_ESC; i < KEY_UNKNOWN; i++)
> + __set_bit(i, kbd->keybit);
> + for (i = KEY_OK; i < KEY_MAX; i++)
> + __set_bit(i, kbd->keybit);
> +
> + ret = input_register_device(kbd);
> + if (ret) {
> + input_free_device(kbd);
> + xenbus_dev_fatal(dev, ret,
> + "input_register_device(kbd)");
> + goto error;
> + }
> + info->kbd = kbd;
> }
> - info->kbd = kbd;
>
> /* pointing device */
> - ptr = input_allocate_device();
> - if (!ptr)
> - goto error_nomem;
> - ptr->name = "Xen Virtual Pointer";
> - ptr->phys = info->phys;
> - ptr->id.bustype = BUS_PCI;
> - ptr->id.vendor = 0x5853;
> - ptr->id.product = 0xfffe;
> -
> - if (abs) {
> - __set_bit(EV_ABS, ptr->evbit);
> - input_set_abs_params(ptr, ABS_X, 0, ptr_size[KPARAM_X], 0, 0);
> - input_set_abs_params(ptr, ABS_Y, 0, ptr_size[KPARAM_Y], 0, 0);
> - } else {
> - input_set_capability(ptr, EV_REL, REL_X);
> - input_set_capability(ptr, EV_REL, REL_Y);
> - }
> - input_set_capability(ptr, EV_REL, REL_WHEEL);
> + if (with_ptr) {
> + unsigned int abs;
> +
> + /* Set input abs params to match backend screen res */
> + abs = xenbus_read_unsigned(dev->otherend,
> + XENKBD_FIELD_FEAT_ABS_POINTER, 0);
> + ptr_size[KPARAM_X] = xenbus_read_unsigned(dev->otherend,
> + XENKBD_FIELD_WIDTH,
> + ptr_size[KPARAM_X]);
> + ptr_size[KPARAM_Y] = xenbus_read_unsigned(dev->otherend,
> + XENKBD_FIELD_HEIGHT,
> + ptr_size[KPARAM_Y]);
> + if (abs) {
> + ret = xenbus_write(XBT_NIL, dev->nodename,
> + XENKBD_FIELD_REQ_ABS_POINTER, "1");
> + if (ret) {
> + pr_warn("xenkbd: can't request abs-pointer\n");
> + abs = 0;
> + }
> + }
>
> - __set_bit(EV_KEY, ptr->evbit);
> - for (i = BTN_LEFT; i <= BTN_TASK; i++)
> - __set_bit(i, ptr->keybit);
> + ptr = input_allocate_device();
> + if (!ptr)
> + goto error_nomem;
> + ptr->name = "Xen Virtual Pointer";
> + ptr->phys = info->phys;
> + ptr->id.bustype = BUS_PCI;
> + ptr->id.vendor = 0x5853;
> + ptr->id.product = 0xfffe;
> +
> + if (abs) {
> + __set_bit(EV_ABS, ptr->evbit);
> + input_set_abs_params(ptr, ABS_X, 0,
> + ptr_size[KPARAM_X], 0, 0);
> + input_set_abs_params(ptr, ABS_Y, 0,
> + ptr_size[KPARAM_Y], 0, 0);
> + } else {
> + input_set_capability(ptr, EV_REL, REL_X);
> + input_set_capability(ptr, EV_REL, REL_Y);
> + }
> + input_set_capability(ptr, EV_REL, REL_WHEEL);
>
> - ret = input_register_device(ptr);
> - if (ret) {
> - input_free_device(ptr);
> - xenbus_dev_fatal(dev, ret, "input_register_device(ptr)");
> - goto error;
> + __set_bit(EV_KEY, ptr->evbit);
> + for (i = BTN_LEFT; i <= BTN_TASK; i++)
> + __set_bit(i, ptr->keybit);
> +
> + ret = input_register_device(ptr);
> + if (ret) {
> + input_free_device(ptr);
> + xenbus_dev_fatal(dev, ret,
> + "input_register_device(ptr)");
> + goto error;
> + }
> + info->ptr = ptr;
> }
> - info->ptr = ptr;
>
> /* multi-touch device */
> - if (touch) {
> + if (with_mtouch) {
> int num_cont, width, height;
>
> mtouch = input_allocate_device();
> @@ -346,6 +376,11 @@ static int xenkbd_probe(struct xenbus_device *dev,
> info->mtouch = mtouch;
> }
>
> + if (!(with_kbd | with_ptr | with_mtouch)) {
> + ret = -ENXIO;
> + goto error;
> + }
> +
> ret = xenkbd_connect_backend(dev, info);
> if (ret < 0)
> goto error;
^ permalink raw reply
* Re: [PATCH v5 3/9] dt-bindings: input: touchscreen: add minimum pressure touchscreen property
From: Rob Herring @ 2018-05-18 16:35 UTC (permalink / raw)
To: Eugen Hristev
Cc: jic23, ludovic.desroches, alexandre.belloni, linux-arm-kernel,
devicetree, linux-kernel, linux-iio, linux-input, nicolas.ferre,
dmitry.torokhov
In-Reply-To: <1525949114-29263-4-git-send-email-eugen.hristev@microchip.com>
On Thu, May 10, 2018 at 01:45:08PM +0300, Eugen Hristev wrote:
> Add a common touchscreen optional property that will specify
> the minimum pressure applied to the screen that is needed
> such that the driver will report the touch event.
>
> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
> ---
> Changes in v5:
> - Modified property name to touchscreen-min-pressure from
> touchscreen-threshold-property
>
> Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt | 3 +++
> 1 file changed, 3 insertions(+)
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v4 1/2] dt-bindings: input: Add Add Spreadtrum SC27xx vibrator documentation
From: Rob Herring @ 2018-05-18 16:54 UTC (permalink / raw)
To: Baolin Wang
Cc: dmitry.torokhov, mark.rutland, orsonzhai, zhang.lyra, linux-input,
devicetree, linux-kernel, xiaotong.lu
In-Reply-To: <44b17778d8f0bf897a316a39516ba2cafecf43c9.1525928437.git.baolin.wang@linaro.org>
On Thu, May 10, 2018 at 01:08:33PM +0800, Baolin Wang wrote:
> From: Xiaotong Lu <xiaotong.lu@spreadtrum.com>
>
> This patch adds the binding documentation for Spreadtrum SC27xx series
> vibrator device.
>
> Signed-off-by: Xiaotong Lu <xiaotong.lu@spreadtrum.com>
> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
> ---
> Changes since v3:
> - Change compatible string to explicit Soc name.
> - Add parent MFD node.
>
> Changes since v2:
> - No updates.
>
> Changes since v1:
> - No updates.
> ---
> .../bindings/input/sprd,sc27xx-vibra.txt | 23 ++++++++++++++++++++
> 1 file changed, 23 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/sprd,sc27xx-vibra.txt
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: Sometimes unusable i2c-hid devices in 4.17-rcX
From: Benjamin Tissoires @ 2018-05-18 18:18 UTC (permalink / raw)
To: Mario.Limonciello; +Cc: linux-input, linux-kernel
In-Reply-To: <82124c4909834ef0bd61b3472ed6af66@ausx13mpc120.AMER.DELL.COM>
On Thu, May 17, 2018 at 4:44 PM, <Mario.Limonciello@dell.com> wrote:
>> -----Original Message-----
>> From: Benjamin Tissoires [mailto:benjamin.tissoires@gmail.com]
>> Sent: Thursday, May 17, 2018 9:28 AM
>> To: Limonciello, Mario
>> Cc: linux-input; linux-kernel@vger.kernel.org
>> Subject: Re: Sometimes unusable i2c-hid devices in 4.17-rcX
>>
>> Hi Mario,
>>
>> On Wed, May 16, 2018 at 10:00 PM, <Mario.Limonciello@dell.com> wrote:
>> > Hi All,
>> >
>> > I've been running 4.16-rc7 on an XPS 9365 for some time and recently moved up
>> to 4.17-rc5.
>> > Immediately I noticed that i2c-hid devices (both touchscreen and touchpad) were
>> not working.
>> > Also when shutting the system down or rebooting it would just hang. (magic sysrq
>> still worked).
>> >
>> > I figured it was an easy to identify regression so I started a bisect but it came up
>> with garbage
>> > that ended in selftests shortly after 4.17-rc2. I realized that's because is still will
>> fail on 4.17-rc2
>> > occasionally, seemingly after trying something newer and warm rebooting.
>> > So it seems like it's "worse" after 4.17-rc2 (doesn't work at all) but semi
>> reproducible on 4.17-rc2.
>> >
>> > Not sure if I'm chasing some initialization race, but wanted to see if anyone else
>> was running into this
>> > or has some ideas?
>>
>> I am reliably running a v4.17-rc3 with a merge on Jiri's tree on the 9360.
>>
>> I doubt it's related to the event processing as I am not encountering
>> those issues.
>>
>> It *could* be related to the interrupts not being properly raised.
>>
>> Could you monitor /proc/interrupts and check if the ones associated
>> with your i2c-hid devices are increasing when you are using them?
>> Also, does the device emits raw HID events? (you can use hid-recorder
>> to check on the hidraw nodes.)
>
Sorry, I couldn't get to it today. Monday is a public holiday here, so
I'll check on this Tuesday.
> I checked both, /proc/interrupts isn't incrementing at all with the DLL077A:01 device.
> Hid-recorder is showing output from the raw HID node.
I don't really understand how the hidraw node can send data while the
interrupts are not raised.
Could you share the output of hid-recorder?
>
> Same thing for the touchscreen, no incrementing for it on the i2c_designware.0 device.
>
> Something notable however;
> When in this bad state hid-recorder didn't show /dev/hidraw1 for the touchscreen (which
> Happens to be a Wacom touch screen).
> It only showed /dev/hidraw0 for the touchpad.
This explains why the touchscreen doesn't increment the interrupts.
Something I missed in the first email is that the hidraw0 node
disappear for the wacom device as the touchpad gets the hidraw0 name.
Could you provide the output of a working kernel configuration of:
sudo hid-recorder /dev/hidraw*
This should provide me the whole logs at the same time of all the
hidraw nodes, and will allow me to reproduce the combination of
wacom/hid-multitouch you are experiencing.
Cheers,
Benjamin
>
>
>>
>> Cheers,
>> Benjamin
>>
>> >
>> > #dmesg | grep 'i2c\|hid' doesn't show any obvious errors when in this state of
>> non functional hid stuff.
>> > [ 2.398649] i2c /dev entries driver
>> > [ 2.881651] hidraw: raw HID events driver (C) Jiri Kosina
>> > [ 3.683583] ish-hid {33AECD58-B679-4E54-9BD9-A04D34F0C226}: [hid-ish]:
>> enum_devices_done OK, num_hid_devices=5
>> > [ 3.701259] hid-generic 001F:8086:22D8.0001: hidraw0: <UNKNOWN> HID
>> v2.00 Device [hid-ishtp 8086:22D8] on
>> > [ 3.702204] hid-generic 001F:8086:22D8.0002: hidraw1: <UNKNOWN> HID
>> v2.00 Device [hid-ishtp 8086:22D8] on
>> > [ 3.703063] hid-generic 001F:8086:22D8.0003: hidraw2: <UNKNOWN> HID
>> v2.00 Device [hid-ishtp 8086:22D8] on
>> > [ 3.704276] hid-generic 001F:8086:22D8.0004: hidraw3: <UNKNOWN> HID
>> v2.00 Device [hid-ishtp 8086:22D8] on
>> > [ 3.704557] hid-generic 001F:8086:22D8.0005: hidraw4: <UNKNOWN> HID
>> v2.00 Device [hid-ishtp 8086:22D8] on
>> > [ 3.750710] psmouse serio1: synaptics: Your touchpad (PNP: DLL077a PNP0f13)
>> says it can support a different bus. If i2c-hid and hid-rmi are not used, you might
>> want to try setting psmouse.synaptics_intertouch to 1 and report this to linux-
>> input@vger.kernel.org.
>> > [ 7.030446] acpi INT33D5:00: intel-hid: created platform device
>> > [ 7.199178] i2c_hid i2c-WCOM482F:00: i2c-WCOM482F:00 supply vdd not
>> found, using dummy regulator
>> > [ 7.246638] input: WCOM482F:00 056A:482F as
>> /devices/pci0000:00/0000:00:15.0/i2c_designware.0/i2c-6/i2c-
>> WCOM482F:00/0018:056A:482F.0006/input/input11
>> > [ 7.246873] hid-generic 0018:056A:482F.0006: input,hidraw0: I2C HID v1.00
>> Mouse [WCOM482F:00 056A:482F] on i2c-WCOM482F:00
>> > [ 7.275279] i2c_hid i2c-DLL077A:01: i2c-DLL077A:01 supply vdd not found,
>> using dummy regulator
>> > [ 7.304107] input: DLL077A:01 06CB:76AF as
>> /devices/pci0000:00/0000:00:15.1/i2c_designware.1/i2c-7/i2c-
>> DLL077A:01/0018:06CB:76AF.0007/input/input14
>> > [ 7.304212] hid-generic 0018:06CB:76AF.0007: input,hidraw1: I2C HID v1.00
>> Mouse [DLL077A:01 06CB:76AF] on i2c-DLL077A:01
>> > [ 7.657123] usbcore: registered new interface driver usbhid
>> > [ 7.657124] usbhid: USB HID core driver
>> > [ 7.722876] input: Wacom HID 482F Pen as
>> /devices/pci0000:00/0000:00:15.0/i2c_designware.0/i2c-6/i2c-
>> WCOM482F:00/0018:056A:482F.0006/input/input15
>> > [ 7.723148] input: Wacom HID 482F Finger as
>> /devices/pci0000:00/0000:00:15.0/i2c_designware.0/i2c-6/i2c-
>> WCOM482F:00/0018:056A:482F.0006/input/input16
>> > [ 7.723611] wacom 0018:056A:482F.0006: hidraw0: I2C HID v1.00 Mouse
>> [WCOM482F:00 056A:482F] on i2c-WCOM482F:00
>> > [ 7.768275] input: DLL077A:01 06CB:76AF Touchpad as
>> /devices/pci0000:00/0000:00:15.1/i2c_designware.1/i2c-7/i2c-
>> DLL077A:01/0018:06CB:76AF.0007/input/input19
>> > [ 7.864201] hid-multitouch 0018:06CB:76AF.0007: input,hidraw0: I2C HID v1.00
>> Mouse [DLL077A:01 06CB:76AF] on i2c-DLL077A:01
>> >
>> > However in this state, I can't rmmod i2c-hid. It just hangs the system with this
>> trace:
>> > [ 243.033779] INFO: task kworker/u8:0:6 blocked for more than 120 seconds.
>> > [ 243.033793] Not tainted 4.17.0-rc1+ #37
>> > [ 243.033798] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this
>> message.
>> > [ 243.033804] kworker/u8:0 D 0 6 2 0x80000000
>> > [ 243.033826] Workqueue: events_power_efficient
>> power_supply_deferred_register_work
>> > [ 243.033832] Call Trace:
>> > [ 243.033850] __schedule+0x3c2/0x890
>> > [ 243.033861] ? __switch_to_asm+0x40/0x70
>> > [ 243.033868] schedule+0x36/0x80
>> > [ 243.033875] schedule_preempt_disabled+0xe/0x10
>> > [ 243.033882] __mutex_lock.isra.4+0x2ae/0x4e0
>> > [ 243.033890] ? __switch_to_asm+0x34/0x70
>> > [ 243.033899] ? __switch_to_asm+0x40/0x70
>> > [ 243.033906] ? __switch_to_asm+0x40/0x70
>> > [ 243.033914] __mutex_lock_slowpath+0x13/0x20
>> > [ 243.033920] ? __mutex_lock_slowpath+0x13/0x20
>> > [ 243.033927] mutex_lock+0x2f/0x40
>> > [ 243.033933] power_supply_deferred_register_work+0x2b/0x50
>> > [ 243.033944] process_one_work+0x148/0x3d0
>> > [ 243.033952] worker_thread+0x4b/0x460
>> > [ 243.033960] kthread+0x102/0x140
>> > [ 243.033967] ? rescuer_thread+0x380/0x380
>> > [ 243.033973] ? kthread_associate_blkcg+0xa0/0xa0
>> > [ 243.033982] ret_from_fork+0x35/0x40
>> > [ 243.034012] INFO: task systemd-udevd:308 blocked for more than 120 seconds.
>> > [ 243.034018] Not tainted 4.17.0-rc1+ #37
>> > [ 243.034022] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this
>> message.
>> > [ 243.034027] systemd-udevd D 0 308 279 0x80000104
>> > [ 243.034033] Call Trace:
>> > [ 243.034041] __schedule+0x3c2/0x890
>> > [ 243.034049] schedule+0x36/0x80
>> > [ 243.034056] schedule_timeout+0x1e7/0x360
>> > [ 243.034066] ? ttwu_do_activate+0x77/0x80
>> > [ 243.034074] wait_for_completion+0xb4/0x140
>> > [ 243.034082] ? wake_up_q+0x70/0x70
>> > [ 243.034090] flush_work+0x12a/0x1e0
>> > [ 243.034097] ? worker_detach_from_pool+0xb0/0xb0
>> > [ 243.034107] __cancel_work_timer+0x112/0x190
>> > [ 243.034116] cancel_delayed_work_sync+0x13/0x20
>> > [ 243.034122] power_supply_unregister+0x37/0xb0
>> > [ 243.034127] devm_power_supply_release+0x11/0x20
>> > [ 243.034135] release_nodes+0x107/0x1f0
>> > [ 243.034147] devres_release_group+0x7c/0xb0
>> > [ 243.034162] wacom_remove+0xce/0x120 [wacom]
>> > [ 243.034178] hid_device_remove+0x4d/0xa0 [hid]
>> > [ 243.034187] device_release_driver_internal+0x155/0x220
>> > [ 243.034198] ? __hid_bus_driver_added+0x40/0x40 [hid]
>> > [ 243.034208] ? hid_destroy_device+0x60/0x60 [hid]
>> > [ 243.034215] device_release_driver+0x12/0x20
>> > [ 243.034221] device_reprobe+0x30/0x50
>> > [ 243.034231] __hid_bus_reprobe_drivers+0x45/0x50 [hid]
>> > [ 243.034239] bus_for_each_dev+0x64/0xb0
>> > [ 243.034250] __hid_bus_driver_added+0x2c/0x40 [hid]
>> > [ 243.034256] bus_for_each_drv+0x67/0xb0
>> > [ 243.034267] __hid_register_driver+0x6f/0x80 [hid]
>> > [ 243.034275] ? 0xffffffffc07f5000
>> > [ 243.034287] mt_driver_init+0x23/0x1000 [hid_multitouch]
>> > [ 243.034296] do_one_initcall+0x4f/0x1ce
>> > [ 243.034303] ? _cond_resched+0x1a/0x50
>> > [ 243.034315] ? kmem_cache_alloc_trace+0xb8/0x1f0
>> > [ 243.034327] do_init_module+0x5f/0x219
>> > [ 243.034335] load_module+0x24c7/0x2be0
>> > [ 243.034348] __do_sys_finit_module+0xe5/0x120
>> > [ 243.034354] ? __do_sys_finit_module+0xe5/0x120
>> > [ 243.034363] __x64_sys_finit_module+0x1a/0x20
>> > [ 243.034370] do_syscall_64+0x54/0x110
>> > [ 243.034380] entry_SYSCALL_64_after_hwframe+0x44/0xa9
>> > [ 243.034386] RIP: 0033:0x7f2ec539c839
>> > [ 243.034390] RSP: 002b:00007ffcfc620298 EFLAGS: 00000246 ORIG_RAX:
>> 0000000000000139
>> > [ 243.034396] RAX: ffffffffffffffda RBX: 000055d6a0147ec0 RCX:
>> 00007f2ec539c839
>> > [ 243.034399] RDX: 0000000000000000 RSI: 00007f2ec507b0e5 RDI:
>> 000000000000000f
>> > [ 243.034402] RBP: 00007f2ec507b0e5 R08: 0000000000000000 R09:
>> 00007ffcfc6203b0
>> > [ 243.034405] R10: 000000000000000f R11: 0000000000000246 R12:
>> 0000000000000000
>> > [ 243.034408] R13: 000055d6a017b3a0 R14: 0000000000020000 R15:
>> 000055d6a0147ec0
>> > --
>> > To unsubscribe from this list: send the line "unsubscribe linux-input" in
>> > the body of a message to majordomo@vger.kernel.org
>> > More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 1/6] ARM: OMAP1: ams-delta: add GPIO lookup tables
From: Janusz Krzysztofik @ 2018-05-18 21:09 UTC (permalink / raw)
To: Tony Lindgren, Dmitry Torokhov, Boris Brezillon, Tomi Valkeinen,
Mark Brown
Cc: Aaro Koskinen, Richard Weinberger, Peter Ujfalusi, Jarkko Nikula,
Liam Girdwood, linux-arm-kernel, linux-omap, linux-kernel,
linux-input, linux-mtd, linux-fbdev, alsa-devel,
Janusz Krzysztofik
Scope of the change is limited to GPIO pins used by board specific
device drivers which will be updated by follow-up patches of the
series. Those are some OMAP GPIO (gpio-0-15) and most of Amstrad Delta
latch2 GPIO bank pins. Remaining pins of those banks, as well as
Amstrad Delta latch1 pins, will be addressed later.
Assign a label ("latch2") to the bank, enumerate its pins and put that
information, together with OMAP GPIO bank pins, in GPIO lookup tables.
Assign lookup tables to devices as soon as those devices are registered
and their names can be obtained.
A step froward in:
- removal of hard-coded GPIO numbers from drivers,
- removal of board mach includes from drivers,
- switching to dynamically assigned GPIO numbers.
Created and compile tested agains linux-4.17-rc3
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
arch/arm/mach-omap1/board-ams-delta.c | 102 ++++++++++++++++++++++++++++++++++
1 file changed, 102 insertions(+)
diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
index 52e8e53ca154..4b78e73f8bf7 100644
--- a/arch/arm/mach-omap1/board-ams-delta.c
+++ b/arch/arm/mach-omap1/board-ams-delta.c
@@ -12,6 +12,7 @@
* published by the Free Software Foundation.
*/
#include <linux/gpio/driver.h>
+#include <linux/gpio/machine.h>
#include <linux/gpio.h>
#include <linux/kernel.h>
#include <linux/init.h>
@@ -202,7 +203,10 @@ static struct resource latch2_resources[] = {
},
};
+#define LATCH2_LABEL "latch2"
+
static struct bgpio_pdata latch2_pdata = {
+ .label = LATCH2_LABEL,
.base = AMS_DELTA_LATCH2_GPIO_BASE,
.ngpio = AMS_DELTA_LATCH2_NGPIO,
};
@@ -217,6 +221,23 @@ static struct platform_device latch2_gpio_device = {
},
};
+#define LATCH2_PIN_LCD_VBLEN 0
+#define LATCH2_PIN_LCD_NDISP 1
+#define LATCH2_PIN_NAND_NCE 2
+#define LATCH2_PIN_NAND_NRE 3
+#define LATCH2_PIN_NAND_NWP 4
+#define LATCH2_PIN_NAND_NWE 5
+#define LATCH2_PIN_NAND_ALE 6
+#define LATCH2_PIN_NAND_CLE 7
+#define LATCH2_PIN_KEYBRD_PWR 8
+#define LATCH2_PIN_KEYBRD_DATAOUT 9
+#define LATCH2_PIN_SCARD_RSTIN 10
+#define LATCH2_PIN_SCARD_CMDVCC 11
+#define LATCH2_PIN_MODEM_NRESET 12
+#define LATCH2_PIN_MODEM_CODEC 13
+#define LATCH2_PIN_HOOKFLASH1 14
+#define LATCH2_PIN_HOOKFLASH2 15
+
static const struct gpio latch_gpios[] __initconst = {
{
.gpio = LATCH1_GPIO_BASE + 6,
@@ -323,6 +344,22 @@ static struct platform_device ams_delta_nand_device = {
.resource = ams_delta_nand_resources,
};
+#define OMAP_GPIO_LABEL "gpio-0-15"
+
+static struct gpiod_lookup_table ams_delta_nand_gpio_table = {
+ .table = {
+ GPIO_LOOKUP(OMAP_GPIO_LABEL, AMS_DELTA_GPIO_PIN_NAND_RB, "rdy",
+ 0),
+ GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_NAND_NCE, "nce", 0),
+ GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_NAND_NRE, "nre", 0),
+ GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_NAND_NWP, "nwp", 0),
+ GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_NAND_NWE, "nwe", 0),
+ GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_NAND_ALE, "ale", 0),
+ GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_NAND_CLE, "cle", 0),
+ { },
+ },
+};
+
static struct resource ams_delta_kp_resources[] = {
[0] = {
.start = INT_KEYBOARD,
@@ -358,6 +395,14 @@ static struct platform_device ams_delta_lcd_device = {
.id = -1,
};
+static struct gpiod_lookup_table ams_delta_lcd_gpio_table = {
+ .table = {
+ GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_LCD_VBLEN, "vblen", 0),
+ GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_LCD_NDISP, "ndisp", 0),
+ { },
+ },
+};
+
static const struct gpio_led gpio_leds[] __initconst = {
{
.name = "camera",
@@ -449,11 +494,35 @@ static struct platform_device ams_delta_audio_device = {
.id = -1,
};
+static struct gpiod_lookup_table ams_delta_audio_gpio_table = {
+ .table = {
+ GPIO_LOOKUP(OMAP_GPIO_LABEL, AMS_DELTA_GPIO_PIN_HOOK_SWITCH,
+ "hook_switch", 0),
+ GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_MODEM_CODEC,
+ "modem_codec", 0),
+ { },
+ },
+};
+
static struct platform_device cx20442_codec_device = {
.name = "cx20442-codec",
.id = -1,
};
+static struct gpiod_lookup_table ams_delta_serio_gpio_table = {
+ .table = {
+ GPIO_LOOKUP(OMAP_GPIO_LABEL, AMS_DELTA_GPIO_PIN_KEYBRD_DATA,
+ "data", 0),
+ GPIO_LOOKUP(OMAP_GPIO_LABEL, AMS_DELTA_GPIO_PIN_KEYBRD_CLK,
+ "clock", 0),
+ GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_KEYBRD_PWR,
+ "power", 0),
+ GPIO_LOOKUP(LATCH2_LABEL, LATCH2_PIN_KEYBRD_DATAOUT,
+ "dataout", 0),
+ { },
+ },
+};
+
static struct platform_device *ams_delta_devices[] __initdata = {
&latch1_gpio_device,
&latch2_gpio_device,
@@ -468,6 +537,16 @@ static struct platform_device *late_devices[] __initdata = {
&cx20442_codec_device,
};
+static struct gpiod_lookup_table *ams_delta_gpio_tables[] __initdata = {
+ &ams_delta_audio_gpio_table,
+ &ams_delta_serio_gpio_table,
+};
+
+static struct gpiod_lookup_table *late_gpio_tables[] __initdata = {
+ &ams_delta_lcd_gpio_table,
+ &ams_delta_nand_gpio_table,
+};
+
static void __init ams_delta_init(void)
{
/* mux pins for uarts */
@@ -500,6 +579,20 @@ static void __init ams_delta_init(void)
gpio_led_register_device(-1, &leds_pdata);
platform_add_devices(ams_delta_devices, ARRAY_SIZE(ams_delta_devices));
+ /*
+ * As soon as devices have been registered, assign their dev_names
+ * to respective GPIO lookup tables before they are added.
+ */
+ ams_delta_audio_gpio_table.dev_id =
+ dev_name(&ams_delta_audio_device.dev);
+ /*
+ * No device name is assigned to GPIO lookup table for serio device
+ * as long as serio driver is not converted to platform device driver.
+ */
+
+ gpiod_add_lookup_tables(ams_delta_gpio_tables,
+ ARRAY_SIZE(ams_delta_gpio_tables));
+
ams_delta_init_fiq();
omap_writew(omap_readw(ARM_RSTCT1) | 0x0004, ARM_RSTCT1);
@@ -570,6 +663,15 @@ static int __init late_init(void)
platform_add_devices(late_devices, ARRAY_SIZE(late_devices));
+ /*
+ * As soon as devices have been registered, assign their dev_names
+ * to respective GPIO lookup tables before they are added.
+ */
+ ams_delta_lcd_gpio_table.dev_id = dev_name(&ams_delta_lcd_device.dev);
+ ams_delta_nand_gpio_table.dev_id = dev_name(&ams_delta_nand_device.dev);
+
+ gpiod_add_lookup_tables(late_gpio_tables, ARRAY_SIZE(late_gpio_tables));
+
err = platform_device_register(&modem_nreset_device);
if (err) {
pr_err("Couldn't register the modem regulator device\n");
--
2.16.1
^ permalink raw reply related
* [PATCH 2/6] Input: ams_delta_serio: use GPIO lookup table
From: Janusz Krzysztofik @ 2018-05-18 21:09 UTC (permalink / raw)
To: Tony Lindgren, Dmitry Torokhov, Boris Brezillon, Tomi Valkeinen,
Mark Brown
Cc: Aaro Koskinen, Richard Weinberger, Peter Ujfalusi, Jarkko Nikula,
Liam Girdwood, linux-arm-kernel, linux-omap, linux-kernel,
linux-input, linux-mtd, linux-fbdev, alsa-devel,
Janusz Krzysztofik
In-Reply-To: <20180518210954.29044-1-jmkrzyszt@gmail.com>
Now as the Amstrad Delta board provides GPIO lookup tables, switch from
GPIO numbers to GPIO descriptors and use the table to locate required
GPIO pins.
Declare static variables for storing GPIO descriptors and replace
gpio_ functions with their gpiod_ equivalents.
Pin naming used by the driver should be followed while respective GPIO
lookup table is initialized by a board init code.
Created and tested against linux-4.17-rc3, on top of patch 1/6 "ARM:
OMAP1: ams-delta: add GPIO lookup tables"
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
drivers/input/serio/ams_delta_serio.c | 98 +++++++++++++++++++----------------
1 file changed, 53 insertions(+), 45 deletions(-)
diff --git a/drivers/input/serio/ams_delta_serio.c b/drivers/input/serio/ams_delta_serio.c
index 3df501c3421b..dd1f8f118c08 100644
--- a/drivers/input/serio/ams_delta_serio.c
+++ b/drivers/input/serio/ams_delta_serio.c
@@ -20,14 +20,13 @@
* 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/gpio/consumer.h>
#include <linux/irq.h>
#include <linux/serio.h>
#include <linux/slab.h>
#include <linux/module.h>
#include <asm/mach-types.h>
-#include <mach/board-ams-delta.h>
#include <mach/ams-delta-fiq.h>
@@ -36,6 +35,10 @@ MODULE_DESCRIPTION("AMS Delta (E3) keyboard port driver");
MODULE_LICENSE("GPL");
static struct serio *ams_delta_serio;
+static struct gpio_desc *gpiod_data;
+static struct gpio_desc *gpiod_clock;
+static struct gpio_desc *gpiod_power;
+static struct gpio_desc *gpiod_dataout;
static int check_data(int data)
{
@@ -92,7 +95,7 @@ static irqreturn_t ams_delta_serio_interrupt(int irq, void *dev_id)
static int ams_delta_serio_open(struct serio *serio)
{
/* enable keyboard */
- gpio_set_value(AMS_DELTA_GPIO_PIN_KEYBRD_PWR, 1);
+ gpiod_set_value(gpiod_power, 1);
return 0;
}
@@ -100,32 +103,9 @@ static int ams_delta_serio_open(struct serio *serio)
static void ams_delta_serio_close(struct serio *serio)
{
/* disable keyboard */
- gpio_set_value(AMS_DELTA_GPIO_PIN_KEYBRD_PWR, 0);
+ gpiod_set_value(gpiod_power, 0);
}
-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",
- },
- {
- .gpio = AMS_DELTA_GPIO_PIN_KEYBRD_PWR,
- .flags = GPIOF_OUT_INIT_LOW,
- .label = "serio-power",
- },
- {
- .gpio = AMS_DELTA_GPIO_PIN_KEYBRD_DATAOUT,
- .flags = GPIOF_OUT_INIT_LOW,
- .label = "serio-dataout",
- },
-};
-
static int __init ams_delta_serio_init(void)
{
int err;
@@ -145,36 +125,62 @@ static int __init ams_delta_serio_init(void)
strlcpy(ams_delta_serio->phys, "GPIO/serio0",
sizeof(ams_delta_serio->phys));
- err = gpio_request_array(ams_delta_gpios,
- ARRAY_SIZE(ams_delta_gpios));
- if (err) {
- pr_err("ams_delta_serio: Couldn't request gpio pins\n");
+ gpiod_data = gpiod_get(NULL, "data", GPIOD_IN);
+ if (IS_ERR(gpiod_data)) {
+ err = PTR_ERR(gpiod_data);
+ pr_err("%s: 'data' GPIO request failed (%d)\n", __func__,
+ err);
goto serio;
}
+ gpiod_clock = gpiod_get(NULL, "clock", GPIOD_IN);
+ if (IS_ERR(gpiod_clock)) {
+ err = PTR_ERR(gpiod_clock);
+ pr_err("%s: 'clock' GPIO request failed (%d)\n", __func__,
+ err);
+ goto gpio_data;
+ }
+ gpiod_power = gpiod_get(NULL, "power", GPIOD_OUT_LOW);
+ if (IS_ERR(gpiod_power)) {
+ err = PTR_ERR(gpiod_power);
+ pr_err("%s: 'power' GPIO request failed (%d)\n", __func__,
+ err);
+ goto gpio_clock;
+ }
+ gpiod_dataout = gpiod_get(NULL, "dataout", GPIOD_OUT_LOW);
+ if (IS_ERR(gpiod_dataout)) {
+ err = PTR_ERR(gpiod_dataout);
+ pr_err("%s: 'dataout' GPIO request failed (%d)\n",
+ __func__, err);
+ goto gpio_power;
+ }
- err = request_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK),
- ams_delta_serio_interrupt, IRQ_TYPE_EDGE_RISING,
- "ams-delta-serio", 0);
+ err = request_irq(gpiod_to_irq(gpiod_clock),
+ ams_delta_serio_interrupt, IRQ_TYPE_EDGE_RISING,
+ "ams-delta-serio", 0);
if (err < 0) {
- pr_err("ams_delta_serio: couldn't request gpio interrupt %d\n",
- gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK));
- goto gpio;
+ pr_err("%s: 'clock' GPIO interrupt request failed (%d)\n",
+ __func__, err);
+ goto gpio_dataout;
}
/*
* 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);
+ irq_set_handler(gpiod_to_irq(gpiod_clock), handle_simple_irq);
serio_register_port(ams_delta_serio);
dev_info(&ams_delta_serio->dev, "%s\n", ams_delta_serio->name);
return 0;
-gpio:
- gpio_free_array(ams_delta_gpios,
- ARRAY_SIZE(ams_delta_gpios));
+gpio_dataout:
+ gpiod_put(gpiod_dataout);
+gpio_power:
+ gpiod_put(gpiod_power);
+gpio_clock:
+ gpiod_put(gpiod_clock);
+gpio_data:
+ gpiod_put(gpiod_data);
serio:
kfree(ams_delta_serio);
return err;
@@ -184,8 +190,10 @@ module_init(ams_delta_serio_init);
static void __exit ams_delta_serio_exit(void)
{
serio_unregister_port(ams_delta_serio);
- free_irq(gpio_to_irq(AMS_DELTA_GPIO_PIN_KEYBRD_CLK), 0);
- gpio_free_array(ams_delta_gpios,
- ARRAY_SIZE(ams_delta_gpios));
+ free_irq(gpiod_to_irq(gpiod_clock), 0);
+ gpiod_put(gpiod_dataout);
+ gpiod_put(gpiod_power);
+ gpiod_put(gpiod_clock);
+ gpiod_put(gpiod_data);
}
module_exit(ams_delta_serio_exit);
--
2.16.1
^ permalink raw reply related
* [PATCH 3/6] ASoC: ams_delta: use GPIO lookup table
From: Janusz Krzysztofik @ 2018-05-18 21:09 UTC (permalink / raw)
To: Tony Lindgren, Dmitry Torokhov, Boris Brezillon, Tomi Valkeinen,
Mark Brown
Cc: Aaro Koskinen, Richard Weinberger, Peter Ujfalusi, Jarkko Nikula,
Liam Girdwood, linux-arm-kernel, linux-omap, linux-kernel,
linux-input, linux-mtd, linux-fbdev, alsa-devel,
Janusz Krzysztofik
In-Reply-To: <20180518210954.29044-1-jmkrzyszt@gmail.com>
Now as the Amstrad Delta board provides GPIO lookup tables, switch from
GPIO numbers to GPIO descriptors and use the table to locate required
GPIO pins.
The card uses two pins, one for jack and the other for voice modem
codec DAI control.
For jack pin, remove hardcoded GPIO number and use GPIO descriptor
based variant of jack GPIO initialization.
For modem_codec pin, declare static variable for storing its GPIO
descriptor, obtain it on card initialization and replace obsolete
ams_delta_latch2_write() with gpiod_set_value(). For that to work,
don't request the modem_codec pin from the board init code anymore.
If the modem_codec GPIO lookup fails, skip initialization of
functionality of the card which depends on its availability.
Pin naming used by the driver should be followed while respective GPIO
lookup table is initialized by a board init code.
Created and tested against linux-4.17-rc3, on top of patch 1/6 "ARM:
OMAP1: ams-delta: add GPIO lookup tables"
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
arch/arm/mach-omap1/board-ams-delta.c | 5 -----
sound/soc/omap/ams-delta.c | 38 +++++++++++++++++++----------------
2 files changed, 21 insertions(+), 22 deletions(-)
diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
index 4b78e73f8bf7..80f54cb54276 100644
--- a/arch/arm/mach-omap1/board-ams-delta.c
+++ b/arch/arm/mach-omap1/board-ams-delta.c
@@ -259,11 +259,6 @@ static const struct gpio latch_gpios[] __initconst = {
.flags = GPIOF_OUT_INIT_LOW,
.label = "scard_cmdvcc",
},
- {
- .gpio = AMS_DELTA_GPIO_PIN_MODEM_CODEC,
- .flags = GPIOF_OUT_INIT_LOW,
- .label = "modem_codec",
- },
{
.gpio = AMS_DELTA_LATCH2_GPIO_BASE + 14,
.flags = GPIOF_OUT_INIT_LOW,
diff --git a/sound/soc/omap/ams-delta.c b/sound/soc/omap/ams-delta.c
index 77a30f0f0c96..4dce494dfbd3 100644
--- a/sound/soc/omap/ams-delta.c
+++ b/sound/soc/omap/ams-delta.c
@@ -22,7 +22,7 @@
*
*/
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
#include <linux/spinlock.h>
#include <linux/tty.h>
#include <linux/module.h>
@@ -32,7 +32,6 @@
#include <asm/mach-types.h>
-#include <mach/board-ams-delta.h>
#include <linux/platform_data/asoc-ti-mcbsp.h>
#include "omap-mcbsp.h"
@@ -213,7 +212,6 @@ static const struct snd_kcontrol_new ams_delta_audio_controls[] = {
static struct snd_soc_jack ams_delta_hook_switch;
static struct snd_soc_jack_gpio ams_delta_hook_switch_gpios[] = {
{
- .gpio = 4,
.name = "hook_switch",
.report = SND_JACK_HEADSET,
.invert = 1,
@@ -259,6 +257,7 @@ static struct timer_list cx81801_timer;
static bool cx81801_cmd_pending;
static bool ams_delta_muted;
static DEFINE_SPINLOCK(ams_delta_lock);
+static struct gpio_desc *gpiod_modem_codec;
static void cx81801_timeout(struct timer_list *unused)
{
@@ -272,7 +271,7 @@ static void cx81801_timeout(struct timer_list *unused)
/* Reconnect the codec DAI back from the modem to the CPU DAI
* only if digital mute still off */
if (!muted)
- ams_delta_latch2_write(AMS_DELTA_LATCH2_MODEM_CODEC, 0);
+ gpiod_set_value(gpiod_modem_codec, 0);
}
/* Line discipline .open() */
@@ -381,8 +380,7 @@ static void cx81801_receive(struct tty_struct *tty,
/* Apply config pulse by connecting the codec to the modem
* if not already done */
if (apply)
- ams_delta_latch2_write(AMS_DELTA_LATCH2_MODEM_CODEC,
- AMS_DELTA_LATCH2_MODEM_CODEC);
+ gpiod_set_value(gpiod_modem_codec, 1);
break;
}
}
@@ -432,8 +430,7 @@ static int ams_delta_digital_mute(struct snd_soc_dai *dai, int mute)
spin_unlock_bh(&ams_delta_lock);
if (apply)
- ams_delta_latch2_write(AMS_DELTA_LATCH2_MODEM_CODEC,
- mute ? AMS_DELTA_LATCH2_MODEM_CODEC : 0);
+ gpiod_set_value(gpiod_modem_codec, !!mute);
return 0;
}
@@ -469,14 +466,6 @@ static int ams_delta_cx20442_init(struct snd_soc_pcm_runtime *rtd)
/* Store a pointer to the codec structure for tty ldisc use */
cx20442_codec = rtd->codec_dai->component;
- /* Set up digital mute if not provided by the codec */
- if (!codec_dai->driver->ops) {
- codec_dai->driver->ops = &ams_delta_dai_ops;
- } else {
- ams_delta_ops.startup = ams_delta_startup;
- ams_delta_ops.shutdown = ams_delta_shutdown;
- }
-
/* Add hook switch - can be used to control the codec from userspace
* even if line discipline fails */
ret = snd_soc_card_jack_new(card, "hook_switch", SND_JACK_HEADSET,
@@ -486,7 +475,7 @@ static int ams_delta_cx20442_init(struct snd_soc_pcm_runtime *rtd)
"Failed to allocate resources for hook switch, "
"will continue without one.\n");
else {
- ret = snd_soc_jack_add_gpios(&ams_delta_hook_switch,
+ ret = snd_soc_jack_add_gpiods(card->dev, &ams_delta_hook_switch,
ARRAY_SIZE(ams_delta_hook_switch_gpios),
ams_delta_hook_switch_gpios);
if (ret)
@@ -495,6 +484,21 @@ static int ams_delta_cx20442_init(struct snd_soc_pcm_runtime *rtd)
"will continue with hook switch inactive.\n");
}
+ gpiod_modem_codec = devm_gpiod_get(card->dev, "modem_codec",
+ GPIOD_OUT_HIGH);
+ if (IS_ERR(gpiod_modem_codec)) {
+ dev_warn(card->dev, "Failed to obtain modem_codec GPIO\n");
+ return 0;
+ }
+
+ /* Set up digital mute if not provided by the codec */
+ if (!codec_dai->driver->ops) {
+ codec_dai->driver->ops = &ams_delta_dai_ops;
+ } else {
+ ams_delta_ops.startup = ams_delta_startup;
+ ams_delta_ops.shutdown = ams_delta_shutdown;
+ }
+
/* Register optional line discipline for over the modem control */
ret = tty_register_ldisc(N_V253, &cx81801_ops);
if (ret) {
--
2.16.1
^ permalink raw reply related
* [PATCH 4/6] fbdev: omapfb: lcd_ams_delta: use GPIO lookup table
From: Janusz Krzysztofik @ 2018-05-18 21:09 UTC (permalink / raw)
To: Tony Lindgren, Dmitry Torokhov, Boris Brezillon, Tomi Valkeinen,
Mark Brown
Cc: linux-fbdev, alsa-devel, Aaro Koskinen, Richard Weinberger,
Janusz Krzysztofik, Liam Girdwood, linux-kernel, Peter Ujfalusi,
linux-mtd, Jarkko Nikula, linux-input, linux-omap,
linux-arm-kernel
In-Reply-To: <20180518210954.29044-1-jmkrzyszt@gmail.com>
Now as the Amstrad Delta board provides GPIO lookup tables, switch from
GPIO numbers to GPIO descriptors and use the table to locate required
GPIO pins.
Declare static variables for storing GPIO descriptors and replace
gpio_ functions with their gpiod_ equivalents. Move GPIO lookup
to the driver probe function so device initialization can be postponed
instead of aborted if the GPIO pin is not yet available.
Pin naming used by the driver should be followed while respective GPIO
lookup table is initialized by a board init code.
Created and tested against linux-4.17-rc3, on top of patch 1/6 "ARM:
OMAP1: ams-delta: add GPIO lookup tables"
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
drivers/video/fbdev/omap/lcd_ams_delta.c | 59 ++++++++++++++------------------
1 file changed, 26 insertions(+), 33 deletions(-)
diff --git a/drivers/video/fbdev/omap/lcd_ams_delta.c b/drivers/video/fbdev/omap/lcd_ams_delta.c
index a4ee947006c7..19b6425b54be 100644
--- a/drivers/video/fbdev/omap/lcd_ams_delta.c
+++ b/drivers/video/fbdev/omap/lcd_ams_delta.c
@@ -24,11 +24,10 @@
#include <linux/platform_device.h>
#include <linux/io.h>
#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
#include <linux/lcd.h>
-#include <linux/gpio.h>
#include <mach/hardware.h>
-#include <mach/board-ams-delta.h>
#include "omapfb.h"
@@ -41,6 +40,8 @@
/* LCD class device section */
static int ams_delta_lcd;
+static struct gpio_desc *gpiod_vblen;
+static struct gpio_desc *gpiod_ndisp;
static int ams_delta_lcd_set_power(struct lcd_device *dev, int power)
{
@@ -99,41 +100,17 @@ static struct lcd_ops ams_delta_lcd_ops = {
/* omapfb panel section */
-static const struct gpio _gpios[] = {
- {
- .gpio = AMS_DELTA_GPIO_PIN_LCD_VBLEN,
- .flags = GPIOF_OUT_INIT_LOW,
- .label = "lcd_vblen",
- },
- {
- .gpio = AMS_DELTA_GPIO_PIN_LCD_NDISP,
- .flags = GPIOF_OUT_INIT_LOW,
- .label = "lcd_ndisp",
- },
-};
-
-static int ams_delta_panel_init(struct lcd_panel *panel,
- struct omapfb_device *fbdev)
-{
- return gpio_request_array(_gpios, ARRAY_SIZE(_gpios));
-}
-
-static void ams_delta_panel_cleanup(struct lcd_panel *panel)
-{
- gpio_free_array(_gpios, ARRAY_SIZE(_gpios));
-}
-
static int ams_delta_panel_enable(struct lcd_panel *panel)
{
- gpio_set_value(AMS_DELTA_GPIO_PIN_LCD_NDISP, 1);
- gpio_set_value(AMS_DELTA_GPIO_PIN_LCD_VBLEN, 1);
+ gpiod_set_value(gpiod_ndisp, 1);
+ gpiod_set_value(gpiod_vblen, 1);
return 0;
}
static void ams_delta_panel_disable(struct lcd_panel *panel)
{
- gpio_set_value(AMS_DELTA_GPIO_PIN_LCD_VBLEN, 0);
- gpio_set_value(AMS_DELTA_GPIO_PIN_LCD_NDISP, 0);
+ gpiod_set_value(gpiod_vblen, 0);
+ gpiod_set_value(gpiod_ndisp, 0);
}
static struct lcd_panel ams_delta_panel = {
@@ -154,8 +131,6 @@ static struct lcd_panel ams_delta_panel = {
.pcd = 0,
.acb = 37,
- .init = ams_delta_panel_init,
- .cleanup = ams_delta_panel_cleanup,
.enable = ams_delta_panel_enable,
.disable = ams_delta_panel_disable,
};
@@ -166,9 +141,27 @@ static struct lcd_panel ams_delta_panel = {
static int ams_delta_panel_probe(struct platform_device *pdev)
{
struct lcd_device *lcd_device = NULL;
-#ifdef CONFIG_LCD_CLASS_DEVICE
int ret;
+ gpiod_vblen = devm_gpiod_get(&pdev->dev, "vblen", GPIOD_OUT_LOW);
+ if (IS_ERR(gpiod_vblen)) {
+ ret = PTR_ERR(gpiod_vblen);
+ dev_err(&pdev->dev, "VBLEN GPIO request failed (%d)\n", ret);
+ if (ret == -ENODEV || ret == -ENOENT)
+ ret = -EPROBE_DEFER;
+ return ret;
+ }
+
+ gpiod_ndisp = devm_gpiod_get(&pdev->dev, "ndisp", GPIOD_OUT_LOW);
+ if (IS_ERR(gpiod_ndisp)) {
+ ret = PTR_ERR(gpiod_ndisp);
+ dev_err(&pdev->dev, "NDISP GPIO request failed (%d)\n", ret);
+ if (ret == -ENODEV || ret == -ENOENT)
+ ret = -EPROBE_DEFER;
+ return ret;
+ }
+
+#ifdef CONFIG_LCD_CLASS_DEVICE
lcd_device = lcd_device_register("omapfb", &pdev->dev, NULL,
&ams_delta_lcd_ops);
--
2.16.1
^ permalink raw reply related
* [PATCH 5/6] mtd: rawnand: ams-delta: use GPIO lookup table
From: Janusz Krzysztofik @ 2018-05-18 21:09 UTC (permalink / raw)
To: Tony Lindgren, Dmitry Torokhov, Boris Brezillon, Tomi Valkeinen,
Mark Brown
Cc: Aaro Koskinen, Richard Weinberger, Peter Ujfalusi, Jarkko Nikula,
Liam Girdwood, linux-arm-kernel, linux-omap, linux-kernel,
linux-input, linux-mtd, linux-fbdev, alsa-devel,
Janusz Krzysztofik
In-Reply-To: <20180518210954.29044-1-jmkrzyszt@gmail.com>
Now as the Amstrad Delta board provides GPIO lookup tables, switch from
GPIO numbers to GPIO descriptors and use the table to locate required
GPIO pins.
Declare static variables for storing GPIO descriptors and replace
gpio_ functions with their gpiod_ equivalents. Return -EPROBE_DEFER
if the GPIO pins are not yet available so device initialization is
postponed instead of aborted.
Pin naming used by the driver should be followed while respective GPIO
lookup table is initialized by a board init code.
Created and tested against linux-4.17-rc3, on top of patch 1/6 "ARM:
OMAP1: ams-delta: add GPIO lookup tables"
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
drivers/mtd/nand/raw/ams-delta.c | 110 +++++++++++++++++++++------------------
1 file changed, 58 insertions(+), 52 deletions(-)
diff --git a/drivers/mtd/nand/raw/ams-delta.c b/drivers/mtd/nand/raw/ams-delta.c
index 37a3cc21c7bc..c44be2f5a65c 100644
--- a/drivers/mtd/nand/raw/ams-delta.c
+++ b/drivers/mtd/nand/raw/ams-delta.c
@@ -20,23 +20,28 @@
#include <linux/slab.h>
#include <linux/module.h>
#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
#include <linux/mtd/mtd.h>
#include <linux/mtd/rawnand.h>
#include <linux/mtd/partitions.h>
-#include <linux/gpio.h>
#include <linux/platform_data/gpio-omap.h>
#include <asm/io.h>
#include <asm/sizes.h>
-#include <mach/board-ams-delta.h>
-
#include <mach/hardware.h>
/*
* MTD structure for E3 (Delta)
*/
static struct mtd_info *ams_delta_mtd = NULL;
+static struct gpio_desc *gpiod_rdy;
+static struct gpio_desc *gpiod_nce;
+static struct gpio_desc *gpiod_nre;
+static struct gpio_desc *gpiod_nwp;
+static struct gpio_desc *gpiod_nwe;
+static struct gpio_desc *gpiod_ale;
+static struct gpio_desc *gpiod_cle;
/*
* Define partitions for flash devices
@@ -70,9 +75,9 @@ static void ams_delta_write_byte(struct mtd_info *mtd, u_char byte)
writew(0, io_base + OMAP_MPUIO_IO_CNTL);
writew(byte, this->IO_ADDR_W);
- gpio_set_value(AMS_DELTA_GPIO_PIN_NAND_NWE, 0);
+ gpiod_set_value(gpiod_nwe, 0);
ndelay(40);
- gpio_set_value(AMS_DELTA_GPIO_PIN_NAND_NWE, 1);
+ gpiod_set_value(gpiod_nwe, 1);
}
static u_char ams_delta_read_byte(struct mtd_info *mtd)
@@ -81,11 +86,11 @@ static u_char ams_delta_read_byte(struct mtd_info *mtd)
struct nand_chip *this = mtd_to_nand(mtd);
void __iomem *io_base = (void __iomem *)nand_get_controller_data(this);
- gpio_set_value(AMS_DELTA_GPIO_PIN_NAND_NRE, 0);
+ gpiod_set_value(gpiod_nre, 0);
ndelay(40);
writew(~0, io_base + OMAP_MPUIO_IO_CNTL);
res = readw(this->IO_ADDR_R);
- gpio_set_value(AMS_DELTA_GPIO_PIN_NAND_NRE, 1);
+ gpiod_set_value(gpiod_nre, 1);
return res;
}
@@ -120,12 +125,9 @@ static void ams_delta_hwcontrol(struct mtd_info *mtd, int cmd,
{
if (ctrl & NAND_CTRL_CHANGE) {
- gpio_set_value(AMS_DELTA_GPIO_PIN_NAND_NCE,
- (ctrl & NAND_NCE) == 0);
- gpio_set_value(AMS_DELTA_GPIO_PIN_NAND_CLE,
- (ctrl & NAND_CLE) != 0);
- gpio_set_value(AMS_DELTA_GPIO_PIN_NAND_ALE,
- (ctrl & NAND_ALE) != 0);
+ gpiod_set_value(gpiod_nce, !(ctrl & NAND_NCE));
+ gpiod_set_value(gpiod_cle, !!(ctrl & NAND_CLE));
+ gpiod_set_value(gpiod_ale, !!(ctrl & NAND_ALE));
}
if (cmd != NAND_CMD_NONE)
@@ -134,41 +136,9 @@ static void ams_delta_hwcontrol(struct mtd_info *mtd, int cmd,
static int ams_delta_nand_ready(struct mtd_info *mtd)
{
- return gpio_get_value(AMS_DELTA_GPIO_PIN_NAND_RB);
+ return gpiod_get_value(gpiod_rdy);
}
-static const struct gpio _mandatory_gpio[] = {
- {
- .gpio = AMS_DELTA_GPIO_PIN_NAND_NCE,
- .flags = GPIOF_OUT_INIT_HIGH,
- .label = "nand_nce",
- },
- {
- .gpio = AMS_DELTA_GPIO_PIN_NAND_NRE,
- .flags = GPIOF_OUT_INIT_HIGH,
- .label = "nand_nre",
- },
- {
- .gpio = AMS_DELTA_GPIO_PIN_NAND_NWP,
- .flags = GPIOF_OUT_INIT_HIGH,
- .label = "nand_nwp",
- },
- {
- .gpio = AMS_DELTA_GPIO_PIN_NAND_NWE,
- .flags = GPIOF_OUT_INIT_HIGH,
- .label = "nand_nwe",
- },
- {
- .gpio = AMS_DELTA_GPIO_PIN_NAND_ALE,
- .flags = GPIOF_OUT_INIT_LOW,
- .label = "nand_ale",
- },
- {
- .gpio = AMS_DELTA_GPIO_PIN_NAND_CLE,
- .flags = GPIOF_OUT_INIT_LOW,
- .label = "nand_cle",
- },
-};
/*
* Main initialization routine
@@ -216,12 +186,15 @@ static int ams_delta_init(struct platform_device *pdev)
this->write_buf = ams_delta_write_buf;
this->read_buf = ams_delta_read_buf;
this->cmd_ctrl = ams_delta_hwcontrol;
- if (gpio_request(AMS_DELTA_GPIO_PIN_NAND_RB, "nand_rdy") == 0) {
+
+ gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy", GPIOD_IN);
+ if (!IS_ERR_OR_NULL(gpiod_rdy)) {
this->dev_ready = ams_delta_nand_ready;
} else {
this->dev_ready = NULL;
pr_notice("Couldn't request gpio for Delta NAND ready.\n");
}
+
/* 25 us command delay time */
this->chip_delay = 30;
this->ecc.mode = NAND_ECC_SOFT;
@@ -230,7 +203,44 @@ static int ams_delta_init(struct platform_device *pdev)
platform_set_drvdata(pdev, io_base);
/* Set chip enabled, but */
- err = gpio_request_array(_mandatory_gpio, ARRAY_SIZE(_mandatory_gpio));
+ gpiod_nwp = devm_gpiod_get(&pdev->dev, "nwp", GPIOD_OUT_HIGH);
+ if (IS_ERR(gpiod_nwp)) {
+ err = PTR_ERR(gpiod_nwp);
+ dev_err(&pdev->dev, "NWP GPIO request failed (%d)\n", err);
+ goto err_gpiod;
+ }
+ gpiod_nce = devm_gpiod_get(&pdev->dev, "nce", GPIOD_OUT_HIGH);
+ if (IS_ERR(gpiod_nce)) {
+ err = PTR_ERR(gpiod_nce);
+ dev_err(&pdev->dev, "NCE GPIO request failed (%d)\n", err);
+ goto err_gpiod;
+ }
+ gpiod_nre = devm_gpiod_get(&pdev->dev, "nre", GPIOD_OUT_HIGH);
+ if (IS_ERR(gpiod_nre)) {
+ err = PTR_ERR(gpiod_nre);
+ dev_err(&pdev->dev, "NRE GPIO request failed (%d)\n", err);
+ goto err_gpiod;
+ }
+ gpiod_nwe = devm_gpiod_get(&pdev->dev, "nwe", GPIOD_OUT_HIGH);
+ if (IS_ERR(gpiod_nwe)) {
+ err = PTR_ERR(gpiod_nwe);
+ dev_err(&pdev->dev, "NWE GPIO request failed (%d)\n", err);
+ goto err_gpiod;
+ }
+ gpiod_ale = devm_gpiod_get(&pdev->dev, "ale", GPIOD_OUT_LOW);
+ if (IS_ERR(gpiod_ale)) {
+ err = PTR_ERR(gpiod_ale);
+ dev_err(&pdev->dev, "ALE GPIO request failed (%d)\n", err);
+ goto err_gpiod;
+ }
+ gpiod_cle = devm_gpiod_get(&pdev->dev, "cle", GPIOD_OUT_LOW);
+ if (IS_ERR(gpiod_cle)) {
+ err = PTR_ERR(gpiod_cle);
+ dev_err(&pdev->dev, "CLE GPIO request failed (%d)\n", err);
+ }
+err_gpiod:
+ if (err == -ENODEV || err == -ENOENT)
+ err = -EPROBE_DEFER;
if (err)
goto out_gpio;
@@ -246,9 +256,7 @@ static int ams_delta_init(struct platform_device *pdev)
goto out;
out_mtd:
- gpio_free_array(_mandatory_gpio, ARRAY_SIZE(_mandatory_gpio));
out_gpio:
- gpio_free(AMS_DELTA_GPIO_PIN_NAND_RB);
iounmap(io_base);
out_free:
kfree(this);
@@ -266,8 +274,6 @@ static int ams_delta_cleanup(struct platform_device *pdev)
/* Release resources, unregister device */
nand_release(ams_delta_mtd);
- gpio_free_array(_mandatory_gpio, ARRAY_SIZE(_mandatory_gpio));
- gpio_free(AMS_DELTA_GPIO_PIN_NAND_RB);
iounmap(io_base);
/* Free the MTD device structure */
--
2.16.1
^ permalink raw reply related
* [PATCH 6/6] ARM: OMAP1: ams-delta: make board header file local to mach-omap1
From: Janusz Krzysztofik @ 2018-05-18 21:09 UTC (permalink / raw)
To: Tony Lindgren, Dmitry Torokhov, Boris Brezillon, Tomi Valkeinen,
Mark Brown
Cc: Aaro Koskinen, Richard Weinberger, Peter Ujfalusi, Jarkko Nikula,
Liam Girdwood, linux-arm-kernel, linux-omap, linux-kernel,
linux-input, linux-mtd, linux-fbdev, alsa-devel,
Janusz Krzysztofik
In-Reply-To: <20180518210954.29044-1-jmkrzyszt@gmail.com>
Now as the AMS Delta board header file is no longer included by
drivers, move it to the root directory of mach-omap1.
Signed-off-by: Janusz Krzysztofik <jmkrzyszt@gmail.com>
---
arch/arm/mach-omap1/ams-delta-fiq-handler.S | 2 +-
arch/arm/mach-omap1/ams-delta-fiq.c | 3 +--
arch/arm/mach-omap1/board-ams-delta.c | 2 +-
arch/arm/mach-omap1/{include/mach => }/board-ams-delta.h | 0
4 files changed, 3 insertions(+), 4 deletions(-)
rename arch/arm/mach-omap1/{include/mach => }/board-ams-delta.h (100%)
diff --git a/arch/arm/mach-omap1/ams-delta-fiq-handler.S b/arch/arm/mach-omap1/ams-delta-fiq-handler.S
index bf608441b357..9005c00db948 100644
--- a/arch/arm/mach-omap1/ams-delta-fiq-handler.S
+++ b/arch/arm/mach-omap1/ams-delta-fiq-handler.S
@@ -16,9 +16,9 @@
#include <linux/linkage.h>
#include <asm/assembler.h>
-#include <mach/board-ams-delta.h>
#include <mach/ams-delta-fiq.h>
+#include "board-ams-delta.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 d7ca9e2b40d2..30aedcc3f2b3 100644
--- a/arch/arm/mach-omap1/ams-delta-fiq.c
+++ b/arch/arm/mach-omap1/ams-delta-fiq.c
@@ -19,11 +19,10 @@
#include <linux/module.h>
#include <linux/io.h>
-#include <mach/board-ams-delta.h>
-
#include <asm/fiq.h>
#include <mach/ams-delta-fiq.h>
+#include "board-ams-delta.h"
static struct fiq_handler fh = {
.name = "ams-delta-fiq"
diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c
index 80f54cb54276..17d69eb64df3 100644
--- a/arch/arm/mach-omap1/board-ams-delta.c
+++ b/arch/arm/mach-omap1/board-ams-delta.c
@@ -36,7 +36,6 @@
#include <asm/mach/arch.h>
#include <asm/mach/map.h>
-#include <mach/board-ams-delta.h>
#include <linux/platform_data/keypad-omap.h>
#include <mach/mux.h>
@@ -45,6 +44,7 @@
#include "camera.h"
#include <mach/usb.h>
+#include "board-ams-delta.h"
#include "iomap.h"
#include "common.h"
diff --git a/arch/arm/mach-omap1/include/mach/board-ams-delta.h b/arch/arm/mach-omap1/board-ams-delta.h
similarity index 100%
rename from arch/arm/mach-omap1/include/mach/board-ams-delta.h
rename to arch/arm/mach-omap1/board-ams-delta.h
--
2.16.1
^ permalink raw reply related
* Re: [PATCH 5/6] mtd: rawnand: ams-delta: use GPIO lookup table
From: Andy Shevchenko @ 2018-05-18 21:21 UTC (permalink / raw)
To: Janusz Krzysztofik
Cc: Tony Lindgren, Dmitry Torokhov, Boris Brezillon, Tomi Valkeinen,
Mark Brown, Aaro Koskinen, Richard Weinberger, Peter Ujfalusi,
Jarkko Nikula, Liam Girdwood, linux-arm Mailing List,
Linux OMAP Mailing List, Linux Kernel Mailing List, linux-input,
open list:MEMORY TECHNOLOGY..., linux-fbdev,
ALSA Development Mailing List
In-Reply-To: <20180518210954.29044-5-jmkrzyszt@gmail.com>
On Sat, May 19, 2018 at 12:09 AM, Janusz Krzysztofik
<jmkrzyszt@gmail.com> wrote:
> + gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy", GPIOD_IN);
> + if (!IS_ERR_OR_NULL(gpiod_rdy)) {
So, is it optional or not at the end?
If it is, why do we check for NULL?
> this->dev_ready = ams_delta_nand_ready;
> } else {
> this->dev_ready = NULL;
> pr_notice("Couldn't request gpio for Delta NAND ready.\n");
dev_notice() ?
> }
> +err_gpiod:
> + if (err == -ENODEV || err == -ENOENT)
> + err = -EPROBE_DEFER;
Hmm...
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH 5/6] mtd: rawnand: ams-delta: use GPIO lookup table
From: Janusz Krzysztofik @ 2018-05-18 23:15 UTC (permalink / raw)
To: Andy Shevchenko
Cc: linux-fbdev, ALSA Development Mailing List, Aaro Koskinen,
Tony Lindgren, Richard Weinberger, Mark Brown, Dmitry Torokhov,
Liam Girdwood, Linux Kernel Mailing List, Peter Ujfalusi,
Boris Brezillon, Tomi Valkeinen, open list:MEMORY TECHNOLOGY...,
linux-arm Mailing List, linux-input, Linux OMAP Mailing List,
Jarkko Nikula
In-Reply-To: <CAHp75VdNUW6KoM6oupyQ80A1WVRk7vewwDt6WEZOyjrAUifqRg@mail.gmail.com>
On Friday, May 18, 2018 11:21:14 PM CEST Andy Shevchenko wrote:
> On Sat, May 19, 2018 at 12:09 AM, Janusz Krzysztofik
>
> <jmkrzyszt@gmail.com> wrote:
> > + gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy", GPIOD_IN);
> > + if (!IS_ERR_OR_NULL(gpiod_rdy)) {
>
> So, is it optional or not at the end?
> If it is, why do we check for NULL?
As far as I can understand, nand_chip->dev_ready() callback is optional.
That's why I decided to use the _optional variant of devm_gpiod_get(). In case
of ams-delta, the dev_ready() callback depends on availability of the 'rdy'
GPIO pin. As a consequence, I'm checking for both NULL and ERR in order to
decide if dev_ready() will be supported.
I can pretty well replace it with the standard form and check for ERR only if
the purpose of the _optional form is different.
> > this->dev_ready = ams_delta_nand_ready;
> >
> > } else {
> >
> > this->dev_ready = NULL;
> > pr_notice("Couldn't request gpio for Delta NAND
> > ready.\n");
>
> dev_notice() ?
Sure, but maybe in a separate patch? That's not a new code just being added
but an existing one, not the merit of the change.
> > }
> >
> > +err_gpiod:
> > + if (err == -ENODEV || err == -ENOENT)
> > + err = -EPROBE_DEFER;
>
> Hmm...
Amstrad Delta uses gpio-mmio driver. Unfortunatelty that driver is not
availble before device init phase, unlike other crucial GPIO drivers which are
initialized earlier, e.g. during the postcore or at latetst the subsys phase.
Hence, devices which depend on GPIO pins provided by gpio-mmio must either be
declared late or fail softly so they get another chance of being probed
succesfully.
I thought of replacing the gpio-mmio platform driver with bgpio functions it
exports but for now I haven't implemented it, not even shared the idea.
Does it really hurt to return -EPROBE_DEFER if a GPIO pin can't be obtained?
Thanks,
Janusz
^ permalink raw reply
* Re: [BUG] i2c-hid: ELAN Touchpad does not work on ASUS X580GD
From: Hans de Goede @ 2018-05-19 11:58 UTC (permalink / raw)
To: Jarkko Nikula, Chris Chiu
Cc: Daniel Drake, Jian-Hong Pan, Jiri Kosina, Benjamin Tissoires,
Jani Nikula, Dmitry Torokhov, Adrian Salido, Jason Gerecke,
linux-input, Andy Shevchenko, Mika Westerberg, Wolfram Sang,
linux-i2c, Linux Kernel, Linux Upstreaming Team
In-Reply-To: <91f87c43-0ae1-00ec-6b3e-819b639aa1b2@linux.intel.com>
Hi,
On 18-05-18 15:15, Jarkko Nikula wrote:
> On 05/18/2018 10:48 AM, Hans de Goede wrote:
>> Could it be the i2c input clock definition in drivers/mfd/intel-lpss-pci.c
>> is also wrong for Apollo Lake (N3450) ? There are lots of people having
>> various issues with i2c attached touchpads on Apollo Lake devices, this bug:
>> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1728244
>>
>> Is sort of a collection bug for these. Various models laptops, lots of
>> reporters. Note not sure thie is an i2c-designware issue, but it would
>> be good to double check the input clock on Apollo Lake.
>>
> Does i2c_designware_core.dyndbg=+p and i2c-hid.debug=1 command line arguments give any useful debug information from those machines?
A first dmesg with these flags has been provided:
https://launchpadlibrarian.net/370884912/dmesg.log
Regards,
Hans
^ permalink raw reply
* Re: [PATCH 5/6] mtd: rawnand: ams-delta: use GPIO lookup table
From: Andy Shevchenko @ 2018-05-19 18:00 UTC (permalink / raw)
To: Janusz Krzysztofik
Cc: linux-fbdev, ALSA Development Mailing List, Aaro Koskinen,
Tony Lindgren, Richard Weinberger, Mark Brown, Dmitry Torokhov,
Liam Girdwood, Linux Kernel Mailing List, Peter Ujfalusi,
Boris Brezillon, Tomi Valkeinen, open list:MEMORY TECHNOLOGY...,
linux-arm Mailing List, linux-input, Linux OMAP Mailing List,
Jarkko Nikula
In-Reply-To: <3427199.r4OBoDP6Xz@z50>
On Sat, May 19, 2018 at 2:15 AM, Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote:
> On Friday, May 18, 2018 11:21:14 PM CEST Andy Shevchenko wrote:
>> On Sat, May 19, 2018 at 12:09 AM, Janusz Krzysztofik
>>
>> <jmkrzyszt@gmail.com> wrote:
>> > + gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy", GPIOD_IN);
>> > + if (!IS_ERR_OR_NULL(gpiod_rdy)) {
>>
>> So, is it optional or not at the end?
>> If it is, why do we check for NULL?
>
> As far as I can understand, nand_chip->dev_ready() callback is optional.
> That's why I decided to use the _optional variant of devm_gpiod_get(). In case
> of ams-delta, the dev_ready() callback depends on availability of the 'rdy'
> GPIO pin. As a consequence, I'm checking for both NULL and ERR in order to
> decide if dev_ready() will be supported.
>
> I can pretty well replace it with the standard form and check for ERR only if
> the purpose of the _optional form is different.
NULL check in practice discards the _optional part of gpiod_get(). So,
either you use non-optional variant and decide how to handle an
errors, or user _optional w/o NULL check.
>> > +err_gpiod:
>> > + if (err == -ENODEV || err == -ENOENT)
>> > + err = -EPROBE_DEFER;
>>
>> Hmm...
>
> Amstrad Delta uses gpio-mmio driver. Unfortunatelty that driver is not
> availble before device init phase, unlike other crucial GPIO drivers which are
> initialized earlier, e.g. during the postcore or at latetst the subsys phase.
> Hence, devices which depend on GPIO pins provided by gpio-mmio must either be
> declared late or fail softly so they get another chance of being probed
> succesfully.
>
> I thought of replacing the gpio-mmio platform driver with bgpio functions it
> exports but for now I haven't implemented it, not even shared the idea.
>
> Does it really hurt to return -EPROBE_DEFER if a GPIO pin can't be obtained?
I'm only concerned if it would be an infinite defer in the case when
driver will never appear.
But I don't remember the details.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH 5/6] mtd: rawnand: ams-delta: use GPIO lookup table
From: Janusz Krzysztofik @ 2018-05-19 21:55 UTC (permalink / raw)
To: Andy Shevchenko
Cc: linux-fbdev, ALSA Development Mailing List, Aaro Koskinen,
Tony Lindgren, Richard Weinberger, Mark Brown, Dmitry Torokhov,
Liam Girdwood, Linux Kernel Mailing List, Peter Ujfalusi,
Boris Brezillon, Tomi Valkeinen, open list:MEMORY TECHNOLOGY...,
linux-arm Mailing List, linux-input, Linux OMAP Mailing List,
Jarkko Nikula
In-Reply-To: <CAHp75VcUxjubx=zOGyCjmpjCk5RBLPCaHcaOieDie9LdTCa+5g@mail.gmail.com>
On Saturday, May 19, 2018 8:00:38 PM CEST Andy Shevchenko wrote:
> On Sat, May 19, 2018 at 2:15 AM, Janusz Krzysztofik <jmkrzyszt@gmail.com>
wrote:
> > On Friday, May 18, 2018 11:21:14 PM CEST Andy Shevchenko wrote:
> >> On Sat, May 19, 2018 at 12:09 AM, Janusz Krzysztofik
> >>
> >> <jmkrzyszt@gmail.com> wrote:
> >> > + gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy",
> >> > GPIOD_IN);
> >> > + if (!IS_ERR_OR_NULL(gpiod_rdy)) {
> >>
> >> So, is it optional or not at the end?
> >> If it is, why do we check for NULL?
> >
> > As far as I can understand, nand_chip->dev_ready() callback is optional.
> > That's why I decided to use the _optional variant of devm_gpiod_get(). In
> > case of ams-delta, the dev_ready() callback depends on availability of
> > the 'rdy' GPIO pin. As a consequence, I'm checking for both NULL and ERR
> > in order to decide if dev_ready() will be supported.
> >
> > I can pretty well replace it with the standard form and check for ERR only
> > if the purpose of the _optional form is different.
>
> NULL check in practice discards the _optional part of gpiod_get(). So,
> either you use non-optional variant and decide how to handle an
> errors, or user _optional w/o NULL check.
OK, I'm going to use something like the below while submitting v2:
- gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy", GPIOD_IN);
- if (!IS_ERR_OR_NULL(gpiod_rdy)) {
- this->dev_ready = ams_delta_nand_ready;
- } else {
- this->dev_ready = NULL;
- pr_notice("Couldn't request gpio for Delta NAND ready.\n");
+ priv->gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy",
+ GPIOD_IN);
+ if (IS_ERR(priv->gpiod_rdy)) {
+ err = PTR_ERR(priv->gpiod_nwp);
+ dev_warn(&pdev->dev, "RDY GPIO request failed (%d)\n", err);
+ goto err_gpiod;
}
+ if (priv->gpiod_rdy)
+ this->dev_ready = ams_delta_nand_ready;
>
> >> > +err_gpiod:
> >> > + if (err == -ENODEV || err == -ENOENT)
> >> > + err = -EPROBE_DEFER;
> >>
> >> Hmm...
> >
> > Amstrad Delta uses gpio-mmio driver. Unfortunatelty that driver is not
> > availble before device init phase, unlike other crucial GPIO drivers which
> > are initialized earlier, e.g. during the postcore or at latetst the
> > subsys phase. Hence, devices which depend on GPIO pins provided by
> > gpio-mmio must either be declared late or fail softly so they get another
> > chance of being probed succesfully.
> >
> > I thought of replacing the gpio-mmio platform driver with bgpio functions
> > it exports but for now I haven't implemented it, not even shared the
> > idea.
> >
> > Does it really hurt to return -EPROBE_DEFER if a GPIO pin can't be
> > obtained?
> I'm only concerned if it would be an infinite defer in the case when
> driver will never appear.
> But I don't remember the details.
Deferred probes are handled effectively during late_initcall, no risk of
infinite defer, see drivers/base/dd.c for details.
Thanks,
Janusz
^ permalink raw reply
* Re: [PATCH v2 3/3] ARM: dts: imx28/imx53: enable edt-ft5x06 wakeup source
From: Shawn Guo @ 2018-05-20 13:05 UTC (permalink / raw)
To: Daniel Mack
Cc: mark.rutland, devicetree, dmitry.torokhov, robh+dt, kernel,
linux-input, fabio.estevam, linux-arm-kernel
In-Reply-To: <20180517090552.5704-4-daniel@zonque.org>
On Thu, May 17, 2018 at 11:05:52AM +0200, Daniel Mack wrote:
> The touchscreen driver no longer configures the device as wakeup source by
> default. A "wakeup-source" property is needed.
>
> To avoid regressions, this patch changes the DTS files for the only two
> users of this driver that didn't have this property yet.
>
> Signed-off-by: Daniel Mack <daniel@zonque.org>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> Cc: Rob Herring <robh+dt@kernel.org>
Applied this one, thanks.
^ permalink raw reply
* Re: [PATCH 5/6] mtd: rawnand: ams-delta: use GPIO lookup table
From: Andy Shevchenko @ 2018-05-20 14:44 UTC (permalink / raw)
To: Janusz Krzysztofik
Cc: linux-fbdev, ALSA Development Mailing List, Aaro Koskinen,
Tony Lindgren, Richard Weinberger, Mark Brown, Dmitry Torokhov,
Liam Girdwood, Linux Kernel Mailing List, Peter Ujfalusi,
Boris Brezillon, Tomi Valkeinen, open list:MEMORY TECHNOLOGY...,
linux-arm Mailing List, linux-input, Linux OMAP Mailing List,
Jarkko Nikula
In-Reply-To: <5456625.lDWjtgZygK@z50>
On Sun, May 20, 2018 at 12:55 AM, Janusz Krzysztofik
<jmkrzyszt@gmail.com> wrote:
> On Saturday, May 19, 2018 8:00:38 PM CEST Andy Shevchenko wrote:
>> On Sat, May 19, 2018 at 2:15 AM, Janusz Krzysztofik <jmkrzyszt@gmail.com>
> wrote:
>> NULL check in practice discards the _optional part of gpiod_get(). So,
>> either you use non-optional variant and decide how to handle an
>> errors, or user _optional w/o NULL check.
>
> OK, I'm going to use something like the below while submitting v2:
>
> - gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy", GPIOD_IN);
> - if (!IS_ERR_OR_NULL(gpiod_rdy)) {
> - this->dev_ready = ams_delta_nand_ready;
> - } else {
> - this->dev_ready = NULL;
> - pr_notice("Couldn't request gpio for Delta NAND ready.\n");
> + priv->gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy",
> + GPIOD_IN);
> + if (IS_ERR(priv->gpiod_rdy)) {
> + err = PTR_ERR(priv->gpiod_nwp);
> + dev_warn(&pdev->dev, "RDY GPIO request failed (%d)\n", err);
> + goto err_gpiod;
> }
>
> + if (priv->gpiod_rdy)
> + this->dev_ready = ams_delta_nand_ready;
This makes sense.
Though, I completely dislike "rdy" name of GPIO. Where is it documented?
>> >> > +err_gpiod:
>> >> > + if (err == -ENODEV || err == -ENOENT)
>> >> > + err = -EPROBE_DEFER;
>> >>
>> >> Hmm...
>> >
>> > Amstrad Delta uses gpio-mmio driver. Unfortunatelty that driver is not
>> > availble before device init phase, unlike other crucial GPIO drivers which
>> > are initialized earlier, e.g. during the postcore or at latetst the
>> > subsys phase. Hence, devices which depend on GPIO pins provided by
>> > gpio-mmio must either be declared late or fail softly so they get another
>> > chance of being probed succesfully.
>> >
>> > I thought of replacing the gpio-mmio platform driver with bgpio functions
>> > it exports but for now I haven't implemented it, not even shared the
>> > idea.
>> >
>> > Does it really hurt to return -EPROBE_DEFER if a GPIO pin can't be
>> > obtained?
>> I'm only concerned if it would be an infinite defer in the case when
>> driver will never appear.
>> But I don't remember the details.
>
> Deferred probes are handled effectively during late_initcall, no risk of
> infinite defer, see drivers/base/dd.c for details.
Yes, but the code you provided in patch looks somehow suspicious. OK,
I let Linus decide whtat to do with that.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH 5/6] mtd: rawnand: ams-delta: use GPIO lookup table
From: Janusz Krzysztofik @ 2018-05-20 15:37 UTC (permalink / raw)
To: Andy Shevchenko
Cc: linux-fbdev, ALSA Development Mailing List, Aaro Koskinen,
Tony Lindgren, Richard Weinberger, Mark Brown, Dmitry Torokhov,
Liam Girdwood, Linux Kernel Mailing List, Peter Ujfalusi,
Boris Brezillon, Tomi Valkeinen, open list:MEMORY TECHNOLOGY...,
linux-arm Mailing List, linux-input, Linux OMAP Mailing List,
Jarkko Nikula
In-Reply-To: <CAHp75Ved2nw1JKnXT2Jxnw4XLsby1eVJU6cfBjE2sp4LP4Kwmg@mail.gmail.com>
On Sunday, May 20, 2018 4:44:31 PM CEST Andy Shevchenko wrote:
> On Sun, May 20, 2018 at 12:55 AM, Janusz Krzysztofik
>
> <jmkrzyszt@gmail.com> wrote:
> > On Saturday, May 19, 2018 8:00:38 PM CEST Andy Shevchenko wrote:
> >> On Sat, May 19, 2018 at 2:15 AM, Janusz Krzysztofik <jmkrzyszt@gmail.com>
> >
> > wrote:
> >> NULL check in practice discards the _optional part of gpiod_get(). So,
> >> either you use non-optional variant and decide how to handle an
> >> errors, or user _optional w/o NULL check.
> >
> > OK, I'm going to use something like the below while submitting v2:
> >
> > - gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy", GPIOD_IN);
> > - if (!IS_ERR_OR_NULL(gpiod_rdy)) {
> > - this->dev_ready = ams_delta_nand_ready;
> > - } else {
> > - this->dev_ready = NULL;
> > - pr_notice("Couldn't request gpio for Delta NAND
> > ready.\n");
> > + priv->gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy",
> > + GPIOD_IN);
> > + if (IS_ERR(priv->gpiod_rdy)) {
> > + err = PTR_ERR(priv->gpiod_nwp);
> > + dev_warn(&pdev->dev, "RDY GPIO request failed (%d)\n",
> > err); + goto err_gpiod;
> >
> > }
> >
> > + if (priv->gpiod_rdy)
> > + this->dev_ready = ams_delta_nand_ready;
>
> This makes sense.
>
> Though, I completely dislike "rdy" name of GPIO. Where is it documented?
No documentation files for Amstrad Delta nor for its NAND driver specifically
exist under Documentation/. However, there exist some for generic GPIO NAND
driver where the pin name "rdy" is used explicitly:
Documentation/driver-api/gpio/drivers-on-gpio.rst
Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
You can find that mnemonic used across drivers/mtd/nand/, standalone or as a
suffix, including the Amstrad Delta NAND driver before the change discussed.
To be honest, I don't like it much either, but I'm just using it instead of
inventing something new.
Thanks,
Janusz
^ permalink raw reply
* Re: [PATCH 5/6] mtd: rawnand: ams-delta: use GPIO lookup table
From: Andy Shevchenko @ 2018-05-20 16:17 UTC (permalink / raw)
To: Janusz Krzysztofik
Cc: linux-fbdev, ALSA Development Mailing List, Aaro Koskinen,
Tony Lindgren, Richard Weinberger, Mark Brown, Dmitry Torokhov,
Liam Girdwood, Linux Kernel Mailing List, Peter Ujfalusi,
Boris Brezillon, Tomi Valkeinen, open list:MEMORY TECHNOLOGY...,
linux-arm Mailing List, linux-input, Linux OMAP Mailing List,
Jarkko Nikula
In-Reply-To: <3505791.x6nWYTo8eb@z50>
On Sun, May 20, 2018 at 6:37 PM, Janusz Krzysztofik <jmkrzyszt@gmail.com> wrote:
> On Sunday, May 20, 2018 4:44:31 PM CEST Andy Shevchenko wrote:
>> Though, I completely dislike "rdy" name of GPIO. Where is it documented?
>
> No documentation files for Amstrad Delta nor for its NAND driver specifically
> exist under Documentation/. However, there exist some for generic GPIO NAND
> driver where the pin name "rdy" is used explicitly:
> Documentation/driver-api/gpio/drivers-on-gpio.rst
> Documentation/devicetree/bindings/mtd/gpio-control-nand.txt
> You can find that mnemonic used across drivers/mtd/nand/, standalone or as a
> suffix, including the Amstrad Delta NAND driver before the change discussed.
> To be honest, I don't like it much either, but I'm just using it instead of
> inventing something new.
OK, that's what I was looking for. Since it's already in use and
documented, then it's fine for me.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [alsa-devel] [PATCH 5/6] mtd: rawnand: ams-delta: use GPIO lookup table
From: Ladislav Michl @ 2018-05-20 19:27 UTC (permalink / raw)
To: Janusz Krzysztofik
Cc: Andy Shevchenko, linux-fbdev, ALSA Development Mailing List,
Aaro Koskinen, Tony Lindgren, Richard Weinberger, Mark Brown,
Dmitry Torokhov, Liam Girdwood, Linux Kernel Mailing List,
Peter Ujfalusi, Boris Brezillon, Tomi Valkeinen,
open list:MEMORY TECHNOLOGY..., linux-arm Mailing List,
linux-input, Linux OMAP Mailing List
In-Reply-To: <5456625.lDWjtgZygK@z50>
On Sat, May 19, 2018 at 11:55:51PM +0200, Janusz Krzysztofik wrote:
> On Saturday, May 19, 2018 8:00:38 PM CEST Andy Shevchenko wrote:
> > On Sat, May 19, 2018 at 2:15 AM, Janusz Krzysztofik <jmkrzyszt@gmail.com>
> wrote:
> > > On Friday, May 18, 2018 11:21:14 PM CEST Andy Shevchenko wrote:
> > >> On Sat, May 19, 2018 at 12:09 AM, Janusz Krzysztofik
> > >>
> > >> <jmkrzyszt@gmail.com> wrote:
> > >> > + gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy",
> > >> > GPIOD_IN);
> > >> > + if (!IS_ERR_OR_NULL(gpiod_rdy)) {
> > >>
> > >> So, is it optional or not at the end?
> > >> If it is, why do we check for NULL?
> > >
> > > As far as I can understand, nand_chip->dev_ready() callback is optional.
> > > That's why I decided to use the _optional variant of devm_gpiod_get(). In
> > > case of ams-delta, the dev_ready() callback depends on availability of
> > > the 'rdy' GPIO pin. As a consequence, I'm checking for both NULL and ERR
> > > in order to decide if dev_ready() will be supported.
> > >
> > > I can pretty well replace it with the standard form and check for ERR only
> > > if the purpose of the _optional form is different.
> >
> > NULL check in practice discards the _optional part of gpiod_get(). So,
> > either you use non-optional variant and decide how to handle an
> > errors, or user _optional w/o NULL check.
>
> OK, I'm going to use something like the below while submitting v2:
>
> - gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy", GPIOD_IN);
> - if (!IS_ERR_OR_NULL(gpiod_rdy)) {
> - this->dev_ready = ams_delta_nand_ready;
> - } else {
> - this->dev_ready = NULL;
> - pr_notice("Couldn't request gpio for Delta NAND ready.\n");
> + priv->gpiod_rdy = devm_gpiod_get_optional(&pdev->dev, "rdy",
> + GPIOD_IN);
> + if (IS_ERR(priv->gpiod_rdy)) {
> + err = PTR_ERR(priv->gpiod_nwp);
??? --------------------------------^^^^^^^^^
> + dev_warn(&pdev->dev, "RDY GPIO request failed (%d)\n", err);
> + goto err_gpiod;
Driver will just use worst case delay instead of RDY signal, so this
is perhaps too strict. I will work with degraded performance.
ladis
> }
>
> + if (priv->gpiod_rdy)
> + this->dev_ready = ams_delta_nand_ready;
>
> >
> > >> > +err_gpiod:
> > >> > + if (err == -ENODEV || err == -ENOENT)
> > >> > + err = -EPROBE_DEFER;
> > >>
> > >> Hmm...
> > >
> > > Amstrad Delta uses gpio-mmio driver. Unfortunatelty that driver is not
> > > availble before device init phase, unlike other crucial GPIO drivers which
> > > are initialized earlier, e.g. during the postcore or at latetst the
> > > subsys phase. Hence, devices which depend on GPIO pins provided by
> > > gpio-mmio must either be declared late or fail softly so they get another
> > > chance of being probed succesfully.
> > >
> > > I thought of replacing the gpio-mmio platform driver with bgpio functions
> > > it exports but for now I haven't implemented it, not even shared the
> > > idea.
> > >
> > > Does it really hurt to return -EPROBE_DEFER if a GPIO pin can't be
> > > obtained?
> > I'm only concerned if it would be an infinite defer in the case when
> > driver will never appear.
> > But I don't remember the details.
>
> Deferred probes are handled effectively during late_initcall, no risk of
> infinite defer, see drivers/base/dd.c for details.
>
> Thanks,
> Janusz
>
>
>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
^ 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