From: chris.ruehl@gtsys.com.hk (Chris Ruehl)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] pinctrl: imx1-core: Fix debug output pin array index
Date: Thu, 09 Jul 2015 16:28:08 +0800 [thread overview]
Message-ID: <559E3098.9070609@gtsys.com.hk> (raw)
In-Reply-To: <20150709071252.GF1426@pengutronix.de>
Uwe,
On Thursday, July 09, 2015 03:12 PM, Uwe Kleine-K?nig wrote:
> Hello Markus,
>
> Cc += Chris Ruehl
>
> On Wed, Jul 08, 2015 at 04:16:06PM +0200, Markus Pargmann wrote:
>> The pins do not have a 1:1 mapping from index to pin_id. Unfortunately
>> the debug output assumes exactly that.
>>
>> The first driver using imx1-core was imx27, which had exactly this 1:1
>> mapping. It was accidently removed when removing all unused pads which
>> were listed:
>> 607af165c047 (pinctrl: i.MX27: Remove nonexistent pad definitions)
>>
>> The patch fixes this issue by printing the pin_id directly and not the
>> pin name.
> Knowing a bit about the imx pinctrl drivers I failed to understand what
> you wrote here. Probably because I first though that "1:1 mapping" is a
> hardware property. What about:
>
> Subject: pinctrl: imx1-core: Fix debug output in .pin_config_set callback
>
> imx1_pinconf_set assumes that the array of pins in struct
> imx1_pinctrl_soc_info can be indexed by pin id to get the
> pinctrl_pin_desc for a pin. This used to be correct up to commit
> 607af165c047 ("pinctrl: i.MX27: Remove nonexistent pad definitions")
> which removed some entries from the array and so made it wrong to access
> the array by pin id.
>
> Implement the easiest fix by not resolving the pin id to a name but
> printing the id instead.
>
> Fixes: 607af165c047 ("pinctrl: i.MX27: Remove nonexistent pad definitions")
> Cc: stable at vger.kernel.org
> Reported-by: Chris Ruehl <chris.ruehl@gtsys.com.hk>
> Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
>
> Having the pad name in the output is nice, is it worth to search for the
> right pinctrl_pin_desc in the array? The array is still sorted, so a
> binary search would do, maybe a function for this already exists?
>
> How did Chris notice the error? Just a bogus output, or did it crash the
> kernel? That would be worth to note in the commit log, too.
>
> Otherwise the change looks fine.
>
> Best regards
> Uwe
I had a crash on a array overrun.
I'd applied a patch to the linux-usb (which was spit out by the maintainer perl
script)
chris at wheezyvm:~/kernel.d/linux-next$ cat
0001-Pinctrl-imx1-fix-wrong-pin-name-resolving.patch
From 50d56e5f626b2ea86211818cf487514c96f60487 Mon Sep 17 00:00:00 2001
From: Chris Ruehl <chris.ruehl@gtsys.com.hk>
Date: Sat, 23 May 2015 15:02:44 +0800
Subject: [PATCH] Pinctrl: imx1: fix wrong pin-name resolving
Bug in function imx1_pinconf_set() cause crash when
princtrl debug is enabled and the pin_id becomes larger
then the info->pins[] contains.
imx27-pinctrl 10015000.iomuxc: request pin 134 (MX27_PAD_UART2_TXD) for
1000b000.serial
imx27-pinctrl 10015000.iomuxc: request pin 135 (MX27_PAD_UART2_RXD) for
1000b000.serial
imx27-pinctrl 10015000.iomuxc: request pin 131 (MX27_PAD_UART2_CTS) for
1000b000.serial
imx27-pinctrl 10015000.iomuxc: request pin 132 (MX27_PAD_UART2_RTS) for
1000b000.serial
imx27-pinctrl 10015000.iomuxc: enable function uart group uart2-1
imx27-pinctrl 10015000.iomuxc: imx1_pmx_set, pin 0x86, function 0, gpio 0,
direction 1, oconf 0, iconfa 0, iconfb 0
imx27-pinctrl 10015000.iomuxc: imx1_pmx_set, pin 0x87, function 0, gpio 0,
direction 0, oconf 0, iconfa 0, iconfb 0
imx27-pinctrl 10015000.iomuxc: imx1_pmx_set, pin 0x83, function 0, gpio 0,
direction 1, oconf 0, iconfa 0, iconfb 0
imx27-pinctrl 10015000.iomuxc: imx1_pmx_set, pin 0x84, function 0, gpio 0,
direction 0, oconf 0, iconfa 0, iconfb 0
imx27-pinctrl 10015000.iomuxc: num_configs=1 PinID=134
imx27-pinctrl 10015000.iomuxc: pinconf set pullup pin MX27_PAD_UART1_RTS
imx27-pinctrl 10015000.iomuxc: num_configs=1 PinID=135
imx27-pinctrl 10015000.iomuxc: pinconf set pullup pin MX27_PAD_RTCK
imx27-pinctrl 10015000.iomuxc: num_configs=1 PinID=131
imx27-pinctrl 10015000.iomuxc: pinconf set pullup pin MX27_PAD_UART1_TXD
imx27-pinctrl 10015000.iomuxc: num_configs=1 PinID=132
imx27-pinctrl 10015000.iomuxc: pinconf set pullup pin MX27_PAD_UART1_RXD
...
imx27-pinctrl 10015000.iomuxc: write: register 0xf4415508 offset 4 value 0x3
imx27-pinctrl 10015000.iomuxc: write: register 0xf4415510 offset 4 value 0x0
imx27-pinctrl 10015000.iomuxc: write: register 0xf4415518 offset 4 value 0x0
imx27-pinctrl 10015000.iomuxc: imx1_pmx_set, pin 0xb5, function 0, gpio 0,
direction 1, oconf 0, iconfa 0, iconfb 0
imx27-pinctrl 10015000.iomuxc: num_configs=1 PinID=171
Unable to handle kernel paging request at virtual address 6c61765f
pgd = c0004000
6c61765f] *pgd=00000000
Internal error: Oops: 5 [#1] ARM
Modules linked in:
CPU: 0 PID: 1 Comm: swapper Not tainted 4.1.0-rc4-next-20150522-dirty #8
Hardware name: GTSYS i.MX27GTSIR (Device Tree Support)
task: ce832000 ti: ce848000 task.ti: ce848000
PC is at strnlen+0x28/0x3c
LR is at string.isra.4+0x34/0xcc
pc : [<c01ae188>] lr : [<c01af9a4>] psr: 20000093
sp : ce849a88 ip : ce849a98 fp : ce849a94
r10: c05d7e3a r9 : c05d81e4 r8 : 00000000
r7 : 6c61765f r6 : c05d81e4 r5 : ffffffff r4 : c05d7e3a
r3 : 6c61765f r2 : 6c61765f r1 : 6c61765e r0 : 6c61765f
Flags: nzCv IRQs off FIQs on Mode SVC_32 ISA ARM Segment kernel
Signed-off-by: Chris Ruehl <chris.ruehl@gtsys.com.hk>
---
drivers/pinctrl/freescale/pinctrl-imx1-core.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/pinctrl/freescale/pinctrl-imx1-core.c
b/drivers/pinctrl/freescale/pinctrl-imx1-core.c
index 5ac59fb..8408bd8 100644
--- a/drivers/pinctrl/freescale/pinctrl-imx1-core.c
+++ b/drivers/pinctrl/freescale/pinctrl-imx1-core.c
@@ -403,14 +403,16 @@ static int imx1_pinconf_set(struct pinctrl_dev *pctldev,
unsigned num_configs)
{
struct imx1_pinctrl *ipctl = pinctrl_dev_get_drvdata(pctldev);
- const struct imx1_pinctrl_soc_info *info = ipctl->info;
+ struct pin_desc *desc;
int i;
+ desc = pin_desc_get(pctldev, pin_id);
+
for (i = 0; i != num_configs; ++i) {
imx1_write_bit(ipctl, pin_id, configs[i] & 0x01, MX1_PUEN);
dev_dbg(ipctl->dev, "pinconf set pullup pin %s\n",
- info->pins[pin_id].name);
+ desc->name);
}
return 0;
--
1.7.10.4
--
GTSYS Limited RFID Technology
9/F, Unit E, R07, Kwai Shing Industrial Building Phase 2,
42-46 Tai Lin Pai Road, Kwai Chung, N.T., Hong Kong
Tel (852) 9079 9521
Disclaimer: http://www.gtsys.com.hk/email/classified.html
next prev parent reply other threads:[~2015-07-09 8:28 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-08 14:16 [PATCH] pinctrl: imx1-core: Fix debug output pin array index Markus Pargmann
2015-07-09 7:12 ` Uwe Kleine-König
2015-07-09 8:28 ` Chris Ruehl [this message]
2015-07-09 13:30 ` Markus Pargmann
2015-07-09 18:06 ` Uwe Kleine-König
2015-07-10 10:02 ` Markus Pargmann
2015-07-10 1:24 ` Chris Ruehl
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=559E3098.9070609@gtsys.com.hk \
--to=chris.ruehl@gtsys.com.hk \
--cc=linux-arm-kernel@lists.infradead.org \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).