ARM Sunxi Platform Development
 help / color / Atom feed
* [PATCH] pinctrl: sunxi: Don't underestimate number of functions
@ 2021-07-22 13:25 Andre Przywara
  2021-07-22 14:09 ` Maxime Ripard
  0 siblings, 1 reply; 2+ messages in thread
From: Andre Przywara @ 2021-07-22 13:25 UTC (permalink / raw)
  To: Linus Walleij, Maxime Ripard, Chen-Yu Tsai
  Cc: Jernej Skrabec, Icenowy Zheng, linux-gpio, linux-arm-kernel, linux-sunxi

When we are building all the various pinctrl structures for the
Allwinner pinctrl devices, we do some estimation about the maximum
number of distinct function (names) that we will need.

So far we take the number of pins as an upper bound, even though we
can actually have up to four special functions per pin. This wasn't a
problem until now, since we indeed have typically far more pins than
functions, and most pins share common functions.

However the H616 "-r" pin controller has only two pins, but four
functions, so we run over the end of the array when we are looking for
a matching function name in sunxi_pinctrl_add_function - there is no
NULL sentinel left that would terminate the loop:

[    8.200648] Unable to handle kernel paging request at virtual address fffdff7efbefaff5
[    8.209179] Mem abort info:
....
[    8.368456] Call trace:
[    8.370925]  __pi_strcmp+0x90/0xf0
[    8.374559]  sun50i_h616_r_pinctrl_probe+0x1c/0x28
[    8.379557]  platform_probe+0x68/0xd8

Do an actual worst case allocation (4 functions per pin, three common
functions and the sentinel) for the initial array allocation. This is
now heavily overestimating the number of functions in the common case,
but we will reallocate this array later with the actual number of
functions, so it's only temporarily.

Fixes: 561c1cf17c46 ("pinctrl: sunxi: Add support for the Allwinner H616-R pin controller")
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 drivers/pinctrl/sunxi/pinctrl-sunxi.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
index dc8d39ae045b..9c7679c06dca 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
+++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
@@ -1219,10 +1219,12 @@ static int sunxi_pinctrl_build_state(struct platform_device *pdev)
 	}
 
 	/*
-	 * We suppose that we won't have any more functions than pins,
-	 * we'll reallocate that later anyway
+	 * Find an upper bound for the maximum number of functions: in
+	 * the worst case we have gpio_in, gpio_out, irq and up to four
+	 * special functions per pin, plus one entry for the sentinel.
+	 * We'll reallocate that later anyway.
 	 */
-	pctl->functions = kcalloc(pctl->ngroups,
+	pctl->functions = kcalloc(4 * pctl->ngroups + 4,
 				  sizeof(*pctl->functions),
 				  GFP_KERNEL);
 	if (!pctl->functions)
-- 
2.17.6


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] pinctrl: sunxi: Don't underestimate number of functions
  2021-07-22 13:25 [PATCH] pinctrl: sunxi: Don't underestimate number of functions Andre Przywara
@ 2021-07-22 14:09 ` Maxime Ripard
  0 siblings, 0 replies; 2+ messages in thread
From: Maxime Ripard @ 2021-07-22 14:09 UTC (permalink / raw)
  To: Andre Przywara
  Cc: Linus Walleij, Chen-Yu Tsai, Jernej Skrabec, Icenowy Zheng,
	linux-gpio, linux-arm-kernel, linux-sunxi


[-- Attachment #1: Type: text/plain, Size: 1670 bytes --]

On Thu, Jul 22, 2021 at 02:25:48PM +0100, Andre Przywara wrote:
> When we are building all the various pinctrl structures for the
> Allwinner pinctrl devices, we do some estimation about the maximum
> number of distinct function (names) that we will need.
> 
> So far we take the number of pins as an upper bound, even though we
> can actually have up to four special functions per pin. This wasn't a
> problem until now, since we indeed have typically far more pins than
> functions, and most pins share common functions.
> 
> However the H616 "-r" pin controller has only two pins, but four
> functions, so we run over the end of the array when we are looking for
> a matching function name in sunxi_pinctrl_add_function - there is no
> NULL sentinel left that would terminate the loop:
> 
> [    8.200648] Unable to handle kernel paging request at virtual address fffdff7efbefaff5
> [    8.209179] Mem abort info:
> ....
> [    8.368456] Call trace:
> [    8.370925]  __pi_strcmp+0x90/0xf0
> [    8.374559]  sun50i_h616_r_pinctrl_probe+0x1c/0x28
> [    8.379557]  platform_probe+0x68/0xd8
> 
> Do an actual worst case allocation (4 functions per pin, three common
> functions and the sentinel) for the initial array allocation. This is
> now heavily overestimating the number of functions in the common case,
> but we will reallocate this array later with the actual number of
> functions, so it's only temporarily.
> 
> Fixes: 561c1cf17c46 ("pinctrl: sunxi: Add support for the Allwinner H616-R pin controller")
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>

Acked-by: Maxime Ripard <maxime@cerno.tech>

Thanks!
Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, back to index

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-22 13:25 [PATCH] pinctrl: sunxi: Don't underestimate number of functions Andre Przywara
2021-07-22 14:09 ` Maxime Ripard

ARM Sunxi Platform Development

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-sunxi/0 linux-sunxi/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-sunxi linux-sunxi/ https://lore.kernel.org/linux-sunxi \
		linux-sunxi@lists.linux.dev
	public-inbox-index linux-sunxi

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/dev.linux.lists.linux-sunxi


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git