linux-kernel - Re: [PATCH v2 1/2] pinctrl: Add driver for Awinic AW9523/B I2C GPIO Expander

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] [day] [month] [year] [list]

Date:   Sat, 23 Jan 2021 00:39:13 +0100
From:   AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...ainline.org>
To:     Linus Walleij <linus.walleij@...aro.org>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        konrad.dybcio@...ainline.org, marijn.suijten@...ainline.org,
        martin.botka@...ainline.org, phone-devel@...r.kernel.org,
        "open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>, Rob Herring <robh+dt@...nel.org>
Subject: Re: [PATCH v2 1/2] pinctrl: Add driver for Awinic AW9523/B I2C GPIO
 Expander

Il 22/01/21 10:59, Linus Walleij ha scritto:
> On Mon, Jan 18, 2021 at 3:38 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@...ainline.org> wrote:
> 
>> By the way, this is really LEVEL irq, not EDGE... To avoid any
>> misunderstanding, I think that the best way to show you what I
>> am seeing is to just copy-paste the relevant piece from the
>> datasheet for this hardware (it's not a confidential datasheet
>> and freely found on the internet).
>>
>> Check this out:
>> " External MCU is required acknowledge by INTN pin. INTN is open-drain
>> out-
>> put, low-level active, and need external pull-up resistor.
> 
> This talks about what polarity (active low) the pin from the expander
> to the SoC/CPU is. It has nothing to do with the line into the
> expander.
> 
>> When AW9523B detect port change, any input state from high-level to
>> low-level or from
>>    low-level to high-level will generate interrupt after
>> 8us internal deglitch. "
>>
>> ...but since the datasheet is sometimes unclear about "things" (I am
>> mostly sure that they have translated it to english from chinese), I
>> have actually checked whether the INTN pin was pushed LOW when one of
>> the inputs goes from HIGH to LOW.. and.. it does... and as you imagine
>> yeah.. it's slow.. and yes, as slow as you can imagine. :)
>>
>> So, in short, this chip is raising an interrupt when any input changes
>> state, regardless of the change being LOW->HIGH or HIGH->LOW.
> 
> This means that the expander only supports
> IRQ_TYPE_EDGE_BOTH and nothing else.
> 
> "port change" above means edges.
> 
> Augment your driver to only accept this type.
> 
> The consumers better request IRQ_TYPE_EDGE_BOTH
> (from a device tree for example) and consumers better
> handle the fact that they get interrupts on both rising
> and falling edge as well, else they may need special
> code to handle it. This is not a very nice feature of
> the expander, it would be more helpful to users to
> get interrupts on only rising or only falling edges, but
> as written, it will generate interrupts on both transitions.
> 
> Yours,
> Linus Walleij
> 

I see the reading mistake now... oh wow, that was... sad, from me.
I will fix this ASAP and will send back a v3.

Thank you!

- Angelo