linux-kernel - Re: [PATCH v5 08/10] irqchip: Add Sunplus SP7021 interrupt controller driver

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: <ce867937861df454823eb703bfd29256@kernel.org>
Date:   Wed, 08 Dec 2021 07:45:11 +0000
From:   Marc Zyngier <maz@...nel.org>
To:     qinjian[覃健] <qinjian@...lus1.com>
Cc:     robh+dt@...nel.org, mturquette@...libre.com, sboyd@...nel.org,
        tglx@...utronix.de, p.zabel@...gutronix.de, linux@...linux.org.uk,
        broonie@...nel.org, arnd@...db.de,
        linux-arm-kernel@...ts.infradead.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-clk@...r.kernel.org,
        Wells Lu 呂芳騰 
        <wells.lu@...plus.com>
Subject: Re: [PATCH v5 08/10] irqchip: Add Sunplus SP7021 interrupt controller
 driver

On 2021-12-08 07:15, qinjian[覃健] wrote:
>> > +void sp_intc_set_ext(u32 hwirq, int ext_num)
>> > +{
>> > +	sp_intc_assign_bit(hwirq, REG_INTR_PRIORITY, !ext_num);
>> > +}
>> > +EXPORT_SYMBOL_GPL(sp_intc_set_ext);
>> 
>> No way. We don't export random symbols without a good justification,
>> and you didn't give any.
>> 
> 
> This function called by SP7021 display driver to decide DISPLAY_IRQ
> routing to which parent irq (EXT_INT0 or EXT_INT1).

Based on what? How can a display driver decide which parent is
appropriate? What improvement does this bring?

> In previous patches, which defined in DT, parsed & processed @
> sp_intc_xlate_of()
> From your comment, this is a SW decision. So I removed it from DT &
> intc driver, only
> export this function to access the related intc reg.

If a decision has to be made, it has to be done in the interrupt
controller driver, or rely on the standard API. Exporting random
low level functions to other random drivers is not acceptable.

         M.
-- 
Jazz is not dead. It just smells funny...