linux-kernel - Re: [PATCH v3 2/2] Add GPIO support for Realtek DHC(Digital Home Center) RTD SoCs.

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]

Date:   Wed, 13 Dec 2023 15:40:00 +0200
From:   Andy Shevchenko <andy@...nel.org>
To:     TY_Chang[張子逸] <tychang@...ltek.com>
Cc:     Linus Walleij <linus.walleij@...aro.org>,
        Bartosz Golaszewski <brgl@...ev.pl>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        "linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 2/2] Add GPIO support for Realtek DHC(Digital Home
 Center) RTD SoCs.

On Tue, Dec 12, 2023 at 09:55:59AM +0000, TY_Chang[張子逸] wrote:
> >On Thu, Dec 07, 2023 at 06:07:23PM +0800, TY Chang wrote:

...

> >> This driver enables configuration of GPIO direction, GPIO values, GPIO
> >> debounce settings and handles GPIO interrupts.
> >
> >Why gpio-regmap can't be used?
> 
> I will try to use gpio-remap in the next version.

If it appears that it makes code uglier / complicated, please add the note
somewhere to answer the above question.

...

> >> +     if (index > data->info->num_dir)
> >> +             return -EINVAL;
> >
> >When this conditional can be true?
> >Same Q to the similar checks over the code.
> 
> It is only to check if the offset value is missing in the rtd_gpio_info.
> I'm uncertain about the necessity of these checks. If they are not necessary,
> I will remove the num_* members in the rtd_gpio_info structure along with
> these checks.

My understanding that these checks are equivalent to the

	if (offset >= ngpio)

one, which is performed by GPIO library, i.o.w. you will never get an offset
outside the range of supported GPIO lines.

If my understanding is wrong, these checks need a comment why.

...

> >> +     if (irq == data->irqs[0])
> >> +             get_reg_offset = &rtd_gpio_gpa_offset;
> >> +     else if (irq == data->irqs[1])
> >> +             get_reg_offset = &rtd_gpio_gpda_offset;
> >
> >Can't it be done before entering into chained IRQ handler?
> 
> I will revise it.

Thinking about this more, perhaps you can register two IRQ chips with
different functions, so this won't be part of the very critical interrupt
handler (as we all want to reduce overhead in it as much as possible).
Anyway, think about this and try different options, choose the one you
think the best.

-- 
With Best Regards,
Andy Shevchenko