linux-kernel - Re: [PATCH v2 02/17] regmap-irq: Add get_irq_reg to support unusual register layouts

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: <CANhJrGMqUmnSvyNRgRyp40YnGQkD3N_2AZLn94NDp+4RG0_x5w@mail.gmail.com>
Date:   Thu, 23 Jun 2022 11:54:29 +0300
From:   Matti Vaittinen <mazziesaccount@...il.com>
To:     Aidan MacDonald <aidanmacdonald.0x0@...il.com>
Cc:     Mark Brown <broonie@...nel.org>,
        Linus Walleij <linus.walleij@...aro.org>,
        Bartosz Golaszewski <brgl@...ev.pl>,
        Rob Herring <robh+dt@...nel.org>,
        krzysztof.kozlowski+dt@...aro.org, Chen-Yu Tsai <wens@...e.org>,
        jic23@...nel.org, Lee Jones <lee.jones@...aro.org>,
        Sebastian Reichel <sre@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Liam Girdwood <lgirdwood@...il.com>, lars@...afoo.de,
        "Rafael J . Wysocki" <rafael@...nel.org>, quic_gurus@...cinc.com,
        "open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
        devicetree <devicetree@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-iio@...r.kernel.org, Linux PM list <linux-pm@...r.kernel.org>
Subject: Re: [PATCH v2 02/17] regmap-irq: Add get_irq_reg to support unusual
 register layouts

Hi dee Ho peeps!

Sorry for the late reply.

pe 10. kesäk. 2022 klo 18.43 Aidan MacDonald
(aidanmacdonald.0x0@...il.com) kirjoitti:
>
> Mark Brown <broonie@...nel.org> writes:
>
> > On Tue, Jun 07, 2022 at 04:53:09PM +0100, Aidan MacDonald wrote:
> >
> >> -    if (!chip->sub_reg_offsets || !chip->not_fixed_stride) {
> >> +    if (chip->get_irq_reg) {
> >> +            reg = chip->get_irq_reg(base_reg, i);
> >> +    } else if (!chip->sub_reg_offsets || !chip->not_fixed_stride) {
> >
> > It seems like it would be cleaner and clearer to refactor things so that
> > we always have a get_irq_reg() with standard chips getting given a
> > default implementation which implements the current behaviour.
>
> I don't think that is a good way to clean things up. I only intended
> get_irq_reg() to be a quick hack to solve a problem; in my opinion it
> would be a poor abstraction to base the API around.
>
> What I'd suggest is something that will simplify regmap-irq. Instead of
> defining the base registers, etc. in the chip, introduce a new struct
> to describe a register group:
>
>     struct regmap_irq_reg_group {
>         unsigned int status_base;
>         unsigned int mask_base;
>         ...
>
>         unsigned int irq_reg_stride;
>
>         int num_regs;
>     };
>
> The idea is that the registers in a group are linearly mapped using the
> formula "base + (i * irq_reg_stride)". Then it's possible to allow for
> multiple register groups in regmap_irq_chip:
>
>     struct regmap_irq_chip {
>         const struct regmap_irq_reg_group *groups;
>         unsigned int num_groups;
>
>         unsigned int main_status_base;
>         unsigned int num_main_status_bits;
>         int num_main_regs;
>
>         ...
>     };
>
> It should be straightforward to fit existing chips into this model.
>
> - Chips that use a main status + sub-block IRQ layout will define
>   one register group for each sub-block and continue to describe the
>   location of the main status registers inside of regmap_irq_chip.
>   A group will only get polled if the corresponding main status bit
>   is set -- n'th group is polled if n'th bit is set.

Does this work for devices where a single main status bit can flag
IRQs in more than one sub-registers?

Best Regards
 -- Matti

-- 

Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~

Discuss - Estimate - Plan - Report and finally accomplish this:
void do_work(int time) __attribute__ ((const));