All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Warren <swarren@wwwdotorg.org>
To: Nick Dyer <nick.dyer@itdev.co.uk>
Cc: Yufeng Shen <miletus@chromium.org>,
	Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	benson Leung <bleung@chromium.org>,
	Daniel Kurtz <djkurtz@chromium.org>,
	Henrik Rydberg <rydberg@euromail.se>,
	Joonyoung Shim <jy0922.shim@samsung.com>,
	Alan Bowens <Alan.Bowens@atmel.com>,
	linux-input <linux-input@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Peter Meerwald <pmeerw@pmeerw.net>,
	Olof Johansson <olofj@chromium.org>, Sekhar Nori <nsekhar@ti.com>
Subject: Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc
Date: Tue, 29 Jul 2014 13:26:25 -0600	[thread overview]
Message-ID: <53D7F561.1000603@wwwdotorg.org> (raw)
In-Reply-To: <53D7D47E.5060906@itdev.co.uk>

On 07/29/2014 11:06 AM, Nick Dyer wrote:
> On 29/07/14 17:16, Stephen Warren wrote:
>> I then tried updating the firmware. This didn't work at all.
>>
>> First I tried via mxt-app:
>>
>>> root@localhost:~# ./obp-utils/mxt-app -d i2c-dev:1-004b --flash
>>> 130.1_1.0.170.bin
>>> Version:1.16-65-g0a4c
>>> Opening firmware file 130.1_1.0.170.bin
>>> Registered i2c-dev adapter:1 address:0x4b
>>> Chip detected
>>> Current firmware version: 1.0.AA
>>> Skipping version check
>>> Resetting in bootloader mode
>>> Registered i2c-dev adapter:1 address:0x25
>>> Error Remote I/O error (121) reading from i2c
>>> Bootloader read failure
>>> Bootloader not found
>>
>> Then I power-cycled and tried via the atmel_mxt_ts modules' sysfs files:
>>
>>> root@localhost:~# echo 1 >
>>> /sys/devices/soc0/7000c400.i2c/i2c-1/1-004b/update_fw
>>> [   38.495420] atmel_mxt_ts 1-004b: mxt_bootloader_read: i2c recv failed
>>> (-121)
>>> [   38.506208] atmel_mxt_ts 1-004b: mxt_bootloader_read: i2c recv failed
>>> (-121)
>>> [   38.513836] atmel_mxt_ts 1-004b: The firmware update failed(-121)
>>> -bash: echo: write error: Remote I/O error
>
> OK - that's the same error in both cases, it has tried to switch the device
> into bootloader mode, however it is not responding on the bootloader I2C
> address.
>
> Couple of things to check:
> - is the device in deep sleep mode when you run the mxt-app command? can
> you try doing "mxt-app [device] -W -T7 FFFF" which will make sure it is
> definitely not sleeping first.

That didn't hekp.

> - if you do "mxt-app [device] -i" straight after the --flash failure, does
> it respond (which would mean it hasn't actioned the reset-into-bootloader
> command)

That still works, so I assume it wasn't reset into bootloader mode. I 
also tried mxt-app --reset-bootloader, and mxt-app -i still works after 
that too.

>> I also found that removing the module (even without attempting a FW update)
>> yields:
>>
>> After attempted FW update via sysfs:
>>
>>> root@localhost:~# rmmod ./atmel_mxt_ts.ko
>>> [   81.995672] Unable to handle kernel NULL pointer dereference at virtual address 00000364
>
> Not good. It's trying to free an input device which isn't registered at
> that point. In a later patch in my series I add a guard around that
> (mxt_free_input_device):
> https://github.com/ndyer/linux/commit/bb4800ff8c185

It looks like 8d01a84b86b0 "Input: atmel_mxt_ts - implement T100 touch 
object support" from that branch is what solves the issue.

Ah, in linux-next, it's simply a double-unregister; both mxt_remove() 
and mxt_free_object_table() call 
input_unregister_device(data->input_dev). I think it would make sense to 
send the following patch for 3.17. Do you agree?

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c 
b/drivers/input/touchscreen/atmel_mxt_ts.c
index 03b85711cb70..da497dbf9735 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -1353,8 +1353,10 @@ static int mxt_get_info(struct mxt_data *data)

  static void mxt_free_object_table(struct mxt_data *data)
  {
-	input_unregister_device(data->input_dev);
-	data->input_dev = NULL;
+	if (data->input_dev) {
+		input_unregister_device(data->input_dev);
+		data->input_dev = NULL;
+	}

  	kfree(data->object_table);
  	data->object_table = NULL;
@@ -2194,7 +2196,6 @@ static int mxt_remove(struct i2c_client *client)

  	sysfs_remove_group(&client->dev.kobj, &mxt_attr_group);
  	free_irq(data->irq, data);
-	input_unregister_device(data->input_dev);
  	mxt_free_object_table(data);
  	kfree(data);


  reply	other threads:[~2014-07-29 19:26 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-03 15:01 nick.dyer
2014-07-03 15:01 ` [PATCH 01/15] Input: atmel_mxt_ts - initialise IRQ before probing nick.dyer
2014-07-03 15:01 ` [PATCH 02/15] Input: atmel_mxt_ts - move input device init into separate function nick.dyer
2014-07-03 15:01 ` [PATCH 03/15] Input: atmel_mxt_ts - set pointer emulation on touchpads nick.dyer
2014-07-03 15:01 ` [PATCH 04/15] Input: atmel_mxt_ts - implement device tree support nick.dyer
2014-07-22 20:37   ` Stephen Warren
2014-07-23 15:13     ` Nick Dyer
2014-07-23 21:36   ` Stephen Warren
2014-07-24 15:10     ` Nick Dyer
2014-07-24 16:04       ` Stephen Warren
2014-07-03 15:01 ` [PATCH 05/15] Input: atmel_mxt_ts - download device config using firmware loader nick.dyer
2014-07-03 15:01 ` [PATCH 06/15] Input: atmel_mxt_ts - calculate and check CRC in config file nick.dyer
2014-07-03 15:01 ` [PATCH 07/15] Input: atmel_mxt_ts - use deep sleep mode when stopped nick.dyer
2014-07-03 15:01 ` [PATCH 08/15] Input: atmel_mxt_ts - handle APP_CRC_FAIL on startup nick.dyer
2014-07-03 15:01 ` [PATCH 09/15] Input: atmel_mxt_ts - handle bootloader previously unlocked nick.dyer
2014-07-03 15:01 ` [PATCH 10/15] Input: atmel_mxt_ts - add bootloader addresses for new chips nick.dyer
2014-07-03 15:01 ` [PATCH 11/15] Input: atmel_mxt_ts - recover from bootloader on probe nick.dyer
2014-07-03 15:01 ` [PATCH 12/15] Input: atmel_mxt_ts - add support for dynamic message size nick.dyer
2014-07-03 15:01 ` [PATCH 13/15] Input: atmel_mxt_ts - decode T6 status messages nick.dyer
2014-07-03 15:01 ` [PATCH 14/15] Input: atmel_mxt_ts - split message handler into separate functions nick.dyer
2014-07-03 15:01 ` [PATCH 15/15] Input: atmel_mxt_ts - implement T44 message handling nick.dyer
2014-07-07 11:21 ` [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc Sekhar Nori
2014-07-07 11:21   ` Sekhar Nori
2014-07-07 11:38   ` Nick Dyer
2014-07-08 12:28     ` Sekhar Nori
2014-07-08 12:28       ` Sekhar Nori
2014-07-22 20:34 ` Stephen Warren
2014-07-23 15:30   ` Nick Dyer
2014-07-23 17:22     ` Stephen Warren
2014-07-23 20:29       ` Dmitry Torokhov
2014-07-23 21:39         ` Stephen Warren
2014-07-24 13:47       ` Nick Dyer
2014-07-24 21:19         ` Stephen Warren
2014-07-25 14:10           ` Nick Dyer
2014-07-25 20:06             ` Stephen Warren
2014-07-25 20:06               ` Stephen Warren
2014-07-28 17:28               ` Dmitry Torokhov
2014-07-28 20:20               ` Yufeng Shen
2014-07-28 21:23                 ` Stephen Warren
2014-07-28 23:42                   ` Stephen Warren
2014-07-29  0:10                     ` Yufeng Shen
2014-07-29 16:16                       ` Stephen Warren
2014-07-29 17:06                         ` Nick Dyer
2014-07-29 19:26                           ` Stephen Warren [this message]
2014-09-02 15:45                             ` Stephen Warren
2014-07-29 16:43                       ` Nick Dyer
2014-07-29 16:26                     ` Nick Dyer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=53D7F561.1000603@wwwdotorg.org \
    --to=swarren@wwwdotorg.org \
    --cc=Alan.Bowens@atmel.com \
    --cc=bleung@chromium.org \
    --cc=djkurtz@chromium.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=jy0922.shim@samsung.com \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=miletus@chromium.org \
    --cc=nick.dyer@itdev.co.uk \
    --cc=nsekhar@ti.com \
    --cc=olofj@chromium.org \
    --cc=pmeerw@pmeerw.net \
    --cc=rydberg@euromail.se \
    --subject='Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.