[RFT 1/2] pinctrl: samsung: Fix NULL pointer exception on external interrupts on S3C24xx

[RFT 1/2] pinctrl: samsung: Fix NULL pointer exception on external interrupts on S3C24xx

[Krzysztof Kozlowski]

After commit 8b1bd11c1f8f ("pinctrl: samsung: Add the support the
multiple IORESOURCE_MEM for one pin-bank"), the S3C24xx (and probably
S3C64xx as well) fails:

	Unable to handle kernel NULL pointer dereference at virtual address 000000a8
	...
	(s3c24xx_demux_eint4_7) from [<c004469c>] (__handle_domain_irq+0x6c/0xcc)
	(__handle_domain_irq) from [<c0009444>] (s3c24xx_handle_irq+0x6c/0x12c)
	(s3c24xx_handle_irq) from [<c000e5fc>] (__irq_svc+0x5c/0x78)

Mentioned commit moved the pointer to controller's base IO memory address
from each controller's driver data (samsung_pinctrl_drv_data) to per-bank
structure (samsung_pin_bank).  The external interrupt demux
handlers (s3c24xx_demux_eint()) tried to get this base address from opaque
pointer stored under irq_chip data:

	struct irq_data *irqd = irq_desc_get_irq_data(desc);
	struct samsung_pin_bank *bank = irq_data_get_irq_chip_data(irqd);
	...
	pend = readl(bank->eint_base + EINTPEND_REG);

which is wrong because this is hardware irq and it bank was never set
for this irq_chip.

For S3C24xx and S3C64xx, this partially reverts mentioned commit by
bringing back the virt_base stored under each controller's driver data
(samsung_pinctrl_drv_data).  This virt_base address will be now
duplicated:
 - samsung_pinctrl_drv_data->virt_base: used on S3C24xx and S3C64xx,
 - samsung_pin_bank->pctl_base: used on Exynos.

Fixes: 8b1bd11c1f8f ("pinctrl: samsung: Add the support the multiple IORESOURCE_MEM for one pin-bank")
Cc: <stable@vger.kernel.org>
Cc: Sergio Prado <sergio.prado@e-labworks.com>
Reported-by: Sergio Prado <sergio.prado@e-labworks.com>
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>

---

Tested on Odroid XU3.
Please kindly test on S3C24xx and S3C64xx. Other tests, including
Exynos5433 are also welcomed.
---
 drivers/pinctrl/samsung/pinctrl-s3c24xx.c | 37 +++++++++++++++-------------
 drivers/pinctrl/samsung/pinctrl-s3c64xx.c | 40 ++++++++++++++-----------------
 drivers/pinctrl/samsung/pinctrl-samsung.c |  5 ++++
 drivers/pinctrl/samsung/pinctrl-samsung.h |  5 ++++
 4 files changed, 49 insertions(+), 38 deletions(-)

diff --git a/drivers/pinctrl/samsung/pinctrl-s3c24xx.c b/drivers/pinctrl/samsung/pinctrl-s3c24xx.c
index 49774851e84a..edf27264b603 100644
--- a/drivers/pinctrl/samsung/pinctrl-s3c24xx.c
+++ b/drivers/pinctrl/samsung/pinctrl-s3c24xx.c
@@ -151,7 +151,7 @@ static void s3c24xx_eint_set_function(struct samsung_pinctrl_drv_data *d,
 	u32 val;
 
 	/* Make sure that pin is configured as interrupt */
-	reg = bank->pctl_base + bank->pctl_offset;
+	reg = d->virt_base + bank->pctl_offset;
 	shift = pin * bank_type->fld_width[PINCFG_TYPE_FUNC];
 	mask = (1 << bank_type->fld_width[PINCFG_TYPE_FUNC]) - 1;
 
@@ -184,7 +184,7 @@ static int s3c24xx_eint_type(struct irq_data *data, unsigned int type)
 	s3c24xx_eint_set_handler(data, type);
 
 	/* Set up interrupt trigger */
-	reg = bank->eint_base + EINT_REG(index);
+	reg = d->virt_base + EINT_REG(index);
 	shift = EINT_OFFS(index);
 
 	val = readl(reg);
@@ -259,29 +259,32 @@ static void s3c2410_demux_eint0_3(struct irq_desc *desc)
 static void s3c2412_eint0_3_ack(struct irq_data *data)
 {
 	struct samsung_pin_bank *bank = irq_data_get_irq_chip_data(data);
+	struct samsung_pinctrl_drv_data *d = bank->drvdata;
 
 	unsigned long bitval = 1UL << data->hwirq;
-	writel(bitval, bank->eint_base + EINTPEND_REG);
+	writel(bitval, d->virt_base + EINTPEND_REG);
 }
 
 static void s3c2412_eint0_3_mask(struct irq_data *data)
 {
 	struct samsung_pin_bank *bank = irq_data_get_irq_chip_data(data);
+	struct samsung_pinctrl_drv_data *d = bank->drvdata;
 	unsigned long mask;
 
-	mask = readl(bank->eint_base + EINTMASK_REG);
+	mask = readl(d->virt_base + EINTMASK_REG);
 	mask |= (1UL << data->hwirq);
-	writel(mask, bank->eint_base + EINTMASK_REG);
+	writel(mask, d->virt_base + EINTMASK_REG);
 }
 
 static void s3c2412_eint0_3_unmask(struct irq_data *data)
 {
 	struct samsung_pin_bank *bank = irq_data_get_irq_chip_data(data);
+	struct samsung_pinctrl_drv_data *d = bank->drvdata;
 	unsigned long mask;
 
-	mask = readl(bank->eint_base + EINTMASK_REG);
+	mask = readl(d->virt_base + EINTMASK_REG);
 	mask &= ~(1UL << data->hwirq);
-	writel(mask, bank->eint_base + EINTMASK_REG);
+	writel(mask, d->virt_base + EINTMASK_REG);
 }
 
 static struct irq_chip s3c2412_eint0_3_chip = {
@@ -316,31 +319,34 @@ static void s3c2412_demux_eint0_3(struct irq_desc *desc)
 static void s3c24xx_eint_ack(struct irq_data *data)
 {
 	struct samsung_pin_bank *bank = irq_data_get_irq_chip_data(data);
+	struct samsung_pinctrl_drv_data *d = bank->drvdata;
 	unsigned char index = bank->eint_offset + data->hwirq;
 
-	writel(1UL << index, bank->eint_base + EINTPEND_REG);
+	writel(1UL << index, d->virt_base + EINTPEND_REG);
 }
 
 static void s3c24xx_eint_mask(struct irq_data *data)
 {
 	struct samsung_pin_bank *bank = irq_data_get_irq_chip_data(data);
+	struct samsung_pinctrl_drv_data *d = bank->drvdata;
 	unsigned char index = bank->eint_offset + data->hwirq;
 	unsigned long mask;
 
-	mask = readl(bank->eint_base + EINTMASK_REG);
+	mask = readl(d->virt_base + EINTMASK_REG);
 	mask |= (1UL << index);
-	writel(mask, bank->eint_base + EINTMASK_REG);
+	writel(mask, d->virt_base + EINTMASK_REG);
 }
 
 static void s3c24xx_eint_unmask(struct irq_data *data)
 {
 	struct samsung_pin_bank *bank = irq_data_get_irq_chip_data(data);
+	struct samsung_pinctrl_drv_data *d = bank->drvdata;
 	unsigned char index = bank->eint_offset + data->hwirq;
 	unsigned long mask;
 
-	mask = readl(bank->eint_base + EINTMASK_REG);
+	mask = readl(d->virt_base + EINTMASK_REG);
 	mask &= ~(1UL << index);
-	writel(mask, bank->eint_base + EINTMASK_REG);
+	writel(mask, d->virt_base + EINTMASK_REG);
 }
 
 static struct irq_chip s3c24xx_eint_chip = {
@@ -356,14 +362,13 @@ static inline void s3c24xx_demux_eint(struct irq_desc *desc,
 {
 	struct s3c24xx_eint_data *data = irq_desc_get_handler_data(desc);
 	struct irq_chip *chip = irq_desc_get_chip(desc);
-	struct irq_data *irqd = irq_desc_get_irq_data(desc);
-	struct samsung_pin_bank *bank = irq_data_get_irq_chip_data(irqd);
+	struct samsung_pinctrl_drv_data *d = data->drvdata;
 	unsigned int pend, mask;
 
 	chained_irq_enter(chip, desc);
 
-	pend = readl(bank->eint_base + EINTPEND_REG);
-	mask = readl(bank->eint_base + EINTMASK_REG);
+	pend = readl(d->virt_base + EINTPEND_REG);
+	mask = readl(d->virt_base + EINTMASK_REG);
 
 	pend &= ~mask;
 	pend &= range;
diff --git a/drivers/pinctrl/samsung/pinctrl-s3c64xx.c b/drivers/pinctrl/samsung/pinctrl-s3c64xx.c
index 4a88d7446e87..e63663b32907 100644
--- a/drivers/pinctrl/samsung/pinctrl-s3c64xx.c
+++ b/drivers/pinctrl/samsung/pinctrl-s3c64xx.c
@@ -280,7 +280,7 @@ static void s3c64xx_irq_set_function(struct samsung_pinctrl_drv_data *d,
 	u32 val;
 
 	/* Make sure that pin is configured as interrupt */
-	reg = bank->pctl_base + bank->pctl_offset;
+	reg = d->virt_base + bank->pctl_offset;
 	shift = pin;
 	if (bank_type->fld_width[PINCFG_TYPE_FUNC] * shift >= 32) {
 		/* 4-bit bank type with 2 con regs */
@@ -308,8 +308,9 @@ static void s3c64xx_irq_set_function(struct samsung_pinctrl_drv_data *d,
 static inline void s3c64xx_gpio_irq_set_mask(struct irq_data *irqd, bool mask)
 {
 	struct samsung_pin_bank *bank = irq_data_get_irq_chip_data(irqd);
+	struct samsung_pinctrl_drv_data *d = bank->drvdata;
 	unsigned char index = EINT_OFFS(bank->eint_offset) + irqd->hwirq;
-	void __iomem *reg = bank->eint_base + EINTMASK_REG(bank->eint_offset);
+	void __iomem *reg = d->virt_base + EINTMASK_REG(bank->eint_offset);
 	u32 val;
 
 	val = readl(reg);
@@ -333,8 +334,9 @@ static void s3c64xx_gpio_irq_mask(struct irq_data *irqd)
 static void s3c64xx_gpio_irq_ack(struct irq_data *irqd)
 {
 	struct samsung_pin_bank *bank = irq_data_get_irq_chip_data(irqd);
+	struct samsung_pinctrl_drv_data *d = bank->drvdata;
 	unsigned char index = EINT_OFFS(bank->eint_offset) + irqd->hwirq;
-	void __iomem *reg = bank->eint_base + EINTPEND_REG(bank->eint_offset);
+	void __iomem *reg = d->virt_base + EINTPEND_REG(bank->eint_offset);
 
 	writel(1 << index, reg);
 }
@@ -357,7 +359,7 @@ static int s3c64xx_gpio_irq_set_type(struct irq_data *irqd, unsigned int type)
 	s3c64xx_irq_set_handler(irqd, type);
 
 	/* Set up interrupt trigger */
-	reg = bank->eint_base + EINTCON_REG(bank->eint_offset);
+	reg = d->virt_base + EINTCON_REG(bank->eint_offset);
 	shift = EINT_OFFS(bank->eint_offset) + irqd->hwirq;
 	shift = 4 * (shift / 4); /* 4 EINTs per trigger selector */
 
@@ -409,8 +411,7 @@ static void s3c64xx_eint_gpio_irq(struct irq_desc *desc)
 {
 	struct irq_chip *chip = irq_desc_get_chip(desc);
 	struct s3c64xx_eint_gpio_data *data = irq_desc_get_handler_data(desc);
-	struct irq_data *irqd = irq_desc_get_irq_data(desc);
-	struct samsung_pin_bank *bank = irq_data_get_irq_chip_data(irqd);
+	struct samsung_pinctrl_drv_data *drvdata = data->drvdata;
 
 	chained_irq_enter(chip, desc);
 
@@ -420,7 +421,7 @@ static void s3c64xx_eint_gpio_irq(struct irq_desc *desc)
 		unsigned int pin;
 		unsigned int virq;
 
-		svc = readl(bank->eint_base + SERVICE_REG);
+		svc = readl(drvdata->virt_base + SERVICE_REG);
 		group = SVC_GROUP(svc);
 		pin = svc & SVC_NUM_MASK;
 
@@ -515,15 +516,15 @@ static inline void s3c64xx_eint0_irq_set_mask(struct irq_data *irqd, bool mask)
 {
 	struct s3c64xx_eint0_domain_data *ddata =
 					irq_data_get_irq_chip_data(irqd);
-	struct samsung_pin_bank *bank = ddata->bank;
+	struct samsung_pinctrl_drv_data *d = ddata->bank->drvdata;
 	u32 val;
 
-	val = readl(bank->eint_base + EINT0MASK_REG);
+	val = readl(d->virt_base + EINT0MASK_REG);
 	if (mask)
 		val |= 1 << ddata->eints[irqd->hwirq];
 	else
 		val &= ~(1 << ddata->eints[irqd->hwirq]);
-	writel(val, bank->eint_base + EINT0MASK_REG);
+	writel(val, d->virt_base + EINT0MASK_REG);
 }
 
 static void s3c64xx_eint0_irq_unmask(struct irq_data *irqd)
@@ -540,10 +541,10 @@ static void s3c64xx_eint0_irq_ack(struct irq_data *irqd)
 {
 	struct s3c64xx_eint0_domain_data *ddata =
 					irq_data_get_irq_chip_data(irqd);
-	struct samsung_pin_bank *bank = ddata->bank;
+	struct samsung_pinctrl_drv_data *d = ddata->bank->drvdata;
 
 	writel(1 << ddata->eints[irqd->hwirq],
-					bank->eint_base + EINT0PEND_REG);
+					d->virt_base + EINT0PEND_REG);
 }
 
 static int s3c64xx_eint0_irq_set_type(struct irq_data *irqd, unsigned int type)
@@ -551,7 +552,7 @@ static int s3c64xx_eint0_irq_set_type(struct irq_data *irqd, unsigned int type)
 	struct s3c64xx_eint0_domain_data *ddata =
 					irq_data_get_irq_chip_data(irqd);
 	struct samsung_pin_bank *bank = ddata->bank;
-	struct samsung_pinctrl_drv_data *d = ddata->bank->drvdata;
+	struct samsung_pinctrl_drv_data *d = bank->drvdata;
 	void __iomem *reg;
 	int trigger;
 	u8 shift;
@@ -566,7 +567,7 @@ static int s3c64xx_eint0_irq_set_type(struct irq_data *irqd, unsigned int type)
 	s3c64xx_irq_set_handler(irqd, type);
 
 	/* Set up interrupt trigger */
-	reg = bank->eint_base + EINT0CON0_REG;
+	reg = d->virt_base + EINT0CON0_REG;
 	shift = ddata->eints[irqd->hwirq];
 	if (shift >= EINT_MAX_PER_REG) {
 		reg += 4;
@@ -598,19 +599,14 @@ static struct irq_chip s3c64xx_eint0_irq_chip = {
 static inline void s3c64xx_irq_demux_eint(struct irq_desc *desc, u32 range)
 {
 	struct irq_chip *chip = irq_desc_get_chip(desc);
-	struct irq_data *irqd = irq_desc_get_irq_data(desc);
-	struct s3c64xx_eint0_domain_data *ddata =
-					irq_data_get_irq_chip_data(irqd);
-	struct samsung_pin_bank *bank = ddata->bank;
-
 	struct s3c64xx_eint0_data *data = irq_desc_get_handler_data(desc);
-
+	struct samsung_pinctrl_drv_data *drvdata = data->drvdata;
 	unsigned int pend, mask;
 
 	chained_irq_enter(chip, desc);
 
-	pend = readl(bank->eint_base + EINT0PEND_REG);
-	mask = readl(bank->eint_base + EINT0MASK_REG);
+	pend = readl(drvdata->virt_base + EINT0PEND_REG);
+	mask = readl(drvdata->virt_base + EINT0MASK_REG);
 
 	pend = pend & range & ~mask;
 	pend &= range;
diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.c b/drivers/pinctrl/samsung/pinctrl-samsung.c
index f542642eed8d..a25c3ffae25c 100644
--- a/drivers/pinctrl/samsung/pinctrl-samsung.c
+++ b/drivers/pinctrl/samsung/pinctrl-samsung.c
@@ -1013,6 +1013,11 @@ samsung_pinctrl_get_soc_data(struct samsung_pinctrl_drv_data *d,
 		bank->eint_base = virt_base[0];
 		bank->pctl_base = virt_base[bdata->pctl_res_idx];
 	}
+	/*
+	 * For legacy platforms which need to access IO memory through
+	 * samsung_pinctrl_drv_data:
+	 */
+	d->virt_base = virt_base[bdata->pctl_res_idx];
 
 	for_each_child_of_node(node, np) {
 		if (!of_find_property(np, "gpio-controller", NULL))
diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.h b/drivers/pinctrl/samsung/pinctrl-samsung.h
index 515a61035e54..61c4cab0ad24 100644
--- a/drivers/pinctrl/samsung/pinctrl-samsung.h
+++ b/drivers/pinctrl/samsung/pinctrl-samsung.h
@@ -247,6 +247,10 @@ struct samsung_pin_ctrl {
 /**
  * struct samsung_pinctrl_drv_data: wrapper for holding driver data together.
  * @node: global list node
+ * @virt_base: register base address of the controller; this will be equal
+ *             to each bank samsung_pin_bank->pctl_base and used on legacy
+ *             platforms (like S3C24XX or S3C64XX) which has to access the base
+ *             through samsung_pinctrl_drv_data, not samsung_pin_bank).
  * @dev: device instance representing the controller.
  * @irq: interrpt number used by the controller to notify gpio interrupts.
  * @ctrl: pin controller instance managed by the driver.
@@ -262,6 +266,7 @@ struct samsung_pin_ctrl {
  */
 struct samsung_pinctrl_drv_data {
 	struct list_head		node;
+	void __iomem			*virt_base;
 	struct device			*dev;
 	int				irq;
 
-- 
2.9.3

[RFT 2/2] pinctrl: samsung: Fix invalid register offset used for Exynos5433 external interrupts

[Krzysztof Kozlowski]

When setting the pin function for external interrupts, the driver used
wrong IO memory address base.  The pin function register is always under
pctl_base, not the eint_base.

By updating wrong register, the external interrupts for chosen GPIO
would not work at all and some other GPIO might be configured to wrong
value.

Platforms other than Exynos5433 should not be affected as eint_base
equals pctl_base in such case.

Fixes: 8b1bd11c1f8f ("pinctrl: samsung: Add the support the multiple IORESOURCE_MEM for one pin-bank")
Cc: <stable@vger.kernel.org>
Reported-by: Tomasz Figa <tomasz.figa@gmail.com>
Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>

---

Tested on Odroid XU3 (so actually this particular case was not
reproduced).
Please kindly test on Exynos5433.
---
 drivers/pinctrl/samsung/pinctrl-exynos.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/samsung/pinctrl-exynos.c b/drivers/pinctrl/samsung/pinctrl-exynos.c
index 96068b40d32a..d95a746f45d7 100644
--- a/drivers/pinctrl/samsung/pinctrl-exynos.c
+++ b/drivers/pinctrl/samsung/pinctrl-exynos.c
@@ -174,10 +174,10 @@ static int exynos_irq_request_resources(struct irq_data *irqd)
 
 	spin_lock_irqsave(&bank->slock, flags);
 
-	con = readl(bank->eint_base + reg_con);
+	con = readl(bank->pctl_base + reg_con);
 	con &= ~(mask << shift);
 	con |= EXYNOS_EINT_FUNC << shift;
-	writel(con, bank->eint_base + reg_con);
+	writel(con, bank->pctl_base + reg_con);
 
 	spin_unlock_irqrestore(&bank->slock, flags);
 
@@ -206,10 +206,10 @@ static void exynos_irq_release_resources(struct irq_data *irqd)
 
 	spin_lock_irqsave(&bank->slock, flags);
 
-	con = readl(bank->eint_base + reg_con);
+	con = readl(bank->pctl_base + reg_con);
 	con &= ~(mask << shift);
 	con |= FUNC_INPUT << shift;
-	writel(con, bank->eint_base + reg_con);
+	writel(con, bank->pctl_base + reg_con);
 
 	spin_unlock_irqrestore(&bank->slock, flags);
 
-- 
2.9.3

Re: [RFT 1/2] pinctrl: samsung: Fix NULL pointer exception on external interrupts on S3C24xx

[Yao Lihua]

Hi Krzysztof,

Another Oops on S3C6410.

[    2.842434] Unable to handle kernel NULL pointer dereference at virtual 
address 00000900
[    2.848477] pgd = c0004000
[    2.851165] [00000900] *pgd=00000000
[    2.854738] Internal error: Oops: 5 [#1] PREEMPT ARM
[    2.859669] Modules linked in:
[    2.862716] CPU: 0 PID: 1 Comm: swapper Not tainted 4.12.0-rc4 #118
[    2.868953] Hardware name: Samsung S3C64xx (Flattened Device Tree)
[    2.875115] task: cf890000 task.stack: cf88e000
[    2.879642] PC is at s3c64xx_eint0_irq_set_type+0x94/0xec
[    2.885013] LR is at s3c64xx_eint0_irq_set_type+0x94/0xec
[    2.890393] pc : [<c02f3c20>]    lr : [<c02f3c20>]    psr: 20000093
[    2.890393] sp : cf88fd18  ip : 00000590  fp : cf213e10
[    2.901837] r10: 00000000  r9 : cf212234  r8 : 00000006
[    2.907044] r7 : cf8d8710  r6 : 00000900  r5 : cf83fbd8  r4 : cf212210
[    2.913551] r3 : 00000001  r2 : cf88e000  r1 : cf88e004  r0 : 00000019
[    2.920060] Flags: nzCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment none
[    2.927263] Control: 00c5387d  Table: 50004008  DAC: 00000051
[    2.932990] Process swapper (pid: 1, stack limit = 0xcf88e188)
[    2.938802] Stack: (0xcf88fd18 to 0xcf890000)
[    2.943146] fd00: cf21228c cf212200
[    2.951309] fd20: c02f3b8c 00000000 c07ba9e4 00000003 60000013 c005bae8 
60000013 cf21225c
[    2.959465] fd40: 00000050 cf212200 cf076900 00000050 cf21225c c005c094 
00000001 00000050
[    2.967622] fd60: 00000050 00000000 00000000 cf20bc3c cf076900 cf212210 
cf212200 c005c39c
[    2.975776] fd80: 00000000 00000083 00000050 c0bb49a8 00000083 c03c35e0 
cf20bc3c cf20bc3c
[    2.983934] fda0: cf98ee10 c005c49c c0bb49a8 cf20bc3c cf1e3ef0 c0bb49a8 
00000050 c03c35e0
[    2.992090] fdc0: 00000083 c005e598 cf20bc3c c0530fe8 cf20bc10 cf213e2c 
00000000 c0bb49a8
[    3.000245] fde0: cf20bc10 cf98ee10 00000083 c03c2e80 c0bb49a8 cf20bc3c 
00000000 c017f644
[    3.008401] fe00: 00000000 00000000 cf20a400 cf20bc3c cfdb68a0 00000000 
c03c35e0 cf98ee00
[    3.016558] fe20: cf2150f0 cf20bc6c 00000001 cf98ee10 c03c2be8 c07c61c8 
c07c61b4 c07c61c8
[    3.024712] fe40: 00000000 00000001 c08120d0 c034d508 c034d4b4 cf98ee10 
c08120d0 00000000
[    3.032869] fe60: c08120d8 c034b8c4 c058517c 00000000 cf98efa0 00000001 
c058517c cf98ee10
[    3.041025] fe80: c07c61c8 cf98ee44 00000000 00000000 c0760ff8 00000000 
c071d4fc c034ba18
[    3.049181] fea0: c07c61c8 c034b974 00000000 c0349f44 cf808a70 cf9b4e20 
c07c61c8 cf1de4e0
[    3.057337] fec0: c07c1848 c034ae98 c06aa8a8 c07c61c8 00000007 c07c61c8 
00000007 c0749370
[    3.065494] fee0: c07e0000 c034c570 c034cecc c0767048 00000007 c0009824 
0000004e c05338c4
[    3.073648] ff00: cf8d8000 00000000 00000000 00000000 0000009e 00000000 
cfffcdbd 00000000
[    3.081805] ff20: 0000009e 0000009e cfffcdc4 c003b714 c06ef210 00000000 
00000007 00000007
[    3.089961] ff40: cfffcdc0 cfffcdbd 00000000 dae6109a c0756860 00000007 
c0756840 c07e0000
[    3.098117] ff60: 0000009e 00000000 c071d4fc c071dda0 00000007 00000007 
00000000 c071d4fc
[    3.106272] ff80: 00000000 00000000 c052ae90 00000000 00000000 00000000 
00000000 00000000
[    3.114430] ffa0: 00000000 c052ae98 00000000 c000f7b8 00000000 00000000 
00000000 00000000
[    3.122584] ffc0: 00000000 00000000 00000000 00000000 00000000 00000000 
00000000 00000000
[    3.130742] ffe0: 00000000 00000000 00000000 00000000 00000013 00000000 
ffffffff ffffffff
[    3.138927] [<c02f3c20>] (s3c64xx_eint0_irq_set_type) from [<c005bae8>] 
(__irq_set_trigger+0x60/0x154)
[    3.148195] [<c005bae8>] (__irq_set_trigger) from [<c005c094>] 
(__setup_irq+0x4b8/0x5ac)
[    3.156259] [<c005c094>] (__setup_irq) from [<c005c39c>] 
(request_threaded_irq+0x120/0x19c)
[    3.164589] [<c005c39c>] (request_threaded_irq) from [<c005c49c>] 
(request_any_context_irq+0x84/0x88)
[    3.173789] [<c005c49c>] (request_any_context_irq) from [<c005e598>] 
(devm_request_any_context_irq+0x60/0xa8)
[    3.183694] [<c005e598>] (devm_request_any_context_irq) from [<c03c2e80>] 
(gpio_keys_probe+0x298/0x85c)
[    3.193065] [<c03c2e80>] (gpio_keys_probe) from [<c034d508>] 
(platform_drv_probe+0x54/0xa4)
[    3.201383] [<c034d508>] (platform_drv_probe) from [<c034b8c4>] 
(driver_probe_device+0x248/0x2f8)
[    3.210229] [<c034b8c4>] (driver_probe_device) from [<c034ba18>] 
(__driver_attach+0xa4/0xa8)
[    3.218645] [<c034ba18>] (__driver_attach) from [<c0349f44>] 
(bus_for_each_dev+0x64/0x88)
[    3.226800] [<c0349f44>] (bus_for_each_dev) from [<c034ae98>] 
(bus_add_driver+0x16c/0x1fc)
[    3.235047] [<c034ae98>] (bus_add_driver) from [<c034c570>] 
(driver_register+0x78/0xf4)
[    3.243028] [<c034c570>] (driver_register) from [<c0009824>] 
(do_one_initcall+0x50/0x194)
[    3.251192] [<c0009824>] (do_one_initcall) from [<c071dda0>] 
(kernel_init_freeable+0x174/0x244)
[    3.259862] [<c071dda0>] (kernel_init_freeable) from [<c052ae98>] 
(kernel_init+0x8/0xf0)
[    3.267935] [<c052ae98>] (kernel_init) from [<c000f7b8>] 
(ret_from_fork+0x14/0x3c)

> diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.c b/drivers/pinctrl/samsung/pinctrl-samsung.c
> index f542642eed8d..a25c3ffae25c 100644
> --- a/drivers/pinctrl/samsung/pinctrl-samsung.c
> +++ b/drivers/pinctrl/samsung/pinctrl-samsung.c
> @@ -1013,6 +1013,11 @@ samsung_pinctrl_get_soc_data(struct samsung_pinctrl_drv_data *d,
>   		bank->eint_base = virt_base[0];
>   		bank->pctl_base = virt_base[bdata->pctl_res_idx];
>   	}
> +	/*
> +	 * For legacy platforms which need to access IO memory through
> +	 * samsung_pinctrl_drv_data:
> +	 */
> +	d->virt_base = virt_base[bdata->pctl_res_idx];
>   
>
     for (i = 0; i < ctrl->nr_banks; ++i, ++bdata, ++bank) {
         ......
     }
     /*
      * For legacy platforms which need to access IO memory through
      * samsung_pinctrl_drv_data:
      */
     d->virt_base = virt_base[bdata->pctl_res_idx];
                                                     ^
                                                      **bdata** is invalid here.


Thanks!
Lihua

Re: [RFT 1/2] pinctrl: samsung: Fix NULL pointer exception on external interrupts on S3C24xx

[Krzysztof Kozlowski]

On Thu, Jun 15, 2017 at 10:42:30PM +0800, Yao Lihua wrote:
> Hi Krzysztof,
> 
> Another Oops on S3C6410.

(...)

 > diff --git a/drivers/pinctrl/samsung/pinctrl-samsung.c b/drivers/pinctrl/samsung/pinctrl-samsung.c
> > index f542642eed8d..a25c3ffae25c 100644
> > --- a/drivers/pinctrl/samsung/pinctrl-samsung.c
> > +++ b/drivers/pinctrl/samsung/pinctrl-samsung.c
> > @@ -1013,6 +1013,11 @@ samsung_pinctrl_get_soc_data(struct samsung_pinctrl_drv_data *d,
> >   		bank->eint_base = virt_base[0];
> >   		bank->pctl_base = virt_base[bdata->pctl_res_idx];
> >   	}
> > +	/*
> > +	 * For legacy platforms which need to access IO memory through
> > +	 * samsung_pinctrl_drv_data:
> > +	 */
> > +	d->virt_base = virt_base[bdata->pctl_res_idx];
> > 
>     for (i = 0; i < ctrl->nr_banks; ++i, ++bdata, ++bank) {
>         ......
>     }
>     /*
>      * For legacy platforms which need to access IO memory through
>      * samsung_pinctrl_drv_data:
>      */
>     d->virt_base = virt_base[bdata->pctl_res_idx];
>                                                     ^
>                                                      **bdata** is invalid here.

Ah, of course, apparently it worked in my case by coincidence. Thanks
for spotting this.


Best regards,
Krzysztof

Re: [RFT 2/2] pinctrl: samsung: Fix invalid register offset used for Exynos5433 external interrupts

[Sylwester Nawrocki]

On 06/14/2017 03:18 PM, Krzysztof Kozlowski wrote:
> When setting the pin function for external interrupts, the driver used
> wrong IO memory address base.  The pin function register is always under
> pctl_base, not the eint_base.
> 
> By updating wrong register, the external interrupts for chosen GPIO
> would not work at all and some other GPIO might be configured to wrong
> value.
> 
> Platforms other than Exynos5433 should not be affected as eint_base
> equals pctl_base in such case.
> 
> Fixes: 8b1bd11c1f8f ("pinctrl: samsung: Add the support the multiple IORESOURCE_MEM for one pin-bank")
> Cc:<stable@vger.kernel.org>
> Reported-by: Tomasz Figa<tomasz.figa@gmail.com>
> Signed-off-by: Krzysztof Kozlowski<krzk@kernel.org>

The patch looks good to me, I'll try to test it later today.

Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>

-- 
Thanks,
Sylwester

Re: [RFT 2/2] pinctrl: samsung: Fix invalid register offset used for Exynos5433 external interrupts

[Sylwester Nawrocki]

On 06/14/2017 03:18 PM, Krzysztof Kozlowski wrote:
> When setting the pin function for external interrupts, the driver used
> wrong IO memory address base.  The pin function register is always under
> pctl_base, not the eint_base.
> 
> By updating wrong register, the external interrupts for chosen GPIO
> would not work at all and some other GPIO might be configured to wrong
> value.
> 
> Platforms other than Exynos5433 should not be affected as eint_base
> equals pctl_base in such case.
> 
> Fixes: 8b1bd11c1f8f ("pinctrl: samsung: Add the support the multiple 
> IORESOURCE_MEM for one pin-bank")
> Cc:<stable@vger.kernel.org>
> Reported-by: Tomasz Figa<tomasz.figa@gmail.com>
> Signed-off-by: Krzysztof Kozlowski<krzk@kernel.org>

I've tested this patch by comparing GPFx_CON register values, as 
nothing seems to be currently using pins from the GPF1...GPF5 banks 
as external interrupts on TM2, the only board based on Exynos5433.
I changed sii8620 interrupt-parent temporarily to &gpf1 and GPF1_CON
got configured properly with the patch and improperly without 
the patch. I guess this can be considered as

Tested-by: Sylwester Nawrocki <s.nawrocki@samsung.com>

-- 
Thanks,
Sylwester