Re: [PATCH] pinctrl: imx1-core: Fix debug output pin array index

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxxx
	Reported-by: Chris Ruehl <chris.ruehl@xxxxxxxxxxxx>
	Signed-off-by: Markus Pargmann <mpa@xxxxxxxxxxxxxx>

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@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@xxxxxxxxxxxx>
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@xxxxxxxxxxxx>
---
 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
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]