lkml.org 
[lkml]   [2021]   [Nov]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v2 3/5] pinctrl: renesas: pinctrl-rzg2l: Add support to get/set pin config for GPIO port pins
    Hi Geert,

    On Tue, Nov 9, 2021 at 3:00 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
    >
    > Hi Prabhakar,
    >
    > On Tue, Nov 9, 2021 at 3:31 PM Lad, Prabhakar
    > <prabhakar.csengg@gmail.com> wrote:
    > > On Mon, Nov 8, 2021 at 3:36 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
    > > > On Fri, Oct 29, 2021 at 2:44 PM Lad Prabhakar
    > > > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
    > > > > Add support to get/set pin config for GPIO port pins.
    > > > >
    > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
    > > > > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
    > > >
    > > > Thanks for your patch!
    > > >
    > > > > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
    > > > > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
    > > >
    > > > > @@ -495,6 +512,14 @@ static int rzg2l_pinctrl_pinconf_get(struct pinctrl_dev *pctldev,
    > > > > port = RZG2L_SINGLE_PIN_GET_PORT(*pin_data);
    > > > > cfg = RZG2L_SINGLE_PIN_GET_CFGS(*pin_data);
    > > > > bit = RZG2L_SINGLE_PIN_GET_BIT(*pin_data);
    > > > > + } else {
    > > > > + cfg = RZG2L_GPIO_PORT_GET_CFGS(*pin_data);
    > > > > + port = RZG2L_PIN_ID_TO_PORT(_pin);
    > > > > + bit = RZG2L_PIN_ID_TO_PIN(_pin);
    > > > > + port_pin = true;
    > > >
    > > > Instead of setting this flag, perhaps port should be adjusted?
    > >
    > > Something like below?
    > >
    > > #define RZG2L_PORT_START_OFFSET 0x10
    > >
    > > port = RZG2L_PIN_ID_TO_PORT_pin) + RZG2L_PORT_START_OFFSET;
    > > rzg2l_validate_gpio_pin(pctrl, *pin_data, port - RZG2L_PORT_START_OFFSET, bit)
    >
    > Or adjust port after the call to rzg2l_validate_gpio_pin(), to avoid adding
    > the offset first, and subtracting it again for calling the latter?
    >
    > > and rename port -> port_offset in rzg2l_pinctrl_pinconf_get/set
    >
    > That makes sense. Currently "port" has two meanings: it can mean
    > either the GPIO port index, or the global register index covering both
    > single function pin groups and GPIO port indices.
    > RZG2L_SINGLE_PIN_GET_PORT() returns the latter.
    > RZG2L_PIN_ID_TO_PORT() returns the former, thus needing an extra offset
    > to convert to the global register index.
    >
    for symmetry will rename the below:
    RZG2L_SINGLE_PIN_GET_PORT -> RZG2L_SINGLE_PIN_GET_PORT_OFFSET

    Introduce a new macros:
    #define RZG2L_PORT_START_OFFSET 0x10
    #define RZG2L_PIN_ID_TO_PORT_OFFSET(id) (((id) / RZG2L_PINS_PER_PORT)
    + RZG2L_PORT_START_OFFSET)

    And use the above two in rzg2l_pinctrl_pinconf_get/set along with
    renaming port -> port_offset

    And for rzg2l_validate_gpio_pin() will use below instead:
    rzg2l_validate_gpio_pin(pctrl, *pin_data, RZG2L_PIN_ID_TO_PORT(_pin), bit);

    > > Or
    > > would you prefer to change the RZG2L_PIN_ID_TO_PORT macro and adjust
    > > the entire file?
    >
    > Changing RZG2L_PIN_ID_TO_PORT() would imply changing all macros
    > accessing GPIO registers, and is thus quite intrusive.
    >
    Agreed, I will drop this option.

    Cheers,
    Prabhakar
    > > > Then rzg2l_r{ead,mw}_pin_config() don't have to care about that
    > > > anymore.
    > > >
    > > Agreed.
    >
    > 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

    \
     
     \ /
      Last update: 2021-11-09 16:26    [W:3.188 / U:0.536 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site