Thread (118 messages) 118 messages, 8 authors, 2023-09-28

Re: [PATCH 23/37] pinctrl: renesas: rzg2l: index all registers based on port offset

From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: 2023-09-20 13:21:04
Also in: linux-clk, linux-devicetree, linux-gpio, linux-mmc, linux-renesas-soc, linux-serial, lkml

Hi Claudiu,

Thanks for your patch!

On Tue, Sep 12, 2023 at 6:53 AM Claudiu [off-list ref] wrote:
From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

To get address that needs to be read/write for specific port
functionalities the P(), PM(), PMC(), PFC(), PIN(), IOLH() IEN(), ISEL()
macros are used. Some of these macros received as argument the hardware
port identifier, some hardware port offset address (e.g. ISEL() received
port identifier, IOLH() received port offset address). This makes hard to
extend the current driver for SoCs were port identifiers are not continuous
in memory map of pin controller. This is the case for RZ/G3S pin controller
were ports are mapped as follows:

port offset    port identifier
-----------    ---------------
0x20           P0
0x21           P5
0x22           P6
0x23           P11
0x24           P12
0x25           P13
0x26           P14
0x27           P15
0x28           P16
0x29           P17
0x2a           P18
0x30           P1
0x31           P2
0x32           P3
0x33           P4
0x34           P7
0x35           P8
0x36           P8
0x37           P10

To make this achievable change all the above macros used to get the address
of a port register for specific port functionality based on port hardware
address. Shortly, all the above macros will get as argument the port
offset address listed in the above table.

With this RZG2L_SINGLE_PIN_GET_PORT_OFFSET() and
RZG2L_PIN_ID_TO_PORT_OFFSET() were replaced by
and RZG2L_GPIO_PORT_GET_INDEX()?
RZG2L_PIN_CFG_TO_PORT_OFFSET(), RZG2L_SINGLE_PIN_GET_CFGS() and
RZG2L_GPIO_PORT_GET_CFGS() were replaced by RZG2L_PIN_CFG_TO_CAPS().

Also rzg2l_pinctrl_set_pfc_mode() don't need port argument anymore.
Also rzg2l_gpio_direction_input() and rzg2l_gpio_direction_output() don't
need to translate port and bit locally as this can be done by
rzg2l_gpio_set_direction().

To use the same naming for port, bit/pin and register offset the
port_offset variable names in different places was replaced by variable
named off and there is no need to initialize anymore cfg and bit in
different code places.

Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
This looks like a nice cleanup, thanks a lot!
Prabhakar: do you like it, too?

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
You can find a few suggestions for improvement below...
quoted hunk ↗ jump to hunk
--- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
+++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
@@ -202,9 +202,11 @@ static int rzg2l_pinctrl_set_mux(struct pinctrl_dev *pctldev,
                                 unsigned int group_selector)
 {
        struct rzg2l_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
+       const struct pinctrl_pin_desc *pin_desc;
+       unsigned int i, *psel_val, *pin_data;
        struct function_desc *func;
-       unsigned int i, *psel_val;
        struct group_desc *group;
+       u32 port, pin, off;
Please move the new variable declarations inside the for(), and
combine them with their initialization.
quoted hunk ↗ jump to hunk
        int *pins;

        func = pinmux_generic_get_function(pctldev, func_selector);
@@ -218,11 +220,17 @@ static int rzg2l_pinctrl_set_mux(struct pinctrl_dev *pctldev,
        pins = group->pins;

        for (i = 0; i < group->num_pins; i++) {
-               dev_dbg(pctrl->dev, "port:%u pin: %u PSEL:%u\n",
-                       RZG2L_PIN_ID_TO_PORT(pins[i]), RZG2L_PIN_ID_TO_PIN(pins[i]),
-                       psel_val[i]);
-               rzg2l_pinctrl_set_pfc_mode(pctrl, RZG2L_PIN_ID_TO_PORT(pins[i]),
-                                          RZG2L_PIN_ID_TO_PIN(pins[i]), psel_val[i]);
+               pin_desc = &pctrl->desc.pins[pins[i]];
+               pin_data = pin_desc->drv_data;
+
+               port = RZG2L_PIN_ID_TO_PORT(pins[i]);
As port is unused but in the debug print, please drop the variable,
and use RZG2L_PIN_ID_TO_PORT() in the debug print below.
+               pin = RZG2L_PIN_ID_TO_PIN(pins[i]);
+               off = RZG2L_PIN_CFG_TO_PORT_OFFSET(*pin_data);
+
+               dev_dbg(pctrl->dev, "port:%u pin: %u off:%x PSEL:%u\n", port,
+                       pin, off, psel_val[i]);
+
+               rzg2l_pinctrl_set_pfc_mode(pctrl, pin, off, psel_val[i]);
        }
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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help