RE: [PATCH net-next 1/3] dt-bindings: net: fsl, fec: add "fsl, wakeup-irq" property

[Joakim Zhang]

Hi Andrew,

> -----Original Message-----
> From: Andrew Lunn <andrew@xxxxxxx>
> Sent: 2021年8月10日 22:33
> To: Joakim Zhang <qiangqing.zhang@xxxxxxx>
> Cc: Florian Fainelli <f.fainelli@xxxxxxxxx>; davem@xxxxxxxxxxxxx;
> kuba@xxxxxxxxxx; robh+dt@xxxxxxxxxx; shawnguo@xxxxxxxxxx;
> s.hauer@xxxxxxxxxxxxxx; festevam@xxxxxxxxx; kernel@xxxxxxxxxxxxxx;
> dl-linux-imx <linux-imx@xxxxxxx>; netdev@xxxxxxxxxxxxxxx;
> devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH net-next 1/3] dt-bindings: net: fsl, fec: add "fsl,
> wakeup-irq" property
> 
> > > > 1) FEC controller has up to 4 interrupt lines and all of these are
> > > > routed to GIC
> > > interrupt controller.
> > > > 2) FEC has a wakeup interrupt signal and always are mixed with
> > > > other
> > > interrupt signals, and then output to one interrupt line.
> > > > 3) For legacy SoCs, wakeup interrupt are mixed to int0 line, but
> > > > for i.MX8M
> > > serials, are mixed to int2 line.
> 
> So you need to know which of the interrupts listed is the wake up interrupt.
> 
> I can see a few ways to do this:
> 
> The FEC driver already has quirks. Add a quirk to fec_imx8mq_info and
> fec_imx8qm_info to indicate these should use int2.
> 
> or
> 
> Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
> 
>   b) two cells
>   ------------
>   The #interrupt-cells property is set to 2 and the first cell defines the
>   index of the interrupt within the controller, while the second cell is used
>   to specify any of the following flags:
>     - bits[3:0] trigger type and level flags
>         1 = low-to-high edge triggered
>         2 = high-to-low edge triggered
>         4 = active high level-sensitive
>         8 = active low level-sensitive
> 
> You could add
> 
>        18 = wakeup source
> 
> and extend to core to either do all the work for you, or tell you this interrupt is
> flagged as being a wakeup source. This solution has the advantage of it should
> be usable in other drivers.

Thanks a lot for your comments first!

I just look into the irq code, if we extend bit[5] to carry wakeup info ( due to bit[4] is used for IRQ_TYPE_PROBE), 
then configure it in the TYPE field of 'interrupts' property, so that interrupt controller would know which interrupt
is wakeup capable. 
I think there is no much work core would do, may just set this interrupt wakup capable. Another functionality is
driver side get this info to identify which mixed interrupt has wakeup capability, we can export symbol from kernel/irq/irqdomain.c.

The intention is to let driver know which interrupt is wakeup capable, I would choose to provider this in specific driver,
instead of interrupt controller, it seems to me that others may all choose this solution for wakeup mixed interrupt.

So I would prefer solution 1, it's easier and under-control. I can have a try if you strongly recommend solution 2.

Best Regards,
Joakim Zhang
> 	  Andrew