linux-kernel - Re: [PATCH 1/3] gpio: regmap: Support registers with more than one bit per GPIO

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4c9092d20a35ef3fd6a1723e07adad79@walle.cc>
Date:   Mon, 04 Jul 2022 14:28:21 +0200
From:   Michael Walle <michael@...le.cc>
To:     Aidan MacDonald <aidanmacdonald.0x0@...il.com>
Cc:     linus.walleij@...aro.org, brgl@...ev.pl,
        linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org,
        Andy Shevchenko <andy.shevchenko@...il.com>
Subject: Re: [PATCH 1/3] gpio: regmap: Support registers with more than one
 bit per GPIO

Am 2022-07-03 13:10, schrieb Aidan MacDonald:
> Some devices use a multi-bit register field to change the GPIO
> input/output direction. Add the ->reg_field_xlate() callback to
> support such devices in gpio-regmap.
> 
> ->reg_field_xlate() builds on ->reg_mask_xlate() by allowing the
> driver to return a mask and values to describe a register field.
> gpio-regmap will use the mask to isolate the field and compare or
> update it using the values to implement GPIO level and direction
> get and set ops.

Thanks for working on this. Here are my thoughts on how to improve
it:
  - I'm wary on the value translation of the set and get, you
    don't need that at the moment, correct? I'd concentrate on
    the direction for now.
  - I'd add a xlate_direction(), see below for an example
  - because we can then handle the value too, we don't need the
    invert handling in the {set,get}_direction. drop it there
    and handle it in a simple_xlat. In gpio_regmap,
    store "reg_dir_base" and "invert_direction", derived from
    config->reg_dir_in_base and config->reg_dir_out_base.

static int gpio_regmap_simple_xlat_direction(struct gpio_regmap *gpio
                                              unsigend int base,
                                              unsigned int offset,
                                              unsigned int *dir_out,
                                              unsigned int *dir_in)
{
     unsigned int line = offset % gpio->ngpio_per_reg;
     unsigned int mask = BIT(line);

     if (!gpio->invert_direction) {
         *dir_out = mask;
         *dir_in = 0;
     } else {
         *dir_out = 0;
         *dir_in = mask;
     }

     return 0;
}

And in the {set,get}_direction() you can then check both
values and convert it from or to GPIO_LINE_DIRECTION_{OUT,IN}.

Thoughts?

-michael