* [PATCH v3] pinctrl: renesas: checker: Rework drive and bias pin iteration
@ 2022-04-19 8:08 Geert Uytterhoeven
2022-04-19 8:14 ` Wolfram Sang
2022-04-19 18:44 ` Geert Uytterhoeven
0 siblings, 2 replies; 4+ messages in thread
From: Geert Uytterhoeven @ 2022-04-19 8:08 UTC (permalink / raw)
To: Linus Walleij, Wolfram Sang
Cc: linux-gpio, linux-renesas-soc, Geert Uytterhoeven
The checker code to iterate over all drive strength and bias register
description items is cumbersome, due to the repeated calculation of
indices, and the use of hardcoded array sizes. The latter was done
under the assumption they would never need to be changed, which turned
out to be false.
Increase readability by introducing helper macros to access drive
strength and bias register description items.
Increase maintainability by replacing hardcoded numbers by array sizes
calculated at compile-time.
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v3:
- Add and use drive_ofs() and bias_ofs() helpers, as suggested by
Wolfram,
v2:
- Replace "pinctrl: renesas: checker: Fix for drive reg field
increase" by independent proper solution.
To be inserted into renesas-pinctrl-for-v5.19 before commit
d5c9688095d29a6c ("pinctrl: renesas: Allow up to 10 fields for
drive_regs") in renesas-pinctrl-for-v5.19.
---
drivers/pinctrl/renesas/core.c | 61 ++++++++++++++++++----------------
1 file changed, 33 insertions(+), 28 deletions(-)
diff --git a/drivers/pinctrl/renesas/core.c b/drivers/pinctrl/renesas/core.c
index d0d4714731c14cf5..d3515d09a0818422 100644
--- a/drivers/pinctrl/renesas/core.c
+++ b/drivers/pinctrl/renesas/core.c
@@ -1007,7 +1007,18 @@ static void __init sh_pfc_compare_groups(const char *drvname,
static void __init sh_pfc_check_info(const struct sh_pfc_soc_info *info)
{
const struct pinmux_drive_reg *drive_regs = info->drive_regs;
+#define drive_nfields ARRAY_SIZE(drive_regs->fields)
+#define drive_ofs(i) drive_regs[(i) / drive_nfields]
+#define drive_reg(i) drive_ofs(i).reg
+#define drive_bit(i) ((i) % drive_nfields)
+#define drive_field(i) drive_ofs(i).fields[drive_bit(i)]
const struct pinmux_bias_reg *bias_regs = info->bias_regs;
+#define bias_npins ARRAY_SIZE(bias_regs->pins)
+#define bias_ofs(i) bias_regs[(i) / bias_npins]
+#define bias_puen(i) bias_ofs(i).puen
+#define bias_pud(i) bias_ofs(i).pud
+#define bias_bit(i) ((i) % bias_npins)
+#define bias_pin(i) bias_ofs(i).pins[bias_bit(i)]
const char *drvname = info->name;
unsigned int *refcnts;
unsigned int i, j, k;
@@ -1076,17 +1087,17 @@ static void __init sh_pfc_check_info(const struct sh_pfc_soc_info *info)
if (!drive_regs) {
sh_pfc_err_once(drive, "SH_PFC_PIN_CFG_DRIVE_STRENGTH flag set but drive_regs missing\n");
} else {
- for (j = 0; drive_regs[j / 8].reg; j++) {
- if (!drive_regs[j / 8].fields[j % 8].pin &&
- !drive_regs[j / 8].fields[j % 8].offset &&
- !drive_regs[j / 8].fields[j % 8].size)
+ for (j = 0; drive_reg(j); j++) {
+ if (!drive_field(j).pin &&
+ !drive_field(j).offset &&
+ !drive_field(j).size)
continue;
- if (drive_regs[j / 8].fields[j % 8].pin == pin->pin)
+ if (drive_field(j).pin == pin->pin)
break;
}
- if (!drive_regs[j / 8].reg)
+ if (!drive_reg(j))
sh_pfc_err("pin %s: SH_PFC_PIN_CFG_DRIVE_STRENGTH flag set but not in drive_regs\n",
pin->name);
}
@@ -1164,20 +1175,17 @@ static void __init sh_pfc_check_info(const struct sh_pfc_soc_info *info)
for (i = 0; drive_regs && drive_regs[i].reg; i++)
sh_pfc_check_drive_reg(info, &drive_regs[i]);
- for (i = 0; drive_regs && drive_regs[i / 8].reg; i++) {
- if (!drive_regs[i / 8].fields[i % 8].pin &&
- !drive_regs[i / 8].fields[i % 8].offset &&
- !drive_regs[i / 8].fields[i % 8].size)
+ for (i = 0; drive_regs && drive_reg(i); i++) {
+ if (!drive_field(i).pin && !drive_field(i).offset &&
+ !drive_field(i).size)
continue;
for (j = 0; j < i; j++) {
- if (drive_regs[i / 8].fields[i % 8].pin ==
- drive_regs[j / 8].fields[j % 8].pin &&
- drive_regs[j / 8].fields[j % 8].offset &&
- drive_regs[j / 8].fields[j % 8].size) {
+ if (drive_field(i).pin == drive_field(j).pin &&
+ drive_field(j).offset && drive_field(j).size) {
sh_pfc_err("drive_reg 0x%x:%u/0x%x:%u: pin conflict\n",
- drive_regs[i / 8].reg, i % 8,
- drive_regs[j / 8].reg, j % 8);
+ drive_reg(i), drive_bit(i),
+ drive_reg(j), drive_bit(j));
}
}
}
@@ -1186,26 +1194,23 @@ static void __init sh_pfc_check_info(const struct sh_pfc_soc_info *info)
for (i = 0; bias_regs && (bias_regs[i].puen || bias_regs[i].pud); i++)
sh_pfc_check_bias_reg(info, &bias_regs[i]);
- for (i = 0; bias_regs &&
- (bias_regs[i / 32].puen || bias_regs[i / 32].pud); i++) {
- if (bias_regs[i / 32].pins[i % 32] == SH_PFC_PIN_NONE)
+ for (i = 0; bias_regs && (bias_puen(i) || bias_pud(i)); i++) {
+ if (bias_pin(i) == SH_PFC_PIN_NONE)
continue;
for (j = 0; j < i; j++) {
- if (bias_regs[i / 32].pins[i % 32] !=
- bias_regs[j / 32].pins[j % 32])
+ if (bias_pin(i) != bias_pin(j))
continue;
- if (bias_regs[i / 32].puen && bias_regs[j / 32].puen)
+ if (bias_puen(i) && bias_puen(j))
sh_pfc_err("bias_reg 0x%x:%u/0x%x:%u: pin conflict\n",
- bias_regs[i / 32].puen, i % 32,
- bias_regs[j / 32].puen, j % 32);
- if (bias_regs[i / 32].pud && bias_regs[j / 32].pud)
+ bias_puen(i), bias_bit(i),
+ bias_puen(j), bias_bit(j));
+ if (bias_pud(i) && bias_pud(j))
sh_pfc_err("bias_reg 0x%x:%u/0x%x:%u: pin conflict\n",
- bias_regs[i / 32].pud, i % 32,
- bias_regs[j / 32].pud, j % 32);
+ bias_pud(i), bias_bit(i),
+ bias_pud(j), bias_bit(j));
}
-
}
/* Check ioctrl registers */
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v3] pinctrl: renesas: checker: Rework drive and bias pin iteration
2022-04-19 8:08 [PATCH v3] pinctrl: renesas: checker: Rework drive and bias pin iteration Geert Uytterhoeven
@ 2022-04-19 8:14 ` Wolfram Sang
2022-04-19 18:44 ` Geert Uytterhoeven
1 sibling, 0 replies; 4+ messages in thread
From: Wolfram Sang @ 2022-04-19 8:14 UTC (permalink / raw)
To: Geert Uytterhoeven; +Cc: Linus Walleij, linux-gpio, linux-renesas-soc
[-- Attachment #1: Type: text/plain, Size: 809 bytes --]
On Tue, Apr 19, 2022 at 10:08:32AM +0200, Geert Uytterhoeven wrote:
> The checker code to iterate over all drive strength and bias register
> description items is cumbersome, due to the repeated calculation of
> indices, and the use of hardcoded array sizes. The latter was done
> under the assumption they would never need to be changed, which turned
> out to be false.
>
> Increase readability by introducing helper macros to access drive
> strength and bias register description items.
> Increase maintainability by replacing hardcoded numbers by array sizes
> calculated at compile-time.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Yay, no hardcoded maximum anymore and much better to read. Win-win!
Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3] pinctrl: renesas: checker: Rework drive and bias pin iteration
2022-04-19 8:08 [PATCH v3] pinctrl: renesas: checker: Rework drive and bias pin iteration Geert Uytterhoeven
@ 2022-04-19 18:44 ` Geert Uytterhoeven
2022-04-19 18:44 ` Geert Uytterhoeven
1 sibling, 0 replies; 4+ messages in thread
From: Geert Uytterhoeven @ 2022-04-19 18:44 UTC (permalink / raw)
To: Linus Walleij, Wolfram Sang
Cc: open list:GPIO SUBSYSTEM, Linux-Renesas, kbuild-all
On Tue, Apr 19, 2022 at 10:08 AM Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
> The checker code to iterate over all drive strength and bias register
> description items is cumbersome, due to the repeated calculation of
> indices, and the use of hardcoded array sizes. The latter was done
> under the assumption they would never need to be changed, which turned
> out to be false.
>
> Increase readability by introducing helper macros to access drive
> strength and bias register description items.
> Increase maintainability by replacing hardcoded numbers by array sizes
> calculated at compile-time.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> v3:
> - Add and use drive_ofs() and bias_ofs() helpers, as suggested by
> Wolfram,
> --- a/drivers/pinctrl/renesas/core.c
> +++ b/drivers/pinctrl/renesas/core.c
> @@ -1007,7 +1007,18 @@ static void __init sh_pfc_compare_groups(const char *drvname,
> static void __init sh_pfc_check_info(const struct sh_pfc_soc_info *info)
> {
> const struct pinmux_drive_reg *drive_regs = info->drive_regs;
> +#define drive_nfields ARRAY_SIZE(drive_regs->fields)
> +#define drive_ofs(i) drive_regs[(i) / drive_nfields]
> +#define drive_reg(i) drive_ofs(i).reg
> +#define drive_bit(i) ((i) % drive_nfields)
> +#define drive_field(i) drive_ofs(i).fields[drive_bit(i)]
> const struct pinmux_bias_reg *bias_regs = info->bias_regs;
> +#define bias_npins ARRAY_SIZE(bias_regs->pins)
> +#define bias_ofs(i) bias_regs[(i) / bias_npins]
> +#define bias_puen(i) bias_ofs(i).puen
> +#define bias_pud(i) bias_ofs(i).pud
> +#define bias_bit(i) ((i) % bias_npins)
> +#define bias_pin(i) bias_ofs(i).pins[bias_bit(i)]
> const char *drvname = info->name;
> unsigned int *refcnts;
> unsigned int i, j, k;
> @@ -1164,20 +1175,17 @@ static void __init sh_pfc_check_info(const struct sh_pfc_soc_info *info)
> for (i = 0; drive_regs && drive_regs[i].reg; i++)
> sh_pfc_check_drive_reg(info, &drive_regs[i]);
>
> - for (i = 0; drive_regs && drive_regs[i / 8].reg; i++) {
> - if (!drive_regs[i / 8].fields[i % 8].pin &&
> - !drive_regs[i / 8].fields[i % 8].offset &&
> - !drive_regs[i / 8].fields[i % 8].size)
> + for (i = 0; drive_regs && drive_reg(i); i++) {
> + if (!drive_field(i).pin && !drive_field(i).offset &&
> + !drive_field(i).size)
> continue;
>
> for (j = 0; j < i; j++) {
> - if (drive_regs[i / 8].fields[i % 8].pin ==
> - drive_regs[j / 8].fields[j % 8].pin &&
> - drive_regs[j / 8].fields[j % 8].offset &&
> - drive_regs[j / 8].fields[j % 8].size) {
> + if (drive_field(i).pin == drive_field(j).pin &&
> + drive_field(j).offset && drive_field(j).size) {
> sh_pfc_err("drive_reg 0x%x:%u/0x%x:%u: pin conflict\n",
^^ ^^
> - drive_regs[i / 8].reg, i % 8,
> - drive_regs[j / 8].reg, j % 8);
> + drive_reg(i), drive_bit(i),
> + drive_reg(j), drive_bit(j));
Whoops, as reported by kernel test robot for 64-bit builds, drive_bit()
is no longer unsigned int, but size_t[*], hence "%zu" should be used
for printing. The same is true for bias_bit().
Will fix up tomorrow...
[*] A bit counter-intuitive from the mathematical point of view,
but as "size_t" is either "unsigned int" or "unsigned long",
"unsigned int % size_t" is ... "size_t"!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3] pinctrl: renesas: checker: Rework drive and bias pin iteration
@ 2022-04-19 18:44 ` Geert Uytterhoeven
0 siblings, 0 replies; 4+ messages in thread
From: Geert Uytterhoeven @ 2022-04-19 18:44 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 4276 bytes --]
On Tue, Apr 19, 2022 at 10:08 AM Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
> The checker code to iterate over all drive strength and bias register
> description items is cumbersome, due to the repeated calculation of
> indices, and the use of hardcoded array sizes. The latter was done
> under the assumption they would never need to be changed, which turned
> out to be false.
>
> Increase readability by introducing helper macros to access drive
> strength and bias register description items.
> Increase maintainability by replacing hardcoded numbers by array sizes
> calculated at compile-time.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> v3:
> - Add and use drive_ofs() and bias_ofs() helpers, as suggested by
> Wolfram,
> --- a/drivers/pinctrl/renesas/core.c
> +++ b/drivers/pinctrl/renesas/core.c
> @@ -1007,7 +1007,18 @@ static void __init sh_pfc_compare_groups(const char *drvname,
> static void __init sh_pfc_check_info(const struct sh_pfc_soc_info *info)
> {
> const struct pinmux_drive_reg *drive_regs = info->drive_regs;
> +#define drive_nfields ARRAY_SIZE(drive_regs->fields)
> +#define drive_ofs(i) drive_regs[(i) / drive_nfields]
> +#define drive_reg(i) drive_ofs(i).reg
> +#define drive_bit(i) ((i) % drive_nfields)
> +#define drive_field(i) drive_ofs(i).fields[drive_bit(i)]
> const struct pinmux_bias_reg *bias_regs = info->bias_regs;
> +#define bias_npins ARRAY_SIZE(bias_regs->pins)
> +#define bias_ofs(i) bias_regs[(i) / bias_npins]
> +#define bias_puen(i) bias_ofs(i).puen
> +#define bias_pud(i) bias_ofs(i).pud
> +#define bias_bit(i) ((i) % bias_npins)
> +#define bias_pin(i) bias_ofs(i).pins[bias_bit(i)]
> const char *drvname = info->name;
> unsigned int *refcnts;
> unsigned int i, j, k;
> @@ -1164,20 +1175,17 @@ static void __init sh_pfc_check_info(const struct sh_pfc_soc_info *info)
> for (i = 0; drive_regs && drive_regs[i].reg; i++)
> sh_pfc_check_drive_reg(info, &drive_regs[i]);
>
> - for (i = 0; drive_regs && drive_regs[i / 8].reg; i++) {
> - if (!drive_regs[i / 8].fields[i % 8].pin &&
> - !drive_regs[i / 8].fields[i % 8].offset &&
> - !drive_regs[i / 8].fields[i % 8].size)
> + for (i = 0; drive_regs && drive_reg(i); i++) {
> + if (!drive_field(i).pin && !drive_field(i).offset &&
> + !drive_field(i).size)
> continue;
>
> for (j = 0; j < i; j++) {
> - if (drive_regs[i / 8].fields[i % 8].pin ==
> - drive_regs[j / 8].fields[j % 8].pin &&
> - drive_regs[j / 8].fields[j % 8].offset &&
> - drive_regs[j / 8].fields[j % 8].size) {
> + if (drive_field(i).pin == drive_field(j).pin &&
> + drive_field(j).offset && drive_field(j).size) {
> sh_pfc_err("drive_reg 0x%x:%u/0x%x:%u: pin conflict\n",
^^ ^^
> - drive_regs[i / 8].reg, i % 8,
> - drive_regs[j / 8].reg, j % 8);
> + drive_reg(i), drive_bit(i),
> + drive_reg(j), drive_bit(j));
Whoops, as reported by kernel test robot for 64-bit builds, drive_bit()
is no longer unsigned int, but size_t[*], hence "%zu" should be used
for printing. The same is true for bias_bit().
Will fix up tomorrow...
[*] A bit counter-intuitive from the mathematical point of view,
but as "size_t" is either "unsigned int" or "unsigned long",
"unsigned int % size_t" is ... "size_t"!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert(a)linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-04-19 18:44 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-19 8:08 [PATCH v3] pinctrl: renesas: checker: Rework drive and bias pin iteration Geert Uytterhoeven
2022-04-19 8:14 ` Wolfram Sang
2022-04-19 18:44 ` Geert Uytterhoeven
2022-04-19 18:44 ` Geert Uytterhoeven
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.