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

[Marc Zyngier]

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...